[RFC 8/8] drm/fence: add out-fences support
Gustavo Padovan
gustavo at padovan.org
Fri Apr 15 19:15:48 UTC 2016
2016-04-15 Daniel Vetter <daniel at ffwll.ch>:
> On Thu, Apr 14, 2016 at 06:29:41PM -0700, Gustavo Padovan wrote:
> > From: Gustavo Padovan <gustavo.padovan at collabora.co.uk>
> >
> > Support DRM out-fences creating a sync_file with a fence for each crtc
> > update with the DRM_MODE_ATOMIC_OUT_FENCE flag.
> >
> > We then send an struct drm_out_fences array with the out-fences fds back in
> > the drm_atomic_ioctl() as an out arg in the out_fences_ptr field.
> >
> > struct drm_out_fences {
> > __u32 crtc_id;
> > __u32 fd;
> > };
> >
> > Signed-off-by: Gustavo Padovan <gustavo.padovan at collabora.co.uk>
> > ---
> > drivers/gpu/drm/drm_atomic.c | 109 +++++++++++++++++++++++++++++++++++-
> > drivers/gpu/drm/drm_atomic_helper.c | 1 +
> > include/drm/drm_crtc.h | 3 +
> > include/uapi/drm/drm_mode.h | 7 +++
> > 4 files changed, 119 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> > index 0b95526..af6e051 100644
> > --- a/drivers/gpu/drm/drm_atomic.c
> > +++ b/drivers/gpu/drm/drm_atomic.c
> > @@ -1560,6 +1560,103 @@ void drm_atomic_clean_old_fb(struct drm_device *dev,
> > }
> > EXPORT_SYMBOL(drm_atomic_clean_old_fb);
> >
> > +static int drm_atomic_get_out_fences(struct drm_device *dev,
> > + struct drm_atomic_state *state,
> > + uint32_t __user *out_fences_ptr,
> > + uint64_t count_out_fences,
> > + uint64_t user_data)
> > +{
> > + struct drm_crtc *crtc;
> > + struct drm_crtc_state *crtc_state;
> > + struct drm_out_fences *out_fences;
> > + struct sync_file **sync_file;
> > + int num_fences = 0;
> > + int i, ret;
> > +
> > + out_fences = kcalloc(count_out_fences, sizeof(*out_fences),
> > + GFP_KERNEL);
> > + if (!out_fences)
> > + return -ENOMEM;
> > +
> > + sync_file = kcalloc(count_out_fences, sizeof(*sync_file),
> > + GFP_KERNEL);
> > + if (!sync_file) {
> > + kfree(out_fences);
> > + return -ENOMEM;
> > + }
> > +
> > + for_each_crtc_in_state(state, crtc, crtc_state, i) {
> > + struct drm_pending_vblank_event *e;
> > + struct fence *fence;
> > + char name[32];
> > + int fd;
> > +
> > + fence = sync_timeline_create_fence(crtc->timeline,
> > + crtc->fence_seqno);
> > + if (!fence) {
> > + ret = -ENOMEM;
> > + goto out;
> > + }
> > +
> > + snprintf(name, sizeof(name), "crtc-%d_%lu",
> > + drm_crtc_index(crtc), crtc->fence_seqno++);
> > +
> > + sync_file[i] = sync_file_create(name, fence);
> > + if(!sync_file[i]) {
> > + ret = -ENOMEM;
> > + goto out;
> > + }
> > +
> > + fd = get_unused_fd_flags(O_CLOEXEC);
> > + if (fd < 0) {
> > + ret = fd;
> > + goto out;
> > + }
> > +
> > + sync_file_install(sync_file[i], fd);
> > +
> > + if (crtc_state->event) {
> > + crtc_state->event->base.fence = fence;
> > + } else {
> > + e = create_vblank_event(dev, NULL, fence, user_data);
> > + if (!e) {
> > + put_unused_fd(fd);
> > + ret = -ENOMEM;
> > + goto out;
> > + }
> > +
> > + crtc_state->event = e;
> > + }
> > + if (num_fences > count_out_fences) {
> > + put_unused_fd(fd);
> > + ret = -EINVAL;
> > + goto out;
> > + }
> > +
> > + fence_get(fence);
> > +
> > + out_fences[num_fences].crtc_id = crtc->base.id;
> > + out_fences[num_fences].fd = fd;
> > + num_fences++;
> > + }
> > +
> > + if (copy_to_user(out_fences_ptr, out_fences,
> > + num_fences * sizeof(*out_fences))) {
> > + ret = -EFAULT;
> > + goto out;
> > + }
> > +
> > + return 0;
> > +
> > +out:
> > + for (i = 0 ; i < count_out_fences ; i++) {
> > + if (sync_file[i])
> > + sync_file_put(sync_file[i]);
> > + }
> > +
> > + return ret;
> > +}
> > +
> > int drm_mode_atomic_ioctl(struct drm_device *dev,
> > void *data, struct drm_file *file_priv)
> > {
> > @@ -1568,6 +1665,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
> > uint32_t __user *count_props_ptr = (uint32_t __user *)(unsigned long)(arg->count_props_ptr);
> > uint32_t __user *props_ptr = (uint32_t __user *)(unsigned long)(arg->props_ptr);
> > uint64_t __user *prop_values_ptr = (uint64_t __user *)(unsigned long)(arg->prop_values_ptr);
> > + uint32_t __user *out_fences_ptr = (uint32_t __user *)(unsigned long)(arg->out_fences_ptr);
> > unsigned int copied_objs, copied_props;
> > struct drm_atomic_state *state;
> > struct drm_modeset_acquire_ctx ctx;
> > @@ -1601,7 +1699,8 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
> >
> > /* can't test and expect an event at the same time. */
> > if ((arg->flags & DRM_MODE_ATOMIC_TEST_ONLY) &&
> > - (arg->flags & DRM_MODE_PAGE_FLIP_EVENT))
> > + (arg->flags & (DRM_MODE_PAGE_FLIP_EVENT
> > + | DRM_MODE_ATOMIC_OUT_FENCE)))
> > return -EINVAL;
> >
> > drm_modeset_acquire_init(&ctx, 0);
> > @@ -1693,6 +1792,14 @@ retry:
> > }
> > }
> >
> > + if (arg->flags & DRM_MODE_ATOMIC_OUT_FENCE) {
>
> OUT_FENCE and TEST_ONLY probably don't make sense, and need to be
> rejected. Needs a testcase, too.
I've added the check for this above. But a test case is still missing.
>
> > + ret = drm_atomic_get_out_fences(dev, state, out_fences_ptr,
> > + arg->count_out_fences,
> > + arg->user_data);
> > + if (ret < 0)
> > + goto out;
> > + }
> > +
> > if (arg->flags & DRM_MODE_ATOMIC_TEST_ONLY) {
>
> If anything fails below this point we need to clean up the sync_file/fd
> mess. Might be easier to first create sync_file objects only, and only
> install the fd once atomic has succeeded. You probably want to reserve the
> fd slots beforehand though.
>
> That means a bunch more per-crtc state in drm_atomic_state. We should
> probably take all the per-crtc pointers and throw them into a small
> struct, to avoid allocating individual arrays for everything. So
>
> struct drm_atomic_state_per_crtc {
> struct drm_crtc *crtc;
> struct drm_crtc_state *state;
> struct sync_file *sync_file;
> int fd;
> };
That is good idea. I've left the clean up out for this RFC because I
didn't had any good approach on how to do it.
Thanks for this suggestion and all the other comments in the patches.
They were really helpful to improve this work.
Gustavo
More information about the dri-devel
mailing list