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

[Takashi Iwai <tiwai@suse.de>]

On Mon, 27 Feb 2017 22:27:58 +0100,
Rafael J. Wysocki wrote:
> 
> On Mon, Feb 27, 2017 at 3:40 PM, Takashi Iwai <tiwai@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@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.

The driver is available (not upstreamed, though), but it's not bound
due to _STA above.  That's why I'm interested in this thread.


thanks,

Takashi
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx