On Feb 25, 2017 4:40 AM, "Christian König" <deathsim...@vodafone.de> wrote:
Am 24.02.2017 um 19:20 schrieb Andres Rodriguez:
> This information is intended to provide the required data to associate
> amdgpu tracepoints with their corresponding dma_fence_* events.
>
> Signed-off-by: Andres Rodriguez <andre...@gmail.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
> index 01623d1..cc9a31d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
> @@ -130,6 +130,9 @@ TRACE_EVENT(amdgpu_sched_run_job,
> __field(struct amd_sched_job *, sched_job)
> __field(struct amdgpu_ib *, ib)
> __field(struct dma_fence *, fence)
> + __string(timeline,
> job->base.s_fence->finished.ops->get_timeline_name(&job->bas
> e.s_fence->finished))
> + __field(unsigned int, context)
> + __field(unsigned int, seqno)
> __field(char *, ring_name)
> __field(u32, num_ibs)
> ),
> @@ -139,12 +142,16 @@ TRACE_EVENT(amdgpu_sched_run_job,
> __entry->sched_job = &job->base;
> __entry->ib = job->ibs;
> __entry->fence = &job->base.s_fence->finished;
> + __assign_str(timeline,
> job->base.s_fence->finished.ops->get_timeline_name(&job->bas
> e.s_fence->finished))
>
Not 100% sure, but we might be able to use a char * field here instead of
the extra overhead of embedding a string, the timeline names are never
freed IIRC.
I'll give this a try.
+ __entry->context = job->base.s_fence->finished.co
> ntext;
> + __entry->seqno = job->base.s_fence->finished.se
> qno;
> __entry->ring_name = job->ring->name;
> __entry->num_ibs = job->num_ibs;
> ),
> - TP_printk("adev=%p, sched_job=%p, first ib=%p, sched fence=%p,
> ring name=%s, num_ibs=%u",
> + TP_printk("adev=%p, sched_job=%p, first ib=%p, sched fence=%p,
> timeline=%s, context=%u, seqno=%u, ring name=%s, num_ibs=%u",
>
If you have time for another patch please drop the pinters here as well.
Scheduler job, IBs and fence are all heavily reallocated (sometimes even
with a slab allocator). So the pointers are completely meaningless. The
adev pointer is superseded by the timeline name and context numbers.
The pointers do provide some useful data. But you need to make sure you
read it in between an allocation event and a fence signal event. It is
extremely terrible for reading from a trace text file.
How do you feel about if we replaced the job pointer with a job id, and
replaced the fence pointers with the fence data?
Anyway that should be done in an extra patch, so this one is Reviewed-by:
Christian König <christian.koe...@amd.com>.
Regards,
Christian.
__entry->adev, __entry->sched_job, __entry->ib,
> - __entry->fence, __entry->ring_name, __entry->num_ibs)
> + __entry->fence, __get_str(timeline),
> __entry->context, __entry->seqno,
> + __entry->ring_name, __entry->num_ibs)
> );
>
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
