[igt-dev] [i-g-t] tests/kms_setmode: Handle unsupported modes for invalid crtc test

[Surendrakumar Upadhyay, TejaskumarX]

> -----Original Message-----
> From: Modem, Bhanuprakash <bhanuprakash.modem at intel.com>
> Sent: 20 August 2021 16:48
> To: Nautiyal, Ankit K <ankit.k.nautiyal at intel.com>; Surendrakumar
> Upadhyay, TejaskumarX
> <tejaskumarx.surendrakumar.upadhyay at intel.com>; igt-
> dev at lists.freedesktop.org
> Subject: RE: [igt-dev] [i-g-t] tests/kms_setmode: Handle unsupported modes
> for invalid crtc test
> 
> > From: igt-dev <igt-dev-bounces at lists.freedesktop.org> On Behalf Of
> > Nautiyal, Ankit K
> > Sent: Friday, August 20, 2021 4:26 PM
> > To: Surendrakumar Upadhyay, TejaskumarX
> > <tejaskumarx.surendrakumar.upadhyay at intel.com>;
> > igt-dev at lists.freedesktop.org
> > Subject: Re: [igt-dev] [i-g-t] tests/kms_setmode: Handle unsupported
> > modes for invalid crtc test
> >
> > Thanks for the patch. This indeed was missing for the invalid tests
> > with multi-displays.
> >
> > Patch looks good to me.
> >
> > Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal at intel.com>
> >
> >
> > On 8/19/2021 1:12 PM, Tejas Upadhyay wrote:
> > > Unsupported modes in crtc need to return proper error in order to
> > > report back that applied config was failed or not. Thus added ENOENT
> > > error.
> > >
> > > Test affected (fail -> pass after change) :
> > > igt at kms_setmode@invalid-clone-single-[crtc|crtc-stealing]
> > >
> > > Signed-off-by: Tejas Upadhyay
> > > <tejaskumarx.surendrakumar.upadhyay at intel.com>
> > > ---
> > >   tests/kms_setmode.c | 7 ++++++-
> > >   1 file changed, 6 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/tests/kms_setmode.c b/tests/kms_setmode.c index
> > > 89220b83..f322a373 100644
> > > --- a/tests/kms_setmode.c
> > > +++ b/tests/kms_setmode.c
> > > @@ -569,8 +569,10 @@ int __test_crtc_config(struct crtc_config
> > > *crtcs, int
> > crtc_count,
> > >   	for (int i = 0; i < crtc->cconfs->connector->count_modes; i++) {
> > >   		uint32_t *ids;
> > >
> > > -		if (!crtc_supports_mode(crtc, &crtc->cconfs->connector-
> >modes[i]))
> > > +		if (!crtc_supports_mode(crtc,
> > > +&crtc->cconfs->connector->modes[i]))
> > {
> > > +			ret = -ENOENT;
> 
> I guess, we can drop this crtc_supports_mode() check, since our intension is
> to catch EINVAL only as there are N #of possibilities to fail.

With dropping this check, we are seeing failure in invalid-clone-single-crtc-stealing. See below :
  Test id#1 CRTC count 1
    CRTC[80] [Pipe A] Mode: 3840x2160 at 60Hz Connectors: eDP-1[236], HDMI-A-1[245]
(kms_setmode:9928) igt_fb-DEBUG: igt_create_fb_with_bo_size(width=3840, height=2160, format=XR24(0x34325258), modifier=0x0, size=0)
(kms_setmode:9928) igt_fb-DEBUG: igt_create_fb_with_bo_size(handle=2, pitch=15360)
(kms_setmode:9928) ioctl_wrappers-DEBUG: Test requirement passed: igt_has_fb_modifiers(fd)
(kms_setmode:9928) igt_fb-DEBUG: Test requirement passed: cairo_surface_status(fb->cairo_surface) == CAIRO_STATUS_SUCCESS
(kms_setmode:9928) igt_fb-DEBUG: Test requirement passed: cairo_surface_status(fb->cairo_surface) == CAIRO_STATUS_SUCCESS
(kms_setmode:9928) CRITICAL: Test assertion failure function test_stealing, file ../tests/kms_setmode.c:410:
(kms_setmode:9928) CRITICAL: Failed assertion: ret == 0
(kms_setmode:9928) CRITICAL: Last errno: 22, Invalid argument
(kms_setmode:9928) CRITICAL: error: -22 != 0

While with this patch all the tests passes. Can we go ahead and merge this patch?

Thanks,
Tejas 

> 
> > >   			continue;
> > > +		}
> > >
> > >   		crtc->mode = crtc->cconfs->connector->modes[i];
> > >
> > > @@ -622,6 +624,9 @@ int __test_crtc_config(struct crtc_config
> > > *crtcs, int
> > crtc_count,
> > >   		return ret;
> > >   	}
> > >
> > > +	if (ret < 0)
> > > +		*config_failed = true;
> 
> Please consider above comment.
> 
> - Bhanu
> 
> > > +
> > >   	/* When all crtcs[base].modes are tried & failed to fit into link BW. */
> > >   	return ret;
> > >   }