[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