[PATCH 0/7] drm/i915/perf: add OA interrupt support

[PATCH 0/7] drm/i915/perf: add OA interrupt support

[Lionel Landwerlin]

Taking the RFC off this series.

To quite the vTune team that tried the previous version :

    "It reduces data collection overhead in VTune by 11x. It is great!"

The GPA team's report on the previous version was a drop in CPU
consumption from 17~20% down to 2~3%.

This version includes :

   - a fix for an issue reported by Chris on the IMR register access
     on Haswell

   - the ability to completely disable the i915 OA head/tail polling

   - a new ioctl on the perf stream file descript (not the i915 drm
     master/render node) to force i915 to look at the OA head/tail
     register (see explanation in last patch).

Cheers,

Lionel Landwerlin (7):
  drm/i915/perf: rework aging tail workaround
  drm/i915/perf: reset pollin when perf stream is enabled
  drm/i915/perf: only append status when data is available
  drm/i915/perf: add new open param to configure polling of OA buffer
  drm/i915: handle interrupts from the OA unit
  drm/i915/perf: add interrupt enabling parameter
  drm/i915/perf: add flushing ioctl

 drivers/gpu/drm/i915/i915_drv.h         |  59 +++-
 drivers/gpu/drm/i915/i915_irq.c         |  39 ++-
 drivers/gpu/drm/i915/i915_perf.c        | 388 +++++++++++++++---------
 drivers/gpu/drm/i915/i915_reg.h         |   7 +
 drivers/gpu/drm/i915/intel_ringbuffer.c |   2 +
 include/uapi/drm/i915_drm.h             |  35 +++
 6 files changed, 357 insertions(+), 173 deletions(-)

--
2.20.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 1/7] drm/i915/perf: rework aging tail workaround

[Lionel Landwerlin]

We're about to introduce an options to open the perf stream, giving
the user ability to configure how often it wants the kernel to poll
the OA registers for available data.

Right now the workaround against the OA tail pointer race condition
requires at least twice the internal kernel polling timer to make any
data available.

This changes introduce checks on the OA data written into the circular
buffer to make as much data as possible available on the first
iteration of the polling timer.

v2: Use OA_TAKEN macro without the gtt_offset (Lionel)

Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h  |  32 ++---
 drivers/gpu/drm/i915/i915_perf.c | 200 ++++++++++++++-----------------
 2 files changed, 103 insertions(+), 129 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index da055a86db4d..8cb93718224f 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1877,6 +1877,12 @@ struct drm_i915_private {
 			 */
 			struct ratelimit_state spurious_report_rs;
 
+			/**
+			 * For rate limiting any notifications of tail pointer
+			 * race.
+			 */
+			struct ratelimit_state tail_pointer_race;
+
 			bool periodic;
 			int period_exponent;
 
@@ -1917,23 +1923,11 @@ struct drm_i915_private {
 				spinlock_t ptr_lock;
 
 				/**
-				 * One 'aging' tail pointer and one 'aged'
-				 * tail pointer ready to used for reading.
-				 *
-				 * Initial values of 0xffffffff are invalid
-				 * and imply that an update is required
-				 * (and should be ignored by an attempted
-				 * read)
-				 */
-				struct {
-					u32 offset;
-				} tails[2];
-
-				/**
-				 * Index for the aged tail ready to read()
-				 * data up to.
+				 * The last HW tail reported by HW. The data
+				 * might not have made it to memory yet
+				 * though.
 				 */
-				unsigned int aged_tail_idx;
+				u32 aging_tail;
 
 				/**
 				 * A monotonic timestamp for when the current
@@ -1952,6 +1946,12 @@ struct drm_i915_private {
 				 * data to userspace.
 				 */
 				u32 head;
+
+				/**
+				 * The last tail verified tail that can be
+				 * read by userspace.
+				 */
+				u32 tail;
 			} oa_buffer;
 
 			u32 gen7_latched_oastatus1;
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index faff6cf1aaa1..575701e2aabc 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -233,23 +233,14 @@
  * for this earlier, as part of the oa_buffer_check to avoid lots of redundant
  * read() attempts.
  *
- * In effect we define a tail pointer for reading that lags the real tail
- * pointer by at least %OA_TAIL_MARGIN_NSEC nanoseconds, which gives enough
- * time for the corresponding reports to become visible to the CPU.
- *
- * To manage this we actually track two tail pointers:
- *  1) An 'aging' tail with an associated timestamp that is tracked until we
- *     can trust the corresponding data is visible to the CPU; at which point
- *     it is considered 'aged'.
- *  2) An 'aged' tail that can be used for read()ing.
- *
- * The two separate pointers let us decouple read()s from tail pointer aging.
- *
- * The tail pointers are checked and updated at a limited rate within a hrtimer
- * callback (the same callback that is used for delivering EPOLLIN events)
- *
- * Initially the tails are marked invalid with %INVALID_TAIL_PTR which
- * indicates that an updated tail pointer is needed.
+ * We workaround this issue in oa_buffer_check() by reading the reports in the
+ * OA buffer, starting from the tail reported by the HW until we find 2
+ * consecutive reports with their first 2 dwords of not at 0. Those dwords are
+ * also set to 0 once read and the whole buffer is cleared upon OA buffer
+ * initialization. The first dword is the reason for this report while the
+ * second is the timestamp, making the chances of having those 2 fields at 0
+ * fairly unlikely. A more detailed explanation is available in
+ * oa_buffer_check().
  *
  * Most of the implementation details for this workaround are in
  * oa_buffer_check_unlocked() and _append_oa_reports()
@@ -262,7 +253,6 @@
  * enabled without any periodic sampling.
  */
 #define OA_TAIL_MARGIN_NSEC	100000ULL
-#define INVALID_TAIL_PTR	0xffffffff
 
 /* frequency for checking whether the OA unit has written new reports to the
  * circular OA buffer...
@@ -449,10 +439,10 @@ static u32 gen7_oa_hw_tail_read(struct drm_i915_private *dev_priv)
  */
 static bool oa_buffer_check_unlocked(struct drm_i915_private *dev_priv)
 {
+	u32 gtt_offset = i915_ggtt_offset(dev_priv->perf.oa.oa_buffer.vma);
 	int report_size = dev_priv->perf.oa.oa_buffer.format_size;
 	unsigned long flags;
-	unsigned int aged_idx;
-	u32 head, hw_tail, aged_tail, aging_tail;
+	u32 hw_tail;
 	u64 now;
 
 	/* We have to consider the (unlikely) possibility that read() errors
@@ -461,16 +451,6 @@ static bool oa_buffer_check_unlocked(struct drm_i915_private *dev_priv)
 	 */
 	spin_lock_irqsave(&dev_priv->perf.oa.oa_buffer.ptr_lock, flags);
 
-	/* NB: The head we observe here might effectively be a little out of
-	 * date (between head and tails[aged_idx].offset if there is currently
-	 * a read() in progress.
-	 */
-	head = dev_priv->perf.oa.oa_buffer.head;
-
-	aged_idx = dev_priv->perf.oa.oa_buffer.aged_tail_idx;
-	aged_tail = dev_priv->perf.oa.oa_buffer.tails[aged_idx].offset;
-	aging_tail = dev_priv->perf.oa.oa_buffer.tails[!aged_idx].offset;
-
 	hw_tail = dev_priv->perf.oa.ops.oa_hw_tail_read(dev_priv);
 
 	/* The tail pointer increases in 64 byte increments,
@@ -480,63 +460,75 @@ static bool oa_buffer_check_unlocked(struct drm_i915_private *dev_priv)
 
 	now = ktime_get_mono_fast_ns();
 
-	/* Update the aged tail
-	 *
-	 * Flip the tail pointer available for read()s once the aging tail is
-	 * old enough to trust that the corresponding data will be visible to
-	 * the CPU...
-	 *
-	 * Do this before updating the aging pointer in case we may be able to
-	 * immediately start aging a new pointer too (if new data has become
-	 * available) without needing to wait for a later hrtimer callback.
-	 */
-	if (aging_tail != INVALID_TAIL_PTR &&
-	    ((now - dev_priv->perf.oa.oa_buffer.aging_timestamp) >
-	     OA_TAIL_MARGIN_NSEC)) {
-
-		aged_idx ^= 1;
-		dev_priv->perf.oa.oa_buffer.aged_tail_idx = aged_idx;
+	if (hw_tail == dev_priv->perf.oa.oa_buffer.aging_tail) {
+		/* If the HW tail hasn't move since the last check and the HW
+		 * tail has been aging for long enough, declare it the new
+		 * tail.
+		 */
+		if ((now - dev_priv->perf.oa.oa_buffer.aging_timestamp) >
+		    OA_TAIL_MARGIN_NSEC) {
+			dev_priv->perf.oa.oa_buffer.tail =
+				dev_priv->perf.oa.oa_buffer.aging_tail;
+		}
+	} else {
+		u32 head, tail, landed_report_heads;
 
-		aged_tail = aging_tail;
+		/* NB: The head we observe here might effectively be a little out of
+		 * date (between head and tails[aged_idx].offset if there is currently
+		 * a read() in progress.
+		 */
+		head = dev_priv->perf.oa.oa_buffer.head - gtt_offset;
 
-		/* Mark that we need a new pointer to start aging... */
-		dev_priv->perf.oa.oa_buffer.tails[!aged_idx].offset = INVALID_TAIL_PTR;
-		aging_tail = INVALID_TAIL_PTR;
-	}
+		hw_tail -= gtt_offset;
+		tail = hw_tail;
 
-	/* Update the aging tail
-	 *
-	 * We throttle aging tail updates until we have a new tail that
-	 * represents >= one report more data than is already available for
-	 * reading. This ensures there will be enough data for a successful
-	 * read once this new pointer has aged and ensures we will give the new
-	 * pointer time to age.
-	 */
-	if (aging_tail == INVALID_TAIL_PTR &&
-	    (aged_tail == INVALID_TAIL_PTR ||
-	     OA_TAKEN(hw_tail, aged_tail) >= report_size)) {
-		struct i915_vma *vma = dev_priv->perf.oa.oa_buffer.vma;
-		u32 gtt_offset = i915_ggtt_offset(vma);
-
-		/* Be paranoid and do a bounds check on the pointer read back
-		 * from hardware, just in case some spurious hardware condition
-		 * could put the tail out of bounds...
+		/* Walk the stream backward until we find at least 2 reports
+		 * with dword 0 & 1 not at 0. Since the circular buffer
+		 * pointers progress by increments of 64 bytes and that
+		 * reports can be up to 256 bytes long, we can't tell whether
+		 * a report has fully landed in memory before the first 2
+		 * dwords of the following report have effectively landed.
+		 *
+		 * This is assuming that the writes of the OA unit land in
+		 * memory in the order they were written to.
+		 * If not : (╯°□°）╯︵ ┻━┻
 		 */
-		if (hw_tail >= gtt_offset &&
-		    hw_tail < (gtt_offset + OA_BUFFER_SIZE)) {
-			dev_priv->perf.oa.oa_buffer.tails[!aged_idx].offset =
-				aging_tail = hw_tail;
-			dev_priv->perf.oa.oa_buffer.aging_timestamp = now;
-		} else {
-			DRM_ERROR("Ignoring spurious out of range OA buffer tail pointer = %u\n",
-				  hw_tail);
+		landed_report_heads = 0;
+		while (OA_TAKEN(tail, head) >= report_size) {
+			u32 previous_tail = (tail - report_size) & (OA_BUFFER_SIZE - 1);
+			u8 *report = dev_priv->perf.oa.oa_buffer.vaddr + previous_tail;
+			u32 *report32 = (void *) report;
+
+			/* Head of the report indicated by the HW tail register has
+			 * indeed landed into memory.
+			 */
+			if (report32[0] != 0 || report[1] != 0) {
+				landed_report_heads++;
+
+				if (landed_report_heads >= 2)
+					break;
+			}
+
+			tail = previous_tail;
+		}
+
+		if (abs(tail - hw_tail) >= (2 * report_size)) {
+			if (__ratelimit(&dev_priv->perf.oa.tail_pointer_race)) {
+				DRM_NOTE("unlanded report(s) head=0x%x "
+					 "tail=0x%x hw_tail=0x%x\n",
+					 head, tail, hw_tail);
+			}
 		}
+
+		dev_priv->perf.oa.oa_buffer.tail = gtt_offset + tail;
+		dev_priv->perf.oa.oa_buffer.aging_tail = gtt_offset + hw_tail;
+		dev_priv->perf.oa.oa_buffer.aging_timestamp = now;
 	}
 
 	spin_unlock_irqrestore(&dev_priv->perf.oa.oa_buffer.ptr_lock, flags);
 
-	return aged_tail == INVALID_TAIL_PTR ?
-		false : OA_TAKEN(aged_tail, head) >= report_size;
+	return OA_TAKEN(dev_priv->perf.oa.oa_buffer.tail - gtt_offset,
+			dev_priv->perf.oa.oa_buffer.head - gtt_offset) >= report_size;
 }
 
 /**
@@ -655,7 +647,6 @@ static int gen8_append_oa_reports(struct i915_perf_stream *stream,
 	u32 mask = (OA_BUFFER_SIZE - 1);
 	size_t start_offset = *offset;
 	unsigned long flags;
-	unsigned int aged_tail_idx;
 	u32 head, tail;
 	u32 taken;
 	int ret = 0;
@@ -666,18 +657,10 @@ static int gen8_append_oa_reports(struct i915_perf_stream *stream,
 	spin_lock_irqsave(&dev_priv->perf.oa.oa_buffer.ptr_lock, flags);
 
 	head = dev_priv->perf.oa.oa_buffer.head;
-	aged_tail_idx = dev_priv->perf.oa.oa_buffer.aged_tail_idx;
-	tail = dev_priv->perf.oa.oa_buffer.tails[aged_tail_idx].offset;
+	tail = dev_priv->perf.oa.oa_buffer.tail;
 
 	spin_unlock_irqrestore(&dev_priv->perf.oa.oa_buffer.ptr_lock, flags);
 
-	/*
-	 * An invalid tail pointer here means we're still waiting for the poll
-	 * hrtimer callback to give us a pointer
-	 */
-	if (tail == INVALID_TAIL_PTR)
-		return -EAGAIN;
-
 	/*
 	 * NB: oa_buffer.head/tail include the gtt_offset which we don't want
 	 * while indexing relative to oa_buf_base.
@@ -806,13 +789,10 @@ static int gen8_append_oa_reports(struct i915_perf_stream *stream,
 		}
 
 		/*
-		 * The above reason field sanity check is based on
-		 * the assumption that the OA buffer is initially
-		 * zeroed and we reset the field after copying so the
-		 * check is still meaningful once old reports start
-		 * being overwritten.
+		 * Clear out the first 2 dword as a mean to detect unlanded
+		 * reports.
 		 */
-		report32[0] = 0;
+		report32[0] = report32[1] = 0;
 	}
 
 	if (start_offset != *offset) {
@@ -944,7 +924,6 @@ static int gen7_append_oa_reports(struct i915_perf_stream *stream,
 	u32 mask = (OA_BUFFER_SIZE - 1);
 	size_t start_offset = *offset;
 	unsigned long flags;
-	unsigned int aged_tail_idx;
 	u32 head, tail;
 	u32 taken;
 	int ret = 0;
@@ -955,17 +934,10 @@ static int gen7_append_oa_reports(struct i915_perf_stream *stream,
 	spin_lock_irqsave(&dev_priv->perf.oa.oa_buffer.ptr_lock, flags);
 
 	head = dev_priv->perf.oa.oa_buffer.head;
-	aged_tail_idx = dev_priv->perf.oa.oa_buffer.aged_tail_idx;
-	tail = dev_priv->perf.oa.oa_buffer.tails[aged_tail_idx].offset;
+	tail = dev_priv->perf.oa.oa_buffer.tail;
 
 	spin_unlock_irqrestore(&dev_priv->perf.oa.oa_buffer.ptr_lock, flags);
 
-	/* An invalid tail pointer here means we're still waiting for the poll
-	 * hrtimer callback to give us a pointer
-	 */
-	if (tail == INVALID_TAIL_PTR)
-		return -EAGAIN;
-
 	/* NB: oa_buffer.head/tail include the gtt_offset which we don't want
 	 * while indexing relative to oa_buf_base.
 	 */
@@ -1020,13 +992,10 @@ static int gen7_append_oa_reports(struct i915_perf_stream *stream,
 		if (ret)
 			break;
 
-		/* The above report-id field sanity check is based on
-		 * the assumption that the OA buffer is initially
-		 * zeroed and we reset the field after copying so the
-		 * check is still meaningful once old reports start
-		 * being overwritten.
+		/* Clear out the first 2 dwords as a mean to detect unlanded
+		 * reports.
 		 */
-		report32[0] = 0;
+		report32[0] = report32[1] = 0;
 	}
 
 	if (start_offset != *offset) {
@@ -1397,8 +1366,8 @@ static void gen7_init_oa_buffer(struct drm_i915_private *dev_priv)
 	I915_WRITE(GEN7_OASTATUS1, gtt_offset | OABUFFER_SIZE_16M); /* tail */
 
 	/* Mark that we need updated tail pointers to read from... */
-	dev_priv->perf.oa.oa_buffer.tails[0].offset = INVALID_TAIL_PTR;
-	dev_priv->perf.oa.oa_buffer.tails[1].offset = INVALID_TAIL_PTR;
+	dev_priv->perf.oa.oa_buffer.aging_tail =
+		dev_priv->perf.oa.oa_buffer.tail = gtt_offset;
 
 	spin_unlock_irqrestore(&dev_priv->perf.oa.oa_buffer.ptr_lock, flags);
 
@@ -1453,8 +1422,8 @@ static void gen8_init_oa_buffer(struct drm_i915_private *dev_priv)
 	I915_WRITE(GEN8_OATAILPTR, gtt_offset & GEN8_OATAILPTR_MASK);
 
 	/* Mark that we need updated tail pointers to read from... */
-	dev_priv->perf.oa.oa_buffer.tails[0].offset = INVALID_TAIL_PTR;
-	dev_priv->perf.oa.oa_buffer.tails[1].offset = INVALID_TAIL_PTR;
+	dev_priv->perf.oa.oa_buffer.aging_tail =
+		dev_priv->perf.oa.oa_buffer.tail = gtt_offset;
 
 	/*
 	 * Reset state used to recognise context switches, affecting which
@@ -2043,6 +2012,11 @@ static int i915_oa_stream_init(struct i915_perf_stream *stream,
 	ratelimit_set_flags(&dev_priv->perf.oa.spurious_report_rs,
 			    RATELIMIT_MSG_ON_RELEASE);
 
+	ratelimit_state_init(&dev_priv->perf.oa.tail_pointer_race,
+			     5 * HZ, 10);
+	ratelimit_set_flags(&dev_priv->perf.oa.tail_pointer_race,
+			    RATELIMIT_MSG_ON_RELEASE);
+
 	stream->sample_size = sizeof(struct drm_i915_perf_record_header);
 
 	format_size = dev_priv->perf.oa.oa_formats[props->oa_format].size;
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/7] drm/i915/perf: reset pollin when perf stream is enabled

[Lionel Landwerlin]

No issues have been raised about this yet, but this should be reset
every time we enable the stream, otherwise we might have a stale value
from the previous round of enable/disable.

Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
---
 drivers/gpu/drm/i915/i915_perf.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index 575701e2aabc..4c5d2ee4d6e3 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -1876,6 +1876,8 @@ static void i915_oa_stream_enable(struct i915_perf_stream *stream)
 {
 	struct drm_i915_private *dev_priv = stream->dev_priv;
 
+	dev_priv->perf.oa.pollin = false;
+
 	dev_priv->perf.oa.ops.oa_enable(stream);
 
 	if (dev_priv->perf.oa.periodic)
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 3/7] drm/i915/perf: only append status when data is available

[Lionel Landwerlin]

The only bit of the status register we currently report in the
i915-perf stream is the "report loss" bit. Only report this when we
have some data to report with it. There was a kind of inconsistency
here in that we could report report loss without appending the reports
associated with the loss.

Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
---
 drivers/gpu/drm/i915/i915_perf.c | 54 ++++++++++++++++++++------------
 1 file changed, 34 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index 4c5d2ee4d6e3..b37c7ad0cde6 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -636,6 +636,7 @@ static int append_oa_sample(struct i915_perf_stream *stream,
  * Returns: 0 on success, negative error code on failure.
  */
 static int gen8_append_oa_reports(struct i915_perf_stream *stream,
+				  u32 oastatus,
 				  char __user *buf,
 				  size_t count,
 				  size_t *offset)
@@ -681,6 +682,21 @@ static int gen8_append_oa_reports(struct i915_perf_stream *stream,
 		      head, tail))
 		return -EIO;
 
+	/*
+	 * If there is nothing to read, don't append the status report yet,
+	 * wait until we have some data available.
+	 */
+	if (!OA_TAKEN(tail, head))
+		return 0;
+
+	if (oastatus & GEN8_OASTATUS_REPORT_LOST) {
+		ret = append_oa_status(stream, buf, count, offset,
+				       DRM_I915_PERF_RECORD_OA_REPORT_LOST);
+		if (ret)
+			return ret;
+		I915_WRITE(GEN8_OASTATUS,
+			   oastatus & ~GEN8_OASTATUS_REPORT_LOST);
+	}
 
 	for (/* none */;
 	     (taken = OA_TAKEN(tail, head));
@@ -880,16 +896,7 @@ static int gen8_oa_read(struct i915_perf_stream *stream,
 		oastatus = I915_READ(GEN8_OASTATUS);
 	}
 
-	if (oastatus & GEN8_OASTATUS_REPORT_LOST) {
-		ret = append_oa_status(stream, buf, count, offset,
-				       DRM_I915_PERF_RECORD_OA_REPORT_LOST);
-		if (ret)
-			return ret;
-		I915_WRITE(GEN8_OASTATUS,
-			   oastatus & ~GEN8_OASTATUS_REPORT_LOST);
-	}
-
-	return gen8_append_oa_reports(stream, buf, count, offset);
+	return gen8_append_oa_reports(stream, oastatus, buf, count, offset);
 }
 
 /**
@@ -913,6 +920,7 @@ static int gen8_oa_read(struct i915_perf_stream *stream,
  * Returns: 0 on success, negative error code on failure.
  */
 static int gen7_append_oa_reports(struct i915_perf_stream *stream,
+				  u32 oastatus1,
 				  char __user *buf,
 				  size_t count,
 				  size_t *offset)
@@ -956,6 +964,21 @@ static int gen7_append_oa_reports(struct i915_perf_stream *stream,
 		      head, tail))
 		return -EIO;
 
+	/*
+	 * If there is nothing to read, don't append the status report yet,
+	 * wait until we have some data available.
+	 */
+	if (!OA_TAKEN(tail, head))
+		return 0;
+
+	if (unlikely(oastatus1 & GEN7_OASTATUS1_REPORT_LOST)) {
+		ret = append_oa_status(stream, buf, count, offset,
+				       DRM_I915_PERF_RECORD_OA_REPORT_LOST);
+		if (ret)
+			return ret;
+		dev_priv->perf.oa.gen7_latched_oastatus1 |=
+			GEN7_OASTATUS1_REPORT_LOST;
+	}
 
 	for (/* none */;
 	     (taken = OA_TAKEN(tail, head));
@@ -1089,16 +1112,7 @@ static int gen7_oa_read(struct i915_perf_stream *stream,
 		oastatus1 = I915_READ(GEN7_OASTATUS1);
 	}
 
-	if (unlikely(oastatus1 & GEN7_OASTATUS1_REPORT_LOST)) {
-		ret = append_oa_status(stream, buf, count, offset,
-				       DRM_I915_PERF_RECORD_OA_REPORT_LOST);
-		if (ret)
-			return ret;
-		dev_priv->perf.oa.gen7_latched_oastatus1 |=
-			GEN7_OASTATUS1_REPORT_LOST;
-	}
-
-	return gen7_append_oa_reports(stream, buf, count, offset);
+	return gen7_append_oa_reports(stream, oastatus1, buf, count, offset);
 }
 
 /**
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 4/7] drm/i915/perf: add new open param to configure polling of OA buffer

[Lionel Landwerlin]

This new parameter let's the application choose how often the OA
buffer should be checked on the CPU side for data availability. Longer
polling period tend to reduce CPU overhead if the application does not
care about somewhat real time data collection.

v2: Allow disabling polling completely with 0 value (Lionel)

Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h  |  6 +++++
 drivers/gpu/drm/i915/i915_perf.c | 43 ++++++++++++++++++++++++++------
 include/uapi/drm/i915_drm.h      |  8 ++++++
 3 files changed, 50 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 8cb93718224f..d535df8f7d0a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1379,6 +1379,12 @@ struct i915_perf_stream {
 	 * @oa_config: The OA configuration used by the stream.
 	 */
 	struct i915_oa_config *oa_config;
+
+	/**
+	 * @poll_oa_period: The period in nanoseconds at which the OA
+	 * buffer should be checked for available data.
+	 */
+	u64 poll_oa_period;
 };
 
 /**
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index b37c7ad0cde6..9ad45ad31b43 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -254,11 +254,11 @@
  */
 #define OA_TAIL_MARGIN_NSEC	100000ULL
 
-/* frequency for checking whether the OA unit has written new reports to the
- * circular OA buffer...
+/* The default frequency for checking whether the OA unit has written new
+ * reports to the circular OA buffer...
  */
-#define POLL_FREQUENCY 200
-#define POLL_PERIOD (NSEC_PER_SEC / POLL_FREQUENCY)
+#define DEFAULT_POLL_FREQUENCY 200
+#define DEFAULT_POLL_PERIOD (NSEC_PER_SEC / DEFAULT_POLL_FREQUENCY)
 
 /* for sysctl proc_dointvec_minmax of dev.i915.perf_stream_paranoid */
 static int zero;
@@ -335,6 +335,8 @@ static const struct i915_oa_format gen8_plus_oa_formats[I915_OA_FORMAT_MAX] = {
  * @oa_format: An OA unit HW report format
  * @oa_periodic: Whether to enable periodic OA unit sampling
  * @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
  *
  * As read_properties_unlocked() enumerates and validates the properties given
  * to open a stream of metrics the configuration is built up in the structure
@@ -351,6 +353,7 @@ struct perf_open_properties {
 	int oa_format;
 	bool oa_periodic;
 	int oa_period_exponent;
+	u64 poll_oa_period;
 };
 
 static void free_oa_config(struct drm_i915_private *dev_priv,
@@ -1894,9 +1897,9 @@ static void i915_oa_stream_enable(struct i915_perf_stream *stream)
 
 	dev_priv->perf.oa.ops.oa_enable(stream);
 
-	if (dev_priv->perf.oa.periodic)
+	if (dev_priv->perf.oa.periodic && stream->poll_oa_period)
 		hrtimer_start(&dev_priv->perf.oa.poll_check_timer,
-			      ns_to_ktime(POLL_PERIOD),
+			      ns_to_ktime(stream->poll_oa_period),
 			      HRTIMER_MODE_REL_PINNED);
 }
 
@@ -2260,13 +2263,15 @@ static enum hrtimer_restart oa_poll_check_timer_cb(struct hrtimer *hrtimer)
 	struct drm_i915_private *dev_priv =
 		container_of(hrtimer, typeof(*dev_priv),
 			     perf.oa.poll_check_timer);
+	struct i915_perf_stream *stream = dev_priv->perf.oa.exclusive_stream;
 
 	if (oa_buffer_check_unlocked(dev_priv)) {
 		dev_priv->perf.oa.pollin = true;
 		wake_up(&dev_priv->perf.oa.poll_wq);
 	}
 
-	hrtimer_forward_now(hrtimer, ns_to_ktime(POLL_PERIOD));
+	hrtimer_forward_now(hrtimer,
+			    ns_to_ktime(stream->poll_oa_period));
 
 	return HRTIMER_RESTART;
 }
@@ -2587,6 +2592,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;
 
 	ret = i915_oa_stream_init(stream, param, props);
 	if (ret)
@@ -2642,6 +2648,7 @@ static u64 oa_exponent_to_ns(struct drm_i915_private *dev_priv, int exponent)
 /**
  * read_properties_unlocked - validate + copy userspace stream open properties
  * @dev_priv: i915 device instance
+ * @open_flags: Flags set by userspace for the opening of the stream
  * @uprops: The array of u64 key value pairs given by userspace
  * @n_props: The number of key value pairs expected in @uprops
  * @props: The stream configuration built up while validating properties
@@ -2655,6 +2662,7 @@ static u64 oa_exponent_to_ns(struct drm_i915_private *dev_priv, int exponent)
  * rule out defining new properties with ordering requirements in the future.
  */
 static int read_properties_unlocked(struct drm_i915_private *dev_priv,
+				    u32 open_flags,
 				    u64 __user *uprops,
 				    u32 n_props,
 				    struct perf_open_properties *props)
@@ -2663,6 +2671,7 @@ static int read_properties_unlocked(struct drm_i915_private *dev_priv,
 	u32 i;
 
 	memset(props, 0, sizeof(struct perf_open_properties));
+	props->poll_oa_period = DEFAULT_POLL_PERIOD;
 
 	if (!n_props) {
 		DRM_DEBUG("No i915 perf properties given\n");
@@ -2766,6 +2775,14 @@ static int read_properties_unlocked(struct drm_i915_private *dev_priv,
 			props->oa_periodic = true;
 			props->oa_period_exponent = value;
 			break;
+		case DRM_I915_PERF_PROP_POLL_OA_DELAY:
+			if (value > 0 && value < 100000 /* 100us */) {
+				DRM_DEBUG("OA availability timer too small (%lluns < 100us)\n",
+					  value);
+				return -EINVAL;
+			}
+			props->poll_oa_period = value;
+			break;
 		case DRM_I915_PERF_PROP_MAX:
 			MISSING_CASE(id);
 			return -EINVAL;
@@ -2774,6 +2791,17 @@ static int read_properties_unlocked(struct drm_i915_private *dev_priv,
 		uprop += 2;
 	}
 
+	/*
+	 * Blocking read need to be waken up by some mechanism. If no polling
+	 * of the HEAD/TAIL register is done by the kernel, we'll never be
+	 * able to wake up.
+	 */
+	if ((open_flags & I915_PERF_FLAG_FD_NONBLOCK) == 0 &&
+	    !props->poll_oa_period) {
+		DRM_DEBUG("Requesting a blocking stream with no polling period.\n");
+		return -EINVAL;
+	}
+
 	return 0;
 }
 
@@ -2824,6 +2852,7 @@ int i915_perf_open_ioctl(struct drm_device *dev, void *data,
 	}
 
 	ret = read_properties_unlocked(dev_priv,
+				       param->flags,
 				       u64_to_user_ptr(param->properties_ptr),
 				       param->num_properties,
 				       &props);
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 298b2e197744..0f4cf6c17e9a 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -1540,6 +1540,14 @@ enum drm_i915_perf_property_id {
 	 */
 	DRM_I915_PERF_PROP_OA_EXPONENT,
 
+	/**
+	 * Specifying this property sets up a hrtimer in nanoseconds at which
+	 * the i915 driver will check the OA buffer for available data. A
+	 * value of 0 means no hrtimer will be started. Values below 100
+	 * microseconds are not allowed.
+	 */
+	DRM_I915_PERF_PROP_POLL_OA_DELAY,
+
 	DRM_I915_PERF_PROP_MAX /* non-ABI */
 };
 
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 5/7] drm/i915: handle interrupts from the OA unit

[Lionel Landwerlin]

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.

v2: Use spin_lock_irq() to access the IMR register on Haswell (Chris)

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(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index d535df8f7d0a..850cf35e6163 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1385,6 +1385,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;
 };
 
 /**
@@ -1877,6 +1883,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 94187e68d39a..7996048565a7 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1214,6 +1214,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)
 {
@@ -1478,6 +1484,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);
 }
@@ -1499,6 +1508,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])
 {
@@ -1508,6 +1523,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)
@@ -1530,7 +1546,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]);
@@ -1554,9 +1570,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)) {
@@ -3010,6 +3028,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);
@@ -4041,6 +4061,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) {
@@ -4170,7 +4194,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
 		};
 
 	dev_priv->pm_ier = 0x0;
@@ -4289,12 +4314,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 9ad45ad31b43..82c4e0c08859 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -337,6 +337,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
@@ -354,6 +355,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,
@@ -1844,6 +1846,13 @@ static void gen7_oa_enable(struct i915_perf_stream *stream)
 	 */
 	gen7_init_oa_buffer(dev_priv);
 
+	if (stream->oa_interrupt_monitor) {
+		spin_lock_irq(&dev_priv->irq_lock);
+		gen5_enable_gt_irq(dev_priv,
+				   GT_PERFMON_BUFFER_HALF_FULL_INTERRUPT);
+		spin_unlock_irq(&dev_priv->irq_lock);
+	}
+
 	I915_WRITE(GEN7_OACONTROL,
 		   (ctx_id & GEN7_OACONTROL_CTX_MASK) |
 		   (period_exponent <<
@@ -1870,6 +1879,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
@@ -1895,6 +1907,10 @@ static void i915_oa_stream_enable(struct i915_perf_stream *stream)
 
 	dev_priv->perf.oa.pollin = false;
 
+	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 && stream->poll_oa_period)
@@ -1907,6 +1923,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_irq(&dev_priv->irq_lock);
+		gen5_disable_gt_irq(dev_priv,
+				    GT_PERFMON_BUFFER_HALF_FULL_INTERRUPT);
+		spin_unlock_irq(&dev_priv->irq_lock);
+	}
+
 	I915_WRITE(GEN7_OACONTROL, 0);
 	if (intel_wait_for_register(dev_priv,
 				    GEN7_OACONTROL, GEN7_OACONTROL_ENABLE, 0,
@@ -1918,6 +1941,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,
@@ -2593,6 +2618,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 fad5a9e8b44d..02f1d3bb85ab 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -229,6 +229,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 */
@@ -641,6 +642,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
@@ -2892,7 +2896,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)
@@ -7184,6 +7190,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 26b7274a2d43..a208fb2382ad 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2222,6 +2222,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;
 
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 6/7] drm/i915/perf: add interrupt enabling parameter

[Lionel Landwerlin]

This let's the application choose to be driven by the interrupt
mechanism of the HW. In conjuction with long periods for checks for
the availability of data on the CPU, this can reduce the CPU load when
doing capture of OA data.

Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
---
 drivers/gpu/drm/i915/i915_perf.c | 54 +++++++++++++++++++++++---------
 include/uapi/drm/i915_drm.h      |  8 +++++
 2 files changed, 48 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index 82c4e0c08859..da721fce2543 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -243,7 +243,7 @@
  * oa_buffer_check().
  *
  * Most of the implementation details for this workaround are in
- * oa_buffer_check_unlocked() and _append_oa_reports()
+ * oa_buffer_check() and _append_oa_reports()
  *
  * Note for posterity: previously the driver used to define an effective tail
  * pointer that lagged the real pointer by a 'tail margin' measured in bytes
@@ -418,9 +418,11 @@ static u32 gen7_oa_hw_tail_read(struct drm_i915_private *dev_priv)
 	return oastatus1 & GEN7_OASTATUS1_TAIL_MASK;
 }
 
+
 /**
- * oa_buffer_check_unlocked - check for data and update tail ptr state
+ * oa_buffer_check - check for data and update tail ptr state
  * @dev_priv: i915 device instance
+ * @lock: whether to take the oa_buffer spin lock
  *
  * This is either called via fops (for blocking reads in user ctx) or the poll
  * check hrtimer (atomic ctx) to check the OA buffer tail pointer and check
@@ -442,8 +444,9 @@ static u32 gen7_oa_hw_tail_read(struct drm_i915_private *dev_priv)
  *
  * Returns: %true if the OA buffer contains data, else %false
  */
-static bool oa_buffer_check_unlocked(struct drm_i915_private *dev_priv)
+static bool oa_buffer_check(struct drm_i915_private *dev_priv, bool lock)
 {
+	u64 half_full_count = atomic64_read(&dev_priv->perf.oa.half_full_count);
 	u32 gtt_offset = i915_ggtt_offset(dev_priv->perf.oa.oa_buffer.vma);
 	int report_size = dev_priv->perf.oa.oa_buffer.format_size;
 	unsigned long flags;
@@ -454,7 +457,8 @@ static bool oa_buffer_check_unlocked(struct drm_i915_private *dev_priv)
 	 * could result in an OA buffer reset which might reset the head,
 	 * tails[] and aged_tail state.
 	 */
-	spin_lock_irqsave(&dev_priv->perf.oa.oa_buffer.ptr_lock, flags);
+	if (lock)
+		spin_lock_irqsave(&dev_priv->perf.oa.oa_buffer.ptr_lock, flags);
 
 	hw_tail = dev_priv->perf.oa.ops.oa_hw_tail_read(dev_priv);
 
@@ -530,7 +534,10 @@ static bool oa_buffer_check_unlocked(struct drm_i915_private *dev_priv)
 		dev_priv->perf.oa.oa_buffer.aging_timestamp = now;
 	}
 
-	spin_unlock_irqrestore(&dev_priv->perf.oa.oa_buffer.ptr_lock, flags);
+	dev_priv->perf.oa.half_full_count_last = half_full_count;
+
+	if (lock)
+		spin_unlock_irqrestore(&dev_priv->perf.oa.oa_buffer.ptr_lock, flags);
 
 	return OA_TAKEN(dev_priv->perf.oa.oa_buffer.tail - gtt_offset,
 			dev_priv->perf.oa.oa_buffer.head - gtt_offset) >= report_size;
@@ -1124,9 +1131,9 @@ static int gen7_oa_read(struct i915_perf_stream *stream,
  * i915_oa_wait_unlocked - handles blocking IO until OA data available
  * @stream: An i915-perf stream opened for OA metrics
  *
- * Called when userspace tries to read() from a blocking stream FD opened
- * for OA metrics. It waits until the hrtimer callback finds a non-empty
- * OA buffer and wakes us.
+ * Called when userspace tries to read() from a blocking stream FD opened for
+ * OA metrics. It waits until either the hrtimer callback finds a non-empty OA
+ * buffer or the OA interrupt kicks in and wakes us.
  *
  * Note: it's acceptable to have this return with some false positives
  * since any subsequent read handling will return -EAGAIN if there isn't
@@ -1143,7 +1150,7 @@ static int i915_oa_wait_unlocked(struct i915_perf_stream *stream)
 		return -EIO;
 
 	return wait_event_interruptible(dev_priv->perf.oa.poll_wq,
-					oa_buffer_check_unlocked(dev_priv));
+					oa_buffer_check(dev_priv, true));
 }
 
 /**
@@ -1964,6 +1971,10 @@ static void i915_oa_stream_disable(struct i915_perf_stream *stream)
 
 	dev_priv->perf.oa.ops.oa_disable(stream);
 
+	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);
+
 	if (dev_priv->perf.oa.periodic)
 		hrtimer_cancel(&dev_priv->perf.oa.poll_check_timer);
 }
@@ -2290,7 +2301,7 @@ static enum hrtimer_restart oa_poll_check_timer_cb(struct hrtimer *hrtimer)
 			     perf.oa.poll_check_timer);
 	struct i915_perf_stream *stream = dev_priv->perf.oa.exclusive_stream;
 
-	if (oa_buffer_check_unlocked(dev_priv)) {
+	if (oa_buffer_check(dev_priv, true)) {
 		dev_priv->perf.oa.pollin = true;
 		wake_up(&dev_priv->perf.oa.poll_wq);
 	}
@@ -2326,6 +2337,16 @@ static __poll_t i915_perf_poll_locked(struct drm_i915_private *dev_priv,
 
 	stream->ops->poll_wait(stream, file, wait);
 
+	/*
+	 * Only check the half buffer full notifications if requested by the
+	 * user.
+	 */
+	if (stream->oa_interrupt_monitor &&
+	    (dev_priv->perf.oa.half_full_count_last !=
+	     atomic64_read(&dev_priv->perf.oa.half_full_count))) {
+		dev_priv->perf.oa.pollin = oa_buffer_check(dev_priv, true);
+	}
+
 	/* Note: we don't explicitly check whether there's something to read
 	 * here since this path may be very hot depending on what else
 	 * userspace is polling, or on the timeout in use. We rely solely on
@@ -2809,6 +2830,9 @@ static int read_properties_unlocked(struct drm_i915_private *dev_priv,
 			}
 			props->poll_oa_period = value;
 			break;
+		case DRM_I915_PERF_PROP_OA_ENABLE_INTERRUPT:
+			props->oa_interrupt_monitor = value != 0;
+			break;
 		case DRM_I915_PERF_PROP_MAX:
 			MISSING_CASE(id);
 			return -EINVAL;
@@ -2819,12 +2843,14 @@ static int read_properties_unlocked(struct drm_i915_private *dev_priv,
 
 	/*
 	 * Blocking read need to be waken up by some mechanism. If no polling
-	 * of the HEAD/TAIL register is done by the kernel, we'll never be
-	 * able to wake up.
+	 * of the HEAD/TAIL register is done by the kernel and no interrupt is
+	 * enabled, we'll never be able to wake up.
 	 */
 	if ((open_flags & I915_PERF_FLAG_FD_NONBLOCK) == 0 &&
-	    !props->poll_oa_period) {
-		DRM_DEBUG("Requesting a blocking stream with no polling period.\n");
+	    !props->poll_oa_period &&
+	    !props->oa_interrupt_monitor) {
+		DRM_DEBUG("Requesting a blocking stream with no polling period "
+			  "& no interrupt.\n");
 		return -EINVAL;
 	}
 
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 0f4cf6c17e9a..b6db5e544a35 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -1548,6 +1548,14 @@ enum drm_i915_perf_property_id {
 	 */
 	DRM_I915_PERF_PROP_POLL_OA_DELAY,
 
+	/**
+	 * Specifying this property sets up the interrupt mechanism for the OA
+	 * buffer in i915. This option in conjuction with a long polling delay
+	 * for avaibility of OA data can reduce CPU load significantly if you
+	 * do not care about OA data being read as soon as it's available.
+	 */
+	DRM_I915_PERF_PROP_OA_ENABLE_INTERRUPT,
+
 	DRM_I915_PERF_PROP_MAX /* non-ABI */
 };
 
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 7/7] drm/i915/perf: add flushing ioctl

[Lionel Landwerlin]

With the currently available parameters for the i915-perf stream,
there are still situations that are not well covered :

If an application opens the stream with polling disable or at very low
frequency and OA interrupt enabled, no data will be available even
though somewhere between nothing and half of the OA buffer worth of
data might have landed in memory.

To solve this issue we have a new flush ioctl on the perf stream that
forces the i915-perf driver to look at the state of the buffer when
called and makes any data available through both poll() & read() type
syscalls.

Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
---
 drivers/gpu/drm/i915/i915_perf.c | 17 +++++++++++++++++
 include/uapi/drm/i915_drm.h      | 19 +++++++++++++++++++
 2 files changed, 36 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index da721fce2543..6c98ffa2135e 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -2433,6 +2433,20 @@ static void i915_perf_disable_locked(struct i915_perf_stream *stream)
 		stream->ops->disable(stream);
 }
 
+/**
+ * i915_perf_flush_data - handle `I915_PERF_IOCTL_FLUSH_DATA` ioctl
+ * @stream: An enabled i915 perf stream
+ *
+ * The intention is to flush all the data available for reading from the OA
+ * buffer
+ */
+static void i915_perf_flush_data(struct i915_perf_stream *stream)
+{
+	struct drm_i915_private *dev_priv = stream->dev_priv;
+
+	dev_priv->perf.oa.pollin = oa_buffer_check(stream->dev_priv, true);
+}
+
 /**
  * i915_perf_ioctl - support ioctl() usage with i915 perf stream FDs
  * @stream: An i915 perf stream
@@ -2456,6 +2470,9 @@ static long i915_perf_ioctl_locked(struct i915_perf_stream *stream,
 	case I915_PERF_IOCTL_DISABLE:
 		i915_perf_disable_locked(stream);
 		return 0;
+	case I915_PERF_IOCTL_FLUSH_DATA:
+		i915_perf_flush_data(stream);
+		return 0;
 	}
 
 	return -EINVAL;
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index b6db5e544a35..0f0d20080572 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -1594,6 +1594,25 @@ struct drm_i915_perf_open_param {
  */
 #define I915_PERF_IOCTL_DISABLE	_IO('i', 0x1)
 
+/**
+ * Actively check the availability of data from a stream.
+ *
+ * A stream data availability can be driven by two types of events :
+ *
+ *   - if enabled, the kernel's hrtimer checking the amount of available data
+ *     in the OA buffer through head/tail registers.
+ *
+ *   - if enabled, the OA unit's interrupt mechanism
+ *
+ * The kernel hrtimer incur a cost of running callback at fixed time
+ * intervals, while the OA interrupt might only happen rarely. In the
+ * situation where the application has disabled the kernel's hrtimer and only
+ * uses the OA interrupt to know about available data, the application can
+ * request an active check of the available OA data through this ioctl. This
+ * will make any data in the OA buffer available with either poll() or read().
+ */
+#define I915_PERF_IOCTL_FLUSH_DATA _IO('i', 0x2)
+
 /**
  * Common to all i915 perf records
  */
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 5/7] drm/i915: handle interrupts from the OA unit

[Chris Wilson]

Quoting Lionel Landwerlin (2019-01-16 15:36:20)
> @@ -1877,6 +1883,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;

Eh? But why a relatively expensive atomic64. You only need one bit, and
reading the tail pointer from WB memory should just be cheap. You should
be able to sample the perf ringbuffer pointers very cheaply... What am I
missing?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 5/7] drm/i915: handle interrupts from the OA unit

[Lionel Landwerlin]

On 16/01/2019 15:52, Chris Wilson wrote:
> Quoting Lionel Landwerlin (2019-01-16 15:36:20)
>> @@ -1877,6 +1883,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;
> Eh? But why a relatively expensive atomic64. You only need one bit, and
> reading the tail pointer from WB memory should just be cheap. You should
> be able to sample the perf ringbuffer pointers very cheaply... What am I
> missing?
> -Chris
>
Fair comment.

The thing is with this series there are 2 mechanism that notify the poll_wq.

One is the hrtimer that kicks in at regular interval and polls the 
register with the workaround.

The other is the interrupt which doesn't read the registers and workaround.


Maybe a better way to do it would be to have 2 wait queues, one triggers 
a workqueue for the oa_buffer_check after the interrupt, the other 
notifies the existing poll_wq.


-

Lionel

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 5/7] drm/i915: handle interrupts from the OA unit

[Lionel Landwerlin]

On 16/01/2019 15:58, Lionel Landwerlin wrote:
> On 16/01/2019 15:52, Chris Wilson wrote:
>> Quoting Lionel Landwerlin (2019-01-16 15:36:20)
>>> @@ -1877,6 +1883,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;
>> Eh? But why a relatively expensive atomic64. You only need one bit, and
>> reading the tail pointer from WB memory should just be cheap. You should
>> be able to sample the perf ringbuffer pointers very cheaply... What am I
>> missing?
>> -Chris
>>
> Fair comment.
>
> The thing is with this series there are 2 mechanism that notify the 
> poll_wq.
>
> One is the hrtimer that kicks in at regular interval and polls the 
> register with the workaround.
>
> The other is the interrupt which doesn't read the registers and 
> workaround.


What I meant is that I need a way to know which one did the wake up so I 
can call oa_check_buffer() in the interrupt case, that's wait the 
atomic64 is there for.


>
>
> Maybe a better way to do it would be to have 2 wait queues, one 
> triggers a workqueue for the oa_buffer_check after the interrupt, the 
> other notifies the existing poll_wq.
>
>
> -
>
> Lionel
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 5/7] drm/i915: handle interrupts from the OA unit

[Chris Wilson]

Quoting Lionel Landwerlin (2019-01-16 15:58:00)
> On 16/01/2019 15:52, Chris Wilson wrote:
> > Quoting Lionel Landwerlin (2019-01-16 15:36:20)
> >> @@ -1877,6 +1883,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;
> > Eh? But why a relatively expensive atomic64. You only need one bit, and
> > reading the tail pointer from WB memory should just be cheap. You should
> > be able to sample the perf ringbuffer pointers very cheaply... What am I
> > missing?
> > -Chris
> >
> Fair comment.
> 
> The thing is with this series there are 2 mechanism that notify the poll_wq.
> 
> One is the hrtimer that kicks in at regular interval and polls the 
> register with the workaround.
> 
> The other is the interrupt which doesn't read the registers and workaround.

What's the complication with the workaround?

> Maybe a better way to do it would be to have 2 wait queues, one triggers 
> a workqueue for the oa_buffer_check after the interrupt, the other 
> notifies the existing poll_wq.

Ok, I was expecting that both the hrtimer, interrupt and general signal
handling (spurious wakeups) fed into the same mechanism to sample the
ringbuffer and notify the client if ready. (And I presume that we sample
from the client's process context anyway to save on the context switch.)
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 5/7] drm/i915: handle interrupts from the OA unit

[Chris Wilson]

Quoting Lionel Landwerlin (2019-01-16 16:02:51)
> On 16/01/2019 15:58, Lionel Landwerlin wrote:
> > On 16/01/2019 15:52, Chris Wilson wrote:
> >> Quoting Lionel Landwerlin (2019-01-16 15:36:20)
> >>> @@ -1877,6 +1883,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;
> >> Eh? But why a relatively expensive atomic64. You only need one bit, and
> >> reading the tail pointer from WB memory should just be cheap. You should
> >> be able to sample the perf ringbuffer pointers very cheaply... What am I
> >> missing?
> >> -Chris
> >>
> > Fair comment.
> >
> > The thing is with this series there are 2 mechanism that notify the 
> > poll_wq.
> >
> > One is the hrtimer that kicks in at regular interval and polls the 
> > register with the workaround.
> >
> > The other is the interrupt which doesn't read the registers and 
> > workaround.
> 
> 
> What I meant is that I need a way to know which one did the wake up so I 
> can call oa_check_buffer() in the interrupt case, that's wait the 
> atomic64 is there for.

I thought oa_check_buffer() was just "is there any data for me?"
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 5/7] drm/i915: handle interrupts from the OA unit

[Lionel Landwerlin]

On 16/01/2019 16:05, Chris Wilson wrote:
> Quoting Lionel Landwerlin (2019-01-16 15:58:00)
>> On 16/01/2019 15:52, Chris Wilson wrote:
>>> Quoting Lionel Landwerlin (2019-01-16 15:36:20)
>>>> @@ -1877,6 +1883,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;
>>> Eh? But why a relatively expensive atomic64. You only need one bit, and
>>> reading the tail pointer from WB memory should just be cheap. You should
>>> be able to sample the perf ringbuffer pointers very cheaply... What am I
>>> missing?
>>> -Chris
>>>
>> Fair comment.
>>
>> The thing is with this series there are 2 mechanism that notify the poll_wq.
>>
>> One is the hrtimer that kicks in at regular interval and polls the
>> register with the workaround.
>>
>> The other is the interrupt which doesn't read the registers and workaround.
> What's the complication with the workaround?


It's a bit more than just looking at registers, we actually have to look 
at the content of the buffer to figure out what landed in memory.

The register values are not synchronized with the memory writes...


