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?
> +
> + 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
