[Intel-gfx] [PATCH] drm/i915/trace: Hide backend specific fields behind Kconfig

[John Harrison]

On 10/25/2021 09:34, Matthew Brost wrote:
> Hide the guc_id and tail fields, for request trace points, behind
> CONFIG_DRM_I915_LOW_LEVEL_TRACEPOINTS Kconfig option. Trace points
> are ABI (maybe?) so don't change them without kernel developers Kconfig
> options.
The i915 sw arch team have previously hard blocked requests for changes 
to trace points from user land tool developers on the grounds that trace 
points are not ABI and are free to change at whim as and when the i915 
internal implementation changes. They are purely for use of developers 
to debug the i915 driver as the i915 driver currently stands at any 
given instant.

So I don't see how it can be argued that we must not update any trace 
points to allow for debugging of i915 scheduling issues on current 
platforms. And having to enable extra config options just to keep 
existing higher level trace points usable seems broken.

John.


>
> Signed-off-by: Matthew Brost <matthew.brost at intel.com>
> ---
>   drivers/gpu/drm/i915/i915_trace.h | 27 +++++++++++++++++++++++++++
>   1 file changed, 27 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h
> index 9795f456cccf..4f5238d02b51 100644
> --- a/drivers/gpu/drm/i915/i915_trace.h
> +++ b/drivers/gpu/drm/i915/i915_trace.h
> @@ -787,6 +787,7 @@ TRACE_EVENT(i915_request_queue,
>   		      __entry->ctx, __entry->seqno, __entry->flags)
>   );
>   
> +#if defined(CONFIG_DRM_I915_LOW_LEVEL_TRACEPOINTS)
>   DECLARE_EVENT_CLASS(i915_request,
>   	    TP_PROTO(struct i915_request *rq),
>   	    TP_ARGS(rq),
> @@ -816,6 +817,32 @@ DECLARE_EVENT_CLASS(i915_request,
>   		      __entry->guc_id, __entry->ctx, __entry->seqno,
>   		      __entry->tail)
>   );
> +#else
> +DECLARE_EVENT_CLASS(i915_request,
> +	    TP_PROTO(struct i915_request *rq),
> +	    TP_ARGS(rq),
> +
> +	    TP_STRUCT__entry(
> +			     __field(u32, dev)
> +			     __field(u64, ctx)
> +			     __field(u16, class)
> +			     __field(u16, instance)
> +			     __field(u32, seqno)
> +			     ),
> +
> +	    TP_fast_assign(
> +			   __entry->dev = rq->engine->i915->drm.primary->index;
> +			   __entry->class = rq->engine->uabi_class;
> +			   __entry->instance = rq->engine->uabi_instance;
> +			   __entry->ctx = rq->fence.context;
> +			   __entry->seqno = rq->fence.seqno;
> +			   ),
> +
> +	    TP_printk("dev=%u, engine=%u:%u, ctx=%llu, seqno=%u",
> +		      __entry->dev, __entry->class, __entry->instance,
> +		      __entry->ctx, __entry->seqno)
> +);
> +#endif
>   
>   DEFINE_EVENT(i915_request, i915_request_add,
>   	     TP_PROTO(struct i915_request *rq),