linux-kernel - Re: [PATCH v2 0/3] drm: backlight quirk infrastructure and lower minimum for Framework AMD 13

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Mon, 24 Jun 2024 18:15:44 +0200
From: Thomas Weißschuh <linux@...ssschuh.net>
To: Hans de Goede <hdegoede@...hat.com>
Cc: Alex Deucher <alexander.deucher@....com>, 
	Christian König <christian.koenig@....com>, David Airlie <airlied@...il.com>, 
	Daniel Vetter <daniel@...ll.ch>, Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, 
	Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>, 
	Harry Wentland <harry.wentland@....com>, Leo Li <sunpeng.li@....com>, 
	Rodrigo Siqueira <Rodrigo.Siqueira@....com>, Mario Limonciello <mario.limonciello@....com>, 
	Matt Hartley <matt.hartley@...il.com>, Kieran Levin <ktl@...mework.net>, amd-gfx@...ts.freedesktop.org, 
	dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org, Dustin Howett <dustin@...ett.net>
Subject: Re: [PATCH v2 0/3] drm: backlight quirk infrastructure and lower
 minimum for Framework AMD 13

Hi Hans!

thanks for your feedback!

On 2024-06-24 11:11:40+0000, Hans de Goede wrote:
> On 6/23/24 10:51 AM, Thomas Weißschuh 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 quirk infrastructure for backlight configuration to
> > override the settings provided by the firmware.
> > Also add amdgpu as a user of that infrastructure and a quirk for the
> > Framework 13 matte panel.
> > Most likely this will also work for the glossy panel, but I can't test
> > that.
> > 
> > One solution would be a fixed firmware version, but given that the
> > problem exists since the release of the hardware, it has been known for
> > a month that the hardware can go lower and there was no acknowledgment
> > from Framework in any way, I'd like to explore this alternative
> > way forward.
> 
> There are many panels where the brightness can go lower then the advertised
> minimum brightness by the firmware (e.g. VBT for i915). For most users
> the minimum brightness is fine, especially since going lower often may lead
> to an unreadable screen when indoors (not in the full sun) during daylight
> hours. And some users get confused by the unreadable screen and find it
> hard to recover things from this state.

There are a fair amount of complaints on the Framework forums about this.
And that specific panel is actually readable even at 0% PWM.

> So IMHO we should not be overriding the minimum brightness from the firmware
> using quirks because:
> 
> a) This is going to be an endless game of whack-a-mole

Indeed, but IMO it is better to maintain the list in the kernel than
forcing all users to resort to random forum advise and fiddle with
lowlevel system configuration.

> b) The new value may be too low for certain users / use-cases

The various userspace wrappers already are applying a safety
threshold to not go to "0".
At least gnome-settings-daemon and brightnessctl do not go below 1% of
brightness_max. They already have to deal with panels that can go
completely dark.

> With that said I realize that there are also many users who want to have
> a lower minimum brightness value for use in the evening, since they find
> the available minimum value still too bright. I know some people want this
> for e.g. various ThinkPad models too.

>From my experience with ThinkPads, the default brightness range there
was fine for me. But on the Framework 13 AMD it is not.

> So rather then quirking this, with the above mentioned disadvantages I believe
> that it would be better to extend the existing video=eDP-1:.... kernel
> commandline parsing to allow overriding the minimum brightness in a driver
> agnostic way.

I'm not a fan. It seems much too complicated for most users.

Some more background to the Framework 13 AMD case:
The same panel on the Intel variant already goes darker.
The last responses we got from Framework didn't indicate that the high
minimum brightness was intentional [0], [1].
Coincidentally the "12" returned from ATIF matches
AMDGPU_DM_DEFAULT_MIN_BACKLIGHT, so maybe the firmware is just not set
up completely.

> The minimum brightness override set this way will still need hooking up
> in each driver separately but by using the video=eDP-1:... mechanism
> we can document how to do this in driver independent manner. since
> I know there have been multiple requests for something like this in
> the past I believe that having a single uniform way for users to do this
> will be good.
> 
> Alternatively we could have each driver have a driver specific module-
> parameter for this. Either way I think we need some way for users to
> override this as a config/setting tweak rather then use quirks for this.

This also seems much too complicated for normal users.

[0] https://community.frame.work/t/25711/26
[1] https://community.frame.work/t/47036/7