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