Re: [PATCH i-g-t 7/9] tests/perf_pmu: Tests for i915 PMU API

Chris Wilson
2017-10-10T12:37:26Z

Quoting Tvrtko Ursulin (2017-10-10 10:30:06)
> +static void
> +event_wait(int gem_fd, const struct intel_execution_engine2 *e)
> +{
> + struct drm_i915_gem_exec_object2 obj = { };
> + struct drm_i915_gem_execbuffer2 eb = { };
> + data_t data;
> + igt_display_t *display = &data.display;
> + const uint32_t DERRMR = 0x44050;
> + unsigned int valid_tests = 0;
> + uint32_t batch[8], *b;
> + igt_output_t *output;
> + uint32_t bb_handle;
> + uint32_t reg;
> + enum pipe p;
> + int fd;
> +
> + igt_require(intel_gen(intel_get_drm_devid(gem_fd)) >= 6);
> + igt_require(intel_register_access_init(intel_get_pci_device(),
> + false, gem_fd) == 0);
> +
> + /**
> + * We will use the display to render event forwarind so need to
> + * program the DERRMR register and restore it at exit.

DERRMR is always masked until we need it. If you really wanted to
preserve the old value, SRM, LRM around the test. Not that fussed, just
a general dislike of direct poking of registers.

> + *
> + * We will emit a MI_WAIT_FOR_EVENT listening for vblank events,
> + * have a background helper to indirectly enable vblank irqs, and
> + * listen to the recorded time spent in engine wait state as reported
> + * by the PMU.
> + */
> + reg = intel_register_read(DERRMR);
> +
> + kmstest_set_vt_graphics_mode();
> + igt_display_init(&data.display, gem_fd);
> +
> + bb_handle = gem_create(gem_fd, 4096);
> +
> + b = batch;
> + *b++ = MI_LOAD_REGISTER_IMM;
> + *b++ = DERRMR;
> + *b++ = reg & ~((1 << 3) | (1 << 11) | (1 << 21));
> + *b++ = MI_WAIT_FOR_EVENT | MI_WAIT_FOR_PIPE_A_VBLANK;
> + *b++ = MI_LOAD_REGISTER_IMM;
> + *b++ = DERRMR;
> + *b++ = reg;
> + *b++ = MI_BATCH_BUFFER_END;

> +static int chain_nop(int gem_fd, unsigned long sz, int in_fence, bool sync)
> +{
> + struct drm_i915_gem_exec_object2 obj = {};
> + struct drm_i915_gem_execbuffer2 eb =
> + { .buffer_count = 1, .buffers_ptr = (uintptr_t)&obj};
> + const uint32_t bbe = 0xa << 23;
> +
> + sz = ALIGN(sz, sizeof(uint32_t));
> +
> + obj.handle = gem_create(gem_fd, sz);
> + gem_write(gem_fd, obj.handle, sz - sizeof(bbe), &bbe, sizeof(bbe));
> +
> + eb.flags = I915_EXEC_RENDER | I915_EXEC_FENCE_OUT;
> +
> + if (in_fence >= 0) {
> + eb.flags |= I915_EXEC_FENCE_IN;
> + eb.rsvd2 = in_fence;
> + }
> +
> + gem_execbuf_wr(gem_fd, &eb);

On the same ctx/engine, this shouldn't be generating interrupts between
the batches. The fence should be resolved to an i915 request and then we
see that the requests are naturally ordered so the fence is elided.
> +
> + if (sync)
> + gem_sync(gem_fd, obj.handle);

So it looks like this will remain the only interrupt generator.

If we exported the out-fence and then polled that, that is currently
hooked up to an interrupt only path.

I don't have anything else to say! Afaict you have everything covered,
so the only way to find what's not is by letting it go live!

Give or take more tuning of the interrupt test,
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris