On Mon, Aug 06, 2018 at 06:01:02PM +0200, Enric Balletbo i Serra wrote: > From: Gustavo Padovan <gustavo.padovan@xxxxxxxxxxxxx> > > This flag tells core to jump ahead the queued update if the conditions > in drm_atomic_async_check() are met. That means we are only able to do an > async update if no modeset is pending and update for the same plane is > not queued. > > It uses the already in place infrastructure for async updates. > > It is useful for cursor updates and async PageFlips over the atomic > ioctl, otherwise in some cases updates may be delayed to the point the > user will notice it. Note that for now it's only enabled for cursor > planes. > > DRM_MODE_ATOMIC_AMEND should be passed to the Atomic IOCTL to use this > feature. > > Signed-off-by: Gustavo Padovan <gustavo.padovan@xxxxxxxxxxxxx> > Signed-off-by: Enric Balletbo i Serra <enric.balletbo@xxxxxxxxxxxxx> > --- > Hi, > > This is an attempt to introduce the new ATOMIC_AMEND flag for atomic > operations, see the commit message for a more detailed description. > > This was tested using a small program that exercises the uAPI for easy > sanity testing. The program created by Alexandros can be found here > [2]. To test, just build the program and use the --atomic flag to use the > cursor plane in normal (blocking mode), and --atomic-async to use the cursor > plane with the ATOMIC_AMEND flag. E.g. > > drm_cursor --atomic > > or > > drm_cursor --atomic-async Would be really lovely to bake these into igt-based regression tests. i915 doesn't yet have async update support, but I hope we'll eventually gain that, and then our CI could make sure this keeps working. > The test worked on a Samsung Chromebook Plus on top of mainline plus > the patch to update cursors asynchronously through atomic for the > drm/rockchip driver. > > Alexandros also did a proof-of-concept to use this flag and draw cursors > using atomic if possible on ozone [1]. > > Best regards, > Enric > > [1] https://chromium-review.googlesource.com/c/chromium/src/+/1092711 > [2] https://gitlab.collabora.com/alf/drm-cursor > > > Changes in v1: > - Only enable it if userspace requests it. > - Only allow async update for cursor type planes. > - Rename ASYNC_UPDATE for ATOMIC_AMEND. > > drivers/gpu/drm/drm_atomic.c | 5 +++++ > drivers/gpu/drm/drm_atomic_helper.c | 10 +++++++++- > include/uapi/drm/drm_mode.h | 4 +++- > 3 files changed, 17 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c > index 895741e9cd7d..7b3e4aa2874d 100644 > --- a/drivers/gpu/drm/drm_atomic.c > +++ b/drivers/gpu/drm/drm_atomic.c > @@ -2338,6 +2338,10 @@ int drm_mode_atomic_ioctl(struct drm_device *dev, > (arg->flags & DRM_MODE_PAGE_FLIP_EVENT)) > return -EINVAL; > > + if ((arg->flags & DRM_MODE_ATOMIC_ALLOW_MODESET) && > + (arg->flags & DRM_MODE_ATOMIC_AMEND)) > + return -EINVAL; > + > drm_modeset_acquire_init(&ctx, DRM_MODESET_ACQUIRE_INTERRUPTIBLE); > > state = drm_atomic_state_alloc(dev); > @@ -2346,6 +2350,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev, > > state->acquire_ctx = &ctx; > state->allow_modeset = !!(arg->flags & DRM_MODE_ATOMIC_ALLOW_MODESET); > + state->async_update = !!(arg->flags & DRM_MODE_ATOMIC_AMEND); > > retry: > plane_mask = 0; > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c > index 81e32199d3ef..59495f61c583 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c > @@ -902,7 +902,7 @@ int drm_atomic_helper_check(struct drm_device *dev, > if (ret) > return ret; > > - if (state->legacy_cursor_update) > + if (state->async_update || state->legacy_cursor_update) Hm, it would be really cool if we could rip out the legacy_cursor_update hack and also use async_update in the cursor compat helpers. At least if available. > state->async_update = !drm_atomic_helper_async_check(dev, state); > > return ret; > @@ -1539,6 +1539,14 @@ int drm_atomic_helper_async_check(struct drm_device *dev, > if (new_plane_state->fence) > return -EINVAL; > > + /* Only allow async update for cursor type planes. */ > + if (plane->type != DRM_PLANE_TYPE_CURSOR) > + return -EINVAL; Why limit to cursor planes? The async_update hooks are per-plane. > + > + /* Don't do an async update if there isn't a pending commit. */ > + if (!old_plane_state->commit) > + return -EINVAL; Hm, why this? Won't this break legacy cursor updates now for drivers which implement this using async_update? If userspace wants to avoid this, they can do so by properly tracking their non-AMEND updates and making sure they've waited until those all completed. Also, how is userspace supposed to figure out async_update exists/works? Usual approach is a getparam. Another semantics questions: Should this fail if we fall back to a non-async update? I think that would be best, since userspace can the decide on its own whether it wants to fall back to a normal commit or do something else. And the cursor compat code could do the same, i.e. try first with AMEND, then a normal commit? Cheers, Daniel > + > /* > * Don't do an async update if there is an outstanding commit modifying > * the plane. This prevents our async update's changes from getting > diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h > index 4b3a1bb58e68..6dae18428123 100644 > --- a/include/uapi/drm/drm_mode.h > +++ b/include/uapi/drm/drm_mode.h > @@ -724,13 +724,15 @@ struct drm_mode_destroy_dumb { > #define DRM_MODE_ATOMIC_TEST_ONLY 0x0100 > #define DRM_MODE_ATOMIC_NONBLOCK 0x0200 > #define DRM_MODE_ATOMIC_ALLOW_MODESET 0x0400 > +#define DRM_MODE_ATOMIC_AMEND 0x0800 > > #define DRM_MODE_ATOMIC_FLAGS (\ > DRM_MODE_PAGE_FLIP_EVENT |\ > DRM_MODE_PAGE_FLIP_ASYNC |\ > DRM_MODE_ATOMIC_TEST_ONLY |\ > DRM_MODE_ATOMIC_NONBLOCK |\ > - DRM_MODE_ATOMIC_ALLOW_MODESET) > + DRM_MODE_ATOMIC_ALLOW_MODESET |\ > + DRM_MODE_ATOMIC_AMEND) > > struct drm_mode_atomic { > __u32 flags; > -- > 2.18.0 > > _______________________________________________ > dri-devel mailing list > dri-devel@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
