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

[Karthik B S]

On 6/10/2022 7:59 PM, Juha-Pekka Heikkila wrote:
> On 2.6.2022 12.42, B, Jeevan wrote:
>>> -----Original Message-----
>>> From: B S, Karthik <karthik.b.s at intel.com>
>>> Sent: Wednesday, June 1, 2022 10:59 AM
>>> To: igt-dev at lists.freedesktop.org
>>> Cc: B, Jeevan <jeevan.b at intel.com>; B S, Karthik 
>>> <karthik.b.s at 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 at intel.com>
>> Reviewed-by: Jeevan B <jeevan.b at intel.com>

Hi JP,

Thanks for pointing this out. Didn't notice this as pre-merge shows all 
green on CI. Will find the root cause and float a fix asap or will 
revert it.

Thanks,
Karthik.B.S
>
> 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
>>
>