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

[Juha-Pekka Heikkila]

On 23.3.2021 2.24, 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.

Just to note those underruns are not only problem there. I assume 
they're merely symptom of some misprogramming.

Here is just plain crc fail while running on sprite plane, ie while 
changes are in use:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9879/shard-iclb7/igt@kms_plane@pixel-format-pipe-c-planes-source-clamping.html

Here is suddenly reported read from powered down well:
https://intel-gfx-ci.01.org/tree/drm-tip/IGT_6038/shard-iclb3/igt@kms_plane@pixel-format-pipe-a-planes-source-clamping.html

In pixel format test there's separate path for planar formats and all 
these problems start to show when use planar formats. I'd guess that's 
the place to look for.

/Juha-Pekka

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