All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] drm/i915: Mark concurrent submissions with a weak-dependency
@ 2020-05-06 14:36 ` Chris Wilson
  0 siblings, 0 replies; 6+ messages in thread
From: Chris Wilson @ 2020-05-06 14:36 UTC (permalink / raw)
  To: intel-gfx; +Cc: Chris Wilson, Tvrtko Ursulin, stable

We recorded the dependencies for WAIT_FOR_SUBMIT in order that we could
correctly perform priority inheritance from the parallel branches to the
common trunk. However, for the purpose of timeslicing and reset
handling, the dependency is weak -- as we the pair of requests are
allowed to run in parallel and not in strict succession. So for example
we do need to suspend one if the other hangs.

The real significance though is that this allows us to rearrange
groups of WAIT_FOR_SUBMIT linked requests along the single engine, and
so can resolve user level inter-batch scheduling dependencies from user
semaphores.

Fixes: c81471f5e95c ("drm/i915: Copy across scheduler behaviour flags across submit fences")
Testcase: igt/gem_exec_fence/submit
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: <stable@vger.kernel.org> # v5.6+
---
 drivers/gpu/drm/i915/gt/intel_lrc.c         | 9 +++++++++
 drivers/gpu/drm/i915/i915_request.c         | 8 ++++++--
 drivers/gpu/drm/i915/i915_scheduler.c       | 6 +++---
 drivers/gpu/drm/i915/i915_scheduler.h       | 3 ++-
 drivers/gpu/drm/i915/i915_scheduler_types.h | 1 +
 5 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index dc3f2ee7136d..10109f661bcb 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -1880,6 +1880,9 @@ static void defer_request(struct i915_request *rq, struct list_head * const pl)
 			struct i915_request *w =
 				container_of(p->waiter, typeof(*w), sched);
 
+			if (p->flags & I915_DEPENDENCY_WEAK)
+				continue;
+
 			/* Leave semaphores spinning on the other engines */
 			if (w->engine != rq->engine)
 				continue;
@@ -2726,6 +2729,9 @@ static void __execlists_hold(struct i915_request *rq)
 			struct i915_request *w =
 				container_of(p->waiter, typeof(*w), sched);
 
+			if (p->flags & I915_DEPENDENCY_WEAK)
+				continue;
+
 			/* Leave semaphores spinning on the other engines */
 			if (w->engine != rq->engine)
 				continue;
@@ -2850,6 +2856,9 @@ static void __execlists_unhold(struct i915_request *rq)
 			struct i915_request *w =
 				container_of(p->waiter, typeof(*w), sched);
 
+			if (p->flags & I915_DEPENDENCY_WEAK)
+				continue;
+
 			/* Propagate any change in error status */
 			if (rq->fence.error)
 				i915_request_set_error_once(w, rq->fence.error);
diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
index 22635bbabf06..02a5644ae105 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -1038,7 +1038,9 @@ i915_request_await_request(struct i915_request *to, struct i915_request *from)
 		return 0;
 
 	if (to->engine->schedule) {
-		ret = i915_sched_node_add_dependency(&to->sched, &from->sched);
+		ret = i915_sched_node_add_dependency(&to->sched,
+						     &from->sched,
+						     I915_DEPENDENCY_EXTERNAL);
 		if (ret < 0)
 			return ret;
 	}
@@ -1200,7 +1202,9 @@ __i915_request_await_execution(struct i915_request *to,
 
 	/* Couple the dependency tree for PI on this exposed to->fence */
 	if (to->engine->schedule) {
-		err = i915_sched_node_add_dependency(&to->sched, &from->sched);
+		err = i915_sched_node_add_dependency(&to->sched,
+						     &from->sched,
+						     I915_DEPENDENCY_WEAK);
 		if (err < 0)
 			return err;
 	}
diff --git a/drivers/gpu/drm/i915/i915_scheduler.c b/drivers/gpu/drm/i915/i915_scheduler.c
index 37cfcf5b321b..6e2d4190099f 100644
--- a/drivers/gpu/drm/i915/i915_scheduler.c
+++ b/drivers/gpu/drm/i915/i915_scheduler.c
@@ -462,7 +462,8 @@ bool __i915_sched_node_add_dependency(struct i915_sched_node *node,
 }
 
 int i915_sched_node_add_dependency(struct i915_sched_node *node,
-				   struct i915_sched_node *signal)
+				   struct i915_sched_node *signal,
+				   unsigned long flags)
 {
 	struct i915_dependency *dep;
 
@@ -473,8 +474,7 @@ int i915_sched_node_add_dependency(struct i915_sched_node *node,
 	local_bh_disable();
 
 	if (!__i915_sched_node_add_dependency(node, signal, dep,
-					      I915_DEPENDENCY_EXTERNAL |
-					      I915_DEPENDENCY_ALLOC))
+					      flags | I915_DEPENDENCY_ALLOC))
 		i915_dependency_free(dep);
 
 	local_bh_enable(); /* kick submission tasklet */
diff --git a/drivers/gpu/drm/i915/i915_scheduler.h b/drivers/gpu/drm/i915/i915_scheduler.h
index d1dc4efef77b..6f0bf00fc569 100644
--- a/drivers/gpu/drm/i915/i915_scheduler.h
+++ b/drivers/gpu/drm/i915/i915_scheduler.h
@@ -34,7 +34,8 @@ bool __i915_sched_node_add_dependency(struct i915_sched_node *node,
 				      unsigned long flags);
 
 int i915_sched_node_add_dependency(struct i915_sched_node *node,
-				   struct i915_sched_node *signal);
+				   struct i915_sched_node *signal,
+				   unsigned long flags);
 
 void i915_sched_node_fini(struct i915_sched_node *node);
 
diff --git a/drivers/gpu/drm/i915/i915_scheduler_types.h b/drivers/gpu/drm/i915/i915_scheduler_types.h
index d18e70550054..7186875088a0 100644
--- a/drivers/gpu/drm/i915/i915_scheduler_types.h
+++ b/drivers/gpu/drm/i915/i915_scheduler_types.h
@@ -78,6 +78,7 @@ struct i915_dependency {
 	unsigned long flags;
 #define I915_DEPENDENCY_ALLOC		BIT(0)
 #define I915_DEPENDENCY_EXTERNAL	BIT(1)
+#define I915_DEPENDENCY_WEAK		BIT(2)
 };
 
 #endif /* _I915_SCHEDULER_TYPES_H_ */
-- 
2.20.1


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

* [Intel-gfx] [PATCH 1/2] drm/i915: Mark concurrent submissions with a weak-dependency
@ 2020-05-06 14:36 ` Chris Wilson
  0 siblings, 0 replies; 6+ messages in thread
