[Intel-gfx] [PATCH 14/17] drm/atomic-helper: implement ->page_flip

[Sean Paul]

On Sun, Nov 02, 2014 at 02:19:27PM +0100, Daniel Vetter wrote:
> Currently there is no way to implement async flips using atomic, that
> essentially requires us to be able to cancel pending requests
> mid-flight.
>
> To be able to do that (and I guess we want this since vblank synced
> updates whic opportunistically cancel still pending updates seem to be

s/whic/which/

> wanted) we'd need to add a mandatory cancellation mode. Depending upon
> the exact semantics we decide upon that could mean that userspace will
> not get completion events, or will get them all stacked up.
>
> So reject async updates for now. Also async updates usually means not
> vblank synced at all, and I guess for drivers which want to support
> this they should simply add a special pageflip handler (since usually
> you need a special flip cmd to achieve this). That kind of async flip
> is pretty much exclusively just used for games and benchmarks where
> dropping just one frame means you'll get a headshot or something bad
> like that ... And so slight amounts of tearing is acceptable.
>
> v2: Fixup kerneldoc, reported by Paulo.
>
> v3: Use the set_crtc_for_plane function to assign the crtc, since
> otherwise the book-keeping is off.
>
> v4: Update crtc->primary->fb since ->page_flip is the only driver
> callback where the core won't do this itself. We might want to fix
> this inconsistency eventually.
>
> Cc: Paulo Zanoni <przanoni at gmail.com>
> Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>

Minor nit regarding error handing, either way:

Reviewed-by: Sean Paul <seanpaul at chromium.org>

> ---
>  drivers/gpu/drm/drm_atomic_helper.c | 90 +++++++++++++++++++++++++++++++++++++
>  include/drm/drm_atomic_helper.h     |  5 +++
>  2 files changed, 95 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 92ae34bde44d..70bd67cf86e3 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -1617,3 +1617,93 @@ backoff:
>   goto retry;
>  }
>  EXPORT_SYMBOL(drm_atomic_helper_connector_set_property);
> +
> +/**
> + * drm_atomic_helper_page_flip - execute a legacy page flip
> + * @crtc: DRM crtc
> + * @fb: DRM framebuffer
> + * @event: optional DRM event to signal upon completion
> + * @flags: flip flags for non-vblank sync'ed updates
> + *
> + * Provides a default page flip implementation using the atomic driver interface.
> + *
> + * Note that for now so called async page flips (i.e. updates which are not
> + * synchronized to vblank) are not supported, since the atomic interfaces have
> + * no provisions for this yet.
> + *
> + * Returns:
> + * Returns 0 on success, negative errno numbers on failure.
> + */
> +int drm_atomic_helper_page_flip(struct drm_crtc *crtc,
> + struct drm_framebuffer *fb,
> + struct drm_pending_vblank_event *event,
> + uint32_t flags)
> +{
> + struct drm_plane *plane = crtc->primary;
> + struct drm_atomic_state *state;
> + struct drm_plane_state *plane_state;
> + struct drm_crtc_state *crtc_state;
> + int ret = 0;
> +
> + if (flags & DRM_MODE_PAGE_FLIP_ASYNC)
> + return -EINVAL;
> +
> + state = drm_atomic_state_alloc(plane->dev);
> + if (!state)
> + return -ENOMEM;
> +
> + state->acquire_ctx = drm_modeset_legacy_acquire_ctx(crtc);
> +retry:
> + crtc_state = drm_atomic_get_crtc_state(state, crtc);
> + if (IS_ERR(crtc_state)) {
> + ret = PTR_ERR(crtc_state);
> + if (ret == -EDEADLK)
> + goto backoff;
> + else
> + goto fail;

This function would benefit from the error cleanup you did in the previous
patch (ie, doing the -EDEADLK check in fail and punting to backoff there).

> + }
> + crtc_state->event = event;
> +
> + plane_state = drm_atomic_get_plane_state(state, plane);
> + if (IS_ERR(plane_state)) {
> + ret = PTR_ERR(plane_state);
> + if (ret == -EDEADLK)
> + goto backoff;
> + else
> + goto fail;
> + }
> +
> + drm_atomic_set_crtc_for_plane(plane_state, crtc);
> + plane_state->fb = fb;
> +
> + ret = drm_atomic_async_commit(state);
> + if (ret == -EDEADLK)
> + goto backoff;
> +
> + /* Driver takes ownership of state on successful async commit. */
> + if (ret == 0) {
> + /* TODO: ->page_flip is the only driver callback where the core
> + * doesn't update plane->fb. For now patch it up here. */
> + plane->fb = plane->state->fb;
> +
> + return 0;
> + }
> +
> +fail:
> + drm_atomic_state_free(state);
> +
> + return ret;
> +backoff:
> + drm_atomic_legacy_backoff(state);
> + drm_atomic_state_clear(state);
> +
> + /*
> + * Someone might have exchanged the framebuffer while we dropped locks
> + * in the backoff code. We need to fix up the fb refcount tracking the
> + * core does for us.
> + */
> + plane->old_fb = plane->fb;
> +
> + goto retry;
> +}
> +EXPORT_SYMBOL(drm_atomic_helper_page_flip);
> diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
> index 8cd6fe7a48e5..28a2f3a815fd 100644
> --- a/include/drm/drm_atomic_helper.h
> +++ b/include/drm/drm_atomic_helper.h
> @@ -69,5 +69,10 @@ int drm_atomic_helper_plane_set_property(struct drm_plane *plane,
>  int drm_atomic_helper_connector_set_property(struct drm_connector *connector,
>   struct drm_property *property,
>   uint64_t val);
> +int drm_atomic_helper_page_flip(struct drm_crtc *crtc,
> + struct drm_framebuffer *fb,
> + struct drm_pending_vblank_event *event,
> + uint32_t flags);
> +
>
>  #endif /* DRM_ATOMIC_HELPER_H_ */
> --
> 2.1.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel