diff mbox

[01/26] drm/i915: Split out verbose PPGTT dumping

Message ID 20140320115742.GA4463@nuc-i3427.alporthouse.com (mailing list archive)
State New, archived
Headers show

Commit Message

Chris Wilson March 20, 2014, 11:57 a.m. UTC 
On Mon, Mar 17, 2014 at 10:48:33PM -0700, Ben Widawsky wrote:
> There often is not enough memory to dump the full contents of the PPGTT.
> As a temporary bandage, to continue getting valuable basic PPGTT info,
> wrap the dangerous, memory hungry part inside of a new verbose version
> of the debugfs file.
> 
> Also while here we can split out the ppgtt print function so it's more
> reusable.
> 
> I'd really like to get ppgtt info into our error state, but I found it too
> difficult to make work in the limited time I have. Maybe Mika can find a way.

ppgtt_info is not printing out the user contexts - those are the
most interesting ones that should get dynamically allocated.

=0 ickle:/usr/src/linux$ git diff | cat

Comments

Chris Wilson March 20, 2014, 12:08 p.m. UTC | #1 
On Thu, Mar 20, 2014 at 11:57:42AM +0000, Chris Wilson wrote:
>  static void gen6_ppgtt_info(struct seq_file *m, struct drm_device *dev, bool verbose)
> @@ -1838,14 +1841,11 @@ static void gen6_ppgtt_info(struct seq_file *m, struct drm_device *dev, bool ver
>  
>  	list_for_each_entry_reverse(file, &dev->filelist, lhead) {
>  		struct drm_i915_file_private *file_priv = file->driver_priv;
> -		struct i915_hw_ppgtt *pvt_ppgtt;
>  
> -		pvt_ppgtt = ctx_to_ppgtt(file_priv->private_default_ctx);
>  		seq_printf(m, "proc: %s\n",
>  			   get_pid_task(file->pid, PIDTYPE_PID)->comm);

And 
	seq_printf(m, "\nproc: %s\n",
for good measure

> -		print_ppgtt(m, pvt_ppgtt, "Default context");
> -		if (verbose)
> -			idr_for_each(&file_priv->context_idr, per_file_ctx, m);
> +		idr_for_each(&file_priv->context_idr, per_file_ctx,
> +			     (void *)((unsigned long)m | verbose));
>  	}
>  }
>  
> 
> -- 
> Chris Wilson, Intel Open Source Technology Centre
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
Ben Widawsky March 22, 2014, 6:13 p.m. UTC | #2 
On Thu, Mar 20, 2014 at 12:08:00PM +0000, Chris Wilson wrote:
> On Thu, Mar 20, 2014 at 11:57:42AM +0000, Chris Wilson wrote:
> >  static void gen6_ppgtt_info(struct seq_file *m, struct drm_device *dev, bool verbose)
> > @@ -1838,14 +1841,11 @@ static void gen6_ppgtt_info(struct seq_file *m, struct drm_device *dev, bool ver
> >  
> >  	list_for_each_entry_reverse(file, &dev->filelist, lhead) {
> >  		struct drm_i915_file_private *file_priv = file->driver_priv;
> > -		struct i915_hw_ppgtt *pvt_ppgtt;
> >  
> > -		pvt_ppgtt = ctx_to_ppgtt(file_priv->private_default_ctx);
> >  		seq_printf(m, "proc: %s\n",
> >  			   get_pid_task(file->pid, PIDTYPE_PID)->comm);
> 
> And 
> 	seq_printf(m, "\nproc: %s\n",
> for good measure
> 
> > -		print_ppgtt(m, pvt_ppgtt, "Default context");
> > -		if (verbose)
> > -			idr_for_each(&file_priv->context_idr, per_file_ctx, m);
> > +		idr_for_each(&file_priv->context_idr, per_file_ctx,
> > +			     (void *)((unsigned long)m | verbose));
> >  	}
> >  }
> >  
> > 
> > -- 

Thanks, I like it. I'm assuming you didn't want the count_pt_pages stuck
in at this point (your diff was based on the end result)? I can do that
if you prefer it. It seems pointless to me though.
Chris Wilson March 22, 2014, 8:59 p.m. UTC | #3 
On Sat, Mar 22, 2014 at 11:13:17AM -0700, Ben Widawsky wrote:
> On Thu, Mar 20, 2014 at 12:08:00PM +0000, Chris Wilson wrote:
> > On Thu, Mar 20, 2014 at 11:57:42AM +0000, Chris Wilson wrote:
> > >  static void gen6_ppgtt_info(struct seq_file *m, struct drm_device *dev, bool verbose)
> > > @@ -1838,14 +1841,11 @@ static void gen6_ppgtt_info(struct seq_file *m, struct drm_device *dev, bool ver
> > >  
> > >  	list_for_each_entry_reverse(file, &dev->filelist, lhead) {
> > >  		struct drm_i915_file_private *file_priv = file->driver_priv;
> > > -		struct i915_hw_ppgtt *pvt_ppgtt;
> > >  
> > > -		pvt_ppgtt = ctx_to_ppgtt(file_priv->private_default_ctx);
> > >  		seq_printf(m, "proc: %s\n",
> > >  			   get_pid_task(file->pid, PIDTYPE_PID)->comm);
> > 
> > And 
> > 	seq_printf(m, "\nproc: %s\n",
> > for good measure
> > 
> > > -		print_ppgtt(m, pvt_ppgtt, "Default context");
> > > -		if (verbose)
> > > -			idr_for_each(&file_priv->context_idr, per_file_ctx, m);
> > > +		idr_for_each(&file_priv->context_idr, per_file_ctx,
> > > +			     (void *)((unsigned long)m | verbose));
> > >  	}
> > >  }
> > >  
> > > 
> > > -- 
> 
> Thanks, I like it. I'm assuming you didn't want the count_pt_pages stuck
> in at this point (your diff was based on the end result)? I can do that
> if you prefer it. It seems pointless to me though.

Maybe pointless to you, but it helped me to know when they were fully
populated - without having to look back at the constants.
-Chris
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 04d40fa..442a075 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1749,17 +1749,6 @@  static int i915_swizzle_info(struct seq_file *m, void *data)
 	return 0;
 }
 
-static int per_file_ctx(int id, void *ptr, void *data)
-{
-	struct i915_hw_context *ctx = ptr;
-	struct seq_file *m = data;
-	struct i915_hw_ppgtt *ppgtt = ctx_to_ppgtt(ctx);
-
-	ppgtt->debug_dump(ppgtt, m);
-
-	return 0;
-}
-
 static void gen8_ppgtt_info(struct seq_file *m, struct drm_device *dev, int verbose)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -1804,7 +1793,21 @@  static void print_ppgtt(struct seq_file *m, struct i915_hw_ppgtt *ppgtt, const c
 {
 	seq_printf(m, "%s:\n", name);
 	seq_printf(m, "pd gtt offset: 0x%08x\n", ppgtt->pd.pd_offset);
-	seq_printf(m, "\tpd pages: %zu\n", gen6_ppgtt_count_pt_pages(ppgtt));
+	seq_printf(m, "\tpd pages: %zu / %d\n", gen6_ppgtt_count_pt_pages(ppgtt), ppgtt->num_pd_entries);
+}
+
+static int per_file_ctx(int id, void *ptr, void *data)
+{
+	struct i915_hw_context *ctx = ptr;
+	struct seq_file *m = (void *)((unsigned long)data & ~1);
+	bool verbose = (unsigned long)data & 1;
+	struct i915_hw_ppgtt *ppgtt = ctx_to_ppgtt(ctx);
+
+	print_ppgtt(m, ppgtt, ctx->id == DEFAULT_CONTEXT_ID ? "Default context" : "User context");
+	if (verbose)
+		ppgtt->debug_dump(ppgtt, m);
+
+	return 0;
 }
 
 static void gen6_ppgtt_info(struct seq_file *m, struct drm_device *dev, bool verbose)
@@ -1838,14 +1841,11 @@  static void gen6_ppgtt_info(struct seq_file *m, struct drm_device *dev, bool ver
 
 	list_for_each_entry_reverse(file, &dev->filelist, lhead) {
 		struct drm_i915_file_private *file_priv = file->driver_priv;
-		struct i915_hw_ppgtt *pvt_ppgtt;
 
-		pvt_ppgtt = ctx_to_ppgtt(file_priv->private_default_ctx);
 		seq_printf(m, "proc: %s\n",
 			   get_pid_task(file->pid, PIDTYPE_PID)->comm);
-		print_ppgtt(m, pvt_ppgtt, "Default context");
-		if (verbose)
-			idr_for_each(&file_priv->context_idr, per_file_ctx, m);
+		idr_for_each(&file_priv->context_idr, per_file_ctx,
+			     (void *)((unsigned long)m | verbose));
 	}
 }