There is a comment in the code (i915_perf_poll_locked) about not 
checking the register after each wakeup because that may be a very hot path.

The atomic64 sounded like a lesser evil.


>
>> Maybe a better way to do it would be to have 2 wait queues, one triggers
>> a workqueue for the oa_buffer_check after the interrupt, the other
>> notifies the existing poll_wq.
> Ok, I was expecting that both the hrtimer, interrupt and general signal
> handling (spurious wakeups) fed into the same mechanism to sample the
> ringbuffer and notify the client if ready. (And I presume that we sample
> from the client's process context anyway to save on the context switch.)


Correct, the client only deals with the output of oa_buffer_check().

The interrupt is just missing the oa_buffer_check() step which maybe we 
can do with a 2 stage thing.

Either timer+buffer_check -> wakeup

Or interrupt-> workqueue+buffer_check -> wakeup


-

Lionel


> -Chris
>

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 5/7] drm/i915: handle interrupts from the OA unit

[Chris Wilson]

Quoting Lionel Landwerlin (2019-01-16 16:25:26)
> On 16/01/2019 16:05, Chris Wilson wrote:
> > Quoting Lionel Landwerlin (2019-01-16 15:58:00)
> >> On 16/01/2019 15:52, Chris Wilson wrote:
> >>> Quoting Lionel Landwerlin (2019-01-16 15:36:20)
> >>>> @@ -1877,6 +1883,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;
> >>> Eh? But why a relatively expensive atomic64. You only need one bit, and
> >>> reading the tail pointer from WB memory should just be cheap. You should
> >>> be able to sample the perf ringbuffer pointers very cheaply... What am I
> >>> missing?
> >>> -Chris
> >>>
> >> Fair comment.
> >>
> >> The thing is with this series there are 2 mechanism that notify the poll_wq.
> >>
> >> One is the hrtimer that kicks in at regular interval and polls the
> >> register with the workaround.
> >>
> >> The other is the interrupt which doesn't read the registers and workaround.
> > What's the complication with the workaround?
> 
> 
> It's a bit more than just looking at registers, we actually have to look 
> at the content of the buffer to figure out what landed in memory.
> 
> The register values are not synchronized with the memory writes...

I don't want to look at registers at all for polling, and you shouldn't
need to since communication is via a ringbuf.
 
> There is a comment in the code (i915_perf_poll_locked) about not 
> checking the register after each wakeup because that may be a very hot path.
> 
> The atomic64 sounded like a lesser evil.

I'm clearly not understanding something here...

Does the hardware not do:
	update ringbuf data;
	wmb() (or post to global observation point in their parlance)
	update ringbuf tail

Then we just need to sample the ringbuf tail and compare against how far
we read last time?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/7] drm/i915/perf: reset pollin when perf stream is enabled

[Matthew Auld]

On Wed, 16 Jan 2019 at 15:36, Lionel Landwerlin
<lionel.g.landwerlin@intel.com> wrote:
>
> No issues have been raised about this yet, but this should be reset
> every time we enable the stream, otherwise we might have a stale value
> from the previous round of enable/disable.
>
> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_perf.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
> index 575701e2aabc..4c5d2ee4d6e3 100644
> --- a/drivers/gpu/drm/i915/i915_perf.c
> +++ b/drivers/gpu/drm/i915/i915_perf.c
> @@ -1876,6 +1876,8 @@ static void i915_oa_stream_enable(struct i915_perf_stream *stream)
>  {
>         struct drm_i915_private *dev_priv = stream->dev_priv;
>
> +       dev_priv->perf.oa.pollin = false;
> +
>         dev_priv->perf.oa.ops.oa_enable(stream);

Don't we fumigate oa.polling way down in ops.oa_enable() ?
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 5/7] drm/i915: handle interrupts from the OA unit

[Lionel Landwerlin]

On 16/01/2019 16:31, Chris Wilson wrote:
> Quoting Lionel Landwerlin (2019-01-16 16:25:26)
>> On 16/01/2019 16:05, Chris Wilson wrote:
>>> Quoting Lionel Landwerlin (2019-01-16 15:58:00)
>>>> On 16/01/2019 15:52, Chris Wilson wrote:
>>>>> Quoting Lionel Landwerlin (2019-01-16 15:36:20)
>>>>>> @@ -1877,6 +1883,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;
>>>>> Eh? But why a relatively expensive atomic64. You only need one bit, and
>>>>> reading the tail pointer from WB memory should just be cheap. You should
>>>>> be able to sample the perf ringbuffer pointers very cheaply... What am I
>>>>> missing?
>>>>> -Chris
>>>>>
>>>> Fair comment.
>>>>
>>>> The thing is with this series there are 2 mechanism that notify the poll_wq.
>>>>
>>>> One is the hrtimer that kicks in at regular interval and polls the
>>>> register with the workaround.
>>>>
>>>> The other is the interrupt which doesn't read the registers and workaround.
>>> What's the complication with the workaround?
>>
>> It's a bit more than just looking at registers, we actually have to look
>> at the content of the buffer to figure out what landed in memory.
>>
>> The register values are not synchronized with the memory writes...
> I don't want to look at registers at all for polling, and you shouldn't
> need to since communication is via a ringbuf.
>   
>> There is a comment in the code (i915_perf_poll_locked) about not
>> checking the register after each wakeup because that may be a very hot path.
>>
>> The atomic64 sounded like a lesser evil.
> I'm clearly not understanding something here...
>
> Does the hardware not do:
> 	update ringbuf data;
> 	wmb() (or post to global observation point in their parlance)
> 	update ringbuf tail


As far as I understand, the OA unit :

     sends its memory write requests to the memory controller

     immediately updates the ringbuf tail, without waiting for the 
previous requests to complete



>
> Then we just need to sample the ringbuf tail and compare against how far
> we read last time?
> -Chris
>

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/7] drm/i915/perf: reset pollin when perf stream is enabled

[Lionel Landwerlin]

On 16/01/2019 17:13, Matthew Auld wrote:
> On Wed, 16 Jan 2019 at 15:36, Lionel Landwerlin
> <lionel.g.landwerlin@intel.com> wrote:
>> No issues have been raised about this yet, but this should be reset
>> every time we enable the stream, otherwise we might have a stale value
>> from the previous round of enable/disable.
>>
>> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_perf.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
>> index 575701e2aabc..4c5d2ee4d6e3 100644
>> --- a/drivers/gpu/drm/i915/i915_perf.c
>> +++ b/drivers/gpu/drm/i915/i915_perf.c
>> @@ -1876,6 +1876,8 @@ static void i915_oa_stream_enable(struct i915_perf_stream *stream)
>>   {
>>          struct drm_i915_private *dev_priv = stream->dev_priv;
>>
>> +       dev_priv->perf.oa.pollin = false;
>> +
>>          dev_priv->perf.oa.ops.oa_enable(stream);
> Don't we fumigate oa.polling way down in ops.oa_enable() ?
>
Duh! You're right... It's the last step for gen7/8_init_oa_buffer().

I think I would like to move it into the generic setup if that's okay 
and leave the specific stuff to register manipulations.

Will update.


-

Lionel

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/7] drm/i915/perf: only append status when data is available

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 39790 bytes --]

Hi Lionel,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on drm-intel/for-linux-next]
[also build test WARNING on v5.0-rc2 next-20190116]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Lionel-Landwerlin/drm-i915-perf-rework-aging-tail-workaround/20190117-000149
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
reproduce: make htmldocs

All warnings (new ones prefixed by >>):

   include/linux/dma-buf.h:304: warning: Function parameter or member 'cb_excl.cb' not described in 'dma_buf'
   include/linux/dma-buf.h:304: warning: Function parameter or member 'cb_excl.poll' not described in 'dma_buf'
   include/linux/dma-buf.h:304: warning: Function parameter or member 'cb_excl.active' not described in 'dma_buf'
   include/linux/dma-buf.h:304: warning: Function parameter or member 'cb_shared.cb' not described in 'dma_buf'
   include/linux/dma-buf.h:304: warning: Function parameter or member 'cb_shared.poll' not described in 'dma_buf'
   include/linux/dma-buf.h:304: warning: Function parameter or member 'cb_shared.active' not described in 'dma_buf'
   include/linux/dma-fence-array.h:54: warning: Function parameter or member 'work' not described in 'dma_fence_array'
   include/linux/firmware/intel/stratix10-svc-client.h:1: warning: no structured comments found
   include/linux/gpio/driver.h:371: warning: Function parameter or member 'init_valid_mask' not described in 'gpio_chip'
   include/linux/iio/hw-consumer.h:1: warning: no structured comments found
   include/linux/input/sparse-keymap.h:46: warning: Function parameter or member 'sw' not described in 'key_entry'
   drivers/mtd/nand/raw/nand_base.c:420: warning: Function parameter or member 'chip' not described in 'nand_fill_oob'
   drivers/mtd/nand/raw/nand_bbt.c:173: warning: Function parameter or member 'this' not described in 'read_bbt'
   drivers/mtd/nand/raw/nand_bbt.c:173: warning: Excess function parameter 'chip' description in 'read_bbt'
   include/linux/regulator/machine.h:199: warning: Function parameter or member 'max_uV_step' not described in 'regulation_constraints'
   include/linux/regulator/driver.h:228: warning: Function parameter or member 'resume' not described in 'regulator_ops'
   arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.esw0' not described in 'irb'
   arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.esw1' not described in 'irb'
   arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.esw2' not described in 'irb'
   arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.esw3' not described in 'irb'
   arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.eadm' not described in 'irb'
   drivers/slimbus/stream.c:1: warning: no structured comments found
   include/linux/spi/spi.h:180: warning: Function parameter or member 'driver_override' not described in 'spi_device'
   drivers/target/target_core_device.c:1: warning: no structured comments found
   drivers/usb/typec/bus.c:1: warning: no structured comments found
   drivers/usb/typec/class.c:1: warning: no structured comments found
   include/linux/w1.h:281: warning: Function parameter or member 'of_match_table' not described in 'w1_family'
   fs/direct-io.c:257: warning: Excess function parameter 'offset' description in 'dio_complete'
   fs/file_table.c:1: warning: no structured comments found
   fs/libfs.c:477: warning: Excess function parameter 'available' description in 'simple_write_end'
   fs/posix_acl.c:646: warning: Function parameter or member 'inode' not described in 'posix_acl_update_mode'
   fs/posix_acl.c:646: warning: Function parameter or member 'mode_p' not described in 'posix_acl_update_mode'
   fs/posix_acl.c:646: warning: Function parameter or member 'acl' not described in 'posix_acl_update_mode'
   drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c:294: warning: Excess function parameter 'mm' description in 'amdgpu_mn_invalidate_range_start_hsa'
   drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c:294: warning: Excess function parameter 'start' description in 'amdgpu_mn_invalidate_range_start_hsa'
   drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c:294: warning: Excess function parameter 'end' description in 'amdgpu_mn_invalidate_range_start_hsa'
   drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c:343: warning: Excess function parameter 'mm' description in 'amdgpu_mn_invalidate_range_end'
   drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c:343: warning: Excess function parameter 'start' description in 'amdgpu_mn_invalidate_range_end'
   drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c:343: warning: Excess function parameter 'end' description in 'amdgpu_mn_invalidate_range_end'
   drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c:183: warning: Function parameter or member 'blockable' not described in 'amdgpu_mn_read_lock'
   drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c:295: warning: Function parameter or member 'range' not described in 'amdgpu_mn_invalidate_range_start_hsa'
   drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c:295: warning: Excess function parameter 'mm' description in 'amdgpu_mn_invalidate_range_start_hsa'
   drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c:295: warning: Excess function parameter 'start' description in 'amdgpu_mn_invalidate_range_start_hsa'
   drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c:295: warning: Excess function parameter 'end' description in 'amdgpu_mn_invalidate_range_start_hsa'
   drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c:344: warning: Function parameter or member 'range' not described in 'amdgpu_mn_invalidate_range_end'
   drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c:344: warning: Excess function parameter 'mm' description in 'amdgpu_mn_invalidate_range_end'
   drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c:344: warning: Excess function parameter 'start' description in 'amdgpu_mn_invalidate_range_end'
   drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c:344: warning: Excess function parameter 'end' description in 'amdgpu_mn_invalidate_range_end'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:382: warning: cannot understand function prototype: 'struct amdgpu_vm_pt_cursor '
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:383: warning: cannot understand function prototype: 'struct amdgpu_vm_pt_cursor '
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:555: warning: Function parameter or member 'adev' not described in 'for_each_amdgpu_vm_pt_leaf'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:555: warning: Function parameter or member 'vm' not described in 'for_each_amdgpu_vm_pt_leaf'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:555: warning: Function parameter or member 'start' not described in 'for_each_amdgpu_vm_pt_leaf'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:555: warning: Function parameter or member 'end' not described in 'for_each_amdgpu_vm_pt_leaf'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:555: warning: Function parameter or member 'cursor' not described in 'for_each_amdgpu_vm_pt_leaf'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:603: warning: Function parameter or member 'adev' not described in 'for_each_amdgpu_vm_pt_dfs_safe'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:603: warning: Function parameter or member 'vm' not described in 'for_each_amdgpu_vm_pt_dfs_safe'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:603: warning: Function parameter or member 'cursor' not described in 'for_each_amdgpu_vm_pt_dfs_safe'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:603: warning: Function parameter or member 'entry' not described in 'for_each_amdgpu_vm_pt_dfs_safe'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:845: warning: Function parameter or member 'level' not described in 'amdgpu_vm_bo_param'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1353: warning: Function parameter or member 'params' not described in 'amdgpu_vm_update_func'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1353: warning: Function parameter or member 'bo' not described in 'amdgpu_vm_update_func'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1353: warning: Function parameter or member 'pe' not described in 'amdgpu_vm_update_func'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1353: warning: Function parameter or member 'addr' not described in 'amdgpu_vm_update_func'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1353: warning: Function parameter or member 'count' not described in 'amdgpu_vm_update_func'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1353: warning: Function parameter or member 'incr' not described in 'amdgpu_vm_update_func'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1353: warning: Function parameter or member 'flags' not described in 'amdgpu_vm_update_func'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1520: warning: Function parameter or member 'params' not described in 'amdgpu_vm_update_huge'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1520: warning: Function parameter or member 'bo' not described in 'amdgpu_vm_update_huge'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1520: warning: Function parameter or member 'level' not described in 'amdgpu_vm_update_huge'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1520: warning: Function parameter or member 'pe' not described in 'amdgpu_vm_update_huge'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1520: warning: Function parameter or member 'addr' not described in 'amdgpu_vm_update_huge'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1520: warning: Function parameter or member 'count' not described in 'amdgpu_vm_update_huge'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1520: warning: Function parameter or member 'incr' not described in 'amdgpu_vm_update_huge'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1520: warning: Function parameter or member 'flags' not described in 'amdgpu_vm_update_huge'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:3096: warning: Function parameter or member 'pasid' not described in 'amdgpu_vm_make_compute'
   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h:128: warning: Incorrect use of kernel-doc format: Documentation Makefile include scripts source @atomic_obj
   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h:203: warning: Function parameter or member 'atomic_obj' not described in 'amdgpu_display_manager'
   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h:203: warning: Function parameter or member 'atomic_obj_lock' not described in 'amdgpu_display_manager'
   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h:203: warning: Function parameter or member 'backlight_link' not described in 'amdgpu_display_manager'
   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h:203: warning: Function parameter or member 'backlight_caps' not described in 'amdgpu_display_manager'
   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h:203: warning: Function parameter or member 'freesync_module' not described in 'amdgpu_display_manager'
   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h:203: warning: Function parameter or member 'fw_dmcu' not described in 'amdgpu_display_manager'
   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h:203: warning: Function parameter or member 'dmcu_fw_version' not described in 'amdgpu_display_manager'
   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c:1: warning: no structured comments found
   include/drm/drm_drv.h:618: warning: Function parameter or member 'gem_prime_pin' not described in 'drm_driver'
   include/drm/drm_drv.h:618: warning: Function parameter or member 'gem_prime_unpin' not described in 'drm_driver'
   include/drm/drm_drv.h:618: warning: Function parameter or member 'gem_prime_res_obj' not described in 'drm_driver'
   include/drm/drm_drv.h:618: warning: Function parameter or member 'gem_prime_get_sg_table' not described in 'drm_driver'
   include/drm/drm_drv.h:618: warning: Function parameter or member 'gem_prime_import_sg_table' not described in 'drm_driver'
   include/drm/drm_drv.h:618: warning: Function parameter or member 'gem_prime_vmap' not described in 'drm_driver'
   include/drm/drm_drv.h:618: warning: Function parameter or member 'gem_prime_vunmap' not described in 'drm_driver'
   include/drm/drm_drv.h:618: warning: Function parameter or member 'gem_prime_mmap' not described in 'drm_driver'
   include/drm/drm_atomic_state_helper.h:1: warning: no structured comments found
   drivers/gpu/drm/drm_dp_helper.c:1364: warning: Function parameter or member 'dsc_dpcd' not described in 'drm_dp_dsc_sink_max_slice_count'
   drivers/gpu/drm/drm_dp_helper.c:1364: warning: Function parameter or member 'is_edp' not described in 'drm_dp_dsc_sink_max_slice_count'
   drivers/gpu/drm/i915/i915_vma.h:49: warning: cannot understand function prototype: 'struct i915_vma '
   drivers/gpu/drm/i915/i915_vma.h:1: warning: no structured comments found
   drivers/gpu/drm/i915/intel_guc_fwif.h:536: warning: cannot understand function prototype: 'struct guc_log_buffer_state '
   drivers/gpu/drm/i915/i915_trace.h:1: warning: no structured comments found
