[PATCH 0/7] panfrost,panthor: Cached maps and explicit flushing

[PATCH 0/7] panfrost,panthor: Cached maps and explicit flushing

[Faith Ekstrand]

This series implements cached maps and explicit flushing for both panfrost
and panthor.  To avoid code/bug duplication, the tricky guts of the cache
flusing ioctl which walk the sg list are broken into a new common shmem
helper which can be used by any driver.

The PanVK MR to use this lives here:

https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/36385

Faith Ekstrand (6):
  drm/shmem: Add a drm_gem_shmem_sync_mmap() helper
  drm/panthor: Add a PANTHOR_BO_SYNC ioctl
  drm/panthor: Bump the driver version to 1.6
  drm/panfrost: Add flag to map GEM object Write-Back Cacheable
  drm/panfrost: Add a PANFROST_SYNC_BO ioctl
  drm/panfrost: Bump the driver version to 1.5

Loïc Molinari (1):
  drm/panthor: Add flag to map GEM object Write-Back Cacheable

 drivers/gpu/drm/drm_gem_shmem_helper.c  | 64 +++++++++++++++++++++++
 drivers/gpu/drm/panfrost/panfrost_drv.c | 67 +++++++++++++++++++++++--
 drivers/gpu/drm/panfrost/panfrost_gem.c | 23 +++++++++
 drivers/gpu/drm/panfrost/panfrost_gem.h |  2 +
 drivers/gpu/drm/panthor/panthor_drv.c   | 58 +++++++++++++++++++--
 drivers/gpu/drm/panthor/panthor_gem.c   | 23 +++++++++
 drivers/gpu/drm/panthor/panthor_gem.h   |  3 ++
 include/drm/drm_gem_shmem_helper.h      |  3 ++
 include/uapi/drm/panfrost_drm.h         | 46 +++++++++++++++++
 include/uapi/drm/panthor_drm.h          | 65 ++++++++++++++++++++++++
 10 files changed, 348 insertions(+), 6 deletions(-)

-- 
2.50.1

[PATCH 1/7] drm/shmem: Add a drm_gem_shmem_sync_mmap() helper

[Faith Ekstrand]

This enables syncing mapped GEM objects between the CPU and GPU via calls
to dma_sync_*().  It's a bit annoying as it requires walking the sg_table
so it's best if every driver doesn't hand-roll it.

Signed-off-by: Faith Ekstrand <faith.ekstrand@collabora.com>
---
 drivers/gpu/drm/drm_gem_shmem_helper.c | 64 ++++++++++++++++++++++++++
 include/drm/drm_gem_shmem_helper.h     |  3 ++
 2 files changed, 67 insertions(+)

diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c
index 8ac0b1fa5287..50907c1fa11f 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -658,6 +658,70 @@ int drm_gem_shmem_mmap(struct drm_gem_shmem_object *shmem, struct vm_area_struct
 }
 EXPORT_SYMBOL_GPL(drm_gem_shmem_mmap);
 
+/**
+ * drm_gem_shmem_sync_mmap - Sync memor-mapped data to/from the device
+ * @shmem: shmem GEM object
+ * @offset: Offset into the GEM object
+ * @size: Size of the area to sync
+ *
+ * Returns:
+ * 0 on success or a negative error code on failure.
+ */
+int
+drm_gem_shmem_sync_mmap(struct drm_gem_shmem_object *shmem,
+			size_t offset, size_t size,
+			enum dma_data_direction dir)
+{
+	struct drm_device *dev = shmem->base.dev;
+	struct sg_table *sgt;
+	struct scatterlist *sgl;
+	unsigned int count;
+
+	if (dir == DMA_NONE)
+		return 0;
+
+	/* Don't bother if it's WC-mapped */
+	if (shmem->map_wc)
+		return 0;
+
+	if (size == 0)
+		return 0;
+
+	if (offset + size < offset || offset + size > shmem->base.size)
+		return -EINVAL;
+
+	sgt = drm_gem_shmem_get_pages_sgt(shmem);
+	if (IS_ERR(sgt))
+		return PTR_ERR(sgt);
+
+	for_each_sgtable_dma_sg(sgt, sgl, count) {
+		if (size == 0)
+			break;
+
+		dma_addr_t paddr = sg_dma_address(sgl);
+		size_t len = sg_dma_len(sgl);
+
+		if (len <= offset) {
+			offset -= len;
+			continue;
+		}
+
+		paddr += offset;
+		len -= offset;
+		len = min_t(size_t, len, size);
+		size -= len;
+		offset = 0;
+
+		if (dir == DMA_FROM_DEVICE)
+			dma_sync_single_for_cpu(dev->dev, paddr, len, dir);
+		else
+			dma_sync_single_for_device(dev->dev, paddr, len, dir);
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(drm_gem_shmem_sync_mmap);
+
 /**
  * drm_gem_shmem_print_info() - Print &drm_gem_shmem_object info for debugfs
  * @shmem: shmem GEM object
diff --git a/include/drm/drm_gem_shmem_helper.h b/include/drm/drm_gem_shmem_helper.h
index 92f5db84b9c2..8e057a8e6c83 100644
--- a/include/drm/drm_gem_shmem_helper.h
+++ b/include/drm/drm_gem_shmem_helper.h
@@ -121,6 +121,9 @@ int drm_gem_shmem_vmap_locked(struct drm_gem_shmem_object *shmem,
 void drm_gem_shmem_vunmap_locked(struct drm_gem_shmem_object *shmem,
 				 struct iosys_map *map);
 int drm_gem_shmem_mmap(struct drm_gem_shmem_object *shmem, struct vm_area_struct *vma);
+int drm_gem_shmem_sync_mmap(struct drm_gem_shmem_object *shmem,
+			    size_t offset, size_t size,
+			    enum dma_data_direction dir);
 
 int drm_gem_shmem_pin_locked(struct drm_gem_shmem_object *shmem);
 void drm_gem_shmem_unpin_locked(struct drm_gem_shmem_object *shmem);
-- 
2.50.1

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

[Faith Ekstrand]

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;
+
 	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.
+	 *
+	 * 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),
 };
 
 /**
-- 
2.50.1

[PATCH 3/7] drm/panthor: Add a PANTHOR_BO_SYNC ioctl

[Faith Ekstrand]

Signed-off-by: Faith Ekstrand <faith.ekstrand@collabora.com>
---
 drivers/gpu/drm/panthor/panthor_drv.c | 47 ++++++++++++++++++++++-
 drivers/gpu/drm/panthor/panthor_gem.c | 20 ++++++++++
 drivers/gpu/drm/panthor/panthor_gem.h |  3 ++
 include/uapi/drm/panthor_drm.h        | 55 +++++++++++++++++++++++++++
 4 files changed, 124 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/panthor/panthor_drv.c b/drivers/gpu/drm/panthor/panthor_drv.c
index 06ae6a2aeb16..1527966604e1 100644
--- a/drivers/gpu/drm/panthor/panthor_drv.c
+++ b/drivers/gpu/drm/panthor/panthor_drv.c
@@ -175,7 +175,8 @@ panthor_get_uobj_array(const struct drm_panthor_obj_array *in, u32 min_stride,
 		 PANTHOR_UOBJ_DECL(struct drm_panthor_sync_op, timeline_value), \
 		 PANTHOR_UOBJ_DECL(struct drm_panthor_queue_submit, syncs), \
 		 PANTHOR_UOBJ_DECL(struct drm_panthor_queue_create, ringbuf_size), \
-		 PANTHOR_UOBJ_DECL(struct drm_panthor_vm_bind_op, syncs))
+		 PANTHOR_UOBJ_DECL(struct drm_panthor_vm_bind_op, syncs), \
+		 PANTHOR_UOBJ_DECL(struct drm_panthor_bo_sync_op, size))
 
 /**
  * PANTHOR_UOBJ_SET() - Copy a kernel object to a user object.
@@ -1398,6 +1399,49 @@ static int panthor_ioctl_set_user_mmio_offset(struct drm_device *ddev,
 	return 0;
 }
 
+#define PANTHOR_BO_SYNC_OP_FLAGS \
+	(DRM_PANTHOR_BO_SYNC_TO_DEVICE | \
+	 DRM_PANTHOR_BO_SYNC_FROM_DEVICE)
+
+static int panthor_ioctl_bo_sync(struct drm_device *ddev, void *data,
+				 struct drm_file *file)
+{
+	struct drm_panthor_bo_sync *args = data;
+	struct drm_panthor_bo_sync_op *ops;
+	struct drm_gem_object *obj;
+	int ret = 0;
+
+	ret = PANTHOR_UOBJ_GET_ARRAY(ops, &args->ops);
+	if (ret)
+		return ret;
+
+	for (u32 i = 0; i < args->ops.count; i++) {
+		if (ops[i].flags & ~PANTHOR_BO_SYNC_OP_FLAGS) {
+			ret = -EINVAL;
+			goto err_ops;
+		}
+
+		obj = drm_gem_object_lookup(file, ops[i].handle);
+		if (!obj) {
+			ret = -ENOENT;
+			goto err_ops;
+		}
+
+		ret = panthor_gem_bo_sync(obj, ops[i].flags,
+					  ops[i].offset, ops[i].size);
+
+		drm_gem_object_put(obj);
+
+		if (ret)
+			goto err_ops;
+	}
+
+err_ops:
+	kvfree(ops);
+
+	return ret;
+}
+
 static int
 panthor_open(struct drm_device *ddev, struct drm_file *file)
 {
@@ -1481,6 +1525,7 @@ static const struct drm_ioctl_desc panthor_drm_driver_ioctls[] = {
 	PANTHOR_IOCTL(GROUP_SUBMIT, group_submit, DRM_RENDER_ALLOW),
 	PANTHOR_IOCTL(BO_SET_LABEL, bo_set_label, DRM_RENDER_ALLOW),
 	PANTHOR_IOCTL(SET_USER_MMIO_OFFSET, set_user_mmio_offset, DRM_RENDER_ALLOW),
+	PANTHOR_IOCTL(BO_SYNC, bo_sync, DRM_RENDER_ALLOW),
 };
 
 static int panthor_mmap(struct file *filp, struct vm_area_struct *vma)
diff --git a/drivers/gpu/drm/panthor/panthor_gem.c b/drivers/gpu/drm/panthor/panthor_gem.c
index 530bad12d545..31f5d76002ec 100644
--- a/drivers/gpu/drm/panthor/panthor_gem.c
+++ b/drivers/gpu/drm/panthor/panthor_gem.c
@@ -344,6 +344,26 @@ panthor_gem_kernel_bo_set_label(struct panthor_kernel_bo *bo, const char *label)
 	panthor_gem_bo_set_label(bo->obj, str);
 }
 
+int
+panthor_gem_bo_sync(struct drm_gem_object *obj, u32 flags,
+		    u64 offset, u64 size)
+{
+	struct panthor_gem_object *bo = to_panthor_bo(obj);
+	enum dma_data_direction dir = DMA_NONE;
+
+	if ((flags & DRM_PANTHOR_BO_SYNC_TO_DEVICE) &&
+	    (flags & DRM_PANTHOR_BO_SYNC_FROM_DEVICE))
+		return -EINVAL;
+	else if (flags & DRM_PANTHOR_BO_SYNC_TO_DEVICE)
+		dir = DMA_TO_DEVICE;
+	else if (flags & DRM_PANTHOR_BO_SYNC_FROM_DEVICE)
+		dir = DMA_FROM_DEVICE;
+	else
+		return 0;
+
+	return drm_gem_shmem_sync_mmap(&bo->base, offset, size, dir);
+}
+
 #ifdef CONFIG_DEBUG_FS
 struct gem_size_totals {
 	size_t size;
diff --git a/drivers/gpu/drm/panthor/panthor_gem.h b/drivers/gpu/drm/panthor/panthor_gem.h
index 8fc7215e9b90..36fe392ee627 100644
--- a/drivers/gpu/drm/panthor/panthor_gem.h
+++ b/drivers/gpu/drm/panthor/panthor_gem.h
@@ -159,6 +159,9 @@ panthor_gem_create_with_handle(struct drm_file *file,
 void panthor_gem_bo_set_label(struct drm_gem_object *obj, const char *label);
 void panthor_gem_kernel_bo_set_label(struct panthor_kernel_bo *bo, const char *label);
 
+int panthor_gem_bo_sync(struct drm_gem_object *obj, u32 flags,
+			u64 offset, u64 size);
+
 static inline u64
 panthor_kernel_bo_gpuva(struct panthor_kernel_bo *bo)
 {
diff --git a/include/uapi/drm/panthor_drm.h b/include/uapi/drm/panthor_drm.h
index bf47369c0220..77b3ca5f6150 100644
--- a/include/uapi/drm/panthor_drm.h
+++ b/include/uapi/drm/panthor_drm.h
@@ -144,6 +144,9 @@ enum drm_panthor_ioctl_id {
 	 * pgoff_t size.
 	 */
 	DRM_PANTHOR_SET_USER_MMIO_OFFSET,
+
+	/** @DRM_PANTHOR_BO_SYNC: Sync BO data to/from the device */
+	DRM_PANTHOR_BO_SYNC,
 };
 
 /**
@@ -1047,6 +1050,56 @@ struct drm_panthor_set_user_mmio_offset {
 	__u64 offset;
 };
 
+/**
+ * enum drm_panthor_bo_sync_op_flags - BO sync flags
+ */
+enum drm_panthor_bo_sync_op_flags {
+	/**
+	 * @DRM_PANTHOR_BO_SYNC_TO_DEVICE: Sync data from the CPU to the
+	 * device.
+	 */
+	DRM_PANTHOR_BO_SYNC_TO_DEVICE = (1 << 0),
+
+	/**
+	 * @DRM_PANTHOR_BO_SYNC_TO_DEVICE: Sync data from the device to the
+	 * CPU.
+	 */
+	DRM_PANTHOR_BO_SYNC_FROM_DEVICE = (1 << 1),
+};
+
+/**
+ * struct drm_panthor_bo_sync_op - BO map sync op
+ */
+struct drm_panthor_bo_sync_op {
+	/** @handle: Handle of the buffer object to sync. */
+	__u32 handle;
+
+	/** @flags: Flags controlling the sync operation. */
+	__u32 flags;
+
+	/**
+	 * @offset: Offset into the BO at which the sync range starts.
+	 *
+	 * This will be rounded down to the nearest cache line as needed.
+	 */
+	__u64 offset;
+
+	/**
+	 * @size: Size of the range to sync
+	 *
+	 * @size + @offset will be rounded up to the nearest cache line as
+	 * needed.
+	 */
+	__u64 size;
+};
+
+struct drm_panthor_bo_sync {
+	/**
+	 * @ops: Array of struct drm_panthor_bo_sync_op sync operations.
+	 */
+	struct drm_panthor_obj_array ops;
+};
+
 /**
  * DRM_IOCTL_PANTHOR() - Build a Panthor IOCTL number
  * @__access: Access type. Must be R, W or RW.
@@ -1093,6 +1146,8 @@ enum {
 		DRM_IOCTL_PANTHOR(WR, BO_SET_LABEL, bo_set_label),
 	DRM_IOCTL_PANTHOR_SET_USER_MMIO_OFFSET =
 		DRM_IOCTL_PANTHOR(WR, SET_USER_MMIO_OFFSET, set_user_mmio_offset),
+	DRM_IOCTL_PANTHOR_BO_SYNC =
+		DRM_IOCTL_PANTHOR(WR, BO_SYNC, bo_sync),
 };
 
 #if defined(__cplusplus)
-- 
2.50.1

[PATCH 4/7] drm/panthor: Bump the driver version to 1.6

[Faith Ekstrand]

Signed-off-by: Faith Ekstrand <faith.ekstrand@collabora.com>
---
 drivers/gpu/drm/panthor/panthor_drv.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/panthor/panthor_drv.c b/drivers/gpu/drm/panthor/panthor_drv.c
index 1527966604e1..2da06057ce37 100644
--- a/drivers/gpu/drm/panthor/panthor_drv.c
+++ b/drivers/gpu/drm/panthor/panthor_drv.c
@@ -1658,6 +1658,8 @@ static void panthor_debugfs_init(struct drm_minor *minor)
  * - 1.3 - adds DRM_PANTHOR_GROUP_STATE_INNOCENT flag
  * - 1.4 - adds DRM_IOCTL_PANTHOR_BO_SET_LABEL ioctl
  * - 1.5 - adds DRM_PANTHOR_SET_USER_MMIO_OFFSET ioctl
+ * - 1.6 - adds DRM_PANTHOR_BO_WB_MMAP flag
+ *       - adds DRM_IOCTL_PANTHOR_BO_SYNC ioctl
  */
 static const struct drm_driver panthor_drm_driver = {
 	.driver_features = DRIVER_RENDER | DRIVER_GEM | DRIVER_SYNCOBJ |
@@ -1671,7 +1673,7 @@ static const struct drm_driver panthor_drm_driver = {
 	.name = "panthor",
 	.desc = "Panthor DRM driver",
 	.major = 1,
-	.minor = 5,
+	.minor = 6,
 
 	.gem_create_object = panthor_gem_create_object,
 	.gem_prime_import_sg_table = drm_gem_shmem_prime_import_sg_table,
-- 
2.50.1

[PATCH 5/7] drm/panfrost: Add flag to map GEM object Write-Back Cacheable

[Faith Ekstrand]

Signed-off-by: Faith Ekstrand <faith.ekstrand@collabora.com>
---
 drivers/gpu/drm/panfrost/panfrost_drv.c | 7 +++++--
 drivers/gpu/drm/panfrost/panfrost_gem.c | 3 +++
 include/uapi/drm/panfrost_drm.h         | 1 +
 3 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c
index 1ea6c509a5d5..ac2a3939f0c1 100644
--- a/drivers/gpu/drm/panfrost/panfrost_drv.c
+++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
@@ -116,6 +116,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)
 {
@@ -125,8 +129,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 */
diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.c b/drivers/gpu/drm/panfrost/panfrost_gem.c
index bb73f2a68a12..9a707055d946 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 = true;
+
 	return bo;
 }
 
diff --git a/include/uapi/drm/panfrost_drm.h b/include/uapi/drm/panfrost_drm.h
index ed67510395bd..e09b6f25acb2 100644
--- a/include/uapi/drm/panfrost_drm.h
+++ b/include/uapi/drm/panfrost_drm.h
@@ -90,6 +90,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.
-- 
2.50.1

[PATCH 6/7] drm/panfrost: Add a PANFROST_SYNC_BO ioctl

[Faith Ekstrand]

Signed-off-by: Faith Ekstrand <faith.ekstrand@collabora.com>
---
 drivers/gpu/drm/panfrost/panfrost_drv.c | 57 +++++++++++++++++++++++++
 drivers/gpu/drm/panfrost/panfrost_gem.c | 20 +++++++++
 drivers/gpu/drm/panfrost/panfrost_gem.h |  2 +
 include/uapi/drm/panfrost_drm.h         | 45 +++++++++++++++++++
 4 files changed, 124 insertions(+)

diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c
index ac2a3939f0c1..7d6e2b803a2b 100644
--- a/drivers/gpu/drm/panfrost/panfrost_drv.c
+++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
@@ -539,6 +539,62 @@ static int panfrost_ioctl_set_label_bo(struct drm_device *ddev, void *data,
 	return ret;
 }
 
+#define PANFROST_BO_SYNC_OP_FLAGS \
+	(PANFROST_BO_SYNC_TO_DEVICE | \
+	 PANFROST_BO_SYNC_FROM_DEVICE)
+
+static int panfrost_ioctl_sync_bo(struct drm_device *ddev, void *data,
+				  struct drm_file *file)
+{
+	struct drm_panfrost_sync_bo *args = data;
+	struct drm_panfrost_bo_sync_op *ops;
+	struct drm_gem_object *obj;
+	int ret;
+	u32 i;
+
+	if (args->pad)
+		return -EINVAL;
+
+	ops = kvmalloc_array(args->op_count, sizeof(*ops), GFP_KERNEL);
+	if (!ops) {
+		DRM_DEBUG("Failed to allocate incoming BO sync ops array\n");
+		return -ENOMEM;
+	}
+
+	if (copy_from_user(ops, (void __user *)(uintptr_t)args->ops,
+			   args->op_count * sizeof(*ops))) {
+		DRM_DEBUG("Failed to copy in BO sync ops\n");
+		ret = -EFAULT;
+		goto err_ops;
+	}
+
+	for (i = 0; i < args->op_count; i++) {
+		if (ops[i].flags & ~PANFROST_BO_SYNC_OP_FLAGS) {
+			ret = -EINVAL;
+			goto err_ops;
+		}
+
+		obj = drm_gem_object_lookup(file, ops[i].handle);
+		if (!obj) {
+			ret = -ENOENT;
+			goto err_ops;
+		}
+
+		ret = panfrost_gem_sync(obj, ops[i].flags,
+					ops[i].offset, ops[i].size);
+
+		drm_gem_object_put(obj);
+
+		if (ret)
+			goto err_ops;
+	}
+
+err_ops:
+	kvfree(ops);
+
+	return ret;
+}
+
 int panfrost_unstable_ioctl_check(void)
 {
 	if (!unstable_ioctls)
@@ -606,6 +662,7 @@ static const struct drm_ioctl_desc panfrost_drm_driver_ioctls[] = {
 	PANFROST_IOCTL(PERFCNT_DUMP,	perfcnt_dump,	DRM_RENDER_ALLOW),
 	PANFROST_IOCTL(MADVISE,		madvise,	DRM_RENDER_ALLOW),
 	PANFROST_IOCTL(SET_LABEL_BO,	set_label_bo,	DRM_RENDER_ALLOW),
+	PANFROST_IOCTL(SYNC_BO,		sync_bo,	DRM_RENDER_ALLOW),
 };
 
 static void panfrost_gpu_show_fdinfo(struct panfrost_device *pfdev,
diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.c b/drivers/gpu/drm/panfrost/panfrost_gem.c
index 9a707055d946..805a14145206 100644
--- a/drivers/gpu/drm/panfrost/panfrost_gem.c
+++ b/drivers/gpu/drm/panfrost/panfrost_gem.c
@@ -368,6 +368,26 @@ panfrost_gem_set_label(struct drm_gem_object *obj, const char *label)
 	kfree_const(old_label);
 }
 
+int
+panfrost_gem_sync(struct drm_gem_object *obj, u32 flags,
+		  u32 offset, u32 size)
+{
+	struct panfrost_gem_object *bo = to_panfrost_bo(obj);
+	enum dma_data_direction dir = DMA_NONE;
+
+	if ((flags & PANFROST_BO_SYNC_TO_DEVICE) &&
+	    (flags & PANFROST_BO_SYNC_FROM_DEVICE))
+		return -EINVAL;
+	else if (flags & PANFROST_BO_SYNC_TO_DEVICE)
+		dir = DMA_TO_DEVICE;
+	else if (flags & PANFROST_BO_SYNC_FROM_DEVICE)
+		dir = DMA_FROM_DEVICE;
+	else
+		return 0;
+
+	return drm_gem_shmem_sync_mmap(&bo->base, offset, size, dir);
+}
+
 void
 panfrost_gem_internal_set_label(struct drm_gem_object *obj, const char *label)
 {
diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.h b/drivers/gpu/drm/panfrost/panfrost_gem.h
index 8de3e76f2717..6a0e090aa59b 100644
--- a/drivers/gpu/drm/panfrost/panfrost_gem.h
+++ b/drivers/gpu/drm/panfrost/panfrost_gem.h
@@ -148,6 +148,8 @@ int panfrost_gem_shrinker_init(struct drm_device *dev);
 void panfrost_gem_shrinker_cleanup(struct drm_device *dev);
 
 void panfrost_gem_set_label(struct drm_gem_object *obj, const char *label);
+int panfrost_gem_sync(struct drm_gem_object *obj, u32 flags,
+		      u32 offset, u32 size);
 void panfrost_gem_internal_set_label(struct drm_gem_object *obj, const char *label);
 
 #ifdef CONFIG_DEBUG_FS
diff --git a/include/uapi/drm/panfrost_drm.h b/include/uapi/drm/panfrost_drm.h
index e09b6f25acb2..166535132455 100644
--- a/include/uapi/drm/panfrost_drm.h
+++ b/include/uapi/drm/panfrost_drm.h
@@ -22,6 +22,7 @@ extern "C" {
 #define DRM_PANFROST_PERFCNT_DUMP		0x07
 #define DRM_PANFROST_MADVISE			0x08
 #define DRM_PANFROST_SET_LABEL_BO		0x09
+#define DRM_PANFROST_SYNC_BO			0x0a
 
 #define DRM_IOCTL_PANFROST_SUBMIT		DRM_IOW(DRM_COMMAND_BASE + DRM_PANFROST_SUBMIT, struct drm_panfrost_submit)
 #define DRM_IOCTL_PANFROST_WAIT_BO		DRM_IOW(DRM_COMMAND_BASE + DRM_PANFROST_WAIT_BO, struct drm_panfrost_wait_bo)
@@ -31,6 +32,7 @@ extern "C" {
 #define DRM_IOCTL_PANFROST_GET_BO_OFFSET	DRM_IOWR(DRM_COMMAND_BASE + DRM_PANFROST_GET_BO_OFFSET, struct drm_panfrost_get_bo_offset)
 #define DRM_IOCTL_PANFROST_MADVISE		DRM_IOWR(DRM_COMMAND_BASE + DRM_PANFROST_MADVISE, struct drm_panfrost_madvise)
 #define DRM_IOCTL_PANFROST_SET_LABEL_BO		DRM_IOWR(DRM_COMMAND_BASE + DRM_PANFROST_SET_LABEL_BO, struct drm_panfrost_set_label_bo)
+#define DRM_IOCTL_PANFROST_SYNC_BO		DRM_IOWR(DRM_COMMAND_BASE + DRM_PANFROST_SET_LABEL_BO, struct drm_panfrost_sync_bo)
 
 /*
  * Unstable ioctl(s): only exposed when the unsafe unstable_ioctls module
@@ -249,6 +251,49 @@ struct drm_panfrost_set_label_bo {
 	__u64 label;
 };
 
+/* Valid flags to pass to drm_panfrost_bo_sync_op */
+#define PANFROST_BO_SYNC_TO_DEVICE	1
+#define PANFROST_BO_SYNC_FROM_DEVICE	2
+
+/**
+ * struct drm_panthor_bo_flush_map_op - BO map sync op
+ */
+struct drm_panfrost_bo_sync_op {
+	/** @handle: Handle of the buffer object to sync. */
+	__u32 handle;
+
+	/** @flags: Flags controlling the sync operation. */
+	__u32 flags;
+
+	/**
+	 * @offset: Offset into the BO at which the sync range starts.
+	 *
+	 * This will be rounded down to the nearest cache line as needed.
+	 */
+	__u32 offset;
+
+	/**
+	 * @size: Size of the range to sync
+	 *
+	 * @size + @offset will be rounded up to the nearest cache line as
+	 * needed.
+	 */
+	__u32 size;
+};
+
+/**
+ * struct drm_panfrost_sync_bo - ioctl argument for syncing BO maps
+ */
+struct drm_panfrost_sync_bo {
+	/** Array of struct drm_panfrost_bo_sync_op */
+	__u64 ops;
+
+	/** Number of BO sync ops */
+	__u32 op_count;
+
+	__u32 pad;
+};
+
 /* Definitions for coredump decoding in user space */
 #define PANFROSTDUMP_MAJOR 1
 #define PANFROSTDUMP_MINOR 0
-- 
2.50.1

[PATCH 7/7] drm/panfrost: Bump the driver version to 1.5

[Faith Ekstrand]

Signed-off-by: Faith Ekstrand <faith.ekstrand@collabora.com>
---
 drivers/gpu/drm/panfrost/panfrost_drv.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c
index 7d6e2b803a2b..8b841c6c8150 100644
--- a/drivers/gpu/drm/panfrost/panfrost_drv.c
+++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
@@ -759,6 +759,7 @@ static void panfrost_debugfs_init(struct drm_minor *minor)
  * - 1.3 - adds JD_REQ_CYCLE_COUNT job requirement for SUBMIT
  *       - adds SYSTEM_TIMESTAMP and SYSTEM_TIMESTAMP_FREQUENCY queries
  * - 1.4 - adds SET_LABEL_BO
+ * - 1.5 - adds PANFROST_BO_MAP_WB and PANFROST_IOCTL_SYNC_BO
  */
 static const struct drm_driver panfrost_drm_driver = {
 	.driver_features	= DRIVER_RENDER | DRIVER_GEM | DRIVER_SYNCOBJ,
@@ -771,7 +772,7 @@ static const struct drm_driver panfrost_drm_driver = {
 	.name			= "panfrost",
 	.desc			= "panfrost DRM",
 	.major			= 1,
-	.minor			= 4,
+	.minor			= 5,
 
 	.gem_create_object	= panfrost_gem_create_object,
 	.gem_prime_import_sg_table = panfrost_gem_prime_import_sg_table,
-- 
2.50.1

Re: [PATCH 0/7] panfrost,panthor: Cached maps and explicit flushing

[Boris Brezillon]

+panthor/panfrost maintainers/devs

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

> This series implements cached maps and explicit flushing for both panfrost
> and panthor.  To avoid code/bug duplication, the tricky guts of the cache
> flusing ioctl which walk the sg list are broken into a new common shmem
> helper which can be used by any driver.
> 
> The PanVK MR to use this lives here:
> 
> https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/36385
> 
> Faith Ekstrand (6):
>   drm/shmem: Add a drm_gem_shmem_sync_mmap() helper
>   drm/panthor: Add a PANTHOR_BO_SYNC ioctl
>   drm/panthor: Bump the driver version to 1.6
>   drm/panfrost: Add flag to map GEM object Write-Back Cacheable
>   drm/panfrost: Add a PANFROST_SYNC_BO ioctl
>   drm/panfrost: Bump the driver version to 1.5
> 
> Loïc Molinari (1):
>   drm/panthor: Add flag to map GEM object Write-Back Cacheable
> 
>  drivers/gpu/drm/drm_gem_shmem_helper.c  | 64 +++++++++++++++++++++++
>  drivers/gpu/drm/panfrost/panfrost_drv.c | 67 +++++++++++++++++++++++--
>  drivers/gpu/drm/panfrost/panfrost_gem.c | 23 +++++++++
>  drivers/gpu/drm/panfrost/panfrost_gem.h |  2 +
>  drivers/gpu/drm/panthor/panthor_drv.c   | 58 +++++++++++++++++++--
>  drivers/gpu/drm/panthor/panthor_gem.c   | 23 +++++++++
>  drivers/gpu/drm/panthor/panthor_gem.h   |  3 ++
>  include/drm/drm_gem_shmem_helper.h      |  3 ++
>  include/uapi/drm/panfrost_drm.h         | 46 +++++++++++++++++
>  include/uapi/drm/panthor_drm.h          | 65 ++++++++++++++++++++++++
>  10 files changed, 348 insertions(+), 6 deletions(-)
> 

Re: [PATCH 1/7] drm/shmem: Add a drm_gem_shmem_sync_mmap() helper

[Boris Brezillon]

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

> This enables syncing mapped GEM objects between the CPU and GPU via calls
> to dma_sync_*().  It's a bit annoying as it requires walking the sg_table
> so it's best if every driver doesn't hand-roll it.
> 
> Signed-off-by: Faith Ekstrand <faith.ekstrand@collabora.com>
> ---
>  drivers/gpu/drm/drm_gem_shmem_helper.c | 64 ++++++++++++++++++++++++++
>  include/drm/drm_gem_shmem_helper.h     |  3 ++
>  2 files changed, 67 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c
> index 8ac0b1fa5287..50907c1fa11f 100644
> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
> @@ -658,6 +658,70 @@ int drm_gem_shmem_mmap(struct drm_gem_shmem_object *shmem, struct vm_area_struct
>  }
>  EXPORT_SYMBOL_GPL(drm_gem_shmem_mmap);
>  
> +/**
> + * drm_gem_shmem_sync_mmap - Sync memor-mapped data to/from the device

                                     ^ CPU-mapped

> + * @shmem: shmem GEM object
> + * @offset: Offset into the GEM object
> + * @size: Size of the area to sync
> + *
> + * Returns:
> + * 0 on success or a negative error code on failure.
> + */
> +int
> +drm_gem_shmem_sync_mmap(struct drm_gem_shmem_object *shmem,
> +			size_t offset, size_t size,
> +			enum dma_data_direction dir)

Feels weird to name this function sync_mmap when all it's being passed
is a region is a GEM, not a virtual address in the process address
space. I can also imagine this helper being used for kernel-mapped BOs,
making _mmap a bit of a lie. Maybe we should just go for _sync() or
_sync_cpu(). 

> +{
> +	struct drm_device *dev = shmem->base.dev;
> +	struct sg_table *sgt;
> +	struct scatterlist *sgl;
> +	unsigned int count;
> +
> +	if (dir == DMA_NONE)
> +		return 0;
> +
> +	/* Don't bother if it's WC-mapped */
> +	if (shmem->map_wc)
> +		return 0;
> +
> +	if (size == 0)
> +		return 0;
> +
> +	if (offset + size < offset || offset + size > shmem->base.size)
> +		return -EINVAL;
> +
> +	sgt = drm_gem_shmem_get_pages_sgt(shmem);
> +	if (IS_ERR(sgt))
> +		return PTR_ERR(sgt);
> +
> +	for_each_sgtable_dma_sg(sgt, sgl, count) {
> +		if (size == 0)
> +			break;
> +
> +		dma_addr_t paddr = sg_dma_address(sgl);
> +		size_t len = sg_dma_len(sgl);
> +
> +		if (len <= offset) {
> +			offset -= len;
> +			continue;
> +		}
> +
> +		paddr += offset;
> +		len -= offset;
> +		len = min_t(size_t, len, size);
> +		size -= len;
> +		offset = 0;
> +
> +		if (dir == DMA_FROM_DEVICE)
> +			dma_sync_single_for_cpu(dev->dev, paddr, len, dir);
> +		else
> +			dma_sync_single_for_device(dev->dev, paddr, len, dir);
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(drm_gem_shmem_sync_mmap);
> +
>  /**
>   * drm_gem_shmem_print_info() - Print &drm_gem_shmem_object info for debugfs
>   * @shmem: shmem GEM object
> diff --git a/include/drm/drm_gem_shmem_helper.h b/include/drm/drm_gem_shmem_helper.h
> index 92f5db84b9c2..8e057a8e6c83 100644
> --- a/include/drm/drm_gem_shmem_helper.h
> +++ b/include/drm/drm_gem_shmem_helper.h
> @@ -121,6 +121,9 @@ int drm_gem_shmem_vmap_locked(struct drm_gem_shmem_object *shmem,
>  void drm_gem_shmem_vunmap_locked(struct drm_gem_shmem_object *shmem,
>  				 struct iosys_map *map);
>  int drm_gem_shmem_mmap(struct drm_gem_shmem_object *shmem, struct vm_area_struct *vma);
> +int drm_gem_shmem_sync_mmap(struct drm_gem_shmem_object *shmem,
> +			    size_t offset, size_t size,
> +			    enum dma_data_direction dir);
>  
>  int drm_gem_shmem_pin_locked(struct drm_gem_shmem_object *shmem);
>  void drm_gem_shmem_unpin_locked(struct drm_gem_shmem_object *shmem);

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

[Boris Brezillon]

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.

>  };
>  
>  /**

Re: [PATCH 3/7] drm/panthor: Add a PANTHOR_BO_SYNC ioctl

[Boris Brezillon]

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

It probably deserve a quick description of why this is needed (same for
the previous commit BTW).

> Signed-off-by: Faith Ekstrand <faith.ekstrand@collabora.com>
> ---
>  drivers/gpu/drm/panthor/panthor_drv.c | 47 ++++++++++++++++++++++-
>  drivers/gpu/drm/panthor/panthor_gem.c | 20 ++++++++++
>  drivers/gpu/drm/panthor/panthor_gem.h |  3 ++
>  include/uapi/drm/panthor_drm.h        | 55 +++++++++++++++++++++++++++
>  4 files changed, 124 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/panthor/panthor_drv.c b/drivers/gpu/drm/panthor/panthor_drv.c
> index 06ae6a2aeb16..1527966604e1 100644
> --- a/drivers/gpu/drm/panthor/panthor_drv.c
> +++ b/drivers/gpu/drm/panthor/panthor_drv.c
> @@ -175,7 +175,8 @@ panthor_get_uobj_array(const struct drm_panthor_obj_array *in, u32 min_stride,
>  		 PANTHOR_UOBJ_DECL(struct drm_panthor_sync_op, timeline_value), \
>  		 PANTHOR_UOBJ_DECL(struct drm_panthor_queue_submit, syncs), \
>  		 PANTHOR_UOBJ_DECL(struct drm_panthor_queue_create, ringbuf_size), \
> -		 PANTHOR_UOBJ_DECL(struct drm_panthor_vm_bind_op, syncs))
> +		 PANTHOR_UOBJ_DECL(struct drm_panthor_vm_bind_op, syncs), \
> +		 PANTHOR_UOBJ_DECL(struct drm_panthor_bo_sync_op, size))
>  
>  /**
>   * PANTHOR_UOBJ_SET() - Copy a kernel object to a user object.
> @@ -1398,6 +1399,49 @@ static int panthor_ioctl_set_user_mmio_offset(struct drm_device *ddev,
>  	return 0;
>  }
>  
> +#define PANTHOR_BO_SYNC_OP_FLAGS \
> +	(DRM_PANTHOR_BO_SYNC_TO_DEVICE | \
> +	 DRM_PANTHOR_BO_SYNC_FROM_DEVICE)
> +
> +static int panthor_ioctl_bo_sync(struct drm_device *ddev, void *data,
> +				 struct drm_file *file)
> +{
> +	struct drm_panthor_bo_sync *args = data;
> +	struct drm_panthor_bo_sync_op *ops;
> +	struct drm_gem_object *obj;
> +	int ret = 0;
> +
> +	ret = PANTHOR_UOBJ_GET_ARRAY(ops, &args->ops);
> +	if (ret)
> +		return ret;
> +
> +	for (u32 i = 0; i < args->ops.count; i++) {
> +		if (ops[i].flags & ~PANTHOR_BO_SYNC_OP_FLAGS) {
> +			ret = -EINVAL;
> +			goto err_ops;
> +		}
> +
> +		obj = drm_gem_object_lookup(file, ops[i].handle);
> +		if (!obj) {
> +			ret = -ENOENT;
> +			goto err_ops;
> +		}
> +
> +		ret = panthor_gem_bo_sync(obj, ops[i].flags,
> +					  ops[i].offset, ops[i].size);
> +
> +		drm_gem_object_put(obj);
> +
> +		if (ret)
> +			goto err_ops;
> +	}
> +
> +err_ops:
> +	kvfree(ops);
> +
> +	return ret;
> +}
> +
>  static int
>  panthor_open(struct drm_device *ddev, struct drm_file *file)
>  {
> @@ -1481,6 +1525,7 @@ static const struct drm_ioctl_desc panthor_drm_driver_ioctls[] = {
>  	PANTHOR_IOCTL(GROUP_SUBMIT, group_submit, DRM_RENDER_ALLOW),
>  	PANTHOR_IOCTL(BO_SET_LABEL, bo_set_label, DRM_RENDER_ALLOW),
>  	PANTHOR_IOCTL(SET_USER_MMIO_OFFSET, set_user_mmio_offset, DRM_RENDER_ALLOW),
> +	PANTHOR_IOCTL(BO_SYNC, bo_sync, DRM_RENDER_ALLOW),
>  };
>  
>  static int panthor_mmap(struct file *filp, struct vm_area_struct *vma)
> diff --git a/drivers/gpu/drm/panthor/panthor_gem.c b/drivers/gpu/drm/panthor/panthor_gem.c
> index 530bad12d545..31f5d76002ec 100644
> --- a/drivers/gpu/drm/panthor/panthor_gem.c
> +++ b/drivers/gpu/drm/panthor/panthor_gem.c
> @@ -344,6 +344,26 @@ panthor_gem_kernel_bo_set_label(struct panthor_kernel_bo *bo, const char *label)
>  	panthor_gem_bo_set_label(bo->obj, str);
>  }
>  
> +int
> +panthor_gem_bo_sync(struct drm_gem_object *obj, u32 flags,
> +		    u64 offset, u64 size)
> +{
> +	struct panthor_gem_object *bo = to_panthor_bo(obj);
> +	enum dma_data_direction dir = DMA_NONE;
> +
> +	if ((flags & DRM_PANTHOR_BO_SYNC_TO_DEVICE) &&
> +	    (flags & DRM_PANTHOR_BO_SYNC_FROM_DEVICE))
> +		return -EINVAL;
> +	else if (flags & DRM_PANTHOR_BO_SYNC_TO_DEVICE)
> +		dir = DMA_TO_DEVICE;
> +	else if (flags & DRM_PANTHOR_BO_SYNC_FROM_DEVICE)
> +		dir = DMA_FROM_DEVICE;
> +	else
> +		return 0;
> +
> +	return drm_gem_shmem_sync_mmap(&bo->base, offset, size, dir);
> +}
> +
>  #ifdef CONFIG_DEBUG_FS
>  struct gem_size_totals {
>  	size_t size;
> diff --git a/drivers/gpu/drm/panthor/panthor_gem.h b/drivers/gpu/drm/panthor/panthor_gem.h
> index 8fc7215e9b90..36fe392ee627 100644
> --- a/drivers/gpu/drm/panthor/panthor_gem.h
> +++ b/drivers/gpu/drm/panthor/panthor_gem.h
> @@ -159,6 +159,9 @@ panthor_gem_create_with_handle(struct drm_file *file,
>  void panthor_gem_bo_set_label(struct drm_gem_object *obj, const char *label);
>  void panthor_gem_kernel_bo_set_label(struct panthor_kernel_bo *bo, const char *label);
>  
> +int panthor_gem_bo_sync(struct drm_gem_object *obj, u32 flags,
> +			u64 offset, u64 size);
> +
>  static inline u64
>  panthor_kernel_bo_gpuva(struct panthor_kernel_bo *bo)
>  {
> diff --git a/include/uapi/drm/panthor_drm.h b/include/uapi/drm/panthor_drm.h
> index bf47369c0220..77b3ca5f6150 100644
> --- a/include/uapi/drm/panthor_drm.h
> +++ b/include/uapi/drm/panthor_drm.h
> @@ -144,6 +144,9 @@ enum drm_panthor_ioctl_id {
>  	 * pgoff_t size.
>  	 */
>  	DRM_PANTHOR_SET_USER_MMIO_OFFSET,
> +
> +	/** @DRM_PANTHOR_BO_SYNC: Sync BO data to/from the device */
> +	DRM_PANTHOR_BO_SYNC,
>  };
>  
>  /**
> @@ -1047,6 +1050,56 @@ struct drm_panthor_set_user_mmio_offset {
>  	__u64 offset;
>  };
>  
> +/**
> + * enum drm_panthor_bo_sync_op_flags - BO sync flags
> + */
> +enum drm_panthor_bo_sync_op_flags {
> +	/**
> +	 * @DRM_PANTHOR_BO_SYNC_TO_DEVICE: Sync data from the CPU to the
> +	 * device.
> +	 */
> +	DRM_PANTHOR_BO_SYNC_TO_DEVICE = (1 << 0),
> +
> +	/**
> +	 * @DRM_PANTHOR_BO_SYNC_TO_DEVICE: Sync data from the device to the
> +	 * CPU.
> +	 */
> +	DRM_PANTHOR_BO_SYNC_FROM_DEVICE = (1 << 1),
> +};
> +
> +/**
> + * struct drm_panthor_bo_sync_op - BO map sync op
> + */
> +struct drm_panthor_bo_sync_op {
> +	/** @handle: Handle of the buffer object to sync. */
> +	__u32 handle;
> +
> +	/** @flags: Flags controlling the sync operation. */

We should probably mention the constraints on flags (TO or FROM_DEVICE,
but not both at the same time).

> +	__u32 flags;
> +
> +	/**
> +	 * @offset: Offset into the BO at which the sync range starts.
> +	 *
> +	 * This will be rounded down to the nearest cache line as needed.
> +	 */
> +	__u64 offset;
> +
> +	/**
> +	 * @size: Size of the range to sync
> +	 *
> +	 * @size + @offset will be rounded up to the nearest cache line as
> +	 * needed.
> +	 */
> +	__u64 size;
> +};
> +
> +struct drm_panthor_bo_sync {
> +	/**
> +	 * @ops: Array of struct drm_panthor_bo_sync_op sync operations.
> +	 */
> +	struct drm_panthor_obj_array ops;
> +};
> +
>  /**
>   * DRM_IOCTL_PANTHOR() - Build a Panthor IOCTL number
>   * @__access: Access type. Must be R, W or RW.
> @@ -1093,6 +1146,8 @@ enum {
>  		DRM_IOCTL_PANTHOR(WR, BO_SET_LABEL, bo_set_label),
>  	DRM_IOCTL_PANTHOR_SET_USER_MMIO_OFFSET =
>  		DRM_IOCTL_PANTHOR(WR, SET_USER_MMIO_OFFSET, set_user_mmio_offset),
> +	DRM_IOCTL_PANTHOR_BO_SYNC =
> +		DRM_IOCTL_PANTHOR(WR, BO_SYNC, bo_sync),
>  };
>  
>  #if defined(__cplusplus)

Re: [PATCH 1/7] drm/shmem: Add a drm_gem_shmem_sync_mmap() helper

[kernel test robot]

Hi Faith,

kernel test robot noticed the following build warnings:

[auto build test WARNING on drm-exynos/exynos-drm-next]
[also build test WARNING on linus/master v6.17-rc2 next-20250822]
[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/Faith-Ekstrand/drm-shmem-Add-a-drm_gem_shmem_sync_mmap-helper/20250822-223306
base:   https://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git exynos-drm-next
patch link:    https://lore.kernel.org/r/20250822142954.902402-2-faith.ekstrand%40collabora.com
patch subject: [PATCH 1/7] drm/shmem: Add a drm_gem_shmem_sync_mmap() helper
config: x86_64-buildonly-randconfig-003-20250823 (https://download.01.org/0day-ci/archive/20250823/202508231334.pWN5ScdS-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250823/202508231334.pWN5ScdS-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/202508231334.pWN5ScdS-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> Warning: drivers/gpu/drm/drm_gem_shmem_helper.c:673 function parameter 'dir' not described in 'drm_gem_shmem_sync_mmap'

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 6/7] drm/panfrost: Add a PANFROST_SYNC_BO ioctl

[kernel test robot]

Hi Faith,

kernel test robot noticed the following build warnings:

[auto build test WARNING on drm-exynos/exynos-drm-next]
[also build test WARNING on linus/master v6.17-rc2 next-20250822]
[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/Faith-Ekstrand/drm-shmem-Add-a-drm_gem_shmem_sync_mmap-helper/20250822-223306
base:   https://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git exynos-drm-next
patch link:    https://lore.kernel.org/r/20250822142954.902402-7-faith.ekstrand%40collabora.com
patch subject: [PATCH 6/7] drm/panfrost: Add a PANFROST_SYNC_BO ioctl
config: um-randconfig-r133-20250824 (https://download.01.org/0day-ci/archive/20250824/202508241157.XlCVeP0d-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14+deb12u1) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250824/202508241157.XlCVeP0d-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/202508241157.XlCVeP0d-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/gpu/drm/panfrost/panfrost_drv.c:664:9: sparse: sparse: Initializer entry defined twice
   drivers/gpu/drm/panfrost/panfrost_drv.c:665:9: sparse:   also defined here

vim +664 drivers/gpu/drm/panfrost/panfrost_drv.c

f3ba91228e8e917 Rob Herring     2018-09-10  650  
f3ba91228e8e917 Rob Herring     2018-09-10  651  static const struct drm_ioctl_desc panfrost_drm_driver_ioctls[] = {
f3ba91228e8e917 Rob Herring     2018-09-10  652  #define PANFROST_IOCTL(n, func, flags) \
f3ba91228e8e917 Rob Herring     2018-09-10  653  	DRM_IOCTL_DEF_DRV(PANFROST_##n, panfrost_ioctl_##func, flags)
f3ba91228e8e917 Rob Herring     2018-09-10  654  
c1572b756066235 Emil Velikov    2019-11-01  655  	PANFROST_IOCTL(SUBMIT,		submit,		DRM_RENDER_ALLOW),
f3ba91228e8e917 Rob Herring     2018-09-10  656  	PANFROST_IOCTL(WAIT_BO,		wait_bo,	DRM_RENDER_ALLOW),
f3ba91228e8e917 Rob Herring     2018-09-10  657  	PANFROST_IOCTL(CREATE_BO,	create_bo,	DRM_RENDER_ALLOW),
f3ba91228e8e917 Rob Herring     2018-09-10  658  	PANFROST_IOCTL(MMAP_BO,		mmap_bo,	DRM_RENDER_ALLOW),
f3ba91228e8e917 Rob Herring     2018-09-10  659  	PANFROST_IOCTL(GET_PARAM,	get_param,	DRM_RENDER_ALLOW),
f3ba91228e8e917 Rob Herring     2018-09-10  660  	PANFROST_IOCTL(GET_BO_OFFSET,	get_bo_offset,	DRM_RENDER_ALLOW),
7786fd1087774c6 Boris Brezillon 2019-06-18  661  	PANFROST_IOCTL(PERFCNT_ENABLE,	perfcnt_enable,	DRM_RENDER_ALLOW),
7786fd1087774c6 Boris Brezillon 2019-06-18  662  	PANFROST_IOCTL(PERFCNT_DUMP,	perfcnt_dump,	DRM_RENDER_ALLOW),
013b6510131568c Rob Herring     2019-08-05  663  	PANFROST_IOCTL(MADVISE,		madvise,	DRM_RENDER_ALLOW),
2f684bbbcb27048 Adrián Larumbe  2025-05-20 @664  	PANFROST_IOCTL(SET_LABEL_BO,	set_label_bo,	DRM_RENDER_ALLOW),
0f6b6cfee96633e Faith Ekstrand  2025-08-22  665  	PANFROST_IOCTL(SYNC_BO,		sync_bo,	DRM_RENDER_ALLOW),
f3ba91228e8e917 Rob Herring     2018-09-10  666  };
f3ba91228e8e917 Rob Herring     2018-09-10  667  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 1/7] drm/shmem: Add a drm_gem_shmem_sync_mmap() helper

[Adrian Larumbe]

Hi Faith,

On 22.08.2025 10:29, Faith Ekstrand wrote:
> This enables syncing mapped GEM objects between the CPU and GPU via calls
> to dma_sync_*().  It's a bit annoying as it requires walking the sg_table
> so it's best if every driver doesn't hand-roll it.
>
> Signed-off-by: Faith Ekstrand <faith.ekstrand@collabora.com>
> ---
>  drivers/gpu/drm/drm_gem_shmem_helper.c | 64 ++++++++++++++++++++++++++
>  include/drm/drm_gem_shmem_helper.h     |  3 ++
>  2 files changed, 67 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c
> index 8ac0b1fa5287..50907c1fa11f 100644
> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
> @@ -658,6 +658,70 @@ int drm_gem_shmem_mmap(struct drm_gem_shmem_object *shmem, struct vm_area_struct
>  }
>  EXPORT_SYMBOL_GPL(drm_gem_shmem_mmap);
>
> +/**
> + * drm_gem_shmem_sync_mmap - Sync memor-mapped data to/from the device
> + * @shmem: shmem GEM object
> + * @offset: Offset into the GEM object
> + * @size: Size of the area to sync
> + *
> + * Returns:
> + * 0 on success or a negative error code on failure.
> + */
> +int
> +drm_gem_shmem_sync_mmap(struct drm_gem_shmem_object *shmem,
> +			size_t offset, size_t size,
> +			enum dma_data_direction dir)
> +{
> +	struct drm_device *dev = shmem->base.dev;
> +	struct sg_table *sgt;
> +	struct scatterlist *sgl;
> +	unsigned int count;
> +
> +	if (dir == DMA_NONE)
> +		return 0;
> +
> +	/* Don't bother if it's WC-mapped */
> +	if (shmem->map_wc)
> +		return 0;
> +
> +	if (size == 0)
> +		return 0;
> +
> +	if (offset + size < offset || offset + size > shmem->base.size)
> +		return -EINVAL;
> +
> +	sgt = drm_gem_shmem_get_pages_sgt(shmem);
> +	if (IS_ERR(sgt))
> +		return PTR_ERR(sgt);

This will allocate pages when the BO had no backing storage yet, otherwise it'll increase the
refcnt on those pages, but seems to me this will leave the reference count imbalanced.

I'd say running this function on an object with no storage backing should be considered a no-op:

```
	if (!shmem->pages)
		return 0;
```

On top of that, there are code paths in which we allocate pages, but produce no sgtable for them,
like drm_gem_shmem_mmap(), so maybe we could do as follows:

```
	if (!shmem->sgt) {
		int ret;

		ret = dma_resv_lock_interruptible(shmem->base.resv, NULL);
		if (ret)
			return ret;

		sgt = drm_gem_shmem_get_sg_table(shmem);
		if (IS_ERR(sgt))
			return ret;

		shmem->sgt = sgt;
	}
```

Although I think this is something you could factorise with what is being done in drm_gem_shmem_get_pages_sgt_locked.

> +	for_each_sgtable_dma_sg(sgt, sgl, count) {
> +		if (size == 0)
> +			break;
> +
> +		dma_addr_t paddr = sg_dma_address(sgl);
> +		size_t len = sg_dma_len(sgl);
> +
> +		if (len <= offset) {
> +			offset -= len;
> +			continue;
> +		}
> +
> +		paddr += offset;
> +		len -= offset;
> +		len = min_t(size_t, len, size);
> +		size -= len;
> +		offset = 0;
> +
> +		if (dir == DMA_FROM_DEVICE)
> +			dma_sync_single_for_cpu(dev->dev, paddr, len, dir);
> +		else
> +			dma_sync_single_for_device(dev->dev, paddr, len, dir);
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(drm_gem_shmem_sync_mmap);
> +
>  /**
>   * drm_gem_shmem_print_info() - Print &drm_gem_shmem_object info for debugfs
>   * @shmem: shmem GEM object
> diff --git a/include/drm/drm_gem_shmem_helper.h b/include/drm/drm_gem_shmem_helper.h
> index 92f5db84b9c2..8e057a8e6c83 100644
> --- a/include/drm/drm_gem_shmem_helper.h
> +++ b/include/drm/drm_gem_shmem_helper.h
> @@ -121,6 +121,9 @@ int drm_gem_shmem_vmap_locked(struct drm_gem_shmem_object *shmem,
>  void drm_gem_shmem_vunmap_locked(struct drm_gem_shmem_object *shmem,
>  				 struct iosys_map *map);
>  int drm_gem_shmem_mmap(struct drm_gem_shmem_object *shmem, struct vm_area_struct *vma);
> +int drm_gem_shmem_sync_mmap(struct drm_gem_shmem_object *shmem,
> +			    size_t offset, size_t size,
> +			    enum dma_data_direction dir);
>
>  int drm_gem_shmem_pin_locked(struct drm_gem_shmem_object *shmem);
>  void drm_gem_shmem_unpin_locked(struct drm_gem_shmem_object *shmem);
> --
> 2.50.1


Adrian Larumbe

Re: [PATCH 4/7] drm/panthor: Bump the driver version to 1.6

[Adrian Larumbe]

On 22.08.2025 10:29, Faith Ekstrand wrote:
> Signed-off-by: Faith Ekstrand <faith.ekstrand@collabora.com>
> ---
>  drivers/gpu/drm/panthor/panthor_drv.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/panthor/panthor_drv.c b/drivers/gpu/drm/panthor/panthor_drv.c
> index 1527966604e1..2da06057ce37 100644
> --- a/drivers/gpu/drm/panthor/panthor_drv.c
> +++ b/drivers/gpu/drm/panthor/panthor_drv.c
> @@ -1658,6 +1658,8 @@ static void panthor_debugfs_init(struct drm_minor *minor)
>   * - 1.3 - adds DRM_PANTHOR_GROUP_STATE_INNOCENT flag
>   * - 1.4 - adds DRM_IOCTL_PANTHOR_BO_SET_LABEL ioctl
>   * - 1.5 - adds DRM_PANTHOR_SET_USER_MMIO_OFFSET ioctl
> + * - 1.6 - adds DRM_PANTHOR_BO_WB_MMAP flag
> + *       - adds DRM_IOCTL_PANTHOR_BO_SYNC ioctl
>   */
>  static const struct drm_driver panthor_drm_driver = {
>  	.driver_features = DRIVER_RENDER | DRIVER_GEM | DRIVER_SYNCOBJ |
> @@ -1671,7 +1673,7 @@ static const struct drm_driver panthor_drm_driver = {
>  	.name = "panthor",
>  	.desc = "Panthor DRM driver",
>  	.major = 1,
> -	.minor = 5,
> +	.minor = 6,

I tink this might be something you need to do in the previous patch, the moment the new
ioctl is made available in the panthor_drm_driver_ioctls array.


>  	.gem_create_object = panthor_gem_create_object,
>  	.gem_prime_import_sg_table = drm_gem_shmem_prime_import_sg_table,
> --
> 2.50.1


Adrian Larumbe

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

[Adrian Larumbe]

On 22.08.2025 10:29, Faith Ekstrand wrote:
> Signed-off-by: Faith Ekstrand <faith.ekstrand@collabora.com>
> ---
>  drivers/gpu/drm/panfrost/panfrost_drv.c | 7 +++++--
>  drivers/gpu/drm/panfrost/panfrost_gem.c | 3 +++
>  include/uapi/drm/panfrost_drm.h         | 1 +
>  3 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c
> index 1ea6c509a5d5..ac2a3939f0c1 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_drv.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
> @@ -116,6 +116,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)
>  {
> @@ -125,8 +129,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 */
> diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.c b/drivers/gpu/drm/panfrost/panfrost_gem.c
> index bb73f2a68a12..9a707055d946 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 = true;
> +

How come in the case of Panthor,

         if (flags & DRM_PANTHOR_BO_WB_MMAP)
              shmem->map_wc = false;

but here, it means we would map the BO WC instead?

>  	return bo;
>  }
>
> diff --git a/include/uapi/drm/panfrost_drm.h b/include/uapi/drm/panfrost_drm.h
> index ed67510395bd..e09b6f25acb2 100644
> --- a/include/uapi/drm/panfrost_drm.h
> +++ b/include/uapi/drm/panfrost_drm.h
> @@ -90,6 +90,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.
> --
> 2.50.1

Adrian Larumbe

Re: [PATCH 1/7] drm/shmem: Add a drm_gem_shmem_sync_mmap() helper

[Boris Brezillon]

Hi Adrian,

On Mon, 1 Sep 2025 09:18:49 +0100
Adrian Larumbe <adrian.larumbe@collabora.com> wrote:

> Hi Faith,
> 
> On 22.08.2025 10:29, Faith Ekstrand wrote:
> > This enables syncing mapped GEM objects between the CPU and GPU via calls
> > to dma_sync_*().  It's a bit annoying as it requires walking the sg_table
> > so it's best if every driver doesn't hand-roll it.
> >
> > Signed-off-by: Faith Ekstrand <faith.ekstrand@collabora.com>
> > ---
> >  drivers/gpu/drm/drm_gem_shmem_helper.c | 64 ++++++++++++++++++++++++++
> >  include/drm/drm_gem_shmem_helper.h     |  3 ++
> >  2 files changed, 67 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c
> > index 8ac0b1fa5287..50907c1fa11f 100644
> > --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
> > +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
> > @@ -658,6 +658,70 @@ int drm_gem_shmem_mmap(struct drm_gem_shmem_object *shmem, struct vm_area_struct
> >  }
> >  EXPORT_SYMBOL_GPL(drm_gem_shmem_mmap);
> >
> > +/**
> > + * drm_gem_shmem_sync_mmap - Sync memor-mapped data to/from the device
> > + * @shmem: shmem GEM object
> > + * @offset: Offset into the GEM object
> > + * @size: Size of the area to sync
> > + *
> > + * Returns:
> > + * 0 on success or a negative error code on failure.
> > + */
> > +int
> > +drm_gem_shmem_sync_mmap(struct drm_gem_shmem_object *shmem,
> > +			size_t offset, size_t size,
> > +			enum dma_data_direction dir)
> > +{
> > +	struct drm_device *dev = shmem->base.dev;
> > +	struct sg_table *sgt;
> > +	struct scatterlist *sgl;
> > +	unsigned int count;
> > +
> > +	if (dir == DMA_NONE)
> > +		return 0;
> > +
> > +	/* Don't bother if it's WC-mapped */
> > +	if (shmem->map_wc)
> > +		return 0;
> > +
> > +	if (size == 0)
> > +		return 0;
> > +
> > +	if (offset + size < offset || offset + size > shmem->base.size)
> > +		return -EINVAL;
> > +
> > +	sgt = drm_gem_shmem_get_pages_sgt(shmem);
> > +	if (IS_ERR(sgt))
> > +		return PTR_ERR(sgt);  
> 
> This will allocate pages when the BO had no backing storage yet, otherwise it'll increase the
> refcnt on those pages, but seems to me this will leave the reference count imbalanced.

That may be how we want things to be, but that's not the case in
practice. At the moment, drm_gem_shmem_get_pages_sgt() only increases
the pages refcount if no sgt exists and one needs to be created,
otherwise it simply returns the sgt attached to drm_gem_shmem_object.
This implicit ref on pages is only released when the GEM object is
destroyed, meaning the sgt/pages lifetime is bound to the GEM object
lifetime. That's definitely something we need to address if we want to
have a generic GEM-shmem shrinker, but I don't think this is needed
here.

> 
> I'd say running this function on an object with no storage backing should be considered a no-op:
> 
> ```
> 	if (!shmem->pages)
> 		return 0;
> ```

The mmap()/vmap() implementations call get_pages(), so I would expect
the !shmem->pages case to return an error, not zero. Of course, it
no longer stands if drivers overload the mmap() implementation and call
get_pages() in their fault handler path, in which case, I agree,
returning zero is better, but that's probably something I would let the
drivers check themselves before calling drm_gem_shmem_sync_mmap().

> 
> On top of that, there are code paths in which we allocate pages, but produce no sgtable for them,
> like drm_gem_shmem_mmap(), so maybe we could do as follows:
> 
> ```
> 	if (!shmem->sgt) {
> 		int ret;
> 
> 		ret = dma_resv_lock_interruptible(shmem->base.resv, NULL);
> 		if (ret)
> 			return ret;
> 
> 		sgt = drm_gem_shmem_get_sg_table(shmem);
> 		if (IS_ERR(sgt))
> 			return ret;
> 
> 		shmem->sgt = sgt;
> 	}
> ```

I'm not too sure we want to force an sgt creation if the driver doesn't
need it. What we want to do though is force drivers who need an sgt
to explicitly call _get_pages() instead of relying on the implicit
pages ref currently held by shmem::sgt. But it's all orthogonal to the
changes being proposed in this patch, I think.

Regards,

Boris