| Message ID | 20210316194639.287216-1-alexander.deucher@amd.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/2] platform/x86: Add missing LPS0 functions for AMD | expand |
Hi, On 3/16/21 8:46 PM, Alex Deucher wrote: > These are supposedly not required for AMD platforms, > but at least some HP laptops seem to require it to > properly turn off the keyboard backlight. > > Based on a patch from Marcin Bachry <hegel666@gmail.com>. > > Bug: https://gitlab.freedesktop.org/drm/amd/-/issues/1230 > Signed-off-by: Alex Deucher <alexander.deucher@amd.com> > Cc: Marcin Bachry <hegel666@gmail.com> Thanks, patch looks good to me: Reviewed-by: Hans de Goede <hdegoede@redhat.com> Regards, Hans > --- > drivers/acpi/x86/s2idle.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c > index 2b69536cdccb..2d7ddb8a8cb6 100644 > --- a/drivers/acpi/x86/s2idle.c > +++ b/drivers/acpi/x86/s2idle.c > @@ -42,6 +42,8 @@ static const struct acpi_device_id lps0_device_ids[] = { > > /* AMD */ > #define ACPI_LPS0_DSM_UUID_AMD "e3f32452-febc-43ce-9039-932122d37721" > +#define ACPI_LPS0_ENTRY_AMD 2 > +#define ACPI_LPS0_EXIT_AMD 3 > #define ACPI_LPS0_SCREEN_OFF_AMD 4 > #define ACPI_LPS0_SCREEN_ON_AMD 5 > > @@ -408,6 +410,7 @@ int acpi_s2idle_prepare_late(void) > > if (acpi_s2idle_vendor_amd()) { > acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_OFF_AMD); > + acpi_sleep_run_lps0_dsm(ACPI_LPS0_ENTRY_AMD); > } else { > acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_OFF); > acpi_sleep_run_lps0_dsm(ACPI_LPS0_ENTRY); > @@ -422,6 +425,7 @@ void acpi_s2idle_restore_early(void) > return; > > if (acpi_s2idle_vendor_amd()) { > + acpi_sleep_run_lps0_dsm(ACPI_LPS0_EXIT_AMD); > acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_ON_AMD); > } else { > acpi_sleep_run_lps0_dsm(ACPI_LPS0_EXIT); >
Hi, On 3/16/21 8:46 PM, Alex Deucher wrote: > ACPI_LPS0_ENTRY_AMD/ACPI_LPS0_EXIT_AMD are supposedly not > required for AMD platforms, and on some platforms they are > not even listed in the function mask but at least some HP > laptops seem to require it to properly support s0ix. > > Based on a patch from Marcin Bachry <hegel666@gmail.com>. > > Bug: https://gitlab.freedesktop.org/drm/amd/-/issues/1230 > Signed-off-by: Alex Deucher <alexander.deucher@amd.com> > Cc: Marcin Bachry <hegel666@gmail.com> > --- > drivers/acpi/x86/s2idle.c | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c > index 2d7ddb8a8cb6..dc3cc021125e 100644 > --- a/drivers/acpi/x86/s2idle.c > +++ b/drivers/acpi/x86/s2idle.c > @@ -317,11 +317,16 @@ static void lpi_check_constraints(void) > } > } > > +static bool acpi_s2idle_vendor_amd(void) > +{ > + return boot_cpu_data.x86_vendor == X86_VENDOR_AMD; > +} > + > static void acpi_sleep_run_lps0_dsm(unsigned int func) > { > union acpi_object *out_obj; > > - if (!(lps0_dsm_func_mask & (1 << func))) > + if (!acpi_s2idle_vendor_amd() && !(lps0_dsm_func_mask & (1 << func))) > return; > > out_obj = acpi_evaluate_dsm(lps0_device_handle, &lps0_dsm_guid, rev_id, func, NULL); Skipping the dsm_func_mask feels a bit wrong to me. The commit message talks about there being a need to unconditional make the calls in the case of the ACPI_LPS0_ENTRY_AMD/ACPI_LPS0_EXIT_AMD calls. Maybe instead add a "skip_func_mask" boolean parameter to acpi_sleep_run_lps0_dsm() and set that to false everywhere except for the 2 ACPI_LPS0_ENTRY_AMD/ACPI_LPS0_EXIT_AMD calls ? This way we can control when to skip the check on a call by call basis, rather then always skipping it on all AMD systems. Regards, Hans > @@ -331,11 +336,6 @@ static void acpi_sleep_run_lps0_dsm(unsigned int func) > func, out_obj ? "successful" : "failed"); > } > > -static bool acpi_s2idle_vendor_amd(void) > -{ > - return boot_cpu_data.x86_vendor == X86_VENDOR_AMD; > -} > - > static int lps0_device_attach(struct acpi_device *adev, > const struct acpi_device_id *not_used) > { >
diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c index 2b69536cdccb..2d7ddb8a8cb6 100644 --- a/drivers/acpi/x86/s2idle.c +++ b/drivers/acpi/x86/s2idle.c @@ -42,6 +42,8 @@ static const struct acpi_device_id lps0_device_ids[] = { /* AMD */ #define ACPI_LPS0_DSM_UUID_AMD "e3f32452-febc-43ce-9039-932122d37721" +#define ACPI_LPS0_ENTRY_AMD 2 +#define ACPI_LPS0_EXIT_AMD 3 #define ACPI_LPS0_SCREEN_OFF_AMD 4 #define ACPI_LPS0_SCREEN_ON_AMD 5 @@ -408,6 +410,7 @@ int acpi_s2idle_prepare_late(void) if (acpi_s2idle_vendor_amd()) { acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_OFF_AMD); + acpi_sleep_run_lps0_dsm(ACPI_LPS0_ENTRY_AMD); } else { acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_OFF); acpi_sleep_run_lps0_dsm(ACPI_LPS0_ENTRY); @@ -422,6 +425,7 @@ void acpi_s2idle_restore_early(void) return; if (acpi_s2idle_vendor_amd()) { + acpi_sleep_run_lps0_dsm(ACPI_LPS0_EXIT_AMD); acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_ON_AMD); } else { acpi_sleep_run_lps0_dsm(ACPI_LPS0_EXIT);
These are supposedly not required for AMD platforms, but at least some HP laptops seem to require it to properly turn off the keyboard backlight. Based on a patch from Marcin Bachry <hegel666@gmail.com>. Bug: https://gitlab.freedesktop.org/drm/amd/-/issues/1230 Signed-off-by: Alex Deucher <alexander.deucher@amd.com> Cc: Marcin Bachry <hegel666@gmail.com> --- drivers/acpi/x86/s2idle.c | 4 ++++ 1 file changed, 4 insertions(+)