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

[Karthik B S]

On 6/13/2022 10:41 AM, Karthik B S wrote:
> 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.

Floated https://patchwork.freedesktop.org/series/105040/ which fixes the 
issue. Could you please take a look.

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