* [PATCH 1/2] drm/i915/selftests: Pinned the mock kernel context @ 2018-05-23 9:51 Chris Wilson 2018-05-23 9:51 ` [PATCH 2/2] drm/i915: Look for an active kernel context before switching Chris Wilson ` (3 more replies) 0 siblings, 4 replies; 6+ messages in thread From: Chris Wilson @ 2018-05-23 9:51 UTC (permalink / raw) To: intel-gfx The driver assumes that the kernel context is pinned and always available for use from any process or atomic context. Make it so for selftesting as well! Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/selftests/mock_engine.c | 7 +++++++ .../gpu/drm/i915/selftests/mock_gem_device.c | 17 +++++++---------- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i915/selftests/mock_engine.c b/drivers/gpu/drm/i915/selftests/mock_engine.c index f1ac7453053e..c2a0451336cf 100644 --- a/drivers/gpu/drm/i915/selftests/mock_engine.c +++ b/drivers/gpu/drm/i915/selftests/mock_engine.c @@ -212,8 +212,13 @@ struct intel_engine_cs *mock_engine(struct drm_i915_private *i915, if (!engine->base.buffer) goto err_breadcrumbs; + if (IS_ERR(intel_context_pin(i915->kernel_context, &engine->base))) + goto err_ring; + return &engine->base; +err_ring: + mock_ring_free(engine->base.buffer); err_breadcrumbs: intel_engine_fini_breadcrumbs(&engine->base); i915_timeline_fini(&engine->base.timeline); @@ -254,6 +259,8 @@ void mock_engine_free(struct intel_engine_cs *engine) if (ce) intel_context_unpin(ce); + __intel_context_unpin(engine->i915->kernel_context, engine); + mock_ring_free(engine->buffer); intel_engine_fini_breadcrumbs(engine); diff --git a/drivers/gpu/drm/i915/selftests/mock_gem_device.c b/drivers/gpu/drm/i915/selftests/mock_gem_device.c index 94baedfa0f74..c97075c5ccaf 100644 --- a/drivers/gpu/drm/i915/selftests/mock_gem_device.c +++ b/drivers/gpu/drm/i915/selftests/mock_gem_device.c @@ -136,8 +136,6 @@ static struct dev_pm_domain pm_domain = { struct drm_i915_private *mock_gem_device(void) { struct drm_i915_private *i915; - struct intel_engine_cs *engine; - enum intel_engine_id id; struct pci_dev *pdev; int err; @@ -233,13 +231,13 @@ struct drm_i915_private *mock_gem_device(void) mock_init_ggtt(i915); mkwrite_device_info(i915)->ring_mask = BIT(0); - i915->engine[RCS] = mock_engine(i915, "mock", RCS); - if (!i915->engine[RCS]) - goto err_unlock; - i915->kernel_context = mock_context(i915, NULL); if (!i915->kernel_context) - goto err_engine; + goto err_unlock; + + i915->engine[RCS] = mock_engine(i915, "mock", RCS); + if (!i915->engine[RCS]) + goto err_context; mutex_unlock(&i915->drm.struct_mutex); @@ -247,9 +245,8 @@ struct drm_i915_private *mock_gem_device(void) return i915; -err_engine: - for_each_engine(engine, i915, id) - mock_engine_free(engine); +err_context: + i915_gem_contexts_fini(i915); err_unlock: mutex_unlock(&i915->drm.struct_mutex); kmem_cache_destroy(i915->priorities); -- 2.17.0 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] drm/i915: Look for an active kernel context before switching 2018-05-23 9:51 [PATCH 1/2] drm/i915/selftests: Pinned the mock kernel context Chris Wilson @ 2018-05-23 9:51 ` Chris Wilson 2018-05-23 10:31 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915/selftests: Pinned the mock kernel context Patchwork ` (2 subsequent siblings) 3 siblings, 0 replies; 6+ messages in thread From: Chris Wilson @ 2018-05-23 9:51 UTC (permalink / raw) To: intel-gfx; +Cc: Mika Kuoppala We were not very carefully checking to see if an older request on the engine was an earlier switch-to-kernel-context before deciding to emit a new switch. The end result would be that we could get into a permanent loop of trying to emit a new request to perform the switch simply to flush the existing switch. What we need is a means of tracking the completion of each timeline versus the kernel context, that is to detect if a more recent request has been submitted that would result in a switch away from the kernel context. To realise this, we need only to look in our syncmap on the kernel context and check that we have synchronized against all active rings. Reported-by: Mika Kuoppala <mika.kuoppala@intel.com> Fixes: a89d1f921c15 ("drm/i915: Split i915_gem_timeline into individual timelines") Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Mika Kuoppala <mika.kuoppala@intel.com> --- drivers/gpu/drm/i915/i915_gem_context.c | 44 +++++++++++++++++++++---- 1 file changed, 38 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index b69b18ef8120..6c9301e8ef6b 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -587,18 +587,45 @@ last_request_on_engine(struct i915_timeline *timeline, return NULL; } -static bool engine_has_idle_kernel_context(struct intel_engine_cs *engine) +static bool engine_has_kernel_context_barrier(struct intel_engine_cs *engine) { - struct list_head * const active_rings = &engine->i915->gt.active_rings; + struct drm_i915_private *i915 = engine->i915; + struct i915_timeline *barrier = + to_intel_context(i915->kernel_context, engine)->ring->timeline; struct intel_ring *ring; + bool any_active = false; - lockdep_assert_held(&engine->i915->drm.struct_mutex); + lockdep_assert_held(&i915->drm.struct_mutex); + list_for_each_entry(ring, &i915->gt.active_rings, active_link) { + struct i915_request *rq; + + if (ring->timeline == barrier) + continue; - list_for_each_entry(ring, active_rings, active_link) { - if (last_request_on_engine(ring->timeline, engine)) + rq = last_request_on_engine(ring->timeline, engine); + if (!rq) + continue; + + /* + * Was this request submitted after the previous + * switch-to-kernel-context? + */ + if (!i915_timeline_sync_is_later(barrier, &rq->fence)) return false; + + any_active = true; } + /* + * If any other timeline was still active and behind the last barrier, + * then our last switch-to-kernel-context must still be queued and + * will run last (leaving the engine in the kernel context when it + * eventually idles). + */ + if (any_active) + return true; + + /* The engine is idle; check that it is idling in the kernel context. */ return intel_engine_has_kernel_context(engine); } @@ -608,6 +635,7 @@ int i915_gem_switch_to_kernel_context(struct drm_i915_private *i915) enum intel_engine_id id; lockdep_assert_held(&i915->drm.struct_mutex); + GEM_BUG_ON(!i915->kernel_context); i915_retire_requests(i915); @@ -615,7 +643,8 @@ int i915_gem_switch_to_kernel_context(struct drm_i915_private *i915) struct intel_ring *ring; struct i915_request *rq; - if (engine_has_idle_kernel_context(engine)) + GEM_BUG_ON(!to_intel_context(i915->kernel_context, engine)); + if (engine_has_kernel_context_barrier(engine)) continue; rq = i915_request_alloc(engine, i915->kernel_context); @@ -626,6 +655,9 @@ int i915_gem_switch_to_kernel_context(struct drm_i915_private *i915) list_for_each_entry(ring, &i915->gt.active_rings, active_link) { struct i915_request *prev; + if (ring == rq->ring) + continue; + prev = last_request_on_engine(ring->timeline, engine); if (prev) i915_sw_fence_await_sw_fence_gfp(&rq->submit, -- 2.17.0 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 6+ messages in thread
* ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915/selftests: Pinned the mock kernel context 2018-05-23 9:51 [PATCH 1/2] drm/i915/selftests: Pinned the mock kernel context Chris Wilson 2018-05-23 9:51 ` [PATCH 2/2] drm/i915: Look for an active kernel context before switching Chris Wilson @ 2018-05-23 10:31 ` Patchwork 2018-05-23 12:41 ` [PATCH 1/2] " Mika Kuoppala 2018-05-23 12:45 ` ✓ Fi.CI.IGT: success for series starting with [1/2] " Patchwork 3 siblings, 0 replies; 6+ messages in thread From: Patchwork @ 2018-05-23 10:31 UTC (permalink / raw) To: Chris Wilson; +Cc: intel-gfx == Series Details == Series: series starting with [1/2] drm/i915/selftests: Pinned the mock kernel context URL : https://patchwork.freedesktop.org/series/43621/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4223 -> Patchwork_9093 = == Summary - SUCCESS == No regressions found. External URL: https://patchwork.freedesktop.org/api/1.0/series/43621/revisions/1/mbox/ == Known issues == Here are the changes found in Patchwork_9093 that come from known issues: === IGT changes === ==== Issues hit ==== igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a: fi-cnl-psr: PASS -> DMESG-WARN (fdo#104951) ==== Possible fixes ==== igt@kms_flip@basic-flip-vs-wf_vblank: fi-hsw-4770: FAIL (fdo#100368) -> PASS fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368 fdo#104951 https://bugs.freedesktop.org/show_bug.cgi?id=104951 == Participating hosts (44 -> 39) == Missing (5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-skl-6700hq == Build changes == * Linux: CI_DRM_4223 -> Patchwork_9093 CI_DRM_4223: 58d58cc95cbd55f7669ab3003916e4b9385224e5 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4493: 0b381c7d1067a4fe520b72d4d391d4920834cbe0 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_9093: 5297f535f59166359a466b265d7836096dcf807f @ git://anongit.freedesktop.org/gfx-ci/linux piglit_4493: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit == Linux commits == 5297f535f591 drm/i915: Look for an active kernel context before switching 938c15d00ce2 drm/i915/selftests: Pinned the mock kernel context == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9093/issues.html _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] drm/i915/selftests: Pinned the mock kernel context 2018-05-23 9:51 [PATCH 1/2] drm/i915/selftests: Pinned the mock kernel context Chris Wilson 2018-05-23 9:51 ` [PATCH 2/2] drm/i915: Look for an active kernel context before switching Chris Wilson 2018-05-23 10:31 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915/selftests: Pinned the mock kernel context Patchwork @ 2018-05-23 12:41 ` Mika Kuoppala 2018-05-23 12:45 ` ✓ Fi.CI.IGT: success for series starting with [1/2] " Patchwork 3 siblings, 0 replies; 6+ messages in thread From: Mika Kuoppala @ 2018-05-23 12:41 UTC (permalink / raw) To: Chris Wilson, intel-gfx Chris Wilson <chris@chris-wilson.co.uk> writes: > The driver assumes that the kernel context is pinned and always > available for use from any process or atomic context. Make it so for > selftesting as well! > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > --- > drivers/gpu/drm/i915/selftests/mock_engine.c | 7 +++++++ > .../gpu/drm/i915/selftests/mock_gem_device.c | 17 +++++++---------- > 2 files changed, 14 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/i915/selftests/mock_engine.c b/drivers/gpu/drm/i915/selftests/mock_engine.c > index f1ac7453053e..c2a0451336cf 100644 > --- a/drivers/gpu/drm/i915/selftests/mock_engine.c > +++ b/drivers/gpu/drm/i915/selftests/mock_engine.c > @@ -212,8 +212,13 @@ struct intel_engine_cs *mock_engine(struct drm_i915_private *i915, > if (!engine->base.buffer) > goto err_breadcrumbs; > > + if (IS_ERR(intel_context_pin(i915->kernel_context, &engine->base))) > + goto err_ring; > + > return &engine->base; > > +err_ring: > + mock_ring_free(engine->base.buffer); > err_breadcrumbs: > intel_engine_fini_breadcrumbs(&engine->base); > i915_timeline_fini(&engine->base.timeline); > @@ -254,6 +259,8 @@ void mock_engine_free(struct intel_engine_cs *engine) > if (ce) > intel_context_unpin(ce); > > + __intel_context_unpin(engine->i915->kernel_context, engine); > + > mock_ring_free(engine->buffer); > > intel_engine_fini_breadcrumbs(engine); > diff --git a/drivers/gpu/drm/i915/selftests/mock_gem_device.c b/drivers/gpu/drm/i915/selftests/mock_gem_device.c > index 94baedfa0f74..c97075c5ccaf 100644 > --- a/drivers/gpu/drm/i915/selftests/mock_gem_device.c > +++ b/drivers/gpu/drm/i915/selftests/mock_gem_device.c > @@ -136,8 +136,6 @@ static struct dev_pm_domain pm_domain = { > struct drm_i915_private *mock_gem_device(void) > { > struct drm_i915_private *i915; > - struct intel_engine_cs *engine; > - enum intel_engine_id id; > struct pci_dev *pdev; > int err; > > @@ -233,13 +231,13 @@ struct drm_i915_private *mock_gem_device(void) > mock_init_ggtt(i915); > > mkwrite_device_info(i915)->ring_mask = BIT(0); > - i915->engine[RCS] = mock_engine(i915, "mock", RCS); > - if (!i915->engine[RCS]) > - goto err_unlock; > - > i915->kernel_context = mock_context(i915, NULL); > if (!i915->kernel_context) > - goto err_engine; > + goto err_unlock; > + > + i915->engine[RCS] = mock_engine(i915, "mock", RCS); > + if (!i915->engine[RCS]) > + goto err_context; > > mutex_unlock(&i915->drm.struct_mutex); > > @@ -247,9 +245,8 @@ struct drm_i915_private *mock_gem_device(void) > > return i915; > > -err_engine: > - for_each_engine(engine, i915, id) > - mock_engine_free(engine); > +err_context: > + i915_gem_contexts_fini(i915); For me it looks like there has been hw_ida leak in this error path, now plugged by this patch. Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> > err_unlock: > mutex_unlock(&i915->drm.struct_mutex); > kmem_cache_destroy(i915->priorities); > -- > 2.17.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 6+ messages in thread
* ✓ Fi.CI.IGT: success for series starting with [1/2] drm/i915/selftests: Pinned the mock kernel context 2018-05-23 9:51 [PATCH 1/2] drm/i915/selftests: Pinned the mock kernel context Chris Wilson ` (2 preceding siblings ...) 2018-05-23 12:41 ` [PATCH 1/2] " Mika Kuoppala @ 2018-05-23 12:45 ` Patchwork 3 siblings, 0 replies; 6+ messages in thread From: Patchwork @ 2018-05-23 12:45 UTC (permalink / raw) To: Chris Wilson; +Cc: intel-gfx == Series Details == Series: series starting with [1/2] drm/i915/selftests: Pinned the mock kernel context URL : https://patchwork.freedesktop.org/series/43621/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4223_full -> Patchwork_9093_full = == Summary - WARNING == Minor unknown changes coming with Patchwork_9093_full need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_9093_full, 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/43621/revisions/1/mbox/ == Possible new issues == Here are the unknown changes that may have been introduced in Patchwork_9093_full: === IGT changes === ==== Warnings ==== igt@gem_exec_schedule@deep-vebox: shard-kbl: PASS -> SKIP == Known issues == Here are the changes found in Patchwork_9093_full that come from known issues: === IGT changes === ==== Issues hit ==== igt@drv_selftest@live_gtt: shard-apl: PASS -> INCOMPLETE (fdo#103927) igt@gem_eio@in-flight-suspend: shard-snb: PASS -> INCOMPLETE (fdo#105411) shard-hsw: PASS -> INCOMPLETE (fdo#103540) igt@kms_atomic_transition@1x-modeset-transitions-nonblocking-fencing: shard-glk: PASS -> FAIL (fdo#105703) igt@kms_flip@flip-vs-expired-vblank-interruptible: shard-glk: PASS -> FAIL (fdo#105707) igt@kms_frontbuffer_tracking@fbc-2p-primscrn-shrfb-msflip-blt: shard-glk: PASS -> FAIL (fdo#103167, fdo#104724) igt@kms_vblank@pipe-c-ts-continuation-dpms-suspend: shard-glk: PASS -> INCOMPLETE (fdo#103359, k.org#198133) ==== Possible fixes ==== igt@gem_ppgtt@blt-vs-render-ctx0: shard-kbl: INCOMPLETE (fdo#103665, fdo#106023) -> PASS igt@kms_flip@plain-flip-fb-recreate: shard-hsw: FAIL (fdo#103928) -> PASS igt@kms_flip@plain-flip-fb-recreate-interruptible: shard-glk: FAIL (fdo#100368) -> PASS igt@kms_flip_tiling@flip-x-tiled: shard-glk: FAIL (fdo#103822, fdo#104724) -> PASS +1 igt@kms_setmode@basic: shard-kbl: FAIL (fdo#99912) -> PASS fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368 fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167 fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359 fdo#103540 https://bugs.freedesktop.org/show_bug.cgi?id=103540 fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665 fdo#103822 https://bugs.freedesktop.org/show_bug.cgi?id=103822 fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927 fdo#103928 https://bugs.freedesktop.org/show_bug.cgi?id=103928 fdo#104724 https://bugs.freedesktop.org/show_bug.cgi?id=104724 fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411 fdo#105703 https://bugs.freedesktop.org/show_bug.cgi?id=105703 fdo#105707 https://bugs.freedesktop.org/show_bug.cgi?id=105707 fdo#106023 https://bugs.freedesktop.org/show_bug.cgi?id=106023 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 (9 -> 9) == No changes in participating hosts == Build changes == * Linux: CI_DRM_4223 -> Patchwork_9093 CI_DRM_4223: 58d58cc95cbd55f7669ab3003916e4b9385224e5 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4493: 0b381c7d1067a4fe520b72d4d391d4920834cbe0 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_9093: 5297f535f59166359a466b265d7836096dcf807f @ git://anongit.freedesktop.org/gfx-ci/linux piglit_4493: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9093/shards.html _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] drm/i915/selftests: Pin the mock kernel context @ 2018-05-23 14:53 Chris Wilson 2018-05-23 14:53 ` [PATCH 2/2] drm/i915: Look for an active kernel context before switching Chris Wilson 0 siblings, 1 reply; 6+ messages in thread From: Chris Wilson @ 2018-05-23 14:53 UTC (permalink / raw) To: intel-gfx The driver assumes that the kernel context is pinned and always available for use from any process or atomic context. Make it so for selftesting as well! Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> --- drivers/gpu/drm/i915/selftests/mock_engine.c | 7 +++++++ .../gpu/drm/i915/selftests/mock_gem_device.c | 17 +++++++---------- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i915/selftests/mock_engine.c b/drivers/gpu/drm/i915/selftests/mock_engine.c index f1ac7453053e..c2a0451336cf 100644 --- a/drivers/gpu/drm/i915/selftests/mock_engine.c +++ b/drivers/gpu/drm/i915/selftests/mock_engine.c @@ -212,8 +212,13 @@ struct intel_engine_cs *mock_engine(struct drm_i915_private *i915, if (!engine->base.buffer) goto err_breadcrumbs; + if (IS_ERR(intel_context_pin(i915->kernel_context, &engine->base))) + goto err_ring; + return &engine->base; +err_ring: + mock_ring_free(engine->base.buffer); err_breadcrumbs: intel_engine_fini_breadcrumbs(&engine->base); i915_timeline_fini(&engine->base.timeline); @@ -254,6 +259,8 @@ void mock_engine_free(struct intel_engine_cs *engine) if (ce) intel_context_unpin(ce); + __intel_context_unpin(engine->i915->kernel_context, engine); + mock_ring_free(engine->buffer); intel_engine_fini_breadcrumbs(engine); diff --git a/drivers/gpu/drm/i915/selftests/mock_gem_device.c b/drivers/gpu/drm/i915/selftests/mock_gem_device.c index 94baedfa0f74..c97075c5ccaf 100644 --- a/drivers/gpu/drm/i915/selftests/mock_gem_device.c +++ b/drivers/gpu/drm/i915/selftests/mock_gem_device.c @@ -136,8 +136,6 @@ static struct dev_pm_domain pm_domain = { struct drm_i915_private *mock_gem_device(void) { struct drm_i915_private *i915; - struct intel_engine_cs *engine; - enum intel_engine_id id; struct pci_dev *pdev; int err; @@ -233,13 +231,13 @@ struct drm_i915_private *mock_gem_device(void) mock_init_ggtt(i915); mkwrite_device_info(i915)->ring_mask = BIT(0); - i915->engine[RCS] = mock_engine(i915, "mock", RCS); - if (!i915->engine[RCS]) - goto err_unlock; - i915->kernel_context = mock_context(i915, NULL); if (!i915->kernel_context) - goto err_engine; + goto err_unlock; + + i915->engine[RCS] = mock_engine(i915, "mock", RCS); + if (!i915->engine[RCS]) + goto err_context; mutex_unlock(&i915->drm.struct_mutex); @@ -247,9 +245,8 @@ struct drm_i915_private *mock_gem_device(void) return i915; -err_engine: - for_each_engine(engine, i915, id) - mock_engine_free(engine); +err_context: + i915_gem_contexts_fini(i915); err_unlock: mutex_unlock(&i915->drm.struct_mutex); kmem_cache_destroy(i915->priorities); -- 2.17.0 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] drm/i915: Look for an active kernel context before switching 2018-05-23 14:53 [PATCH 1/2] drm/i915/selftests: Pin " Chris Wilson @ 2018-05-23 14:53 ` Chris Wilson 0 siblings, 0 replies; 6+ messages in thread From: Chris Wilson @ 2018-05-23 14:53 UTC (permalink / raw) To: intel-gfx; +Cc: Mika Kuoppala We were not very carefully checking to see if an older request on the engine was an earlier switch-to-kernel-context before deciding to emit a new switch. The end result would be that we could get into a permanent loop of trying to emit a new request to perform the switch simply to flush the existing switch. What we need is a means of tracking the completion of each timeline versus the kernel context, that is to detect if a more recent request has been submitted that would result in a switch away from the kernel context. To realise this, we need only to look in our syncmap on the kernel context and check that we have synchronized against all active rings. v2: Since all ringbuffer clients currently share the same timeline, we do have to use the gem_context to distinguish clients. As a bonus, include all the tracing used to debug the death inside suspend. Reported-by: Mika Kuoppala <mika.kuoppala@intel.com> Fixes: a89d1f921c15 ("drm/i915: Split i915_gem_timeline into individual timelines") Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Mika Kuoppala <mika.kuoppala@intel.com> --- drivers/gpu/drm/i915/i915_gem.c | 7 +++ drivers/gpu/drm/i915/i915_gem_context.c | 67 +++++++++++++++++++++---- drivers/gpu/drm/i915/i915_request.c | 5 +- 3 files changed, 67 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 03874b50ada9..05f44ca35a06 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -3703,6 +3703,9 @@ static int wait_for_engines(struct drm_i915_private *i915) int i915_gem_wait_for_idle(struct drm_i915_private *i915, unsigned int flags) { + GEM_TRACE("flags=%x (%s)\n", + flags, flags & I915_WAIT_LOCKED ? "locked" : "unlocked"); + /* If the device is asleep, we have no requests outstanding */ if (!READ_ONCE(i915->gt.awake)) return 0; @@ -3719,6 +3722,7 @@ int i915_gem_wait_for_idle(struct drm_i915_private *i915, unsigned int flags) return err; } i915_retire_requests(i915); + GEM_BUG_ON(i915->gt.active_requests); return wait_for_engines(i915); } else { @@ -4901,6 +4905,7 @@ static void assert_kernel_context_is_current(struct drm_i915_private *i915) struct intel_engine_cs *engine; enum intel_engine_id id; + GEM_BUG_ON(i915->gt.active_requests); for_each_engine(engine, i915, id) { GEM_BUG_ON(__i915_gem_active_peek(&engine->timeline.last_request)); GEM_BUG_ON(engine->last_retired_context->gem_context != kctx); @@ -4932,6 +4937,8 @@ int i915_gem_suspend(struct drm_i915_private *dev_priv) struct drm_device *dev = &dev_priv->drm; int ret; + GEM_TRACE("\n"); + intel_runtime_pm_get(dev_priv); intel_suspend_gt_powersave(dev_priv); diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index b69b18ef8120..82198bfacb7f 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -581,24 +581,58 @@ last_request_on_engine(struct i915_timeline *timeline, rq = i915_gem_active_raw(&timeline->last_request, &engine->i915->drm.struct_mutex); - if (rq && rq->engine == engine) + if (rq && rq->engine == engine) { + GEM_TRACE("last request for %s on engine %s: %llx:%d\n", + timeline->name, engine->name, + rq->fence.context, rq->fence.seqno); return rq; + } return NULL; } -static bool engine_has_idle_kernel_context(struct intel_engine_cs *engine) +static bool engine_has_kernel_context_barrier(struct intel_engine_cs *engine) { - struct list_head * const active_rings = &engine->i915->gt.active_rings; + struct drm_i915_private *i915 = engine->i915; + struct i915_timeline *barrier = + to_intel_context(i915->kernel_context, engine)->ring->timeline; struct intel_ring *ring; + bool any_active = false; - lockdep_assert_held(&engine->i915->drm.struct_mutex); + lockdep_assert_held(&i915->drm.struct_mutex); + list_for_each_entry(ring, &i915->gt.active_rings, active_link) { + struct i915_request *rq; + + rq = last_request_on_engine(ring->timeline, engine); + if (!rq) + continue; - list_for_each_entry(ring, active_rings, active_link) { - if (last_request_on_engine(ring->timeline, engine)) + if (rq->gem_context == i915->kernel_context) + continue; + + /* + * Was this request submitted after the previous + * switch-to-kernel-context? + */ + if (!i915_timeline_sync_is_later(barrier, &rq->fence)) { + GEM_TRACE("%s needs barrier\n", ring->timeline->name); return false; + } + + GEM_TRACE("%s has barrier\n", ring->timeline->name); + any_active = true; } + /* + * If any other timeline was still active and behind the last barrier, + * then our last switch-to-kernel-context must still be queued and + * will run last (leaving the engine in the kernel context when it + * eventually idles). + */ + if (any_active) + return true; + + /* The engine is idle; check that it is idling in the kernel context. */ return intel_engine_has_kernel_context(engine); } @@ -607,7 +641,10 @@ int i915_gem_switch_to_kernel_context(struct drm_i915_private *i915) struct intel_engine_cs *engine; enum intel_engine_id id; + GEM_TRACE("\n"); + lockdep_assert_held(&i915->drm.struct_mutex); + GEM_BUG_ON(!i915->kernel_context); i915_retire_requests(i915); @@ -615,9 +652,12 @@ int i915_gem_switch_to_kernel_context(struct drm_i915_private *i915) struct intel_ring *ring; struct i915_request *rq; - if (engine_has_idle_kernel_context(engine)) + GEM_BUG_ON(!to_intel_context(i915->kernel_context, engine)); + if (engine_has_kernel_context_barrier(engine)) continue; + GEM_TRACE("emit barrier on %s\n", engine->name); + rq = i915_request_alloc(engine, i915->kernel_context); if (IS_ERR(rq)) return PTR_ERR(rq); @@ -627,10 +667,15 @@ int i915_gem_switch_to_kernel_context(struct drm_i915_private *i915) struct i915_request *prev; prev = last_request_on_engine(ring->timeline, engine); - if (prev) - i915_sw_fence_await_sw_fence_gfp(&rq->submit, - &prev->submit, - I915_FENCE_GFP); + if (!prev) + continue; + + if (prev->gem_context == i915->kernel_context) + continue; + + i915_sw_fence_await_sw_fence_gfp(&rq->submit, + &prev->submit, + I915_FENCE_GFP); } /* diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c index fc499bcbd105..f187250e60c6 100644 --- a/drivers/gpu/drm/i915/i915_request.c +++ b/drivers/gpu/drm/i915/i915_request.c @@ -320,6 +320,7 @@ static void advance_ring(struct i915_request *request) * is just about to be. Either works, if we miss the last two * noops - they are safe to be replayed on a reset. */ + GEM_TRACE("marking %s as inactive\n", ring->timeline->name); tail = READ_ONCE(request->tail); list_del(&ring->active_link); } else { @@ -1095,8 +1096,10 @@ void __i915_request_add(struct i915_request *request, bool flush_caches) i915_gem_active_set(&timeline->last_request, request); list_add_tail(&request->ring_link, &ring->request_list); - if (list_is_first(&request->ring_link, &ring->request_list)) + if (list_is_first(&request->ring_link, &ring->request_list)) { + GEM_TRACE("marking %s as active\n", ring->timeline->name); list_add(&ring->active_link, &request->i915->gt.active_rings); + } request->emitted_jiffies = jiffies; /* -- 2.17.0 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-05-23 14:53 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-05-23 9:51 [PATCH 1/2] drm/i915/selftests: Pinned the mock kernel context Chris Wilson 2018-05-23 9:51 ` [PATCH 2/2] drm/i915: Look for an active kernel context before switching Chris Wilson 2018-05-23 10:31 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915/selftests: Pinned the mock kernel context Patchwork 2018-05-23 12:41 ` [PATCH 1/2] " Mika Kuoppala 2018-05-23 12:45 ` ✓ Fi.CI.IGT: success for series starting with [1/2] " Patchwork 2018-05-23 14:53 [PATCH 1/2] drm/i915/selftests: Pin " Chris Wilson 2018-05-23 14:53 ` [PATCH 2/2] drm/i915: Look for an active kernel context before switching 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.