From: Chris Wilson @ 2020-05-06 14:36 UTC (permalink / raw)
  To: intel-gfx; +Cc: stable, Chris Wilson

We recorded the dependencies for WAIT_FOR_SUBMIT in order that we could
correctly perform priority inheritance from the parallel branches to the
common trunk. However, for the purpose of timeslicing and reset
handling, the dependency is weak -- as we the pair of requests are
allowed to run in parallel and not in strict succession. So for example
we do need to suspend one if the other hangs.

The real significance though is that this allows us to rearrange
groups of WAIT_FOR_SUBMIT linked requests along the single engine, and
so can resolve user level inter-batch scheduling dependencies from user
semaphores.

Fixes: c81471f5e95c ("drm/i915: Copy across scheduler behaviour flags across submit fences")
Testcase: igt/gem_exec_fence/submit
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: <stable@vger.kernel.org> # v5.6+
---
 drivers/gpu/drm/i915/gt/intel_lrc.c         | 9 +++++++++
 drivers/gpu/drm/i915/i915_request.c         | 8 ++++++--
 drivers/gpu/drm/i915/i915_scheduler.c       | 6 +++---
 drivers/gpu/drm/i915/i915_scheduler.h       | 3 ++-
 drivers/gpu/drm/i915/i915_scheduler_types.h | 1 +
 5 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index dc3f2ee7136d..10109f661bcb 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -1880,6 +1880,9 @@ static void defer_request(struct i915_request *rq, struct list_head * const pl)
 			struct i915_request *w =
 				container_of(p->waiter, typeof(*w), sched);
 
+			if (p->flags & I915_DEPENDENCY_WEAK)
+				continue;
+
 			/* Leave semaphores spinning on the other engines */
 			if (w->engine != rq->engine)
 				continue;
@@ -2726,6 +2729,9 @@ static void __execlists_hold(struct i915_request *rq)
 			struct i915_request *w =
 				container_of(p->waiter, typeof(*w), sched);
 
+			if (p->flags & I915_DEPENDENCY_WEAK)
+				continue;
+
 			/* Leave semaphores spinning on the other engines */
 			if (w->engine != rq->engine)
 				continue;
@@ -2850,6 +2856,9 @@ static void __execlists_unhold(struct i915_request *rq)
 			struct i915_request *w =
 				container_of(p->waiter, typeof(*w), sched);
 
+			if (p->flags & I915_DEPENDENCY_WEAK)
+				continue;
+
 			/* Propagate any change in error status */
 			if (rq->fence.error)
 				i915_request_set_error_once(w, rq->fence.error);
diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
index 22635bbabf06..02a5644ae105 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -1038,7 +1038,9 @@ i915_request_await_request(struct i915_request *to, struct i915_request *from)
 		return 0;
 
 	if (to->engine->schedule) {
-		ret = i915_sched_node_add_dependency(&to->sched, &from->sched);
+		ret = i915_sched_node_add_dependency(&to->sched,
+						     &from->sched,
+						     I915_DEPENDENCY_EXTERNAL);
 		if (ret < 0)
 			return ret;
 	}
@@ -1200,7 +1202,9 @@ __i915_request_await_execution(struct i915_request *to,
 
 	/* Couple the dependency tree for PI on this exposed to->fence */
 	if (to->engine->schedule) {
-		err = i915_sched_node_add_dependency(&to->sched, &from->sched);
+		err = i915_sched_node_add_dependency(&to->sched,
+						     &from->sched,
+						     I915_DEPENDENCY_WEAK);
 		if (err < 0)
 			return err;
 	}
diff --git a/drivers/gpu/drm/i915/i915_scheduler.c b/drivers/gpu/drm/i915/i915_scheduler.c
index 37cfcf5b321b..6e2d4190099f 100644
--- a/drivers/gpu/drm/i915/i915_scheduler.c
+++ b/drivers/gpu/drm/i915/i915_scheduler.c
@@ -462,7 +462,8 @@ bool __i915_sched_node_add_dependency(struct i915_sched_node *node,
 }
 
 int i915_sched_node_add_dependency(struct i915_sched_node *node,
-				   struct i915_sched_node *signal)
+				   struct i915_sched_node *signal,
+				   unsigned long flags)
 {
 	struct i915_dependency *dep;
 
@@ -473,8 +474,7 @@ int i915_sched_node_add_dependency(struct i915_sched_node *node,
 	local_bh_disable();
 
 	if (!__i915_sched_node_add_dependency(node, signal, dep,
-					      I915_DEPENDENCY_EXTERNAL |
-					      I915_DEPENDENCY_ALLOC))
+					      flags | I915_DEPENDENCY_ALLOC))
 		i915_dependency_free(dep);
 
 	local_bh_enable(); /* kick submission tasklet */
diff --git a/drivers/gpu/drm/i915/i915_scheduler.h b/drivers/gpu/drm/i915/i915_scheduler.h
index d1dc4efef77b..6f0bf00fc569 100644
--- a/drivers/gpu/drm/i915/i915_scheduler.h
+++ b/drivers/gpu/drm/i915/i915_scheduler.h
@@ -34,7 +34,8 @@ bool __i915_sched_node_add_dependency(struct i915_sched_node *node,
 				      unsigned long flags);
 
 int i915_sched_node_add_dependency(struct i915_sched_node *node,
-				   struct i915_sched_node *signal);
+				   struct i915_sched_node *signal,
+				   unsigned long flags);
 
 void i915_sched_node_fini(struct i915_sched_node *node);
 
diff --git a/drivers/gpu/drm/i915/i915_scheduler_types.h b/drivers/gpu/drm/i915/i915_scheduler_types.h
index d18e70550054..7186875088a0 100644
--- a/drivers/gpu/drm/i915/i915_scheduler_types.h
+++ b/drivers/gpu/drm/i915/i915_scheduler_types.h
@@ -78,6 +78,7 @@ struct i915_dependency {
 	unsigned long flags;
 #define I915_DEPENDENCY_ALLOC		BIT(0)
 #define I915_DEPENDENCY_EXTERNAL	BIT(1)
+#define I915_DEPENDENCY_WEAK		BIT(2)
 };
 
 #endif /* _I915_SCHEDULER_TYPES_H_ */
-- 
2.20.1

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

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

* [Intel-gfx] [PATCH 2/2] drm/i915/gt: Suppress internal I915_PRIORITY_WAIT for timeslicing
  2020-05-06 14:36 ` [Intel-gfx] " Chris Wilson
  (?)
@ 2020-05-06 14:36 ` Chris Wilson
  2020-05-06 14:46   ` Chris Wilson
  2020-05-07  7:07   ` Chris Wilson
  -1 siblings, 2 replies; 6+ messages in thread
From: Chris Wilson @ 2020-05-06 14:36 UTC (permalink / raw)
  To: intel-gfx; +Cc: Chris Wilson

Make sure we ignore the I915_PRIORITY_WAIT hint when looking at
timeslicing, as we do not treat it as a preemption request but as a soft
ordering hint. If we apply the hint, then when we recompute the ordering
after unwinding for the timeslice, we will often leave the order
unchanged due to the soft-hint. However, if we apply it to all those we
unwind, then the two equivalent levels may be reordered, and since the
dependencies will be replayed in order, we will not change the order of
dependencies.

There is a small issue with the lack of cross-engine priority bumping on
unwind, leaving the total graph slightly unordered; but that will not
result in any misordering of rendering on remote machines as any
signalers will also be live. Though there may be a danger that this will
upset our sanitychecks.

Why keep the I915_PRIORITY_WAIT soft-hint, I hear Tvrtko ask? Despite
the many hairy tricks we play to have the hint and then ignore it, I
still like the concept of codel and the promise that it gives for low
latency of independent queues!

Testcase: igt/gem_exec_fence/submit
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_lrc.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index 10109f661bcb..3606a7946707 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -414,6 +414,12 @@ static inline int rq_prio(const struct i915_request *rq)
 	return READ_ONCE(rq->sched.attr.priority);
 }
 
+static int __effective_prio(int prio)
+{
+	BUILD_BUG_ON(__NO_PREEMPTION & ~I915_PRIORITY_MASK); /* only internal */
+	return prio | __NO_PREEMPTION;
+}
+
 static int effective_prio(const struct i915_request *rq)
 {
 	int prio = rq_prio(rq);
@@ -439,8 +445,7 @@ static int effective_prio(const struct i915_request *rq)
 		prio |= I915_PRIORITY_NOSEMAPHORE;
 
 	/* Restrict mere WAIT boosts from triggering preemption */
-	BUILD_BUG_ON(__NO_PREEMPTION & ~I915_PRIORITY_MASK); /* only internal */
-	return prio | __NO_PREEMPTION;
+	return __effective_prio(prio);
 }
 
 static int queue_prio(const struct intel_engine_execlists *execlists)
@@ -1126,6 +1131,7 @@ __unwind_incomplete_requests(struct intel_engine_cs *engine)
 			continue; /* XXX */
 
 		__i915_request_unsubmit(rq);
+		rq->sched.attr.priority |= __NO_PREEMPTION;
 
 		/*
 		 * Push the request back into the queue for later resubmission.
@@ -1930,7 +1936,7 @@ need_timeslice(const struct intel_engine_cs *engine,
 	if (!list_is_last(&rq->sched.link, &engine->active.requests))
 		hint = max(hint, rq_prio(list_next_entry(rq, sched.link)));
 
-	return hint >= effective_prio(rq);
+	return __effective_prio(hint) >= effective_prio(rq);
 }
 
 static bool
@@ -1965,7 +1971,7 @@ switch_prio(struct intel_engine_cs *engine, const struct i915_request *rq)
 	if (list_is_last(&rq->sched.link, &engine->active.requests))
 		return INT_MIN;
 
-	return rq_prio(list_next_entry(rq, sched.link));
+	return __effective_prio(rq_prio(list_next_entry(rq, sched.link)));
 }
 
 static inline unsigned long
-- 
2.20.1

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

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

* Re: [Intel-gfx] [PATCH 2/2] drm/i915/gt: Suppress internal I915_PRIORITY_WAIT for timeslicing
  2020-05-06 14:36 ` [Intel-gfx] [PATCH 2/2] drm/i915/gt: Suppress internal I915_PRIORITY_WAIT for timeslicing Chris Wilson
@ 2020-05-06 14:46   ` Chris Wilson
  2020-05-07  7:07   ` Chris Wilson
  1 sibling, 0 replies; 6+ messages in thread
From: Chris Wilson @ 2020-05-06 14:46 UTC (permalink / raw)
  To: intel-gfx

Quoting Chris Wilson (2020-05-06 15:36:16)
> Make sure we ignore the I915_PRIORITY_WAIT hint when looking at
> timeslicing, as we do not treat it as a preemption request but as a soft
> ordering hint. If we apply the hint, then when we recompute the ordering
> after unwinding for the timeslice, we will often leave the order
> unchanged due to the soft-hint. However, if we apply it to all those we
> unwind, then the two equivalent levels may be reordered, and since the
> dependencies will be replayed in order, we will not change the order of
> dependencies.
> 
> There is a small issue with the lack of cross-engine priority bumping on
> unwind, leaving the total graph slightly unordered; but that will not
> result in any misordering of rendering on remote machines as any
> signalers will also be live. Though there may be a danger that this will
> upset our sanitychecks.
> 
> Why keep the I915_PRIORITY_WAIT soft-hint, I hear Tvrtko ask? Despite
> the many hairy tricks we play to have the hint and then ignore it, I
> still like the concept of codel and the promise that it gives for low
> latency of independent queues!

Tvrtko is warned not to ask when we replace strict priorities with
isoschronous and psuedo-deadline scheduling.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* [Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [1/2] drm/i915: Mark concurrent submissions with a weak-dependency
  2020-05-06 14:36 ` [Intel-gfx] " Chris Wilson
  (?)
  (?)
@ 2020-05-06 17:09 ` Patchwork
  -1 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2020-05-06 17:09 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/2] drm/i915: Mark concurrent submissions with a weak-dependency
URL   : https://patchwork.freedesktop.org/series/76999/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_8434 -> Patchwork_17590
====================================================

Summary
-------

  **FAILURE**

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

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

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@i915_selftest@live@execlists:
    - fi-skl-lmem:        [PASS][1] -> [INCOMPLETE][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8434/fi-skl-lmem/igt@i915_selftest@live@execlists.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17590/fi-skl-lmem/igt@i915_selftest@live@execlists.html

  * igt@i915_selftest@live@gt_lrc:
    - fi-skl-6700k2:      [PASS][3] -> [INCOMPLETE][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8434/fi-skl-6700k2/igt@i915_selftest@live@gt_lrc.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17590/fi-skl-6700k2/igt@i915_selftest@live@gt_lrc.html

  


Participating hosts (50 -> 43)
------------------------------

  Additional (1): fi-kbl-7560u 
  Missing    (8): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-bwr-2160 fi-ctg-p8600 fi-byt-clapper fi-bdw-samus 


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

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_8434 -> Patchwork_17590

  CI-20190529: 20190529
  CI_DRM_8434: 2951bac393beb4f095468de8b7cc53c8e3a092c2 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5635: e83abfca61d407d12eee4d25bb0e8686337a7791 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_17590: 2833b6c29cc99422bd7fee78c8c51237c86a7787 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

2833b6c29cc9 drm/i915/gt: Suppress internal I915_PRIORITY_WAIT for timeslicing
e39457734fee drm/i915: Mark concurrent submissions with a weak-dependency

== Logs ==

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

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

* Re: [Intel-gfx] [PATCH 2/2] drm/i915/gt: Suppress internal I915_PRIORITY_WAIT for timeslicing
  2020-05-06 14:36 ` [Intel-gfx] [PATCH 2/2] drm/i915/gt: Suppress internal I915_PRIORITY_WAIT for timeslicing Chris Wilson
  2020-05-06 14:46   ` Chris Wilson
@ 2020-05-07  7:07   ` Chris Wilson
  1 sibling, 0 replies; 6+ messages in thread
From: Chris Wilson @ 2020-05-07  7:07 UTC (permalink / raw)
  To: intel-gfx

Quoting Chris Wilson (2020-05-06 15:36:16)
> Make sure we ignore the I915_PRIORITY_WAIT hint when looking at
> timeslicing, as we do not treat it as a preemption request but as a soft
> ordering hint. If we apply the hint, then when we recompute the ordering
> after unwinding for the timeslice, we will often leave the order
> unchanged due to the soft-hint. However, if we apply it to all those we
> unwind, then the two equivalent levels may be reordered, and since the
> dependencies will be replayed in order, we will not change the order of
> dependencies.
> 
> There is a small issue with the lack of cross-engine priority bumping on
> unwind, leaving the total graph slightly unordered; but that will not
> result in any misordering of rendering on remote machines as any
> signalers will also be live. Though there may be a danger that this will
> upset our sanitychecks.
> 
> Why keep the I915_PRIORITY_WAIT soft-hint, I hear Tvrtko ask? Despite
> the many hairy tricks we play to have the hint and then ignore it, I
> still like the concept of codel and the promise that it gives for low
> latency of independent queues!
> 
> Testcase: igt/gem_exec_fence/submit
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_lrc.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> index 10109f661bcb..3606a7946707 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> @@ -414,6 +414,12 @@ static inline int rq_prio(const struct i915_request *rq)
>         return READ_ONCE(rq->sched.attr.priority);
>  }
>  
> +static int __effective_prio(int prio)
> +{
> +       BUILD_BUG_ON(__NO_PREEMPTION & ~I915_PRIORITY_MASK); /* only internal */
> +       return prio | __NO_PREEMPTION;
> +}
> +
>  static int effective_prio(const struct i915_request *rq)
>  {
>         int prio = rq_prio(rq);
> @@ -439,8 +445,7 @@ static int effective_prio(const struct i915_request *rq)
>                 prio |= I915_PRIORITY_NOSEMAPHORE;
>  
>         /* Restrict mere WAIT boosts from triggering preemption */
> -       BUILD_BUG_ON(__NO_PREEMPTION & ~I915_PRIORITY_MASK); /* only internal */
> -       return prio | __NO_PREEMPTION;
> +       return __effective_prio(prio);
>  }
>  
>  static int queue_prio(const struct intel_engine_execlists *execlists)
> @@ -1126,6 +1131,7 @@ __unwind_incomplete_requests(struct intel_engine_cs *engine)
>                         continue; /* XXX */
>  
>                 __i915_request_unsubmit(rq);
> +               rq->sched.attr.priority |= __NO_PREEMPTION;

And this doesn't work as it gives special consideration to anything that
made it to the GPU, stays on the GPU. That is we then cannot select
something from the top of the queue that deserves to be run.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

end of thread, other threads:[~2020-05-07  7:24 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-06 14:36 [PATCH 1/2] drm/i915: Mark concurrent submissions with a weak-dependency Chris Wilson
2020-05-06 14:36 ` [Intel-gfx] " Chris Wilson
2020-05-06 14:36 ` [Intel-gfx] [PATCH 2/2] drm/i915/gt: Suppress internal I915_PRIORITY_WAIT for timeslicing Chris Wilson
2020-05-06 14:46   ` Chris Wilson
2020-05-07  7:07   ` Chris Wilson
2020-05-06 17:09 ` [Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [1/2] drm/i915: Mark concurrent submissions with a weak-dependency Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.