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@nvidia.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: David Airlie airlied@linux.ie
Two minor things, but since you're not touching drm/i915 I don't care that much ...
+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);
The locking here looks superflous and not required to protect anything drm_gem_map_dma_buf does. If drivers need this (and ttm based ones shouldn't really), they can grab it in their ->gem_prime_get_sg_table callback. So I think a follow-up patch to ditch this would be good.
- 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);
+}
Imo these two don't add value, simpler to add a typechecking drm_dmabuf_to_gem_obj static inline somewhere. But then you'd need to pass in the dmabuf fops struct from drivers and export the helpers. More flexible, but also a bit work to redo. Like I've said, I don't care too much ... -Daniel
+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) {
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