diff mbox

[4/4] drm/i915/bxt: Clean up bxt_init_clock_gating

Message ID 1435583252-29608-4-git-send-email-nicholas.hoath@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Nick Hoath June 29, 2015, 1:07 p.m. UTC
Add stepping check for A0 workarounds, and remove the associated
FIXME tags.
Split out unrelated WAs for later condition checking.

v2: Fixed format (PeterL)
v3: Corrected stepping check for WaDisableSDEUnitClockGating
    - Ignoring comment, following hardware spec instead. (ChrisH)
    Added description for TILECTL setting (JonB)

Cc: Peter Lawthers <peter.lawthers@intel.com>
Cc: Chris Harris <chris.harris@intel.com>
Cc: Jon Bloomfield <jon.bloomfield@intel.com>
Signed-off-by: Nick Hoath <nicholas.hoath@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

Comments

Mika Kuoppala June 29, 2015, 2:29 p.m. UTC | #1
Nick Hoath <nicholas.hoath@intel.com> writes:

> Add stepping check for A0 workarounds, and remove the associated
> FIXME tags.
> Split out unrelated WAs for later condition checking.
>
> v2: Fixed format (PeterL)
> v3: Corrected stepping check for WaDisableSDEUnitClockGating
>     - Ignoring comment, following hardware spec instead. (ChrisH)
>     Added description for TILECTL setting (JonB)
>
> Cc: Peter Lawthers <peter.lawthers@intel.com>
> Cc: Chris Harris <chris.harris@intel.com>
> Cc: Jon Bloomfield <jon.bloomfield@intel.com>
> Signed-off-by: Nick Hoath <nicholas.hoath@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 26ef146..86a4ced 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -115,18 +115,24 @@ static void bxt_init_clock_gating(struct drm_device *dev)
>  
>  	gen9_init_clock_gating(dev);
>  
> +	/* WaDisableSDEUnitClockGating:bxt */
> +	I915_WRITE(GEN8_UCGCTL6, I915_READ(GEN8_UCGCTL6) |
> +		   GEN8_SDEUNIT_CLOCK_GATE_DISABLE);
> +
>  	/*
>  	 * FIXME:
> -	 * GEN8_SDEUNIT_CLOCK_GATE_DISABLE applies on A0 only.
>  	 * GEN8_HDCUNIT_CLOCK_GATE_DISABLE_HDCREQ applies on 3x6 GT SKUs only.
>  	 */
> -	 /* WaDisableSDEUnitClockGating:bxt */
>  	I915_WRITE(GEN8_UCGCTL6, I915_READ(GEN8_UCGCTL6) |
> -		   GEN8_SDEUNIT_CLOCK_GATE_DISABLE |
>  		   GEN8_HDCUNIT_CLOCK_GATE_DISABLE_HDCREQ);
>  

I guess you decided not to combine the writes due to FIXME.

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>


> -	/* FIXME: apply on A0 only */
> -	I915_WRITE(TILECTL, I915_READ(TILECTL) | TILECTL_TLBPF);
> +	if (INTEL_REVID(dev) == BXT_REVID_A0) {
> +		/*
> +		 * Hardware specification requires this bit to be
> +		 * set to 1 for A0
> +		 */
> +		I915_WRITE(TILECTL, I915_READ(TILECTL) | TILECTL_TLBPF);
> +	}
>  }
>  
>  static void i915_pineview_get_mem_freq(struct drm_device *dev)
> -- 
> 2.1.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Shuang He June 30, 2015, 2:56 a.m. UTC | #2
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6665
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
ILK                                  302/302              302/302
SNB                                  312/316              312/316
IVB                                  343/343              343/343
BYT                 -2              287/287              285/287
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
*BYT  igt@gem_partial_pwrite_pread@reads      PASS(1)      FAIL(1)
*BYT  igt@gem_partial_pwrite_pread@reads-display      PASS(1)      FAIL(1)
Note: You need to pay more attention to line start with '*'
Nick Hoath Sept. 7, 2015, 1:55 p.m. UTC | #3
On 29/06/2015 15:29, Mika Kuoppala wrote:
> Nick Hoath <nicholas.hoath@intel.com> writes:
>
>> Add stepping check for A0 workarounds, and remove the associated
>> FIXME tags.
>> Split out unrelated WAs for later condition checking.
>>
>> v2: Fixed format (PeterL)
>> v3: Corrected stepping check for WaDisableSDEUnitClockGating
>>      - Ignoring comment, following hardware spec instead. (ChrisH)
>>      Added description for TILECTL setting (JonB)
>>
>> Cc: Peter Lawthers <peter.lawthers@intel.com>
>> Cc: Chris Harris <chris.harris@intel.com>
>> Cc: Jon Bloomfield <jon.bloomfield@intel.com>
>> Signed-off-by: Nick Hoath <nicholas.hoath@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_pm.c | 16 +++++++++++-----
>>   1 file changed, 11 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>> index 26ef146..86a4ced 100644
>> --- a/drivers/gpu/drm/i915/intel_pm.c
>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>> @@ -115,18 +115,24 @@ static void bxt_init_clock_gating(struct drm_device *dev)
>>
>>   	gen9_init_clock_gating(dev);
>>
>> +	/* WaDisableSDEUnitClockGating:bxt */
>> +	I915_WRITE(GEN8_UCGCTL6, I915_READ(GEN8_UCGCTL6) |
>> +		   GEN8_SDEUNIT_CLOCK_GATE_DISABLE);
>> +
>>   	/*
>>   	 * FIXME:
>> -	 * GEN8_SDEUNIT_CLOCK_GATE_DISABLE applies on A0 only.
>>   	 * GEN8_HDCUNIT_CLOCK_GATE_DISABLE_HDCREQ applies on 3x6 GT SKUs only.
>>   	 */
>> -	 /* WaDisableSDEUnitClockGating:bxt */
>>   	I915_WRITE(GEN8_UCGCTL6, I915_READ(GEN8_UCGCTL6) |
>> -		   GEN8_SDEUNIT_CLOCK_GATE_DISABLE |
>>   		   GEN8_HDCUNIT_CLOCK_GATE_DISABLE_HDCREQ);
>>
>
> I guess you decided not to combine the writes due to FIXME.
>
> Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>

