* [PATCH 01/14] drm/i915/hangcheck: Track context changes
@ 2019-05-01 11:45 Chris Wilson
2019-05-01 11:45 ` [PATCH 02/14] drm/i915: Include fence signaled bit in print_request() Chris Wilson
` (23 more replies)
0 siblings, 24 replies; 52+ messages in thread
From: Chris Wilson @ 2019-05-01 11:45 UTC (permalink / raw)
To: intel-gfx
Given sufficient preemption, we may see a busy system that doesn't
advance seqno while performing work across multiple contexts, and given
sufficient pathology not even notice a change in ACTHD. What does change
between the preempting contexts is their RING, so take note of that and
treat a change in the ring address as being an indication of forward
progress.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
drivers/gpu/drm/i915/gt/intel_engine_types.h | 1 +
drivers/gpu/drm/i915/gt/intel_hangcheck.c | 12 +++++++++---
2 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
index 9d64e33f8427..c0ab11b12e14 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
@@ -53,6 +53,7 @@ struct intel_instdone {
struct intel_engine_hangcheck {
u64 acthd;
+ u32 last_ring;
u32 last_seqno;
u32 next_seqno;
unsigned long action_timestamp;
diff --git a/drivers/gpu/drm/i915/gt/intel_hangcheck.c b/drivers/gpu/drm/i915/gt/intel_hangcheck.c
index e5eaa06fe74d..721ab74a382f 100644
--- a/drivers/gpu/drm/i915/gt/intel_hangcheck.c
+++ b/drivers/gpu/drm/i915/gt/intel_hangcheck.c
@@ -27,6 +27,7 @@
struct hangcheck {
u64 acthd;
+ u32 ring;
u32 seqno;
enum intel_engine_hangcheck_action action;
unsigned long action_timestamp;
@@ -134,6 +135,7 @@ static void hangcheck_load_sample(struct intel_engine_cs *engine,
{
hc->acthd = intel_engine_get_active_head(engine);
hc->seqno = intel_engine_get_hangcheck_seqno(engine);
+ hc->ring = ENGINE_READ(engine, RING_START);
}
static void hangcheck_store_sample(struct intel_engine_cs *engine,
@@ -141,18 +143,22 @@ static void hangcheck_store_sample(struct intel_engine_cs *engine,
{
engine->hangcheck.acthd = hc->acthd;
engine->hangcheck.last_seqno = hc->seqno;
+ engine->hangcheck.last_ring = hc->ring;
}
static enum intel_engine_hangcheck_action
hangcheck_get_action(struct intel_engine_cs *engine,
const struct hangcheck *hc)
{
- if (engine->hangcheck.last_seqno != hc->seqno)
- return ENGINE_ACTIVE_SEQNO;
-
if (intel_engine_is_idle(engine))
return ENGINE_IDLE;
+ if (engine->hangcheck.last_ring != hc->ring)
+ return ENGINE_ACTIVE_SEQNO;
+
+ if (engine->hangcheck.last_seqno != hc->seqno)
+ return ENGINE_ACTIVE_SEQNO;
+
return engine_stuck(engine, hc->acthd);
}
--
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] 52+ messages in thread
* [PATCH 02/14] drm/i915: Include fence signaled bit in print_request()
2019-05-01 11:45 [PATCH 01/14] drm/i915/hangcheck: Track context changes Chris Wilson
@ 2019-05-01 11:45 ` Chris Wilson
2019-05-02 13:43 ` Tvrtko Ursulin
2019-05-01 11:45 ` [PATCH 03/14] drm/i915/execlists: Flush the tasklet on parking Chris Wilson
` (22 subsequent siblings)
23 siblings, 1 reply; 52+ messages in thread
From: Chris Wilson @ 2019-05-01 11:45 UTC (permalink / raw)
To: intel-gfx
Show the fence flags view of request completion in addition to the
normal hwsp check and whether signaling is enabled.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/gt/intel_engine_cs.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index 6e40f8ea9a6a..f178f1268f4e 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -1229,8 +1229,11 @@ static void print_request(struct drm_printer *m,
i915_request_completed(rq) ? "!" :
i915_request_started(rq) ? "*" :
"",
+ test_bit(DMA_FENCE_FLAG_SIGNALED_BIT,
+ &rq->fence.flags) ? "+" :
test_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT,
- &rq->fence.flags) ? "+" : "",
+ &rq->fence.flags) ? "-" :
+ "",
buf,
jiffies_to_msecs(jiffies - rq->emitted_jiffies),
name);
--
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] 52+ messages in thread
* [PATCH 03/14] drm/i915/execlists: Flush the tasklet on parking
2019-05-01 11:45 [PATCH 01/14] drm/i915/hangcheck: Track context changes Chris Wilson
2019-05-01 11:45 ` [PATCH 02/14] drm/i915: Include fence signaled bit in print_request() Chris Wilson
@ 2019-05-01 11:45 ` Chris Wilson
2019-05-02 13:48 ` Tvrtko Ursulin
2019-05-01 11:45 ` [PATCH 04/14] drm/i915: Leave engine parking to the engines Chris Wilson
` (21 subsequent siblings)
23 siblings, 1 reply; 52+ messages in thread
From: Chris Wilson @ 2019-05-01 11:45 UTC (permalink / raw)
To: intel-gfx
Tidy up the cleanup sequence by always ensure that the tasklet is
flushed on parking (before we cleanup). The parking provides a
convenient point to ensure that the backend is truly idle.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/gt/intel_lrc.c | 7 ++++++-
drivers/gpu/drm/i915/intel_guc_submission.c | 1 +
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index 851e62ddcb87..7be54b868d8e 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -2331,6 +2331,11 @@ static int gen8_init_rcs_context(struct i915_request *rq)
return i915_gem_render_state_emit(rq);
}
+static void execlists_park(struct intel_engine_cs *engine)
+{
+ tasklet_kill(&engine->execlists.tasklet);
+}
+
void intel_execlists_set_default_submission(struct intel_engine_cs *engine)
{
engine->submit_request = execlists_submit_request;
@@ -2342,7 +2347,7 @@ void intel_execlists_set_default_submission(struct intel_engine_cs *engine)
engine->reset.reset = execlists_reset;
engine->reset.finish = execlists_reset_finish;
- engine->park = NULL;
+ engine->park = execlists_park;
engine->unpark = NULL;
engine->flags |= I915_ENGINE_SUPPORTS_STATS;
diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c
index 4c814344809c..ed94001028f2 100644
--- a/drivers/gpu/drm/i915/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/intel_guc_submission.c
@@ -1363,6 +1363,7 @@ static void guc_interrupts_release(struct drm_i915_private *dev_priv)
static void guc_submission_park(struct intel_engine_cs *engine)
{
+ tasklet_kill(&engine->execlists.tasklet);
intel_engine_unpin_breadcrumbs_irq(engine);
engine->flags &= ~I915_ENGINE_NEEDS_BREADCRUMB_TASKLET;
}
--
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] 52+ messages in thread
* [PATCH 04/14] drm/i915: Leave engine parking to the engines
2019-05-01 11:45 [PATCH 01/14] drm/i915/hangcheck: Track context changes Chris Wilson
2019-05-01 11:45 ` [PATCH 02/14] drm/i915: Include fence signaled bit in print_request() Chris Wilson
2019-05-01 11:45 ` [PATCH 03/14] drm/i915/execlists: Flush the tasklet on parking Chris Wilson
@ 2019-05-01 11:45 ` Chris Wilson
2019-05-02 14:18 ` Tvrtko Ursulin
2019-05-01 11:45 ` [PATCH 05/14] drm/i915: Remove delay for idle_work Chris Wilson
` (20 subsequent siblings)
23 siblings, 1 reply; 52+ messages in thread
From: Chris Wilson @ 2019-05-01 11:45 UTC (permalink / raw)
To: intel-gfx
Drop the check in GEM parking that the engines were already parked. The
intention here was that before we dropped the GT wakeref, we were sure
that no more interrupts could be raised -- however, we have already
dropped the wakeref by this point and the warning is no longer valid.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/i915_gem_pm.c | 18 +-----------------
1 file changed, 1 insertion(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_pm.c b/drivers/gpu/drm/i915/i915_gem_pm.c
index 3b6e8d5be8e1..49b0ce594f20 100644
--- a/drivers/gpu/drm/i915/i915_gem_pm.c
+++ b/drivers/gpu/drm/i915/i915_gem_pm.c
@@ -17,24 +17,8 @@ static void i915_gem_park(struct drm_i915_private *i915)
lockdep_assert_held(&i915->drm.struct_mutex);
- for_each_engine(engine, i915, id) {
- /*
- * We are committed now to parking the engines, make sure there
- * will be no more interrupts arriving later and the engines
- * are truly idle.
- */
- if (wait_for(intel_engine_is_idle(engine), 10)) {
- struct drm_printer p = drm_debug_printer(__func__);
-
- dev_err(i915->drm.dev,
- "%s is not idle before parking\n",
- engine->name);
- intel_engine_dump(engine, &p, NULL);
- }
- tasklet_kill(&engine->execlists.tasklet);
-
+ for_each_engine(engine, i915, id)
i915_gem_batch_pool_fini(&engine->batch_pool);
- }
i915_timelines_park(i915);
i915_vma_parked(i915);
--
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] 52+ messages in thread
* [PATCH 05/14] drm/i915: Remove delay for idle_work
2019-05-01 11:45 [PATCH 01/14] drm/i915/hangcheck: Track context changes Chris Wilson
` (2 preceding siblings ...)
2019-05-01 11:45 ` [PATCH 04/14] drm/i915: Leave engine parking to the engines Chris Wilson
@ 2019-05-01 11:45 ` Chris Wilson
2019-05-02 13:19 ` Tvrtko Ursulin
2019-05-01 11:45 ` [PATCH 06/14] drm/i915: Cancel retire_worker on parking Chris Wilson
` (19 subsequent siblings)
23 siblings, 1 reply; 52+ messages in thread
From: Chris Wilson @ 2019-05-01 11:45 UTC (permalink / raw)
To: intel-gfx
The original intent for the delay before running the idle_work was to
provide a hysteresis to avoid ping-ponging the device runtime-pm. Since
then we have also pulled in some memory management and general device
management for parking. But with the inversion of the wakeref handling,
GEM is no longer responsible for the wakeref and by the time we call the
idle_work, the device is asleep. It seems appropriate now to drop the
delay and just run the worker immediately to flush the cached GEM state
before sleeping.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/i915_debugfs.c | 2 +-
drivers/gpu/drm/i915/i915_drv.h | 2 +-
drivers/gpu/drm/i915/i915_gem_pm.c | 21 +++++++------------
.../gpu/drm/i915/selftests/i915_gem_object.c | 2 +-
.../gpu/drm/i915/selftests/mock_gem_device.c | 4 ++--
5 files changed, 12 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 0e4dffcd4da4..7e8898d0b78b 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -3935,8 +3935,8 @@ i915_drop_caches_set(void *data, u64 val)
if (val & DROP_IDLE) {
do {
flush_delayed_work(&i915->gem.retire_work);
- drain_delayed_work(&i915->gem.idle_work);
} while (READ_ONCE(i915->gt.awake));
+ flush_work(&i915->gem.idle_work);
}
if (val & DROP_FREED)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 13270e19eb87..cbf4a7d8bdae 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2035,7 +2035,7 @@ struct drm_i915_private {
* arrive within a small period of time, we fire
* off the idle_work.
*/
- struct delayed_work idle_work;
+ struct work_struct idle_work;
} gem;
/* For i945gm vblank irq vs. C3 workaround */
diff --git a/drivers/gpu/drm/i915/i915_gem_pm.c b/drivers/gpu/drm/i915/i915_gem_pm.c
index 49b0ce594f20..ae91ad7cb31e 100644
--- a/drivers/gpu/drm/i915/i915_gem_pm.c
+++ b/drivers/gpu/drm/i915/i915_gem_pm.c
@@ -29,12 +29,12 @@ static void i915_gem_park(struct drm_i915_private *i915)
static void idle_work_handler(struct work_struct *work)
{
struct drm_i915_private *i915 =
- container_of(work, typeof(*i915), gem.idle_work.work);
+ container_of(work, typeof(*i915), gem.idle_work);
mutex_lock(&i915->drm.struct_mutex);
intel_wakeref_lock(&i915->gt.wakeref);
- if (!intel_wakeref_active(&i915->gt.wakeref))
+ if (!intel_wakeref_active(&i915->gt.wakeref) && !work_pending(work))
i915_gem_park(i915);
intel_wakeref_unlock(&i915->gt.wakeref);
@@ -74,9 +74,7 @@ static int pm_notifier(struct notifier_block *nb,
break;
case INTEL_GT_PARK:
- mod_delayed_work(i915->wq,
- &i915->gem.idle_work,
- msecs_to_jiffies(100));
+ queue_work(i915->wq, &i915->gem.idle_work);
break;
}
@@ -142,16 +140,11 @@ void i915_gem_suspend(struct drm_i915_private *i915)
* Assert that we successfully flushed all the work and
* reset the GPU back to its idle, low power state.
*/
- GEM_BUG_ON(i915->gt.awake);
- cancel_delayed_work_sync(&i915->gpu_error.hangcheck_work);
-
drain_delayed_work(&i915->gem.retire_work);
+ GEM_BUG_ON(i915->gt.awake);
+ flush_work(&i915->gem.idle_work);
- /*
- * As the idle_work is rearming if it detects a race, play safe and
- * repeat the flush until it is definitely idle.
- */
- drain_delayed_work(&i915->gem.idle_work);
+ cancel_delayed_work_sync(&i915->gpu_error.hangcheck_work);
i915_gem_drain_freed_objects(i915);
@@ -242,7 +235,7 @@ void i915_gem_resume(struct drm_i915_private *i915)
void i915_gem_init__pm(struct drm_i915_private *i915)
{
- INIT_DELAYED_WORK(&i915->gem.idle_work, idle_work_handler);
+ INIT_WORK(&i915->gem.idle_work, idle_work_handler);
INIT_DELAYED_WORK(&i915->gem.retire_work, retire_work_handler);
i915->gem.pm_notifier.notifier_call = pm_notifier;
diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_object.c b/drivers/gpu/drm/i915/selftests/i915_gem_object.c
index 088b2aa05dcd..b926d1cd165d 100644
--- a/drivers/gpu/drm/i915/selftests/i915_gem_object.c
+++ b/drivers/gpu/drm/i915/selftests/i915_gem_object.c
@@ -509,7 +509,7 @@ static void disable_retire_worker(struct drm_i915_private *i915)
intel_gt_pm_get(i915);
cancel_delayed_work_sync(&i915->gem.retire_work);
- cancel_delayed_work_sync(&i915->gem.idle_work);
+ flush_work(&i915->gem.idle_work);
}
static void restore_retire_worker(struct drm_i915_private *i915)
diff --git a/drivers/gpu/drm/i915/selftests/mock_gem_device.c b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
index e4033d0576c4..d919f512042c 100644
--- a/drivers/gpu/drm/i915/selftests/mock_gem_device.c
+++ b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
@@ -59,7 +59,7 @@ static void mock_device_release(struct drm_device *dev)
mutex_unlock(&i915->drm.struct_mutex);
drain_delayed_work(&i915->gem.retire_work);
- drain_delayed_work(&i915->gem.idle_work);
+ flush_work(&i915->gem.idle_work);
i915_gem_drain_workqueue(i915);
mutex_lock(&i915->drm.struct_mutex);
@@ -195,7 +195,7 @@ struct drm_i915_private *mock_gem_device(void)
mock_init_contexts(i915);
INIT_DELAYED_WORK(&i915->gem.retire_work, mock_retire_work_handler);
- INIT_DELAYED_WORK(&i915->gem.idle_work, mock_idle_work_handler);
+ INIT_WORK(&i915->gem.idle_work, mock_idle_work_handler);
i915->gt.awake = true;
--
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] 52+ messages in thread
* [PATCH 06/14] drm/i915: Cancel retire_worker on parking
2019-05-01 11:45 [PATCH 01/14] drm/i915/hangcheck: Track context changes Chris Wilson
` (3 preceding siblings ...)
2019-05-01 11:45 ` [PATCH 05/14] drm/i915: Remove delay for idle_work Chris Wilson
@ 2019-05-01 11:45 ` Chris Wilson
2019-05-02 13:29 ` Tvrtko Ursulin
2019-05-02 14:29 ` [PATCH v2] " Chris Wilson
2019-05-01 11:45 ` [PATCH 07/14] drm/i915: Stop spinning for DROP_IDLE (debugfs/i915_drop_caches) Chris Wilson
` (18 subsequent siblings)
23 siblings, 2 replies; 52+ messages in thread
From: Chris Wilson @ 2019-05-01 11:45 UTC (permalink / raw)
To: intel-gfx
Replace the racy continuation check within retire_work with a definite
kill-switch on idling. The race was being exposed by gem_concurrent_blit
where the retire_worker would be terminated too early leaving us
spinning in debugfs/i915_drop_caches with nothing flushing the
retirement queue.
Although that the igt is trying to idle from one child while submitting
from another may be a contributing factor as to why it runs so slowly...
Testcase: igt/gem_concurrent_blit
Fixes: 79ffac8599c4 ("drm/i915: Invert the GEM wakeref hierarchy")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/i915_gem_pm.c | 18 ++++++++++++------
.../gpu/drm/i915/selftests/mock_gem_device.c | 1 -
2 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_pm.c b/drivers/gpu/drm/i915/i915_gem_pm.c
index ae91ad7cb31e..b239b55f84cd 100644
--- a/drivers/gpu/drm/i915/i915_gem_pm.c
+++ b/drivers/gpu/drm/i915/i915_gem_pm.c
@@ -30,15 +30,23 @@ static void idle_work_handler(struct work_struct *work)
{
struct drm_i915_private *i915 =
container_of(work, typeof(*i915), gem.idle_work);
+ bool restart = true;
+ cancel_delayed_work_sync(&i915->gem.retire_work);
mutex_lock(&i915->drm.struct_mutex);
intel_wakeref_lock(&i915->gt.wakeref);
- if (!intel_wakeref_active(&i915->gt.wakeref) && !work_pending(work))
+ if (!intel_wakeref_active(&i915->gt.wakeref) && !work_pending(work)) {
i915_gem_park(i915);
+ restart = false;
+ }
intel_wakeref_unlock(&i915->gt.wakeref);
mutex_unlock(&i915->drm.struct_mutex);
+ if (restart)
+ queue_delayed_work(i915->wq,
+ &i915->gem.retire_work,
+ round_jiffies_up_relative(HZ));
}
static void retire_work_handler(struct work_struct *work)
@@ -52,10 +60,9 @@ static void retire_work_handler(struct work_struct *work)
mutex_unlock(&i915->drm.struct_mutex);
}
- if (intel_wakeref_active(&i915->gt.wakeref))
- queue_delayed_work(i915->wq,
- &i915->gem.retire_work,
- round_jiffies_up_relative(HZ));
+ queue_delayed_work(i915->wq,
+ &i915->gem.retire_work,
+ round_jiffies_up_relative(HZ));
}
static int pm_notifier(struct notifier_block *nb,
@@ -140,7 +147,6 @@ void i915_gem_suspend(struct drm_i915_private *i915)
* Assert that we successfully flushed all the work and
* reset the GPU back to its idle, low power state.
*/
- drain_delayed_work(&i915->gem.retire_work);
GEM_BUG_ON(i915->gt.awake);
flush_work(&i915->gem.idle_work);
diff --git a/drivers/gpu/drm/i915/selftests/mock_gem_device.c b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
index d919f512042c..9fd02025d382 100644
--- a/drivers/gpu/drm/i915/selftests/mock_gem_device.c
+++ b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
@@ -58,7 +58,6 @@ static void mock_device_release(struct drm_device *dev)
i915_gem_contexts_lost(i915);
mutex_unlock(&i915->drm.struct_mutex);
- drain_delayed_work(&i915->gem.retire_work);
flush_work(&i915->gem.idle_work);
i915_gem_drain_workqueue(i915);
--
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] 52+ messages in thread
* [PATCH 07/14] drm/i915: Stop spinning for DROP_IDLE (debugfs/i915_drop_caches)
2019-05-01 11:45 [PATCH 01/14] drm/i915/hangcheck: Track context changes Chris Wilson
` (4 preceding siblings ...)
2019-05-01 11:45 ` [PATCH 06/14] drm/i915: Cancel retire_worker on parking Chris Wilson
@ 2019-05-01 11:45 ` Chris Wilson
2019-05-02 13:34 ` Tvrtko Ursulin
2019-05-01 11:45 ` [PATCH 08/14] drm/i915: Only reschedule the submission tasklet if preemption is possible Chris Wilson
` (17 subsequent siblings)
23 siblings, 1 reply; 52+ messages in thread
From: Chris Wilson @ 2019-05-01 11:45 UTC (permalink / raw)
To: intel-gfx
If the user is racing a call to debugfs/i915_drop_caches with ongoing
submission from another thread/process, we may never end up idling the
GPU and be uninterruptibly spinning in debugfs/i915_drop_caches trying
to catch an idle moment.
Just flush the work once, that should be enough to park the system under
correct conditions. Outside of those we either have a driver bug or the
user is racing themselves. Sadly, because the user may be provoking the
unwanted situation we can't put a warn here to attract attention to a
probable bug.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/i915_debugfs.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 7e8898d0b78b..2ecefacb1e66 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -3933,9 +3933,7 @@ i915_drop_caches_set(void *data, u64 val)
fs_reclaim_release(GFP_KERNEL);
if (val & DROP_IDLE) {
- do {
- flush_delayed_work(&i915->gem.retire_work);
- } while (READ_ONCE(i915->gt.awake));
+ flush_delayed_work(&i915->gem.retire_work);
flush_work(&i915->gem.idle_work);
}
--
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] 52+ messages in thread
* [PATCH 08/14] drm/i915: Only reschedule the submission tasklet if preemption is possible
2019-05-01 11:45 [PATCH 01/14] drm/i915/hangcheck: Track context changes Chris Wilson
` (5 preceding siblings ...)
2019-05-01 11:45 ` [PATCH 07/14] drm/i915: Stop spinning for DROP_IDLE (debugfs/i915_drop_caches) Chris Wilson
@ 2019-05-01 11:45 ` Chris Wilson
2019-05-03 10:53 ` Tvrtko Ursulin
2019-05-01 11:45 ` [PATCH 09/14] drm/i915: Delay semaphore submission until the start of the signaler Chris Wilson
` (16 subsequent siblings)
23 siblings, 1 reply; 52+ messages in thread
From: Chris Wilson @ 2019-05-01 11:45 UTC (permalink / raw)
To: intel-gfx
If we couple the scheduler more tightly with the execlists policy, we
can apply the preemption policy to the question of whether we need to
kick the tasklet at all for this priority bump.
v2: Rephrase it as a core i915 policy and not an execlists foible.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/gt/intel_engine.h | 18 ------------------
drivers/gpu/drm/i915/gt/intel_lrc.c | 4 ++--
drivers/gpu/drm/i915/gt/selftest_lrc.c | 7 ++++++-
drivers/gpu/drm/i915/i915_request.c | 2 --
drivers/gpu/drm/i915/i915_scheduler.c | 18 +++++++++++-------
drivers/gpu/drm/i915/i915_scheduler.h | 18 ++++++++++++++++++
drivers/gpu/drm/i915/intel_guc_submission.c | 3 ++-
7 files changed, 39 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h b/drivers/gpu/drm/i915/gt/intel_engine.h
index f5b0f27cecb6..06d785533502 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine.h
@@ -106,24 +106,6 @@ hangcheck_action_to_str(const enum intel_engine_hangcheck_action a)
void intel_engines_set_scheduler_caps(struct drm_i915_private *i915);
-static inline bool __execlists_need_preempt(int prio, int last)
-{
- /*
- * Allow preemption of low -> normal -> high, but we do
- * not allow low priority tasks to preempt other low priority
- * tasks under the impression that latency for low priority
- * tasks does not matter (as much as background throughput),
- * so kiss.
- *
- * More naturally we would write
- * prio >= max(0, last);
- * except that we wish to prevent triggering preemption at the same
- * priority level: the task that is running should remain running
- * to preserve FIFO ordering of dependencies.
- */
- return prio > max(I915_PRIORITY_NORMAL - 1, last);
-}
-
static inline void
execlists_set_active(struct intel_engine_execlists *execlists,
unsigned int bit)
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index 7be54b868d8e..35aae7b5c6b9 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -251,8 +251,8 @@ static inline bool need_preempt(const struct intel_engine_cs *engine,
* ourselves, ignore the request.
*/
last_prio = effective_prio(rq);
- if (!__execlists_need_preempt(engine->execlists.queue_priority_hint,
- last_prio))
+ if (!i915_scheduler_need_preempt(engine->execlists.queue_priority_hint,
+ last_prio))
return false;
/*
diff --git a/drivers/gpu/drm/i915/gt/selftest_lrc.c b/drivers/gpu/drm/i915/gt/selftest_lrc.c
index 84538f69185b..4b042893dc0e 100644
--- a/drivers/gpu/drm/i915/gt/selftest_lrc.c
+++ b/drivers/gpu/drm/i915/gt/selftest_lrc.c
@@ -638,14 +638,19 @@ static struct i915_request *dummy_request(struct intel_engine_cs *engine)
GEM_BUG_ON(i915_request_completed(rq));
i915_sw_fence_init(&rq->submit, dummy_notify);
- i915_sw_fence_commit(&rq->submit);
+ set_bit(I915_FENCE_FLAG_ACTIVE, &rq->fence.flags);
return rq;
}
static void dummy_request_free(struct i915_request *dummy)
{
+ /* We have to fake the CS interrupt to kick the next request */
+ i915_sw_fence_commit(&dummy->submit);
+
i915_request_mark_complete(dummy);
+ dma_fence_signal(&dummy->fence);
+
i915_sched_node_fini(&dummy->sched);
i915_sw_fence_fini(&dummy->submit);
diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
index af8c9fa5e066..2e22da66a56c 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -1358,9 +1358,7 @@ long i915_request_wait(struct i915_request *rq,
if (flags & I915_WAIT_PRIORITY) {
if (!i915_request_started(rq) && INTEL_GEN(rq->i915) >= 6)
gen6_rps_boost(rq);
- local_bh_disable(); /* suspend tasklets for reprioritisation */
i915_schedule_bump_priority(rq, I915_PRIORITY_WAIT);
- local_bh_enable(); /* kick tasklets en masse */
}
wait.tsk = current;
diff --git a/drivers/gpu/drm/i915/i915_scheduler.c b/drivers/gpu/drm/i915/i915_scheduler.c
index 39bc4f54e272..88d18600f5db 100644
--- a/drivers/gpu/drm/i915/i915_scheduler.c
+++ b/drivers/gpu/drm/i915/i915_scheduler.c
@@ -261,16 +261,20 @@ sched_lock_engine(const struct i915_sched_node *node,
return engine;
}
-static bool inflight(const struct i915_request *rq,
- const struct intel_engine_cs *engine)
+static inline int rq_prio(const struct i915_request *rq)
{
- const struct i915_request *active;
+ return rq->sched.attr.priority | __NO_PREEMPTION;
+}
+
+static bool kick_tasklet(const struct intel_engine_cs *engine, int prio)
+{
+ const struct i915_request *inflight =
+ port_request(engine->execlists.port);
- if (!i915_request_is_active(rq))
+ if (!inflight)
return false;
- active = port_request(engine->execlists.port);
- return active->hw_context == rq->hw_context;
+ return i915_scheduler_need_preempt(prio, rq_prio(inflight));
}
static void __i915_schedule(struct i915_request *rq,
@@ -400,7 +404,7 @@ static void __i915_schedule(struct i915_request *rq,
* If we are already the currently executing context, don't
* bother evaluating if we should preempt ourselves.
*/
- if (inflight(node_to_request(node), engine))
+ if (!kick_tasklet(engine, prio))
continue;
/* Defer (tasklet) submission until after all of our updates. */
diff --git a/drivers/gpu/drm/i915/i915_scheduler.h b/drivers/gpu/drm/i915/i915_scheduler.h
index 07d243acf553..7eefccff39bf 100644
--- a/drivers/gpu/drm/i915/i915_scheduler.h
+++ b/drivers/gpu/drm/i915/i915_scheduler.h
@@ -52,4 +52,22 @@ static inline void i915_priolist_free(struct i915_priolist *p)
__i915_priolist_free(p);
}
+static inline bool i915_scheduler_need_preempt(int prio, int active)
+{
+ /*
+ * Allow preemption of low -> normal -> high, but we do
+ * not allow low priority tasks to preempt other low priority
+ * tasks under the impression that latency for low priority
+ * tasks does not matter (as much as background throughput),
+ * so kiss.
+ *
+ * More naturally we would write
+ * prio >= max(0, last);
+ * except that we wish to prevent triggering preemption at the same
+ * priority level: the task that is running should remain running
+ * to preserve FIFO ordering of dependencies.
+ */
+ return prio > max(I915_PRIORITY_NORMAL - 1, active);
+}
+
#endif /* _I915_SCHEDULER_H_ */
diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c
index ed94001028f2..cb9964ae229d 100644
--- a/drivers/gpu/drm/i915/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/intel_guc_submission.c
@@ -746,7 +746,8 @@ static bool __guc_dequeue(struct intel_engine_cs *engine)
&engine->i915->guc.preempt_work[engine->id];
int prio = execlists->queue_priority_hint;
- if (__execlists_need_preempt(prio, port_prio(port))) {
+ if (i915_scheduler_need_preempt(prio,
+ port_prio(port))) {
execlists_set_active(execlists,
EXECLISTS_ACTIVE_PREEMPT);
queue_work(engine->i915->guc.preempt_wq,
--
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] 52+ messages in thread
* [PATCH 09/14] drm/i915: Delay semaphore submission until the start of the signaler
2019-05-01 11:45 [PATCH 01/14] drm/i915/hangcheck: Track context changes Chris Wilson
` (6 preceding siblings ...)
2019-05-01 11:45 ` [PATCH 08/14] drm/i915: Only reschedule the submission tasklet if preemption is possible Chris Wilson
@ 2019-05-01 11:45 ` Chris Wilson
2019-05-03 11:03 ` Tvrtko Ursulin
2019-05-01 11:45 ` [PATCH 10/14] drm/i915/execlists: Don't apply priority boost for resets Chris Wilson
` (15 subsequent siblings)
23 siblings, 1 reply; 52+ messages in thread
From: Chris Wilson @ 2019-05-01 11:45 UTC (permalink / raw)
To: intel-gfx
Currently we submit the semaphore busywait as soon as the signaler is
submitted to HW. However, we may submit the signaler as the tail of a
batch of requests, and even not as the first context in the HW list,
i.e. the busywait may start spinning far in advance of the signaler even
starting.
If we wait until the request before the signaler is completed before
submitting the busywait, we prevent the busywait from starting too
early, if the signaler is not first in submission port.
To handle the case where the signaler is at the start of the second (or
later) submission port, we will need to delay the execution callback
until we know the context is promoted to port0. A challenge for later.
Fixes: e88619646971 ("drm/i915: Use HW semaphores for inter-engine synchroni
sation on gen8+")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/i915_request.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
index 2e22da66a56c..8cb3ed5531e3 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -770,6 +770,21 @@ i915_request_create(struct intel_context *ce)
return rq;
}
+static int
+i915_request_await_start(struct i915_request *rq, struct i915_request *signal)
+{
+ if (list_is_first(&signal->ring_link, &signal->ring->request_list))
+ return 0;
+
+ signal = list_prev_entry(signal, ring_link);
+ if (i915_timeline_sync_is_later(rq->timeline, &signal->fence))
+ return 0;
+
+ return i915_sw_fence_await_dma_fence(&rq->submit,
+ &signal->fence, 0,
+ I915_FENCE_GFP);
+}
+
static int
emit_semaphore_wait(struct i915_request *to,
struct i915_request *from,
@@ -788,6 +803,10 @@ emit_semaphore_wait(struct i915_request *to,
&from->fence, 0,
I915_FENCE_GFP);
+ err = i915_request_await_start(to, from);
+ if (err < 0)
+ return err;
+
err = i915_sw_fence_await_dma_fence(&to->semaphore,
&from->fence, 0,
I915_FENCE_GFP);
--
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] 52+ messages in thread
* [PATCH 10/14] drm/i915/execlists: Don't apply priority boost for resets
2019-05-01 11:45 [PATCH 01/14] drm/i915/hangcheck: Track context changes Chris Wilson
` (7 preceding siblings ...)
2019-05-01 11:45 ` [PATCH 09/14] drm/i915: Delay semaphore submission until the start of the signaler Chris Wilson
@ 2019-05-01 11:45 ` Chris Wilson
2019-05-01 11:45 ` [PATCH 11/14] drm/i915: Rearrange i915_scheduler.c Chris Wilson
` (14 subsequent siblings)
23 siblings, 0 replies; 52+ messages in thread
From: Chris Wilson @ 2019-05-01 11:45 UTC (permalink / raw)
To: intel-gfx
Do not treat reset as a normal preemption event and avoid giving the
guilty request a priority boost for simply being active at the time of
reset.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/gt/intel_lrc.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index 35aae7b5c6b9..aa03dd0760e9 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -370,11 +370,11 @@ static void unwind_wa_tail(struct i915_request *rq)
}
static struct i915_request *
-__unwind_incomplete_requests(struct intel_engine_cs *engine)
+__unwind_incomplete_requests(struct intel_engine_cs *engine, int boost)
{
struct i915_request *rq, *rn, *active = NULL;
struct list_head *uninitialized_var(pl);
- int prio = I915_PRIORITY_INVALID | ACTIVE_PRIORITY;
+ int prio = I915_PRIORITY_INVALID | boost;
lockdep_assert_held(&engine->timeline.lock);
@@ -418,8 +418,9 @@ __unwind_incomplete_requests(struct intel_engine_cs *engine)
* in the priority queue, but they will not gain immediate access to
* the GPU.
*/
- if (~prio & ACTIVE_PRIORITY && __i915_request_has_started(active)) {
- prio |= ACTIVE_PRIORITY;
+ if (~prio & boost && __i915_request_has_started(active)) {
+ prio |= boost;
+ GEM_BUG_ON(active->sched.attr.priority >= prio);
active->sched.attr.priority = prio;
list_move_tail(&active->sched.link,
i915_sched_lookup_priolist(engine, prio));
@@ -434,7 +435,7 @@ execlists_unwind_incomplete_requests(struct intel_engine_execlists *execlists)
struct intel_engine_cs *engine =
container_of(execlists, typeof(*engine), execlists);
- return __unwind_incomplete_requests(engine);
+ return __unwind_incomplete_requests(engine, 0);
}
static inline void
@@ -655,7 +656,8 @@ static void complete_preempt_context(struct intel_engine_execlists *execlists)
execlists_cancel_port_requests(execlists);
__unwind_incomplete_requests(container_of(execlists,
struct intel_engine_cs,
- execlists));
+ execlists),
+ ACTIVE_PRIORITY);
}
static void execlists_dequeue(struct intel_engine_cs *engine)
@@ -1908,7 +1910,7 @@ static void __execlists_reset(struct intel_engine_cs *engine, bool stalled)
execlists_cancel_port_requests(execlists);
/* Push back any incomplete requests for replay after the reset. */
- rq = __unwind_incomplete_requests(engine);
+ rq = __unwind_incomplete_requests(engine, 0);
if (!rq)
goto out_replay;
--
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] 52+ messages in thread
* [PATCH 11/14] drm/i915: Rearrange i915_scheduler.c
2019-05-01 11:45 [PATCH 01/14] drm/i915/hangcheck: Track context changes Chris Wilson
` (8 preceding siblings ...)
2019-05-01 11:45 ` [PATCH 10/14] drm/i915/execlists: Don't apply priority boost for resets Chris Wilson
@ 2019-05-01 11:45 ` Chris Wilson
2019-05-01 11:45 ` [PATCH 12/14] drm/i915: Pass i915_sched_node around internally Chris Wilson
` (13 subsequent siblings)
23 siblings, 0 replies; 52+ messages in thread
From: Chris Wilson @ 2019-05-01 11:45 UTC (permalink / raw)
To: intel-gfx
To avoid pulling in a forward declaration in the next patch, move the
i915_sched_node handling to after the main dfs of the scheduler.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/i915_scheduler.c | 210 +++++++++++++-------------
1 file changed, 105 insertions(+), 105 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_scheduler.c b/drivers/gpu/drm/i915/i915_scheduler.c
index 88d18600f5db..834b10ad4ce1 100644
--- a/drivers/gpu/drm/i915/i915_scheduler.c
+++ b/drivers/gpu/drm/i915/i915_scheduler.c
@@ -35,109 +35,6 @@ static inline bool node_signaled(const struct i915_sched_node *node)
return i915_request_completed(node_to_request(node));
}
-void i915_sched_node_init(struct i915_sched_node *node)
-{
- INIT_LIST_HEAD(&node->signalers_list);
- INIT_LIST_HEAD(&node->waiters_list);
- INIT_LIST_HEAD(&node->link);
- node->attr.priority = I915_PRIORITY_INVALID;
- node->semaphores = 0;
- node->flags = 0;
-}
-
-static struct i915_dependency *
-i915_dependency_alloc(void)
-{
- return kmem_cache_alloc(global.slab_dependencies, GFP_KERNEL);
-}
-
-static void
-i915_dependency_free(struct i915_dependency *dep)
-{
- kmem_cache_free(global.slab_dependencies, dep);
-}
-
-bool __i915_sched_node_add_dependency(struct i915_sched_node *node,
- struct i915_sched_node *signal,
- struct i915_dependency *dep,
- unsigned long flags)
-{
- bool ret = false;
-
- spin_lock_irq(&schedule_lock);
-
- if (!node_signaled(signal)) {
- INIT_LIST_HEAD(&dep->dfs_link);
- list_add(&dep->wait_link, &signal->waiters_list);
- list_add(&dep->signal_link, &node->signalers_list);
- dep->signaler = signal;
- dep->flags = flags;
-
- /* Keep track of whether anyone on this chain has a semaphore */
- if (signal->flags & I915_SCHED_HAS_SEMAPHORE_CHAIN &&
- !node_started(signal))
- node->flags |= I915_SCHED_HAS_SEMAPHORE_CHAIN;
-
- ret = true;
- }
-
- spin_unlock_irq(&schedule_lock);
-
- return ret;
-}
-
-int i915_sched_node_add_dependency(struct i915_sched_node *node,
- struct i915_sched_node *signal)
-{
- struct i915_dependency *dep;
-
- dep = i915_dependency_alloc();
- if (!dep)
- return -ENOMEM;
-
- if (!__i915_sched_node_add_dependency(node, signal, dep,
- I915_DEPENDENCY_ALLOC))
- i915_dependency_free(dep);
-
- return 0;
-}
-
-void i915_sched_node_fini(struct i915_sched_node *node)
-{
- struct i915_dependency *dep, *tmp;
-
- GEM_BUG_ON(!list_empty(&node->link));
-
- spin_lock_irq(&schedule_lock);
-
- /*
- * Everyone we depended upon (the fences we wait to be signaled)
- * should retire before us and remove themselves from our list.
- * However, retirement is run independently on each timeline and
- * so we may be called out-of-order.
- */
- list_for_each_entry_safe(dep, tmp, &node->signalers_list, signal_link) {
- GEM_BUG_ON(!node_signaled(dep->signaler));
- GEM_BUG_ON(!list_empty(&dep->dfs_link));
-
- list_del(&dep->wait_link);
- if (dep->flags & I915_DEPENDENCY_ALLOC)
- i915_dependency_free(dep);
- }
-
- /* Remove ourselves from everyone who depends upon us */
- list_for_each_entry_safe(dep, tmp, &node->waiters_list, wait_link) {
- GEM_BUG_ON(dep->signaler != node);
- GEM_BUG_ON(!list_empty(&dep->dfs_link));
-
- list_del(&dep->signal_link);
- if (dep->flags & I915_DEPENDENCY_ALLOC)
- i915_dependency_free(dep);
- }
-
- spin_unlock_irq(&schedule_lock);
-}
-
static inline struct i915_priolist *to_priolist(struct rb_node *rb)
{
return rb_entry(rb, struct i915_priolist, node);
@@ -239,6 +136,11 @@ i915_sched_lookup_priolist(struct intel_engine_cs *engine, int prio)
return &p->requests[idx];
}
+void __i915_priolist_free(struct i915_priolist *p)
+{
+ kmem_cache_free(global.slab_priorities, p);
+}
+
struct sched_cache {
struct list_head *priolist;
};
@@ -440,9 +342,107 @@ void i915_schedule_bump_priority(struct i915_request *rq, unsigned int bump)
spin_unlock_irqrestore(&schedule_lock, flags);
}
-void __i915_priolist_free(struct i915_priolist *p)
+void i915_sched_node_init(struct i915_sched_node *node)
{
- kmem_cache_free(global.slab_priorities, p);
+ INIT_LIST_HEAD(&node->signalers_list);
+ INIT_LIST_HEAD(&node->waiters_list);
+ INIT_LIST_HEAD(&node->link);
+ node->attr.priority = I915_PRIORITY_INVALID;
+ node->semaphores = 0;
+ node->flags = 0;
+}
+
+static struct i915_dependency *
+i915_dependency_alloc(void)
+{
+ return kmem_cache_alloc(global.slab_dependencies, GFP_KERNEL);
+}
+
+static void
+i915_dependency_free(struct i915_dependency *dep)
+{
+ kmem_cache_free(global.slab_dependencies, dep);
+}
+
+bool __i915_sched_node_add_dependency(struct i915_sched_node *node,
+ struct i915_sched_node *signal,
+ struct i915_dependency *dep,
+ unsigned long flags)
+{
+ bool ret = false;
+
+ spin_lock_irq(&schedule_lock);
+
+ if (!node_signaled(signal)) {
+ INIT_LIST_HEAD(&dep->dfs_link);
+ list_add(&dep->wait_link, &signal->waiters_list);
+ list_add(&dep->signal_link, &node->signalers_list);
+ dep->signaler = signal;
+ dep->flags = flags;
+
+ /* Keep track of whether anyone on this chain has a semaphore */
+ if (signal->flags & I915_SCHED_HAS_SEMAPHORE_CHAIN &&
+ !node_started(signal))
+ node->flags |= I915_SCHED_HAS_SEMAPHORE_CHAIN;
+
+ ret = true;
+ }
+
+ spin_unlock_irq(&schedule_lock);
+
+ return ret;
+}
+
+int i915_sched_node_add_dependency(struct i915_sched_node *node,
+ struct i915_sched_node *signal)
+{
+ struct i915_dependency *dep;
+
+ dep = i915_dependency_alloc();
+ if (!dep)
+ return -ENOMEM;
+
+ if (!__i915_sched_node_add_dependency(node, signal, dep,
+ I915_DEPENDENCY_ALLOC))
+ i915_dependency_free(dep);
+
+ return 0;
+}
+
+void i915_sched_node_fini(struct i915_sched_node *node)
+{
+ struct i915_dependency *dep, *tmp;
+
+ GEM_BUG_ON(!list_empty(&node->link));
+
+ spin_lock_irq(&schedule_lock);
+
+ /*
+ * Everyone we depended upon (the fences we wait to be signaled)
+ * should retire before us and remove themselves from our list.
+ * However, retirement is run independently on each timeline and
+ * so we may be called out-of-order.
+ */
+ list_for_each_entry_safe(dep, tmp, &node->signalers_list, signal_link) {
+ GEM_BUG_ON(!node_signaled(dep->signaler));
+ GEM_BUG_ON(!list_empty(&dep->dfs_link));
+
+ list_del(&dep->wait_link);
+ if (dep->flags & I915_DEPENDENCY_ALLOC)
+ i915_dependency_free(dep);
+ }
+
+ /* Remove ourselves from everyone who depends upon us */
+ list_for_each_entry_safe(dep, tmp, &node->waiters_list, wait_link) {
+ GEM_BUG_ON(dep->signaler != node);
+ GEM_BUG_ON(!list_empty(&dep->dfs_link));
+
+ list_del(&dep->signal_link);
+ if (dep->flags & I915_DEPENDENCY_ALLOC)
+ i915_dependency_free(dep);
+ }
+
+ spin_unlock_irq(&schedule_lock);
}
static void i915_global_scheduler_shrink(void)
--
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] 52+ messages in thread
* [PATCH 12/14] drm/i915: Pass i915_sched_node around internally
2019-05-01 11:45 [PATCH 01/14] drm/i915/hangcheck: Track context changes Chris Wilson
` (9 preceding siblings ...)
2019-05-01 11:45 ` [PATCH 11/14] drm/i915: Rearrange i915_scheduler.c Chris Wilson
@ 2019-05-01 11:45 ` Chris Wilson
2019-05-01 11:45 ` [PATCH 13/14] drm/i915: Bump signaler priority on adding a waiter Chris Wilson
` (12 subsequent siblings)
23 siblings, 0 replies; 52+ messages in thread
From: Chris Wilson @ 2019-05-01 11:45 UTC (permalink / raw)
To: intel-gfx
To simplify the next patch, update bump_priority and schedule to accept
the internal i915_sched_ndoe directly and not expect a request pointer.
add/remove: 0/0 grow/shrink: 2/1 up/down: 8/-15 (-7)
Function old new delta
i915_schedule_bump_priority 109 113 +4
i915_schedule 50 54 +4
__i915_schedule 922 907 -15
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/i915_scheduler.c | 33 +++++++++++++++------------
1 file changed, 18 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_scheduler.c b/drivers/gpu/drm/i915/i915_scheduler.c
index 834b10ad4ce1..05eb50558aba 100644
--- a/drivers/gpu/drm/i915/i915_scheduler.c
+++ b/drivers/gpu/drm/i915/i915_scheduler.c
@@ -179,7 +179,7 @@ static bool kick_tasklet(const struct intel_engine_cs *engine, int prio)
return i915_scheduler_need_preempt(prio, rq_prio(inflight));
}
-static void __i915_schedule(struct i915_request *rq,
+static void __i915_schedule(struct i915_sched_node *rq,
const struct i915_sched_attr *attr)
{
struct intel_engine_cs *engine;
@@ -193,13 +193,13 @@ static void __i915_schedule(struct i915_request *rq,
lockdep_assert_held(&schedule_lock);
GEM_BUG_ON(prio == I915_PRIORITY_INVALID);
- if (i915_request_completed(rq))
+ if (prio <= READ_ONCE(rq->attr.priority))
return;
- if (prio <= READ_ONCE(rq->sched.attr.priority))
+ if (node_signaled(rq))
return;
- stack.signaler = &rq->sched;
+ stack.signaler = rq;
list_add(&stack.dfs_link, &dfs);
/*
@@ -250,9 +250,9 @@ static void __i915_schedule(struct i915_request *rq,
* execlists_submit_request()), we can set our own priority and skip
* acquiring the engine locks.
*/
- if (rq->sched.attr.priority == I915_PRIORITY_INVALID) {
- GEM_BUG_ON(!list_empty(&rq->sched.link));
- rq->sched.attr = *attr;
+ if (rq->attr.priority == I915_PRIORITY_INVALID) {
+ GEM_BUG_ON(!list_empty(&rq->link));
+ rq->attr = *attr;
if (stack.dfs_link.next == stack.dfs_link.prev)
return;
@@ -261,7 +261,7 @@ static void __i915_schedule(struct i915_request *rq,
}
memset(&cache, 0, sizeof(cache));
- engine = rq->engine;
+ engine = node_to_request(rq)->engine;
spin_lock(&engine->timeline.lock);
/* Fifo and depth-first replacement ensure our deps execute before us */
@@ -319,13 +319,20 @@ static void __i915_schedule(struct i915_request *rq,
void i915_schedule(struct i915_request *rq, const struct i915_sched_attr *attr)
{
spin_lock_irq(&schedule_lock);
- __i915_schedule(rq, attr);
+ __i915_schedule(&rq->sched, attr);
spin_unlock_irq(&schedule_lock);
}
+static void __bump_priority(struct i915_sched_node *node, unsigned int bump)
+{
+ struct i915_sched_attr attr = node->attr;
+
+ attr.priority |= bump;
+ __i915_schedule(node, &attr);
+}
+
void i915_schedule_bump_priority(struct i915_request *rq, unsigned int bump)
{
- struct i915_sched_attr attr;
unsigned long flags;
GEM_BUG_ON(bump & ~I915_PRIORITY_MASK);
@@ -334,11 +341,7 @@ void i915_schedule_bump_priority(struct i915_request *rq, unsigned int bump)
return;
spin_lock_irqsave(&schedule_lock, flags);
-
- attr = rq->sched.attr;
- attr.priority |= bump;
- __i915_schedule(rq, &attr);
-
+ __bump_priority(&rq->sched, bump);
spin_unlock_irqrestore(&schedule_lock, flags);
}
--
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] 52+ messages in thread
* [PATCH 13/14] drm/i915: Bump signaler priority on adding a waiter
2019-05-01 11:45 [PATCH 01/14] drm/i915/hangcheck: Track context changes Chris Wilson
` (10 preceding siblings ...)
2019-05-01 11:45 ` [PATCH 12/14] drm/i915: Pass i915_sched_node around internally Chris Wilson
@ 2019-05-01 11:45 ` Chris Wilson
2019-05-01 14:59 ` [PATCH] " Chris Wilson
` (2 more replies)
2019-05-01 11:45 ` [PATCH 14/14] drm/i915: Convert inconsistent static engine tables into an init error Chris Wilson
` (11 subsequent siblings)
23 siblings, 3 replies; 52+ messages in thread
From: Chris Wilson @ 2019-05-01 11:45 UTC (permalink / raw)
To: intel-gfx
The handling of the no-preemption priority level imposes the restriction
that we need to maintain the implied ordering even though preemption is
disabled. Otherwise we may end up with an AB-BA deadlock across multiple
engine due to a real preemption event reordering the no-preemption
WAITs. To resolve this issue we currently promote all requests to WAIT
on unsubmission, however this interferes with the timeslicing
requirement that we do not apply any implicit promotion that will defeat
the round-robin timeslice list. (If we automatically promote the active
request it will go back to the head of the queue and not the tail!)
So we need implicit promotion to prevent reordering around semaphores
where we are not allowed to preempt, and we must avoid implicit
promotion on unsubmission. So instead of at unsubmit, if we apply that
implicit promotion on adding the dependency, we avoid the semaphore
deadlock and we also reduce the gains made by the promotion for user
space waiting. Furthermore, by keeping the earlier dependencies at a
higher level, we reduce the search space for timeslicing without
altering runtime scheduling too badly (no dependencies at all will be
assigned a higher priority for rrul).
Testcase: igt/gem_concurrent_blit
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/i915_request.c | 9 ---------
drivers/gpu/drm/i915/i915_scheduler.c | 9 +++++++++
2 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
index 8cb3ed5531e3..065da1bcbb4c 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -468,15 +468,6 @@ void __i915_request_unsubmit(struct i915_request *request)
/* We may be recursing from the signal callback of another i915 fence */
spin_lock_nested(&request->lock, SINGLE_DEPTH_NESTING);
- /*
- * As we do not allow WAIT to preempt inflight requests,
- * once we have executed a request, along with triggering
- * any execution callbacks, we must preserve its ordering
- * within the non-preemptible FIFO.
- */
- BUILD_BUG_ON(__NO_PREEMPTION & ~I915_PRIORITY_MASK); /* only internal */
- request->sched.attr.priority |= __NO_PREEMPTION;
-
if (test_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, &request->fence.flags))
i915_request_cancel_breadcrumb(request);
diff --git a/drivers/gpu/drm/i915/i915_scheduler.c b/drivers/gpu/drm/i915/i915_scheduler.c
index 05eb50558aba..ecc3e83ef28d 100644
--- a/drivers/gpu/drm/i915/i915_scheduler.c
+++ b/drivers/gpu/drm/i915/i915_scheduler.c
@@ -388,6 +388,15 @@ bool __i915_sched_node_add_dependency(struct i915_sched_node *node,
!node_started(signal))
node->flags |= I915_SCHED_HAS_SEMAPHORE_CHAIN;
+ /*
+ * As we do not allow WAIT to preempt inflight requests,
+ * once we have executed a request, along with triggering
+ * any execution callbacks, we must preserve its ordering
+ * within the non-preemptible FIFO.
+ */
+ BUILD_BUG_ON(__NO_PREEMPTION & ~I915_PRIORITY_MASK);
+ __bump_priority(signal, __NO_PREEMPTION);
+
ret = true;
}
--
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] 52+ messages in thread
* [PATCH 14/14] drm/i915: Convert inconsistent static engine tables into an init error
2019-05-01 11:45 [PATCH 01/14] drm/i915/hangcheck: Track context changes Chris Wilson
` (11 preceding siblings ...)
2019-05-01 11:45 ` [PATCH 13/14] drm/i915: Bump signaler priority on adding a waiter Chris Wilson
@ 2019-05-01 11:45 ` Chris Wilson
2019-05-01 12:29 ` ✗ Fi.CI.SPARSE: warning for series starting with [01/14] drm/i915/hangcheck: Track context changes Patchwork
` (10 subsequent siblings)
23 siblings, 0 replies; 52+ messages in thread
From: Chris Wilson @ 2019-05-01 11:45 UTC (permalink / raw)
To: intel-gfx
Remove the modification of the "constant" device info by promoting the
inconsistent intel_engine static table into an initialisation error.
Now, if we add a new engine into the device_info, we must first add that
engine information into the intel_engines.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/gt/intel_engine_cs.c | 28 ++++++++---------------
1 file changed, 9 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index f178f1268f4e..0b3da6d2ef59 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -372,15 +372,14 @@ void intel_engines_cleanup(struct drm_i915_private *i915)
*/
int intel_engines_init_mmio(struct drm_i915_private *i915)
{
- struct intel_device_info *device_info = mkwrite_device_info(i915);
- const unsigned int engine_mask = INTEL_INFO(i915)->engine_mask;
- unsigned int mask = 0;
unsigned int i;
int err;
- WARN_ON(engine_mask == 0);
- WARN_ON(engine_mask &
- GENMASK(BITS_PER_TYPE(mask) - 1, I915_NUM_ENGINES));
+ /* We always presume we have at least RCS available for later probing */
+ if (GEM_WARN_ON(!HAS_ENGINE(i915, RCS0))) {
+ err = -ENODEV;
+ goto cleanup;
+ }
if (i915_inject_load_failure())
return -ENODEV;
@@ -392,25 +391,16 @@ int intel_engines_init_mmio(struct drm_i915_private *i915)
err = intel_engine_setup(i915, i);
if (err)
goto cleanup;
-
- mask |= BIT(i);
}
- /*
- * Catch failures to update intel_engines table when the new engines
- * are added to the driver by a warning and disabling the forgotten
- * engines.
- */
- if (WARN_ON(mask != engine_mask))
- device_info->engine_mask = mask;
-
- /* We always presume we have at least RCS available for later probing */
- if (WARN_ON(!HAS_ENGINE(i915, RCS0))) {
+ /* Catch failures to update intel_engines table for new engines. */
+ if (GEM_WARN_ON(INTEL_INFO(i915)->engine_mask >> i)) {
err = -ENODEV;
goto cleanup;
}
- RUNTIME_INFO(i915)->num_engines = hweight32(mask);
+ RUNTIME_INFO(i915)->num_engines =
+ hweight32(INTEL_INFO(i915)->engine_mask);
i915_check_and_clear_faults(i915);
--
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] 52+ messages in thread
* ✗ Fi.CI.SPARSE: warning for series starting with [01/14] drm/i915/hangcheck: Track context changes
2019-05-01 11:45 [PATCH 01/14] drm/i915/hangcheck: Track context changes Chris Wilson
` (12 preceding siblings ...)
2019-05-01 11:45 ` [PATCH 14/14] drm/i915: Convert inconsistent static engine tables into an init error Chris Wilson
@ 2019-05-01 12:29 ` Patchwork
2019-05-01 12:47 ` ✗ Fi.CI.BAT: failure " Patchwork
` (9 subsequent siblings)
23 siblings, 0 replies; 52+ messages in thread
From: Patchwork @ 2019-05-01 12:29 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [01/14] drm/i915/hangcheck: Track context changes
URL : https://patchwork.freedesktop.org/series/60153/
State : warning
== Summary ==
$ dim sparse origin/drm-tip
Sparse version: v0.5.2
Commit: drm/i915/hangcheck: Track context changes
Okay!
Commit: drm/i915: Include fence signaled bit in print_request()
Okay!
Commit: drm/i915/execlists: Flush the tasklet on parking
Okay!
Commit: drm/i915: Leave engine parking to the engines
Okay!
Commit: drm/i915: Remove delay for idle_work
Okay!
Commit: drm/i915: Cancel retire_worker on parking
Okay!
Commit: drm/i915: Stop spinning for DROP_IDLE (debugfs/i915_drop_caches)
Okay!
Commit: drm/i915: Only reschedule the submission tasklet if preemption is possible
-O:drivers/gpu/drm/i915/gt/intel_engine.h:124:23: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/gt/intel_engine.h:124:23: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/i915_scheduler.h:70:23: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/i915_scheduler.h:70:23: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/i915_scheduler.h:70:23: warning: expression using sizeof(void)
Commit: drm/i915: Delay semaphore submission until the start of the signaler
Okay!
Commit: drm/i915/execlists: Don't apply priority boost for resets
Okay!
Commit: drm/i915: Rearrange i915_scheduler.c
Okay!
Commit: drm/i915: Pass i915_sched_node around internally
Okay!
Commit: drm/i915: Bump signaler priority on adding a waiter
Okay!
Commit: drm/i915: Convert inconsistent static engine tables into an init error
Okay!
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 52+ messages in thread
* ✗ Fi.CI.BAT: failure for series starting with [01/14] drm/i915/hangcheck: Track context changes
2019-05-01 11:45 [PATCH 01/14] drm/i915/hangcheck: Track context changes Chris Wilson
` (13 preceding siblings ...)
2019-05-01 12:29 ` ✗ Fi.CI.SPARSE: warning for series starting with [01/14] drm/i915/hangcheck: Track context changes Patchwork
@ 2019-05-01 12:47 ` Patchwork
2019-05-01 15:22 ` ✗ Fi.CI.SPARSE: warning for series starting with [01/14] drm/i915/hangcheck: Track context changes (rev2) Patchwork
` (8 subsequent siblings)
23 siblings, 0 replies; 52+ messages in thread
From: Patchwork @ 2019-05-01 12:47 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [01/14] drm/i915/hangcheck: Track context changes
URL : https://patchwork.freedesktop.org/series/60153/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_6021 -> Patchwork_12923
====================================================
Summary
-------
**FAILURE**
Serious unknown changes coming with Patchwork_12923 absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_12923, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL: https://patchwork.freedesktop.org/api/1.0/series/60153/revisions/1/mbox/
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_12923:
### IGT changes ###
#### Possible regressions ####
* igt@i915_selftest@live_execlists:
- fi-cfl-8700k: [PASS][1] -> [INCOMPLETE][2]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/fi-cfl-8700k/igt@i915_selftest@live_execlists.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12923/fi-cfl-8700k/igt@i915_selftest@live_execlists.html
- fi-kbl-7567u: [PASS][3] -> [INCOMPLETE][4]
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/fi-kbl-7567u/igt@i915_selftest@live_execlists.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12923/fi-kbl-7567u/igt@i915_selftest@live_execlists.html
- fi-whl-u: [PASS][5] -> [INCOMPLETE][6]
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/fi-whl-u/igt@i915_selftest@live_execlists.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12923/fi-whl-u/igt@i915_selftest@live_execlists.html
- fi-kbl-7500u: NOTRUN -> [INCOMPLETE][7]
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12923/fi-kbl-7500u/igt@i915_selftest@live_execlists.html
- fi-kbl-8809g: [PASS][8] -> [INCOMPLETE][9]
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/fi-kbl-8809g/igt@i915_selftest@live_execlists.html
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12923/fi-kbl-8809g/igt@i915_selftest@live_execlists.html
- fi-kbl-x1275: [PASS][10] -> [INCOMPLETE][11]
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/fi-kbl-x1275/igt@i915_selftest@live_execlists.html
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12923/fi-kbl-x1275/igt@i915_selftest@live_execlists.html
- fi-skl-6600u: [PASS][12] -> [INCOMPLETE][13]
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/fi-skl-6600u/igt@i915_selftest@live_execlists.html
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12923/fi-skl-6600u/igt@i915_selftest@live_execlists.html
- fi-bsw-n3050: [PASS][14] -> [INCOMPLETE][15]
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/fi-bsw-n3050/igt@i915_selftest@live_execlists.html
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12923/fi-bsw-n3050/igt@i915_selftest@live_execlists.html
- fi-bsw-kefka: [PASS][16] -> [INCOMPLETE][17]
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/fi-bsw-kefka/igt@i915_selftest@live_execlists.html
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12923/fi-bsw-kefka/igt@i915_selftest@live_execlists.html
- fi-skl-6700k2: [PASS][18] -> [INCOMPLETE][19]
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/fi-skl-6700k2/igt@i915_selftest@live_execlists.html
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12923/fi-skl-6700k2/igt@i915_selftest@live_execlists.html
- fi-skl-6260u: [PASS][20] -> [INCOMPLETE][21]
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/fi-skl-6260u/igt@i915_selftest@live_execlists.html
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12923/fi-skl-6260u/igt@i915_selftest@live_execlists.html
- fi-skl-6770hq: [PASS][22] -> [INCOMPLETE][23]
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/fi-skl-6770hq/igt@i915_selftest@live_execlists.html
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12923/fi-skl-6770hq/igt@i915_selftest@live_execlists.html
- fi-bdw-5557u: [PASS][24] -> [INCOMPLETE][25]
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/fi-bdw-5557u/igt@i915_selftest@live_execlists.html
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12923/fi-bdw-5557u/igt@i915_selftest@live_execlists.html
- fi-kbl-r: [PASS][26] -> [INCOMPLETE][27]
[26]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/fi-kbl-r/igt@i915_selftest@live_execlists.html
[27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12923/fi-kbl-r/igt@i915_selftest@live_execlists.html
- fi-skl-lmem: [PASS][28] -> [INCOMPLETE][29]
[28]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/fi-skl-lmem/igt@i915_selftest@live_execlists.html
[29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12923/fi-skl-lmem/igt@i915_selftest@live_execlists.html
- fi-cfl-8109u: [PASS][30] -> [INCOMPLETE][31]
[30]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/fi-cfl-8109u/igt@i915_selftest@live_execlists.html
[31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12923/fi-cfl-8109u/igt@i915_selftest@live_execlists.html
* igt@runner@aborted:
- fi-bdw-gvtdvm: NOTRUN -> [FAIL][32]
[32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12923/fi-bdw-gvtdvm/igt@runner@aborted.html
- fi-cfl-8109u: NOTRUN -> [FAIL][33]
[33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12923/fi-cfl-8109u/igt@runner@aborted.html
- fi-bxt-j4205: NOTRUN -> [FAIL][34]
[34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12923/fi-bxt-j4205/igt@runner@aborted.html
- fi-whl-u: NOTRUN -> [FAIL][35]
[35]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12923/fi-whl-u/igt@runner@aborted.html
- fi-icl-u3: NOTRUN -> [FAIL][36]
[36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12923/fi-icl-u3/igt@runner@aborted.html
- fi-bxt-dsi: NOTRUN -> [FAIL][37]
[37]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12923/fi-bxt-dsi/igt@runner@aborted.html
- fi-bsw-n3050: NOTRUN -> [FAIL][38]
[38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12923/fi-bsw-n3050/igt@runner@aborted.html
- fi-kbl-x1275: NOTRUN -> [FAIL][39]
[39]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12923/fi-kbl-x1275/igt@runner@aborted.html
- fi-bsw-kefka: NOTRUN -> [FAIL][40]
[40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12923/fi-bsw-kefka/igt@runner@aborted.html
- fi-cfl-8700k: NOTRUN -> [FAIL][41]
[41]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12923/fi-cfl-8700k/igt@runner@aborted.html
- fi-kbl-8809g: NOTRUN -> [FAIL][42]
[42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12923/fi-kbl-8809g/igt@runner@aborted.html
- fi-kbl-r: NOTRUN -> [FAIL][43]
[43]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12923/fi-kbl-r/igt@runner@aborted.html
- fi-bdw-5557u: NOTRUN -> [FAIL][44]
[44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12923/fi-bdw-5557u/igt@runner@aborted.html
#### Suppressed ####
The following results come from untrusted machines, tests, or statuses.
They do not affect the overall result.
* igt@i915_selftest@live_execlists:
- {fi-cml-u}: [PASS][45] -> [INCOMPLETE][46]
[45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/fi-cml-u/igt@i915_selftest@live_execlists.html
[46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12923/fi-cml-u/igt@i915_selftest@live_execlists.html
* igt@runner@aborted:
- {fi-cml-u}: NOTRUN -> [FAIL][47]
[47]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12923/fi-cml-u/igt@runner@aborted.html
Known issues
------------
Here are the changes found in Patchwork_12923 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@i915_selftest@live_execlists:
- fi-bdw-gvtdvm: [PASS][48] -> [INCOMPLETE][49] ([fdo#105600])
[48]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/fi-bdw-gvtdvm/igt@i915_selftest@live_execlists.html
[49]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12923/fi-bdw-gvtdvm/igt@i915_selftest@live_execlists.html
- fi-skl-gvtdvm: [PASS][50] -> [INCOMPLETE][51] ([fdo#105600])
[50]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/fi-skl-gvtdvm/igt@i915_selftest@live_execlists.html
[51]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12923/fi-skl-gvtdvm/igt@i915_selftest@live_execlists.html
- fi-icl-u3: [PASS][52] -> [INCOMPLETE][53] ([fdo#107713] / [fdo#109567])
[52]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/fi-icl-u3/igt@i915_selftest@live_execlists.html
[53]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12923/fi-icl-u3/igt@i915_selftest@live_execlists.html
- fi-bxt-j4205: [PASS][54] -> [INCOMPLETE][55] ([fdo#103927])
[54]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/fi-bxt-j4205/igt@i915_selftest@live_execlists.html
[55]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12923/fi-bxt-j4205/igt@i915_selftest@live_execlists.html
- fi-glk-dsi: [PASS][56] -> [INCOMPLETE][57] ([fdo#103359] / [k.org#198133])
[56]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/fi-glk-dsi/igt@i915_selftest@live_execlists.html
[57]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12923/fi-glk-dsi/igt@i915_selftest@live_execlists.html
- fi-bxt-dsi: [PASS][58] -> [INCOMPLETE][59] ([fdo#103927])
[58]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/fi-bxt-dsi/igt@i915_selftest@live_execlists.html
[59]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12923/fi-bxt-dsi/igt@i915_selftest@live_execlists.html
* igt@i915_selftest@live_hangcheck:
- fi-skl-iommu: [PASS][60] -> [INCOMPLETE][61] ([fdo#108602] / [fdo#108744])
[60]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/fi-skl-iommu/igt@i915_selftest@live_hangcheck.html
[61]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12923/fi-skl-iommu/igt@i915_selftest@live_hangcheck.html
* igt@kms_pipe_crc_basic@read-crc-pipe-b:
- fi-glk-dsi: [PASS][62] -> [FAIL][63] ([fdo#103191])
[62]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/fi-glk-dsi/igt@kms_pipe_crc_basic@read-crc-pipe-b.html
[63]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12923/fi-glk-dsi/igt@kms_pipe_crc_basic@read-crc-pipe-b.html
#### Possible fixes ####
* igt@kms_chamelium@dp-crc-fast:
- fi-kbl-7500u: [DMESG-WARN][64] ([fdo#103841]) -> [PASS][65]
[64]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/fi-kbl-7500u/igt@kms_chamelium@dp-crc-fast.html
[65]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12923/fi-kbl-7500u/igt@kms_chamelium@dp-crc-fast.html
* igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
- fi-blb-e6850: [INCOMPLETE][66] ([fdo#107718]) -> [PASS][67]
[66]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/fi-blb-e6850/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html
[67]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12923/fi-blb-e6850/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html
#### Warnings ####
* igt@i915_pm_rpm@basic-pci-d3-state:
- fi-kbl-guc: [SKIP][68] ([fdo#109271]) -> [INCOMPLETE][69] ([fdo#107807])
[68]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/fi-kbl-guc/igt@i915_pm_rpm@basic-pci-d3-state.html
[69]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12923/fi-kbl-guc/igt@i915_pm_rpm@basic-pci-d3-state.html
* igt@runner@aborted:
- fi-kbl-7500u: [FAIL][70] ([fdo#103841]) -> [FAIL][71] ([fdo#108622])
[70]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/fi-kbl-7500u/igt@runner@aborted.html
[71]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12923/fi-kbl-7500u/igt@runner@aborted.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
[fdo#103359]: https://bugs.freedesktop.org/show_bug.cgi?id=103359
[fdo#103841]: https://bugs.freedesktop.org/show_bug.cgi?id=103841
[fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
[fdo#105600]: https://bugs.freedesktop.org/show_bug.cgi?id=105600
[fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
[fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
[fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807
[fdo#108602]: https://bugs.freedesktop.org/show_bug.cgi?id=108602
[fdo#108622]: https://bugs.freedesktop.org/show_bug.cgi?id=108622
[fdo#108744]: https://bugs.freedesktop.org/show_bug.cgi?id=108744
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[fdo#109567]: https://bugs.freedesktop.org/show_bug.cgi?id=109567
[k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133
Participating hosts (52 -> 43)
------------------------------
Additional (1): fi-pnv-d510
Missing (10): fi-kbl-soraka fi-hsw-4770r fi-ilk-m540 fi-hsw-4200u fi-icl-u2 fi-bsw-cyan fi-ctg-p8600 fi-icl-y fi-byt-clapper fi-bdw-samus
Build changes
-------------
* Linux: CI_DRM_6021 -> Patchwork_12923
CI_DRM_6021: 850aa4220e8bf7609b03bf89bce146305704bec6 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4971: fc5e0467eb6913d21ad932aa8a31c77fdb5a9c77 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_12923: 35ad13519e877872c7a4ad93771292cd66feea68 @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
35ad13519e87 drm/i915: Convert inconsistent static engine tables into an init error
668dbfecdb9d drm/i915: Bump signaler priority on adding a waiter
df05186878fd drm/i915: Pass i915_sched_node around internally
f1fd7b854665 drm/i915: Rearrange i915_scheduler.c
a32b90b936b0 drm/i915/execlists: Don't apply priority boost for resets
aee38ba394c3 drm/i915: Delay semaphore submission until the start of the signaler
f0c2161e845c drm/i915: Only reschedule the submission tasklet if preemption is possible
05cfaf6dba73 drm/i915: Stop spinning for DROP_IDLE (debugfs/i915_drop_caches)
cc96a6c8aff1 drm/i915: Cancel retire_worker on parking
3284a2d44ffa drm/i915: Remove delay for idle_work
d3716ba300d9 drm/i915: Leave engine parking to the engines
459db28e0e30 drm/i915/execlists: Flush the tasklet on parking
c29bbd2adbe4 drm/i915: Include fence signaled bit in print_request()
6e3d926bf23b drm/i915/hangcheck: Track context changes
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12923/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 52+ messages in thread
* [PATCH] drm/i915: Bump signaler priority on adding a waiter
2019-05-01 11:45 ` [PATCH 13/14] drm/i915: Bump signaler priority on adding a waiter Chris Wilson
@ 2019-05-01 14:59 ` Chris Wilson
2019-05-01 16:29 ` [PATCH v2] " Chris Wilson
2019-05-01 16:32 ` Chris Wilson
2 siblings, 0 replies; 52+ messages in thread
From: Chris Wilson @ 2019-05-01 14:59 UTC (permalink / raw)
To: intel-gfx
The handling of the no-preemption priority level imposes the restriction
that we need to maintain the implied ordering even though preemption is
disabled. Otherwise we may end up with an AB-BA deadlock across multiple
engine due to a real preemption event reordering the no-preemption
WAITs. To resolve this issue we currently promote all requests to WAIT
on unsubmission, however this interferes with the timeslicing
requirement that we do not apply any implicit promotion that will defeat
the round-robin timeslice list. (If we automatically promote the active
request it will go back to the head of the queue and not the tail!)
So we need implicit promotion to prevent reordering around semaphores
where we are not allowed to preempt, and we must avoid implicit
promotion on unsubmission. So instead of at unsubmit, if we apply that
implicit promotion on adding the dependency, we avoid the semaphore
deadlock and we also reduce the gains made by the promotion for user
space waiting. Furthermore, by keeping the earlier dependencies at a
higher level, we reduce the search space for timeslicing without
altering runtime scheduling too badly (no dependencies at all will be
assigned a higher priority for rrul).
Testcase: igt/gem_concurrent_blit
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/gt/selftest_lrc.c | 12 ++++++++----
drivers/gpu/drm/i915/i915_request.c | 9 ---------
drivers/gpu/drm/i915/i915_scheduler.c | 9 +++++++++
3 files changed, 17 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/selftest_lrc.c b/drivers/gpu/drm/i915/gt/selftest_lrc.c
index 4b042893dc0e..5b3d8e33f1cf 100644
--- a/drivers/gpu/drm/i915/gt/selftest_lrc.c
+++ b/drivers/gpu/drm/i915/gt/selftest_lrc.c
@@ -98,12 +98,14 @@ static int live_busywait_preempt(void *arg)
ctx_hi = kernel_context(i915);
if (!ctx_hi)
goto err_unlock;
- ctx_hi->sched.priority = INT_MAX;
+ ctx_hi->sched.priority =
+ I915_USER_PRIORITY(I915_CONTEXT_MAX_USER_PRIORITY);
ctx_lo = kernel_context(i915);
if (!ctx_lo)
goto err_ctx_hi;
- ctx_lo->sched.priority = INT_MIN;
+ ctx_lo->sched.priority =
+ I915_USER_PRIORITY(I915_CONTEXT_MIN_USER_PRIORITY);
obj = i915_gem_object_create_internal(i915, PAGE_SIZE);
if (IS_ERR(obj)) {
@@ -958,12 +960,14 @@ static int live_preempt_hang(void *arg)
ctx_hi = kernel_context(i915);
if (!ctx_hi)
goto err_spin_lo;
- ctx_hi->sched.priority = I915_CONTEXT_MAX_USER_PRIORITY;
+ ctx_hi->sched.priority =
+ I915_USER_PRIORITY(I915_CONTEXT_MAX_USER_PRIORITY);
ctx_lo = kernel_context(i915);
if (!ctx_lo)
goto err_ctx_hi;
- ctx_lo->sched.priority = I915_CONTEXT_MIN_USER_PRIORITY;
+ ctx_lo->sched.priority =
+ I915_USER_PRIORITY(I915_CONTEXT_MIN_USER_PRIORITY);
for_each_engine(engine, i915, id) {
struct i915_request *rq;
diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
index 8cb3ed5531e3..065da1bcbb4c 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -468,15 +468,6 @@ void __i915_request_unsubmit(struct i915_request *request)
/* We may be recursing from the signal callback of another i915 fence */
spin_lock_nested(&request->lock, SINGLE_DEPTH_NESTING);
- /*
- * As we do not allow WAIT to preempt inflight requests,
- * once we have executed a request, along with triggering
- * any execution callbacks, we must preserve its ordering
- * within the non-preemptible FIFO.
- */
- BUILD_BUG_ON(__NO_PREEMPTION & ~I915_PRIORITY_MASK); /* only internal */
- request->sched.attr.priority |= __NO_PREEMPTION;
-
if (test_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, &request->fence.flags))
i915_request_cancel_breadcrumb(request);
diff --git a/drivers/gpu/drm/i915/i915_scheduler.c b/drivers/gpu/drm/i915/i915_scheduler.c
index 05eb50558aba..ecc3e83ef28d 100644
--- a/drivers/gpu/drm/i915/i915_scheduler.c
+++ b/drivers/gpu/drm/i915/i915_scheduler.c
@@ -388,6 +388,15 @@ bool __i915_sched_node_add_dependency(struct i915_sched_node *node,
!node_started(signal))
node->flags |= I915_SCHED_HAS_SEMAPHORE_CHAIN;
+ /*
+ * As we do not allow WAIT to preempt inflight requests,
+ * once we have executed a request, along with triggering
+ * any execution callbacks, we must preserve its ordering
+ * within the non-preemptible FIFO.
+ */
+ BUILD_BUG_ON(__NO_PREEMPTION & ~I915_PRIORITY_MASK);
+ __bump_priority(signal, __NO_PREEMPTION);
+
ret = true;
}
--
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] 52+ messages in thread
* ✗ Fi.CI.SPARSE: warning for series starting with [01/14] drm/i915/hangcheck: Track context changes (rev2)
2019-05-01 11:45 [PATCH 01/14] drm/i915/hangcheck: Track context changes Chris Wilson
` (14 preceding siblings ...)
2019-05-01 12:47 ` ✗ Fi.CI.BAT: failure " Patchwork
@ 2019-05-01 15:22 ` Patchwork
2019-05-01 15:35 ` ✓ Fi.CI.BAT: success " Patchwork
` (7 subsequent siblings)
23 siblings, 0 replies; 52+ messages in thread
From: Patchwork @ 2019-05-01 15:22 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [01/14] drm/i915/hangcheck: Track context changes (rev2)
URL : https://patchwork.freedesktop.org/series/60153/
State : warning
== Summary ==
$ dim sparse origin/drm-tip
Sparse version: v0.5.2
Commit: drm/i915/hangcheck: Track context changes
Okay!
Commit: drm/i915: Include fence signaled bit in print_request()
Okay!
Commit: drm/i915/execlists: Flush the tasklet on parking
Okay!
Commit: drm/i915: Leave engine parking to the engines
Okay!
Commit: drm/i915: Remove delay for idle_work
Okay!
Commit: drm/i915: Cancel retire_worker on parking
Okay!
Commit: drm/i915: Stop spinning for DROP_IDLE (debugfs/i915_drop_caches)
Okay!
Commit: drm/i915: Only reschedule the submission tasklet if preemption is possible
-O:drivers/gpu/drm/i915/gt/intel_engine.h:124:23: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/gt/intel_engine.h:124:23: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/i915_scheduler.h:70:23: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/i915_scheduler.h:70:23: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/i915_scheduler.h:70:23: warning: expression using sizeof(void)
Commit: drm/i915: Delay semaphore submission until the start of the signaler
Okay!
Commit: drm/i915/execlists: Don't apply priority boost for resets
Okay!
Commit: drm/i915: Rearrange i915_scheduler.c
Okay!
Commit: drm/i915: Pass i915_sched_node around internally
Okay!
Commit: drm/i915: Bump signaler priority on adding a waiter
Okay!
Commit: drm/i915: Convert inconsistent static engine tables into an init error
Okay!
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 52+ messages in thread
* ✓ Fi.CI.BAT: success for series starting with [01/14] drm/i915/hangcheck: Track context changes (rev2)
2019-05-01 11:45 [PATCH 01/14] drm/i915/hangcheck: Track context changes Chris Wilson
` (15 preceding siblings ...)
2019-05-01 15:22 ` ✗ Fi.CI.SPARSE: warning for series starting with [01/14] drm/i915/hangcheck: Track context changes (rev2) Patchwork
@ 2019-05-01 15:35 ` Patchwork
2019-05-01 16:42 ` ✗ Fi.CI.SPARSE: warning for series starting with [01/14] drm/i915/hangcheck: Track context changes (rev3) Patchwork
` (6 subsequent siblings)
23 siblings, 0 replies; 52+ messages in thread
From: Patchwork @ 2019-05-01 15:35 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [01/14] drm/i915/hangcheck: Track context changes (rev2)
URL : https://patchwork.freedesktop.org/series/60153/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_6021 -> Patchwork_12925
====================================================
Summary
-------
**SUCCESS**
No regressions found.
External URL: https://patchwork.freedesktop.org/api/1.0/series/60153/revisions/2/mbox/
Known issues
------------
Here are the changes found in Patchwork_12925 that come from known issues:
### IGT changes ###
#### Possible fixes ####
* igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
- fi-blb-e6850: [INCOMPLETE][1] ([fdo#107718]) -> [PASS][2]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/fi-blb-e6850/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12925/fi-blb-e6850/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html
#### Warnings ####
* igt@i915_pm_rpm@basic-pci-d3-state:
- fi-kbl-guc: [SKIP][3] ([fdo#109271]) -> [INCOMPLETE][4] ([fdo#107807])
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/fi-kbl-guc/igt@i915_pm_rpm@basic-pci-d3-state.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12925/fi-kbl-guc/igt@i915_pm_rpm@basic-pci-d3-state.html
[fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
[fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
Participating hosts (52 -> 46)
------------------------------
Additional (1): fi-pnv-d510
Missing (7): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-bsw-cyan fi-ctg-p8600 fi-byt-clapper fi-bdw-samus
Build changes
-------------
* Linux: CI_DRM_6021 -> Patchwork_12925
CI_DRM_6021: 850aa4220e8bf7609b03bf89bce146305704bec6 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4971: fc5e0467eb6913d21ad932aa8a31c77fdb5a9c77 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_12925: 44d3f4a1fc72b988930cbbc495db611d0ff07d2e @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
44d3f4a1fc72 drm/i915: Convert inconsistent static engine tables into an init error
72d20cf4bf9d drm/i915: Bump signaler priority on adding a waiter
eb5a92c844ee drm/i915: Pass i915_sched_node around internally
e66579587990 drm/i915: Rearrange i915_scheduler.c
4e8130e19b73 drm/i915/execlists: Don't apply priority boost for resets
294ff64dcdc3 drm/i915: Delay semaphore submission until the start of the signaler
ee299f13ddc6 drm/i915: Only reschedule the submission tasklet if preemption is possible
10ed7d87fab6 drm/i915: Stop spinning for DROP_IDLE (debugfs/i915_drop_caches)
07be304f9b61 drm/i915: Cancel retire_worker on parking
6215a5f82de4 drm/i915: Remove delay for idle_work
e6839ab1c412 drm/i915: Leave engine parking to the engines
0643d22dc542 drm/i915/execlists: Flush the tasklet on parking
8d54e3388676 drm/i915: Include fence signaled bit in print_request()
f55b819edc02 drm/i915/hangcheck: Track context changes
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12925/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 52+ messages in thread
* [PATCH v2] drm/i915: Bump signaler priority on adding a waiter
2019-05-01 11:45 ` [PATCH 13/14] drm/i915: Bump signaler priority on adding a waiter Chris Wilson
2019-05-01 14:59 ` [PATCH] " Chris Wilson
@ 2019-05-01 16:29 ` Chris Wilson
2019-05-01 16:32 ` Chris Wilson
2 siblings, 0 replies; 52+ messages in thread
From: Chris Wilson @ 2019-05-01 16:29 UTC (permalink / raw)
To: intel-gfx
The handling of the no-preemption priority level imposes the restriction
that we need to maintain the implied ordering even though preemption is
disabled. Otherwise we may end up with an AB-BA deadlock across multiple
engine due to a real preemption event reordering the no-preemption
WAITs. To resolve this issue we currently promote all requests to WAIT
on unsubmission, however this interferes with the timeslicing
requirement that we do not apply any implicit promotion that will defeat
the round-robin timeslice list. (If we automatically promote the active
request it will go back to the head of the queue and not the tail!)
So we need implicit promotion to prevent reordering around semaphores
where we are not allowed to preempt, and we must avoid implicit
promotion on unsubmission. So instead of at unsubmit, if we apply that
implicit promotion on adding the dependency, we avoid the semaphore
deadlock and we also reduce the gains made by the promotion for user
space waiting. Furthermore, by keeping the earlier dependencies at a
higher level, we reduce the search space for timeslicing without
altering runtime scheduling too badly (no dependencies at all will be
assigned a higher priority for rrul).
v2: Limit the bump to external edges (as originally intended) i.e.
between contexts and out to the user.
Testcase: igt/gem_concurrent_blit
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/gt/selftest_lrc.c | 12 ++++++++----
drivers/gpu/drm/i915/i915_request.c | 13 +++----------
drivers/gpu/drm/i915/i915_scheduler.c | 15 +++++++++++++--
drivers/gpu/drm/i915/i915_scheduler.h | 3 ++-
drivers/gpu/drm/i915/i915_scheduler_types.h | 3 ++-
5 files changed, 28 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/selftest_lrc.c b/drivers/gpu/drm/i915/gt/selftest_lrc.c
index 4b042893dc0e..5b3d8e33f1cf 100644
--- a/drivers/gpu/drm/i915/gt/selftest_lrc.c
+++ b/drivers/gpu/drm/i915/gt/selftest_lrc.c
@@ -98,12 +98,14 @@ static int live_busywait_preempt(void *arg)
ctx_hi = kernel_context(i915);
if (!ctx_hi)
goto err_unlock;
- ctx_hi->sched.priority = INT_MAX;
+ ctx_hi->sched.priority =
+ I915_USER_PRIORITY(I915_CONTEXT_MAX_USER_PRIORITY);
ctx_lo = kernel_context(i915);
if (!ctx_lo)
goto err_ctx_hi;
- ctx_lo->sched.priority = INT_MIN;
+ ctx_lo->sched.priority =
+ I915_USER_PRIORITY(I915_CONTEXT_MIN_USER_PRIORITY);
obj = i915_gem_object_create_internal(i915, PAGE_SIZE);
if (IS_ERR(obj)) {
@@ -958,12 +960,14 @@ static int live_preempt_hang(void *arg)
ctx_hi = kernel_context(i915);
if (!ctx_hi)
goto err_spin_lo;
- ctx_hi->sched.priority = I915_CONTEXT_MAX_USER_PRIORITY;
+ ctx_hi->sched.priority =
+ I915_USER_PRIORITY(I915_CONTEXT_MAX_USER_PRIORITY);
ctx_lo = kernel_context(i915);
if (!ctx_lo)
goto err_ctx_hi;
- ctx_lo->sched.priority = I915_CONTEXT_MIN_USER_PRIORITY;
+ ctx_lo->sched.priority =
+ I915_USER_PRIORITY(I915_CONTEXT_MIN_USER_PRIORITY);
for_each_engine(engine, i915, id) {
struct i915_request *rq;
diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
index 8cb3ed5531e3..1a04894a904b 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -468,15 +468,6 @@ void __i915_request_unsubmit(struct i915_request *request)
/* We may be recursing from the signal callback of another i915 fence */
spin_lock_nested(&request->lock, SINGLE_DEPTH_NESTING);
- /*
- * As we do not allow WAIT to preempt inflight requests,
- * once we have executed a request, along with triggering
- * any execution callbacks, we must preserve its ordering
- * within the non-preemptible FIFO.
- */
- BUILD_BUG_ON(__NO_PREEMPTION & ~I915_PRIORITY_MASK); /* only internal */
- request->sched.attr.priority |= __NO_PREEMPTION;
-
if (test_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, &request->fence.flags))
i915_request_cancel_breadcrumb(request);
@@ -861,7 +852,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;
}
diff --git a/drivers/gpu/drm/i915/i915_scheduler.c b/drivers/gpu/drm/i915/i915_scheduler.c
index 05eb50558aba..319eb8703451 100644
--- a/drivers/gpu/drm/i915/i915_scheduler.c
+++ b/drivers/gpu/drm/i915/i915_scheduler.c
@@ -388,6 +388,16 @@ bool __i915_sched_node_add_dependency(struct i915_sched_node *node,
!node_started(signal))
node->flags |= I915_SCHED_HAS_SEMAPHORE_CHAIN;
+ /*
+ * As we do not allow WAIT to preempt inflight requests,
+ * once we have executed a request, along with triggering
+ * any execution callbacks, we must preserve its ordering
+ * within the non-preemptible FIFO.
+ */
+ BUILD_BUG_ON(__NO_PREEMPTION & ~I915_PRIORITY_MASK);
+ if (flags & I915_DEPENDENCY_EXTERNAL)
+ __bump_priority(signal, __NO_PREEMPTION);
+
ret = true;
}
@@ -397,7 +407,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;
@@ -406,7 +417,7 @@ int i915_sched_node_add_dependency(struct i915_sched_node *node,
return -ENOMEM;
if (!__i915_sched_node_add_dependency(node, signal, dep,
- I915_DEPENDENCY_ALLOC))
+ flags | I915_DEPENDENCY_ALLOC))
i915_dependency_free(dep);
return 0;
diff --git a/drivers/gpu/drm/i915/i915_scheduler.h b/drivers/gpu/drm/i915/i915_scheduler.h
index 7eefccff39bf..fc7b6baa1355 100644
--- a/drivers/gpu/drm/i915/i915_scheduler.h
+++ b/drivers/gpu/drm/i915/i915_scheduler.h
@@ -33,7 +33,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 166a457884b2..3e309631bd0b 100644
--- a/drivers/gpu/drm/i915/i915_scheduler_types.h
+++ b/drivers/gpu/drm/i915/i915_scheduler_types.h
@@ -66,7 +66,8 @@ struct i915_dependency {
struct list_head wait_link;
struct list_head dfs_link;
unsigned long flags;
-#define I915_DEPENDENCY_ALLOC BIT(0)
+#define I915_DEPENDENCY_ALLOC BIT(0)
+#define I915_DEPENDENCY_EXTERNAL BIT(1)
};
#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] 52+ messages in thread
* [PATCH v2] drm/i915: Bump signaler priority on adding a waiter
2019-05-01 11:45 ` [PATCH 13/14] drm/i915: Bump signaler priority on adding a waiter Chris Wilson
2019-05-01 14:59 ` [PATCH] " Chris Wilson
2019-05-01 16:29 ` [PATCH v2] " Chris Wilson
@ 2019-05-01 16:32 ` Chris Wilson
2 siblings, 0 replies; 52+ messages in thread
From: Chris Wilson @ 2019-05-01 16:32 UTC (permalink / raw)
To: intel-gfx
The handling of the no-preemption priority level imposes the restriction
that we need to maintain the implied ordering even though preemption is
disabled. Otherwise we may end up with an AB-BA deadlock across multiple
engine due to a real preemption event reordering the no-preemption
WAITs. To resolve this issue we currently promote all requests to WAIT
on unsubmission, however this interferes with the timeslicing
requirement that we do not apply any implicit promotion that will defeat
the round-robin timeslice list. (If we automatically promote the active
request it will go back to the head of the queue and not the tail!)
So we need implicit promotion to prevent reordering around semaphores
where we are not allowed to preempt, and we must avoid implicit
promotion on unsubmission. So instead of at unsubmit, if we apply that
implicit promotion on adding the dependency, we avoid the semaphore
deadlock and we also reduce the gains made by the promotion for user
space waiting. Furthermore, by keeping the earlier dependencies at a
higher level, we reduce the search space for timeslicing without
altering runtime scheduling too badly (no dependencies at all will be
assigned a higher priority for rrul).
v2: Limit the bump to external edges (as originally intended) i.e.
between contexts and out to the user.
Testcase: igt/gem_concurrent_blit
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/gt/selftest_lrc.c | 12 ++++++++----
drivers/gpu/drm/i915/i915_request.c | 9 ---------
drivers/gpu/drm/i915/i915_scheduler.c | 11 +++++++++++
drivers/gpu/drm/i915/i915_scheduler_types.h | 3 ++-
4 files changed, 21 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/selftest_lrc.c b/drivers/gpu/drm/i915/gt/selftest_lrc.c
index 4b042893dc0e..5b3d8e33f1cf 100644
--- a/drivers/gpu/drm/i915/gt/selftest_lrc.c
+++ b/drivers/gpu/drm/i915/gt/selftest_lrc.c
@@ -98,12 +98,14 @@ static int live_busywait_preempt(void *arg)
ctx_hi = kernel_context(i915);
if (!ctx_hi)
goto err_unlock;
- ctx_hi->sched.priority = INT_MAX;
+ ctx_hi->sched.priority =
+ I915_USER_PRIORITY(I915_CONTEXT_MAX_USER_PRIORITY);
ctx_lo = kernel_context(i915);
if (!ctx_lo)
goto err_ctx_hi;
- ctx_lo->sched.priority = INT_MIN;
+ ctx_lo->sched.priority =
+ I915_USER_PRIORITY(I915_CONTEXT_MIN_USER_PRIORITY);
obj = i915_gem_object_create_internal(i915, PAGE_SIZE);
if (IS_ERR(obj)) {
@@ -958,12 +960,14 @@ static int live_preempt_hang(void *arg)
ctx_hi = kernel_context(i915);
if (!ctx_hi)
goto err_spin_lo;
- ctx_hi->sched.priority = I915_CONTEXT_MAX_USER_PRIORITY;
+ ctx_hi->sched.priority =
+ I915_USER_PRIORITY(I915_CONTEXT_MAX_USER_PRIORITY);
ctx_lo = kernel_context(i915);
if (!ctx_lo)
goto err_ctx_hi;
- ctx_lo->sched.priority = I915_CONTEXT_MIN_USER_PRIORITY;
+ ctx_lo->sched.priority =
+ I915_USER_PRIORITY(I915_CONTEXT_MIN_USER_PRIORITY);
for_each_engine(engine, i915, id) {
struct i915_request *rq;
diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
index 8cb3ed5531e3..065da1bcbb4c 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -468,15 +468,6 @@ void __i915_request_unsubmit(struct i915_request *request)
/* We may be recursing from the signal callback of another i915 fence */
spin_lock_nested(&request->lock, SINGLE_DEPTH_NESTING);
- /*
- * As we do not allow WAIT to preempt inflight requests,
- * once we have executed a request, along with triggering
- * any execution callbacks, we must preserve its ordering
- * within the non-preemptible FIFO.
- */
- BUILD_BUG_ON(__NO_PREEMPTION & ~I915_PRIORITY_MASK); /* only internal */
- request->sched.attr.priority |= __NO_PREEMPTION;
-
if (test_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, &request->fence.flags))
i915_request_cancel_breadcrumb(request);
diff --git a/drivers/gpu/drm/i915/i915_scheduler.c b/drivers/gpu/drm/i915/i915_scheduler.c
index 05eb50558aba..ea0ab0c8f571 100644
--- a/drivers/gpu/drm/i915/i915_scheduler.c
+++ b/drivers/gpu/drm/i915/i915_scheduler.c
@@ -388,6 +388,16 @@ bool __i915_sched_node_add_dependency(struct i915_sched_node *node,
!node_started(signal))
node->flags |= I915_SCHED_HAS_SEMAPHORE_CHAIN;
+ /*
+ * As we do not allow WAIT to preempt inflight requests,
+ * once we have executed a request, along with triggering
+ * any execution callbacks, we must preserve its ordering
+ * within the non-preemptible FIFO.
+ */
+ BUILD_BUG_ON(__NO_PREEMPTION & ~I915_PRIORITY_MASK);
+ if (flags & I915_DEPENDENCY_EXTERNAL)
+ __bump_priority(signal, __NO_PREEMPTION);
+
ret = true;
}
@@ -406,6 +416,7 @@ int i915_sched_node_add_dependency(struct i915_sched_node *node,
return -ENOMEM;
if (!__i915_sched_node_add_dependency(node, signal, dep,
+ I915_DEPENDENCY_EXTERNAL |
I915_DEPENDENCY_ALLOC))
i915_dependency_free(dep);
diff --git a/drivers/gpu/drm/i915/i915_scheduler_types.h b/drivers/gpu/drm/i915/i915_scheduler_types.h
index 166a457884b2..3e309631bd0b 100644
--- a/drivers/gpu/drm/i915/i915_scheduler_types.h
+++ b/drivers/gpu/drm/i915/i915_scheduler_types.h
@@ -66,7 +66,8 @@ struct i915_dependency {
struct list_head wait_link;
struct list_head dfs_link;
unsigned long flags;
-#define I915_DEPENDENCY_ALLOC BIT(0)
+#define I915_DEPENDENCY_ALLOC BIT(0)
+#define I915_DEPENDENCY_EXTERNAL BIT(1)
};
#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] 52+ messages in thread
* ✗ Fi.CI.SPARSE: warning for series starting with [01/14] drm/i915/hangcheck: Track context changes (rev3)
2019-05-01 11:45 [PATCH 01/14] drm/i915/hangcheck: Track context changes Chris Wilson
` (16 preceding siblings ...)
2019-05-01 15:35 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2019-05-01 16:42 ` Patchwork
2019-05-01 16:52 ` ✓ Fi.CI.BAT: success " Patchwork
` (5 subsequent siblings)
23 siblings, 0 replies; 52+ messages in thread
From: Patchwork @ 2019-05-01 16:42 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [01/14] drm/i915/hangcheck: Track context changes (rev3)
URL : https://patchwork.freedesktop.org/series/60153/
State : warning
== Summary ==
$ dim sparse origin/drm-tip
Sparse version: v0.5.2
Commit: drm/i915/hangcheck: Track context changes
Okay!
Commit: drm/i915: Include fence signaled bit in print_request()
Okay!
Commit: drm/i915/execlists: Flush the tasklet on parking
Okay!
Commit: drm/i915: Leave engine parking to the engines
Okay!
Commit: drm/i915: Remove delay for idle_work
Okay!
Commit: drm/i915: Cancel retire_worker on parking
Okay!
Commit: drm/i915: Stop spinning for DROP_IDLE (debugfs/i915_drop_caches)
Okay!
Commit: drm/i915: Only reschedule the submission tasklet if preemption is possible
-O:drivers/gpu/drm/i915/gt/intel_engine.h:124:23: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/gt/intel_engine.h:124:23: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/i915_scheduler.h:70:23: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/i915_scheduler.h:70:23: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/i915_scheduler.h:70:23: warning: expression using sizeof(void)
Commit: drm/i915: Delay semaphore submission until the start of the signaler
Okay!
Commit: drm/i915/execlists: Don't apply priority boost for resets
Okay!
Commit: drm/i915: Rearrange i915_scheduler.c
Okay!
Commit: drm/i915: Pass i915_sched_node around internally
Okay!
Commit: drm/i915: Bump signaler priority on adding a waiter
Okay!
Commit: drm/i915: Convert inconsistent static engine tables into an init error
Okay!
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 52+ messages in thread
* ✓ Fi.CI.BAT: success for series starting with [01/14] drm/i915/hangcheck: Track context changes (rev3)
2019-05-01 11:45 [PATCH 01/14] drm/i915/hangcheck: Track context changes Chris Wilson
` (17 preceding siblings ...)
2019-05-01 16:42 ` ✗ Fi.CI.SPARSE: warning for series starting with [01/14] drm/i915/hangcheck: Track context changes (rev3) Patchwork
@ 2019-05-01 16:52 ` Patchwork
2019-05-01 17:01 ` ✗ Fi.CI.SPARSE: warning for series starting with [01/14] drm/i915/hangcheck: Track context changes (rev4) Patchwork
` (4 subsequent siblings)
23 siblings, 0 replies; 52+ messages in thread
From: Patchwork @ 2019-05-01 16:52 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [01/14] drm/i915/hangcheck: Track context changes (rev3)
URL : https://patchwork.freedesktop.org/series/60153/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_6021 -> Patchwork_12927
====================================================
Summary
-------
**SUCCESS**
No regressions found.
External URL: https://patchwork.freedesktop.org/api/1.0/series/60153/revisions/3/mbox/
Known issues
------------
Here are the changes found in Patchwork_12927 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@i915_pm_rpm@module-reload:
- fi-skl-6770hq: [PASS][1] -> [DMESG-WARN][2] ([fdo#105541])
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/fi-skl-6770hq/igt@i915_pm_rpm@module-reload.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12927/fi-skl-6770hq/igt@i915_pm_rpm@module-reload.html
#### Possible fixes ####
* igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
- fi-blb-e6850: [INCOMPLETE][3] ([fdo#107718]) -> [PASS][4]
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/fi-blb-e6850/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12927/fi-blb-e6850/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html
#### Warnings ####
* igt@i915_pm_rpm@basic-pci-d3-state:
- fi-kbl-guc: [SKIP][5] ([fdo#109271]) -> [INCOMPLETE][6] ([fdo#107807])
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/fi-kbl-guc/igt@i915_pm_rpm@basic-pci-d3-state.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12927/fi-kbl-guc/igt@i915_pm_rpm@basic-pci-d3-state.html
[fdo#105541]: https://bugs.freedesktop.org/show_bug.cgi?id=105541
[fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
[fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
Participating hosts (52 -> 46)
------------------------------
Additional (1): fi-pnv-d510
Missing (7): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-bsw-cyan fi-ctg-p8600 fi-byt-clapper fi-bdw-samus
Build changes
-------------
* Linux: CI_DRM_6021 -> Patchwork_12927
CI_DRM_6021: 850aa4220e8bf7609b03bf89bce146305704bec6 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4971: fc5e0467eb6913d21ad932aa8a31c77fdb5a9c77 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_12927: aa242739648eef99230bfe29776b2b5a8dfb0953 @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
aa242739648e drm/i915: Convert inconsistent static engine tables into an init error
acea16a2683b drm/i915: Bump signaler priority on adding a waiter
4d93039126e6 drm/i915: Pass i915_sched_node around internally
237dea2e6369 drm/i915: Rearrange i915_scheduler.c
9fb71130c987 drm/i915/execlists: Don't apply priority boost for resets
89b9e63d0de9 drm/i915: Delay semaphore submission until the start of the signaler
aab09ac1c0d0 drm/i915: Only reschedule the submission tasklet if preemption is possible
c04e7edf12e3 drm/i915: Stop spinning for DROP_IDLE (debugfs/i915_drop_caches)
0c4ed77e0ff2 drm/i915: Cancel retire_worker on parking
1373aa28b84f drm/i915: Remove delay for idle_work
9a055938ab47 drm/i915: Leave engine parking to the engines
bfb97e815599 drm/i915/execlists: Flush the tasklet on parking
abdf4d8765ad drm/i915: Include fence signaled bit in print_request()
cecc697e31f0 drm/i915/hangcheck: Track context changes
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12927/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 52+ messages in thread
* ✗ Fi.CI.SPARSE: warning for series starting with [01/14] drm/i915/hangcheck: Track context changes (rev4)
2019-05-01 11:45 [PATCH 01/14] drm/i915/hangcheck: Track context changes Chris Wilson
` (18 preceding siblings ...)
2019-05-01 16:52 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2019-05-01 17:01 ` Patchwork
2019-05-01 17:37 ` ✓ Fi.CI.BAT: success " Patchwork
` (3 subsequent siblings)
23 siblings, 0 replies; 52+ messages in thread
From: Patchwork @ 2019-05-01 17:01 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [01/14] drm/i915/hangcheck: Track context changes (rev4)
URL : https://patchwork.freedesktop.org/series/60153/
State : warning
== Summary ==
$ dim sparse origin/drm-tip
Sparse version: v0.5.2
Commit: drm/i915/hangcheck: Track context changes
Okay!
Commit: drm/i915: Include fence signaled bit in print_request()
Okay!
Commit: drm/i915/execlists: Flush the tasklet on parking
Okay!
Commit: drm/i915: Leave engine parking to the engines
Okay!
Commit: drm/i915: Remove delay for idle_work
Okay!
Commit: drm/i915: Cancel retire_worker on parking
Okay!
Commit: drm/i915: Stop spinning for DROP_IDLE (debugfs/i915_drop_caches)
Okay!
Commit: drm/i915: Only reschedule the submission tasklet if preemption is possible
-O:drivers/gpu/drm/i915/gt/intel_engine.h:124:23: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/gt/intel_engine.h:124:23: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/i915_scheduler.h:70:23: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/i915_scheduler.h:70:23: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/i915_scheduler.h:70:23: warning: expression using sizeof(void)
Commit: drm/i915: Delay semaphore submission until the start of the signaler
Okay!
Commit: drm/i915/execlists: Don't apply priority boost for resets
Okay!
Commit: drm/i915: Rearrange i915_scheduler.c
Okay!
Commit: drm/i915: Pass i915_sched_node around internally
Okay!
Commit: drm/i915: Bump signaler priority on adding a waiter
Okay!
Commit: drm/i915: Convert inconsistent static engine tables into an init error
Okay!
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 52+ messages in thread
* ✓ Fi.CI.BAT: success for series starting with [01/14] drm/i915/hangcheck: Track context changes (rev4)
2019-05-01 11:45 [PATCH 01/14] drm/i915/hangcheck: Track context changes Chris Wilson
` (19 preceding siblings ...)
2019-05-01 17:01 ` ✗ Fi.CI.SPARSE: warning for series starting with [01/14] drm/i915/hangcheck: Track context changes (rev4) Patchwork
@ 2019-05-01 17:37 ` Patchwork
2019-05-02 9:35 ` ✗ Fi.CI.IGT: failure " Patchwork
` (2 subsequent siblings)
23 siblings, 0 replies; 52+ messages in thread
From: Patchwork @ 2019-05-01 17:37 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [01/14] drm/i915/hangcheck: Track context changes (rev4)
URL : https://patchwork.freedesktop.org/series/60153/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_6021 -> Patchwork_12928
====================================================
Summary
-------
**SUCCESS**
No regressions found.
External URL: https://patchwork.freedesktop.org/api/1.0/series/60153/revisions/4/mbox/
Known issues
------------
Here are the changes found in Patchwork_12928 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@i915_selftest@live_hangcheck:
- fi-skl-iommu: [PASS][1] -> [INCOMPLETE][2] ([fdo#108602] / [fdo#108744])
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/fi-skl-iommu/igt@i915_selftest@live_hangcheck.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12928/fi-skl-iommu/igt@i915_selftest@live_hangcheck.html
#### Possible fixes ####
* igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
- fi-blb-e6850: [INCOMPLETE][3] ([fdo#107718]) -> [PASS][4]
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/fi-blb-e6850/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12928/fi-blb-e6850/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html
#### Warnings ####
* igt@i915_pm_rpm@basic-pci-d3-state:
- fi-kbl-guc: [SKIP][5] ([fdo#109271]) -> [INCOMPLETE][6] ([fdo#107807])
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/fi-kbl-guc/igt@i915_pm_rpm@basic-pci-d3-state.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12928/fi-kbl-guc/igt@i915_pm_rpm@basic-pci-d3-state.html
[fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
[fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807
[fdo#108602]: https://bugs.freedesktop.org/show_bug.cgi?id=108602
[fdo#108744]: https://bugs.freedesktop.org/show_bug.cgi?id=108744
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
Participating hosts (52 -> 46)
------------------------------
Additional (1): fi-pnv-d510
Missing (7): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-bsw-cyan fi-ctg-p8600 fi-byt-clapper fi-bdw-samus
Build changes
-------------
* Linux: CI_DRM_6021 -> Patchwork_12928
CI_DRM_6021: 850aa4220e8bf7609b03bf89bce146305704bec6 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4971: fc5e0467eb6913d21ad932aa8a31c77fdb5a9c77 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_12928: 01f589b56645790b5775e319e3f4ff827e465555 @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
01f589b56645 drm/i915: Convert inconsistent static engine tables into an init error
f2d6eddeb32d drm/i915: Bump signaler priority on adding a waiter
5218802aa1f6 drm/i915: Pass i915_sched_node around internally
026843605d46 drm/i915: Rearrange i915_scheduler.c
cf13ef929e7c drm/i915/execlists: Don't apply priority boost for resets
5b463d70273d drm/i915: Delay semaphore submission until the start of the signaler
2f91bc3c1649 drm/i915: Only reschedule the submission tasklet if preemption is possible
5bd685806aa5 drm/i915: Stop spinning for DROP_IDLE (debugfs/i915_drop_caches)
639f5996c165 drm/i915: Cancel retire_worker on parking
79fa465f1d46 drm/i915: Remove delay for idle_work
fcabf363d051 drm/i915: Leave engine parking to the engines
3067553e8d99 drm/i915/execlists: Flush the tasklet on parking
784762bc47d9 drm/i915: Include fence signaled bit in print_request()
d288339c7974 drm/i915/hangcheck: Track context changes
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12928/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 52+ messages in thread
* ✗ Fi.CI.IGT: failure for series starting with [01/14] drm/i915/hangcheck: Track context changes (rev4)
2019-05-01 11:45 [PATCH 01/14] drm/i915/hangcheck: Track context changes Chris Wilson
` (20 preceding siblings ...)
2019-05-01 17:37 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2019-05-02 9:35 ` Patchwork
2019-05-02 16:45 ` ✗ Fi.CI.BAT: failure for series starting with [01/14] drm/i915/hangcheck: Track context changes (rev5) Patchwork
2019-05-03 10:36 ` [PATCH 01/14] drm/i915/hangcheck: Track context changes Tvrtko Ursulin
23 siblings, 0 replies; 52+ messages in thread
From: Patchwork @ 2019-05-02 9:35 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [01/14] drm/i915/hangcheck: Track context changes (rev4)
URL : https://patchwork.freedesktop.org/series/60153/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_6021_full -> Patchwork_12928_full
====================================================
Summary
-------
**FAILURE**
Serious unknown changes coming with Patchwork_12928_full absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_12928_full, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_12928_full:
### IGT changes ###
#### Possible regressions ####
* igt@gem_ppgtt@flink-and-close-vma-leak:
- shard-iclb: [PASS][1] -> [FAIL][2]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/shard-iclb5/igt@gem_ppgtt@flink-and-close-vma-leak.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12928/shard-iclb7/igt@gem_ppgtt@flink-and-close-vma-leak.html
* igt@i915_pm_rpm@cursor:
- shard-iclb: [PASS][3] -> [DMESG-WARN][4]
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/shard-iclb3/igt@i915_pm_rpm@cursor.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12928/shard-iclb4/igt@i915_pm_rpm@cursor.html
Known issues
------------
Here are the changes found in Patchwork_12928_full that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@i915_suspend@debugfs-reader:
- shard-apl: [PASS][5] -> [DMESG-WARN][6] ([fdo#108566]) +1 similar issue
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/shard-apl8/igt@i915_suspend@debugfs-reader.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12928/shard-apl2/igt@i915_suspend@debugfs-reader.html
* igt@kms_flip@2x-flip-vs-expired-vblank:
- shard-glk: [PASS][7] -> [FAIL][8] ([fdo#105363])
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/shard-glk3/igt@kms_flip@2x-flip-vs-expired-vblank.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12928/shard-glk4/igt@kms_flip@2x-flip-vs-expired-vblank.html
* igt@kms_flip@bo-too-big:
- shard-skl: [PASS][9] -> [INCOMPLETE][10] ([fdo#110581])
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/shard-skl4/igt@kms_flip@bo-too-big.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12928/shard-skl2/igt@kms_flip@bo-too-big.html
* igt@kms_flip@flip-vs-expired-vblank-interruptible:
- shard-iclb: [PASS][11] -> [INCOMPLETE][12] ([fdo#107713] / [fdo#110581]) +1 similar issue
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/shard-iclb2/igt@kms_flip@flip-vs-expired-vblank-interruptible.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12928/shard-iclb4/igt@kms_flip@flip-vs-expired-vblank-interruptible.html
* igt@kms_flip@flip-vs-suspend:
- shard-hsw: [PASS][13] -> [INCOMPLETE][14] ([fdo#103540] / [fdo#110581]) +1 similar issue
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/shard-hsw6/igt@kms_flip@flip-vs-suspend.html
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12928/shard-hsw1/igt@kms_flip@flip-vs-suspend.html
- shard-skl: [PASS][15] -> [INCOMPLETE][16] ([fdo#109507] / [fdo#110581])
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/shard-skl9/igt@kms_flip@flip-vs-suspend.html
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12928/shard-skl5/igt@kms_flip@flip-vs-suspend.html
* igt@kms_flip_tiling@flip-to-x-tiled:
- shard-iclb: [PASS][17] -> [FAIL][18] ([fdo#108134])
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/shard-iclb1/igt@kms_flip_tiling@flip-to-x-tiled.html
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12928/shard-iclb6/igt@kms_flip_tiling@flip-to-x-tiled.html
* igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-pwrite:
- shard-iclb: [PASS][19] -> [FAIL][20] ([fdo#103167]) +5 similar issues
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/shard-iclb5/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-pwrite.html
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12928/shard-iclb7/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-pwrite.html
* igt@kms_frontbuffer_tracking@psr-suspend:
- shard-skl: [PASS][21] -> [INCOMPLETE][22] ([fdo#104108] / [fdo#106978] / [fdo#110581])
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/shard-skl10/igt@kms_frontbuffer_tracking@psr-suspend.html
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12928/shard-skl8/igt@kms_frontbuffer_tracking@psr-suspend.html
* igt@kms_plane@pixel-format-pipe-b-planes:
- shard-glk: [PASS][23] -> [SKIP][24] ([fdo#109271])
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/shard-glk9/igt@kms_plane@pixel-format-pipe-b-planes.html
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12928/shard-glk4/igt@kms_plane@pixel-format-pipe-b-planes.html
* igt@kms_plane@pixel-format-pipe-c-planes-source-clamping:
- shard-iclb: [PASS][25] -> [INCOMPLETE][26] ([fdo#107713] / [fdo#110036 ] / [fdo#110581])
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/shard-iclb1/igt@kms_plane@pixel-format-pipe-c-planes-source-clamping.html
[26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12928/shard-iclb6/igt@kms_plane@pixel-format-pipe-c-planes-source-clamping.html
* igt@kms_plane_lowres@pipe-a-tiling-x:
- shard-iclb: [PASS][27] -> [FAIL][28] ([fdo#103166])
[27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/shard-iclb3/igt@kms_plane_lowres@pipe-a-tiling-x.html
[28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12928/shard-iclb4/igt@kms_plane_lowres@pipe-a-tiling-x.html
* igt@kms_plane_scaling@pipe-c-scaler-with-pixel-format:
- shard-glk: [PASS][29] -> [SKIP][30] ([fdo#109271] / [fdo#109278])
[29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/shard-glk9/igt@kms_plane_scaling@pipe-c-scaler-with-pixel-format.html
[30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12928/shard-glk4/igt@kms_plane_scaling@pipe-c-scaler-with-pixel-format.html
* igt@kms_psr@psr2_cursor_render:
- shard-iclb: [PASS][31] -> [SKIP][32] ([fdo#109441]) +1 similar issue
[31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/shard-iclb2/igt@kms_psr@psr2_cursor_render.html
[32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12928/shard-iclb1/igt@kms_psr@psr2_cursor_render.html
* igt@kms_setmode@basic:
- shard-kbl: [PASS][33] -> [FAIL][34] ([fdo#99912])
[33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/shard-kbl3/igt@kms_setmode@basic.html
[34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12928/shard-kbl3/igt@kms_setmode@basic.html
* igt@tools_test@tools_test:
- shard-apl: [PASS][35] -> [SKIP][36] ([fdo#109271])
[35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/shard-apl7/igt@tools_test@tools_test.html
[36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12928/shard-apl4/igt@tools_test@tools_test.html
#### Possible fixes ####
* igt@gem_ctx_switch@basic-all-light:
- shard-iclb: [INCOMPLETE][37] ([fdo#107713] / [fdo#109100]) -> [PASS][38]
[37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/shard-iclb1/igt@gem_ctx_switch@basic-all-light.html
[38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12928/shard-iclb3/igt@gem_ctx_switch@basic-all-light.html
* igt@gem_exec_blt@dumb-buf-min:
- shard-hsw: [INCOMPLETE][39] ([fdo#103540]) -> [PASS][40]
[39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/shard-hsw6/igt@gem_exec_blt@dumb-buf-min.html
[40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12928/shard-hsw4/igt@gem_exec_blt@dumb-buf-min.html
* igt@gem_softpin@noreloc-s3:
- shard-kbl: [DMESG-WARN][41] ([fdo#108566]) -> [PASS][42]
[41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/shard-kbl1/igt@gem_softpin@noreloc-s3.html
[42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12928/shard-kbl6/igt@gem_softpin@noreloc-s3.html
* igt@gem_workarounds@suspend-resume:
- shard-apl: [DMESG-WARN][43] ([fdo#108566]) -> [PASS][44] +5 similar issues
[43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/shard-apl8/igt@gem_workarounds@suspend-resume.html
[44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12928/shard-apl5/igt@gem_workarounds@suspend-resume.html
* igt@i915_pm_rpm@system-suspend-execbuf:
- shard-skl: [INCOMPLETE][45] ([fdo#104108] / [fdo#107773] / [fdo#107807]) -> [PASS][46]
[45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/shard-skl10/igt@i915_pm_rpm@system-suspend-execbuf.html
[46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12928/shard-skl2/igt@i915_pm_rpm@system-suspend-execbuf.html
* igt@i915_selftest@mock_requests:
- shard-skl: [INCOMPLETE][47] ([fdo#110550]) -> [PASS][48]
[47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/shard-skl5/igt@i915_selftest@mock_requests.html
[48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12928/shard-skl5/igt@i915_selftest@mock_requests.html
* igt@kms_draw_crc@draw-method-xrgb2101010-render-xtiled:
- shard-skl: [FAIL][49] ([fdo#103184] / [fdo#103232]) -> [PASS][50]
[49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/shard-skl10/igt@kms_draw_crc@draw-method-xrgb2101010-render-xtiled.html
[50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12928/shard-skl8/igt@kms_draw_crc@draw-method-xrgb2101010-render-xtiled.html
* igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-indfb-draw-mmap-gtt:
- shard-iclb: [FAIL][51] ([fdo#103167]) -> [PASS][52] +1 similar issue
[51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/shard-iclb2/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-indfb-draw-mmap-gtt.html
[52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12928/shard-iclb5/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-indfb-draw-mmap-gtt.html
* igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-indfb-draw-pwrite:
- shard-skl: [FAIL][53] ([fdo#108040]) -> [PASS][54] +1 similar issue
[53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/shard-skl10/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-indfb-draw-pwrite.html
[54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12928/shard-skl8/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-indfb-draw-pwrite.html
* igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-move:
- shard-skl: [FAIL][55] ([fdo#103167]) -> [PASS][56]
[55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/shard-skl10/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-move.html
[56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12928/shard-skl8/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-move.html
* igt@kms_plane_alpha_blend@pipe-a-coverage-7efc:
- shard-skl: [FAIL][57] ([fdo#108145]) -> [PASS][58]
[57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/shard-skl10/igt@kms_plane_alpha_blend@pipe-a-coverage-7efc.html
[58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12928/shard-skl8/igt@kms_plane_alpha_blend@pipe-a-coverage-7efc.html
* igt@kms_plane_lowres@pipe-a-tiling-y:
- shard-iclb: [FAIL][59] ([fdo#103166]) -> [PASS][60]
[59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/shard-iclb6/igt@kms_plane_lowres@pipe-a-tiling-y.html
[60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12928/shard-iclb8/igt@kms_plane_lowres@pipe-a-tiling-y.html
* igt@kms_psr@no_drrs:
- shard-iclb: [FAIL][61] ([fdo#108341]) -> [PASS][62]
[61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/shard-iclb1/igt@kms_psr@no_drrs.html
[62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12928/shard-iclb6/igt@kms_psr@no_drrs.html
* igt@kms_setmode@basic:
- shard-apl: [FAIL][63] ([fdo#99912]) -> [PASS][64]
[63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/shard-apl3/igt@kms_setmode@basic.html
[64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12928/shard-apl1/igt@kms_setmode@basic.html
* igt@kms_vblank@pipe-a-wait-busy:
- shard-iclb: [INCOMPLETE][65] ([fdo#107713]) -> [PASS][66]
[65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/shard-iclb3/igt@kms_vblank@pipe-a-wait-busy.html
[66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12928/shard-iclb4/igt@kms_vblank@pipe-a-wait-busy.html
#### Warnings ####
* igt@kms_cursor_crc@cursor-256x256-suspend:
- shard-skl: [INCOMPLETE][67] ([fdo#104108]) -> [INCOMPLETE][68] ([fdo#104108] / [fdo#110581])
[67]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/shard-skl2/igt@kms_cursor_crc@cursor-256x256-suspend.html
[68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12928/shard-skl6/igt@kms_cursor_crc@cursor-256x256-suspend.html
* igt@kms_flip@2x-flip-vs-suspend:
- shard-glk: [INCOMPLETE][69] ([fdo#103359] / [k.org#198133]) -> [INCOMPLETE][70] ([fdo#103359] / [fdo#110581] / [k.org#198133])
[69]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/shard-glk9/igt@kms_flip@2x-flip-vs-suspend.html
[70]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12928/shard-glk7/igt@kms_flip@2x-flip-vs-suspend.html
* igt@kms_frontbuffer_tracking@psr-2p-scndscrn-spr-indfb-fullscreen:
- shard-skl: [SKIP][71] ([fdo#109271]) -> [INCOMPLETE][72] ([fdo#110581])
[71]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/shard-skl8/igt@kms_frontbuffer_tracking@psr-2p-scndscrn-spr-indfb-fullscreen.html
[72]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12928/shard-skl10/igt@kms_frontbuffer_tracking@psr-2p-scndscrn-spr-indfb-fullscreen.html
* igt@kms_plane_scaling@pipe-b-scaler-with-clipping-clamping:
- shard-skl: [INCOMPLETE][73] ([fdo#108972]) -> [INCOMPLETE][74] ([fdo#108972] / [fdo#110581])
[73]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6021/shard-skl2/igt@kms_plane_scaling@pipe-b-scaler-with-clipping-clamping.html
[74]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12928/shard-skl3/igt@kms_plane_scaling@pipe-b-scaler-with-clipping-clamping.html
[fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
[fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
[fdo#103184]: https://bugs.freedesktop.org/show_bug.cgi?id=103184
[fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
[fdo#103359]: https://bugs.freedesktop.org/show_bug.cgi?id=103359
[fdo#103540]: https://bugs.freedesktop.org/show_bug.cgi?id=103540
[fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
[fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
[fdo#106978]: https://bugs.freedesktop.org/show_bug.cgi?id=106978
[fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
[fdo#107773]: https://bugs.freedesktop.org/show_bug.cgi?id=107773
[fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807
[fdo#108040]: https://bugs.freedesktop.org/show_bug.cgi?id=108040
[fdo#108134]: https://bugs.freedesktop.org/show_bug.cgi?id=108134
[fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
[fdo#108341]: https://bugs.freedesktop.org/show_bug.cgi?id=108341
[fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
[fdo#108972]: https://bugs.freedesktop.org/show_bug.cgi?id=108972
[fdo#109100]: https://bugs.freedesktop.org/show_bug.cgi?id=109100
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
[fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
[fdo#109507]: https://bugs.freedesktop.org/show_bug.cgi?id=109507
[fdo#110036 ]: https://bugs.freedesktop.org/show_bug.cgi?id=110036
[fdo#110550]: https://bugs.freedesktop.org/show_bug.cgi?id=110550
[fdo#110581]: https://bugs.freedesktop.org/show_bug.cgi?id=110581
[fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912
[k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133
Participating hosts (10 -> 10)
------------------------------
No changes in participating hosts
Build changes
-------------
* Linux: CI_DRM_6021 -> Patchwork_12928
CI_DRM_6021: 850aa4220e8bf7609b03bf89bce146305704bec6 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4971: fc5e0467eb6913d21ad932aa8a31c77fdb5a9c77 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_12928: 01f589b56645790b5775e319e3f4ff827e465555 @ 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_12928/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH 05/14] drm/i915: Remove delay for idle_work
2019-05-01 11:45 ` [PATCH 05/14] drm/i915: Remove delay for idle_work Chris Wilson
@ 2019-05-02 13:19 ` Tvrtko Ursulin
2019-05-02 13:22 ` Chris Wilson
0 siblings, 1 reply; 52+ messages in thread
From: Tvrtko Ursulin @ 2019-05-02 13:19 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On 01/05/2019 12:45, Chris Wilson wrote:
> The original intent for the delay before running the idle_work was to
> provide a hysteresis to avoid ping-ponging the device runtime-pm. Since
> then we have also pulled in some memory management and general device
> management for parking. But with the inversion of the wakeref handling,
> GEM is no longer responsible for the wakeref and by the time we call the
> idle_work, the device is asleep. It seems appropriate now to drop the
> delay and just run the worker immediately to flush the cached GEM state
> before sleeping.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 2 +-
> drivers/gpu/drm/i915/i915_drv.h | 2 +-
> drivers/gpu/drm/i915/i915_gem_pm.c | 21 +++++++------------
> .../gpu/drm/i915/selftests/i915_gem_object.c | 2 +-
> .../gpu/drm/i915/selftests/mock_gem_device.c | 4 ++--
> 5 files changed, 12 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 0e4dffcd4da4..7e8898d0b78b 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -3935,8 +3935,8 @@ i915_drop_caches_set(void *data, u64 val)
> if (val & DROP_IDLE) {
> do {
> flush_delayed_work(&i915->gem.retire_work);
> - drain_delayed_work(&i915->gem.idle_work);
> } while (READ_ONCE(i915->gt.awake));
> + flush_work(&i915->gem.idle_work);
> }
>
> if (val & DROP_FREED)
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 13270e19eb87..cbf4a7d8bdae 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2035,7 +2035,7 @@ struct drm_i915_private {
> * arrive within a small period of time, we fire
> * off the idle_work.
> */
> - struct delayed_work idle_work;
> + struct work_struct idle_work;
> } gem;
>
> /* For i945gm vblank irq vs. C3 workaround */
> diff --git a/drivers/gpu/drm/i915/i915_gem_pm.c b/drivers/gpu/drm/i915/i915_gem_pm.c
> index 49b0ce594f20..ae91ad7cb31e 100644
> --- a/drivers/gpu/drm/i915/i915_gem_pm.c
> +++ b/drivers/gpu/drm/i915/i915_gem_pm.c
> @@ -29,12 +29,12 @@ static void i915_gem_park(struct drm_i915_private *i915)
> static void idle_work_handler(struct work_struct *work)
> {
> struct drm_i915_private *i915 =
> - container_of(work, typeof(*i915), gem.idle_work.work);
> + container_of(work, typeof(*i915), gem.idle_work);
>
> mutex_lock(&i915->drm.struct_mutex);
>
> intel_wakeref_lock(&i915->gt.wakeref);
> - if (!intel_wakeref_active(&i915->gt.wakeref))
> + if (!intel_wakeref_active(&i915->gt.wakeref) && !work_pending(work))
What is the reason for the !work_pending check?
Regards,
Tvrtko
> i915_gem_park(i915);
> intel_wakeref_unlock(&i915->gt.wakeref);
>
> @@ -74,9 +74,7 @@ static int pm_notifier(struct notifier_block *nb,
> break;
>
> case INTEL_GT_PARK:
> - mod_delayed_work(i915->wq,
> - &i915->gem.idle_work,
> - msecs_to_jiffies(100));
> + queue_work(i915->wq, &i915->gem.idle_work);
> break;
> }
>
> @@ -142,16 +140,11 @@ void i915_gem_suspend(struct drm_i915_private *i915)
> * Assert that we successfully flushed all the work and
> * reset the GPU back to its idle, low power state.
> */
> - GEM_BUG_ON(i915->gt.awake);
> - cancel_delayed_work_sync(&i915->gpu_error.hangcheck_work);
> -
> drain_delayed_work(&i915->gem.retire_work);
> + GEM_BUG_ON(i915->gt.awake);
> + flush_work(&i915->gem.idle_work);
>
> - /*
> - * As the idle_work is rearming if it detects a race, play safe and
> - * repeat the flush until it is definitely idle.
> - */
> - drain_delayed_work(&i915->gem.idle_work);
> + cancel_delayed_work_sync(&i915->gpu_error.hangcheck_work);
>
> i915_gem_drain_freed_objects(i915);
>
> @@ -242,7 +235,7 @@ void i915_gem_resume(struct drm_i915_private *i915)
>
> void i915_gem_init__pm(struct drm_i915_private *i915)
> {
> - INIT_DELAYED_WORK(&i915->gem.idle_work, idle_work_handler);
> + INIT_WORK(&i915->gem.idle_work, idle_work_handler);
> INIT_DELAYED_WORK(&i915->gem.retire_work, retire_work_handler);
>
> i915->gem.pm_notifier.notifier_call = pm_notifier;
> diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_object.c b/drivers/gpu/drm/i915/selftests/i915_gem_object.c
> index 088b2aa05dcd..b926d1cd165d 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_gem_object.c
> +++ b/drivers/gpu/drm/i915/selftests/i915_gem_object.c
> @@ -509,7 +509,7 @@ static void disable_retire_worker(struct drm_i915_private *i915)
> intel_gt_pm_get(i915);
>
> cancel_delayed_work_sync(&i915->gem.retire_work);
> - cancel_delayed_work_sync(&i915->gem.idle_work);
> + flush_work(&i915->gem.idle_work);
> }
>
> static void restore_retire_worker(struct drm_i915_private *i915)
> diff --git a/drivers/gpu/drm/i915/selftests/mock_gem_device.c b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
> index e4033d0576c4..d919f512042c 100644
> --- a/drivers/gpu/drm/i915/selftests/mock_gem_device.c
> +++ b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
> @@ -59,7 +59,7 @@ static void mock_device_release(struct drm_device *dev)
> mutex_unlock(&i915->drm.struct_mutex);
>
> drain_delayed_work(&i915->gem.retire_work);
> - drain_delayed_work(&i915->gem.idle_work);
> + flush_work(&i915->gem.idle_work);
> i915_gem_drain_workqueue(i915);
>
> mutex_lock(&i915->drm.struct_mutex);
> @@ -195,7 +195,7 @@ struct drm_i915_private *mock_gem_device(void)
> mock_init_contexts(i915);
>
> INIT_DELAYED_WORK(&i915->gem.retire_work, mock_retire_work_handler);
> - INIT_DELAYED_WORK(&i915->gem.idle_work, mock_idle_work_handler);
> + INIT_WORK(&i915->gem.idle_work, mock_idle_work_handler);
>
> i915->gt.awake = true;
>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH 05/14] drm/i915: Remove delay for idle_work
2019-05-02 13:19 ` Tvrtko Ursulin
@ 2019-05-02 13:22 ` Chris Wilson
2019-05-02 13:51 ` Tvrtko Ursulin
0 siblings, 1 reply; 52+ messages in thread
From: Chris Wilson @ 2019-05-02 13:22 UTC (permalink / raw)
To: Tvrtko Ursulin, intel-gfx
Quoting Tvrtko Ursulin (2019-05-02 14:19:38)
>
> On 01/05/2019 12:45, Chris Wilson wrote:
> > diff --git a/drivers/gpu/drm/i915/i915_gem_pm.c b/drivers/gpu/drm/i915/i915_gem_pm.c
> > index 49b0ce594f20..ae91ad7cb31e 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_pm.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_pm.c
> > @@ -29,12 +29,12 @@ static void i915_gem_park(struct drm_i915_private *i915)
> > static void idle_work_handler(struct work_struct *work)
> > {
> > struct drm_i915_private *i915 =
> > - container_of(work, typeof(*i915), gem.idle_work.work);
> > + container_of(work, typeof(*i915), gem.idle_work);
> >
> > mutex_lock(&i915->drm.struct_mutex);
> >
> > intel_wakeref_lock(&i915->gt.wakeref);
> > - if (!intel_wakeref_active(&i915->gt.wakeref))
> > + if (!intel_wakeref_active(&i915->gt.wakeref) && !work_pending(work))
>
> What is the reason for the !work_pending check?
Just that we are going to be called again, so wait until the next time to
see if we still need to park.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH 06/14] drm/i915: Cancel retire_worker on parking
2019-05-01 11:45 ` [PATCH 06/14] drm/i915: Cancel retire_worker on parking Chris Wilson
@ 2019-05-02 13:29 ` Tvrtko Ursulin
2019-05-02 13:33 ` Chris Wilson
2019-05-02 14:29 ` [PATCH v2] " Chris Wilson
1 sibling, 1 reply; 52+ messages in thread
From: Tvrtko Ursulin @ 2019-05-02 13:29 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On 01/05/2019 12:45, Chris Wilson wrote:
> Replace the racy continuation check within retire_work with a definite
> kill-switch on idling. The race was being exposed by gem_concurrent_blit
> where the retire_worker would be terminated too early leaving us
> spinning in debugfs/i915_drop_caches with nothing flushing the
> retirement queue.
>
> Although that the igt is trying to idle from one child while submitting
> from another may be a contributing factor as to why it runs so slowly...
>
> Testcase: igt/gem_concurrent_blit
> Fixes: 79ffac8599c4 ("drm/i915: Invert the GEM wakeref hierarchy")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
> drivers/gpu/drm/i915/i915_gem_pm.c | 18 ++++++++++++------
> .../gpu/drm/i915/selftests/mock_gem_device.c | 1 -
> 2 files changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_pm.c b/drivers/gpu/drm/i915/i915_gem_pm.c
> index ae91ad7cb31e..b239b55f84cd 100644
> --- a/drivers/gpu/drm/i915/i915_gem_pm.c
> +++ b/drivers/gpu/drm/i915/i915_gem_pm.c
> @@ -30,15 +30,23 @@ static void idle_work_handler(struct work_struct *work)
> {
> struct drm_i915_private *i915 =
> container_of(work, typeof(*i915), gem.idle_work);
> + bool restart = true;
>
> + cancel_delayed_work_sync(&i915->gem.retire_work);
> mutex_lock(&i915->drm.struct_mutex);
>
You don't want to run another retire here? Since the retire worker might
have just been canceled I thought you should.
Regards,
Tvrtko
> intel_wakeref_lock(&i915->gt.wakeref);
> - if (!intel_wakeref_active(&i915->gt.wakeref) && !work_pending(work))
> + if (!intel_wakeref_active(&i915->gt.wakeref) && !work_pending(work)) {
> i915_gem_park(i915);
> + restart = false;
> + }
> intel_wakeref_unlock(&i915->gt.wakeref);
>
> mutex_unlock(&i915->drm.struct_mutex);
> + if (restart)
> + queue_delayed_work(i915->wq,
> + &i915->gem.retire_work,
> + round_jiffies_up_relative(HZ));
> }
>
> static void retire_work_handler(struct work_struct *work)
> @@ -52,10 +60,9 @@ static void retire_work_handler(struct work_struct *work)
> mutex_unlock(&i915->drm.struct_mutex);
> }
>
> - if (intel_wakeref_active(&i915->gt.wakeref))
> - queue_delayed_work(i915->wq,
> - &i915->gem.retire_work,
> - round_jiffies_up_relative(HZ));
> + queue_delayed_work(i915->wq,
> + &i915->gem.retire_work,
> + round_jiffies_up_relative(HZ));
> }
>
> static int pm_notifier(struct notifier_block *nb,
> @@ -140,7 +147,6 @@ void i915_gem_suspend(struct drm_i915_private *i915)
> * Assert that we successfully flushed all the work and
> * reset the GPU back to its idle, low power state.
> */
> - drain_delayed_work(&i915->gem.retire_work);
> GEM_BUG_ON(i915->gt.awake);
> flush_work(&i915->gem.idle_work);
>
> diff --git a/drivers/gpu/drm/i915/selftests/mock_gem_device.c b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
> index d919f512042c..9fd02025d382 100644
> --- a/drivers/gpu/drm/i915/selftests/mock_gem_device.c
> +++ b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
> @@ -58,7 +58,6 @@ static void mock_device_release(struct drm_device *dev)
> i915_gem_contexts_lost(i915);
> mutex_unlock(&i915->drm.struct_mutex);
>
> - drain_delayed_work(&i915->gem.retire_work);
> flush_work(&i915->gem.idle_work);
> i915_gem_drain_workqueue(i915);
>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH 06/14] drm/i915: Cancel retire_worker on parking
2019-05-02 13:29 ` Tvrtko Ursulin
@ 2019-05-02 13:33 ` Chris Wilson
2019-05-02 14:20 ` Tvrtko Ursulin
0 siblings, 1 reply; 52+ messages in thread
From: Chris Wilson @ 2019-05-02 13:33 UTC (permalink / raw)
To: Tvrtko Ursulin, intel-gfx
Quoting Tvrtko Ursulin (2019-05-02 14:29:50)
>
> On 01/05/2019 12:45, Chris Wilson wrote:
> > Replace the racy continuation check within retire_work with a definite
> > kill-switch on idling. The race was being exposed by gem_concurrent_blit
> > where the retire_worker would be terminated too early leaving us
> > spinning in debugfs/i915_drop_caches with nothing flushing the
> > retirement queue.
> >
> > Although that the igt is trying to idle from one child while submitting
> > from another may be a contributing factor as to why it runs so slowly...
> >
> > Testcase: igt/gem_concurrent_blit
> > Fixes: 79ffac8599c4 ("drm/i915: Invert the GEM wakeref hierarchy")
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_gem_pm.c | 18 ++++++++++++------
> > .../gpu/drm/i915/selftests/mock_gem_device.c | 1 -
> > 2 files changed, 12 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_gem_pm.c b/drivers/gpu/drm/i915/i915_gem_pm.c
> > index ae91ad7cb31e..b239b55f84cd 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_pm.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_pm.c
> > @@ -30,15 +30,23 @@ static void idle_work_handler(struct work_struct *work)
> > {
> > struct drm_i915_private *i915 =
> > container_of(work, typeof(*i915), gem.idle_work);
> > + bool restart = true;
> >
> > + cancel_delayed_work_sync(&i915->gem.retire_work);
> > mutex_lock(&i915->drm.struct_mutex);
> >
>
> You don't want to run another retire here? Since the retire worker might
> have just been canceled I thought you should.
Why though? If there are retires outstanding, we won't sleep and want to
defer parking until after the next cycle.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH 07/14] drm/i915: Stop spinning for DROP_IDLE (debugfs/i915_drop_caches)
2019-05-01 11:45 ` [PATCH 07/14] drm/i915: Stop spinning for DROP_IDLE (debugfs/i915_drop_caches) Chris Wilson
@ 2019-05-02 13:34 ` Tvrtko Ursulin
2019-05-02 14:00 ` Chris Wilson
0 siblings, 1 reply; 52+ messages in thread
From: Tvrtko Ursulin @ 2019-05-02 13:34 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On 01/05/2019 12:45, Chris Wilson wrote:
> If the user is racing a call to debugfs/i915_drop_caches with ongoing
> submission from another thread/process, we may never end up idling the
> GPU and be uninterruptibly spinning in debugfs/i915_drop_caches trying
> to catch an idle moment.
>
> Just flush the work once, that should be enough to park the system under
> correct conditions. Outside of those we either have a driver bug or the
> user is racing themselves. Sadly, because the user may be provoking the
> unwanted situation we can't put a warn here to attract attention to a
> probable bug.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 7e8898d0b78b..2ecefacb1e66 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -3933,9 +3933,7 @@ i915_drop_caches_set(void *data, u64 val)
> fs_reclaim_release(GFP_KERNEL);
>
> if (val & DROP_IDLE) {
> - do {
> - flush_delayed_work(&i915->gem.retire_work);
> - } while (READ_ONCE(i915->gt.awake));
> + flush_delayed_work(&i915->gem.retire_work);
> flush_work(&i915->gem.idle_work);
> }
>
>
What were supposed to be semantics of DROP_IDLE? Now it seems rather
weak. Should it for instance also imply DROP_ACTIVE?
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH 02/14] drm/i915: Include fence signaled bit in print_request()
2019-05-01 11:45 ` [PATCH 02/14] drm/i915: Include fence signaled bit in print_request() Chris Wilson
@ 2019-05-02 13:43 ` Tvrtko Ursulin
0 siblings, 0 replies; 52+ messages in thread
From: Tvrtko Ursulin @ 2019-05-02 13:43 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On 01/05/2019 12:45, Chris Wilson wrote:
> Show the fence flags view of request completion in addition to the
> normal hwsp check and whether signaling is enabled.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
> drivers/gpu/drm/i915/gt/intel_engine_cs.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> index 6e40f8ea9a6a..f178f1268f4e 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> @@ -1229,8 +1229,11 @@ static void print_request(struct drm_printer *m,
> i915_request_completed(rq) ? "!" :
> i915_request_started(rq) ? "*" :
> "",
> + test_bit(DMA_FENCE_FLAG_SIGNALED_BIT,
> + &rq->fence.flags) ? "+" :
> test_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT,
> - &rq->fence.flags) ? "+" : "",
> + &rq->fence.flags) ? "-" :
> + "",
> buf,
> jiffies_to_msecs(jiffies - rq->emitted_jiffies),
> name);
>
We need a decoding cheat sheet now..
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Can we make it somewhat self-explanatory? Maybe only in my head..
">-|" (started/signaling enabled/completed)
">+" (started/signaled)
Imagine more fun if completed would be '<'. :)
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH 03/14] drm/i915/execlists: Flush the tasklet on parking
2019-05-01 11:45 ` [PATCH 03/14] drm/i915/execlists: Flush the tasklet on parking Chris Wilson
@ 2019-05-02 13:48 ` Tvrtko Ursulin
2019-05-02 13:53 ` Chris Wilson
0 siblings, 1 reply; 52+ messages in thread
From: Tvrtko Ursulin @ 2019-05-02 13:48 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On 01/05/2019 12:45, Chris Wilson wrote:
> Tidy up the cleanup sequence by always ensure that the tasklet is
> flushed on parking (before we cleanup). The parking provides a
> convenient point to ensure that the backend is truly idle.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
> drivers/gpu/drm/i915/gt/intel_lrc.c | 7 ++++++-
> drivers/gpu/drm/i915/intel_guc_submission.c | 1 +
> 2 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> index 851e62ddcb87..7be54b868d8e 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> @@ -2331,6 +2331,11 @@ static int gen8_init_rcs_context(struct i915_request *rq)
> return i915_gem_render_state_emit(rq);
> }
>
> +static void execlists_park(struct intel_engine_cs *engine)
> +{
> + tasklet_kill(&engine->execlists.tasklet);
Isn't it actually a problem if tasklet is scheduled and unstarted, or
even in progress at the point of engine getting parked?
Regards,
Tvrtko
> +}
> +
> void intel_execlists_set_default_submission(struct intel_engine_cs *engine)
> {
> engine->submit_request = execlists_submit_request;
> @@ -2342,7 +2347,7 @@ void intel_execlists_set_default_submission(struct intel_engine_cs *engine)
> engine->reset.reset = execlists_reset;
> engine->reset.finish = execlists_reset_finish;
>
> - engine->park = NULL;
> + engine->park = execlists_park;
> engine->unpark = NULL;
>
> engine->flags |= I915_ENGINE_SUPPORTS_STATS;
> diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c
> index 4c814344809c..ed94001028f2 100644
> --- a/drivers/gpu/drm/i915/intel_guc_submission.c
> +++ b/drivers/gpu/drm/i915/intel_guc_submission.c
> @@ -1363,6 +1363,7 @@ static void guc_interrupts_release(struct drm_i915_private *dev_priv)
>
> static void guc_submission_park(struct intel_engine_cs *engine)
> {
> + tasklet_kill(&engine->execlists.tasklet);
> intel_engine_unpin_breadcrumbs_irq(engine);
> engine->flags &= ~I915_ENGINE_NEEDS_BREADCRUMB_TASKLET;
> }
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH 05/14] drm/i915: Remove delay for idle_work
2019-05-02 13:22 ` Chris Wilson
@ 2019-05-02 13:51 ` Tvrtko Ursulin
2019-05-02 14:23 ` Chris Wilson
0 siblings, 1 reply; 52+ messages in thread
From: Tvrtko Ursulin @ 2019-05-02 13:51 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On 02/05/2019 14:22, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-05-02 14:19:38)
>>
>> On 01/05/2019 12:45, Chris Wilson wrote:
>>> diff --git a/drivers/gpu/drm/i915/i915_gem_pm.c b/drivers/gpu/drm/i915/i915_gem_pm.c
>>> index 49b0ce594f20..ae91ad7cb31e 100644
>>> --- a/drivers/gpu/drm/i915/i915_gem_pm.c
>>> +++ b/drivers/gpu/drm/i915/i915_gem_pm.c
>>> @@ -29,12 +29,12 @@ static void i915_gem_park(struct drm_i915_private *i915)
>>> static void idle_work_handler(struct work_struct *work)
>>> {
>>> struct drm_i915_private *i915 =
>>> - container_of(work, typeof(*i915), gem.idle_work.work);
>>> + container_of(work, typeof(*i915), gem.idle_work);
>>>
>>> mutex_lock(&i915->drm.struct_mutex);
>>>
>>> intel_wakeref_lock(&i915->gt.wakeref);
>>> - if (!intel_wakeref_active(&i915->gt.wakeref))
>>> + if (!intel_wakeref_active(&i915->gt.wakeref) && !work_pending(work))
>>
>> What is the reason for the !work_pending check?
>
> Just that we are going to be called again, so wait until the next time to
> see if we still need to park.
When does it get called again? If a whole new cycle of unpark-park
happened before the previous park was able to finish?
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH 03/14] drm/i915/execlists: Flush the tasklet on parking
2019-05-02 13:48 ` Tvrtko Ursulin
@ 2019-05-02 13:53 ` Chris Wilson
2019-05-02 14:14 ` Tvrtko Ursulin
0 siblings, 1 reply; 52+ messages in thread
From: Chris Wilson @ 2019-05-02 13:53 UTC (permalink / raw)
To: Tvrtko Ursulin, intel-gfx
Quoting Tvrtko Ursulin (2019-05-02 14:48:18)
>
> On 01/05/2019 12:45, Chris Wilson wrote:
> > Tidy up the cleanup sequence by always ensure that the tasklet is
> > flushed on parking (before we cleanup). The parking provides a
> > convenient point to ensure that the backend is truly idle.
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> > drivers/gpu/drm/i915/gt/intel_lrc.c | 7 ++++++-
> > drivers/gpu/drm/i915/intel_guc_submission.c | 1 +
> > 2 files changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> > index 851e62ddcb87..7be54b868d8e 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> > @@ -2331,6 +2331,11 @@ static int gen8_init_rcs_context(struct i915_request *rq)
> > return i915_gem_render_state_emit(rq);
> > }
> >
> > +static void execlists_park(struct intel_engine_cs *engine)
> > +{
> > + tasklet_kill(&engine->execlists.tasklet);
>
> Isn't it actually a problem if tasklet is scheduled and unstarted, or
> even in progress at the point of engine getting parked?
That would be a broken driver. :|
We must be quite sure that engine isn't going to send an interrupt as we
are just about to drop the wakeref we need to service that interrupt.
tasklet_kill()
GEM_BUG_ON(engine->execlists.active);
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH 07/14] drm/i915: Stop spinning for DROP_IDLE (debugfs/i915_drop_caches)
2019-05-02 13:34 ` Tvrtko Ursulin
@ 2019-05-02 14:00 ` Chris Wilson
2019-05-02 14:16 ` Tvrtko Ursulin
0 siblings, 1 reply; 52+ messages in thread
From: Chris Wilson @ 2019-05-02 14:00 UTC (permalink / raw)
To: Tvrtko Ursulin, intel-gfx
Quoting Tvrtko Ursulin (2019-05-02 14:34:11)
>
> On 01/05/2019 12:45, Chris Wilson wrote:
> > If the user is racing a call to debugfs/i915_drop_caches with ongoing
> > submission from another thread/process, we may never end up idling the
> > GPU and be uninterruptibly spinning in debugfs/i915_drop_caches trying
> > to catch an idle moment.
> >
> > Just flush the work once, that should be enough to park the system under
> > correct conditions. Outside of those we either have a driver bug or the
> > user is racing themselves. Sadly, because the user may be provoking the
> > unwanted situation we can't put a warn here to attract attention to a
> > probable bug.
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> > drivers/gpu/drm/i915/i915_debugfs.c | 4 +---
> > 1 file changed, 1 insertion(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> > index 7e8898d0b78b..2ecefacb1e66 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -3933,9 +3933,7 @@ i915_drop_caches_set(void *data, u64 val)
> > fs_reclaim_release(GFP_KERNEL);
> >
> > if (val & DROP_IDLE) {
> > - do {
> > - flush_delayed_work(&i915->gem.retire_work);
> > - } while (READ_ONCE(i915->gt.awake));
> > + flush_delayed_work(&i915->gem.retire_work);
> > flush_work(&i915->gem.idle_work);
> > }
> >
> >
>
> What were supposed to be semantics of DROP_IDLE? Now it seems rather
> weak. Should it for instance also imply DROP_ACTIVE?
All I need for DROP_IDLE is that the idle worker is flushed. I've always
assumed you would pass in DROP_ACTIVE | DROP_RETIRE | DROP_IDLE as the
trifecta.
The biggest problem here is that's it is an uninterruptible loop.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH 03/14] drm/i915/execlists: Flush the tasklet on parking
2019-05-02 13:53 ` Chris Wilson
@ 2019-05-02 14:14 ` Tvrtko Ursulin
2019-05-02 14:21 ` Chris Wilson
0 siblings, 1 reply; 52+ messages in thread
From: Tvrtko Ursulin @ 2019-05-02 14:14 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On 02/05/2019 14:53, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-05-02 14:48:18)
>>
>> On 01/05/2019 12:45, Chris Wilson wrote:
>>> Tidy up the cleanup sequence by always ensure that the tasklet is
>>> flushed on parking (before we cleanup). The parking provides a
>>> convenient point to ensure that the backend is truly idle.
>>>
>>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>>> ---
>>> drivers/gpu/drm/i915/gt/intel_lrc.c | 7 ++++++-
>>> drivers/gpu/drm/i915/intel_guc_submission.c | 1 +
>>> 2 files changed, 7 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
>>> index 851e62ddcb87..7be54b868d8e 100644
>>> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
>>> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
>>> @@ -2331,6 +2331,11 @@ static int gen8_init_rcs_context(struct i915_request *rq)
>>> return i915_gem_render_state_emit(rq);
>>> }
>>>
>>> +static void execlists_park(struct intel_engine_cs *engine)
>>> +{
>>> + tasklet_kill(&engine->execlists.tasklet);
>>
>> Isn't it actually a problem if tasklet is scheduled and unstarted, or
>> even in progress at the point of engine getting parked?
>
> That would be a broken driver. :|
>
> We must be quite sure that engine isn't going to send an interrupt as we
> are just about to drop the wakeref we need to service that interrupt.
>
> tasklet_kill()
> GEM_BUG_ON(engine->execlists.active);
Or instead of both:
/* Tasklet must not be running or scheduled at this point. */
GEM_BUG_ON(engine->execlists.tasklet.state);
?
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH 07/14] drm/i915: Stop spinning for DROP_IDLE (debugfs/i915_drop_caches)
2019-05-02 14:00 ` Chris Wilson
@ 2019-05-02 14:16 ` Tvrtko Ursulin
0 siblings, 0 replies; 52+ messages in thread
From: Tvrtko Ursulin @ 2019-05-02 14:16 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On 02/05/2019 15:00, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-05-02 14:34:11)
>>
>> On 01/05/2019 12:45, Chris Wilson wrote:
>>> If the user is racing a call to debugfs/i915_drop_caches with ongoing
>>> submission from another thread/process, we may never end up idling the
>>> GPU and be uninterruptibly spinning in debugfs/i915_drop_caches trying
>>> to catch an idle moment.
>>>
>>> Just flush the work once, that should be enough to park the system under
>>> correct conditions. Outside of those we either have a driver bug or the
>>> user is racing themselves. Sadly, because the user may be provoking the
>>> unwanted situation we can't put a warn here to attract attention to a
>>> probable bug.
>>>
>>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>>> ---
>>> drivers/gpu/drm/i915/i915_debugfs.c | 4 +---
>>> 1 file changed, 1 insertion(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
>>> index 7e8898d0b78b..2ecefacb1e66 100644
>>> --- a/drivers/gpu/drm/i915/i915_debugfs.c
>>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
>>> @@ -3933,9 +3933,7 @@ i915_drop_caches_set(void *data, u64 val)
>>> fs_reclaim_release(GFP_KERNEL);
>>>
>>> if (val & DROP_IDLE) {
>>> - do {
>>> - flush_delayed_work(&i915->gem.retire_work);
>>> - } while (READ_ONCE(i915->gt.awake));
>>> + flush_delayed_work(&i915->gem.retire_work);
>>> flush_work(&i915->gem.idle_work);
>>> }
>>>
>>>
>>
>> What were supposed to be semantics of DROP_IDLE? Now it seems rather
>> weak. Should it for instance also imply DROP_ACTIVE?
>
> All I need for DROP_IDLE is that the idle worker is flushed. I've always
> assumed you would pass in DROP_ACTIVE | DROP_RETIRE | DROP_IDLE as the
> trifecta.
>
> The biggest problem here is that's it is an uninterruptible loop.
Okay.. lets see if IGT is playing ball. :)
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH 04/14] drm/i915: Leave engine parking to the engines
2019-05-01 11:45 ` [PATCH 04/14] drm/i915: Leave engine parking to the engines Chris Wilson
@ 2019-05-02 14:18 ` Tvrtko Ursulin
0 siblings, 0 replies; 52+ messages in thread
From: Tvrtko Ursulin @ 2019-05-02 14:18 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On 01/05/2019 12:45, Chris Wilson wrote:
> Drop the check in GEM parking that the engines were already parked. The
> intention here was that before we dropped the GT wakeref, we were sure
> that no more interrupts could be raised -- however, we have already
> dropped the wakeref by this point and the warning is no longer valid.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
> drivers/gpu/drm/i915/i915_gem_pm.c | 18 +-----------------
> 1 file changed, 1 insertion(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_pm.c b/drivers/gpu/drm/i915/i915_gem_pm.c
> index 3b6e8d5be8e1..49b0ce594f20 100644
> --- a/drivers/gpu/drm/i915/i915_gem_pm.c
> +++ b/drivers/gpu/drm/i915/i915_gem_pm.c
> @@ -17,24 +17,8 @@ static void i915_gem_park(struct drm_i915_private *i915)
>
> lockdep_assert_held(&i915->drm.struct_mutex);
>
> - for_each_engine(engine, i915, id) {
> - /*
> - * We are committed now to parking the engines, make sure there
> - * will be no more interrupts arriving later and the engines
> - * are truly idle.
> - */
> - if (wait_for(intel_engine_is_idle(engine), 10)) {
> - struct drm_printer p = drm_debug_printer(__func__);
> -
> - dev_err(i915->drm.dev,
> - "%s is not idle before parking\n",
> - engine->name);
> - intel_engine_dump(engine, &p, NULL);
> - }
> - tasklet_kill(&engine->execlists.tasklet);
> -
> + for_each_engine(engine, i915, id)
> i915_gem_batch_pool_fini(&engine->batch_pool);
> - }
>
> i915_timelines_park(i915);
> i915_vma_parked(i915);
>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH 06/14] drm/i915: Cancel retire_worker on parking
2019-05-02 13:33 ` Chris Wilson
@ 2019-05-02 14:20 ` Tvrtko Ursulin
2019-05-02 14:26 ` Chris Wilson
0 siblings, 1 reply; 52+ messages in thread
From: Tvrtko Ursulin @ 2019-05-02 14:20 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On 02/05/2019 14:33, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-05-02 14:29:50)
>>
>> On 01/05/2019 12:45, Chris Wilson wrote:
>>> Replace the racy continuation check within retire_work with a definite
>>> kill-switch on idling. The race was being exposed by gem_concurrent_blit
>>> where the retire_worker would be terminated too early leaving us
>>> spinning in debugfs/i915_drop_caches with nothing flushing the
>>> retirement queue.
>>>
>>> Although that the igt is trying to idle from one child while submitting
>>> from another may be a contributing factor as to why it runs so slowly...
>>>
>>> Testcase: igt/gem_concurrent_blit
>>> Fixes: 79ffac8599c4 ("drm/i915: Invert the GEM wakeref hierarchy")
>>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>>> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>> ---
>>> drivers/gpu/drm/i915/i915_gem_pm.c | 18 ++++++++++++------
>>> .../gpu/drm/i915/selftests/mock_gem_device.c | 1 -
>>> 2 files changed, 12 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_gem_pm.c b/drivers/gpu/drm/i915/i915_gem_pm.c
>>> index ae91ad7cb31e..b239b55f84cd 100644
>>> --- a/drivers/gpu/drm/i915/i915_gem_pm.c
>>> +++ b/drivers/gpu/drm/i915/i915_gem_pm.c
>>> @@ -30,15 +30,23 @@ static void idle_work_handler(struct work_struct *work)
>>> {
>>> struct drm_i915_private *i915 =
>>> container_of(work, typeof(*i915), gem.idle_work);
>>> + bool restart = true;
>>>
>>> + cancel_delayed_work_sync(&i915->gem.retire_work);
>>> mutex_lock(&i915->drm.struct_mutex);
>>>
>>
>> You don't want to run another retire here? Since the retire worker might
>> have just been canceled I thought you should.
>
> Why though? If there are retires outstanding, we won't sleep and want to
> defer parking until after the next cycle.
In this case what is the point of cancel_delayed_work_*sync* and not
just the async cancel?
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH 03/14] drm/i915/execlists: Flush the tasklet on parking
2019-05-02 14:14 ` Tvrtko Ursulin
@ 2019-05-02 14:21 ` Chris Wilson
2019-05-02 14:24 ` Tvrtko Ursulin
0 siblings, 1 reply; 52+ messages in thread
From: Chris Wilson @ 2019-05-02 14:21 UTC (permalink / raw)
To: Tvrtko Ursulin, intel-gfx
Quoting Tvrtko Ursulin (2019-05-02 15:14:08)
>
> On 02/05/2019 14:53, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2019-05-02 14:48:18)
> >>
> >> On 01/05/2019 12:45, Chris Wilson wrote:
> >>> Tidy up the cleanup sequence by always ensure that the tasklet is
> >>> flushed on parking (before we cleanup). The parking provides a
> >>> convenient point to ensure that the backend is truly idle.
> >>>
> >>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> >>> ---
> >>> drivers/gpu/drm/i915/gt/intel_lrc.c | 7 ++++++-
> >>> drivers/gpu/drm/i915/intel_guc_submission.c | 1 +
> >>> 2 files changed, 7 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> >>> index 851e62ddcb87..7be54b868d8e 100644
> >>> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> >>> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> >>> @@ -2331,6 +2331,11 @@ static int gen8_init_rcs_context(struct i915_request *rq)
> >>> return i915_gem_render_state_emit(rq);
> >>> }
> >>>
> >>> +static void execlists_park(struct intel_engine_cs *engine)
> >>> +{
> >>> + tasklet_kill(&engine->execlists.tasklet);
> >>
> >> Isn't it actually a problem if tasklet is scheduled and unstarted, or
> >> even in progress at the point of engine getting parked?
> >
> > That would be a broken driver. :|
> >
> > We must be quite sure that engine isn't going to send an interrupt as we
> > are just about to drop the wakeref we need to service that interrupt.
> >
> > tasklet_kill()
> > GEM_BUG_ON(engine->execlists.active);
>
> Or instead of both:
>
> /* Tasklet must not be running or scheduled at this point. */
> GEM_BUG_ON(engine->execlists.tasklet.state);
There's the dilemma that we start parking based on retirement not
final CS event.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH 05/14] drm/i915: Remove delay for idle_work
2019-05-02 13:51 ` Tvrtko Ursulin
@ 2019-05-02 14:23 ` Chris Wilson
0 siblings, 0 replies; 52+ messages in thread
From: Chris Wilson @ 2019-05-02 14:23 UTC (permalink / raw)
To: Tvrtko Ursulin, intel-gfx
Quoting Tvrtko Ursulin (2019-05-02 14:51:31)
>
> On 02/05/2019 14:22, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2019-05-02 14:19:38)
> >>
> >> On 01/05/2019 12:45, Chris Wilson wrote:
> >>> diff --git a/drivers/gpu/drm/i915/i915_gem_pm.c b/drivers/gpu/drm/i915/i915_gem_pm.c
> >>> index 49b0ce594f20..ae91ad7cb31e 100644
> >>> --- a/drivers/gpu/drm/i915/i915_gem_pm.c
> >>> +++ b/drivers/gpu/drm/i915/i915_gem_pm.c
> >>> @@ -29,12 +29,12 @@ static void i915_gem_park(struct drm_i915_private *i915)
> >>> static void idle_work_handler(struct work_struct *work)
> >>> {
> >>> struct drm_i915_private *i915 =
> >>> - container_of(work, typeof(*i915), gem.idle_work.work);
> >>> + container_of(work, typeof(*i915), gem.idle_work);
> >>>
> >>> mutex_lock(&i915->drm.struct_mutex);
> >>>
> >>> intel_wakeref_lock(&i915->gt.wakeref);
> >>> - if (!intel_wakeref_active(&i915->gt.wakeref))
> >>> + if (!intel_wakeref_active(&i915->gt.wakeref) && !work_pending(work))
> >>
> >> What is the reason for the !work_pending check?
> >
> > Just that we are going to be called again, so wait until the next time to
> > see if we still need to park.
>
> When does it get called again? If a whole new cycle of unpark-park
> happened before the previous park was able to finish?
work_pending() implies that we've done at least one cycle while we
waited for the locks and the work is already queued to be rerun.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH 03/14] drm/i915/execlists: Flush the tasklet on parking
2019-05-02 14:21 ` Chris Wilson
@ 2019-05-02 14:24 ` Tvrtko Ursulin
2019-05-02 14:33 ` Chris Wilson
0 siblings, 1 reply; 52+ messages in thread
From: Tvrtko Ursulin @ 2019-05-02 14:24 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On 02/05/2019 15:21, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-05-02 15:14:08)
>>
>> On 02/05/2019 14:53, Chris Wilson wrote:
>>> Quoting Tvrtko Ursulin (2019-05-02 14:48:18)
>>>>
>>>> On 01/05/2019 12:45, Chris Wilson wrote:
>>>>> Tidy up the cleanup sequence by always ensure that the tasklet is
>>>>> flushed on parking (before we cleanup). The parking provides a
>>>>> convenient point to ensure that the backend is truly idle.
>>>>>
>>>>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>>>>> ---
>>>>> drivers/gpu/drm/i915/gt/intel_lrc.c | 7 ++++++-
>>>>> drivers/gpu/drm/i915/intel_guc_submission.c | 1 +
>>>>> 2 files changed, 7 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
>>>>> index 851e62ddcb87..7be54b868d8e 100644
>>>>> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
>>>>> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
>>>>> @@ -2331,6 +2331,11 @@ static int gen8_init_rcs_context(struct i915_request *rq)
>>>>> return i915_gem_render_state_emit(rq);
>>>>> }
>>>>>
>>>>> +static void execlists_park(struct intel_engine_cs *engine)
>>>>> +{
>>>>> + tasklet_kill(&engine->execlists.tasklet);
>>>>
>>>> Isn't it actually a problem if tasklet is scheduled and unstarted, or
>>>> even in progress at the point of engine getting parked?
>>>
>>> That would be a broken driver. :|
>>>
>>> We must be quite sure that engine isn't going to send an interrupt as we
>>> are just about to drop the wakeref we need to service that interrupt.
>>>
>>> tasklet_kill()
>>> GEM_BUG_ON(engine->execlists.active);
>>
>> Or instead of both:
>>
>> /* Tasklet must not be running or scheduled at this point. */
>> GEM_BUG_ON(engine->execlists.tasklet.state);
>
> There's the dilemma that we start parking based on retirement not
> final CS event.
But engine->park() is called once the last engine pm reference is
dropped. Are we dropping the last reference with a CS event pending?
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH 06/14] drm/i915: Cancel retire_worker on parking
2019-05-02 14:20 ` Tvrtko Ursulin
@ 2019-05-02 14:26 ` Chris Wilson
0 siblings, 0 replies; 52+ messages in thread
From: Chris Wilson @ 2019-05-02 14:26 UTC (permalink / raw)
To: Tvrtko Ursulin, intel-gfx
Quoting Tvrtko Ursulin (2019-05-02 15:20:52)
>
> On 02/05/2019 14:33, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2019-05-02 14:29:50)
> >>
> >> On 01/05/2019 12:45, Chris Wilson wrote:
> >>> Replace the racy continuation check within retire_work with a definite
> >>> kill-switch on idling. The race was being exposed by gem_concurrent_blit
> >>> where the retire_worker would be terminated too early leaving us
> >>> spinning in debugfs/i915_drop_caches with nothing flushing the
> >>> retirement queue.
> >>>
> >>> Although that the igt is trying to idle from one child while submitting
> >>> from another may be a contributing factor as to why it runs so slowly...
> >>>
> >>> Testcase: igt/gem_concurrent_blit
> >>> Fixes: 79ffac8599c4 ("drm/i915: Invert the GEM wakeref hierarchy")
> >>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> >>> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>> ---
> >>> drivers/gpu/drm/i915/i915_gem_pm.c | 18 ++++++++++++------
> >>> .../gpu/drm/i915/selftests/mock_gem_device.c | 1 -
> >>> 2 files changed, 12 insertions(+), 7 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/i915/i915_gem_pm.c b/drivers/gpu/drm/i915/i915_gem_pm.c
> >>> index ae91ad7cb31e..b239b55f84cd 100644
> >>> --- a/drivers/gpu/drm/i915/i915_gem_pm.c
> >>> +++ b/drivers/gpu/drm/i915/i915_gem_pm.c
> >>> @@ -30,15 +30,23 @@ static void idle_work_handler(struct work_struct *work)
> >>> {
> >>> struct drm_i915_private *i915 =
> >>> container_of(work, typeof(*i915), gem.idle_work);
> >>> + bool restart = true;
> >>>
> >>> + cancel_delayed_work_sync(&i915->gem.retire_work);
> >>> mutex_lock(&i915->drm.struct_mutex);
> >>>
> >>
> >> You don't want to run another retire here? Since the retire worker might
> >> have just been canceled I thought you should.
> >
> > Why though? If there are retires outstanding, we won't sleep and want to
> > defer parking until after the next cycle.
>
> In this case what is the point of cancel_delayed_work_*sync* and not
> just the async cancel?
There's an non-sync version? Ah ha!
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 52+ messages in thread
* [PATCH v2] drm/i915: Cancel retire_worker on parking
2019-05-01 11:45 ` [PATCH 06/14] drm/i915: Cancel retire_worker on parking Chris Wilson
2019-05-02 13:29 ` Tvrtko Ursulin
@ 2019-05-02 14:29 ` Chris Wilson
1 sibling, 0 replies; 52+ messages in thread
From: Chris Wilson @ 2019-05-02 14:29 UTC (permalink / raw)
To: intel-gfx
Replace the racy continuation check within retire_work with a definite
kill-switch on idling. The race was being exposed by gem_concurrent_blit
where the retire_worker would be terminated too early leaving us
spinning in debugfs/i915_drop_caches with nothing flushing the
retirement queue.
Although that the igt is trying to idle from one child while submitting
from another may be a contributing factor as to why it runs so slowly...
v2: Use the non-sync version of cancel_delayed_work(), we only need to
stop it from being scheduled as we independently check whether now is
the right time to be parking.
Testcase: igt/gem_concurrent_blit
Fixes: 79ffac8599c4 ("drm/i915: Invert the GEM wakeref hierarchy")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/i915_gem_pm.c | 18 ++++++++++++------
.../gpu/drm/i915/selftests/mock_gem_device.c | 1 -
2 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_pm.c b/drivers/gpu/drm/i915/i915_gem_pm.c
index ae91ad7cb31e..fa9c2ebd966a 100644
--- a/drivers/gpu/drm/i915/i915_gem_pm.c
+++ b/drivers/gpu/drm/i915/i915_gem_pm.c
@@ -30,15 +30,23 @@ static void idle_work_handler(struct work_struct *work)
{
struct drm_i915_private *i915 =
container_of(work, typeof(*i915), gem.idle_work);
+ bool restart = true;
+ cancel_delayed_work(&i915->gem.retire_work);
mutex_lock(&i915->drm.struct_mutex);
intel_wakeref_lock(&i915->gt.wakeref);
- if (!intel_wakeref_active(&i915->gt.wakeref) && !work_pending(work))
+ if (!intel_wakeref_active(&i915->gt.wakeref) && !work_pending(work)) {
i915_gem_park(i915);
+ restart = false;
+ }
intel_wakeref_unlock(&i915->gt.wakeref);
mutex_unlock(&i915->drm.struct_mutex);
+ if (restart)
+ queue_delayed_work(i915->wq,
+ &i915->gem.retire_work,
+ round_jiffies_up_relative(HZ));
}
static void retire_work_handler(struct work_struct *work)
@@ -52,10 +60,9 @@ static void retire_work_handler(struct work_struct *work)
mutex_unlock(&i915->drm.struct_mutex);
}
- if (intel_wakeref_active(&i915->gt.wakeref))
- queue_delayed_work(i915->wq,
- &i915->gem.retire_work,
- round_jiffies_up_relative(HZ));
+ queue_delayed_work(i915->wq,
+ &i915->gem.retire_work,
+ round_jiffies_up_relative(HZ));
}
static int pm_notifier(struct notifier_block *nb,
@@ -140,7 +147,6 @@ void i915_gem_suspend(struct drm_i915_private *i915)
* Assert that we successfully flushed all the work and
* reset the GPU back to its idle, low power state.
*/
- drain_delayed_work(&i915->gem.retire_work);
GEM_BUG_ON(i915->gt.awake);
flush_work(&i915->gem.idle_work);
diff --git a/drivers/gpu/drm/i915/selftests/mock_gem_device.c b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
index d919f512042c..9fd02025d382 100644
--- a/drivers/gpu/drm/i915/selftests/mock_gem_device.c
+++ b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
@@ -58,7 +58,6 @@ static void mock_device_release(struct drm_device *dev)
i915_gem_contexts_lost(i915);
mutex_unlock(&i915->drm.struct_mutex);
- drain_delayed_work(&i915->gem.retire_work);
flush_work(&i915->gem.idle_work);
i915_gem_drain_workqueue(i915);
--
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] 52+ messages in thread
* Re: [PATCH 03/14] drm/i915/execlists: Flush the tasklet on parking
2019-05-02 14:24 ` Tvrtko Ursulin
@ 2019-05-02 14:33 ` Chris Wilson
0 siblings, 0 replies; 52+ messages in thread
From: Chris Wilson @ 2019-05-02 14:33 UTC (permalink / raw)
To: Tvrtko Ursulin, intel-gfx
Quoting Tvrtko Ursulin (2019-05-02 15:24:16)
>
> On 02/05/2019 15:21, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2019-05-02 15:14:08)
> >>
> >> On 02/05/2019 14:53, Chris Wilson wrote:
> >>> Quoting Tvrtko Ursulin (2019-05-02 14:48:18)
> >>>>
> >>>> On 01/05/2019 12:45, Chris Wilson wrote:
> >>>>> Tidy up the cleanup sequence by always ensure that the tasklet is
> >>>>> flushed on parking (before we cleanup). The parking provides a
> >>>>> convenient point to ensure that the backend is truly idle.
> >>>>>
> >>>>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> >>>>> ---
> >>>>> drivers/gpu/drm/i915/gt/intel_lrc.c | 7 ++++++-
> >>>>> drivers/gpu/drm/i915/intel_guc_submission.c | 1 +
> >>>>> 2 files changed, 7 insertions(+), 1 deletion(-)
> >>>>>
> >>>>> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> >>>>> index 851e62ddcb87..7be54b868d8e 100644
> >>>>> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> >>>>> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> >>>>> @@ -2331,6 +2331,11 @@ static int gen8_init_rcs_context(struct i915_request *rq)
> >>>>> return i915_gem_render_state_emit(rq);
> >>>>> }
> >>>>>
> >>>>> +static void execlists_park(struct intel_engine_cs *engine)
> >>>>> +{
> >>>>> + tasklet_kill(&engine->execlists.tasklet);
> >>>>
> >>>> Isn't it actually a problem if tasklet is scheduled and unstarted, or
> >>>> even in progress at the point of engine getting parked?
> >>>
> >>> That would be a broken driver. :|
> >>>
> >>> We must be quite sure that engine isn't going to send an interrupt as we
> >>> are just about to drop the wakeref we need to service that interrupt.
> >>>
> >>> tasklet_kill()
> >>> GEM_BUG_ON(engine->execlists.active);
> >>
> >> Or instead of both:
> >>
> >> /* Tasklet must not be running or scheduled at this point. */
> >> GEM_BUG_ON(engine->execlists.tasklet.state);
> >
> > There's the dilemma that we start parking based on retirement not
> > final CS event.
>
> But engine->park() is called once the last engine pm reference is
> dropped. Are we dropping the last reference with a CS event pending?
Potentially we are.
i915_request_retire() -> context->exit() -> engine->park()
At no point along that chain do we actually check we have flushed the
backend. The tasklet_kill() would flush if the interrupt had already
been sent, but that's not very strict.
Oh well, you've talked me into to re-adding the wait loop here.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 52+ messages in thread
* ✗ Fi.CI.BAT: failure for series starting with [01/14] drm/i915/hangcheck: Track context changes (rev5)
2019-05-01 11:45 [PATCH 01/14] drm/i915/hangcheck: Track context changes Chris Wilson
` (21 preceding siblings ...)
2019-05-02 9:35 ` ✗ Fi.CI.IGT: failure " Patchwork
@ 2019-05-02 16:45 ` Patchwork
2019-05-03 10:36 ` [PATCH 01/14] drm/i915/hangcheck: Track context changes Tvrtko Ursulin
23 siblings, 0 replies; 52+ messages in thread
From: Patchwork @ 2019-05-02 16:45 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [01/14] drm/i915/hangcheck: Track context changes (rev5)
URL : https://patchwork.freedesktop.org/series/60153/
State : failure
== Summary ==
Applying: drm/i915/hangcheck: Track context changes
Applying: drm/i915: Include fence signaled bit in print_request()
Using index info to reconstruct a base tree...
M drivers/gpu/drm/i915/gt/intel_engine_cs.c
Falling back to patching base and 3-way merge...
Auto-merging drivers/gpu/drm/i915/gt/intel_engine_cs.c
CONFLICT (content): Merge conflict in drivers/gpu/drm/i915/gt/intel_engine_cs.c
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch' to see the failed patch
Patch failed at 0002 drm/i915: Include fence signaled bit in print_request()
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH 01/14] drm/i915/hangcheck: Track context changes
2019-05-01 11:45 [PATCH 01/14] drm/i915/hangcheck: Track context changes Chris Wilson
` (22 preceding siblings ...)
2019-05-02 16:45 ` ✗ Fi.CI.BAT: failure for series starting with [01/14] drm/i915/hangcheck: Track context changes (rev5) Patchwork
@ 2019-05-03 10:36 ` Tvrtko Ursulin
2019-05-03 10:43 ` Chris Wilson
23 siblings, 1 reply; 52+ messages in thread
From: Tvrtko Ursulin @ 2019-05-03 10:36 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On 01/05/2019 12:45, Chris Wilson wrote:
> Given sufficient preemption, we may see a busy system that doesn't
> advance seqno while performing work across multiple contexts, and given
> sufficient pathology not even notice a change in ACTHD. What does change
> between the preempting contexts is their RING, so take note of that and
> treat a change in the ring address as being an indication of forward
> progress.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/gt/intel_engine_types.h | 1 +
> drivers/gpu/drm/i915/gt/intel_hangcheck.c | 12 +++++++++---
> 2 files changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> index 9d64e33f8427..c0ab11b12e14 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> @@ -53,6 +53,7 @@ struct intel_instdone {
>
> struct intel_engine_hangcheck {
> u64 acthd;
> + u32 last_ring;
> u32 last_seqno;
> u32 next_seqno;
> unsigned long action_timestamp;
> diff --git a/drivers/gpu/drm/i915/gt/intel_hangcheck.c b/drivers/gpu/drm/i915/gt/intel_hangcheck.c
> index e5eaa06fe74d..721ab74a382f 100644
> --- a/drivers/gpu/drm/i915/gt/intel_hangcheck.c
> +++ b/drivers/gpu/drm/i915/gt/intel_hangcheck.c
> @@ -27,6 +27,7 @@
>
> struct hangcheck {
> u64 acthd;
> + u32 ring;
> u32 seqno;
> enum intel_engine_hangcheck_action action;
> unsigned long action_timestamp;
> @@ -134,6 +135,7 @@ static void hangcheck_load_sample(struct intel_engine_cs *engine,
> {
> hc->acthd = intel_engine_get_active_head(engine);
> hc->seqno = intel_engine_get_hangcheck_seqno(engine);
> + hc->ring = ENGINE_READ(engine, RING_START);
> }
>
> static void hangcheck_store_sample(struct intel_engine_cs *engine,
> @@ -141,18 +143,22 @@ static void hangcheck_store_sample(struct intel_engine_cs *engine,
> {
> engine->hangcheck.acthd = hc->acthd;
> engine->hangcheck.last_seqno = hc->seqno;
> + engine->hangcheck.last_ring = hc->ring;
> }
>
> static enum intel_engine_hangcheck_action
> hangcheck_get_action(struct intel_engine_cs *engine,
> const struct hangcheck *hc)
> {
> - if (engine->hangcheck.last_seqno != hc->seqno)
> - return ENGINE_ACTIVE_SEQNO;
> -
> if (intel_engine_is_idle(engine))
> return ENGINE_IDLE;
>
> + if (engine->hangcheck.last_ring != hc->ring)
> + return ENGINE_ACTIVE_SEQNO;
> +
> + if (engine->hangcheck.last_seqno != hc->seqno)
> + return ENGINE_ACTIVE_SEQNO;
> +
> return engine_stuck(engine, hc->acthd);
> }
>
>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
This should be associated with engine seqno removal, right? Not sure if
it triggers in reality to be really needed.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH 01/14] drm/i915/hangcheck: Track context changes
2019-05-03 10:36 ` [PATCH 01/14] drm/i915/hangcheck: Track context changes Tvrtko Ursulin
@ 2019-05-03 10:43 ` Chris Wilson
0 siblings, 0 replies; 52+ messages in thread
From: Chris Wilson @ 2019-05-03 10:43 UTC (permalink / raw)
To: Tvrtko Ursulin, intel-gfx
Quoting Tvrtko Ursulin (2019-05-03 11:36:55)
>
> On 01/05/2019 12:45, Chris Wilson wrote:
> > Given sufficient preemption, we may see a busy system that doesn't
> > advance seqno while performing work across multiple contexts, and given
> > sufficient pathology not even notice a change in ACTHD. What does change
> > between the preempting contexts is their RING, so take note of that and
> > treat a change in the ring address as being an indication of forward
> > progress.
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> > ---
> > drivers/gpu/drm/i915/gt/intel_engine_types.h | 1 +
> > drivers/gpu/drm/i915/gt/intel_hangcheck.c | 12 +++++++++---
> > 2 files changed, 10 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> > index 9d64e33f8427..c0ab11b12e14 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
> > +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> > @@ -53,6 +53,7 @@ struct intel_instdone {
> >
> > struct intel_engine_hangcheck {
> > u64 acthd;
> > + u32 last_ring;
> > u32 last_seqno;
> > u32 next_seqno;
> > unsigned long action_timestamp;
> > diff --git a/drivers/gpu/drm/i915/gt/intel_hangcheck.c b/drivers/gpu/drm/i915/gt/intel_hangcheck.c
> > index e5eaa06fe74d..721ab74a382f 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_hangcheck.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_hangcheck.c
> > @@ -27,6 +27,7 @@
> >
> > struct hangcheck {
> > u64 acthd;
> > + u32 ring;
> > u32 seqno;
> > enum intel_engine_hangcheck_action action;
> > unsigned long action_timestamp;
> > @@ -134,6 +135,7 @@ static void hangcheck_load_sample(struct intel_engine_cs *engine,
> > {
> > hc->acthd = intel_engine_get_active_head(engine);
> > hc->seqno = intel_engine_get_hangcheck_seqno(engine);
> > + hc->ring = ENGINE_READ(engine, RING_START);
> > }
> >
> > static void hangcheck_store_sample(struct intel_engine_cs *engine,
> > @@ -141,18 +143,22 @@ static void hangcheck_store_sample(struct intel_engine_cs *engine,
> > {
> > engine->hangcheck.acthd = hc->acthd;
> > engine->hangcheck.last_seqno = hc->seqno;
> > + engine->hangcheck.last_ring = hc->ring;
> > }
> >
> > static enum intel_engine_hangcheck_action
> > hangcheck_get_action(struct intel_engine_cs *engine,
> > const struct hangcheck *hc)
> > {
> > - if (engine->hangcheck.last_seqno != hc->seqno)
> > - return ENGINE_ACTIVE_SEQNO;
> > -
> > if (intel_engine_is_idle(engine))
> > return ENGINE_IDLE;
> >
> > + if (engine->hangcheck.last_ring != hc->ring)
> > + return ENGINE_ACTIVE_SEQNO;
> > +
> > + if (engine->hangcheck.last_seqno != hc->seqno)
> > + return ENGINE_ACTIVE_SEQNO;
> > +
> > return engine_stuck(engine, hc->acthd);
> > }
> >
> >
>
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> This should be associated with engine seqno removal, right? Not sure if
> it triggers in reality to be really needed.
Yeah, I'm not convinced we have a pressing need until timeslicing as
userspace can only create 1024 preemption events by itself, and that
should be ok... I can imagine that userspace submits a semaphore at
address 0 in each and waits 1s before submitting the next preemption
event... That would fire the current hangcheck. (Possibly legitimately
but the blame would not be effective at defeating the hostile client.)
It wasn't until timeslicing that I noticed the effect in practice.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH 08/14] drm/i915: Only reschedule the submission tasklet if preemption is possible
2019-05-01 11:45 ` [PATCH 08/14] drm/i915: Only reschedule the submission tasklet if preemption is possible Chris Wilson
@ 2019-05-03 10:53 ` Tvrtko Ursulin
2019-05-03 10:58 ` Chris Wilson
0 siblings, 1 reply; 52+ messages in thread
From: Tvrtko Ursulin @ 2019-05-03 10:53 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On 01/05/2019 12:45, Chris Wilson wrote:
> If we couple the scheduler more tightly with the execlists policy, we
> can apply the preemption policy to the question of whether we need to
> kick the tasklet at all for this priority bump.
>
> v2: Rephrase it as a core i915 policy and not an execlists foible.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
> drivers/gpu/drm/i915/gt/intel_engine.h | 18 ------------------
> drivers/gpu/drm/i915/gt/intel_lrc.c | 4 ++--
> drivers/gpu/drm/i915/gt/selftest_lrc.c | 7 ++++++-
> drivers/gpu/drm/i915/i915_request.c | 2 --
> drivers/gpu/drm/i915/i915_scheduler.c | 18 +++++++++++-------
> drivers/gpu/drm/i915/i915_scheduler.h | 18 ++++++++++++++++++
> drivers/gpu/drm/i915/intel_guc_submission.c | 3 ++-
> 7 files changed, 39 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h b/drivers/gpu/drm/i915/gt/intel_engine.h
> index f5b0f27cecb6..06d785533502 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine.h
> +++ b/drivers/gpu/drm/i915/gt/intel_engine.h
> @@ -106,24 +106,6 @@ hangcheck_action_to_str(const enum intel_engine_hangcheck_action a)
>
> void intel_engines_set_scheduler_caps(struct drm_i915_private *i915);
>
> -static inline bool __execlists_need_preempt(int prio, int last)
> -{
> - /*
> - * Allow preemption of low -> normal -> high, but we do
> - * not allow low priority tasks to preempt other low priority
> - * tasks under the impression that latency for low priority
> - * tasks does not matter (as much as background throughput),
> - * so kiss.
> - *
> - * More naturally we would write
> - * prio >= max(0, last);
> - * except that we wish to prevent triggering preemption at the same
> - * priority level: the task that is running should remain running
> - * to preserve FIFO ordering of dependencies.
> - */
> - return prio > max(I915_PRIORITY_NORMAL - 1, last);
> -}
> -
> static inline void
> execlists_set_active(struct intel_engine_execlists *execlists,
> unsigned int bit)
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> index 7be54b868d8e..35aae7b5c6b9 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> @@ -251,8 +251,8 @@ static inline bool need_preempt(const struct intel_engine_cs *engine,
> * ourselves, ignore the request.
> */
> last_prio = effective_prio(rq);
> - if (!__execlists_need_preempt(engine->execlists.queue_priority_hint,
> - last_prio))
> + if (!i915_scheduler_need_preempt(engine->execlists.queue_priority_hint,
> + last_prio))
> return false;
>
> /*
> diff --git a/drivers/gpu/drm/i915/gt/selftest_lrc.c b/drivers/gpu/drm/i915/gt/selftest_lrc.c
> index 84538f69185b..4b042893dc0e 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_lrc.c
> @@ -638,14 +638,19 @@ static struct i915_request *dummy_request(struct intel_engine_cs *engine)
> GEM_BUG_ON(i915_request_completed(rq));
>
> i915_sw_fence_init(&rq->submit, dummy_notify);
> - i915_sw_fence_commit(&rq->submit);
> + set_bit(I915_FENCE_FLAG_ACTIVE, &rq->fence.flags);
>
> return rq;
> }
>
> static void dummy_request_free(struct i915_request *dummy)
> {
> + /* We have to fake the CS interrupt to kick the next request */
> + i915_sw_fence_commit(&dummy->submit);
> +
> i915_request_mark_complete(dummy);
> + dma_fence_signal(&dummy->fence);
> +
> i915_sched_node_fini(&dummy->sched);
> i915_sw_fence_fini(&dummy->submit);
>
> diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
> index af8c9fa5e066..2e22da66a56c 100644
> --- a/drivers/gpu/drm/i915/i915_request.c
> +++ b/drivers/gpu/drm/i915/i915_request.c
> @@ -1358,9 +1358,7 @@ long i915_request_wait(struct i915_request *rq,
> if (flags & I915_WAIT_PRIORITY) {
> if (!i915_request_started(rq) && INTEL_GEN(rq->i915) >= 6)
> gen6_rps_boost(rq);
> - local_bh_disable(); /* suspend tasklets for reprioritisation */
> i915_schedule_bump_priority(rq, I915_PRIORITY_WAIT);
> - local_bh_enable(); /* kick tasklets en masse */
> }
>
> wait.tsk = current;
> diff --git a/drivers/gpu/drm/i915/i915_scheduler.c b/drivers/gpu/drm/i915/i915_scheduler.c
> index 39bc4f54e272..88d18600f5db 100644
> --- a/drivers/gpu/drm/i915/i915_scheduler.c
> +++ b/drivers/gpu/drm/i915/i915_scheduler.c
> @@ -261,16 +261,20 @@ sched_lock_engine(const struct i915_sched_node *node,
> return engine;
> }
>
> -static bool inflight(const struct i915_request *rq,
> - const struct intel_engine_cs *engine)
> +static inline int rq_prio(const struct i915_request *rq)
> {
> - const struct i915_request *active;
> + return rq->sched.attr.priority | __NO_PREEMPTION;
> +}
> +
> +static bool kick_tasklet(const struct intel_engine_cs *engine, int prio)
> +{
> + const struct i915_request *inflight =
> + port_request(engine->execlists.port);
>
> - if (!i915_request_is_active(rq))
> + if (!inflight)
> return false;
>
> - active = port_request(engine->execlists.port);
> - return active->hw_context == rq->hw_context;
> + return i915_scheduler_need_preempt(prio, rq_prio(inflight));
> }
>
> static void __i915_schedule(struct i915_request *rq,
> @@ -400,7 +404,7 @@ static void __i915_schedule(struct i915_request *rq,
> * If we are already the currently executing context, don't
> * bother evaluating if we should preempt ourselves.
> */
> - if (inflight(node_to_request(node), engine))
> + if (!kick_tasklet(engine, prio))
> continue;
I don't see other callers for kick_tasklet in the series so I am
thinking why not make the function called kick_tasklet actually kick the
tasklet? ;)
Could call it maybe_kick_tasklet and just end the loop with it.
We could even abstract to the engine like engine->signal_preemption() or
something, to hide the tasklet from the scheduler. But probably not
worth it right now.
Regards,
Tvrtko
>
> /* Defer (tasklet) submission until after all of our updates. */
> diff --git a/drivers/gpu/drm/i915/i915_scheduler.h b/drivers/gpu/drm/i915/i915_scheduler.h
> index 07d243acf553..7eefccff39bf 100644
> --- a/drivers/gpu/drm/i915/i915_scheduler.h
> +++ b/drivers/gpu/drm/i915/i915_scheduler.h
> @@ -52,4 +52,22 @@ static inline void i915_priolist_free(struct i915_priolist *p)
> __i915_priolist_free(p);
> }
>
> +static inline bool i915_scheduler_need_preempt(int prio, int active)
> +{
> + /*
> + * Allow preemption of low -> normal -> high, but we do
> + * not allow low priority tasks to preempt other low priority
> + * tasks under the impression that latency for low priority
> + * tasks does not matter (as much as background throughput),
> + * so kiss.
> + *
> + * More naturally we would write
> + * prio >= max(0, last);
> + * except that we wish to prevent triggering preemption at the same
> + * priority level: the task that is running should remain running
> + * to preserve FIFO ordering of dependencies.
> + */
> + return prio > max(I915_PRIORITY_NORMAL - 1, active);
> +}
> +
> #endif /* _I915_SCHEDULER_H_ */
> diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c
> index ed94001028f2..cb9964ae229d 100644
> --- a/drivers/gpu/drm/i915/intel_guc_submission.c
> +++ b/drivers/gpu/drm/i915/intel_guc_submission.c
> @@ -746,7 +746,8 @@ static bool __guc_dequeue(struct intel_engine_cs *engine)
> &engine->i915->guc.preempt_work[engine->id];
> int prio = execlists->queue_priority_hint;
>
> - if (__execlists_need_preempt(prio, port_prio(port))) {
> + if (i915_scheduler_need_preempt(prio,
> + port_prio(port))) {
> execlists_set_active(execlists,
> EXECLISTS_ACTIVE_PREEMPT);
> queue_work(engine->i915->guc.preempt_wq,
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH 08/14] drm/i915: Only reschedule the submission tasklet if preemption is possible
2019-05-03 10:53 ` Tvrtko Ursulin
@ 2019-05-03 10:58 ` Chris Wilson
0 siblings, 0 replies; 52+ messages in thread
From: Chris Wilson @ 2019-05-03 10:58 UTC (permalink / raw)
To: Tvrtko Ursulin, intel-gfx
Quoting Tvrtko Ursulin (2019-05-03 11:53:31)
>
> On 01/05/2019 12:45, Chris Wilson wrote:
> > If we couple the scheduler more tightly with the execlists policy, we
> > can apply the preemption policy to the question of whether we need to
> > kick the tasklet at all for this priority bump.
> >
> > v2: Rephrase it as a core i915 policy and not an execlists foible.
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > ---
> > drivers/gpu/drm/i915/gt/intel_engine.h | 18 ------------------
> > drivers/gpu/drm/i915/gt/intel_lrc.c | 4 ++--
> > drivers/gpu/drm/i915/gt/selftest_lrc.c | 7 ++++++-
> > drivers/gpu/drm/i915/i915_request.c | 2 --
> > drivers/gpu/drm/i915/i915_scheduler.c | 18 +++++++++++-------
> > drivers/gpu/drm/i915/i915_scheduler.h | 18 ++++++++++++++++++
> > drivers/gpu/drm/i915/intel_guc_submission.c | 3 ++-
> > 7 files changed, 39 insertions(+), 31 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h b/drivers/gpu/drm/i915/gt/intel_engine.h
> > index f5b0f27cecb6..06d785533502 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_engine.h
> > +++ b/drivers/gpu/drm/i915/gt/intel_engine.h
> > @@ -106,24 +106,6 @@ hangcheck_action_to_str(const enum intel_engine_hangcheck_action a)
> >
> > void intel_engines_set_scheduler_caps(struct drm_i915_private *i915);
> >
> > -static inline bool __execlists_need_preempt(int prio, int last)
> > -{
> > - /*
> > - * Allow preemption of low -> normal -> high, but we do
> > - * not allow low priority tasks to preempt other low priority
> > - * tasks under the impression that latency for low priority
> > - * tasks does not matter (as much as background throughput),
> > - * so kiss.
> > - *
> > - * More naturally we would write
> > - * prio >= max(0, last);
> > - * except that we wish to prevent triggering preemption at the same
> > - * priority level: the task that is running should remain running
> > - * to preserve FIFO ordering of dependencies.
> > - */
> > - return prio > max(I915_PRIORITY_NORMAL - 1, last);
> > -}
> > -
> > static inline void
> > execlists_set_active(struct intel_engine_execlists *execlists,
> > unsigned int bit)
> > diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> > index 7be54b868d8e..35aae7b5c6b9 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> > @@ -251,8 +251,8 @@ static inline bool need_preempt(const struct intel_engine_cs *engine,
> > * ourselves, ignore the request.
> > */
> > last_prio = effective_prio(rq);
> > - if (!__execlists_need_preempt(engine->execlists.queue_priority_hint,
> > - last_prio))
> > + if (!i915_scheduler_need_preempt(engine->execlists.queue_priority_hint,
> > + last_prio))
> > return false;
> >
> > /*
> > diff --git a/drivers/gpu/drm/i915/gt/selftest_lrc.c b/drivers/gpu/drm/i915/gt/selftest_lrc.c
> > index 84538f69185b..4b042893dc0e 100644
> > --- a/drivers/gpu/drm/i915/gt/selftest_lrc.c
> > +++ b/drivers/gpu/drm/i915/gt/selftest_lrc.c
> > @@ -638,14 +638,19 @@ static struct i915_request *dummy_request(struct intel_engine_cs *engine)
> > GEM_BUG_ON(i915_request_completed(rq));
> >
> > i915_sw_fence_init(&rq->submit, dummy_notify);
> > - i915_sw_fence_commit(&rq->submit);
> > + set_bit(I915_FENCE_FLAG_ACTIVE, &rq->fence.flags);
> >
> > return rq;
> > }
> >
> > static void dummy_request_free(struct i915_request *dummy)
> > {
> > + /* We have to fake the CS interrupt to kick the next request */
> > + i915_sw_fence_commit(&dummy->submit);
> > +
> > i915_request_mark_complete(dummy);
> > + dma_fence_signal(&dummy->fence);
> > +
> > i915_sched_node_fini(&dummy->sched);
> > i915_sw_fence_fini(&dummy->submit);
> >
> > diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
> > index af8c9fa5e066..2e22da66a56c 100644
> > --- a/drivers/gpu/drm/i915/i915_request.c
> > +++ b/drivers/gpu/drm/i915/i915_request.c
> > @@ -1358,9 +1358,7 @@ long i915_request_wait(struct i915_request *rq,
> > if (flags & I915_WAIT_PRIORITY) {
> > if (!i915_request_started(rq) && INTEL_GEN(rq->i915) >= 6)
> > gen6_rps_boost(rq);
> > - local_bh_disable(); /* suspend tasklets for reprioritisation */
> > i915_schedule_bump_priority(rq, I915_PRIORITY_WAIT);
> > - local_bh_enable(); /* kick tasklets en masse */
> > }
> >
> > wait.tsk = current;
> > diff --git a/drivers/gpu/drm/i915/i915_scheduler.c b/drivers/gpu/drm/i915/i915_scheduler.c
> > index 39bc4f54e272..88d18600f5db 100644
> > --- a/drivers/gpu/drm/i915/i915_scheduler.c
> > +++ b/drivers/gpu/drm/i915/i915_scheduler.c
> > @@ -261,16 +261,20 @@ sched_lock_engine(const struct i915_sched_node *node,
> > return engine;
> > }
> >
> > -static bool inflight(const struct i915_request *rq,
> > - const struct intel_engine_cs *engine)
> > +static inline int rq_prio(const struct i915_request *rq)
> > {
> > - const struct i915_request *active;
> > + return rq->sched.attr.priority | __NO_PREEMPTION;
> > +}
> > +
> > +static bool kick_tasklet(const struct intel_engine_cs *engine, int prio)
> > +{
> > + const struct i915_request *inflight =
> > + port_request(engine->execlists.port);
> >
> > - if (!i915_request_is_active(rq))
> > + if (!inflight)
> > return false;
> >
> > - active = port_request(engine->execlists.port);
> > - return active->hw_context == rq->hw_context;
> > + return i915_scheduler_need_preempt(prio, rq_prio(inflight));
> > }
> >
> > static void __i915_schedule(struct i915_request *rq,
> > @@ -400,7 +404,7 @@ static void __i915_schedule(struct i915_request *rq,
> > * If we are already the currently executing context, don't
> > * bother evaluating if we should preempt ourselves.
> > */
> > - if (inflight(node_to_request(node), engine))
> > + if (!kick_tasklet(engine, prio))
> > continue;
>
> I don't see other callers for kick_tasklet in the series so I am
> thinking why not make the function called kick_tasklet actually kick the
> tasklet? ;)
You don't see the ghostly presence of set_preemption_timeout() here? Ok,
that could be pushed into the function as well.
> Could call it maybe_kick_tasklet and just end the loop with it.
>
> We could even abstract to the engine like engine->signal_preemption() or
> something, to hide the tasklet from the scheduler. But probably not
> worth it right now.
Haven't quite figured out the best approach. I've done both so far for
preemption-timeout, at the moment I've pulled the hrtimer into
i915_scheduler.c (frankly because it was easier to on the old rebasing);
but the dust is nowhere close to settled.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH 09/14] drm/i915: Delay semaphore submission until the start of the signaler
2019-05-01 11:45 ` [PATCH 09/14] drm/i915: Delay semaphore submission until the start of the signaler Chris Wilson
@ 2019-05-03 11:03 ` Tvrtko Ursulin
0 siblings, 0 replies; 52+ messages in thread
From: Tvrtko Ursulin @ 2019-05-03 11:03 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On 01/05/2019 12:45, Chris Wilson wrote:
> Currently we submit the semaphore busywait as soon as the signaler is
> submitted to HW. However, we may submit the signaler as the tail of a
> batch of requests, and even not as the first context in the HW list,
> i.e. the busywait may start spinning far in advance of the signaler even
> starting.
>
> If we wait until the request before the signaler is completed before
> submitting the busywait, we prevent the busywait from starting too
> early, if the signaler is not first in submission port.
>
> To handle the case where the signaler is at the start of the second (or
> later) submission port, we will need to delay the execution callback
> until we know the context is promoted to port0. A challenge for later.
>
> Fixes: e88619646971 ("drm/i915: Use HW semaphores for inter-engine synchroni
> sation on gen8+")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
> drivers/gpu/drm/i915/i915_request.c | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
> index 2e22da66a56c..8cb3ed5531e3 100644
> --- a/drivers/gpu/drm/i915/i915_request.c
> +++ b/drivers/gpu/drm/i915/i915_request.c
> @@ -770,6 +770,21 @@ i915_request_create(struct intel_context *ce)
> return rq;
> }
>
> +static int
> +i915_request_await_start(struct i915_request *rq, struct i915_request *signal)
> +{
> + if (list_is_first(&signal->ring_link, &signal->ring->request_list))
> + return 0;
> +
> + signal = list_prev_entry(signal, ring_link);
> + if (i915_timeline_sync_is_later(rq->timeline, &signal->fence))
> + return 0;
> +
> + return i915_sw_fence_await_dma_fence(&rq->submit,
> + &signal->fence, 0,
> + I915_FENCE_GFP);
> +}
> +
> static int
> emit_semaphore_wait(struct i915_request *to,
> struct i915_request *from,
> @@ -788,6 +803,10 @@ emit_semaphore_wait(struct i915_request *to,
> &from->fence, 0,
> I915_FENCE_GFP);
>
> + err = i915_request_await_start(to, from);
> + if (err < 0)
> + return err;
> +
> err = i915_sw_fence_await_dma_fence(&to->semaphore,
> &from->fence, 0,
> I915_FENCE_GFP);
>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 52+ messages in thread
end of thread, other threads:[~2019-05-03 11:03 UTC | newest]
Thread overview: 52+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-01 11:45 [PATCH 01/14] drm/i915/hangcheck: Track context changes Chris Wilson
2019-05-01 11:45 ` [PATCH 02/14] drm/i915: Include fence signaled bit in print_request() Chris Wilson
2019-05-02 13:43 ` Tvrtko Ursulin
2019-05-01 11:45 ` [PATCH 03/14] drm/i915/execlists: Flush the tasklet on parking Chris Wilson
2019-05-02 13:48 ` Tvrtko Ursulin
2019-05-02 13:53 ` Chris Wilson
2019-05-02 14:14 ` Tvrtko Ursulin
2019-05-02 14:21 ` Chris Wilson
2019-05-02 14:24 ` Tvrtko Ursulin
2019-05-02 14:33 ` Chris Wilson
2019-05-01 11:45 ` [PATCH 04/14] drm/i915: Leave engine parking to the engines Chris Wilson
2019-05-02 14:18 ` Tvrtko Ursulin
2019-05-01 11:45 ` [PATCH 05/14] drm/i915: Remove delay for idle_work Chris Wilson
2019-05-02 13:19 ` Tvrtko Ursulin
2019-05-02 13:22 ` Chris Wilson
2019-05-02 13:51 ` Tvrtko Ursulin
2019-05-02 14:23 ` Chris Wilson
2019-05-01 11:45 ` [PATCH 06/14] drm/i915: Cancel retire_worker on parking Chris Wilson
2019-05-02 13:29 ` Tvrtko Ursulin
2019-05-02 13:33 ` Chris Wilson
2019-05-02 14:20 ` Tvrtko Ursulin
2019-05-02 14:26 ` Chris Wilson
2019-05-02 14:29 ` [PATCH v2] " Chris Wilson
2019-05-01 11:45 ` [PATCH 07/14] drm/i915: Stop spinning for DROP_IDLE (debugfs/i915_drop_caches) Chris Wilson
2019-05-02 13:34 ` Tvrtko Ursulin
2019-05-02 14:00 ` Chris Wilson
2019-05-02 14:16 ` Tvrtko Ursulin
2019-05-01 11:45 ` [PATCH 08/14] drm/i915: Only reschedule the submission tasklet if preemption is possible Chris Wilson
2019-05-03 10:53 ` Tvrtko Ursulin
2019-05-03 10:58 ` Chris Wilson
2019-05-01 11:45 ` [PATCH 09/14] drm/i915: Delay semaphore submission until the start of the signaler Chris Wilson
2019-05-03 11:03 ` Tvrtko Ursulin
2019-05-01 11:45 ` [PATCH 10/14] drm/i915/execlists: Don't apply priority boost for resets Chris Wilson
2019-05-01 11:45 ` [PATCH 11/14] drm/i915: Rearrange i915_scheduler.c Chris Wilson
2019-05-01 11:45 ` [PATCH 12/14] drm/i915: Pass i915_sched_node around internally Chris Wilson
2019-05-01 11:45 ` [PATCH 13/14] drm/i915: Bump signaler priority on adding a waiter Chris Wilson
2019-05-01 14:59 ` [PATCH] " Chris Wilson
2019-05-01 16:29 ` [PATCH v2] " Chris Wilson
2019-05-01 16:32 ` Chris Wilson
2019-05-01 11:45 ` [PATCH 14/14] drm/i915: Convert inconsistent static engine tables into an init error Chris Wilson
2019-05-01 12:29 ` ✗ Fi.CI.SPARSE: warning for series starting with [01/14] drm/i915/hangcheck: Track context changes Patchwork
2019-05-01 12:47 ` ✗ Fi.CI.BAT: failure " Patchwork
2019-05-01 15:22 ` ✗ Fi.CI.SPARSE: warning for series starting with [01/14] drm/i915/hangcheck: Track context changes (rev2) Patchwork
2019-05-01 15:35 ` ✓ Fi.CI.BAT: success " Patchwork
2019-05-01 16:42 ` ✗ Fi.CI.SPARSE: warning for series starting with [01/14] drm/i915/hangcheck: Track context changes (rev3) Patchwork
2019-05-01 16:52 ` ✓ Fi.CI.BAT: success " Patchwork
2019-05-01 17:01 ` ✗ Fi.CI.SPARSE: warning for series starting with [01/14] drm/i915/hangcheck: Track context changes (rev4) Patchwork
2019-05-01 17:37 ` ✓ Fi.CI.BAT: success " Patchwork
2019-05-02 9:35 ` ✗ Fi.CI.IGT: failure " Patchwork
2019-05-02 16:45 ` ✗ Fi.CI.BAT: failure for series starting with [01/14] drm/i915/hangcheck: Track context changes (rev5) Patchwork
2019-05-03 10:36 ` [PATCH 01/14] drm/i915/hangcheck: Track context changes Tvrtko Ursulin
2019-05-03 10:43 ` Chris Wilson
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.