Re: [PATCH] drm/i915: Implement display w/a #1143

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Fri, Jan 19, 2018 at 12:07:47PM -0800, Rodrigo Vivi wrote:
> On Fri, Jan 19, 2018 at 06:45:49PM +0000, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Apparently SKL/KBL need some manual help to get the
> > programmed HDMI vswing to stick. Implement the relevant
> > workaround (display w/a #1143).
> > 
> > Note that the relevant chicken bits live in a transcoder register
> > even though the bits affect a specific DDI port rather than a
> > specific transcoder. Hence we must pick the correct transcoder
> > register instance based on the port rather than based on the
> > cpu_transcoder.
> > 
> > Also note that for completeness I included support for DDI A/E
> > in the code even though we never have HDMI on those ports.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h  |  8 ++++++--
> >  drivers/gpu/drm/i915/intel_ddi.c | 42 ++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 48 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 10e1269ad6af..2e6d0dc01dc7 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -7012,8 +7012,12 @@ enum {
> >  #define CHICKEN_TRANS_A         0x420c0
> >  #define CHICKEN_TRANS_B         0x420c4
> >  #define CHICKEN_TRANS(trans) _MMIO_TRANS(trans, CHICKEN_TRANS_A, CHICKEN_TRANS_B)
> > -#define PSR2_VSC_ENABLE_PROG_HEADER    (1<<12)
> > -#define PSR2_ADD_VERTICAL_LINE_COUNT   (1<<15)
> > +#define  DDI_TRAINING_OVERRIDE_ENABLE	(1<<19)
> > +#define  DDI_TRAINING_OVERRIDE_VALUE	(1<<18)
> > +#define  DDIE_TRAINING_OVERRIDE_ENABLE	(1<<17) /* CHICKEN_TRANS_A only */
> > +#define  DDIE_TRAINING_OVERRIDE_VALUE	(1<<16) /* CHICKEN_TRANS_A only */
> 
> Wa on Bspec doesnt mention port E, although it makes sense to have...
> maybe a WARN_ON if DDIE but not TRANS_A then?!

We pick the registers instance purely based on the port, so
there's no point in such a WARN_ON() as it would just be
asserting that we didn't mess up a part of the 
port_to_transcoder[] table.

> 
> > +#define  PSR2_ADD_VERTICAL_LINE_COUNT   (1<<15)
> > +#define  PSR2_VSC_ENABLE_PROG_HEADER    (1<<12)
> >  
> >  #define DISP_ARB_CTL	_MMIO(0x45000)
> >  #define  DISP_FBC_MEMORY_WAKE		(1<<31)
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> > index 6260a882fbe4..7b1ab003279f 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -2433,6 +2433,48 @@ static void intel_enable_ddi_hdmi(struct intel_encoder *encoder,
> >  					  crtc_state->hdmi_high_tmds_clock_ratio,
> >  					  crtc_state->hdmi_scrambling);
> >  
> > +	/* Display WA #1143: skl,kbl */
> > +	if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)) {
> 
> on KBL, but not on CFL?! Strange... Art?
> 
> > +		/*
> > +		 * For some reason these chicken bits have been
> > +		 * stuffed into a transcoder register, event though
> > +		 * the bits affect a specific DDI port rather than
> > +		 * a specific transcoder.
> > +		 */
> > +		static const enum transcoder port_to_transcoder[] = {
> > +			[PORT_A] = TRANSCODER_EDP,
> > +			[PORT_B] = TRANSCODER_A,
> > +			[PORT_C] = TRANSCODER_B,
> > +			[PORT_D] = TRANSCODER_C,
> > +			[PORT_E] = TRANSCODER_A,
> > +		};
> > +		enum transcoder transcoder = port_to_transcoder[port];
> > +		u32 val;
> > +
> > +		val = I915_READ(CHICKEN_TRANS(transcoder));
> > +
> > +		if (port == PORT_E)
> > +			val |= DDIE_TRAINING_OVERRIDE_ENABLE |
> > +				DDIE_TRAINING_OVERRIDE_VALUE;
> > +		else
> > +			val |= DDI_TRAINING_OVERRIDE_ENABLE |
> > +				DDI_TRAINING_OVERRIDE_VALUE;
> > +
> > +		I915_WRITE(CHICKEN_TRANS(transcoder), val);
> > +		POSTING_READ(CHICKEN_TRANS(transcoder));
> > +
> > +		udelay(1);
> > +
> > +		if (port == PORT_E)
> > +			val &= ~(DDIE_TRAINING_OVERRIDE_ENABLE |
> > +				 DDIE_TRAINING_OVERRIDE_VALUE);
> > +		else
> > +			val &= ~(DDI_TRAINING_OVERRIDE_ENABLE |
> > +				 DDI_TRAINING_OVERRIDE_VALUE);
> > +
> > +		I915_WRITE(CHICKEN_TRANS(transcoder), val);
> > +	}
> > +
> >  	/* In HDMI/DVI mode, the port width, and swing/emphasis values
> >  	 * are ignored so nothing special needs to be done besides
> >  	 * enabling the port.
> > -- 
> > 2.13.6
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx