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

["Surendrakumar Upadhyay, TejaskumarX" <tejaskumarx.surendrakumar.upadhyay@intel.com>]

Ok I will do corrective change and send V2. I will remove check itself, that also works.

Thanks,
Tejas

> -----Original Message-----
> From: Modem, Bhanuprakash <bhanuprakash.modem@intel.com>
> Sent: 20 August 2021 16:48
> To: Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>; Surendrakumar
> Upadhyay, TejaskumarX
> <tejaskumarx.surendrakumar.upadhyay@intel.com>; igt-
> dev@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@lists.freedesktop.org> On Behalf Of
> > Nautiyal, Ankit K
> > Sent: Friday, August 20, 2021 4:26 PM
> > To: Surendrakumar Upadhyay, TejaskumarX
> > <tejaskumarx.surendrakumar.upadhyay@intel.com>;
> > igt-dev@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@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@kms_setmode@invalid-clone-single-[crtc|crtc-stealing]
> > >
> > > Signed-off-by: Tejas Upadhyay
> > > <tejaskumarx.surendrakumar.upadhyay@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.
> 
> > >   			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;
> > >   }