[PATCH drm 4/6] drm: add drm_gem_prime_page_flip() helper
Daniel Vetter
daniel at ffwll.ch
Thu Oct 22 13:25:42 PDT 2015
On Thu, Oct 22, 2015 at 01:00:57PM -0700, Alex Goins wrote:
> From: agoins <agoins at nvidia.com>
>
> Adds drm_gem_prime_page_flip(), a helper implementation of
> prime_page_flip() to be used by DRM drivers.
>
> Signed-off-by: Alex Goins <agoins at nvidia.com>
> ---
> drivers/gpu/drm/drm_prime.c | 147 ++++++++++++++++++++++++++++++++++++++++++++
> include/drm/drmP.h | 3 +
> 2 files changed, 150 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> index 2956a65..175bf4a 100644
> --- a/drivers/gpu/drm/drm_prime.c
> +++ b/drivers/gpu/drm/drm_prime.c
> @@ -61,6 +61,11 @@
> * use the drm_gem_prime_{import,export} helpers.
> */
>
> +struct drm_prime_fence {
> + struct fence base;
> + spinlock_t lock;
> +};
> +
> struct drm_prime_fence_ctx {
> uint32_t context;
> uint32_t seqno;
> @@ -78,6 +83,16 @@ struct drm_prime_attachment {
> enum dma_data_direction dir;
> };
>
> +struct drm_gem_prime_page_flip_cb {
> + struct fence_cb base;
> + struct drm_device *dev;
> + uint32_t crtc_id;
> + uint32_t fb_id;
> + uint32_t flags;
> + uint64_t user_data;
> + struct drm_file *file_priv;
> +};
> +
> static int drm_prime_add_buf_handle(struct drm_prime_file_private *prime_fpriv,
> struct dma_buf *dma_buf, uint32_t handle)
> {
> @@ -316,6 +331,28 @@ static const struct dma_buf_ops drm_gem_prime_dmabuf_ops = {
> .vunmap = drm_gem_dmabuf_vunmap,
> };
>
> +static const char *drm_gem_prime_get_driver_name(struct fence *fence)
> +{
> + return "PRIME";
> +}
> +
> +static const char *drm_gem_prime_get_timeline_name(struct fence *fence)
> +{
> + return "prime.swonly";
> +}
> +
> +static bool drm_gem_prime_enable_signaling(struct fence *fence)
> +{
> + return true; /* SW signaling only */
> +}
> +
> +static const struct fence_ops drm_gem_prime_fence_ops = {
> + .get_driver_name = drm_gem_prime_get_driver_name,
> + .get_timeline_name = drm_gem_prime_get_timeline_name,
> + .enable_signaling = drm_gem_prime_enable_signaling,
> + .wait = fence_default_wait,
> +};
> +
> /**
> * DOC: PRIME Helpers
> *
> @@ -680,6 +717,116 @@ int drm_prime_fd_to_handle_ioctl(struct drm_device *dev, void *data,
> args->fd, &args->handle);
> }
>
> +/* Callback used by drm_gem_prime_page_flip(). Is added to a fence such that
> + * when the fence is signaled, page flip is requested with given parameters. */
> +static void drm_gem_prime_page_flip_cb(struct fence *fence,
> + struct fence_cb *cb)
> +{
> + struct drm_gem_prime_page_flip_cb *page_flip_cb =
> + container_of(cb, struct drm_gem_prime_page_flip_cb, base);
> +
> + drm_mode_page_flip(page_flip_cb->dev,
> + page_flip_cb->crtc_id,
> + page_flip_cb->fb_id,
> + page_flip_cb->flags,
> + page_flip_cb->user_data,
> + page_flip_cb->file_priv);
> +
> + kfree(cb);
> +}
> +
> +/**
> + * drm_gem_prime_page_flip -
> + * helper library implementation of the page flip callback
> + *
> + * @dev: DRM device
> + * @file_priv: DRM file info
> + * @handle: buffer handle to fence
> + * @fb_id: FB to update to
> + * @crtc_id: CRTC to update
> + * @user_data: Returned as user_data field in in the vblank event struct
> + * @flags: Flags modifying page flip behavior, same as drm_mode_page_flip.
> + *
> + * This is the implementation of the prime_page_flip function for GEM drivers
> + * using the PRIME helpers.
> + */
> +int drm_gem_prime_page_flip(struct drm_device *dev,
> + struct drm_file *file_priv, uint32_t handle,
> + uint32_t fb_id, uint32_t crtc_id,
> + uint64_t user_data, uint32_t flags)
> +{
> + int ret;
> +
> + struct dma_buf *dmabuf;
> + struct fence *oldfence;
> + struct drm_prime_fence *fence;
> + struct drm_gem_prime_page_flip_cb *page_flip_cb;
> + struct drm_prime_member *member;
> + struct drm_prime_fence_ctx *fctx;
> +
> + mutex_lock(&file_priv->prime.lock);
> +
> + member = drm_prime_lookup_member_by_handle(&file_priv->prime,
> + handle);
> + if (!member) {
> + /* Buffer is not a member of PRIME */
> + ret = -EINVAL;
> + goto out;
> + }
> +
> + dmabuf = member->dma_buf;
> + fctx = &member->fctx;
> +
> + /* Clear out old fence callbacks */
> + oldfence = reservation_object_get_excl(dmabuf->resv);
> + if (oldfence)
> + fence_signal(oldfence);
This looks really strange - you force-complete the fence already attached
(which might be from any driver attached to this dma-buf)?
The creator of the fence is supposed to complete it when whatever async
operation that is scheduled and which is using this dma-buf is done. So
the way to go about this here is to take out the excl fence from the
reservation object, and wait for it to complete.
Maarten Lankhorst had patches to do that for i915, but they just didn't go
anywhere since i915 gem locking is a bit ... antique. But if your goal is
to only fix up the page_flip path, and only for i915 as the display
driver, then the only thing you need to do is:
- grab the reservation of the gem bo underlying the fb
- grab the exclusive fence (fence_get)
- wait for that fence to complete in the async worker (we already have the
mmio_flip stuff for when you do the flip from the cpu)
- drop the fence once done with fence_put
Works as long as the producer correctly sets the exclusive fence (doesn't
matter whether that's done with an explicit ioctl or at command
submission). And that's already done by nouveau (well ttm) afaik.
No changes at all in userspace required.
Now if you want to do proper explicit fencing for kms, that's an entirely
different story. For that we want something like android's hw composer
in/out fences, and obviously for atomic (because interface extensions for
legacy modeset ioctls just don't make sense). But that means we need the
full stack, using open-source userspace, and that's a lot more pain.
For your limited use-case it really would be simplest to fix up the i915
mmio flip code to handle fences attached to dma-bufs, and teach the
nvidia blob to attach a suitable exclusive fence somehow.
Wrt merge coordination: RSN (well that's the case since months) all the
legacy pageflip code will be ripped out from i915 and replaced by async
atomic updates). So except for a proof-of-concept I'd prefer if we just
implement this in the atomic codepaths directly.
Cheers, Daniel
> +
> + /* Create and initialize new fence */
> + fence = kmalloc(sizeof(*fence), GFP_KERNEL);
> + if (!fence) {
> + ret = -ENOMEM;
> + goto out;
> + }
> +
> + spin_lock_init(&fence->lock);
> + fence_init(&fence->base, &drm_gem_prime_fence_ops, &fence->lock,
> + fctx->context, fctx->seqno++);
> +
> + /* Create and initialize new page flip callback */
> + page_flip_cb = kmalloc(sizeof(*page_flip_cb), GFP_KERNEL);
> + if (!page_flip_cb) {
> + ret = -ENOMEM;
> + fence_free(&fence->base);
> + goto out;
> + }
> +
> + *page_flip_cb = (struct drm_gem_prime_page_flip_cb) {
> + .dev = dev,
> + .crtc_id = crtc_id,
> + .fb_id = fb_id,
> + .flags = flags,
> + .user_data = user_data,
> + .file_priv = file_priv,
> + };
> +
> + ret = fence_add_callback(&fence->base, &page_flip_cb->base,
> + drm_gem_prime_page_flip_cb);
> + if (ret) {
> + fence_free(&fence->base);
> + kfree(page_flip_cb);
> + goto out;
> + }
> +
> + /* Add new fence */
> + reservation_object_add_excl_fence(dmabuf->resv, &fence->base);
> + fence_put(&fence->base); /* Reservation object has reference */
> +out:
> + mutex_unlock(&file_priv->prime.lock);
> + return ret;
> +}
> +EXPORT_SYMBOL(drm_gem_prime_page_flip);
> +
> /**
> * drm_prime_pages_to_sg - converts a page array into an sg list
> * @pages: pointer to the array of page pointers to convert
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index 4d3b842..015d19b 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -1028,6 +1028,9 @@ extern struct drm_gem_object *drm_gem_prime_import(struct drm_device *dev,
> struct dma_buf *dma_buf);
> extern int drm_gem_prime_fd_to_handle(struct drm_device *dev,
> struct drm_file *file_priv, int prime_fd, uint32_t *handle);
> +extern int drm_gem_prime_page_flip(struct drm_device *dev,
> + struct drm_file *file_priv, uint32_t handle, uint32_t fb_id,
> + uint32_t crtc_id, uint64_t user_data, uint32_t flags);
> extern void drm_gem_dmabuf_release(struct dma_buf *dma_buf);
>
> extern int drm_prime_sg_to_page_addr_arrays(struct sg_table *sgt, struct page **pages,
> --
> 1.9.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
More information about the dri-devel
mailing list