diff mbox

[v2] drm/i915: Handle i915_ppgtt_put correctly

Message ID 1408459781-18500-1-git-send-email-michel.thierry@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Michel Thierry Aug. 19, 2014, 2:49 p.m. UTC
Unfortunately, the gem_obj/vma relationship is not symmetrical; a gem_obj
can look up for the same vma more than once (where the ppgtt refcount is
incremented), but will free the vma only once (i915_gem_free_object).

This difference in refcount get/put means that the ppgtt is not removed
after the context and vma are destroyed, because sometimes the refcount
will never go back to zero.

v2: Just move the ppgtt refcount into vma_create.

OTC-Jira: VIZ-3719
Signed-off-by: Michel Thierry <michel.thierry@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

Comments

Daniel Vetter Aug. 26, 2014, 12:39 p.m. UTC | #1
On Tue, Aug 19, 2014 at 03:49:41PM +0100, Michel Thierry wrote:
> Unfortunately, the gem_obj/vma relationship is not symmetrical; a gem_obj
> can look up for the same vma more than once (where the ppgtt refcount is
> incremented), but will free the vma only once (i915_gem_free_object).
> 
> This difference in refcount get/put means that the ppgtt is not removed
> after the context and vma are destroyed, because sometimes the refcount
> will never go back to zero.
> 
> v2: Just move the ppgtt refcount into vma_create.
> 
> OTC-Jira: VIZ-3719
> Signed-off-by: Michel Thierry <michel.thierry@intel.com>

Queued for -next, thanks for the patch.
-Daniel
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 840365c..95f6df4 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -2251,8 +2251,10 @@  static struct i915_vma *__i915_gem_vma_create(struct drm_i915_gem_object *obj,
 	/* Keep GGTT vmas first to make debug easier */
 	if (i915_is_ggtt(vm))
 		list_add(&vma->vma_link, &obj->vma_list);
-	else
+	else {
 		list_add_tail(&vma->vma_link, &obj->vma_list);
+		i915_ppgtt_get(i915_vm_to_ppgtt(vm));
+	}
 
 	return vma;
 }
@@ -2267,8 +2269,5 @@  i915_gem_obj_lookup_or_create_vma(struct drm_i915_gem_object *obj,
 	if (!vma)
 		vma = __i915_gem_vma_create(obj, vm);
 
-	if (!i915_is_ggtt(vm))
-		i915_ppgtt_get(i915_vm_to_ppgtt(vm));
-
 	return vma;
 }