Re: [PATCH 4/8] drm/amd/display: declare amdgpu_dm_atomic_state_template as an event class

[Tvrtko Ursulin <tvrtko.ursulin@igalia.com>]

On 24/09/2026 22:10, Melissa Wen wrote:
> Don't create an amdgpu_dm_atomic_state_template event since this is just
> a template. Declare the template as an event class that will be used by
> actual event tracers.
> 
> Fixes: e8a982355f96 ("drm/amd/display: Add tracepoint for amdgpu_dm")
> Signed-off-by: Melissa Wen <mwen@igalia.com>
> ---
>   .../amd/display/amdgpu_dm/amdgpu_dm_trace.h   | 56 +++++++++----------
>   1 file changed, 26 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_trace.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_trace.h
> index f5c2f589f691..c59ba41a9721 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_trace.h
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_trace.h
> @@ -314,36 +314,32 @@ DEFINE_EVENT(amdgpu_dm_plane_state_template, amdgpu_dm_atomic_update_cursor,
>   	     TP_PROTO(const struct drm_plane_state *state),
>   	     TP_ARGS(state));
>   
> -TRACE_EVENT(amdgpu_dm_atomic_state_template,
> -	    TP_PROTO(const struct drm_atomic_commit *state),
> -	    TP_ARGS(state),
> -
> -	    TP_STRUCT__entry(
> -			     __field(const struct drm_atomic_commit *, state)
> -			     __field(bool, allow_modeset)
> -			     __field(bool, legacy_cursor_update)
> -			     __field(bool, async_update)
> -			     __field(bool, duplicated)
> -			     __field(int, num_connector)
> -			     __field(int, num_private_objs)
> -	    ),
> -
> -	    TP_fast_assign(
> -			   __entry->state = state;
> -			   __entry->allow_modeset = state->allow_modeset;
> -			   __entry->legacy_cursor_update = state->legacy_cursor_update;
> -			   __entry->async_update = state->async_update;
> -			   __entry->duplicated = state->duplicated;
> -			   __entry->num_connector = state->num_connector;
> -			   __entry->num_private_objs = state->num_private_objs;
> -	    ),
> -
> -	    TP_printk("state=%p allow_modeset=%d legacy_cursor_update=%d "
> -		      "async_update=%d duplicated=%d num_connector=%d "
> -		      "num_private_objs=%d",
> -		      __entry->state, __entry->allow_modeset, __entry->legacy_cursor_update,
> -		      __entry->async_update, __entry->duplicated, __entry->num_connector,
> -		      __entry->num_private_objs)
> +DECLARE_EVENT_CLASS(amdgpu_dm_atomic_state_template,
> +		    TP_PROTO(const struct drm_atomic_commit *state),
> +		    TP_ARGS(state),
> +		    TP_STRUCT__entry(__field(const struct drm_atomic_commit *, state)
> +				     __field(bool, allow_modeset)
> +				     __field(bool, legacy_cursor_update)
> +				     __field(bool, async_update)
> +				     __field(bool, duplicated)
> +				     __field(int, num_connector)
> +				     __field(int, num_private_objs)
> +		    ),
> +		    TP_fast_assign(__entry->state = state;
> +				   __entry->allow_modeset = state->allow_modeset;
> +				   __entry->legacy_cursor_update = state->legacy_cursor_update;
> +				   __entry->async_update = state->async_update;
> +				   __entry->duplicated = state->duplicated;
> +				   __entry->num_connector = state->num_connector;
> +				   __entry->num_private_objs = state->num_private_objs;
> +		    ),
> +		    TP_printk("state=%p allow_modeset=%d legacy_cursor_update=%d "
> +			      "async_update=%d duplicated=%d num_connector=%d "
> +			      "num_private_objs=%d",
> +			      __entry->state, __entry->allow_modeset,
> +			      __entry->legacy_cursor_update, __entry->async_update,
> +			      __entry->duplicated, __entry->num_connector,
> +			      __entry->num_private_objs)
>   );
>   
>   DEFINE_EVENT(amdgpu_dm_atomic_state_template, amdgpu_dm_atomic_commit_tail_begin,

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>

Regards,

Tvrtko

P.S. A side question - are %p useful in these tracepoints? I imagine one 
has to know the object lifetimes really well, or make sure state init 
and free are also traced to make sense of what is what.