mbox series

[v2,0/3] Adjust ACPI video detection fallback path

Message ID 20221208010910.7621-1-mario.limonciello@amd.com
Headers show
Series Adjust ACPI video detection fallback path | expand

Message

Mario Limonciello Dec. 8, 2022, 1:09 a.m. UTC
In kernel 6.1 the backlight registration code was overhauled so that
at most one backlight device got registered. As part of this change
there was code added to still allow making an acpi_video0 device if the
BIOS contained backlight control methods but no native or vendor drivers
registered.

Even after the overhaul this fallback logic is failing on the BIOS from
a number of motherboard manufacturers supporting Ryzen APUs.
What happens is the amdgpu driver finishes registration and as expected
doesn't create a backlight control device since no eDP panels are connected
to a desktop.

Then 8 seconds later the ACPI video detection code creates an
acpi_video0 device that is non-operational. GNOME then creates a
backlight slider.

To avoid this situation from happening make two sets of changes:

Prevent desktop problems w/ fallback logic
------------------------------------------
1) Add support for the video detect code to let native drivers cancel the
fallback logic if they didn't find a panel.

This is done this way so that if another driver decides that the ACPI
mechanism is still needed it can instead directly call the registration
function.

2) Add code to amdgpu to notify the ACPI video detection code that no panel
was detected on an APU.

Disable fallback logic by default
---------------------------------
This fallback logic was introduced to prevent regressions in the backlight
overhaul.  As it has been deemed unnecessary by Hans explicitly disable the
timeout.  If this turns out to be mistake and this part is reverted, the
other patches for preventing desktop problems will avoid regressions on
desktops.

Mario Limonciello (3):
  ACPI: video: Allow GPU drivers to report no panels
  drm/amd/display: Report to ACPI video if no panels were found
  ACPI: video: Don't enable fallback path for creating ACPI backlight by
    default

 drivers/acpi/acpi_video.c                       | 17 ++++++++++++-----
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c   |  4 ++++
 include/acpi/video.h                            |  1 +
 3 files changed, 17 insertions(+), 5 deletions(-)

Comments

Rafael J. Wysocki Dec. 8, 2022, 11:32 a.m. UTC | #1
On Thu, Dec 8, 2022 at 2:09 AM Mario Limonciello
<mario.limonciello@amd.com> wrote:
>
> The current logic for the ACPI backlight detection will create
> a backlight device if no native or vendor drivers have created
> 8 seconds after the system has booted if the ACPI tables
> included backlight control methods.
>
> If the GPU drivers have loaded, they may be able to report whether
> any LCD panels were found.  Allow using this information to factor
> in whether to enable the fallback logic for making an acpi_video0
> backlight device.
>
> Suggested-by: Hans de Goede <hdegoede@redhat.com>
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
> v1->v2:
>  * Cancel registration for backlight device instead (Hans)
>  * drop desktop check (Dan)
>
>  drivers/acpi/acpi_video.c | 11 +++++++++++
>  include/acpi/video.h      |  1 +
>  2 files changed, 12 insertions(+)
>
> diff --git a/drivers/acpi/acpi_video.c b/drivers/acpi/acpi_video.c
> index 32953646caeb..f64fdb029090 100644
> --- a/drivers/acpi/acpi_video.c
> +++ b/drivers/acpi/acpi_video.c
> @@ -2178,6 +2178,17 @@ static bool should_check_lcd_flag(void)
>         return false;
>  }
>
> +/*
> + * At least one graphics driver has reported that no LCD is connected
> + * via the native interface. cancel the registration for fallback acpi_video0.
> + * If another driver still deems this necessary, it can explicitly register it.
> + */
> +void acpi_video_report_nolcd(void)
> +{
> +       cancel_delayed_work(&video_bus_register_backlight_work);
> +}
> +EXPORT_SYMBOL(acpi_video_report_nolcd);
> +
>  int acpi_video_register(void)
>  {
>         int ret = 0;
> diff --git a/include/acpi/video.h b/include/acpi/video.h
> index a275c35e5249..1fccb111c197 100644
> --- a/include/acpi/video.h
> +++ b/include/acpi/video.h
> @@ -53,6 +53,7 @@ enum acpi_backlight_type {
>  };
>
>  #if IS_ENABLED(CONFIG_ACPI_VIDEO)
> +extern void acpi_video_report_nolcd(void);

It looks like a stub is needed for the other case.  Apparently, things
fail to compile due to the lack of it.

>  extern int acpi_video_register(void);
>  extern void acpi_video_unregister(void);
>  extern void acpi_video_register_backlight(void);
> --
> 2.34.1
>