On Thu, Nov 10, 2016 at 11:03 PM, Matthew Auld <matthew.william.auld@gmail.com > wrote:
The absence of the above files would indicate a failure in the kernel,On 11/09, Robert Bragg wrote:
> +
> +igt_main
> +{
> + igt_skip_on_simulation();
> +
> + igt_fixture {
> + struct stat sb;
> + int ret;
> +
> + drm_fd = drm_open_driver_render(DRIVER_INTEL);
> + devid = intel_get_drm_devid(drm_fd);
> + device = drm_get_card();
> +
> + igt_require(IS_HASWELL(devid));
> + igt_require(lookup_hsw_render_basic_id());
> +
> + ret = stat("/proc/sys/dev/i915/perf_stream_paranoid", &sb);
> + igt_require(ret == 0);
> + ret = stat("/proc/sys/dev/i915/oa_max_sample_rate", &sb);
> + igt_require(ret == 0);
so would it not be more apt to assert, rather than skip ?
The test could be running against an older kernel which won't have these files and we should skip all the tests in that case.
E.g. Chris asked me to maintain compatibility within the gem_exec_parse test for older versions of the command parser so I suppose i-g-t tries to maintain backwards compatibility.
We could potentially require one and assert the other.
> +
> + gt_frequency_range_save();
> +
> + write_u64_file("/proc/sys/dev/i915/perf_stream_paranoid", 1); Don't we also want to ensure that the oa_max_sample_rate is also in a
"good" starting state before we begin, especially since we ensure that
we leave in its default state when cleaning up ?
Explicitly setting the max rate interferes with being able to assert what the default is, but that's already a problem with the cleanup fixture explicitly setting the rate.
What I'm doing now is initializing oa_max_sample_rate before tests, as suggested here, and I've also added a test_sysctl_defaults() test that's run very early, just after the i915-ref-count test.
Anyway, I think it all looks pretty reasonable to me and it looks like
we have a good amount of coverage, so you can have my r-b with Chris'
comment addressed.
Thanks, I've moved the ref counting test in front of the first fixture as suggested by chris (with just the requirements check that the i915-perf interface exists in another fixture before).
_______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
