* [PATCH AUTOSEL 4.14 4/6] ACPI: video: Change disable_backlight_sysfs_if quirks to acpi_backlight=native [not found] <20221009235808.1232269-1-sashal@kernel.org> @ 2022-10-09 23:58 ` Sasha Levin 2022-10-10 7:37 ` Hans de Goede 0 siblings, 1 reply; 3+ messages in thread From: Sasha Levin @ 2022-10-09 23:58 UTC (permalink / raw) To: linux-kernel, stable Cc: Hans de Goede, Rafael J . Wysocki, Arvid Norlander, Sasha Levin, rafael, linux-acpi From: Hans de Goede <hdegoede@redhat.com> [ Upstream commit c5b94f5b7819348c59f9949b2b75c341a114cdd4 ] Some Toshibas have a broken acpi-video interface for brightness control and need a special firmware call on resume to turn the panel back on. So far these have been using the disable_backlight_sysfs_if workaround to deal with this. 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 kind of special vendor specific handling really belongs in the vendor specific acpi driver. An earlier patch in this series modifies toshiba_acpi to make the necessary HCI_SET call on resume on affected models. With toshiba_acpi taking care of the HCI_SET call on resume, the acpi_video code no longer needs to call _BCM on resume. So instead of using the (now broken) disable_backlight_sysfs_if workaround, simply setting acpi_backlight=native to disable the broken apci-video interface is sufficient fix things now. After this there are no more users of the disable_backlight_sysfs_if flag and as discussed above the flag also no longer works as intended, so remove the disable_backlight_sysfs_if flag entirely. Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> Tested-by: Arvid Norlander <lkml@vorpal.se> Signed-off-by: Hans de Goede <hdegoede@redhat.com> Signed-off-by: Sasha Levin <sashal@kernel.org> --- drivers/acpi/acpi_video.c | 48 ------------------------------------- drivers/acpi/video_detect.c | 35 +++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 48 deletions(-) diff --git a/drivers/acpi/acpi_video.c b/drivers/acpi/acpi_video.c index 5a69260edf80..324be2a29d68 100644 --- a/drivers/acpi/acpi_video.c +++ b/drivers/acpi/acpi_video.c @@ -63,9 +63,6 @@ module_param(brightness_switch_enabled, bool, 0644); static bool allow_duplicates; module_param(allow_duplicates, bool, 0644); -static int disable_backlight_sysfs_if = -1; -module_param(disable_backlight_sysfs_if, int, 0444); - #define REPORT_OUTPUT_KEY_EVENTS 0x01 #define REPORT_BRIGHTNESS_KEY_EVENTS 0x02 static int report_key_events = -1; @@ -397,14 +394,6 @@ static int video_set_bqc_offset(const struct dmi_system_id *d) return 0; } -static int video_disable_backlight_sysfs_if( - const struct dmi_system_id *d) -{ - if (disable_backlight_sysfs_if == -1) - disable_backlight_sysfs_if = 1; - return 0; -} - static int video_set_device_id_scheme(const struct dmi_system_id *d) { device_id_scheme = true; @@ -477,40 +466,6 @@ static const struct dmi_system_id video_dmi_table[] = { }, }, - /* - * Some machines have a broken acpi-video interface for brightness - * control, but still need an acpi_video_device_lcd_set_level() call - * on resume to turn the backlight power on. We Enable backlight - * control on these systems, but do not register a backlight sysfs - * as brightness control does not work. - */ - { - /* https://bugzilla.kernel.org/show_bug.cgi?id=21012 */ - .callback = video_disable_backlight_sysfs_if, - .ident = "Toshiba Portege R700", - .matches = { - DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"), - DMI_MATCH(DMI_PRODUCT_NAME, "PORTEGE R700"), - }, - }, - { - /* https://bugs.freedesktop.org/show_bug.cgi?id=82634 */ - .callback = video_disable_backlight_sysfs_if, - .ident = "Toshiba Portege R830", - .matches = { - DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"), - DMI_MATCH(DMI_PRODUCT_NAME, "PORTEGE R830"), - }, - }, - { - /* https://bugzilla.kernel.org/show_bug.cgi?id=21012 */ - .callback = video_disable_backlight_sysfs_if, - .ident = "Toshiba Satellite R830", - .matches = { - DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"), - DMI_MATCH(DMI_PRODUCT_NAME, "SATELLITE R830"), - }, - }, /* * Some machine's _DOD IDs don't have bit 31(Device ID Scheme) set * but the IDs actually follow the Device ID Scheme. @@ -1772,9 +1727,6 @@ static void acpi_video_dev_register_backlight(struct acpi_video_device *device) if (result) return; - if (disable_backlight_sysfs_if > 0) - return; - name = kasprintf(GFP_KERNEL, "acpi_video%d", count); if (!name) return; diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c index 490ae990bd3c..62975cfcce68 100644 --- a/drivers/acpi/video_detect.c +++ b/drivers/acpi/video_detect.c @@ -447,6 +447,41 @@ static const struct dmi_system_id video_detect_dmi_table[] = { DMI_MATCH(DMI_BOARD_NAME, "PF5LUXG"), }, }, + /* + * These Toshibas have a broken acpi-video interface for brightness + * control. They also have an issue where the panel is off after + * suspend until a special firmware call is made to turn it back + * on. This is handled by the toshiba_acpi kernel module, so that + * module must be enabled for these models to work correctly. + */ + { + /* https://bugzilla.kernel.org/show_bug.cgi?id=21012 */ + .callback = video_detect_force_native, + /* Toshiba Portégé R700 */ + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"), + DMI_MATCH(DMI_PRODUCT_NAME, "PORTEGE R700"), + }, + }, + { + /* Portégé: https://bugs.freedesktop.org/show_bug.cgi?id=82634 */ + /* Satellite: https://bugzilla.kernel.org/show_bug.cgi?id=21012 */ + .callback = video_detect_force_native, + /* Toshiba Satellite/Portégé R830 */ + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"), + DMI_MATCH(DMI_PRODUCT_NAME, "R830"), + }, + }, + { + .callback = video_detect_force_native, + /* Toshiba Satellite/Portégé Z830 */ + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"), + DMI_MATCH(DMI_PRODUCT_NAME, "Z830"), + }, + }, + /* * Desktops which falsely report a backlight and which our heuristics * for this do not catch. -- 2.35.1 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH AUTOSEL 4.14 4/6] ACPI: video: Change disable_backlight_sysfs_if quirks to acpi_backlight=native 2022-10-09 23:58 ` [PATCH AUTOSEL 4.14 4/6] ACPI: video: Change disable_backlight_sysfs_if quirks to acpi_backlight=native Sasha Levin @ 2022-10-10 7:37 ` Hans de Goede 2022-10-16 14:48 ` Sasha Levin 0 siblings, 1 reply; 3+ messages in thread From: Hans de Goede @ 2022-10-10 7:37 UTC (permalink / raw) To: Sasha Levin, linux-kernel, stable Cc: Rafael J . Wysocki, Arvid Norlander, rafael, linux-acpi Hi, On 10/10/22 01:58, Sasha Levin wrote: > From: Hans de Goede <hdegoede@redhat.com> > > [ Upstream commit c5b94f5b7819348c59f9949b2b75c341a114cdd4 ] > > Some Toshibas have a broken acpi-video interface for brightness control > and need a special firmware call on resume to turn the panel back on. > So far these have been using the disable_backlight_sysfs_if workaround > to deal with this. > > 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 kind of special vendor specific handling really belongs in > the vendor specific acpi driver. An earlier patch in this series > modifies toshiba_acpi to make the necessary HCI_SET call on resume > on affected models. > > With toshiba_acpi taking care of the HCI_SET call on resume, > the acpi_video code no longer needs to call _BCM on resume. > > So instead of using the (now broken) disable_backlight_sysfs_if > workaround, simply setting acpi_backlight=native to disable > the broken apci-video interface is sufficient fix things now. > > After this there are no more users of the disable_backlight_sysfs_if > flag and as discussed above the flag also no longer works as intended, > so remove the disable_backlight_sysfs_if flag entirely. > > Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > Tested-by: Arvid Norlander <lkml@vorpal.se> > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > Signed-off-by: Sasha Levin <sashal@kernel.org> This patch goes hand in hand with: commit 3cb1f40dfdc3 ("drivers/platform: toshiba_acpi: Call HCI_PANEL_POWER_ON on resume on some models") and without that commit also being present it will cause a regression on the quirked Toshiba models. This really is part of the big x86/ACPI backlight handling refactor which has landed in 6.1 and as such is not intended for older kernels, please drop this from the stable series. Regards, Hans > --- > drivers/acpi/acpi_video.c | 48 ------------------------------------- > drivers/acpi/video_detect.c | 35 +++++++++++++++++++++++++++ > 2 files changed, 35 insertions(+), 48 deletions(-) > > diff --git a/drivers/acpi/acpi_video.c b/drivers/acpi/acpi_video.c > index 5a69260edf80..324be2a29d68 100644 > --- a/drivers/acpi/acpi_video.c > +++ b/drivers/acpi/acpi_video.c > @@ -63,9 +63,6 @@ module_param(brightness_switch_enabled, bool, 0644); > static bool allow_duplicates; > module_param(allow_duplicates, bool, 0644); > > -static int disable_backlight_sysfs_if = -1; > -module_param(disable_backlight_sysfs_if, int, 0444); > - > #define REPORT_OUTPUT_KEY_EVENTS 0x01 > #define REPORT_BRIGHTNESS_KEY_EVENTS 0x02 > static int report_key_events = -1; > @@ -397,14 +394,6 @@ static int video_set_bqc_offset(const struct dmi_system_id *d) > return 0; > } > > -static int video_disable_backlight_sysfs_if( > - const struct dmi_system_id *d) > -{ > - if (disable_backlight_sysfs_if == -1) > - disable_backlight_sysfs_if = 1; > - return 0; > -} > - > static int video_set_device_id_scheme(const struct dmi_system_id *d) > { > device_id_scheme = true; > @@ -477,40 +466,6 @@ static const struct dmi_system_id video_dmi_table[] = { > }, > }, > > - /* > - * Some machines have a broken acpi-video interface for brightness > - * control, but still need an acpi_video_device_lcd_set_level() call > - * on resume to turn the backlight power on. We Enable backlight > - * control on these systems, but do not register a backlight sysfs > - * as brightness control does not work. > - */ > - { > - /* https://bugzilla.kernel.org/show_bug.cgi?id=21012 */ > - .callback = video_disable_backlight_sysfs_if, > - .ident = "Toshiba Portege R700", > - .matches = { > - DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"), > - DMI_MATCH(DMI_PRODUCT_NAME, "PORTEGE R700"), > - }, > - }, > - { > - /* https://bugs.freedesktop.org/show_bug.cgi?id=82634 */ > - .callback = video_disable_backlight_sysfs_if, > - .ident = "Toshiba Portege R830", > - .matches = { > - DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"), > - DMI_MATCH(DMI_PRODUCT_NAME, "PORTEGE R830"), > - }, > - }, > - { > - /* https://bugzilla.kernel.org/show_bug.cgi?id=21012 */ > - .callback = video_disable_backlight_sysfs_if, > - .ident = "Toshiba Satellite R830", > - .matches = { > - DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"), > - DMI_MATCH(DMI_PRODUCT_NAME, "SATELLITE R830"), > - }, > - }, > /* > * Some machine's _DOD IDs don't have bit 31(Device ID Scheme) set > * but the IDs actually follow the Device ID Scheme. > @@ -1772,9 +1727,6 @@ static void acpi_video_dev_register_backlight(struct acpi_video_device *device) > if (result) > return; > > - if (disable_backlight_sysfs_if > 0) > - return; > - > name = kasprintf(GFP_KERNEL, "acpi_video%d", count); > if (!name) > return; > diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c > index 490ae990bd3c..62975cfcce68 100644 > --- a/drivers/acpi/video_detect.c > +++ b/drivers/acpi/video_detect.c > @@ -447,6 +447,41 @@ static const struct dmi_system_id video_detect_dmi_table[] = { > DMI_MATCH(DMI_BOARD_NAME, "PF5LUXG"), > }, > }, > + /* > + * These Toshibas have a broken acpi-video interface for brightness > + * control. They also have an issue where the panel is off after > + * suspend until a special firmware call is made to turn it back > + * on. This is handled by the toshiba_acpi kernel module, so that > + * module must be enabled for these models to work correctly. > + */ > + { > + /* https://bugzilla.kernel.org/show_bug.cgi?id=21012 */ > + .callback = video_detect_force_native, > + /* Toshiba Portégé R700 */ > + .matches = { > + DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"), > + DMI_MATCH(DMI_PRODUCT_NAME, "PORTEGE R700"), > + }, > + }, > + { > + /* Portégé: https://bugs.freedesktop.org/show_bug.cgi?id=82634 */ > + /* Satellite: https://bugzilla.kernel.org/show_bug.cgi?id=21012 */ > + .callback = video_detect_force_native, > + /* Toshiba Satellite/Portégé R830 */ > + .matches = { > + DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"), > + DMI_MATCH(DMI_PRODUCT_NAME, "R830"), > + }, > + }, > + { > + .callback = video_detect_force_native, > + /* Toshiba Satellite/Portégé Z830 */ > + .matches = { > + DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"), > + DMI_MATCH(DMI_PRODUCT_NAME, "Z830"), > + }, > + }, > + > /* > * Desktops which falsely report a backlight and which our heuristics > * for this do not catch. ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH AUTOSEL 4.14 4/6] ACPI: video: Change disable_backlight_sysfs_if quirks to acpi_backlight=native 2022-10-10 7:37 ` Hans de Goede @ 2022-10-16 14:48 ` Sasha Levin 0 siblings, 0 replies; 3+ messages in thread From: Sasha Levin @ 2022-10-16 14:48 UTC (permalink / raw) To: Hans de Goede Cc: linux-kernel, stable, Rafael J . Wysocki, Arvid Norlander, rafael, linux-acpi On Mon, Oct 10, 2022 at 09:37:16AM +0200, Hans de Goede wrote: >Hi, > >On 10/10/22 01:58, Sasha Levin wrote: >> From: Hans de Goede <hdegoede@redhat.com> >> >> [ Upstream commit c5b94f5b7819348c59f9949b2b75c341a114cdd4 ] >> >> Some Toshibas have a broken acpi-video interface for brightness control >> and need a special firmware call on resume to turn the panel back on. >> So far these have been using the disable_backlight_sysfs_if workaround >> to deal with this. >> >> 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 kind of special vendor specific handling really belongs in >> the vendor specific acpi driver. An earlier patch in this series >> modifies toshiba_acpi to make the necessary HCI_SET call on resume >> on affected models. >> >> With toshiba_acpi taking care of the HCI_SET call on resume, >> the acpi_video code no longer needs to call _BCM on resume. >> >> So instead of using the (now broken) disable_backlight_sysfs_if >> workaround, simply setting acpi_backlight=native to disable >> the broken apci-video interface is sufficient fix things now. >> >> After this there are no more users of the disable_backlight_sysfs_if >> flag and as discussed above the flag also no longer works as intended, >> so remove the disable_backlight_sysfs_if flag entirely. >> >> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> >> Tested-by: Arvid Norlander <lkml@vorpal.se> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >> Signed-off-by: Sasha Levin <sashal@kernel.org> > >This patch goes hand in hand with: > >commit 3cb1f40dfdc3 ("drivers/platform: toshiba_acpi: Call HCI_PANEL_POWER_ON on resume on some models") > >and without that commit also being present it will cause a regression on >the quirked Toshiba models. > >This really is part of the big x86/ACPI backlight handling refactor which >has landed in 6.1 and as such is not intended for older kernels, please >drop this from the stable series. Will do, thanks! -- Thanks, Sasha ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-10-16 14:48 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20221009235808.1232269-1-sashal@kernel.org>
2022-10-09 23:58 ` [PATCH AUTOSEL 4.14 4/6] ACPI: video: Change disable_backlight_sysfs_if quirks to acpi_backlight=native Sasha Levin
2022-10-10 7:37 ` Hans de Goede
2022-10-16 14:48 ` Sasha Levin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).