[PATCH] ACPI: video: Add backlight=native DMI quirk for Dell G15 5515

[PATCH] ACPI: video: Add backlight=native DMI quirk for Dell G15 5515

[Hans de Goede]

The Dell G15 5515 has the WMI interface (and WMI call returns) expected
by the nvidia-wmi-ec-backlight interface. But the backlight class device
registered by the nvidia-wmi-ec-backlight driver does not actually work.

The amdgpu_bl0 native GPU backlight class device does actually work,
add a backlight=native DMI quirk for this.

Cc: Daniel Dadap <ddadap@nvidia.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/acpi/video_detect.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c
index 841f6213b4de..88acc09773bb 100644
--- a/drivers/acpi/video_detect.c
+++ b/drivers/acpi/video_detect.c
@@ -645,6 +645,16 @@ static const struct dmi_system_id video_detect_dmi_table[] = {
 		},
 	},
 
+	/* Models which have nvidia-ec-wmi support, but should not use it. */
+	{
+	 .callback = video_detect_force_native,
+	 /* Dell G15 5515 */
+	 .matches = {
+		DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+		DMI_MATCH(DMI_PRODUCT_NAME, "Dell G15 5515"),
+		},
+	},
+
 	/*
 	 * Desktops which falsely report a backlight and which our heuristics
 	 * for this do not catch.
-- 
2.37.3

RE: [PATCH] ACPI: video: Add backlight=native DMI quirk for Dell G15 5515

[Limonciello, Mario]

[Public]

> -----Original Message-----
> From: Hans de Goede <hdegoede@redhat.com>
> Sent: Monday, October 31, 2022 16:26
> To: Rafael J . Wysocki <rafael@kernel.org>
> Cc: Hans de Goede <hdegoede@redhat.com>; Len Brown
> <lenb@kernel.org>; linux-acpi@vger.kernel.org; platform-driver-
> x86@vger.kernel.org; Daniel Dadap <ddadap@nvidia.com>
> Subject: [PATCH] ACPI: video: Add backlight=native DMI quirk for Dell G15
> 5515
> 
> The Dell G15 5515 has the WMI interface (and WMI call returns) expected
> by the nvidia-wmi-ec-backlight interface. But the backlight class device
> registered by the nvidia-wmi-ec-backlight driver does not actually work.
> 

Is there a way to detect this from nvidia-wmi-ec-backlight?  I would think it's
cleaner to have that driver return -ENODEV so you end up with just the "working"
amdgpu_bl0.

> The amdgpu_bl0 native GPU backlight class device does actually work,
> add a backlight=native DMI quirk for this.
> 
> Cc: Daniel Dadap <ddadap@nvidia.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/acpi/video_detect.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c
> index 841f6213b4de..88acc09773bb 100644
> --- a/drivers/acpi/video_detect.c
> +++ b/drivers/acpi/video_detect.c
> @@ -645,6 +645,16 @@ static const struct dmi_system_id
> video_detect_dmi_table[] = {
>  		},
>  	},
> 
> +	/* Models which have nvidia-ec-wmi support, but should not use it.
> */
> +	{
> +	 .callback = video_detect_force_native,
> +	 /* Dell G15 5515 */
> +	 .matches = {
> +		DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
> +		DMI_MATCH(DMI_PRODUCT_NAME, "Dell G15 5515"),
> +		},
> +	},
> +
>  	/*
>  	 * Desktops which falsely report a backlight and which our heuristics
>  	 * for this do not catch.
> --
> 2.37.3

Re: [PATCH] ACPI: video: Add backlight=native DMI quirk for Dell G15 5515

[Daniel Dadap]

Thanks, Hans:

On Mon, Oct 31, 2022 at 10:25:56PM +0100, Hans de Goede wrote:
> The Dell G15 5515 has the WMI interface (and WMI call returns) expected
> by the nvidia-wmi-ec-backlight interface. But the backlight class device
> registered by the nvidia-wmi-ec-backlight driver does not actually work.
> 
> The amdgpu_bl0 native GPU backlight class device does actually work,
> add a backlight=native DMI quirk for this.

I'm not certain whether you have direct access to the system, but I am
curious as to what mode the system is in when it exposes a non-working
WMI backlight interface. The expectation is that the WMI EC backlight
interface should only be exposed when the system is in dynamic mode
(different vendors may call this differeint things in the EFI setup
menus), and the native backlight interfaces for the GPUs, be they
discrete or integrated, should only be exposed when a mode that keeps
only one of the GPUs connected to the panel; i.e., in discrete-only mode
the discrete GPU should have a working native backlight interface, and
in integrated-only mode, or a hybrid mode which does not allow dynamic
mux switching, the integrated GPU should have a working native backlight
interface.

For this notebook the AMD GPU would be the integrated one, so if the
system is *not* in dynamic switch mode, and the WMI interface is
reporting that the backlight should be handled by the EC, then that is
certainly a bug in the system, and this would be the correct workaround,
when the system is operating in an iGPU-only or non-dynamic hybrid mode.
However, if the system is switched to a dynamic mode, then this would
break the EC backlight, which should be the corect backlight interface
for that case. Another possible system bug would be if the condition
described in the change description occurs when the system *is* in a
dynamic mode, but is routing backlight control to the iGPU anyway. That
would be a messier bug to deal with, since it would also be necessary to
determine which backlight interface is functional when the display is
switched over to the dGPU.

If it's not too much trouble could you (or the reporter, if you don't
have the system) check which backlight interfaces are exposed, and which
of those actually work, for each of the different hybrid GPU modes? I am
not certain what they would be called on the setup screen, but the
possible modes should be:

* Integrated only
* Discrete only
* Hybrid, without dynamic switching
* Hybrid, with dynamic switching

, or a subset thereof. Usually I would expect the minimal subset to be
discrete only plus one or both of the hybrid modes. Since we're not sure
what these are called on the setup screen it is probably best to simply
report the names of the mode as they are listed in the UI.

> Cc: Daniel Dadap <ddadap@nvidia.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/acpi/video_detect.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c
> index 841f6213b4de..88acc09773bb 100644
> --- a/drivers/acpi/video_detect.c
> +++ b/drivers/acpi/video_detect.c
> @@ -645,6 +645,16 @@ static const struct dmi_system_id video_detect_dmi_table[] = {
>  		},
>  	},
>  
> +	/* Models which have nvidia-ec-wmi support, but should not use it. */
> +	{
> +	 .callback = video_detect_force_native,
> +	 /* Dell G15 5515 */
> +	 .matches = {
> +		DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
> +		DMI_MATCH(DMI_PRODUCT_NAME, "Dell G15 5515"),
> +		},
> +	},
> +
>  	/*
>  	 * Desktops which falsely report a backlight and which our heuristics
>  	 * for this do not catch.
> -- 
> 2.37.3
> 

Re: [PATCH] ACPI: video: Add backlight=native DMI quirk for Dell G15 5515

[Daniel Dadap]

On Mon, Oct 31, 2022 at 09:51:55PM +0000, Limonciello, Mario wrote:
> [Public]
> 
> > -----Original Message-----
> > From: Hans de Goede <hdegoede@redhat.com>
> > Sent: Monday, October 31, 2022 16:26
> > To: Rafael J . Wysocki <rafael@kernel.org>
> > Cc: Hans de Goede <hdegoede@redhat.com>; Len Brown
> > <lenb@kernel.org>; linux-acpi@vger.kernel.org; platform-driver-
> > x86@vger.kernel.org; Daniel Dadap <ddadap@nvidia.com>
> > Subject: [PATCH] ACPI: video: Add backlight=native DMI quirk for Dell G15
> > 5515
> > 
> > The Dell G15 5515 has the WMI interface (and WMI call returns) expected
> > by the nvidia-wmi-ec-backlight interface. But the backlight class device
> > registered by the nvidia-wmi-ec-backlight driver does not actually work.
> > 
> 
> Is there a way to detect this from nvidia-wmi-ec-backlight?  I would think it's
> cleaner to have that driver return -ENODEV so you end up with just the "working"
> amdgpu_bl0.

That would be ideal, although I'm not certain whether this would work
with the new backlight control type selection model where video.ko
determines what type of backlight interface the system is supposed to
have. I am assuming that the below patch is against a kernel predating
the recent changes that Hans made to rework the backlight subsystem,
because IIUC it shouldn't be possible for both the EC backlight driver
and the iGPU's native one to both be active at the same time. I would
expect that without this patch, there would be no working backlight
interfaces on this system when running a newer kernel, since
__acpi_video_get_backlight_type() would return
acpi_backlight_nvidia_wmi_ec, which means
acpi_video_backlight_use_native() would return false, and the iGPU
native driver should not be registering a brightness handler.
 
> > The amdgpu_bl0 native GPU backlight class device does actually work,
> > add a backlight=native DMI quirk for this.
> > 
> > Cc: Daniel Dadap <ddadap@nvidia.com>
> > Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> > ---
> >  drivers/acpi/video_detect.c | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c
> > index 841f6213b4de..88acc09773bb 100644
> > --- a/drivers/acpi/video_detect.c
> > +++ b/drivers/acpi/video_detect.c
> > @@ -645,6 +645,16 @@ static const struct dmi_system_id
> > video_detect_dmi_table[] = {
> >  		},
> >  	},
> > 
> > +	/* Models which have nvidia-ec-wmi support, but should not use it.
> > */
> > +	{
> > +	 .callback = video_detect_force_native,
> > +	 /* Dell G15 5515 */
> > +	 .matches = {
> > +		DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
> > +		DMI_MATCH(DMI_PRODUCT_NAME, "Dell G15 5515"),
> > +		},
> > +	},
> > +
> >  	/*
> >  	 * Desktops which falsely report a backlight and which our heuristics
> >  	 * for this do not catch.
> > --
> > 2.37.3

Re: [PATCH] ACPI: video: Add backlight=native DMI quirk for Dell G15 5515

[Hans de Goede]

Hi,

On 10/31/22 23:09, Daniel Dadap wrote:
> Thanks, Hans:
> 
> On Mon, Oct 31, 2022 at 10:25:56PM +0100, Hans de Goede wrote:
>> The Dell G15 5515 has the WMI interface (and WMI call returns) expected
>> by the nvidia-wmi-ec-backlight interface. But the backlight class device
>> registered by the nvidia-wmi-ec-backlight driver does not actually work.
>>
>> The amdgpu_bl0 native GPU backlight class device does actually work,
>> add a backlight=native DMI quirk for this.
> 
> I'm not certain whether you have direct access to the system, but I am
> curious as to what mode the system is in when it exposes a non-working
> WMI backlight interface. The expectation is that the WMI EC backlight
> interface should only be exposed when the system is in dynamic mode
> (different vendors may call this differeint things in the EFI setup
> menus), and the native backlight interfaces for the GPUs, be they
> discrete or integrated, should only be exposed when a mode that keeps
> only one of the GPUs connected to the panel; i.e., in discrete-only mode
> the discrete GPU should have a working native backlight interface, and
> in integrated-only mode, or a hybrid mode which does not allow dynamic
> mux switching, the integrated GPU should have a working native backlight
> interface.
> 
> For this notebook the AMD GPU would be the integrated one, so if the
> system is *not* in dynamic switch mode, and the WMI interface is
> reporting that the backlight should be handled by the EC, then that is
> certainly a bug in the system, and this would be the correct workaround,
> when the system is operating in an iGPU-only or non-dynamic hybrid mode.
> However, if the system is switched to a dynamic mode, then this would
> break the EC backlight, which should be the corect backlight interface
> for that case. Another possible system bug would be if the condition
> described in the change description occurs when the system *is* in a
> dynamic mode, but is routing backlight control to the iGPU anyway. That
> would be a messier bug to deal with, since it would also be necessary to
> determine which backlight interface is functional when the display is
> switched over to the dGPU.
> 
> If it's not too much trouble could you (or the reporter, if you don't
> have the system) check which backlight interfaces are exposed, and which
> of those actually work, for each of the different hybrid GPU modes? I am
> not certain what they would be called on the setup screen, but the
> possible modes should be:
> 
> * Integrated only
> * Discrete only
> * Hybrid, without dynamic switching
> * Hybrid, with dynamic switching
> 
> , or a subset thereof. Usually I would expect the minimal subset to be
> discrete only plus one or both of the hybrid modes. Since we're not sure
> what these are called on the setup screen it is probably best to simply
> report the names of the mode as they are listed in the UI.

Isn't the whole idea behind nvidia-wmi-ec-backlight that the backlight's
pwm input is connected to the EC (it is sorta in the name) and then always
is controlled by the EC independent of the mode ?

At least I was under the impression that that was the whole idea?

I will forward your email to the reporter and ask them to get
in touch with you and then lets see from there.

Regards,

Hans



> 
>> Cc: Daniel Dadap <ddadap@nvidia.com>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>  drivers/acpi/video_detect.c | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>>
>> diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c
>> index 841f6213b4de..88acc09773bb 100644
>> --- a/drivers/acpi/video_detect.c
>> +++ b/drivers/acpi/video_detect.c
>> @@ -645,6 +645,16 @@ static const struct dmi_system_id video_detect_dmi_table[] = {
>>  		},
>>  	},
>>  
>> +	/* Models which have nvidia-ec-wmi support, but should not use it. */
>> +	{
>> +	 .callback = video_detect_force_native,
>> +	 /* Dell G15 5515 */
>> +	 .matches = {
>> +		DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
>> +		DMI_MATCH(DMI_PRODUCT_NAME, "Dell G15 5515"),
>> +		},
>> +	},
>> +
>>  	/*
>>  	 * Desktops which falsely report a backlight and which our heuristics
>>  	 * for this do not catch.
>> -- 
>> 2.37.3
>>
> 

Re: [PATCH] ACPI: video: Add backlight=native DMI quirk for Dell G15 5515

[Hans de Goede]

Hi,

On 10/31/22 23:20, Daniel Dadap wrote:
> On Mon, Oct 31, 2022 at 09:51:55PM +0000, Limonciello, Mario wrote:
>> [Public]
>>
>>> -----Original Message-----
>>> From: Hans de Goede <hdegoede@redhat.com>
>>> Sent: Monday, October 31, 2022 16:26
>>> To: Rafael J . Wysocki <rafael@kernel.org>
>>> Cc: Hans de Goede <hdegoede@redhat.com>; Len Brown
>>> <lenb@kernel.org>; linux-acpi@vger.kernel.org; platform-driver-
>>> x86@vger.kernel.org; Daniel Dadap <ddadap@nvidia.com>
>>> Subject: [PATCH] ACPI: video: Add backlight=native DMI quirk for Dell G15
>>> 5515
>>>
>>> The Dell G15 5515 has the WMI interface (and WMI call returns) expected
>>> by the nvidia-wmi-ec-backlight interface. But the backlight class device
>>> registered by the nvidia-wmi-ec-backlight driver does not actually work.
>>>
>>
>> Is there a way to detect this from nvidia-wmi-ec-backlight?  I would think it's
>> cleaner to have that driver return -ENODEV so you end up with just the "working"
>> amdgpu_bl0.
> 
> That would be ideal, although I'm not certain whether this would work
> with the new backlight control type selection model where video.ko
> determines what type of backlight interface the system is supposed to
> have. I am assuming that the below patch is against a kernel predating
> the recent changes that Hans made to rework the backlight subsystem,
> because IIUC it shouldn't be possible for both the EC backlight driver
> and the iGPU's native one to both be active at the same time. I would
> expect that without this patch, there would be no working backlight
> interfaces on this system when running a newer kernel, since
> __acpi_video_get_backlight_type() would return
> acpi_backlight_nvidia_wmi_ec, which means
> acpi_video_backlight_use_native() would return false, and the iGPU
> native driver should not be registering a brightness handler.

The reporter needs to blacklist the nvidia-wmi-ec-backlight module
because with 6.0 they otherwise get both that backlight + the native
amdgpu backlight in /sys/class/backlight and then userspace will
prefer the nvidia-wmi-ec-backlight one which according to the reporter
does not work.

You are correct that with 6.1 the issue is that the native driver
now honors the acpi_video_get_backlight_type() return and no longer
registers. This is fixed by this patch.

The good news is that with 6.1 nvidia-wmi-ec-backlight now also honors
acpi_video_get_backlight_type() so the quirk also avoids the need
to blacklist nvidia-wmi-ec-backlight and with the quirk in 6.1 everything
just works OOTB (with the current BIOS settings for the GPU mode).

Regards,

Hans


>  
>>> The amdgpu_bl0 native GPU backlight class device does actually work,
>>> add a backlight=native DMI quirk for this.
>>>
>>> Cc: Daniel Dadap <ddadap@nvidia.com>
>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>> ---
>>>  drivers/acpi/video_detect.c | 10 ++++++++++
>>>  1 file changed, 10 insertions(+)
>>>
>>> diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c
>>> index 841f6213b4de..88acc09773bb 100644
>>> --- a/drivers/acpi/video_detect.c
>>> +++ b/drivers/acpi/video_detect.c
>>> @@ -645,6 +645,16 @@ static const struct dmi_system_id
>>> video_detect_dmi_table[] = {
>>>  		},
>>>  	},
>>>
>>> +	/* Models which have nvidia-ec-wmi support, but should not use it.
>>> */
>>> +	{
>>> +	 .callback = video_detect_force_native,
>>> +	 /* Dell G15 5515 */
>>> +	 .matches = {
>>> +		DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
>>> +		DMI_MATCH(DMI_PRODUCT_NAME, "Dell G15 5515"),
>>> +		},
>>> +	},
>>> +
>>>  	/*
>>>  	 * Desktops which falsely report a backlight and which our heuristics
>>>  	 * for this do not catch.
>>> --
>>> 2.37.3
> 

Re: [PATCH] ACPI: video: Add backlight=native DMI quirk for Dell G15 5515

[Hans de Goede]

Hi Daniel,

On 10/31/22 23:09, Daniel Dadap wrote:
> Thanks, Hans:
> 
> On Mon, Oct 31, 2022 at 10:25:56PM +0100, Hans de Goede wrote:
>> The Dell G15 5515 has the WMI interface (and WMI call returns) expected
>> by the nvidia-wmi-ec-backlight interface. But the backlight class device
>> registered by the nvidia-wmi-ec-backlight driver does not actually work.
>>
>> The amdgpu_bl0 native GPU backlight class device does actually work,
>> add a backlight=native DMI quirk for this.
> 
> I'm not certain whether you have direct access to the system, but I am
> curious as to what mode the system is in when it exposes a non-working
> WMI backlight interface. The expectation is that the WMI EC backlight
> interface should only be exposed when the system is in dynamic mode
> (different vendors may call this differeint things in the EFI setup
> menus), and the native backlight interfaces for the GPUs, be they
> discrete or integrated, should only be exposed when a mode that keeps
> only one of the GPUs connected to the panel; i.e., in discrete-only mode
> the discrete GPU should have a working native backlight interface, and
> in integrated-only mode, or a hybrid mode which does not allow dynamic
> mux switching, the integrated GPU should have a working native backlight
> interface.
> 
> For this notebook the AMD GPU would be the integrated one, so if the
> system is *not* in dynamic switch mode, and the WMI interface is
> reporting that the backlight should be handled by the EC, then that is
> certainly a bug in the system, and this would be the correct workaround,
> when the system is operating in an iGPU-only or non-dynamic hybrid mode.
> However, if the system is switched to a dynamic mode, then this would
> break the EC backlight, which should be the corect backlight interface
> for that case. Another possible system bug would be if the condition
> described in the change description occurs when the system *is* in a
> dynamic mode, but is routing backlight control to the iGPU anyway. That
> would be a messier bug to deal with, since it would also be necessary to
> determine which backlight interface is functional when the display is
> switched over to the dGPU.
> 
> If it's not too much trouble could you (or the reporter, if you don't
> have the system) check which backlight interfaces are exposed, and which
> of those actually work, for each of the different hybrid GPU modes? I am
> not certain what they would be called on the setup screen, but the
> possible modes should be:
> 
> * Integrated only
> * Discrete only
> * Hybrid, without dynamic switching
> * Hybrid, with dynamic switching
> 
> , or a subset thereof. Usually I would expect the minimal subset to be
> discrete only plus one or both of the hybrid modes. Since we're not sure
> what these are called on the setup screen it is probably best to simply
> report the names of the mode as they are listed in the UI.

I have heard back from the reporter about this and they report that
there is no option in the BIOS to select the mode. And currently the
machine is running in Hybrid mode, with it being unclear if this
supports dynamic switching or not.

Regards,

Hans



> 
>> Cc: Daniel Dadap <ddadap@nvidia.com>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>  drivers/acpi/video_detect.c | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>>
>> diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c
>> index 841f6213b4de..88acc09773bb 100644
>> --- a/drivers/acpi/video_detect.c
>> +++ b/drivers/acpi/video_detect.c
>> @@ -645,6 +645,16 @@ static const struct dmi_system_id video_detect_dmi_table[] = {
>>  		},
>>  	},
>>  
>> +	/* Models which have nvidia-ec-wmi support, but should not use it. */
>> +	{
>> +	 .callback = video_detect_force_native,
>> +	 /* Dell G15 5515 */
>> +	 .matches = {
>> +		DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
>> +		DMI_MATCH(DMI_PRODUCT_NAME, "Dell G15 5515"),
>> +		},
>> +	},
>> +
>>  	/*
>>  	 * Desktops which falsely report a backlight and which our heuristics
>>  	 * for this do not catch.
>> -- 
>> 2.37.3
>>
> 

Re: [PATCH] ACPI: video: Add backlight=native DMI quirk for Dell G15 5515

[Daniel Dadap]

On Tue, Nov 01, 2022 at 10:52:03AM +0100, Hans de Goede wrote:
> Hi,
> 
> On 10/31/22 23:09, Daniel Dadap wrote:
> > Thanks, Hans:
> > 
> > On Mon, Oct 31, 2022 at 10:25:56PM +0100, Hans de Goede wrote:
> >> The Dell G15 5515 has the WMI interface (and WMI call returns) expected
> >> by the nvidia-wmi-ec-backlight interface. But the backlight class device
> >> registered by the nvidia-wmi-ec-backlight driver does not actually work.
> >>
> >> The amdgpu_bl0 native GPU backlight class device does actually work,
> >> add a backlight=native DMI quirk for this.
> > 
> > I'm not certain whether you have direct access to the system, but I am
> > curious as to what mode the system is in when it exposes a non-working
> > WMI backlight interface. The expectation is that the WMI EC backlight
> > interface should only be exposed when the system is in dynamic mode
> > (different vendors may call this differeint things in the EFI setup
> > menus), and the native backlight interfaces for the GPUs, be they
> > discrete or integrated, should only be exposed when a mode that keeps
> > only one of the GPUs connected to the panel; i.e., in discrete-only mode
> > the discrete GPU should have a working native backlight interface, and
> > in integrated-only mode, or a hybrid mode which does not allow dynamic
> > mux switching, the integrated GPU should have a working native backlight
> > interface.
> > 
> > For this notebook the AMD GPU would be the integrated one, so if the
> > system is *not* in dynamic switch mode, and the WMI interface is
> > reporting that the backlight should be handled by the EC, then that is
> > certainly a bug in the system, and this would be the correct workaround,
> > when the system is operating in an iGPU-only or non-dynamic hybrid mode.
> > However, if the system is switched to a dynamic mode, then this would
> > break the EC backlight, which should be the corect backlight interface
> > for that case. Another possible system bug would be if the condition
> > described in the change description occurs when the system *is* in a
> > dynamic mode, but is routing backlight control to the iGPU anyway. That
> > would be a messier bug to deal with, since it would also be necessary to
> > determine which backlight interface is functional when the display is
> > switched over to the dGPU.
> > 
> > If it's not too much trouble could you (or the reporter, if you don't
> > have the system) check which backlight interfaces are exposed, and which
> > of those actually work, for each of the different hybrid GPU modes? I am
> > not certain what they would be called on the setup screen, but the
> > possible modes should be:
> > 
> > * Integrated only
> > * Discrete only
> > * Hybrid, without dynamic switching
> > * Hybrid, with dynamic switching
> > 
> > , or a subset thereof. Usually I would expect the minimal subset to be
> > discrete only plus one or both of the hybrid modes. Since we're not sure
> > what these are called on the setup screen it is probably best to simply
> > report the names of the mode as they are listed in the UI.
> 
> Isn't the whole idea behind nvidia-wmi-ec-backlight that the backlight's
> pwm input is connected to the EC (it is sorta in the name) and then always
> is controlled by the EC independent of the mode ?
> 
> At least I was under the impression that that was the whole idea?

Almost, but not quite. The idea is that the PWM is connected to and
controlled by the EC when the system is in a dynamic mux switching mode.
Then, when the display is muxed from one GPU to the other, the backlight
level remains consistent because it's not under control of whichever GPU
it happens to be connected to at the moment. If the system is not in
dynamic mux mode, then there is only ever one GPU connected for the
entire time the system is powered on, so there's no need for the EC to
act as a GPU-agnostic backlight control. Querying the WMI-wrapped ACPI
method labeled WMI_BRIGHTNESS_METHOD_SOURCE by enum wmi_brightness_method
is supposed to tell the EC backlight driver who currently owns backlight
control. The most likely possibilities for the bug here are:

1) WMI_BRIGHTNESS_METHOD_SOURCE returns WMI_BRIGHTNESS_SOURCE_EC in a
   non-dynamic mux mode, instead of returning the correct value of
   WMI_BRIGHTNESS_SOURCE_GPU
2) The system really is in a dynamic mux mode, but backlight control is
   incorrectly routed to the GPU which is muxed to at boot time.

If we're looking at case (1), then the proposed quirk is fine, because
the display will not switch which GPU it is connected to, so whichever
native backlight driver comes online is the correct one. If it's case
(2), then the correct workaround would depend upon what happens when the
mux is switched to the other GPU. If backlight control switches to the
other GPU, then the below quirk should work under the future backlight
subsystem where there's one backlight interface per display connector,
as long as both GPU drivers register their native backlight controllers.
Userspace would have to deal with coordinating the level across switches
and scaling it, if necessary. If switching the mux does end up making
the EC backlight control like it's supposed to (possibly even after
switching back to the original muxed-at-boot GPU), then the below quirk
would break backlight control after the mux is switched.

I suppose this is all moot at the moment, because we don't actually have
support for dynamic mux switching on Linux, yet. I have a proposal for
that which I've been meaning to send out to dri-devel; I'll try to do
that soon. So in the meantime, this quirk is a "good enough" workaround.
It would be helpful to add a comment to the quirk that we will need to
figure out what the correct way to deal with this system is once dynamic
mux switching support is available.

> I will forward your email to the reporter and ask them to get
> in touch with you and then lets see from there.
> 
> Regards,
> 
> Hans
> 
> 
> 
> > 
> >> Cc: Daniel Dadap <ddadap@nvidia.com>
> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> >> ---
> >>  drivers/acpi/video_detect.c | 10 ++++++++++
> >>  1 file changed, 10 insertions(+)
> >>
> >> diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c
> >> index 841f6213b4de..88acc09773bb 100644
> >> --- a/drivers/acpi/video_detect.c
> >> +++ b/drivers/acpi/video_detect.c
> >> @@ -645,6 +645,16 @@ static const struct dmi_system_id video_detect_dmi_table[] = {
> >>  		},
> >>  	},
> >>  
> >> +	/* Models which have nvidia-ec-wmi support, but should not use it. */
> >> +	{
> >> +	 .callback = video_detect_force_native,
> >> +	 /* Dell G15 5515 */
> >> +	 .matches = {
> >> +		DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
> >> +		DMI_MATCH(DMI_PRODUCT_NAME, "Dell G15 5515"),
> >> +		},
> >> +	},
> >> +
> >>  	/*
> >>  	 * Desktops which falsely report a backlight and which our heuristics
> >>  	 * for this do not catch.
> >> -- 
> >> 2.37.3
> >>
> > 
> 

Re: [PATCH] ACPI: video: Add backlight=native DMI quirk for Dell G15 5515

[Daniel Dadap]

On Tue, Nov 01, 2022 at 03:09:04PM +0100, Hans de Goede wrote:
> Hi Daniel,
> 
> On 10/31/22 23:09, Daniel Dadap wrote:
> > Thanks, Hans:
> > 
> > On Mon, Oct 31, 2022 at 10:25:56PM +0100, Hans de Goede wrote:
> >> The Dell G15 5515 has the WMI interface (and WMI call returns) expected
> >> by the nvidia-wmi-ec-backlight interface. But the backlight class device
> >> registered by the nvidia-wmi-ec-backlight driver does not actually work.
> >>
> >> The amdgpu_bl0 native GPU backlight class device does actually work,
> >> add a backlight=native DMI quirk for this.
> > 
> > I'm not certain whether you have direct access to the system, but I am
> > curious as to what mode the system is in when it exposes a non-working
> > WMI backlight interface. The expectation is that the WMI EC backlight
> > interface should only be exposed when the system is in dynamic mode
> > (different vendors may call this differeint things in the EFI setup
> > menus), and the native backlight interfaces for the GPUs, be they
> > discrete or integrated, should only be exposed when a mode that keeps
> > only one of the GPUs connected to the panel; i.e., in discrete-only mode
> > the discrete GPU should have a working native backlight interface, and
> > in integrated-only mode, or a hybrid mode which does not allow dynamic
> > mux switching, the integrated GPU should have a working native backlight
> > interface.
> > 
> > For this notebook the AMD GPU would be the integrated one, so if the
> > system is *not* in dynamic switch mode, and the WMI interface is
> > reporting that the backlight should be handled by the EC, then that is
> > certainly a bug in the system, and this would be the correct workaround,
> > when the system is operating in an iGPU-only or non-dynamic hybrid mode.
> > However, if the system is switched to a dynamic mode, then this would
> > break the EC backlight, which should be the corect backlight interface
> > for that case. Another possible system bug would be if the condition
> > described in the change description occurs when the system *is* in a
> > dynamic mode, but is routing backlight control to the iGPU anyway. That
> > would be a messier bug to deal with, since it would also be necessary to
> > determine which backlight interface is functional when the display is
> > switched over to the dGPU.
> > 
> > If it's not too much trouble could you (or the reporter, if you don't
> > have the system) check which backlight interfaces are exposed, and which
> > of those actually work, for each of the different hybrid GPU modes? I am
> > not certain what they would be called on the setup screen, but the
> > possible modes should be:
> > 
> > * Integrated only
> > * Discrete only
> > * Hybrid, without dynamic switching
> > * Hybrid, with dynamic switching
> > 
> > , or a subset thereof. Usually I would expect the minimal subset to be
> > discrete only plus one or both of the hybrid modes. Since we're not sure
> > what these are called on the setup screen it is probably best to simply
> > report the names of the mode as they are listed in the UI.
> 
> I have heard back from the reporter about this and they report that
> there is no option in the BIOS to select the mode. And currently the
> machine is running in Hybrid mode, with it being unclear if this
> supports dynamic switching or not.

Okay. I forgot that there are some dynamic mux systems which only have
the single "Hybrid, with dynamic switching" mode. And of course for
non-dynamic mux systems "Hybrid, without dynamic switching" is the only
possibility, since there's no mux to switch. As I mentioned in my other
message, the quirk you have here is good enough while we don't support
dynamic switching, and even after we do support dynamic switching it
will be fine if this is actually a non-dynamic system which is reporting
itself as a dynamic one.

A dump of the system's ACPI tables would be useful to try to determine
what this system is actually supposed to do. If the reporter is willing
to share a tarball of the files generated by `acpidump -b`, I can look
through the tables to see if there are more clues as to the intended
behavior. In the meantime, I will ask around to see if somebody at NVIDIA
has one of these systems and it is available for some experiments. If not,
I can hack up a test kernel module to help the reporter poke around a bit.
 
> Regards,
> 
> Hans
> 
> 
> 
> > 
> >> Cc: Daniel Dadap <ddadap@nvidia.com>
> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> >> ---
> >>  drivers/acpi/video_detect.c | 10 ++++++++++
> >>  1 file changed, 10 insertions(+)
> >>
> >> diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c
> >> index 841f6213b4de..88acc09773bb 100644
> >> --- a/drivers/acpi/video_detect.c
> >> +++ b/drivers/acpi/video_detect.c
> >> @@ -645,6 +645,16 @@ static const struct dmi_system_id video_detect_dmi_table[] = {
> >>  		},
> >>  	},
> >>  
> >> +	/* Models which have nvidia-ec-wmi support, but should not use it. */
> >> +	{
> >> +	 .callback = video_detect_force_native,
> >> +	 /* Dell G15 5515 */
> >> +	 .matches = {
> >> +		DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
> >> +		DMI_MATCH(DMI_PRODUCT_NAME, "Dell G15 5515"),
> >> +		},
> >> +	},
> >> +
> >>  	/*
> >>  	 * Desktops which falsely report a backlight and which our heuristics
> >>  	 * for this do not catch.
> >> -- 
> >> 2.37.3
> >>
> > 
> 
>