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

[Philipp Stanner]

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 at pengutronix.de>
> > Acked-by: Maíra Canal <mcanal at igalia.com>
> > Signed-off-by: Pierre-Eric Pelloux-Prayer
> > <pierre-eric.pelloux-prayer at 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. */
>