On Mon, 13 Nov 2023 09:44:15 +0000 Simon Ser <cont...@emersion.fr> wrote:
> On Monday, November 13th, 2023 at 10:38, Pekka Paalanen <ppaala...@gmail.com> > wrote: > > > On Mon, 13 Nov 2023 09:18:39 +0000 > > Simon Ser cont...@emersion.fr wrote: > > > > > 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? > > > > > > Hi Simon, > > > > a fly-by question... > > > > > 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) > > > > It should not be a no-op for the hw, because the hw might be in the > > middle of a VRR front-porch waiting period, and the commit needs to end > > the waiting immediately rather than time out? > > I'm not sure Would people not want to use async commits to trigger new VRR scanout cycles without content updates? I seem to recall previous comments that switching between sync and async commit modes may take a moment (intel's one last sync flip), so using sync once in a while then using async otherwise is probably not a good idea. > > > - Set another plane prop to same value: ignored, sends immediate uevent > > > (special codepath) > > > > If the sync case says "same", then shouldn't this say "same" as well to > > be consistent? > > Okay, I think I chose my words badly. By "same" I meant "same as > previous item in the list". > > Here I tried to be more explicit and explain why it's the same behavior. > We have a special path in the kernel code that ignores the change, but > the effective result is that it doesn't differ from the sync case. > > Here's a fixed matrix where I don't use confusing words: > > - Sync atomic page-flip > - Set FB_ID to different value: programs hw for page-flip, sends uevent > - Set FB_ID to same value: programs hw for page-flip, sends uevent > (important > for VRR) > - Set another plane prop to same value: programs hw for page-flip, sends > uevent Programming hw for page-flip probably triggers a new VRR scanout cycle, even if the FB address didn't change. > - 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: updates hw with new FB address (no-op for the > hw), > sends immediate uevent > - Set another plane prop to same value: ignored, sends immediate uevent > (special codepath) Just like Michel points out: if this case has a special case ignoring the set, then this case will not trigger a new VRR scanout cycle like the corresponding sync case does. > - Set another plane prop to different value: always rejected Thanks, pq
pgpj0Dtol077h.pgp
Description: OpenPGP digital signature
