[ANNOUNCE] weston 12.0.94
Hi all, This is the RC2 release for Weston 13.0.0. Full commit history below: Aske Bækdal Møller (1): clients: keyboard: fix delete before cursor Derek Foreman (5): vnc: Remove scanout plane optimization libweston: Don't clip damage to paint node visible region libweston: Don't set VISIBILITY_DIRTY on non-primary planes vnc: Fix cursor updates drm-backend: Fix cursor updates with overlapping heads Marius Vlad (1): build: bump to version 12.0.94 for the RC2 release git tag: 12.0.94 https://gitlab.freedesktop.org/wayland/weston/-/releases/12.0.94/downloads/weston-12.0.94.tar.xz SHA256: 7c463d7630af5d49bcf92e09e42725eaf22a9a0851e8c90d4d830e9a7c95575b weston-12.0.94.tar.xz SHA512: 7e5fd718cbee30a466e14be7e35fd9cc1a7eb42c147a77271692fd04af62b6eda7c9c6ac4acfb787e184b0cf34563bad8b98c4159b03b9ccf43ef9958b7d916a weston-12.0.94.tar.xz PGP: https://gitlab.freedesktop.org/wayland/weston/-/releases/12.0.94/downloads/weston-12.0.94.tar.xz.sig signature.asc Description: PGP signature
Re: [PATCH v6 6/6] drm/doc: Define KMS atomic state set
On Monday, November 13th, 2023 at 11:15, Pekka Paalanen wrote: > > > On Mon, 13 Nov 2023 09:44:15 + > 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 + > > > 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 > > Wou
Re: [PATCH v6 6/6] drm/doc: Define KMS atomic state set
On Mon, 13 Nov 2023 09:44:15 + Simon Ser wrote: > On Monday, November 13th, 2023 at 10:38, Pekka Paalanen > wrote: > > > On Mon, 13 Nov 2023 09:18:39 + > > 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
Re: [RFC PATCH 01/10] drm/doc/rfc: Describe why prescriptive color pipeline is needed
On Fri, 10 Nov 2023 15:27:03 -0500 Harry Wentland wrote: > On 2023-11-09 04:20, Pekka Paalanen wrote: > > On Wed, 8 Nov 2023 11:27:35 -0500 > > Harry Wentland wrote: > > > >> On 2023-11-08 11:19, Pekka Paalanen wrote: > >>> On Wed, 8 Nov 2023 09:31:17 -0500 > >>> Harry Wentland wrote: > >>> > On 2023-11-08 06:40, Sebastian Wick wrote: > > On Wed, Nov 8, 2023 at 11:16 AM Pekka Paalanen > > wrote: > > > > ... > > > >> An incremental UAPI development approach is fine by me, meaning that > >> pipelines might not be complete at first, but I believe that requires > >> telling userspace whether the driver developers consider the pipeline > >> complete (no undescribed operations that would significantly change > >> results from the expected results given the UAPI exposed pipeline). > >> > >> The prime example of what I would like to know is that if a FB > >> contains PQ-encoded image and I use a color pipeline to scale that > >> image up, will the interpolation happen before or after the non-linear > >> colorop that decodes PQ. That is a significant difference as pointed > >> out by Joshua. > >> > > That's fair and I want to give that to you. My concern stems from > the sentiment that I hear that any pipeline that doesn't explicitly > advertise this is useless. I don't agree there. Let's not let perfect > be the enemy of good. > >>> > >>> It's up to the use case. The policy of what is sufficient should reside > >>> in userspace. > >>> > >>> What about matching compositor shader composition with KMS? > >>> > >>> Can we use that as a rough precision threshold? If userspace implements > >>> the exact same color pipeline as the KMS UAPI describes, then that and > >>> the KMS composited result should be indistinguishable in side-by-side > >>> or alternating visual inspection on any monitor in isolation. > >>> > >>> Did this whole effort not start from wanting to off-load things to > >>> display hardware but still maintain visual equivalence to software/GPU > >>> composition? > >>> > >> > >> I agree with you and I want all that as well. > >> > >> All I'm saying is that every userspace won't have the same policy of > >> what is sufficient. Just because Weston has a very high threshold > >> doesn't mean we can't move forward with a workable solution for other > >> userspace, as long as we don't do something that prevents us from > >> doing the perfect solution eventually. > >> > >> And yes, I do want a solution that works for Weston and hear your > >> comments on what that requires. > > > > I totally agree. > > > > How will that be reflected in the UAPI? If some pipelines are different > > from others in correctness/strictness perspective, how will userspace > > tell them apart? > > > > Is the current proposal along the lines of: userspace creates a > > software pipeline first, and if it cannot map all operations on it to > > KMS color pipeline colorops, then the KMS pipeline is not sufficient? > > > > The gist being, if the software pipeline is scaling the image for > > example, then it must find a scaling colorop in the KMS pipeline if it > > cares about the scaling correctness. > > > > With a simplified model of an imaginary color pipeline I expect this > to look like this: > > Color Pipeline 1: >EOTF Curve - CTM > > Color Pipeline 2: >EOTF Curve - scale - CTM > > Realistically both would most likely map to the same HW blocks. > > Assuming userspace A and B do the following: >EOTF Curve - scale - CTM > > Userspace A doesn't care about scaling and would only look for: >EOTF Curve - CTM > > and find a match with Color Pipeline 1. > > Userspace B cares about scaling and would look for >EOTF Curve - scale - CTM > > and find a match with Color Pipeline 2. > > If Color Pipeline 2 is not exposed in the first iteration of the > driver's implementation userspace A would still be able to make > use of it, but userspace B would not, since it requires a defined > scale operation. If the driver then exposes Color Pipeline 2 in a > second iteration userspace B can find a match for what it needs > and make use of it. > > Realistically userspace B would not attempt to implement a DRM/KMS > color pipeline implementation unless it knows that there's a driver > that can do what it needs. > > > Another example is YUV pixel format on an FB that magically turns into > > some kind of RGB when sampled, but there is no colorop to tell what > > happens. I suppose userspace cannot assume that the lack of colorop > > there means an identity YUV->RGB matrix either? How to model > > that? I guess the already mentioned pixel format requirements on a > > pipeline would help, making it impossible to use a pipeline without a > > YUV->RGB colorop on a YUV FB unless the lack of colorop does indeed > > mean an identity matrix. > > > > I agree. > > > The same with sub-sampled YUV which p
Re: [PATCH v6 6/6] drm/doc: Define KMS atomic state set
On 11/13/23 10:47, Simon Ser wrote: > On Monday, November 13th, 2023 at 10:41, Michel Dänzer > wrote: > >> On 11/13/23 10:18, Simon Ser 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? >>> >>> 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 >> >> A page flip is programmed even if FB_ID isn't touched? > > I believe so. Set CRTC_X on a plane to the same value as before, and the > CRTC gets implicitly included in the atomic commit? > >>> - 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) >> >> No-op implies it doesn't trigger scanning out a frame with VRR, if >> scanout is currently in vertical blank. Is that the case? If so, async >> flips can't reliably trigger scanning out a frame with VRR. > > By no-op I mean that the hw is programmed for an immediate async flip > with the same buffer addr as the previous one. So this doesn't actually > change anything. If a flip is programmed to the HW, it's not a no-op any more than in the sync case, in particular not with VRR. -- Earthling Michel Dänzer| https://redhat.com Libre software enthusiast | Mesa and Xwayland developer
Re: [PATCH v6 6/6] drm/doc: Define KMS atomic state set
On Monday, November 13th, 2023 at 10:41, Michel Dänzer wrote: > On 11/13/23 10:18, Simon Ser 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? > > > > 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 > > A page flip is programmed even if FB_ID isn't touched? I believe so. Set CRTC_X on a plane to the same value as before, and the CRTC gets implicitly included in the atomic commit? > > - 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) > > No-op implies it doesn't trigger scanning out a frame with VRR, if > scanout is currently in vertical blank. Is that the case? If so, async > flips can't reliably trigger scanning out a frame with VRR. By no-op I mean that the hw is programmed for an immediate async flip with the same buffer addr as the previous one. So this doesn't actually change anything.
Re: [PATCH v6 6/6] drm/doc: Define KMS atomic state set
On Monday, November 13th, 2023 at 10:38, Pekka Paalanen wrote: > On Mon, 13 Nov 2023 09:18:39 + > 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 > > - 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 -
Re: [PATCH v6 6/6] drm/doc: Define KMS atomic state set
On 11/13/23 10:18, Simon Ser wrote: > On Monday, October 23rd, 2023 at 10:25, Simon Ser 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 A page flip is programmed even if FB_ID isn't touched? > - 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) No-op implies it doesn't trigger scanning out a frame with VRR, if scanout is currently in vertical blank. Is that the case? If so, async flips can't reliably trigger scanning out a frame with VRR. -- Earthling Michel Dänzer| https://redhat.com Libre software enthusiast | Mesa and Xwayland developer
Re: [PATCH v6 6/6] drm/doc: Define KMS atomic state set
On Mon, 13 Nov 2023 09:18:39 + Simon Ser wrote: > On Monday, October 23rd, 2023 at 10:25, Simon Ser 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? > - 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? > - Set another plane prop to different value: always rejected > > To me sync and async look consistent. Thanks, pq pgpQXmLWL0FFw.pgp Description: OpenPGP digital signature
Re: [PATCH v6 6/6] drm/doc: Define KMS atomic state set
On Monday, October 23rd, 2023 at 10:25, Simon Ser 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.
