On Fri, 2017-12-01 at 11:13 +0000, Chris Wilson wrote:
> A new context assumes that all of its registers are in the default state
> when it is created. What may happen is that a register written by one
> context may leak into the second, causing mass confusion.
>
> v2: Extend back to Sandybridge (etc)
> v3: Check context preserves registers across suspend/hibernate and resets.
> v4: Complete the remapping onto the new class:instance
> v5: Not like that, like this, try again to use class:instance
> v6: Prepare for retrospective gen4 contexts!
> v7: Repaint register set name to nonpriv, as this is what bspec calls the
> registers that are writable by userspace.
>
> Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
<SNIP>
> +static uint32_t read_regs(int fd,
> + uint32_t ctx,
> + const struct intel_execution_engine2 *e,
> + unsigned int flags)
> +{
> + const unsigned int gen = intel_gen(intel_get_drm_devid(fd));
> + const unsigned int gen_bit = 1 << gen;
You did define BIT() in the beginning...
<SNIP>
> + n = 0;
> + for (const struct named_register *r = nonpriv_registers; r->name; r++) {
> + if (!(r->engine_mask & engine_bit))
> + continue;
> + if (!(r->gen_mask & gen_bit))
> + continue;
> +
> + for (unsigned count = r->count ?: 1, offset = r->offset;
> + count--; offset += 4) {
> + *b++ = 0x24 << 23 | (1 + r64b); /* SRM */
> + *b++ = offset;
> + reloc[n].target_handle = obj[0].handle;
I still kind of loathe the obj[0] vs obj[1] when it's not going to be
about comparing the two. obj_regs and obj_batch is just requires so
much fewer scroll-ups to see what was 0 and what was 1...
> + reloc[n].presumed_offset = 0;
> + reloc[n].offset = (b - batch) * sizeof(*b);
> + reloc[n].delta = offset;
This might confuse somebody. At least call the variable r_offset (or
reg_offset).
> + reloc[n].read_domains = I915_GEM_DOMAIN_RENDER;
> + reloc[n].write_domain = I915_GEM_DOMAIN_RENDER;
> + *b++ = offset;
> + if (r64b)
> + *b++ = 0;
> + n++;
I can't see why this is not either inside the for () or bound closer to
filling the relocation, here it's bit of a "what?".
<SNIP>
> +static void restore_regs(int fd,
> + uint32_t ctx,
> + const struct intel_execution_engine2 *e,
> + unsigned int flags,
> + uint32_t regs)
> +{
> + const unsigned int gen = intel_gen(intel_get_drm_devid(fd));
> + const unsigned int gen_bit = 1 << gen;
> + const unsigned int engine_bit = ENGINE(e->class, e->instance);
> + const bool r64b = gen >= 8;
> + struct drm_i915_gem_exec_object2 obj[2];
> + struct drm_i915_gem_execbuffer2 execbuf;
> + struct drm_i915_gem_relocation_entry *reloc;
> + unsigned int batch_size, n;
> + uint32_t *batch, *b;
> +
> + if (gen < 7) /* no LRM */
> + return;
Considering this restriction, would not it be possible to just CPU
mmap and have this as a bunch of LRIs?
<SNIP>
> +static void preservation(int fd,
> + const struct intel_execution_engine2 *e,
> + unsigned int flags)
> +{
> + static const uint32_t values[] = {
> + 0x0,
> + 0xffffffff,
> + 0xcccccccc,
> + 0x33333333,
> + 0x55555555,
> + 0xaaaaaaaa,
> + 0xdeadbeef
> + };
> + const unsigned int num_values = ARRAY_SIZE(values);
> + unsigned int engine =
> + gem_class_instance_to_eb_flags(fd, e->class, e->instance);
> + uint32_t ctx[num_values +1 ];
"+ 1"
<SNIP>
> +static unsigned int __has_context_isolation(int fd)
> +{
> + struct drm_i915_getparam gp;
> + int value = 0;
> +
> + memset(&gp, 0, sizeof(gp));
> + gp.param = 50; /* I915_PARAM_HAS_CONTEXT_ISOLATION */
> + gp.value = &value;
> +
> + igt_ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);
> + errno = 0;
> +
> + return value;
> +}
<SNIP>
> +++ b/tests/gem_exec_fence.c
> @@ -698,7 +698,7 @@ static bool has_submit_fence(int fd)
> int value = 0;
>
> memset(&gp, 0, sizeof(gp));
> - gp.param = 50; /* I915_PARAM_HAS_EXEC_SUBMIT_FENCE */
> + gp.param = 0xdeadbeef ^ 51; /* I915_PARAM_HAS_EXEC_SUBMIT_FENCE */
> gp.value = &value;
>
> ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp, sizeof(gp));
Probably wan't to sort the param stuff out :)
With the params corrected, this is;
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
Regards, Joonas
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
