| Message ID | 1414511304-4462-1-git-send-email-arun.siluvery@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Oct 28, 2014 at 03:48:24PM +0000, Arun Siluvery wrote: > +WaForceEnableNonCoherent:chv > +WaHdcDisableFetchWhenMasked:chv > -WaDisableDopClockGating:chv > -WaDisableSamplerPowerBypass:chv > -WaDisableGunitClockGating:chv > -WaDisableFfDopClockGating:chv > -WaDisableDopClockGating:chv > > WaDisablePartialInstShootdown:chv and > WaDisableThreadStallDopClockGating:chv are related to the > same register so combine them. Please split into at least two patches (one to add new w/as and another to remove old ones). Otherwise reverting is a pita in case we find that one of the dropped w/as was actually still needed. > > v2: Remove pre-production WA instead of restricting them > based on revision id (Ville) > > For: VIZ-4090 > Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com> > --- > drivers/gpu/drm/i915/i915_reg.h | 1 + > drivers/gpu/drm/i915/intel_pm.c | 12 ------------ > drivers/gpu/drm/i915/intel_ringbuffer.c | 22 +++++++++++----------- > 3 files changed, 12 insertions(+), 23 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 77fce96..9d39700 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -5024,6 +5024,7 @@ enum punit_power_well { > /* GEN8 chicken */ > #define HDC_CHICKEN0 0x7300 > #define HDC_FORCE_NON_COHERENT (1<<4) > +#define HDC_DONOT_FETCH_MEM_WHEN_MASKED (1<<11) > #define HDC_FENCE_DEST_SLM_DISABLE (1<<14) > > /* WaCatErrorRejectionIssue */ > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 7a69eba..93db25f 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -5944,18 +5944,6 @@ static void cherryview_init_clock_gating(struct drm_device *dev) > /* WaDisableSDEUnitClockGating:chv */ > I915_WRITE(GEN8_UCGCTL6, I915_READ(GEN8_UCGCTL6) | > GEN8_SDEUNIT_CLOCK_GATE_DISABLE); > - > - /* WaDisableGunitClockGating:chv (pre-production hw) */ > - I915_WRITE(VLV_GUNIT_CLOCK_GATE, I915_READ(VLV_GUNIT_CLOCK_GATE) | > - GINT_DIS); OK > - > - /* WaDisableFfDopClockGating:chv (pre-production hw) */ > - I915_WRITE(GEN6_RC_SLEEP_PSMI_CONTROL, > - _MASKED_BIT_ENABLE(GEN8_FF_DOP_CLOCK_GATE_DISABLE)); OK > - > - /* WaDisableDopClockGating:chv (pre-production hw) */ > - I915_WRITE(GEN6_UCGCTL1, I915_READ(GEN6_UCGCTL1) | > - GEN6_EU_TCUNIT_CLOCK_GATE_DISABLE); OK, I think. This was the weird w/a where it seemed hard to figure out what it needed. Nothing in BSpec about needing this bit on chv. > } > > static void g4x_init_clock_gating(struct drm_device *dev) > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c > index a8f72e8..368b20a 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > @@ -788,20 +788,20 @@ static int chv_init_workarounds(struct intel_engine_cs *ring) > struct drm_i915_private *dev_priv = dev->dev_private; > > /* WaDisablePartialInstShootdown:chv */ > - WA_SET_BIT_MASKED(GEN8_ROW_CHICKEN, > - PARTIAL_INSTRUCTION_SHOOTDOWN_DISABLE); > - > /* WaDisableThreadStallDopClockGating:chv */ > WA_SET_BIT_MASKED(GEN8_ROW_CHICKEN, > - STALL_DOP_GATING_DISABLE); > - > - /* WaDisableDopClockGating:chv (pre-production hw) */ > - WA_SET_BIT_MASKED(GEN7_ROW_CHICKEN2, > - DOP_CLOCK_GATING_DISABLE); OK, again the weird w/a but Bspec seems to agree at least. > + PARTIAL_INSTRUCTION_SHOOTDOWN_DISABLE | > + STALL_DOP_GATING_DISABLE); Bspec says bit 5 is MBZ now, and yet the w/a database says it's forever. And the hardware accepts 1 there so it's not like many other MBZ bits that you can't set even if you try. Also Bspec has three different definitions for this bit on gen8, all disagree with each other and one definiton even manages to disagree with itself. And reading the hsd stuff I'm not the only that has been confused by this, and yet I see no conclusion there as to how this bit should be configured. Oh well, I guess we can leave it set for now and maybe eventually someone will figure out what we're supposed to do. > > - /* WaDisableSamplerPowerBypass:chv (pre-production hw) */ > - WA_SET_BIT_MASKED(HALF_SLICE_CHICKEN3, > - GEN8_SAMPLER_POWER_BYPASS_DIS); OK > + /* Use Force Non-Coherent whenever executing a 3D context. This is a > + * workaround for a possible hang in the unlikely event a TLB > + * invalidation occurs during a PSD flush. > + */ > + /* WaForceEnableNonCoherent:chv */ > + /* WaHdcDisableFetchWhenMasked:chv */ > + WA_SET_BIT_MASKED(HDC_CHICKEN0, > + HDC_FORCE_NON_COHERENT | > + HDC_DONOT_FETCH_MEM_WHEN_MASKED); OK Right, so split it up a bit and you can add Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> to the patches. > > return 0; > } > -- > 2.1.2 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On 28/10/2014 17:06, Ville Syrjälä wrote: > On Tue, Oct 28, 2014 at 03:48:24PM +0000, Arun Siluvery wrote: >> +WaForceEnableNonCoherent:chv >> +WaHdcDisableFetchWhenMasked:chv >> -WaDisableDopClockGating:chv >> -WaDisableSamplerPowerBypass:chv >> -WaDisableGunitClockGating:chv >> -WaDisableFfDopClockGating:chv >> -WaDisableDopClockGating:chv >> >> WaDisablePartialInstShootdown:chv and >> WaDisableThreadStallDopClockGating:chv are related to the >> same register so combine them. > > Please split into at least two patches (one to add new w/as and another to > remove old ones). Otherwise reverting is a pita in case we find that one > of the dropped w/as was actually still needed. > I thought of doing that but then combined them as these are early pre-production ones and we may not need them in future but I agree splitting them helps in reverting them if required. >> >> v2: Remove pre-production WA instead of restricting them >> based on revision id (Ville) >> >> For: VIZ-4090 >> Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com> >> --- >> drivers/gpu/drm/i915/i915_reg.h | 1 + >> drivers/gpu/drm/i915/intel_pm.c | 12 ------------ >> drivers/gpu/drm/i915/intel_ringbuffer.c | 22 +++++++++++----------- >> 3 files changed, 12 insertions(+), 23 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h >> index 77fce96..9d39700 100644 >> --- a/drivers/gpu/drm/i915/i915_reg.h >> +++ b/drivers/gpu/drm/i915/i915_reg.h >> @@ -5024,6 +5024,7 @@ enum punit_power_well { >> /* GEN8 chicken */ >> #define HDC_CHICKEN0 0x7300 >> #define HDC_FORCE_NON_COHERENT (1<<4) >> +#define HDC_DONOT_FETCH_MEM_WHEN_MASKED (1<<11) >> #define HDC_FENCE_DEST_SLM_DISABLE (1<<14) >> >> /* WaCatErrorRejectionIssue */ >> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c >> index 7a69eba..93db25f 100644 >> --- a/drivers/gpu/drm/i915/intel_pm.c >> +++ b/drivers/gpu/drm/i915/intel_pm.c >> @@ -5944,18 +5944,6 @@ static void cherryview_init_clock_gating(struct drm_device *dev) >> /* WaDisableSDEUnitClockGating:chv */ >> I915_WRITE(GEN8_UCGCTL6, I915_READ(GEN8_UCGCTL6) | >> GEN8_SDEUNIT_CLOCK_GATE_DISABLE); >> - >> - /* WaDisableGunitClockGating:chv (pre-production hw) */ >> - I915_WRITE(VLV_GUNIT_CLOCK_GATE, I915_READ(VLV_GUNIT_CLOCK_GATE) | >> - GINT_DIS); > > OK > >> - >> - /* WaDisableFfDopClockGating:chv (pre-production hw) */ >> - I915_WRITE(GEN6_RC_SLEEP_PSMI_CONTROL, >> - _MASKED_BIT_ENABLE(GEN8_FF_DOP_CLOCK_GATE_DISABLE)); > > OK > >> - >> - /* WaDisableDopClockGating:chv (pre-production hw) */ >> - I915_WRITE(GEN6_UCGCTL1, I915_READ(GEN6_UCGCTL1) | >> - GEN6_EU_TCUNIT_CLOCK_GATE_DISABLE); > > OK, I think. This was the weird w/a where it seemed hard to figure out > what it needed. Nothing in BSpec about needing this bit on chv. > >> } >> >> static void g4x_init_clock_gating(struct drm_device *dev) >> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c >> index a8f72e8..368b20a 100644 >> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c >> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c >> @@ -788,20 +788,20 @@ static int chv_init_workarounds(struct intel_engine_cs *ring) >> struct drm_i915_private *dev_priv = dev->dev_private; >> >> /* WaDisablePartialInstShootdown:chv */ >> - WA_SET_BIT_MASKED(GEN8_ROW_CHICKEN, >> - PARTIAL_INSTRUCTION_SHOOTDOWN_DISABLE); >> - >> /* WaDisableThreadStallDopClockGating:chv */ >> WA_SET_BIT_MASKED(GEN8_ROW_CHICKEN, >> - STALL_DOP_GATING_DISABLE); >> - >> - /* WaDisableDopClockGating:chv (pre-production hw) */ >> - WA_SET_BIT_MASKED(GEN7_ROW_CHICKEN2, >> - DOP_CLOCK_GATING_DISABLE); > > OK, again the weird w/a but Bspec seems to agree at least. > >> + PARTIAL_INSTRUCTION_SHOOTDOWN_DISABLE | >> + STALL_DOP_GATING_DISABLE); > > Bspec says bit 5 is MBZ now, and yet the w/a database says it's > forever. And the hardware accepts 1 there so it's not like many other > MBZ bits that you can't set even if you try. Also Bspec has three > different definitions for this bit on gen8, all disagree with each other > and one definiton even manages to disagree with itself. And reading the > hsd stuff I'm not the only that has been confused by this, and yet I see > no conclusion there as to how this bit should be configured. > > Oh well, I guess we can leave it set for now and maybe eventually > someone will figure out what we're supposed to do. > I am using w/a database as reference and it says forever, spec seems to disagree but probably not yet updated. regards Arun >> >> - /* WaDisableSamplerPowerBypass:chv (pre-production hw) */ >> - WA_SET_BIT_MASKED(HALF_SLICE_CHICKEN3, >> - GEN8_SAMPLER_POWER_BYPASS_DIS); > > OK > >> + /* Use Force Non-Coherent whenever executing a 3D context. This is a >> + * workaround for a possible hang in the unlikely event a TLB >> + * invalidation occurs during a PSD flush. >> + */ >> + /* WaForceEnableNonCoherent:chv */ >> + /* WaHdcDisableFetchWhenMasked:chv */ >> + WA_SET_BIT_MASKED(HDC_CHICKEN0, >> + HDC_FORCE_NON_COHERENT | >> + HDC_DONOT_FETCH_MEM_WHEN_MASKED); > > OK > > Right, so split it up a bit and you can add > > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > to the patches. > >> >> return 0; >> } >> -- >> 2.1.2 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx >
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 77fce96..9d39700 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -5024,6 +5024,7 @@ enum punit_power_well { /* GEN8 chicken */ #define HDC_CHICKEN0 0x7300 #define HDC_FORCE_NON_COHERENT (1<<4) +#define HDC_DONOT_FETCH_MEM_WHEN_MASKED (1<<11) #define HDC_FENCE_DEST_SLM_DISABLE (1<<14) /* WaCatErrorRejectionIssue */ diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 7a69eba..93db25f 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -5944,18 +5944,6 @@ static void cherryview_init_clock_gating(struct drm_device *dev) /* WaDisableSDEUnitClockGating:chv */ I915_WRITE(GEN8_UCGCTL6, I915_READ(GEN8_UCGCTL6) | GEN8_SDEUNIT_CLOCK_GATE_DISABLE); - - /* WaDisableGunitClockGating:chv (pre-production hw) */ - I915_WRITE(VLV_GUNIT_CLOCK_GATE, I915_READ(VLV_GUNIT_CLOCK_GATE) | - GINT_DIS); - - /* WaDisableFfDopClockGating:chv (pre-production hw) */ - I915_WRITE(GEN6_RC_SLEEP_PSMI_CONTROL, - _MASKED_BIT_ENABLE(GEN8_FF_DOP_CLOCK_GATE_DISABLE)); - - /* WaDisableDopClockGating:chv (pre-production hw) */ - I915_WRITE(GEN6_UCGCTL1, I915_READ(GEN6_UCGCTL1) | - GEN6_EU_TCUNIT_CLOCK_GATE_DISABLE); } static void g4x_init_clock_gating(struct drm_device *dev) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index a8f72e8..368b20a 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -788,20 +788,20 @@ static int chv_init_workarounds(struct intel_engine_cs *ring) struct drm_i915_private *dev_priv = dev->dev_private; /* WaDisablePartialInstShootdown:chv */ - WA_SET_BIT_MASKED(GEN8_ROW_CHICKEN, - PARTIAL_INSTRUCTION_SHOOTDOWN_DISABLE); - /* WaDisableThreadStallDopClockGating:chv */ WA_SET_BIT_MASKED(GEN8_ROW_CHICKEN, - STALL_DOP_GATING_DISABLE); - - /* WaDisableDopClockGating:chv (pre-production hw) */ - WA_SET_BIT_MASKED(GEN7_ROW_CHICKEN2, - DOP_CLOCK_GATING_DISABLE); + PARTIAL_INSTRUCTION_SHOOTDOWN_DISABLE | + STALL_DOP_GATING_DISABLE); - /* WaDisableSamplerPowerBypass:chv (pre-production hw) */ - WA_SET_BIT_MASKED(HALF_SLICE_CHICKEN3, - GEN8_SAMPLER_POWER_BYPASS_DIS); + /* Use Force Non-Coherent whenever executing a 3D context. This is a + * workaround for a possible hang in the unlikely event a TLB + * invalidation occurs during a PSD flush. + */ + /* WaForceEnableNonCoherent:chv */ + /* WaHdcDisableFetchWhenMasked:chv */ + WA_SET_BIT_MASKED(HDC_CHICKEN0, + HDC_FORCE_NON_COHERENT | + HDC_DONOT_FETCH_MEM_WHEN_MASKED); return 0; }
+WaForceEnableNonCoherent:chv +WaHdcDisableFetchWhenMasked:chv -WaDisableDopClockGating:chv -WaDisableSamplerPowerBypass:chv -WaDisableGunitClockGating:chv -WaDisableFfDopClockGating:chv -WaDisableDopClockGating:chv WaDisablePartialInstShootdown:chv and WaDisableThreadStallDopClockGating:chv are related to the same register so combine them. v2: Remove pre-production WA instead of restricting them based on revision id (Ville) For: VIZ-4090 Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com> --- drivers/gpu/drm/i915/i915_reg.h | 1 + drivers/gpu/drm/i915/intel_pm.c | 12 ------------ drivers/gpu/drm/i915/intel_ringbuffer.c | 22 +++++++++++----------- 3 files changed, 12 insertions(+), 23 deletions(-)