Re: [PATCH v2 12/13] drm/panfrost: Add flag to map GEM object Write-Back Cacheable

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

On Fri, 10 Oct 2025 16:00:45 +0100
Steven Price <steven.price@arm.com> wrote:

> On 10/10/2025 11:11, Boris Brezillon wrote:
> > From: Faith Ekstrand <faith.ekstrand@collabora.com>
> > 
> > Will be used by the UMD to optimize CPU accesses to buffers
> > that are frequently read by the CPU, or on which the access
> > pattern makes non-cacheable mappings inefficient.
> > 
> > Mapping buffers CPU-cached implies taking care of the CPU
> > cache maintenance in the UMD, unless the GPU is IO coherent.
> > 
> > v2:
> > - Add more to the commit message
> > 
> > Signed-off-by: Faith Ekstrand <faith.ekstrand@collabora.com>
> > Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>  
> 
> Should we not expose the PANFROST_BO_WB_MMAP flag through
> DRM_IOCTL_PANFROST_QUERY_BO_INFO? The documentation states that we
> return "Flags passed at creation time", but then this new flag isn't
> returned.

It was meant to be, I just got the test reversed in
panfrost_ioctl_query_bo_info().

> 
> Thanks,
> Steve
> 
> > ---
> >  drivers/gpu/drm/panfrost/panfrost_drv.c | 10 ++++++++--
> >  drivers/gpu/drm/panfrost/panfrost_gem.c |  3 +++
> >  include/uapi/drm/panfrost_drm.h         |  1 +
> >  3 files changed, 12 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c
> > index 00c0881fa2f0..0f51b1dc1abc 100644
> > --- a/drivers/gpu/drm/panfrost/panfrost_drv.c
> > +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
> > @@ -125,6 +125,10 @@ static int panfrost_ioctl_get_param(struct drm_device *ddev, void *data, struct
> >  	return 0;
> >  }
> >  
> > +#define PANFROST_BO_FLAGS	(PANFROST_BO_NOEXEC | \
> > +				 PANFROST_BO_HEAP | \
> > +				 PANFROST_BO_WB_MMAP)
> > +
> >  static int panfrost_ioctl_create_bo(struct drm_device *dev, void *data,
> >  		struct drm_file *file)
> >  {
> > @@ -134,8 +138,7 @@ static int panfrost_ioctl_create_bo(struct drm_device *dev, void *data,
> >  	struct panfrost_gem_mapping *mapping;
> >  	int ret;
> >  
> > -	if (!args->size || args->pad ||
> > -	    (args->flags & ~(PANFROST_BO_NOEXEC | PANFROST_BO_HEAP)))
> > +	if (!args->size || args->pad || (args->flags & ~PANFROST_BO_FLAGS))
> >  		return -EINVAL;
> >  
> >  	/* Heaps should never be executable */
> > @@ -661,6 +664,9 @@ static int panfrost_ioctl_query_bo_info(struct drm_device *dev, void *data,
> >  
> >  		if (bo->is_heap)
> >  			args->create_flags |= PANFROST_BO_HEAP;
> > +
> > +		if (bo->base.map_wc)
> > +			args->create_flags |= PANFROST_BO_WB_MMAP;

Oops, it's actually

		if (!bo->base.map_wc)
			args->create_flags |= PANFROST_BO_WB_MMAP;

> >  	}
> >  
> >  	drm_gem_object_put(gem_obj);
> > diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.c b/drivers/gpu/drm/panfrost/panfrost_gem.c
> > index da0362202d94..0e8028ee9d1f 100644
> > --- a/drivers/gpu/drm/panfrost/panfrost_gem.c
> > +++ b/drivers/gpu/drm/panfrost/panfrost_gem.c
> > @@ -320,6 +320,9 @@ panfrost_gem_create(struct drm_device *dev, size_t size, u32 flags)
> >  	bo->noexec = !!(flags & PANFROST_BO_NOEXEC);
> >  	bo->is_heap = !!(flags & PANFROST_BO_HEAP);
> >  
> > +	if (flags & PANFROST_BO_WB_MMAP)
> > +		bo->base.map_wc = false;
> > +
> >  	return bo;
> >  }
> >  
> > diff --git a/include/uapi/drm/panfrost_drm.h b/include/uapi/drm/panfrost_drm.h
> > index 5832a291adc4..2c1e28f9dd07 100644
> > --- a/include/uapi/drm/panfrost_drm.h
> > +++ b/include/uapi/drm/panfrost_drm.h
> > @@ -104,6 +104,7 @@ struct drm_panfrost_wait_bo {
> >  /* Valid flags to pass to drm_panfrost_create_bo */
> >  #define PANFROST_BO_NOEXEC	1
> >  #define PANFROST_BO_HEAP	2
> > +#define PANFROST_BO_WB_MMAP	4
> >  
> >  /**
> >   * struct drm_panfrost_create_bo - ioctl argument for creating Panfrost BOs.  
>