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

[Stefan Bader]

On 21.06.23 04:02, AceLan Kao wrote:
> Hi Stefan,
> 
> Stefan Bader <stefan.bader at canonical.com> 於 2023年6月20日 週二 下午3:39寫道：
>>
>> On 19.06.23 11:55, AceLan Kao wrote:
>>> From: "Chia-Lin Kao (AceLan)" <acelan.kao at 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 at 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
>>

-- 
- Stefan

-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_0xE8675DEECBEECEA3.asc
Type: application/pgp-keys
Size: 44613 bytes
Desc: OpenPGP public key
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20230621/6256fcd1/attachment-0001.key>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20230621/6256fcd1/attachment-0001.sig>