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;
MIME-Version: 1.0
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>
 <f27f0d06-0b18-06bb-cb1f-042527c1ca31@redhat.com> <CAFv23Q==ghYCW-ukKQJZ_JhYgS4CSCDwbg9sZL9_B1Pa_LviWg@mail.gmail.com>
 <0132feaa-6a89-2a8a-9eb2-c444b61c01b3@redhat.com>
In-Reply-To: <0132feaa-6a89-2a8a-9eb2-c444b61c01b3@redhat.com>
From:   AceLan Kao <acelan.kao@canonical.com>
Date:   Thu, 15 Jun 2023 09:53:19 +0800
Message-ID: <CAFv23QmzjKA75Z=r9v-2BzePwaD_25MepBvpWXArThypH_rMsA@mail.gmail.com>
Subject: Re: [PATCH] platform/x86: dell-laptop: Add drm module soft dependency
To:     Hans de Goede <hdegoede@redhat.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>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Hi Hans,

Hans de Goede <hdegoede@redhat.com> =E6=96=BC 2023=E5=B9=B46=E6=9C=8813=E6=
=97=A5 =E9=80=B1=E4=BA=8C =E4=B8=8B=E5=8D=884:54=E5=AF=AB=E9=81=93=EF=BC=9A
>
> Hi AceLan,
>
> On 6/13/23 09:30, AceLan Kao wrote:
> > Hans de Goede <hdegoede@redhat.com> =E6=96=BC 2023=E5=B9=B46=E6=9C=888=
=E6=97=A5 =E9=80=B1=E5=9B=9B =E4=B8=8B=E5=8D=885:16=E5=AF=AB=E9=81=93=EF=BC=
=9A
> >>
> >> Hi AceLan,
> >>
> >> On 6/8/23 05:04, AceLan Kao wrote:
> >>> Hans de Goede <hdegoede@redhat.com> =E6=96=BC 2023=E5=B9=B46=E6=9C=88=
8=E6=97=A5 =E9=80=B1=E5=9B=9B =E4=B8=8A=E5=8D=883:16=E5=AF=AB=E9=81=93=EF=
=BC=9A
> >>>>
> >>>> 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 backli=
ght type
> >>>>>> is larger than the current registered one.
> >>>>>> Do this check in backlight_device_register() and unregister backli=
ght
> >>>>>> 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 behavi=
our
> >>>>> changed with later versions of Windows. I don't think there's a sin=
gle
> >>>>> 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 us=
e on
> >>>>> this platform? The simplest solution may actually just be for
> >>>>> dell-laptop to refuse to register a backlight if the platform claim=
s to
> >>>>> be Windows 8 or later.
> >>>>
> >>>> I like that idea.
> >>>>
> >>>> AceLan, I guess that you hit this easy while testing on a (developme=
nt)
> >>>> 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 >=3D 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@redh=
at.com/
> >>
> >> 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_backl=
ight
> > and then nvidia creates nvidia_0 later.
> >
> > Now, set to hybrid mode in the BIOS, I found I still got 2 backlight in=
terfaces
> >    $ 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 a=
nything
> > 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=3Dvideo work independent
from GPU driver")
Thanks.