[PATCH] drm/i915: Be more careful to drop the GT wakeref

[PATCH] drm/i915: Be more careful to drop the GT wakeref

[Chris Wilson]

Since we can retire requests from multiple paths, we cannot assume that
i915_gem_retire_requests() is the sole path on which we can transition
to gt.active_requests == 0. A consequence of this is that we would skip
the function if we had already retired all the requests and not
scheduled the idle worker.

This is fallout from changing the routine from considering active_engines
(over which it was the only consumer) to active_requests.

Fixes: 28176ef4cfa5 ("drm/i915: Reserve space in the global seqno during request allocation")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem_request.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c
index b9b5253cf3cd..2b763026b4c1 100644
--- a/drivers/gpu/drm/i915/i915_gem_request.c
+++ b/drivers/gpu/drm/i915/i915_gem_request.c
@@ -763,6 +763,8 @@ static void i915_gem_mark_busy(const struct intel_engine_cs *engine)
 	if (dev_priv->gt.awake)
 		return;
 
+	GEM_BUG_ON(!dev_priv->gt.active_requests);
+
 	intel_runtime_pm_get_noresume(dev_priv);
 	dev_priv->gt.awake = true;
 
@@ -1143,11 +1145,9 @@ void i915_gem_retire_requests(struct drm_i915_private *dev_priv)
 
 	lockdep_assert_held(&dev_priv->drm.struct_mutex);
 
-	if (!dev_priv->gt.active_requests)
+	if (!dev_priv->gt.awake)
 		return;
 
-	GEM_BUG_ON(!dev_priv->gt.awake);
-
 	for_each_engine(engine, dev_priv, id)
 		engine_retire_requests(engine);
 
-- 
2.10.2

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

[PATCH] drm/i915: Be more careful to drop the GT wakeref

[Chris Wilson]

Since we can retire requests from multiple paths, we cannot assume that
i915_gem_retire_requests() is the sole path on which we can transition
to gt.active_requests == 0. A consequence of this is that we would skip
the function if we had already retired all the requests and not
scheduled the idle worker.

This is fallout from changing the routine from considering active_engines
(over which it was the only consumer) to active_requests.

v2: Move kicking the idle working to i915_gem_request_retire() otherwise
we could postpone the idle callback everytime we called retire_requests
even though we did no work.

Fixes: 28176ef4cfa5 ("drm/i915: Reserve space in the global seqno during request allocation")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem_request.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c
index b9b5253cf3cd..ae4d6e080006 100644
--- a/drivers/gpu/drm/i915/i915_gem_request.c
+++ b/drivers/gpu/drm/i915/i915_gem_request.c
@@ -218,7 +218,10 @@ static void i915_gem_request_retire(struct drm_i915_gem_request *request)
 	 */
 	list_del(&request->ring_link);
 	request->ring->last_retired_head = request->postfix;
-	request->i915->gt.active_requests--;
+	if (!--request->i915->gt.active_requests)
+		mod_delayed_work(request->i915->wq,
+				 &request->i915->gt.idle_work,
+				 msecs_to_jiffies(100));
 
 	/* Walk through the active list, calling retire on each. This allows
 	 * objects to track their GPU activity and mark themselves as idle
@@ -763,6 +766,8 @@ static void i915_gem_mark_busy(const struct intel_engine_cs *engine)
 	if (dev_priv->gt.awake)
 		return;
 
+	GEM_BUG_ON(!dev_priv->gt.active_requests);
+
 	intel_runtime_pm_get_noresume(dev_priv);
 	dev_priv->gt.awake = true;
 
@@ -1143,16 +1148,9 @@ void i915_gem_retire_requests(struct drm_i915_private *dev_priv)
 
 	lockdep_assert_held(&dev_priv->drm.struct_mutex);
 
-	if (!dev_priv->gt.active_requests)
+	if (!dev_priv->gt.awake)
 		return;
 
-	GEM_BUG_ON(!dev_priv->gt.awake);
-
 	for_each_engine(engine, dev_priv, id)
 		engine_retire_requests(engine);
-
-	if (!dev_priv->gt.active_requests)
-		mod_delayed_work(dev_priv->wq,
-				 &dev_priv->gt.idle_work,
-				 msecs_to_jiffies(100));
 }
-- 
2.10.2

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

✓ Fi.CI.BAT: success for drm/i915: Be more careful to drop the GT wakeref

[Patchwork]

== Series Details ==

Series: drm/i915: Be more careful to drop the GT wakeref
URL   : https://patchwork.freedesktop.org/series/15349/
State : success

== Summary ==

Series 15349v1 drm/i915: Be more careful to drop the GT wakeref
https://patchwork.freedesktop.org/api/1.0/series/15349/revisions/1/mbox/


fi-bdw-5557u     total:244  pass:229  dwarn:0   dfail:0   fail:0   skip:15 
fi-bsw-n3050     total:244  pass:204  dwarn:0   dfail:0   fail:0   skip:40 
fi-bxt-t5700     total:244  pass:216  dwarn:0   dfail:0   fail:0   skip:28 
fi-byt-j1900     total:244  pass:216  dwarn:0   dfail:0   fail:0   skip:28 
fi-byt-n2820     total:244  pass:212  dwarn:0   dfail:0   fail:0   skip:32 
fi-hsw-4770      total:244  pass:224  dwarn:0   dfail:0   fail:0   skip:20 
fi-hsw-4770r     total:244  pass:224  dwarn:0   dfail:0   fail:0   skip:20 
fi-ilk-650       total:244  pass:191  dwarn:0   dfail:0   fail:0   skip:53 
fi-ivb-3520m     total:244  pass:222  dwarn:0   dfail:0   fail:0   skip:22 
fi-ivb-3770      total:244  pass:222  dwarn:0   dfail:0   fail:0   skip:22 
fi-kbl-7200u     total:244  pass:222  dwarn:0   dfail:0   fail:0   skip:22 
fi-skl-6260u     total:244  pass:230  dwarn:0   dfail:0   fail:0   skip:14 
fi-skl-6700hq    total:244  pass:223  dwarn:0   dfail:0   fail:0   skip:21 
fi-skl-6700k     total:244  pass:222  dwarn:1   dfail:0   fail:0   skip:21 
fi-skl-6770hq    total:244  pass:230  dwarn:0   dfail:0   fail:0   skip:14 
fi-snb-2520m     total:244  pass:212  dwarn:0   dfail:0   fail:0   skip:32 
fi-snb-2600      total:244  pass:211  dwarn:0   dfail:0   fail:0   skip:33 

df273900d97a2fba0523a93ec782394eb082cba2 drm-intel-nightly: 2016y-11m-15d-12h-14m-39s UTC integration manifest
9c564ae drm/i915: Be more careful to drop the GT wakeref

== Logs ==

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

[PATCH v3] drm/i915: Be more careful to drop the GT wakeref

[Chris Wilson]

Since we can retire requests from multiple paths, we cannot assume that
i915_gem_retire_requests() is the sole path on which we can transition
to gt.active_requests == 0. A consequence of this is that we would skip
the function if we had already retired all the requests and not
scheduled the idle worker.

This is fallout from changing the routine from considering active_engines
(over which it was the only consumer) to active_requests.

v2: Move kicking the idle working to i915_gem_request_retire() otherwise
we could postpone the idle callback everytime we called retire_requests
even though we did no work.
v3: We only need to move the idle work kicking!

Fixes: 28176ef4cfa5 ("drm/i915: Reserve space in the global seqno during request allocation")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem_request.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c
index b9b5253cf3cd..b2b9e23d5902 100644
--- a/drivers/gpu/drm/i915/i915_gem_request.c
+++ b/drivers/gpu/drm/i915/i915_gem_request.c
@@ -218,7 +218,12 @@ static void i915_gem_request_retire(struct drm_i915_gem_request *request)
 	 */
 	list_del(&request->ring_link);
 	request->ring->last_retired_head = request->postfix;
-	request->i915->gt.active_requests--;
+	if (!--request->i915->gt.active_requests) {
+		GEM_BUG_ON(!dev_priv->gt.awake);
+		mod_delayed_work(request->i915->wq,
+				 &request->i915->gt.idle_work,
+				 msecs_to_jiffies(100));
+	}
 
 	/* Walk through the active list, calling retire on each. This allows
 	 * objects to track their GPU activity and mark themselves as idle
@@ -763,6 +768,8 @@ static void i915_gem_mark_busy(const struct intel_engine_cs *engine)
 	if (dev_priv->gt.awake)
 		return;
 
+	GEM_BUG_ON(!dev_priv->gt.active_requests);
+
 	intel_runtime_pm_get_noresume(dev_priv);
 	dev_priv->gt.awake = true;
 
@@ -1150,9 +1157,4 @@ void i915_gem_retire_requests(struct drm_i915_private *dev_priv)
 
 	for_each_engine(engine, dev_priv, id)
 		engine_retire_requests(engine);
-
-	if (!dev_priv->gt.active_requests)
-		mod_delayed_work(dev_priv->wq,
-				 &dev_priv->gt.idle_work,
-				 msecs_to_jiffies(100));
 }
-- 
2.10.2

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

[PATCH v4] drm/i915: Be more careful to drop the GT wakeref

[Chris Wilson]

Since we can retire requests from multiple paths, we cannot assume that
i915_gem_retire_requests() is the sole path on which we can transition
to gt.active_requests == 0. A consequence of this is that we would skip
the function if we had already retired all the requests and not
scheduled the idle worker.

This is fallout from changing the routine from considering active_engines
(over which it was the only consumer) to active_requests.

v2: Move kicking the idle working to i915_gem_request_retire() otherwise
we could postpone the idle callback everytime we called retire_requests
even though we did no work.
v3: We only need to move the idle work kicking!

Fixes: 28176ef4cfa5 ("drm/i915: Reserve space in the global seqno during request allocation")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem_request.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c
index b9b5253cf3cd..ad6057dc495e 100644
--- a/drivers/gpu/drm/i915/i915_gem_request.c
+++ b/drivers/gpu/drm/i915/i915_gem_request.c
@@ -218,7 +218,12 @@ static void i915_gem_request_retire(struct drm_i915_gem_request *request)
 	 */
 	list_del(&request->ring_link);
 	request->ring->last_retired_head = request->postfix;
-	request->i915->gt.active_requests--;
+	if (!--request->i915->gt.active_requests) {
+		GEM_BUG_ON(!request->i915->gt.awake);
+		mod_delayed_work(request->i915->wq,
+				 &request->i915->gt.idle_work,
+				 msecs_to_jiffies(100));
+	}
 
 	/* Walk through the active list, calling retire on each. This allows
 	 * objects to track their GPU activity and mark themselves as idle
@@ -763,6 +768,8 @@ static void i915_gem_mark_busy(const struct intel_engine_cs *engine)
 	if (dev_priv->gt.awake)
 		return;
 
+	GEM_BUG_ON(!dev_priv->gt.active_requests);
+
 	intel_runtime_pm_get_noresume(dev_priv);
 	dev_priv->gt.awake = true;
 
@@ -1150,9 +1157,4 @@ void i915_gem_retire_requests(struct drm_i915_private *dev_priv)
 
 	for_each_engine(engine, dev_priv, id)
 		engine_retire_requests(engine);
-
-	if (!dev_priv->gt.active_requests)
-		mod_delayed_work(dev_priv->wq,
-				 &dev_priv->gt.idle_work,
-				 msecs_to_jiffies(100));
 }
-- 
2.10.2

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

✓ Fi.CI.BAT: success for drm/i915: Be more careful to drop the GT wakeref (rev4)

[Patchwork]

== Series Details ==

Series: drm/i915: Be more careful to drop the GT wakeref (rev4)
URL   : https://patchwork.freedesktop.org/series/15349/
State : success

== Summary ==

Series 15349v4 drm/i915: Be more careful to drop the GT wakeref
https://patchwork.freedesktop.org/api/1.0/series/15349/revisions/4/mbox/


fi-bdw-5557u     total:244  pass:229  dwarn:0   dfail:0   fail:0   skip:15 
fi-bsw-n3050     total:244  pass:204  dwarn:0   dfail:0   fail:0   skip:40 
fi-bxt-t5700     total:244  pass:216  dwarn:0   dfail:0   fail:0   skip:28 
fi-byt-j1900     total:244  pass:216  dwarn:0   dfail:0   fail:0   skip:28 
fi-byt-n2820     total:244  pass:212  dwarn:0   dfail:0   fail:0   skip:32 
fi-hsw-4770      total:244  pass:224  dwarn:0   dfail:0   fail:0   skip:20 
fi-hsw-4770r     total:244  pass:224  dwarn:0   dfail:0   fail:0   skip:20 
fi-ilk-650       total:244  pass:191  dwarn:0   dfail:0   fail:0   skip:53 
fi-ivb-3520m     total:244  pass:222  dwarn:0   dfail:0   fail:0   skip:22 
fi-ivb-3770      total:244  pass:222  dwarn:0   dfail:0   fail:0   skip:22 
fi-kbl-7200u     total:244  pass:222  dwarn:0   dfail:0   fail:0   skip:22 
fi-skl-6260u     total:244  pass:230  dwarn:0   dfail:0   fail:0   skip:14 
fi-skl-6700hq    total:244  pass:223  dwarn:0   dfail:0   fail:0   skip:21 
fi-skl-6700k     total:244  pass:222  dwarn:1   dfail:0   fail:0   skip:21 
fi-skl-6770hq    total:244  pass:230  dwarn:0   dfail:0   fail:0   skip:14 
fi-snb-2520m     total:244  pass:212  dwarn:0   dfail:0   fail:0   skip:32 
fi-snb-2600      total:244  pass:211  dwarn:0   dfail:0   fail:0   skip:33 

04145fe15cf8c81c221e62fc9d65d93053f9bd1a drm-intel-nightly: 2016y-11m-15d-14h-47m-07s UTC integration manifest
bcdc305 drm/i915: Be more careful to drop the GT wakeref

== Logs ==

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

[PATCH v5] drm/i915: Be more careful to drop the GT wakeref

[Chris Wilson]

Since we can retire requests from multiple paths, we cannot assume that
i915_gem_retire_requests() is the sole path on which we can transition
to gt.active_requests == 0. A consequence of this is that we would skip
the function if we had already retired all the requests and not
scheduled the idle worker.

This is fallout from changing the routine from considering active_engines
(over which it was the only consumer) to active_requests.

v2: Move kicking the idle working to i915_gem_request_retire() otherwise
we could postpone the idle callback everytime we called retire_requests
even though we did no work.
v3: We only need to move the idle work kicking!
v4: Drop the BUG_ON(!awake) as we may be called from the shrinker in the
middle of constructing a request before we have marked the device awake.

Fixes: 28176ef4cfa5 ("drm/i915: Reserve space in the global seqno during request allocation")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem_request.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c
index b9b5253cf3cd..c227bca30357 100644
--- a/drivers/gpu/drm/i915/i915_gem_request.c
+++ b/drivers/gpu/drm/i915/i915_gem_request.c
@@ -218,7 +218,12 @@ static void i915_gem_request_retire(struct drm_i915_gem_request *request)
 	 */
 	list_del(&request->ring_link);
 	request->ring->last_retired_head = request->postfix;
