On 08/08/2018 12:11, Chris Wilson wrote:
Quoting Tvrtko Ursulin (2018-06-25 18:32:37)From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx> Keep the user interrupt enabled while intel_engine_notify tracepoint is enabled. We use tracepoint (de)registration callbacks to enable user interrupts on all devices (future proofing and avoiding ugly global pointers) and all engines. Premise is that if someone is listening, they want to see interrupts logged. Commit to be improved... Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> --- drivers/gpu/drm/i915/Makefile | 1 + drivers/gpu/drm/i915/i915_drv.c | 2 + drivers/gpu/drm/i915/i915_drv.h | 2 + drivers/gpu/drm/i915/i915_trace.h | 50 ++++++++------- drivers/gpu/drm/i915/i915_tracing.c | 96 +++++++++++++++++++++++++++++ drivers/gpu/drm/i915/i915_tracing.h | 13 ++++ 6 files changed, 141 insertions(+), 23 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 4c6adae23e18..ee082addd328 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -77,6 +77,7 @@ i915-y += i915_cmd_parser.o \ i915_request.o \ i915_timeline.o \ i915_trace_points.o \ + i915_tracing.o \No fancy obj-$(TRACING) ? :)
Just a miss, marking as TODO.
i915_vma.o \ intel_breadcrumbs.o \ intel_engine_cs.o \ diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 8a3ea18d8416..c634583baf57 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -1271,6 +1271,7 @@ static void i915_driver_register(struct drm_i915_private *dev_priv) INIT_LIST_HEAD(&dev_priv->driver_list_link); mutex_lock(&i915_driver_list_lock); list_add_tail(&dev_priv->driver_list_link, &i915_driver_list); + i915_tracing_register(dev_priv); mutex_unlock(&i915_driver_list_lock); }@@ -1285,6 +1286,7 @@ static void i915_driver_unregister(struct drm_i915_private *dev_priv) mutex_lock(&i915_driver_list_lock);list_del(&dev_priv->driver_list_link); + i915_tracing_unregister(dev_priv); mutex_unlock(&i915_driver_list_lock);/*diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 685bfdca3a72..4e3230713491 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2144,6 +2144,8 @@ struct drm_i915_private {struct list_head driver_list_link; + bool engine_notify_tracepoint;+ /* * 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_tracing.c b/drivers/gpu/drm/i915/i915_tracing.c new file mode 100644 index 000000000000..a9f486278109 --- /dev/null +++ b/drivers/gpu/drm/i915/i915_tracing.c @@ -0,0 +1,96 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright © 2018 Intel Corporation + * + */ + +#include "i915_tracing.h" + +#include "i915_drv.h" +#include "intel_ringbuffer.h" + +void i915_tracing_register(struct drm_i915_private *i915) +{ + struct intel_engine_cs *engine; + struct drm_i915_private *p; + enum intel_engine_id id; + bool enable = false; + + lockdep_assert_held(&i915_driver_list_lock); + + list_for_each_entry(p, &i915_driver_list, driver_list_link) { + enable = p->engine_notify_tracepoint; + if (enable) + break; + }All on this list are expected to have the same value of engine_notify_tracepoint. As far as I can tell, engine_notify_tracepoint is just a global and there's no particular advantage in having it per-device. The caveat is that i915_tracing_register/unregister must be called under the same lock as the list_add/del -- I wonder if that device list should just be moved to i915_tracing.c and kept entirely private? (Then any future use case would also likely instantiate their own mutex and list?)
Agreed, probably better to keep it private since exported list is totally useless now. If there are no fundamental objections on the idea I'll re-spin with that change.
Regards, Tvrtko _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