Looks like this one has fallen through the cracks & not been merged...

>
>
>> -	/* FIXME: apply on A0 only */
>> -	I915_WRITE(TILECTL, I915_READ(TILECTL) | TILECTL_TLBPF);
>> +	if (INTEL_REVID(dev) == BXT_REVID_A0) {
>> +		/*
>> +		 * Hardware specification requires this bit to be
>> +		 * set to 1 for A0
>> +		 */
>> +		I915_WRITE(TILECTL, I915_READ(TILECTL) | TILECTL_TLBPF);
>> +	}
>>   }
>>
>>   static void i915_pineview_get_mem_freq(struct drm_device *dev)
>> --
>> 2.1.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Daniel Vetter Sept. 7, 2015, 4:19 p.m. UTC | #4
On Mon, Sep 07, 2015 at 02:55:08PM +0100, Nick Hoath wrote:
> On 29/06/2015 15:29, Mika Kuoppala wrote:
> >Nick Hoath <nicholas.hoath@intel.com> writes:
> >
> >>Add stepping check for A0 workarounds, and remove the associated
> >>FIXME tags.
> >>Split out unrelated WAs for later condition checking.
> >>
> >>v2: Fixed format (PeterL)
> >>v3: Corrected stepping check for WaDisableSDEUnitClockGating
> >>     - Ignoring comment, following hardware spec instead. (ChrisH)
> >>     Added description for TILECTL setting (JonB)
> >>
> >>Cc: Peter Lawthers <peter.lawthers@intel.com>
> >>Cc: Chris Harris <chris.harris@intel.com>
> >>Cc: Jon Bloomfield <jon.bloomfield@intel.com>
> >>Signed-off-by: Nick Hoath <nicholas.hoath@intel.com>
> >>---
> >>  drivers/gpu/drm/i915/intel_pm.c | 16 +++++++++++-----
> >>  1 file changed, 11 insertions(+), 5 deletions(-)
> >>
> >>diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> >>index 26ef146..86a4ced 100644
> >>--- a/drivers/gpu/drm/i915/intel_pm.c
> >>+++ b/drivers/gpu/drm/i915/intel_pm.c
> >>@@ -115,18 +115,24 @@ static void bxt_init_clock_gating(struct drm_device *dev)
> >>
> >>  	gen9_init_clock_gating(dev);
> >>
> >>+	/* WaDisableSDEUnitClockGating:bxt */
> >>+	I915_WRITE(GEN8_UCGCTL6, I915_READ(GEN8_UCGCTL6) |
> >>+		   GEN8_SDEUNIT_CLOCK_GATE_DISABLE);
> >>+
> >>  	/*
> >>  	 * FIXME:
> >>-	 * GEN8_SDEUNIT_CLOCK_GATE_DISABLE applies on A0 only.
> >>  	 * GEN8_HDCUNIT_CLOCK_GATE_DISABLE_HDCREQ applies on 3x6 GT SKUs only.
> >>  	 */
> >>-	 /* WaDisableSDEUnitClockGating:bxt */
> >>  	I915_WRITE(GEN8_UCGCTL6, I915_READ(GEN8_UCGCTL6) |
> >>-		   GEN8_SDEUNIT_CLOCK_GATE_DISABLE |
> >>  		   GEN8_HDCUNIT_CLOCK_GATE_DISABLE_HDCREQ);
> >>
> >
> >I guess you decided not to combine the writes due to FIXME.
> >
> >Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
> 
> Looks like this one has fallen through the cracks & not been merged...

Thanks for the ping, applied to dinq.
-Daniel
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 26ef146..86a4ced 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -115,18 +115,24 @@  static void bxt_init_clock_gating(struct drm_device *dev)
 
 	gen9_init_clock_gating(dev);
 
+	/* WaDisableSDEUnitClockGating:bxt */
+	I915_WRITE(GEN8_UCGCTL6, I915_READ(GEN8_UCGCTL6) |
+		   GEN8_SDEUNIT_CLOCK_GATE_DISABLE);
+
 	/*
 	 * FIXME:
-	 * GEN8_SDEUNIT_CLOCK_GATE_DISABLE applies on A0 only.
 	 * GEN8_HDCUNIT_CLOCK_GATE_DISABLE_HDCREQ applies on 3x6 GT SKUs only.
 	 */
-	 /* WaDisableSDEUnitClockGating:bxt */
 	I915_WRITE(GEN8_UCGCTL6, I915_READ(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);
+	if (INTEL_REVID(dev) == BXT_REVID_A0) {
+		/*
+		 * Hardware specification requires this bit to be
+		 * set to 1 for A0
+		 */
+		I915_WRITE(TILECTL, I915_READ(TILECTL) | TILECTL_TLBPF);
+	}
 }
 
 static void i915_pineview_get_mem_freq(struct drm_device *dev)