Re: [Intel-gfx] [PATCH v7 01/63] drm/i915: Do not share hwsp across contexts any more, v7.

2021-01-28 Thread Chris Wilson
This series still willfully breaks ABI resulting in iris aborting among
others, introduces an unrecoverable dos, despite knowing how to avoid
both.It is unconscionable that this design was not revised to avoid
such breakage, after being rejected.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH v7 01/63] drm/i915: Do not share hwsp across contexts any more, v7.

2021-01-28 Thread Maarten Lankhorst
Instead of sharing pages with breadcrumbs, give each timeline a
single page. This allows unrelated timelines not to share locks
any more during command submission.

As an additional benefit, seqno wraparound no longer requires
i915_vma_pin, which means we no longer need to worry about a
potential -EDEADLK at a point where we are ready to submit.

Changes since v1:
- Fix erroneous i915_vma_acquire that should be a i915_vma_release (ickle).
- Extra check for completion in intel_read_hwsp().
Changes since v2:
- Fix inconsistent indent in hwsp_alloc() (kbuild)
- memset entire cacheline to 0.
Changes since v3:
- Do same in intel_timeline_reset_seqno(), and clflush for good measure.
Changes since v4:
- Use refcounting on timeline, instead of relying on i915_active.
- Fix waiting on kernel requests.
Changes since v5:
- Bump amount of slots to maximum (256), for best wraparounds.
- Add hwsp_offset to i915_request to fix potential wraparound hang.
- Ensure timeline wrap test works with the changes.
- Assign hwsp in intel_timeline_read_hwsp() within the rcu lock to
  fix a hang.
Changes since v6:
- Rename i915_request_active_offset to i915_request_active_seqno(),
  and elaborate the function. (tvrtko)

Signed-off-by: Maarten Lankhorst 
Reviewed-by: Thomas Hellström  #v1
Reported-by: kernel test robot 
---
 drivers/gpu/drm/i915/gt/gen2_engine_cs.c  |   2 +-
 drivers/gpu/drm/i915/gt/gen6_engine_cs.c  |   8 +-
 drivers/gpu/drm/i915/gt/gen8_engine_cs.c  |  13 +-
 drivers/gpu/drm/i915/gt/intel_engine_cs.c |   1 +
 drivers/gpu/drm/i915/gt/intel_gt_types.h  |   4 -
 drivers/gpu/drm/i915/gt/intel_timeline.c  | 422 --
 .../gpu/drm/i915/gt/intel_timeline_types.h|  17 +-
 drivers/gpu/drm/i915/gt/selftest_engine_cs.c  |   5 +-
 drivers/gpu/drm/i915/gt/selftest_timeline.c   |  83 ++--
 drivers/gpu/drm/i915/i915_request.c   |   4 -
 drivers/gpu/drm/i915/i915_request.h   |  31 +-
 11 files changed, 175 insertions(+), 415 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/gen2_engine_cs.c 
b/drivers/gpu/drm/i915/gt/gen2_engine_cs.c
index b491a64919c8..9646200d2792 100644
--- a/drivers/gpu/drm/i915/gt/gen2_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/gen2_engine_cs.c
@@ -143,7 +143,7 @@ static u32 *__gen2_emit_breadcrumb(struct i915_request *rq, 
u32 *cs,
   int flush, int post)
 {
GEM_BUG_ON(i915_request_active_timeline(rq)->hwsp_ggtt != 
rq->engine->status_page.vma);
-   
GEM_BUG_ON(offset_in_page(i915_request_active_timeline(rq)->hwsp_offset) != 
I915_GEM_HWS_SEQNO_ADDR);
+   GEM_BUG_ON(offset_in_page(rq->hwsp_seqno) != I915_GEM_HWS_SEQNO_ADDR);
 
*cs++ = MI_FLUSH;
 
diff --git a/drivers/gpu/drm/i915/gt/gen6_engine_cs.c 
b/drivers/gpu/drm/i915/gt/gen6_engine_cs.c
index ce38d1bcaba3..b388ceeeb1c9 100644
--- a/drivers/gpu/drm/i915/gt/gen6_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/gen6_engine_cs.c
@@ -161,7 +161,7 @@ u32 *gen6_emit_breadcrumb_rcs(struct i915_request *rq, u32 
*cs)
 PIPE_CONTROL_DC_FLUSH_ENABLE |
 PIPE_CONTROL_QW_WRITE |
 PIPE_CONTROL_CS_STALL);
-   *cs++ = i915_request_active_timeline(rq)->hwsp_offset |
+   *cs++ = i915_request_active_seqno(rq) |
PIPE_CONTROL_GLOBAL_GTT;
*cs++ = rq->fence.seqno;
 
@@ -359,7 +359,7 @@ u32 *gen7_emit_breadcrumb_rcs(struct i915_request *rq, u32 
*cs)
 PIPE_CONTROL_QW_WRITE |
 PIPE_CONTROL_GLOBAL_GTT_IVB |
 PIPE_CONTROL_CS_STALL);
-   *cs++ = i915_request_active_timeline(rq)->hwsp_offset;
+   *cs++ = i915_request_active_seqno(rq);
*cs++ = rq->fence.seqno;
 
*cs++ = MI_USER_INTERRUPT;
@@ -374,7 +374,7 @@ u32 *gen7_emit_breadcrumb_rcs(struct i915_request *rq, u32 
*cs)
 u32 *gen6_emit_breadcrumb_xcs(struct i915_request *rq, u32 *cs)
 {
GEM_BUG_ON(i915_request_active_timeline(rq)->hwsp_ggtt != 
rq->engine->status_page.vma);
-   
GEM_BUG_ON(offset_in_page(i915_request_active_timeline(rq)->hwsp_offset) != 
I915_GEM_HWS_SEQNO_ADDR);
+   GEM_BUG_ON(offset_in_page(rq->hwsp_seqno) != I915_GEM_HWS_SEQNO_ADDR);
 
*cs++ = MI_FLUSH_DW | MI_FLUSH_DW_OP_STOREDW | MI_FLUSH_DW_STORE_INDEX;
*cs++ = I915_GEM_HWS_SEQNO_ADDR | MI_FLUSH_DW_USE_GTT;
@@ -394,7 +394,7 @@ u32 *gen7_emit_breadcrumb_xcs(struct i915_request *rq, u32 
*cs)
int i;
 
GEM_BUG_ON(i915_request_active_timeline(rq)->hwsp_ggtt != 
rq->engine->status_page.vma);
-   
GEM_BUG_ON(offset_in_page(i915_request_active_timeline(rq)->hwsp_offset) != 
I915_GEM_HWS_SEQNO_ADDR);
+   GEM_BUG_ON(offset_in_page(rq->hwsp_seqno) != I915_GEM_HWS_SEQNO_ADDR);
 
*cs++ = MI_FLUSH_DW | MI_INVALIDATE_TLB |
MI_FLUSH_DW_OP_STOREDW | MI_FLUSH_DW_STORE_INDEX;
diff --git a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c 
b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
index cac80af7ad1c..6b9c34d3ac8d 100644
--- a/drivers/gpu/drm/i915/gt/gen8_