AMD General
Regards,
Prike
> -----Original Message-----
> From: Pierre-Eric Pelloux-Prayer <[email protected]>
> Sent: Wednesday, May 13, 2026 5:14 PM
> To: Liang, Prike <[email protected]>; [email protected]
> Cc: Deucher, Alexander <[email protected]>; Koenig, Christian
> <[email protected]>
> Subject: Re: [PATCH 2/4] drm/amdgpu: add userq queue state transition
> tracepoints
>
>
>
> Le 11/05/2026 à 15:54, Prike Liang a écrit :
> > Add ftrace events around user queue preempt, restore, map and unmap
> > operations to profile runtime queue state transitions.
> >
> > Signed-off-by: Prike Liang <[email protected]>
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h | 25
> +++++++++++++++++++++++
> > drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 12 ++++++++++-
> > 2 files changed, 36 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
> > index 5a01f63d1f32..484fbb00068b 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
> > @@ -636,6 +636,31 @@ DEFINE_EVENT(amdgpu_userq_queue_result,
> amdgpu_userq_destroy_end,
> > TP_PROTO(struct amdgpu_usermode_queue *queue, int result),
> > TP_ARGS(queue, result));
> >
> > +
> > +DEFINE_EVENT(amdgpu_userq_queue, amdgpu_userq_preempt_start,
> > + TP_PROTO(struct amdgpu_usermode_queue *queue),
> > + TP_ARGS(queue));
> > +DEFINE_EVENT(amdgpu_userq_queue, amdgpu_userq_restore_start,
> > + TP_PROTO(struct amdgpu_usermode_queue *queue),
> > + TP_ARGS(queue));
> > +DEFINE_EVENT(amdgpu_userq_queue, amdgpu_userq_map_start,
> > + TP_PROTO(struct amdgpu_usermode_queue *queue),
> > + TP_ARGS(queue));
> > +DEFINE_EVENT(amdgpu_userq_queue, amdgpu_userq_unmap_start,
> > + TP_PROTO(struct amdgpu_usermode_queue *queue),
> > + TP_ARGS(queue));
> > +DEFINE_EVENT(amdgpu_userq_queue_result, amdgpu_userq_preempt_end,
> > + TP_PROTO(struct amdgpu_usermode_queue *queue, int result),
> > + TP_ARGS(queue, result));
> > +DEFINE_EVENT(amdgpu_userq_queue_result, amdgpu_userq_restore_end,
> > + TP_PROTO(struct amdgpu_usermode_queue *queue, int result),
> > + TP_ARGS(queue, result));
> > +DEFINE_EVENT(amdgpu_userq_queue_result, amdgpu_userq_map_end,
> > + TP_PROTO(struct amdgpu_usermode_queue *queue, int result),
> > + TP_ARGS(queue, result));
> > +DEFINE_EVENT(amdgpu_userq_queue_result, amdgpu_userq_unmap_end,
> > + TP_PROTO(struct amdgpu_usermode_queue *queue, int result),
> > + TP_ARGS(queue, result));
> > #undef AMDGPU_JOB_GET_TIMELINE_NAME
> > #endif
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> > index 50c46d31fbae..83aee0810513 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> > @@ -340,13 +340,16 @@ static int amdgpu_userq_preempt_helper(struct
> amdgpu_usermode_queue *queue)
> > int r;
> >
> > if (queue->state == AMDGPU_USERQ_STATE_MAPPED) {
> > + trace_amdgpu_userq_preempt_start(queue);
> > r = userq_funcs->preempt(queue);
> > if (r) {
> > queue->state = AMDGPU_USERQ_STATE_HUNG;
> > + trace_amdgpu_userq_preempt_end(queue, r);
> > return r;
> > } else {
> > queue->state = AMDGPU_USERQ_STATE_PREEMPTED;
> > }
> > + trace_amdgpu_userq_preempt_end(queue, r);
>
> I prefer having only 2 trace points: trace_amdgpu_userq_state_change_start /
> end.
> The _start event would print the current state and the _end one would print
> the new
> state.
Yeah, we can simplify the queue state tracker this way, but it wouldn't be as
straightforward and may require further parsing of the queue state to profile
the queue's state transitions from the tooling library perspective.
Thanks,
Prike
> Also these events should be used eveywhere "queue->state" is modified.
>
> Pierre-Eric
>
>
> > }
> > return 0;
> > }
> > @@ -360,12 +363,14 @@ static int amdgpu_userq_restore_helper(struct
> amdgpu_usermode_queue *queue)
> > int r = 0;
> >
> > if (queue->state == AMDGPU_USERQ_STATE_PREEMPTED) {
> > + trace_amdgpu_userq_restore_start(queue);
> > r = userq_funcs->restore(queue);
> > if (r) {
> > queue->state = AMDGPU_USERQ_STATE_HUNG;
> > } else {
> > queue->state = AMDGPU_USERQ_STATE_MAPPED;
> > }
> > + trace_amdgpu_userq_restore_end(queue, r);
> > }
> >
> > return r;
> > @@ -381,14 +386,16 @@ static int amdgpu_userq_unmap_helper(struct
> > amdgpu_usermode_queue *queue)
> >
> > if ((queue->state == AMDGPU_USERQ_STATE_MAPPED) ||
> > (queue->state == AMDGPU_USERQ_STATE_PREEMPTED)) {
> > -
> > + trace_amdgpu_userq_unmap_start(queue);
> > r = userq_funcs->unmap(queue);
> > if (r) {
> > queue->state = AMDGPU_USERQ_STATE_HUNG;
> > + trace_amdgpu_userq_unmap_end(queue, r);
> > return r;
> > } else {
> > queue->state = AMDGPU_USERQ_STATE_UNMAPPED;
> > }
> > + trace_amdgpu_userq_unmap_end(queue, 0);
> > }
> >
> > return 0;
> > @@ -403,13 +410,16 @@ static int amdgpu_userq_map_helper(struct
> amdgpu_usermode_queue *queue)
> > int r;
> >
> > if (queue->state == AMDGPU_USERQ_STATE_UNMAPPED) {
> > + trace_amdgpu_userq_map_start(queue);
> > r = userq_funcs->map(queue);
> > if (r) {
> > queue->state = AMDGPU_USERQ_STATE_HUNG;
> > + trace_amdgpu_userq_map_end(queue, r);
> > return r;
> > } else {
> > queue->state = AMDGPU_USERQ_STATE_MAPPED;
> > }
> > + trace_amdgpu_userq_map_end(queue, 0);
> > }
> >
> > return 0;
