[Intel-gfx] [PATCH v2 33/38] drm/i915: Test creation of partial VMA

[Joonas Lahtinen]

On to, 2017-01-19 at 11:41 +0000, Chris Wilson wrote:
> Mock testing to ensure we can create and lookup partial VMA.
> 
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>

<SNIP>

> +static bool assert_partial(struct drm_i915_gem_object *obj,
> +			   struct i915_vma *vma,
> +			   unsigned long offset,
> +			   unsigned long size)
> +{
> +	struct sgt_iter sgt;

Confusing name, could rather be "sgti" or just "i", or "iter".

> +static int igt_vma_partial(void *arg)
> +{
> +	struct drm_i915_private *i915 = arg;
> +	const unsigned int npages = 1021; /* prime! */

#define THE_MAGIC_PRIME 1021

> +	for (loop = 0; loop <= 1; loop++) { /* exercise both create/lookup */

I'd like the phase array/variable more. "loop" variable is kinda
confusing easily.

> +		unsigned int count, nvma;
> +

Make a comment here that a whole VMA is also created at the end and it
needs to be accounted. This is why the phase array might be more
readable.

> +		nvma = loop;
> +		for_each_prime_number_from(sz, 1, npages) {
> +			for_each_prime_number_from(offset, 0, npages - sz) {
> +				struct i915_address_space *vm =
> +					&i915->ggtt.base;

Could be out of the loop, too.

<SNIP>

> +
> +		/* Create a mapping for the entire object, just for extra fun */
> +		vma = i915_vma_instance(obj, &i915->ggtt.base, NULL);

No helper for this block?

With the variable renamed;

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

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