Re: [Intel-gfx] [PATCH v2 3/4] drm/i915: Extract skl_wm_latency()

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Wed, Feb 22, 2023 at 09:25:56AM +0000, Hogander, Jouni wrote:
> On Fri, 2023-01-27 at 19:30 +0200, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Extract the skl+ wm latency determination into a small helper
> > so that everyone has the same idea what the latency should be.
> > 
> > This introduces a slight functional change in that
> > skl_cursor_allocation() will now start to account for the
> > extra 4 usec that the kbk/cfl/cml IPC w/a adds.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/skl_watermark.c | 40 +++++++++++++-----
> > --
> >  1 file changed, 26 insertions(+), 14 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c
> > b/drivers/gpu/drm/i915/display/skl_watermark.c
> > index ae4e9e680c2e..65c746d018b5 100644
> > --- a/drivers/gpu/drm/i915/display/skl_watermark.c
> > +++ b/drivers/gpu/drm/i915/display/skl_watermark.c
> > @@ -704,6 +704,28 @@ static void skl_compute_plane_wm(const struct
> > intel_crtc_state *crtc_state,
> >                                  const struct skl_wm_level
> > *result_prev,
> >                                  struct skl_wm_level *result /* out
> > */);
> >  
> > +static unsigned int skl_wm_latency(struct drm_i915_private *i915,
> > int level,
> > +                                  const struct skl_wm_params *wp)
> > +{
> > +       unsigned int latency = i915->display.wm.skl_latency[level];
> > +
> > +       if (latency == 0)
> > +               return 0;
> 
> What does it mean if latency is 0 here? Why it is ok to skip checks for
> ipc_enabled and memory bandwidth workaround?

Zero latency means this watermark level is disabled.

> > +
> > +       /*
> > +        * WaIncreaseLatencyIPCEnabled: kbl,cfl
> > +        * Display WA #1141: kbl,cfl
> > +        */
> > +       if ((IS_KABYLAKE(i915) || IS_COFFEELAKE(i915) ||
> > IS_COMETLAKE(i915)) &&
> > +           skl_watermark_ipc_enabled(i915))
> > +               latency += 4;
> > +
> > +       if (skl_needs_memory_bw_wa(i915) && wp->x_tiled)
> > +               latency += 15;
> > +
> > +       return latency;
> > +}
> > +
> >  static unsigned int
> >  skl_cursor_allocation(const struct intel_crtc_state *crtc_state,
> >                       int num_active)
> > @@ -723,7 +745,7 @@ skl_cursor_allocation(const struct
> > intel_crtc_state *crtc_state,
> >         drm_WARN_ON(&i915->drm, ret);
> >  
> >         for (level = 0; level <= max_level; level++) {
> > -               unsigned int latency = i915-
> > >display.wm.skl_latency[level];
> > +               unsigned int latency = skl_wm_latency(i915, level,
> > &wp);
> >  
> >                 skl_compute_plane_wm(crtc_state, plane, level,
> > latency, &wp, &wm, &wm);
> >                 if (wm.min_ddb_alloc == U16_MAX)
> > @@ -1834,17 +1856,6 @@ static void skl_compute_plane_wm(const struct
> > intel_crtc_state *crtc_state,
> >                 return;
> >         }
> >  
> > -       /*
> > -        * WaIncreaseLatencyIPCEnabled: kbl,cfl
> > -        * Display WA #1141: kbl,cfl
> > -        */
> > -       if ((IS_KABYLAKE(i915) || IS_COFFEELAKE(i915) ||
> > IS_COMETLAKE(i915)) &&
> > -           skl_watermark_ipc_enabled(i915))
> > -               latency += 4;
> > -
> > -       if (skl_needs_memory_bw_wa(i915) && wp->x_tiled)
> > -               latency += 15;
> > -
> >         method1 = skl_wm_method1(i915, wp->plane_pixel_rate,
> >                                  wp->cpp, latency, wp-
> > >dbuf_block_size);
> >         method2 = skl_wm_method2(wp->plane_pixel_rate,
> > @@ -1971,7 +1982,7 @@ skl_compute_wm_levels(const struct
> > intel_crtc_state *crtc_state,
> >  
> >         for (level = 0; level <= max_level; level++) {
> >                 struct skl_wm_level *result = &levels[level];
> > -               unsigned int latency = i915-
> > >display.wm.skl_latency[level];
> > +               unsigned int latency = skl_wm_latency(i915, level,
> > wm_params);
> >  
> >                 skl_compute_plane_wm(crtc_state, plane, level,
> > latency,
> >                                      wm_params, result_prev, result);
> > @@ -1991,7 +2002,8 @@ static void tgl_compute_sagv_wm(const struct
> > intel_crtc_state *crtc_state,
> >         unsigned int latency = 0;
> >  
> >         if (i915->display.sagv.block_time_us)
> > -               latency = i915->display.sagv.block_time_us + i915-
> > >display.wm.skl_latency[0];
> > +               latency = i915->display.sagv.block_time_us +
> > +                       skl_wm_latency(i915, 0, wm_params);
> >  
> >         skl_compute_plane_wm(crtc_state, plane, 0, latency,
> >                              wm_params, &levels[0],
> 

-- 
Ville Syrjälä
Intel