Re: [PATCH v3 1/3] drm: add prime helpers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Aaron,

On Tuesday 18 June 2013 16:28:15 Aaron Plattner wrote:
> On 06/18/2013 04:08 PM, Laurent Pinchart wrote:
> > Hi Aaron,
> > 
> > A bit late, but here's a small question.
> > 
> > On Tuesday 15 January 2013 12:47:42 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@xxxxxxxxxx>
> >> Cc: Daniel Vetter <daniel.vetter@xxxxxxxx>
> >> Cc: David Airlie <airlied@xxxxxxxx>
> >> ---
> >> 
> >>   Documentation/DocBook/drm.tmpl |   4 +
> >>   drivers/gpu/drm/drm_prime.c    | 186
> >>   +++++++++++++++++++++++++++++++++++++-
> >>   include/drm/drmP.h             |  12 +++
> >>   3 files changed, 201 insertions(+), 1 deletion(-)
> > 
> > [snip]
> > 
> >> 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
> > 
> > [snip]
> > 
> >> +/**
> >> + * 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);
> > 
> > Why do you use 0600 instead of the flags passed by the caller ?
> 
> Because I copied & pasted it from i915_gem_prime_export prior to commit
> 5b42427fc38ecb9056c4e64deaff36d6d6ba1b67 and didn't notice until you
> pointed it out just now.

That's a pretty valid reason I guess :-) Should I submit a patch or are you 
already working on one ?

> >> +}
> >> +EXPORT_SYMBOL(drm_gem_prime_export);

-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux