diff mbox

[v2] drm/i915: fix for WaDisableDopClockGating:bdw

Message ID 20170212133252.20990-1-robert@sixbynine.org (mailing list archive)
State New, archived
Headers show

Commit Message

Robert Bragg Feb. 12, 2017, 1:32 p.m. UTC
This workaround for BDW was incomplete as it also requires EUTC clock
gating to be disabled via UCGCTL1.

v2: read modify write UCGTL1 in broadwell_init_clock_gating (Ville)

Signed-off-by: Robert Bragg <robert@sixbynine.org>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c         | 8 ++++++++
 drivers/gpu/drm/i915/intel_ringbuffer.c | 6 +++++-
 2 files changed, 13 insertions(+), 1 deletion(-)

Comments

Ville Syrjälä Feb. 13, 2017, 2:28 p.m. UTC | #1
On Sun, Feb 12, 2017 at 01:32:52PM +0000, Robert Bragg wrote:
> This workaround for BDW was incomplete as it also requires EUTC clock
> gating to be disabled via UCGCTL1.
> 
> v2: read modify write UCGTL1 in broadwell_init_clock_gating (Ville)
> 
> Signed-off-by: Robert Bragg <robert@sixbynine.org>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Do we know if this fixes something real? And if so, do we want cc:stable?

> ---
>  drivers/gpu/drm/i915/intel_pm.c         | 8 ++++++++
>  drivers/gpu/drm/i915/intel_ringbuffer.c | 6 +++++-
>  2 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index c0b0f5a4b9f1..3c13be8985f1 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -7229,6 +7229,14 @@ static void broadwell_init_clock_gating(struct drm_i915_private *dev_priv)
>  		   | KVM_CONFIG_CHANGE_NOTIFICATION_SELECT);
>  
>  	lpt_init_clock_gating(dev_priv);
> +
> +	/* WaDisableDopClockGating:bdw
> +	 *
> +	 * Also see the CHICKEN2 write in bdw_init_workarounds() to disable DOP
> +	 * clock gating.
> +	 */
> +	I915_WRITE(GEN6_UCGCTL1,
> +		   I915_READ(GEN6_UCGCTL1) | GEN6_EU_TCUNIT_CLOCK_GATE_DISABLE);
>  }
>  
>  static void haswell_init_clock_gating(struct drm_i915_private *dev_priv)
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index d3d1e64f2498..d93d5f8f02d8 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -825,7 +825,11 @@ static int bdw_init_workarounds(struct intel_engine_cs *engine)
>  	/* WaDisableThreadStallDopClockGating:bdw (pre-production) */
>  	WA_SET_BIT_MASKED(GEN8_ROW_CHICKEN, STALL_DOP_GATING_DISABLE);
>  
> -	/* WaDisableDopClockGating:bdw */
> +	/* WaDisableDopClockGating:bdw
> +	 *
> +	 * Also see the related UCGTCL1 write in broadwell_init_clock_gating()
> +	 * to disable EUTC clock gating.
> +	 */
>  	WA_SET_BIT_MASKED(GEN7_ROW_CHICKEN2,
>  			  DOP_CLOCK_GATING_DISABLE);
>  
> -- 
> 2.11.1
Robert Bragg Feb. 13, 2017, 2:54 p.m. UTC | #2
On Mon, Feb 13, 2017 at 2:28 PM, Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
> On Sun, Feb 12, 2017 at 01:32:52PM +0000, Robert Bragg wrote:
>> This workaround for BDW was incomplete as it also requires EUTC clock
>> gating to be disabled via UCGCTL1.
>>
>> v2: read modify write UCGTL1 in broadwell_init_clock_gating (Ville)
>>
>> Signed-off-by: Robert Bragg <robert@sixbynine.org>
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Do we know if this fixes something real? And if so, do we want cc:stable?

I looked at this as I was reviewing what workarounds I need to deal
with for gen8+ OA unit enabling. I had previously been applying this
WA within i915_perf.c with a long standing comment that I should cross
reference with VPG to see which platforms really needed it. So I just
noticed that i915 is now aiming to handle the WA but it looked
incomplete. I can't say I've observed the issue myself but my
understanding is that it may affect OA metrics in some cases.

Br,
- Robert

>
>> ---
>>  drivers/gpu/drm/i915/intel_pm.c         | 8 ++++++++
>>  drivers/gpu/drm/i915/intel_ringbuffer.c | 6 +++++-
>>  2 files changed, 13 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>> index c0b0f5a4b9f1..3c13be8985f1 100644
>> --- a/drivers/gpu/drm/i915/intel_pm.c
>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>> @@ -7229,6 +7229,14 @@ static void broadwell_init_clock_gating(struct drm_i915_private *dev_priv)
>>                  | KVM_CONFIG_CHANGE_NOTIFICATION_SELECT);
>>
>>       lpt_init_clock_gating(dev_priv);
>> +
>> +     /* WaDisableDopClockGating:bdw
>> +      *
>> +      * Also see the CHICKEN2 write in bdw_init_workarounds() to disable DOP
>> +      * clock gating.
>> +      */
>> +     I915_WRITE(GEN6_UCGCTL1,
>> +                I915_READ(GEN6_UCGCTL1) | GEN6_EU_TCUNIT_CLOCK_GATE_DISABLE);
>>  }
>>
>>  static void haswell_init_clock_gating(struct drm_i915_private *dev_priv)
>> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
>> index d3d1e64f2498..d93d5f8f02d8 100644
>> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
>> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
>> @@ -825,7 +825,11 @@ static int bdw_init_workarounds(struct intel_engine_cs *engine)
>>       /* WaDisableThreadStallDopClockGating:bdw (pre-production) */
>>       WA_SET_BIT_MASKED(GEN8_ROW_CHICKEN, STALL_DOP_GATING_DISABLE);
>>
>> -     /* WaDisableDopClockGating:bdw */
>> +     /* WaDisableDopClockGating:bdw
>> +      *
>> +      * Also see the related UCGTCL1 write in broadwell_init_clock_gating()
>> +      * to disable EUTC clock gating.
>> +      */
>>       WA_SET_BIT_MASKED(GEN7_ROW_CHICKEN2,
>>                         DOP_CLOCK_GATING_DISABLE);
>>
>> --
>> 2.11.1
>
> --
> Ville Syrjälä
> Intel OTC
Ville Syrjälä Feb. 14, 2017, 8:32 p.m. UTC | #3
On Mon, Feb 13, 2017 at 02:54:01PM +0000, Robert Bragg wrote:
> On Mon, Feb 13, 2017 at 2:28 PM, Ville Syrjälä
> <ville.syrjala@linux.intel.com> wrote:
> > On Sun, Feb 12, 2017 at 01:32:52PM +0000, Robert Bragg wrote:
> >> This workaround for BDW was incomplete as it also requires EUTC clock
> >> gating to be disabled via UCGCTL1.
> >>
> >> v2: read modify write UCGTL1 in broadwell_init_clock_gating (Ville)
> >>
> >> Signed-off-by: Robert Bragg <robert@sixbynine.org>
> >> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Do we know if this fixes something real? And if so, do we want cc:stable?
> 
> I looked at this as I was reviewing what workarounds I need to deal
> with for gen8+ OA unit enabling. I had previously been applying this
> WA within i915_perf.c with a long standing comment that I should cross
> reference with VPG to see which platforms really needed it. So I just
> noticed that i915 is now aiming to handle the WA but it looked
> incomplete. I can't say I've observed the issue myself but my
> understanding is that it may affect OA metrics in some cases.

OK. Sounds like it's not stable material at this point so I've gone
and pushed it to dinq. Thanks for the patch.

> 
> Br,
> - Robert
> 
> >
> >> ---
> >>  drivers/gpu/drm/i915/intel_pm.c         | 8 ++++++++
> >>  drivers/gpu/drm/i915/intel_ringbuffer.c | 6 +++++-
> >>  2 files changed, 13 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> >> index c0b0f5a4b9f1..3c13be8985f1 100644
> >> --- a/drivers/gpu/drm/i915/intel_pm.c
> >> +++ b/drivers/gpu/drm/i915/intel_pm.c
> >> @@ -7229,6 +7229,14 @@ static void broadwell_init_clock_gating(struct drm_i915_private *dev_priv)
> >>                  | KVM_CONFIG_CHANGE_NOTIFICATION_SELECT);
> >>
> >>       lpt_init_clock_gating(dev_priv);
> >> +
> >> +     /* WaDisableDopClockGating:bdw
> >> +      *
> >> +      * Also see the CHICKEN2 write in bdw_init_workarounds() to disable DOP
> >> +      * clock gating.
> >> +      */
> >> +     I915_WRITE(GEN6_UCGCTL1,
> >> +                I915_READ(GEN6_UCGCTL1) | GEN6_EU_TCUNIT_CLOCK_GATE_DISABLE);
> >>  }
> >>
> >>  static void haswell_init_clock_gating(struct drm_i915_private *dev_priv)
> >> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> >> index d3d1e64f2498..d93d5f8f02d8 100644
> >> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> >> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> >> @@ -825,7 +825,11 @@ static int bdw_init_workarounds(struct intel_engine_cs *engine)
> >>       /* WaDisableThreadStallDopClockGating:bdw (pre-production) */
> >>       WA_SET_BIT_MASKED(GEN8_ROW_CHICKEN, STALL_DOP_GATING_DISABLE);
> >>
> >> -     /* WaDisableDopClockGating:bdw */
> >> +     /* WaDisableDopClockGating:bdw
> >> +      *
> >> +      * Also see the related UCGTCL1 write in broadwell_init_clock_gating()
> >> +      * to disable EUTC clock gating.
> >> +      */
> >>       WA_SET_BIT_MASKED(GEN7_ROW_CHICKEN2,
> >>                         DOP_CLOCK_GATING_DISABLE);
> >>
> >> --
> >> 2.11.1
> >
> > --
> > Ville Syrjälä
> > Intel OTC
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index c0b0f5a4b9f1..3c13be8985f1 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -7229,6 +7229,14 @@  static void broadwell_init_clock_gating(struct drm_i915_private *dev_priv)
 		   | KVM_CONFIG_CHANGE_NOTIFICATION_SELECT);
 
 	lpt_init_clock_gating(dev_priv);
+
+	/* WaDisableDopClockGating:bdw
+	 *
+	 * Also see the CHICKEN2 write in bdw_init_workarounds() to disable DOP
+	 * clock gating.
+	 */
+	I915_WRITE(GEN6_UCGCTL1,
+		   I915_READ(GEN6_UCGCTL1) | GEN6_EU_TCUNIT_CLOCK_GATE_DISABLE);
 }
 
 static void haswell_init_clock_gating(struct drm_i915_private *dev_priv)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index d3d1e64f2498..d93d5f8f02d8 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -825,7 +825,11 @@  static int bdw_init_workarounds(struct intel_engine_cs *engine)
 	/* WaDisableThreadStallDopClockGating:bdw (pre-production) */
 	WA_SET_BIT_MASKED(GEN8_ROW_CHICKEN, STALL_DOP_GATING_DISABLE);
 
-	/* WaDisableDopClockGating:bdw */
+	/* WaDisableDopClockGating:bdw
+	 *
+	 * Also see the related UCGTCL1 write in broadwell_init_clock_gating()
+	 * to disable EUTC clock gating.
+	 */
 	WA_SET_BIT_MASKED(GEN7_ROW_CHICKEN2,
 			  DOP_CLOCK_GATING_DISABLE);