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

[Petri Latvala]

On Mon, Mar 22, 2021 at 08:24:26PM -0400, Lyude Paul wrote:
> hm - I gave my reviewed-by: here before but taking a closer look at this, I am
> seeing underruns being noted in the pixel format tests:
> 
> https://intel-gfx-ci.01.org/tree/drm-tip/index.html?testfilter=kms_plane@pixel-format&hosts=tgl%7Cicl
> 
> I haven't gotten a closer look yet, but I'm extremely doubtful this is an error
> on the test's part unless those underruns were there before.

The underruns are the regression. The commit might be doing the right
thing as far as the documented interface is concerned but Intel folks
need to figure out the actual cause of the underruns and help get this
in without regressions.


-- 
Petri Latvala



> 
> On Mon, 2021-03-22 at 10:25 +0200, Martin Peres wrote:
> > The patch led to a regression in i915 results. The regression wasn't
> > caught pre-merge because the test was blacklisted for pre-merge by
> > yours truly due to its prohibitive execution time, but my creation
> > came back to bite me in the ass, Frakenstein-style.
> > 
> > It is currently unclear if the regression introduced by this change is
> > an i915 bug, a KMS spec violation introduced by the change, or
> > generally a loose-spec and just incompatible HW between NVIDIA and
> > Intel. While we figure this thing out, let's just revert the change
> > and put the cost of integration on the people proposing the
> > change (Lyude and I).
> > 
> > Sorry for the noise!
> > 
> > Signed-off-by: Martin Peres <martin.peres at mupuf.org>
> > Cc: Jani Saarinen <jani.saarinen at intel.com>
> > Cc: Lyude Paul <lyude at redhat.com>
> > Cc: juhapekka.heikkila at gmail.com
> > ---
> >  tests/kms_plane.c | 9 ++++-----
> >  1 file changed, 4 insertions(+), 5 deletions(-)
> > 
> > diff --git a/tests/kms_plane.c b/tests/kms_plane.c
> > index c87983a4..298a9375 100644
> > --- a/tests/kms_plane.c
> > +++ b/tests/kms_plane.c
> > @@ -817,10 +817,9 @@ 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_fb_t
> > *primary_fb)
> > +                             igt_output_t *output, igt_plane_t *plane)
> >  {
> >         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;
> > @@ -880,7 +879,7 @@ static bool test_format_plane(data_t *data, enum pipe pipe,
> >                         height = test_fb.height;
> >                 }
> >  
> > -               igt_plane_set_fb(plane, clear_fb);
> > +               igt_plane_set_fb(plane, NULL);
> >  
> >                 igt_remove_fb(data->drm_fd, &test_fb);
> >         }
> > @@ -938,7 +937,7 @@ static bool test_format_plane(data_t *data, enum pipe pipe,
> >  
> >         igt_pipe_crc_stop(data->pipe_crc);
> >  
> > -       igt_plane_set_fb(plane, clear_fb);
> > +       igt_plane_set_fb(plane, NULL);
> >         igt_remove_fb(data->drm_fd, &fb);
> >  
> >         return result;
> > @@ -1011,7 +1010,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,
> > &primary_fb);
> > +               result &= test_format_plane(data, pipe, output, plane);
> >         }
> >  
> >         test_fini(data);
> 
> -- 
> Sincerely,
>    Lyude Paul (she/her)
>    Software Engineer at Red Hat
>    
> Note: I deal with a lot of emails and have a lot of bugs on my plate. If you've
> asked me a question, are waiting for a review/merge on a patch, etc. and I
> haven't responded in a while, please feel free to send me another email to check
> on my status. I don't bite!
> 
> _______________________________________________
> igt-dev mailing list
> igt-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev