Discuss what to do with amdgpu panel power savings
There has been some strong feedback about how amdgpu panel power savings works from various people.
-
Xaver Hugl from KDE had feedback about how the compositor should be in control of things that control the color accuracy. This has lead to some kernel patches that were developed to allow the compositor to opt a driver out of features that affect color accuracy. This effectively causes panel power saving to be turned off when the compositor chooses this option. Compositors can then set the policies accordingly for when power profiles daemon can use the power saving features and power profiles daemon would set the policy of how aggressively to configure it. https://lore.kernel.org/dri-devel/20240606020404.210989-1-mario.limonciello@amd.com/
-
Some users on Framework forums have reported that the values used for power saver on battery ("3") degrade the quality of the image too much. This is a personal preference for users. https://community.frame.work/t/solved-color-issues-in-linux-6-9/52158
-
Sebastien Wick from mutter (GNOME) feels that the way this was implemented is wrong and a regression. He thinks the sysfs file should be reverted and this should ONLY be changed by the compositor. https://gitlab.gnome.org/GNOME/mutter/-/issues/3589
-
One user reported a bug in mutter where the hotplug event from amdgpu to turn on panel power savings causes GNOME shell to behave in unexpected ways. https://gitlab.gnome.org/GNOME/mutter/-/issues/3578
For a refresher, here is how everything currently works:
flowchart LR
subgraph kernel
amdgpu
battery
end
subgraph power-profiles-daemon
panel_power_savings(amdgpu action)
core
end
subgraph PPD clients
GNOME
powerprofilesctl
KDE
end
GNOME -.- dbus
dbus -.-> core
KDE -.- dbus
powerprofilesctl -.- dbus
battery --> core
core --- panel_power_savings
panel_power_savings -- sysfs --> amdgpu
The default policy for panel power savings is:
Power Saver | Balanced | Performance | |
---|---|---|---|
Battery | 3 | 1 | 0 |
AC | 0 | 0 | 0 |
Currently there is a proposal !199 (merged) in place to change it so that the values are "progressive". This means the values would change dynamically based upon the remaining battery life. The more intense values for panel power savings would only be applied when the battery was very low.
However this still masks a core problem that not all users want to have panel power savings turned on at all and there needs to be a more discoverable way for them to turn it off. There are a few ways to approach the problem, and I want to discuss them.
-
Revert the sysfs file, revert the panel power savings action from PPD and then make every single compositor implement policy for when to turn on panel power savings. This is what Sebastian from mutter advocates for, but I'm really not a fan of it. By letting the compositor control whether it's on or off we can have the policy of intensity implemented in a central location (ppd). The downside is that users still don't have a configurable way to change the intensity. Furthermore; this can be argued as a regression because userspace (PPD) is using this file and no longer could.
-
If we reverted the sysfs file we can change PPD to advertise a dbus property of what the requested panel power savings value is, and then compositors can use that to directly set the policy via a DRM property as a modeset. This allows us to keep the policy value in PPD but the action of changing it in the compositor. I think this is a lot of complexity.
-
We can add support for PPD to turn on and off actions by dbus interface and save the result to a configuration file loaded by PPD at startup. Then clients like GNOME and mutter can have a page that lets users decide what PPD power savings features they want turned on. I personally like this, and it actually also supports the fact that there are other users that want to have some actions that might not be popular with everyone. For example: #158 (closed). We can even take this a step further then and set panel power savings as default "off" but require using the dbus interface to turn it on for the first time.