AMD-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Zimmermann <tzimmermann@suse.de>
To: Boris Brezillon <boris.brezillon@collabora.com>,
	Steven Price <steven.price@arm.com>
Cc: dri-devel@lists.freedesktop.org,
	"Maarten Lankhorst" <maarten.lankhorst@linux.intel.com>,
	"Maxime Ripard" <mripard@kernel.org>,
	"David Airlie" <airlied@gmail.com>,
	"Simona Vetter" <simona@ffwll.ch>,
	"Faith Ekstrand" <faith.ekstrand@collabora.com>,
	"Thierry Reding" <thierry.reding@gmail.com>,
	"Mikko Perttunen" <mperttunen@nvidia.com>,
	"Melissa Wen" <mwen@igalia.com>,
	"Maíra Canal" <mcanal@igalia.com>,
	"Lucas De Marchi" <lucas.demarchi@intel.com>,
	"Thomas Hellström" <thomas.hellstrom@linux.intel.com>,
	"Rodrigo Vivi" <rodrigo.vivi@intel.com>,
	"Frank Binns" <frank.binns@imgtec.com>,
	"Matt Coster" <matt.coster@imgtec.com>,
	"Rob Clark" <robin.clark@oss.qualcomm.com>,
	"Dmitry Baryshkov" <lumag@kernel.org>,
	"Abhinav Kumar" <abhinav.kumar@linux.dev>,
	"Jessica Zhang" <jessica.zhang@oss.qualcomm.com>,
	"Sean Paul" <sean@poorly.run>,
	"Marijn Suijten" <marijn.suijten@somainline.org>,
	"Alex Deucher" <alexander.deucher@amd.com>,
	"Christian König" <christian.koenig@amd.com>,
	amd-gfx@lists.freedesktop.org, kernel@collabora.com
Subject: Re: [PATCH v6 01/16] drm/prime: Simplify life of drivers needing custom dma_buf_ops
Date: Thu, 27 Nov 2025 09:34:43 +0100	[thread overview]
Message-ID: <2e789ff6-b79f-4577-bc69-f74dfed6acfa@suse.de> (raw)
In-Reply-To: <20251126124455.3656651-2-boris.brezillon@collabora.com>

Hi

Am 26.11.25 um 13:44 schrieb Boris Brezillon:
> drm_gem_is_prime_exported_dma_buf() checks the dma_buf->ops against
> drm_gem_prime_dmabuf_ops, which makes it impossible to use if the
> driver implements custom dma_buf_ops. Instead of duplicating a bunch
> of helpers to work around it, let's provide a way for drivers to
> expose their custom dma_buf_ops so the core prime helpers can rely on
> that instead of hardcoding &drm_gem_prime_dmabuf_ops.

This can't go in as-is. I've spent an awful amount of patches on 
removing buffer callbacks from struct drm_driver. Let's please not go 
back to that.

>
> v5:
> - New patch
>
> v6:
> - Pass custom dma_buf_ops directly instead of through a getter
>
> Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
> ---
>   drivers/gpu/drm/drm_prime.c | 10 ++++++++--
>   include/drm/drm_drv.h       |  8 ++++++++
>   2 files changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> index 21809a82187b..86fd95f0c105 100644
> --- a/drivers/gpu/drm/drm_prime.c
> +++ b/drivers/gpu/drm/drm_prime.c
> @@ -904,6 +904,12 @@ unsigned long drm_prime_get_contiguous_size(struct sg_table *sgt)
>   }
>   EXPORT_SYMBOL(drm_prime_get_contiguous_size);
>   
> +static const struct dma_buf_ops *
> +drm_gem_prime_get_dma_buf_ops(struct drm_device *dev)
> +{
> +	return dev->driver->dma_buf_ops ?: &drm_gem_prime_dmabuf_ops;
> +}
> +
>   /**
>    * drm_gem_prime_export - helper library implementation of the export callback
>    * @obj: GEM object to export
> @@ -920,7 +926,7 @@ struct dma_buf *drm_gem_prime_export(struct drm_gem_object *obj,
>   	struct dma_buf_export_info exp_info = {
>   		.exp_name = KBUILD_MODNAME, /* white lie for debug */
>   		.owner = dev->driver->fops->owner,
> -		.ops = &drm_gem_prime_dmabuf_ops,
> +		.ops = drm_gem_prime_get_dma_buf_ops(dev),

Rather provide a new function drm_gem_prime_export_with_ops() that takes 
an additional dma_ops instance. The current drm_gem_prime_export() would 
call it with &drm_gem_prime_dmabuf_ops.

If this really does not work, you could add a pointer to dma_buf_ops to 
drm_gem_object_funcs and fetch that from drm_gem_prime_export(). We 
already vm_ops there.

Other drivers, such as amdgpu, would also benefit from such a change

>   		.size = obj->size,
>   		.flags = flags,
>   		.priv = obj,
> @@ -947,7 +953,7 @@ bool drm_gem_is_prime_exported_dma_buf(struct drm_device *dev,
>   {
>   	struct drm_gem_object *obj = dma_buf->priv;
>   
> -	return (dma_buf->ops == &drm_gem_prime_dmabuf_ops) && (obj->dev == dev);
> +	return dma_buf->ops == drm_gem_prime_get_dma_buf_ops(dev) && obj->dev == dev;

This is a bit more complicated and the test has been a pain point 
before. For this case, I think we should add a GEM callback for this

struct drm_gem_object_funcs {
     bool (*exported_by)(struct drm_gem_object *obj, struct drm_device *dev)
}

next to the existing export callback.

And drm_gem_is_prime_exported_dma_buf would then do

{
     if (obj->funcs->exported_by)
        return obj->funcs-<exported_by(obj, dev)

    return /* what we currently test */
}

IIRC amdgpu would again benefit from this.

These changes will isolate dma_buf handling near GEM code, keep 
drm_driver clean, and even allow for a driver to have different 
implementations of dma_buf_ops.

Best regards
Thomas

>   }
>   EXPORT_SYMBOL(drm_gem_is_prime_exported_dma_buf);
>   
> diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
> index 42fc085f986d..1c6dae60d523 100644
> --- a/include/drm/drm_drv.h
> +++ b/include/drm/drm_drv.h
> @@ -431,6 +431,14 @@ struct drm_driver {
>   	 * some examples.
>   	 */
>   	const struct file_operations *fops;
> +
> +	/**
> +	 * @dma_buf_ops:
> +	 *
> +	 * dma_buf_ops to use for buffers exported by this driver. When NULL,
> +	 * the drm_prime logic defaults to &drm_gem_prime_dmabuf_ops.
> +	 */
> +	const struct dma_buf_ops *dma_buf_ops;
>   };
>   
>   void *__devm_drm_dev_alloc(struct device *parent,

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)



  parent reply	other threads:[~2025-11-27  8:34 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-26 12:44 [PATCH v6 00/16] drm/panfrost, panthor: Cached maps and explicit flushing Boris Brezillon
2025-11-26 12:44 ` [PATCH v6 01/16] drm/prime: Simplify life of drivers needing custom dma_buf_ops Boris Brezillon
2025-11-26 16:20   ` Steven Price
2025-11-27  8:34   ` Thomas Zimmermann [this message]
2025-11-27  8:42     ` Thomas Zimmermann
2025-11-27  8:58       ` Thomas Zimmermann
2025-11-27 10:44         ` Boris Brezillon
2025-11-27 12:32           ` Boris Brezillon
2025-11-27 13:05             ` Boris Brezillon
2025-11-27 10:34       ` Boris Brezillon
2025-11-27 10:40     ` Boris Brezillon
2025-11-26 12:44 ` [PATCH v6 02/16] drm/shmem: Provide a generic {begin, end}_cpu_access() implementation Boris Brezillon
2025-11-26 16:23   ` [PATCH v6 02/16] drm/shmem: Provide a generic {begin,end}_cpu_access() implementation Steven Price
2025-11-26 12:44 ` [PATCH v6 03/16] drm/shmem: Add a drm_gem_shmem_sync() helper Boris Brezillon
2025-11-26 12:44 ` [PATCH v6 04/16] drm/panthor: Provide a custom dma_buf implementation Boris Brezillon
2025-11-26 12:44 ` [PATCH v6 05/16] drm/panthor: Fix panthor_gpu_coherency_set() Boris Brezillon
2025-11-26 12:44 ` [PATCH v6 06/16] drm/panthor: Expose the selected coherency protocol to the UMD Boris Brezillon
2025-11-26 13:25   ` Boris Brezillon
2025-11-26 12:44 ` [PATCH v6 07/16] drm/panthor: Add a PANTHOR_BO_SYNC ioctl Boris Brezillon
2025-11-26 16:25   ` Steven Price
2025-11-26 12:44 ` [PATCH v6 08/16] drm/panthor: Add an ioctl to query BO flags Boris Brezillon
2025-11-26 12:44 ` [PATCH v6 09/16] drm/panthor: Add flag to map GEM object Write-Back Cacheable Boris Brezillon
2025-11-26 16:27   ` Steven Price
2025-11-26 12:44 ` [PATCH v6 10/16] drm/panthor: Bump the driver version to 1.6 Boris Brezillon
2025-11-26 12:44 ` [PATCH v6 11/16] drm/panfrost: Provide a custom dma_buf implementation Boris Brezillon
2025-11-26 12:44 ` [PATCH v6 12/16] drm/panfrost: Expose the selected coherency protocol to the UMD Boris Brezillon
2025-11-26 12:44 ` [PATCH v6 13/16] drm/panfrost: Add a PANFROST_SYNC_BO ioctl Boris Brezillon
2025-11-26 16:47   ` Steven Price
2025-11-26 12:44 ` [PATCH v6 14/16] drm/panfrost: Add an ioctl to query BO flags Boris Brezillon
2025-11-26 12:44 ` [PATCH v6 15/16] drm/panfrost: Add flag to map GEM object Write-Back Cacheable Boris Brezillon
2025-11-26 12:44 ` [PATCH v6 16/16] drm/panfrost: Bump the driver version to 1.6 Boris Brezillon
2025-11-26 16:20 ` [PATCH v6 00/16] drm/panfrost,panthor: Cached maps and explicit flushing Thomas Zimmermann
2025-11-26 17:24   ` Boris Brezillon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2e789ff6-b79f-4577-bc69-f74dfed6acfa@suse.de \
    --to=tzimmermann@suse.de \
    --cc=abhinav.kumar@linux.dev \
    --cc=airlied@gmail.com \
    --cc=alexander.deucher@amd.com \
    --cc=amd-gfx@lists.freedesktop.org \
    --cc=boris.brezillon@collabora.com \
    --cc=christian.koenig@amd.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=faith.ekstrand@collabora.com \
    --cc=frank.binns@imgtec.com \
    --cc=jessica.zhang@oss.qualcomm.com \
    --cc=kernel@collabora.com \
    --cc=lucas.demarchi@intel.com \
    --cc=lumag@kernel.org \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=marijn.suijten@somainline.org \
    --cc=matt.coster@imgtec.com \
    --cc=mcanal@igalia.com \
    --cc=mperttunen@nvidia.com \
    --cc=mripard@kernel.org \
    --cc=mwen@igalia.com \
    --cc=robin.clark@oss.qualcomm.com \
    --cc=rodrigo.vivi@intel.com \
    --cc=sean@poorly.run \
    --cc=simona@ffwll.ch \
    --cc=steven.price@arm.com \
    --cc=thierry.reding@gmail.com \
    --cc=thomas.hellstrom@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox