Re: [PATCH i-g-t] tests/kms: correct index validation logic

[Karthik B S <karthik.b.s@intel.com>]

Hi Jing-Ping,

On 4/21/2026 7:37 AM, Jing-Ping Jan wrote:
> Hi Kamil,
> On Tue, Apr 21, 2026 at 2:16 AM Kamil Konieczny
> <kamil.konieczny@linux.intel.com> wrote:
>> Hi Jing-Ping,
>> On 2026-04-20 at 15:49:25 +0800, Jing-Ping Jan wrote:
>>> Based on the original implementation and the comment above the function,
>>> it appears the expected behavior is for the plane indices to remain
>>> consistent with those in crtc->planes. Therefore, after swapping the old
>>> and new primary planes, we must update the index in igt_plane_t to
>>> ensure it matches the corresponding index in crtc->planes.
>>>
>>> Therefore, we should expect the indices of the old and new primary
>>> planes to remain unchanged after swapping them.
>>>
>>> Signed-off-by: Jing-Ping Jan <jingpingjan@google.com>
>>> ---
>>>   lib/igt_kms.c | 4 ++--
>>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
>>> index bec96f267..405a04640 100644
>>> --- a/lib/igt_kms.c
>>> +++ b/lib/igt_kms.c
>>> @@ -2982,8 +2982,8 @@ void igt_display_reset_outputs(igt_display_t *display)
>>>                                igt_swap(*old_primary, *new_primary);
>>>                                igt_swap(old_primary->index, new_primary->index);
>>>
>>> -                             igt_assert_neq(old_primary->index, 0);
>>> -                             igt_assert_eq(new_primary->index, 0);
>>> +                             igt_assert_eq(old_primary->index, 0);
>>> +                             igt_assert_neq(new_primary->index, 0);
>> Do you have an error report for this? This code is lib
>> and should also trigger in Intel CI, unless it is a dead
>> code.
>>
>> +cc Karthik, Swati, and J-P
>>
>> Regards,
>> Kamil
>>
>>>                        } else {
>>>                                igt_assert(old_primary == new_primary);
>>>
>>> --
>>> 2.54.0.rc1.513.gad8abe7a5a-goog
>>>
> I do not have a specific error report available at this time. This
> change is based on an issue we encountered and the existing comment in
> the code, which indicates that plane indices should remain consistent
> with those in crtc->planes after a swap.
>
> Regarding the lack of Intel CI hits, I suspect the bug depends on the
> primary plane not being the first plane in the driver's index. CI
> likely hasn't encountered this because it doesn't test this specific
> plane ordering.
>
> Please see the following output from our testing:
>
> (kms_getfb:20359) INFO: IGT-Version: IGT_VERSION-GIT (arm64) (Linux:
> 6.6.118-android15-8-g34da635f9b82-ab14801981-4k aarch64)
> (kms_getfb:20359) INFO: Using IGT_SRANDOM=1771906178 for randomisation
> (kms_getfb:20359) INFO: Opened device: /dev/dri/card0
> (kms_getfb:20359) DEBUG: Test requirement passed: !(fd<0)
> (kms_getfb:20359) DEBUG: Test requirement passed: has_getfb_iface(fd)
> (kms_getfb:20359) DEBUG: display: init {
> (kms_getfb:20359) DEBUG: Test requirement passed:
> igt_has_drm_cap(drm_fd, DRM_CAP_VBLANK_HIGH_CRTC)
> (kms_getfb:20359) DEBUG: Test requirement passed:
> resources->count_crtcs <= IGT_MAX_PIPES
> (kms_getfb:20359) DEBUG: display:     DSI-1: Selecting pipe None
> (kms_getfb:20359) DEBUG: display:     DSI-2: Selecting pipe None
> (kms_getfb:20359) DEBUG: display:     DP-1: Selecting pipe None
> (kms_getfb:20359) DEBUG: display:     DP-1: Selecting pipe None
> (kms_getfb:20359) DEBUG: Test requirement passed: !(pipe >=
> igt_display_n_crtcs(display) || !igt_crtc_for_pipe(display,
> pipe)->valid)
> (kms_getfb:20359) DEBUG: Test requirement passed: !(pipe >=
> igt_display_n_crtcs(display) || !igt_crtc_for_pipe(display,
> pipe)->valid)
> (kms_getfb:20359) DEBUG: igt_create_fb_with_bo_size(width=2152,
> height=2076, format=XR24(0x34325258), modifier=0x0, size=0)
> (kms_getfb:20359) DEBUG: igt_create_fb_with_bo_size(handle=1, pitch=8640)
> (kms_getfb:20359) DEBUG: Test requirement passed: igt_has_fb_modifiers(fd)
> (kms_getfb:20359) DEBUG: Test requirement passed:
> cairo_surface_status(fb->cairo_surface) == CAIRO_STATUS_SUCCESS
> (kms_getfb:20359) DEBUG: Test requirement passed: !(pipe >=
> igt_display_n_crtcs(display) || !igt_crtc_for_pipe(display,
> pipe)->valid)
> (kms_getfb:20359) DEBUG: Test requirement passed: !(pipe >=
> igt_display_n_crtcs(display) || !igt_crtc_for_pipe(display,
> pipe)->valid)
> (kms_getfb:20359) DEBUG: igt_create_fb_with_bo_size(width=1080,
> height=2364, format=XR24(0x34325258), modifier=0x0, size=0)
> (kms_getfb:20359) DEBUG: igt_create_fb_with_bo_size(handle=1, pitch=4352)
> (kms_getfb:20359) DEBUG: Test requirement passed: igt_has_fb_modifiers(fd)
> (kms_getfb:20359) DEBUG: Test requirement passed:
> cairo_surface_status(fb->cairo_surface) == CAIRO_STATUS_SUCCESS
> (kms_getfb:20359) CRITICAL: Test assertion failure function
> igt_display_reset_outputs, file
> external/igt-gpu-tools/lib/igt_kms.c:2985:
> (kms_getfb:20359) CRITICAL: Failed assertion: old_primary->index != 0
> (kms_getfb:20359) CRITICAL: error: 0 == 0
> (kms_getfb:20359) INFO: Stack trace: not implemented

This change LGTM, thanks for the fix.  Looks to be a miss in the 
previous update. Please add fixes tag.

Fixes: ac37e1174cc4 ("lib/kms: Pimp the primary plane swapping")

With this,

Reviewed-by: Karthik B S <karthik.b.s@intel.com>

>
> Regards,
> Jing-Ping Jan