Re: [PATCH 1/2] drm/i915/tracepoints: Don't compile-out low-level tracepoints

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 11/09/2017 14:09, Michał Winiarski wrote:
> There's no reason to hide those tracepoints.
> Let's also remove the DRM_I915_LOW_LEVEL_TRACEPOINTS Kconfig option.

No numbers from (micro-)bechmarks showing how small the impact of doing 
this is? I thought John was compiling this data. It will be just a no-op 
on the fast path, but a bit more generated code.

Assuming that will be fine, the only potentially problematic aspect that 
comes to mind is the fact meaning of these tracepoints is a bit 
different between execlists and guc. But maybe that is thinking to low 
level (!) - in fact they are in both cases at points where i915 is 
passing/receiving requests to/from hardware so not an issue?

Regards,

Tvrtko

> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: John Harrison <john.c.harrison@intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
> ---
>   drivers/gpu/drm/i915/Kconfig.debug | 11 -----------
>   drivers/gpu/drm/i915/i915_trace.h  | 24 ------------------------
>   2 files changed, 35 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/Kconfig.debug b/drivers/gpu/drm/i915/Kconfig.debug
> index aed7d207ea84..63bfac79a403 100644
> --- a/drivers/gpu/drm/i915/Kconfig.debug
> +++ b/drivers/gpu/drm/i915/Kconfig.debug
> @@ -90,17 +90,6 @@ config DRM_I915_SELFTEST
>   
>   	  If in doubt, say "N".
>   
> -config DRM_I915_LOW_LEVEL_TRACEPOINTS
> -        bool "Enable low level request tracing events"
> -        depends on DRM_I915
> -        default n
> -        help
> -          Choose this option to turn on low level request tracing events.
> -          This provides the ability to precisely monitor engine utilisation
> -          and also analyze the request dependency resolving timeline.
> -
> -          If in doubt, say "N".
> -
>   config DRM_I915_DEBUG_VBLANK_EVADE
>   	bool "Enable extra debug warnings for vblank evasion"
>   	depends on DRM_I915
> diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h
> index 92f4c5bb7aa7..b8f037986ae2 100644
> --- a/drivers/gpu/drm/i915/i915_trace.h
> +++ b/drivers/gpu/drm/i915/i915_trace.h
> @@ -702,7 +702,6 @@ DEFINE_EVENT(i915_gem_request, i915_gem_request_add,
>   	    TP_ARGS(req)
>   );
>   
> -#if defined(CONFIG_DRM_I915_LOW_LEVEL_TRACEPOINTS)
>   DEFINE_EVENT(i915_gem_request, i915_gem_request_submit,
>   	     TP_PROTO(struct drm_i915_gem_request *req),
>   	     TP_ARGS(req)
> @@ -751,29 +750,6 @@ DEFINE_EVENT(i915_gem_request, i915_gem_request_out,
>   	     TP_PROTO(struct drm_i915_gem_request *req),
>   	     TP_ARGS(req)
>   );
> -#else
> -#if !defined(TRACE_HEADER_MULTI_READ)
> -static inline void
> -trace_i915_gem_request_submit(struct drm_i915_gem_request *req)
> -{
> -}
> -
> -static inline void
> -trace_i915_gem_request_execute(struct drm_i915_gem_request *req)
> -{
> -}
> -
> -static inline void
> -trace_i915_gem_request_in(struct drm_i915_gem_request *req, unsigned int port)
> -{
> -}
> -
> -static inline void
> -trace_i915_gem_request_out(struct drm_i915_gem_request *req)
> -{
> -}
> -#endif
> -#endif
>   
>   TRACE_EVENT(intel_engine_notify,
>   	    TP_PROTO(struct intel_engine_cs *engine, bool waiters),
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx