[PATCH i-g-t] tests/intel/gem_exec_schedule: Update for MTL RCS/CCS w/a

[John Harrison]

On 1/8/2024 17:55, Dixit, Ashutosh wrote:
> On Mon, 08 Jan 2024 16:12:00 -0800, John.C.Harrison at Intel.com wrote:
> Hi John,
>
>> From: johnharr <johnharr at invalid-email.com>
>>
>> The RCS/CCS w/a means that if a spinners is sent to both RCS and CCS
>> engines then the system is dead and requires a reset. Even
>> pre-emptible spinners cannot be pre-empted due to the nature of the
>> w/a. So do not get into that situation.
>>
>> Signed-off-by: John Harrison <John.C.Harrison at Intel.com>
>> ---
>>   tests/intel/gem_exec_schedule.c | 14 ++++++++++++++
>>   1 file changed, 14 insertions(+)
>>
>> diff --git a/tests/intel/gem_exec_schedule.c b/tests/intel/gem_exec_schedule.c
>> index 183ee1a214bb..1b086733ad50 100644
>> --- a/tests/intel/gem_exec_schedule.c
>> +++ b/tests/intel/gem_exec_schedule.c
>> @@ -443,6 +443,14 @@
>>
>>   IGT_TEST_DESCRIPTION("Check that we can control the order of execution");
>>
>> +/*
>> + * On some platforms, there are hardware w/a's that are not compatible
>> + * with these tests.
> This is too general. I would basically add the commit description here in
> the comment. Also, not sure if we add the WA HSD number in IGT's?
Yeah, wasn't sure how much detail we should include here and was wanting 
to keep it general. I can add the text from the email description.

>
>> + */
>> +static bool skip_bad_engine(int fd, const struct intel_execution_engine2 *e) {
>> +	return IS_METEORLAKE(intel_get_drm_devid(fd)) && (e->class == I915_ENGINE_CLASS_COMPUTE);
>> +}
>> +
>>   static unsigned int offset_in_page(void *addr)
>>   {
>> 	return (uintptr_t)addr & 4095;
>> @@ -2006,6 +2014,9 @@ static igt_spin_t *__noise(int fd, uint64_t ahnd, const intel_ctx_t *ctx,
>> 	gem_context_set_priority(fd, ctx->id, prio);
>>
>> 	for_each_ctx_engine(fd, ctx, e) {
>> +		if (skip_bad_engine(fd, e))
>> +			continue;
>> +
>> 		if (spin == NULL) {
>> 			spin = __igt_spin_new(fd,
>> 					      .ahnd = ahnd,
>> @@ -2313,6 +2324,9 @@ static void preempt_self(int fd, const intel_ctx_cfg_t *cfg,
>> 	n = 0;
>> 	gem_context_set_priority(fd, ctx[HI]->id, MIN_PRIO);
>> 	for_each_ctx_cfg_engine(fd, cfg, e) {
>> +		if (skip_bad_engine(fd, e))
>> +			continue;
>> +
>> 		spin[n] = __igt_spin_new(fd,
>> 					 .ahnd = ahnd[NOISE],
>> 					 .ctx = ctx[NOISE],
> Aren't there other places (even in this file) where spinners are being
> scheduled on both RCS and CCS? We need this skip only in the two places above?
The only pre-merge failures in CI for the w/a patch itself was these 
three tests - preempt-other, preempt-other-chain (same core path as 
preempt-other) and preempt-self. If I run the whole of the 
gem_exec_schedule test then I also get a failure on wide at rcs0 and 
wide at ccs0 but I get those on a straight drm-tip tree without the w/a 
applied anyway. So yes, as far as I can tell, these are the only places 
where we have a problem.

Note that simply running a spinner on all engines is not a problem. The 
problem is attempting to pre-empt that spinner. If the spinner is ended 
normally, then everything is fine.

John.

>
> Thanks.
> --
> Ashutosh