[PATCH 0/2] Add support for Panel Power Savings property

[Leo Li]

On 2024-05-21 13:32, Mario Limonciello wrote:
> On 5/21/2024 12:27, Leo Li wrote:
>>
>>
>> On 2024-05-21 12:21, Mario Limonciello wrote:
>>> On 5/21/2024 11:14, Xaver Hugl wrote:
>>>> Am Di., 21. Mai 2024 um 16:00 Uhr schrieb Mario Limonciello
>>>> <mario.limonciello at amd.com>:
>>>>>
>>>>> On 5/21/2024 08:43, Simon Ser wrote:
>>>>>> This makes sense to me in general. I like the fact that it's simple and
>>>>>> vendor-neutral.
>>>>>>
>>>>>> Do we want to hardcode "panel" in the name? Are we sure that this will
>>>>>> ever only apply to panels?
>>>>>>
>>>>>> Do we want to use a name which reflects the intent, rather than the
>>>>>> mechanism? In other words, something like "color fidelity" = "preferred"
>>>>>> maybe? (I don't know, just throwing ideas around.)
>>>>>
>>>>> In that vein, how about:
>>>>>
>>>>> "power saving policy"
>>>>> --> "power saving"
>>>>> --> "color fidelity"
>>>>
>>>> It's not just about colors though, is it? The compositor might want to
>>>> disable it to increase the backlight brightness in bright
>>>> environments, so "color fidelity" doesn't really sound right
>>>
>>> Either of these better?
>>>
>>> "power saving policy"
>>> --> "power saving"
>>> --> "accuracy"
>>>
>>> "power saving policy"
>>> --> "allowed"
>>> --> "forbidden"
>>>
>>> Or any other idea?
>>
>> Another consideration in addition to accuracy is latency.
>>
>> I suppose a compositor may want to disable features such as PSR for use-cases 
>> requiring low latency. Touch and stylus input are some examples.
>>
>> I wonder if flags would work better than enums? A compositor can set something 
>> like `REQUIRE_ACCURACY & REQUIRE_LOW_LATENCY`, for example.
>>
> 
> I thought we had said the PSR latency issue discussed at the hackfest was likely 
> just a bug and that nominally it won't matter?

Ah, either my memory failed, or I failed to clarify. Let me fix that below.

> 
> If it really will matter enough then yeah I think you're right that flags would 
> be better.  More drivers would probably want to opt into the property for the 
> purpose of turning off PSR/PSR2/panel replay as well then.

Latency technically shouldn't be a problem for Panel Replay, but is a problem
for PSR/PSR2 due to their design. At least it is a problem for amdgpu, not sure
about other drivers.

FWIU, in short, when a panel (DPRX) goes into self-refresh, it's clock can drift
from the gpu (DPTX). Since there's no resync mechanism in PSR/PSR2, it is
possible for the panel to lag behind by 1-frame-time(ms) for a noticeable period
of time. Panel replay has a resync mechanism, so it theoretically can resync
within 1 frame.

Since, fwict, replay panels are still rare, I think it'll be nice for
compositors that care about latency to have the option to temporarily disable
PSR/PSR2.

- Leo

> 
>> - Leo
>>
>>>
>>>>
>>>>>>
>>>>>> Would be nice to add documentation for the property in the "standard
>>>>>> connector properties" section.
>>>>>
>>>>> Ack.
>>>>>
>>>
>