| Message ID | 20170628183733.30055-1-michel.thierry@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 28/06/17 11:37, Michel Thierry wrote: > Platforms with per-engine reset enabled (i915.reset=2) are unlikely to > perform a full chip reset, keeping the reset_count unmodified. In order > to keep the expectations of this test, enforce that full GPU reset is > enabled (i915.reset=1). > > Later on, we can expand the reset_stats ioctl to also return the number > of per-engine resets and use reset_count + reset_engine_count when > checking for the updated reset count. > > v2: Rebase, don't use gem_gpu_reset_type directly, since we now have > additional helpers. > > Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com> > Signed-off-by: Michel Thierry <michel.thierry@intel.com> > --- > tests/gem_reset_stats.c | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > > diff --git a/tests/gem_reset_stats.c b/tests/gem_reset_stats.c > index 73afeeb2..9ac08aab 100644 > --- a/tests/gem_reset_stats.c > +++ b/tests/gem_reset_stats.c > @@ -27,6 +27,8 @@ > > #define _GNU_SOURCE > #include "igt.h" > +#include "igt_sysfs.h" > +#include <limits.h> > #include <stdbool.h> > #include <unistd.h> > #include <stdlib.h> > @@ -777,15 +779,24 @@ igt_main > int fd; > > bool has_reset_stats; > + bool using_full_reset; > fd = drm_open_driver(DRIVER_INTEL); > devid = intel_get_drm_devid(fd); > > has_reset_stats = gem_has_reset_stats(fd); > > + igt_assert(igt_sysfs_set_parameter > + (fd, "reset", "%d", 1 /* only global reset */)); > + > + using_full_reset = !gem_engine_reset_enabled(fd) && > + gem_gpu_reset_enabled(fd); > + > close(fd); > > igt_require_f(has_reset_stats, > "No reset stats ioctl support. Too old kernel?\n"); > + igt_require_f(using_full_reset, > + "Full GPU reset is not enabled. Is enable_hangcheck set?\n"); > } > > igt_subtest("params") > @@ -831,4 +842,13 @@ igt_main > igt_subtest_f("defer-hangcheck-%s", e->name) > RUN_TEST(defer_hangcheck(e)); > } > + > + igt_fixture { > + int fd; > + > + fd = drm_open_driver(DRIVER_INTEL); > + igt_assert(igt_sysfs_set_parameter > + (fd, "reset", "%d", INT_MAX /* any reset method */)); I would still suggest that we restore the reset value we had at the beginning of the test. I think that since we disabled single engine, this would actually set the parameter to an unsafe value. Thanks, Antonio > + close(fd); > + } > } >
On 06/07/17 15:50, Antonio Argenziano wrote: >> + >> + igt_fixture { >> + int fd; >> + >> + fd = drm_open_driver(DRIVER_INTEL); >> + igt_assert(igt_sysfs_set_parameter >> + (fd, "reset", "%d", INT_MAX /* any reset method */)); > > I would still suggest that we restore the reset value we had at the > beginning of the test. I think that since we disabled single engine, > this would actually set the parameter to an unsafe value. I talked with Antonio offline about it, but long story short, it doesn't matter much. Writing INT_MAX to i915.reset is safe; a platform without reset-engine support will still only do (and most important, report via the get-param ioctl) that it can only do full-gpu-reset, even though i915.reset > 1. -Michel
diff --git a/tests/gem_reset_stats.c b/tests/gem_reset_stats.c index 73afeeb2..9ac08aab 100644 --- a/tests/gem_reset_stats.c +++ b/tests/gem_reset_stats.c @@ -27,6 +27,8 @@ #define _GNU_SOURCE #include "igt.h" +#include "igt_sysfs.h" +#include <limits.h> #include <stdbool.h> #include <unistd.h> #include <stdlib.h> @@ -777,15 +779,24 @@ igt_main int fd; bool has_reset_stats; + bool using_full_reset; fd = drm_open_driver(DRIVER_INTEL); devid = intel_get_drm_devid(fd); has_reset_stats = gem_has_reset_stats(fd); + igt_assert(igt_sysfs_set_parameter + (fd, "reset", "%d", 1 /* only global reset */)); + + using_full_reset = !gem_engine_reset_enabled(fd) && + gem_gpu_reset_enabled(fd); + close(fd); igt_require_f(has_reset_stats, "No reset stats ioctl support. Too old kernel?\n"); + igt_require_f(using_full_reset, + "Full GPU reset is not enabled. Is enable_hangcheck set?\n"); } igt_subtest("params") @@ -831,4 +842,13 @@ igt_main igt_subtest_f("defer-hangcheck-%s", e->name) RUN_TEST(defer_hangcheck(e)); } + + igt_fixture { + int fd; + + fd = drm_open_driver(DRIVER_INTEL); + igt_assert(igt_sysfs_set_parameter + (fd, "reset", "%d", INT_MAX /* any reset method */)); + close(fd); + } }
Platforms with per-engine reset enabled (i915.reset=2) are unlikely to perform a full chip reset, keeping the reset_count unmodified. In order to keep the expectations of this test, enforce that full GPU reset is enabled (i915.reset=1). Later on, we can expand the reset_stats ioctl to also return the number of per-engine resets and use reset_count + reset_engine_count when checking for the updated reset count. v2: Rebase, don't use gem_gpu_reset_type directly, since we now have additional helpers. Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com> Signed-off-by: Michel Thierry <michel.thierry@intel.com> --- tests/gem_reset_stats.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)