[Intel-gfx] [PATCH] drm/i915: Don't override output type for DDI HDMI

[Takashi Iwai]

On Thu, 19 Nov 2015 17:04:20 +0100,
Takashi Iwai wrote:
> 
> On Thu, 19 Nov 2015 16:51:05 +0100,
> Daniel Vetter wrote:
> > 
> > On Thu, Nov 19, 2015 at 12:09:56PM +0100, Takashi Iwai wrote:
> > > Currently a DDI port may register the DP hotplug handler even though
> > > it's used with HDMI, and the DP HPD handler overrides the encoder
> > > type forcibly to DP.  This caused the inconsistency on a machine
> > > connected with a HDMI monitor; upon a hotplug event, the DDI port is
> > > suddenly switched to be handled as a DP although the same monitor is
> > > kept connected, and this leads to the erroneous blank output.
> > > 
> > > This patch papers over the bug by excluding the previous HDMI encoder
> > > type from this override.  This should be fixed more fundamentally,
> > > e.g. by moving the encoder type reset from the HPD or by having
> > > individual encoder objects for HDMI and DP.  But since the bug has
> > > been present for a long time (3.17), it's better to have a
> > > quick-n-dirty fix for now, and keep working on a cleaner fix.
> > > 
> > > Bugzilla: http://bugzilla.opensuse.org/show_bug.cgi?id=955190
> > > Fixes: 0e32b39ceed6 ('drm/i915: add DP 1.2 MST support (v0.7)')
> > > Cc: <stable at vger.kernel.org> # v3.17+
> > > Signed-off-by: Takashi Iwai <tiwai at suse.de>
> > 
> > If you plug in a hmid screen (using a level-shifter adapter), then a DP
> > screen, does still everything work?
>  
> That was my slight concern, too, although it seems working as long as
> I tested several HSW machines.  Maybe better to double-check.

I checked again some machines, and they all seem working.

Actually the encoder type is set again in each detection function
(intel_hdmi_detect(), intel_dp_detect()) no matter with or without my
workaround.  The problem happens when a HPD is triggered only to DP
while the HDMI is kept on.  Then intel_hdmi_detect() won't be called
so the wrongly overridden encoder type remains.

Why the override is still needed in intel_dp_hpd_pulse() isn't clear
to me, but at least it influences on the port power domain handling.
And my change won't affect in this regard.


Takashi

> 
> > We rely upon the detection code to correctly adjust the encoder type,
> > otherwise the subsequent modeset will pick the wrong code (even when
> > userspace asked the kernel to light up the right connector). I'm not sure
> > whether this is now broken ...
> 
> I guess the only case this might matter is MST, as the encoder
> override didn't exist before the MST support patch, so the non-MST
> case should keep working even with this change.
> 
> 
> thanks,
> 
> Takashi
> 
> > -Daniel
> > 
> > > ---
> > >  drivers/gpu/drm/i915/intel_dp.c | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > > index 09bdd94ca3ba..d34e64300d66 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > @@ -5153,7 +5153,8 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
> > >  	enum intel_display_power_domain power_domain;
> > >  	enum irqreturn ret = IRQ_NONE;
> > >  
> > > -	if (intel_dig_port->base.type != INTEL_OUTPUT_EDP)
> > > +	if (intel_dig_port->base.type != INTEL_OUTPUT_EDP &&
> > > +	    intel_dig_port->base.type != INTEL_OUTPUT_HDMI)
> > >  		intel_dig_port->base.type = INTEL_OUTPUT_DISPLAYPORT;
> > >  
> > >  	if (long_hpd && intel_dig_port->base.type == INTEL_OUTPUT_EDP) {
> > > -- 
> > > 2.6.3
> > > 
> > 
> > -- 
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
> >