Re: [PATCH] drm/i915/bxt: Enable existing gen9 harware workarounds for Broxton

[Imre Deak <imre.deak@intel.com>]

On ti, 2015-04-07 at 11:45 +0100, Nick Hoath wrote:
> Signed-off-by: Nick Hoath <nicholas.hoath@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h         |  8 +++++
>  drivers/gpu/drm/i915/intel_pm.c         |  2 ++
>  drivers/gpu/drm/i915/intel_ringbuffer.c | 53 +++++++++++++++++++--------------
>  3 files changed, 41 insertions(+), 22 deletions(-)

The patch needs to be rebased on the latest -nightly and your
"[PATCH 0/3] drm/i915/bxt: add workarounds" patchset. It'd be best to
resend it as part of that patchset.

> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 7e1a0fd9..91eef06 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -5339,6 +5339,7 @@ enum skl_disp_power_wells {
>  
>  #define GEN8_L3SQCREG4				0xb118
>  #define  GEN8_LQSC_RO_PERF_DIS			(1<<27)
> +#define  GEN8_PIPELINE_FLUSH_COHERENT_LINES	(1<<21)
>  
>  /* GEN8 chicken */
>  #define HDC_CHICKEN0				0x7300
> @@ -7324,4 +7325,11 @@ enum skl_disp_power_wells {
>  #define _PALETTE_A (dev_priv->info.display_mmio_offset + 0xa000)
>  #define _PALETTE_B (dev_priv->info.display_mmio_offset + 0xa800)
>  
> +/*
> + * Chicken Registers for LLC/eLLC Hot Spotting Avoidance Mode for
> + * 3D/Media Compressed Resources
> + */
> +#define GEN9_CHICKEN_MISC1_REG				0x42080
> +#define GEN9_CHICKEN_MISC1_NEW_HASH_ENABLE		(1<<15)

Please move this next to CHICKEN_PAR1_1 and use the standard two space
indent before the flag name. Also no need for the comment, it doesn't
fully describe the register and we don't have similar comments for other
registers either.

> +
>  #endif /* _I915_REG_H_ */
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index c3c473d..bbb5d64 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -96,6 +96,8 @@ static void skl_init_clock_gating(struct drm_device *dev)
>  
>  static void bxt_init_clock_gating(struct drm_device *dev)
>  {
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +
>  	gen9_init_clock_gating(dev);
>  
>  	/* WaVSRefCountFullforceMissDisable:bxt */
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 44c7b99..741bdfa 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -916,7 +916,7 @@ static int gen9_init_workarounds(struct intel_engine_cs *ring)
>  	struct drm_device *dev = ring->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  
> -	/* WaDisablePartialInstShootdown:skl */
> +	/* WaDisablePartialInstShootdown:skl,bxt */

It would be better to split out all the parts that only update the WA
comments into one separate patch.

>  	WA_SET_BIT_MASKED(GEN8_ROW_CHICKEN,
>  			  PARTIAL_INSTRUCTION_SHOOTDOWN_DISABLE);
>  
> @@ -924,45 +924,43 @@ static int gen9_init_workarounds(struct intel_engine_cs *ring)
>  	WA_SET_BIT_MASKED(HALF_SLICE_CHICKEN3,
>  			  GEN9_DISABLE_OCL_OOB_SUPPRESS_LOGIC);
>  
> -	if (INTEL_REVID(dev) == SKL_REVID_A0 ||
> -	    INTEL_REVID(dev) == SKL_REVID_B0) {
> -		/* WaDisableDgMirrorFixInHalfSliceChicken5:skl */
> +	if (
> +	    (IS_SKYLAKE(dev) && (INTEL_REVID(dev) == SKL_REVID_A0 ||
> +	    INTEL_REVID(dev) == SKL_REVID_B0)) ||
> +	    (IS_BROXTON(dev) && INTEL_REVID(dev) == BXT_REVID_A0)
> +	    ) {

To align with the coding style please remove the whitespace after the
opening '(' and before the closing ')'.

I assume we should use the GT/Display device stepping, not the SOC
stepping. Accordingly the above check should be '< BXT_REVID_B0', since
steppings 0,1,2 all map to A0 GT/Display device stepping.

Please split out the enabling for each workaround into a separate patch.

> +		/* WaDisableDgMirrorFixInHalfSliceChicken5:skl,bxt */
>  		WA_CLR_BIT_MASKED(GEN9_HALF_SLICE_CHICKEN5,
>  				  GEN9_DG_MIRROR_FIX_ENABLE);
>  	}
>  
> -	if (IS_SKYLAKE(dev) && INTEL_REVID(dev) <= SKL_REVID_B0) {
> -		/* WaSetDisablePixMaskCammingAndRhwoInCommonSliceChicken:skl */
> +	if (
> +	    (IS_SKYLAKE(dev) && INTEL_REVID(dev) <= SKL_REVID_B0) ||
> +	    (IS_BROXTON(dev) && INTEL_REVID(dev) == BXT_REVID_A0)

As above '< BXT_REVID_B0'

> +	   ) {
> +		/* WaSetDisablePixMaskCammingAndRhwoInCommonSliceChicken:skl,bxt */
>  		WA_SET_BIT_MASKED(GEN7_COMMON_SLICE_CHICKEN1,
>  				  GEN9_RHWO_OPTIMIZATION_DISABLE);
>  		WA_SET_BIT_MASKED(GEN9_SLICE_COMMON_ECO_CHICKEN0,
>  				  DISABLE_PIXEL_MASK_CAMMING);
>  	}
>  
> -	if (INTEL_REVID(dev) >= SKL_REVID_C0) {
> -		/* WaEnableYV12BugFixInHalfSliceChicken7:skl */
> +	if (
> +	    (IS_SKYLAKE(dev) && INTEL_REVID(dev) >= SKL_REVID_C0) ||
> +	    (IS_BROXTON(dev))
> +	   ) {
> +		/* WaEnableYV12BugFixInHalfSliceChicken7:skl,bxt */
>  		WA_SET_BIT_MASKED(GEN9_HALF_SLICE_CHICKEN7,
>  				  GEN9_ENABLE_YV12_BUGFIX);
>  	}
>  
> -	if (INTEL_REVID(dev) <= SKL_REVID_D0) {
> -		/*
> -		 *Use Force Non-Coherent whenever executing a 3D context. This
> -		 * is a workaround for a possible hang in the unlikely event
> -		 * a TLB invalidation occurs during a PSD flush.
> -		 */
> -		/* WaForceEnableNonCoherent:skl */
> -		WA_SET_BIT_MASKED(HDC_CHICKEN0,
> -				  HDC_FORCE_NON_COHERENT);
> -	}

This should also be in a separate patch.

> -
> -	/* Wa4x4STCOptimizationDisable:skl */
> +	/* Wa4x4STCOptimizationDisable:skl,bxt */
>  	WA_SET_BIT_MASKED(CACHE_MODE_1, GEN8_4x4_STC_OPTIMIZATION_DISABLE);
>  
> -	/* WaDisablePartialResolveInVc:skl */
> +	/* WaDisablePartialResolveInVc:skl,bxt */
>  	WA_SET_BIT_MASKED(CACHE_MODE_1, GEN9_PARTIAL_RESOLVE_IN_VC_DISABLE);
>  
> -	/* WaCcsTlbPrefetchDisable:skl */
> +	/* WaCcsTlbPrefetchDisable:skl,bxt */
>  	WA_CLR_BIT_MASKED(GEN9_HALF_SLICE_CHICKEN5,
>  			  GEN9_CCS_TLB_PREFETCH_ENABLE);
>  
> @@ -1024,6 +1022,17 @@ static int skl_init_workarounds(struct intel_engine_cs *ring)
>  		WA_SET_BIT_MASKED(HIZ_CHICKEN,
>  				  BDW_HIZ_POWER_COMPILER_CLOCK_GATING_DISABLE);
>  
> +	if (INTEL_REVID(dev) <= SKL_REVID_D0) {
> +		/*
> +		 *Use Force Non-Coherent whenever executing a 3D context. This
> +		 * is a workaround for a possible hang in the unlikely event
> +		 * a TLB invalidation occurs during a PSD flush.
> +		 */
> +		/* WaForceEnableNonCoherent:skl */
> +		WA_SET_BIT_MASKED(HDC_CHICKEN0,
> +				  HDC_FORCE_NON_COHERENT);
> +	}
> +
>  	return skl_tune_iz_hashing(ring);
>  }
>  


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