diff mbox series

[v6,7/7] drm/doc: document some tracepoints as uAPI

Message ID 20241114100113.150647-8-pierre-eric.pelloux-prayer@amd.com (mailing list archive)
State New, archived
Headers show
Series Improve gpu_scheduler trace events + uAPI | expand

Commit Message

Pierre-Eric Pelloux-Prayer Nov. 14, 2024, 10:01 a.m. UTC
This commit adds a document section in drm-uapi.rst about tracepoints,
and mark the events gpu_scheduler_trace.h as stable uAPI.

The goal is to explicitly state that tools can rely on the fields,
formats and semantics of these events.

Acked-by: Lucas Stach <l.stach@pengutronix.de>
Acked-by: Maíra Canal <mcanal@igalia.com>
Signed-off-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>
---
 Documentation/gpu/drm-uapi.rst                | 19 ++++++++++++++++
 .../gpu/drm/scheduler/gpu_scheduler_trace.h   | 22 +++++++++++++++++++
 2 files changed, 41 insertions(+)

Comments

Lucas Stach Nov. 14, 2024, 11:30 a.m. UTC | #1
Hi,

Am Donnerstag, dem 14.11.2024 um 11:01 +0100 schrieb Pierre-Eric
Pelloux-Prayer:
> This commit adds a document section in drm-uapi.rst about tracepoints,
> and mark the events gpu_scheduler_trace.h as stable uAPI.
> 
> The goal is to explicitly state that tools can rely on the fields,
> formats and semantics of these events.
> 
> Acked-by: Lucas Stach <l.stach@pengutronix.de>
> Acked-by: Maíra Canal <mcanal@igalia.com>
> Signed-off-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>
> ---
>  Documentation/gpu/drm-uapi.rst                | 19 ++++++++++++++++
>  .../gpu/drm/scheduler/gpu_scheduler_trace.h   | 22 +++++++++++++++++++
>  2 files changed, 41 insertions(+)
> 
> diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst
> index b75cc9a70d1f..9603ac0f4c09 100644
> --- a/Documentation/gpu/drm-uapi.rst
> +++ b/Documentation/gpu/drm-uapi.rst
> @@ -583,3 +583,22 @@ dma-buf interoperability
>  
>  Please see Documentation/userspace-api/dma-buf-alloc-exchange.rst for
>  information on how dma-buf is integrated and exposed within DRM.
> +
> +
> +Trace events
> +============
> +
> +See Documentation/trace/tracepoints.rst for information about using
> +Linux Kernel Tracepoints.
> +In the DRM subsystem, some events are considered stable uAPI to avoid
> +breaking tools (e.g.: GPUVis, umr) relying on them. Stable means that fields
> +cannot be removed, nor their formatting updated. Adding new fields is
> +possible, under the normal uAPI requirements.
> +
> +Stable uAPI events
> +------------------
> +
> +From ``drivers/gpu/drm/scheduler/gpu_scheduler_trace.h``
> +
> +.. kernel-doc::  drivers/gpu/drm/scheduler/gpu_scheduler_trace.h
> +   :doc: uAPI trace events
> \ No newline at end of file
> diff --git a/drivers/gpu/drm/scheduler/gpu_scheduler_trace.h b/drivers/gpu/drm/scheduler/gpu_scheduler_trace.h
> index 8340c7c0c6b6..ec230e558961 100644
> --- a/drivers/gpu/drm/scheduler/gpu_scheduler_trace.h
> +++ b/drivers/gpu/drm/scheduler/gpu_scheduler_trace.h
> @@ -33,6 +33,28 @@
>  #define TRACE_SYSTEM gpu_scheduler
>  #define TRACE_INCLUDE_FILE gpu_scheduler_trace
>  
> +
> +/**
> + * DOC: uAPI trace events
> + *
> + * ``drm_sched_job``, ``drm_run_job``, ``drm_sched_process_job``,
> + * and ``drm_sched_job_wait_dep`` are considered stable uAPI.
> + *
> + * Common trace events attributes:
> + *
> + * * ``id``    - this is &drm_sched_job->id. It uniquely idenfies a job
> + *   inside a &struct drm_gpu_scheduler.
> + *
> + * * ``dev``   - the dev_name() of the device running the job.
> + *
> + * * ``ring``  - the hardware ring running the job. Together with ``dev`` it
> + *   uniquely identifies where the job is going to be executed.
> + *
It might be nitpicky, but as we change the format of the tracepoints
anyway and are about to declare them a ABI: I don't really like the
ring name. Yes, in most hardware implementations today the mechanism to
queue jobs is a ring buffer, but there are other mechanisms to schedule
jobs (see for example the lima driver). Maybe we could rename this to
something a bit more generic like "dev_queue" or something like that?

Regards,
Lucas

> + * * ``fence`` - the &dma_fence.context and the &dma_fence.seqno of
> + *   &drm_sched_fence.finished
> + *
> + */
> +
>  #ifndef __TRACE_EVENT_GPU_SCHEDULER_PRINT_FN
>  #define __TRACE_EVENT_GPU_SCHEDULER_PRINT_FN
>  /* Similar to trace_print_array_seq but for fences. */
Philipp Stanner Nov. 15, 2024, 3:22 p.m. UTC | #2
On Thu, 2024-11-14 at 12:30 +0100, Lucas Stach wrote:
> Hi,
> 
> Am Donnerstag, dem 14.11.2024 um 11:01 +0100 schrieb Pierre-Eric
> Pelloux-Prayer:
> > This commit adds a document section in drm-uapi.rst about
> > tracepoints,
> > and mark the events gpu_scheduler_trace.h as stable uAPI.
> > 
> > The goal is to explicitly state that tools can rely on the fields,
> > formats and semantics of these events.
> > 
> > Acked-by: Lucas Stach <l.stach@pengutronix.de>
> > Acked-by: Maíra Canal <mcanal@igalia.com>
> > Signed-off-by: Pierre-Eric Pelloux-Prayer
> > <pierre-eric.pelloux-prayer@amd.com>
> > ---
> >  Documentation/gpu/drm-uapi.rst                | 19
> > ++++++++++++++++
> >  .../gpu/drm/scheduler/gpu_scheduler_trace.h   | 22
> > +++++++++++++++++++
> >  2 files changed, 41 insertions(+)
> > 
> > diff --git a/Documentation/gpu/drm-uapi.rst
> > b/Documentation/gpu/drm-uapi.rst
> > index b75cc9a70d1f..9603ac0f4c09 100644
> > --- a/Documentation/gpu/drm-uapi.rst
> > +++ b/Documentation/gpu/drm-uapi.rst
> > @@ -583,3 +583,22 @@ dma-buf interoperability
> >  
> >  Please see Documentation/userspace-api/dma-buf-alloc-exchange.rst
> > for
> >  information on how dma-buf is integrated and exposed within DRM.
> > +
> > +
> > +Trace events
> > +============
> > +
> > +See Documentation/trace/tracepoints.rst for information about
> > using
> > +Linux Kernel Tracepoints.
> > +In the DRM subsystem, some events are considered stable uAPI to
> > avoid
> > +breaking tools (e.g.: GPUVis, umr) relying on them. Stable means
> > that fields
> > +cannot be removed, nor their formatting updated. Adding new fields
> > is
> > +possible, under the normal uAPI requirements.
> > +
> > +Stable uAPI events
> > +------------------
> > +
> > +From ``drivers/gpu/drm/scheduler/gpu_scheduler_trace.h``
> > +
> > +.. kernel-doc::  drivers/gpu/drm/scheduler/gpu_scheduler_trace.h
> > +   :doc: uAPI trace events
> > \ No newline at end of file
> > diff --git a/drivers/gpu/drm/scheduler/gpu_scheduler_trace.h
> > b/drivers/gpu/drm/scheduler/gpu_scheduler_trace.h
> > index 8340c7c0c6b6..ec230e558961 100644
> > --- a/drivers/gpu/drm/scheduler/gpu_scheduler_trace.h
> > +++ b/drivers/gpu/drm/scheduler/gpu_scheduler_trace.h
> > @@ -33,6 +33,28 @@
> >  #define TRACE_SYSTEM gpu_scheduler
> >  #define TRACE_INCLUDE_FILE gpu_scheduler_trace
> >  
> > +
> > +/**
> > + * DOC: uAPI trace events
> > + *
> > + * ``drm_sched_job``, ``drm_run_job``, ``drm_sched_process_job``,
> > + * and ``drm_sched_job_wait_dep`` are considered stable uAPI.
> > + *
> > + * Common trace events attributes:
> > + *
> > + * * ``id``    - this is &drm_sched_job->id. It uniquely idenfies
> > a job
> > + *   inside a &struct drm_gpu_scheduler.
> > + *
> > + * * ``dev``   - the dev_name() of the device running the job.
> > + *
> > + * * ``ring``  - the hardware ring running the job. Together with
> > ``dev`` it
> > + *   uniquely identifies where the job is going to be executed.
> > + *
> It might be nitpicky, but as we change the format of the tracepoints
> anyway and are about to declare them a ABI: I don't really like the
> ring name. Yes, in most hardware implementations today the mechanism
> to
> queue jobs is a ring buffer, but there are other mechanisms to
> schedule
> jobs (see for example the lima driver). Maybe we could rename this to
> something a bit more generic like "dev_queue" or something like that?

While it might be true that the term isn't optimal or even formally
correct, it is the term which is used everywhere, including
presentations at conferences such as XDC and Plumbers.

So I think the price we'd pay for breaking consistency with the
established term would be higher than the gained formal correctness.

Greetings,
P.


> 
> Regards,
> Lucas
> 
> > + * * ``fence`` - the &dma_fence.context and the &dma_fence.seqno
> > of
> > + *   &drm_sched_fence.finished
> > + *
> > + */
> > +
> >  #ifndef __TRACE_EVENT_GPU_SCHEDULER_PRINT_FN
> >  #define __TRACE_EVENT_GPU_SCHEDULER_PRINT_FN
> >  /* Similar to trace_print_array_seq but for fences. */
>
diff mbox series

