Message ID | 1458758847-21170-2-git-send-email-gustavo@padovan.org (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Hi, 2016? 03? 24? 03:47? Gustavo Padovan ?(?) ? ?: > From: Gustavo Padovan <gustavo.padovan@collabora.co.uk> > > FENCE_FD can now be set by the user during an atomic IOCTL, it > will be used by atomic_commit to wait until the sync_file is signalled, > i.e., the framebuffer is ready for scanout. It seems that this patch makes Linux DRM to be dependent of Android which uses explicit sync interfaces. So was there any a consensus that Android sync driver is used for DMA buffer synchronization as a Linux standard? I see the Android sync driver exists in staging yet. Isn't the implicit sync interface used for DMA device as a Linux standard? I don't see why Android specific things are spreaded into Linux DRM. Thanks, Inki Dae > > Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk> > --- > drivers/gpu/drm/drm_atomic.c | 4 ++++ > drivers/gpu/drm/drm_atomic_helper.c | 1 + > drivers/gpu/drm/drm_crtc.c | 7 +++++++ > include/drm/drm_crtc.h | 3 +++ > 4 files changed, 15 insertions(+) > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c > index 8fb469c..8bc364c 100644 > --- a/drivers/gpu/drm/drm_atomic.c > +++ b/drivers/gpu/drm/drm_atomic.c > @@ -609,6 +609,8 @@ int drm_atomic_plane_set_property(struct drm_plane *plane, > drm_atomic_set_fb_for_plane(state, fb); > if (fb) > drm_framebuffer_unreference(fb); > + } else if (property == config->prop_fence_fd) { > + state->fence_fd = val; > } else if (property == config->prop_crtc_id) { > struct drm_crtc *crtc = drm_crtc_find(dev, val); > return drm_atomic_set_crtc_for_plane(state, crtc); > @@ -666,6 +668,8 @@ drm_atomic_plane_get_property(struct drm_plane *plane, > > if (property == config->prop_fb_id) { > *val = (state->fb) ? state->fb->base.id : 0; > + } else if (property == config->prop_fence_fd) { > + *val = state->fence_fd; > } else if (property == config->prop_crtc_id) { > *val = (state->crtc) ? state->crtc->base.id : 0; > } else if (property == config->prop_crtc_x) { > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c > index 2b430b0..4f91f84 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c > @@ -2594,6 +2594,7 @@ void drm_atomic_helper_plane_reset(struct drm_plane *plane) > if (plane->state) { > plane->state->plane = plane; > plane->state->rotation = BIT(DRM_ROTATE_0); > + plane->state->fence_fd = -1; > } > } > EXPORT_SYMBOL(drm_atomic_helper_plane_reset); > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c > index 65258ac..165f199 100644 > --- a/drivers/gpu/drm/drm_crtc.c > +++ b/drivers/gpu/drm/drm_crtc.c > @@ -1291,6 +1291,7 @@ int drm_universal_plane_init(struct drm_device *dev, struct drm_plane *plane, > > if (drm_core_check_feature(dev, DRIVER_ATOMIC)) { > drm_object_attach_property(&plane->base, config->prop_fb_id, 0); > + drm_object_attach_property(&plane->base, config->prop_fence_fd, -1); > drm_object_attach_property(&plane->base, config->prop_crtc_id, 0); > drm_object_attach_property(&plane->base, config->prop_crtc_x, 0); > drm_object_attach_property(&plane->base, config->prop_crtc_y, 0); > @@ -1546,6 +1547,12 @@ static int drm_mode_create_standard_properties(struct drm_device *dev) > return -ENOMEM; > dev->mode_config.prop_fb_id = prop; > > + prop = drm_property_create_signed_range(dev, DRM_MODE_PROP_ATOMIC, > + "FENCE_FD", -1, INT_MAX); > + if (!prop) > + return -ENOMEM; > + dev->mode_config.prop_fence_fd = prop; > + > prop = drm_property_create_object(dev, DRM_MODE_PROP_ATOMIC, > "CRTC_ID", DRM_MODE_OBJECT_CRTC); > if (!prop) > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h > index 8c7fb3d..a8f6ec0 100644 > --- a/include/drm/drm_crtc.h > +++ b/include/drm/drm_crtc.h > @@ -1239,6 +1239,7 @@ struct drm_connector { > * @crtc: currently bound CRTC, NULL if disabled > * @fb: currently bound framebuffer > * @fence: optional fence to wait for before scanning out @fb > + * @fence_fd: fd representing the sync_fence > * @crtc_x: left position of visible portion of plane on crtc > * @crtc_y: upper position of visible portion of plane on crtc > * @crtc_w: width of visible portion of plane on crtc > @@ -1257,6 +1258,7 @@ struct drm_plane_state { > struct drm_crtc *crtc; /* do not write directly, use drm_atomic_set_crtc_for_plane() */ > struct drm_framebuffer *fb; /* do not write directly, use drm_atomic_set_fb_for_plane() */ > struct fence *fence; > + int fence_fd; > > /* Signed dest location allows it to be partially off screen */ > int32_t crtc_x, crtc_y; > @@ -2098,6 +2100,7 @@ struct drm_mode_config { > struct drm_property *prop_crtc_w; > struct drm_property *prop_crtc_h; > struct drm_property *prop_fb_id; > + struct drm_property *prop_fence_fd; > struct drm_property *prop_crtc_id; > struct drm_property *prop_active; > struct drm_property *prop_mode_id; >
On Wed, Mar 23, 2016 at 2:47 PM, Gustavo Padovan <gustavo@padovan.org> wrote: > From: Gustavo Padovan <gustavo.padovan@collabora.co.uk> > > FENCE_FD can now be set by the user during an atomic IOCTL, it > will be used by atomic_commit to wait until the sync_file is signalled, > i.e., the framebuffer is ready for scanout. ok, so I've been slacking on writing up the reasons that I don't like the idea of using a property for fd's (including fence fd's).. I did at one point assume we'd use properties for fence fd's, but that idea falls apart pretty quickly when you think about the 'struct file' vs fd lifecycle. It could *possibly* work if it was a write-only property, but I don't think that is a good solution. The issue is that 'struct file' / 'int fd' have a fundamentally different lifecycle compared to 'kms obj' / 'u32 obj_id'. For the kms objects (like framebuffers) where we can use them with properties, the id is tied to the kernel side object. This is not true for file descriptors. Resulting in a few problems: 1) if it was a readable property, reading it would (should) take a reference.. we can't just return fence_fd that was previously set (since in the mean time userspace might have close()d the fd). So we have to return a fresh fd. And if userspace (like modetest) doesn't realize it is responsible to close() that fd we have a leak 2) basically we shouldn't be tracking fd's on the kernel side, since we can only count on them being valid during the duration of the ioctl call. Once the call returns, you must assume userspace has close()d the fd. So in the ioctl we need to convert fd -> file, and from then on out track the file object (or in this case the underlying fence object). I guess we *could* have something analogous to dmabuf, where we import the fence fd and get a kms drm_fence object (with an id tied to the kernel side object), and then use a property to attach the drm_fence object.. but that seems kind of pointless and just trying to force the 'everything is a property' mantra. I think it is really better to pass in an array of 'struct { u32 plane; int fd }' (which the atomic ioctl code converts into 'struct fence's and attaches to the plane state) and an array of 'struct { u32 crtc; int fd }' filled in on the kernel side for the out-fences. (I guess I can't think of any case where we'd have more out-fences than in-fences so we could possibly re-use the same array.. but I don't think we have to care about that sort of micro-optimization.. on the gpu side, the submit/execbuf ioctl already passes in a larger table of gem obj id's and reloc info, and that happens even more frequently than atomic ioctl.) BR, -R > Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk> > --- > drivers/gpu/drm/drm_atomic.c | 4 ++++ > drivers/gpu/drm/drm_atomic_helper.c | 1 + > drivers/gpu/drm/drm_crtc.c | 7 +++++++ > include/drm/drm_crtc.h | 3 +++ > 4 files changed, 15 insertions(+) > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c > index 8fb469c..8bc364c 100644 > --- a/drivers/gpu/drm/drm_atomic.c > +++ b/drivers/gpu/drm/drm_atomic.c > @@ -609,6 +609,8 @@ int drm_atomic_plane_set_property(struct drm_plane *plane, > drm_atomic_set_fb_for_plane(state, fb); > if (fb) > drm_framebuffer_unreference(fb); > + } else if (property == config->prop_fence_fd) { > + state->fence_fd = val; > } else if (property == config->prop_crtc_id) { > struct drm_crtc *crtc = drm_crtc_find(dev, val); > return drm_atomic_set_crtc_for_plane(state, crtc); > @@ -666,6 +668,8 @@ drm_atomic_plane_get_property(struct drm_plane *plane, > > if (property == config->prop_fb_id) { > *val = (state->fb) ? state->fb->base.id : 0; > + } else if (property == config->prop_fence_fd) { > + *val = state->fence_fd; > } else if (property == config->prop_crtc_id) { > *val = (state->crtc) ? state->crtc->base.id : 0; > } else if (property == config->prop_crtc_x) { > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c > index 2b430b0..4f91f84 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c > @@ -2594,6 +2594,7 @@ void drm_atomic_helper_plane_reset(struct drm_plane *plane) > if (plane->state) { > plane->state->plane = plane; > plane->state->rotation = BIT(DRM_ROTATE_0); > + plane->state->fence_fd = -1; > } > } > EXPORT_SYMBOL(drm_atomic_helper_plane_reset); > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c > index 65258ac..165f199 100644 > --- a/drivers/gpu/drm/drm_crtc.c > +++ b/drivers/gpu/drm/drm_crtc.c > @@ -1291,6 +1291,7 @@ int drm_universal_plane_init(struct drm_device *dev, struct drm_plane *plane, > > if (drm_core_check_feature(dev, DRIVER_ATOMIC)) { > drm_object_attach_property(&plane->base, config->prop_fb_id, 0); > + drm_object_attach_property(&plane->base, config->prop_fence_fd, -1); > drm_object_attach_property(&plane->base, config->prop_crtc_id, 0); > drm_object_attach_property(&plane->base, config->prop_crtc_x, 0); > drm_object_attach_property(&plane->base, config->prop_crtc_y, 0); > @@ -1546,6 +1547,12 @@ static int drm_mode_create_standard_properties(struct drm_device *dev) > return -ENOMEM; > dev->mode_config.prop_fb_id = prop; > > + prop = drm_property_create_signed_range(dev, DRM_MODE_PROP_ATOMIC, > + "FENCE_FD", -1, INT_MAX); > + if (!prop) > + return -ENOMEM; > + dev->mode_config.prop_fence_fd = prop; > + > prop = drm_property_create_object(dev, DRM_MODE_PROP_ATOMIC, > "CRTC_ID", DRM_MODE_OBJECT_CRTC); > if (!prop) > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h > index 8c7fb3d..a8f6ec0 100644 > --- a/include/drm/drm_crtc.h > +++ b/include/drm/drm_crtc.h > @@ -1239,6 +1239,7 @@ struct drm_connector { > * @crtc: currently bound CRTC, NULL if disabled > * @fb: currently bound framebuffer > * @fence: optional fence to wait for before scanning out @fb > + * @fence_fd: fd representing the sync_fence > * @crtc_x: left position of visible portion of plane on crtc > * @crtc_y: upper position of visible portion of plane on crtc > * @crtc_w: width of visible portion of plane on crtc > @@ -1257,6 +1258,7 @@ struct drm_plane_state { > struct drm_crtc *crtc; /* do not write directly, use drm_atomic_set_crtc_for_plane() */ > struct drm_framebuffer *fb; /* do not write directly, use drm_atomic_set_fb_for_plane() */ > struct fence *fence; > + int fence_fd; > > /* Signed dest location allows it to be partially off screen */ > int32_t crtc_x, crtc_y; > @@ -2098,6 +2100,7 @@ struct drm_mode_config { > struct drm_property *prop_crtc_w; > struct drm_property *prop_crtc_h; > struct drm_property *prop_fb_id; > + struct drm_property *prop_fence_fd; > struct drm_property *prop_crtc_id; > struct drm_property *prop_active; > struct drm_property *prop_mode_id; > -- > 2.5.0 >
Hi, On 5 April 2016 at 13:36, Rob Clark <robdclark@gmail.com> wrote: > ok, so I've been slacking on writing up the reasons that I don't like > the idea of using a property for fd's (including fence fd's).. I did > at one point assume we'd use properties for fence fd's, but that idea > falls apart pretty quickly when you think about the 'struct file' vs > fd lifecycle. It could *possibly* work if it was a write-only > property, but I don't think that is a good solution. I've been assuming that it would have to be write-only; I don't believe there would be any meaningful usecases for read. Do you have any in mind, or is it just a symmetry/cleanliness thing? > The issue is that 'struct file' / 'int fd' have a fundamentally > different lifecycle compared to 'kms obj' / 'u32 obj_id'. For the kms > objects (like framebuffers) where we can use them with properties, the > id is tied to the kernel side object. This is not true for file > descriptors. Resulting in a few problems: Surely the fence FD tied to the kernel-side struct fence, in exactly the same way, no? > 1) if it was a readable property, reading it would (should) take a > reference.. we can't just return fence_fd that was previously set > (since in the mean time userspace might have close()d the fd). So we > have to return a fresh fd. And if userspace (like modetest) doesn't > realize it is responsible to close() that fd we have a leak Again, assuming that read would always return -1. > 2) basically we shouldn't be tracking fd's on the kernel side, since > we can only count on them being valid during the duration of the ioctl > call. Once the call returns, you must assume userspace has close()d > the fd. So in the ioctl we need to convert fd -> file, and from then > on out track the file object (or in this case the underlying fence > object). Right, it would have to be the same as KMS objects, where userspace passes in an ID (in this case an entry in a non-IDR table, but still), and the kernel maps that to struct fence and handles the lifetime. So, almost exactly the same as what we do with KMS objects ... > I guess we *could* have something analogous to dmabuf, where we import > the fence fd and get a kms drm_fence object (with an id tied to the > kernel side object), and then use a property to attach the drm_fence > object.. but that seems kind of pointless and just trying to force the > 'everything is a property' mantra. I think that would be pointless indirection as well. > I think it is really better to pass in an array of 'struct { u32 > plane; int fd }' (which the atomic ioctl code converts into 'struct > fence's and attaches to the plane state) and an array of 'struct { u32 > crtc; int fd }' filled in on the kernel side for the out-fences. Mmm, it definitely makes ioctl parsing harder, and still you rely on drivers to implement the more-difficult-to-not-screw-up part, which (analagous to crtc_state->event) is the driver managing the lifecycle of that component of the state. We already enforce this with events though, and the difficult part wasn't in the userspace interface, but the backend handling. This doesn't change at all regardless of whether we use a property or an external array, so ... Cheers, Daniel
On Tue, Apr 5, 2016 at 8:57 AM, Daniel Stone <daniel@fooishbar.org> wrote: > Hi, > > On 5 April 2016 at 13:36, Rob Clark <robdclark@gmail.com> wrote: >> ok, so I've been slacking on writing up the reasons that I don't like >> the idea of using a property for fd's (including fence fd's).. I did >> at one point assume we'd use properties for fence fd's, but that idea >> falls apart pretty quickly when you think about the 'struct file' vs >> fd lifecycle. It could *possibly* work if it was a write-only >> property, but I don't think that is a good solution. > > I've been assuming that it would have to be write-only; I don't > believe there would be any meaningful usecases for read. Do you have > any in mind, or is it just a symmetry/cleanliness thing? no, don't see a use-case for it.. but this patch didn't seem to be preventing it. And it was storing the fence_fd on the kernel side which is a no-no as well. >> The issue is that 'struct file' / 'int fd' have a fundamentally >> different lifecycle compared to 'kms obj' / 'u32 obj_id'. For the kms >> objects (like framebuffers) where we can use them with properties, the >> id is tied to the kernel side object. This is not true for file >> descriptors. Resulting in a few problems: > > Surely the fence FD tied to the kernel-side struct fence, in exactly > the same way, no? well, what I mean is you can't keep around the int fd on the kernel side, like this patch does A write-only property, which immediately (ie. during the ioctl call) is converted into a fence object, would work. Although given that we need to handle fences differently (ie. not a property) for out-fences, it seems odd to shoehorn them into a property for in-fences. >> 1) if it was a readable property, reading it would (should) take a >> reference.. we can't just return fence_fd that was previously set >> (since in the mean time userspace might have close()d the fd). So we >> have to return a fresh fd. And if userspace (like modetest) doesn't >> realize it is responsible to close() that fd we have a leak > > Again, assuming that read would always return -1. > >> 2) basically we shouldn't be tracking fd's on the kernel side, since >> we can only count on them being valid during the duration of the ioctl >> call. Once the call returns, you must assume userspace has close()d >> the fd. So in the ioctl we need to convert fd -> file, and from then >> on out track the file object (or in this case the underlying fence >> object). > > Right, it would have to be the same as KMS objects, where userspace > passes in an ID (in this case an entry in a non-IDR table, but still), > and the kernel maps that to struct fence and handles the lifetime. So, > almost exactly the same as what we do with KMS objects ... > >> I guess we *could* have something analogous to dmabuf, where we import >> the fence fd and get a kms drm_fence object (with an id tied to the >> kernel side object), and then use a property to attach the drm_fence >> object.. but that seems kind of pointless and just trying to force the >> 'everything is a property' mantra. > > I think that would be pointless indirection as well. > >> I think it is really better to pass in an array of 'struct { u32 >> plane; int fd }' (which the atomic ioctl code converts into 'struct >> fence's and attaches to the plane state) and an array of 'struct { u32 >> crtc; int fd }' filled in on the kernel side for the out-fences. > > Mmm, it definitely makes ioctl parsing harder, and still you rely on > drivers to implement the more-difficult-to-not-screw-up part, which > (analagous to crtc_state->event) is the driver managing the lifecycle > of that component of the state. We already enforce this with events > though, and the difficult part wasn't in the userspace interface, but > the backend handling. This doesn't change at all regardless of whether > we use a property or an external array, so ... hmm, I'm assuming that the in/out arrays are handled in drm_mode_atomic_ioctl() and the drivers never see 'em.. BR, -R > Cheers, > Daniel
Hi, On 5 April 2016 at 15:04, Rob Clark <robdclark@gmail.com> wrote: > On Tue, Apr 5, 2016 at 8:57 AM, Daniel Stone <daniel@fooishbar.org> wrote: >> I've been assuming that it would have to be write-only; I don't >> believe there would be any meaningful usecases for read. Do you have >> any in mind, or is it just a symmetry/cleanliness thing? > > no, don't see a use-case for it.. but this patch didn't seem to be > preventing it. And it was storing the fence_fd on the kernel side > which is a no-no as well. Yeah, both should be fixed. :) >>> The issue is that 'struct file' / 'int fd' have a fundamentally >>> different lifecycle compared to 'kms obj' / 'u32 obj_id'. For the kms >>> objects (like framebuffers) where we can use them with properties, the >>> id is tied to the kernel side object. This is not true for file >>> descriptors. Resulting in a few problems: >> >> Surely the fence FD tied to the kernel-side struct fence, in exactly >> the same way, no? > > well, what I mean is you can't keep around the int fd on the kernel > side, like this patch does > > A write-only property, which immediately (ie. during the ioctl call) > is converted into a fence object, would work. Although given that we > need to handle fences differently (ie. not a property) for out-fences, > it seems odd to shoehorn them into a property for in-fences. Depends on how you look at it, I guess. From the point of view of all fence-like things being consistent, yes, it falls down. But from the point of view of in-fences being attached to an FB, and out-fences (like events) being separately attached to a request, it's a lot more consistent. >>> I think it is really better to pass in an array of 'struct { u32 >>> plane; int fd }' (which the atomic ioctl code converts into 'struct >>> fence's and attaches to the plane state) and an array of 'struct { u32 >>> crtc; int fd }' filled in on the kernel side for the out-fences. >> >> Mmm, it definitely makes ioctl parsing harder, and still you rely on >> drivers to implement the more-difficult-to-not-screw-up part, which >> (analagous to crtc_state->event) is the driver managing the lifecycle >> of that component of the state. We already enforce this with events >> though, and the difficult part wasn't in the userspace interface, but >> the backend handling. This doesn't change at all regardless of whether >> we use a property or an external array, so ... > > hmm, I'm assuming that the in/out arrays are handled in > drm_mode_atomic_ioctl() and the drivers never see 'em.. True, but it complicates the (already not hugely straightforward) parsing that the ioctl has to do. It also makes extending the ioctl a little harder to do in future, because you're adding in two variable-size elements, and have to do some fairly complicated parsing to even figure out what the size _should_ be. So I'd rather not do it if there was any way out of it; at the very least it'd have to be userptr rather than array. Cheers, Daniel
On Tue, Apr 5, 2016 at 10:19 AM, Daniel Stone <daniel@fooishbar.org> wrote: > >> hmm, I'm assuming that the in/out arrays are handled in >> drm_mode_atomic_ioctl() and the drivers never see 'em.. > > True, but it complicates the (already not hugely straightforward) > parsing that the ioctl has to do. It also makes extending the ioctl a > little harder to do in future, because you're adding in two > variable-size elements, and have to do some fairly complicated parsing > to even figure out what the size _should_ be. So I'd rather not do it > if there was any way out of it; at the very least it'd have to be > userptr rather than array. oh, I was assuming ptr to a user array (rather than var length ioctl struct) for both in and out fences. I guess my wording didn't make that clear. But yeah, it would be madness to do anything else. BR, -R
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 8fb469c..8bc364c 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -609,6 +609,8 @@ int drm_atomic_plane_set_property(struct drm_plane *plane, drm_atomic_set_fb_for_plane(state, fb); if (fb) drm_framebuffer_unreference(fb); + } else if (property == config->prop_fence_fd) { + state->fence_fd = val; } else if (property == config->prop_crtc_id) { struct drm_crtc *crtc = drm_crtc_find(dev, val); return drm_atomic_set_crtc_for_plane(state, crtc); @@ -666,6 +668,8 @@ drm_atomic_plane_get_property(struct drm_plane *plane, if (property == config->prop_fb_id) { *val = (state->fb) ? state->fb->base.id : 0; + } else if (property == config->prop_fence_fd) { + *val = state->fence_fd; } else if (property == config->prop_crtc_id) { *val = (state->crtc) ? state->crtc->base.id : 0; } else if (property == config->prop_crtc_x) { diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 2b430b0..4f91f84 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -2594,6 +2594,7 @@ void drm_atomic_helper_plane_reset(struct drm_plane *plane) if (plane->state) { plane->state->plane = plane; plane->state->rotation = BIT(DRM_ROTATE_0); + plane->state->fence_fd = -1; } } EXPORT_SYMBOL(drm_atomic_helper_plane_reset); diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 65258ac..165f199 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -1291,6 +1291,7 @@ int drm_universal_plane_init(struct drm_device *dev, struct drm_plane *plane, if (drm_core_check_feature(dev, DRIVER_ATOMIC)) { drm_object_attach_property(&plane->base, config->prop_fb_id, 0); + drm_object_attach_property(&plane->base, config->prop_fence_fd, -1); drm_object_attach_property(&plane->base, config->prop_crtc_id, 0); drm_object_attach_property(&plane->base, config->prop_crtc_x, 0); drm_object_attach_property(&plane->base, config->prop_crtc_y, 0); @@ -1546,6 +1547,12 @@ static int drm_mode_create_standard_properties(struct drm_device *dev) return -ENOMEM; dev->mode_config.prop_fb_id = prop; + prop = drm_property_create_signed_range(dev, DRM_MODE_PROP_ATOMIC, + "FENCE_FD", -1, INT_MAX); + if (!prop) + return -ENOMEM; + dev->mode_config.prop_fence_fd = prop; + prop = drm_property_create_object(dev, DRM_MODE_PROP_ATOMIC, "CRTC_ID", DRM_MODE_OBJECT_CRTC); if (!prop) diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 8c7fb3d..a8f6ec0 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -1239,6 +1239,7 @@ struct drm_connector { * @crtc: currently bound CRTC, NULL if disabled * @fb: currently bound framebuffer * @fence: optional fence to wait for before scanning out @fb + * @fence_fd: fd representing the sync_fence * @crtc_x: left position of visible portion of plane on crtc * @crtc_y: upper position of visible portion of plane on crtc * @crtc_w: width of visible portion of plane on crtc @@ -1257,6 +1258,7 @@ struct drm_plane_state { struct drm_crtc *crtc; /* do not write directly, use drm_atomic_set_crtc_for_plane() */ struct drm_framebuffer *fb; /* do not write directly, use drm_atomic_set_fb_for_plane() */ struct fence *fence; + int fence_fd; /* Signed dest location allows it to be partially off screen */ int32_t crtc_x, crtc_y; @@ -2098,6 +2100,7 @@ struct drm_mode_config { struct drm_property *prop_crtc_w; struct drm_property *prop_crtc_h; struct drm_property *prop_fb_id; + struct drm_property *prop_fence_fd; struct drm_property *prop_crtc_id; struct drm_property *prop_active; struct drm_property *prop_mode_id;