| Message ID | 1461587888-5047-2-git-send-email-imre.deak@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Imre Deak <imre.deak@intel.com> writes: > [ text/plain ] > No need for hard-coding the register value, the corresponding fields are > defined properly in BSpec. > > No functional change. > > Signed-off-by: Imre Deak <imre.deak@intel.com> > --- > drivers/gpu/drm/i915/i915_reg.h | 3 ++- > drivers/gpu/drm/i915/intel_pm.c | 3 ++- > 2 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index c21b71c..0cb2e17 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -6073,7 +6073,8 @@ enum skl_disp_power_wells { > #define VLV_B0_WA_L3SQCREG1_VALUE 0x00D30000 > > #define GEN8_L3SQCREG1 _MMIO(0xB100) > -#define BDW_WA_L3SQCREG1_DEFAULT 0x784000 > +#define L3_GENERAL_PRIO_CREDITS(x) (((x) >> 1) << 19) > +#define L3_HIGH_PRIO_CREDITS(x) (((x) >> 1) << 14) > The values listed for CHV it seems that the macros would produce one less than intended. For example chv 34 -> 0b10010 -Mika > #define GEN7_L3CNTLREG1 _MMIO(0xB01C) > #define GEN7_WA_FOR_GEN7_L3_CONTROL 0x3C47FF8C > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index a6fd4dd..a9b7626 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -6710,7 +6710,8 @@ static void broadwell_init_clock_gating(struct drm_device *dev) > */ > misccpctl = I915_READ(GEN7_MISCCPCTL); > I915_WRITE(GEN7_MISCCPCTL, misccpctl & ~GEN7_DOP_CLOCK_GATE_ENABLE); > - I915_WRITE(GEN8_L3SQCREG1, BDW_WA_L3SQCREG1_DEFAULT); > + I915_WRITE(GEN8_L3SQCREG1, L3_GENERAL_PRIO_CREDITS(30) | > + L3_HIGH_PRIO_CREDITS(2)); > /* > * Wait at least 100 clocks before re-enabling clock gating. See > * the definition of L3SQCREG1 in BSpec. > -- > 2.5.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On ti, 2016-04-26 at 11:11 +0300, Mika Kuoppala wrote: > Imre Deak <imre.deak@intel.com> writes: > > > [ text/plain ] > > No need for hard-coding the register value, the corresponding > > fields are > > defined properly in BSpec. > > > > No functional change. > > > > Signed-off-by: Imre Deak <imre.deak@intel.com> > > --- > > drivers/gpu/drm/i915/i915_reg.h | 3 ++- > > drivers/gpu/drm/i915/intel_pm.c | 3 ++- > > 2 files changed, 4 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > > b/drivers/gpu/drm/i915/i915_reg.h > > index c21b71c..0cb2e17 100644 > > --- a/drivers/gpu/drm/i915/i915_reg.h > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > @@ -6073,7 +6073,8 @@ enum skl_disp_power_wells { > > #define VLV_B0_WA_L3SQCREG1_VALUE 0x00D30000 > > > > #define GEN8_L3SQCREG1 _MMIO(0xB100 > > ) > > -#define BDW_WA_L3SQCREG1_DEFAULT 0x784000 > > +#define L3_GENERAL_PRIO_CREDITS(x) (((x) >> 1) << > > 19) > > +#define L3_HIGH_PRIO_CREDITS(x) (((x) >> 1) << 14) > > > > The values listed for CHV it seems that the macros would produce > one less than intended. For example chv 34 -> 0b10010 Yes, Ville noticed this too. I assumed the same formula as used on the other platforms (BDW, SKL, BXT) and haven't noticed the difference wrt. CHV. According to Ville's tests using the formula in the spec (and setting the new credits 30, 2) leads to a hang. So my suspicion is that it's a bug in the spec, I will file a change request there. --Imre > > -Mika > > > > #define GEN7_L3CNTLREG1 _MMIO(0xB01 > > C) > > #define GEN7_WA_FOR_GEN7_L3_CONTROL 0x3C47 > > FF8C > > diff --git a/drivers/gpu/drm/i915/intel_pm.c > > b/drivers/gpu/drm/i915/intel_pm.c > > index a6fd4dd..a9b7626 100644 > > --- a/drivers/gpu/drm/i915/intel_pm.c > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > @@ -6710,7 +6710,8 @@ static void > > broadwell_init_clock_gating(struct drm_device *dev) > > */ > > misccpctl = I915_READ(GEN7_MISCCPCTL); > > I915_WRITE(GEN7_MISCCPCTL, misccpctl & > > ~GEN7_DOP_CLOCK_GATE_ENABLE); > > - I915_WRITE(GEN8_L3SQCREG1, BDW_WA_L3SQCREG1_DEFAULT); > > + I915_WRITE(GEN8_L3SQCREG1, L3_GENERAL_PRIO_CREDITS(30) | > > + L3_HIGH_PRIO_CREDITS(2)); > > /* > > * Wait at least 100 clocks before re-enabling clock > > gating. See > > * the definition of L3SQCREG1 in BSpec.
On ti, 2016-04-26 at 12:03 +0300, Imre Deak wrote: > On ti, 2016-04-26 at 11:11 +0300, Mika Kuoppala wrote: > > Imre Deak <imre.deak@intel.com> writes: > > > > > [ text/plain ] > > > No need for hard-coding the register value, the corresponding > > > fields are > > > defined properly in BSpec. > > > > > > No functional change. > > > > > > Signed-off-by: Imre Deak <imre.deak@intel.com> > > > --- > > > drivers/gpu/drm/i915/i915_reg.h | 3 ++- > > > drivers/gpu/drm/i915/intel_pm.c | 3 ++- > > > 2 files changed, 4 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > > > b/drivers/gpu/drm/i915/i915_reg.h > > > index c21b71c..0cb2e17 100644 > > > --- a/drivers/gpu/drm/i915/i915_reg.h > > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > > @@ -6073,7 +6073,8 @@ enum skl_disp_power_wells { > > > #define VLV_B0_WA_L3SQCREG1_VALUE 0x00D30000 > > > > > > #define GEN8_L3SQCREG1 _MMIO(0xB100 > > > ) > > > -#define BDW_WA_L3SQCREG1_DEFAULT 0x784000 > > > +#define L3_GENERAL_PRIO_CREDITS(x) (((x) >> 1) << > > > 19) > > > +#define L3_HIGH_PRIO_CREDITS(x) (((x) >> 1) << 14) > > > > > > > The values listed for CHV it seems that the macros would produce > > one less than intended. For example chv 34 -> 0b10010 > > Yes, Ville noticed this too. I assumed the same formula as used on the > other platforms (BDW, SKL, BXT) and haven't noticed the difference wrt. > CHV. > > According to Ville's tests using the formula in the spec (and setting > the new credits 30, 2) leads to a hang. So my suspicion is that it's a Sorry, I meant setting the new credits 38, 2. > bug in the spec, I will file a change request there. > > --Imre > > > > > -Mika > > > > > > > #define GEN7_L3CNTLREG1 _MMIO(0xB01 > > > C) > > > #define GEN7_WA_FOR_GEN7_L3_CONTROL 0x3C47 > > > FF8C > > > diff --git a/drivers/gpu/drm/i915/intel_pm.c > > > b/drivers/gpu/drm/i915/intel_pm.c > > > index a6fd4dd..a9b7626 100644 > > > --- a/drivers/gpu/drm/i915/intel_pm.c > > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > > @@ -6710,7 +6710,8 @@ static void > > > broadwell_init_clock_gating(struct drm_device *dev) > > > */ > > > misccpctl = I915_READ(GEN7_MISCCPCTL); > > > I915_WRITE(GEN7_MISCCPCTL, misccpctl & > > > ~GEN7_DOP_CLOCK_GATE_ENABLE); > > > - I915_WRITE(GEN8_L3SQCREG1, BDW_WA_L3SQCREG1_DEFAULT); > > > + I915_WRITE(GEN8_L3SQCREG1, L3_GENERAL_PRIO_CREDITS(30) | > > > + L3_HIGH_PRIO_CREDITS(2)); > > > /* > > > * Wait at least 100 clocks before re-enabling clock > > > gating. See > > > * the definition of L3SQCREG1 in BSpec. > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Mon, Apr 25, 2016 at 03:38:06PM +0300, Imre Deak wrote: > No need for hard-coding the register value, the corresponding fields are > defined properly in BSpec. > > No functional change. > > Signed-off-by: Imre Deak <imre.deak@intel.com> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/i915/i915_reg.h | 3 ++- > drivers/gpu/drm/i915/intel_pm.c | 3 ++- > 2 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index c21b71c..0cb2e17 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -6073,7 +6073,8 @@ enum skl_disp_power_wells { > #define VLV_B0_WA_L3SQCREG1_VALUE 0x00D30000 > > #define GEN8_L3SQCREG1 _MMIO(0xB100) > -#define BDW_WA_L3SQCREG1_DEFAULT 0x784000 > +#define L3_GENERAL_PRIO_CREDITS(x) (((x) >> 1) << 19) > +#define L3_HIGH_PRIO_CREDITS(x) (((x) >> 1) << 14) > > #define GEN7_L3CNTLREG1 _MMIO(0xB01C) > #define GEN7_WA_FOR_GEN7_L3_CONTROL 0x3C47FF8C > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index a6fd4dd..a9b7626 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -6710,7 +6710,8 @@ static void broadwell_init_clock_gating(struct drm_device *dev) > */ > misccpctl = I915_READ(GEN7_MISCCPCTL); > I915_WRITE(GEN7_MISCCPCTL, misccpctl & ~GEN7_DOP_CLOCK_GATE_ENABLE); > - I915_WRITE(GEN8_L3SQCREG1, BDW_WA_L3SQCREG1_DEFAULT); > + I915_WRITE(GEN8_L3SQCREG1, L3_GENERAL_PRIO_CREDITS(30) | > + L3_HIGH_PRIO_CREDITS(2)); > /* > * Wait at least 100 clocks before re-enabling clock gating. See > * the definition of L3SQCREG1 in BSpec. > -- > 2.5.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://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 c21b71c..0cb2e17 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -6073,7 +6073,8 @@ enum skl_disp_power_wells { #define VLV_B0_WA_L3SQCREG1_VALUE 0x00D30000 #define GEN8_L3SQCREG1 _MMIO(0xB100) -#define BDW_WA_L3SQCREG1_DEFAULT 0x784000 +#define L3_GENERAL_PRIO_CREDITS(x) (((x) >> 1) << 19) +#define L3_HIGH_PRIO_CREDITS(x) (((x) >> 1) << 14) #define GEN7_L3CNTLREG1 _MMIO(0xB01C) #define GEN7_WA_FOR_GEN7_L3_CONTROL 0x3C47FF8C diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index a6fd4dd..a9b7626 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -6710,7 +6710,8 @@ static void broadwell_init_clock_gating(struct drm_device *dev) */ misccpctl = I915_READ(GEN7_MISCCPCTL); I915_WRITE(GEN7_MISCCPCTL, misccpctl & ~GEN7_DOP_CLOCK_GATE_ENABLE); - I915_WRITE(GEN8_L3SQCREG1, BDW_WA_L3SQCREG1_DEFAULT); + I915_WRITE(GEN8_L3SQCREG1, L3_GENERAL_PRIO_CREDITS(30) | + L3_HIGH_PRIO_CREDITS(2)); /* * Wait at least 100 clocks before re-enabling clock gating. See * the definition of L3SQCREG1 in BSpec.
No need for hard-coding the register value, the corresponding fields are defined properly in BSpec. No functional change. Signed-off-by: Imre Deak <imre.deak@intel.com> --- drivers/gpu/drm/i915/i915_reg.h | 3 ++- drivers/gpu/drm/i915/intel_pm.c | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-)