[PATCH 0/6] ACPI: video: Fix missing acpi_video# devices on some systems

[PATCH 0/6] ACPI: video: Fix missing acpi_video# devices on some systems

[Hans de Goede]

Hi Rafael,

This patch series consists of 2 parts:

1. Fix missing acpi_video# devices on some systems, currently in kernels
   >= 6.1.5 and >= 6.2.0 acpi_video# backlight class devices will only
   get registered (by default) when a GPU driver asks for this by calling
   acpi_video_register_backlight(). This is causing backlight control to
   be missing on some systems.

   Patches 1-4 fix this and ideally these should be send to Linus for
   an upcoming 6.3-rc# release.

2. Now that the dust has settled a bit on the backlight refactor we can
   do some further cleanups. This is done in patches 5 + 6. Note that
   patch 5 depends on patch 2.

Regards,

Hans


Hans de Goede (6):
  ACPI: video: Add auto_detect arg to __acpi_video_get_backlight_type()
  ACPI: video: Make acpi_backlight=video work independent from GPU
    driver
  ACPI: video: Add acpi_backlight=video quirk for Apple iMac14,1 and
    iMac14,2
  ACPI: video: Add acpi_backlight=video quirk for Lenovo ThinkPad W530
  ACPI: video: Remove register_backlight_delay module option and code
  ACPI: video: Remove desktops without backlight DMI quirks

 drivers/acpi/acpi_video.c                     | 53 +++--------
 drivers/acpi/video_detect.c                   | 87 ++++++++++---------
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  4 -
 include/acpi/video.h                          | 17 +++-
 4 files changed, 71 insertions(+), 90 deletions(-)

-- 
2.39.1

[PATCH 1/6] ACPI: video: Add auto_detect arg to __acpi_video_get_backlight_type()

[Hans de Goede]

Allow callers of __acpi_video_get_backlight_type() to pass a pointer
to a bool which will get set to false if the backlight-type comes from
the cmdline or a DMI quirk and set to true if auto-detection was used.

And make __acpi_video_get_backlight_type() non static so that it can
be called directly outside of video_detect.c .

While at it turn the acpi_video_get_backlight_type() and
acpi_video_backlight_use_native() wrappers into static inline functions
in include/acpi/video.h, so that we need to export one less symbol.

Fixes: 5aa9d943e9b6 ("ACPI: video: Don't enable fallback path for creating ACPI backlight by default")
Cc: stable@vger.kernel.org
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/acpi/video_detect.c | 21 ++++++++-------------
 include/acpi/video.h        | 15 +++++++++++++--
 2 files changed, 21 insertions(+), 15 deletions(-)

diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c
index fd7cbce8076e..f7c218dd8742 100644
--- a/drivers/acpi/video_detect.c
+++ b/drivers/acpi/video_detect.c
@@ -782,7 +782,7 @@ static bool prefer_native_over_acpi_video(void)
  * Determine which type of backlight interface to use on this system,
  * First check cmdline, then dmi quirks, then do autodetect.
  */
-static enum acpi_backlight_type __acpi_video_get_backlight_type(bool native)
+enum acpi_backlight_type __acpi_video_get_backlight_type(bool native, bool *auto_detect)
 {
 	static DEFINE_MUTEX(init_mutex);
 	static bool nvidia_wmi_ec_present;
@@ -807,6 +807,9 @@ static enum acpi_backlight_type __acpi_video_get_backlight_type(bool native)
 		native_available = true;
 	mutex_unlock(&init_mutex);
 
+	if (auto_detect)
+		*auto_detect = false;
+
 	/*
 	 * The below heuristics / detection steps are in order of descending
 	 * presedence. The commandline takes presedence over anything else.
@@ -818,6 +821,9 @@ static enum acpi_backlight_type __acpi_video_get_backlight_type(bool native)
 	if (acpi_backlight_dmi != acpi_backlight_undef)
 		return acpi_backlight_dmi;
 
+	if (auto_detect)
+		*auto_detect = true;
+
 	/* Special cases such as nvidia_wmi_ec and apple gmux. */
 	if (nvidia_wmi_ec_present)
 		return acpi_backlight_nvidia_wmi_ec;
@@ -837,15 +843,4 @@ static enum acpi_backlight_type __acpi_video_get_backlight_type(bool native)
 	/* No ACPI video/native (old hw), use vendor specific fw methods. */
 	return acpi_backlight_vendor;
 }
