[igt-dev] [PATCH i-g-t] tests/kms_concurrent: check if second primary plane is valid

[Abhinav Kumar]

Thanks, I have added the comment on top of the if
and applied the change.

Abhinav

On 3/25/2022 8:21 AM, Mark Yacoub wrote:
> On Thu, Mar 24, 2022 at 5:00 PM Abhinav Kumar <quic_abhinavk at quicinc.com> wrote:
>>
>>
>>
>> On 3/24/2022 1:56 PM, Mark Yacoub wrote:
>>> On Thu, Mar 24, 2022 at 3:57 PM Abhinav Kumar <quic_abhinavk at quicinc.com> wrote:
>>>>
>>>> prepare_planes() in kms_concurrent loops through the list
>>>> of planes and assigns them to the data->plane array.
>>>>
>>>> During this assignment, it skips planes which are of type
>>>> DRM_PLANE_TYPE_PRIMARY as they are assigned separately.
>>>>
>>>> However, only one primary plane is assigned and prepared for
>>>> the test.
>>> do you not need the other primary plane to be assigned and prepared
>>> for the test?
>>
>> I am just fixing the current behavior of the test which is prepare one
>> primary plane and rest of the overlay planes before the commit.
>>
>> This was broken without this change.
>>
>> If we want to change the behavior of the test to prepare multiple
>> primary planes and rest of the overlay planes, that would be a separate
>> change in my opinion.
> I'm okay with keeping the current behavior. but please add a comment
> on top of the if-statement why some plans might be null. with this
> being done:
>>
>> Let me know what you think.
>>
>> Thanks
>>
>> Abhinav
>>>>
>>>> This causes an issue later on in the test when there are
>>>> multiple primary planes as the second one is not added
>>>> to the data->plane list causing a segmentation fault
>>>> when trying to access the second primary plane.
>>>>
>>>> Check if the data->plane is valid before accessing its
>>>> members to fix this crash.
>>>>
>>>> This fixes the crash and makes kms_concurrent pass on devices
>>>> having multiple primary planes.
>>>>
>>>> localhost /usr/local/libexec/igt-gpu-tools # ./kms_concurrent
>>>> IGT-Version: 1.26-NO-GIT (arm) (Linux: 5.17.0-rc2-lockdep-75821-g6200d34d858f aarch64)
>>>> Starting subtest: pipe-A
>>>> Testing resolution with connector eDP-1 using pipe A with seed 1648151339
>>>> Subtest pipe-A: SUCCESS (0.429s)
>>>> Starting subtest: pipe-B
>>>> Testing resolution with connector eDP-1 using pipe B with seed 1648151340
>>>> Subtest pipe-B: SUCCESS (0.592s)
>>>> Test requirement not met in function igt_require_pipe, file ../igt-gpu-tools-9999/lib/igt_kms.c:2135:
>>>> Test requirement: !(pipe >= display->n_pipes || !display->pipes[pipe].enabled)
>>>> Pipe C does not exist or not enabled
>>>> Subtest pipe-C: SKIP
>>>> Test requirement not met in function igt_require_pipe, file ../igt-gpu-tools-9999/lib/igt_kms.c:2135:
>>>> Test requirement: !(pipe >= display->n_pipes || !display->pipes[pipe].enabled)
>>>> Pipe D does not exist or not enabled
>>>> Subtest pipe-D: SKIP
>>>> Test requirement not met in function igt_require_pipe, file ../igt-gpu-tools-9999/lib/igt_kms.c:2135:
>>>> Test requirement: !(pipe >= display->n_pipes || !display->pipes[pipe].enabled)
>>>> Pipe E does not exist or not enabled
>>>> Subtest pipe-E: SKIP
>>>> Test requirement not met in function igt_require_pipe, file ../igt-gpu-tools-9999/lib/igt_kms.c:2135:
>>>> Test requirement: !(pipe >= display->n_pipes || !display->pipes[pipe].enabled)
>>>> Pipe F does not exist or not enabled
>>>> Subtest pipe-F: SKIP
>>>>
>>>> Signed-off-by: Abhinav Kumar <quic_abhinavk at quicinc.com>
> Reviewed-by: Mark Yacoub <markyacoub at chromium.org>
>>>> ---
>>>>    tests/kms_concurrent.c | 2 +-
>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/tests/kms_concurrent.c b/tests/kms_concurrent.c
>>>> index 1b8f4b0..9cbeec4 100644
>>>> --- a/tests/kms_concurrent.c
>>>> +++ b/tests/kms_concurrent.c
>>>> @@ -116,7 +116,7 @@ create_fb_for_mode_position(data_t *data, drmModeModeInfo *mode,
>>>>                           0.0f, 0.0f, 1.0f);
>>>>
>>>>           for (int i = 0; i < max_planes; i++) {
>>>> -               if (data->plane[i]->type == DRM_PLANE_TYPE_PRIMARY)
>>>> +               if (data->plane[i] && data->plane[i]->type == DRM_PLANE_TYPE_PRIMARY)
>>>>                           continue;
>>>>
>>>>                   igt_paint_color(cr, rect_x[i], rect_y[i],
>>>> --
>>>> 2.7.4
>>>>