[PATCH] drm/amd: Drop abm_level property

[Mario Limonciello]

On 3/6/2024 12:59, Harry Wentland wrote:
> 
> 
> On 2024-03-06 13:02, Mario Limonciello wrote:
>> On 3/6/2024 12:00, Xaver Hugl wrote:
>>> Am Mi., 6. März 2024 um 18:19 Uhr schrieb Mario Limonciello
>>> <mario.limonciello at amd.com>:
>>>> So the idea being if the compositor isn't using it we let
>>>> power-profiles-daemon (or any other software) take control via sysfs and
>>>> if the compositor does want to control it then it then it writes a DRM
>>>> cap and we destroy the sysfs file?
>>>
>>> Yes. That way still only one party controls it at a given time, and we
>>> can get both good default behavior for display servers that don't care
>>> (like Xorg or compositors without color management support), and
>>> compositors that want to put in the effort can do more specific things
>>> with it.
>>
>> I think that's a very good solution.
>>
>> Harry, Hamza, what do you guys think?
> 
> In theory I like it. But how will this look in practice? Is PPD or compositor
> on the scene first? Would it be possible to yank the sysfs away from PPD?
> 

I double checked the existing PPD code to see how well this case maps out.

* If the compositor shows up first then PPD just wouldn't find amdgpu 
support when it probed.
* If PPD goes first and then the compositor I expect right now that PPD 
wouldn't explode.  All the cases that would update it check that the 
file exists before trying to write it.

I think the only risk is TOCTOU for that check relative to when it would 
write the file, but the worst case is you have an error in the journal 
log that it tried to write the file but it's not present (there is 
already error handling everywhere for this).

What we can do to smooth it out even more is export a CHANGE uevent from 
amdgpu when this happens.  PPD can react to the change event and totally 
stop the amdgpu plugin.

> DRM client caps are set by the client when the client interacts with DRM.
> At driver creation there is no client. How will the driver set things up?
> 

You jog my mind for another idea.

1) We could reintroduce abm_level property.
2) If client connects with the cap set, we make any reads or writes to 
the sysfs return -EBUSY and we make amdgpu issue a CHANGE uevent.

* Unchanged PPD will likely log a message to the journal that writes 
fail, but won't explode.
* A changed PPD could react to the uevent and look for -EBUSY to not 
bother even logging a message to the journal.

> A user might switch between DRM clients (login manager, to desktop compositor,
> maybe to another VT with a different compositor). I know everything but the
> login manager to desktop compositor hand-off is today considered exotic, but
> what if someone starts building a use-case for it? I've done a bunch of gamescope
> or IGT work in a different VT while I've had Plasma running on its default
> VT.
> 

If going off the cap of the client to decide whether or not to report 
-EBUSY to sysfs I think it handles the handoff cleanly.

> If someone can sketch this out, with answers to all the questions above and
> any other questions you can come up (be creative), I'd be happy to review.
> Alternatively we can discuss this at the hackfest and maybe arrive at a
> solution.
> 
Yes; I think we should use this thread as a basis for that discussion.