Re: [PATCH i-g-t 1/3] lib/dummyload: Support returning output fence

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

On 19/12/2017 22:42, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2017-12-19 15:45:41)
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> Support creating spin batches which return an output fence using new
>> __igt_spin_batch_new_fence / igt_spin_batch_new_fence API.
>>
>> This will be used fromthe perf_pmu@interrupts test to ensure user
>> interrupt generation from a batch with controlled duration.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> ---
>>   lib/igt_dummyload.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++-------
>>   lib/igt_dummyload.h | 10 +++++++++
>>   2 files changed, 67 insertions(+), 8 deletions(-)
>>
>> diff --git a/lib/igt_dummyload.c b/lib/igt_dummyload.c
>> index d19b4e5ea3d2..ef08ad580246 100644
>> --- a/lib/igt_dummyload.c
>> +++ b/lib/igt_dummyload.c
>> @@ -70,9 +70,9 @@ fill_reloc(struct drm_i915_gem_relocation_entry *reloc,
>>          reloc->write_domain = write_domains;
>>   }
>>   
>> -static void emit_recursive_batch(igt_spin_t *spin,
>> -                                int fd, uint32_t ctx, unsigned engine,
>> -                                uint32_t dep)
>> +static int emit_recursive_batch(igt_spin_t *spin,
>> +                               int fd, uint32_t ctx, unsigned engine,
>> +                               uint32_t dep, bool out_fence)
>>   {
>>   #define SCRATCH 0
>>   #define BATCH 1
>> @@ -87,6 +87,7 @@ static void emit_recursive_batch(igt_spin_t *spin,
>>   
>>          nengine = 0;
>>          if (engine == -1) {
>> +               igt_assert_eq(out_fence, false);
> 
> Didn't fancy merging the fences together to return a composite out_fence?

No, just did not think of it. Will do that.

> 
>>                  for_each_engine(fd, engine)
>>                          if (engine)
>>                                  engines[nengine++] = engine;
>> @@ -165,22 +166,31 @@ static void emit_recursive_batch(igt_spin_t *spin,
>>          execbuf.buffers_ptr = to_user_pointer(obj + (2 - execbuf.buffer_count));
>>          execbuf.rsvd1 = ctx;
>>   
>> +       if (out_fence)
>> +               execbuf.flags = I915_EXEC_FENCE_OUT;
> 
> if (out_fence)
> 	execbuf.flags |= I915_EXEC_FENCE_OUT;
> 
> Just to make future changes easier?
> 
> Might also be good to insert a igt_require(gem_has_exec_fence(fd)) here
> as well. (Or earlier?)

Ack.

>> +igt_spin_t *__igt_spin_batch_new_fence(int fd,
>> +                                      uint32_t ctx,
>> +                                      unsigned engine);
>> +
>> +igt_spin_t *igt_spin_batch_new_fence(int fd,
>> +                                    uint32_t ctx,
>> +                                    unsigned engine);
> 
> Ok for now, I expect these will mangled into a new spin-batch factory
> later on.

Yeah, I was thinking whether a more generic constructor would be better, 
but then decided against sprinkling changes all over the place.

> With an igt_require(),
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> 
> If you want to merge the N engines' out_fences into one, that would save
> me a task.

Thanks, will do.

Regards,

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