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. BR, Jani. -- Jani Nikula, Intel
