[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <e934ece8-d70d-44fd-abe6-fcecae8abc85@amd.com>
Date: Fri, 14 Nov 2025 13:17:25 -0600
From: Mario Limonciello <mario.limonciello@....com>
To: Jani Nikula <jani.nikula@...ux.intel.com>, Simona Vetter
<simona@...ll.ch>, xaver.hugl@....org
Cc: Alex Deucher <alexander.deucher@....com>,
Christian König <christian.koenig@....com>,
David Airlie <airlied@...il.com>,
"open list:RADEON and AMDGPU DRM DRIVERS" <amd-gfx@...ts.freedesktop.org>,
"open list:DRM DRIVERS" <dri-devel@...ts.freedesktop.org>,
open list <linux-kernel@...r.kernel.org>,
Simona Vetter <simona.vetter@...ll.ch>,
Harry Wentland <Harry.Wentland@....com>,
Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>,
Thomas Zimmermann <tzimmermann@...e.de>, Maxime Ripard <mripard@...nel.org>
Subject: Re: [PATCH] drm/amd: Move adaptive backlight modulation property to
drm core
+Xaver
On 11/14/2025 2:39 AM, Jani Nikula wrote:
<snip>
>>
>> So this is basically Content Adaptive Brightness Control, but with the
>> technology ("backlight" and "modulation") unnecessarily encoded in the
>> ABI.
>>
>> You could have the same knob for adjusting CABC implemented in an OLED
>> panel, controlled via DPCD.
>>
>>> + *
>>> + * sysfs
>>> + * The ABM property is exposed to userspace via sysfs interface
>>> + * located at 'amdgpu/panel_power_savings' under the DRM device.
>>
>> Err what? Seriously suggesting that to the common ABI? We shouldn't have
>> sysfs involved at all, let alone vendor specific sysfs.
>>
>>> + * off
>>> + * Adaptive backlight modulation is disabled.
>>> + * min
>>> + * Adaptive backlight modulation is enabled at minimum intensity.
>>> + * bias min
>>> + * Adaptive backlight modulation is enabled at a more intense
>>> + * level than 'min'.
>>> + * bias max
>>> + * Adaptive backlight modulation is enabled at a more intense
>>> + * level than 'bias min'.
>>> + * max
>>> + * Adaptive backlight modulation is enabled at maximum intensity.
>>
>> So values 0-4 but with names. I don't know what "bias" means here, and I
>> probably shouldn't even have to know. It's an implementation detail
>> leaking to the ABI.
>>
>> In the past I've encountered CABC with different modes based on the use
>> case, e.g. "video" or "game", but I don't know how those would map to
>> the intensities.
>>
>> I'm concerned the ABI serves AMD hardware, no one else, and everyone
>> else coming after this is forced to shoehorn their implementation into
>> this.
>
> Apparently Harry had the same concerns [1].
>
So let me explain how we got here. At the display next hackfest last
year (2024) we talked about how to get compositors to indicate they want
technologies like this to get out the way. A patch series was made that
would allow compositor to say "Require color accuracy" or "Require low
latency" are required.
https://lore.kernel.org/amd-gfx/20240703051722.328-1-mario.limonciello@amd.com/
This got reverted because userspace didn't have an implementation ready
to go at the time. One was created and so I rebased/resent the series
earlier this year.
https://lore.kernel.org/amd-gfx/20250621152657.1048807-1-superm1@kernel.org/
Xaver had some change of heart and wanted to talk about it at the next
hackfest.
https://lore.kernel.org/amd-gfx/CAFZQkGxUwodf5bW0qQkXoPoz0CFFA1asJfUxFftMGgs5-VK2Hw@mail.gmail.com/
So we talked about it again at the hackfest this year and the decision
was not everyone can an octagon into a peg hole, so we're better off
re-introducing vendor properties for this. So series was respun per
that discussion.
https://lore.kernel.org/amd-gfx/20250718192045.2091650-1-superm1@kernel.org/
Userspace implementation was done and so we merged this for 6.19.
https://lore.kernel.org/amd-gfx/CAFZQkGwLWcyS0SqCHoiGsJd5J_u4aBJ0HMV5Bx3NknLdLkr8Uw@mail.gmail.com/
Then Simona suggested we need to make some changes where the propertye
should be in generic documentation etc:
https://lore.kernel.org/amd-gfx/aQUz-mbM_WlXn_uZ@phenom.ffwll.local/
So that's where we are now with this patch. I can clean it up per the
feedback so far - but I think we need to be in agreement that this
property is actually the way forward or we should revert the property in
amdgpu instead of this moving approach and keep discussing.
Powered by blists - more mailing lists