intel-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [Intel-gfx] [PATCH 0/3] Avoid reading OA reports before they land
@ 2023-05-31 23:56 Umesh Nerlige Ramappa
  2023-05-31 23:56 ` [Intel-gfx] [PATCH 1/3] i915/perf: Drop the aging_tail logic in perf OA Umesh Nerlige Ramappa
                   ` (5 more replies)
  0 siblings, 6 replies; 11+ messages in thread
From: Umesh Nerlige Ramappa @ 2023-05-31 23:56 UTC (permalink / raw)
  To: intel-gfx; +Cc: Lionel G Landwerlin

Fix OA issue seen on DG2 where parts of OA reports are zeroed out or
have stale values. This was due to the fact that rewind logic was not
being run when the tail pointer was aged. The series drops the complex
aging/aged logic and just checks the reports for validity.

rev1 - https://patchwork.freedesktop.org/series/118054/

Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>

Umesh Nerlige Ramappa (3):
  i915/perf: Drop the aging_tail logic in perf OA
  i915/perf: Do not add ggtt offset to hw_tail
  i915/perf: Drop the aged_tail from rewind logic

 drivers/gpu/drm/i915/i915_perf.c       | 76 ++++++++++----------------
 drivers/gpu/drm/i915/i915_perf_types.h | 12 ----
 2 files changed, 28 insertions(+), 60 deletions(-)

-- 
2.36.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Intel-gfx] [PATCH 1/3] i915/perf: Drop the aging_tail logic in perf OA
  2023-05-31 23:56 [Intel-gfx] [PATCH 0/3] Avoid reading OA reports before they land Umesh Nerlige Ramappa
@ 2023-05-31 23:56 ` Umesh Nerlige Ramappa
  2023-06-01  4:06   ` Dixit, Ashutosh
  2023-05-31 23:56 ` [Intel-gfx] [PATCH 2/3] i915/perf: Do not add ggtt offset to hw_tail Umesh Nerlige Ramappa
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 11+ messages in thread
From: Umesh Nerlige Ramappa @ 2023-05-31 23:56 UTC (permalink / raw)
  To: intel-gfx; +Cc: Lionel G Landwerlin

On DG2, capturing OA reports while running heavy render workloads
sometimes results in invalid OA reports where 64-byte chunks inside
reports have stale values. Under memory pressure, high OA sampling rates
(13.3 us) and heavy render workload, occasionally, the OA HW TAIL
pointer does not progress as fast as the sampling rate. When these
glitches occur, the TAIL pointer takes approx. 200us to progress.  While
this is expected behavior from the HW perspective, invalid reports are
not expected.

In oa_buffer_check_unlocked(), when we execute the if condition, we are
updating the oa_buffer.tail to the aging tail and then setting pollin
based on this tail value, however, we do not have a chance to rewind and
validate the reports prior to setting pollin. The validation happens
in a subsequent call to oa_buffer_check_unlocked(). If a read occurs
before this validation, then we end up reading reports up until this
oa_buffer.tail value which includes invalid reports. Though found on
DG2, this affects all platforms.

Start by dropping the aging tail logic.

Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
---
 drivers/gpu/drm/i915/i915_perf.c       | 74 ++++++++++----------------
 drivers/gpu/drm/i915/i915_perf_types.h | 12 -----
 2 files changed, 28 insertions(+), 58 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index 58284156428d..29124dcba8e2 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -531,8 +531,7 @@ static void oa_context_id_squash(struct i915_perf_stream *stream, u32 *report)
  * (See description of OA_TAIL_MARGIN_NSEC above for further details.)
  *
  * Besides returning true when there is data available to read() this function
- * also updates the tail, aging_tail and aging_timestamp in the oa_buffer
- * object.
+ * also updates the tail in the oa_buffer object.
  *
  * Note: It's safe to read OA config state here unlocked, assuming that this is
  * only called while the stream is enabled, while the global OA configuration
@@ -544,10 +543,10 @@ static bool oa_buffer_check_unlocked(struct i915_perf_stream *stream)
 {
 	u32 gtt_offset = i915_ggtt_offset(stream->oa_buffer.vma);
 	int report_size = stream->oa_buffer.format->size;
+	u32 head, tail, aged_tail;
 	unsigned long flags;
 	bool pollin;
 	u32 hw_tail;
-	u64 now;
 	u32 partial_report_size;
 
 	/* We have to consider the (unlikely) possibility that read() errors
@@ -568,27 +567,16 @@ static bool oa_buffer_check_unlocked(struct i915_perf_stream *stream)
 	/* Subtract partial amount off the tail */
 	hw_tail = gtt_offset + OA_TAKEN(hw_tail, partial_report_size);
 
-	now = ktime_get_mono_fast_ns();
 
-	if (hw_tail == stream->oa_buffer.aging_tail &&
-	    (now - stream->oa_buffer.aging_timestamp) > OA_TAIL_MARGIN_NSEC) {
-		/* 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.
-		 */
-		stream->oa_buffer.tail = stream->oa_buffer.aging_tail;
-	} else {
-		u32 head, tail, aged_tail;
-
-		/* NB: The head we observe here might effectively be a little
-		 * out of date. If a read() is in progress, the head could be
-		 * anywhere between this head and stream->oa_buffer.tail.
-		 */
-		head = stream->oa_buffer.head - gtt_offset;
-		aged_tail = stream->oa_buffer.tail - gtt_offset;
+	/* NB: The head we observe here might effectively be a little
+	 * out of date. If a read() is in progress, the head could be
+	 * anywhere between this head and stream->oa_buffer.tail.
+	 */
+	head = stream->oa_buffer.head - gtt_offset;
+	aged_tail = stream->oa_buffer.tail - gtt_offset;
 
-		hw_tail -= gtt_offset;
-		tail = hw_tail;
+	hw_tail -= gtt_offset;
+	tail = hw_tail;
 
 		/* Walk the stream backward until we find a report with report
 		 * id and timestmap not at 0. Since the circular buffer pointers
@@ -596,31 +584,28 @@ static bool oa_buffer_check_unlocked(struct i915_perf_stream *stream)
 		 * to 256 bytes long, we can't tell whether a report has fully
 		 * landed in memory before the report id and timestamp 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 : (╯°□°)╯︵ ┻━┻
-		 */
-		while (OA_TAKEN(tail, aged_tail) >= report_size) {
-			void *report = stream->oa_buffer.vaddr + tail;
+	 *
+	 * This is assuming that the writes of the OA unit land in
+	 * memory in the order they were written to.
+	 * If not : (╯°□°)╯︵ ┻━┻
+	 */
+	while (OA_TAKEN(tail, aged_tail) >= report_size) {
+		void *report = stream->oa_buffer.vaddr + tail;
 
-			if (oa_report_id(stream, report) ||
-			    oa_timestamp(stream, report))
-				break;
+		if (oa_report_id(stream, report) ||
+		    oa_timestamp(stream, report))
+			break;
 
-			tail = (tail - report_size) & (OA_BUFFER_SIZE - 1);
-		}
+		tail = (tail - report_size) & (OA_BUFFER_SIZE - 1);
+	}
 
-		if (OA_TAKEN(hw_tail, tail) > report_size &&
-		    __ratelimit(&stream->perf->tail_pointer_race))
-			drm_notice(&stream->uncore->i915->drm,
-				   "unlanded report(s) head=0x%x tail=0x%x hw_tail=0x%x\n",
-				   head, tail, hw_tail);
+	if (OA_TAKEN(hw_tail, tail) > report_size &&
+	    __ratelimit(&stream->perf->tail_pointer_race))
+		drm_notice(&stream->uncore->i915->drm,
+			   "unlanded report(s) head=0x%x tail=0x%x hw_tail=0x%x\n",
+		 head, tail, hw_tail);
 
-		stream->oa_buffer.tail = gtt_offset + tail;
-		stream->oa_buffer.aging_tail = gtt_offset + hw_tail;
-		stream->oa_buffer.aging_timestamp = now;
-	}
+	stream->oa_buffer.tail = gtt_offset + tail;
 
 	pollin = OA_TAKEN(stream->oa_buffer.tail - gtt_offset,
 			  stream->oa_buffer.head - gtt_offset) >= report_size;
@@ -1727,7 +1712,6 @@ static void gen7_init_oa_buffer(struct i915_perf_stream *stream)
 			   gtt_offset | OABUFFER_SIZE_16M);
 
 	/* Mark that we need updated tail pointers to read from... */
-	stream->oa_buffer.aging_tail = INVALID_TAIL_PTR;
 	stream->oa_buffer.tail = gtt_offset;
 
 	spin_unlock_irqrestore(&stream->oa_buffer.ptr_lock, flags);
@@ -1779,7 +1763,6 @@ static void gen8_init_oa_buffer(struct i915_perf_stream *stream)
 	intel_uncore_write(uncore, GEN8_OATAILPTR, gtt_offset & GEN8_OATAILPTR_MASK);
 
 	/* Mark that we need updated tail pointers to read from... */
-	stream->oa_buffer.aging_tail = INVALID_TAIL_PTR;
 	stream->oa_buffer.tail = gtt_offset;
 
 	/*
@@ -1833,7 +1816,6 @@ static void gen12_init_oa_buffer(struct i915_perf_stream *stream)
 			   gtt_offset & GEN12_OAG_OATAILPTR_MASK);
 
 	/* Mark that we need updated tail pointers to read from... */
-	stream->oa_buffer.aging_tail = INVALID_TAIL_PTR;
 	stream->oa_buffer.tail = gtt_offset;
 
 	/*
diff --git a/drivers/gpu/drm/i915/i915_perf_types.h b/drivers/gpu/drm/i915/i915_perf_types.h
index 66dd5f74de05..fe3a5dae8c22 100644
--- a/drivers/gpu/drm/i915/i915_perf_types.h
+++ b/drivers/gpu/drm/i915/i915_perf_types.h
@@ -312,18 +312,6 @@ struct i915_perf_stream {
 		 */
 		spinlock_t ptr_lock;
 
-		/**
-		 * @aging_tail: The last HW tail reported by HW. The data
-		 * might not have made it to memory yet though.
-		 */
-		u32 aging_tail;
-
-		/**
-		 * @aging_timestamp: A monotonic timestamp for when the current aging tail pointer
-		 * was read; used to determine when it is old enough to trust.
-		 */
-		u64 aging_timestamp;
-
 		/**
 		 * @head: Although we can always read back the head pointer register,
 		 * we prefer to avoid trusting the HW state, just to avoid any
-- 
2.36.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Intel-gfx] [PATCH 2/3] i915/perf: Do not add ggtt offset to hw_tail
  2023-05-31 23:56 [Intel-gfx] [PATCH 0/3] Avoid reading OA reports before they land Umesh Nerlige Ramappa
  2023-05-31 23:56 ` [Intel-gfx] [PATCH 1/3] i915/perf: Drop the aging_tail logic in perf OA Umesh Nerlige Ramappa
@ 2023-05-31 23:56 ` Umesh Nerlige Ramappa
  2023-06-01  4:07   ` Dixit, Ashutosh
  2023-05-31 23:56 ` [Intel-gfx] [PATCH 3/3] i915/perf: Drop the aged_tail from rewind logic Umesh Nerlige Ramappa
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 11+ messages in thread
From: Umesh Nerlige Ramappa @ 2023-05-31 23:56 UTC (permalink / raw)
  To: intel-gfx; +Cc: Lionel G Landwerlin

ggtt offset for hw_tail is not required for the calculations, so drop
it.

Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
---
 drivers/gpu/drm/i915/i915_perf.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index 29124dcba8e2..beb1269422ca 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -565,7 +565,7 @@ static bool oa_buffer_check_unlocked(struct i915_perf_stream *stream)
 	partial_report_size %= report_size;
 
 	/* Subtract partial amount off the tail */
-	hw_tail = gtt_offset + OA_TAKEN(hw_tail, partial_report_size);
+	hw_tail = OA_TAKEN(hw_tail, partial_report_size);
 
 
 	/* NB: The head we observe here might effectively be a little
@@ -575,7 +575,6 @@ static bool oa_buffer_check_unlocked(struct i915_perf_stream *stream)
 	head = stream->oa_buffer.head - gtt_offset;
 	aged_tail = stream->oa_buffer.tail - gtt_offset;
 
-	hw_tail -= gtt_offset;
 	tail = hw_tail;
 
 		/* Walk the stream backward until we find a report with report
-- 
2.36.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Intel-gfx] [PATCH 3/3] i915/perf: Drop the aged_tail from rewind logic
  2023-05-31 23:56 [Intel-gfx] [PATCH 0/3] Avoid reading OA reports before they land Umesh Nerlige Ramappa
  2023-05-31 23:56 ` [Intel-gfx] [PATCH 1/3] i915/perf: Drop the aging_tail logic in perf OA Umesh Nerlige Ramappa
  2023-05-31 23:56 ` [Intel-gfx] [PATCH 2/3] i915/perf: Do not add ggtt offset to hw_tail Umesh Nerlige Ramappa
@ 2023-05-31 23:56 ` Umesh Nerlige Ramappa
  2023-06-01  4:13   ` Dixit, Ashutosh
  2023-06-01  3:41 ` [Intel-gfx] ✗ Fi.CI.SPARSE: warning for Avoid reading OA reports before they land Patchwork
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 11+ messages in thread
From: Umesh Nerlige Ramappa @ 2023-05-31 23:56 UTC (permalink / raw)
  To: intel-gfx; +Cc: Lionel G Landwerlin

Instead of aged_tail use an iterator that starts from the hw_tail and
goes backward until the oa_buffer.tail looking for valid reports.

Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
---
 drivers/gpu/drm/i915/i915_perf.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index beb1269422ca..39f5ab1911c8 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -543,7 +543,7 @@ static bool oa_buffer_check_unlocked(struct i915_perf_stream *stream)
 {
 	u32 gtt_offset = i915_ggtt_offset(stream->oa_buffer.vma);
 	int report_size = stream->oa_buffer.format->size;
-	u32 head, tail, aged_tail;
+	u32 head, tail, iter;
 	unsigned long flags;
 	bool pollin;
 	u32 hw_tail;
@@ -567,15 +567,14 @@ static bool oa_buffer_check_unlocked(struct i915_perf_stream *stream)
 	/* Subtract partial amount off the tail */
 	hw_tail = OA_TAKEN(hw_tail, partial_report_size);
 
-
 	/* NB: The head we observe here might effectively be a little
 	 * out of date. If a read() is in progress, the head could be
 	 * anywhere between this head and stream->oa_buffer.tail.
 	 */
 	head = stream->oa_buffer.head - gtt_offset;
-	aged_tail = stream->oa_buffer.tail - gtt_offset;
+	tail = stream->oa_buffer.tail - gtt_offset;
 
-	tail = hw_tail;
+	iter = hw_tail;
 
 		/* Walk the stream backward until we find a report with report
 		 * id and timestmap not at 0. Since the circular buffer pointers
@@ -588,23 +587,23 @@ static bool oa_buffer_check_unlocked(struct i915_perf_stream *stream)
 	 * memory in the order they were written to.
 	 * If not : (╯°□°)╯︵ ┻━┻
 	 */
-	while (OA_TAKEN(tail, aged_tail) >= report_size) {
-		void *report = stream->oa_buffer.vaddr + tail;
+	while (OA_TAKEN(iter, tail) >= report_size) {
+		void *report = stream->oa_buffer.vaddr + iter;
 
 		if (oa_report_id(stream, report) ||
 		    oa_timestamp(stream, report))
 			break;
 
-		tail = (tail - report_size) & (OA_BUFFER_SIZE - 1);
+		iter = (iter - report_size) & (OA_BUFFER_SIZE - 1);
 	}
 
-	if (OA_TAKEN(hw_tail, tail) > report_size &&
+	if (OA_TAKEN(hw_tail, iter) > report_size &&
 	    __ratelimit(&stream->perf->tail_pointer_race))
 		drm_notice(&stream->uncore->i915->drm,
 			   "unlanded report(s) head=0x%x tail=0x%x hw_tail=0x%x\n",
 		 head, tail, hw_tail);
 
-	stream->oa_buffer.tail = gtt_offset + tail;
+	stream->oa_buffer.tail = gtt_offset + iter;
 
 	pollin = OA_TAKEN(stream->oa_buffer.tail - gtt_offset,
 			  stream->oa_buffer.head - gtt_offset) >= report_size;
-- 
2.36.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Intel-gfx] ✗ Fi.CI.SPARSE: warning for Avoid reading OA reports before they land
  2023-05-31 23:56 [Intel-gfx] [PATCH 0/3] Avoid reading OA reports before they land Umesh Nerlige Ramappa
                   ` (2 preceding siblings ...)
  2023-05-31 23:56 ` [Intel-gfx] [PATCH 3/3] i915/perf: Drop the aged_tail from rewind logic Umesh Nerlige Ramappa
@ 2023-06-01  3:41 ` Patchwork
  2023-06-01  3:55 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
  2023-06-02 19:20 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
  5 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2023-06-01  3:41 UTC (permalink / raw)
  To: Umesh Nerlige Ramappa; +Cc: intel-gfx

== Series Details ==

Series: Avoid reading OA reports before they land
URL   : https://patchwork.freedesktop.org/series/118678/
State : warning

== Summary ==

Error: dim sparse failed
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.



^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Intel-gfx] ✓ Fi.CI.BAT: success for Avoid reading OA reports before they land
  2023-05-31 23:56 [Intel-gfx] [PATCH 0/3] Avoid reading OA reports before they land Umesh Nerlige Ramappa
                   ` (3 preceding siblings ...)
  2023-06-01  3:41 ` [Intel-gfx] ✗ Fi.CI.SPARSE: warning for Avoid reading OA reports before they land Patchwork
@ 2023-06-01  3:55 ` Patchwork
  2023-06-02 19:20 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
  5 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2023-06-01  3:55 UTC (permalink / raw)
  To: Umesh Nerlige Ramappa; +Cc: intel-gfx

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

== Series Details ==

Series: Avoid reading OA reports before they land
URL   : https://patchwork.freedesktop.org/series/118678/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_13210 -> Patchwork_118678v1
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118678v1/index.html

Participating hosts (37 -> 38)
------------------------------

  Additional (1): fi-kbl-soraka 

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_huc_copy@huc-copy:
    - fi-kbl-soraka:      NOTRUN -> [SKIP][1] ([fdo#109271] / [i915#2190])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118678v1/fi-kbl-soraka/igt@gem_huc_copy@huc-copy.html

  * igt@gem_lmem_swapping@basic:
    - fi-kbl-soraka:      NOTRUN -> [SKIP][2] ([fdo#109271] / [i915#4613]) +3 similar issues
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118678v1/fi-kbl-soraka/igt@gem_lmem_swapping@basic.html

  * igt@i915_selftest@live@gt_heartbeat:
    - fi-kbl-soraka:      NOTRUN -> [DMESG-FAIL][3] ([i915#5334] / [i915#7872])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118678v1/fi-kbl-soraka/igt@i915_selftest@live@gt_heartbeat.html

  * igt@i915_selftest@live@gt_pm:
    - fi-kbl-soraka:      NOTRUN -> [DMESG-FAIL][4] ([i915#1886] / [i915#7913])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118678v1/fi-kbl-soraka/igt@i915_selftest@live@gt_pm.html

  * igt@i915_selftest@live@slpc:
    - bat-rpls-2:         NOTRUN -> [DMESG-WARN][5] ([i915#6367])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118678v1/bat-rpls-2/igt@i915_selftest@live@slpc.html

  * igt@i915_suspend@basic-s2idle-without-i915:
    - bat-rpls-2:         NOTRUN -> [ABORT][6] ([i915#6687])
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118678v1/bat-rpls-2/igt@i915_suspend@basic-s2idle-without-i915.html

  * igt@kms_chamelium_frames@hdmi-crc-fast:
    - fi-kbl-soraka:      NOTRUN -> [SKIP][7] ([fdo#109271]) +14 similar issues
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118678v1/fi-kbl-soraka/igt@kms_chamelium_frames@hdmi-crc-fast.html

  * igt@kms_setmode@basic-clone-single-crtc:
    - fi-kbl-soraka:      NOTRUN -> [SKIP][8] ([fdo#109271] / [i915#4579])
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118678v1/fi-kbl-soraka/igt@kms_setmode@basic-clone-single-crtc.html

  
#### Possible fixes ####

  * igt@i915_selftest@live@gt_mocs:
    - {bat-mtlp-8}:       [DMESG-FAIL][9] ([i915#7059]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13210/bat-mtlp-8/igt@i915_selftest@live@gt_mocs.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118678v1/bat-mtlp-8/igt@i915_selftest@live@gt_mocs.html

  * igt@i915_selftest@live@reset:
    - bat-rpls-2:         [ABORT][11] ([i915#4983] / [i915#7461] / [i915#7913] / [i915#7981] / [i915#8347]) -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13210/bat-rpls-2/igt@i915_selftest@live@reset.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118678v1/bat-rpls-2/igt@i915_selftest@live@reset.html

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

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [i915#1886]: https://gitlab.freedesktop.org/drm/intel/issues/1886
  [i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
  [i915#4579]: https://gitlab.freedesktop.org/drm/intel/issues/4579
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#4983]: https://gitlab.freedesktop.org/drm/intel/issues/4983
  [i915#5334]: https://gitlab.freedesktop.org/drm/intel/issues/5334
  [i915#6367]: https://gitlab.freedesktop.org/drm/intel/issues/6367
  [i915#6687]: https://gitlab.freedesktop.org/drm/intel/issues/6687
  [i915#7059]: https://gitlab.freedesktop.org/drm/intel/issues/7059
  [i915#7461]: https://gitlab.freedesktop.org/drm/intel/issues/7461
  [i915#7872]: https://gitlab.freedesktop.org/drm/intel/issues/7872
  [i915#7913]: https://gitlab.freedesktop.org/drm/intel/issues/7913
  [i915#7981]: https://gitlab.freedesktop.org/drm/intel/issues/7981
  [i915#8347]: https://gitlab.freedesktop.org/drm/intel/issues/8347


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

  * Linux: CI_DRM_13210 -> Patchwork_118678v1

  CI-20190529: 20190529
  CI_DRM_13210: a66da4c33d8ede541aea9ba6d0d73b556a072d54 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7314: ab70dfcdecf93a17fcaddb774855f726325fa0dd @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_118678v1: a66da4c33d8ede541aea9ba6d0d73b556a072d54 @ git://anongit.freedesktop.org/gfx-ci/linux


### Linux commits

0e89ab79a9c1 i915/perf: Drop the aged_tail from rewind logic
bafc4a5fe8ea i915/perf: Do not add ggtt offset to hw_tail
9db63fd2191b i915/perf: Drop the aging_tail logic in perf OA

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118678v1/index.html

[-- Attachment #2: Type: text/html, Size: 6315 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Intel-gfx] [PATCH 1/3] i915/perf: Drop the aging_tail logic in perf OA
  2023-05-31 23:56 ` [Intel-gfx] [PATCH 1/3] i915/perf: Drop the aging_tail logic in perf OA Umesh Nerlige Ramappa
@ 2023-06-01  4:06   ` Dixit, Ashutosh
  0 siblings, 0 replies; 11+ messages in thread
From: Dixit, Ashutosh @ 2023-06-01  4:06 UTC (permalink / raw)
  To: Umesh Nerlige Ramappa; +Cc: Lionel G Landwerlin, intel-gfx

On Wed, 31 May 2023 16:56:32 -0700, Umesh Nerlige Ramappa wrote:
>

Hi Umesh,

> On DG2, capturing OA reports while running heavy render workloads
> sometimes results in invalid OA reports where 64-byte chunks inside
> reports have stale values. Under memory pressure, high OA sampling rates
> (13.3 us) and heavy render workload, occasionally, the OA HW TAIL
> pointer does not progress as fast as the sampling rate. When these
> glitches occur, the TAIL pointer takes approx. 200us to progress.  While
> this is expected behavior from the HW perspective, invalid reports are
> not expected.
>
> In oa_buffer_check_unlocked(), when we execute the if condition, we are
> updating the oa_buffer.tail to the aging tail and then setting pollin
> based on this tail value, however, we do not have a chance to rewind and
> validate the reports prior to setting pollin. The validation happens
> in a subsequent call to oa_buffer_check_unlocked(). If a read occurs
> before this validation, then we end up reading reports up until this
> oa_buffer.tail value which includes invalid reports. Though found on
> DG2, this affects all platforms.
>
> Start by dropping the aging tail logic.

The patch is fine. But let's also say here in the commit message that the
aging tail logic is not needed because we are now doing explicit detection
of landed reports.

Also please add the previous bug links here since this is really the patch
which fixes the issue (other patches are more like cleanup, I'll explain).

I will R-b this after your repost the series.

Thanks.
--
Ashutosh

> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_perf.c       | 74 ++++++++++----------------
>  drivers/gpu/drm/i915/i915_perf_types.h | 12 -----
>  2 files changed, 28 insertions(+), 58 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
> index 58284156428d..29124dcba8e2 100644
> --- a/drivers/gpu/drm/i915/i915_perf.c
> +++ b/drivers/gpu/drm/i915/i915_perf.c
> @@ -531,8 +531,7 @@ static void oa_context_id_squash(struct i915_perf_stream *stream, u32 *report)
>   * (See description of OA_TAIL_MARGIN_NSEC above for further details.)
>   *
>   * Besides returning true when there is data available to read() this function
> - * also updates the tail, aging_tail and aging_timestamp in the oa_buffer
> - * object.
> + * also updates the tail in the oa_buffer object.
>   *
>   * Note: It's safe to read OA config state here unlocked, assuming that this is
>   * only called while the stream is enabled, while the global OA configuration
> @@ -544,10 +543,10 @@ static bool oa_buffer_check_unlocked(struct i915_perf_stream *stream)
>  {
>	u32 gtt_offset = i915_ggtt_offset(stream->oa_buffer.vma);
>	int report_size = stream->oa_buffer.format->size;
> +	u32 head, tail, aged_tail;
>	unsigned long flags;
>	bool pollin;
>	u32 hw_tail;
> -	u64 now;
>	u32 partial_report_size;
>
>	/* We have to consider the (unlikely) possibility that read() errors
> @@ -568,27 +567,16 @@ static bool oa_buffer_check_unlocked(struct i915_perf_stream *stream)
>	/* Subtract partial amount off the tail */
>	hw_tail = gtt_offset + OA_TAKEN(hw_tail, partial_report_size);
>
> -	now = ktime_get_mono_fast_ns();
>
> -	if (hw_tail == stream->oa_buffer.aging_tail &&
> -	    (now - stream->oa_buffer.aging_timestamp) > OA_TAIL_MARGIN_NSEC) {
> -		/* 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.
> -		 */
> -		stream->oa_buffer.tail = stream->oa_buffer.aging_tail;
> -	} else {
> -		u32 head, tail, aged_tail;
> -
> -		/* NB: The head we observe here might effectively be a little
> -		 * out of date. If a read() is in progress, the head could be
> -		 * anywhere between this head and stream->oa_buffer.tail.
> -		 */
> -		head = stream->oa_buffer.head - gtt_offset;
> -		aged_tail = stream->oa_buffer.tail - gtt_offset;
> +	/* NB: The head we observe here might effectively be a little
> +	 * out of date. If a read() is in progress, the head could be
> +	 * anywhere between this head and stream->oa_buffer.tail.
> +	 */
> +	head = stream->oa_buffer.head - gtt_offset;
> +	aged_tail = stream->oa_buffer.tail - gtt_offset;
>
> -		hw_tail -= gtt_offset;
> -		tail = hw_tail;
> +	hw_tail -= gtt_offset;
> +	tail = hw_tail;
>
>		/* Walk the stream backward until we find a report with report
>		 * id and timestmap not at 0. Since the circular buffer pointers
> @@ -596,31 +584,28 @@ static bool oa_buffer_check_unlocked(struct i915_perf_stream *stream)
>		 * to 256 bytes long, we can't tell whether a report has fully
>		 * landed in memory before the report id and timestamp 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 : (╯°□°)╯︵ ┻━┻
> -		 */
> -		while (OA_TAKEN(tail, aged_tail) >= report_size) {
> -			void *report = stream->oa_buffer.vaddr + tail;
> +	 *
> +	 * This is assuming that the writes of the OA unit land in
> +	 * memory in the order they were written to.
> +	 * If not : (╯°□°)╯︵ ┻━┻
> +	 */
> +	while (OA_TAKEN(tail, aged_tail) >= report_size) {
> +		void *report = stream->oa_buffer.vaddr + tail;
>
> -			if (oa_report_id(stream, report) ||
> -			    oa_timestamp(stream, report))
> -				break;
> +		if (oa_report_id(stream, report) ||
> +		    oa_timestamp(stream, report))
> +			break;
>
> -			tail = (tail - report_size) & (OA_BUFFER_SIZE - 1);
> -		}
> +		tail = (tail - report_size) & (OA_BUFFER_SIZE - 1);
> +	}
>
> -		if (OA_TAKEN(hw_tail, tail) > report_size &&
> -		    __ratelimit(&stream->perf->tail_pointer_race))
> -			drm_notice(&stream->uncore->i915->drm,
> -				   "unlanded report(s) head=0x%x tail=0x%x hw_tail=0x%x\n",
> -				   head, tail, hw_tail);
> +	if (OA_TAKEN(hw_tail, tail) > report_size &&
> +	    __ratelimit(&stream->perf->tail_pointer_race))
> +		drm_notice(&stream->uncore->i915->drm,
> +			   "unlanded report(s) head=0x%x tail=0x%x hw_tail=0x%x\n",
> +		 head, tail, hw_tail);
>
> -		stream->oa_buffer.tail = gtt_offset + tail;
> -		stream->oa_buffer.aging_tail = gtt_offset + hw_tail;
> -		stream->oa_buffer.aging_timestamp = now;
> -	}
> +	stream->oa_buffer.tail = gtt_offset + tail;
>
>	pollin = OA_TAKEN(stream->oa_buffer.tail - gtt_offset,
>			  stream->oa_buffer.head - gtt_offset) >= report_size;
> @@ -1727,7 +1712,6 @@ static void gen7_init_oa_buffer(struct i915_perf_stream *stream)
>			   gtt_offset | OABUFFER_SIZE_16M);
>
>	/* Mark that we need updated tail pointers to read from... */
> -	stream->oa_buffer.aging_tail = INVALID_TAIL_PTR;
>	stream->oa_buffer.tail = gtt_offset;
>
>	spin_unlock_irqrestore(&stream->oa_buffer.ptr_lock, flags);
> @@ -1779,7 +1763,6 @@ static void gen8_init_oa_buffer(struct i915_perf_stream *stream)
>	intel_uncore_write(uncore, GEN8_OATAILPTR, gtt_offset & GEN8_OATAILPTR_MASK);
>
>	/* Mark that we need updated tail pointers to read from... */
> -	stream->oa_buffer.aging_tail = INVALID_TAIL_PTR;
>	stream->oa_buffer.tail = gtt_offset;
>
>	/*
> @@ -1833,7 +1816,6 @@ static void gen12_init_oa_buffer(struct i915_perf_stream *stream)
>			   gtt_offset & GEN12_OAG_OATAILPTR_MASK);
>
>	/* Mark that we need updated tail pointers to read from... */
> -	stream->oa_buffer.aging_tail = INVALID_TAIL_PTR;
>	stream->oa_buffer.tail = gtt_offset;
>
>	/*
> diff --git a/drivers/gpu/drm/i915/i915_perf_types.h b/drivers/gpu/drm/i915/i915_perf_types.h
> index 66dd5f74de05..fe3a5dae8c22 100644
> --- a/drivers/gpu/drm/i915/i915_perf_types.h
> +++ b/drivers/gpu/drm/i915/i915_perf_types.h
> @@ -312,18 +312,6 @@ struct i915_perf_stream {
>		 */
>		spinlock_t ptr_lock;
>
> -		/**
> -		 * @aging_tail: The last HW tail reported by HW. The data
> -		 * might not have made it to memory yet though.
> -		 */
> -		u32 aging_tail;
> -
> -		/**
> -		 * @aging_timestamp: A monotonic timestamp for when the current aging tail pointer
> -		 * was read; used to determine when it is old enough to trust.
> -		 */
> -		u64 aging_timestamp;
> -
>		/**
>		 * @head: Although we can always read back the head pointer register,
>		 * we prefer to avoid trusting the HW state, just to avoid any
> --
> 2.36.1
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Intel-gfx] [PATCH 2/3] i915/perf: Do not add ggtt offset to hw_tail
  2023-05-31 23:56 ` [Intel-gfx] [PATCH 2/3] i915/perf: Do not add ggtt offset to hw_tail Umesh Nerlige Ramappa
@ 2023-06-01  4:07   ` Dixit, Ashutosh
  0 siblings, 0 replies; 11+ messages in thread
From: Dixit, Ashutosh @ 2023-06-01  4:07 UTC (permalink / raw)
  To: Umesh Nerlige Ramappa; +Cc: Lionel G Landwerlin, intel-gfx

On Wed, 31 May 2023 16:56:33 -0700, Umesh Nerlige Ramappa wrote:
>
> ggtt offset for hw_tail is not required for the calculations, so drop
> it.
>
> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_perf.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
> index 29124dcba8e2..beb1269422ca 100644
> --- a/drivers/gpu/drm/i915/i915_perf.c
> +++ b/drivers/gpu/drm/i915/i915_perf.c
> @@ -565,7 +565,7 @@ static bool oa_buffer_check_unlocked(struct i915_perf_stream *stream)
>	partial_report_size %= report_size;
>
>	/* Subtract partial amount off the tail */
> -	hw_tail = gtt_offset + OA_TAKEN(hw_tail, partial_report_size);
> +	hw_tail = OA_TAKEN(hw_tail, partial_report_size);
>
>
>	/* NB: The head we observe here might effectively be a little
> @@ -575,7 +575,6 @@ static bool oa_buffer_check_unlocked(struct i915_perf_stream *stream)
>	head = stream->oa_buffer.head - gtt_offset;
>	aged_tail = stream->oa_buffer.tail - gtt_offset;
>
> -	hw_tail -= gtt_offset;

Yes gtt_offset addition/subtraction is now redundant after we drop the if-else.

Reviewed-by: Ashutosh Dixit <ashutosh.dixit@intel.com>


>	tail = hw_tail;
>
>		/* Walk the stream backward until we find a report with report
> --
> 2.36.1
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Intel-gfx] [PATCH 3/3] i915/perf: Drop the aged_tail from rewind logic
  2023-05-31 23:56 ` [Intel-gfx] [PATCH 3/3] i915/perf: Drop the aged_tail from rewind logic Umesh Nerlige Ramappa
@ 2023-06-01  4:13   ` Dixit, Ashutosh
  2023-06-01 18:16     ` Umesh Nerlige Ramappa
  0 siblings, 1 reply; 11+ messages in thread
From: Dixit, Ashutosh @ 2023-06-01  4:13 UTC (permalink / raw)
  To: Umesh Nerlige Ramappa; +Cc: Lionel G Landwerlin, intel-gfx

On Wed, 31 May 2023 16:56:34 -0700, Umesh Nerlige Ramappa wrote:
>

Hi Umesh,

> Instead of aged_tail use an iterator that starts from the hw_tail and
> goes backward until the oa_buffer.tail looking for valid reports.

Hmm I don't think this description is correct. All this patch is doing is
the following:

a. s/aged_tail/tail/
b. s/tail/iter/

So basically I don't think we need this patch. All we want to do here is
change the variable name aged_tail to something else (to completely remove
the concept of aging from the OA code) but other changes such as name
change to iter etc. is unnecessary.

So I would just keep the patch simple and change the name aged_tail to
advertized_tail or exported_tail or read_tail, because basically
stream->oa_buffer.tail is the tail which the writer updates (or advertizes
or exports) for the reader.

So we only should rename aged_tail here, the other changes are not needed.

We could even squash this change into Patch 1 or Patch 2, since it is
really a trivial variable rename.

Thanks.
--
Ashutosh


>
> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_perf.c | 17 ++++++++---------
>  1 file changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
> index beb1269422ca..39f5ab1911c8 100644
> --- a/drivers/gpu/drm/i915/i915_perf.c
> +++ b/drivers/gpu/drm/i915/i915_perf.c
> @@ -543,7 +543,7 @@ static bool oa_buffer_check_unlocked(struct i915_perf_stream *stream)
>  {
>	u32 gtt_offset = i915_ggtt_offset(stream->oa_buffer.vma);
>	int report_size = stream->oa_buffer.format->size;
> -	u32 head, tail, aged_tail;
> +	u32 head, tail, iter;
>	unsigned long flags;
>	bool pollin;
>	u32 hw_tail;
> @@ -567,15 +567,14 @@ static bool oa_buffer_check_unlocked(struct i915_perf_stream *stream)
>	/* Subtract partial amount off the tail */
>	hw_tail = OA_TAKEN(hw_tail, partial_report_size);
>
> -
>	/* NB: The head we observe here might effectively be a little
>	 * out of date. If a read() is in progress, the head could be
>	 * anywhere between this head and stream->oa_buffer.tail.
>	 */
>	head = stream->oa_buffer.head - gtt_offset;
> -	aged_tail = stream->oa_buffer.tail - gtt_offset;
> +	tail = stream->oa_buffer.tail - gtt_offset;
>
> -	tail = hw_tail;
> +	iter = hw_tail;
>
>		/* Walk the stream backward until we find a report with report
>		 * id and timestmap not at 0. Since the circular buffer pointers
> @@ -588,23 +587,23 @@ static bool oa_buffer_check_unlocked(struct i915_perf_stream *stream)
>	 * memory in the order they were written to.
>	 * If not : (╯°□°)╯︵ ┻━┻
>	 */
> -	while (OA_TAKEN(tail, aged_tail) >= report_size) {
> -		void *report = stream->oa_buffer.vaddr + tail;
> +	while (OA_TAKEN(iter, tail) >= report_size) {
> +		void *report = stream->oa_buffer.vaddr + iter;
>
>		if (oa_report_id(stream, report) ||
>		    oa_timestamp(stream, report))
>			break;
>
> -		tail = (tail - report_size) & (OA_BUFFER_SIZE - 1);
> +		iter = (iter - report_size) & (OA_BUFFER_SIZE - 1);
>	}
>
> -	if (OA_TAKEN(hw_tail, tail) > report_size &&
> +	if (OA_TAKEN(hw_tail, iter) > report_size &&
>	    __ratelimit(&stream->perf->tail_pointer_race))
>		drm_notice(&stream->uncore->i915->drm,
>			   "unlanded report(s) head=0x%x tail=0x%x hw_tail=0x%x\n",
>		 head, tail, hw_tail);
>
> -	stream->oa_buffer.tail = gtt_offset + tail;
> +	stream->oa_buffer.tail = gtt_offset + iter;
>
>	pollin = OA_TAKEN(stream->oa_buffer.tail - gtt_offset,
>			  stream->oa_buffer.head - gtt_offset) >= report_size;
> --
> 2.36.1
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Intel-gfx] [PATCH 3/3] i915/perf: Drop the aged_tail from rewind logic
  2023-06-01  4:13   ` Dixit, Ashutosh
@ 2023-06-01 18:16     ` Umesh Nerlige Ramappa
  0 siblings, 0 replies; 11+ messages in thread
From: Umesh Nerlige Ramappa @ 2023-06-01 18:16 UTC (permalink / raw)
  To: Dixit, Ashutosh; +Cc: Lionel G Landwerlin, intel-gfx

On Wed, May 31, 2023 at 09:13:02PM -0700, Dixit, Ashutosh wrote:
>On Wed, 31 May 2023 16:56:34 -0700, Umesh Nerlige Ramappa wrote:
>>
>
>Hi Umesh,
>
>> Instead of aged_tail use an iterator that starts from the hw_tail and
>> goes backward until the oa_buffer.tail looking for valid reports.
>
>Hmm I don't think this description is correct. All this patch is doing is
>the following:
>
>a. s/aged_tail/tail/
>b. s/tail/iter/
>
>So basically I don't think we need this patch. All we want to do here is
>change the variable name aged_tail to something else (to completely remove
>the concept of aging from the OA code) but other changes such as name
>change to iter etc. is unnecessary.
>
>So I would just keep the patch simple and change the name aged_tail to
>advertized_tail or exported_tail or read_tail, because basically
>stream->oa_buffer.tail is the tail which the writer updates (or advertizes
>or exports) for the reader.
>
>So we only should rename aged_tail here, the other changes are not needed.
>
>We could even squash this change into Patch 1 or Patch 2, since it is
>really a trivial variable rename.

The whole point was just readability. head/tail point to what the user 
consumes. hw_tail points to the actual hw register value and iter is 
just loop iterator.

Since the intent of the series is to just get rid of aging/aged logic, I 
can just s/aged_tail/read_tail/ and squash it with 1 since it belongs 
more to 1 than 2, although, I still like the my current patch (maybe 
with additional description in the commit message to clarify that the 
patch is just renames for readability).

Will post next rev with the simple rename and squash.

Thanks,
Umesh

>
>Thanks.
>--
>Ashutosh
>
>
>>
>> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_perf.c | 17 ++++++++---------
>>  1 file changed, 8 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
>> index beb1269422ca..39f5ab1911c8 100644
>> --- a/drivers/gpu/drm/i915/i915_perf.c
>> +++ b/drivers/gpu/drm/i915/i915_perf.c
>> @@ -543,7 +543,7 @@ static bool oa_buffer_check_unlocked(struct i915_perf_stream *stream)
>>  {
>>	u32 gtt_offset = i915_ggtt_offset(stream->oa_buffer.vma);
>>	int report_size = stream->oa_buffer.format->size;
>> -	u32 head, tail, aged_tail;
>> +	u32 head, tail, iter;
>>	unsigned long flags;
>>	bool pollin;
>>	u32 hw_tail;
>> @@ -567,15 +567,14 @@ static bool oa_buffer_check_unlocked(struct i915_perf_stream *stream)
>>	/* Subtract partial amount off the tail */
>>	hw_tail = OA_TAKEN(hw_tail, partial_report_size);
>>
>> -
>>	/* NB: The head we observe here might effectively be a little
>>	 * out of date. If a read() is in progress, the head could be
>>	 * anywhere between this head and stream->oa_buffer.tail.
>>	 */
>>	head = stream->oa_buffer.head - gtt_offset;
>> -	aged_tail = stream->oa_buffer.tail - gtt_offset;
>> +	tail = stream->oa_buffer.tail - gtt_offset;
>>
>> -	tail = hw_tail;
>> +	iter = hw_tail;
>>
>>		/* Walk the stream backward until we find a report with report
>>		 * id and timestmap not at 0. Since the circular buffer pointers
>> @@ -588,23 +587,23 @@ static bool oa_buffer_check_unlocked(struct i915_perf_stream *stream)
>>	 * memory in the order they were written to.
>>	 * If not : (╯°□°)╯︵ ┻━┻
>>	 */
>> -	while (OA_TAKEN(tail, aged_tail) >= report_size) {
>> -		void *report = stream->oa_buffer.vaddr + tail;
>> +	while (OA_TAKEN(iter, tail) >= report_size) {
>> +		void *report = stream->oa_buffer.vaddr + iter;
>>
>>		if (oa_report_id(stream, report) ||
>>		    oa_timestamp(stream, report))
>>			break;
>>
>> -		tail = (tail - report_size) & (OA_BUFFER_SIZE - 1);
>> +		iter = (iter - report_size) & (OA_BUFFER_SIZE - 1);
>>	}
>>
>> -	if (OA_TAKEN(hw_tail, tail) > report_size &&
>> +	if (OA_TAKEN(hw_tail, iter) > report_size &&
>>	    __ratelimit(&stream->perf->tail_pointer_race))
>>		drm_notice(&stream->uncore->i915->drm,
>>			   "unlanded report(s) head=0x%x tail=0x%x hw_tail=0x%x\n",
>>		 head, tail, hw_tail);
>>
>> -	stream->oa_buffer.tail = gtt_offset + tail;
>> +	stream->oa_buffer.tail = gtt_offset + iter;
>>
>>	pollin = OA_TAKEN(stream->oa_buffer.tail - gtt_offset,
>>			  stream->oa_buffer.head - gtt_offset) >= report_size;
>> --
>> 2.36.1
>>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Intel-gfx] ✓ Fi.CI.IGT: success for Avoid reading OA reports before they land
  2023-05-31 23:56 [Intel-gfx] [PATCH 0/3] Avoid reading OA reports before they land Umesh Nerlige Ramappa
                   ` (4 preceding siblings ...)
  2023-06-01  3:55 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
@ 2023-06-02 19:20 ` Patchwork
  5 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2023-06-02 19:20 UTC (permalink / raw)
  To: Umesh Nerlige Ramappa; +Cc: intel-gfx

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

== Series Details ==

Series: Avoid reading OA reports before they land
URL   : https://patchwork.freedesktop.org/series/118678/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_13210_full -> Patchwork_118678v1_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

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

  Missing    (1): shard-rkl0 

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_fair@basic-pace-share@rcs0:
    - shard-glk:          [PASS][1] -> [FAIL][2] ([i915#2842])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13210/shard-glk2/igt@gem_exec_fair@basic-pace-share@rcs0.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118678v1/shard-glk9/igt@gem_exec_fair@basic-pace-share@rcs0.html

  * igt@gen7_exec_parse@oacontrol-tracking:
    - shard-apl:          NOTRUN -> [SKIP][3] ([fdo#109271]) +18 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118678v1/shard-apl2/igt@gen7_exec_parse@oacontrol-tracking.html

  * igt@kms_ccs@pipe-a-crc-primary-rotation-180-y_tiled_gen12_rc_ccs_cc:
    - shard-apl:          NOTRUN -> [SKIP][4] ([fdo#109271] / [i915#3886])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118678v1/shard-apl4/igt@kms_ccs@pipe-a-crc-primary-rotation-180-y_tiled_gen12_rc_ccs_cc.html

  * igt@kms_cursor_crc@cursor-onscreen-32x32:
    - shard-apl:          NOTRUN -> [SKIP][5] ([fdo#109271] / [i915#4579])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118678v1/shard-apl2/igt@kms_cursor_crc@cursor-onscreen-32x32.html

  * igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size:
    - shard-apl:          [PASS][6] -> [FAIL][7] ([i915#2346])
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13210/shard-apl1/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118678v1/shard-apl7/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size.html

  * igt@kms_flip@flip-vs-expired-vblank@a-hdmi-a2:
    - shard-glk:          [PASS][8] -> [FAIL][9] ([i915#79])
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13210/shard-glk3/igt@kms_flip@flip-vs-expired-vblank@a-hdmi-a2.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118678v1/shard-glk4/igt@kms_flip@flip-vs-expired-vblank@a-hdmi-a2.html

  * igt@kms_plane_scaling@invalid-num-scalers@pipe-a-hdmi-a-1-invalid-num-scalers:
    - shard-snb:          NOTRUN -> [SKIP][10] ([fdo#109271]) +16 similar issues
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118678v1/shard-snb1/igt@kms_plane_scaling@invalid-num-scalers@pipe-a-hdmi-a-1-invalid-num-scalers.html

  * igt@kms_plane_scaling@plane-downscale-with-pixel-format-factor-0-5@pipe-b-hdmi-a-1:
    - shard-snb:          NOTRUN -> [SKIP][11] ([fdo#109271] / [i915#4579]) +8 similar issues
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118678v1/shard-snb1/igt@kms_plane_scaling@plane-downscale-with-pixel-format-factor-0-5@pipe-b-hdmi-a-1.html

  * igt@kms_setmode@basic@pipe-a-vga-1:
    - shard-snb:          NOTRUN -> [FAIL][12] ([i915#5465]) +1 similar issue
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118678v1/shard-snb5/igt@kms_setmode@basic@pipe-a-vga-1.html

  
#### Possible fixes ####

  * igt@drm_fdinfo@most-busy-idle-check-all@rcs0:
    - {shard-rkl}:        [FAIL][13] ([i915#7742]) -> [PASS][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13210/shard-rkl-4/igt@drm_fdinfo@most-busy-idle-check-all@rcs0.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118678v1/shard-rkl-3/igt@drm_fdinfo@most-busy-idle-check-all@rcs0.html

  * igt@gem_barrier_race@remote-request@rcs0:
    - shard-apl:          [ABORT][15] ([i915#7461] / [i915#8211] / [i915#8234]) -> [PASS][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13210/shard-apl6/igt@gem_barrier_race@remote-request@rcs0.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118678v1/shard-apl2/igt@gem_barrier_race@remote-request@rcs0.html

  * igt@gem_ctx_freq@sysfs:
    - {shard-dg1}:        [FAIL][17] ([i915#6786]) -> [PASS][18]
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13210/shard-dg1-19/igt@gem_ctx_freq@sysfs.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118678v1/shard-dg1-14/igt@gem_ctx_freq@sysfs.html

  * igt@gem_ctx_isolation@preservation-s3@vcs0:
    - shard-apl:          [ABORT][19] ([i915#180]) -> [PASS][20] +1 similar issue
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13210/shard-apl6/igt@gem_ctx_isolation@preservation-s3@vcs0.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118678v1/shard-apl4/igt@gem_ctx_isolation@preservation-s3@vcs0.html

  * igt@gem_exec_fair@basic-none-solo@rcs0:
    - shard-apl:          [FAIL][21] ([i915#2842]) -> [PASS][22]
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13210/shard-apl3/igt@gem_exec_fair@basic-none-solo@rcs0.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118678v1/shard-apl1/igt@gem_exec_fair@basic-none-solo@rcs0.html

  * igt@gem_exec_fair@basic-pace-solo@rcs0:
    - {shard-rkl}:        [FAIL][23] ([i915#2842]) -> [PASS][24]
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13210/shard-rkl-1/igt@gem_exec_fair@basic-pace-solo@rcs0.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118678v1/shard-rkl-1/igt@gem_exec_fair@basic-pace-solo@rcs0.html

  * igt@i915_pm_lpsp@kms-lpsp@kms-lpsp-hdmi-a:
    - {shard-rkl}:        [SKIP][25] ([i915#1937] / [i915#4579]) -> [PASS][26]
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13210/shard-rkl-4/igt@i915_pm_lpsp@kms-lpsp@kms-lpsp-hdmi-a.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118678v1/shard-rkl-7/igt@i915_pm_lpsp@kms-lpsp@kms-lpsp-hdmi-a.html

  * igt@i915_pm_rc6_residency@rc6-idle@vecs0:
    - {shard-dg1}:        [FAIL][27] ([i915#3591]) -> [PASS][28]
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13210/shard-dg1-17/igt@i915_pm_rc6_residency@rc6-idle@vecs0.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118678v1/shard-dg1-19/igt@i915_pm_rc6_residency@rc6-idle@vecs0.html

  * igt@i915_pm_rpm@modeset-lpsp-stress:
    - {shard-rkl}:        [SKIP][29] ([i915#1397]) -> [PASS][30] +1 similar issue
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13210/shard-rkl-4/igt@i915_pm_rpm@modeset-lpsp-stress.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118678v1/shard-rkl-7/igt@i915_pm_rpm@modeset-lpsp-stress.html

  * igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions:
    - shard-apl:          [FAIL][31] ([i915#2346]) -> [PASS][32]
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13210/shard-apl4/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118678v1/shard-apl6/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions.html

  * igt@kms_plane_scaling@intel-max-src-size@pipe-a-hdmi-a-2:
    - {shard-rkl}:        [FAIL][33] ([i915#8292]) -> [PASS][34]
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13210/shard-rkl-3/igt@kms_plane_scaling@intel-max-src-size@pipe-a-hdmi-a-2.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118678v1/shard-rkl-2/igt@kms_plane_scaling@intel-max-src-size@pipe-a-hdmi-a-2.html

  * igt@kms_rotation_crc@bad-pixel-format:
    - {shard-rkl}:        [ABORT][35] ([i915#8311]) -> [PASS][36]
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13210/shard-rkl-1/igt@kms_rotation_crc@bad-pixel-format.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118678v1/shard-rkl-3/igt@kms_rotation_crc@bad-pixel-format.html

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

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109283]: https://bugs.freedesktop.org/show_bug.cgi?id=109283
  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
  [fdo#109300]: https://bugs.freedesktop.org/show_bug.cgi?id=109300
  [fdo#109303]: https://bugs.freedesktop.org/show_bug.cgi?id=109303
  [fdo#109309]: https://bugs.freedesktop.org/show_bug.cgi?id=109309
  [fdo#109315]: https://bugs.freedesktop.org/show_bug.cgi?id=109315
  [fdo#110189]: https://bugs.freedesktop.org/show_bug.cgi?id=110189
  [fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068
  [fdo#111614]: https://bugs.freedesktop.org/show_bug.cgi?id=111614
  [fdo#111615]: https://bugs.freedesktop.org/show_bug.cgi?id=111615
  [fdo#111825]: https://bugs.freedesktop.org/show_bug.cgi?id=111825
  [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
  [i915#1257]: https://gitlab.freedesktop.org/drm/intel/issues/1257
  [i915#1397]: https://gitlab.freedesktop.org/drm/intel/issues/1397
  [i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
  [i915#1825]: https://gitlab.freedesktop.org/drm/intel/issues/1825
  [i915#1937]: https://gitlab.freedesktop.org/drm/intel/issues/1937
  [i915#2346]: https://gitlab.freedesktop.org/drm/intel/issues/2346
  [i915#2575]: https://gitlab.freedesktop.org/drm/intel/issues/2575
  [i915#2587]: https://gitlab.freedesktop.org/drm/intel/issues/2587
  [i915#2672]: https://gitlab.freedesktop.org/drm/intel/issues/2672
  [i915#2681]: https://gitlab.freedesktop.org/drm/intel/issues/2681
  [i915#2842]: https://gitlab.freedesktop.org/drm/intel/issues/2842
  [i915#3023]: https://gitlab.freedesktop.org/drm/intel/issues/3023
  [i915#3281]: https://gitlab.freedesktop.org/drm/intel/issues/3281
  [i915#3282]: https://gitlab.freedesktop.org/drm/intel/issues/3282
  [i915#3458]: https://gitlab.freedesktop.org/drm/intel/issues/3458
  [i915#3539]: https://gitlab.freedesktop.org/drm/intel/issues/3539
  [i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
  [i915#3591]: https://gitlab.freedesktop.org/drm/intel/issues/3591
  [i915#3638]: https://gitlab.freedesktop.org/drm/intel/issues/3638
  [i915#3689]: https://gitlab.freedesktop.org/drm/intel/issues/3689
  [i915#3734]: https://gitlab.freedesktop.org/drm/intel/issues/3734
  [i915#3778]: https://gitlab.freedesktop.org/drm/intel/issues/3778
  [i915#3886]: https://gitlab.freedesktop.org/drm/intel/issues/3886
  [i915#3955]: https://gitlab.freedesktop.org/drm/intel/issues/3955
  [i915#4036]: https://gitlab.freedesktop.org/drm/intel/issues/4036
  [i915#4070]: https://gitlab.freedesktop.org/drm/intel/issues/4070
  [i915#4077]: https://gitlab.freedesktop.org/drm/intel/issues/4077
  [i915#4078]: https://gitlab.freedesktop.org/drm/intel/issues/4078
  [i915#4083]: https://gitlab.freedesktop.org/drm/intel/issues/4083
  [i915#4098]: https://gitlab.freedesktop.org/drm/intel/issues/4098
  [i915#4270]: https://gitlab.freedesktop.org/drm/intel/issues/4270
  [i915#4349]: https://gitlab.freedesktop.org/drm/intel/issues/4349
  [i915#4538]: https://gitlab.freedesktop.org/drm/intel/issues/4538
  [i915#4579]: https://gitlab.freedesktop.org/drm/intel/issues/4579
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#4771]: https://gitlab.freedesktop.org/drm/intel/issues/4771
  [i915#4816]: https://gitlab.freedesktop.org/drm/intel/issues/4816
  [i915#4833]: https://gitlab.freedesktop.org/drm/intel/issues/4833
  [i915#4852]: https://gitlab.freedesktop.org/drm/intel/issues/4852
  [i915#4860]: https://gitlab.freedesktop.org/drm/intel/issues/4860
  [i915#5176]: https://gitlab.freedesktop.org/drm/intel/issues/5176
  [i915#5235]: https://gitlab.freedesktop.org/drm/intel/issues/5235
  [i915#5286]: https://gitlab.freedesktop.org/drm/intel/issues/5286
  [i915#5289]: https://gitlab.freedesktop.org/drm/intel/issues/5289
  [i915#5325]: https://gitlab.freedesktop.org/drm/intel/issues/5325
  [i915#533]: https://gitlab.freedesktop.org/drm/intel/issues/533
  [i915#5354]: https://gitlab.freedesktop.org/drm/intel/issues/5354
  [i915#5439]: https://gitlab.freedesktop.org/drm/intel/issues/5439
  [i915#5465]: https://gitlab.freedesktop.org/drm/intel/issues/5465
  [i915#6095]: https://gitlab.freedesktop.org/drm/intel/issues/6095
  [i915#6268]: https://gitlab.freedesktop.org/drm/intel/issues/6268
  [i915#6301]: https://gitlab.freedesktop.org/drm/intel/issues/6301
  [i915#6524]: https://gitlab.freedesktop.org/drm/intel/issues/6524
  [i915#658]: https://gitlab.freedesktop.org/drm/intel/issues/658
  [i915#6768]: https://gitlab.freedesktop.org/drm/intel/issues/6768
  [i915#6786]: https://gitlab.freedesktop.org/drm/intel/issues/6786
  [i915#7456]: https://gitlab.freedesktop.org/drm/intel/issues/7456
  [i915#7461]: https://gitlab.freedesktop.org/drm/intel/issues/7461
  [i915#7561]: https://gitlab.freedesktop.org/drm/intel/issues/7561
  [i915#7711]: https://gitlab.freedesktop.org/drm/intel/issues/7711
  [i915#7742]: https://gitlab.freedesktop.org/drm/intel/issues/7742
  [i915#7828]: https://gitlab.freedesktop.org/drm/intel/issues/7828
  [i915#79]: https://gitlab.freedesktop.org/drm/intel/issues/79
  [i915#7975]: https://gitlab.freedesktop.org/drm/intel/issues/7975
  [i915#8211]: https://gitlab.freedesktop.org/drm/intel/issues/8211
  [i915#8213]: https://gitlab.freedesktop.org/drm/intel/issues/8213
  [i915#8234]: https://gitlab.freedesktop.org/drm/intel/issues/8234
  [i915#8247]: https://gitlab.freedesktop.org/drm/intel/issues/8247
  [i915#8292]: https://gitlab.freedesktop.org/drm/intel/issues/8292
  [i915#8311]: https://gitlab.freedesktop.org/drm/intel/issues/8311
  [i915#8411]: https://gitlab.freedesktop.org/drm/intel/issues/8411
  [i915#8414]: https://gitlab.freedesktop.org/drm/intel/issues/8414
  [i915#8555]: https://gitlab.freedesktop.org/drm/intel/issues/8555


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

  * Linux: CI_DRM_13210 -> Patchwork_118678v1

  CI-20190529: 20190529
  CI_DRM_13210: a66da4c33d8ede541aea9ba6d0d73b556a072d54 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7314: ab70dfcdecf93a17fcaddb774855f726325fa0dd @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_118678v1: a66da4c33d8ede541aea9ba6d0d73b556a072d54 @ 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_118678v1/index.html

[-- Attachment #2: Type: text/html, Size: 11625 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2023-06-02 19:20 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-31 23:56 [Intel-gfx] [PATCH 0/3] Avoid reading OA reports before they land Umesh Nerlige Ramappa
2023-05-31 23:56 ` [Intel-gfx] [PATCH 1/3] i915/perf: Drop the aging_tail logic in perf OA Umesh Nerlige Ramappa
2023-06-01  4:06   ` Dixit, Ashutosh
2023-05-31 23:56 ` [Intel-gfx] [PATCH 2/3] i915/perf: Do not add ggtt offset to hw_tail Umesh Nerlige Ramappa
2023-06-01  4:07   ` Dixit, Ashutosh
2023-05-31 23:56 ` [Intel-gfx] [PATCH 3/3] i915/perf: Drop the aged_tail from rewind logic Umesh Nerlige Ramappa
2023-06-01  4:13   ` Dixit, Ashutosh
2023-06-01 18:16     ` Umesh Nerlige Ramappa
2023-06-01  3:41 ` [Intel-gfx] ✗ Fi.CI.SPARSE: warning for Avoid reading OA reports before they land Patchwork
2023-06-01  3:55 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2023-06-02 19:20 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).