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

[Lyude Paul]

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.

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!