[Intel-gfx] Fixing i915/opregion issues with firmware which lists more then 8 output devices

[Jani Nikula]

On Fri, 06 Nov 2015, Hans de Goede <hdegoede at redhat.com> wrote:
> Hi,
>
> On 06-11-15 11:19, Jani Nikula wrote:
>> On Thu, 05 Nov 2015, Hans de Goede <hdegoede at redhat.com> wrote:
>>> Hi,
>>>
>>> As discussed in the past, the i915 opregion code does not do the
>>> right thing wrt the CADL field when there are more then 8 outputs,
>>> this is causing issues on many different types of Asus laptops.
>>>
>>> This thread has details and a number of attempts to fix this:
>>>
>>> https://lkml.org/lkml/2014/2/11/1032
>>>
>>> This is impacting many users, here is an incomplete list of bug reports:
>>>
>>> https://bugzilla.kernel.org/show_bug.cgi?id=70241
>>> https://bugzilla.kernel.org/show_bug.cgi?id=88941
>>> https://bugzilla.redhat.com/show_bug.cgi?id=1144866
>>> https://bugzilla.redhat.com/show_bug.cgi?id=1277785
>>>
>>> And I'm pretty sure that is just the tip of the iceberg, some users
>>> have even analyzed their BIOS AML code and come up with an AML
>>> hack in an attempt to fix this:
>>>
>>> http://blog.yjwong.name/fixing-display-backlight-hotkeys-on-asus-n550jk/
>>>
>>> It would be really great of someone from Intel could step up and start
>>> working on a proper fix for this.
>>
>> Hans, thanks for bringing this up again.
>>
>> IIUC this would require something along the lines of:
>>
>> - Properly map acpi video bus children and DRM connectors in
>>    intel_didl_outputs(). This is likely the hardest part. Save the
>>    display ids in struct intel_connector.
>>
>> - Replace intel_setup_cadls() with functions to add/remove display ids
>>    from opregion CADL.
>>
>> - Call the above functions as part of the modeset sequence (either from
>>    intel_display.c or from the encoder hooks, whichever is best) to keep
>>    CADL up-to-date about active outputs. Here, getting DP MST right is
>>    probably the hard one.
>>
>> If the mapping goes wrong, and some id that used to be in CADL with the
>> current code gets dropped, this may regress. That's the biggest risk,
>> really.
>>
>> For the backlight/hotkey stuff, the minimal fix may be to keep the
>> stupid static initialization of the CADL in intel_setup_cadls() like we
>> have now, but ensuring the embedded displays are included in the list.
>
> This is what Aaron's first attempt at fixing this more or less did:
>
> https://lkml.org/lkml/2014/2/11/1032
>
> Unless someone actually steps up to do a proper fix for this *now*,
> maybe the i915 driver needs to incorporate a slightly revised / improved
> version of that patch as a work around for now ?

I dug into this a bit. (Well, more than just a bit. Friday night, doing
ACPI dumps and deciphering AML. What was I thinking?)

It seems that part of the problem is caused by

commit 3143751ff51a163b77f7efd389043e038f3e008e
Author: Zhang Rui <rui.zhang at intel.com>
Date:   Mon Mar 29 15:12:16 2010 +0800

    drm/i915: set DIDL using the ACPI video output device _ADR method return.

which apparently caters for buggy bioses. The relevant bug report is
[1].

IIUC, the graphics driver is supposed to set up the IDs (so that would
take care of the "hardest part" above). On a couple of modern machines
that I checked, the above commit causes i915 to ask the AML code using
_ADR, which looks at DIDL, which is what we're trying to initialize so
it's zero, and then AML pulls IDs out of thin air, also for devices that
don't really exist because AML doesn't know.

I'll probably submit a patch doing a revert of the above, if only to
test. But we'll still have to figure out something for the regressions
it probably causes for the buggy machines. *sigh*.

This just reinforces my opinion that adding hacks to fix some broken
machines is not worth it. The no regressions rule turns the hacks into a
nightmare when it breaks good hardware.


BR,
Jani.


[1] https://bugzilla.kernel.org/show_bug.cgi?id=15054


-- 
Jani Nikula, Intel Open Source Technology Center