| Message ID | 20210611143409.827727-1-tejaskumarx.surendrakumar.upadhyay@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/gen11: use ffs for minconfig slice/subslice | expand |
On Fri, Jun 11, 2021 at 08:04:09PM +0530, Tejas Upadhyay wrote: > w/a on gen11 platforms not working as expected and causing > more harm on the RC6 flow. Because of subslice steering > disturbance w/a read is failing. By using ffs we can default > steering of slice/sublice to minconfig hence w/a will pass > and any warns will go away. > > Fixes: fb899dd8ea9c ("drm/i915: Apply Wa_1406680159:icl,ehl as an engine workaround") > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> > Cc: Matt Roper <matthew.d.roper@intel.com> > Signed-off-by: Tejas Upadhyay <tejaskumarx.surendrakumar.upadhyay@intel.com> > --- > drivers/gpu/drm/i915/gt/intel_workarounds.c | 14 +++++++++++--- > drivers/gpu/drm/i915/intel_pm.c | 10 +++++++--- > 2 files changed, 18 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c > index b62d1e31a645..68b14141088a 100644 > --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c > +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c > @@ -991,13 +991,21 @@ wa_init_mcr(struct drm_i915_private *i915, struct i915_wa_list *wal) > l3_en = ~0; > } > > - slice = fls(sseu->slice_mask) - 1; > - subslice = fls(l3_en & intel_sseu_get_subslices(sseu, slice)); > + if (GRAPHICS_VER(i915) == 11) { > + slice = ffs(sseu->slice_mask) - 1; > + subslice = ffs(l3_en & intel_sseu_get_subslices(sseu, slice)); > + } else { > + slice = fls(sseu->slice_mask) - 1; > + subslice = fls(l3_en & intel_sseu_get_subslices(sseu, slice)); > + } > if (!subslice) { > drm_warn(&i915->drm, > "No common index found between subslice mask %x and L3 bank mask %x!\n", > intel_sseu_get_subslices(sseu, slice), l3_en); > - subslice = fls(l3_en); > + if (GRAPHICS_VER(i915) == 11) > + subslice = ffs(l3_en); > + else > + subslice = fls(l3_en); > drm_WARN_ON(&i915->drm, !subslice); > } > subslice--; > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 45fefa0ed160..d1352ec3546a 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -4049,9 +4049,13 @@ skl_ddb_entry_for_slices(struct drm_i915_private *dev_priv, u8 slice_mask, > ddb->end = 0; > return; > } > - > - ddb->start = (ffs(slice_mask) - 1) * slice_size; > - ddb->end = fls(slice_mask) * slice_size; > + if (GRAPHICS_VER(dev_priv) == 11) { > + ddb->start = (fls(slice_mask) - 1) * slice_size; > + ddb->end = ffs(slice_mask) * slice_size; > + } else { > + ddb->start = (ffs(slice_mask) - 1) * slice_size; > + ddb->end = fls(slice_mask) * slice_size; > + } This code has nothing to do with GT slices. > > WARN_ON(ddb->start >= ddb->end); > WARN_ON(ddb->end > INTEL_INFO(dev_priv)->dbuf.size); > -- > 2.31.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> -----Original Message----- > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > Sent: 11 June 2021 23:36 > To: Surendrakumar Upadhyay, TejaskumarX > <tejaskumarx.surendrakumar.upadhyay@intel.com> > Cc: intel-gfx@lists.freedesktop.org > Subject: Re: [Intel-gfx] [PATCH] drm/i915/gen11: use ffs for minconfig > slice/subslice > > On Fri, Jun 11, 2021 at 08:04:09PM +0530, Tejas Upadhyay wrote: > > w/a on gen11 platforms not working as expected and causing more harm > > on the RC6 flow. Because of subslice steering disturbance w/a read is > > failing. By using ffs we can default steering of slice/sublice to > > minconfig hence w/a will pass and any warns will go away. > > > > Fixes: fb899dd8ea9c ("drm/i915: Apply Wa_1406680159:icl,ehl as an > > engine workaround") > > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> > > Cc: Matt Roper <matthew.d.roper@intel.com> > > Signed-off-by: Tejas Upadhyay > > <tejaskumarx.surendrakumar.upadhyay@intel.com> > > --- > > drivers/gpu/drm/i915/gt/intel_workarounds.c | 14 +++++++++++--- > > drivers/gpu/drm/i915/intel_pm.c | 10 +++++++--- > > 2 files changed, 18 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c > > b/drivers/gpu/drm/i915/gt/intel_workarounds.c > > index b62d1e31a645..68b14141088a 100644 > > --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c > > +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c > > @@ -991,13 +991,21 @@ wa_init_mcr(struct drm_i915_private *i915, > struct i915_wa_list *wal) > > l3_en = ~0; > > } > > > > - slice = fls(sseu->slice_mask) - 1; > > - subslice = fls(l3_en & intel_sseu_get_subslices(sseu, slice)); > > + if (GRAPHICS_VER(i915) == 11) { > > + slice = ffs(sseu->slice_mask) - 1; > > + subslice = ffs(l3_en & intel_sseu_get_subslices(sseu, slice)); > > + } else { > > + slice = fls(sseu->slice_mask) - 1; > > + subslice = fls(l3_en & intel_sseu_get_subslices(sseu, slice)); > > + } > > if (!subslice) { > > drm_warn(&i915->drm, > > "No common index found between subslice mask %x > and L3 bank mask %x!\n", > > intel_sseu_get_subslices(sseu, slice), l3_en); > > - subslice = fls(l3_en); > > + if (GRAPHICS_VER(i915) == 11) > > + subslice = ffs(l3_en); > > + else > > + subslice = fls(l3_en); > > drm_WARN_ON(&i915->drm, !subslice); > > } > > subslice--; > > diff --git a/drivers/gpu/drm/i915/intel_pm.c > > b/drivers/gpu/drm/i915/intel_pm.c index 45fefa0ed160..d1352ec3546a > > 100644 > > --- a/drivers/gpu/drm/i915/intel_pm.c > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > @@ -4049,9 +4049,13 @@ skl_ddb_entry_for_slices(struct > drm_i915_private *dev_priv, u8 slice_mask, > > ddb->end = 0; > > return; > > } > > - > > - ddb->start = (ffs(slice_mask) - 1) * slice_size; > > - ddb->end = fls(slice_mask) * slice_size; > > + if (GRAPHICS_VER(dev_priv) == 11) { > > + ddb->start = (fls(slice_mask) - 1) * slice_size; > > + ddb->end = ffs(slice_mask) * slice_size; > > + } else { > > + ddb->start = (ffs(slice_mask) - 1) * slice_size; > > + ddb->end = fls(slice_mask) * slice_size; > > + } > > This code has nothing to do with GT slices. Without this change we are observing "gem_exec_suspend (basic-s0) Starting subtest: basic-S0" test hangs and crash eventually. Thus change identified and added. Would you please help reviewing? Also I am seeing ICL igt@kms_frontbuffer_tracking@fbc-suspend is seeing workaround(0x9524) lost warning after this patch while EHL and JSL are working fine. does someone has insight why that should be the case? Thanks, Tejas > > > > > WARN_ON(ddb->start >= ddb->end); > > WARN_ON(ddb->end > INTEL_INFO(dev_priv)->dbuf.size); > > -- > > 2.31.1 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Ville Syrjälä > Intel
On Sat, Jun 12, 2021 at 09:55:02AM +0000, Surendrakumar Upadhyay, TejaskumarX wrote: > > > > -----Original Message----- > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Sent: 11 June 2021 23:36 > > To: Surendrakumar Upadhyay, TejaskumarX > > <tejaskumarx.surendrakumar.upadhyay@intel.com> > > Cc: intel-gfx@lists.freedesktop.org > > Subject: Re: [Intel-gfx] [PATCH] drm/i915/gen11: use ffs for minconfig > > slice/subslice > > > > On Fri, Jun 11, 2021 at 08:04:09PM +0530, Tejas Upadhyay wrote: > > > w/a on gen11 platforms not working as expected and causing more harm > > > on the RC6 flow. Because of subslice steering disturbance w/a read is > > > failing. By using ffs we can default steering of slice/sublice to > > > minconfig hence w/a will pass and any warns will go away. > > > > > > Fixes: fb899dd8ea9c ("drm/i915: Apply Wa_1406680159:icl,ehl as an > > > engine workaround") > > > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> > > > Cc: Matt Roper <matthew.d.roper@intel.com> > > > Signed-off-by: Tejas Upadhyay > > > <tejaskumarx.surendrakumar.upadhyay@intel.com> > > > --- > > > drivers/gpu/drm/i915/gt/intel_workarounds.c | 14 +++++++++++--- > > > drivers/gpu/drm/i915/intel_pm.c | 10 +++++++--- > > > 2 files changed, 18 insertions(+), 6 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c > > > b/drivers/gpu/drm/i915/gt/intel_workarounds.c > > > index b62d1e31a645..68b14141088a 100644 > > > --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c > > > +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c > > > @@ -991,13 +991,21 @@ wa_init_mcr(struct drm_i915_private *i915, > > struct i915_wa_list *wal) > > > l3_en = ~0; > > > } > > > > > > - slice = fls(sseu->slice_mask) - 1; > > > - subslice = fls(l3_en & intel_sseu_get_subslices(sseu, slice)); > > > + if (GRAPHICS_VER(i915) == 11) { > > > + slice = ffs(sseu->slice_mask) - 1; > > > + subslice = ffs(l3_en & intel_sseu_get_subslices(sseu, slice)); > > > + } else { > > > + slice = fls(sseu->slice_mask) - 1; > > > + subslice = fls(l3_en & intel_sseu_get_subslices(sseu, slice)); > > > + } > > > if (!subslice) { > > > drm_warn(&i915->drm, > > > "No common index found between subslice mask %x > > and L3 bank mask %x!\n", > > > intel_sseu_get_subslices(sseu, slice), l3_en); > > > - subslice = fls(l3_en); > > > + if (GRAPHICS_VER(i915) == 11) > > > + subslice = ffs(l3_en); > > > + else > > > + subslice = fls(l3_en); > > > drm_WARN_ON(&i915->drm, !subslice); > > > } > > > subslice--; > > > diff --git a/drivers/gpu/drm/i915/intel_pm.c > > > b/drivers/gpu/drm/i915/intel_pm.c index 45fefa0ed160..d1352ec3546a > > > 100644 > > > --- a/drivers/gpu/drm/i915/intel_pm.c > > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > > @@ -4049,9 +4049,13 @@ skl_ddb_entry_for_slices(struct > > drm_i915_private *dev_priv, u8 slice_mask, > > > ddb->end = 0; > > > return; > > > } > > > - > > > - ddb->start = (ffs(slice_mask) - 1) * slice_size; > > > - ddb->end = fls(slice_mask) * slice_size; > > > + if (GRAPHICS_VER(dev_priv) == 11) { > > > + ddb->start = (fls(slice_mask) - 1) * slice_size; > > > + ddb->end = ffs(slice_mask) * slice_size; > > > + } else { > > > + ddb->start = (ffs(slice_mask) - 1) * slice_size; > > > + ddb->end = fls(slice_mask) * slice_size; > > > + } > > > > This code has nothing to do with GT slices. > > Without this change we are observing "gem_exec_suspend (basic-s0) > Starting subtest: basic-S0" test hangs and crash eventually. Thus > change identified and added. Would you please help reviewing? > > Also I am seeing ICL igt@kms_frontbuffer_tracking@fbc-suspend is > seeing workaround(0x9524) lost warning after this patch while EHL and > JSL are working fine. does someone has insight why that should be > the case? See the patch I sent last week: https://patchwork.freedesktop.org/series/91367/ and my response to the CI results here that explains the ICL behavior: https://lists.freedesktop.org/archives/intel-gfx/2021-June/269097.html Basically the fact that we're trying to combine subslice steering and l3bank steering into a single value prevents us from using the minconfig, even after switching to ffs(). ICL has been broken all along too and we just didn't notice because we were getting "lucky" and reading back random garbage that happened to have a '1' in the relevant bit. The next platform that we're getting ready to start upstreaming soon adds a bunch more types of multicast steering, so the right approach is to extract some of the refactoring from that platform and apply it to gen11+ l3bank ranges too. Something along the lines of this series that I sent to trybot: https://patchwork.freedesktop.org/series/91421/ although it looks like I've still got some bugs that need to be hammered out before its ready. Matt > > Thanks, > Tejas > > > > > > > > WARN_ON(ddb->start >= ddb->end); > > > WARN_ON(ddb->end > INTEL_INFO(dev_priv)->dbuf.size); > > > -- > > > 2.31.1 > > > > > > _______________________________________________ > > > Intel-gfx mailing list > > > Intel-gfx@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > -- > > Ville Syrjälä > > Intel > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c index b62d1e31a645..68b14141088a 100644 --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c @@ -991,13 +991,21 @@ wa_init_mcr(struct drm_i915_private *i915, struct i915_wa_list *wal) l3_en = ~0; } - slice = fls(sseu->slice_mask) - 1; - subslice = fls(l3_en & intel_sseu_get_subslices(sseu, slice)); + if (GRAPHICS_VER(i915) == 11) { + slice = ffs(sseu->slice_mask) - 1; + subslice = ffs(l3_en & intel_sseu_get_subslices(sseu, slice)); + } else { + slice = fls(sseu->slice_mask) - 1; + subslice = fls(l3_en & intel_sseu_get_subslices(sseu, slice)); + } if (!subslice) { drm_warn(&i915->drm, "No common index found between subslice mask %x and L3 bank mask %x!\n", intel_sseu_get_subslices(sseu, slice), l3_en); - subslice = fls(l3_en); + if (GRAPHICS_VER(i915) == 11) + subslice = ffs(l3_en); + else + subslice = fls(l3_en); drm_WARN_ON(&i915->drm, !subslice); } subslice--; diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 45fefa0ed160..d1352ec3546a 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -4049,9 +4049,13 @@ skl_ddb_entry_for_slices(struct drm_i915_private *dev_priv, u8 slice_mask, ddb->end = 0; return; } - - ddb->start = (ffs(slice_mask) - 1) * slice_size; - ddb->end = fls(slice_mask) * slice_size; + if (GRAPHICS_VER(dev_priv) == 11) { + ddb->start = (fls(slice_mask) - 1) * slice_size; + ddb->end = ffs(slice_mask) * slice_size; + } else { + ddb->start = (ffs(slice_mask) - 1) * slice_size; + ddb->end = fls(slice_mask) * slice_size; + } WARN_ON(ddb->start >= ddb->end); WARN_ON(ddb->end > INTEL_INFO(dev_priv)->dbuf.size);
w/a on gen11 platforms not working as expected and causing more harm on the RC6 flow. Because of subslice steering disturbance w/a read is failing. By using ffs we can default steering of slice/sublice to minconfig hence w/a will pass and any warns will go away. Fixes: fb899dd8ea9c ("drm/i915: Apply Wa_1406680159:icl,ehl as an engine workaround") Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> Cc: Matt Roper <matthew.d.roper@intel.com> Signed-off-by: Tejas Upadhyay <tejaskumarx.surendrakumar.upadhyay@intel.com> --- drivers/gpu/drm/i915/gt/intel_workarounds.c | 14 +++++++++++--- drivers/gpu/drm/i915/intel_pm.c | 10 +++++++--- 2 files changed, 18 insertions(+), 6 deletions(-)