Hi All, On 8/24/22 20:49, Arvid Norlander wrote: > Toshiba Satellite Z830 needs the quirk video_disable_backlight_sysfs_if > for proper backlight control after suspend/resume cycles. > > Toshiba Portege Z830 is simply the same laptop rebranded for certain > markets (I looked through the manual to other language sections to confirm > this) and thus also needs this quirk. > > Thanks to Hans de Goede for suggesting this fix. > > Link: https://www.spinics.net/lists/platform-driver-x86/msg34394.html > Signed-off-by: Arvid Norlander <lkml@xxxxxxxxx> So I've been thinking a bit about this and this is going to be a problem with my pending backlight refactor work. *** Note this patch should still be merged as a fix for 6.0 and older *** On these models acpi_video_get_backlight_type() returns acpi_backlight_video, otherwise setting disable_backlight_sysfs_if would not do anything. After my "drm/kms: Stop registering multiple /sys/class/backlight devs for a single display" series (1), the intel_backlight will no longer gets registered, that now only happens when acpi_video_get_backlight_type() returns acpi_backlight_native. This will break the disable_backlight_sysfs_if workaround because after this there then won't be any devices under /sys/class/backlight at all. I have been thinking about how to fix this, here are my notes I have written on this. -toshiba r830 / z830 problem after series: -duplicate DMI match in video_detect, force native -make disable_backlight_sysfs_if still work even when acpi_video_register_backlight() does not get called, just do the backlight_init directly from acpi_video_register() when disable_backlight_sysfs_if is set, as before my refactor series This will work, but it makes the code (even more) ugly / convoluted. Arvid, I wonder if instead of using disable_backlight_sysfs_if you can try: 0. Remove disable_backlight_sysfs_if from cmdline / quirk 1. Adding acpi_backlight=native to the kernel commandline 2. In toshiba_acpi_resume() add a HCI_PANEL_POWER_ON PANEL_ON and see if that also fixes things ? If that is the case then we can: 1. Move the DMI quirks for disable_backlight_sysfs_if from acpi_video.c to video_detect.c to force native mode by quirk 2. Add the DMI table with the models needing this to toshiba_acpi.c and then based on that call HCI_PANEL_POWER_ON PANEL_ON on resume from there 3. Since there are no more quirks using it, remove the disable_backlight_sysfs_if hack / workaround from acpi_video.c This will give a nice-cleanup of the generic acpi_video.c code moving the toshiba specific fixup to toshiba_acpi where it really belongs. Regards, Hans 1) https://lore.kernel.org/platform-driver-x86/20220825143726.269890-1-hdegoede@xxxxxxxxxx/T/ > --- > drivers/acpi/acpi_video.c | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/drivers/acpi/acpi_video.c b/drivers/acpi/acpi_video.c > index 5cbe2196176d..2a4990733cf0 100644 > --- a/drivers/acpi/acpi_video.c > +++ b/drivers/acpi/acpi_video.c > @@ -496,6 +496,22 @@ static const struct dmi_system_id video_dmi_table[] = { > DMI_MATCH(DMI_PRODUCT_NAME, "SATELLITE R830"), > }, > }, > + { > + .callback = video_disable_backlight_sysfs_if, > + .ident = "Toshiba Satellite Z830", > + .matches = { > + DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"), > + DMI_MATCH(DMI_PRODUCT_NAME, "SATELLITE Z830"), > + }, > + }, > + { > + .callback = video_disable_backlight_sysfs_if, > + .ident = "Toshiba Portege Z830", > + .matches = { > + DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"), > + DMI_MATCH(DMI_PRODUCT_NAME, "PORTEGE Z830"), > + }, > + }, > /* > * Some machine's _DOD IDs don't have bit 31(Device ID Scheme) set > * but the IDs actually follow the Device ID Scheme.