[PATCH 1/4] drm/i915: Wait for inflight writes before checking intel_engine_is_idle()

[PATCH 1/4] drm/i915: Wait for inflight writes before checking intel_engine_is_idle()

[Chris Wilson]

Some GPUs may have writes inflight much longer than expected, so before
declaring the GPU is idle, try to flush them using any
engine->irq_seqno_barrier() if available. By allowing them to be
flushed, we can be a little more confident that the GPU really is idle
when we think it is!

References: https://bugs.freedesktop.org/show_bug.cgi?id=98836
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_engine_cs.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index c76a64483d64..ff6d0e1d1306 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -1074,6 +1074,10 @@ bool intel_engine_is_idle(struct intel_engine_cs *engine)
 {
 	struct drm_i915_private *dev_priv = engine->i915;
 
+	/* We have to allow time for writes to land from the GPU. */
+	if (engine->irq_seqno_barrier)
+		engine->irq_seqno_barrier(engine);
+
 	/* Any inflight/incomplete requests? */
 	if (!i915_seqno_passed(intel_engine_get_seqno(engine),
 			       intel_engine_last_submit(engine)))
-- 
2.11.0

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

[PATCH 2/4] drm/i915: Wait for all engines to be idle as part of i915_gem_wait_for_idle()

[Chris Wilson]

Make i915_gem_wait_for_idle() be a little heavier in order to try and
guarantee that the GPU is indeed idle (by checking each engine
individually is idle, i.e. all writes are complete and the rings
stopped) after waiting for in-flight requests to be completed.

References: https://bugs.freedesktop.org/show_bug.cgi?id=98836
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_gem.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index ec194d3e6cd3..5f0aff016eb1 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3054,6 +3054,19 @@ static int wait_for_timeline(struct i915_gem_timeline *tl, unsigned int flags)
 	return 0;
 }
 
+static int wait_for_engines(struct drm_i915_private *i915)
+{
+	struct intel_engine_cs *engine;
+	enum intel_engine_id id;
+
+	for_each_engine(engine, i915, id) {
+		if (GEM_WARN_ON(wait_for(intel_engine_is_idle(engine), 50)))
+			return -EBUSY;
+	}
+
+	return 0;
+}
+
 int i915_gem_wait_for_idle(struct drm_i915_private *i915, unsigned int flags)
 {
 	int ret;
@@ -3070,10 +3083,10 @@ int i915_gem_wait_for_idle(struct drm_i915_private *i915, unsigned int flags)
 			if (ret)
 				return ret;
 		}
+
+		ret = wait_for_engines(i915);
 	} else {
 		ret = wait_for_timeline(&i915->gt.global_timeline, flags);
-		if (ret)
-			return ret;
 	}
 
 	return 0;
-- 
2.11.0

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

[PATCH 3/4] drm/i915: Remove redudant wait for each engine to idle from seqno wrap

[Chris Wilson]

