Re: [igt-dev] [PATCH i-g-t v2] tests/i915/kms_big_fb: Move cleanup code to fixture

[Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>]

On 2.6.2022 12.42, B, Jeevan wrote:
>> -----Original Message-----
>> From: B S, Karthik <karthik.b.s@intel.com>
>> Sent: Wednesday, June 1, 2022 10:59 AM
>> To: igt-dev@lists.freedesktop.org
>> Cc: B, Jeevan <jeevan.b@intel.com>; B S, Karthik <karthik.b.s@intel.com>
>> Subject: [PATCH i-g-t v2] tests/i915/kms_big_fb: Move cleanup code to
>> fixture
>>
>> When a subtest fails, it exits from the assert and does not run the cleanup
>> part of the subtest. To avoid this, move the cleanup code to igt_fixture
>> outside the subtest.
>>
>> Signed-off-by: Karthik B S <karthik.b.s@intel.com>
> Reviewed-by: Jeevan B <jeevan.b@intel.com>

Guys, this set broke big_fb tests. See any of these tests that were ran 
https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7210/shards-all.html?testfilter=kms_big_fb

For example 
https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7210/shard-dg1-17/igt@kms_big_fb@linear-16bpp-rotate-0.html

..
Starting subtest: linear-16bpp-rotate-0
Subtest linear-16bpp-rotate-0: SUCCESS (1.781s)
(kms_big_fb:992) intel_allocator-CRITICAL: Test assertion failure 
function handle_request, file ../lib/intel_allocator.c:477:
(kms_big_fb:992) intel_allocator-CRITICAL: Failed assertion: al
Test kms_big_fb failed.
..

/Juha-Pekka


>> ---
>>   tests/i915/kms_big_fb.c | 46 ++++++++++++++++++-----------------------
>>   1 file changed, 20 insertions(+), 26 deletions(-)
>>
>> diff --git a/tests/i915/kms_big_fb.c b/tests/i915/kms_big_fb.c index
>> 91ad3b4d..abf974bb 100644
>> --- a/tests/i915/kms_big_fb.c
>> +++ b/tests/i915/kms_big_fb.c
>> @@ -264,12 +264,6 @@ static void prep_fb(data_t *data)
>>   	generate_pattern(data, &data->big_fb, 640, 480);  }
>>
>> -static void cleanup_fb(data_t *data)
>> -{
>> -	igt_remove_fb(data->drm_fd, &data->big_fb);
>> -	data->big_fb.fb_id = 0;
>> -}
>> -
>>   static void set_c8_lut(data_t *data)
>>   {
>>   	igt_pipe_t *pipe = &data->display.pipes[data->pipe]; @@ -463,14
>> +457,6 @@ static bool test_pipe(data_t *data)
>>   	if (data->format == DRM_FORMAT_C8)
>>   		unset_lut(data);
>>
>> -	igt_pipe_crc_free(data->pipe_crc);
>> -
>> -	igt_output_set_pipe(data->output, PIPE_ANY);
>> -
>> -	igt_remove_fb(data->drm_fd, &data->small_fb);
>> -
>> -	intel_bb_destroy(data->ibb);
>> -
>>   	return ret;
>>   }
>>
>> @@ -563,14 +549,6 @@ max_hw_stride_async_flip_test(data_t *data)
>>   	}
>>   	igt_reset_timeout();
>>
>> -	igt_pipe_crc_free(data->pipe_crc);
>> -	igt_output_set_pipe(data->output, PIPE_NONE);
>> -	igt_remove_fb(data->drm_fd, &data->big_fb);
>> -	igt_remove_fb(data->drm_fd, &data->big_fb_flip[0]);
>> -	igt_remove_fb(data->drm_fd, &data->big_fb_flip[1]);
>> -
>> -	intel_bb_destroy(data->ibb);
>> -
>>   	return true;
>>   }
>>
>> @@ -773,6 +751,22 @@ set_max_hw_stride(data_t *data)
>>   	}
>>   }
>>
>> +static void test_cleanup(data_t *data)
>> +{
>> +	if (!data->output)
>> +		return;
>> +
>> +	igt_pipe_crc_free(data->pipe_crc);
>> +	igt_output_set_pipe(data->output, PIPE_NONE);
>> +	igt_remove_fb(data->drm_fd, &data->big_fb);
>> +	igt_remove_fb(data->drm_fd, &data->big_fb_flip[0]);
>> +	igt_remove_fb(data->drm_fd, &data->big_fb_flip[1]);
>> +	igt_remove_fb(data->drm_fd, &data->small_fb);
>> +
>> +	intel_bb_destroy(data->ibb);
>> +	data->output = NULL;
>> +}
>> +
>>   static data_t data = {};
>>
>>   static const struct {
>> @@ -918,7 +912,7 @@ igt_main
>>   			}
>>
>>   			igt_fixture
>> -				cleanup_fb(&data);
>> +				test_cleanup(&data);
>>   		}
>>   	}
>>
>> @@ -972,10 +966,10 @@ igt_main
>>   							test_scanout(&data);
>>   					}
>>   					data.async_flip_test = false;
>> -				}
>>
>> -				igt_fixture
>> -					cleanup_fb(&data);
>> +					igt_fixture
>> +						test_cleanup(&data);
>> +				}
>>   			}
>>   		}
>>   	}
>> --
>> 2.22.0
>