[Intel-gfx] [PATCH 2/2] drm/i915/ibx: check port in infoframe_enabled

[Jesse Barnes]

On Thu, 20 Nov 2014 22:57:32 +0100
Daniel Vetter <daniel at ffwll.ch> wrote:

> On Thu, Nov 20, 2014 at 01:24:14PM -0800, Jesse Barnes wrote:
> > Just like on g4x we need to check the port enable bit here.
> > 
> > Signed-off-by: Jesse Barnes <jbarnes at virtuousgeek.org>
> > ---
> >  drivers/gpu/drm/i915/intel_hdmi.c | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
> > b/drivers/gpu/drm/i915/intel_hdmi.c index ec87333..cc48b51 100644
> > --- a/drivers/gpu/drm/i915/intel_hdmi.c
> > +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> > @@ -222,10 +222,15 @@ static bool ibx_infoframe_enabled(struct
> > drm_encoder *encoder) struct drm_device *dev = encoder->dev;
> >  	struct drm_i915_private *dev_priv = dev->dev_private;
> >  	struct intel_crtc *intel_crtc =
> > to_intel_crtc(encoder->crtc);
> > +	struct intel_digital_port *intel_dig_port =
> > enc_to_dig_port(encoder); int reg =
> > TVIDEO_DIP_CTL(intel_crtc->pipe); u32 val = I915_READ(reg);
> > +	u32 port = VIDEO_DIP_PORT(intel_dig_port->port);
> >  
> > -	return val & VIDEO_DIP_ENABLE;
> > +	if (port == (val & VIDEO_DIP_PORT_MASK))
> > +		return val & VIDEO_DIP_ENABLE;
> 
> I think the only thing that can go wrong is us or the bios making a
> complete mess out of the pipe->port assignment for infoframes here. So
> I've thought more of
> 
> if (val & VIDEO_DIP_ENABLE) {
> 	WARN_ON(port != (val & VIDEO_DIP_PORT_MASK));
> 	return true;
> }
> 
> return false;
> 
> I.e. just double-checking that everything is as expected. Otherwise
> we'd need to collect infoframes from all pipes I think. But since we
> don't have any code to clean up such a mess I hope it doesn't exist
> in the real world out there.

So in that case, we'll set the has_infoframes to true even if
infoframes are enabled on the wrong port, which means we might skip a
mode set later which would clean them up and set the right port...

I like the idea of the WARN though.

Jesse