| Message ID | 1443188026-1222-5-git-send-email-arun.siluvery@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Sep 25, 2015 at 02:33:38PM +0100, Arun Siluvery wrote: > Merge WaDisableSamplerPowerBypassForSOPingPong and another WA which has no name > as they are part of same register. This will save an entry in WA array. > > Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_ringbuffer.c | 18 ++++++++---------- > 1 file changed, 8 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c > index ad16ef4..963b3ca 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > @@ -914,9 +914,14 @@ static int gen9_init_workarounds(struct intel_engine_cs *ring) > WA_SET_BIT_MASKED(GEN8_ROW_CHICKEN, > PARTIAL_INSTRUCTION_SHOOTDOWN_DISABLE); > > - /* Syncing dependencies between camera and graphics:skl,bxt */ > - WA_SET_BIT_MASKED(HALF_SLICE_CHICKEN3, > - GEN9_DISABLE_OCL_OOB_SUPPRESS_LOGIC); > + /* WA: Syncing dependencies between camera and graphics:skl,bxt */ > + /* WaDisableSamplerPowerBypassForSOPingPong:skl,bxt */ > + tmp = GEN9_DISABLE_OCL_OOB_SUPPRESS_LOGIC; > + if (IS_SKYLAKE(dev) || > + (IS_BROXTON(dev) && INTEL_REVID(dev) <= BXT_REVID_B0)) { > + tmp |= GEN8_SAMPLER_POWER_BYPASS_DIS; > + } > + WA_SET_BIT_MASKED(HALF_SLICE_CHICKEN3, tmp); I really dislike these platform+stepping checks in the shared codepath. If there's any difference between the platforms, IMO the w/a should go into the per-platform function. > > if ((IS_SKYLAKE(dev) && (INTEL_REVID(dev) == SKL_REVID_A0 || > INTEL_REVID(dev) == SKL_REVID_B0)) || > @@ -967,13 +972,6 @@ static int gen9_init_workarounds(struct intel_engine_cs *ring) > tmp |= HDC_FORCE_CSR_NON_COHERENT_OVR_DISABLE; > WA_SET_BIT_MASKED(HDC_CHICKEN0, tmp); > > - /* WaDisableSamplerPowerBypassForSOPingPong:skl,bxt */ > - if (IS_SKYLAKE(dev) || > - (IS_BROXTON(dev) && INTEL_REVID(dev) <= BXT_REVID_B0)) { > - WA_SET_BIT_MASKED(HALF_SLICE_CHICKEN3, > - GEN8_SAMPLER_POWER_BYPASS_DIS); > - } > - > /* WaDisableSTUnitPowerOptimization:skl,bxt */ > WA_SET_BIT_MASKED(HALF_SLICE_CHICKEN2, GEN8_ST_PO_DISABLE); > > -- > 1.9.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Fri, 25 Sep 2015, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > On Fri, Sep 25, 2015 at 02:33:38PM +0100, Arun Siluvery wrote: >> Merge WaDisableSamplerPowerBypassForSOPingPong and another WA which has no name >> as they are part of same register. This will save an entry in WA array. >> >> Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com> >> --- >> drivers/gpu/drm/i915/intel_ringbuffer.c | 18 ++++++++---------- >> 1 file changed, 8 insertions(+), 10 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c >> index ad16ef4..963b3ca 100644 >> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c >> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c >> @@ -914,9 +914,14 @@ static int gen9_init_workarounds(struct intel_engine_cs *ring) >> WA_SET_BIT_MASKED(GEN8_ROW_CHICKEN, >> PARTIAL_INSTRUCTION_SHOOTDOWN_DISABLE); >> >> - /* Syncing dependencies between camera and graphics:skl,bxt */ >> - WA_SET_BIT_MASKED(HALF_SLICE_CHICKEN3, >> - GEN9_DISABLE_OCL_OOB_SUPPRESS_LOGIC); >> + /* WA: Syncing dependencies between camera and graphics:skl,bxt */ >> + /* WaDisableSamplerPowerBypassForSOPingPong:skl,bxt */ >> + tmp = GEN9_DISABLE_OCL_OOB_SUPPRESS_LOGIC; >> + if (IS_SKYLAKE(dev) || >> + (IS_BROXTON(dev) && INTEL_REVID(dev) <= BXT_REVID_B0)) { >> + tmp |= GEN8_SAMPLER_POWER_BYPASS_DIS; >> + } >> + WA_SET_BIT_MASKED(HALF_SLICE_CHICKEN3, tmp); > > I really dislike these platform+stepping checks in the shared codepath. > If there's any difference between the platforms, IMO the w/a should go into > the per-platform function. Agreed, also on a more generic level than these specific functions. BR, Jani. > >> >> if ((IS_SKYLAKE(dev) && (INTEL_REVID(dev) == SKL_REVID_A0 || >> INTEL_REVID(dev) == SKL_REVID_B0)) || >> @@ -967,13 +972,6 @@ static int gen9_init_workarounds(struct intel_engine_cs *ring) >> tmp |= HDC_FORCE_CSR_NON_COHERENT_OVR_DISABLE; >> WA_SET_BIT_MASKED(HDC_CHICKEN0, tmp); >> >> - /* WaDisableSamplerPowerBypassForSOPingPong:skl,bxt */ >> - if (IS_SKYLAKE(dev) || >> - (IS_BROXTON(dev) && INTEL_REVID(dev) <= BXT_REVID_B0)) { >> - WA_SET_BIT_MASKED(HALF_SLICE_CHICKEN3, >> - GEN8_SAMPLER_POWER_BYPASS_DIS); >> - } >> - >> /* WaDisableSTUnitPowerOptimization:skl,bxt */ >> WA_SET_BIT_MASKED(HALF_SLICE_CHICKEN2, GEN8_ST_PO_DISABLE); >> >> -- >> 1.9.1 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Ville Syrjälä > Intel OTC > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Mon, Sep 28, 2015 at 11:47:06AM +0300, Jani Nikula wrote: > On Fri, 25 Sep 2015, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > > On Fri, Sep 25, 2015 at 02:33:38PM +0100, Arun Siluvery wrote: > >> Merge WaDisableSamplerPowerBypassForSOPingPong and another WA which has no name > >> as they are part of same register. This will save an entry in WA array. > >> > >> Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com> > >> --- > >> drivers/gpu/drm/i915/intel_ringbuffer.c | 18 ++++++++---------- > >> 1 file changed, 8 insertions(+), 10 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c > >> index ad16ef4..963b3ca 100644 > >> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > >> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > >> @@ -914,9 +914,14 @@ static int gen9_init_workarounds(struct intel_engine_cs *ring) > >> WA_SET_BIT_MASKED(GEN8_ROW_CHICKEN, > >> PARTIAL_INSTRUCTION_SHOOTDOWN_DISABLE); > >> > >> - /* Syncing dependencies between camera and graphics:skl,bxt */ > >> - WA_SET_BIT_MASKED(HALF_SLICE_CHICKEN3, > >> - GEN9_DISABLE_OCL_OOB_SUPPRESS_LOGIC); > >> + /* WA: Syncing dependencies between camera and graphics:skl,bxt */ > >> + /* WaDisableSamplerPowerBypassForSOPingPong:skl,bxt */ > >> + tmp = GEN9_DISABLE_OCL_OOB_SUPPRESS_LOGIC; > >> + if (IS_SKYLAKE(dev) || > >> + (IS_BROXTON(dev) && INTEL_REVID(dev) <= BXT_REVID_B0)) { > >> + tmp |= GEN8_SAMPLER_POWER_BYPASS_DIS; > >> + } > >> + WA_SET_BIT_MASKED(HALF_SLICE_CHICKEN3, tmp); > > > > I really dislike these platform+stepping checks in the shared codepath. > > If there's any difference between the platforms, IMO the w/a should go into > > the per-platform function. > > Agreed, also on a more generic level than these specific functions. Concurred, sharing code just to reduce the line count is a bad idea, code should only be shared if it actually does the same thing. Better to be verbose than accidentally break things. -Daniel
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index ad16ef4..963b3ca 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -914,9 +914,14 @@ static int gen9_init_workarounds(struct intel_engine_cs *ring) WA_SET_BIT_MASKED(GEN8_ROW_CHICKEN, PARTIAL_INSTRUCTION_SHOOTDOWN_DISABLE); - /* Syncing dependencies between camera and graphics:skl,bxt */ - WA_SET_BIT_MASKED(HALF_SLICE_CHICKEN3, - GEN9_DISABLE_OCL_OOB_SUPPRESS_LOGIC); + /* WA: Syncing dependencies between camera and graphics:skl,bxt */ + /* WaDisableSamplerPowerBypassForSOPingPong:skl,bxt */ + tmp = GEN9_DISABLE_OCL_OOB_SUPPRESS_LOGIC; + if (IS_SKYLAKE(dev) || + (IS_BROXTON(dev) && INTEL_REVID(dev) <= BXT_REVID_B0)) { + tmp |= GEN8_SAMPLER_POWER_BYPASS_DIS; + } + WA_SET_BIT_MASKED(HALF_SLICE_CHICKEN3, tmp); if ((IS_SKYLAKE(dev) && (INTEL_REVID(dev) == SKL_REVID_A0 || INTEL_REVID(dev) == SKL_REVID_B0)) || @@ -967,13 +972,6 @@ static int gen9_init_workarounds(struct intel_engine_cs *ring) tmp |= HDC_FORCE_CSR_NON_COHERENT_OVR_DISABLE; WA_SET_BIT_MASKED(HDC_CHICKEN0, tmp); - /* WaDisableSamplerPowerBypassForSOPingPong:skl,bxt */ - if (IS_SKYLAKE(dev) || - (IS_BROXTON(dev) && INTEL_REVID(dev) <= BXT_REVID_B0)) { - WA_SET_BIT_MASKED(HALF_SLICE_CHICKEN3, - GEN8_SAMPLER_POWER_BYPASS_DIS); - } - /* WaDisableSTUnitPowerOptimization:skl,bxt */ WA_SET_BIT_MASKED(HALF_SLICE_CHICKEN2, GEN8_ST_PO_DISABLE);
Merge WaDisableSamplerPowerBypassForSOPingPong and another WA which has no name as they are part of same register. This will save an entry in WA array. Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com> --- drivers/gpu/drm/i915/intel_ringbuffer.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-)