diff mbox series

ACPI: video: Drop should_check_lcd_flag()

Message ID 20231115174811.7571-1-hdegoede@redhat.com (mailing list archive)
State Mainlined, archived
Headers show
Series ACPI: video: Drop should_check_lcd_flag() | expand

Commit Message

Hans de Goede Nov. 15, 2023, 5:48 p.m. UTC
Since commit 3dbc80a3e4c5 ("ACPI: video: Make backlight class device
registration a separate step (v2)") acpi_video# backlights are no longer
automatically registered. Instead they now only get registered when
the GPU/KMS driver calls acpi_video_register_backlight() which it only
does when it has detected an internal LCD panel.

This fixes the issue of sometimes a non-working acpi_video# backlight
showing up on Desktops / HDMI-sticks without an internal LCD display
in a more complete and robust manner then the LCD flag check which
gets enabled by the should_check_lcd_flag() helper does.

Therefor the should_check_lcd_flag() helper is no longer necessary.

The lcd_only flag itself is still necessary to only register
a single backlight device (for the right output) on the ESPRIMO Mobile
M9410 which has 2 ACPI video connector nodes with a _BCM control method,
which is the issue for which the flag was originally introduced in
commit e50b9be14ab0 ("ACPI / video: only register backlight for LCD
device").

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/acpi/acpi_video.c | 56 +--------------------------------------
 1 file changed, 1 insertion(+), 55 deletions(-)

Comments

Rafael J. Wysocki Nov. 20, 2023, 4:50 p.m. UTC | #1
On Wed, Nov 15, 2023 at 6:48 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Since commit 3dbc80a3e4c5 ("ACPI: video: Make backlight class device
> registration a separate step (v2)") acpi_video# backlights are no longer
> automatically registered. Instead they now only get registered when
> the GPU/KMS driver calls acpi_video_register_backlight() which it only
> does when it has detected an internal LCD panel.
>
> This fixes the issue of sometimes a non-working acpi_video# backlight
> showing up on Desktops / HDMI-sticks without an internal LCD display
> in a more complete and robust manner then the LCD flag check which
> gets enabled by the should_check_lcd_flag() helper does.
>
> Therefor the should_check_lcd_flag() helper is no longer necessary.
>
> The lcd_only flag itself is still necessary to only register
> a single backlight device (for the right output) on the ESPRIMO Mobile
> M9410 which has 2 ACPI video connector nodes with a _BCM control method,
> which is the issue for which the flag was originally introduced in
> commit e50b9be14ab0 ("ACPI / video: only register backlight for LCD
> device").
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/acpi/acpi_video.c | 56 +--------------------------------------
>  1 file changed, 1 insertion(+), 55 deletions(-)
>
> diff --git a/drivers/acpi/acpi_video.c b/drivers/acpi/acpi_video.c
> index d321ca7160d9..5eded14f8853 100644
> --- a/drivers/acpi/acpi_video.c
> +++ b/drivers/acpi/acpi_video.c
> @@ -67,7 +67,7 @@ MODULE_PARM_DESC(hw_changes_brightness,
>  static bool device_id_scheme = false;
>  module_param(device_id_scheme, bool, 0444);
>
> -static int only_lcd = -1;
> +static int only_lcd;
>  module_param(only_lcd, int, 0444);
>
>  static bool may_report_brightness_keys;
> @@ -2141,57 +2141,6 @@ static int __init intel_opregion_present(void)
>         return opregion;
>  }
>
> -/* Check if the chassis-type indicates there is no builtin LCD panel */
> -static bool dmi_is_desktop(void)
> -{
> -       const char *chassis_type;
> -       unsigned long type;
> -
> -       chassis_type = dmi_get_system_info(DMI_CHASSIS_TYPE);
> -       if (!chassis_type)
> -               return false;
> -
> -       if (kstrtoul(chassis_type, 10, &type) != 0)
> -               return false;
> -
> -       switch (type) {
> -       case 0x03: /* Desktop */
> -       case 0x04: /* Low Profile Desktop */
> -       case 0x05: /* Pizza Box */
> -       case 0x06: /* Mini Tower */
> -       case 0x07: /* Tower */
> -       case 0x10: /* Lunch Box */
> -       case 0x11: /* Main Server Chassis */
> -               return true;
> -       }
> -
> -       return false;
> -}
> -
> -/*
> - * We're seeing a lot of bogus backlight interfaces on newer machines
> - * without a LCD such as desktops, servers and HDMI sticks. Checking the
> - * lcd flag fixes this, enable this by default on any machines which are:
> - * 1.  Win8 ready (where we also prefer the native backlight driver, so
> - *     normally the acpi_video code should not register there anyways); *and*
> - * 2.1 Report a desktop/server DMI chassis-type, or
> - * 2.2 Are an ACPI-reduced-hardware platform (and thus won't use the EC for
> -       backlight control)
> - */
> -static bool should_check_lcd_flag(void)
> -{
> -       if (!acpi_osi_is_win8())
> -               return false;
> -
> -       if (dmi_is_desktop())
> -               return true;
> -
> -       if (acpi_reduced_hardware())
> -               return true;
> -
> -       return false;
> -}
> -
>  int acpi_video_register(void)
>  {
>         int ret = 0;
> @@ -2205,9 +2154,6 @@ int acpi_video_register(void)
>                 goto leave;
>         }
>
> -       if (only_lcd == -1)
> -               only_lcd = should_check_lcd_flag();
> -
>         dmi_check_system(video_dmi_table);
>
>         ret = acpi_bus_register_driver(&acpi_video_bus);
> --

Applied as 6.8 material, thanks!
diff mbox series

Patch

diff --git a/drivers/acpi/acpi_video.c b/drivers/acpi/acpi_video.c
index d321ca7160d9..5eded14f8853 100644
--- a/drivers/acpi/acpi_video.c
+++ b/drivers/acpi/acpi_video.c
@@ -67,7 +67,7 @@  MODULE_PARM_DESC(hw_changes_brightness,
 static bool device_id_scheme = false;
 module_param(device_id_scheme, bool, 0444);
 
-static int only_lcd = -1;
+static int only_lcd;
 module_param(only_lcd, int, 0444);
 
 static bool may_report_brightness_keys;
@@ -2141,57 +2141,6 @@  static int __init intel_opregion_present(void)
 	return opregion;
 }
 
-/* Check if the chassis-type indicates there is no builtin LCD panel */
-static bool dmi_is_desktop(void)
-{
-	const char *chassis_type;
-	unsigned long type;
-
-	chassis_type = dmi_get_system_info(DMI_CHASSIS_TYPE);
-	if (!chassis_type)
-		return false;
-
-	if (kstrtoul(chassis_type, 10, &type) != 0)
-		return false;
-
-	switch (type) {
-	case 0x03: /* Desktop */
-	case 0x04: /* Low Profile Desktop */
-	case 0x05: /* Pizza Box */
-	case 0x06: /* Mini Tower */
-	case 0x07: /* Tower */
-	case 0x10: /* Lunch Box */
-	case 0x11: /* Main Server Chassis */
-		return true;
-	}
-
-	return false;
-}
-
-/*
- * We're seeing a lot of bogus backlight interfaces on newer machines
- * without a LCD such as desktops, servers and HDMI sticks. Checking the
- * lcd flag fixes this, enable this by default on any machines which are:
- * 1.  Win8 ready (where we also prefer the native backlight driver, so
- *     normally the acpi_video code should not register there anyways); *and*
- * 2.1 Report a desktop/server DMI chassis-type, or
- * 2.2 Are an ACPI-reduced-hardware platform (and thus won't use the EC for
-       backlight control)
- */
-static bool should_check_lcd_flag(void)
-{
-	if (!acpi_osi_is_win8())
-		return false;
-
-	if (dmi_is_desktop())
-		return true;
-
-	if (acpi_reduced_hardware())
-		return true;
-
-	return false;
-}
-
 int acpi_video_register(void)
 {
 	int ret = 0;
@@ -2205,9 +2154,6 @@  int acpi_video_register(void)
 		goto leave;
 	}
 
-	if (only_lcd == -1)
-		only_lcd = should_check_lcd_flag();
-
 	dmi_check_system(video_dmi_table);
 
 	ret = acpi_bus_register_driver(&acpi_video_bus);