[igt-dev] [Patch V3] [i-g-t] tests/i915: test pass for no caching case

Dixit, Ashutosh ashutosh.dixit at intel.com
Thu May 20 23:01:25 UTC 2021


On Thu, 20 May 2021 02:40:41 -0700, <viswax.krishna.raveendra.talabattula at intel.com> wrote:

Your commit description is good!

> diff --git a/tests/i915/gem_userptr_blits.c b/tests/i915/gem_userptr_blits.c
> index aad5f141..a90bcbfa 100644
> --- a/tests/i915/gem_userptr_blits.c
> +++ b/tests/i915/gem_userptr_blits.c
> @@ -2032,15 +2032,35 @@ static void test_set_caching(int i915)
>
>	for (int idx = 0; idx < ARRAY_SIZE(levels); idx++) {
>		gem_userptr(i915, page, 4096, 0, 0, &handle);
> -		igt_assert_eq(__gem_set_caching(i915, handle, levels[idx]), 0);
> +		if (levels[idx] == I915_CACHING_CACHED)
> +			igt_assert_eq(__gem_set_caching(i915, handle, levels[idx]), 0);
> +		else
> +			if (__gem_set_caching(i915, handle, levels[idx]) == 0)
> +				igt_warn("Deprecated return code 0 from __gem_set_caching\n");
> +			else
> +				igt_assert_eq(__gem_set_caching(i915, handle, levels[idx]), -ENXIO);

The logic is now correct. However, I still don't like __gem_set_caching
being called a second time here. How about something like this (in all 3
places):

	int ret;

	ret = __gem_set_caching(i915, handle, levels[idx]);
	if (levels[idx] == I915_CACHING_NONE)
		ret != 0 ? igt_assert_eq(ret, -ENXIO) :
		igt_warn("Deprecated userptr SET_CACHING behavior\n");
	else
		igt_assert_eq(ret, 0);

Hopefully the conditional statement works ;) otherwise change to if-else.


More information about the igt-dev mailing list