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
signature.asc
Description: PGP signature
