HI,
On 21-04-17 12:33, Rafael J. Wysocki wrote:
On Friday, April 21, 2017 11:59:34 AM Hans de Goede wrote:
Hi,
On 19-04-17 22:14, Rafael J. Wysocki wrote:
On Wed, Apr 19, 2017 at 2:04 PM, Hans de Goede <hdegoede@xxxxxxxxxx> wrote:
Several Bay / Cherry Trail 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 Cherry Trail 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 uses it when it needs it), this
commit makes acpi_bus_get_status() always set status to ACPI_STA_DEFAULT
for the LPSS PWM device, fixing the lack of backlight control.
Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
---
Changes in v2:
-Use pr_debug instead of ACPI_DEBUG_PRINT
Changes in v3:
-Un-inline acpi_set_device_status and do the always_present_device_ids
table check inside the un-inlined version of it
Changes in v4:
-Use dev_info instead of pr_debug
-Not only check for ACPI HID but also for CPU (SoC) model so as to not
for devices present on other models then for which the quirk is intended and
to avoid enabling unrelated ACPI devices which happen to use the same HID
Changes in v5:
-Only do the dev_info once per device (acpi_set_device_status gets called
multiple times per device during boot)
Changes in v6:
-Allow specifying more then one CPU-model for a single HID
-Not only match the HID but also the UID, like on Cherry Trail, on some Bay
Trail Windows 10 tablets we need to enable the PWM controller to get working
backlight even though _STA returns 0. The Bay Trail SoC has 2 PWM controllers
and we only need the first one. UID matching will allows adding an entry for
Bay Trail which only enables the first PWM controller
Changes in v7:
-Put the actual x86 specific matching code and table with always present
device HID + UID + CPU model entries in a new x86/x86_utils.c file which
provides an acpi_device_always_present() helper function on x86, on
non x86 a stub which always returns false is provided
-Squash in the addition of the Bay Trail PWM entry in the table as it
is there for the same reasons as the Cherry Trail entry is there
<snip>
diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index 34fbe02..4d952cc 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -132,6 +132,16 @@ int acpi_bus_get_status(struct acpi_device *device)
}
EXPORT_SYMBOL(acpi_bus_get_status);
+void acpi_set_device_status(struct acpi_device *adev, u32 sta)
+{
+ u32 *status = (u32 *)&adev->status;
+
+ if (acpi_device_always_present(adev))
+ *status = ACPI_STA_DEFAULT;
+ else
+ *status = sta;
*((u32 *)&adev->status) = acpi_device_always_present(adev) ?
ACPI_STA_DEFAULT : sta;
and I guess it could still be static inline?
But that said, evaluating _STA for the always present devices is
pointless (as Lukas noticed), so why not to modify
acpi_bus_get_status() to do something like:
if (acpi_device_always_present(adev)) {
acpi_set_device_status(adev, ACPI_STA_DEFAULT);
return 0;
}
upfront
Hehe, my v1 of this patch actually did the check in acpi_bus_get_status()
I moved it to acpi_set_device_status() on your request. Moving it back
is fine with me and indeed seems cleaner.
and modify the other path invoking acpi_set_device_status() accordingly.
And in that path, which I seem to have overlooked before, the
acpi_set_device_status() call is too early for invoking
acpi_device_always_present(adev), so the latter should be called
directly from acpi_add_single_object() like
acpi_init_device_object(device, handle, type, sta);
if (acpi_device_always_present(adev))
acpi_set_device_status(adev, ACPI_STA_DEFAULT);
That is not necessary, the place(s) where we care about status being
fixed-up for always-present devices, so that they get scanned / their
platform device initiated, is in acpi_bus_attach() which
already calls acpi_bus_get_status() and thus gets the
acpi_device_always_present() check applied before checking.
For hotplugged devices there are acpi_scan_bus_check and
acpi_scan_device_check but those both also already call
acpi_bus_get_status() before checking adev->status.
OK
Which probably means that we don't need to initialize adev->status
in acpi_init_device_object() to anything meaningful, do we?
Right, I don't think that is necessary. But maybe it is necessary
for some special cases (e.g. power resources) ?
Regards,
Hans
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
