Re: [PATCH] platform/x86: dell-laptop: Add drm module soft dependency

[Hans de Goede]

Hi,

On 6/7/23 09:47, Matthew Garrett wrote:
> On Wed, Jun 07, 2023 at 03:39:33PM +0800, AceLan Kao wrote:
> 
>> What do you think if we unregister backlight devices if the backlight type
>> is larger than the current registered one.
>> Do this check in backlight_device_register() and unregister backlight
>> devices by the order raw(1) > platform(2) > firmware(3)
>> And maybe introduce a sticky bit into the backlight device if the backlight
>> driver doesn't want to be removed.
> 
> Hans looked at doing this, but there were some awkward corner cases. 
> When we first introduced this functionality, firmware was preferred to 
> platform was preferred to raw - but on Intel, at least, this behaviour 
> changed with later versions of Windows. I don't think there's a single 
> static policy that works, I think you need to pay attention to the hints 
> the platform gives you. How does Windows know which interface to use on 
> this platform? The simplest solution may actually just be for 
> dell-laptop to refuse to register a backlight if the platform claims to 
> be Windows 8 or later.

I like that idea.

AceLan, I guess that you hit this easy while testing on a (development)
Meteor Lake platform ?

I have had other/similar reports about Meteor Lake platforms.

On hw from the last 10 years dell-laptop will not register
its vendor-type backlight class device because
acpi_video_get_backlight_type() will return acpi_backlight_video
there (1) so it does not matter if the GPU driver shows up only
later (2).

But it seems that on Meteor Lake the ACPI tables will no longer
contain acpi_video backlight control support which causes
acpi_video_get_backlight_type() to return acpi_backlight_vendor (2).
triggering the issue you are seeing.

Can you give the attached patch a try please ?

Regards,

Hans


1) Starting with kernel >= 6.2 acpi_video.c will only register
the /sys/class/backlight/acpi_video# node after a drm/kms drivers
asks it to register it.

2) The native GPU driver will tell the drivers/acpi/video_detect.c
code that native backlight control is available changing
the return of acpi_video_get_backlight_type() to native, which
is what loading the native GPU driver first also fixes this issue.
From 59f48a8deb525d9c7513e2c0dffc7f30a4356030 Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@xxxxxxxxxx>
Date: Wed, 7 Jun 2023 20:33:12 +0200
Subject: [PATCH] ACPI: video: Stop trying to use vendor backlight control on
 laptops from after ~2012

There have been 2 separate reports now about a non working
"dell_backlight" device getting registered under /sys/class/backlight
with MeteorLake (development) platforms.

On hw from the last 10 years dell-laptop will not register "dell_backlight"
because acpi_video_get_backlight_type() will return acpi_backlight_video
there if called before the GPU/kms driver loads. So it does not matter if
the GPU driver's native backlight gets registered after dell-laptop loads.

But it seems that on Meteor Lake the ACPI tables will no longer
contain acpi_video backlight control support which causes
acpi_video_get_backlight_type() to return acpi_backlight_vendor causing
"dell_backlight" to get registered if the dell-laptop module is loaded
before the GPU/kms driver.

Vendor specific backlight control like the "dell_backlight" device is
only necessary on quite old hw (from before acpi_video backlight control
was introduced). Work around "dell_backlight" registering on very new
hw (where acpi_video backlight control seems to be no more) by making
acpi_video_get_backlight_type() use acpi_backlight_none instead
of acpi_backlight_vendor as final fallback when the ACPI tables have
support for Windows 8 or later (laptops from after ~2012).

Suggested-by: Matthew Garrett <mjg59@xxxxxxxxxxxxx>
Reported-by: AceLan Kao <acelan.kao@xxxxxxxxxxxxx>
Closes: https://lore.kernel.org/platform-driver-x86/20230607034331.576623-1-acelan.kao@xxxxxxxxxxxxx/
Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
---
 drivers/acpi/video_detect.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c
index b87783c5872d..eb014c0eba42 100644
--- a/drivers/acpi/video_detect.c
+++ b/drivers/acpi/video_detect.c
@@ -844,6 +844,27 @@ enum acpi_backlight_type __acpi_video_get_backlight_type(bool native, bool *auto
 	if (native_available)
 		return acpi_backlight_native;
 
+	/*
+	 * The vendor specific BIOS interfaces are only necessary for
+	 * laptops from before ~2008.
+	 *
+	 * For laptops from ~2008 till ~2023 this point is never reached
+	 * because on those (video_caps & ACPI_VIDEO_BACKLIGHT) above is true.
+	 *
+	 * Laptops from after ~2023 no longer support ACPI_VIDEO_BACKLIGHT,
+	 * if this point is reached on those, this likely means that
+	 * the GPU kms driver which sets native_available has not loaded yet.
+	 *
+	 * Returning acpi_backlight_vendor in this case is known to sometimes
+	 * cause a non working vendor specific /sys/class/backlight device to
+	 * get registered.
+	 *
+	 * Return acpi_backlight_none on laptops with ACPI tables written
+	 * for Windows 8 (laptops from after ~2012) to avoid this problem.
+	 */
+	if (acpi_osi_is_win8())
+		return acpi_backlight_none;
+
 	/* No ACPI video/native (old hw), use vendor specific fw methods. */
 	return acpi_backlight_vendor;
 }
-- 
2.40.1