-
-enum acpi_backlight_type acpi_video_get_backlight_type(void)
-{
-	return __acpi_video_get_backlight_type(false);
-}
-EXPORT_SYMBOL(acpi_video_get_backlight_type);
-
-bool acpi_video_backlight_use_native(void)
-{
-	return __acpi_video_get_backlight_type(true) == acpi_backlight_native;
-}
-EXPORT_SYMBOL(acpi_video_backlight_use_native);
+EXPORT_SYMBOL(__acpi_video_get_backlight_type);
diff --git a/include/acpi/video.h b/include/acpi/video.h
index 8ed9bec03e53..ff5a8da5d883 100644
--- a/include/acpi/video.h
+++ b/include/acpi/video.h
@@ -59,8 +59,6 @@ extern void acpi_video_unregister(void);
 extern void acpi_video_register_backlight(void);
 extern int acpi_video_get_edid(struct acpi_device *device, int type,
 			       int device_id, void **edid);
-extern enum acpi_backlight_type acpi_video_get_backlight_type(void);
-extern bool acpi_video_backlight_use_native(void);
 /*
  * Note: The value returned by acpi_video_handles_brightness_key_presses()
  * may change over time and should not be cached.
@@ -69,6 +67,19 @@ extern bool acpi_video_handles_brightness_key_presses(void);
 extern int acpi_video_get_levels(struct acpi_device *device,
 				 struct acpi_video_device_brightness **dev_br,
 				 int *pmax_level);
+
+extern enum acpi_backlight_type __acpi_video_get_backlight_type(bool native,
+								bool *auto_detect);
+
+static inline enum acpi_backlight_type acpi_video_get_backlight_type(void)
+{
+	return __acpi_video_get_backlight_type(false, NULL);
+}
+
+static inline bool acpi_video_backlight_use_native(void)
+{
+	return __acpi_video_get_backlight_type(true, NULL) == acpi_backlight_native;
+}
 #else
 static inline void acpi_video_report_nolcd(void) { return; };
 static inline int acpi_video_register(void) { return -ENODEV; }
-- 
2.39.1

[PATCH 2/6] ACPI: video: Make acpi_backlight=video work independent from GPU driver

[Hans de Goede]

Commit 3dbc80a3e4c5 ("ACPI: video: Make backlight class device
registration a separate step (v2)") combined with
commit 5aa9d943e9b6 ("ACPI: video: Don't enable fallback path for
creating ACPI backlight by default")

Means that the video.ko code now fully depends on the GPU driver calling
acpi_video_register_backlight() for the acpi_video# backlight class
devices to get registered.

This means that if the GPU driver does not do this, acpi_backlight=video
on the cmdline, or DMI quirks for selecting acpi_video# will not work.

This is a problem on for example Apple iMac14,1 all-in-ones where
the monitor's LCD panel shows up as a regular DP connection instead of
eDP so the GPU driver will not call acpi_video_register_backlight() [1].

Fix this by making video.ko directly register the acpi_video# devices
when these have been explicitly requested either on the cmdline or
through DMI quirks (rather then auto-detection being used).

[1] GPU drivers only call acpi_video_register_backlight() when an internal
panel is detected, to avoid non working acpi_video# devices getting
registered on desktops which unfortunately is a real issue.

Fixes: 5aa9d943e9b6 ("ACPI: video: Don't enable fallback path for creating ACPI backlight by default")
Cc: stable@vger.kernel.org
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/acpi/acpi_video.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/acpi_video.c b/drivers/acpi/acpi_video.c
index 97b711e57bff..c7a6d0b69dab 100644
--- a/drivers/acpi/acpi_video.c
+++ b/drivers/acpi/acpi_video.c
@@ -1984,6 +1984,7 @@ static int instance;
 static int acpi_video_bus_add(struct acpi_device *device)
 {
 	struct acpi_video_bus *video;
+	bool auto_detect;
 	int error;
 	acpi_status status;
 
@@ -2045,10 +2046,20 @@ static int acpi_video_bus_add(struct acpi_device *device)
 	mutex_unlock(&video_list_lock);
 
 	/*
-	 * The userspace visible backlight_device gets registered separately
-	 * from acpi_video_register_backlight().
+	 * If backlight-type auto-detection is used then a native backlight may
+	 * show up later and this may change the result from video to native.
+	 * Therefor normally the userspace visible /sys/class/backlight device
+	 * gets registered separately by the GPU driver calling
+	 * acpi_video_register_backlight() when an internal panel is detected.
+	 * Register the backlight now when not using auto-detection, so that
+	 * when the kernel cmdline or DMI-quirks are used the backlight will
+	 * get registered even if acpi_video_register_backlight() is not called.
 	 */
 	acpi_video_run_bcl_for_osi(video);
+	if (__acpi_video_get_backlight_type(false, &auto_detect) == acpi_backlight_video &&
+	    !auto_detect)
+		acpi_video_bus_register_backlight(video);
+
 	acpi_video_bus_add_notify_handler(video);
 
 	return 0;
-- 
2.39.1

[PATCH 3/6] ACPI: video: Add acpi_backlight=video quirk for Apple iMac14,1 and iMac14,2

[Hans de Goede]

On the Apple iMac14,1 and iMac14,2 all-in-ones (monitors with builtin "PC")
the connection between the GPU and the panel is seen by the GPU driver as
regular DP instead of eDP, causing the GPU driver to never call
acpi_video_register_backlight().

(GPU drivers only call acpi_video_register_backlight() when an internal
 panel is detected, to avoid non working acpi_video# devices getting
 registered on desktops which unfortunately is a real issue.)

Fix the missing acpi_video# backlight device on these all-in-ones by
adding a acpi_backlight=video DMI quirk, so that video.ko will
immediately register the backlight device instead of waiting for
an acpi_video_register_backlight() call.

Fixes: 5aa9d943e9b6 ("ACPI: video: Don't enable fallback path for creating ACPI backlight by default")
Cc: stable@vger.kernel.org
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/acpi/video_detect.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c
index f7c218dd8742..295744fe7c92 100644
--- a/drivers/acpi/video_detect.c
+++ b/drivers/acpi/video_detect.c
@@ -276,6 +276,29 @@ static const struct dmi_system_id video_detect_dmi_table[] = {
 		},
 	},
 
+	/*
+	 * Models which need acpi_video backlight control where the GPU drivers
+	 * do not call acpi_video_register_backlight() because no internal panel
+	 * is detected. Typically these are all-in-ones (monitors with builtin
+	 * PC) where the panel connection shows up as regular DP instead of eDP.
+	 */
+	{
+	 .callback = video_detect_force_video,
+	 /* Apple iMac14,1 */
+	 .matches = {
+		DMI_MATCH(DMI_SYS_VENDOR, "Apple Inc."),
+		DMI_MATCH(DMI_PRODUCT_NAME, "iMac14,1"),
+		},
+	},
+	{
+	 .callback = video_detect_force_video,
+	 /* Apple iMac14,2 */
+	 .matches = {
+		DMI_MATCH(DMI_SYS_VENDOR, "Apple Inc."),
+		DMI_MATCH(DMI_PRODUCT_NAME, "iMac14,2"),
+		},
+	},
+
 	/*
 	 * These models have a working acpi_video backlight control, and using
 	 * native backlight causes a regression where backlight does not work
-- 
2.39.1

[PATCH 4/6] ACPI: video: Add acpi_backlight=video quirk for Lenovo ThinkPad W530

[Hans de Goede]

The Lenovo ThinkPad W530 uses a nvidia k1000m GPU. When this gets used
together with one of the older nvidia binary driver series (the latest
series does not support it), then backlight control does not work.

This is caused by commit 3dbc80a3e4c5 ("ACPI: video: Make backlight
class device registration a separate step (v2)") combined with
commit 5aa9d943e9b6 ("ACPI: video: Don't enable fallback path for
creating ACPI backlight by default").

After these changes the acpi_video# backlight device is only registered
when requested by a GPU driver calling acpi_video_register_backlight()
which the nvidia binary driver does not do.

I realize that using the nvidia binary driver is not a supported use-case
and users can workaround this by adding acpi_backlight=video on the kernel
commandline, but the ThinkPad W530 is a popular model under Linux users,
so it seems worthwhile to add a quirk for this.

I will also email Nvidia asking them to make the driver call
acpi_video_register_backlight() when an internal LCD panel is detected.
So maybe the next maintenance release of the drivers will fix this...

Fixes: 5aa9d943e9b6 ("ACPI: video: Don't enable fallback path for creating ACPI backlight by default")
Cc: stable@vger.kernel.org
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/acpi/video_detect.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c
index 295744fe7c92..e85729fc481f 100644
--- a/drivers/acpi/video_detect.c
+++ b/drivers/acpi/video_detect.c
@@ -299,6 +299,20 @@ static const struct dmi_system_id video_detect_dmi_table[] = {
 		},
 	},
 
+	/*
+	 * Older models with nvidia GPU which need acpi_video backlight
+	 * control and where the old nvidia binary driver series does not
+	 * call acpi_video_register_backlight().
+	 */
+	{
+	 .callback = video_detect_force_video,
+	 /* ThinkPad W530 */
+	 .matches = {
+		DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
+		DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad W530"),
+		},
+	},
+
 	/*
 	 * These models have a working acpi_video backlight control, and using
 	 * native backlight causes a regression where backlight does not work
-- 
2.39.1

[PATCH 5/6] ACPI: video: Remove register_backlight_delay module option and code

[Hans de Goede]

Since commit 5aa9d943e9b6 ("ACPI: video: Don't enable fallback path for
creating ACPI backlight by default"), the delayed registerering of
acpi_video# backlight devices has been disabled by default.

The few bugreports where this option was used as a workaround were all
cases where the GPU driver did not call acpi_video_register_backlight()
and the workaround was to pass video.register_backlight_delay=1.

With the recent "ACPI: video: Make acpi_backlight=video work independent
from GPU driver" changes acpi_backlight=video can be used to achieve
the same result. So there is no need for the register_backlight_delay
option + code anymore.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/acpi/acpi_video.c                     | 38 -------------------
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  4 --
 include/acpi/video.h                          |  2 -
 3 files changed, 44 deletions(-)

diff --git a/drivers/acpi/acpi_video.c b/drivers/acpi/acpi_video.c
index c7a6d0b69dab..62f4364e4460 100644
--- a/drivers/acpi/acpi_video.c
+++ b/drivers/acpi/acpi_video.c
@@ -70,12 +70,6 @@ module_param(device_id_scheme, bool, 0444);
 static int only_lcd = -1;
 module_param(only_lcd, int, 0444);
 
-static int register_backlight_delay;
-module_param(register_backlight_delay, int, 0444);
-MODULE_PARM_DESC(register_backlight_delay,
-	"Delay in seconds before doing fallback (non GPU driver triggered) "
-	"backlight registration, set to 0 to disable.");
-
 static bool may_report_brightness_keys;
 static int register_count;
 static DEFINE_MUTEX(register_count_mutex);
@@ -84,9 +78,6 @@ static LIST_HEAD(video_bus_head);
 static int acpi_video_bus_add(struct acpi_device *device);
 static void acpi_video_bus_remove(struct acpi_device *device);
 static void acpi_video_bus_notify(struct acpi_device *device, u32 event);
-static void acpi_video_bus_register_backlight_work(struct work_struct *ignored);
-static DECLARE_DELAYED_WORK(video_bus_register_backlight_work,
-			    acpi_video_bus_register_backlight_work);
 
 /*
  * Indices in the _BCL method response: the first two items are special,
@@ -2096,11 +2087,6 @@ static void acpi_video_bus_remove(struct acpi_device *device)
 	kfree(video);
 }
 
-static void acpi_video_bus_register_backlight_work(struct work_struct *ignored)
-{
-	acpi_video_register_backlight();
-}
-
 static int __init is_i740(struct pci_dev *dev)
 {
 	if (dev->device == 0x00D1)
@@ -2183,17 +2169,6 @@ static bool should_check_lcd_flag(void)
 	return false;
 }
 
-/*
- * At least one graphics driver has reported that no LCD is connected
- * via the native interface. cancel the registration for fallback acpi_video0.
- * If another driver still deems this necessary, it can explicitly register it.
- */
-void acpi_video_report_nolcd(void)
-{
-	cancel_delayed_work(&video_bus_register_backlight_work);
-}
-EXPORT_SYMBOL(acpi_video_report_nolcd);
-
 int acpi_video_register(void)
 {
 	int ret = 0;
@@ -2222,18 +2197,6 @@ int acpi_video_register(void)
 	 */
 	register_count = 1;
 
-	/*
-	 * acpi_video_bus_add() skips registering the userspace visible
-	 * backlight_device. The intend is for this to be registered by the
-	 * drm/kms driver calling acpi_video_register_backlight() *after* it is
-	 * done setting up its own native backlight device. The delayed work
-	 * ensures that acpi_video_register_backlight() always gets called
-	 * eventually, in case there is no drm/kms driver or it is disabled.
-	 */
-	if (register_backlight_delay)
-		schedule_delayed_work(&video_bus_register_backlight_work,
-				      register_backlight_delay * HZ);
-
 leave:
 	mutex_unlock(&register_count_mutex);
 	return ret;
@@ -2244,7 +2207,6 @@ void acpi_video_unregister(void)
 {
 	mutex_lock(&register_count_mutex);
 	if (register_count) {
-		cancel_delayed_work_sync(&video_bus_register_backlight_work);
 		acpi_bus_unregister_driver(&acpi_video_bus);
 		register_count = 0;
 		may_report_brightness_keys = false;
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 7c96faa73426..d832f9ac43d4 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -4436,10 +4436,6 @@ static int amdgpu_dm_initialize_drm_device(struct amdgpu_device *adev)
 		amdgpu_set_panel_orientation(&aconnector->base);
 	}
 
-	/* If we didn't find a panel, notify the acpi video detection */
-	if (dm->adev->flags & AMD_IS_APU && dm->num_of_edps == 0)
-		acpi_video_report_nolcd();
-
 	/* Software is initialized. Now we can register interrupt handlers. */
 	switch (adev->asic_type) {
 #if defined(CONFIG_DRM_AMD_DC_SI)
diff --git a/include/acpi/video.h b/include/acpi/video.h
index ff5a8da5d883..4230392b5b0b 100644
--- a/include/acpi/video.h
+++ b/include/acpi/video.h
@@ -53,7 +53,6 @@ enum acpi_backlight_type {
 };
 
 #if IS_ENABLED(CONFIG_ACPI_VIDEO)
-extern void acpi_video_report_nolcd(void);
 extern int acpi_video_register(void);
 extern void acpi_video_unregister(void);
 extern void acpi_video_register_backlight(void);
@@ -81,7 +80,6 @@ static inline bool acpi_video_backlight_use_native(void)
 	return __acpi_video_get_backlight_type(true, NULL) == acpi_backlight_native;
 }
 #else
-static inline void acpi_video_report_nolcd(void) { return; };
 static inline int acpi_video_register(void) { return -ENODEV; }
 static inline void acpi_video_unregister(void) { return; }
 static inline void acpi_video_register_backlight(void) { return; }
-- 
2.39.1

[PATCH 6/6] ACPI: video: Remove desktops without backlight DMI quirks

[Hans de Goede]

After the recent backlight changes acpi_video# backlight devices are only
registered when explicitly requested from the cmdline, by DMI quirk or by
the GPU driver.

This means that we no longer get false-positive backlight control support
advertised on desktop boards.

Remove the 3 DMI quirks for desktop boards where the false-positive issue
was fixed through quirks before. Note many more desktop boards were
affected but we never build a full quirk list for this.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/acpi/video_detect.c | 29 -----------------------------
 1 file changed, 29 deletions(-)

diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c
index e85729fc481f..07fa1bc843f2 100644
--- a/drivers/acpi/video_detect.c
+++ b/drivers/acpi/video_detect.c
@@ -768,35 +768,6 @@ static const struct dmi_system_id video_detect_dmi_table[] = {
 		DMI_MATCH(DMI_PRODUCT_NAME, "Vostro 15 3535"),
 		},
 	},
-
-	/*
-	 * Desktops which falsely report a backlight and which our heuristics
-	 * for this do not catch.
-	 */
-	{
-	 .callback = video_detect_force_none,
-	 /* Dell OptiPlex 9020M */
-	 .matches = {
-		DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
-		DMI_MATCH(DMI_PRODUCT_NAME, "OptiPlex 9020M"),
-		},
-	},
-	{
-	 .callback = video_detect_force_none,
-	 /* GIGABYTE GB-BXBT-2807 */
-	 .matches = {
-		DMI_MATCH(DMI_SYS_VENDOR, "GIGABYTE"),
-		DMI_MATCH(DMI_PRODUCT_NAME, "GB-BXBT-2807"),
-		},
-	},
-	{
-	 .callback = video_detect_force_none,
-	 /* MSI MS-7721 */
-	 .matches = {
-		DMI_MATCH(DMI_SYS_VENDOR, "MSI"),
-		DMI_MATCH(DMI_PRODUCT_NAME, "MS-7721"),
-		},
-	},
 	{ },
 };
 
-- 
2.39.1

Re: [PATCH 5/6] ACPI: video: Remove register_backlight_delay module option and code

[Limonciello, Mario]

On 4/3/2023 11:03, Hans de Goede wrote:
> Since commit 5aa9d943e9b6 ("ACPI: video: Don't enable fallback path for
> creating ACPI backlight by default"), the delayed registerering of

registering

> acpi_video# backlight devices has been disabled by default.
> 
> The few bugreports where this option was used as a workaround were all
> cases where the GPU driver did not call acpi_video_register_backlight()
> and the workaround was to pass video.register_backlight_delay=1.
> 
> With the recent "ACPI: video: Make acpi_backlight=video work independent
> from GPU driver" changes acpi_backlight=video can be used to achieve
> the same result. So there is no need for the register_backlight_delay
> option + code anymore.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>   drivers/acpi/acpi_video.c                     | 38 -------------------
>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  4 --
>   include/acpi/video.h                          |  2 -
>   3 files changed, 44 deletions(-)
> 
> diff --git a/drivers/acpi/acpi_video.c b/drivers/acpi/acpi_video.c
> index c7a6d0b69dab..62f4364e4460 100644
> --- a/drivers/acpi/acpi_video.c
> +++ b/drivers/acpi/acpi_video.c
> @@ -70,12 +70,6 @@ module_param(device_id_scheme, bool, 0444);
>   static int only_lcd = -1;
>   module_param(only_lcd, int, 0444);
>   
> -static int register_backlight_delay;
> -module_param(register_backlight_delay, int, 0444);
> -MODULE_PARM_DESC(register_backlight_delay,
> -	"Delay in seconds before doing fallback (non GPU driver triggered) "
> -	"backlight registration, set to 0 to disable.");
> -
>   static bool may_report_brightness_keys;
>   static int register_count;
>   static DEFINE_MUTEX(register_count_mutex);
> @@ -84,9 +78,6 @@ static LIST_HEAD(video_bus_head);
>   static int acpi_video_bus_add(struct acpi_device *device);
>   static void acpi_video_bus_remove(struct acpi_device *device);
>   static void acpi_video_bus_notify(struct acpi_device *device, u32 event);
> -static void acpi_video_bus_register_backlight_work(struct work_struct *ignored);
> -static DECLARE_DELAYED_WORK(video_bus_register_backlight_work,
> -			    acpi_video_bus_register_backlight_work);
>   
>   /*
>    * Indices in the _BCL method response: the first two items are special,
> @@ -2096,11 +2087,6 @@ static void acpi_video_bus_remove(struct acpi_device *device)
>   	kfree(video);
>   }
>   
> -static void acpi_video_bus_register_backlight_work(struct work_struct *ignored)
> -{
> -	acpi_video_register_backlight();
> -}
> -
>   static int __init is_i740(struct pci_dev *dev)
>   {
>   	if (dev->device == 0x00D1)
> @@ -2183,17 +2169,6 @@ static bool should_check_lcd_flag(void)
>   	return false;
>   }
>   
> -/*
> - * At least one graphics driver has reported that no LCD is connected
> - * via the native interface. cancel the registration for fallback acpi_video0.
> - * If another driver still deems this necessary, it can explicitly register it.
> - */
> -void acpi_video_report_nolcd(void)
> -{
> -	cancel_delayed_work(&video_bus_register_backlight_work);
> -}
> -EXPORT_SYMBOL(acpi_video_report_nolcd);
> -
>   int acpi_video_register(void)
>   {
>   	int ret = 0;
> @@ -2222,18 +2197,6 @@ int acpi_video_register(void)
>   	 */
>   	register_count = 1;
>   
> -	/*
> -	 * acpi_video_bus_add() skips registering the userspace visible
> -	 * backlight_device. The intend is for this to be registered by the
> -	 * drm/kms driver calling acpi_video_register_backlight() *after* it is
> -	 * done setting up its own native backlight device. The delayed work
> -	 * ensures that acpi_video_register_backlight() always gets called
> -	 * eventually, in case there is no drm/kms driver or it is disabled.
> -	 */
> -	if (register_backlight_delay)
> -		schedule_delayed_work(&video_bus_register_backlight_work,
> -				      register_backlight_delay * HZ);
> -
>   leave:
>   	mutex_unlock(&register_count_mutex);
>   	return ret;
> @@ -2244,7 +2207,6 @@ void acpi_video_unregister(void)
>   {
>   	mutex_lock(&register_count_mutex);
>   	if (register_count) {
> -		cancel_delayed_work_sync(&video_bus_register_backlight_work);
>   		acpi_bus_unregister_driver(&acpi_video_bus);
>   		register_count = 0;
>   		may_report_brightness_keys = false;
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 7c96faa73426..d832f9ac43d4 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -4436,10 +4436,6 @@ static int amdgpu_dm_initialize_drm_device(struct amdgpu_device *adev)
>   		amdgpu_set_panel_orientation(&aconnector->base);
>   	}
>   
> -	/* If we didn't find a panel, notify the acpi video detection */
> -	if (dm->adev->flags & AMD_IS_APU && dm->num_of_edps == 0)
> -		acpi_video_report_nolcd();
> -
>   	/* Software is initialized. Now we can register interrupt handlers. */
>   	switch (adev->asic_type) {
>   #if defined(CONFIG_DRM_AMD_DC_SI)
> diff --git a/include/acpi/video.h b/include/acpi/video.h
> index ff5a8da5d883..4230392b5b0b 100644
> --- a/include/acpi/video.h
> +++ b/include/acpi/video.h
> @@ -53,7 +53,6 @@ enum acpi_backlight_type {
>   };
>   
>   #if IS_ENABLED(CONFIG_ACPI_VIDEO)
> -extern void acpi_video_report_nolcd(void);
>   extern int acpi_video_register(void);
>   extern void acpi_video_unregister(void);
>   extern void acpi_video_register_backlight(void);
> @@ -81,7 +80,6 @@ static inline bool acpi_video_backlight_use_native(void)
>   	return __acpi_video_get_backlight_type(true, NULL) == acpi_backlight_native;
>   }
>   #else
> -static inline void acpi_video_report_nolcd(void) { return; };
>   static inline int acpi_video_register(void) { return -ENODEV; }
>   static inline void acpi_video_unregister(void) { return; }
>   static inline void acpi_video_register_backlight(void) { return; }

Re: [PATCH 0/6] ACPI: video: Fix missing acpi_video# devices on some systems

[Limonciello, Mario]

On 4/3/2023 11:03, Hans de Goede wrote:
> Hi Rafael,
> 
> This patch series consists of 2 parts:
> 
> 1. Fix missing acpi_video# devices on some systems, currently in kernels
>     >= 6.1.5 and >= 6.2.0 acpi_video# backlight class devices will only
>     get registered (by default) when a GPU driver asks for this by calling
>     acpi_video_register_backlight(). This is causing backlight control to
>     be missing on some systems.
> 
>     Patches 1-4 fix this and ideally these should be send to Linus for
>     an upcoming 6.3-rc# release.
> 
> 2. Now that the dust has settled a bit on the backlight refactor we can
>     do some further cleanups. This is done in patches 5 + 6. Note that
>     patch 5 depends on patch 2.
> 
> Regards,
> 
> Hans
> 
> 
> Hans de Goede (6):
>    ACPI: video: Add auto_detect arg to __acpi_video_get_backlight_type()
>    ACPI: video: Make acpi_backlight=video work independent from GPU
>      driver
>    ACPI: video: Add acpi_backlight=video quirk for Apple iMac14,1 and
>      iMac14,2
>    ACPI: video: Add acpi_backlight=video quirk for Lenovo ThinkPad W530
>    ACPI: video: Remove register_backlight_delay module option and code
>    ACPI: video: Remove desktops without backlight DMI quirks
> 
>   drivers/acpi/acpi_video.c                     | 53 +++--------
>   drivers/acpi/video_detect.c                   | 87 ++++++++++---------
>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  4 -
>   include/acpi/video.h                          | 17 +++-
>   4 files changed, 71 insertions(+), 90 deletions(-)
> 

One minor nit on a patch, otherwise:

Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>

Re: [PATCH 0/6] ACPI: video: Fix missing acpi_video# devices on some systems

[Hans de Goede]

Hi,

On 4/3/23 18:41, Limonciello, Mario wrote:
> On 4/3/2023 11:03, Hans de Goede wrote:
>> Hi Rafael,
>>
>> This patch series consists of 2 parts:
>>
>> 1. Fix missing acpi_video# devices on some systems, currently in kernels
>>     >= 6.1.5 and >= 6.2.0 acpi_video# backlight class devices will only
>>     get registered (by default) when a GPU driver asks for this by calling
>>     acpi_video_register_backlight(). This is causing backlight control to
>>     be missing on some systems.
>>
>>     Patches 1-4 fix this and ideally these should be send to Linus for
>>     an upcoming 6.3-rc# release.
>>
>> 2. Now that the dust has settled a bit on the backlight refactor we can
>>     do some further cleanups. This is done in patches 5 + 6. Note that
>>     patch 5 depends on patch 2.
>>
>> Regards,
>>
>> Hans
>>
>>
>> Hans de Goede (6):
>>    ACPI: video: Add auto_detect arg to __acpi_video_get_backlight_type()
>>    ACPI: video: Make acpi_backlight=video work independent from GPU
>>      driver
>>    ACPI: video: Add acpi_backlight=video quirk for Apple iMac14,1 and
>>      iMac14,2
>>    ACPI: video: Add acpi_backlight=video quirk for Lenovo ThinkPad W530
>>    ACPI: video: Remove register_backlight_delay module option and code
>>    ACPI: video: Remove desktops without backlight DMI quirks
>>
>>   drivers/acpi/acpi_video.c                     | 53 +++--------
>>   drivers/acpi/video_detect.c                   | 87 ++++++++++---------
>>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  4 -
>>   include/acpi/video.h                          | 17 +++-
>>   4 files changed, 71 insertions(+), 90 deletions(-)
>>
> 
> One minor nit on a patch, otherwise:
> 
> Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>

Thank you.

Rafael, can you fix up the typo in the commit msg of 5/6
while merging or do you want a new version ?

Regards,

Hans

Re: [PATCH 0/6] ACPI: video: Fix missing acpi_video# devices on some systems

[Hans de Goede]

Hi,

On 4/4/23 11:52, Hans de Goede wrote:
> Hi,
> 
> On 4/3/23 18:41, Limonciello, Mario wrote:
>> On 4/3/2023 11:03, Hans de Goede wrote:
>>> Hi Rafael,
>>>
>>> This patch series consists of 2 parts:
>>>
>>> 1. Fix missing acpi_video# devices on some systems, currently in kernels
>>>     >= 6.1.5 and >= 6.2.0 acpi_video# backlight class devices will only
>>>     get registered (by default) when a GPU driver asks for this by calling
>>>     acpi_video_register_backlight(). This is causing backlight control to
>>>     be missing on some systems.
>>>
>>>     Patches 1-4 fix this and ideally these should be send to Linus for
>>>     an upcoming 6.3-rc# release.
>>>
>>> 2. Now that the dust has settled a bit on the backlight refactor we can
>>>     do some further cleanups. This is done in patches 5 + 6. Note that
>>>     patch 5 depends on patch 2.
>>>
>>> Regards,
>>>
>>> Hans
>>>
>>>
>>> Hans de Goede (6):
>>>    ACPI: video: Add auto_detect arg to __acpi_video_get_backlight_type()
>>>    ACPI: video: Make acpi_backlight=video work independent from GPU
>>>      driver
>>>    ACPI: video: Add acpi_backlight=video quirk for Apple iMac14,1 and
>>>      iMac14,2
>>>    ACPI: video: Add acpi_backlight=video quirk for Lenovo ThinkPad W530
>>>    ACPI: video: Remove register_backlight_delay module option and code
>>>    ACPI: video: Remove desktops without backlight DMI quirks
>>>
>>>   drivers/acpi/acpi_video.c                     | 53 +++--------
>>>   drivers/acpi/video_detect.c                   | 87 ++++++++++---------
>>>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  4 -
>>>   include/acpi/video.h                          | 17 +++-
>>>   4 files changed, 71 insertions(+), 90 deletions(-)
>>>
>>
>> One minor nit on a patch, otherwise:
>>
>> Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
> 
> Thank you.
> 
> Rafael, can you fix up the typo in the commit msg of 5/6
> while merging or do you want a new version ?

I just noticed this caused by 6/6, which I somehow missed before:

  CC [M]  drivers/acpi/video_detect.o
drivers/acpi/video_detect.c:133:12: warning: ‘video_detect_force_none’ defined but not used [-Wunused-function]
  133 | static int video_detect_force_none(const struct dmi_system_id *d)
      |            ^~~~~~~~~~~~~~~~~~~~~~~


So I need to prepare a new version to fix this. I'll also take
care of fixing the commit msg of 5/6 in v2.

Regards,

Hans