On 04/11/14 22:09, 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
> 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.
>
> v5: Use set_crtc_for_connector as suggested by Sean.
>
> Cc: Sean Paul <seanpaul@xxxxxxxxxxxx>
> Cc: Paulo Zanoni <przanoni@xxxxxxxxx>
> Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
> ---
> drivers/gpu/drm/drm_atomic_helper.c | 92 +++++++++++++++++++++++++++++++++++++
> include/drm/drm_atomic_helper.h | 5 ++
> 2 files changed, 97 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 790edc18b2cb..36ccc42d43db 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -1633,3 +1633,95 @@ 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;
> + }
> + 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;
> + }
> +
> + ret = drm_atomic_set_crtc_for_plane(plane_state, crtc);
> + if (ret == -EDEADLK)
> + goto backoff;
The kerneldoc for this function states that it can return -ENOMEM too
and, even if it didn't, this approach to error handling is potentially
fragile as the kernel changes.
> + plane_state->fb = fb;
> +
> + ret = drm_atomic_async_commit(state);
> + if (ret == -EDEADLK)
> + goto backoff;
A fussy point but this would be easier to read if it followed the same
pattern as the above error checks with a "goto fail". The the success
path below can be outdented and made more obvious.
> +
> + /* 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_ */
>
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
