Re: [PATCH 2/7] drm/panthor: Add flag to map GEM object Write-Back Cacheable

[Boris Brezillon <boris.brezillon@collabora.com>]

On Fri, 22 Aug 2025 10:29:11 -0400
Faith Ekstrand <faith@gfxstrand.net> wrote:

> From: Loïc Molinari <loic.molinari@collabora.com>
> 
> Signed-off-by: Loïc Molinari <loic.molinari@collabora.com>
> Signed-off-by: Faith Ekstrand <faith.ekstrand@collabora.com>
> ---
>  drivers/gpu/drm/panthor/panthor_drv.c |  7 ++++++-
>  drivers/gpu/drm/panthor/panthor_gem.c |  3 +++
>  include/uapi/drm/panthor_drm.h        | 10 ++++++++++
>  3 files changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/panthor/panthor_drv.c b/drivers/gpu/drm/panthor/panthor_drv.c
> index 1116f2d2826e..06ae6a2aeb16 100644
> --- a/drivers/gpu/drm/panthor/panthor_drv.c
> +++ b/drivers/gpu/drm/panthor/panthor_drv.c
> @@ -899,7 +899,8 @@ static int panthor_ioctl_vm_destroy(struct drm_device *ddev, void *data,
>  	return panthor_vm_pool_destroy_vm(pfile->vms, args->id);
>  }
>  
> -#define PANTHOR_BO_FLAGS		DRM_PANTHOR_BO_NO_MMAP
> +#define PANTHOR_BO_FLAGS		(DRM_PANTHOR_BO_NO_MMAP | \
> +					 DRM_PANTHOR_BO_WB_MMAP)
>  
>  static int panthor_ioctl_bo_create(struct drm_device *ddev, void *data,
>  				   struct drm_file *file)
> @@ -918,6 +919,10 @@ static int panthor_ioctl_bo_create(struct drm_device *ddev, void *data,
>  		goto out_dev_exit;
>  	}
>  
> +	if ((args->flags & DRM_PANTHOR_BO_NO_MMAP) &&
> +	    (args->flags & DRM_PANTHOR_BO_WB_MMAP))
> +		return -EINVAL;

I know it's obvious, but I'd still add a comment explaining why WB_MMAP
can't be set if NO_MMAP is.

> +
>  	if (args->exclusive_vm_id) {
>  		vm = panthor_vm_pool_get_vm(pfile->vms, args->exclusive_vm_id);
>  		if (!vm) {
> diff --git a/drivers/gpu/drm/panthor/panthor_gem.c b/drivers/gpu/drm/panthor/panthor_gem.c
> index a123bc740ba1..530bad12d545 100644
> --- a/drivers/gpu/drm/panthor/panthor_gem.c
> +++ b/drivers/gpu/drm/panthor/panthor_gem.c
> @@ -283,6 +283,9 @@ panthor_gem_create_with_handle(struct drm_file *file,
>  	bo = to_panthor_bo(&shmem->base);
>  	bo->flags = flags;
>  
> +	if (flags & DRM_PANTHOR_BO_WB_MMAP)
> +		shmem->map_wc = false;
> +
>  	if (exclusive_vm) {
>  		bo->exclusive_vm_root_gem = panthor_vm_root_gem(exclusive_vm);
>  		drm_gem_object_get(bo->exclusive_vm_root_gem);
> diff --git a/include/uapi/drm/panthor_drm.h b/include/uapi/drm/panthor_drm.h
> index e1f43deb7eca..bf47369c0220 100644
> --- a/include/uapi/drm/panthor_drm.h
> +++ b/include/uapi/drm/panthor_drm.h
> @@ -635,6 +635,16 @@ struct drm_panthor_vm_get_state {
>  enum drm_panthor_bo_flags {
>  	/** @DRM_PANTHOR_BO_NO_MMAP: The buffer object will never be CPU-mapped in userspace. */
>  	DRM_PANTHOR_BO_NO_MMAP = (1 << 0),
> +
> +	/**
> +	 *@DRM_PANTHOR_BO_WB_MMAP: Force "Write-Back Cacheable" CPU mapping.

          ^ missing space after '*'

> +	 *
> +	 * CPU map the buffer object in userspace by forcing the "Write-Back
> +	 * Cacheable" cacheability attribute. The mapping otherwise uses the
> +	 * "Non-Cacheable" attribute if the ACE-Lite coherency protocol isn't
> +	 * supported by the GPU.
> +	 */
> +	DRM_PANTHOR_BO_WB_MMAP = (1 << 1),

We probably want to expose the coherency caps before introducing this
flag (through some DEV_QUERY argument), so the UMD knows when it can
skip cache maintenance operations.

>  };
>  
>  /**