Re: [PATCH] drm/i915: RMW register cycles considered evil

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Mon, Jul 06, 2015 at 02:42:02PM +0200, Daniel Vetter wrote:
> Especially for workarounds which is stuff that's almost impossible to
> verify: The initial state from the firmware on boot-up and after
> resume could be different, which will hide bugs when we do an RMW
> cycle.

If you're really worried about that then we should then explicitly
initialize all the registers that might affect stuff.

For a bunch of GT registers we could just do a GPU reset at driver
init. That that won't help with UCGCTL and such.

I'm also worried that if we don't use RMWs for early parts, the hardware
folks may still change the default for some ofhte other bits, and then
we end up clobbering those.

> 
> Hence never do them, and if it's required we need a special mask.
> 
> Cc: Damien Lespiau <damien.lespiau@intel.com>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Nick Hoath <nicholas.hoath@intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 166ae51f5a5b..565f78d6a21d 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -57,7 +57,7 @@ static void gen9_init_clock_gating(struct drm_device *dev)
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  
>  	/* WaEnableLbsSlaRetryTimerDecrement:skl */
> -	I915_WRITE(BDW_SCRATCH1, I915_READ(BDW_SCRATCH1) |
> +	I915_WRITE(BDW_SCRATCH1,
>  		   GEN9_LBS_SLA_RETRY_TIMER_DECREMENT_ENABLE);
>  }
>  
> @@ -72,18 +72,18 @@ static void skl_init_clock_gating(struct drm_device *dev)
>  		 * WaDisableSDEUnitClockGating:skl
>  		 * WaSetGAPSunitClckGateDisable:skl
>  		 */
> -		I915_WRITE(GEN8_UCGCTL6, I915_READ(GEN8_UCGCTL6) |
> +		I915_WRITE(GEN8_UCGCTL6,
>  			   GEN8_GAPSUNIT_CLOCK_GATE_DISABLE |
>  			   GEN8_SDEUNIT_CLOCK_GATE_DISABLE);
>  
>  		/* WaDisableVFUnitClockGating:skl */
> -		I915_WRITE(GEN6_UCGCTL2, I915_READ(GEN6_UCGCTL2) |
> +		I915_WRITE(GEN6_UCGCTL2,
>  			   GEN6_VFUNIT_CLOCK_GATE_DISABLE);
>  	}
>  
>  	if (INTEL_REVID(dev) <= SKL_REVID_D0) {
>  		/* WaDisableHDCInvalidation:skl */
> -		I915_WRITE(GAM_ECOCHK, I915_READ(GAM_ECOCHK) |
> +		I915_WRITE(GAM_ECOCHK,
>  			   BDW_DISABLE_HDC_INVALIDATION);
>  
>  		/* WaDisableChickenBitTSGBarrierAckForFFSliceCS:skl */
> @@ -93,7 +93,7 @@ static void skl_init_clock_gating(struct drm_device *dev)
>  
>  	if (INTEL_REVID(dev) <= SKL_REVID_E0)
>  		/* WaDisableLSQCROPERFforOCL:skl */
> -		I915_WRITE(GEN8_L3SQCREG4, I915_READ(GEN8_L3SQCREG4) |
> +		I915_WRITE(GEN8_L3SQCREG4,
>  			   GEN8_LQSC_RO_PERF_DIS);
>  }
>  
> @@ -109,12 +109,12 @@ static void bxt_init_clock_gating(struct drm_device *dev)
>  	 * GEN8_HDCUNIT_CLOCK_GATE_DISABLE_HDCREQ applies on 3x6 GT SKUs only.
>  	 */
>  	 /* WaDisableSDEUnitClockGating:bxt */
> -	I915_WRITE(GEN8_UCGCTL6, I915_READ(GEN8_UCGCTL6) |
> +	I915_WRITE(GEN8_UCGCTL6,
>  		   GEN8_SDEUNIT_CLOCK_GATE_DISABLE |
>  		   GEN8_HDCUNIT_CLOCK_GATE_DISABLE_HDCREQ);
>  
>  	/* FIXME: apply on A0 only */
> -	I915_WRITE(TILECTL, I915_READ(TILECTL) | TILECTL_TLBPF);
> +	I915_WRITE(TILECTL, TILECTL_TLBPF);
>  }
>  
>  static void i915_pineview_get_mem_freq(struct drm_device *dev)
> -- 
> 2.1.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx