On 10-10-2025 19:02, Ville Syrjälä wrote:
On Fri, Oct 10, 2025 at 02:15:57PM +0530, Arun R Murthy wrote:
struct drm_crtc_state {
/**
* @async_flip:
*
* This is set when DRM_MODE_PAGE_FLIP_ASYNC is set in the legacy
* PAGE_FLIP IOCTL. It's not wired up for the atomic IOCTL
itself yet.
*/
bool async_flip;
In the existing code the flag async_flip was intended for the legacy
PAGE_FLIP IOCTL. But the same is being used for atomic IOCTL.
As per the hardware feature is concerned, async flip is a plane feature
and is to be treated per plane basis and not per pipe basis.
You can't treat is as a purely per plane thing. The real problem that
neesd solving is that commit completion happens on a per-crtc basis,
and userspace is dumb enough to try to shove both sync and async stuff
into the same commit. That simply cannot work (unless we downgrade
all such async commits into sync commits).
Yes, the commit completion should happen per-crtc basis. Also as said
below in
order to handle multiple async flips, this async_flip flag which is in
crtc_state
should be moved to plane_state. Then we will have to work on the driver
policy for async and sync flips.
The sort of compromise that people seem to have agreed on is that we
can allow objects and properties in the async commit that would normally
require a sync commit, but only if their value isn't changing. And
then someone is supposed to filter those out so they don't end up
requiring a real sync commit on the hardware.
Do you mean to allow only fb change for sync flips along with async flip?
Noted, will take care of this while implementing.
I think the only sane way forward is to make the drm core do said
filtering, so that in the end the driver will only see the things
in the commit that are actually supposed to take part in the async
commit. I think in practice it would just involve removing all the
planes with zero property changes from the atomic state before
handing it over to the driver. But so far no one has volunteered
to write that code.
This will be done in this series. The first set of patches has been
floated to
get the feedback on these kind of policies. Will take care of this!
There is of course still the remaining problem of what happens when
there are multiple async flips for different planes in the same commit.
The hardware will complete those at different times, so the driver will
still need to make sure it doesn't signal full commit completion until
all the async flips have finished. Maybe we could eventually add
something to the common commit machinery to help with this, but I
guess we should first implement it in a few drivers.
Yes policy will be added for the same to handle multiple async flips
within a
single commit.
Sure we can take up the implementation with i915 first.
Thanks for the feedback. With all these comments, I can list down the below
tasks
1. Use async_flip flag in plane_state along with a plane property for async
flip enabling. (Done in this RFC)
2. Policy to filter out any changes other than fb. Add-on to this allow even
sync flips if there is no change in the property except fb.
3. For multiple async commits ensure notification/releasing fb after
completion of all the async flips in that crtc.
With these task listed out, was think if we break the patch series into 3
as listed in the above task will be easier to handle rather then coming
up with a huge list of patches in a single series.
Thanks and Regards,
Arun R Murthy
-------------------
