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

From: "Dixit, Ashutosh" <ashutosh.dixit@intel.com>
To: viswax.krishna.raveendra.talabattula@intel.com
Cc: igt-dev@lists.freedesktop.org, Mahesh.Meena@intel.com
Subject: Re: [igt-dev] [Patch V3] [i-g-t] tests/i915: test pass for no caching case
Date: Thu, 20 May 2021 16:01:25 -0700	[thread overview]
Message-ID: <87o8d5dn7e.wl-ashutosh.dixit@intel.com> (raw)
In-Reply-To: <20210520094041.771946-1-viswax.krishna.raveendra.talabattula@intel.com>

On Thu, 20 May 2021 02:40:41 -0700, <viswax.krishna.raveendra.talabattula@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.
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev