[1/1,SRU,L,M] UBUNTU: SAUCE: ACPI: video: Dell AIO UART backlight detection

From: "Chia-Lin Kao (AceLan)" <acelan.kao@canonical.com>

BugLink: https://launchpad.net/bugs/2008882

The similar functionality has been reverted by below commit as the
function acpi_video_set_dmi_backlight_type() has been removed from
upstream.
2a773621f3495 ("UBUNTU: SAUCE: platform/x86: dell-uart-backlight: remove acpi_video_set_dmi_backlight_type()")

Now, we use new way to auto detect the Dell AIO platforms which
contains "DELL0501" HID.

Signed-off-by: Chia-Lin Kao (AceLan) <acelan.kao@canonical.com>
---
 drivers/acpi/video_detect.c | 5 +++++
 1 file changed, 5 insertions(+)

On 19.06.23 11:55, AceLan Kao wrote:
> From: "Chia-Lin Kao (AceLan)" <acelan.kao@canonical.com>
> 
> BugLink: https://launchpad.net/bugs/2008882
> 
> The similar functionality has been reverted by below commit as the
> function acpi_video_set_dmi_backlight_type() has been removed from
> upstream.
> 2a773621f3495 ("UBUNTU: SAUCE: platform/x86: dell-uart-backlight: remove acpi_video_set_dmi_backlight_type()")
> 
> Now, we use new way to auto detect the Dell AIO platforms which
> contains "DELL0501" HID.
> 
> Signed-off-by: Chia-Lin Kao (AceLan) <acelan.kao@canonical.com>
> ---
>   drivers/acpi/video_detect.c | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c
> index bcc25d457581..065dd576693b 100644
> --- a/drivers/acpi/video_detect.c
> +++ b/drivers/acpi/video_detect.c
> @@ -775,6 +775,7 @@ enum acpi_backlight_type __acpi_video_get_backlight_type(bool native, bool *auto
>   	static DEFINE_MUTEX(init_mutex);
>   	static bool nvidia_wmi_ec_present;
>   	static bool apple_gmux_present;
> +	static bool dell_uart_backlight_present;
>   	static bool native_available;
>   	static bool init_done;
>   	static long video_caps;
> @@ -789,6 +790,7 @@ enum acpi_backlight_type __acpi_video_get_backlight_type(bool native, bool *auto
>   				    &video_caps, NULL);
>   		nvidia_wmi_ec_present = nvidia_wmi_ec_supported();
>   		apple_gmux_present = apple_gmux_detect(NULL, NULL);
> +		dell_uart_backlight_present = acpi_dev_found("DELL0501");
>   		init_done = true;
>   	}
>   	if (native)
> @@ -819,6 +821,9 @@ enum acpi_backlight_type __acpi_video_get_backlight_type(bool native, bool *auto
>   	if (apple_gmux_present)
>   		return acpi_backlight_apple_gmux;
>   
> +	if (dell_uart_backlight_present)
> +		return acpi_backlight_vendor;
> +
>   	/* Use ACPI video if available, except when native should be preferred. */
>   	if ((video_caps & ACPI_VIDEO_BACKLIGHT) &&
>   	     !(native_available && prefer_native_over_acpi_video()))

Rejected for the following reasons:
- Non upstream change does not fulfill SRU requirements
- No reason given why this is not done upstream and rolled back
- Not a reason to reject but for BugLinks we use **bugs**.launchpad.net
   as a standard

-Stefan

Hi Stefan,

Stefan Bader <stefan.bader@canonical.com> 於 2023年6月20日 週二 下午3:39寫道：
>
> On 19.06.23 11:55, AceLan Kao wrote:
> > From: "Chia-Lin Kao (AceLan)" <acelan.kao@canonical.com>
> >
> > BugLink: https://launchpad.net/bugs/2008882
> >
> > The similar functionality has been reverted by below commit as the
> > function acpi_video_set_dmi_backlight_type() has been removed from
> > upstream.
> > 2a773621f3495 ("UBUNTU: SAUCE: platform/x86: dell-uart-backlight: remove acpi_video_set_dmi_backlight_type()")
> >
> > Now, we use new way to auto detect the Dell AIO platforms which
> > contains "DELL0501" HID.
> >
> > Signed-off-by: Chia-Lin Kao (AceLan) <acelan.kao@canonical.com>
> > ---
> >   drivers/acpi/video_detect.c | 5 +++++
> >   1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c
> > index bcc25d457581..065dd576693b 100644
> > --- a/drivers/acpi/video_detect.c
> > +++ b/drivers/acpi/video_detect.c
> > @@ -775,6 +775,7 @@ enum acpi_backlight_type __acpi_video_get_backlight_type(bool native, bool *auto
> >       static DEFINE_MUTEX(init_mutex);
> >       static bool nvidia_wmi_ec_present;
> >       static bool apple_gmux_present;
> > +     static bool dell_uart_backlight_present;
> >       static bool native_available;
> >       static bool init_done;
> >       static long video_caps;
> > @@ -789,6 +790,7 @@ enum acpi_backlight_type __acpi_video_get_backlight_type(bool native, bool *auto
> >                                   &video_caps, NULL);
> >               nvidia_wmi_ec_present = nvidia_wmi_ec_supported();
> >               apple_gmux_present = apple_gmux_detect(NULL, NULL);
> > +             dell_uart_backlight_present = acpi_dev_found("DELL0501");
> >               init_done = true;
> >       }
> >       if (native)
> > @@ -819,6 +821,9 @@ enum acpi_backlight_type __acpi_video_get_backlight_type(bool native, bool *auto
> >       if (apple_gmux_present)
> >               return acpi_backlight_apple_gmux;
> >
> > +     if (dell_uart_backlight_present)
> > +             return acpi_backlight_vendor;
> > +
> >       /* Use ACPI video if available, except when native should be preferred. */
> >       if ((video_caps & ACPI_VIDEO_BACKLIGHT) &&
> >            !(native_available && prefer_native_over_acpi_video()))
>
> Rejected for the following reasons:
> - Non upstream change does not fulfill SRU requirements
That's because the dell-uart-backlight driver only exists in ubuntu
kernels, so the fix for dell-uart-backlight driver won't submit to
upstream.
(drivers/platform/x86/dell/dell-uart-backlight.c)
> - No reason given why this is not done upstream and rolled back
Upstream requested me to rewrite 'dell-uart-backlight' using 'serdev'
in the kernel version 4.x.
However, at that time, there were not many examples available for me
to learn from,
so we kept the driver in our kernels only since then.
> - Not a reason to reject but for BugLinks we use **bugs**.launchpad.net
>    as a standard
Got it

Do you need me to submit v2 with above explanination?
>
> -Stefan
>

On 21.06.23 04:02, AceLan Kao wrote:
> Hi Stefan,
> 
> Stefan Bader <stefan.bader@canonical.com> 於 2023年6月20日 週二 下午3:39寫道：
>>
>> On 19.06.23 11:55, AceLan Kao wrote:
>>> From: "Chia-Lin Kao (AceLan)" <acelan.kao@canonical.com>
>>>
>>> BugLink: https://launchpad.net/bugs/2008882
>>>
>>> The similar functionality has been reverted by below commit as the
>>> function acpi_video_set_dmi_backlight_type() has been removed from
>>> upstream.
>>> 2a773621f3495 ("UBUNTU: SAUCE: platform/x86: dell-uart-backlight: remove acpi_video_set_dmi_backlight_type()")
>>>
>>> Now, we use new way to auto detect the Dell AIO platforms which
>>> contains "DELL0501" HID.
>>>
>>> Signed-off-by: Chia-Lin Kao (AceLan) <acelan.kao@canonical.com>
>>> ---
>>>    drivers/acpi/video_detect.c | 5 +++++
>>>    1 file changed, 5 insertions(+)
>>>
>>> diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c
>>> index bcc25d457581..065dd576693b 100644
>>> --- a/drivers/acpi/video_detect.c
>>> +++ b/drivers/acpi/video_detect.c
>>> @@ -775,6 +775,7 @@ enum acpi_backlight_type __acpi_video_get_backlight_type(bool native, bool *auto
>>>        static DEFINE_MUTEX(init_mutex);
>>>        static bool nvidia_wmi_ec_present;
>>>        static bool apple_gmux_present;
>>> +     static bool dell_uart_backlight_present;
>>>        static bool native_available;
>>>        static bool init_done;
>>>        static long video_caps;
>>> @@ -789,6 +790,7 @@ enum acpi_backlight_type __acpi_video_get_backlight_type(bool native, bool *auto
>>>                                    &video_caps, NULL);
>>>                nvidia_wmi_ec_present = nvidia_wmi_ec_supported();
>>>                apple_gmux_present = apple_gmux_detect(NULL, NULL);
>>> +             dell_uart_backlight_present = acpi_dev_found("DELL0501");
>>>                init_done = true;
>>>        }
>>>        if (native)
>>> @@ -819,6 +821,9 @@ enum acpi_backlight_type __acpi_video_get_backlight_type(bool native, bool *auto
>>>        if (apple_gmux_present)
>>>                return acpi_backlight_apple_gmux;
>>>
>>> +     if (dell_uart_backlight_present)
>>> +             return acpi_backlight_vendor;
>>> +
>>>        /* Use ACPI video if available, except when native should be preferred. */
>>>        if ((video_caps & ACPI_VIDEO_BACKLIGHT) &&
>>>             !(native_available && prefer_native_over_acpi_video()))
>>
>> Rejected for the following reasons:
>> - Non upstream change does not fulfill SRU requirements
> That's because the dell-uart-backlight driver only exists in ubuntu
> kernels, so the fix for dell-uart-backlight driver won't submit to
> upstream.
> (drivers/platform/x86/dell/dell-uart-backlight.c)
>> - No reason given why this is not done upstream and rolled back
> Upstream requested me to rewrite 'dell-uart-backlight' using 'serdev'
> in the kernel version 4.x.
> However, at that time, there were not many examples available for me
> to learn from,
> so we kept the driver in our kernels only since then.
>> - Not a reason to reject but for BugLinks we use **bugs**.launchpad.net
>>     as a standard
> Got it
> 
> Do you need me to submit v2 with above explanination?

Yes, please. Generally we should be pushing more towards getting 
upstream solutions done. So the question is whether we should put this 
into Mantic or not. As long as we keep carrying special solutions there 
is no incentive in getting things done properly and we end up with 
additional burden.


>>
>> -Stefan
>>