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 <[email protected]>

<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 <[email protected]>

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to