[Intel-gfx] [PATCH 33/48] drm/i915: Do aliasing PPGTT init with contexts

Ben Widawsky benjamin.widawsky at intel.com
Fri Dec 6 23:11:18 CET 2013


We have a default context which suits the aliasing PPGTT well. Tie them
together so it looks like any other context/PPGTT pair. This makes the
code cleaner as it won't have to special case aliasing as often.

The patch has one slightly tricky part in the default context creation
function. In the future (and on aliased setup) we create a new VM for a
context (potentially). However, if we have aliasing PPGTT, which occurs
at this point in time for all platforms GEN6+, we can simply manage the
refcounting to allow things to behave as normal. Now is a good time to
recall that the aliasing_ppgtt doesn't have a real VM, it uses the GGTT
drm_mm.

Signed-off-by: Ben Widawsky <ben at bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_dma.c         |   4 +-
 drivers/gpu/drm/i915/i915_drv.h         |   1 -
 drivers/gpu/drm/i915/i915_gem.c         |  13 +----
 drivers/gpu/drm/i915/i915_gem_context.c | 100 +++++++++++++++++++++++++++-----
 drivers/gpu/drm/i915/i915_gem_gtt.c     |  32 ----------
 5 files changed, 87 insertions(+), 63 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 5158a0c..22a3a67 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1364,7 +1364,7 @@ cleanup_gem:
 	i915_gem_cleanup_ringbuffer(dev);
 	i915_gem_context_fini(dev);
 	mutex_unlock(&dev->struct_mutex);
-	i915_gem_cleanup_aliasing_ppgtt(dev);
+	WARN_ON(dev_priv->mm.aliasing_ppgtt);
 	drm_mm_takedown(&dev_priv->gtt.base.mm);
 cleanup_power:
 	intel_display_power_put(dev, POWER_DOMAIN_VGA);
@@ -1755,8 +1755,8 @@ int i915_driver_unload(struct drm_device *dev)
 		i915_gem_free_all_phys_object(dev);
 		i915_gem_cleanup_ringbuffer(dev);
 		i915_gem_context_fini(dev);
+		WARN_ON(dev_priv->mm.aliasing_ppgtt);
 		mutex_unlock(&dev->struct_mutex);
-		i915_gem_cleanup_aliasing_ppgtt(dev);
 		i915_gem_cleanup_stolen(dev);
 
 		if (!I915_NEED_GFX_HWS(dev))
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index cf3c98c..65d2b36 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2257,7 +2257,6 @@ int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data,
 				   struct drm_file *file);
 
 /* i915_gem_gtt.c */
-void i915_gem_cleanup_aliasing_ppgtt(struct drm_device *dev);
 void i915_check_and_clear_faults(struct drm_device *dev);
 void i915_gem_suspend_gtt_mappings(struct drm_device *dev);
 void i915_gem_restore_gtt_mappings(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index d7d83b8..e7345a72 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4404,7 +4404,6 @@ int
 i915_gem_init_hw(struct drm_device *dev)
 {
 	drm_i915_private_t *dev_priv = dev->dev_private;
-	struct i915_hw_ppgtt *ppgtt;
 	int ret, i;
 
 	if (INTEL_INFO(dev)->gen < 6 && !intel_enable_gtt())
@@ -4445,16 +4444,6 @@ i915_gem_init_hw(struct drm_device *dev)
 		goto err_out;
 	}
 
-	if (dev_priv->mm.aliasing_ppgtt) {
-		ppgtt = dev_priv->mm.aliasing_ppgtt;
-		ret = ppgtt->enable(ppgtt);
-		if (ret) {
-			i915_gem_cleanup_aliasing_ppgtt(dev);
-			DRM_INFO("PPGTT enable failed. This is not fatal, but unexpected\n");
-			ret = 0;
-		}
-	}
-
 	return 0;
 
 err_out:
@@ -4485,8 +4474,8 @@ int i915_gem_init(struct drm_device *dev)
 	ret = i915_gem_init_hw(dev);
 	mutex_unlock(&dev->struct_mutex);
 	if (ret) {
+		WARN_ON(dev_priv->mm.aliasing_ppgtt);
 		i915_gem_context_fini(dev);
-		i915_gem_cleanup_aliasing_ppgtt(dev);
 		drm_mm_takedown(&dev_priv->gtt.base.mm);
 		return ret;
 	}
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 80064aa..28709f8 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -157,6 +157,25 @@ void i915_gem_context_free(struct kref *ctx_ref)
 	kfree(ctx);
 }
 
