Re: [PATCH v3 11/22] drm/i915/cnl: Move GT and Display workarounds from init_clock_gating

[Oscar Mateo <oscar.mateo@xxxxxxxxx>]

On 10/18/2017 05:44 AM, Ville Syrjälä wrote:
> On Fri, Oct 13, 2017 at 01:54:05PM -0700, Oscar Mateo wrote:
> > To their rightful place inside intel_workarounds.c
> >
> > Signed-off-by: Oscar Mateo <oscar.mateo@xxxxxxxxx>
> > Cc: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> > Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> > Cc: Mika Kuoppala <mika.kuoppala@xxxxxxxxxxxxxxx>
> > Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > ---
> >   drivers/gpu/drm/i915/intel_pm.c          | 32 +-------------------------------
> >   drivers/gpu/drm/i915/intel_workarounds.c | 26 +++++++++++++++++++++++++-
> >   2 files changed, 26 insertions(+), 32 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index e4b3904..3f92bed1 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -8509,36 +8509,6 @@ static void cnp_init_clock_gating(struct drm_i915_private *dev_priv)
> >   		   CNP_PWM_CGE_GATING_DISABLE);
> >   }
> >   
> > -static void cnl_init_clock_gating(struct drm_i915_private *dev_priv)
> > -{
> > -	u32 val;
> > -	cnp_init_clock_gating(dev_priv);
> > -
> > -	/* This is not an Wa. Enable for better image quality */
> > -	I915_WRITE(_3D_CHICKEN3,
> > -		   _MASKED_BIT_ENABLE(_3D_CHICKEN3_AA_LINE_QUALITY_FIX_ENABLE));
> > -
> > -	/* WaEnableChickenDCPR:cnl */
> > -	I915_WRITE(GEN8_CHICKEN_DCPR_1,
> > -		   I915_READ(GEN8_CHICKEN_DCPR_1) | MASK_WAKEMEM);
> > -
> > -	/* WaFbcWakeMemOn:cnl */
> > -	I915_WRITE(DISP_ARB_CTL, I915_READ(DISP_ARB_CTL) |
> > -		   DISP_FBC_MEMORY_WAKE);
> > -
> > -	/* WaSarbUnitClockGatingDisable:cnl (pre-prod) */
> > -	if (IS_CNL_REVID(dev_priv, CNL_REVID_A0, CNL_REVID_B0))
> > -		I915_WRITE(SLICE_UNIT_LEVEL_CLKGATE,
> > -			   I915_READ(SLICE_UNIT_LEVEL_CLKGATE) |
> > -			   SARBUNIT_CLKGATE_DIS);
> > -
> > -	/* Display WA #1133: WaFbcSkipSegments:cnl */
> > -	val = I915_READ(ILK_DPFC_CHICKEN);
> > -	val &= ~GLK_SKIP_SEG_COUNT_MASK;
> > -	val |= GLK_SKIP_SEG_EN | GLK_SKIP_SEG_COUNT(1);
> > -	I915_WRITE(ILK_DPFC_CHICKEN, val);
> > -}
> > -
> >   static void cfl_init_clock_gating(struct drm_i915_private *dev_priv)
> >   {
> >   	cnp_init_clock_gating(dev_priv);
> > @@ -9030,7 +9000,7 @@ static void nop_init_clock_gating(struct drm_i915_private *dev_priv)
> >   void intel_init_clock_gating_hooks(struct drm_i915_private *dev_priv)
> >   {
> >   	if (IS_CANNONLAKE(dev_priv))
> > -		dev_priv->display.init_clock_gating = cnl_init_clock_gating;
> > +		dev_priv->display.init_clock_gating = nop_init_clock_gating;
> >   	else if (IS_COFFEELAKE(dev_priv))
> >   		dev_priv->display.init_clock_gating = cfl_init_clock_gating;
> >   	else if (IS_SKYLAKE(dev_priv))
> > diff --git a/drivers/gpu/drm/i915/intel_workarounds.c b/drivers/gpu/drm/i915/intel_workarounds.c
> > index aea9391..222f45c 100644
> > --- a/drivers/gpu/drm/i915/intel_workarounds.c
> > +++ b/drivers/gpu/drm/i915/intel_workarounds.c
> > @@ -724,6 +724,10 @@ static int cfl_gt_workarounds_init(struct drm_i915_private *dev_priv)
> >   
> >   static int cnl_gt_workarounds_init(struct drm_i915_private *dev_priv)
> >   {
> > +	/* This is not an Wa. Enable for better image quality */
> > +	GT_WA_SET_BIT_MASKED(_3D_CHICKEN3,
> > +			     _3D_CHICKEN3_AA_LINE_QUALITY_FIX_ENABLE);
> > +
> >   	/* WaDisableI2mCycleOnWRPort:cnl (pre-prod) */
> >   	if (IS_CNL_REVID(dev_priv, CNL_REVID_B0, CNL_REVID_B0))
> >   		GT_WA_SET_BIT(GAMT_CHKN_BIT_REG,
> > @@ -856,6 +860,27 @@ static int cfl_display_workarounds_init(struct drm_i915_private *dev_priv)
> >   
> >   static int cnl_display_workarounds_init(struct drm_i915_private *dev_priv)
> >   {
> > +	if (HAS_PCH_CNP(dev_priv)) {
> > +		/* Wa #1181 */
> > +		DISPLAY_WA_SET_BIT(SOUTH_DSPCLK_GATE_D,
> > +				   CNP_PWM_CGE_GATING_DISABLE);
> > +	}
> Where did this come from?

From the call to cnp_init_clock_gating. I can carry the whole function 
over, if you prefer it that way?

> > +
> > +	/* WaEnableChickenDCPR:cnl */
> > +	DISPLAY_WA_SET_BIT(GEN8_CHICKEN_DCPR_1, MASK_WAKEMEM);
> > +
> > +	/* WaFbcWakeMemOn:cnl */
> > +	DISPLAY_WA_SET_BIT(DISP_ARB_CTL, DISP_FBC_MEMORY_WAKE);
> > +
> > +	/* WaSarbUnitClockGatingDisable:cnl (pre-prod) */
> > +	if (IS_CNL_REVID(dev_priv, CNL_REVID_A0, CNL_REVID_B0))
> > +		DISPLAY_WA_SET_BIT(SLICE_UNIT_LEVEL_CLKGATE,
> > +				   SARBUNIT_CLKGATE_DIS);
> That doesn't look like display to me.

you are right. I'll reclassify it (plus some others I have found that 
were incorrectly classified as display). Thanks!

> > +
> > +	/* Display WA #1133: WaFbcSkipSegments:cnl */
> > +	DISPLAY_WA_SET_FIELD(ILK_DPFC_CHICKEN, GLK_SKIP_SEG_COUNT_MASK,
> > +			     GLK_SKIP_SEG_EN | GLK_SKIP_SEG_COUNT(1));
> > +
> >   	return 0;
> >   }
> >   
> > @@ -863,7 +888,6 @@ static int display_workarounds_init(struct drm_i915_private *dev_priv)
> >   {
> >   	int err;
> >   
> > -	DISPLAY_WA_SET_BIT(_MMIO(0), 0); /* shut up gcc temporarily */
> >   	dev_priv->workarounds.display_wa_count = 0;
> >   
> >   	if (INTEL_GEN(dev_priv) < 8)
> > --
> > 1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx