Re: [PATCH v3 6/8] drm: add drm_mode_atomic_commit event
From: Steven Rostedt <rostedt@goodmis.org>
Date: 2024-02-16 15:57:59
Also in:
amd-gfx, dri-devel, linux-media
On Fri, 16 Feb 2024 16:09:55 +0100 Pierre-Eric Pelloux-Prayer [off-list ref] wrote:
quoted hunk ↗ jump to hunk
Signed-off-by: Pierre-Eric Pelloux-Prayer <redacted> --- drivers/gpu/drm/drm_atomic_uapi.c | 21 +++++++++++++++++++++ drivers/gpu/drm/drm_trace.h | 23 +++++++++++++++++++++++ 2 files changed, 44 insertions(+)diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c index 29d4940188d4..f31b5c6f870b 100644 --- a/drivers/gpu/drm/drm_atomic_uapi.c +++ b/drivers/gpu/drm/drm_atomic_uapi.c@@ -41,6 +41,7 @@ #include <linux/file.h> #include "drm_crtc_internal.h" +#include "drm_trace.h" /** * DOC: overview@@ -1503,6 +1504,26 @@ int drm_mode_atomic_ioctl(struct drm_device *dev, drm_mode_object_put(obj); } + if (trace_drm_mode_atomic_commit_enabled()) { + struct drm_crtc_state *crtc_state; + struct drm_crtc *crtc; + int *crtcs; + int i, num_crtcs; + + crtcs = kcalloc(dev->mode_config.num_crtc, sizeof(int), + GFP_KERNEL); + + if (crtcs) { + num_crtcs = 0; + for_each_new_crtc_in_state(state, crtc, crtc_state, i) + crtcs[num_crtcs++] = drm_crtc_index(crtc);
Hmm, looking deeper into this, could you just do the loop the trace event? That is how different is the config.num_crtc compared to the final num_crtcs? That way, we don't need to do this allocation if it's not too different. That is, pass in the dev->mode_config.num_crtc to the tracepoint instead of num_crtcs.
quoted hunk ↗ jump to hunk
+ + trace_drm_mode_atomic_commit(file_priv, crtcs, num_crtcs, arg->flags); + + kfree(crtcs); + } + } + ret = prepare_signaling(dev, state, arg, file_priv, &fence_state, &num_fences); if (ret)diff --git a/drivers/gpu/drm/drm_trace.h b/drivers/gpu/drm/drm_trace.h index 11c6dd577e8e..63489923c289 100644 --- a/drivers/gpu/drm/drm_trace.h +++ b/drivers/gpu/drm/drm_trace.h@@ -66,6 +66,29 @@ TRACE_EVENT(drm_vblank_event_delivered, __entry->seq) ); +TRACE_EVENT(drm_mode_atomic_commit, + TP_PROTO(struct drm_file *file, int *crtcs, int ncrtcs, uint32_t flags), + TP_ARGS(file, crtcs, ncrtcs, flags), + TP_STRUCT__entry( + __field(struct drm_file *, file) + __dynamic_array(u32, crtcs, ncrtcs)
Here the ncrtcs is what is passed in. It will always be allocated to that size though.
+ __field(uint32_t, ncrtcs) + __field(uint32_t, flags) + ), + TP_fast_assign( + unsigned int i; + + __entry->file = file; + for (i = 0; i < ncrtcs; i++) + ((u32 *)__get_dynamic_array(crtcs))[i] = crtcs[i];
Here we have: int n = 0; for_each_new_crtc_in_state(state, crtc, crtc_state, i) ((u32 *)__get_dynamic_array(crtcs))[n++] = drm_crtc_index(crtc); __entry->ncrtcs = n; But this is only viable if the ncrtcs is close to the same size as dev->mode_config.num_crtc, otherwise it's not worth it. -- Steve
+ __entry->ncrtcs = ncrtcs;
+ __entry->flags = flags;
+ ),
+ TP_printk("file=%p, pid=%8d, flags=%08x, crtcs=%s", __entry->file,
+ pid_nr(__entry->file->pid), __entry->flags,
+ __print_array(__get_dynamic_array(crtcs), __entry->ncrtcs, 4))
+);
+
#endif /* _DRM_TRACE_H_ */
/* This part must be outside protection */