Patch

diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst
index b75cc9a70d1f..9603ac0f4c09 100644
--- a/Documentation/gpu/drm-uapi.rst
+++ b/Documentation/gpu/drm-uapi.rst
@@ -583,3 +583,22 @@  dma-buf interoperability
 
 Please see Documentation/userspace-api/dma-buf-alloc-exchange.rst for
 information on how dma-buf is integrated and exposed within DRM.
+
+
+Trace events
+============
+
+See Documentation/trace/tracepoints.rst for information about using
+Linux Kernel Tracepoints.
+In the DRM subsystem, some events are considered stable uAPI to avoid
+breaking tools (e.g.: GPUVis, umr) relying on them. Stable means that fields
+cannot be removed, nor their formatting updated. Adding new fields is
+possible, under the normal uAPI requirements.
+
+Stable uAPI events
+------------------
+
+From ``drivers/gpu/drm/scheduler/gpu_scheduler_trace.h``
+
+.. kernel-doc::  drivers/gpu/drm/scheduler/gpu_scheduler_trace.h
+   :doc: uAPI trace events
\ No newline at end of file
diff --git a/drivers/gpu/drm/scheduler/gpu_scheduler_trace.h b/drivers/gpu/drm/scheduler/gpu_scheduler_trace.h
index 8340c7c0c6b6..ec230e558961 100644
--- a/drivers/gpu/drm/scheduler/gpu_scheduler_trace.h
+++ b/drivers/gpu/drm/scheduler/gpu_scheduler_trace.h
@@ -33,6 +33,28 @@ 
 #define TRACE_SYSTEM gpu_scheduler
 #define TRACE_INCLUDE_FILE gpu_scheduler_trace
 
+
+/**
+ * DOC: uAPI trace events
+ *
+ * ``drm_sched_job``, ``drm_run_job``, ``drm_sched_process_job``,
+ * and ``drm_sched_job_wait_dep`` are considered stable uAPI.
+ *
+ * Common trace events attributes:
+ *
+ * * ``id``    - this is &drm_sched_job->id. It uniquely idenfies a job
+ *   inside a &struct drm_gpu_scheduler.
+ *
+ * * ``dev``   - the dev_name() of the device running the job.
+ *
+ * * ``ring``  - the hardware ring running the job. Together with ``dev`` it
+ *   uniquely identifies where the job is going to be executed.
+ *
+ * * ``fence`` - the &dma_fence.context and the &dma_fence.seqno of
+ *   &drm_sched_fence.finished
+ *
+ */
+
 #ifndef __TRACE_EVENT_GPU_SCHEDULER_PRINT_FN
 #define __TRACE_EVENT_GPU_SCHEDULER_PRINT_FN
 /* Similar to trace_print_array_seq but for fences. */