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

[Mark Yacoub]

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
> >>