Re: [Intel-gfx] [PATCH] RFC/RFT drm/i915/oa: Drop aging-tail

2019-02-19 Thread Lionel Landwerlin

On 19/02/2019 10:28, Chris Wilson wrote:

Switch to using coherent reads that are serialised with the register
read to avoid the memory latency problem in favour over an arbitrary
delay. The only zeroes seen during testing on HSW+ have been from
configuration changes that do not update (therefore were truly zero
entries and should be skipped).

Signed-off-by: Chris Wilson 
Cc: Lionel Landwerlin 
Cc: Matthew Auld 
---
  drivers/gpu/drm/i915/i915_drv.h  |  59 ---
  drivers/gpu/drm/i915/i915_perf.c | 625 +--
  2 files changed, 87 insertions(+), 597 deletions(-)



I took the I915_READ_FW() changes + the i915_vma_(un)pin_iomap and I'm 
still seeing reads of the HW tail register pointing 2 reports behind the 
last one that actually has its reason & timestamp fields != 0.


That is within a run where at the timestamp register went from 
0xa21d5813 to 0xa3b3441a for example.


But the DRM_NOTE("Skipping spurious, invalid OA report\n"); didn't fire 
once, meaning the reports had their data landing some time after the 
oa_buffer_check() call.



To me this seems to show there is clearly an issue with the HW and that 
we need the workaround.



-Lionel


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

Re: [Intel-gfx] [PATCH] RFC/RFT drm/i915/oa: Drop aging-tail

2019-02-19 Thread Lionel Landwerlin

On 19/02/2019 10:28, Chris Wilson wrote:

   */
  void i915_perf_init(struct drm_i915_private *dev_priv)
  {
+   if (!i915_has_memcpy_from_wc())
+   return;
+


Does this put restrictions on particular platforms or is it just a 
compiler feature?



-Lionel

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

Re: [Intel-gfx] [PATCH] RFC/RFT drm/i915/oa: Drop aging-tail

2019-02-19 Thread Lionel Landwerlin

On 19/02/2019 10:28, Chris Wilson wrote:

Switch to using coherent reads that are serialised with the register
read to avoid the memory latency problem in favour over an arbitrary
delay. The only zeroes seen during testing on HSW+ have been from
configuration changes that do not update (therefore were truly zero
entries and should be skipped).

Signed-off-by: Chris Wilson
Cc: Lionel Landwerlin
Cc: Matthew Auld
---
  drivers/gpu/drm/i915/i915_drv.h  |  59 ---
  drivers/gpu/drm/i915/i915_perf.c | 625 +--
  2 files changed, 87 insertions(+), 597 deletions(-)



Thanks,

I'll try i915_vma_pin_iomap() + I915_READ_FW() and see if the DRM_NOTE 
go away.


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

[Intel-gfx] [PATCH] RFC/RFT drm/i915/oa: Drop aging-tail

2019-02-19 Thread Chris Wilson
Switch to using coherent reads that are serialised with the register
read to avoid the memory latency problem in favour over an arbitrary
delay. The only zeroes seen during testing on HSW+ have been from
configuration changes that do not update (therefore were truly zero
entries and should be skipped).

Signed-off-by: Chris Wilson 
Cc: Lionel Landwerlin 
Cc: Matthew Auld 
---
 drivers/gpu/drm/i915/i915_drv.h  |  59 ---
 drivers/gpu/drm/i915/i915_perf.c | 625 +--
 2 files changed, 87 insertions(+), 597 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 5c8d0489a1cd..773c205e1bbe 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1881,12 +1881,6 @@ struct drm_i915_private {
wait_queue_head_t poll_wq;
bool pollin;
 
-   /**
-* For rate limiting any notifications of spurious
-* invalid OA reports
-*/
-   struct ratelimit_state spurious_report_rs;
-
bool periodic;
int period_exponent;
 
@@ -1899,59 +1893,6 @@ struct drm_i915_private {
int format;
int format_size;
 
-   /**
-* Locks reads and writes to all head/tail state
-*
-* Consider: the head and tail pointer state
-* needs to be read consistently from a hrtimer
-* callback (atomic context) and read() fop
-* (user context) with tail pointer updates
-* happening in atomic context and head updates
-* in user context and the (unlikely)
-* possibility of read() errors needing to
-* reset all head/tail state.
-*
-* Note: Contention or performance aren't
-* currently a significant concern here
-* considering the relatively low frequency of
-* hrtimer callbacks (5ms period) and that
-* reads typically only happen in response to a
-* hrtimer event and likely complete before the
-* next callback.
-*
-* Note: This lock is not held *while* reading
-* and copying data to userspace so the value
-* of head observed in htrimer callbacks won't
-* represent any partial consumption of data.
-*/
-   spinlock_t ptr_lock;
-
-   /**
-* One 'aging' tail pointer and one 'aged'
-* tail pointer ready to used for reading.
-*
-* Initial values of 0x 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.
-*/
-   unsigned int aged_tail_idx;
-
-   /**
-* 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;
-
/**
 * Although we can always read back the head
 * pointer register, we prefer to avoid
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index 9ebf99f3d8d3..0569f0159d16 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -217,53 +217,8 @@
  * that the overflow cases are unlikely in normal operation.
  */
 #define OA_BUFFER_SIZE SZ_16M
-
 #define OA_TAKEN(tail, head)   ((tail - head) & (OA_BUFFER_SIZE - 1))
 
-/**
- * DOC: OA Tail Pointer Race
- *
- * There's a