Re: [PATCH 8/9] drm: kselftest for drm_mm and top-down alignment

[Joonas Lahtinen <joonas.lahtinen@linux.intel.com>]

On pe, 2016-12-09 at 13:08 +0000, Chris Wilson wrote:
> Check that if we request top-down allocation with a particular alignment
> from drm_mm_insert_node() that the start of the node matches our
> request.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

<SNIP>

> @@ -1038,6 +1038,98 @@ static int igt_topdown(void *ignored)
>  	return ret;
>  }
>  
> +static int igt_topdown_align(void *ignored)
> +{
> +	struct drm_mm mm;
> +	struct drm_mm_node tmp, resv;
> +	int ret = -EINVAL;
> +	int n, m, err;
> +
> +	drm_mm_init(&mm, 0, ~0ull);

U64_MAX

> +	memset(&tmp, 0, sizeof(tmp));
> +	memset(&resv, 0, sizeof(resv));
> +
> +	for (m = 0; m < 32; m++) {
> +		u64 end = ~0ull;
> +
> +		if (m) {
> +			resv.size = BIT_ULL(m);
> +			end -= resv.size;
> +			resv.start = end;
> +
> +			err = drm_mm_reserve_node(&mm, &resv);
> +			if (err) {
> +				pr_err("reservation of sentinel node failed\n");
> +				ret = err;
> +				goto out;
> +			}
> +		}
> +
> +		for (n = 0; n < 63 - m; n++) {
> +			u64 align = BIT_ULL(n);
> +
> +			err = drm_mm_insert_node_generic(&mm, &tmp, 1, align, 0,
> +							 DRM_MM_SEARCH_BELOW,
> +							 DRM_MM_CREATE_TOP);
> +			drm_mm_remove_node(&tmp);
> +			if (err) {
> +				pr_err("insert failed, ret=%d\n", err);
> +				ret = err;
> +				goto out;
> +			}

Just drm_mm_remove_node(&tmp) here to avoid an unnecessary extra splat.
I think the tests should be valid code.

> +
> +			if (tmp.start & (align - 1)) {
> +				pr_err("insert alignment failed, aligment=%llx, start=%llx\n",
> +				       align, tmp.start);
> +				goto out;
> +			}
> +
> +			if (tmp.start < end - align) {
> +				pr_err("topdown insert failed, start=%llx, align=%llx, end=%llx\n",
> +				       tmp.start, align, end);
> +				goto out;
> +			}
> +		}
> +
> +		for_each_prime(n, min(8192ull, end - 1)) {
> +			u64 rem;
> +
> +			err = drm_mm_insert_node_generic(&mm, &tmp, n, 0, 0,
> +							 DRM_MM_SEARCH_BELOW,
> +							 DRM_MM_CREATE_TOP);
> +			drm_mm_remove_node(&tmp);
> +			if (err) {
> +				pr_err("insert failed, ret=%d\n", err);
> +				ret = err;
> +				goto out;
> +			}

Ditto.

In addition to those, I would use more meaningful variable names than m
& n.

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