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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On 27-02-17 22:49, Rafael J. Wysocki wrote:
On Mon, Feb 27, 2017 at 10:29 PM, Hans de Goede <hdegoede@xxxxxxxxxx> wrote:
Hi,


On 27-02-17 22:25, Rafael J. Wysocki wrote:

On Mon, Feb 27, 2017 at 3:25 PM, Hans de Goede <hdegoede@xxxxxxxxxx>
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@xxxxxxxxxx>
---
 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())


Yes, the device ID check.

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.


Fair enough.

Two problems with this approach.

One is that it doesn't prevent _STA from being evaluated as
acpi_bus_get_status_handle() is called directly from a couple of
places.

Yes I noticed that, but that is not a problem for this
(and I would assume most) devices. Intervening directly
in acpi_bus_get_status_handle is harder as there is no
access to the hid there.

But if you modify acpi_set_device_status(), that should make it
consistent AFAICS.

And this is just a quirk for devices where _STA is known to return
garbage sometimes and I'd call it a quirk openly.

Ok, currently acpi_set_device_status() is an inline function in
include/acpi/acpi_bus.h. If I understand you correctly you want me
to uninline it and have a table with quirks which specify an override
value to apply for certain acpi_ids in the uninlined version, correct ?

Or is there some existing quirk mechanism I should tie into ?

Regards,

Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux