Hi Christian! Em 20/10/2024 17:41, Christian Gmeiner escreveu:
From: Christian Gmeiner <cgmeiner@xxxxxxxxxx> This patch adds a new ioctl, DRM_IOCTL_V3D_PERFMON_SET_GLOBAL, which allows the configuration of a global performance monitor (perfmon). The global perfmon is used for all jobs, ensuring consistent performance tracking across submissions.
Usually we write in the imperative form: Add a new ioctl, ...
Signed-off-by: Christian Gmeiner <cgmeiner@xxxxxxxxxx> --- drivers/gpu/drm/v3d/v3d_drv.c | 3 ++ drivers/gpu/drm/v3d/v3d_drv.h | 10 ++++ drivers/gpu/drm/v3d/v3d_perfmon.c | 49 +++++++++++++++++++ .../gpu/drm/v3d/v3d_performance_counters.h | 6 +++ drivers/gpu/drm/v3d/v3d_sched.c | 10 +++- include/uapi/drm/v3d_drm.h | 15 ++++++ 6 files changed, 91 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/v3d/v3d_drv.c b/drivers/gpu/drm/v3d/v3d_drv.c index d7ff1f5fa481..f1753ee2af25 100644 --- a/drivers/gpu/drm/v3d/v3d_drv.c +++ b/drivers/gpu/drm/v3d/v3d_drv.c @@ -214,6 +214,7 @@ static const struct drm_ioctl_desc v3d_drm_ioctls[] = { DRM_IOCTL_DEF_DRV(V3D_PERFMON_GET_VALUES, v3d_perfmon_get_values_ioctl, DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(V3D_SUBMIT_CPU, v3d_submit_cpu_ioctl, DRM_RENDER_ALLOW | DRM_AUTH), DRM_IOCTL_DEF_DRV(V3D_PERFMON_GET_COUNTER, v3d_perfmon_get_counter_ioctl, DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(V3D_PERFMON_SET_GLOBAL, v3d_perfmon_set_global_ioctl, DRM_RENDER_ALLOW), };static const struct drm_driver v3d_drm_driver = {@@ -350,6 +351,8 @@ static int v3d_platform_drm_probe(struct platform_device *pdev) if (ret) goto drm_unregister;+ atomic_set(&v3d->num_perfmon, 0);+ return 0;drm_unregister:diff --git a/drivers/gpu/drm/v3d/v3d_drv.h b/drivers/gpu/drm/v3d/v3d_drv.h index cf4b23369dc4..9491d730d99f 100644 --- a/drivers/gpu/drm/v3d/v3d_drv.h +++ b/drivers/gpu/drm/v3d/v3d_drv.h @@ -61,6 +61,8 @@ struct v3d_queue_state { struct v3d_stats stats; };+struct v3d_dev;+
Forward declarations go in the beginning of the file, along with the other ones:
struct clk; struct platform_device; struct reset_control; +struct v3d_dev;
/* Performance monitor object. The perform lifetime is controlled by userspace * using perfmon related ioctls. A perfmon can be attached to a submit_cl * request, and when this is the case, HW perf counters will be activated just @@ -68,6 +70,9 @@ struct v3d_queue_state { * done. This way, only events related to a specific job will be counted. */ struct v3d_perfmon { + /* Pointer back to v3d instance this perfmon belongs. */ + struct v3d_dev *v3d; + /* Tracks the number of users of the perfmon, when this counter reaches * zero the perfmon is destroyed. */ @@ -179,6 +184,9 @@ struct v3d_dev { u32 num_allocated; u32 pages_allocated; } bo_stats; + + /* Keep track of current number of allocated perfmons. */ + atomic_t num_perfmon; };static inline struct v3d_dev *@@ -584,6 +592,8 @@ int v3d_perfmon_get_values_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv); int v3d_perfmon_get_counter_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv); +int v3d_perfmon_set_global_ioctl(struct drm_device *dev, void *data, + struct drm_file *file_priv);/* v3d_sysfs.c */int v3d_sysfs_init(struct device *dev);
[...]
diff --git a/include/uapi/drm/v3d_drm.h b/include/uapi/drm/v3d_drm.h index 87fc5bb0a61e..960d392d75a3 100644 --- a/include/uapi/drm/v3d_drm.h +++ b/include/uapi/drm/v3d_drm.h @@ -43,6 +43,7 @@ extern "C" { #define DRM_V3D_PERFMON_GET_VALUES 0x0a #define DRM_V3D_SUBMIT_CPU 0x0b #define DRM_V3D_PERFMON_GET_COUNTER 0x0c +#define DRM_V3D_PERFMON_SET_GLOBAL 0x0d#define DRM_IOCTL_V3D_SUBMIT_CL DRM_IOWR(DRM_COMMAND_BASE + DRM_V3D_SUBMIT_CL, struct drm_v3d_submit_cl)#define DRM_IOCTL_V3D_WAIT_BO DRM_IOWR(DRM_COMMAND_BASE + DRM_V3D_WAIT_BO, struct drm_v3d_wait_bo) @@ -61,6 +62,8 @@ extern "C" { #define DRM_IOCTL_V3D_SUBMIT_CPU DRM_IOW(DRM_COMMAND_BASE + DRM_V3D_SUBMIT_CPU, struct drm_v3d_submit_cpu) #define DRM_IOCTL_V3D_PERFMON_GET_COUNTER DRM_IOWR(DRM_COMMAND_BASE + DRM_V3D_PERFMON_GET_COUNTER, \ struct drm_v3d_perfmon_get_counter) +#define DRM_IOCTL_V3D_PERFMON_SET_GLOBAL DRM_IOW(DRM_COMMAND_BASE + DRM_V3D_PERFMON_SET_GLOBAL, \ + struct drm_v3d_perfmon_set_global)#define DRM_V3D_SUBMIT_CL_FLUSH_CACHE 0x01#define DRM_V3D_SUBMIT_EXTENSION 0x02 @@ -765,6 +768,18 @@ struct drm_v3d_perfmon_get_counter { __u8 reserved[7]; };+/**
Using /** means that you are writting a kernel-doc comment [1], so make sure to describe each struct member, otherwise it's going to generate build warnings with W=1.
+ * struct drm_v3d_perfmon_set_global - ioctl to define a + * global performance counter that is used if a job has + * not assigned one on its own. + */ + +#define DRM_V3D_PERFMON_CLEAR_GLOBAL 0x0001
I would keep this define above the struct comment.
+struct drm_v3d_perfmon_set_global {
+ __u32 flags;
+ __u32 id;
+};
+
#if defined(__cplusplus)
}
#endif
[1] https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html
