[PATCH] drm/i915: Only unpin the default ctx object if it exists

[PATCH] drm/i915: Only unpin the default ctx object if it exists

[Chris Wilson]

Since commit 691e6415c891b8b2b082a120b896b443531c4d45
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Wed Apr 9 09:07:36 2014 +0100

    drm/i915: Always use kref tracking for all contexts.

we have contexts everywhere, and so we must be careful to distinguish
fake contexts, which do not have an associated bo, and real ones, which
do. In particular, we now need to be careful not to dereference NULL
pointers.

This is one such example, as the commit highlighted above failed to move
the unpinning of the default ctx object into the real-context-only
branch.

Reported-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=78792
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Ben Widawsky <benjamin.widawsky@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_context.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 55b3e52..4c7cd24 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -454,6 +454,8 @@ void i915_gem_context_fini(struct drm_device *dev)
 			i915_gem_context_unreference(dctx);
 			dev_priv->ring[RCS].last_context = NULL;
 		}
+
+		i915_gem_object_ggtt_unpin(dctx->obj);
 	}
 
 	for (i = 0; i < I915_NUM_RINGS; i++) {
@@ -466,7 +468,6 @@ void i915_gem_context_fini(struct drm_device *dev)
 		ring->last_context = NULL;
 	}
 
-	i915_gem_object_ggtt_unpin(dctx->obj);
 	i915_gem_context_unreference(dctx);
 }
 
-- 
1.7.9.5

Re: [PATCH] drm/i915: Only unpin the default ctx object if it exists

[Daniel Vetter]

On Fri, May 16, 2014 at 06:59:00PM +0100, Chris Wilson wrote:
> Since commit 691e6415c891b8b2b082a120b896b443531c4d45
> Author: Chris Wilson <chris@chris-wilson.co.uk>
> Date:   Wed Apr 9 09:07:36 2014 +0100
> 
>     drm/i915: Always use kref tracking for all contexts.
> 
> we have contexts everywhere, and so we must be careful to distinguish
> fake contexts, which do not have an associated bo, and real ones, which
> do. In particular, we now need to be careful not to dereference NULL
> pointers.
> 
> This is one such example, as the commit highlighted above failed to move
> the unpinning of the default ctx object into the real-context-only
> branch.
> 
> Reported-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=78792
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Ben Widawsky <benjamin.widawsky@intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>

I'll leave it to Jani to decide whether this is justified for -fixes, it
has my t-d & r-b. Queued for -next, thanks for the patch.
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_gem_context.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index 55b3e52..4c7cd24 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -454,6 +454,8 @@ void i915_gem_context_fini(struct drm_device *dev)
>  			i915_gem_context_unreference(dctx);
>  			dev_priv->ring[RCS].last_context = NULL;
>  		}
> +
> +		i915_gem_object_ggtt_unpin(dctx->obj);
>  	}
>  
>  	for (i = 0; i < I915_NUM_RINGS; i++) {
> @@ -466,7 +468,6 @@ void i915_gem_context_fini(struct drm_device *dev)
>  		ring->last_context = NULL;
>  	}
>  
> -	i915_gem_object_ggtt_unpin(dctx->obj);
>  	i915_gem_context_unreference(dctx);
>  }
>  
> -- 
> 1.7.9.5
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch