[PATCH v3 1/3] drm: add prime helpers
Daniel Vetter
daniel at ffwll.ch
Fri Apr 12 07:58:41 PDT 2013
On Tue, Jan 15, 2013 at 12:47:42PM -0800, Aaron Plattner wrote:
> Instead of reimplementing all of the dma_buf functionality in every driver,
> create helpers drm_prime_import and drm_prime_export that implement them in
> terms of new, lower-level hook functions:
>
> gem_prime_pin: callback when a buffer is created, used to pin buffers into GTT
> gem_prime_get_sg_table: convert a drm_gem_object to an sg_table for export
> gem_prime_import_sg_table: convert an sg_table into a drm_gem_object
> gem_prime_vmap, gem_prime_vunmap: map and unmap an object
>
> These hooks are optional; drivers can opt in by using drm_gem_prime_import and
> drm_gem_prime_export as the .gem_prime_import and .gem_prime_export fields of
> struct drm_driver.
>
> v2:
> - Drop .begin_cpu_access. None of the drivers this code replaces implemented
> it. Having it here was a leftover from when I was trying to include i915 in
> this rework.
> - Use mutex_lock instead of mutex_lock_interruptible, as these three drivers
> did. This patch series shouldn't change that behavior.
> - Rename helpers to gem_prime_get_sg_table and gem_prime_import_sg_table.
> Rename struct sg_table* variables to 'sgt' for clarity.
> - Update drm.tmpl for these new hooks.
>
> v3:
> - Pass the vaddr down to the driver. This lets drivers that just call vunmap on
> the pointer avoid having to store the pointer in their GEM private structures.
> - Move documentation into a /** DOC */ comment in drm_prime.c and include it in
> drm.tmpl with a !P line. I tried to use !F lines to include documentation of
> the individual functions from drmP.h, but the docproc / kernel-doc scripts
> barf on that file, so hopefully this is good enough for now.
> - apply refcount fix from commit be8a42ae60addd8b6092535c11b42d099d6470ec
> ("drm/prime: drop reference on imported dma-buf come from gem")
>
> Signed-off-by: Aaron Plattner <aplattner at nvidia.com>
> Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
> Cc: David Airlie <airlied at linux.ie>
> ---
> Documentation/DocBook/drm.tmpl | 4 +
> drivers/gpu/drm/drm_prime.c | 186 ++++++++++++++++++++++++++++++++++++++++-
> include/drm/drmP.h | 12 +++
> 3 files changed, 201 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
> index 4ee2304..ed40576 100644
> --- a/Documentation/DocBook/drm.tmpl
> +++ b/Documentation/DocBook/drm.tmpl
> @@ -743,6 +743,10 @@ char *date;</synopsis>
> These two operations are mandatory for GEM drivers that support DRM
> PRIME.
> </para>
> + <sect4>
> + <title>DRM PRIME Helper Functions Reference</title>
> +!Pdrivers/gpu/drm/drm_prime.c PRIME Helpers
> + </sect4>
> </sect3>
> <sect3 id="drm-gem-objects-mapping">
> <title>GEM Objects Mapping</title>
> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> index 7f12573..366910d 100644
> --- a/drivers/gpu/drm/drm_prime.c
> +++ b/drivers/gpu/drm/drm_prime.c
> @@ -53,7 +53,8 @@
> * Self-importing: if userspace is using PRIME as a replacement for flink
> * then it will get a fd->handle request for a GEM object that it created.
> * Drivers should detect this situation and return back the gem object
> - * from the dma-buf private.
> + * from the dma-buf private. Prime will do this automatically for drivers that
> + * use the drm_gem_prime_{import,export} helpers.
> */
>
> struct drm_prime_member {
> @@ -62,6 +63,137 @@ struct drm_prime_member {
> uint32_t handle;
> };
>
> +static struct sg_table *drm_gem_map_dma_buf(struct dma_buf_attachment *attach,
> + enum dma_data_direction dir)
> +{
> + struct drm_gem_object *obj = attach->dmabuf->priv;
> + struct sg_table *sgt;
> +
> + mutex_lock(&obj->dev->struct_mutex);
> +
> + sgt = obj->dev->driver->gem_prime_get_sg_table(obj);
> +
> + if (!IS_ERR_OR_NULL(sgt))
> + dma_map_sg(attach->dev, sgt->sgl, sgt->nents, dir);
> +
> + mutex_unlock(&obj->dev->struct_mutex);
> + return sgt;
> +}
> +
> +static void drm_gem_unmap_dma_buf(struct dma_buf_attachment *attach,
> + struct sg_table *sgt, enum dma_data_direction dir)
> +{
> + dma_unmap_sg(attach->dev, sgt->sgl, sgt->nents, dir);
> + sg_free_table(sgt);
> + kfree(sgt);
> +}
> +
> +static void drm_gem_dmabuf_release(struct dma_buf *dma_buf)
> +{
> + struct drm_gem_object *obj = dma_buf->priv;
> +
> + if (obj->export_dma_buf == dma_buf) {
> + /* drop the reference on the export fd holds */
> + obj->export_dma_buf = NULL;
> + drm_gem_object_unreference_unlocked(obj);
> + }
> +}
> +
> +static void *drm_gem_dmabuf_vmap(struct dma_buf *dma_buf)
> +{
> + struct drm_gem_object *obj = dma_buf->priv;
> + struct drm_device *dev = obj->dev;
> +
> + return dev->driver->gem_prime_vmap(obj);
> +}
> +
> +static void drm_gem_dmabuf_vunmap(struct dma_buf *dma_buf, void *vaddr)
> +{
> + struct drm_gem_object *obj = dma_buf->priv;
> + struct drm_device *dev = obj->dev;
> +
> + dev->driver->gem_prime_vunmap(obj, vaddr);
> +}
> +
> +static void *drm_gem_dmabuf_kmap_atomic(struct dma_buf *dma_buf,
> + unsigned long page_num)
> +{
> + return NULL;
> +}
> +
> +static void drm_gem_dmabuf_kunmap_atomic(struct dma_buf *dma_buf,
> + unsigned long page_num, void *addr)
> +{
> +
> +}
> +static void *drm_gem_dmabuf_kmap(struct dma_buf *dma_buf,
> + unsigned long page_num)
> +{
> + return NULL;
> +}
> +
> +static void drm_gem_dmabuf_kunmap(struct dma_buf *dma_buf,
> + unsigned long page_num, void *addr)
> +{
> +
> +}
> +
> +static int drm_gem_dmabuf_mmap(struct dma_buf *dma_buf,
> + struct vm_area_struct *vma)
> +{
> + return -EINVAL;
> +}
> +
> +static const struct dma_buf_ops drm_gem_prime_dmabuf_ops = {
> + .map_dma_buf = drm_gem_map_dma_buf,
> + .unmap_dma_buf = drm_gem_unmap_dma_buf,
> + .release = drm_gem_dmabuf_release,
> + .kmap = drm_gem_dmabuf_kmap,
> + .kmap_atomic = drm_gem_dmabuf_kmap_atomic,
> + .kunmap = drm_gem_dmabuf_kunmap,
> + .kunmap_atomic = drm_gem_dmabuf_kunmap_atomic,
> + .mmap = drm_gem_dmabuf_mmap,
> + .vmap = drm_gem_dmabuf_vmap,
> + .vunmap = drm_gem_dmabuf_vunmap,
> +};
> +
> +/**
> + * DOC: PRIME Helpers
> + *
> + * Drivers can implement @gem_prime_export and @gem_prime_import in terms of
> + * simpler APIs by using the helper functions @drm_gem_prime_export and
> + * @drm_gem_prime_import. These functions implement dma-buf support in terms of
> + * five lower-level driver callbacks:
> + *
> + * Export callbacks:
> + *
> + * - @gem_prime_pin (optional): prepare a GEM object for exporting
> + *
> + * - @gem_prime_get_sg_table: provide a scatter/gather table of pinned pages
> + *
> + * - @gem_prime_vmap: vmap a buffer exported by your driver
> + *
> + * - @gem_prime_vunmap: vunmap a buffer exported by your driver
> + *
> + * Import callback:
> + *
> + * - @gem_prime_import_sg_table (import): produce a GEM object from another
> + * driver's scatter/gather table
> + */
> +
> +struct dma_buf *drm_gem_prime_export(struct drm_device *dev,
> + struct drm_gem_object *obj, int flags)
> +{
> + if (dev->driver->gem_prime_pin) {
> + int ret = dev->driver->gem_prime_pin(obj);
> + if (ret)
> + return ERR_PTR(ret);
> + }
> + return dma_buf_export(obj, &drm_gem_prime_dmabuf_ops, obj->size,
> + 0600);
> +}
> +EXPORT_SYMBOL(drm_gem_prime_export);
> +
> int drm_gem_prime_handle_to_fd(struct drm_device *dev,
> struct drm_file *file_priv, uint32_t handle, uint32_t flags,
> int *prime_fd)
> @@ -117,6 +249,58 @@ int drm_gem_prime_handle_to_fd(struct drm_device *dev,
> }
> EXPORT_SYMBOL(drm_gem_prime_handle_to_fd);
>
> +struct drm_gem_object *drm_gem_prime_import(struct drm_device *dev,
> + struct dma_buf *dma_buf)
> +{
> + struct dma_buf_attachment *attach;
> + struct sg_table *sgt;
> + struct drm_gem_object *obj;
> + int ret;
> +
> + if (!dev->driver->gem_prime_import_sg_table)
> + return ERR_PTR(-EINVAL);
> +
> + if (dma_buf->ops == &drm_gem_prime_dmabuf_ops) {
This here breaks self-import checks since it smashes all buffers from all
drivers using these helpers into one set. Which means e.g. nouveau will
happily import a buffer from radoen as it's own. The only reason afaics
that shit didn't completely hit the fan is that i915 still has it's own
buffer ops, and everyone seems to only care about sharing with i915.
So after a few months of subconscious pondering (well, just re-read the
code to review Dave's patch) I now finally know why I totally didn't like
that we move the vtable into the helpers ;-)
I think someone needs to fix this or we need to revert this patch here.
Cheers, Daniel
> + obj = dma_buf->priv;
> + if (obj->dev == dev) {
> + /*
> + * Importing dmabuf exported from out own gem increases
> + * refcount on gem itself instead of f_count of dmabuf.
> + */
> + drm_gem_object_reference(obj);
> + dma_buf_put(dma_buf);
> + return obj;
> + }
> + }
> +
> + attach = dma_buf_attach(dma_buf, dev->dev);
> + if (IS_ERR(attach))
> + return ERR_PTR(PTR_ERR(attach));
> +
> + sgt = dma_buf_map_attachment(attach, DMA_BIDIRECTIONAL);
> + if (IS_ERR_OR_NULL(sgt)) {
> + ret = PTR_ERR(sgt);
> + goto fail_detach;
> + }
> +
> + obj = dev->driver->gem_prime_import_sg_table(dev, dma_buf->size, sgt);
> + if (IS_ERR(obj)) {
> + ret = PTR_ERR(obj);
> + goto fail_unmap;
> + }
> +
> + obj->import_attach = attach;
> +
> + return obj;
> +
> +fail_unmap:
> + dma_buf_unmap_attachment(attach, sgt, DMA_BIDIRECTIONAL);
> +fail_detach:
> + dma_buf_detach(dma_buf, attach);
> + return ERR_PTR(ret);
> +}
> +EXPORT_SYMBOL(drm_gem_prime_import);
> +
> int drm_gem_prime_fd_to_handle(struct drm_device *dev,
> struct drm_file *file_priv, int prime_fd, uint32_t *handle)
> {
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index fad21c9..2d4ca6f 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -919,6 +919,14 @@ struct drm_driver {
> /* import dmabuf -> GEM */
> struct drm_gem_object * (*gem_prime_import)(struct drm_device *dev,
> struct dma_buf *dma_buf);
> + /* low-level interface used by drm_gem_prime_{import,export} */
> + int (*gem_prime_pin)(struct drm_gem_object *obj);
> + struct sg_table *(*gem_prime_get_sg_table)(struct drm_gem_object *obj);
> + struct drm_gem_object *(*gem_prime_import_sg_table)(
> + struct drm_device *dev, size_t size,
> + struct sg_table *sgt);
> + void *(*gem_prime_vmap)(struct drm_gem_object *obj);
> + void (*gem_prime_vunmap)(struct drm_gem_object *obj, void *vaddr);
>
> /* vga arb irq handler */
> void (*vgaarb_irq)(struct drm_device *dev, bool state);
> @@ -1540,9 +1548,13 @@ extern int drm_clients_info(struct seq_file *m, void* data);
> extern int drm_gem_name_info(struct seq_file *m, void *data);
>
>
> +extern struct dma_buf *drm_gem_prime_export(struct drm_device *dev,
> + struct drm_gem_object *obj, int flags);
> extern int drm_gem_prime_handle_to_fd(struct drm_device *dev,
> struct drm_file *file_priv, uint32_t handle, uint32_t flags,
> int *prime_fd);
> +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);
>
> --
> 1.8.1.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
More information about the dri-devel
mailing list