| Message ID | 20211118093907.18510-1-stanislav.lisovskiy@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/dg2: Implement WM0 cursor WA for DG2 | expand |
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. 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;
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
On Wed, 24 Nov 2021, "Lisovskiy, Stanislav" <stanislav.lisovskiy@intel.com> wrote: > 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: >> > +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. *shrug* Not sure it matters, just an observation going through mails. BR, Jani.
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) +{ + /* + * 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;
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(-)