-	request->i915->gt.active_requests--;
+	if (!--request->i915->gt.active_requests) {
+		GEM_BUG_ON(!request->i915->gt.awake);
+		mod_delayed_work(request->i915->wq,
+				 &request->i915->gt.idle_work,
+				 msecs_to_jiffies(100));
+	}
 
 	/* Walk through the active list, calling retire on each. This allows
 	 * objects to track their GPU activity and mark themselves as idle
@@ -763,6 +768,8 @@ static void i915_gem_mark_busy(const struct intel_engine_cs *engine)
 	if (dev_priv->gt.awake)
 		return;
 
+	GEM_BUG_ON(!dev_priv->gt.active_requests);
+
 	intel_runtime_pm_get_noresume(dev_priv);
 	dev_priv->gt.awake = true;
 
@@ -1146,13 +1153,6 @@ void i915_gem_retire_requests(struct drm_i915_private *dev_priv)
 	if (!dev_priv->gt.active_requests)
 		return;
 
-	GEM_BUG_ON(!dev_priv->gt.awake);
-
 	for_each_engine(engine, dev_priv, id)
 		engine_retire_requests(engine);
-
-	if (!dev_priv->gt.active_requests)
-		mod_delayed_work(dev_priv->wq,
-				 &dev_priv->gt.idle_work,
-				 msecs_to_jiffies(100));
 }
-- 
2.10.2

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

✗ Fi.CI.BAT: warning for drm/i915: Be more careful to drop the GT wakeref (rev5)

[Patchwork]

== Series Details ==

Series: drm/i915: Be more careful to drop the GT wakeref (rev5)
URL   : https://patchwork.freedesktop.org/series/15349/
State : warning

== Summary ==

Series 15349v5 drm/i915: Be more careful to drop the GT wakeref
https://patchwork.freedesktop.org/api/1.0/series/15349/revisions/5/mbox/

Test drv_module_reload_basic:
                pass       -> DMESG-WARN (fi-skl-6770hq)

fi-bdw-5557u     total:244  pass:229  dwarn:0   dfail:0   fail:0   skip:15 
fi-bsw-n3050     total:244  pass:204  dwarn:0   dfail:0   fail:0   skip:40 
fi-bxt-t5700     total:244  pass:216  dwarn:0   dfail:0   fail:0   skip:28 
fi-byt-j1900     total:244  pass:216  dwarn:0   dfail:0   fail:0   skip:28 
fi-byt-n2820     total:244  pass:212  dwarn:0   dfail:0   fail:0   skip:32 
fi-hsw-4770      total:244  pass:224  dwarn:0   dfail:0   fail:0   skip:20 
fi-hsw-4770r     total:244  pass:224  dwarn:0   dfail:0   fail:0   skip:20 
fi-ilk-650       total:244  pass:191  dwarn:0   dfail:0   fail:0   skip:53 
fi-ivb-3520m     total:244  pass:222  dwarn:0   dfail:0   fail:0   skip:22 
fi-ivb-3770      total:244  pass:222  dwarn:0   dfail:0   fail:0   skip:22 
fi-kbl-7200u     total:244  pass:222  dwarn:0   dfail:0   fail:0   skip:22 
fi-skl-6260u     total:244  pass:230  dwarn:0   dfail:0   fail:0   skip:14 
fi-skl-6700hq    total:244  pass:223  dwarn:0   dfail:0   fail:0   skip:21 
fi-skl-6700k     total:244  pass:222  dwarn:1   dfail:0   fail:0   skip:21 
fi-skl-6770hq    total:244  pass:229  dwarn:1   dfail:0   fail:0   skip:14 
fi-snb-2520m     total:244  pass:212  dwarn:0   dfail:0   fail:0   skip:32 
fi-snb-2600      total:244  pass:211  dwarn:0   dfail:0   fail:0   skip:33 

04145fe15cf8c81c221e62fc9d65d93053f9bd1a drm-intel-nightly: 2016y-11m-15d-14h-47m-07s UTC integration manifest
7c521f5 drm/i915: Be more careful to drop the GT wakeref

== Logs ==

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

Re: [PATCH v5] drm/i915: Be more careful to drop the GT wakeref

[Joonas Lahtinen]

On ti, 2016-11-15 at 16:46 +0000, Chris Wilson wrote:
> @@ -218,7 +218,12 @@ static void i915_gem_request_retire(struct drm_i915_gem_request *request)
>  	 */
>  	list_del(&request->ring_link);
>  	request->ring->last_retired_head = request->postfix;
> -	request->i915->gt.active_requests--;
> +	if (!--request->i915->gt.active_requests) {

BUG_ON(!request->i915->gt.active_requests) at the top?

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v5] drm/i915: Be more careful to drop the GT wakeref

