2023-06-07 04:00:12

by AceLan Kao

[permalink] [raw]
Subject: [PATCH] platform/x86: dell-laptop: Add drm module soft dependency

From: "Chia-Lin Kao (AceLan)" <[email protected]>

dell_laptop is somethines loaded before nvidia driver, causing it to
create its own backlight interface before native backlight is set.
This results in the presence of 2 backlight interfaces in sysfs and
leads to the backlight can't be adjusted.
To work around this issue, delaying the loading of dell_laptop until
after drm module has been loaded.

Signed-off-by: Chia-Lin Kao (AceLan) <[email protected]>
---
drivers/platform/x86/dell/dell-laptop.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/platform/x86/dell/dell-laptop.c b/drivers/platform/x86/dell/dell-laptop.c
index 1321687d923e..535e6c3df529 100644
--- a/drivers/platform/x86/dell/dell-laptop.c
+++ b/drivers/platform/x86/dell/dell-laptop.c
@@ -2316,6 +2316,7 @@ static void __exit dell_exit(void)
late_initcall(dell_init);
module_exit(dell_exit);

+MODULE_SOFTDEP("post: drm");
MODULE_AUTHOR("Matthew Garrett <[email protected]>");
MODULE_AUTHOR("Gabriele Mazzotta <[email protected]>");
MODULE_AUTHOR("Pali Rohár <[email protected]>");
--
2.34.1



2023-06-07 05:03:02

by Matthew Garrett

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

On Wed, Jun 07, 2023 at 11:43:31AM +0800, AceLan Kao wrote:
> From: "Chia-Lin Kao (AceLan)" <[email protected]>
>
> dell_laptop is somethines loaded before nvidia driver, causing it to
> create its own backlight interface before native backlight is set.
> This results in the presence of 2 backlight interfaces in sysfs and
> leads to the backlight can't be adjusted.

It seems like this approach would still be broken if the nvidia module
isn't available at the time the dell module is loaded?

2023-06-07 05:34:29

by Matthew Garrett

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

On Wed, Jun 07, 2023 at 01:19:40PM +0800, AceLan Kao wrote:
> Gfx drivers(i915/amdgpu/nvidia) depend on the drm driver, so delaying
> the loading of dell_laptop after drm can ease the issue the most.
> Right, it's still possible to encounter the issue, unfortunately, we
> do not have a better solution for it at the moment.

We could unregister inappropriate backlight drivers when a more
appropriate one is loaded, or the policy decision around which driver to
use could be made in userland?

2023-06-07 05:39:05

by AceLan Kao

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

Gfx drivers(i915/amdgpu/nvidia) depend on the drm driver, so delaying
the loading of dell_laptop after drm can ease the issue the most.
Right, it's still possible to encounter the issue, unfortunately, we
do not have a better solution for it at the moment.


Matthew Garrett <[email protected]> 於 2023年6月7日 週三 下午12:20寫道:
>
> On Wed, Jun 07, 2023 at 11:43:31AM +0800, AceLan Kao wrote:
> > From: "Chia-Lin Kao (AceLan)" <[email protected]>
> >
> > dell_laptop is somethines loaded before nvidia driver, causing it to
> > create its own backlight interface before native backlight is set.
> > This results in the presence of 2 backlight interfaces in sysfs and
> > leads to the backlight can't be adjusted.
>
> It seems like this approach would still be broken if the nvidia module
> isn't available at the time the dell module is loaded?

2023-06-07 06:27:29

by AceLan Kao

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

Matthew Garrett <[email protected]> 於 2023年6月7日 週三 下午1:27寫道:
>
> On Wed, Jun 07, 2023 at 01:19:40PM +0800, AceLan Kao wrote:
> > Gfx drivers(i915/amdgpu/nvidia) depend on the drm driver, so delaying
> > the loading of dell_laptop after drm can ease the issue the most.
> > Right, it's still possible to encounter the issue, unfortunately, we
> > do not have a better solution for it at the moment.
>
> We could unregister inappropriate backlight drivers when a more
> appropriate one is loaded, or the policy decision around which driver to
> use could be made in userland?
It's hard to decide which backlight driver is redundant, and it's kind of ugly
to unregister the backlight driver which is registered by other driver and maybe
problematic.

I'm not familiar with userland, but I think we should handle this
issue within the
kernel and aim to register only one functional backlight driver.

2023-06-07 06:33:15

by Matthew Garrett

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

On Wed, Jun 07, 2023 at 02:13:31PM +0800, AceLan Kao wrote:
> Matthew Garrett <[email protected]> 於 2023年6月7日 週三 下午1:27寫道:
> >
> > On Wed, Jun 07, 2023 at 01:19:40PM +0800, AceLan Kao wrote:
> > > Gfx drivers(i915/amdgpu/nvidia) depend on the drm driver, so delaying
> > > the loading of dell_laptop after drm can ease the issue the most.
> > > Right, it's still possible to encounter the issue, unfortunately, we
> > > do not have a better solution for it at the moment.
> >
> > We could unregister inappropriate backlight drivers when a more
> > appropriate one is loaded, or the policy decision around which driver to
> > use could be made in userland?
> It's hard to decide which backlight driver is redundant, and it's kind of ugly
> to unregister the backlight driver which is registered by other driver and maybe
> problematic.

But you're relying on registering the working backlight first, which is
an inherently racy thing? We shouldn't be relying on order of
initialisation to make this work, either we should only export a working
interface or we should expose enough information for whatever is using
the interfaces to make an appropriate policy decision itself.

2023-06-07 07:25:36

by Pali Rohár

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

On Wednesday 07 June 2023 07:23:41 Matthew Garrett wrote:
> On Wed, Jun 07, 2023 at 02:13:31PM +0800, AceLan Kao wrote:
> > Matthew Garrett <[email protected]> 於 2023年6月7日 週三 下午1:27寫道:
> > >
> > > On Wed, Jun 07, 2023 at 01:19:40PM +0800, AceLan Kao wrote:
> > > > Gfx drivers(i915/amdgpu/nvidia) depend on the drm driver, so delaying
> > > > the loading of dell_laptop after drm can ease the issue the most.
> > > > Right, it's still possible to encounter the issue, unfortunately, we
> > > > do not have a better solution for it at the moment.
> > >
> > > We could unregister inappropriate backlight drivers when a more
> > > appropriate one is loaded, or the policy decision around which driver to
> > > use could be made in userland?
> > It's hard to decide which backlight driver is redundant, and it's kind of ugly
> > to unregister the backlight driver which is registered by other driver and maybe
> > problematic.
>
> But you're relying on registering the working backlight first, which is
> an inherently racy thing? We shouldn't be relying on order of
> initialisation to make this work, either we should only export a working
> interface or we should expose enough information for whatever is using
> the interfaces to make an appropriate policy decision itself.

IIRC, drm drivers unregister redundant fbcon drivers (vesafb), so cannot
drm drivers use similar strategy also for backlight drivers and
unregister the redundant? If every backlight driver would have some
"flag" which say if it should be unregistered by drm then maybe it could
work? Or are there some other pitfalls?

2023-06-07 07:46:26

by AceLan Kao

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

Pali Rohár <[email protected]> 於 2023年6月7日 週三 下午2:56寫道:
>
> On Wednesday 07 June 2023 07:23:41 Matthew Garrett wrote:
> > On Wed, Jun 07, 2023 at 02:13:31PM +0800, AceLan Kao wrote:
> > > Matthew Garrett <[email protected]> 於 2023年6月7日 週三 下午1:27寫道:
> > > >
> > > > On Wed, Jun 07, 2023 at 01:19:40PM +0800, AceLan Kao wrote:
> > > > > Gfx drivers(i915/amdgpu/nvidia) depend on the drm driver, so delaying
> > > > > the loading of dell_laptop after drm can ease the issue the most.
> > > > > Right, it's still possible to encounter the issue, unfortunately, we
> > > > > do not have a better solution for it at the moment.
> > > >
> > > > We could unregister inappropriate backlight drivers when a more
> > > > appropriate one is loaded, or the policy decision around which driver to
> > > > use could be made in userland?
> > > It's hard to decide which backlight driver is redundant, and it's kind of ugly
> > > to unregister the backlight driver which is registered by other driver and maybe
> > > problematic.
> >
> > But you're relying on registering the working backlight first, which is
> > an inherently racy thing? We shouldn't be relying on order of
> > initialisation to make this work, either we should only export a working
> > interface or we should expose enough information for whatever is using
> > the interfaces to make an appropriate policy decision itself.
>
> IIRC, drm drivers unregister redundant fbcon drivers (vesafb), so cannot
> drm drivers use similar strategy also for backlight drivers and
> unregister the redundant? If every backlight driver would have some
> "flag" which say if it should be unregistered by drm then maybe it could
> work? Or are there some other pitfalls?
Matthew,

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.

Pali,

No, it doesn't work by doing this in the drm driver if the backlight driver
is registered after the drm driver has been loaded.

2023-06-07 08:14:48

by Matthew Garrett

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

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.

2023-06-07 19:27:44

by Hans de Goede

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

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.


Attachments:
0001-ACPI-video-Stop-trying-to-use-vendor-backlight-contr.patch (3.06 kB)

2023-06-08 03:25:36

by AceLan Kao

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

Hans de Goede <[email protected]> 於 2023年6月8日 週四 上午3:16寫道:
>
> 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.

Hi Hans,

Yes, this patch works for me, thanks.

BTW, I encountered this issue on the RPL platform.

2023-06-08 09:29:26

by Hans de Goede

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

Hi AceLan,

On 6/8/23 05:04, AceLan Kao wrote:
> Hans de Goede <[email protected]> 於 2023年6月8日 週四 上午3:16寫道:
>>
>> 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 why loading the native GPU driver first also fixes this issue.
>
> Hi Hans,
>
> Yes, this patch works for me, thanks.
>
> BTW, I encountered this issue on the RPL platform.

Thank you for testing. I have updated the commit message
to reflect that this impacts both RPL and MTL platforms
and submitted the fix upstream:

https://lore.kernel.org/linux-acpi/[email protected]/

Regards,

Hans


2023-06-13 07:43:47

by AceLan Kao

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

Hans de Goede <[email protected]> 於 2023年6月8日 週四 下午5:16寫道:
>
> Hi AceLan,
>
> On 6/8/23 05:04, AceLan Kao wrote:
> > Hans de Goede <[email protected]> 於 2023年6月8日 週四 上午3:16寫道:
> >>
> >> 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 why loading the native GPU driver first also fixes this issue.
> >
> > Hi Hans,
> >
> > Yes, this patch works for me, thanks.
> >
> > BTW, I encountered this issue on the RPL platform.
>
> Thank you for testing. I have updated the commit message
> to reflect that this impacts both RPL and MTL platforms
> and submitted the fix upstream:
>
> https://lore.kernel.org/linux-acpi/[email protected]/
>
> Regards,
>
> Hans
>

Hi Hans,

I got another issue on the same platform.
The first issue was that when set to DSC graphics only in the BIOS,
I encountered the issue I reported here, dell_laptop creates dell_backlight
and then nvidia creates nvidia_0 later.

Now, set to hybrid mode in the BIOS, I found I still got 2 backlight interfaces
$ ls /sys/class/backlight/
acpi_video0 intel_backlight
acpi_video0 is redundant and non-working.

Do you think should I set this platform to the dmi quirk? Or is there anything
I could try to get rid of this?

Thanks.

