On 06/18/2013 04:37 PM, Laurent Pinchart wrote: > 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 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(-) >>> >>> [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 ?
:) Sorry! I'm not working on this right now, but I can put it on my queue if you like. You'll probably be able to fix and test it sooner than I will, though. >>>> +} >>>> +EXPORT_SYMBOL(drm_gem_prime_export); -- Aaron