+static struct i915_hw_ppgtt *
+create_vm_for_ctx(struct drm_device *dev, struct i915_hw_context *ctx)
+{
+	struct i915_hw_ppgtt *ppgtt;
+	int ret;
+
+	ppgtt = kzalloc(sizeof(*ppgtt), GFP_KERNEL);
+	if (!ppgtt)
+		return ERR_PTR(-ENOMEM);
+
+	ret = i915_gem_init_ppgtt(dev, ppgtt);
+	if (ret) {
+		kfree(ppgtt);
+		return ERR_PTR(ret);
+	}
+
+	return ppgtt;
+}
+
 static struct i915_hw_context *
 create_hw_context(struct drm_device *dev,
 		  struct drm_i915_file_private *file_priv)
@@ -223,31 +242,70 @@ static inline bool is_default_context(struct i915_hw_context *ctx)
  * well as an idle case.
  */
 static struct i915_hw_context *
-create_default_context(struct drm_device *dev)
+create_default_context(struct drm_device *dev,
+		       struct drm_i915_file_private *file_priv,
+		       bool create_vm)
 {
+	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct i915_hw_context *ctx;
-	int ret;
+	int ret = 0;
 
 	BUG_ON(!mutex_is_locked(&dev->struct_mutex));
 
-	ctx = create_hw_context(dev, NULL);
+	/* Not yet supported */
+	BUG_ON(file_priv);
+
+	ctx = create_hw_context(dev, file_priv);
 	if (IS_ERR(ctx))
 		return ctx;
 
-	/* We may need to do things with the shrinker which require us to
-	 * immediately switch back to the default context. This can cause a
-	 * problem as pinning the default context also requires GTT space which
-	 * may not be available. To avoid this we always pin the
-	 * default context.
-	 */
-	ret = i915_gem_obj_ggtt_pin(ctx->obj, get_context_alignment(dev),
-				    false, false);
-	if (ret) {
-		DRM_DEBUG_DRIVER("Couldn't pin %d\n", ret);
-		goto err_destroy;
+	if (create_vm) {
+		struct i915_hw_ppgtt *ppgtt = create_vm_for_ctx(dev, ctx);
+
+		if (IS_ERR_OR_NULL(ppgtt)) {
+			DRM_ERROR("PPGTT setup failed (%ld)\n", PTR_ERR(ppgtt));
+			ret = PTR_ERR(ppgtt);
+			goto err_destroy;
+		} else
+			ctx->vm = &ppgtt->base;
+
+		/* This case is reserved for the global default context and
+		 * should only happen once. */
+		if (!file_priv) {
+			if (WARN_ON(dev_priv->mm.aliasing_ppgtt)) {
+				ret = -EEXIST;
+				goto err_destroy;
+			}
+
+			dev_priv->mm.aliasing_ppgtt = ppgtt;
+
+			/* We may need to do things with the shrinker which
+			 * require us to immediately switch back to the default
+			 * context. This can cause a problem as pinning the
+			 * default context also requires GTT space which may not
+			 * be available. To avoid this we always pin the default
+			 * context.
+			 */
+			ret = i915_gem_obj_ggtt_pin(ctx->obj,
+						    get_context_alignment(dev),
+						    false, false);
+			if (ret) {
+				DRM_DEBUG_DRIVER("Couldn't pin %d\n", ret);
+				goto err_destroy;
+			}
+		}
+	} else if (USES_ALIASING_PPGTT(dev)) {
+		/* For platforms which only have aliasing PPGTT, we fake the
+		 * address space and refcounting. */
+		kref_get(&dev_priv->mm.aliasing_ppgtt->ref);
 	}
 
-	DRM_DEBUG_DRIVER("Default HW context loaded\n");
+	/* TODO: Until full ppgtt... */
+	if (USES_ALIASING_PPGTT(dev))
+		ctx->vm = &dev_priv->mm.aliasing_ppgtt->base;
+	else
+		ctx->vm = &dev_priv->gtt.base;
+
 	return ctx;
 
 err_destroy:
@@ -319,8 +377,9 @@ int i915_gem_context_init(struct drm_device *dev)
 		return -E2BIG;
 	}
 
+	dev_priv->ring[RCS].default_context =
+		create_default_context(dev, NULL, USES_ALIASING_PPGTT(dev));
 
-	dev_priv->ring[RCS].default_context = create_default_context(dev);
 	if (IS_ERR_OR_NULL(dev_priv->ring[RCS].default_context)) {
 		DRM_DEBUG_DRIVER("Disabling HW Contexts; create failed %ld\n",
 				 PTR_ERR(dev_priv->ring[RCS].default_context));
@@ -384,6 +443,7 @@ void i915_gem_context_fini(struct drm_device *dev)
 
 	i915_gem_object_ggtt_unpin(dctx->obj);
 	i915_gem_context_unreference(dctx);
+	dev_priv->mm.aliasing_ppgtt = NULL;
 }
 
 int i915_gem_context_enable(struct drm_i915_private *dev_priv)
@@ -394,11 +454,19 @@ int i915_gem_context_enable(struct drm_i915_private *dev_priv)
 	if (!HAS_HW_CONTEXTS(dev_priv->dev))
 		return 0;
 
+	/* This is the only place the aliasing PPGTT gets enabled, which means
+	 * it has to happen before we bail on reset */
+	if (dev_priv->mm.aliasing_ppgtt) {
+		struct i915_hw_ppgtt *ppgtt = dev_priv->mm.aliasing_ppgtt;
+		ppgtt->enable(ppgtt);
+	}
+
 	/* FIXME: We should make this work, even in reset */
 	if (i915_reset_in_progress(&dev_priv->gpu_error))
 		return 0;
 
 	BUG_ON(!dev_priv->ring[RCS].default_context);
+
 	for_each_ring(ring, dev_priv, i) {
 		ret = do_switch(ring, ring->default_context);
 		if (ret)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index ea626ba..88e49b1 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -911,19 +911,6 @@ int i915_gem_init_ppgtt(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt)
 	return ret;
 }
 
-void i915_gem_cleanup_aliasing_ppgtt(struct drm_device *dev)
-{
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct i915_hw_ppgtt *ppgtt = dev_priv->mm.aliasing_ppgtt;
-
-	if (!ppgtt)
-		return;
-
-	kref_put(&dev_priv->mm.aliasing_ppgtt->ref, ppgtt_release);
-
-	dev_priv->mm.aliasing_ppgtt = NULL;
-}
-
 static void __always_unused
 ppgtt_bind_vma(struct i915_vma *vma,
 	       enum i915_cache_level cache_level,
@@ -1424,25 +1411,6 @@ void i915_gem_init_global_gtt(struct drm_device *dev)
 	mappable_size = dev_priv->gtt.mappable_end;
 
 	i915_gem_setup_global_gtt(dev, 0, mappable_size, gtt_size);
-	if (USES_ALIASING_PPGTT(dev)) {
-		struct i915_hw_ppgtt *ppgtt;
-		int ret;
-
-		ppgtt = kzalloc(sizeof(*ppgtt), GFP_KERNEL);
-		if (!ppgtt) {
-			DRM_ERROR("Aliased PPGTT setup failed -ENOMEM\n");
-			return;
-		}
-
-		ret = i915_gem_init_ppgtt(dev, ppgtt);
-		if (!ret) {
-			dev_priv->mm.aliasing_ppgtt = ppgtt;
-			return;
-		}
-
-		kfree(ppgtt);
-		DRM_ERROR("Aliased PPGTT setup failed %d\n", ret);
-	}
 }
 
 static int setup_scratch_page(struct drm_device *dev)
-- 
1.8.4.2




More information about the Intel-gfx mailing list