On 11/18/2025 2:47 AM, Maxime Ripard wrote:
On Mon, Nov 17, 2025 at 11:05:10AM +0200, Jani Nikula wrote:
On Fri, 14 Nov 2025, Mario Limonciello <[email protected]> wrote:
+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/[email protected]/
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/[email protected]/
Xaver had some change of heart and wanted to talk about it at the next
hackfest.
https://lore.kernel.org/amd-gfx/cafzqkgxuwodf5bw0qqkxopoz0cffa1asjfuxfftmggs5-vk...@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/[email protected]/
Userspace implementation was done and so we merged this for 6.19.
https://lore.kernel.org/amd-gfx/cafzqkgwlwcys0sqchoigsjd5j_u4abj0hmv5bx3nknldlkr...@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/[email protected]/
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.
IMO we should either
- admit we can't do a generic property for this *and* keep the vendor
specific property details hidden in drivers, or
- figure out a generic property and add that in drm core
But I'm pretty much against adding an AMD vendor specific property in
drm core.
Agreed
Maxime
OK - I will discard this patch.
Simona, please any comments if you still want to see anything change
from the vendor property.
