[Intel-gfx] [PATCH] Implement batch and ring buffer dumping

[Eric Anholt]

On Tue, 2009-02-03 at 17:06 -0500, Ben Gamari wrote:


> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 6a9e3a8..b143a48 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2585,12 +2581,17 @@ i915_gem_execbuffer(struct drm_device *dev, void *data,
>  	exec_offset = exec_list[args->buffer_count - 1].offset;
>  
>  #if WATCH_EXEC
> -	i915_gem_dump_object(object_list[args->buffer_count - 1],
> +	i915_gem_dump_object(batch_obj,
>  			      args->batch_len,
>  			      __func__,
>  			      ~0);
>  #endif
>  
> +	/* Record batchbuffer in recent batches list */
> +	dev_priv->mm.recent_batch_list[dev_priv->mm.recent_batch_head] = batch_obj;
> +	dev_priv->mm.recent_batch_head = (dev_priv->mm.recent_batch_head+1) %
> +		I915_GEM_RECENT_BATCH_LEN;
> +
>  	/* Exec the batchbuffer */
>  	ret = i915_dispatch_gem_execbuffer(dev, args, exec_offset);
>  	if (ret) {

Needs refcounting on these batchbuffers, or you'll have trivial oopses
from userland.

> diff --git a/drivers/gpu/drm/i915/i915_gem_debugfs.c b/drivers/gpu/drm/i915/i915_gem_debugfs.c
> index 7ad49d7..c5379f0 100644
> --- a/drivers/gpu/drm/i915/i915_gem_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_gem_debugfs.c
> @@ -176,6 +176,93 @@ static int i915_hws_info(struct seq_file *m, void *data)
>  	return 0;
>  }
>  
> +static void i915_dump_pages(struct seq_file *m, struct page **page_list, int page_count)
> +{
> +	int page, i;
> +	int offset = 0;
> +	unsigned char *page_vma;
> +	int *ptr;
> +
> +	for (page = 0; page < page_count; page++) {
> +		page_vma = kmap(page_list[page]);
> +		for (i = 0; i < PAGE_SIZE; i++) {
> +			ptr = (int *) (page_vma + i);
> +			seq_printf(m, "%08x :  %08x\n", offset++, *ptr);
> +		}
> +		kunmap(page_list[page]);
> +	}
> +}
> +
> +static int i915_batch_buffer_info(struct seq_file *m, void *data)
> +{
> +	struct drm_info_node *node = (struct drm_info_node *) m->private;
> +	struct drm_device *dev = node->minor->dev;
> +	drm_i915_private_t *dev_priv = dev->dev_private;
> +	struct drm_gem_object *obj;
> +	struct drm_i915_gem_object *obj_priv;
> +	uintptr_t n = (uintptr_t) node->info_ent->data;
> +	int free_page_list = 0;
> +	int ret;

This function needs locking of the struct mutex to prevent trivial races
for userland.

> +	obj = dev_priv->mm.recent_batch_list[n];
> +	if (obj == NULL)
> +		return 0;
> +
> +	obj_priv = obj->driver_private;
> +	if (obj_priv->page_list == NULL) {
> +		free_page_list = 1;
> +		ret = i915_gem_object_get_page_list(obj);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	seq_printf(m, "GttOffset=%08x\n", obj_priv->gtt_offset);

"gtt_offset = 0x%08x\n" please :)

> +	i915_dump_pages(m, obj_priv->page_list, obj->size / PAGE_SIZE);
> +
> +	if (free_page_list)
> +		i915_gem_object_free_page_list(obj);
> +	return 0;
> +}
> +
> +static int i915_ringbuf_info(struct seq_file *m, void *data)
> +{
> +	struct drm_info_node *node = (struct drm_info_node *) m->private;
> +	struct drm_device *dev = node->minor->dev;
> +	drm_i915_private_t *dev_priv = dev->dev_private;
> +	u8 *virt;
> +	uint32_t *ptr, off;
> +
> +	virt = dev_priv->ring.virtual_start;
> +
> +	for (off = 0; off < dev_priv->ring.Size; off += 4) {
> +		ptr = (uint32_t *)(virt + off);
> +		seq_printf(m, "%08x :  %08x\n", off, *ptr);
> +	}

Check that the ringbuf is actually set up, to prevent an oops from
userland when checking this outside of X on non-KMS.  I think.

> +	return 0;
> +}
> +
> +static int i915_execinfo_info(struct seq_file *m, void *data)
> +{
> +	struct drm_info_node *node = (struct drm_info_node *) m->private;
> +	struct drm_device *dev = node->minor->dev;
> +	drm_i915_private_t *dev_priv = dev->dev_private;
> +	unsigned int head, tail, mask;
> +
> +	head = dev_priv->ring.head & HEAD_ADDR;
> +	tail = dev_priv->ring.tail & TAIL_ADDR;
> +	mask = dev_priv->ring.tail_mask;
> +
> +	seq_printf(m, "RingHead :  %08x\n", head);
> +	seq_printf(m, "RingTail :  %08x\n", tail);
> +	seq_printf(m, "RingMask :  %08x\n", mask);
> +	seq_printf(m, "RingSize :  %08lx\n", dev_priv->ring.Size);
> +	seq_printf(m, "Acthd :  %08x\n", I915_READ(ACTHD_I965));
> +
> +	return 0;
> +}

I think these two nodes would be better named i915_ringbuffer_data and
i915_ringbuffer_info.  Also, I think that reading the ring head reg
would be more valuable than supplying the cached value.

-- 
Eric Anholt
eric at anholt.net                         eric.anholt at intel.com


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 197 bytes
Desc: This is a digitally signed message part
URL: <http://lists.freedesktop.org/archives/intel-gfx/attachments/20090203/5aa92163/attachment.sig>