>> drivers/gpu/drm/i915/i915_perf.c:644: warning: Function parameter or member 'oastatus' not described in 'gen8_append_oa_reports'
>> drivers/gpu/drm/i915/i915_perf.c:928: warning: Function parameter or member 'oastatus1' not described in 'gen7_append_oa_reports'
   include/linux/skbuff.h:876: warning: Function parameter or member 'dev_scratch' not described in 'sk_buff'
   include/linux/skbuff.h:876: warning: Function parameter or member 'list' not described in 'sk_buff'
   include/linux/skbuff.h:876: warning: Function parameter or member 'ip_defrag_offset' not described in 'sk_buff'
   include/linux/skbuff.h:876: warning: Function parameter or member 'skb_mstamp_ns' not described in 'sk_buff'
   include/linux/skbuff.h:876: warning: Function parameter or member '__cloned_offset' not described in 'sk_buff'
   include/linux/skbuff.h:876: warning: Function parameter or member 'head_frag' not described in 'sk_buff'
   include/linux/skbuff.h:876: warning: Function parameter or member '__pkt_type_offset' not described in 'sk_buff'
   include/linux/skbuff.h:876: warning: Function parameter or member 'encapsulation' not described in 'sk_buff'
   include/linux/skbuff.h:876: warning: Function parameter or member 'encap_hdr_csum' not described in 'sk_buff'
   include/linux/skbuff.h:876: warning: Function parameter or member 'csum_valid' not described in 'sk_buff'
   include/linux/skbuff.h:876: warning: Function parameter or member '__pkt_vlan_present_offset' not described in 'sk_buff'
   include/linux/skbuff.h:876: warning: Function parameter or member 'vlan_present' not described in 'sk_buff'
   include/linux/skbuff.h:876: warning: Function parameter or member 'csum_complete_sw' not described in 'sk_buff'
   include/linux/skbuff.h:876: warning: Function parameter or member 'csum_level' not described in 'sk_buff'
   include/linux/skbuff.h:876: warning: Function parameter or member 'inner_protocol_type' not described in 'sk_buff'
   include/linux/skbuff.h:876: warning: Function parameter or member 'remcsum_offload' not described in 'sk_buff'
   include/linux/skbuff.h:876: warning: Function parameter or member 'sender_cpu' not described in 'sk_buff'
   include/linux/skbuff.h:876: warning: Function parameter or member 'reserved_tailroom' not described in 'sk_buff'
   include/linux/skbuff.h:876: warning: Function parameter or member 'inner_ipproto' not described in 'sk_buff'
   include/net/sock.h:238: warning: Function parameter or member 'skc_addrpair' not described in 'sock_common'
   include/net/sock.h:238: warning: Function parameter or member 'skc_portpair' not described in 'sock_common'
   include/net/sock.h:238: warning: Function parameter or member 'skc_ipv6only' not described in 'sock_common'
   include/net/sock.h:238: warning: Function parameter or member 'skc_net_refcnt' not described in 'sock_common'
   include/net/sock.h:238: warning: Function parameter or member 'skc_v6_daddr' not described in 'sock_common'
   include/net/sock.h:238: warning: Function parameter or member 'skc_v6_rcv_saddr' not described in 'sock_common'
   include/net/sock.h:238: warning: Function parameter or member 'skc_cookie' not described in 'sock_common'
   include/net/sock.h:238: warning: Function parameter or member 'skc_listener' not described in 'sock_common'
   include/net/sock.h:238: warning: Function parameter or member 'skc_tw_dr' not described in 'sock_common'
   include/net/sock.h:238: warning: Function parameter or member 'skc_rcv_wnd' not described in 'sock_common'
   include/net/sock.h:238: warning: Function parameter or member 'skc_tw_rcv_nxt' not described in 'sock_common'
   include/net/sock.h:513: warning: Function parameter or member 'sk_backlog.rmem_alloc' not described in 'sock'
   include/net/sock.h:513: warning: Function parameter or member 'sk_backlog.len' not described in 'sock'
   include/net/sock.h:513: warning: Function parameter or member 'sk_backlog.head' not described in 'sock'
   include/net/sock.h:513: warning: Function parameter or member 'sk_backlog.tail' not described in 'sock'
   include/net/sock.h:513: warning: Function parameter or member 'sk_wq_raw' not described in 'sock'
   include/net/sock.h:513: warning: Function parameter or member 'tcp_rtx_queue' not described in 'sock'
   include/net/sock.h:513: warning: Function parameter or member 'sk_route_forced_caps' not described in 'sock'
   include/net/sock.h:513: warning: Function parameter or member 'sk_txtime_report_errors' not described in 'sock'
   include/net/sock.h:513: warning: Function parameter or member 'sk_validate_xmit_skb' not described in 'sock'
   include/linux/netdevice.h:2048: warning: Function parameter or member 'adj_list.upper' not described in 'net_device'
   include/linux/netdevice.h:2048: warning: Function parameter or member 'adj_list.lower' not described in 'net_device'
   include/linux/netdevice.h:2048: warning: Function parameter or member 'gso_partial_features' not described in 'net_device'
   include/linux/netdevice.h:2048: warning: Function parameter or member 'switchdev_ops' not described in 'net_device'
   include/linux/netdevice.h:2048: warning: Function parameter or member 'l3mdev_ops' not described in 'net_device'
   include/linux/netdevice.h:2048: warning: Function parameter or member 'xfrmdev_ops' not described in 'net_device'
   include/linux/netdevice.h:2048: warning: Function parameter or member 'tlsdev_ops' not described in 'net_device'
   include/linux/netdevice.h:2048: warning: Function parameter or member 'name_assign_type' not described in 'net_device'
   include/linux/netdevice.h:2048: warning: Function parameter or member 'ieee802154_ptr' not described in 'net_device'
   include/linux/netdevice.h:2048: warning: Function parameter or member 'mpls_ptr' not described in 'net_device'
   include/linux/netdevice.h:2048: warning: Function parameter or member 'xdp_prog' not described in 'net_device'
   include/linux/netdevice.h:2048: warning: Function parameter or member 'gro_flush_timeout' not described in 'net_device'
   include/linux/netdevice.h:2048: warning: Function parameter or member 'nf_hooks_ingress' not described in 'net_device'
   include/linux/netdevice.h:2048: warning: Function parameter or member '____cacheline_aligned_in_smp' not described in 'net_device'
   include/linux/netdevice.h:2048: warning: Function parameter or member 'qdisc_hash' not described in 'net_device'
   include/linux/netdevice.h:2048: warning: Function parameter or member 'xps_cpus_map' not described in 'net_device'
   include/linux/netdevice.h:2048: warning: Function parameter or member 'xps_rxqs_map' not described in 'net_device'
   include/linux/phylink.h:56: warning: Function parameter or member '__ETHTOOL_DECLARE_LINK_MODE_MASK(advertising' not described in 'phylink_link_state'
   include/linux/phylink.h:56: warning: Function parameter or member '__ETHTOOL_DECLARE_LINK_MODE_MASK(lp_advertising' not described in 'phylink_link_state'
   Documentation/admin-guide/cgroup-v2.rst:1509: WARNING: Block quote ends without a blank line; unexpected unindent.
   Documentation/admin-guide/cgroup-v2.rst:1511: WARNING: Block quote ends without a blank line; unexpected unindent.
   Documentation/admin-guide/cgroup-v2.rst:1512: WARNING: Block quote ends without a blank line; unexpected unindent.
   include/linux/interrupt.h:252: WARNING: Inline emphasis start-string without end-string.
   include/net/mac80211.h:1214: ERROR: Unexpected indentation.
   include/net/mac80211.h:1221: WARNING: Block quote ends without a blank line; unexpected unindent.
   include/linux/wait.h:110: WARNING: Block quote ends without a blank line; unexpected unindent.
   include/linux/wait.h:113: ERROR: Unexpected indentation.
   include/linux/wait.h:115: WARNING: Block quote ends without a blank line; unexpected unindent.
   kernel/time/hrtimer.c:1120: WARNING: Block quote ends without a blank line; unexpected unindent.
   kernel/signal.c:344: WARNING: Inline literal start-string without end-string.
   include/linux/kernel.h:137: WARNING: Inline interpreted text or phrase reference start-string without end-string.
   Documentation/driver-api/dmaengine/dmatest.rst:63: ERROR: Unexpected indentation.
   include/uapi/linux/firewire-cdev.h:312: WARNING: Inline literal start-string without end-string.
   Documentation/driver-api/gpio/board.rst:209: ERROR: Unexpected indentation.
   drivers/ata/libata-core.c:5959: ERROR: Unknown target name: "hw".
   drivers/message/fusion/mptbase.c:5057: WARNING: Definition list ends without a blank line; unexpected unindent.
   drivers/tty/serial/serial_core.c:1948: WARNING: Definition list ends without a blank line; unexpected unindent.
   include/linux/mtd/rawnand.h:1192: WARNING: Inline strong start-string without end-string.
   include/linux/mtd/rawnand.h:1194: WARNING: Inline strong start-string without end-string.
   include/linux/regulator/driver.h:287: ERROR: Unknown target name: "regulator_regmap_x_voltage".
   Documentation/driver-api/soundwire/locking.rst:50: ERROR: Inconsistent literal block quoting.
   Documentation/driver-api/soundwire/locking.rst:51: WARNING: Line block ends without a blank line.
   Documentation/driver-api/soundwire/locking.rst:55: WARNING: Inline substitution_reference start-string without end-string.
   Documentation/driver-api/soundwire/locking.rst:56: WARNING: Line block ends without a blank line.
   include/linux/spi/spi.h:368: ERROR: Unexpected indentation.
   fs/posix_acl.c:635: WARNING: Inline emphasis start-string without end-string.
   Documentation/filesystems/path-lookup.rst:347: WARNING: Title underline too short.

vim +644 drivers/gpu/drm/i915/i915_perf.c

d79651522 Robert Bragg      2016-11-07  617  
d79651522 Robert Bragg      2016-11-07  618  /**
d79651522 Robert Bragg      2016-11-07  619   * Copies all buffered OA reports into userspace read() buffer.
d79651522 Robert Bragg      2016-11-07  620   * @stream: An i915-perf stream opened for OA metrics
d79651522 Robert Bragg      2016-11-07  621   * @buf: destination buffer given by userspace
d79651522 Robert Bragg      2016-11-07  622   * @count: the number of bytes userspace wants to read
d79651522 Robert Bragg      2016-11-07  623   * @offset: (inout): the current position for writing into @buf
d79651522 Robert Bragg      2016-11-07  624   *
16d98b31f Robert Bragg      2016-12-07  625   * Notably any error condition resulting in a short read (-%ENOSPC or
16d98b31f Robert Bragg      2016-12-07  626   * -%EFAULT) will be returned even though one or more records may
d79651522 Robert Bragg      2016-11-07  627   * have been successfully copied. In this case it's up to the caller
d79651522 Robert Bragg      2016-11-07  628   * to decide if the error should be squashed before returning to
d79651522 Robert Bragg      2016-11-07  629   * userspace.
d79651522 Robert Bragg      2016-11-07  630   *
d79651522 Robert Bragg      2016-11-07  631   * Note: reports are consumed from the head, and appended to the
e81b3a555 Robert Bragg      2017-05-11  632   * tail, so the tail chases the head?... If you think that's mad
d79651522 Robert Bragg      2016-11-07  633   * and back-to-front you're not alone, but this follows the
d79651522 Robert Bragg      2016-11-07  634   * Gen PRM naming convention.
16d98b31f Robert Bragg      2016-12-07  635   *
16d98b31f Robert Bragg      2016-12-07  636   * Returns: 0 on success, negative error code on failure.
d79651522 Robert Bragg      2016-11-07  637   */
19f81df28 Robert Bragg      2017-06-13  638  static int gen8_append_oa_reports(struct i915_perf_stream *stream,
6e39236ad Lionel Landwerlin 2019-01-16  639  				  u32 oastatus,
19f81df28 Robert Bragg      2017-06-13  640  				  char __user *buf,
19f81df28 Robert Bragg      2017-06-13  641  				  size_t count,
19f81df28 Robert Bragg      2017-06-13  642  				  size_t *offset)
19f81df28 Robert Bragg      2017-06-13  643  {
19f81df28 Robert Bragg      2017-06-13 @644  	struct drm_i915_private *dev_priv = stream->dev_priv;
19f81df28 Robert Bragg      2017-06-13  645  	int report_size = dev_priv->perf.oa.oa_buffer.format_size;
19f81df28 Robert Bragg      2017-06-13  646  	u8 *oa_buf_base = dev_priv->perf.oa.oa_buffer.vaddr;
19f81df28 Robert Bragg      2017-06-13  647  	u32 gtt_offset = i915_ggtt_offset(dev_priv->perf.oa.oa_buffer.vma);
fe8416864 Joonas Lahtinen   2018-11-16  648  	u32 mask = (OA_BUFFER_SIZE - 1);
19f81df28 Robert Bragg      2017-06-13  649  	size_t start_offset = *offset;
19f81df28 Robert Bragg      2017-06-13  650  	unsigned long flags;
19f81df28 Robert Bragg      2017-06-13  651  	u32 head, tail;
19f81df28 Robert Bragg      2017-06-13  652  	u32 taken;
19f81df28 Robert Bragg      2017-06-13  653  	int ret = 0;
19f81df28 Robert Bragg      2017-06-13  654  
19f81df28 Robert Bragg      2017-06-13  655  	if (WARN_ON(!stream->enabled))
19f81df28 Robert Bragg      2017-06-13  656  		return -EIO;
19f81df28 Robert Bragg      2017-06-13  657  
19f81df28 Robert Bragg      2017-06-13  658  	spin_lock_irqsave(&dev_priv->perf.oa.oa_buffer.ptr_lock, flags);
19f81df28 Robert Bragg      2017-06-13  659  
19f81df28 Robert Bragg      2017-06-13  660  	head = dev_priv->perf.oa.oa_buffer.head;
b667af43d Lionel Landwerlin 2019-01-16  661  	tail = dev_priv->perf.oa.oa_buffer.tail;
19f81df28 Robert Bragg      2017-06-13  662  
19f81df28 Robert Bragg      2017-06-13  663  	spin_unlock_irqrestore(&dev_priv->perf.oa.oa_buffer.ptr_lock, flags);
19f81df28 Robert Bragg      2017-06-13  664  
19f81df28 Robert Bragg      2017-06-13  665  	/*
19f81df28 Robert Bragg      2017-06-13  666  	 * NB: oa_buffer.head/tail include the gtt_offset which we don't want
19f81df28 Robert Bragg      2017-06-13  667  	 * while indexing relative to oa_buf_base.
19f81df28 Robert Bragg      2017-06-13  668  	 */
19f81df28 Robert Bragg      2017-06-13  669  	head -= gtt_offset;
19f81df28 Robert Bragg      2017-06-13  670  	tail -= gtt_offset;
19f81df28 Robert Bragg      2017-06-13  671  
19f81df28 Robert Bragg      2017-06-13  672  	/*
19f81df28 Robert Bragg      2017-06-13  673  	 * An out of bounds or misaligned head or tail pointer implies a driver
19f81df28 Robert Bragg      2017-06-13  674  	 * bug since we validate + align the tail pointers we read from the
19f81df28 Robert Bragg      2017-06-13  675  	 * hardware and we are in full control of the head pointer which should
19f81df28 Robert Bragg      2017-06-13  676  	 * only be incremented by multiples of the report size (notably also
19f81df28 Robert Bragg      2017-06-13  677  	 * all a power of two).
19f81df28 Robert Bragg      2017-06-13  678  	 */
fe8416864 Joonas Lahtinen   2018-11-16  679  	if (WARN_ONCE(head > OA_BUFFER_SIZE || head % report_size ||
fe8416864 Joonas Lahtinen   2018-11-16  680  		      tail > OA_BUFFER_SIZE || tail % report_size,
19f81df28 Robert Bragg      2017-06-13  681  		      "Inconsistent OA buffer pointers: head = %u, tail = %u\n",
19f81df28 Robert Bragg      2017-06-13  682  		      head, tail))
19f81df28 Robert Bragg      2017-06-13  683  		return -EIO;
19f81df28 Robert Bragg      2017-06-13  684  
6e39236ad Lionel Landwerlin 2019-01-16  685  	/*
6e39236ad Lionel Landwerlin 2019-01-16  686  	 * If there is nothing to read, don't append the status report yet,
6e39236ad Lionel Landwerlin 2019-01-16  687  	 * wait until we have some data available.
6e39236ad Lionel Landwerlin 2019-01-16  688  	 */
6e39236ad Lionel Landwerlin 2019-01-16  689  	if (!OA_TAKEN(tail, head))
6e39236ad Lionel Landwerlin 2019-01-16  690  		return 0;
6e39236ad Lionel Landwerlin 2019-01-16  691  
6e39236ad Lionel Landwerlin 2019-01-16  692  	if (oastatus & GEN8_OASTATUS_REPORT_LOST) {
6e39236ad Lionel Landwerlin 2019-01-16  693  		ret = append_oa_status(stream, buf, count, offset,
6e39236ad Lionel Landwerlin 2019-01-16  694  				       DRM_I915_PERF_RECORD_OA_REPORT_LOST);
6e39236ad Lionel Landwerlin 2019-01-16  695  		if (ret)
6e39236ad Lionel Landwerlin 2019-01-16  696  			return ret;
6e39236ad Lionel Landwerlin 2019-01-16  697  		I915_WRITE(GEN8_OASTATUS,
6e39236ad Lionel Landwerlin 2019-01-16  698  			   oastatus & ~GEN8_OASTATUS_REPORT_LOST);
6e39236ad Lionel Landwerlin 2019-01-16  699  	}
19f81df28 Robert Bragg      2017-06-13  700  
19f81df28 Robert Bragg      2017-06-13  701  	for (/* none */;
19f81df28 Robert Bragg      2017-06-13  702  	     (taken = OA_TAKEN(tail, head));
19f81df28 Robert Bragg      2017-06-13  703  	     head = (head + report_size) & mask) {
19f81df28 Robert Bragg      2017-06-13  704  		u8 *report = oa_buf_base + head;
19f81df28 Robert Bragg      2017-06-13  705  		u32 *report32 = (void *)report;
19f81df28 Robert Bragg      2017-06-13  706  		u32 ctx_id;
19f81df28 Robert Bragg      2017-06-13  707  		u32 reason;
19f81df28 Robert Bragg      2017-06-13  708  
19f81df28 Robert Bragg      2017-06-13  709  		/*
19f81df28 Robert Bragg      2017-06-13  710  		 * All the report sizes factor neatly into the buffer
19f81df28 Robert Bragg      2017-06-13  711  		 * size so we never expect to see a report split
19f81df28 Robert Bragg      2017-06-13  712  		 * between the beginning and end of the buffer.
19f81df28 Robert Bragg      2017-06-13  713  		 *
19f81df28 Robert Bragg      2017-06-13  714  		 * Given the initial alignment check a misalignment
19f81df28 Robert Bragg      2017-06-13  715  		 * here would imply a driver bug that would result
19f81df28 Robert Bragg      2017-06-13  716  		 * in an overrun.
19f81df28 Robert Bragg      2017-06-13  717  		 */
fe8416864 Joonas Lahtinen   2018-11-16  718  		if (WARN_ON((OA_BUFFER_SIZE - head) < report_size)) {
19f81df28 Robert Bragg      2017-06-13  719  			DRM_ERROR("Spurious OA head ptr: non-integral report offset\n");
19f81df28 Robert Bragg      2017-06-13  720  			break;
19f81df28 Robert Bragg      2017-06-13  721  		}
19f81df28 Robert Bragg      2017-06-13  722  
19f81df28 Robert Bragg      2017-06-13  723  		/*
19f81df28 Robert Bragg      2017-06-13  724  		 * The reason field includes flags identifying what
19f81df28 Robert Bragg      2017-06-13  725  		 * triggered this specific report (mostly timer
19f81df28 Robert Bragg      2017-06-13  726  		 * triggered or e.g. due to a context switch).
19f81df28 Robert Bragg      2017-06-13  727  		 *
19f81df28 Robert Bragg      2017-06-13  728  		 * This field is never expected to be zero so we can
19f81df28 Robert Bragg      2017-06-13  729  		 * check that the report isn't invalid before copying
19f81df28 Robert Bragg      2017-06-13  730  		 * it to userspace...
19f81df28 Robert Bragg      2017-06-13  731  		 */
19f81df28 Robert Bragg      2017-06-13  732  		reason = ((report32[0] >> OAREPORT_REASON_SHIFT) &
19f81df28 Robert Bragg      2017-06-13  733  			  OAREPORT_REASON_MASK);
19f81df28 Robert Bragg      2017-06-13  734  		if (reason == 0) {
19f81df28 Robert Bragg      2017-06-13  735  			if (__ratelimit(&dev_priv->perf.oa.spurious_report_rs))
19f81df28 Robert Bragg      2017-06-13  736  				DRM_NOTE("Skipping spurious, invalid OA report\n");
19f81df28 Robert Bragg      2017-06-13  737  			continue;
19f81df28 Robert Bragg      2017-06-13  738  		}
19f81df28 Robert Bragg      2017-06-13  739  
61d5676b5 Lionel Landwerlin 2018-06-02  740  		ctx_id = report32[2] & dev_priv->perf.oa.specific_ctx_id_mask;
19f81df28 Robert Bragg      2017-06-13  741  
19f81df28 Robert Bragg      2017-06-13  742  		/*
19f81df28 Robert Bragg      2017-06-13  743  		 * Squash whatever is in the CTX_ID field if it's marked as
19f81df28 Robert Bragg      2017-06-13  744  		 * invalid to be sure we avoid false-positive, single-context
19f81df28 Robert Bragg      2017-06-13  745  		 * filtering below...
19f81df28 Robert Bragg      2017-06-13  746  		 *
19f81df28 Robert Bragg      2017-06-13  747  		 * Note: that we don't clear the valid_ctx_bit so userspace can
19f81df28 Robert Bragg      2017-06-13  748  		 * understand that the ID has been squashed by the kernel.
19f81df28 Robert Bragg      2017-06-13  749  		 */
19f81df28 Robert Bragg      2017-06-13  750  		if (!(report32[0] & dev_priv->perf.oa.gen8_valid_ctx_bit))
19f81df28 Robert Bragg      2017-06-13  751  			ctx_id = report32[2] = INVALID_CTX_ID;
19f81df28 Robert Bragg      2017-06-13  752  
19f81df28 Robert Bragg      2017-06-13  753  		/*
19f81df28 Robert Bragg      2017-06-13  754  		 * NB: For Gen 8 the OA unit no longer supports clock gating
19f81df28 Robert Bragg      2017-06-13  755  		 * off for a specific context and the kernel can't securely
19f81df28 Robert Bragg      2017-06-13  756  		 * stop the counters from updating as system-wide / global
19f81df28 Robert Bragg      2017-06-13  757  		 * values.
19f81df28 Robert Bragg      2017-06-13  758  		 *
19f81df28 Robert Bragg      2017-06-13  759  		 * Automatic reports now include a context ID so reports can be
19f81df28 Robert Bragg      2017-06-13  760  		 * filtered on the cpu but it's not worth trying to
19f81df28 Robert Bragg      2017-06-13  761  		 * automatically subtract/hide counter progress for other
19f81df28 Robert Bragg      2017-06-13  762  		 * contexts while filtering since we can't stop userspace
19f81df28 Robert Bragg      2017-06-13  763  		 * issuing MI_REPORT_PERF_COUNT commands which would still
19f81df28 Robert Bragg      2017-06-13  764  		 * provide a side-band view of the real values.
19f81df28 Robert Bragg      2017-06-13  765  		 *
19f81df28 Robert Bragg      2017-06-13  766  		 * To allow userspace (such as Mesa/GL_INTEL_performance_query)
19f81df28 Robert Bragg      2017-06-13  767  		 * to normalize counters for a single filtered context then it
19f81df28 Robert Bragg      2017-06-13  768  		 * needs be forwarded bookend context-switch reports so that it
19f81df28 Robert Bragg      2017-06-13  769  		 * can track switches in between MI_REPORT_PERF_COUNT commands
19f81df28 Robert Bragg      2017-06-13  770  		 * and can itself subtract/ignore the progress of counters
19f81df28 Robert Bragg      2017-06-13  771  		 * associated with other contexts. Note that the hardware
19f81df28 Robert Bragg      2017-06-13  772  		 * automatically triggers reports when switching to a new
19f81df28 Robert Bragg      2017-06-13  773  		 * context which are tagged with the ID of the newly active
19f81df28 Robert Bragg      2017-06-13  774  		 * context. To avoid the complexity (and likely fragility) of
19f81df28 Robert Bragg      2017-06-13  775  		 * reading ahead while parsing reports to try and minimize
19f81df28 Robert Bragg      2017-06-13  776  		 * forwarding redundant context switch reports (i.e. between
19f81df28 Robert Bragg      2017-06-13  777  		 * other, unrelated contexts) we simply elect to forward them
19f81df28 Robert Bragg      2017-06-13  778  		 * all.
19f81df28 Robert Bragg      2017-06-13  779  		 *
19f81df28 Robert Bragg      2017-06-13  780  		 * We don't rely solely on the reason field to identify context
19f81df28 Robert Bragg      2017-06-13  781  		 * switches since it's not-uncommon for periodic samples to
19f81df28 Robert Bragg      2017-06-13  782  		 * identify a switch before any 'context switch' report.
19f81df28 Robert Bragg      2017-06-13  783  		 */
19f81df28 Robert Bragg      2017-06-13  784  		if (!dev_priv->perf.oa.exclusive_stream->ctx ||
19f81df28 Robert Bragg      2017-06-13  785  		    dev_priv->perf.oa.specific_ctx_id == ctx_id ||
19f81df28 Robert Bragg      2017-06-13  786  		    (dev_priv->perf.oa.oa_buffer.last_ctx_id ==
19f81df28 Robert Bragg      2017-06-13  787  		     dev_priv->perf.oa.specific_ctx_id) ||
19f81df28 Robert Bragg      2017-06-13  788  		    reason & OAREPORT_REASON_CTX_SWITCH) {
19f81df28 Robert Bragg      2017-06-13  789  
19f81df28 Robert Bragg      2017-06-13  790  			/*
19f81df28 Robert Bragg      2017-06-13  791  			 * While filtering for a single context we avoid
19f81df28 Robert Bragg      2017-06-13  792  			 * leaking the IDs of other contexts.
19f81df28 Robert Bragg      2017-06-13  793  			 */
19f81df28 Robert Bragg      2017-06-13  794  			if (dev_priv->perf.oa.exclusive_stream->ctx &&
19f81df28 Robert Bragg      2017-06-13  795  			    dev_priv->perf.oa.specific_ctx_id != ctx_id) {
19f81df28 Robert Bragg      2017-06-13  796  				report32[2] = INVALID_CTX_ID;
19f81df28 Robert Bragg      2017-06-13  797  			}
19f81df28 Robert Bragg      2017-06-13  798  
19f81df28 Robert Bragg      2017-06-13  799  			ret = append_oa_sample(stream, buf, count, offset,
19f81df28 Robert Bragg      2017-06-13  800  					       report);
19f81df28 Robert Bragg      2017-06-13  801  			if (ret)
19f81df28 Robert Bragg      2017-06-13  802  				break;
19f81df28 Robert Bragg      2017-06-13  803  
19f81df28 Robert Bragg      2017-06-13  804  			dev_priv->perf.oa.oa_buffer.last_ctx_id = ctx_id;
19f81df28 Robert Bragg      2017-06-13  805  		}
19f81df28 Robert Bragg      2017-06-13  806  
19f81df28 Robert Bragg      2017-06-13  807  		/*
b667af43d Lionel Landwerlin 2019-01-16  808  		 * Clear out the first 2 dword as a mean to detect unlanded
b667af43d Lionel Landwerlin 2019-01-16  809  		 * reports.
19f81df28 Robert Bragg      2017-06-13  810  		 */
b667af43d Lionel Landwerlin 2019-01-16  811  		report32[0] = report32[1] = 0;
19f81df28 Robert Bragg      2017-06-13  812  	}
19f81df28 Robert Bragg      2017-06-13  813  
19f81df28 Robert Bragg      2017-06-13  814  	if (start_offset != *offset) {
19f81df28 Robert Bragg      2017-06-13  815  		spin_lock_irqsave(&dev_priv->perf.oa.oa_buffer.ptr_lock, flags);
19f81df28 Robert Bragg      2017-06-13  816  
19f81df28 Robert Bragg      2017-06-13  817  		/*
19f81df28 Robert Bragg      2017-06-13  818  		 * We removed the gtt_offset for the copy loop above, indexing
19f81df28 Robert Bragg      2017-06-13  819  		 * relative to oa_buf_base so put back here...
19f81df28 Robert Bragg      2017-06-13  820  		 */
19f81df28 Robert Bragg      2017-06-13  821  		head += gtt_offset;
19f81df28 Robert Bragg      2017-06-13  822  
19f81df28 Robert Bragg      2017-06-13  823  		I915_WRITE(GEN8_OAHEADPTR, head & GEN8_OAHEADPTR_MASK);
19f81df28 Robert Bragg      2017-06-13  824  		dev_priv->perf.oa.oa_buffer.head = head;
19f81df28 Robert Bragg      2017-06-13  825  
19f81df28 Robert Bragg      2017-06-13  826  		spin_unlock_irqrestore(&dev_priv->perf.oa.oa_buffer.ptr_lock, flags);
19f81df28 Robert Bragg      2017-06-13  827  	}
19f81df28 Robert Bragg      2017-06-13  828  
19f81df28 Robert Bragg      2017-06-13  829  	return ret;
19f81df28 Robert Bragg      2017-06-13  830  }
19f81df28 Robert Bragg      2017-06-13  831  

:::::: The code at line 644 was first introduced by commit
:::::: 19f81df2859eb10e92d68991cefa39f826dea013 drm/i915/perf: Add OA unit support for Gen 8+

:::::: TO: Robert Bragg <robert@sixbynine.org>
:::::: CC: Ben Widawsky <ben@bwidawsk.net>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 6630 bytes --]

