[igt-dev] [PATCH i-g-t] tests/kms_setmode: skip subtest when valid combination not found
B, Jeevan
jeevan.b at intel.com
Thu Sep 7 09:38:36 UTC 2023
> -----Original Message-----
> From: Deak, Imre <imre.deak at intel.com>
> Sent: Wednesday, September 6, 2023 11:59 PM
> To: B, Jeevan <jeevan.b at intel.com>
> Cc: igt-dev at lists.freedesktop.org
> Subject: Re: [PATCH i-g-t] tests/kms_setmode: skip subtest when valid
> combination not found
>
> On Tue, Sep 05, 2023 at 08:24:33AM +0300, B, Jeevan wrote:
> > > -----Original Message-----
> > > From: Deak, Imre <imre.deak at intel.com>
> > > Sent: Tuesday, September 5, 2023 12:03 AM
> > > To: B, Jeevan <jeevan.b at intel.com>
> > > Cc: igt-dev at lists.freedesktop.org
> > > Subject: Re: [PATCH i-g-t] tests/kms_setmode: skip subtest when
> > > valid combination not found
> > >
> > > On Mon, Sep 04, 2023 at 10:37:49PM +0530, Jeevan B wrote:
> > > > Skip subtest with a valid message instead of "No dynamic tests executed"
> > > > when valid crtc-connector combination is not available.
> > >
> > > Please describe how the change actually fixes things. If no dynamic
> > > test is defined for a subtest the subtest will skip. Is that an issue and if so
> why?
> > Hi Imre,
> >
> > It does not resolve/fixes things, but rather provides clarity when the
> > bugs are generated.
> > Instead of skipping with "No Dynamic Subtest Executed"
>
> I still don't get it. I presume you refer to the CI results generated, but please
> provide an actual example.
>
> After this change dynamic subtests would be defined which can never run on a
> given platform and so would always skip on this platform. Is this somehow
> better than the current way of skipping the whole subtest (always)?
>
> For a given platform type there is also a variation of dynamic tests depending
> on what connected connectors the test sees. But this is the behavior both
> before and after this change.
Ok, Thanks for the Insight. Agree with your comments.
Dropping this idea.
Thanks
Jeevan B
>
> >
> > Thanks
> > Jeevan B
> > >
> > > >
> > > > Signed-off-by: Jeevan B <jeevan.b at intel.com>
> > > > ---
> > > > tests/kms_setmode.c | 64
> > > > ++++++++++++++++++++++-----------------------
> > > > 1 file changed, 32 insertions(+), 32 deletions(-)
> > > >
> > > > diff --git a/tests/kms_setmode.c b/tests/kms_setmode.c index
> > > > a611d938..74292abf 100644
> > > > --- a/tests/kms_setmode.c
> > > > +++ b/tests/kms_setmode.c
> > > > @@ -645,47 +645,47 @@ static void test_one_combination(const
> > > > struct
> > > test_config *tconf,
> > > > int connector_count) {
> > > > struct crtc_config crtcs[MAX_CRTCS];
> > > > - int crtc_count;
> > > > + int crtc_count, i, pos = 0;
> > > > + char test_name[256];
> > > > bool config_valid;
> > > >
> > > > setup_crtcs(tconf, cconfs, connector_count, crtcs,
> > > > &crtc_count, &config_valid);
> > > >
> > > > - if (config_valid == !(tconf->flags & TEST_INVALID)) {
> > > > - int i, pos = 0;
> > > > - char test_name[256];
> > > > -
> > > > - for (i = 0; i < crtc_count; i++) {
> > > > - if (i > 0)
> > > > - pos += snprintf(&test_name[pos],
> > > ARRAY_SIZE(test_name) - pos, "-");
> > > > - pos += get_test_name_str(&crtcs[i],
> > > &test_name[pos], ARRAY_SIZE(test_name) - pos);
> > > > - }
> > > > + for (i = 0; i < crtc_count; i++) {
> > > > + if (i > 0)
> > > > + pos += snprintf(&test_name[pos],
> > > ARRAY_SIZE(test_name) - pos, "-");
> > > > + pos += get_test_name_str(&crtcs[i], &test_name[pos],
> > > ARRAY_SIZE(test_name) - pos);
> > > > + }
> > > >
> > > > - for (i = 0; i < crtc_count; i++) {
> > > > - struct crtc_config *crtc = &crtcs[i];
> > > > -
> > > > - /*
> > > > - * if mode.hdisplay > 5120, then ignore
> > > > - * - last crtc in single/multi-connector config
> > > > - * - consecutive crtcs in multi-connector config
> > > > - *
> > > > - * in multi-connector config ignore if
> > > > - * - previous crtc mode.hdisplay > 5120 and
> > > > - * - current & previous crtcs are consecutive
> > > > - */
> > > > - if (((crtc->mode.hdisplay >
> > > MAX_HDISPLAY_PER_CRTC) &&
> > > > - ((crtc->crtc_idx >= (tconf->resources->count_crtcs
> > > - 1)) ||
> > > > - ((i < (crtc_count - 1)) && (abs(crtcs[i + 1].crtc_idx -
> > > crtc->crtc_idx) <= 1)))) ||
> > > > - ((i > 0) && (crtc[i - 1].mode.hdisplay >
> > > MAX_HDISPLAY_PER_CRTC) &&
> > > > - (abs(crtc->crtc_idx - crtcs[i - 1].crtc_idx) <= 1))) {
> > > > - igt_info("Combo: %s is not possible with
> > > selected mode(s).\n", test_name);
> > > > - goto out;
> > > > + igt_dynamic_f("%s", test_name)
> > > > + if (config_valid == !(tconf->flags & TEST_INVALID)) {
> > > > +
> > > > + for (i = 0; i < crtc_count; i++) {
> > > > + struct crtc_config *crtc = &crtcs[i];
> > > > +
> > > > + /*
> > > > + * if mode.hdisplay > 5120, then ignore
> > > > + * - last crtc in single/multi-connector config
> > > > + * - consecutive crtcs in multi-connector
> > > config
> > > > + *
> > > > + * in multi-connector config ignore if
> > > > + * - previous crtc mode.hdisplay > 5120 and
> > > > + * - current & previous crtcs are consecutive
> > > > + */
> > > > + if (((crtc->mode.hdisplay >
> > > MAX_HDISPLAY_PER_CRTC) &&
> > > > + ((crtc->crtc_idx >=
> > > > + (tconf->resources-
> > > >count_crtcs - 1)) ||
> > > > + ((i < (crtc_count - 1)) &&
> > > > + (abs(crtcs[i +
> > > 1].crtc_idx - crtc->crtc_idx) <= 1)))) ||
> > > > + ((i > 0) && (crtc[i -
> > > > + 1].mode.hdisplay >
> > > MAX_HDISPLAY_PER_CRTC) &&
> > > > + (abs(crtc->crtc_idx - crtcs[i -
> > > > + 1].crtc_idx) <=
> > > 1))) {
> > > > + igt_info("Combo: %s is not
> > > > + possible
> > > with selected mode(s).\n", test_name);
> > > > + goto out;
> > > > + }
> > > > }
> > > > - }
> > > >
> > > > - igt_dynamic_f("%s", test_name)
> > > > test_crtc_config(tconf, crtcs, crtc_count);
> > > > - }
> > > > + } else
> > > > + igt_skip("Valid connector-crtc combination not
> > > found\n");
> > > >
> > > > out:
> > > > cleanup_crtcs(crtcs, crtc_count);
> > > > --
> > > > 2.34.1
> > > >
More information about the igt-dev
mailing list