[Intel-gfx] [PATCH] ACPI / bus: Introduce a list of ids for "always present" devices

[Rafael J. Wysocki]

On Mon, Feb 27, 2017 at 3:40 PM, Takashi Iwai <tiwai at suse.de> wrote:
> On Mon, 27 Feb 2017 15:25:32 +0100,
> Hans de Goede wrote:
>>
>> Hi,
>>
>> On 27-02-17 14:30, Rafael J. Wysocki wrote:
>> > +Mika & Andy
>> >
>> > On Saturday, February 25, 2017 07:23:28 PM Hans de Goede wrote:
>> >> Several cherrytrail devices (all of which ship with windows 10) hide the
>> >> lpss pwm controller in ACPI, typically the _STA method looks like this:
>> >>
>> >>     Method (_STA, 0, NotSerialized)  // _STA: Status
>> >>     {
>> >>         If (OSID == One)
>> >>         {
>> >>             Return (Zero)
>> >>         }
>> >>
>> >>         Return (0x0F)
>> >>     }
>> >>
>> >> Where OSID is some dark magic seen in all cherrytrail ACPI tables making
>> >> the machine behave differently depending on which OS it *thinks* it is
>> >> booting, this gets set in a number of ways which we cannot control, on
>> >> some newer machines it simple hardcoded to "One" aka win10.
>> >>
>> >> This causes the PWM controller to get hidden, which means Linux cannot
>> >> control the backlight level on cht based tablets / laptops.
>> >>
>> >> Since loading the driver for this does no harm (the only in kernel user
>> >> of it is the i915 driver, which will only use it when it needs it), this
>> >> commit makes acpi_bus_get_status() always set status to ACPI_STA_DEFAULT
>> >> for the 80862288 device, fixing the lack of backlight control.
>> >>
>> >> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
>> >> ---
>> >>  drivers/acpi/bus.c | 25 +++++++++++++++++++++++++
>> >>  1 file changed, 25 insertions(+)
>> >>
>> >> diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
>> >> index 95855cb..483d4d0 100644
>> >> --- a/drivers/acpi/bus.c
>> >> +++ b/drivers/acpi/bus.c
>> >> @@ -109,11 +109,36 @@ acpi_status acpi_bus_get_status_handle(acpi_handle handle,
>> >>    return status;
>> >>  }
>> >>
>> >> +/*
>> >> + * Some ACPI devices are hidden (status == 0x0) in recent BIOS-es because
>> >> + * some recent windows drivers bind to one device but poke at multiple
>> >> + * devices at the same time, so the others get hidden.
>> >> + * We work around this by always reporting ACPI_STA_DEFAULT for these
>> >> + * devices. Note this MUST only be done for devices where this is safe.
>> >> + */
>> >> +static const struct acpi_device_id always_present_device_ids[] = {
>> >> +  /*
>> >> +   * Cherrytrail pwm directly poked by GPU driver in win10,
>> >> +   * but Linux uses a separate pwm driver, harmless if not used.
>> >> +   */
>> >> +  { "80862288", },
>> >> +  { }
>> >> +};
>> >> +
>> >>  int acpi_bus_get_status(struct acpi_device *device)
>> >>  {
>> >>    acpi_status status;
>> >>    unsigned long long sta;
>> >>
>> >> +  /* acpi_match_device_ids checks status, so start with default */
>> >> +  acpi_set_device_status(device, ACPI_STA_DEFAULT);
>> >
>> > This shouldn't be done in a "get" routine.
>>
>> With this you mean the acpi_match_device_ids() check I assume ?
>> (acpi_bus_get_status already calls acpi_set_device_status())
>>
>> > Ideally, a scan handler should do that or similar.
>>
>> The problem is that drivers/acpi/scan.c: acpi_bus_attach()
>> calls acpi_bus_get_status() and if it does not set
>> the status to present acpi_bus_attach() exits without bothering
>> with attaching scan handlers, which is why I ended up doing this
>> here.
>
> Oh, this is interesting, please let me join to the party.
>
> We've hit a similar problem, but for other one: namely, it's INT0002
> that is found on a few CHT devices.  It's never bound properly by a
> similar reason, where _STA is always zero on Linux:
>
>             Method (_STA, 0, NotSerialized)  // _STA: Status
>             {
>                 If (SOCS <= 0x04)
>                 {
>                     Return (0x0F)
>                 }
>                 Else
>                 {
>                     Return (Zero)
>                 }
>             }
>
> The device is supposed to be a "virtual GPIO" stuff, and the driver
> hasn't been upstreamed from Intel.  Due to the lack of this driver
> (and it's binding), the machine gets spurious IRQ#9 after the PM
> resume, and it locks up at the second time of PM.

Well, the solution here seems to be to acquire the missing driver
instead of adding quirks to the ACPI core.

Thanks,
Rafael