On Monday, October 23rd, 2023 at 10:25, Simon Ser <cont...@emersion.fr> wrote:
> > > > > > > > > > +An atomic commit with the flag DRM_MODE_PAGE_FLIP_ASYNC is
> > > > > > > > > > allowed to
> > > > > > > > > > +effectively change only the FB_ID property on any planes.
> > > > > > > > > > No-operation changes
> > > > > > > > > > +are ignored as always. [...]
> > > > > > > > > > During the hackfest in Brno, it was mentioned that a commit
> > > > > > > > > > which re-sets the same FB_ID could actually have an effect
> > > > > > > > > > with VRR: It could trigger scanout of the next frame before
> > > > > > > > > > vertical blank has reached its maximum duration. Some kind
> > > > > > > > > > of mechanism is required for this in order to allow user
> > > > > > > > > > space to perform low frame rate compensation.
> > > > > > > >
> > > > > > > > Xaver tested this hypothesis in a flipping the same fb in a VRR
> > > > > > > > monitor
> > > > > > > > and it worked as expected, so this shouldn't be a concern.
> > > > > > > > Right, so it must have some effect. It cannot be simply ignored
> > > > > > > > like in
> > > > > > > > the proposed doc wording. Do we special-case re-setting the
> > > > > > > > same FB_ID
> > > > > > > > as "not a no-op" or "not ignored" or some other way?
> > > > > > > > There's an effect in the refresh rate, the image won't change
> > > > > > > > but it
> > > > > > > > will report that a flip had happened asynchronously so the
> > > > > > > > reported
> > > > > > > > framerate will be increased. Maybe an additional wording could
> > > > > > > > be like:
> > > > > >
> > > > > > Flipping to the same FB_ID will result in a immediate flip as if it
> > > > > > was
> > > > > > changing to a different one, with no effect on the image but
> > > > > > effecting
> > > > > > the reported frame rate.
> > > > >
> > > > > Re-setting FB_ID to its current value is a special case regardless of
> > > > > PAGE_FLIP_ASYNC, is it not?
> > > >
> > > > No. The rule has so far been that all side effects are observed
> > > > even if you flip to the same fb. And that is one of my annoyances
> > > > with this proposal. The rules will now be different for async flips
> > > > vs. everything else.
> > >
> > > Well with the patches the async page-flip case is exactly the same as
> > > the non-async page-flip case. In both cases, if a FB_ID is included in
> > > an atomic commit then the side effects are triggered even if the property
> > > value didn't change. The rules are the same for everything.
> >
> > I see it only checking if FB_ID changes or not. If it doesn't
> > change then the implication is that the side effects will in
> > fact be skipped as not all planes may even support async flips.
>
> Hm right. So the problem is that setting any prop = same value as
> previous one will result in a new page-flip for asynchronous page-flips,
> but will not result in any side-effect for asynchronous page-flips.
>
> Does it actually matter though? For async page-flips, I don't think this
> would result in any actual difference in behavior?
To sum this up, here is a matrix of behavior as seen by user-space:
- Sync atomic page-flip
- Set FB_ID to different value: programs hw for page-flip, sends uevent
- Set FB_ID to same value: same (important for VRR)
- Set another plane prop to same value: same
- Set another plane prop to different value: maybe rejected if modeset
required
- Async atomic page-flip
- Set FB_ID to different value: updates hw with new FB address, sends
immediate uevent
- Set FB_ID to same value: same (no-op for the hw)
- Set another plane prop to same value: ignored, sends immediate uevent
(special codepath)
- Set another plane prop to different value: always rejected
To me sync and async look consistent.
