| Message ID | 1510185589-9100-2-git-send-email-oscar.mateo@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Oscar Mateo <oscar.mateo@intel.com> writes: > GEN8_CONFIG0 (0xD00) is a protected by a lock (bit 31) which is set by > the BIOS, so there is no way we can enable the three chicken bits > mandated by the WA (the BIOS should be doing it instead). > > v2: Rebased > v3: Standalone patch > > Signed-off-by: Oscar Mateo <oscar.mateo@intel.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> > --- > drivers/gpu/drm/i915/i915_reg.h | 3 --- > drivers/gpu/drm/i915/intel_pm.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 f0f8f60..7991d90 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -355,9 +355,6 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg) > #define ECOCHK_PPGTT_WT_HSW (0x2<<3) > #define ECOCHK_PPGTT_WB_HSW (0x3<<3) > > -#define GEN8_CONFIG0 _MMIO(0xD00) > -#define GEN9_DEFAULT_FIXES (1 << 3 | 1 << 2 | 1 << 1) > - > #define GAC_ECO_BITS _MMIO(0x14090) > #define ECOBITS_SNB_BIT (1<<13) > #define ECOBITS_PPGTT_CACHE64B (3<<8) > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index e09377d..5bd49a7 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -75,9 +75,6 @@ static void gen9_init_clock_gating(struct drm_i915_private *dev_priv) > I915_WRITE(CHICKEN_PAR1_1, > I915_READ(CHICKEN_PAR1_1) | SKL_EDP_PSR_FIX_RDWRAP); > > - I915_WRITE(GEN8_CONFIG0, > - I915_READ(GEN8_CONFIG0) | GEN9_DEFAULT_FIXES); > - I am pondering if we should do a verifying read for workarounds and chickens in general. Also my kbl bios seem to omit all of these with lock being on :P Patch is, Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> -Mika > /* WaEnableChickenDCPR:skl,bxt,kbl,glk,cfl */ > I915_WRITE(GEN8_CHICKEN_DCPR_1, > I915_READ(GEN8_CHICKEN_DCPR_1) | MASK_WAKEMEM); > -- > 1.9.1
Quoting Mika Kuoppala (2017-11-09 11:42:45) > Oscar Mateo <oscar.mateo@intel.com> writes: > > > GEN8_CONFIG0 (0xD00) is a protected by a lock (bit 31) which is set by > > the BIOS, so there is no way we can enable the three chicken bits > > mandated by the WA (the BIOS should be doing it instead). > > > > v2: Rebased > > v3: Standalone patch > > > > Signed-off-by: Oscar Mateo <oscar.mateo@intel.com> > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> > > --- > > drivers/gpu/drm/i915/i915_reg.h | 3 --- > > drivers/gpu/drm/i915/intel_pm.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 f0f8f60..7991d90 100644 > > --- a/drivers/gpu/drm/i915/i915_reg.h > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > @@ -355,9 +355,6 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg) > > #define ECOCHK_PPGTT_WT_HSW (0x2<<3) > > #define ECOCHK_PPGTT_WB_HSW (0x3<<3) > > > > -#define GEN8_CONFIG0 _MMIO(0xD00) > > -#define GEN9_DEFAULT_FIXES (1 << 3 | 1 << 2 | 1 << 1) > > - > > #define GAC_ECO_BITS _MMIO(0x14090) > > #define ECOBITS_SNB_BIT (1<<13) > > #define ECOBITS_PPGTT_CACHE64B (3<<8) > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > > index e09377d..5bd49a7 100644 > > --- a/drivers/gpu/drm/i915/intel_pm.c > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > @@ -75,9 +75,6 @@ static void gen9_init_clock_gating(struct drm_i915_private *dev_priv) > > I915_WRITE(CHICKEN_PAR1_1, > > I915_READ(CHICKEN_PAR1_1) | SKL_EDP_PSR_FIX_RDWRAP); > > > > - I915_WRITE(GEN8_CONFIG0, > > - I915_READ(GEN8_CONFIG0) | GEN9_DEFAULT_FIXES); > > - > > I am pondering if we should do a verifying read for workarounds > and chickens in general. We are getting into the igt/tools/intel_workarounds territory. i.e. not the regression checker, but a tool that checks that all known workarounds are applied for the platform. The debate is whether such a tool should be entirely independent of the kernel (carrying its own w/a db) or if we put the entire db into the kernel. I can see arguments in favour of both... -Chris
Quoting Mika Kuoppala (2017-11-09 11:42:45) > Oscar Mateo <oscar.mateo@intel.com> writes: > > > GEN8_CONFIG0 (0xD00) is a protected by a lock (bit 31) which is set by > > the BIOS, so there is no way we can enable the three chicken bits > > mandated by the WA (the BIOS should be doing it instead). > > > > v2: Rebased > > v3: Standalone patch > > > > Signed-off-by: Oscar Mateo <oscar.mateo@intel.com> > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> > > --- > > drivers/gpu/drm/i915/i915_reg.h | 3 --- > > drivers/gpu/drm/i915/intel_pm.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 f0f8f60..7991d90 100644 > > --- a/drivers/gpu/drm/i915/i915_reg.h > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > @@ -355,9 +355,6 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg) > > #define ECOCHK_PPGTT_WT_HSW (0x2<<3) > > #define ECOCHK_PPGTT_WB_HSW (0x3<<3) > > > > -#define GEN8_CONFIG0 _MMIO(0xD00) > > -#define GEN9_DEFAULT_FIXES (1 << 3 | 1 << 2 | 1 << 1) > > - > > #define GAC_ECO_BITS _MMIO(0x14090) > > #define ECOBITS_SNB_BIT (1<<13) > > #define ECOBITS_PPGTT_CACHE64B (3<<8) > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > > index e09377d..5bd49a7 100644 > > --- a/drivers/gpu/drm/i915/intel_pm.c > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > @@ -75,9 +75,6 @@ static void gen9_init_clock_gating(struct drm_i915_private *dev_priv) > > I915_WRITE(CHICKEN_PAR1_1, > > I915_READ(CHICKEN_PAR1_1) | SKL_EDP_PSR_FIX_RDWRAP); > > > > - I915_WRITE(GEN8_CONFIG0, > > - I915_READ(GEN8_CONFIG0) | GEN9_DEFAULT_FIXES); > > - > > I am pondering if we should do a verifying read for workarounds > and chickens in general. > > Also my kbl bios seem to omit all of these with lock being on :P > > Patch is, > Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> Added References: b033bb6d5d3a ("drm/i915/gen9: Enable must set chicken bits in config0 reg") and pushed. Thanks for the patch, -Chris
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index f0f8f60..7991d90 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -355,9 +355,6 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg) #define ECOCHK_PPGTT_WT_HSW (0x2<<3) #define ECOCHK_PPGTT_WB_HSW (0x3<<3) -#define GEN8_CONFIG0 _MMIO(0xD00) -#define GEN9_DEFAULT_FIXES (1 << 3 | 1 << 2 | 1 << 1) - #define GAC_ECO_BITS _MMIO(0x14090) #define ECOBITS_SNB_BIT (1<<13) #define ECOBITS_PPGTT_CACHE64B (3<<8) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index e09377d..5bd49a7 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -75,9 +75,6 @@ static void gen9_init_clock_gating(struct drm_i915_private *dev_priv) I915_WRITE(CHICKEN_PAR1_1, I915_READ(CHICKEN_PAR1_1) | SKL_EDP_PSR_FIX_RDWRAP); - I915_WRITE(GEN8_CONFIG0, - I915_READ(GEN8_CONFIG0) | GEN9_DEFAULT_FIXES); - /* WaEnableChickenDCPR:skl,bxt,kbl,glk,cfl */ I915_WRITE(GEN8_CHICKEN_DCPR_1, I915_READ(GEN8_CHICKEN_DCPR_1) | MASK_WAKEMEM);
GEN8_CONFIG0 (0xD00) is a protected by a lock (bit 31) which is set by the BIOS, so there is no way we can enable the three chicken bits mandated by the WA (the BIOS should be doing it instead). v2: Rebased v3: Standalone patch Signed-off-by: Oscar Mateo <oscar.mateo@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> --- drivers/gpu/drm/i915/i915_reg.h | 3 --- drivers/gpu/drm/i915/intel_pm.c | 3 --- 2 files changed, 6 deletions(-)