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

[Martin Peres <martin.peres@mupuf.org>]

On 21/03/2021 22:02, Juha-Pekka Heikkila wrote:
> On 18.3.2021 8.41, Martin Peres wrote:
>> On 18/03/2021 00:44, Lyude wrote:
>>> From: Lyude Paul <lyude@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@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@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@kms_plane@pixel-format-pipe-[a-d]-planes(-source-clamping)?

Killed by my own creation, isn't it quaint? Thanks a lot for 
investigating the regression, and I'll be wearing my brown bag today :s

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

I would indeed like to hear if this sounds like a kernel bug, a 
violation of the KMS spec, or the spec is not clear-enough on this.

If I don't hear back on this today, I'll send a revert while the 
situation is investigated.

Martin

> 
> /Juha-Pekka
> 
>>>
>>> Signed-off-by: Lyude Paul <lyude@redhat.com>
>>> Cc: Martin Peres <martin.peres@free.fr>
>>> Cc: Ben Skeggs <bskeggs@redhat.com>
>>> Cc: Jeremy Cline <jcline@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@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/igt-dev
> 
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau