| Message ID | 20240213155112.156537-6-pierre-eric.pelloux-prayer@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | dma-fence, drm, amdgpu new trace events | expand |
Am 13.02.24 um 16:50 schrieb Pierre-Eric Pelloux-Prayer: > amdgpu_cs_ioctl already exists but serves a different > purpose. > > amdgpu_cs_ioctl2 marks the beginning of the kernel processing of > the ioctl which is useful for tools to map which events belong to > the same submission (without this, the first event would be the > amdgpu_bo_set_list ones). That's not necessary a good justification for the naming. What exactly was the original trace_amdgpu_cs_ioctl() doing? Regards, Christian. > > Signed-off-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 2 ++ > drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h | 12 ++++++++++++ > 2 files changed, 14 insertions(+) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c > index 6830892383c3..29e43a66d0d6 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c > @@ -1402,6 +1402,8 @@ int amdgpu_cs_ioctl(struct drm_device *dev, void *data, struct drm_file *filp) > return r; > } > > + trace_amdgpu_cs_ioctl2(data); > + > r = amdgpu_cs_pass1(&parser, data); > if (r) > goto error_fini; > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h > index e8ea1cfe7027..24e95560ede5 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h > @@ -189,6 +189,18 @@ TRACE_EVENT(amdgpu_cs_ioctl, > __entry->seqno, __get_str(ring), __entry->num_ibs) > ); > > +TRACE_EVENT(amdgpu_cs_ioctl2, > + TP_PROTO(union drm_amdgpu_cs *cs), > + TP_ARGS(cs), > + TP_STRUCT__entry( > + __field(uint32_t, ctx_id) > + ), > + TP_fast_assign( > + __entry->ctx_id = cs->in.ctx_id; > + ), > + TP_printk("context=%u", __entry->ctx_id) > +); > + > TRACE_EVENT(amdgpu_sched_run_job, > TP_PROTO(struct amdgpu_job *job), > TP_ARGS(job),
Le 14/02/2024 à 13:09, Christian König a écrit : > Am 13.02.24 um 16:50 schrieb Pierre-Eric Pelloux-Prayer: >> amdgpu_cs_ioctl already exists but serves a different >> purpose. >> >> amdgpu_cs_ioctl2 marks the beginning of the kernel processing of >> the ioctl which is useful for tools to map which events belong to >> the same submission (without this, the first event would be the >> amdgpu_bo_set_list ones). > > That's not necessary a good justification for the naming. What exactly was the original trace_amdgpu_cs_ioctl() doing? > trace_amdgpu_cs_ioctl is used right before drm_sched_entity_push_job is called so in a sense it's a duplicate of trace_drm_sched_job. That being said, it's used by gpuvis so I chose to not modify it. As for the new event: initially I named it "amdgpu_cs_parser_init", but since the intent is to mark the beginning of the amdgpu_cs_submit I've renamed it. Any suggestion for a better name? Thanks, Pierre-Eric > Regards, > Christian. > >> >> Signed-off-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com> >> --- >> drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 2 ++ >> drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h | 12 ++++++++++++ >> 2 files changed, 14 insertions(+) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c >> index 6830892383c3..29e43a66d0d6 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c >> @@ -1402,6 +1402,8 @@ int amdgpu_cs_ioctl(struct drm_device *dev, void *data, struct drm_file *filp) >> return r; >> } >> + trace_amdgpu_cs_ioctl2(data); >> + >> r = amdgpu_cs_pass1(&parser, data); >> if (r) >> goto error_fini; >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h >> index e8ea1cfe7027..24e95560ede5 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h >> @@ -189,6 +189,18 @@ TRACE_EVENT(amdgpu_cs_ioctl, >> __entry->seqno, __get_str(ring), __entry->num_ibs) >> ); >> +TRACE_EVENT(amdgpu_cs_ioctl2, >> + TP_PROTO(union drm_amdgpu_cs *cs), >> + TP_ARGS(cs), >> + TP_STRUCT__entry( >> + __field(uint32_t, ctx_id) >> + ), >> + TP_fast_assign( >> + __entry->ctx_id = cs->in.ctx_id; >> + ), >> + TP_printk("context=%u", __entry->ctx_id) >> +); >> + >> TRACE_EVENT(amdgpu_sched_run_job, >> TP_PROTO(struct amdgpu_job *job), >> TP_ARGS(job), >
Am 14.02.24 um 17:38 schrieb Pierre-Eric Pelloux-Prayer: > Le 14/02/2024 à 13:09, Christian König a écrit : >> Am 13.02.24 um 16:50 schrieb Pierre-Eric Pelloux-Prayer: >>> amdgpu_cs_ioctl already exists but serves a different >>> purpose. >>> >>> amdgpu_cs_ioctl2 marks the beginning of the kernel processing of >>> the ioctl which is useful for tools to map which events belong to >>> the same submission (without this, the first event would be the >>> amdgpu_bo_set_list ones). >> >> That's not necessary a good justification for the naming. What >> exactly was the original trace_amdgpu_cs_ioctl() doing? >> > > trace_amdgpu_cs_ioctl is used right before drm_sched_entity_push_job > is called so in a sense it's a duplicate > of trace_drm_sched_job. Ah, yes I remember that I wanted to remove that one as well and got pushback. > > That being said, it's used by gpuvis so I chose to not modify it. > > As for the new event: initially I named it "amdgpu_cs_parser_init", > but since the intent is to mark the > beginning of the amdgpu_cs_submit I've renamed it. > > Any suggestion for a better name? How about amdgpu_cs_start ? Regards, Christian. > > Thanks, > Pierre-Eric > > > >> Regards, >> Christian. >> >>> >>> Signed-off-by: Pierre-Eric Pelloux-Prayer >>> <pierre-eric.pelloux-prayer@amd.com> >>> --- >>> drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 2 ++ >>> drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h | 12 ++++++++++++ >>> 2 files changed, 14 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c >>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c >>> index 6830892383c3..29e43a66d0d6 100644 >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c >>> @@ -1402,6 +1402,8 @@ int amdgpu_cs_ioctl(struct drm_device *dev, >>> void *data, struct drm_file *filp) >>> return r; >>> } >>> + trace_amdgpu_cs_ioctl2(data); >>> + >>> r = amdgpu_cs_pass1(&parser, data); >>> if (r) >>> goto error_fini; >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h >>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h >>> index e8ea1cfe7027..24e95560ede5 100644 >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h >>> @@ -189,6 +189,18 @@ TRACE_EVENT(amdgpu_cs_ioctl, >>> __entry->seqno, __get_str(ring), __entry->num_ibs) >>> ); >>> +TRACE_EVENT(amdgpu_cs_ioctl2, >>> + TP_PROTO(union drm_amdgpu_cs *cs), >>> + TP_ARGS(cs), >>> + TP_STRUCT__entry( >>> + __field(uint32_t, ctx_id) >>> + ), >>> + TP_fast_assign( >>> + __entry->ctx_id = cs->in.ctx_id; >>> + ), >>> + TP_printk("context=%u", __entry->ctx_id) >>> +); >>> + >>> TRACE_EVENT(amdgpu_sched_run_job, >>> TP_PROTO(struct amdgpu_job *job), >>> TP_ARGS(job), >>
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c index 6830892383c3..29e43a66d0d6 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c @@ -1402,6 +1402,8 @@ int amdgpu_cs_ioctl(struct drm_device *dev, void *data, struct drm_file *filp) return r; } + trace_amdgpu_cs_ioctl2(data); + r = amdgpu_cs_pass1(&parser, data); if (r) goto error_fini; diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h index e8ea1cfe7027..24e95560ede5 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h @@ -189,6 +189,18 @@ TRACE_EVENT(amdgpu_cs_ioctl, __entry->seqno, __get_str(ring), __entry->num_ibs) ); +TRACE_EVENT(amdgpu_cs_ioctl2, + TP_PROTO(union drm_amdgpu_cs *cs), + TP_ARGS(cs), + TP_STRUCT__entry( + __field(uint32_t, ctx_id) + ), + TP_fast_assign( + __entry->ctx_id = cs->in.ctx_id; + ), + TP_printk("context=%u", __entry->ctx_id) +); + TRACE_EVENT(amdgpu_sched_run_job, TP_PROTO(struct amdgpu_job *job), TP_ARGS(job),
amdgpu_cs_ioctl already exists but serves a different purpose. amdgpu_cs_ioctl2 marks the beginning of the kernel processing of the ioctl which is useful for tools to map which events belong to the same submission (without this, the first event would be the amdgpu_bo_set_list ones). Signed-off-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com> --- drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 2 ++ drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h | 12 ++++++++++++ 2 files changed, 14 insertions(+)