linux-kernel - Re: [PATCH] drm/amd: Move adaptive backlight modulation property to drm core

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <o737pxpoq5t62xlvgflhypn67o6d7qohl5zm5ufgtojxlkw4cv@ao6d4gscekny>
Date: Tue, 18 Nov 2025 09:47:37 +0100
From: Maxime Ripard <mripard@...nel.org>
To: Jani Nikula <jani.nikula@...ux.intel.com>
Cc: Mario Limonciello <mario.limonciello@....com>, 
	Simona Vetter <simona@...ll.ch>, xaver.hugl@....org, 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>
Subject: Re: [PATCH] drm/amd: Move adaptive backlight modulation property to
 drm core

On Mon, Nov 17, 2025 at 11:05:10AM +0200, Jani Nikula wrote:
> On Fri, 14 Nov 2025, Mario Limonciello <mario.limonciello@....com> 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/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.
> 
> 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

Download attachment "signature.asc" of type "application/pgp-signature" (274 bytes)