2023-06-13 09:11:44

by Hans de Goede

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

Hi AceLan,

On 6/13/23 09:30, AceLan Kao wrote:
> Hans de Goede <[email protected]> 於 2023年6月8日 週四 下午5:16寫道:
>>
>> Hi AceLan,
>>
>> On 6/8/23 05:04, AceLan Kao wrote:
>>> Hans de Goede <[email protected]> 於 2023年6月8日 週四 上午3:16寫道:
>>>>
>>>> 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 why loading the native GPU driver first also fixes this issue.
>>>
>>> Hi Hans,
>>>
>>> Yes, this patch works for me, thanks.
>>>
>>> BTW, I encountered this issue on the RPL platform.
>>
>> Thank you for testing. I have updated the commit message
>> to reflect that this impacts both RPL and MTL platforms
>> and submitted the fix upstream:
>>
>> https://lore.kernel.org/linux-acpi/[email protected]/
>>
>> Regards,
>>
>> Hans
>>
>
> Hi Hans,
>
> I got another issue on the same platform.
> The first issue was that when set to DSC graphics only in the BIOS,
> I encountered the issue I reported here, dell_laptop creates dell_backlight
> and then nvidia creates nvidia_0 later.
>
> Now, set to hybrid mode in the BIOS, I found I still got 2 backlight interfaces
> $ ls /sys/class/backlight/
> acpi_video0 intel_backlight
> acpi_video0 is redundant and non-working.
>
> Do you think should I set this platform to the dmi quirk? Or is there anything
> I could try to get rid of this?


This is very hard to answer without more info.

For starters please make sure that you are testing with the latest kernel and
provide full dmesg output for a boot with the BIOS set to hybrid mode.

Regards,

Hans



2023-06-15 02:21:31

by AceLan Kao

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

Hi Hans,

Hans de Goede <[email protected]> 於 2023年6月13日 週二 下午4:54寫道:
>
> Hi AceLan,
>
> On 6/13/23 09:30, AceLan Kao wrote:
> > Hans de Goede <[email protected]> 於 2023年6月8日 週四 下午5:16寫道:
> >>
> >> Hi AceLan,
> >>
> >> On 6/8/23 05:04, AceLan Kao wrote:
> >>> Hans de Goede <[email protected]> 於 2023年6月8日 週四 上午3:16寫道:
> >>>>
> >>>> 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 why loading the native GPU driver first also fixes this issue.
> >>>
> >>> Hi Hans,
> >>>
> >>> Yes, this patch works for me, thanks.
> >>>
> >>> BTW, I encountered this issue on the RPL platform.
> >>
> >> Thank you for testing. I have updated the commit message
> >> to reflect that this impacts both RPL and MTL platforms
> >> and submitted the fix upstream:
> >>
> >> https://lore.kernel.org/linux-acpi/[email protected]/
> >>
> >> Regards,
> >>
> >> Hans
> >>
> >
> > Hi Hans,
> >
> > I got another issue on the same platform.
> > The first issue was that when set to DSC graphics only in the BIOS,
> > I encountered the issue I reported here, dell_laptop creates dell_backlight
> > and then nvidia creates nvidia_0 later.
> >
> > Now, set to hybrid mode in the BIOS, I found I still got 2 backlight interfaces
> > $ ls /sys/class/backlight/
> > acpi_video0 intel_backlight
> > acpi_video0 is redundant and non-working.
> >
> > Do you think should I set this platform to the dmi quirk? Or is there anything
> > I could try to get rid of this?
>
>
> This is very hard to answer without more info.
>
> For starters please make sure that you are testing with the latest kernel and
> provide full dmesg output for a boot with the BIOS set to hybrid mode.
>
> Regards,
>
> Hans
>
Sorry to bother you, the issue has been fixed by the below commit.
e506731c8f35 ("ACPI: video: Make acpi_backlight=video work independent
from GPU driver")
Thanks.