[Chris Wilson]

On Fri, Nov 18, 2016 at 01:22:11PM +0200, Joonas Lahtinen wrote:
> On ti, 2016-11-15 at 16:46 +0000, Chris Wilson wrote:
> > @@ -218,7 +218,12 @@ static void i915_gem_request_retire(struct drm_i915_gem_request *request)
> >  	 */
> >  	list_del(&request->ring_link);
> >  	request->ring->last_retired_head = request->postfix;
> > -	request->i915->gt.active_requests--;
> > +	if (!--request->i915->gt.active_requests) {
> 
> BUG_ON(!request->i915->gt.active_requests) at the top?

Yup, that would be a nasty bug to catch.
-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 v5] drm/i915: Be more careful to drop the GT wakeref

[Chris Wilson]

On Fri, Nov 18, 2016 at 01:22:11PM +0200, Joonas Lahtinen wrote:
> On ti, 2016-11-15 at 16:46 +0000, Chris Wilson wrote:
> > @@ -218,7 +218,12 @@ static void i915_gem_request_retire(struct drm_i915_gem_request *request)
> >  	 */
> >  	list_del(&request->ring_link);
> >  	request->ring->last_retired_head = request->postfix;
> > -	request->i915->gt.active_requests--;
> > +	if (!--request->i915->gt.active_requests) {
> 
> BUG_ON(!request->i915->gt.active_requests) at the top?
> 
> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Thanks. Hmm, this may catch that one-off report of GEM_BUG_ON(gt.awake)
upon suspend. I don't think that made it to bugzilla.
-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 v5] drm/i915: Be more careful to drop the GT wakeref

[Imre Deak]

On pe, 2016-11-18 at 11:50 +0000, Chris Wilson wrote:
> On Fri, Nov 18, 2016 at 01:22:11PM +0200, Joonas Lahtinen wrote:
> > On ti, 2016-11-15 at 16:46 +0000, Chris Wilson wrote:
> > > @@ -218,7 +218,12 @@ static void i915_gem_request_retire(struct
> > > drm_i915_gem_request *request)
> > >  	 */
> > >  	list_del(&request->ring_link);
> > >  	request->ring->last_retired_head = request->postfix;
> > > -	request->i915->gt.active_requests--;
> > > +	if (!--request->i915->gt.active_requests) {
> > 
> > BUG_ON(!request->i915->gt.active_requests) at the top?
> > 
> > Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> 
> Thanks. Hmm, this may catch that one-off report of
> GEM_BUG_ON(gt.awake)
> upon suspend. I don't think that made it to bugzilla.

This one?:
https://bugs.freedesktop.org/show_bug.cgi?id=98670

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

Re: [PATCH v5] drm/i915: Be more careful to drop the GT wakeref

[Chris Wilson]

On Fri, Nov 18, 2016 at 02:25:20PM +0200, Imre Deak wrote:
> On pe, 2016-11-18 at 11:50 +0000, Chris Wilson wrote:
> > On Fri, Nov 18, 2016 at 01:22:11PM +0200, Joonas Lahtinen wrote:
> > > On ti, 2016-11-15 at 16:46 +0000, Chris Wilson wrote:
> > > > @@ -218,7 +218,12 @@ static void i915_gem_request_retire(struct
> > > > drm_i915_gem_request *request)
> > > >  	 */
> > > >  	list_del(&request->ring_link);
> > > >  	request->ring->last_retired_head = request->postfix;
> > > > -	request->i915->gt.active_requests--;
> > > > +	if (!--request->i915->gt.active_requests) {
> > > 
> > > BUG_ON(!request->i915->gt.active_requests) at the top?
> > > 
> > > Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > 
> > Thanks. Hmm, this may catch that one-off report of
> > GEM_BUG_ON(gt.awake)
> > upon suspend. I don't think that made it to bugzilla.
> 
> This one?:
> https://bugs.freedesktop.org/show_bug.cgi?id=98670

That one.
-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