| Message ID | 20180730120636.26958-1-mika.kuoppala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Revert "drm/i915/icl: WaEnableFloatBlendOptimization" | expand |
Quoting Mika Kuoppala (2018-07-30 13:06:36) > The register for 0xe420 is unable to hold any value, including > this bit. The documentation is also mixed between having a > register bit for toggle and having a state command setup > for it. Apparently the register toggle is deprecated. > > Remove the register toggle as evidence shows it's futile. > > The thing remaining is an apology and humble request for > Mesa folks to resurrect their state setup for this as they > were on right track from start. > > This reverts commit 0bf059f3532bb39c52d917142206a8554fc2f1c5. > > Fixes: 0bf059f3532b ("drm/i915/icl: WaEnableFloatBlendOptimization") > References: HSDES#1406393558 > Cc: Oscar Mateo <oscar.mateo@intel.com> > Cc: Anuj Phogat <anuj.phogat@gmail.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com> > Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> Acked-by: Chris Wilson <chris@chris-wilson.co.uk> The test results do confirm the register is a red herring, but we need someone to confirm that we aren't just using the wrong register etc. -Chris
Quoting Patchwork (2018-07-30 14:23:46) > == Participating hosts (51 -> 47) == > > Additional (2): fi-icl-u fi-kbl-8809g So checking the fi-icl-u results does indeed confirm that live_workarounds is fixed. -Chris
Chris Wilson <chris@chris-wilson.co.uk> writes: > Quoting Patchwork (2018-07-30 14:23:46) >> == Participating hosts (51 -> 47) == >> >> Additional (2): fi-icl-u fi-kbl-8809g > > So checking the fi-icl-u results does indeed confirm that > live_workarounds is fixed. Hmm, gem_workarounds is fixed? I looked at live_workarounds and it seems to check the validity of the whitelists (across resets etc). -Mika
Quoting Mika Kuoppala (2018-07-30 14:59:01) > Chris Wilson <chris@chris-wilson.co.uk> writes: > > > Quoting Patchwork (2018-07-30 14:23:46) > >> == Participating hosts (51 -> 47) == > >> > >> Additional (2): fi-icl-u fi-kbl-8809g > > > > So checking the fi-icl-u results does indeed confirm that > > live_workarounds is fixed. > > Hmm, gem_workarounds is fixed? > > I looked at live_workarounds and it seems to check > the validity of the whitelists (across resets etc). icl hasn't survived long enough to complete a run with gem_workarounds ;) I was just looking at the live_hangcheck results for this series. -Chris
Chris Wilson <chris@chris-wilson.co.uk> writes: > Quoting Mika Kuoppala (2018-07-30 13:06:36) >> The register for 0xe420 is unable to hold any value, including >> this bit. The documentation is also mixed between having a >> register bit for toggle and having a state command setup >> for it. Apparently the register toggle is deprecated. >> >> Remove the register toggle as evidence shows it's futile. >> >> The thing remaining is an apology and humble request for >> Mesa folks to resurrect their state setup for this as they >> were on right track from start. >> >> This reverts commit 0bf059f3532bb39c52d917142206a8554fc2f1c5. >> >> Fixes: 0bf059f3532b ("drm/i915/icl: WaEnableFloatBlendOptimization") >> References: HSDES#1406393558 >> Cc: Oscar Mateo <oscar.mateo@intel.com> >> Cc: Anuj Phogat <anuj.phogat@gmail.com> >> Cc: Chris Wilson <chris@chris-wilson.co.uk> >> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com> >> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> > > Acked-by: Chris Wilson <chris@chris-wilson.co.uk> > > The test results do confirm the register is a red herring, but we need > someone to confirm that we aren't just using the wrong register etc. I will ask around. Thanks for ack. Pushed. -Mika
On Mon, Jul 30, 2018 at 5:07 AM Mika Kuoppala <mika.kuoppala@linux.intel.com> wrote: > The register for 0xe420 is unable to hold any value, including > this bit. The documentation is also mixed between having a > register bit for toggle and having a state command setup > for it. Apparently the register toggle is deprecated. > > CACHE_MODE_SS is not listed in a gfxspecs table which lists all user mode non-privileged registers. So, do you think making any changes to the register from mesa will hold? > Remove the register toggle as evidence shows it's futile. > > The thing remaining is an apology and humble request for > Mesa folks to resurrect their state setup for this as they > were on right track from start. > > This reverts commit 0bf059f3532bb39c52d917142206a8554fc2f1c5. > > Fixes: 0bf059f3532b ("drm/i915/icl: WaEnableFloatBlendOptimization") > References: HSDES#1406393558 > Cc: Oscar Mateo <oscar.mateo@intel.com> > Cc: Anuj Phogat <anuj.phogat@gmail.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com> > Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> > --- > drivers/gpu/drm/i915/i915_reg.h | 3 --- > drivers/gpu/drm/i915/intel_workarounds.c | 3 --- > 2 files changed, 6 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > b/drivers/gpu/drm/i915/i915_reg.h > index 7bdc214ffb6e..e0f5999fff07 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -2859,9 +2859,6 @@ enum i915_power_well_id { > #define GEN8_4x4_STC_OPTIMIZATION_DISABLE (1 << 6) > #define GEN9_PARTIAL_RESOLVE_IN_VC_DISABLE (1 << 1) > > -#define GEN10_CACHE_MODE_SS _MMIO(0xe420) > -#define FLOAT_BLEND_OPTIMIZATION_ENABLE (1 << 4) > - > #define GEN6_BLITTER_ECOSKPD _MMIO(0x221d0) > #define GEN6_BLITTER_LOCK_SHIFT 16 > #define GEN6_BLITTER_FBC_NOTIFY (1 << 3) > diff --git a/drivers/gpu/drm/i915/intel_workarounds.c > b/drivers/gpu/drm/i915/intel_workarounds.c > index f8bb32e974f6..4bcdeaf8d98f 100644 > --- a/drivers/gpu/drm/i915/intel_workarounds.c > +++ b/drivers/gpu/drm/i915/intel_workarounds.c > @@ -508,9 +508,6 @@ static int icl_ctx_workarounds_init(struct > drm_i915_private *dev_priv) > WA_SET_BIT_MASKED(GEN11_COMMON_SLICE_CHICKEN3, > GEN11_BLEND_EMB_FIX_DISABLE_IN_RCC); > > - /* WaEnableFloatBlendOptimization:icl */ > - WA_SET_BIT_MASKED(GEN10_CACHE_MODE_SS, > FLOAT_BLEND_OPTIMIZATION_ENABLE); > - > return 0; > } > > -- > 2.17.1 > > <div dir="ltr"><div class="gmail_default" style="font-family:verdana,sans-serif;font-size:small"><br></div><br><div class="gmail_quote"><div dir="ltr">On Mon, Jul 30, 2018 at 5:07 AM Mika Kuoppala <<a href="mailto:mika.kuoppala@linux.intel.com">mika.kuoppala@linux.intel.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">The register for 0xe420 is unable to hold any value, including<br> this bit. The documentation is also mixed between having a<br> register bit for toggle and having a state command setup<br> for it. Apparently the register toggle is deprecated.<br> <br></blockquote><div> CACHE_MODE_SS is not listed in <div class="gmail_default" style="font-family:verdana,sans-serif;font-size:small;display:inline">a </div>gfxspecs table <div class="gmail_default" style="font-family:verdana,sans-serif;font-size:small;display:inline">which lists all</div> user mode</div><div> non-privileged registers. So, <div class="gmail_default" style="font-family:verdana,sans-serif;font-size:small;display:inline"></div><div class="gmail_default" style="font-family:verdana,sans-serif;font-size:small;display:inline">do you think </div>making any changes <div class="gmail_default" style="font-family:verdana,sans-serif;font-size:small;display:inline">to the register</div></div><div><div class="gmail_default" style="font-family:verdana,sans-serif;font-size:small;display:inline"> from mesa will hold?</div></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> Remove the register toggle as evidence shows it's futile.<br> <br> The thing remaining is an apology and humble request for<br> Mesa folks to resurrect their state setup for this as they<br> were on right track from start.<br> <br> This reverts commit 0bf059f3532bb39c52d917142206a8554fc2f1c5.<br> <br> Fixes: 0bf059f3532b ("drm/i915/icl: WaEnableFloatBlendOptimization")<br> References: HSDES#1406393558<br> Cc: Oscar Mateo <<a href="mailto:oscar.mateo@intel.com" target="_blank">oscar.mateo@intel.com</a>><br> Cc: Anuj Phogat <<a href="mailto:anuj.phogat@gmail.com" target="_blank">anuj.phogat@gmail.com</a>><br> Cc: Chris Wilson <<a href="mailto:chris@chris-wilson.co.uk" target="_blank">chris@chris-wilson.co.uk</a>><br> Cc: Lionel Landwerlin <<a href="mailto:lionel.g.landwerlin@intel.com" target="_blank">lionel.g.landwerlin@intel.com</a>><br> Signed-off-by: Mika Kuoppala <<a href="mailto:mika.kuoppala@linux.intel.com" target="_blank">mika.kuoppala@linux.intel.com</a>><br> ---<br> drivers/gpu/drm/i915/i915_reg.h | 3 ---<br> drivers/gpu/drm/i915/intel_workarounds.c | 3 ---<br> 2 files changed, 6 deletions(-)<br> <br> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h<br> index 7bdc214ffb6e..e0f5999fff07 100644<br> --- a/drivers/gpu/drm/i915/i915_reg.h<br> +++ b/drivers/gpu/drm/i915/i915_reg.h<br> @@ -2859,9 +2859,6 @@ enum i915_power_well_id {<br> #define GEN8_4x4_STC_OPTIMIZATION_DISABLE (1 << 6)<br> #define GEN9_PARTIAL_RESOLVE_IN_VC_DISABLE (1 << 1)<br> <br> -#define GEN10_CACHE_MODE_SS _MMIO(0xe420)<br> -#define FLOAT_BLEND_OPTIMIZATION_ENABLE (1 << 4)<br> -<br> #define GEN6_BLITTER_ECOSKPD _MMIO(0x221d0)<br> #define GEN6_BLITTER_LOCK_SHIFT 16<br> #define GEN6_BLITTER_FBC_NOTIFY (1 << 3)<br> diff --git a/drivers/gpu/drm/i915/intel_workarounds.c b/drivers/gpu/drm/i915/intel_workarounds.c<br> index f8bb32e974f6..4bcdeaf8d98f 100644<br> --- a/drivers/gpu/drm/i915/intel_workarounds.c<br> +++ b/drivers/gpu/drm/i915/intel_workarounds.c<br> @@ -508,9 +508,6 @@ static int icl_ctx_workarounds_init(struct drm_i915_private *dev_priv)<br> WA_SET_BIT_MASKED(GEN11_COMMON_SLICE_CHICKEN3,<br> GEN11_BLEND_EMB_FIX_DISABLE_IN_RCC);<br> <br> - /* WaEnableFloatBlendOptimization:icl */<br> - WA_SET_BIT_MASKED(GEN10_CACHE_MODE_SS, FLOAT_BLEND_OPTIMIZATION_ENABLE);<br> -<br> return 0;<br> }<br> <br> -- <br> 2.17.1<br> <br> </blockquote></div></div>
Quoting Anuj Phogat (2018-08-03 20:24:09) > > > On Mon, Jul 30, 2018 at 5:07 AM Mika Kuoppala <mika.kuoppala@linux.intel.com> > wrote: > > The register for 0xe420 is unable to hold any value, including > this bit. The documentation is also mixed between having a > register bit for toggle and having a state command setup > for it. Apparently the register toggle is deprecated. > > > CACHE_MODE_SS is not listed in > a > gfxspecs table > which lists all > user mode > non-privileged registers. So, > do you think > making any changes > to the register > from mesa will hold? No, a privileged write to the register from inside the ring didn't stick, so something is amiss. -Chris
On Mon, Aug 6, 2018 at 9:14 AM Chris Wilson <chris@chris-wilson.co.uk> wrote: > Quoting Anuj Phogat (2018-08-03 20:24:09) > > > > > > On Mon, Jul 30, 2018 at 5:07 AM Mika Kuoppala < > mika.kuoppala@linux.intel.com> > > wrote: > > > > The register for 0xe420 is unable to hold any value, including > > this bit. The documentation is also mixed between having a > > register bit for toggle and having a state command setup > > for it. Apparently the register toggle is deprecated. > > > > > > CACHE_MODE_SS is not listed in > > a > > gfxspecs table > > which lists all > > user mode > > non-privileged registers. So, > > do you think > > making any changes > > to the register > > from mesa will hold? > > No, a privileged write to the register from inside the ring didn't > stick, so something is amiss. > ok. Mika's commit message confused me where he mentioned about adding back the state setup for this bit in mesa. But, as I understand now the changes won't stick in the register. So, no changes required in mesa. -Chris > <div dir="ltr"><div class="gmail_default" style="font-family:verdana,sans-serif;font-size:small"><br></div><br><div class="gmail_quote"><div dir="ltr">On Mon, Aug 6, 2018 at 9:14 AM Chris Wilson <<a href="mailto:chris@chris-wilson.co.uk">chris@chris-wilson.co.uk</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Quoting Anuj Phogat (2018-08-03 20:24:09)<br> > <br> > <br> > On Mon, Jul 30, 2018 at 5:07 AM Mika Kuoppala <<a href="mailto:mika.kuoppala@linux.intel.com" target="_blank">mika.kuoppala@linux.intel.com</a>><br> > wrote:<br> > <br> > The register for 0xe420 is unable to hold any value, including<br> > this bit. The documentation is also mixed between having a<br> > register bit for toggle and having a state command setup<br> > for it. Apparently the register toggle is deprecated.<br> > <br> > <br> > CACHE_MODE_SS is not listed in<br> > a<br> > gfxspecs table <br> > which lists all<br> > user mode<br> > non-privileged registers. So,<br> > do you think<br> > making any changes<br> > to the register<br> > from mesa will hold?<br> <br> No, a privileged write to the register from inside the ring didn't<br> stick, so something is amiss.<br></blockquote><div><div class="gmail_default" style="font-family:verdana,sans-serif;font-size:small;display:inline">ok. Mika's commit message confused me where he mentioned</div></div><div><div class="gmail_default" style="font-family:verdana,sans-serif;font-size:small;display:inline">about adding back the state </div><span style="font-family:verdana,sans-serif">setup </span><span style="font-family:verdana,sans-serif">for this bit in mesa. <div class="gmail_default" style="font-family:verdana,sans-serif;font-size:small;display:inline">But, as</div></span></div><div><span style="font-family:verdana,sans-serif"><div class="gmail_default" style="font-family:verdana,sans-serif;font-size:small;display:inline">I understand now the changes won't stick in the register. So,</div></span></div><div><span style="font-family:verdana,sans-serif"><div class="gmail_default" style="font-family:verdana,sans-serif;font-size:small;display:inline">no changes required in mesa.</div></span></div><div><div class="gmail_default" style="font-family:verdana,sans-serif;font-size:small;display:inline"><br></div></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> -Chris<br> </blockquote></div></div>
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 7bdc214ffb6e..e0f5999fff07 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -2859,9 +2859,6 @@ enum i915_power_well_id { #define GEN8_4x4_STC_OPTIMIZATION_DISABLE (1 << 6) #define GEN9_PARTIAL_RESOLVE_IN_VC_DISABLE (1 << 1) -#define GEN10_CACHE_MODE_SS _MMIO(0xe420) -#define FLOAT_BLEND_OPTIMIZATION_ENABLE (1 << 4) - #define GEN6_BLITTER_ECOSKPD _MMIO(0x221d0) #define GEN6_BLITTER_LOCK_SHIFT 16 #define GEN6_BLITTER_FBC_NOTIFY (1 << 3) diff --git a/drivers/gpu/drm/i915/intel_workarounds.c b/drivers/gpu/drm/i915/intel_workarounds.c index f8bb32e974f6..4bcdeaf8d98f 100644 --- a/drivers/gpu/drm/i915/intel_workarounds.c +++ b/drivers/gpu/drm/i915/intel_workarounds.c @@ -508,9 +508,6 @@ static int icl_ctx_workarounds_init(struct drm_i915_private *dev_priv) WA_SET_BIT_MASKED(GEN11_COMMON_SLICE_CHICKEN3, GEN11_BLEND_EMB_FIX_DISABLE_IN_RCC); - /* WaEnableFloatBlendOptimization:icl */ - WA_SET_BIT_MASKED(GEN10_CACHE_MODE_SS, FLOAT_BLEND_OPTIMIZATION_ENABLE); - return 0; }
The register for 0xe420 is unable to hold any value, including this bit. The documentation is also mixed between having a register bit for toggle and having a state command setup for it. Apparently the register toggle is deprecated. Remove the register toggle as evidence shows it's futile. The thing remaining is an apology and humble request for Mesa folks to resurrect their state setup for this as they were on right track from start. This reverts commit 0bf059f3532bb39c52d917142206a8554fc2f1c5. Fixes: 0bf059f3532b ("drm/i915/icl: WaEnableFloatBlendOptimization") References: HSDES#1406393558 Cc: Oscar Mateo <oscar.mateo@intel.com> Cc: Anuj Phogat <anuj.phogat@gmail.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> --- drivers/gpu/drm/i915/i915_reg.h | 3 --- drivers/gpu/drm/i915/intel_workarounds.c | 3 --- 2 files changed, 6 deletions(-)