Re: [PATCH] ACPI: video: Invoke _PS0 at boot for ACPI video

[Kai-Heng Feng]

Hi Hans,

On Mon, Jul 10, 2023 at 8:54 PM Hans de Goede <hdegoede@xxxxxxxxxx> wrote:
[snipped]
> > Or put all ACPI devices to D0 at boot?
> > According to the BIOS folks that's what Windows does.
> > This way we can drop acpi_device_fix_up_power* helpers altogether.
>
> Doing that will leave any devices for which we lack a driver at D0 for ever,
> so that IMHO is not a good idea.
>
> I guess calling acpi_device_fix_up_power_extended(device) from the
> ACPI-video code, so that the connector sub-objects are put in D0 is
> somewhat ok. Although I would prefer to see you first try to do
> the same thing from the i915 driver instead.

I was told by vendor that the same design is used on AMD based laptops too.
So I think putting this in ACPI-video is a better approach.

>
> If we do end up doing this from the acpi-video code please add
> a comment above the call why this is done; and as Rafael mentioned
> the commit msg needs to better explain things too.

Sure.

Kai-Heng

>
> Regards,
>
> Hans
>
>
>
> >
> >>
> >> Also can you provide some more info on the hw on which this is being seen:
> >>
> >> 1. What GPU(s) is/are being used
> >
> > Intel GFX.
> >
> > AFAIK AMD based laptops also require this fixup too.
> >
> >> 2. If there is a mux for hybrid gfx in which mode is the mux set ?
> >
> > No. This happens to mux-less and dGPU-less laptops.
> >
> >> 3. Wjich method is used to control the brightness (which backlight-class-devices show up under /sys/class/backlight) ?
> >
> > intel_backlight.
> >
> >>
> >> And can you add this info to the commit msg for the next version of the patch ?
> >
> > Sure.
> > Can putting all devices to D0 be considered too? It's a better
> > solution for the long wrong.
> >
> > Kai-Heng
> >
> >>
> >> Regards,
> >>
> >> Hans
> >>
> >>
> >>
> >>
> >>>
> >>> And yes, Linux doesn't put all of the ACPI devices into D0 during
> >>> initialization, but the above commit has a little to do with that.
> >>>
> >>>> For now explicitly call _PS0 for ACPI video to workaround the issue.
> >>>
> >>> This is not what the patch is doing.
> >>>
> >>>> Signed-off-by: Kai-Heng Feng <kai.heng.feng@xxxxxxxxxxxxx>
> >>>> ---
> >>>>  drivers/acpi/acpi_video.c | 2 ++
> >>>>  1 file changed, 2 insertions(+)
> >>>>
> >>>> diff --git a/drivers/acpi/acpi_video.c b/drivers/acpi/acpi_video.c
> >>>> index 62f4364e4460..793259bd18c8 100644
> >>>> --- a/drivers/acpi/acpi_video.c
> >>>> +++ b/drivers/acpi/acpi_video.c
> >>>> @@ -2027,6 +2027,8 @@ static int acpi_video_bus_add(struct acpi_device *device)
> >>>>         if (error)
> >>>>                 goto err_put_video;
> >>>>
> >>>> +       acpi_device_fix_up_power_extended(device);
> >>>> +
> >>>
> >>> I would like to know what Hans thinks about this.
> >>>
> >>>>         pr_info("%s [%s] (multi-head: %s  rom: %s  post: %s)\n",
> >>>>                ACPI_VIDEO_DEVICE_NAME, acpi_device_bid(device),
> >>>>                video->flags.multihead ? "yes" : "no",
> >>>> --
> >>>
> >>
> >
>