[Intel-gfx] [PATCH 2/2] drm/i915: drm/i915: Fix recursive calls to unmap

2012-01-24 Thread Ben Widawsky
After the ILK vt-d workaround patches it became clear that we had
introduced a bug.  Chris Wilson tracked down the issue to recursive
calls to unmap. This happens because we try to optimize waiting on
requests by calling retire requests after the wait, which may drop the
last reference on an object and end up freeing the object (and then
unmap the object from the gtt).

After the last patch we can now choose to defer processing the retire
list.

Kudos to Chris Wilson for tracking this one down.

This fixes tests/gem_linear_blits in intel-gpu-tools.

v2: v1 used a global, v2 directly modified the call chain.

v3: As Keith Packard pointed out, v2 changed retirement behavior. While
it was intentional, it wasn't related to the bugfix, and so has been
removed.  To do this, changed i915_wait_request to be a macro to assure
old behavior is kept.

v4: Rebased and renamed strictly_idle to defer_retirement.

V5: Don't use a macro port new code, just change all the callers.
Also had a badly named argument in the function definition.

This patch fixes gem_unref_active_buffers from i-g-t in the non-VTd
case (ie. do_idle_maps forced to true).

Reported-by: guang.a.y...@intel.com
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=42180
Signed-off-by: Ben Widawsky 
---
 drivers/gpu/drm/i915/i915_gem_gtt.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 11bddd5..e050b90 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -55,7 +55,7 @@ static bool do_idling(struct drm_i915_private *dev_priv)
 
if (unlikely(dev_priv->mm.gtt->do_idle_maps)) {
dev_priv->mm.interruptible = false;
-   if (i915_gpu_idle(dev_priv->dev, false)) {
+   if (i915_gpu_idle(dev_priv->dev, true)) {
DRM_ERROR("Couldn't idle GPU\n");
/* Wait a bit, in hopes it avoids the hang */
udelay(10);
-- 
1.7.8.4

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH 2/2] drm/i915: drm/i915: Fix recursive calls to unmap

2012-01-24 Thread Ben Widawsky
After the ILK vt-d workaround patches it became clear that we had
introduced a bug.  Chris Wilson tracked down the issue to recursive
calls to unmap. This happens because we try to optimize waiting on
requests by calling retire requests after the wait, which may drop the
last reference on an object and end up freeing the object (and then
unmap the object from the gtt).

After the last patch we can now choose to defer processing the retire
list.

Kudos to Chris Wilson for tracking this one down.

This patch fixes gem_unref_active_buffers from i-g-t. It was tested by
forcing do_idle_maps to true.

This also fixes tests/gem_linear_blits in intel-gpu-tools.

Reported-by: guang.a.y...@intel.com
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=42180
Reviewed-by: Keith Packard 
Signed-off-by: Ben Widawsky 
---
 drivers/gpu/drm/i915/i915_gem_gtt.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index e050b90..11bddd5 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -55,7 +55,7 @@ static bool do_idling(struct drm_i915_private *dev_priv)
 
if (unlikely(dev_priv->mm.gtt->do_idle_maps)) {
dev_priv->mm.interruptible = false;
-   if (i915_gpu_idle(dev_priv->dev, true)) {
+   if (i915_gpu_idle(dev_priv->dev, false)) {
DRM_ERROR("Couldn't idle GPU\n");
/* Wait a bit, in hopes it avoids the hang */
udelay(10);
-- 
1.7.8.4

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 2/2] drm/i915: drm/i915: Fix recursive calls to unmap

2012-01-24 Thread Keith Packard
On Tue, 24 Jan 2012 14:42:03 -0800, Ben Widawsky  wrote:

> This patch fixes gem_unref_active_buffers from i-g-t in the non-VTd
> case (ie. do_idle_maps forced to true).

Nice, one-line fix.

(if you agree with my  in the previous patch, this will have
to be changed to match)

Reviewed-by: Keith Packard 

-- 
keith.pack...@intel.com


pgptOnCwrurzz.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 2/2] drm/i915: drm/i915: Fix recursive calls to unmap

2012-01-24 Thread Ben Widawsky

On 01/24/12 15:00, Keith Packard wrote:

On Tue, 24 Jan 2012 14:42:03 -0800, Ben Widawsky  wrote:


This patch fixes gem_unref_active_buffers from i-g-t in the non-VTd
case (ie. do_idle_maps forced to true).


Nice, one-line fix.

(if you agree with my  in the previous patch, this will have
to be changed to match)

Reviewed-by: Keith Packard



I agree with the bikeshed, however I am not sure what you expect changed 
here. do_idle_maps is still forced to true to test the fix. So I'm going 
to assume you just read the comment too quickly and resubmit with the 
updated patches.

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 2/2] drm/i915: drm/i915: Fix recursive calls to unmap

2012-01-25 Thread Keith Packard
On Tue, 24 Jan 2012 20:02:55 -0800, Ben Widawsky  wrote:

> I agree with the bikeshed, however I am not sure what you expect changed 
> here. do_idle_maps is still forced to true to test the fix. So I'm going 
> to assume you just read the comment too quickly and resubmit with the 
> updated patches.

Just that the boolean sense of the new parameter is reversed, nothing else.

-- 
keith.pack...@intel.com


pgp2RP170IOcW.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx