diff mbox

drm/i915: Only track real ppgtt for a context

Message ID 1407162047-585-1-git-send-email-daniel.vetter@ffwll.ch (mailing list archive)
State New, archived
Headers show

Commit Message

Daniel Vetter Aug. 4, 2014, 2:20 p.m. UTC
There's a bit a confusion since we track the global gtt,
the aliasing and real ppgtt in the ctx->vm pointer. And not
all callers really bother to check for the different cases and just
presume that it points to a real ppgtt.

Now looking closely we don't actually need ->vm to always point at an
address space - the only place that cares actually has fixup code
already to decide whether to look at the per-proces or the global
address space.

So switch to just tracking the ppgtt directly and ditch all the
extraneous code.

v2: Fixup the ppgtt debugfs file to not oops on a NULL ctx->ppgtt.
Also drop the early exit - without aliasing ppgtt we want to dump all
the ppgtts of the contexts if we have full ppgtt.

Cc: "Thierry, Michel" <michel.thierry@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
OTC-Jira: VIZ-3724
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_debugfs.c        | 11 ++++++++---
 drivers/gpu/drm/i915/i915_drv.h            |  3 +--
 drivers/gpu/drm/i915/i915_gem_context.c    | 28 +++++++---------------------
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |  5 +++--
 drivers/gpu/drm/i915/i915_gpu_error.c      | 10 +++++++---
 5 files changed, 26 insertions(+), 31 deletions(-)

Comments

Michel Thierry Aug. 4, 2014, 5:17 p.m. UTC | #1
> -----Original Message-----
> From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch]
> Sent: Monday, August 04, 2014 3:21 PM
> To: Intel Graphics Development
> Cc: Daniel Vetter; Thierry, Michel; Ville Syrjälä
> Subject: [PATCH] drm/i915: Only track real ppgtt for a context
> 
> There's a bit a confusion since we track the global gtt,
> the aliasing and real ppgtt in the ctx->vm pointer. And not
> all callers really bother to check for the different cases and just
> presume that it points to a real ppgtt.
> 
> Now looking closely we don't actually need ->vm to always point at an
> address space - the only place that cares actually has fixup code
> already to decide whether to look at the per-proces or the global
> address space.
> 
> So switch to just tracking the ppgtt directly and ditch all the
> extraneous code.
> 
> v2: Fixup the ppgtt debugfs file to not oops on a NULL ctx->ppgtt.
> Also drop the early exit - without aliasing ppgtt we want to dump all
> the ppgtts of the contexts if we have full ppgtt.
> 
> Cc: "Thierry, Michel" <michel.thierry@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> OTC-Jira: VIZ-3724
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c        | 11 ++++++++---
>  drivers/gpu/drm/i915/i915_drv.h            |  3 +--
>  drivers/gpu/drm/i915/i915_gem_context.c    | 28 +++++++---------------------
>  drivers/gpu/drm/i915/i915_gem_execbuffer.c |  5 +++--
>  drivers/gpu/drm/i915/i915_gpu_error.c      | 10 +++++++---
>  5 files changed, 26 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index 3bf1d20c598b..2bd4beada1bd 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1774,7 +1774,13 @@ static int per_file_ctx(int id, void *ptr, void *data)
>  {
>  	struct intel_context *ctx = ptr;
>  	struct seq_file *m = data;
> -	struct i915_hw_ppgtt *ppgtt = ctx_to_ppgtt(ctx);
> +	struct i915_hw_ppgtt *ppgtt = ctx->ppgtt;
> +
> +	if (!ppgtt) {
> +		seq_puts(m, "  no ppgtt for context %d\n",
> +			 ctx->user_handle);
Should be seq_printf().

> +		return 0;
> +	}
> 
>  	if (i915_gem_context_is_default(ctx))
>  		seq_puts(m, "  default context:\n");
> @@ -1834,8 +1840,7 @@ static void gen6_ppgtt_info(struct seq_file *m,
> --
> 1.9.3

Apart from that, 

Reviewed-by: Michel Thierry <michel.thierry@intel.com>
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 3bf1d20c598b..2bd4beada1bd 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1774,7 +1774,13 @@  static int per_file_ctx(int id, void *ptr, void *data)
 {
 	struct intel_context *ctx = ptr;
 	struct seq_file *m = data;
-	struct i915_hw_ppgtt *ppgtt = ctx_to_ppgtt(ctx);
+	struct i915_hw_ppgtt *ppgtt = ctx->ppgtt;
+
+	if (!ppgtt) {
+		seq_puts(m, "  no ppgtt for context %d\n",
+			 ctx->user_handle);
+		return 0;
+	}
 
 	if (i915_gem_context_is_default(ctx))
 		seq_puts(m, "  default context:\n");
@@ -1834,8 +1840,7 @@  static void gen6_ppgtt_info(struct seq_file *m, struct drm_device *dev)
 		seq_printf(m, "pd gtt offset: 0x%08x\n", ppgtt->pd_offset);
 
 		ppgtt->debug_dump(ppgtt, m);
-	} else
-		return;
+	}
 
 	list_for_each_entry_reverse(file, &dev->filelist, lhead) {
 		struct drm_i915_file_private *file_priv = file->driver_priv;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 0762e19f9bc6..3230b08aff13 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -616,7 +616,7 @@  struct intel_context {
 	uint8_t remap_slice;
 	struct drm_i915_file_private *file_priv;
 	struct i915_ctx_hang_stats hang_stats;
-	struct i915_address_space *vm;
+	struct i915_hw_ppgtt *ppgtt;
 
 	struct {
 		struct drm_i915_gem_object *rcs_state;
@@ -2504,7 +2504,6 @@  i915_gem_object_ggtt_unbind(struct drm_i915_gem_object *obj)
 void i915_gem_object_ggtt_unpin(struct drm_i915_gem_object *obj);
 
 /* i915_gem_context.c */
-#define ctx_to_ppgtt(ctx) container_of((ctx)->vm, struct i915_hw_ppgtt, base)
 int __must_check i915_gem_context_init(struct drm_device *dev);
 void i915_gem_context_fini(struct drm_device *dev);
 void i915_gem_context_reset(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 7a455fcee3a7..c00e5d027774 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -136,15 +136,8 @@  void i915_gem_context_free(struct kref *ctx_ref)
 {
 	struct intel_context *ctx = container_of(ctx_ref,
 						   typeof(*ctx), ref);
-	struct i915_hw_ppgtt *ppgtt = NULL;
 
-	if (ctx->legacy_hw_ctx.rcs_state) {
-		/* We refcount even the aliasing PPGTT to keep the code symmetric */
-		if (USES_PPGTT(ctx->legacy_hw_ctx.rcs_state->base.dev))
-			ppgtt = ctx_to_ppgtt(ctx);
-	}
-
-	i915_ppgtt_put(ppgtt);
+	i915_ppgtt_put(ctx->ppgtt);
 	if (ctx->legacy_hw_ctx.rcs_state)
 		drm_gem_object_unreference(&ctx->legacy_hw_ctx.rcs_state->base);
 	list_del(&ctx->link);
@@ -240,7 +233,6 @@  i915_gem_create_context(struct drm_device *dev,
 			bool create_vm)
 {
 	const bool is_global_default_ctx = file_priv == NULL;
-	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_context *ctx;
 	int ret = 0;
 
@@ -274,15 +266,10 @@  i915_gem_create_context(struct drm_device *dev,
 					 PTR_ERR(ppgtt));
 			ret = PTR_ERR(ppgtt);
 			goto err_unpin;
-		} else
-			ctx->vm = &ppgtt->base;
-	} else if (USES_PPGTT(dev)) {
-		/* For platforms which only have aliasing PPGTT, we fake the
-		 * address space and refcounting. */
-		ctx->vm = &dev_priv->mm.aliasing_ppgtt->base;
-		i915_ppgtt_get(dev_priv->mm.aliasing_ppgtt);
-	} else
-		ctx->vm = &dev_priv->gtt.base;
+		}
+
+		ctx->ppgtt = ppgtt;
+	}
 
 	return ctx;
 
@@ -538,7 +525,6 @@  static int do_switch(struct intel_engine_cs *ring,
 {
 	struct drm_i915_private *dev_priv = ring->dev->dev_private;
 	struct intel_context *from = ring->last_context;
-	struct i915_hw_ppgtt *ppgtt = ctx_to_ppgtt(to);
 	u32 hw_flags = 0;
 	bool uninitialized = false;
 	int ret, i;
@@ -566,8 +552,8 @@  static int do_switch(struct intel_engine_cs *ring,
 	 */
 	from = ring->last_context;
 
-	if (USES_FULL_PPGTT(ring->dev)) {
-		ret = ppgtt->switch_mm(ppgtt, ring, false);
+	if (to->ppgtt) {
+		ret = to->ppgtt->switch_mm(to->ppgtt, ring, false);
 		if (ret)
 			goto unpin_out;
 	}
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 60998fc4e5b2..54b3d8c8b228 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1318,8 +1318,9 @@  i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 
 	i915_gem_context_reference(ctx);
 
-	vm = ctx->vm;
-	if (!USES_FULL_PPGTT(dev))
+	if (ctx->ppgtt)
+		vm = &ctx->ppgtt->base;
+	else
 		vm = &dev_priv->gtt.base;
 
 	eb = eb_create(args);
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index 0b3f69439451..06072120b15d 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -958,6 +958,12 @@  static void i915_gem_record_rings(struct drm_device *dev,
 
 		request = i915_gem_find_active_request(ring);
 		if (request) {
+			struct i915_address_space *vm;
+
+			vm = request->ctx && request->ctx->ppgtt ?
+				&request->ctx->ppgtt->base :
+				&dev_priv->gtt.base;
+
 			/* We need to copy these to an anonymous buffer
 			 * as the simplest method to avoid being overwritten
 			 * by userspace.
@@ -965,9 +971,7 @@  static void i915_gem_record_rings(struct drm_device *dev,
 			error->ring[i].batchbuffer =
 				i915_error_object_create(dev_priv,
 							 request->batch_obj,
-							 request->ctx ?
-							 request->ctx->vm :
-							 &dev_priv->gtt.base);
+							 vm);
 
 			if (HAS_BROKEN_CS_TLB(dev_priv->dev) &&
 			    ring->scratch.obj)