Re: [RFC v2 4/8] drm/panthor: Add panthor perf ioctls

From: Lukas Zapolskas
Date: Wed Mar 26 2025 - 10:48:15 EST

Next message: Roxana Nicolescu: "Re: [PATCH] bcachefs: replace deprecated strncpy() with strscpy()"
Previous message: Prabhakar: "[PATCH 15/15] arm64: defconfig: Enable Renesas RZ/V2N SoC"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On 27/01/2025 14:06, Adrián Larumbe wrote:

On 11.12.2024 16:50, Lukas Zapolskas wrote:

This patch implements the PANTHOR_PERF_CONTROL ioctl series, and
a PANTHOR_GET_UOBJ wrapper to deal with the backwards and forwards
compatibility of the uAPI.

Stub function definitions are added to ensure the patch builds on its own,
and will be removed later in the series.

Signed-off-by: Lukas Zapolskas <lukas.zapolskas@xxxxxxx>
---
drivers/gpu/drm/panthor/panthor_drv.c | 155 ++++++++++++++++++++++++-
drivers/gpu/drm/panthor/panthor_perf.c | 34 ++++++
drivers/gpu/drm/panthor/panthor_perf.h | 19 +++
3 files changed, 206 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/panthor/panthor_drv.c b/drivers/gpu/drm/panthor/panthor_drv.c
index e0ac3107c69e..458175f58b15 100644
--- a/drivers/gpu/drm/panthor/panthor_drv.c
+++ b/drivers/gpu/drm/panthor/panthor_drv.c
@@ -7,6 +7,7 @@
#include <asm/arch_timer.h>
#endif
+#include <linux/cleanup.h>
#include <linux/list.h>
#include <linux/module.h>
#include <linux/of_platform.h>
@@ -31,6 +32,7 @@
#include "panthor_gpu.h"
#include "panthor_heap.h"
#include "panthor_mmu.h"
+#include "panthor_perf.h"
#include "panthor_regs.h"
#include "panthor_sched.h"
@@ -73,6 +75,39 @@ panthor_set_uobj(u64 usr_ptr, u32 usr_size, u32 min_size, u32 kern_size, const v
return 0;
}
+/**
+ * panthor_get_uobj() - Copy kernel object to user object.
+ * @usr_ptr: Users pointer.
+ * @usr_size: Size of the user object.
+ * @min_size: Minimum size for this object.
+ *
+ * Helper automating kernel -> user object copies.
+ *
+ * Don't use this function directly, use PANTHOR_UOBJ_GET() instead.
+ *
+ * Return: valid pointer on success, an encoded error code otherwise.
+ */
+static void*
+panthor_get_uobj(u64 usr_ptr, u32 usr_size, u32 min_size)
+{
+ int ret;
+ void *out_alloc __free(kvfree) = NULL;
+
+ /* User size shouldn't be smaller than the minimal object size. */
+ if (usr_size < min_size)
+ return ERR_PTR(-EINVAL);
+
+ out_alloc = kvmalloc(min_size, GFP_KERNEL);
+ if (!out_alloc)
+ return ERR_PTR(-ENOMEM);
+
+ ret = copy_struct_from_user(out_alloc, min_size, u64_to_user_ptr(usr_ptr), usr_size);
+ if (ret)
+ return ERR_PTR(ret);
+
+ return_ptr(out_alloc);
+}
+
/**
* panthor_get_uobj_array() - Copy a user object array into a kernel accessible object array.
* @in: The object array to copy.
@@ -176,8 +211,11 @@ panthor_get_uobj_array(const struct drm_panthor_obj_array *in, u32 min_stride,
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_perf_info, shader_blocks))
-
+ PANTHOR_UOBJ_DECL(struct drm_panthor_perf_info, shader_blocks), \
+ PANTHOR_UOBJ_DECL(struct drm_panthor_perf_cmd_setup, shader_enable_mask), \
+ PANTHOR_UOBJ_DECL(struct drm_panthor_perf_cmd_start, user_data), \
+ PANTHOR_UOBJ_DECL(struct drm_panthor_perf_cmd_stop, user_data), \
+ PANTHOR_UOBJ_DECL(struct drm_panthor_perf_cmd_sample, user_data))
/**
* PANTHOR_UOBJ_SET() - Copy a kernel object to a user object.
@@ -192,6 +230,24 @@ panthor_get_uobj_array(const struct drm_panthor_obj_array *in, u32 min_stride,
PANTHOR_UOBJ_MIN_SIZE(_src_obj), \
sizeof(_src_obj), &(_src_obj))
+/**
+ * PANTHOR_UOBJ_GET() - Copies a user object from _usr_ptr to a kernel accessible _dest_ptr.
+ * @_dest_ptr: Local varialbe
+ * @_usr_size: Size of the user object.
+ * @_usr_ptr: The pointer of the object in userspace.
+ *
+ * Return: Error code. See panthor_get_uobj().
+ */
+#define PANTHOR_UOBJ_GET(_dest_ptr, _usr_size, _usr_ptr) \
+ ({ \
+ typeof(_dest_ptr) _tmp; \
+ _tmp = panthor_get_uobj(_usr_ptr, _usr_size, \
+ PANTHOR_UOBJ_MIN_SIZE(_tmp[0])); \
+ if (!IS_ERR(_tmp)) \
+ _dest_ptr = _tmp; \
+ PTR_ERR_OR_ZERO(_tmp); \
+ })
+
/**
* PANTHOR_UOBJ_GET_ARRAY() - Copy a user object array to a kernel accessible
* object array.
@@ -1339,6 +1395,99 @@ static int panthor_ioctl_vm_get_state(struct drm_device *ddev, void *data,
return 0;
}
+static int panthor_ioctl_perf_control(struct drm_device *ddev, void *data,
+ struct drm_file *file)
+{
+ struct panthor_device *ptdev = container_of(ddev, struct panthor_device, base);
+ struct panthor_file *pfile = file->driver_priv;
+ struct drm_panthor_perf_control *args = data;
+ int ret;
+
+ if (!args->pointer) {
+ switch (args->cmd) {
+ case DRM_PANTHOR_PERF_COMMAND_SETUP:
+ args->size = sizeof(struct drm_panthor_perf_cmd_setup);
+ return 0;
+
+ case DRM_PANTHOR_PERF_COMMAND_TEARDOWN:
+ args->size = 0;
+ return 0;
+
+ case DRM_PANTHOR_PERF_COMMAND_START:
+ args->size = sizeof(struct drm_panthor_perf_cmd_start);
+ return 0;
+
+ case DRM_PANTHOR_PERF_COMMAND_STOP:
+ args->size = sizeof(struct drm_panthor_perf_cmd_stop);
+ return 0;
+
+ case DRM_PANTHOR_PERF_COMMAND_SAMPLE:
+ args->size = sizeof(struct drm_panthor_perf_cmd_sample);
+ return 0;
+
+ default:
+ return -EINVAL;
+ }
+ }
+
+ switch (args->cmd) {
+ case DRM_PANTHOR_PERF_COMMAND_SETUP:
+ {
+ struct drm_panthor_perf_cmd_setup *setup_args __free(kvfree) = NULL;
+
+ ret = PANTHOR_UOBJ_GET(setup_args, args->size, args->pointer);
+ if (ret)
+ return -EINVAL;
+
+ if (setup_args->pad[0])
+ return -EINVAL;
+
+ ret = panthor_perf_session_setup(ptdev, ptdev->perf, setup_args, pfile);

Shouldn't we return the session id as an output param in setup_args or is the
ioctl's return value enough for this?

Returning it via the ioctl return value has worked for a single session,
but it may have an impact when handling multiple sessions, not sure. Is
there anything done by DRM to normalize the return values from ioctls? I
could not find anything.

+
+ return ret;
+ }
+ case DRM_PANTHOR_PERF_COMMAND_TEARDOWN:
+ {
+ return panthor_perf_session_teardown(pfile, ptdev->perf, args->handle);
+ }
+ case DRM_PANTHOR_PERF_COMMAND_START:
+ {
+ struct drm_panthor_perf_cmd_start *start_args __free(kvfree) = NULL;
+
+ ret = PANTHOR_UOBJ_GET(start_args, args->size, args->pointer);
+ if (ret)
+ return -EINVAL;
+
+ return panthor_perf_session_start(pfile, ptdev->perf, args->handle,
+ start_args->user_data);
+ }
+ case DRM_PANTHOR_PERF_COMMAND_STOP:
+ {
+ struct drm_panthor_perf_cmd_stop *stop_args __free(kvfree) = NULL;
+
+ ret = PANTHOR_UOBJ_GET(stop_args, args->size, args->pointer);
+ if (ret)
+ return -EINVAL;
+
+ return panthor_perf_session_stop(pfile, ptdev->perf, args->handle,
+ stop_args->user_data);
+ }
+ case DRM_PANTHOR_PERF_COMMAND_SAMPLE:
+ {
+ struct drm_panthor_perf_cmd_sample *sample_args __free(kvfree) = NULL;
+
+ ret = PANTHOR_UOBJ_GET(sample_args, args->size, args->pointer);
+ if (ret)
+ return -EINVAL;
+
+ return panthor_perf_session_sample(pfile, ptdev->perf, args->handle,
+ sample_args->user_data);
+ }

For the three cases above, you could define a macro like:

#define perf_cmd(command) \
({ \
struct drm_panthor_perf_cmd_##command * command##_args __free(kvfree) = NULL; \
\
ret = PANTHOR_UOBJ_GET(command##_args, args->size, args->pointer); \
if (ret) \
return -EINVAL; \
return panthor_perf_session_##command(pfile, ptdev->perf, args->handle, command##_args->user_data); \
})

and then do 'perf_cmd(command);' inside each one of them

+ default:
+ return -EINVAL;
+ }
+}
+
static int
panthor_open(struct drm_device *ddev, struct drm_file *file)
{
@@ -1386,6 +1535,7 @@ panthor_postclose(struct drm_device *ddev, struct drm_file *file)
panthor_group_pool_destroy(pfile);
panthor_vm_pool_destroy(pfile);
+ panthor_perf_session_destroy(pfile, pfile->ptdev->perf);

I would perhaps do this first because pools are first created during file
opening, just to undo things in the opposite sequence.

kfree(pfile);
module_put(THIS_MODULE);
@@ -1408,6 +1558,7 @@ static const struct drm_ioctl_desc panthor_drm_driver_ioctls[] = {
PANTHOR_IOCTL(TILER_HEAP_CREATE, tiler_heap_create, DRM_RENDER_ALLOW),
PANTHOR_IOCTL(TILER_HEAP_DESTROY, tiler_heap_destroy, DRM_RENDER_ALLOW),
PANTHOR_IOCTL(GROUP_SUBMIT, group_submit, DRM_RENDER_ALLOW),
+ PANTHOR_IOCTL(PERF_CONTROL, perf_control, DRM_RENDER_ALLOW),
};
static int panthor_mmap(struct file *filp, struct vm_area_struct *vma)
diff --git a/drivers/gpu/drm/panthor/panthor_perf.c b/drivers/gpu/drm/panthor/panthor_perf.c
index e0dc6c4b0cf1..6498279ec036 100644
--- a/drivers/gpu/drm/panthor/panthor_perf.c
+++ b/drivers/gpu/drm/panthor/panthor_perf.c
@@ -63,6 +63,40 @@ void panthor_perf_info_init(struct panthor_device *ptdev)
perf_info->shader_blocks = hweight64(ptdev->gpu_info.shader_present);
}
+int panthor_perf_session_setup(struct panthor_device *ptdev, struct panthor_perf *perf,
+ struct drm_panthor_perf_cmd_setup *setup_args,
+ struct panthor_file *pfile)
+{
+ return -EOPNOTSUPP;
+}
+
+int panthor_perf_session_teardown(struct panthor_file *pfile, struct panthor_perf *perf,
+ u32 sid)
+{
+ return -EOPNOTSUPP;
+}
+
+int panthor_perf_session_start(struct panthor_file *pfile, struct panthor_perf *perf,
+ u32 sid, u64 user_data)
+{
+ return -EOPNOTSUPP;
+}
+
+int panthor_perf_session_stop(struct panthor_file *pfile, struct panthor_perf *perf,
+ u32 sid, u64 user_data)
+{
+ return -EOPNOTSUPP;
+}
+
+int panthor_perf_session_sample(struct panthor_file *pfile, struct panthor_perf *perf,
+ u32 sid, u64 user_data)
+{
+ return -EOPNOTSUPP;
+
+}
+
+void panthor_perf_session_destroy(struct panthor_file *pfile, struct panthor_perf *perf) { }
+
/**
* panthor_perf_init - Initialize the performance counter subsystem.
* @ptdev: Panthor device
diff --git a/drivers/gpu/drm/panthor/panthor_perf.h b/drivers/gpu/drm/panthor/panthor_perf.h
index 90af8b18358c..bfef8874068b 100644
--- a/drivers/gpu/drm/panthor/panthor_perf.h
+++ b/drivers/gpu/drm/panthor/panthor_perf.h
@@ -5,11 +5,30 @@
#ifndef __PANTHOR_PERF_H__
#define __PANTHOR_PERF_H__
+#include <linux/types.h>
+
+struct drm_gem_object;
+struct drm_panthor_perf_cmd_setup;
struct panthor_device;
+struct panthor_file;
+struct panthor_perf;
void panthor_perf_info_init(struct panthor_device *ptdev);
int panthor_perf_init(struct panthor_device *ptdev);
void panthor_perf_unplug(struct panthor_device *ptdev);
+int panthor_perf_session_setup(struct panthor_device *ptdev, struct panthor_perf *perf,
+ struct drm_panthor_perf_cmd_setup *setup_args,
+ struct panthor_file *pfile);
+int panthor_perf_session_teardown(struct panthor_file *pfile, struct panthor_perf *perf,
+ u32 sid);
+int panthor_perf_session_start(struct panthor_file *pfile, struct panthor_perf *perf,
+ u32 sid, u64 user_data);
+int panthor_perf_session_stop(struct panthor_file *pfile, struct panthor_perf *perf,
+ u32 sid, u64 user_data);
+int panthor_perf_session_sample(struct panthor_file *pfile, struct panthor_perf *perf,
+ u32 sid, u64 user_data);
+void panthor_perf_session_destroy(struct panthor_file *pfile, struct panthor_perf *perf);
+
#endif /* __PANTHOR_PERF_H__ */
--
2.25.1

Adrian Larumbe

Next message: Roxana Nicolescu: "Re: [PATCH] bcachefs: replace deprecated strncpy() with strscpy()"
Previous message: Prabhakar: "[PATCH 15/15] arm64: defconfig: Enable Renesas RZ/V2N SoC"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]