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

From: Damien Lespiau <damien.lespiau@intel.com>
To: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Daniel Vetter <daniel.vetter@intel.com>,
	Intel Graphics Development <intel-gfx@lists.freedesktop.org>
Subject: Re: [PATCH] drm/i915: RMW register cycles considered evil
Date: Mon, 6 Jul 2015 13:46:19 +0100	[thread overview]
Message-ID: <20150706124619.GD19207@strange.amr.corp.intel.com> (raw)
In-Reply-To: <1436186522-7834-1-git-send-email-daniel.vetter@ffwll.ch>

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.
> 
> 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>

Eeek. Let's take the problem the other way around: have you verified
it's OK to zero all those other fields?

-- 
Damien

> ---
>  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