RE: [PATCH i-g-t] tests/intel/xe_pxp: Require output for display tests

-----Original Message-----
From: Ceraolo Spurio, Daniele <[email protected]> 
Sent: Monday, July 20, 2026 4:00 PM
To: Cavitt, Jonathan <[email protected]>; [email protected]
Cc: Zuo, Alex <[email protected]>; [email protected]
Subject: Re: [PATCH i-g-t] tests/intel/xe_pxp: Require output for display tests
> 
> On 7/20/2026 3:46 PM, Jonathan Cavitt wrote:
> > The subtests display-pxp-fb and display-black-pxp-fb both call the
> > function igt_create_color_fb with a width and height value generated by
> > getting outputs from for_each_connected_output and selecting the maximum
> > width and height among all available connected outputs.  If there are no
> > such outputs, then the resulting values will be zero, which is an invalid
> > input for the DRM_XE_CREATE_DUMB ioctl.
> >
> > Skip these tests if there are no connected outputs.
> >
> > Signed-off-by: Jonathan Cavitt <[email protected]>
> > Cc: Daniele Ceraolo Spurio <[email protected]>
> > Cc: Ville Syrjälä <[email protected]>
> > ---
> >   tests/intel/xe_pxp.c | 14 ++++++++++++++
> >   1 file changed, 14 insertions(+)
> >
> > diff --git a/tests/intel/xe_pxp.c b/tests/intel/xe_pxp.c
> > index 9320078733..080dc9519e 100644
> > --- a/tests/intel/xe_pxp.c
> > +++ b/tests/intel/xe_pxp.c
> > @@ -868,6 +868,18 @@ static void compare_crcs(int fd, igt_display_t *display, igt_fb_t *ref_fb, igt_f
> >   	}
> >   }
> >   
> > +static void require_connected_output(igt_display_t *display)
> > +{
> > +	igt_output_t *output;
> > +	bool connected_output = false;
> > +
> > +	for_each_connected_output(display, output) {
> > +		connected_output = true;
> > +		break;
> > +	}
> > +	igt_require_f(connected_output, "Display has no connected outputs\n");
> > +}
> 
> We already have igt_display_require_output, any reason not to use that?

I didn't know that function existed at the time, though I'm looking now and it seems like
the two functions differ slightly, resulting in igt_display_require_output being stricter
than requires_connected_output.  Specifically, it appears that igt_display_require_output
also mandates that each connector has a valid crtc connection, whereas the current
requires_connected_output implementation does not.

In practice, it's likely that the distinction there won't matter too much, so I'll switch over
to using the former as suggested.
-Jonathan Cavitt

> 
> > +
> >   /**
> >    * SUBTEST: display-pxp-fb
> >    * Description: Test that an encrypted fb is displayed correctly by comparing
> > @@ -1165,12 +1177,14 @@ int igt_main()
> >   		igt_subtest("display-pxp-fb") {
> >   			require_display(xe_fd, &display);
> >   			require_pxp_render(xe_fd, pxp_supported);
> > +			require_connected_output(&display);
> 
> This should be called from inside require_display()
> 
> Daniele
> 
> >   			test_display_pxp_fb(xe_fd, &display);
> >   		}
> >   
> >   		igt_subtest("display-black-pxp-fb") {
> >   			require_display(xe_fd, &display);
> >   			require_pxp_render(xe_fd, pxp_supported);
> > +			require_connected_output(&display);
> >   			test_display_black_pxp_fb(xe_fd, &display);
> >   		}
> >   	}
> 
>