Re: [PATCH v3] ACPI: video: Put ACPI video and its child devices to D0 at boot

[Rafael J. Wysocki]

On Tue, Jul 25, 2023 at 10:39 AM Hans de Goede <hdegoede@xxxxxxxxxx> wrote:
>
> Hi,
>
> On 7/21/23 17:45, Rafael J. Wysocki wrote:
> > Thanks for the update!
> >
> > On Fri, Jul 21, 2023 at 5:22 PM Kai-Heng Feng
> > <kai.heng.feng@xxxxxxxxxxxxx> wrote:
> >>
> >> Screen brightness can only be changed once on HP ZBook Fury 16 G10.
> >>
> >> Vendor identified the root cause as Linux doesn't invoke _PS0 at boot
> >> for all ACPI devices:
> >>
> >>     Scope (\_SB.PC00.GFX0)
> >>     {
> >>         Scope (DD1F)
> >>         {
> >>             Method (_PS0, 0, Serialized)  // _PS0: Power State 0
> >>             {
> >>                 If (CondRefOf (\_SB.PC00.LPCB.EC0.SSBC))
> >>                 {
> >>                     \_SB.PC00.LPCB.EC0.SSBC ()
> >>                 }
> >>             }
> >>             ...
> >>         }
> >>         ...
> >>     }
> >>
> >> The \_SB.PC00.GFX0.DD1F is the panel device, and its _PS0 needs to be
> >> executed to make the brightness control work properly.
> >>
> >> _PS0 doesn't get invoked for this device because _PSC is missing,
> >> which violates ACPI spec 6.3, section 7.3.6. Commit 7cd8407d53ef
> >> ("ACPI / PM: Do not execute _PS0 for devices without _PSC during
> >> initialization") tried to workaround missing _PSC on defective
> >> firmwares, but got reverted because of regression.
> >>
> >> So the safest approach is to use acpi_device_fix_up_power_extended() to
> >> put ACPI video and its child devices to D0 to workaround the issue.
> >>
> >> Link: https://bugzilla.kernel.org/show_bug.cgi?id=217683
> >> Signed-off-by: Kai-Heng Feng <kai.heng.feng@xxxxxxxxxxxxx>
> >
> > Hans, what do you think?
>
> I believe most (almost all?) child devices of the GFX0 (and other
> acpi-video) ACPI device will not have a _PS0. So wrt child-devices
> this should mostly be a no-op.
>
> However the GFX0 ACPI device is the ACPI companion for the PCI
> device for the GPU, so this will also execute _PS0 on the GPU,
> we do that every suspend/resume cycle so this should be fine.
>
> But if we do see regressions then we may need to rework this
> to skip the _PS0 on the GPU itself.
>
> With that caveat this looks good to me:
>
> Reviewed-by: Hans de Goede <hdegoede@xxxxxxxxxx>

So I've edited the subject (slightly) and the changelog of this patch
and applied it as 6.6 material, thanks!