Message-ID: <d30e50bf-5b8e-47cb-8abf-e474f8490c99@amd.com>
Date: Fri, 16 Feb 2024 11:13:40 -0500
From: Harry Wentland <harry.wentland@....com>
To: Pekka Paalanen <pekka.paalanen@...oniitty.fi>
Cc: Hamza Mahfooz <hamza.mahfooz@....com>, amd-gfx@...ts.freedesktop.org,
 Mario Limonciello <mario.limonciello@....com>, Leo Li <sunpeng.li@....com>,
 Rodrigo Siqueira <Rodrigo.Siqueira@....com>,
 Alex Deucher <alexander.deucher@....com>,
 Christian König <christian.koenig@....com>,
 "Pan, Xinhui" <Xinhui.Pan@....com>, David Airlie <airlied@...il.com>,
 Daniel Vetter <daniel@...ll.ch>, Alex Hung <alex.hung@....com>,
 Srinivasan Shanmugam <srinivasan.shanmugam@....com>,
 Wayne Lin <wayne.lin@....com>, dri-devel@...ts.freedesktop.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] drm/amd/display: add panel_power_savings sysfs entry
 to eDP connectors



On 2024-02-16 11:11, Harry Wentland wrote:
> 
> 
> On 2024-02-16 10:42, Pekka Paalanen wrote:
>> On Fri, 16 Feb 2024 09:33:47 -0500
>> Harry Wentland <harry.wentland@....com> wrote:
>>
>>> On 2024-02-16 03:19, Pekka Paalanen wrote:
>>>> On Fri, 2 Feb 2024 10:28:35 -0500
>>>> Hamza Mahfooz <hamza.mahfooz@....com> wrote:
>>>>   
>>>>> We want programs besides the compositor to be able to enable or disable
>>>>> panel power saving features.  
>>>>
>>>> Could you also explain why, in the commit message, please?
>>>>
>>>> It is unexpected for arbitrary programs to be able to override the KMS
>>>> client, and certainly new ways to do so should not be added without an
>>>> excellent justification.
>>>>
>>>> Maybe debugfs would be more appropriate if the purpose is only testing
>>>> rather than production environments?
>>>>   
>>>>> 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.  
>>>>
>>>> When the DRM property was added, what was used as the userspace to
>>>> prove its workings?
>>>>   
>>>
>>> I don't think there ever was a userspace implementation and doubt any
>>> exists today. Part of that is on me. In hindsight, the KMS prop should
>>> have never gone upstream.
>>>
>>> I suggest we drop the KMS prop entirely.
>>
>> Sounds good. What about the sysfs thing? Should it be a debugfs thing
>> instead, assuming the below question will be resolved?
>>
> 
> 
> It's intended to be used by the power profiles daemon (PPD). I don't think
> debugfs is the right choice. See
> https://gitlab.freedesktop.org/upower/power-profiles-daemon/-/commit/41ed5d33a82b0ceb7b6d473551eb2aa62cade6bc
> 
>>> As for the color accuracy topic, I think it is important that compositors
>>> can have full control over that if needed, while it's also important
>>> for HW vendors to optimize for power when absolute color accuracy is not
>>> needed. An average end-user writing code or working on their slides
>>> would rather have a longer battery life than a perfectly color-accurate
>>> display. We should probably think of a solution that can support both
>>> use-cases.
>>
>> I agree. Maybe this pondering should start from "how would it work from
>> end user perspective"?
>>
>> "Automatically" is probably be most desirable answer. Some kind of
> 
> I agree
> 
>> desktop settings with options like "save power at the expense of image
>> quality":
>> - always
>> - not if watching movies/gaming
>> - on battery
>> - on battery, unless I'm watching movies/gaming
>> - never
>>
> 
> It's interesting that you split out movies/gaming, specifically. AMD's
> ABM algorithm seems to have considered movies in particular when
> evaluating the power/fidelity trade-off.
> 
> I wouldn't think consumer media is very particular about a specific
> color fidelity (despite what HDR specs try to make you believe). Where
> color fidelity would matter to me is when I'd want to edit pictures or
> video.
> 
> The "abm_level" property that we expose is really just that, a setting
> for the strength of the power-savings effect, with 0 being off and 4 being
> maximum strength and power saving, at the expense of fidelity.
> 
> Mario's work is to let the PPD control it and set the ABM levels based on
> the selected power profile:
> 0 - Performance
> 1 - Balance
> 3 - Power
> 
> And I believe we've looked at disabling ABM (setting it to 0) automatically
> if we know we're on AC power.
> 
>> Or maybe there already is something like that, and it only needs to be
>> plumbed through?
>>
>> Which would point towards KMS clients needing to control it, which
>> means a generic KMS prop rather than vendor specific?
>>
>> Or should the desktop compositor be talking to some daemon instead of
>> KMS for this? Maybe they already are?
>>
> 
> I think the intention is for the PPD to be that daemon. Mario can elaborate.
> 

Some more details and screenshots on how the PPD is expected to work and look:
https://linuxconfig.org/how-to-manage-power-profiles-over-d-bus-with-power-profiles-daemon-on-linux

Harry

> Harry
> 
>>
>> Thanks,
>> pq
> 

Powered by blists - more mailing lists