| Message ID | 20170408150511.21013-1-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sat, Apr 08, 2017 at 05:05:11PM +0200, 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. Would it be a viable alternative to respond differently to _OSI queries for these devices by amending acpi_osi_dmi_table[] in drivers/acpi/osi.c? > + pr_debug("Device [%s] is in always present list setting status [%08x]\n", > + adev->pnp.bus_id, ACPI_STA_DEFAULT); In a lot of places in drivers/acpi/, dev_warn(&adev->dev, ...) is used. Otherwise LGTM. Thanks, Lukas
Hi, On 09-04-17 09:26, Lukas Wunner wrote: > On Sat, Apr 08, 2017 at 05:05:11PM +0200, 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. > > Would it be a viable alternative to respond differently to _OSI queries > for these devices by amending acpi_osi_dmi_table[] in drivers/acpi/osi.c? Nope, _OSI influences OSYS not OSID, OSID is some quite bad Cherry Trail hack where the BIOS behaves differently based on whether it thinks the OS it is booting is going to be Android or Windows and there is nothing one can do to influence OSID (other then hardcoding special handling of that variable name in ACPICA I guess). But even if we could influence OSID we really don't want to, for all the same reasons our ACPI implementation always claims to be the latest windows. >> + pr_debug("Device [%s] is in always present list setting status [%08x]\n", >> + adev->pnp.bus_id, ACPI_STA_DEFAULT); > > In a lot of places in drivers/acpi/, dev_warn(&adev->dev, ...) is used. Rafael asked me to use pr_debug here. But I agree that maybe always logging something when this trigger might be a good idea, although warning is too high a loglevel IMHO. So I would got with dev_info. Rafael, what is your take on this ? Regards, Hans
On Sun, Apr 09, 2017 at 11:46:41AM +0200, Hans de Goede wrote: > On 09-04-17 09:26, Lukas Wunner wrote: > > On Sat, Apr 08, 2017 at 05:05:11PM +0200, Hans de Goede wrote: > > > + pr_debug("Device [%s] is in always present list setting status [%08x]\n", > > > + adev->pnp.bus_id, ACPI_STA_DEFAULT); > > > > In a lot of places in drivers/acpi/, dev_warn(&adev->dev, ...) is used. > > Rafael asked me to use pr_debug here. But I agree that maybe always > logging something when this trigger might be a good idea, although > warning is too high a loglevel IMHO. So I would got with dev_info. Sorry for not being clear, I just meant that existing code seems to prefer the dev_*() macros to report device-specific stuff, I wasn't referring to the severity level. Thanks, Lukas
diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c index 34fbe02..25e47ad 100644 --- a/drivers/acpi/bus.c +++ b/drivers/acpi/bus.c @@ -132,6 +132,37 @@ int acpi_bus_get_status(struct acpi_device *device) } EXPORT_SYMBOL(acpi_bus_get_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", }, + { } +}; + +void acpi_set_device_status(struct acpi_device *adev, u32 sta) +{ + u32 *status = (u32 *)&adev->status; + + /* acpi_match_device_ids checks status, so start with default */ + *status = ACPI_STA_DEFAULT; + if (acpi_match_device_ids(adev, always_present_device_ids) == 0) { + pr_debug("Device [%s] is in always present list setting status [%08x]\n", + adev->pnp.bus_id, ACPI_STA_DEFAULT); + return; + } + + *status = sta; +} + void acpi_bus_private_data_handler(acpi_handle handle, void *context) { diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h index 64498d5..a6ae057 100644 --- a/include/acpi/acpi_bus.h +++ b/include/acpi/acpi_bus.h @@ -441,11 +441,6 @@ static inline void *acpi_driver_data(struct acpi_device *d) #define to_acpi_device(d) container_of(d, struct acpi_device, dev) #define to_acpi_driver(d) container_of(d, struct acpi_driver, drv) -static inline void acpi_set_device_status(struct acpi_device *adev, u32 sta) -{ - *((u32 *)&adev->status) = sta; -} - static inline void acpi_set_hp_context(struct acpi_device *adev, struct acpi_hotplug_context *hp) { @@ -494,6 +489,7 @@ void acpi_bus_put_acpi_device(struct acpi_device *adev); acpi_status acpi_bus_get_status_handle(acpi_handle handle, unsigned long long *sta); int acpi_bus_get_status(struct acpi_device *device); +void acpi_set_device_status(struct acpi_device *adev, u32 sta); int acpi_bus_set_power(acpi_handle handle, int state); const char *acpi_power_state_string(int state);
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> --- 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 --- drivers/acpi/bus.c | 31 +++++++++++++++++++++++++++++++ include/acpi/acpi_bus.h | 6 +----- 2 files changed, 32 insertions(+), 5 deletions(-)