Re: [PATH i-g-t v12 2/2] tests: add slice power programming test

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 12/09/2018 12:53, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2018-09-11 15:42:10)
>> +       last_with_engines = -1;
>> +       for (class = 0; class < ~0; class++) {
>> +               for (instance = 0; instance < ~0; instance++) {
>> +                       int ret;
>> +
>> +                       sseu.class = class;
>> +                       sseu.instance = instance;
>> +
>> +                       ret = __gem_context_set_param(fd, &arg);
>> +
>> +                       if (has_engine(fd, class, instance)) {
>> +                               if (engine_supports_sseu(fd, class, instance))
> 
> Meh, <rant>. I just don't like having hardcoded db on this side of the
> ABI. The ABI imo is to ask the kernel if the device/engine is supported,
> and should not allow for assumptions.

Done.

>> +static void
>> +test_dynamic(int fd, unsigned int flags)
>> +{
>> +       uint64_t pg_slice_mask = mask_minus_one(__slice_mask__);
>> +       unsigned int pg_slice_count = __slice_count__ - 1;
>> +       struct drm_i915_gem_context_param_sseu sseu = { };
>> +       struct drm_i915_gem_context_param arg =
>> +               { .param = I915_CONTEXT_PARAM_SSEU,
>> +                 .ctx_id = gem_context_create(fd),
>> +                 .size = sizeof(sseu),
>> +                 .value = to_user_pointer(&sseu) };
>> +       igt_spin_t *spin = NULL;
>> +
>> +       gem_context_get_param(fd, &arg);
>> +
>> +       /* First set the default mask */
>> +       if (flags & TEST_BUSY)
>> +               spin = __spin_sync(fd, arg.ctx_id, I915_EXEC_RENDER);
>> +
>> +       sseu.slice_mask = __slice_mask__;
>> +       gem_context_set_param(fd, &arg);
> 
> I would also suggest a reset test here. Both reset when idle, and by
> hangcheck/forced-reset of the spinner & active context.
> 
> And across suspend.

Reset & suspsend after set param but before execbuf? Or after execbuf 
and then re-read rpcs?

>> +       igt_assert_eq(read_slice_count_busy(fd, arg.ctx_id, 0, spin),
>> +                     __slice_count__);
>> +       igt_assert_eq(read_slice_count(fd, 0, 0), __slice_count__);
> 
> In the read_slice I would suggest having a
> igt_assert(gem_bo_busy(spin->handle));

Done.

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx