diff mbox series

[1/1] drm/i915: Apply Wa_1406680159 as a clk_gating workaround

Message ID 20200422123037.25414-2-radhakrishna.sripada@intel.com (mailing list archive)
State New, archived
Headers show
Series Apply Wa_1406680159 as a clk_gating workaround | expand

Commit Message

Sripada, Radhakrishna April 22, 2020, 12:30 p.m. UTC
The workaround is moved from render engine context to intel_pm
clk gating functions like the previous platforms.

Closes: https://gitlab.freedesktop.org/drm/intel/issues/1222
Fixes: fb899dd8ea9c ("drm/i915: Apply Wa_1406680159:icl,ehl as an engine workaround")
Cc: Matt Atwood <matthew.s.atwood@intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_workarounds.c | 5 -----
 drivers/gpu/drm/i915/intel_pm.c             | 4 ++++
 2 files changed, 4 insertions(+), 5 deletions(-)

Comments

Chris Wilson April 22, 2020, 5:12 p.m. UTC | #1
Quoting Radhakrishna Sripada (2020-04-22 13:30:37)
> The workaround is moved from render engine context to intel_pm
> clk gating functions like the previous platforms.

Why?

> Closes: https://gitlab.freedesktop.org/drm/intel/issues/1222

Oh so you mean so that you don't actually have to test anything.

Rather than ignore any test results, why don't you work on adding the
missing verification?

> Fixes: fb899dd8ea9c ("drm/i915: Apply Wa_1406680159:icl,ehl as an engine workaround")
> Cc: Matt Atwood <matthew.s.atwood@intel.com>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_workarounds.c | 5 -----
>  drivers/gpu/drm/i915/intel_pm.c             | 4 ++++
>  2 files changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> index adddc5c93b48..a9a75e9b670d 100644
> --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> @@ -1486,11 +1486,6 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal)
>                 wa_write_or(wal, UNSLICE_UNIT_LEVEL_CLKGATE2,
>                             PSDUNIT_CLKGATE_DIS);
>  
> -               /* Wa_1406680159:icl,ehl */
> -               wa_write_or(wal,
> -                           SUBSLICE_UNIT_LEVEL_CLKGATE,
> -                           GWUNIT_CLKGATE_DIS);
> -
>                 /*
>                  * Wa_1408767742:icl[a2..forever],ehl[all]
>                  * Wa_1605460711:icl[a0..c0]
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 6f40bfee7304..19293ac001e2 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -6859,6 +6859,10 @@ static void icl_init_clock_gating(struct drm_i915_private *dev_priv)
>         /*Wa_14010594013:icl, ehl */
>         intel_uncore_rmw(&dev_priv->uncore, GEN8_CHICKEN_DCPR_1,
>                          0, CNL_DELAY_PMRSP);
> +
> +       /* Wa_1406680159:icl,ehl */
> +       I915_WRITE(SUBSLICE_UNIT_LEVEL_CLKGATE,
> +                  I915_READ(SUBSLICE_UNIT_LEVEL_CLKGATE) | GWUNIT_CLKGATE_DIS);

And what is this?
-Chris
Matt Roper April 25, 2020, 12:27 a.m. UTC | #2
On Wed, Apr 22, 2020 at 05:30:37AM -0700, Radhakrishna Sripada wrote:
> The workaround is moved from render engine context to intel_pm
> clk gating functions like the previous platforms.

I don't think this is the right way to go.  There's some GT-related
stuff in the clock gating handlers for historical reasons, but we really
want to be moving that stuff out to the proper locations (gt, engine,
ctx workaround section).

In this case the workaround really is an engine workaround that we want
to re-apply on engine reset.  If we move it to the clock gating
function, I think we have some hacks that will still re-apply it on full
GPU reset, but I don't think it will get re-applied on engine resets.

This is a multicast register, so we should double check that we aren't
doing anything wrong with the read steering, just in case subslice 0 is
fused off on this silicon or whatever.  But assuming that looks correct,
I think we need to follow up with the hardware team more on this one to
find out whether it's really a legitimate workaround, whether readback
for this register is broken in general.


Matt

> 
> Closes: https://gitlab.freedesktop.org/drm/intel/issues/1222
> Fixes: fb899dd8ea9c ("drm/i915: Apply Wa_1406680159:icl,ehl as an engine workaround")
> Cc: Matt Atwood <matthew.s.atwood@intel.com>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_workarounds.c | 5 -----
>  drivers/gpu/drm/i915/intel_pm.c             | 4 ++++
>  2 files changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> index adddc5c93b48..a9a75e9b670d 100644
> --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> @@ -1486,11 +1486,6 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal)
>  		wa_write_or(wal, UNSLICE_UNIT_LEVEL_CLKGATE2,
>  			    PSDUNIT_CLKGATE_DIS);
>  
> -		/* Wa_1406680159:icl,ehl */
> -		wa_write_or(wal,
> -			    SUBSLICE_UNIT_LEVEL_CLKGATE,
> -			    GWUNIT_CLKGATE_DIS);
> -
>  		/*
>  		 * Wa_1408767742:icl[a2..forever],ehl[all]
>  		 * Wa_1605460711:icl[a0..c0]
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 6f40bfee7304..19293ac001e2 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -6859,6 +6859,10 @@ static void icl_init_clock_gating(struct drm_i915_private *dev_priv)
>  	/*Wa_14010594013:icl, ehl */
>  	intel_uncore_rmw(&dev_priv->uncore, GEN8_CHICKEN_DCPR_1,
>  			 0, CNL_DELAY_PMRSP);
> +
> +	/* Wa_1406680159:icl,ehl */
> +	I915_WRITE(SUBSLICE_UNIT_LEVEL_CLKGATE,
> +		   I915_READ(SUBSLICE_UNIT_LEVEL_CLKGATE) | GWUNIT_CLKGATE_DIS);
>  }
>  
>  static void tgl_init_clock_gating(struct drm_i915_private *dev_priv)
> -- 
> 2.20.1
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index adddc5c93b48..a9a75e9b670d 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -1486,11 +1486,6 @@  rcs_engine_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal)
 		wa_write_or(wal, UNSLICE_UNIT_LEVEL_CLKGATE2,
 			    PSDUNIT_CLKGATE_DIS);
 
-		/* Wa_1406680159:icl,ehl */
-		wa_write_or(wal,
-			    SUBSLICE_UNIT_LEVEL_CLKGATE,
-			    GWUNIT_CLKGATE_DIS);
-
 		/*
 		 * Wa_1408767742:icl[a2..forever],ehl[all]
 		 * Wa_1605460711:icl[a0..c0]
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 6f40bfee7304..19293ac001e2 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -6859,6 +6859,10 @@  static void icl_init_clock_gating(struct drm_i915_private *dev_priv)
 	/*Wa_14010594013:icl, ehl */
 	intel_uncore_rmw(&dev_priv->uncore, GEN8_CHICKEN_DCPR_1,
 			 0, CNL_DELAY_PMRSP);
+
+	/* Wa_1406680159:icl,ehl */
+	I915_WRITE(SUBSLICE_UNIT_LEVEL_CLKGATE,
+		   I915_READ(SUBSLICE_UNIT_LEVEL_CLKGATE) | GWUNIT_CLKGATE_DIS);
 }
 
 static void tgl_init_clock_gating(struct drm_i915_private *dev_priv)