Re: [PATCH 2/4] drm/i915: Clean up L3 SQC register field definitions

From: Imre Deak <imre.deak@intel.com>
To: Mika Kuoppala <mika.kuoppala@linux.intel.com>,
	intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 2/4] drm/i915: Clean up L3 SQC register field	definitions
Date: Tue, 26 Apr 2016 12:03:16 +0300	[thread overview]
Message-ID: <1461661396.18080.5.camel@intel.com> (raw)
In-Reply-To: <87eg9spz5t.fsf@gaia.fi.intel.com>

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.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx