Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
MIME-Version: 1.0
References: <20230721152143.3108017-1-kai.heng.feng@canonical.com>
 <CAJZ5v0j=ohJ_oUpCY-F_joty9qq3Mz2Bigqa3dHaFaWP2k6ONQ@mail.gmail.com> <8d71e258-0bfd-a0d0-868f-a3299864de5b@redhat.com>
In-Reply-To: <8d71e258-0bfd-a0d0-868f-a3299864de5b@redhat.com>
From:   "Rafael J. Wysocki" <rafael@kernel.org>
Date:   Thu, 17 Aug 2023 19:25:22 +0200
Message-ID: <CAJZ5v0gQGM91XW4AyL1qxGedBZaYJj6UXpVyRJwMossYny9N5g@mail.gmail.com>
Subject: Re: [PATCH v3] ACPI: video: Put ACPI video and its child devices to
 D0 at boot
To:     Hans de Goede <hdegoede@redhat.com>,
        Kai-Heng Feng <kai.heng.feng@canonical.com>
Cc:     linux-acpi@vger.kernel.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8BIT
Precedence: bulk

On Tue, Jul 25, 2023 at 10:39 AM Hans de Goede <hdegoede@redhat.com> 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@canonical.com> 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@canonical.com>
> >
> > 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@redhat.com>

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