On 2/15/2024 11:54, Harry Wentland wrote:
On 2024-02-02 11:20, Mario Limonciello wrote:
On 2/2/2024 09:28, Hamza Mahfooz wrote:
We want programs besides the compositor to be able to enable or disable
panel power saving features. However, since they are currently only
configurable through DRM properties, that isn't possible. So, to remedy
that issue introduce a new "panel_power_savings" sysfs attribute.
I've been trying to avoid looking at this too closely, partly because
I want ABM enablement by default, with control for users. But the
more I think about this the more uncomfortable I get. The key for my
discomfort is that we're going around the back of DRM master to set
a DRM property. This is apt to create lots of weird behaviors,
especially if compositors also decide to implement support for the
abm_level property and then potentially fight PPD, or other users
of this sysfs.
I feel the problem is that specifically both DRM master and sysfs can
simultaneously fight over the value for ABM.
This isn't a totally new problem because previously the user and DRM
master could fight over this (with the user setting it on command line
and DRM master overriding that). That was fixed in a follow up patch
though in that the DRM property isn't attached if a user sets the value
on the command line.
I feel the solution to these concerns is that we should make a knob that
controls whether the DRM property is created or the sysfs file is
created but not let them happen simultaneously.
We already have amdgpu.abmlevel=-1 is the only time sysfs is created.
I'd say we should drop the drm property in that case and add a case for
amdgpu.abmlevel=-2 which will only make the drm property and not the
sysfs value. With that done it turns into:
-2 : DRM master is in control
-1 : sysfs is in control. Software like PPD will tune it as needed.
0-4 : User is in control.
