[Intel-gfx] [PATCH 29/37] drm/i915: Fill different pages of the GTT

[Joonas Lahtinen]

On ke, 2017-01-11 at 21:09 +0000, Chris Wilson wrote:
> Exercise filling different pages of the GTT
> 
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>

<SNIP>

> +static int walk_hole(struct drm_i915_private *i915,
> > +		     struct i915_address_space *vm,
> > +		     u64 hole_start, u64 hole_end)
> +{

<SNIP>

> +	for (addr = hole_start; addr < hole_end; addr += PAGE_SIZE)
{
> +		cond_resched();

Maybe this should be in the previous tests too that fill the GTT (one
would imagine it to take a lot longer than just walking with single
vma).

> +static int igt_ppgtt_walk(void *arg)
> +{

<SNIP>

> +
> +	i915_ppgtt_close(&ppgtt->base);
> +	i915_ppgtt_put(ppgtt);
> +err_unlock:

traditionally called out_unlock; Or being sole label, just out:

> +	mutex_unlock(&dev_priv->drm.struct_mutex);
> +
> +	fake_file_free(dev_priv, file);
> +	return err;
> +}
> +

<SNIP>

> +static int igt_ggtt_walk(void *arg)
> +{
> +	struct drm_i915_private *i915 = arg;
> +	struct i915_ggtt *ggtt = &i915->ggtt;
> +	u64 hole_start = U64_MAX, hole_end = 0, hole_size = 0;

I think I missed this in the ppgtt, but; single assignment per line as
per CodingStyle. This is not obfuscation contest.

> +	u64 this_start, this_end;
> +	struct drm_mm_node *node;
> +	int err;
> +
> +	GEM_BUG_ON(ggtt->base.total & ~PAGE_MASK);

I think single instance of this somewhere early should be enough?

> +
> +	mutex_lock(&i915->drm.struct_mutex);
> +	drm_mm_for_each_hole(node, &ggtt->base.mm, this_start, this_end) {
> +		u64 this_size;
> +
> +		if (ggtt->base.mm.color_adjust)
> +			ggtt->base. mm.color_adjust(node, 0,
> +						    &this_start, &this_end);
> +
> +		this_size = this_end - this_start;
> +		if (this_size > hole_size) {
> +			hole_size = this_size;
> +			hole_start = this_start;
> +			hole_end = this_end;
> +		}
> +	}
> +	pr_info("Found GGTT hole [%llx, %llx], size %llx\n",
> +		hole_start, hole_end, hole_size);
> +	GEM_BUG_ON(hole_start >= hole_end);
> +

Why not just walk all the holes big enough to accommodate GTT_PAGE_SIZE
for better coverage? But for just one hole, with above;

Reviewed-by: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation