Hi, On 9/9/22 19:29, Arvid Norlander wrote: > Hi, > > Given the changes, do you want me to test this again? If so, on what branch? I have just pushed this new version + all your pending toshiba_acpi patches to my review-hans branch: https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans\ If you can give this branch a quick test and let me know if everything works as expected that would be great. Thanks, Hans > > Best regards, > Arvid Norlander > > On 2022-09-09 17:32, Hans de Goede wrote: >> Some Toshibas have a broken acpi-video interface for brightness control, so >> far these have been using a special workaround in drivers/acpi/acpi_video.c >> which gets activated by the disable_backlight_sysfs_if module-param/quirks. >> >> The recent x86/acpi backlight refactoring has broken this workaround: >> 1. This workaround relies on acpi_video_get_backlight_type() returning >> acpi_video so that the acpi_video code actually runs; and >> 2. this relies on the actual native GPU driver to offer the sysfs >> backlight interface to userspace. >> >> After the refactor this breaks since the native driver will no >> longer register its backlight-device if acpi_video_get_backlight_type() >> does not return native and making it return native breaks 1. >> >> Keeping the acpi_video backlight handling on resume active, while not >> using it to set the brightness, is necessary because it does a _BCM >> call on resume which is necessary to turn the panel back on on resume. >> >> Looking at the DSDT shows that this _BCM call results in a Toshiba >> HCI_SET HCI_LCD_BRIGHTNESS call, which turns the panel back on. >> >> This commit makes toshiba_acpi do a HCI_SET HCI_PANEL_POWER_ON call >> on resume on the affected models, so that the (now broken) >> acpi_video disable_backlight_sysfs_if workaround will no longer >> be necessary. >> >> Note this uses HCI_PANEL_POWER_ON instead of HCI_LCD_BRIGHTNESS >> to avoid changing the configured brightness level. >> >> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx> >> --- >> Changes in v2: >> - Add a turn_on_panel_on_resume module parameter to allow overriding >> the DMI quirk based setting >> --- >> drivers/platform/x86/toshiba_acpi.c | 50 +++++++++++++++++++++++++++++ >> 1 file changed, 50 insertions(+) >> >> diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c >> index 900ee68a4c0b..aa55ed0d029d 100644 >> --- a/drivers/platform/x86/toshiba_acpi.c >> +++ b/drivers/platform/x86/toshiba_acpi.c >> @@ -23,6 +23,7 @@ >> #define PROC_INTERFACE_VERSION 1 >> >> #include <linux/compiler.h> >> +#include <linux/dmi.h> >> #include <linux/kernel.h> >> #include <linux/module.h> >> #include <linux/moduleparam.h> >> @@ -50,6 +51,11 @@ MODULE_AUTHOR("John Belmonte"); >> MODULE_DESCRIPTION("Toshiba Laptop ACPI Extras Driver"); >> MODULE_LICENSE("GPL"); >> >> +int turn_on_panel_on_resume = -1; >> +module_param(turn_on_panel_on_resume, int, 0644); >> +MODULE_PARM_DESC(turn_on_panel_on_resume, >> + "Call HCI_PANEL_POWER_ON on resume (-1 = auto, 0 = no, 1 = yes"); >> + >> #define TOSHIBA_WMI_EVENT_GUID "59142400-C6A3-40FA-BADB-8A2652834100" >> >> /* Scan code for Fn key on TOS1900 models */ >> @@ -100,6 +106,7 @@ MODULE_LICENSE("GPL"); >> #define TOS_NOT_INSTALLED 0x8e00 >> >> /* Registers */ >> +#define HCI_PANEL_POWER_ON 0x0002 >> #define HCI_FAN 0x0004 >> #define HCI_TR_BACKLIGHT 0x0005 >> #define HCI_SYSTEM_EVENT 0x0016 >> @@ -3002,6 +3009,43 @@ static const char *find_hci_method(acpi_handle handle) >> return NULL; >> } >> >> +/* >> + * Some Toshibas have a broken acpi-video interface for brightness control, >> + * these are quirked in drivers/acpi/video_detect.c to use the GPU native >> + * (/sys/class/backlight/intel_backlight) instead. >> + * But these need a HCI_SET call to actually turn the panel back on at resume, >> + * without this call the screen stays black at resume. >> + * Either HCI_LCD_BRIGHTNESS (used by acpi_video's _BCM) or HCI_PANEL_POWER_ON >> + * works. toshiba_acpi_resume() uses HCI_PANEL_POWER_ON to avoid changing >> + * the configured brightness level. >> + */ >> +static const struct dmi_system_id turn_on_panel_on_resume_dmi_ids[] = { >> + { >> + /* Toshiba Portégé R700 */ >> + /* https://bugzilla.kernel.org/show_bug.cgi?id=21012 */ >> + .matches = { >> + DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"), >> + DMI_MATCH(DMI_PRODUCT_NAME, "PORTEGE R700"), >> + }, >> + }, >> + { >> + /* Toshiba Satellite/Portégé R830 */ >> + /* Portégé: https://bugs.freedesktop.org/show_bug.cgi?id=82634 */ >> + /* Satellite: https://bugzilla.kernel.org/show_bug.cgi?id=21012 */ >> + .matches = { >> + DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"), >> + DMI_MATCH(DMI_PRODUCT_NAME, "R830"), >> + }, >> + }, >> + { >> + /* Toshiba Satellite/Portégé Z830 */ >> + .matches = { >> + DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"), >> + DMI_MATCH(DMI_PRODUCT_NAME, "Z830"), >> + }, >> + }, >> +}; >> + >> static int toshiba_acpi_add(struct acpi_device *acpi_dev) >> { >> struct toshiba_acpi_dev *dev; >> @@ -3144,6 +3188,9 @@ static int toshiba_acpi_add(struct acpi_device *acpi_dev) >> ret = get_fan_status(dev, &dummy); >> dev->fan_supported = !ret; >> >> + if (turn_on_panel_on_resume == -1) >> + turn_on_panel_on_resume = dmi_check_system(turn_on_panel_on_resume_dmi_ids); >> + >> toshiba_wwan_available(dev); >> if (dev->wwan_supported) >> toshiba_acpi_setup_wwan_rfkill(dev); >> @@ -3260,6 +3307,9 @@ static int toshiba_acpi_resume(struct device *device) >> rfkill_set_hw_state(dev->wwan_rfk, !dev->killswitch); >> } >> >> + if (turn_on_panel_on_resume) >> + hci_write(dev, HCI_PANEL_POWER_ON, 1); >> + >> return 0; >> } >> #endif >