[-- Attachment #3: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.CHECKPATCH: warning for drm/i915/perf: add OA interrupt support (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915/perf: add OA interrupt support (rev2)
URL   : https://patchwork.freedesktop.org/series/54280/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
f0460c1ade9e drm/i915/perf: rework aging tail workaround
-:241: CHECK:SPACING: No space is necessary after a cast
#241: FILE: drivers/gpu/drm/i915/i915_perf.c:500:
+			u32 *report32 = (void *) report;

-:319: CHECK:MULTIPLE_ASSIGNMENTS: multiple assignments should be avoided
#319: FILE: drivers/gpu/drm/i915/i915_perf.c:795:
+		report32[0] = report32[1] = 0;

-:363: CHECK:MULTIPLE_ASSIGNMENTS: multiple assignments should be avoided
#363: FILE: drivers/gpu/drm/i915/i915_perf.c:998:
+		report32[0] = report32[1] = 0;

total: 0 errors, 0 warnings, 3 checks, 354 lines checked
94b8c039703f drm/i915/perf: reset pollin when perf stream is enabled
47f65f25c7c0 drm/i915/perf: only append status when data is available
4fbe19755ae5 drm/i915/perf: add new open param to configure polling of OA buffer
3bbe259283a3 drm/i915: handle interrupts from the OA unit
0c7a8458e075 drm/i915/perf: add interrupt enabling parameter
-:7: WARNING:TYPO_SPELLING: 'conjuction' may be misspelled - perhaps 'conjunction'?
#7: 
mechanism of the HW. In conjuction with long periods for checks for

-:30: CHECK:LINE_SPACING: Please don't use multiple blank lines
#30: FILE: drivers/gpu/drm/i915/i915_perf.c:421:
 
+

-:122: CHECK:UNNECESSARY_PARENTHESES: Unnecessary parentheses around 'dev_priv->perf.oa.half_full_count_last !=
 	     atomic64_read(&dev_priv->perf.oa.half_full_count)'
#122: FILE: drivers/gpu/drm/i915/i915_perf.c:2344:
+	if (stream->oa_interrupt_monitor &&
+	    (dev_priv->perf.oa.half_full_count_last !=
+	     atomic64_read(&dev_priv->perf.oa.half_full_count))) {

-:170: WARNING:TYPO_SPELLING: 'conjuction' may be misspelled - perhaps 'conjunction'?
#170: FILE: include/uapi/drm/i915_drm.h:1553:
+	 * buffer in i915. This option in conjuction with a long polling delay

total: 0 errors, 2 warnings, 2 checks, 145 lines checked
49538d21dc79 drm/i915/perf: add flushing ioctl

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.SPARSE: warning for drm/i915/perf: add OA interrupt support (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915/perf: add OA interrupt support (rev2)
URL   : https://patchwork.freedesktop.org/series/54280/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Sparse version: v0.5.2
Commit: drm/i915/perf: rework aging tail workaround
Okay!

Commit: drm/i915/perf: reset pollin when perf stream is enabled
Okay!

Commit: drm/i915/perf: only append status when data is available
Okay!

Commit: drm/i915/perf: add new open param to configure polling of OA buffer
-drivers/gpu/drm/i915/selftests/../i915_drv.h:3546:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/selftests/../i915_drv.h:3552:16: warning: expression using sizeof(void)

Commit: drm/i915: handle interrupts from the OA unit
-drivers/gpu/drm/i915/selftests/../i915_drv.h:3552:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/selftests/../i915_drv.h:3573:16: warning: expression using sizeof(void)

Commit: drm/i915/perf: add interrupt enabling parameter
+drivers/gpu/drm/i915/i915_perf.c:542:9: warning: context imbalance in 'oa_buffer_check' - different lock contexts for basic block

Commit: drm/i915/perf: add flushing ioctl
Okay!

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for drm/i915/perf: add OA interrupt support (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915/perf: add OA interrupt support (rev2)
URL   : https://patchwork.freedesktop.org/series/54280/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5438 -> Patchwork_11963
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/54280/revisions/2/mbox/

Known issues
------------

  Here are the changes found in Patchwork_11963 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live_hangcheck:
    - fi-bwr-2160:        PASS -> DMESG-FAIL [fdo#108735]

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-kbl-7500u:       PASS -> FAIL [fdo#108767]

  * igt@kms_frontbuffer_tracking@basic:
    - fi-icl-u3:          PASS -> FAIL [fdo#103167]

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence:
    - fi-byt-clapper:     PASS -> FAIL [fdo#103191] / [fdo#107362]

  
#### Possible fixes ####

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
    - fi-blb-e6850:       INCOMPLETE [fdo#107718] -> PASS

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#108735]: https://bugs.freedesktop.org/show_bug.cgi?id=108735
  [fdo#108767]: https://bugs.freedesktop.org/show_bug.cgi?id=108767
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271


Participating hosts (46 -> 42)
------------------------------

  Missing    (4): fi-kbl-soraka fi-ilk-m540 fi-bsw-cyan fi-icl-y 


Build changes
-------------

    * Linux: CI_DRM_5438 -> Patchwork_11963

  CI_DRM_5438: 30c69ebc8e033d471bf006cb0ef49227edb6c4f7 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4776: a76fa4d02cc806e30ed72ba1b8233c694ab1727b @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_11963: 49538d21dc796c9bdf5370681d2b1fb358047f3a @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

49538d21dc79 drm/i915/perf: add flushing ioctl
0c7a8458e075 drm/i915/perf: add interrupt enabling parameter
3bbe259283a3 drm/i915: handle interrupts from the OA unit
4fbe19755ae5 drm/i915/perf: add new open param to configure polling of OA buffer
47f65f25c7c0 drm/i915/perf: only append status when data is available
94b8c039703f drm/i915/perf: reset pollin when perf stream is enabled
f0460c1ade9e drm/i915/perf: rework aging tail workaround

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_11963/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.IGT: failure for drm/i915/perf: add OA interrupt support (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915/perf: add OA interrupt support (rev2)
URL   : https://patchwork.freedesktop.org/series/54280/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_5438_full -> Patchwork_11963_full
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_11963_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_11963_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_11963_full:

### IGT changes ###

#### Possible regressions ####

  * igt@gem_exec_reuse@contexts:
    - shard-skl:          PASS -> DMESG-WARN

  
#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * {igt@runner@aborted}:
    - shard-glk:          ( 2 FAIL ) [fdo#109373] -> FAIL

  
Known issues
------------

  Here are the changes found in Patchwork_11963_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_schedule@pi-ringfull-render:
    - shard-skl:          NOTRUN -> FAIL [fdo#103158]

  * igt@gem_workarounds@suspend-resume:
    - shard-iclb:         PASS -> INCOMPLETE [fdo#107713]

  * igt@kms_atomic_transition@plane-all-transition-fencing:
    - shard-apl:          PASS -> DMESG-WARN [fdo#103558] / [fdo#105602] / [fdo#109225]

  * igt@kms_atomic_transition@plane-all-transition-nonblocking-fencing:
    - shard-iclb:         PASS -> DMESG-WARN [fdo#107724] / [fdo#109225]

  * igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-c:
    - shard-skl:          NOTRUN -> DMESG-WARN [fdo#107956]

  * igt@kms_chv_cursor_fail@pipe-a-128x128-bottom-edge:
    - shard-skl:          PASS -> FAIL [fdo#104671]

  * igt@kms_chv_cursor_fail@pipe-c-256x256-top-edge:
    - shard-skl:          NOTRUN -> FAIL [fdo#104671] +1

  * igt@kms_cursor_crc@cursor-256x256-suspend:
    - shard-skl:          NOTRUN -> INCOMPLETE [fdo#104108]

  * igt@kms_cursor_crc@cursor-64x21-offscreen:
    - shard-skl:          NOTRUN -> FAIL [fdo#103232]

  * igt@kms_draw_crc@draw-method-rgb565-mmap-wc-ytiled:
    - shard-skl:          PASS -> FAIL [fdo#103184]

  * igt@kms_draw_crc@draw-method-xrgb2101010-mmap-wc-untiled:
    - shard-iclb:         PASS -> WARN [fdo#108336] +2

  * igt@kms_draw_crc@draw-method-xrgb8888-pwrite-untiled:
    - shard-skl:          NOTRUN -> FAIL [fdo#108472]

  * igt@kms_fbcon_fbt@psr:
    - shard-skl:          NOTRUN -> FAIL [fdo#107882]

  * igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-indfb-draw-mmap-gtt:
    - shard-skl:          PASS -> FAIL [fdo#105682] +1

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-indfb-draw-mmap-gtt:
    - shard-iclb:         PASS -> DMESG-FAIL [fdo#107724] +8

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-pwrite:
    - shard-iclb:         PASS -> FAIL [fdo#103167] +6

  * igt@kms_frontbuffer_tracking@fbc-1p-rte:
    - shard-apl:          PASS -> DMESG-WARN [fdo#103558] / [fdo#105602] / [fdo#108131]

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-pri-indfb-draw-mmap-wc:
    - shard-glk:          PASS -> FAIL [fdo#103167]

  * igt@kms_frontbuffer_tracking@fbc-modesetfrombusy:
    - shard-skl:          NOTRUN -> FAIL [fdo#105682]

  * igt@kms_frontbuffer_tracking@fbc-rgb101010-draw-render:
    - shard-apl:          PASS -> DMESG-WARN [fdo#103558] / [fdo#105602] +38

  * igt@kms_frontbuffer_tracking@fbcpsr-farfromfence:
    - shard-skl:          PASS -> FAIL [fdo#103167] +2

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-cur-indfb-onoff:
    - shard-iclb:         PASS -> DMESG-WARN [fdo#107724] / [fdo#108336] +6

  * igt@kms_frontbuffer_tracking@psr-1p-rte:
    - shard-skl:          NOTRUN -> FAIL [fdo#103167] +4

  * igt@kms_frontbuffer_tracking@psr-suspend:
    - shard-iclb:         PASS -> DMESG-FAIL [fdo#103167] / [fdo#107724]

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence:
    - shard-skl:          NOTRUN -> FAIL [fdo#103191] / [fdo#107362]

  * igt@kms_pipe_crc_basic@read-crc-pipe-a:
    - shard-skl:          PASS -> FAIL [fdo#107362]

  * igt@kms_plane@pixel-format-pipe-b-planes:
    - shard-skl:          NOTRUN -> DMESG-WARN [fdo#106885] +1

  * igt@kms_plane@plane-panning-top-left-pipe-a-planes:
    - shard-skl:          NOTRUN -> FAIL [fdo#103166] +1

  * igt@kms_plane_alpha_blend@pipe-a-constant-alpha-max:
    - shard-skl:          NOTRUN -> FAIL [fdo#108145] +4

  * igt@kms_plane_alpha_blend@pipe-a-coverage-7efc:
    - shard-skl:          PASS -> FAIL [fdo#107815] / [fdo#108145]

  * igt@kms_plane_alpha_blend@pipe-b-alpha-7efc:
    - shard-skl:          NOTRUN -> FAIL [fdo#107815] / [fdo#108145] +1

  * igt@kms_plane_alpha_blend@pipe-c-alpha-opaque-fb:
    - shard-apl:          PASS -> FAIL [fdo#108145]

  * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
    - shard-skl:          NOTRUN -> FAIL [fdo#107815] +1

  * igt@kms_plane_multiple@atomic-pipe-a-tiling-none:
    - shard-iclb:         PASS -> FAIL [fdo#103166]

  * igt@kms_plane_multiple@atomic-pipe-b-tiling-y:
    - shard-glk:          PASS -> FAIL [fdo#103166] +1
    - shard-apl:          PASS -> FAIL [fdo#103166]

  * igt@kms_plane_multiple@atomic-pipe-c-tiling-yf:
    - shard-skl:          NOTRUN -> FAIL [fdo#103166] / [fdo#107815]

  * igt@kms_setmode@basic:
    - shard-skl:          NOTRUN -> FAIL [fdo#99912]
    - shard-kbl:          PASS -> FAIL [fdo#99912]

  * igt@pm_backlight@fade_with_suspend:
    - shard-skl:          NOTRUN -> FAIL [fdo#107847]

  * igt@pm_rpm@i2c:
    - shard-apl:          PASS -> DMESG-FAIL [fdo#103558] / [fdo#105602]

  * igt@pm_rpm@reg-read-ioctl:
    - shard-iclb:         PASS -> DMESG-WARN [fdo#107724] +15

  
#### Possible fixes ####

  * igt@kms_available_modes_crc@available_mode_test_crc:
    - shard-apl:          FAIL [fdo#106641] -> PASS

  * igt@kms_ccs@pipe-a-crc-sprite-planes-basic:
    - shard-glk:          FAIL [fdo#108145] -> PASS

  * igt@kms_cursor_crc@cursor-128x42-sliding:
    - shard-glk:          FAIL [fdo#103232] -> PASS +2

  * igt@kms_cursor_crc@cursor-64x21-random:
    - shard-apl:          FAIL [fdo#103232] -> PASS +1

  * igt@kms_draw_crc@draw-method-xrgb2101010-blt-ytiled:
    - shard-iclb:         WARN [fdo#108336] -> PASS

  * igt@kms_flip@flip-vs-expired-vblank:
    - shard-skl:          FAIL [fdo#105363] -> PASS

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-cpu:
    - shard-apl:          FAIL [fdo#103167] -> PASS +3

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-move:
    - shard-iclb:         FAIL [fdo#103167] -> PASS +4

  * igt@kms_frontbuffer_tracking@fbc-2p-pri-indfb-multidraw:
    - shard-glk:          FAIL [fdo#103167] -> PASS

  * igt@kms_plane@pixel-format-pipe-a-planes-source-clamping:
    - shard-glk:          FAIL [fdo#108948] -> PASS

  * igt@kms_plane@plane-position-covered-pipe-b-planes:
    - shard-glk:          FAIL [fdo#103166] -> PASS +1

  * igt@kms_plane_lowres@pipe-c-tiling-y:
    - shard-iclb:         DMESG-WARN [fdo#107724] / [fdo#108336] -> PASS +3

  * igt@kms_plane_multiple@atomic-pipe-c-tiling-y:
    - shard-apl:          FAIL [fdo#103166] -> PASS +2
    - shard-iclb:         FAIL [fdo#103166] -> PASS

  * igt@kms_rmfb@rmfb-ioctl:
    - shard-iclb:         DMESG-WARN [fdo#107724] -> PASS +6

  * igt@kms_sysfs_edid_timing:
    - shard-iclb:         FAIL [fdo#100047] -> PASS

  
#### Warnings ####

  * igt@i915_suspend@shrink:
    - shard-iclb:         DMESG-WARN [fdo#107886] / [fdo#109244] -> INCOMPLETE [fdo#106886] / [fdo#107713]

  * igt@kms_ccs@pipe-a-crc-sprite-planes-basic:
    - shard-apl:          FAIL [fdo#106510] / [fdo#108145] -> DMESG-WARN [fdo#103558] / [fdo#105602]

  * igt@kms_ccs@pipe-b-crc-primary-basic:
    - shard-iclb:         FAIL [fdo#107725] -> DMESG-WARN [fdo#107724] / [fdo#108336]

  * igt@kms_cursor_crc@cursor-128x42-sliding:
    - shard-apl:          FAIL [fdo#103232] -> DMESG-FAIL [fdo#103232] / [fdo#103558] / [fdo#105602]

  * igt@kms_cursor_crc@cursor-64x64-suspend:
    - shard-apl:          FAIL [fdo#103191] / [fdo#103232] -> DMESG-FAIL [fdo#103232] / [fdo#103558] / [fdo#105602]

  * igt@kms_plane_alpha_blend@pipe-a-alpha-7efc:
    - shard-apl:          FAIL [fdo#108145] -> DMESG-FAIL [fdo#103558] / [fdo#105602] / [fdo#108145]

  * igt@kms_plane_multiple@atomic-pipe-b-tiling-x:
    - shard-iclb:         FAIL [fdo#103166] -> DMESG-WARN [fdo#107724] / [fdo#108336]

  * igt@kms_rotation_crc@multiplane-rotation-cropping-top:
    - shard-kbl:          DMESG-FAIL [fdo#108950] -> DMESG-WARN [fdo#105604]

  * igt@kms_setmode@basic:
    - shard-apl:          FAIL [fdo#99912] -> DMESG-FAIL [fdo#103558] / [fdo#105602] / [fdo#99912]

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#100047]: https://bugs.freedesktop.org/show_bug.cgi?id=100047
  [fdo#103158]: https://bugs.freedesktop.org/show_bug.cgi?id=103158
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103184]: https://bugs.freedesktop.org/show_bug.cgi?id=103184
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103558]: https://bugs.freedesktop.org/show_bug.cgi?id=103558
  [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
  [fdo#104671]: https://bugs.freedesktop.org/show_bug.cgi?id=104671
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#105602]: https://bugs.freedesktop.org/show_bug.cgi?id=105602
  [fdo#105604]: https://bugs.freedesktop.org/show_bug.cgi?id=105604
  [fdo#105682]: https://bugs.freedesktop.org/show_bug.cgi?id=105682
  [fdo#106510]: https://bugs.freedesktop.org/show_bug.cgi?id=106510
  [fdo#106641]: https://bugs.freedesktop.org/show_bug.cgi?id=106641
  [fdo#106885]: https://bugs.freedesktop.org/show_bug.cgi?id=106885
  [fdo#106886]: https://bugs.freedesktop.org/show_bug.cgi?id=106886
  [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#107725]: https://bugs.freedesktop.org/show_bug.cgi?id=107725
  [fdo#107815]: https://bugs.freedesktop.org/show_bug.cgi?id=107815
  [fdo#107847]: https://bugs.freedesktop.org/show_bug.cgi?id=107847
  [fdo#107882]: https://bugs.freedesktop.org/show_bug.cgi?id=107882
  [fdo#107886]: https://bugs.freedesktop.org/show_bug.cgi?id=107886
  [fdo#107956]: https://bugs.freedesktop.org/show_bug.cgi?id=107956
  [fdo#108131]: https://bugs.freedesktop.org/show_bug.cgi?id=108131
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108336]: https://bugs.freedesktop.org/show_bug.cgi?id=108336
  [fdo#108472]: https://bugs.freedesktop.org/show_bug.cgi?id=108472
  [fdo#108948]: https://bugs.freedesktop.org/show_bug.cgi?id=108948
  [fdo#108950]: https://bugs.freedesktop.org/show_bug.cgi?id=108950
  [fdo#109225]: https://bugs.freedesktop.org/show_bug.cgi?id=109225
  [fdo#109244]: https://bugs.freedesktop.org/show_bug.cgi?id=109244
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109350]: https://bugs.freedesktop.org/show_bug.cgi?id=109350
  [fdo#109373]: https://bugs.freedesktop.org/show_bug.cgi?id=109373
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


Participating hosts (7 -> 7)
------------------------------

  No changes in participating hosts


Build changes
-------------

    * Linux: CI_DRM_5438 -> Patchwork_11963

  CI_DRM_5438: 30c69ebc8e033d471bf006cb0ef49227edb6c4f7 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4776: a76fa4d02cc806e30ed72ba1b8233c694ab1727b @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_11963: 49538d21dc796c9bdf5370681d2b1fb358047f3a @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_11963/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 5/7] drm/i915: handle interrupts from the OA unit

[Lionel Landwerlin]

[-- Attachment #1.1: Type: text/plain, Size: 4135 bytes --]

On 16/01/2019 18:04, Lionel Landwerlin wrote:
> On 16/01/2019 16:31, Chris Wilson wrote:
>> Quoting Lionel Landwerlin (2019-01-16 16:25:26)
>>> On 16/01/2019 16:05, Chris Wilson wrote:
>>>> Quoting Lionel Landwerlin (2019-01-16 15:58:00)
>>>>> On 16/01/2019 15:52, Chris Wilson wrote:
>>>>>> Quoting Lionel Landwerlin (2019-01-16 15:36:20)
>>>>>>> @@ -1877,6 +1883,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;
>>>>>> Eh? But why a relatively expensive atomic64. You only need one 
>>>>>> bit, and
>>>>>> reading the tail pointer from WB memory should just be cheap. You 
>>>>>> should
>>>>>> be able to sample the perf ringbuffer pointers very cheaply... 
>>>>>> What am I
>>>>>> missing?
>>>>>> -Chris
>>>>>>
>>>>> Fair comment.
>>>>>
>>>>> The thing is with this series there are 2 mechanism that notify 
>>>>> the poll_wq.
>>>>>
>>>>> One is the hrtimer that kicks in at regular interval and polls the
>>>>> register with the workaround.
>>>>>
>>>>> The other is the interrupt which doesn't read the registers and 
>>>>> workaround.
>>>> What's the complication with the workaround?
>>>
>>> It's a bit more than just looking at registers, we actually have to 
>>> look
>>> at the content of the buffer to figure out what landed in memory.
>>>
>>> The register values are not synchronized with the memory writes...
>> I don't want to look at registers at all for polling, and you shouldn't
>> need to since communication is via a ringbuf.
>>> There is a comment in the code (i915_perf_poll_locked) about not
>>> checking the register after each wakeup because that may be a very 
>>> hot path.
>>>
>>> The atomic64 sounded like a lesser evil.
>> I'm clearly not understanding something here...
>>
>> Does the hardware not do:
>>     update ringbuf data;
>>     wmb() (or post to global observation point in their parlance)
>>     update ringbuf tail
>
>
> As far as I understand, the OA unit :
>
>     sends its memory write requests to the memory controller
>
>     immediately updates the ringbuf tail, without waiting for the 
> previous requests to complete
>

By experimentation, I've haven't seen a delta between what is available 
in memory and what the OA tail register indicate larger than 768 bytes, 
which is about 3 OA reports at the largest size.
There is probably a maximum number of write requests the OA unit can 
queue before blocking.

So again maybe you would prefer a 2 stage mechanism :

OA interrupt -----> head/tail pointer worker  -----> wake up userspace

                     hrtimer head/tail pointer --|


>
>
>>
>> Then we just need to sample the ringbuf tail and compare against how far
>> we read last time?
>> -Chris
>>
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx



[-- Attachment #1.2: Type: text/html, Size: 7108 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 0/7] drm/i915/perf: add OA interrupt support

[Lionel Landwerlin]

Any taker?

-Lionel

On 16/01/2019 15:36, Lionel Landwerlin wrote:
> Taking the RFC off this series.
>
> To quite the vTune team that tried the previous version :
>
>      "It reduces data collection overhead in VTune by 11x. It is great!"
>
> The GPA team's report on the previous version was a drop in CPU
> consumption from 17~20% down to 2~3%.
>
> This version includes :
>
>     - a fix for an issue reported by Chris on the IMR register access
>       on Haswell
>
>     - the ability to completely disable the i915 OA head/tail polling
>
>     - a new ioctl on the perf stream file descript (not the i915 drm
>       master/render node) to force i915 to look at the OA head/tail
>       register (see explanation in last patch).
>
> Cheers,
>
> Lionel Landwerlin (7):
>    drm/i915/perf: rework aging tail workaround
>    drm/i915/perf: reset pollin when perf stream is enabled
>    drm/i915/perf: only append status when data is available
>    drm/i915/perf: add new open param to configure polling of OA buffer
>    drm/i915: handle interrupts from the OA unit
>    drm/i915/perf: add interrupt enabling parameter
>    drm/i915/perf: add flushing ioctl
>
>   drivers/gpu/drm/i915/i915_drv.h         |  59 +++-
>   drivers/gpu/drm/i915/i915_irq.c         |  39 ++-
>   drivers/gpu/drm/i915/i915_perf.c        | 388 +++++++++++++++---------
>   drivers/gpu/drm/i915/i915_reg.h         |   7 +
>   drivers/gpu/drm/i915/intel_ringbuffer.c |   2 +
>   include/uapi/drm/i915_drm.h             |  35 +++
>   6 files changed, 357 insertions(+), 173 deletions(-)
>
> --
> 2.20.1
>

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 7/7] drm/i915/perf: add flushing ioctl

[Joonas Lahtinen]

Quoting Lionel Landwerlin (2019-01-16 17:36:22)
> With the currently available parameters for the i915-perf stream,
> there are still situations that are not well covered :
> 
> If an application opens the stream with polling disable or at very low
> frequency and OA interrupt enabled, no data will be available even
> though somewhere between nothing and half of the OA buffer worth of
> data might have landed in memory.
> 
> To solve this issue we have a new flush ioctl on the perf stream that
> forces the i915-perf driver to look at the state of the buffer when
> called and makes any data available through both poll() & read() type
> syscalls.
> 
> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>

Link to userspace changes?

Regards, Joonas

> ---
>  drivers/gpu/drm/i915/i915_perf.c | 17 +++++++++++++++++
>  include/uapi/drm/i915_drm.h      | 19 +++++++++++++++++++
>  2 files changed, 36 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
> index da721fce2543..6c98ffa2135e 100644
> --- a/drivers/gpu/drm/i915/i915_perf.c
> +++ b/drivers/gpu/drm/i915/i915_perf.c
> @@ -2433,6 +2433,20 @@ static void i915_perf_disable_locked(struct i915_perf_stream *stream)
>                 stream->ops->disable(stream);
>  }
>  
> +/**
> + * i915_perf_flush_data - handle `I915_PERF_IOCTL_FLUSH_DATA` ioctl
> + * @stream: An enabled i915 perf stream
> + *
> + * The intention is to flush all the data available for reading from the OA
> + * buffer
> + */
> +static void i915_perf_flush_data(struct i915_perf_stream *stream)
> +{
> +       struct drm_i915_private *dev_priv = stream->dev_priv;
> +
> +       dev_priv->perf.oa.pollin = oa_buffer_check(stream->dev_priv, true);
> +}
> +
>  /**
>   * i915_perf_ioctl - support ioctl() usage with i915 perf stream FDs
>   * @stream: An i915 perf stream
> @@ -2456,6 +2470,9 @@ static long i915_perf_ioctl_locked(struct i915_perf_stream *stream,
>         case I915_PERF_IOCTL_DISABLE:
>                 i915_perf_disable_locked(stream);
>                 return 0;
> +       case I915_PERF_IOCTL_FLUSH_DATA:
> +               i915_perf_flush_data(stream);
> +               return 0;
>         }
>  
>         return -EINVAL;
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index b6db5e544a35..0f0d20080572 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -1594,6 +1594,25 @@ struct drm_i915_perf_open_param {
>   */
>  #define I915_PERF_IOCTL_DISABLE        _IO('i', 0x1)
>  
> +/**
> + * Actively check the availability of data from a stream.
> + *
> + * A stream data availability can be driven by two types of events :
> + *
> + *   - if enabled, the kernel's hrtimer checking the amount of available data
> + *     in the OA buffer through head/tail registers.
> + *
> + *   - if enabled, the OA unit's interrupt mechanism
> + *
> + * The kernel hrtimer incur a cost of running callback at fixed time
> + * intervals, while the OA interrupt might only happen rarely. In the
> + * situation where the application has disabled the kernel's hrtimer and only
> + * uses the OA interrupt to know about available data, the application can
> + * request an active check of the available OA data through this ioctl. This
> + * will make any data in the OA buffer available with either poll() or read().
> + */
> +#define I915_PERF_IOCTL_FLUSH_DATA _IO('i', 0x2)
> +
>  /**
>   * Common to all i915 perf records
>   */
> -- 
> 2.20.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 7/7] drm/i915/perf: add flushing ioctl

[Lionel Landwerlin]

[-- Attachment #1.1: Type: text/plain, Size: 968 bytes --]

On 22/01/2019 16:25, Joonas Lahtinen wrote:
> Quoting Lionel Landwerlin (2019-01-16 17:36:22)
>> With the currently available parameters for the i915-perf stream,
>> there are still situations that are not well covered :
>>
>> If an application opens the stream with polling disable or at very low
>> frequency and OA interrupt enabled, no data will be available even
>> though somewhere between nothing and half of the OA buffer worth of
>> data might have landed in memory.
>>
>> To solve this issue we have a new flush ioctl on the perf stream that
>> forces the i915-perf driver to look at the state of the buffer when
>> called and makes any data available through both poll() & read() type
>> syscalls.
>>
>> Signed-off-by: Lionel Landwerlin<lionel.g.landwerlin@intel.com>
> Link to userspace changes?
>
> Regards, Joonas
>
Trying to get them uploaded to a branch on 
https://github.com/intel/metrics-discovery

I'll let you know when it's available.


-Lionel


[-- Attachment #1.2: Type: text/html, Size: 1741 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 7/7] drm/i915/perf: add flushing ioctl

[Lionel Landwerlin]

On 22/01/2019 16:25, Joonas Lahtinen wrote:
> Quoting Lionel Landwerlin (2019-01-16 17:36:22)
>> With the currently available parameters for the i915-perf stream,
>> there are still situations that are not well covered :
>>
>> If an application opens the stream with polling disable or at very low
>> frequency and OA interrupt enabled, no data will be available even
>> though somewhere between nothing and half of the OA buffer worth of
>> data might have landed in memory.
>>
>> To solve this issue we have a new flush ioctl on the perf stream that
>> forces the i915-perf driver to look at the state of the buffer when
>> called and makes any data available through both poll() & read() type
>> syscalls.
>>
>> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> Link to userspace changes?
>
> Regards, Joonas


Hey Joonas,


I'm about to make the changes in gputop for the high frequency sampling 
use case.


One thing I would like to know is whether these new features should be 
reported through a flag.

So far we haven't added any new option to the i915/perf driver since the 
initial upstreaming series.


The way I'm currently detecting newly added parameters is by using 
trying to open the stream with a value that I know will report ENOENT 
rather than EINVAL when the feature is not available :

https://github.com/djdeath/intel-gpu-tools/blob/wip/djdeath/oa-interrupts/tests/perf.c#L4345

Is there a better way to do this?


Thanks,


-Lionel


>
>> ---
>>   drivers/gpu/drm/i915/i915_perf.c | 17 +++++++++++++++++
>>   include/uapi/drm/i915_drm.h      | 19 +++++++++++++++++++
>>   2 files changed, 36 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
>> index da721fce2543..6c98ffa2135e 100644
>> --- a/drivers/gpu/drm/i915/i915_perf.c
>> +++ b/drivers/gpu/drm/i915/i915_perf.c
>> @@ -2433,6 +2433,20 @@ static void i915_perf_disable_locked(struct i915_perf_stream *stream)
>>                  stream->ops->disable(stream);
>>   }
>>   
>> +/**
>> + * i915_perf_flush_data - handle `I915_PERF_IOCTL_FLUSH_DATA` ioctl
>> + * @stream: An enabled i915 perf stream
>> + *
>> + * The intention is to flush all the data available for reading from the OA
>> + * buffer
>> + */
>> +static void i915_perf_flush_data(struct i915_perf_stream *stream)
>> +{
>> +       struct drm_i915_private *dev_priv = stream->dev_priv;
>> +
>> +       dev_priv->perf.oa.pollin = oa_buffer_check(stream->dev_priv, true);
>> +}
>> +
>>   /**
>>    * i915_perf_ioctl - support ioctl() usage with i915 perf stream FDs
>>    * @stream: An i915 perf stream
>> @@ -2456,6 +2470,9 @@ static long i915_perf_ioctl_locked(struct i915_perf_stream *stream,
>>          case I915_PERF_IOCTL_DISABLE:
>>                  i915_perf_disable_locked(stream);
>>                  return 0;
>> +       case I915_PERF_IOCTL_FLUSH_DATA:
>> +               i915_perf_flush_data(stream);
>> +               return 0;
>>          }
>>   
>>          return -EINVAL;
>> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
>> index b6db5e544a35..0f0d20080572 100644
>> --- a/include/uapi/drm/i915_drm.h
>> +++ b/include/uapi/drm/i915_drm.h
>> @@ -1594,6 +1594,25 @@ struct drm_i915_perf_open_param {
>>    */
>>   #define I915_PERF_IOCTL_DISABLE        _IO('i', 0x1)
>>   
>> +/**
>> + * Actively check the availability of data from a stream.
>> + *
>> + * A stream data availability can be driven by two types of events :
>> + *
>> + *   - if enabled, the kernel's hrtimer checking the amount of available data
>> + *     in the OA buffer through head/tail registers.
>> + *
>> + *   - if enabled, the OA unit's interrupt mechanism
>> + *
>> + * The kernel hrtimer incur a cost of running callback at fixed time
>> + * intervals, while the OA interrupt might only happen rarely. In the
>> + * situation where the application has disabled the kernel's hrtimer and only
>> + * uses the OA interrupt to know about available data, the application can
>> + * request an active check of the available OA data through this ioctl. This
>> + * will make any data in the OA buffer available with either poll() or read().
>> + */
>> +#define I915_PERF_IOCTL_FLUSH_DATA _IO('i', 0x2)
>> +
>>   /**
>>    * Common to all i915 perf records
>>    */
>> -- 
>> 2.20.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 7/7] drm/i915/perf: add flushing ioctl

[Joonas Lahtinen]

Quoting Lionel Landwerlin (2019-02-04 17:30:12)
> On 22/01/2019 16:25, Joonas Lahtinen wrote:
> > Quoting Lionel Landwerlin (2019-01-16 17:36:22)
> >> With the currently available parameters for the i915-perf stream,
> >> there are still situations that are not well covered :
> >>
> >> If an application opens the stream with polling disable or at very low
> >> frequency and OA interrupt enabled, no data will be available even
> >> though somewhere between nothing and half of the OA buffer worth of
> >> data might have landed in memory.
> >>
> >> To solve this issue we have a new flush ioctl on the perf stream that
> >> forces the i915-perf driver to look at the state of the buffer when
> >> called and makes any data available through both poll() & read() type
> >> syscalls.
> >>
> >> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> > Link to userspace changes?
> >
> > Regards, Joonas
> 
> 
> Hey Joonas,
> 
> 
> I'm about to make the changes in gputop for the high frequency sampling 
> use case.
> 
> 
> One thing I would like to know is whether these new features should be 
> reported through a flag.
> 
> So far we haven't added any new option to the i915/perf driver since the 
> initial upstreaming series.
> 
> 
> The way I'm currently detecting newly added parameters is by using 
> trying to open the stream with a value that I know will report ENOENT 
> rather than EINVAL when the feature is not available :
> 
> https://github.com/djdeath/intel-gpu-tools/blob/wip/djdeath/oa-interrupts/tests/perf.c#L4345
> 
> Is there a better way to do this?

I think I already responded in IRC, but here goes for e-mail archives.

A GETPARAM with rolling i915/perf version would probably be a justified
versioning interface.

Regards, Joonas

> 
> 
> Thanks,
> 
> 
> -Lionel
> 
> 
> >
> >> ---
> >>   drivers/gpu/drm/i915/i915_perf.c | 17 +++++++++++++++++
> >>   include/uapi/drm/i915_drm.h      | 19 +++++++++++++++++++
> >>   2 files changed, 36 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
> >> index da721fce2543..6c98ffa2135e 100644
> >> --- a/drivers/gpu/drm/i915/i915_perf.c
> >> +++ b/drivers/gpu/drm/i915/i915_perf.c
> >> @@ -2433,6 +2433,20 @@ static void i915_perf_disable_locked(struct i915_perf_stream *stream)
> >>                  stream->ops->disable(stream);
> >>   }
> >>   
> >> +/**
> >> + * i915_perf_flush_data - handle `I915_PERF_IOCTL_FLUSH_DATA` ioctl
> >> + * @stream: An enabled i915 perf stream
> >> + *
> >> + * The intention is to flush all the data available for reading from the OA
> >> + * buffer
> >> + */
> >> +static void i915_perf_flush_data(struct i915_perf_stream *stream)
> >> +{
> >> +       struct drm_i915_private *dev_priv = stream->dev_priv;
> >> +
> >> +       dev_priv->perf.oa.pollin = oa_buffer_check(stream->dev_priv, true);
> >> +}
> >> +
> >>   /**
> >>    * i915_perf_ioctl - support ioctl() usage with i915 perf stream FDs
> >>    * @stream: An i915 perf stream
> >> @@ -2456,6 +2470,9 @@ static long i915_perf_ioctl_locked(struct i915_perf_stream *stream,
> >>          case I915_PERF_IOCTL_DISABLE:
> >>                  i915_perf_disable_locked(stream);
> >>                  return 0;
> >> +       case I915_PERF_IOCTL_FLUSH_DATA:
> >> +               i915_perf_flush_data(stream);
> >> +               return 0;
> >>          }
> >>   
> >>          return -EINVAL;
> >> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> >> index b6db5e544a35..0f0d20080572 100644
> >> --- a/include/uapi/drm/i915_drm.h
> >> +++ b/include/uapi/drm/i915_drm.h
> >> @@ -1594,6 +1594,25 @@ struct drm_i915_perf_open_param {
> >>    */
> >>   #define I915_PERF_IOCTL_DISABLE        _IO('i', 0x1)
> >>   
> >> +/**
> >> + * Actively check the availability of data from a stream.
> >> + *
> >> + * A stream data availability can be driven by two types of events :
> >> + *
> >> + *   - if enabled, the kernel's hrtimer checking the amount of available data
> >> + *     in the OA buffer through head/tail registers.
> >> + *
> >> + *   - if enabled, the OA unit's interrupt mechanism
> >> + *
> >> + * The kernel hrtimer incur a cost of running callback at fixed time
> >> + * intervals, while the OA interrupt might only happen rarely. In the
> >> + * situation where the application has disabled the kernel's hrtimer and only
> >> + * uses the OA interrupt to know about available data, the application can
> >> + * request an active check of the available OA data through this ioctl. This
> >> + * will make any data in the OA buffer available with either poll() or read().
> >> + */
> >> +#define I915_PERF_IOCTL_FLUSH_DATA _IO('i', 0x2)
> >> +
> >>   /**
> >>    * Common to all i915 perf records
> >>    */
> >> -- 
> >> 2.20.1
> >>
> >> _______________________________________________
> >> Intel-gfx mailing list
> >> Intel-gfx@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx