[Intel-gfx] A wrong DDI encoder override from HDMI to DP at hotplug

[Dave Airlie]

On 19 November 2015 at 02:00, Takashi Iwai <tiwai at suse.de> wrote:
> On Wed, 18 Nov 2015 16:38:03 +0100,
> Ville Syrjälä wrote:
>>
>> On Wed, Nov 18, 2015 at 04:23:06PM +0100, Takashi Iwai wrote:
>> > Hi,
>> >
>> > currently a DDI port may register both DP and HDMI and it shares the
>> > same encoder.  The bug we've got a report is about this encoder type:
>> > namely, a machine using DDI port D for HDMI is screwed up because the
>> > encoder is switched to DP suddently.  The details are found in:
>> >   http://bugzilla.opensuse.org/show_bug.cgi?id=955190
>> >
>> > The problem happens in intel_dp_hpd_pulse().  Since the machine
>> > declares both DP and HDMI, the driver registers this callback.  And at
>> > a hotplug event, the function changes the encoder type like:
>> >
>> >     if (intel_dig_port->base.type != INTEL_OUTPUT_EDP)
>> >             intel_dig_port->base.type = INTEL_OUTPUT_DISPLAYPORT;
>> >
>> > After this point, the encoder is handled as DP although the same HDMI
>> > monitor is connected.
>> >
>> > Changing this to exclude INTEL_OUTPUT_HDMI makes the thing working in
>> > the bug report above, but I'm not sure what's the right fix.
>> >
>> > Any suggestions?
>>
>> This has been causing all sorts of troubles for years. I've suggested
>> several times that we should just split the encoder into two, like we
>> have for older platforms, but no one has taken the bait thus far.
>>
>> This particular piece of code to change the encoder type in the
>> hpd_pulse hook came about with the mst code drop from Dave IIRC. At
>> least I never figured out what it's trying to do since -ENOCOMMENT.
>
> Yeah, this was introduced in the commit
>
>   0e32b39ceed665bfa4a77a4bc307b6652b991632
>     drm/i915: add DP 1.2 MST support (v0.7)
>
> so the problem exists for quite some time, since 3.17 kernel.
>
> I guess the patch just overlooked the DDI use case with HDMI.
> Maybe the code was meant like below?
>
>         if (intel_dig_port->base.type == INTEL_OUTPUT_UNKNOWN ||
>             intel_dig_port->base.type == INTEL_OUTPUT_DP_MST)
>                 intel_dig_port->base.type = INTEL_OUTPUT_DISPLAYPORT;
>

That code was copied from intel_dp_detect, which does that check.

But if the HDMI hdp is going via that path then yes it should check
more than EDP.

Dave.