Re: [PATCH v2 0/5] drm: panel-backlight-quirks: Do partial refactor and apply OLED fix

[Mario Limonciello <mario.limonciello@amd.com>]

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.

> - 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@amd.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."