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]
Message-ID: 
 <CAGwozwF-7Z_Yy8-w+EJxcJJ-1dKCtTMY356PR--EqWfyVHt+rA@mail.gmail.com>
Date: Wed, 27 Aug 2025 20:41:18 +0200
From: Antheas Kapenekakis <lkml@...heas.dev>
To: Mario Limonciello <mario.limonciello@....com>
Cc: amd-gfx@...ts.freedesktop.org, dri-devel@...ts.freedesktop.org,
	linux-kernel@...r.kernel.org, philm@...jaro.org,
	Alex Deucher <alexander.deucher@....com>,
 Christian König <christian.koenig@....com>,
	Robert Beckett <bob.beckett@...labora.com>
Subject: Re: [PATCH v2 0/5] drm: panel-backlight-quirks: Do partial refactor
 and apply OLED fix

On Wed, 27 Aug 2025 at 20:38, Mario Limonciello
<mario.limonciello@....com> wrote:
>
> On 8/27/2025 12:43 PM, Antheas Kapenekakis wrote:
> > This is an alternative to [1], since Phil found out there are still invalid
> > values. We need to reconsider the other patch in that series anyway because
> > the latest AMD firmware update might have fixed the Z13.
> >
> > This series refactors the panel-backlight-quirks code to make it easier to
> > add new quirks. Specifically, it adds the ability to bind to a secondary
> > DMI match so that the make of a device can be specified. Then, it makes
> > EDID optional, for devices we know the value should be applied universally.
> >
> > This is then used to add a quirk for OLED panels that have an issue
> > when their backlight is set with a value that contains a 0/1 value in their
> > minor byte. This issue affects four handhelds from three different vendors,
> > three of which are in the field. This quirk applies a |3 mask to the
> > backlight value, which avoids this issue. In addition, the value change
> > is minor enough so that it is essentially a NOOP. There is no need for
> > ensuring it runs only on panels with faulty firmwares.
> >
> > Finally, since allowed by this refactor, a quirk for Steam Decks is added
> > that lowers their minimum brightness to 0, matching SteamOS. This is
> > a nicety commit, which allows for mildly lower minimum brightness, so
> > there is no time sensitivity for having it merged. Mario noted that if
> > that quirk was refactored to use an EDID match via the current interface,
> > it could go through the fixes tree and land sooner, but perhaps it is not
> > worth the effort.
> >
> > [1] https://lore.kernel.org/all/20250824085351.454619-2-lkml@antheas.dev/
> >
> > ---
> > V1: https://lore.kernel.org/all/20250824200202.1744335-1-lkml@antheas.dev/
> >
> > Changes since v1:
> > - Remove leftover quirk from patch 3 (refactor)
> > - Add dangling comma in patch 4 (oled quirk)
> > - Add the next generation Zotac Zone in patch 4 (currently unreleased)
>
> Can you double check with Phil?  I thought there are two affected Zotac
> devices, but I could be wrong.  If we can get both in a single go let's
> do it.

Indeed. V1 already included the released model. Hopefully I did not
get the ID wrong on this one.

> > - Reword patch 1 and 5 subjects
> >
> > Antheas Kapenekakis (5):
> >    drm: panel-backlight-quirks: Make EDID match optional
> >    drm: panel-backlight-quirks: Convert brightness quirk to generic
> >      structure
> >    drm: panel-backlight-quirks: Add secondary DMI match
> >    drm: panel-backlight-quirks: Add brightness mask quirk
> >    drm: panel-backlight-quirks: Add Steam Deck brightness quirk
> >
> >   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  19 ++-
> >   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |   6 +
> >   drivers/gpu/drm/drm_panel_backlight_quirks.c  | 113 ++++++++++++++----
> >   include/drm/drm_utils.h                       |   8 +-
> >   4 files changed, 115 insertions(+), 31 deletions(-)
> >
> >
> > base-commit: 1b237f190eb3d36f52dffe07a40b5eb210280e00
>
> The series looks fine to me.
>
> Reviewed-by: Mario Limonciello <mario.limonciello@....com>
>
> I have two small nits though:
>
> 1) Because this is "effectively" going to limit the amount of brightness
> values available I think there should be a message when a the brightness
> mask quirk is in use that we can get in the logs so that the changed
> behavior isn't totally surprising.  We have some similar messages for
> DMI detected quirks in amdgpu already.
>
>                 drm_info(dev, "support_edp0_on_dp1 attached\n");
>                 drm_info(dev, "aux_hpd_discon_quirk attached\n");
>
> Can you add one for this new quirk?
>
> 2) The comment for 'brightness_mask' in patch 4 should have 'or' as 'OR'
> so it's obvious that it's logical OR to the casual reader and not a
> typographical error.
>
> IE "After deriving brightness, OR it with this mask."
>

Ack. I will give it 1-2 days for more feedback and resend.

Antheas


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