[igt-dev] [PATCH i-g-t] tests/kms_plane: Don't unset primary_fb when testing other planes

[Saarinen, Jani]

Hi, 

> -----Original Message-----
> From: igt-dev <igt-dev-bounces at lists.freedesktop.org> On Behalf Of Juha-Pekka
> Heikkila
> Sent: sunnuntai 21. maaliskuuta 2021 22.03
> To: Martin Peres <martin.peres at mupuf.org>; Lyude <lyude at redhat.com>; igt-
> dev at lists.freedesktop.org; nouveau at lists.freedesktop.org
> Cc: Martin Peres <martin.peres at free.fr>; Ben Skeggs <bskeggs at redhat.com>
> Subject: Re: [igt-dev] [PATCH i-g-t] tests/kms_plane: Don't unset primary_fb when
> testing other planes
> 
> On 18.3.2021 8.41, Martin Peres wrote:
> > On 18/03/2021 00:44, Lyude wrote:
> >> From: Lyude Paul <lyude at redhat.com>
> >>
> >> Currently, nouveau doesn't support having a CRTC without a primary FB
> >> set. We won't reject such configurations, but the behavior is
> >> undefined which will cause CRCs to become undefined. So, avoid
> >> clearing the primary FB while we're testing non-primary planes.
> >
> > Looks good to me:
> >
> > Reviewed-by: Martin Peres <martin.peres at mupuf.org>
> >
> 
> This doesn't look good at all. Code changes were never run on ci, see this:
> https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5614/shards-
> all.html?testfilter=kms_plane at pixel-format
> 
> This happen because of:
> igt-gpu-tools$ git blame tests/intel-ci/blacklist-pre-merge.txt -L 173,+1
> 3e686098d9 (Martin Peres 2020-02-21 11:00:47 +0200 173) igt at kms_plane@pixel-
> format-pipe-[a-d]-planes(-source-clamping)?
> 
> You'll need to include something like this to test and review what did change on pixel
> format tests:
> https://patchwork.freedesktop.org/patch/404706/?series=84370&rev=1
> 
> but as this patch seems to be already merged as untested we now have pixel format
> tests failing reliably ever since:
> https://intel-gfx-ci.01.org/tree/drm-tip/index.html?testfilter=kms_plane@pixel-
> format&hosts=tgl%7Cicl
> 
> I have only intel icl at hand so I have no suggestions for fix. Though, looking at those
> failures Stan (CCd) may have some interest on them.
Can we just simply revert until fixed properly? 


> 
> /Juha-Pekka
> 
> >>
> >> Signed-off-by: Lyude Paul <lyude at redhat.com>
> >> Cc: Martin Peres <martin.peres at free.fr>
> >> Cc: Ben Skeggs <bskeggs at redhat.com>
> >> Cc: Jeremy Cline <jcline at redhat.com>
> >> ---
> >>   tests/kms_plane.c | 9 +++++----
> >>   1 file changed, 5 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/tests/kms_plane.c b/tests/kms_plane.c index
> >> 298a9375..c87983a4 100644
> >> --- a/tests/kms_plane.c
> >> +++ b/tests/kms_plane.c
> >> @@ -817,9 +817,10 @@ enum crc_set { PACKED_CRC_SET,
> >>              MAX_CRC_SET };
> >>   static bool test_format_plane(data_t *data, enum pipe pipe,
> >> -                  igt_output_t *output, igt_plane_t *plane)
> >> +                  igt_output_t *output, igt_plane_t *plane, igt_fb_t
> >> *primary_fb)
> >>   {
> >>       struct igt_fb fb = {};
> >> +    struct igt_fb *clear_fb = plane->type == DRM_PLANE_TYPE_PRIMARY ?
> >> primary_fb : NULL;
> >>       drmModeModeInfo *mode;
> >>       uint32_t format, ref_format;
> >>       uint64_t modifier, ref_modifier; @@ -879,7 +880,7 @@ static
> >> bool test_format_plane(data_t *data, enum pipe pipe,
> >>               height = test_fb.height;
> >>           }
> >> -        igt_plane_set_fb(plane, NULL);
> >> +        igt_plane_set_fb(plane, clear_fb);
> >>           igt_remove_fb(data->drm_fd, &test_fb);
> >>       }
> >> @@ -937,7 +938,7 @@ static bool test_format_plane(data_t *data, enum
> >> pipe pipe,
> >>       igt_pipe_crc_stop(data->pipe_crc);
> >> -    igt_plane_set_fb(plane, NULL);
> >> +    igt_plane_set_fb(plane, clear_fb);
> >>       igt_remove_fb(data->drm_fd, &fb);
> >>       return result;
> >> @@ -1010,7 +1011,7 @@ test_pixel_formats(data_t *data, enum pipe
> >> pipe)
> >>       for_each_plane_on_pipe(&data->display, pipe, plane) {
> >>           if (skip_plane(data, plane))
> >>               continue;
> >> -        result &= test_format_plane(data, pipe, output, plane);
> >> +        result &= test_format_plane(data, pipe, output, plane,
> >> &primary_fb);
> >>       }
> >>       test_fini(data);
> >>
> > _______________________________________________
> > igt-dev mailing list
> > igt-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/igt-dev
> 
> _______________________________________________
> igt-dev mailing list
> igt-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev