Re: [igt-dev] [PATCH i-g-t 2/2] Add i915/gem_ctx_persistence

[Andi Shyti <andi.shyti@intel.com>]

Hi Chris,

On Thu, Oct 10, 2019 at 08:32:58AM +0100, Chris Wilson wrote:
> Sanity test existing persistence and new exciting non-persistent context
> behaviour.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Michał Winiarski <michal.winiarski@intel.com>
> Cc: Jon Bloomfield <jon.bloomfield@intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Andi Shyti <andi.shyti@intel.com>
> ---
>  lib/i915/gem_context.c           |  37 +++
>  lib/i915/gem_context.h           |   8 +
>  lib/igt_dummyload.c              |   3 +-
>  lib/ioctl_wrappers.c             |   1 +
>  tests/Makefile.sources           |   3 +
>  tests/i915/gem_ctx_persistence.c | 407 +++++++++++++++++++++++++++++++
>  tests/meson.build                |   1 +
>  7 files changed, 459 insertions(+), 1 deletion(-)
>  create mode 100644 tests/i915/gem_ctx_persistence.c

I think this patch should be split as ioctl_wrappers,
igt_dummyload have changes that are not related to "Sanity test
existing persistence and new exciting non-persistent context
behaviour"

I think they don't affect the test itself.

Without those changes:

Reviewed-by: Andi Shyti <andi.shyti@intel.com>

Still one question.

> +static bool enable_hangcheck(int i915)

how about adding here a boolean and use this function also for
the test_nohangcheck_hostile() case?

> +{
> +	int enabled = -1;
> +	int dir;
> +
> +	dir = igt_sysfs_open_parameters(i915);
> +	if (dir < 0) /* no parameters, must be default! */
> +		return enabled;
> +
> +	/* If i915.hangcheck is removed, assume the default is good */
> +	igt_sysfs_set(dir, "enable_hangcheck", "1");
> +	igt_sysfs_scanf(dir, "enable_hangcheck", "%d", &enabled);
> +
> +	close(dir);
> +
> +	return enabled;
> +}
> +
> +static void test_idempotent(int i915)
> +{
> +	struct drm_i915_gem_context_param p = {
> +		.param = I915_CONTEXT_PARAM_PERSISTENCE,
> +	};
> +	int expected;
> +
> +	/*
> +	 * Simple test to verify that we are able to read back the same boolean
> +	 * value as we set.
> +	 *
> +	 * Each time we invert the current value so that at the end of the test,
> +	 * if successful, we leave the context in the original state.
> +	 */
> +
> +	gem_context_get_param(i915, &p);
> +	expected = !!p.value;
> +
> +	expected = !expected;

mmhhh... looks familiar :)

Andi
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev