[Intel-gfx] [PATCH v3] drm/i915/tracing: Enable user interrupts while intel_engine_notify is active
Tvrtko Ursulin
tvrtko.ursulin at linux.intel.com
Thu Aug 9 09:09:39 UTC 2018
On 09/08/2018 10:07, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>
> Keep the user interrupt enabled and emit intel_engine_notify tracepoint
> every time as long as it is enabled. Premise is that if someone is
> listening, they want to see interrupts logged.
>
> We use tracepoint (de)registration callbacks to enable user interrupts on
> all devices (future proofing and avoiding ugly global pointers) and all
> engines. And in the user interrupt handler we make sure
> trace_intel_engine_notify is called even when there are no waiters.
>
> v2:
> * Improve makefile. (Chris Wilson)
> * Simplify by dropping the pointeless global driver list. (Chris Wilson)
> * Emit tracepoint when there are no waiters, not just the user interrupt.
> * Commit message tidy.
>
> v3:
> * Favour one return from notify_ring.
> Chris Wilson:
> * Reword commit message a bit for clarity.
> * Add RPM references to driver (un)registration paths.
> * Rename list link member.
> * Handle !CONFIG_TRACEPOINTS in the header.
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Dmitry Rogozhkin <dmitry.v.rogozhkin at intel.com>
> Cc: John Harrison <John.C.Harrison at intel.com>
> Cc: svetlana.kukanova at intel.com
> Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>
Also forgot to credit for tracepoint hooks discovery:
Suggested-by: Chris Wilson <chris at chris-wilson.co.uk>
Regards,
Tvrtko
> ---
> drivers/gpu/drm/i915/Makefile | 3 +
> drivers/gpu/drm/i915/i915_drv.c | 5 ++
> drivers/gpu/drm/i915/i915_drv.h | 2 +
> drivers/gpu/drm/i915/i915_irq.c | 5 +-
> drivers/gpu/drm/i915/i915_trace.h | 50 +++++++-------
> drivers/gpu/drm/i915/i915_tracing.c | 100 ++++++++++++++++++++++++++++
> drivers/gpu/drm/i915/i915_tracing.h | 29 ++++++++
> 7 files changed, 169 insertions(+), 25 deletions(-)
> create mode 100644 drivers/gpu/drm/i915/i915_tracing.c
> create mode 100644 drivers/gpu/drm/i915/i915_tracing.h
>
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 5794f102f9b8..dfc940b32078 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -182,6 +182,9 @@ i915-y += i915_perf.o \
> i915_oa_cnl.o \
> i915_oa_icl.o
>
> +# tracing
> +i915-$(CONFIG_TRACEPOINTS) += i915_tracing.o
> +
> ifeq ($(CONFIG_DRM_I915_GVT),y)
> i915-y += intel_gvt.o
> include $(src)/gvt/Makefile
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 9dce55182c3a..03e224ebc28c 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1281,6 +1281,9 @@ static void i915_driver_register(struct drm_i915_private *dev_priv)
> */
> if (INTEL_INFO(dev_priv)->num_pipes)
> drm_kms_helper_poll_init(dev);
> +
> + /* Notify our tracepoints driver has been registered. */
> + i915_tracing_register(dev_priv);
> }
>
> /**
> @@ -1292,6 +1295,8 @@ static void i915_driver_unregister(struct drm_i915_private *dev_priv)
> intel_fbdev_unregister(dev_priv);
> intel_audio_deinit(dev_priv);
>
> + i915_tracing_unregister(dev_priv);
> +
> /*
> * After flushing the fbdev (incl. a late async config which will
> * have delayed queuing of a hotplug event), then flush the hotplug
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 0b10a30b7d96..00d9e9f65739 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2141,6 +2141,8 @@ struct drm_i915_private {
>
> struct i915_pmu pmu;
>
> + struct list_head tracing_link;
> +
> /*
> * NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
> * will be rejected. Instead look for a better place.
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 8084e35b25c5..0f007a46249e 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1157,10 +1157,10 @@ static void notify_ring(struct intel_engine_cs *engine)
> const u32 seqno = intel_engine_get_seqno(engine);
> struct i915_request *rq = NULL;
> struct task_struct *tsk = NULL;
> - struct intel_wait *wait;
> + struct intel_wait *wait = NULL;
>
> if (unlikely(!engine->breadcrumbs.irq_armed))
> - return;
> + goto out;
>
> rcu_read_lock();
>
> @@ -1219,6 +1219,7 @@ static void notify_ring(struct intel_engine_cs *engine)
>
> rcu_read_unlock();
>
> +out:
> trace_intel_engine_notify(engine, wait);
> }
>
> diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h
> index b50c6b829715..212e7fc1e80e 100644
> --- a/drivers/gpu/drm/i915/i915_trace.h
> +++ b/drivers/gpu/drm/i915/i915_trace.h
> @@ -8,6 +8,7 @@
>
> #include <drm/drmP.h>
> #include "i915_drv.h"
> +#include "i915_tracing.h"
> #include "intel_drv.h"
> #include "intel_ringbuffer.h"
>
> @@ -780,29 +781,32 @@ trace_i915_request_out(struct i915_request *rq)
> #endif
> #endif
>
> -TRACE_EVENT(intel_engine_notify,
> - TP_PROTO(struct intel_engine_cs *engine, bool waiters),
> - TP_ARGS(engine, waiters),
> -
> - TP_STRUCT__entry(
> - __field(u32, dev)
> - __field(u16, class)
> - __field(u16, instance)
> - __field(u32, seqno)
> - __field(bool, waiters)
> - ),
> -
> - TP_fast_assign(
> - __entry->dev = engine->i915->drm.primary->index;
> - __entry->class = engine->uabi_class;
> - __entry->instance = engine->instance;
> - __entry->seqno = intel_engine_get_seqno(engine);
> - __entry->waiters = waiters;
> - ),
> -
> - TP_printk("dev=%u, engine=%u:%u, seqno=%u, waiters=%u",
> - __entry->dev, __entry->class, __entry->instance,
> - __entry->seqno, __entry->waiters)
> +TRACE_EVENT_FN(intel_engine_notify,
> + TP_PROTO(struct intel_engine_cs *engine, bool waiters),
> + TP_ARGS(engine, waiters),
> +
> + TP_STRUCT__entry(
> + __field(u32, dev)
> + __field(u16, class)
> + __field(u16, instance)
> + __field(u32, seqno)
> + __field(bool, waiters)
> + ),
> +
> + TP_fast_assign(
> + __entry->dev = engine->i915->drm.primary->index;
> + __entry->class = engine->uabi_class;
> + __entry->instance = engine->instance;
> + __entry->seqno = intel_engine_get_seqno(engine);
> + __entry->waiters = waiters;
> + ),
> +
> + TP_printk("dev=%u, engine=%u:%u, seqno=%u, waiters=%u",
> + __entry->dev, __entry->class, __entry->instance,
> + __entry->seqno, __entry->waiters),
> +
> + intel_engine_notify_tracepoint_register,
> + intel_engine_notify_tracepoint_unregister
> );
>
> DEFINE_EVENT(i915_request, i915_request_retire,
> diff --git a/drivers/gpu/drm/i915/i915_tracing.c b/drivers/gpu/drm/i915/i915_tracing.c
> new file mode 100644
> index 000000000000..65074f0fcf95
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/i915_tracing.c
> @@ -0,0 +1,100 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright © 2018 Intel Corporation
> + *
> + */
> +
> +#include "i915_tracing.h"
> +
> +#include "i915_drv.h"
> +#include "intel_ringbuffer.h"
> +
> +static DEFINE_MUTEX(driver_list_lock);
> +static LIST_HEAD(driver_list);
> +static bool notify_enabled;
> +
> +static void __i915_enable_notify(struct drm_i915_private *i915)
> +{
> + struct intel_engine_cs *engine;
> + enum intel_engine_id id;
> +
> + intel_runtime_pm_get(i915);
> +
> + for_each_engine(engine, i915, id)
> + intel_engine_pin_breadcrumbs_irq(engine);
> +
> + intel_runtime_pm_put(i915);
> +}
> +
> +static void __i915_disable_notify(struct drm_i915_private *i915)
> +{
> + struct intel_engine_cs *engine;
> + enum intel_engine_id id;
> +
> + intel_runtime_pm_get(i915);
> +
> + for_each_engine(engine, i915, id)
> + intel_engine_unpin_breadcrumbs_irq(engine);
> +
> + intel_runtime_pm_put(i915);
> +}
> +
> +void i915_tracing_register(struct drm_i915_private *i915)
> +{
> + INIT_LIST_HEAD(&i915->tracing_link);
> +
> + mutex_lock(&driver_list_lock);
> +
> + list_add_tail(&i915->tracing_link, &driver_list);
> +
> + if (notify_enabled)
> + __i915_enable_notify(i915);
> +
> + mutex_unlock(&driver_list_lock);
> +}
> +
> +void i915_tracing_unregister(struct drm_i915_private *i915)
> +{
> + mutex_lock(&driver_list_lock);
> +
> + if (notify_enabled)
> + __i915_disable_notify(i915);
> +
> + list_del(&i915->tracing_link);
> +
> + mutex_unlock(&driver_list_lock);
> +}
> +
> +int intel_engine_notify_tracepoint_register(void)
> +{
> + struct drm_i915_private *i915;
> +
> + mutex_lock(&driver_list_lock);
> +
> + GEM_BUG_ON(notify_enabled);
> +
> + list_for_each_entry(i915, &driver_list, tracing_link)
> + __i915_enable_notify(i915);
> +
> + notify_enabled = true;
> +
> + mutex_unlock(&driver_list_lock);
> +
> + return 0;
> +}
> +
> +void intel_engine_notify_tracepoint_unregister(void)
> +{
> + struct drm_i915_private *i915;
> +
> + mutex_lock(&driver_list_lock);
> +
> + GEM_BUG_ON(!notify_enabled);
> +
> + list_for_each_entry(i915, &driver_list, tracing_link)
> + __i915_disable_notify(i915);
> +
> + notify_enabled = false;
> +
> + mutex_unlock(&driver_list_lock);
> +}
> diff --git a/drivers/gpu/drm/i915/i915_tracing.h b/drivers/gpu/drm/i915/i915_tracing.h
> new file mode 100644
> index 000000000000..4d8710f5687b
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/i915_tracing.h
> @@ -0,0 +1,29 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright © 2018 Intel Corporation
> + *
> + */
> +#ifndef _I915_TRACING_H_
> +#define _I915_TRACING_H_
> +
> +#if IS_ENABLED(CONFIG_TRACEPOINTS)
> +
> +#include "i915_drv.h"
> +
> +void i915_tracing_register(struct drm_i915_private *i915);
> +void i915_tracing_unregister(struct drm_i915_private *i915);
> +
> +int intel_engine_notify_tracepoint_register(void);
> +void intel_engine_notify_tracepoint_unregister(void);
> +
> +#else
> +
> +static inline void i915_tracing_register(struct drm_i915_private *i915) { }
> +static inline void i915_tracing_unregister(struct drm_i915_private *i915) { }
> +
> +static inline int intel_engine_notify_tracepoint_register(void) { }
> +static inline void intel_engine_notify_tracepoint_unregister(void) { }
> +
> +#endif
> +
> +#endif
>
More information about the Intel-gfx
mailing list