diff mbox series

drm/i915/icl: Fix clockgating issue when using scalars

Message ID 20190315221838.22444-1-radhakrishna.sripada@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915/icl: Fix clockgating issue when using scalars | expand

Commit Message

Sripada, Radhakrishna March 15, 2019, 10:18 p.m. UTC
Fixes the clock-gating issue when pipe scaling is enabled.
(Lineage #2006604312)

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
Cc: Aditya Swarup <aditya.swarup@intel.com>
Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

Comments

Chris Wilson March 15, 2019, 10:23 p.m. UTC | #1
Scalars as opposed to vector instructions? EU clock gating issues with
certain shaders?

Itym scalers.
-Chris
Ville Syrjälä March 18, 2019, 1:30 p.m. UTC | #2
On Fri, Mar 15, 2019 at 03:18:38PM -0700, Radhakrishna Sripada wrote:
> Fixes the clock-gating issue when pipe scaling is enabled.
> (Lineage #2006604312)
> 
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
> Cc: Aditya Swarup <aditya.swarup@intel.com>
> Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 61acbaf2af75..97344cca89c4 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5193,9 +5193,17 @@ static int skl_update_scaler_plane(struct intel_crtc_state *crtc_state,
>  static void skylake_scaler_disable(struct intel_crtc *crtc)
>  {
>  	int i;
> +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> +	i915_reg_t reg = CLKGATE_DIS_PSL(crtc->pipe);
>  
>  	for (i = 0; i < crtc->num_scalers; i++)
>  		skl_detach_scaler(crtc, i);
> +
> +	/*
> +	 * Wa_2006604312:icl
> +	 */
> +	if (IS_ICELAKE(dev_priv))
> +		I915_WRITE(reg, I915_READ(reg) & ~DPFR_GATING_DIS);

The register doesn't appear to be double buffered so I don't think we
should be doing this here. Instead it should be be somewhere around the
pre/port plane update stuff.

>  }
>  
>  static void skylake_pfit_enable(const struct intel_crtc_state *crtc_state)
> @@ -5205,6 +5213,7 @@ static void skylake_pfit_enable(const struct intel_crtc_state *crtc_state)
>  	enum pipe pipe = crtc->pipe;
>  	const struct intel_crtc_scaler_state *scaler_state =
>  		&crtc_state->scaler_state;
> +	i915_reg_t reg = CLKGATE_DIS_PSL(pipe);
>  
>  	if (crtc_state->pch_pfit.enabled) {
>  		u16 uv_rgb_hphase, uv_rgb_vphase;
> @@ -5232,6 +5241,12 @@ static void skylake_pfit_enable(const struct intel_crtc_state *crtc_state)
>  			      PS_Y_PHASE(0) | PS_UV_RGB_PHASE(uv_rgb_hphase));
>  		I915_WRITE(SKL_PS_WIN_POS(pipe, id), crtc_state->pch_pfit.pos);
>  		I915_WRITE(SKL_PS_WIN_SZ(pipe, id), crtc_state->pch_pfit.size);
> +
> +		/*
> +		 * Wa_2006604312:icl
> +		*/
> +		if (IS_ICELAKE(dev_priv))
> +			I915_WRITE(reg, I915_READ(reg) | DPFR_GATING_DIS);

>  	}
>  }
>  
> @@ -5972,7 +5987,7 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
>  
>  	/* Display WA #1180: WaDisableScalarClockGating: glk, cnl */
>  	psl_clkgate_wa = (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) &&
> -			 pipe_config->pch_pfit.enabled;
> +			  pipe_config->pch_pfit.enabled;

Unrelated change.

>  	if (psl_clkgate_wa)
>  		glk_pipe_scaler_clock_gating_wa(dev_priv, pipe, true);
>  
> -- 
> 2.20.0.rc2.7.g965798d1f299
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Sripada, Radhakrishna March 18, 2019, 9:19 p.m. UTC | #3
On Fri, 2019-03-15 at 22:23 +0000, Chris Wilson wrote:
> Scalars as opposed to vector instructions? EU clock gating issues
> with
> certain shaders?
> 
Hi Chris,

The scalers mentioned for this WA around are specific to display
scaling. They are not related to EU clock gating.

-Radhakrishna(RK) Sripada
> Itym scalers.
> -Chris
Chris Wilson March 18, 2019, 9:22 p.m. UTC | #4
Quoting Sripada, Radhakrishna (2019-03-18 21:19:29)
> On Fri, 2019-03-15 at 22:23 +0000, Chris Wilson wrote:
> > Scalars as opposed to vector instructions? EU clock gating issues
> > with
> > certain shaders?
> > 
> Hi Chris,
> 
> The scalers mentioned for this WA around are specific to display
> scaling. They are not related to EU clock gating.

So now they are _scalers_!

I know, I was trying to point out the typo in the subject in what I
thought was a humorous overly reductive pun.
-Chris
Sripada, Radhakrishna March 18, 2019, 10:32 p.m. UTC | #5
On Mon, 2019-03-18 at 15:30 +0200, Ville Syrjälä wrote:
> On Fri, Mar 15, 2019 at 03:18:38PM -0700, Radhakrishna Sripada wrote:
> > Fixes the clock-gating issue when pipe scaling is enabled.
> > (Lineage #2006604312)
> > 
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
> > Cc: Aditya Swarup <aditya.swarup@intel.com>
> > Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com
> > >
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 17 ++++++++++++++++-
> >  1 file changed, 16 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index 61acbaf2af75..97344cca89c4 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -5193,9 +5193,17 @@ static int skl_update_scaler_plane(struct
> > intel_crtc_state *crtc_state,
> >  static void skylake_scaler_disable(struct intel_crtc *crtc)
> >  {
> >  	int i;
> > +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> > +	i915_reg_t reg = CLKGATE_DIS_PSL(crtc->pipe);
> >  
> >  	for (i = 0; i < crtc->num_scalers; i++)
> >  		skl_detach_scaler(crtc, i);
> > +
> > +	/*
> > +	 * Wa_2006604312:icl
> > +	 */
> > +	if (IS_ICELAKE(dev_priv))
> > +		I915_WRITE(reg, I915_READ(reg) & ~DPFR_GATING_DIS);
> 
> The register doesn't appear to be double buffered so I don't think we
> should be doing this here. Instead it should be be somewhere around
> the
> pre/port plane update stuff.
Sure would work on the lines of Display WA #827. Did not think of the
register not being double buffered. Thanks for pointing it out.
> 
> >  }
> >  
> >  static void skylake_pfit_enable(const struct intel_crtc_state
> > *crtc_state)
> > @@ -5205,6 +5213,7 @@ static void skylake_pfit_enable(const struct
> > intel_crtc_state *crtc_state)
> >  	enum pipe pipe = crtc->pipe;
> >  	const struct intel_crtc_scaler_state *scaler_state =
> >  		&crtc_state->scaler_state;
> > +	i915_reg_t reg = CLKGATE_DIS_PSL(pipe);
> >  
> >  	if (crtc_state->pch_pfit.enabled) {
> >  		u16 uv_rgb_hphase, uv_rgb_vphase;
> > @@ -5232,6 +5241,12 @@ static void skylake_pfit_enable(const struct
> > intel_crtc_state *crtc_state)
> >  			      PS_Y_PHASE(0) |
> > PS_UV_RGB_PHASE(uv_rgb_hphase));
> >  		I915_WRITE(SKL_PS_WIN_POS(pipe, id), crtc_state-
> > >pch_pfit.pos);
> >  		I915_WRITE(SKL_PS_WIN_SZ(pipe, id), crtc_state-
> > >pch_pfit.size);
> > +
> > +		/*
> > +		 * Wa_2006604312:icl
> > +		*/
> > +		if (IS_ICELAKE(dev_priv))
> > +			I915_WRITE(reg, I915_READ(reg) |
> > DPFR_GATING_DIS);
> >  	}
> >  }
> >  
> > @@ -5972,7 +5987,7 @@ static void haswell_crtc_enable(struct
> > intel_crtc_state *pipe_config,
> >  
> >  	/* Display WA #1180: WaDisableScalarClockGating: glk, cnl */
> >  	psl_clkgate_wa = (IS_GEMINILAKE(dev_priv) ||
> > IS_CANNONLAKE(dev_priv)) &&
> > -			 pipe_config->pch_pfit.enabled;
> > +			  pipe_config->pch_pfit.enabled;
> 
> Unrelated change.
My bad will omit in the next rev.

-Radhakrishna(RK) Sripada
> 
> >  	if (psl_clkgate_wa)
> >  		glk_pipe_scaler_clock_gating_wa(dev_priv, pipe, true);
> >  
> > -- 
> > 2.20.0.rc2.7.g965798d1f299
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
>
Sripada, Radhakrishna March 18, 2019, 10:37 p.m. UTC | #6
On Mon, 2019-03-18 at 21:22 +0000, Chris Wilson wrote:
> Quoting Sripada, Radhakrishna (2019-03-18 21:19:29)
> > On Fri, 2019-03-15 at 22:23 +0000, Chris Wilson wrote:
> > > Scalars as opposed to vector instructions? EU clock gating issues
> > > with
> > > certain shaders?
> > > 
> > 
> > Hi Chris,
> > 
> > The scalers mentioned for this WA around are specific to display
> > scaling. They are not related to EU clock gating.
> 
> So now they are _scalers_!
> 
> I know, I was trying to point out the typo in the subject in what I
> thought was a humorous overly reductive pun.
Good one!! With realigned perspective. Will fix it in next rev of the
patch.

-Radhakrishna(RK) Sripada
> -Chris
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 61acbaf2af75..97344cca89c4 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5193,9 +5193,17 @@  static int skl_update_scaler_plane(struct intel_crtc_state *crtc_state,
 static void skylake_scaler_disable(struct intel_crtc *crtc)
 {
 	int i;
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	i915_reg_t reg = CLKGATE_DIS_PSL(crtc->pipe);
 
 	for (i = 0; i < crtc->num_scalers; i++)
 		skl_detach_scaler(crtc, i);
+
+	/*
+	 * Wa_2006604312:icl
+	 */
+	if (IS_ICELAKE(dev_priv))
+		I915_WRITE(reg, I915_READ(reg) & ~DPFR_GATING_DIS);
 }
 
 static void skylake_pfit_enable(const struct intel_crtc_state *crtc_state)
@@ -5205,6 +5213,7 @@  static void skylake_pfit_enable(const struct intel_crtc_state *crtc_state)
 	enum pipe pipe = crtc->pipe;
 	const struct intel_crtc_scaler_state *scaler_state =
 		&crtc_state->scaler_state;
+	i915_reg_t reg = CLKGATE_DIS_PSL(pipe);
 
 	if (crtc_state->pch_pfit.enabled) {
 		u16 uv_rgb_hphase, uv_rgb_vphase;
@@ -5232,6 +5241,12 @@  static void skylake_pfit_enable(const struct intel_crtc_state *crtc_state)
 			      PS_Y_PHASE(0) | PS_UV_RGB_PHASE(uv_rgb_hphase));
 		I915_WRITE(SKL_PS_WIN_POS(pipe, id), crtc_state->pch_pfit.pos);
 		I915_WRITE(SKL_PS_WIN_SZ(pipe, id), crtc_state->pch_pfit.size);
+
+		/*
+		 * Wa_2006604312:icl
+		*/
+		if (IS_ICELAKE(dev_priv))
+			I915_WRITE(reg, I915_READ(reg) | DPFR_GATING_DIS);
 	}
 }
 
@@ -5972,7 +5987,7 @@  static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
 
 	/* Display WA #1180: WaDisableScalarClockGating: glk, cnl */
 	psl_clkgate_wa = (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) &&
-			 pipe_config->pch_pfit.enabled;
+			  pipe_config->pch_pfit.enabled;
 	if (psl_clkgate_wa)
 		glk_pipe_scaler_clock_gating_wa(dev_priv, pipe, true);