Re: [Intel-gfx] [PATCH] drm/i915/dg2: Implement WM0 cursor WA for DG2

["Lisovskiy, Stanislav" <stanislav.lisovskiy@intel.com>]

On Wed, Nov 24, 2021 at 12:23:08PM +0200, Jani Nikula wrote:
> On Thu, 18 Nov 2021, Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> wrote:
> > Bug in the register unit which results in WM1 register
> > used when only WM0 is enabled on cursor.
> > Software workaround is when only WM0 enabled on cursor,
> > copy contents of CUR_WM_0[30:0] (exclude the enable bit)
> > into CUR_WM_1[30:0].
> >
> > v2:  - s/dev_priv/i915/ (Ville Syrjälä)
> >      - Removed unneeded brackets (Ville Syrjälä)
> >
> > HSDES: 14012656716
> >
> > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_pm.c | 18 +++++++++++++-----
> >  1 file changed, 13 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index 89dc7f69baf3..01fa3fac1b57 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -5095,6 +5095,18 @@ skl_check_nv12_wm_level(struct skl_wm_level *wm, struct skl_wm_level *uv_wm,
> >  	}
> >  }
> >  
> > +static bool icl_need_wm1_wa(struct drm_i915_private *i915,
> > +			    enum plane_id plane_id)
> 
> Commit subject having "WM0 cursor WA" and the function saying "wm1_wa"
> looks like a discrepancy to the casual reader.

Agree, not the best naming. Should be either WM0 or WM1.
Thing is that due to nature of that W/A both can be correct.
Patch has been pushed week ago, after r-b from Ville.

I will send a fix anyway.

Stan

> 
> BR,
> Jani.
> 
> > +{
> > +	/*
> > +	 * Wa_1408961008:icl, ehl
> > +	 * Wa_14012656716:tgl, adl
> > +	 * Underruns with WM1+ disabled
> > +	 */
> > +	return DISPLAY_VER(i915) == 11 ||
> > +	       (IS_DISPLAY_VER(i915, 12, 13) && plane_id == PLANE_CURSOR);
> > +}
> > +
> >  static int
> >  skl_allocate_plane_ddb(struct intel_atomic_state *state,
> >  		       struct intel_crtc *crtc)
> > @@ -5265,11 +5277,7 @@ skl_allocate_plane_ddb(struct intel_atomic_state *state,
> >  			skl_check_nv12_wm_level(&wm->wm[level], &wm->uv_wm[level],
> >  						total[plane_id], uv_total[plane_id]);
> >  
> > -			/*
> > -			 * Wa_1408961008:icl, ehl
> > -			 * Underruns with WM1+ disabled
> > -			 */
> > -			if (DISPLAY_VER(dev_priv) == 11 &&
> > +			if (icl_need_wm1_wa(dev_priv, plane_id) &&
> >  			    level == 1 && wm->wm[0].enable) {
> >  				wm->wm[level].blocks = wm->wm[0].blocks;
> >  				wm->wm[level].lines = wm->wm[0].lines;
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center