| Message ID | 20231121231114.703478-2-Felix.Kuehling@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2,1/4] Revert "drm/prime: Unexport helpers for fd/handle conversion" | expand |
Hi, my apologies if this sounds picky or annoying. This change appears to be going in the wrong direction. The goal of the refactoring is to be able to use drm_driver.gem_prime_import and drm_gem_object_funcs.export for the additional import/export code; and hence keep the GEM object code in a single place. Keeping the prime_fd file descriptor within amdkfd will likely help with that. Here's my suggestion: 1) Please keep the internal interfaces drm_gem_prime_handle_to_fd() and drm_gem_prime_fd_to_handle(). They should be called from the _ioctl entry functions as is. That could be stream-lined in a later patch set. 2) From drm_gem_prime_handle_to_fd() and drm_gem_prime_fd_to_handle(), create drm_gem_prime_handle_to_dmabuf() and drm_gem_prime_dmabuf_to_handle(). They should be exported. You can then keep the file-descriptor code in amdkfd and out of the PRIME helpers. 3) Patches 1 and 2 should be squashed into one. 4) And if I'm not mistaken, the additional import/export code can then go into drm_driver.gem_prime_import and drm_gem_object_funcs.export, which are being called from within the PRIME helpers. That's admittedly quite a bit of refactoring. OR simply go back to v1 of this patch set, which was consistent at least. Best regards Thomas Am 22.11.23 um 00:11 schrieb Felix Kuehling: > Change drm_gem_prime_handle_to_fd to drm_gem_prime_handle_to_dmabuf to > export a dmabuf without creating an FD as a user mode handle. This is > more useful for users in kernel mode. > > Suggested-by: Thomas Zimmermann <tzimmermann@suse.de> > Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com> > --- > drivers/gpu/drm/drm_prime.c | 63 ++++++++++++++++++------------------- > include/drm/drm_prime.h | 6 ++-- > 2 files changed, 33 insertions(+), 36 deletions(-) > > diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c > index 834a5e28abbe..d491b5f73eea 100644 > --- a/drivers/gpu/drm/drm_prime.c > +++ b/drivers/gpu/drm/drm_prime.c > @@ -410,26 +410,25 @@ static struct dma_buf *export_and_register_object(struct drm_device *dev, > } > > /** > - * drm_gem_prime_handle_to_fd - PRIME export function for GEM drivers > + * drm_gem_prime_handle_to_dmabuf - PRIME export function for GEM drivers > * @dev: dev to export the buffer from > * @file_priv: drm file-private structure > * @handle: buffer handle to export > * @flags: flags like DRM_CLOEXEC > - * @prime_fd: pointer to storage for the fd id of the create dma-buf > + * @dma_buf: pointer to storage for the dma-buf reference > * > * This is the PRIME export function which must be used mandatorily by GEM > * drivers to ensure correct lifetime management of the underlying GEM object. > * The actual exporting from GEM object to a dma-buf is done through the > * &drm_gem_object_funcs.export callback. > */ > -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) > +struct dma_buf *drm_gem_prime_handle_to_dmabuf(struct drm_device *dev, > + struct drm_file *file_priv, > + uint32_t handle, uint32_t flags) > { > struct drm_gem_object *obj; > int ret = 0; > - struct dma_buf *dmabuf; > + struct dma_buf *dmabuf = NULL; > > mutex_lock(&file_priv->prime.lock); > obj = drm_gem_object_lookup(file_priv, handle); > @@ -441,7 +440,7 @@ int drm_gem_prime_handle_to_fd(struct drm_device *dev, > dmabuf = drm_prime_lookup_buf_by_handle(&file_priv->prime, handle); > if (dmabuf) { > get_dma_buf(dmabuf); > - goto out_have_handle; > + goto out; > } > > mutex_lock(&dev->object_name_lock); > @@ -479,40 +478,22 @@ int drm_gem_prime_handle_to_fd(struct drm_device *dev, > dmabuf, handle); > mutex_unlock(&dev->object_name_lock); > if (ret) > - goto fail_put_dmabuf; > - > -out_have_handle: > - ret = dma_buf_fd(dmabuf, flags); > - /* > - * We must _not_ remove the buffer from the handle cache since the newly > - * created dma buf is already linked in the global obj->dma_buf pointer, > - * and that is invariant as long as a userspace gem handle exists. > - * Closing the handle will clean out the cache anyway, so we don't leak. > - */ > - if (ret < 0) { > - goto fail_put_dmabuf; > - } else { > - *prime_fd = ret; > - ret = 0; > - } > - > - goto out; > - > -fail_put_dmabuf: > - dma_buf_put(dmabuf); > + dma_buf_put(dmabuf); > out: > drm_gem_object_put(obj); > out_unlock: > mutex_unlock(&file_priv->prime.lock); > > - return ret; > + return ret ? ERR_PTR(ret) : dmabuf; > } > -EXPORT_SYMBOL(drm_gem_prime_handle_to_fd); > +EXPORT_SYMBOL(drm_gem_prime_handle_to_dmabuf); > > int drm_prime_handle_to_fd_ioctl(struct drm_device *dev, void *data, > struct drm_file *file_priv) > { > struct drm_prime_handle *args = data; > + struct dma_buf *dmabuf; > + int ret; > > /* check flags are valid */ > if (args->flags & ~(DRM_CLOEXEC | DRM_RDWR)) > @@ -523,8 +504,24 @@ int drm_prime_handle_to_fd_ioctl(struct drm_device *dev, void *data, > args->handle, args->flags, > &args->fd); > } > - return drm_gem_prime_handle_to_fd(dev, file_priv, args->handle, > - args->flags, &args->fd); > + dmabuf = drm_gem_prime_handle_to_dmabuf(dev, file_priv, args->handle, > + args->flags); > + if (IS_ERR(dmabuf)) > + return PTR_ERR(dmabuf); > + ret = dma_buf_fd(dmabuf, args->flags); > + /* > + * We must _not_ remove the buffer from the handle cache since the newly > + * created dma buf is already linked in the global obj->dma_buf pointer, > + * and that is invariant as long as a userspace gem handle exists. > + * Closing the handle will clean out the cache anyway, so we don't leak. > + */ > + if (ret < 0) { > + dma_buf_put(dmabuf); > + } else { > + args->fd = ret; > + ret = 0; > + } > + return ret; > } > > /** > diff --git a/include/drm/drm_prime.h b/include/drm/drm_prime.h > index 2a1d01e5b56b..89e839293d14 100644 > --- a/include/drm/drm_prime.h > +++ b/include/drm/drm_prime.h > @@ -69,9 +69,9 @@ void drm_gem_dmabuf_release(struct dma_buf *dma_buf); > > int drm_gem_prime_fd_to_handle(struct drm_device *dev, > struct drm_file *file_priv, int prime_fd, uint32_t *handle); > -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); > +struct dma_buf *drm_gem_prime_handle_to_dmabuf(struct drm_device *dev, > + struct drm_file *file_priv, > + uint32_t handle, uint32_t flags); > > /* helper functions for exporting */ > int drm_gem_map_attach(struct dma_buf *dma_buf,
Hi Felix,
kernel test robot noticed the following build warnings:
[auto build test WARNING on drm-misc/drm-misc-next]
[also build test WARNING on drm/drm-next drm-exynos/exynos-drm-next drm-intel/for-linux-next drm-intel/for-linux-next-fixes drm-tip/drm-tip linus/master v6.7-rc2 next-20231122]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Felix-Kuehling/drm-prime-Helper-to-export-dmabuf-without-fd/20231122-071410
base: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
patch link: https://lore.kernel.org/r/20231121231114.703478-2-Felix.Kuehling%40amd.com
patch subject: [PATCH v2 2/4] drm/prime: Helper to export dmabuf without fd
config: alpha-allyesconfig (https://download.01.org/0day-ci/archive/20231122/202311221810.IJ6ELH6c-lkp@intel.com/config)
compiler: alpha-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231122/202311221810.IJ6ELH6c-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202311221810.IJ6ELH6c-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> drivers/gpu/drm/drm_prime.c:428: warning: Excess function parameter 'dma_buf' description in 'drm_gem_prime_handle_to_dmabuf'
vim +428 drivers/gpu/drm/drm_prime.c
319c933c71f3db Daniel Vetter 2013-08-15 411
d3977ac8e10a0b Felix Kuehling 2023-11-21 412 /**
f3facd92ee25e0 Felix Kuehling 2023-11-21 413 * drm_gem_prime_handle_to_dmabuf - PRIME export function for GEM drivers
39cc344acd414e Daniel Vetter 2014-01-22 414 * @dev: dev to export the buffer from
39cc344acd414e Daniel Vetter 2014-01-22 415 * @file_priv: drm file-private structure
39cc344acd414e Daniel Vetter 2014-01-22 416 * @handle: buffer handle to export
39cc344acd414e Daniel Vetter 2014-01-22 417 * @flags: flags like DRM_CLOEXEC
f3facd92ee25e0 Felix Kuehling 2023-11-21 418 * @dma_buf: pointer to storage for the dma-buf reference
39cc344acd414e Daniel Vetter 2014-01-22 419 *
39cc344acd414e Daniel Vetter 2014-01-22 420 * This is the PRIME export function which must be used mandatorily by GEM
39cc344acd414e Daniel Vetter 2014-01-22 421 * drivers to ensure correct lifetime management of the underlying GEM object.
39cc344acd414e Daniel Vetter 2014-01-22 422 * The actual exporting from GEM object to a dma-buf is done through the
d693def4fd1c23 Thomas Zimmermann 2020-09-23 423 * &drm_gem_object_funcs.export callback.
39cc344acd414e Daniel Vetter 2014-01-22 424 */
f3facd92ee25e0 Felix Kuehling 2023-11-21 425 struct dma_buf *drm_gem_prime_handle_to_dmabuf(struct drm_device *dev,
f3facd92ee25e0 Felix Kuehling 2023-11-21 426 struct drm_file *file_priv,
f3facd92ee25e0 Felix Kuehling 2023-11-21 427 uint32_t handle, uint32_t flags)
3248877ea17969 Dave Airlie 2011-11-25 @428 {
3248877ea17969 Dave Airlie 2011-11-25 429 struct drm_gem_object *obj;
219b47339ced80 Dave Airlie 2013-04-22 430 int ret = 0;
f3facd92ee25e0 Felix Kuehling 2023-11-21 431 struct dma_buf *dmabuf = NULL;
3248877ea17969 Dave Airlie 2011-11-25 432
d0b2c5334f41bd Daniel Vetter 2013-08-15 433 mutex_lock(&file_priv->prime.lock);
a8ad0bd84f9860 Chris Wilson 2016-05-09 434 obj = drm_gem_object_lookup(file_priv, handle);
d0b2c5334f41bd Daniel Vetter 2013-08-15 435 if (!obj) {
d0b2c5334f41bd Daniel Vetter 2013-08-15 436 ret = -ENOENT;
d0b2c5334f41bd Daniel Vetter 2013-08-15 437 goto out_unlock;
d0b2c5334f41bd Daniel Vetter 2013-08-15 438 }
d0b2c5334f41bd Daniel Vetter 2013-08-15 439
d0b2c5334f41bd Daniel Vetter 2013-08-15 440 dmabuf = drm_prime_lookup_buf_by_handle(&file_priv->prime, handle);
d0b2c5334f41bd Daniel Vetter 2013-08-15 441 if (dmabuf) {
d0b2c5334f41bd Daniel Vetter 2013-08-15 442 get_dma_buf(dmabuf);
f3facd92ee25e0 Felix Kuehling 2023-11-21 443 goto out;
d0b2c5334f41bd Daniel Vetter 2013-08-15 444 }
3248877ea17969 Dave Airlie 2011-11-25 445
d0b2c5334f41bd Daniel Vetter 2013-08-15 446 mutex_lock(&dev->object_name_lock);
3248877ea17969 Dave Airlie 2011-11-25 447 /* re-export the original imported object */
3248877ea17969 Dave Airlie 2011-11-25 448 if (obj->import_attach) {
219b47339ced80 Dave Airlie 2013-04-22 449 dmabuf = obj->import_attach->dmabuf;
319c933c71f3db Daniel Vetter 2013-08-15 450 get_dma_buf(dmabuf);
219b47339ced80 Dave Airlie 2013-04-22 451 goto out_have_obj;
3248877ea17969 Dave Airlie 2011-11-25 452 }
3248877ea17969 Dave Airlie 2011-11-25 453
319c933c71f3db Daniel Vetter 2013-08-15 454 if (obj->dma_buf) {
319c933c71f3db Daniel Vetter 2013-08-15 455 get_dma_buf(obj->dma_buf);
319c933c71f3db Daniel Vetter 2013-08-15 456 dmabuf = obj->dma_buf;
219b47339ced80 Dave Airlie 2013-04-22 457 goto out_have_obj;
219b47339ced80 Dave Airlie 2013-04-22 458 }
219b47339ced80 Dave Airlie 2013-04-22 459
319c933c71f3db Daniel Vetter 2013-08-15 460 dmabuf = export_and_register_object(dev, obj, flags);
4332bf438bbbc3 Daniel Vetter 2013-08-15 461 if (IS_ERR(dmabuf)) {
3248877ea17969 Dave Airlie 2011-11-25 462 /* normally the created dma-buf takes ownership of the ref,
3248877ea17969 Dave Airlie 2011-11-25 463 * but if that fails then drop the ref
3248877ea17969 Dave Airlie 2011-11-25 464 */
4332bf438bbbc3 Daniel Vetter 2013-08-15 465 ret = PTR_ERR(dmabuf);
d0b2c5334f41bd Daniel Vetter 2013-08-15 466 mutex_unlock(&dev->object_name_lock);
219b47339ced80 Dave Airlie 2013-04-22 467 goto out;
3248877ea17969 Dave Airlie 2011-11-25 468 }
219b47339ced80 Dave Airlie 2013-04-22 469
d0b2c5334f41bd Daniel Vetter 2013-08-15 470 out_have_obj:
d0b2c5334f41bd Daniel Vetter 2013-08-15 471 /*
d0b2c5334f41bd Daniel Vetter 2013-08-15 472 * If we've exported this buffer then cheat and add it to the import list
d0b2c5334f41bd Daniel Vetter 2013-08-15 473 * so we get the correct handle back. We must do this under the
d0b2c5334f41bd Daniel Vetter 2013-08-15 474 * protection of dev->object_name_lock to ensure that a racing gem close
d0b2c5334f41bd Daniel Vetter 2013-08-15 475 * ioctl doesn't miss to remove this buffer handle from the cache.
0ff926c7d4f06f Dave Airlie 2012-05-20 476 */
219b47339ced80 Dave Airlie 2013-04-22 477 ret = drm_prime_add_buf_handle(&file_priv->prime,
319c933c71f3db Daniel Vetter 2013-08-15 478 dmabuf, handle);
d0b2c5334f41bd Daniel Vetter 2013-08-15 479 mutex_unlock(&dev->object_name_lock);
219b47339ced80 Dave Airlie 2013-04-22 480 if (ret)
4332bf438bbbc3 Daniel Vetter 2013-08-15 481 dma_buf_put(dmabuf);
219b47339ced80 Dave Airlie 2013-04-22 482 out:
be6ee102341bc4 Emil Velikov 2020-05-15 483 drm_gem_object_put(obj);
d0b2c5334f41bd Daniel Vetter 2013-08-15 484 out_unlock:
d0b2c5334f41bd Daniel Vetter 2013-08-15 485 mutex_unlock(&file_priv->prime.lock);
d0b2c5334f41bd Daniel Vetter 2013-08-15 486
f3facd92ee25e0 Felix Kuehling 2023-11-21 487 return ret ? ERR_PTR(ret) : dmabuf;
b283e92a2315f9 Daniel Vetter 2019-06-18 488 }
f3facd92ee25e0 Felix Kuehling 2023-11-21 489 EXPORT_SYMBOL(drm_gem_prime_handle_to_dmabuf);
b283e92a2315f9 Daniel Vetter 2019-06-18 490
On 2023-11-22 05:32, Thomas Zimmermann wrote: > Hi, > > my apologies if this sounds picky or annoying. This change appears to be > going in the wrong direction. The goal of the refactoring is to be able > to use drm_driver.gem_prime_import and drm_gem_object_funcs.export for > the additional import/export code; and hence keep the GEM object code in > a single place. Keeping the prime_fd file descriptor within amdkfd will > likely help with that. > > Here's my suggestion: > > 1) Please keep the internal interfaces drm_gem_prime_handle_to_fd() > and drm_gem_prime_fd_to_handle(). They should be called from the _ioctl > entry functions as is. That could be stream-lined in a later patch set. > > 2) From drm_gem_prime_handle_to_fd() and drm_gem_prime_fd_to_handle(), > create drm_gem_prime_handle_to_dmabuf() and > drm_gem_prime_dmabuf_to_handle(). Do you mean duplicate the code, or call drm_gem_prime_handle_to_dmabuf from drm_gem_prime_handle_to_fd? > They should be exported. You can then > keep the file-descriptor code in amdkfd and out of the PRIME helpers. > > 3) Patches 1 and 2 should be squashed into one. > > 4) And if I'm not mistaken, the additional import/export code can then > go into drm_driver.gem_prime_import and drm_gem_object_funcs.export, > which are being called from within the PRIME helpers. I'm not sure what you mean by "additional import/export code" that would move into those driver callbacks. > > That's admittedly quite a bit of refactoring. OR simply go back to v1 of > this patch set, which was consistent at least. I think I'd prefer that because I don't really understand what you're trying to achieve. Thanks, Felix > > Best regards > Thomas > > > Am 22.11.23 um 00:11 schrieb Felix Kuehling: >> Change drm_gem_prime_handle_to_fd to drm_gem_prime_handle_to_dmabuf to >> export a dmabuf without creating an FD as a user mode handle. This is >> more useful for users in kernel mode. >> >> Suggested-by: Thomas Zimmermann <tzimmermann@suse.de> >> Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com> >> --- >> drivers/gpu/drm/drm_prime.c | 63 ++++++++++++++++++------------------- >> include/drm/drm_prime.h | 6 ++-- >> 2 files changed, 33 insertions(+), 36 deletions(-) >> >> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c >> index 834a5e28abbe..d491b5f73eea 100644 >> --- a/drivers/gpu/drm/drm_prime.c >> +++ b/drivers/gpu/drm/drm_prime.c >> @@ -410,26 +410,25 @@ static struct dma_buf *export_and_register_object(struct drm_device *dev, >> } >> >> /** >> - * drm_gem_prime_handle_to_fd - PRIME export function for GEM drivers >> + * drm_gem_prime_handle_to_dmabuf - PRIME export function for GEM drivers >> * @dev: dev to export the buffer from >> * @file_priv: drm file-private structure >> * @handle: buffer handle to export >> * @flags: flags like DRM_CLOEXEC >> - * @prime_fd: pointer to storage for the fd id of the create dma-buf >> + * @dma_buf: pointer to storage for the dma-buf reference >> * >> * This is the PRIME export function which must be used mandatorily by GEM >> * drivers to ensure correct lifetime management of the underlying GEM object. >> * The actual exporting from GEM object to a dma-buf is done through the >> * &drm_gem_object_funcs.export callback. >> */ >> -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) >> +struct dma_buf *drm_gem_prime_handle_to_dmabuf(struct drm_device *dev, >> + struct drm_file *file_priv, >> + uint32_t handle, uint32_t flags) >> { >> struct drm_gem_object *obj; >> int ret = 0; >> - struct dma_buf *dmabuf; >> + struct dma_buf *dmabuf = NULL; >> >> mutex_lock(&file_priv->prime.lock); >> obj = drm_gem_object_lookup(file_priv, handle); >> @@ -441,7 +440,7 @@ int drm_gem_prime_handle_to_fd(struct drm_device *dev, >> dmabuf = drm_prime_lookup_buf_by_handle(&file_priv->prime, handle); >> if (dmabuf) { >> get_dma_buf(dmabuf); >> - goto out_have_handle; >> + goto out; >> } >> >> mutex_lock(&dev->object_name_lock); >> @@ -479,40 +478,22 @@ int drm_gem_prime_handle_to_fd(struct drm_device *dev, >> dmabuf, handle); >> mutex_unlock(&dev->object_name_lock); >> if (ret) >> - goto fail_put_dmabuf; >> - >> -out_have_handle: >> - ret = dma_buf_fd(dmabuf, flags); >> - /* >> - * We must _not_ remove the buffer from the handle cache since the newly >> - * created dma buf is already linked in the global obj->dma_buf pointer, >> - * and that is invariant as long as a userspace gem handle exists. >> - * Closing the handle will clean out the cache anyway, so we don't leak. >> - */ >> - if (ret < 0) { >> - goto fail_put_dmabuf; >> - } else { >> - *prime_fd = ret; >> - ret = 0; >> - } >> - >> - goto out; >> - >> -fail_put_dmabuf: >> - dma_buf_put(dmabuf); >> + dma_buf_put(dmabuf); >> out: >> drm_gem_object_put(obj); >> out_unlock: >> mutex_unlock(&file_priv->prime.lock); >> >> - return ret; >> + return ret ? ERR_PTR(ret) : dmabuf; >> } >> -EXPORT_SYMBOL(drm_gem_prime_handle_to_fd); >> +EXPORT_SYMBOL(drm_gem_prime_handle_to_dmabuf); >> >> int drm_prime_handle_to_fd_ioctl(struct drm_device *dev, void *data, >> struct drm_file *file_priv) >> { >> struct drm_prime_handle *args = data; >> + struct dma_buf *dmabuf; >> + int ret; >> >> /* check flags are valid */ >> if (args->flags & ~(DRM_CLOEXEC | DRM_RDWR)) >> @@ -523,8 +504,24 @@ int drm_prime_handle_to_fd_ioctl(struct drm_device *dev, void *data, >> args->handle, args->flags, >> &args->fd); >> } >> - return drm_gem_prime_handle_to_fd(dev, file_priv, args->handle, >> - args->flags, &args->fd); >> + dmabuf = drm_gem_prime_handle_to_dmabuf(dev, file_priv, args->handle, >> + args->flags); >> + if (IS_ERR(dmabuf)) >> + return PTR_ERR(dmabuf); >> + ret = dma_buf_fd(dmabuf, args->flags); >> + /* >> + * We must _not_ remove the buffer from the handle cache since the newly >> + * created dma buf is already linked in the global obj->dma_buf pointer, >> + * and that is invariant as long as a userspace gem handle exists. >> + * Closing the handle will clean out the cache anyway, so we don't leak. >> + */ >> + if (ret < 0) { >> + dma_buf_put(dmabuf); >> + } else { >> + args->fd = ret; >> + ret = 0; >> + } >> + return ret; >> } >> >> /** >> diff --git a/include/drm/drm_prime.h b/include/drm/drm_prime.h >> index 2a1d01e5b56b..89e839293d14 100644 >> --- a/include/drm/drm_prime.h >> +++ b/include/drm/drm_prime.h >> @@ -69,9 +69,9 @@ void drm_gem_dmabuf_release(struct dma_buf *dma_buf); >> >> int drm_gem_prime_fd_to_handle(struct drm_device *dev, >> struct drm_file *file_priv, int prime_fd, uint32_t *handle); >> -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); >> +struct dma_buf *drm_gem_prime_handle_to_dmabuf(struct drm_device *dev, >> + struct drm_file *file_priv, >> + uint32_t handle, uint32_t flags); >> >> /* helper functions for exporting */ >> int drm_gem_map_attach(struct dma_buf *dma_buf,
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c index 834a5e28abbe..d491b5f73eea 100644 --- a/drivers/gpu/drm/drm_prime.c +++ b/drivers/gpu/drm/drm_prime.c @@ -410,26 +410,25 @@ static struct dma_buf *export_and_register_object(struct drm_device *dev, } /** - * drm_gem_prime_handle_to_fd - PRIME export function for GEM drivers + * drm_gem_prime_handle_to_dmabuf - PRIME export function for GEM drivers * @dev: dev to export the buffer from * @file_priv: drm file-private structure * @handle: buffer handle to export * @flags: flags like DRM_CLOEXEC - * @prime_fd: pointer to storage for the fd id of the create dma-buf + * @dma_buf: pointer to storage for the dma-buf reference * * This is the PRIME export function which must be used mandatorily by GEM * drivers to ensure correct lifetime management of the underlying GEM object. * The actual exporting from GEM object to a dma-buf is done through the * &drm_gem_object_funcs.export callback. */ -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) +struct dma_buf *drm_gem_prime_handle_to_dmabuf(struct drm_device *dev, + struct drm_file *file_priv, + uint32_t handle, uint32_t flags) { struct drm_gem_object *obj; int ret = 0; - struct dma_buf *dmabuf; + struct dma_buf *dmabuf = NULL; mutex_lock(&file_priv->prime.lock); obj = drm_gem_object_lookup(file_priv, handle); @@ -441,7 +440,7 @@ int drm_gem_prime_handle_to_fd(struct drm_device *dev, dmabuf = drm_prime_lookup_buf_by_handle(&file_priv->prime, handle); if (dmabuf) { get_dma_buf(dmabuf); - goto out_have_handle; + goto out; } mutex_lock(&dev->object_name_lock); @@ -479,40 +478,22 @@ int drm_gem_prime_handle_to_fd(struct drm_device *dev, dmabuf, handle); mutex_unlock(&dev->object_name_lock); if (ret) - goto fail_put_dmabuf; - -out_have_handle: - ret = dma_buf_fd(dmabuf, flags); - /* - * We must _not_ remove the buffer from the handle cache since the newly - * created dma buf is already linked in the global obj->dma_buf pointer, - * and that is invariant as long as a userspace gem handle exists. - * Closing the handle will clean out the cache anyway, so we don't leak. - */ - if (ret < 0) { - goto fail_put_dmabuf; - } else { - *prime_fd = ret; - ret = 0; - } - - goto out; - -fail_put_dmabuf: - dma_buf_put(dmabuf); + dma_buf_put(dmabuf); out: drm_gem_object_put(obj); out_unlock: mutex_unlock(&file_priv->prime.lock); - return ret; + return ret ? ERR_PTR(ret) : dmabuf; } -EXPORT_SYMBOL(drm_gem_prime_handle_to_fd); +EXPORT_SYMBOL(drm_gem_prime_handle_to_dmabuf); int drm_prime_handle_to_fd_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv) { struct drm_prime_handle *args = data; + struct dma_buf *dmabuf; + int ret; /* check flags are valid */ if (args->flags & ~(DRM_CLOEXEC | DRM_RDWR)) @@ -523,8 +504,24 @@ int drm_prime_handle_to_fd_ioctl(struct drm_device *dev, void *data, args->handle, args->flags, &args->fd); } - return drm_gem_prime_handle_to_fd(dev, file_priv, args->handle, - args->flags, &args->fd); + dmabuf = drm_gem_prime_handle_to_dmabuf(dev, file_priv, args->handle, + args->flags); + if (IS_ERR(dmabuf)) + return PTR_ERR(dmabuf); + ret = dma_buf_fd(dmabuf, args->flags); + /* + * We must _not_ remove the buffer from the handle cache since the newly + * created dma buf is already linked in the global obj->dma_buf pointer, + * and that is invariant as long as a userspace gem handle exists. + * Closing the handle will clean out the cache anyway, so we don't leak. + */ + if (ret < 0) { + dma_buf_put(dmabuf); + } else { + args->fd = ret; + ret = 0; + } + return ret; } /** diff --git a/include/drm/drm_prime.h b/include/drm/drm_prime.h index 2a1d01e5b56b..89e839293d14 100644 --- a/include/drm/drm_prime.h +++ b/include/drm/drm_prime.h @@ -69,9 +69,9 @@ void drm_gem_dmabuf_release(struct dma_buf *dma_buf); int drm_gem_prime_fd_to_handle(struct drm_device *dev, struct drm_file *file_priv, int prime_fd, uint32_t *handle); -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); +struct dma_buf *drm_gem_prime_handle_to_dmabuf(struct drm_device *dev, + struct drm_file *file_priv, + uint32_t handle, uint32_t flags); /* helper functions for exporting */ int drm_gem_map_attach(struct dma_buf *dma_buf,
Change drm_gem_prime_handle_to_fd to drm_gem_prime_handle_to_dmabuf to export a dmabuf without creating an FD as a user mode handle. This is more useful for users in kernel mode. Suggested-by: Thomas Zimmermann <tzimmermann@suse.de> Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com> --- drivers/gpu/drm/drm_prime.c | 63 ++++++++++++++++++------------------- include/drm/drm_prime.h | 6 ++-- 2 files changed, 33 insertions(+), 36 deletions(-)