| Message ID | 20241114043040.1396947-2-suraj.kandpal@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] drm/i915/watermark: Refactor dpkgc code | expand |
On Thu, 2024-11-14 at 10:00 +0530, Suraj Kandpal wrote: > Increase the latency programmed into PKG_C_LATENCY latency to be > a multiple of line time which is written into WM_LINETIME. > > --v2 > -Fix commit subject line [Sai Teja] > -Use individual DISPLAY_VER checks instead of range [Sai Teja] > -Initialize max_linetime [Sai Teja] > > --v3 > -take into account the scenario when adjusted_latency is 0 [Vinod] > > --v4 > -rename adjusted_latency to latency [Mitul] > -fix the condition in which dpkgc is disabled [Vinod] > > --v5 > -Add check to see if max_linetime is 0 [Vinod] > > WA: 22020299601 Is this a normal practice? I didnt find any other than one of your prev patch. I wonder if the bspec number more useful here? Otherwise, looks ok after addressing the comments in prev patch. BR Vinod > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> > --- > drivers/gpu/drm/i915/display/intel_wm.c | 22 +++++++++++++++++----- > 1 file changed, 17 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_wm.c b/drivers/gpu/drm/i915/display/intel_wm.c > index 620873d1244f..ce11a69b36cc 100644 > --- a/drivers/gpu/drm/i915/display/intel_wm.c > +++ b/drivers/gpu/drm/i915/display/intel_wm.c > @@ -157,9 +157,10 @@ intel_program_dpkgc_latency(struct intel_atomic_state *state, > struct intel_display *display = to_intel_display(state); > struct intel_crtc *crtc; > struct intel_crtc_state *new_crtc_state; > - u32 max_latency = LNL_PKG_C_LATENCY_MASK; > + u32 latency = LNL_PKG_C_LATENCY_MASK; > u32 clear = 0, val = 0; > u32 added_waketime = 0; > + u32 max_linetime = 0; > int i; > bool fixed_refresh_rate = false; > > @@ -171,18 +172,29 @@ intel_program_dpkgc_latency(struct intel_atomic_state *state, > new_crtc_state->vrr.vmin == new_crtc_state->vrr.flipline) || > !new_crtc_state->vrr.enable) > fixed_refresh_rate = true; > + > + max_linetime = max(new_crtc_state->linetime, max_linetime); > } > > if (fixed_refresh_rate) { > - max_latency = skl_watermark_max_latency(i915, 1); > - if (max_latency == 0) > - max_latency = LNL_PKG_C_LATENCY_MASK; > + latency = skl_watermark_max_latency(i915, 1); > + > + /* Wa_22020299601 */ > + if (latency) { > + if ((DISPLAY_VER(display) == 20 || DISPLAY_VER(display) == 30) && > + max_linetime) > + latency = max_linetime * > + DIV_ROUND_UP(latency, max_linetime); > + } else { > + latency = LNL_PKG_C_LATENCY_MASK; > + } > + > added_waketime = DSB_EXE_TIME + > display->sagv.block_time_us; > } > > clear |= LNL_ADDED_WAKE_TIME_MASK | LNL_PKG_C_LATENCY_MASK; > - val |= REG_FIELD_PREP(LNL_PKG_C_LATENCY_MASK, max_latency) | > + val |= REG_FIELD_PREP(LNL_PKG_C_LATENCY_MASK, latency) | > REG_FIELD_PREP(LNL_ADDED_WAKE_TIME_MASK, added_waketime); > > intel_de_rmw(display, LNL_PKG_C_LATENCY, clear, val);
> -----Original Message----- > From: Govindapillai, Vinod <vinod.govindapillai@intel.com> > Sent: Thursday, November 14, 2024 7:39 PM > To: Kandpal, Suraj <suraj.kandpal@intel.com>; intel-xe@lists.freedesktop.org; > intel-gfx@lists.freedesktop.org > Cc: Syrjala, Ville <ville.syrjala@intel.com> > Subject: Re: [PATCH 2/2] drm/i915/watermark: Modify latency programmed > into PKG_C_LATENCY > > On Thu, 2024-11-14 at 10:00 +0530, Suraj Kandpal wrote: > > Increase the latency programmed into PKG_C_LATENCY latency to be a > > multiple of line time which is written into WM_LINETIME. > > > > --v2 > > -Fix commit subject line [Sai Teja] > > -Use individual DISPLAY_VER checks instead of range [Sai Teja] > > -Initialize max_linetime [Sai Teja] > > > > --v3 > > -take into account the scenario when adjusted_latency is 0 [Vinod] > > > > --v4 > > -rename adjusted_latency to latency [Mitul] -fix the condition in > > which dpkgc is disabled [Vinod] > > > > --v5 > > -Add check to see if max_linetime is 0 [Vinod] > > > > WA: 22020299601 > Is this a normal practice? I didnt find any other than one of your prev patch. I > wonder if the bspec number more useful here? Yes normally when implementing a Wa we give the HSD number in the above fashion Since it gives a total view of what the problem was and why the Wa is needed and what the Wa is, before that we used to directly write HSD: <hsd no.> but somewhere along the line the Convention was changed to WA: <hsdno.>. So now we either have subject like Add Wa_<wano> Or we add the wa as shown above. Regards, Suraj Kandpal > > Otherwise, looks ok after addressing the comments in prev patch. > > BR > Vinod > > > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_wm.c | 22 +++++++++++++++++----- > > 1 file changed, 17 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_wm.c > > b/drivers/gpu/drm/i915/display/intel_wm.c > > index 620873d1244f..ce11a69b36cc 100644 > > --- a/drivers/gpu/drm/i915/display/intel_wm.c > > +++ b/drivers/gpu/drm/i915/display/intel_wm.c > > @@ -157,9 +157,10 @@ intel_program_dpkgc_latency(struct > > intel_atomic_state *state, > > struct intel_display *display = to_intel_display(state); > > struct intel_crtc *crtc; > > struct intel_crtc_state *new_crtc_state; > > - u32 max_latency = LNL_PKG_C_LATENCY_MASK; > > + u32 latency = LNL_PKG_C_LATENCY_MASK; > > u32 clear = 0, val = 0; > > u32 added_waketime = 0; > > + u32 max_linetime = 0; > > int i; > > bool fixed_refresh_rate = false; > > > > @@ -171,18 +172,29 @@ intel_program_dpkgc_latency(struct > > intel_atomic_state *state, > > new_crtc_state->vrr.vmin == > > new_crtc_state->vrr.flipline) || > > !new_crtc_state->vrr.enable) > > fixed_refresh_rate = true; > > + > > + max_linetime = max(new_crtc_state->linetime, > > +max_linetime); > > } > > > > if (fixed_refresh_rate) { > > - max_latency = skl_watermark_max_latency(i915, 1); > > - if (max_latency == 0) > > - max_latency = LNL_PKG_C_LATENCY_MASK; > > + latency = skl_watermark_max_latency(i915, 1); > > + > > + /* Wa_22020299601 */ > > + if (latency) { > > + if ((DISPLAY_VER(display) == 20 || > > +DISPLAY_VER(display) == 30) && > > + max_linetime) > > + latency = max_linetime * > > + DIV_ROUND_UP(latency, > > +max_linetime); > > + } else { > > + latency = LNL_PKG_C_LATENCY_MASK; > > + } > > + > > added_waketime = DSB_EXE_TIME + > > display->sagv.block_time_us; > > } > > > > clear |= LNL_ADDED_WAKE_TIME_MASK | > LNL_PKG_C_LATENCY_MASK; > > - val |= REG_FIELD_PREP(LNL_PKG_C_LATENCY_MASK, max_latency) | > > + val |= REG_FIELD_PREP(LNL_PKG_C_LATENCY_MASK, latency) | > > REG_FIELD_PREP(LNL_ADDED_WAKE_TIME_MASK, > > added_waketime); > > > > intel_de_rmw(display, LNL_PKG_C_LATENCY, clear, val);
diff --git a/drivers/gpu/drm/i915/display/intel_wm.c b/drivers/gpu/drm/i915/display/intel_wm.c index 620873d1244f..ce11a69b36cc 100644 --- a/drivers/gpu/drm/i915/display/intel_wm.c +++ b/drivers/gpu/drm/i915/display/intel_wm.c @@ -157,9 +157,10 @@ intel_program_dpkgc_latency(struct intel_atomic_state *state, struct intel_display *display = to_intel_display(state); struct intel_crtc *crtc; struct intel_crtc_state *new_crtc_state; - u32 max_latency = LNL_PKG_C_LATENCY_MASK; + u32 latency = LNL_PKG_C_LATENCY_MASK; u32 clear = 0, val = 0; u32 added_waketime = 0; + u32 max_linetime = 0; int i; bool fixed_refresh_rate = false; @@ -171,18 +172,29 @@ intel_program_dpkgc_latency(struct intel_atomic_state *state, new_crtc_state->vrr.vmin == new_crtc_state->vrr.flipline) || !new_crtc_state->vrr.enable) fixed_refresh_rate = true; + + max_linetime = max(new_crtc_state->linetime, max_linetime); } if (fixed_refresh_rate) { - max_latency = skl_watermark_max_latency(i915, 1); - if (max_latency == 0) - max_latency = LNL_PKG_C_LATENCY_MASK; + latency = skl_watermark_max_latency(i915, 1); + + /* Wa_22020299601 */ + if (latency) { + if ((DISPLAY_VER(display) == 20 || DISPLAY_VER(display) == 30) && + max_linetime) + latency = max_linetime * + DIV_ROUND_UP(latency, max_linetime); + } else { + latency = LNL_PKG_C_LATENCY_MASK; + } + added_waketime = DSB_EXE_TIME + display->sagv.block_time_us; } clear |= LNL_ADDED_WAKE_TIME_MASK | LNL_PKG_C_LATENCY_MASK; - val |= REG_FIELD_PREP(LNL_PKG_C_LATENCY_MASK, max_latency) | + val |= REG_FIELD_PREP(LNL_PKG_C_LATENCY_MASK, latency) | REG_FIELD_PREP(LNL_ADDED_WAKE_TIME_MASK, added_waketime); intel_de_rmw(display, LNL_PKG_C_LATENCY, clear, val);
Increase the latency programmed into PKG_C_LATENCY latency to be a multiple of line time which is written into WM_LINETIME. --v2 -Fix commit subject line [Sai Teja] -Use individual DISPLAY_VER checks instead of range [Sai Teja] -Initialize max_linetime [Sai Teja] --v3 -take into account the scenario when adjusted_latency is 0 [Vinod] --v4 -rename adjusted_latency to latency [Mitul] -fix the condition in which dpkgc is disabled [Vinod] --v5 -Add check to see if max_linetime is 0 [Vinod] WA: 22020299601 Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> --- drivers/gpu/drm/i915/display/intel_wm.c | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-)