On Fri, 16 Feb 2024 16:09:55 +0100 Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-pra...@amd.com> wrote: > > Signed-off-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-pra...@amd.com> > --- > 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. > + > + 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 */