Having added the wait upon each engine to idle into the central
i915_gem_wait_for_idle(), we can remove the now redundant wait from
reset_all_global_seqno(). This has the advantage of removing the late
detection of an error (an engine still busy) which left the seqno reset
only partially complete (though it should be safe enough!).

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_gem_request.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c
index 45299a8b5e4b..5439a21472bb 100644
--- a/drivers/gpu/drm/i915/i915_gem_request.c
+++ b/drivers/gpu/drm/i915/i915_gem_request.c
@@ -208,9 +208,6 @@ static int reset_all_global_seqno(struct drm_i915_private *i915, u32 seqno)
 	for_each_engine(engine, i915, id) {
 		struct intel_timeline *tl = &timeline->engine[id];
 
-		if (wait_for(intel_engine_is_idle(engine), 50))
-			return -EBUSY;
-
 		if (!i915_seqno_passed(seqno, tl->seqno)) {
 			/* spin until threads are complete */
 			while (intel_breadcrumbs_busy(engine))
-- 
2.11.0

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

[PATCH 4/4] drm/i915: Combine reset_all_global_seqno() loops into one

[Chris Wilson]

We can merge the pair of loops over the engines and their timelines into
a single loop, making it easier to read and more consistent with the
commentary.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_gem_request.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c
index 5439a21472bb..76e4a83c2e53 100644
--- a/drivers/gpu/drm/i915/i915_gem_request.c
+++ b/drivers/gpu/drm/i915/i915_gem_request.c
@@ -189,7 +189,6 @@ i915_priotree_init(struct i915_priotree *pt)
 
 static int reset_all_global_seqno(struct drm_i915_private *i915, u32 seqno)
 {
-	struct i915_gem_timeline *timeline = &i915->gt.global_timeline;
 	struct intel_engine_cs *engine;
 	enum intel_engine_id id;
 	int ret;
@@ -217,14 +216,10 @@ static int reset_all_global_seqno(struct drm_i915_private *i915, u32 seqno)
 		/* Finally reset hw state */
 		tl->seqno = seqno;
 		intel_engine_init_global_seqno(engine, seqno);
-	}
-
-	list_for_each_entry(timeline, &i915->gt.timelines, link) {
-		for_each_engine(engine, i915, id) {
-			struct intel_timeline *tl = &timeline->engine[id];
 
-			memset(tl->sync_seqno, 0, sizeof(tl->sync_seqno));
-		}
+		list_for_each_entry(timeline, &i915->gt.timelines, link)
+			memset(timeline->engine[id].sync_seqno, 0,
+			       sizeof(timeline->engine[id].sync_seqno));
 	}
 
 	return 0;
-- 
2.11.0

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

✗ Fi.CI.BAT: failure for series starting with [1/4] drm/i915: Wait for inflight writes before checking intel_engine_is_idle()

[Patchwork]

== Series Details ==

Series: series starting with [1/4] drm/i915: Wait for inflight writes before checking intel_engine_is_idle()
URL   : https://patchwork.freedesktop.org/series/22126/
State : failure

== Summary ==

  LD [M]  drivers/net/ethernet/broadcom/genet/genet.o
  LD      net/packet/built-in.o
  LD      kernel/built-in.o
  LD      lib/xz/xz_dec.o
  LD      lib/xz/built-in.o
  LD      drivers/acpi/built-in.o
  LD [M]  sound/pci/hda/snd-hda-intel.o
  LD      drivers/video/fbdev/core/fb.o
  LD      drivers/video/fbdev/core/built-in.o
  LD      drivers/scsi/sd_mod.o
  LD      drivers/video/fbdev/built-in.o
  LD      drivers/scsi/built-in.o
  LD [M]  drivers/usb/serial/usbserial.o
  LD      drivers/pci/built-in.o
  LD [M]  sound/pci/hda/snd-hda-codec-realtek.o
  LD      drivers/usb/gadget/udc/udc-core.o
  LD      drivers/usb/gadget/udc/built-in.o
  LD      drivers/usb/gadget/built-in.o
  LD [M]  drivers/net/ethernet/intel/igbvf/igbvf.o
  LD      drivers/video/console/built-in.o
  LD      drivers/video/built-in.o
drivers/gpu/drm/i915/i915_gem_request.c: In function ‘reset_all_global_seqno’:
drivers/gpu/drm/i915/i915_gem_request.c:199:32: error: ‘timeline’ undeclared (first use in this function)
   struct intel_timeline *tl = &timeline->engine[id];
                                ^
drivers/gpu/drm/i915/i915_gem_request.c:199:32: note: each undeclared identifier is reported only once for each function it appears in
In file included from ./arch/x86/include/asm/percpu.h:44:0,
                 from ./arch/x86/include/asm/current.h:5,
                 from ./arch/x86/include/asm/processor.h:15,
                 from ./include/linux/prefetch.h:14,
                 from drivers/gpu/drm/i915/i915_gem_request.c:25:
./include/linux/kernel.h:852:48: error: initialization from incompatible pointer type [-Werror=incompatible-pointer-types]
  const typeof( ((type *)0)->member ) *__mptr = (ptr); \
                                                ^
./include/linux/list.h:365:2: note: in expansion of macro ‘container_of’
  container_of(ptr, type, member)
  ^
./include/linux/list.h:376:2: note: in expansion of macro ‘list_entry’
  list_entry((ptr)->next, type, member)
  ^
./include/linux/list.h:463:13: note: in expansion of macro ‘list_first_entry’
  for (pos = list_first_entry(head, typeof(*pos), member); \
             ^
drivers/gpu/drm/i915/i915_gem_request.c:211:3: note: in expansion of macro ‘list_for_each_entry’
   list_for_each_entry(timeline, &i915->gt.timelines, link)
   ^
  LD [M]  sound/pci/hda/snd-hda-codec-hdmi.o
  LD      lib/raid6/raid6_pq.o
  LD      lib/raid6/built-in.o
cc1: all warnings being treated as errors
scripts/Makefile.build:294: recipe for target 'drivers/gpu/drm/i915/i915_gem_request.o' failed
make[4]: *** [drivers/gpu/drm/i915/i915_gem_request.o] Error 1
make[4]: *** Waiting for unfinished jobs....
  LD      net/xfrm/built-in.o
  LD      net/ipv6/ipv6.o
  LD      drivers/tty/serial/8250/8250_base.o
  LD      drivers/tty/serial/8250/built-in.o
  LD      drivers/tty/serial/built-in.o
  LD [M]  drivers/net/ethernet/intel/e1000/e1000.o
  LD      net/ipv6/built-in.o
  LD      drivers/usb/core/usbcore.o
  LD      drivers/usb/core/built-in.o
  LD      fs/btrfs/btrfs.o
  LD      fs/btrfs/built-in.o
  LD      lib/lz4/built-in.o
  LD [M]  drivers/net/ethernet/intel/e1000e/e1000e.o
  LD      drivers/md/md-mod.o
  LD      drivers/md/built-in.o
  LD      drivers/usb/host/xhci-hcd.o
  LD      net/ipv4/built-in.o
  CC      arch/x86/kernel/cpu/capflags.o
  AR      lib/lib.a
  LD      arch/x86/kernel/cpu/built-in.o
  EXPORTS lib/lib-ksyms.o
  LD      arch/x86/kernel/built-in.o
  LD [M]  drivers/net/ethernet/intel/igb/igb.o
  LD      drivers/tty/vt/built-in.o
  LD      net/core/built-in.o
  LD      drivers/tty/built-in.o
  LD      arch/x86/built-in.o
  LD [M]  sound/pci/hda/snd-hda-codec-generic.o
  LD      sound/pci/built-in.o
  LD      lib/built-in.o
  LD      net/built-in.o
  LD      sound/built-in.o
  LD      drivers/usb/host/built-in.o
  LD      drivers/usb/built-in.o
  LD      fs/ext4/ext4.o
  LD      fs/ext4/built-in.o
  LD      fs/built-in.o
  LD      drivers/net/ethernet/built-in.o
  LD      drivers/net/built-in.o
scripts/Makefile.build:553: recipe for target 'drivers/gpu/drm/i915' failed
make[3]: *** [drivers/gpu/drm/i915] Error 2
scripts/Makefile.build:553: recipe for target 'drivers/gpu/drm' failed
make[2]: *** [drivers/gpu/drm] Error 2
scripts/Makefile.build:553: recipe for target 'drivers/gpu' failed
make[1]: *** [drivers/gpu] Error 2
Makefile:1002: recipe for target 'drivers' failed
make: *** [drivers] Error 2

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

[PATCH] drm/i915: Combine reset_all_global_seqno() loops into one

[Chris Wilson]

We can merge the pair of loops over the engines and their timelines into
a single loop, making it easier to read and more consistent with the
commentary.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
git add ftw
---
 drivers/gpu/drm/i915/i915_gem_request.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c
index 79c30bbcfdcb..6abc1a8d20c5 100644
--- a/drivers/gpu/drm/i915/i915_gem_request.c
+++ b/drivers/gpu/drm/i915/i915_gem_request.c
@@ -180,7 +180,6 @@ i915_priotree_init(struct i915_priotree *pt)
 
 static int reset_all_global_seqno(struct drm_i915_private *i915, u32 seqno)
 {
-	struct i915_gem_timeline *timeline = &i915->gt.global_timeline;
 	struct intel_engine_cs *engine;
 	enum intel_engine_id id;
 	int ret;
@@ -197,7 +196,8 @@ static int reset_all_global_seqno(struct drm_i915_private *i915, u32 seqno)
 
 	/* If the seqno wraps around, we need to clear the breadcrumb rbtree */
 	for_each_engine(engine, i915, id) {
-		struct intel_timeline *tl = &timeline->engine[id];
+		struct i915_gem_timeline *timeline;
+		struct intel_timeline *tl = engine->timeline;
 
 		if (!i915_seqno_passed(seqno, tl->seqno)) {
 			/* spin until threads are complete */
@@ -208,14 +208,10 @@ static int reset_all_global_seqno(struct drm_i915_private *i915, u32 seqno)
 		/* Finally reset hw state */
 		tl->seqno = seqno;
 		intel_engine_init_global_seqno(engine, seqno);
-	}
-
-	list_for_each_entry(timeline, &i915->gt.timelines, link) {
-		for_each_engine(engine, i915, id) {
-			struct intel_timeline *tl = &timeline->engine[id];
 
-			memset(tl->sync_seqno, 0, sizeof(tl->sync_seqno));
-		}
+		list_for_each_entry(timeline, &i915->gt.timelines, link)
+			memset(timeline->engine[id].sync_seqno, 0,
+			       sizeof(timeline->engine[id].sync_seqno));
 	}
 
 	return 0;
-- 
2.11.0

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

✓ Fi.CI.BAT: success for series starting with [1/4] drm/i915: Wait for inflight writes before checking intel_engine_is_idle() (rev2)

[Patchwork]

== Series Details ==

Series: series starting with [1/4] drm/i915: Wait for inflight writes before checking intel_engine_is_idle() (rev2)
URL   : https://patchwork.freedesktop.org/series/22126/
State : success

== Summary ==

Series 22126v2 Series without cover letter
https://patchwork.freedesktop.org/api/1.0/series/22126/revisions/2/mbox/

Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-c:
                incomplete -> PASS       (fi-skl-6260u)

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time: 428s
fi-bdw-gvtdvm    total:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  time: 433s
fi-bsw-n3050     total:278  pass:239  dwarn:0   dfail:0   fail:0   skip:39  time: 586s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time: 507s
fi-bxt-t5700     total:278  pass:258  dwarn:0   dfail:0   fail:0   skip:20  time: 534s
fi-byt-j1900     total:278  pass:251  dwarn:0   dfail:0   fail:0   skip:27  time: 488s
fi-byt-n2820     total:278  pass:247  dwarn:0   dfail:0   fail:0   skip:31  time: 492s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time: 411s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time: 405s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time: 421s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 487s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 470s
fi-kbl-7500u     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 455s
fi-kbl-7560u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 672s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 447s
fi-skl-6700hq    total:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17  time: 573s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time: 463s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 493s
fi-skl-gvtdvm    total:278  pass:265  dwarn:0   dfail:0   fail:0   skip:13  time: 432s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time: 530s
fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29  time: 405s

e2bbafd9baa779a48160c2ffbb5fae6d6afe8d65 drm-tip: 2017y-03m-29d-20h-27m-37s UTC integration manifest
9255101 drm/i915: Combine reset_all_global_seqno() loops into one
4debcd5 drm/i915: Remove redudant wait for each engine to idle from seqno wrap
9a291fa drm/i915: Wait for all engines to be idle as part of i915_gem_wait_for_idle()
342656d drm/i915: Wait for inflight writes before checking intel_engine_is_idle()

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4349/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/4] drm/i915: Wait for all engines to be idle as part of i915_gem_wait_for_idle()

[Chris Wilson]

On Wed, Mar 29, 2017 at 10:36:17PM +0100, Chris Wilson wrote:
> Make i915_gem_wait_for_idle() be a little heavier in order to try and
> guarantee that the GPU is indeed idle (by checking each engine
> individually is idle, i.e. all writes are complete and the rings
> stopped) after waiting for in-flight requests to be completed.
> 
> References: https://bugs.freedesktop.org/show_bug.cgi?id=98836
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_gem.c | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index ec194d3e6cd3..5f0aff016eb1 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -3054,6 +3054,19 @@ static int wait_for_timeline(struct i915_gem_timeline *tl, unsigned int flags)
>  	return 0;
>  }
>  
> +static int wait_for_engines(struct drm_i915_private *i915)
> +{
> +	struct intel_engine_cs *engine;
> +	enum intel_engine_id id;
> +
> +	for_each_engine(engine, i915, id) {
> +		if (GEM_WARN_ON(wait_for(intel_engine_is_idle(engine), 50)))
> +			return -EBUSY;
> +	}
> +
> +	return 0;
> +}
> +
>  int i915_gem_wait_for_idle(struct drm_i915_private *i915, unsigned int flags)
>  {
>  	int ret;
> @@ -3070,10 +3083,10 @@ int i915_gem_wait_for_idle(struct drm_i915_private *i915, unsigned int flags)
>  			if (ret)
>  				return ret;
>  		}
> +
> +		ret = wait_for_engines(i915);
>  	} else {
>  		ret = wait_for_timeline(&i915->gt.global_timeline, flags);
> -		if (ret)
> -			return ret;
>  	}
>  
>  	return 0;

Forgot to change this to return ret;
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/4] drm/i915: Wait for all engines to be idle as part of i915_gem_wait_for_idle()

[Chris Wilson]

On Wed, Mar 29, 2017 at 10:36:17PM +0100, Chris Wilson wrote:
> Make i915_gem_wait_for_idle() be a little heavier in order to try and
> guarantee that the GPU is indeed idle (by checking each engine
> individually is idle, i.e. all writes are complete and the rings
> stopped) after waiting for in-flight requests to be completed.
> 
> References: https://bugs.freedesktop.org/show_bug.cgi?id=98836
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_gem.c | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index ec194d3e6cd3..5f0aff016eb1 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -3054,6 +3054,19 @@ static int wait_for_timeline(struct i915_gem_timeline *tl, unsigned int flags)
>  	return 0;
>  }
>  
> +static int wait_for_engines(struct drm_i915_private *i915)
> +{
> +	struct intel_engine_cs *engine;
> +	enum intel_engine_id id;
> +
> +	for_each_engine(engine, i915, id) {
> +		if (GEM_WARN_ON(wait_for(intel_engine_is_idle(engine), 50)))

And I might want to consider an inline for
wait_for(intel_engine_is_idle)) to avoid the hideous string :)
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/4] drm/i915: Wait for inflight writes before checking intel_engine_is_idle()

[Chris Wilson]

On Wed, Mar 29, 2017 at 10:36:16PM +0100, Chris Wilson wrote:
> Some GPUs may have writes inflight much longer than expected, so before
> declaring the GPU is idle, try to flush them using any
> engine->irq_seqno_barrier() if available. By allowing them to be
> flushed, we can be a little more confident that the GPU really is idle
> when we think it is!
> 
> References: https://bugs.freedesktop.org/show_bug.cgi?id=98836
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/intel_engine_cs.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
> index c76a64483d64..ff6d0e1d1306 100644
> --- a/drivers/gpu/drm/i915/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
> @@ -1074,6 +1074,10 @@ bool intel_engine_is_idle(struct intel_engine_cs *engine)
>  {
>  	struct drm_i915_private *dev_priv = engine->i915;
>  
> +	/* We have to allow time for writes to land from the GPU. */
> +	if (engine->irq_seqno_barrier)
> +		engine->irq_seqno_barrier(engine);
> +

Problem persists so ignore this patch. 2-4 are still a nice cleanup and
worth persuing.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/4] drm/i915: Wait for inflight writes before checking intel_engine_is_idle()

[Mika Kuoppala]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> On Wed, Mar 29, 2017 at 10:36:16PM +0100, Chris Wilson wrote:
>> Some GPUs may have writes inflight much longer than expected, so before
>> declaring the GPU is idle, try to flush them using any
>> engine->irq_seqno_barrier() if available. By allowing them to be
>> flushed, we can be a little more confident that the GPU really is idle
>> when we think it is!
>> 
>> References: https://bugs.freedesktop.org/show_bug.cgi?id=98836
>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>> ---
>>  drivers/gpu/drm/i915/intel_engine_cs.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>> 
>> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
>> index c76a64483d64..ff6d0e1d1306 100644
>> --- a/drivers/gpu/drm/i915/intel_engine_cs.c
>> +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
>> @@ -1074,6 +1074,10 @@ bool intel_engine_is_idle(struct intel_engine_cs *engine)
>>  {
>>  	struct drm_i915_private *dev_priv = engine->i915;
>>  
>> +	/* We have to allow time for writes to land from the GPU. */
>> +	if (engine->irq_seqno_barrier)
>> +		engine->irq_seqno_barrier(engine);
>> +
>
> Problem persists so ignore this patch. 2-4 are still a nice cleanup and
> worth persuing.

Patch 1 looked good too.

> -Chris
>
> -- 
> Chris Wilson, Intel Open Source Technology Centre
> _______________________________________________
> 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

Re: [PATCH 1/4] drm/i915: Wait for inflight writes before checking intel_engine_is_idle()

[Chris Wilson]

On Thu, Mar 30, 2017 at 11:29:40AM +0300, Mika Kuoppala wrote:
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > On Wed, Mar 29, 2017 at 10:36:16PM +0100, Chris Wilson wrote:
> >> Some GPUs may have writes inflight much longer than expected, so before
> >> declaring the GPU is idle, try to flush them using any
> >> engine->irq_seqno_barrier() if available. By allowing them to be
> >> flushed, we can be a little more confident that the GPU really is idle
> >> when we think it is!
> >> 
> >> References: https://bugs.freedesktop.org/show_bug.cgi?id=98836
> >> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> >> ---
> >>  drivers/gpu/drm/i915/intel_engine_cs.c | 4 ++++
> >>  1 file changed, 4 insertions(+)
> >> 
> >> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
> >> index c76a64483d64..ff6d0e1d1306 100644
> >> --- a/drivers/gpu/drm/i915/intel_engine_cs.c
> >> +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
> >> @@ -1074,6 +1074,10 @@ bool intel_engine_is_idle(struct intel_engine_cs *engine)
> >>  {
> >>  	struct drm_i915_private *dev_priv = engine->i915;
> >>  
> >> +	/* We have to allow time for writes to land from the GPU. */
> >> +	if (engine->irq_seqno_barrier)
> >> +		engine->irq_seqno_barrier(engine);
> >> +
> >
> > Problem persists so ignore this patch. 2-4 are still a nice cleanup and
> > worth persuing.
> 
> Patch 1 looked good too.

It works better as the last step, i.e. once we have decided the ring is
idle, add a little extra wait for any inflight writes. It just isn't
fixing the bug that appears to be a delayed seqno write landing after we
perform the seqno wrap. Quite mysterious, I hoped just adding a delay at
the right spot would fix it, my fear is something is amiss around reset
vs seqno-wrap.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx