| Message ID | 20240919171952.403745-5-lkml@antheas.dev (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | acpi/x86: s2idle: move Display off/on calls outside suspend (fixes ROG Ally suspend) | expand |
On 9/19/2024 12:19, Antheas Kapenekakis wrote: > By moving the Display On/Off calls outside of the suspend sequence, > the racing conditions that made the Ally controller suspend unreliable > are completely fixed. This includes both when mcu_powersave is enabled > and disabled. Therefore, remove the quirk that fixes them only when > mcu_powersave is disabled. > > Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev> Yeah this makes sense to do if the core issue is resolved in the earlier patches. Feel free to include this tag for this patch in future revisions. Reviewed-by: Mario Limonciello <mario.limonciello@amd.com> > --- > drivers/platform/x86/asus-wmi.c | 54 --------------------------------- > 1 file changed, 54 deletions(-) > > diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c > index 37636e5a38e3..2c9656e0afda 100644 > --- a/drivers/platform/x86/asus-wmi.c > +++ b/drivers/platform/x86/asus-wmi.c > @@ -137,29 +137,10 @@ module_param(fnlock_default, bool, 0444); > #define ASUS_MINI_LED_2024_STRONG 0x01 > #define ASUS_MINI_LED_2024_OFF 0x02 > > -/* Controls the power state of the USB0 hub on ROG Ally which input is on */ > -#define ASUS_USB0_PWR_EC0_CSEE "\\_SB.PCI0.SBRG.EC0.CSEE" > -/* 300ms so far seems to produce a reliable result on AC and battery */ > -#define ASUS_USB0_PWR_EC0_CSEE_WAIT 1500 > - > static const char * const ashs_ids[] = { "ATK4001", "ATK4002", NULL }; > > static int throttle_thermal_policy_write(struct asus_wmi *); > > -static const struct dmi_system_id asus_ally_mcu_quirk[] = { > - { > - .matches = { > - DMI_MATCH(DMI_BOARD_NAME, "RC71L"), > - }, > - }, > - { > - .matches = { > - DMI_MATCH(DMI_BOARD_NAME, "RC72L"), > - }, > - }, > - { }, > -}; > - > static bool ashs_present(void) > { > int i = 0; > @@ -269,9 +250,6 @@ struct asus_wmi { > u32 tablet_switch_dev_id; > bool tablet_switch_inverted; > > - /* The ROG Ally device requires the MCU USB device be disconnected before suspend */ > - bool ally_mcu_usb_switch; > - > enum fan_type fan_type; > enum fan_type gpu_fan_type; > enum fan_type mid_fan_type; > @@ -4698,8 +4676,6 @@ static int asus_wmi_add(struct platform_device *pdev) > asus->egpu_enable_available = asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_EGPU); > asus->dgpu_disable_available = asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_DGPU); > asus->kbd_rgb_state_available = asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_TUF_RGB_STATE); > - asus->ally_mcu_usb_switch = acpi_has_method(NULL, ASUS_USB0_PWR_EC0_CSEE) > - && dmi_check_system(asus_ally_mcu_quirk); > > if (asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_MINI_LED_MODE)) > asus->mini_led_dev_id = ASUS_WMI_DEVID_MINI_LED_MODE; > @@ -4892,34 +4868,6 @@ static int asus_hotk_resume(struct device *device) > return 0; > } > > -static int asus_hotk_resume_early(struct device *device) > -{ > - struct asus_wmi *asus = dev_get_drvdata(device); > - > - if (asus->ally_mcu_usb_switch) { > - /* sleep required to prevent USB0 being yanked then reappearing rapidly */ > - if (ACPI_FAILURE(acpi_execute_simple_method(NULL, ASUS_USB0_PWR_EC0_CSEE, 0xB8))) > - dev_err(device, "ROG Ally MCU failed to connect USB dev\n"); > - else > - msleep(ASUS_USB0_PWR_EC0_CSEE_WAIT); > - } > - return 0; > -} > - > -static int asus_hotk_prepare(struct device *device) > -{ > - struct asus_wmi *asus = dev_get_drvdata(device); > - > - if (asus->ally_mcu_usb_switch) { > - /* sleep required to ensure USB0 is disabled before sleep continues */ > - if (ACPI_FAILURE(acpi_execute_simple_method(NULL, ASUS_USB0_PWR_EC0_CSEE, 0xB7))) > - dev_err(device, "ROG Ally MCU failed to disconnect USB dev\n"); > - else > - msleep(ASUS_USB0_PWR_EC0_CSEE_WAIT); > - } > - return 0; > -} > - > static int asus_hotk_restore(struct device *device) > { > struct asus_wmi *asus = dev_get_drvdata(device); > @@ -4964,8 +4912,6 @@ static const struct dev_pm_ops asus_pm_ops = { > .thaw = asus_hotk_thaw, > .restore = asus_hotk_restore, > .resume = asus_hotk_resume, > - .resume_early = asus_hotk_resume_early, > - .prepare = asus_hotk_prepare, > }; > > /* Registration ***************************************************************/
diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c index 37636e5a38e3..2c9656e0afda 100644 --- a/drivers/platform/x86/asus-wmi.c +++ b/drivers/platform/x86/asus-wmi.c @@ -137,29 +137,10 @@ module_param(fnlock_default, bool, 0444); #define ASUS_MINI_LED_2024_STRONG 0x01 #define ASUS_MINI_LED_2024_OFF 0x02 -/* Controls the power state of the USB0 hub on ROG Ally which input is on */ -#define ASUS_USB0_PWR_EC0_CSEE "\\_SB.PCI0.SBRG.EC0.CSEE" -/* 300ms so far seems to produce a reliable result on AC and battery */ -#define ASUS_USB0_PWR_EC0_CSEE_WAIT 1500 - static const char * const ashs_ids[] = { "ATK4001", "ATK4002", NULL }; static int throttle_thermal_policy_write(struct asus_wmi *); -static const struct dmi_system_id asus_ally_mcu_quirk[] = { - { - .matches = { - DMI_MATCH(DMI_BOARD_NAME, "RC71L"), - }, - }, - { - .matches = { - DMI_MATCH(DMI_BOARD_NAME, "RC72L"), - }, - }, - { }, -}; - static bool ashs_present(void) { int i = 0; @@ -269,9 +250,6 @@ struct asus_wmi { u32 tablet_switch_dev_id; bool tablet_switch_inverted; - /* The ROG Ally device requires the MCU USB device be disconnected before suspend */ - bool ally_mcu_usb_switch; - enum fan_type fan_type; enum fan_type gpu_fan_type; enum fan_type mid_fan_type; @@ -4698,8 +4676,6 @@ static int asus_wmi_add(struct platform_device *pdev) asus->egpu_enable_available = asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_EGPU); asus->dgpu_disable_available = asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_DGPU); asus->kbd_rgb_state_available = asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_TUF_RGB_STATE); - asus->ally_mcu_usb_switch = acpi_has_method(NULL, ASUS_USB0_PWR_EC0_CSEE) - && dmi_check_system(asus_ally_mcu_quirk); if (asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_MINI_LED_MODE)) asus->mini_led_dev_id = ASUS_WMI_DEVID_MINI_LED_MODE; @@ -4892,34 +4868,6 @@ static int asus_hotk_resume(struct device *device) return 0; } -static int asus_hotk_resume_early(struct device *device) -{ - struct asus_wmi *asus = dev_get_drvdata(device); - - if (asus->ally_mcu_usb_switch) { - /* sleep required to prevent USB0 being yanked then reappearing rapidly */ - if (ACPI_FAILURE(acpi_execute_simple_method(NULL, ASUS_USB0_PWR_EC0_CSEE, 0xB8))) - dev_err(device, "ROG Ally MCU failed to connect USB dev\n"); - else - msleep(ASUS_USB0_PWR_EC0_CSEE_WAIT); - } - return 0; -} - -static int asus_hotk_prepare(struct device *device) -{ - struct asus_wmi *asus = dev_get_drvdata(device); - - if (asus->ally_mcu_usb_switch) { - /* sleep required to ensure USB0 is disabled before sleep continues */ - if (ACPI_FAILURE(acpi_execute_simple_method(NULL, ASUS_USB0_PWR_EC0_CSEE, 0xB7))) - dev_err(device, "ROG Ally MCU failed to disconnect USB dev\n"); - else - msleep(ASUS_USB0_PWR_EC0_CSEE_WAIT); - } - return 0; -} - static int asus_hotk_restore(struct device *device) { struct asus_wmi *asus = dev_get_drvdata(device); @@ -4964,8 +4912,6 @@ static const struct dev_pm_ops asus_pm_ops = { .thaw = asus_hotk_thaw, .restore = asus_hotk_restore, .resume = asus_hotk_resume, - .resume_early = asus_hotk_resume_early, - .prepare = asus_hotk_prepare, }; /* Registration ***************************************************************/
By moving the Display On/Off calls outside of the suspend sequence, the racing conditions that made the Ally controller suspend unreliable are completely fixed. This includes both when mcu_powersave is enabled and disabled. Therefore, remove the quirk that fixes them only when mcu_powersave is disabled. Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev> --- drivers/platform/x86/asus-wmi.c | 54 --------------------------------- 1 file changed, 54 deletions(-)