Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <f27f0d06-0b18-06bb-cb1f-042527c1ca31@redhat.com>
Date:   Thu, 8 Jun 2023 11:16:17 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.10.0
Subject: Re: [PATCH] platform/x86: dell-laptop: Add drm module soft dependency
Content-Language: en-US, nl
To:     AceLan Kao <acelan.kao@canonical.com>
Cc:     Matthew Garrett <mjg59@srcf.ucam.org>,
        =?UTF-8?Q?Pali_Roh=c3=a1r?= <pali@kernel.org>,
        Mark Gross <markgross@kernel.org>,
        platform-driver-x86@vger.kernel.org, linux-kernel@vger.kernel.org,
        Kai-Heng Feng <kai.heng.feng@canonical.com>
References: <20230607034331.576623-1-acelan.kao@canonical.com>
 <20230607042032.GA28835@srcf.ucam.org>
 <CAFv23QmDNUFcPwvSQt5aUxtmHasfr8wrF72ObvcO-X19gfn=LA@mail.gmail.com>
 <20230607052724.GA29834@srcf.ucam.org>
 <CAFv23QkEdGnEz1q7vbyFCa9S9Dqh-zec72nRGyZ3wAz-8wpbvA@mail.gmail.com>
 <20230607062341.GA30618@srcf.ucam.org> <20230607065604.yaivqbbd3dkawxo4@pali>
 <CAFv23Q==r4newMXE3OWavRSRt-bEi5-qR0Vo-5HGLw4r9J36MA@mail.gmail.com>
 <20230607074732.GA31666@srcf.ucam.org>
 <457e2a4a-e28a-cd24-c129-4ff6162ccc36@redhat.com>
 <CAFv23QmVP4ZFAhAFBSx6ty-4fcQ-u9+uZb0xaD2gCJPCdpPvQg@mail.gmail.com>
From:   Hans de Goede <hdegoede@redhat.com>
In-Reply-To: <CAFv23QmVP4ZFAhAFBSx6ty-4fcQ-u9+uZb0xaD2gCJPCdpPvQg@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: bulk

Hi AceLan,

On 6/8/23 05:04, AceLan Kao wrote:
> Hans de Goede <hdegoede@redhat.com> 於 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/20230608091258.7963-1-hdegoede@redhat.com/

Regards,

Hans