From 3bb335c1e73a709c3c1ff0015faa87e60e03d17d Mon Sep 17 00:00:00 2001
From: Robert Bragg <robert@sixbynine.org>
Date: Thu, 11 May 2017 16:43:27 +0100
Subject: [PATCH] drm/i915/perf: no head/tail ref in gen7_oa_read
Git-commit: 3bb335c1e73a709c3c1ff0015faa87e60e03d17d
Patch-mainline: v4.13-rc1
References: FATE#322643 bsc#1055900

This avoids redundantly passing an (inout) head and tail pointer to
gen7_append_oa_reports() from gen7_oa_read which doesn't need to
reference either itself.

Moving the head/tail reads and writes into gen7_append_oa_reports should
have no functional effect except to avoid some redundant head pointer
writes in cases where nothing was copied to userspace.

This is a stepping stone towards updating how the head and tail pointer
state is managed to improve the workaround for the OA unit's tail
pointer race. It reduces the number of places we need to read/write the
head and tail pointers.

Signed-off-by: Robert Bragg <robert@sixbynine.org>
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/20170511154345.962-5-lionel.g.landwerlin@intel.com
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Acked-by: Takashi Iwai <tiwai@suse.de>

---
 drivers/gpu/drm/i915/i915_perf.c |   50 ++++++++++++++-------------------------
 1 file changed, 18 insertions(+), 32 deletions(-)

--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -420,8 +420,6 @@ static int append_oa_sample(struct i915_
  * @buf: destination buffer given by userspace
  * @count: the number of bytes userspace wants to read
  * @offset: (inout): the current position for writing into @buf
- * @head_ptr: (inout): the current oa buffer cpu read position
- * @tail: the current oa buffer gpu write position
  *
  * Notably any error condition resulting in a short read (-%ENOSPC or
  * -%EFAULT) will be returned even though one or more records may
@@ -439,9 +437,7 @@ static int append_oa_sample(struct i915_
 static int gen7_append_oa_reports(struct i915_perf_stream *stream,
 				  char __user *buf,
 				  size_t count,
-				  size_t *offset,
-				  u32 *head_ptr,
-				  u32 tail)
+				  size_t *offset)
 {
 	struct drm_i915_private *dev_priv = stream->dev_priv;
 	int report_size = dev_priv->perf.oa.oa_buffer.format_size;
@@ -449,14 +445,15 @@ static int gen7_append_oa_reports(struct
 	int tail_margin = dev_priv->perf.oa.tail_margin;
 	u32 gtt_offset = i915_ggtt_offset(dev_priv->perf.oa.oa_buffer.vma);
 	u32 mask = (OA_BUFFER_SIZE - 1);
-	u32 head;
+	size_t start_offset = *offset;
+	u32 head, oastatus1, tail;
 	u32 taken;
 	int ret = 0;
 
 	if (WARN_ON(!stream->enabled))
 		return -EIO;
 
-	head = *head_ptr - gtt_offset;
+	head = dev_priv->perf.oa.oa_buffer.head - gtt_offset;
 
 	/* An out of bounds or misaligned head pointer implies a driver bug
 	 * since we are in full control of head pointer which should only
@@ -467,7 +464,8 @@ static int gen7_append_oa_reports(struct
 		      "Inconsistent OA buffer head pointer = %u\n", head))
 		return -EIO;
 
-	tail -= gtt_offset;
+	oastatus1 = I915_READ(GEN7_OASTATUS1);
+	tail = (oastatus1 & GEN7_OASTATUS1_TAIL_MASK) - gtt_offset;
 
 	/* The OA unit is expected to wrap the tail pointer according to the OA
 	 * buffer size
@@ -477,8 +475,6 @@ static int gen7_append_oa_reports(struct
 			  tail);
 		dev_priv->perf.oa.ops.oa_disable(dev_priv);
 		dev_priv->perf.oa.ops.oa_enable(dev_priv);
-		*head_ptr = I915_READ(GEN7_OASTATUS2) &
-			GEN7_OASTATUS2_HEAD_MASK;
 		return -EIO;
 	}
 
@@ -542,7 +538,17 @@ static int gen7_append_oa_reports(struct
 		report32[0] = 0;
 	}
 
-	*head_ptr = gtt_offset + head;
+	if (start_offset != *offset) {
+		/* We removed the gtt_offset for the copy loop above, indexing
+		 * relative to oa_buf_base so put back here...
+		 */
+		head += gtt_offset;
+
+		I915_WRITE(GEN7_OASTATUS2,
+			   ((head & GEN7_OASTATUS2_HEAD_MASK) |
+			    OA_MEM_SELECT_GGTT));
+		dev_priv->perf.oa.oa_buffer.head = head;
+	}
 
 	return ret;
 }
@@ -570,8 +576,6 @@ static int gen7_oa_read(struct i915_perf
 {
 	struct drm_i915_private *dev_priv = stream->dev_priv;
 	u32 oastatus1;
-	u32 head;
-	u32 tail;
 	int ret;
 
 	if (WARN_ON(!dev_priv->perf.oa.oa_buffer.vaddr))
@@ -579,9 +583,6 @@ static int gen7_oa_read(struct i915_perf
 
 	oastatus1 = I915_READ(GEN7_OASTATUS1);
 
-	head = dev_priv->perf.oa.oa_buffer.head;
-	tail = oastatus1 & GEN7_OASTATUS1_TAIL_MASK;
-
 	/* XXX: On Haswell we don't have a safe way to clear oastatus1
 	 * bits while the OA unit is enabled (while the tail pointer
 	 * may be updated asynchronously) so we ignore status bits
@@ -621,9 +622,6 @@ static int gen7_oa_read(struct i915_perf
 		dev_priv->perf.oa.ops.oa_enable(dev_priv);
 
 		oastatus1 = I915_READ(GEN7_OASTATUS1);
-
-		head = dev_priv->perf.oa.oa_buffer.head;
-		tail = oastatus1 & GEN7_OASTATUS1_TAIL_MASK;
 	}
 
 	if (unlikely(oastatus1 & GEN7_OASTATUS1_REPORT_LOST)) {
@@ -635,19 +633,7 @@ static int gen7_oa_read(struct i915_perf
 			GEN7_OASTATUS1_REPORT_LOST;
 	}
 
-	ret = gen7_append_oa_reports(stream, buf, count, offset,
-				     &head, tail);
-
-	/* Note: we update the head pointer here even if an error
-	 * was returned since the error may represent a short read
-	 * where some some reports were successfully copied.
-	 */
-	I915_WRITE(GEN7_OASTATUS2,
-		   ((head & GEN7_OASTATUS2_HEAD_MASK) |
-		    OA_MEM_SELECT_GGTT));
-	dev_priv->perf.oa.oa_buffer.head = head;
-
-	return ret;
+	return gen7_append_oa_reports(stream, buf, count, offset);
 }
 
 /**