diff mbox series

[RFC,3/4] drm/i915/perf: handle interrupts from the OA unit

Message ID 20181219143747.8997-4-lionel.g.landwerlin@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915/perf: add OA interrupt support | expand

Commit Message

Lionel Landwerlin Dec. 19, 2018, 2:37 p.m. UTC
The OA unit can notify that its circular buffer is half full through
an interrupt and we would like to give the application the ability to
make use of this interrupt to get rid of CPU checks on the OA buffer.

This change wires up the interrupt to the i915-perf stream and leaves
it ignored for now.

Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h         | 21 +++++++++++++
 drivers/gpu/drm/i915/i915_irq.c         | 39 ++++++++++++++++++++-----
 drivers/gpu/drm/i915/i915_perf.c        | 26 +++++++++++++++++
 drivers/gpu/drm/i915/i915_reg.h         |  7 +++++
 drivers/gpu/drm/i915/intel_ringbuffer.c |  2 ++
 5 files changed, 88 insertions(+), 7 deletions(-)

Comments

Chris Wilson Dec. 19, 2018, 4:19 p.m. UTC | #1
Quoting Lionel Landwerlin (2018-12-19 14:37:46)
>  static void free_oa_config(struct drm_i915_private *dev_priv,
> @@ -1852,6 +1854,13 @@ static void gen7_oa_enable(struct i915_perf_stream *stream)
>          */
>         gen7_init_oa_buffer(dev_priv);
>  
> +       if (stream->oa_interrupt_monitor) {
> +               spin_lock(&dev_priv->irq_lock);

Doesn't look like you are already in interrupt context, so spin_lock_irq()
required for dev_priv->irq_lock.
-Chris
Lionel Landwerlin Dec. 19, 2018, 6:03 p.m. UTC | #2
On 19/12/2018 16:19, Chris Wilson wrote:
> Quoting Lionel Landwerlin (2018-12-19 14:37:46)
>>   static void free_oa_config(struct drm_i915_private *dev_priv,
>> @@ -1852,6 +1854,13 @@ static void gen7_oa_enable(struct i915_perf_stream *stream)
>>           */
>>          gen7_init_oa_buffer(dev_priv);
>>   
>> +       if (stream->oa_interrupt_monitor) {
>> +               spin_lock(&dev_priv->irq_lock);
> Doesn't look like you are already in interrupt context, so spin_lock_irq()
> required for dev_priv->irq_lock.
> -Chris
>
Thanks a lot Chris, will fix.
Any comment on the atomic counter? Does that look like a bad design/idea?

Cheers,

-
Lionel
Chris Wilson Dec. 19, 2018, 6:26 p.m. UTC | #3
Quoting Lionel Landwerlin (2018-12-19 18:03:26)
> On 19/12/2018 16:19, Chris Wilson wrote:
> > Quoting Lionel Landwerlin (2018-12-19 14:37:46)
> >>   static void free_oa_config(struct drm_i915_private *dev_priv,
> >> @@ -1852,6 +1854,13 @@ static void gen7_oa_enable(struct i915_perf_stream *stream)
> >>           */
> >>          gen7_init_oa_buffer(dev_priv);
> >>   
> >> +       if (stream->oa_interrupt_monitor) {
> >> +               spin_lock(&dev_priv->irq_lock);
> > Doesn't look like you are already in interrupt context, so spin_lock_irq()
> > required for dev_priv->irq_lock.
> > -Chris
> >
> Thanks a lot Chris, will fix.
> Any comment on the atomic counter? Does that look like a bad design/idea?

It's not illogical, but conceivably you don't need one at all. You just
use the wakeup from the interrupt as a kick to check buffer status, that
should be cheap enough to not worry about short-circuiting the half-full
interrupt check (and even if we do wakeup twice within the same
half-full period, there may be a few results to move along). Without
adding a hrtimer, there should not be many spurious wakeups to soak up
the cpu.
-Chris
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 3669ed0a6376..014294e33b40 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1354,6 +1354,12 @@  struct i915_perf_stream {
 	 * buffer should be checked for available data.
 	 */
 	u64 poll_oa_period;
+
+	/**
+	 * @oa_interrupt_monitor: Whether the stream will be notified by OA
+	 * interrupts.
+	 */
+	bool oa_interrupt_monitor;
 };
 
 /**
@@ -1845,6 +1851,21 @@  struct drm_i915_private {
 			wait_queue_head_t poll_wq;
 			bool pollin;
 
+			/**
+			 * Atomic counter incremented by the interrupt
+			 * handling code for each OA half full interrupt
+			 * received.
+			 */
+			atomic64_t half_full_count;
+
+			/**
+			 * Copy of the atomic half_full_count that was last
+			 * processed in the i915-perf driver. If both counters
+			 * differ, there is data available to read in the OA
+			 * buffer.
+			 */
+			u64 half_full_count_last;
+
 			/**
 			 * For rate limiting any notifications of spurious
 			 * invalid OA reports
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index d447d7d508f4..b1098118f71e 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1222,6 +1222,12 @@  static void notify_ring(struct intel_engine_cs *engine)
 	trace_intel_engine_notify(engine, wait);
 }
 
+static void notify_perfmon_buffer_half_full(struct drm_i915_private *i915)
+{
+	atomic64_inc(&i915->perf.oa.half_full_count);
+	wake_up_all(&i915->perf.oa.poll_wq);
+}
+
 static void vlv_c0_read(struct drm_i915_private *dev_priv,
 			struct intel_rps_ei *ei)
 {
@@ -1486,6 +1492,9 @@  static void snb_gt_irq_handler(struct drm_i915_private *dev_priv,
 		      GT_RENDER_CS_MASTER_ERROR_INTERRUPT))
 		DRM_DEBUG("Command parser error, gt_iir 0x%08x\n", gt_iir);
 
+	if (gt_iir & GT_PERFMON_BUFFER_HALF_FULL_INTERRUPT)
+		notify_perfmon_buffer_half_full(dev_priv);
+
 	if (gt_iir & GT_PARITY_ERROR(dev_priv))
 		ivybridge_parity_error_irq_handler(dev_priv, gt_iir);
 }
@@ -1507,6 +1516,12 @@  gen8_cs_irq_handler(struct intel_engine_cs *engine, u32 iir)
 		tasklet_hi_schedule(&engine->execlists.tasklet);
 }
 
+static void gen8_perfmon_handler(struct drm_i915_private *i915, u32 iir)
+{
+	if (iir & GEN8_GT_PERFMON_BUFFER_HALF_FULL_INTERRUPT)
+		notify_perfmon_buffer_half_full(i915);
+}
+
 static void gen8_gt_irq_ack(struct drm_i915_private *i915,
 			    u32 master_ctl, u32 gt_iir[4])
 {
@@ -1516,6 +1531,7 @@  static void gen8_gt_irq_ack(struct drm_i915_private *i915,
 		      GEN8_GT_BCS_IRQ | \
 		      GEN8_GT_VCS1_IRQ | \
 		      GEN8_GT_VCS2_IRQ | \
+		      GEN8_GT_WDBOX_OACS_IRQ | \
 		      GEN8_GT_VECS_IRQ | \
 		      GEN8_GT_PM_IRQ | \
 		      GEN8_GT_GUC_IRQ)
@@ -1538,7 +1554,7 @@  static void gen8_gt_irq_ack(struct drm_i915_private *i915,
 			raw_reg_write(regs, GEN8_GT_IIR(2), gt_iir[2]);
 	}
 
-	if (master_ctl & GEN8_GT_VECS_IRQ) {
+	if (master_ctl & (GEN8_GT_VECS_IRQ | GEN8_GT_WDBOX_OACS_IRQ)) {
 		gt_iir[3] = raw_reg_read(regs, GEN8_GT_IIR(3));
 		if (likely(gt_iir[3]))
 			raw_reg_write(regs, GEN8_GT_IIR(3), gt_iir[3]);
@@ -1562,9 +1578,11 @@  static void gen8_gt_irq_handler(struct drm_i915_private *i915,
 				    gt_iir[1] >> GEN8_VCS2_IRQ_SHIFT);
 	}
 
-	if (master_ctl & GEN8_GT_VECS_IRQ) {
+	if (master_ctl & (GEN8_GT_VECS_IRQ | GEN8_GT_WDBOX_OACS_IRQ)) {
 		gen8_cs_irq_handler(i915->engine[VECS],
 				    gt_iir[3] >> GEN8_VECS_IRQ_SHIFT);
+		gen8_perfmon_handler(i915,
+				     gt_iir[3] >> GEN8_WD_IRQ_SHIFT);
 	}
 
 	if (master_ctl & (GEN8_GT_PM_IRQ | GEN8_GT_GUC_IRQ)) {
@@ -3018,6 +3036,8 @@  gen11_other_irq_handler(struct drm_i915_private * const i915,
 {
 	if (instance == OTHER_GTPM_INSTANCE)
 		return gen6_rps_irq_handler(i915, iir);
+	if (instance == OTHER_WDOAPERF_INSTANCE)
+		return gen8_perfmon_handler(i915, iir);
 
 	WARN_ONCE(1, "unhandled other interrupt instance=0x%x, iir=0x%x\n",
 		  instance, iir);
@@ -4051,6 +4071,10 @@  static void gen5_gt_irq_postinstall(struct drm_device *dev)
 		gt_irqs |= GT_BLT_USER_INTERRUPT | GT_BSD_USER_INTERRUPT;
 	}
 
+	/* We only expose the i915/perf interface on HSW+. */
+	if (IS_HASWELL(dev_priv))
+		gt_irqs |= GT_PERFMON_BUFFER_HALF_FULL_INTERRUPT;
+
 	GEN3_IRQ_INIT(GT, dev_priv->gt_irq_mask, gt_irqs);
 
 	if (INTEL_GEN(dev_priv) >= 6) {
@@ -4180,7 +4204,8 @@  static void gen8_gt_irq_postinstall(struct drm_i915_private *dev_priv)
 			GT_CONTEXT_SWITCH_INTERRUPT << GEN8_VCS2_IRQ_SHIFT,
 		0,
 		GT_RENDER_USER_INTERRUPT << GEN8_VECS_IRQ_SHIFT |
-			GT_CONTEXT_SWITCH_INTERRUPT << GEN8_VECS_IRQ_SHIFT
+			GT_CONTEXT_SWITCH_INTERRUPT << GEN8_VECS_IRQ_SHIFT |
+			GEN8_GT_PERFMON_BUFFER_HALF_FULL_INTERRUPT << GEN8_WD_IRQ_SHIFT
 		};
 
 	if (HAS_L3_DPF(dev_priv))
@@ -4302,12 +4327,12 @@  static void gen11_gt_irq_postinstall(struct drm_i915_private *dev_priv)
 
 	/*
 	 * RPS interrupts will get enabled/disabled on demand when RPS itself
-	 * is enabled/disabled.
+	 * is enabled/disabled, just enable the OA interrupt for now.
 	 */
-	dev_priv->pm_ier = 0x0;
+	dev_priv->pm_ier = GEN8_GT_PERFMON_BUFFER_HALF_FULL_INTERRUPT;
 	dev_priv->pm_imr = ~dev_priv->pm_ier;
-	I915_WRITE(GEN11_GPM_WGBOXPERF_INTR_ENABLE, 0);
-	I915_WRITE(GEN11_GPM_WGBOXPERF_INTR_MASK,  ~0);
+	I915_WRITE(GEN11_GPM_WGBOXPERF_INTR_ENABLE, dev_priv->pm_ier);
+	I915_WRITE(GEN11_GPM_WGBOXPERF_INTR_MASK,  dev_priv->pm_imr);
 }
 
 static void icp_irq_postinstall(struct drm_device *dev)
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index a8df58ec7d75..21b05437123d 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -347,6 +347,7 @@  static const struct i915_oa_format gen8_plus_oa_formats[I915_OA_FORMAT_MAX] = {
  * @oa_period_exponent: The OA unit sampling period is derived from this
  * @poll_oa_period: The period at which the CPU will check for OA data
  * availability
+ * @oa_interrupt_monitor: Whether we should monitor the OA interrupt.
  *
  * As read_properties_unlocked() enumerates and validates the properties given
  * to open a stream of metrics the configuration is built up in the structure
@@ -364,6 +365,7 @@  struct perf_open_properties {
 	bool oa_periodic;
 	int oa_period_exponent;
 	u64 poll_oa_period;
+	bool oa_interrupt_monitor;
 };
 
 static void free_oa_config(struct drm_i915_private *dev_priv,
@@ -1852,6 +1854,13 @@  static void gen7_oa_enable(struct i915_perf_stream *stream)
 	 */
 	gen7_init_oa_buffer(dev_priv);
 
+	if (stream->oa_interrupt_monitor) {
+		spin_lock(&dev_priv->irq_lock);
+		gen5_enable_gt_irq(dev_priv,
+				   GT_PERFMON_BUFFER_HALF_FULL_INTERRUPT);
+		spin_unlock(&dev_priv->irq_lock);
+	}
+
 	I915_WRITE(GEN7_OACONTROL,
 		   (ctx_id & GEN7_OACONTROL_CTX_MASK) |
 		   (period_exponent <<
@@ -1878,6 +1887,9 @@  static void gen8_oa_enable(struct i915_perf_stream *stream)
 	 */
 	gen8_init_oa_buffer(dev_priv);
 
+	if (stream->oa_interrupt_monitor)
+		I915_WRITE(GEN8_OA_IMR, ~GEN8_OA_IMR_MASK_INTR);
+
 	/*
 	 * Note: we don't rely on the hardware to perform single context
 	 * filtering and instead filter on the cpu based on the context-id
@@ -1901,6 +1913,10 @@  static void i915_oa_stream_enable(struct i915_perf_stream *stream)
 {
 	struct drm_i915_private *dev_priv = stream->dev_priv;
 
+	dev_priv->perf.oa.half_full_count_last = 0;
+	atomic64_set(&dev_priv->perf.oa.half_full_count,
+		     dev_priv->perf.oa.half_full_count_last);
+
 	dev_priv->perf.oa.ops.oa_enable(stream);
 
 	if (dev_priv->perf.oa.periodic)
@@ -1913,6 +1929,13 @@  static void gen7_oa_disable(struct i915_perf_stream *stream)
 {
 	struct drm_i915_private *dev_priv = stream->dev_priv;
 
+	if (stream->oa_interrupt_monitor) {
+		spin_lock(&dev_priv->irq_lock);
+		gen5_disable_gt_irq(dev_priv,
+				    GT_PERFMON_BUFFER_HALF_FULL_INTERRUPT);
+		spin_unlock(&dev_priv->irq_lock);
+	}
+
 	I915_WRITE(GEN7_OACONTROL, 0);
 	if (intel_wait_for_register(dev_priv,
 				    GEN7_OACONTROL, GEN7_OACONTROL_ENABLE, 0,
@@ -1924,6 +1947,8 @@  static void gen8_oa_disable(struct i915_perf_stream *stream)
 {
 	struct drm_i915_private *dev_priv = stream->dev_priv;
 
+	I915_WRITE(GEN8_OA_IMR, 0xffffffff);
+
 	I915_WRITE(GEN8_OACONTROL, 0);
 	if (intel_wait_for_register(dev_priv,
 				    GEN8_OACONTROL, GEN8_OA_COUNTER_ENABLE, 0,
@@ -2599,6 +2624,7 @@  i915_perf_open_ioctl_locked(struct drm_i915_private *dev_priv,
 	stream->dev_priv = dev_priv;
 	stream->ctx = specific_ctx;
 	stream->poll_oa_period = props->poll_oa_period;
+	stream->oa_interrupt_monitor = props->oa_interrupt_monitor;
 
 	ret = i915_oa_stream_init(stream, param, props);
 	if (ret)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 0a7d60509ca7..c30d4cb6decd 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -223,6 +223,7 @@  static inline bool i915_mmio_reg_valid(i915_reg_t reg)
 #define MAX_ENGINE_CLASS	4
 
 #define OTHER_GTPM_INSTANCE	1
+#define OTHER_WDOAPERF_INSTANCE	2
 #define MAX_ENGINE_INSTANCE    3
 
 /* PCI config space */
@@ -617,6 +618,9 @@  static inline bool i915_mmio_reg_valid(i915_reg_t reg)
 #define OABUFFER_SIZE_8M    (6 << 3)
 #define OABUFFER_SIZE_16M   (7 << 3)
 
+#define GEN8_OA_IMR _MMIO(0x2b20)
+#define  GEN8_OA_IMR_MASK_INTR (1 << 28)
+
 /*
  * Flexible, Aggregate EU Counter Registers.
  * Note: these aren't contiguous
@@ -2868,7 +2872,9 @@  enum i915_power_well_id {
 #define GT_BLT_USER_INTERRUPT			(1 << 22)
 #define GT_BSD_CS_ERROR_INTERRUPT		(1 << 15)
 #define GT_BSD_USER_INTERRUPT			(1 << 12)
+#define GEN8_GT_PERFMON_BUFFER_HALF_FULL_INTERRUPT (1 << 12) /* bdw+ */
 #define GT_RENDER_L3_PARITY_ERROR_INTERRUPT_S1	(1 << 11) /* hsw+; rsvd on snb, ivb, vlv */
+#define GT_PERFMON_BUFFER_HALF_FULL_INTERRUPT   (1 <<  9) /* ivb+ but only used on hsw+ */
 #define GT_CONTEXT_SWITCH_INTERRUPT		(1 <<  8)
 #define GT_RENDER_L3_PARITY_ERROR_INTERRUPT	(1 <<  5) /* !snb */
 #define GT_RENDER_PIPECTL_NOTIFY_INTERRUPT	(1 <<  4)
@@ -7160,6 +7166,7 @@  enum {
 #define  GEN8_DE_PIPE_B_IRQ		(1 << 17)
 #define  GEN8_DE_PIPE_A_IRQ		(1 << 16)
 #define  GEN8_DE_PIPE_IRQ(pipe)		(1 << (16 + (pipe)))
+#define  GEN8_GT_WDBOX_OACS_IRQ         (1 << 7)
 #define  GEN8_GT_VECS_IRQ		(1 << 6)
 #define  GEN8_GT_GUC_IRQ		(1 << 5)
 #define  GEN8_GT_PM_IRQ			(1 << 4)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 02f6a9b81083..42f7157636c7 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2277,6 +2277,8 @@  int intel_init_render_ring_buffer(struct intel_engine_cs *engine)
 
 	if (HAS_L3_DPF(dev_priv))
 		engine->irq_keep_mask = GT_RENDER_L3_PARITY_ERROR_INTERRUPT;
+	if (IS_HASWELL(dev_priv))
+		engine->irq_keep_mask |= GT_PERFMON_BUFFER_HALF_FULL_INTERRUPT;
 
 	engine->irq_enable_mask = GT_RENDER_USER_INTERRUPT;