Re: [igt-dev] [PATCH v17 7/7] test: perf_pmu: use the gem_engine_topology library

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

On 05/04/2019 10:09, Andi Shyti wrote:
>>> @@ -177,7 +172,7 @@ static igt_spin_t * __spin_poll(int fd, uint32_t ctx, unsigned long flags)
>>>    		.engine = flags,
>>>    	};
>>> -	if (gem_can_store_dword(fd, flags))
>>> +	if (gem_class_can_store_dword(fd, flags))
>>
>> flags is not a class here but either legacy eb flags or engine map index.
>>
>> I think you can refactor so this function (and the chain of callers) takes a
>> pointer to intel_execution_engine2 and then use class and flags as needed.
> 
> It looks more logical, indeed to have the engine in it, but at
> some point you call
> 
> 	spin = spin_sync(gem_fd, 0, I915_EXEC_RENDER);
> 
> In the previous patchset I had created two branches of spin_sync,
> one that took flags and one that took intel_execution_engine2,
> but it looked to much of useless code.
> 
> In this case I could either create a dummy engine to make things
> work with intel_execution_engine2 or just use flags and minimise
> the changes.

I think it will have to take the engine since it needs both class and 
flags. Or you can make it take those parameters separately? Latter I 
guess makes it easier to convert the frequency test. But you could also 
lookup the RCS in there with the __for_each_physical_engine loop. You 
can do as you prefer. What looks tidier overall.

> In this patch there is still the problem of 4/5 subtests failing,
> but we can discuss that offline and, as you noticed, the
> igt_dummyload commit lost in the meanders of rebasing.

It could be some detail in the missing patch so will see when you post it.

Or the broken logic in __spin_poll is affecting it? I never tested with 
some engines using a pollable spinner and some not, only all or none.

Regards,

Tvrtko
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev