Re: [PATCH] drm/i915: Don't mark PCH underrun reporting as diasabled for transcoder B/C on LPT-H

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

On Fri, Aug 05, 2016 at 10:22:48PM +0200, Daniel Vetter wrote:
> On Fri, Aug 05, 2016 at 08:00:17PM +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Marking PCH transcoder FIFO underrun reporting as disabled for
> > transcoder B/C on LPT-H will block us from enabling the south error
> > interrupt. So let's only mark transcoder A underrun reporting as
> > disabled initially.
> > 
> > This is a little tricky to hit since you need a machine with LPT-H, and
> > the BIOS must enable either pipe B or C at boot. Then i915 would mark
> > the "transcoder B/C" underrun reporting as disabled and never enable it
> > again, meaning south interrupts would never get enabled either. The only
> > other interrupt in there is actually the poison interrupt which, if we
> > could ever trigger it, would just result in a little error in dmesg.
> > 
> > Here's the resulting change in SDEIMR on my HSW when I boot it with
> > multiple displays attached:
> > - (0x000c4004): 0xf115ffff
> > + (0x000c4004): 0xf114ffff
> > 
> > My previous attempt [1] tried to fix this a little differently, but
> > Daniel requested I do this instead.
> > 
> > [1] https://lists.freedesktop.org/archives/intel-gfx/2015-November/081420.html
> > 
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Pushed to dinq. Thanks for the review.

> 
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 19 ++++++++++++++++++-
> >  1 file changed, 18 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 9cbf5431c1e3..888a52c64a26 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -15771,6 +15771,13 @@ static bool intel_encoder_has_connectors(struct intel_encoder *encoder)
> >  	return false;
> >  }
> >  
> > +static bool has_pch_trancoder(struct drm_i915_private *dev_priv,
> > +			      enum transcoder pch_transcoder)
> > +{
> > +	return HAS_PCH_IBX(dev_priv) || HAS_PCH_CPT(dev_priv) ||
> > +		(HAS_PCH_LPT_H(dev_priv) && pch_transcoder == TRANSCODER_A);
> > +}
> > +
> >  static void intel_sanitize_crtc(struct intel_crtc *crtc)
> >  {
> >  	struct drm_device *dev = crtc->base.dev;
> > @@ -15849,7 +15856,17 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc)
> >  		 * worst a fifo underrun happens which also sets this to false.
> >  		 */
> >  		crtc->cpu_fifo_underrun_disabled = true;
> > -		crtc->pch_fifo_underrun_disabled = true;
> > +		/*
> > +		 * We track the PCH trancoder underrun reporting state
> > +		 * within the crtc. With crtc for pipe A housing the underrun
> > +		 * reporting state for PCH transcoder A, crtc for pipe B housing
> > +		 * it for PCH transcoder B, etc. LPT-H has only PCH transcoder A,
> > +		 * and marking underrun reporting as disabled for the non-existing
> > +		 * PCH transcoders B and C would prevent enabling the south
> > +		 * error interrupt (see cpt_can_enable_serr_int()).
> > +		 */
> > +		if (has_pch_trancoder(dev_priv, (enum transcoder)crtc->pipe))
> > +			crtc->pch_fifo_underrun_disabled = true;
> >  	}
> >  }
> >  
> > -- 
> > 2.7.4
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

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