All of lore.kernel.org
 help / color / mirror / Atom feed
* [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 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.