[Intel-gfx] [PATCH] drm/i915: Display WA #1185 WaDisableDARBFClkGating:cnl, glk

[Rodrigo Vivi]

On Fri, Nov 10, 2017 at 08:13:44PM +0000, Ville Syrjälä wrote:
> On Thu, Nov 09, 2017 at 02:26:32PM -0800, Rodrigo Vivi wrote:
> > Display is not sending a PMRsp when a PMReq is received
> > at the same time that all planes are turned off.
> > State machine in the dcprunit is stuck in the WAIT4DONE
> > state which means that there is no fill_done.
> > 
> > WA: disable arbiter clock gating, set bit [27] of 0x46530
> > 
> > v2: As Ville pointed out, based on the description the issue
> >     can happen when disabling the planes, similar to
> >     WaRsPkgCStateDisplayPMReq:hsw
> >     Also description of the issue was updated on commit
> >     message to make it more clear that we need this
> >     earlier.
> 
> I guess we don't know exactly if this has the same ordering requirements
> as WaRsPkgCStateDisplayPMReq:hsw. But playing it safe can't hurt I
> suppose.
> 
> > 
> > Cc: Radhakrishna Sripada <radhakrishna.sripada at intel.com>
> > Cc: Imre Deak <imre.deak at intel.com>
> > Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h      |  1 +
> >  drivers/gpu/drm/i915/intel_display.c | 24 +++++++++++++++---------
> >  2 files changed, 16 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 6ef33422f762..fc8c5f8260f6 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -3819,6 +3819,7 @@ enum {
> >   * GEN9 clock gating regs
> >   */
> >  #define GEN9_CLKGATE_DIS_0		_MMIO(0x46530)
> > +#define   DARBF_GATING_DIS		(1 << 27)
> >  #define   PWM2_GATING_DIS		(1 << 14)
> >  #define   PWM1_GATING_DIS		(1 << 13)
> >  
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 84817ccc5305..a038610b66cc 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -15080,6 +15080,20 @@ get_encoder_power_domains(struct drm_i915_private *dev_priv)
> >  	}
> >  }
> >  
> > +/* System hang if this isn't done before disabling all planes! */
> > +static void intel_early_display_was(struct drm_i915_private *dev_priv)
> > +{
> > +	/* Display WA #1185 WaDisableDARBFClkGating:cnl,glk */
> > +	if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
> 
> Bspec says GLK isn't affected.

Bspec refers to GLK as Gen10 display...
so GEN10:All is also GLK...
but if you look 2 columns ahead you see this:
GEN:BUG:1947072 [CNL, GLK]

> 
> > +		I915_WRITE(GEN9_CLKGATE_DIS_0, I915_READ(GEN9_CLKGATE_DIS_0) |
> > +			   DARBF_GATING_DIS);
> > +
> > +	/* WaRsPkgCStateDisplayPMReq:hsw */
> 
> Please keep the comment about the hsw system hangs here. The next guy to
> touch this code is probably not going to be aware of it, or may simply
> have forgotten it already.

makes sense...
should I keep the one in the function comment as well? and duplicate
the comment on the new one or since we are not that sure we just leave
the comment close to the hsw one?

> 
> With that those addresses this is
> Reviewed-by: Ville Syrjälä <ville.syrjala at linux.intel.com>

thanks

> 
> > +	if (IS_HASWELL(dev_priv))
> > +		I915_WRITE(CHICKEN_PAR1_1,
> > +			   I915_READ(CHICKEN_PAR1_1) | FORCE_ARB_IDLE_PLANES);
> > +}
> > +
> >  /* Scan out the current hw modeset state,
> >   * and sanitizes it to the current state
> >   */
> > @@ -15093,15 +15107,7 @@ intel_modeset_setup_hw_state(struct drm_device *dev,
> >  	struct intel_encoder *encoder;
> >  	int i;
> >  
> > -	if (IS_HASWELL(dev_priv)) {
> > -		/*
> > -		 * WaRsPkgCStateDisplayPMReq:hsw
> > -		 * System hang if this isn't done before disabling all planes!
> > -		 */
> > -		I915_WRITE(CHICKEN_PAR1_1,
> > -			   I915_READ(CHICKEN_PAR1_1) | FORCE_ARB_IDLE_PLANES);
> > -	}
> > -
> > +	intel_early_display_was(dev_priv);
> >  	intel_modeset_readout_hw_state(dev);
> >  
> >  	/* HW state is read out, now we need to sanitize this mess. */
> > -- 
> > 2.13.6
> 
> -- 
> Ville Syrjälä
> Intel OTC