[PATCH v3 0/2] drm: minimum backlight overrides and implementation for amdgpu

[Thomas Weißschuh]

Hi,

On 2024-08-01 10:52:55+0000, Hans de Goede wrote:
> On 7/31/24 10:55 PM, Daniel Vetter wrote:
> > On Wed, Jul 31, 2024 at 08:40:12PM +0300, Jani Nikula wrote:
> >> On Wed, 31 Jul 2024, Thomas Weißschuh <linux at weissschuh.net> wrote:
> >>> The value of "min_input_signal" returned from ATIF on a Framework AMD 13
> >>> is "12". This leads to a fairly bright minimum display backlight.
> >>>
> >>> Add a generic override helper for the user to override the settings
> >>> provided by the firmware through the kernel cmdline.
> >>> Also add amdgpu as a user of that helper.
> >>>
> >>> One solution would be a fixed firmware version, which was announced but
> >>> has no timeline.
> >>
> >> The flip side is that if we add this now, it pretty much has a timeline:
> >> We'll have to carry and support it forever.
> >>
> >> It's not a great prospect for something so specific. Not to mention that
> >> the limits are generally there for electrical minimums that should not
> >> be overridden. And before you know it, we'll have bug reports about
> >> flickering screens...
> > 
> > Yeah I think for this specific case where a fixed firmware is already
> > kinda promised, a quirk is the right fix. Otherwise we open up a can of
> > worms here ... so personally I like v2 a lot more.
> > -Sima
> 
> Ok, with both Jani and Sima preferring the quirk approach from v2,
> I withdraw my objection against v2. So lets go with that version.

Ack.

I want to note though, that there are enough other commandline
options that can mess things up.
An invalid video=MODELINE, custom ACPI tables, etc, so the fallout from the
new commandline variable doesn't seem too bad to me.

> Thomas, sorry about this.

No worries!

> I see that other then a remark from Jeff Johnson about a missing
> MODULE_DESCRIPTION() v2 does not have any reviews yet though.
> 
> So we will need to review that version now. Might be best for
> visibility of the patches in people's review queue to repost
> v2 as v4 with the MODULE_DESCRIPTION() fixed ?

I can do that.

> 
> >>> This helper does conflict with the mode override via the cmdline.
> >>> Only one can be specified.
> >>> IMO the mode override can be extended to also handle "min-brightness"
> >>> when that becomes necessary.
> >>>
> >>> ---
> >>> Changes in v3:
> >>> - Switch to cmdline override parameter
> >>> - Link to v2: https://lore.kernel.org/r/20240623-amdgpu-min-backlight-quirk-v2-0-cecf7f49da9b@weissschuh.net
> >>>
> >>> Changes in v2:
> >>> - Introduce proper drm backlight quirk infrastructure
> >>> - Quirk by EDID and DMI instead of only DMI
> >>> - Limit quirk to only single Framework 13 matte panel
> >>> - Link to v1: https://lore.kernel.org/r/20240610-amdgpu-min-backlight-quirk-v1-1-8459895a5b2a@weissschuh.net
> >>>
> >>> ---
> >>> Thomas Weißschuh (2):
> >>>       drm/connector: add drm_connector_get_cmdline_min_brightness_override()
> >>>       drm/amd/display: implement minimum brightness override
> >>>
> >>>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  6 ++++
> >>>  drivers/gpu/drm/drm_connector.c                   | 34 +++++++++++++++++++++++
> >>>  include/drm/drm_connector.h                       |  2 ++
> >>>  3 files changed, 42 insertions(+)
> >>> ---
> >>> base-commit: 36821612eb3091a21f7f4a907b497064725080c3
> >>> change-id: 20240610-amdgpu-min-backlight-quirk-8402fd8e736a
> >>>
> >>> Best regards,
> >>
> >> -- 
> >> Jani Nikula, Intel
> > 
>