From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from NAM02-BN1-obe.outbound.protection.outlook.com (mail-bn1nam07on2069.outbound.protection.outlook.com [40.107.212.69]) by gabe.freedesktop.org (Postfix) with ESMTPS id C099789DA2 for ; Mon, 27 Sep 2021 19:10:47 +0000 (UTC) From: "Pillai, Aurabindo" Date: Mon, 27 Sep 2021 19:10:45 +0000 Message-ID: References: <20210927160700.1570885-1-aurabindo.pillai@amd.com> In-Reply-To: Content-Language: en-US Content-Type: multipart/alternative; boundary="_000_DM4PR12MB529578D5A447184B322867638BA79DM4PR12MB5295namp_" MIME-Version: 1.0 Subject: Re: [igt-dev] [PATCH i-g-t] tests/kms_flip : skip subtest bo-too-big List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: igt-dev-bounces@lists.freedesktop.org Sender: "igt-dev" To: Mark Yacoub Cc: Development mailing list for IGT GPU Tools , Mark Yacoub , "Siqueira, Rodrigo" List-ID: --_000_DM4PR12MB529578D5A447184B322867638BA79DM4PR12MB5295namp_ Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable [AMD Official Use Only] Skipping using the continue statement has the side effect of this subtest b= eing absent from the list of subtests, when running the test executable wit= h paramter --list-subtest. Is that okay ? -- Regards, Jay ________________________________ From: Mark Yacoub Sent: Monday, September 27, 2021 1:34 PM To: Pillai, Aurabindo Cc: Development mailing list for IGT GPU Tools ; Mark Yacoub ; Siqueira, Rodrigo Subject: Re: [igt-dev] [PATCH i-g-t] tests/kms_flip : skip subtest bo-too-b= ig so for the sake of consistency, I'm wondering if we can skip the test the same way we skip other tests such as ``` if (tests[i].flags & TEST_NO_2X_OUTPUT) continue; ``` because we're not failing a certain condition but a whole test, it might be better not to run the test in the first place unless it's intel. So in both main loops that call run_test we can do if if (tests[i].flags & TEST_BO_TOOBIG && !intel) continue; this way we're not too worried where the flag would be used across the test and not skip halfway through a test. On Mon, Sep 27, 2021 at 1:28 PM Pillai, Aurabindo wrote: > > [AMD Official Use Only] > > > Hi Mark, > > > Both bo-too-big and bo-too-big-interruptible shall be skipped with this p= atch. The other location where TEST_BO_TOOBIG is mentioned is in the same f= unction. > > -- > > Regards, > Jay > ________________________________ > From: Mark Yacoub > Sent: Monday, September 27, 2021 1:21 PM > To: Pillai, Aurabindo > Cc: Development mailing list for IGT GPU Tools ; Mark Yacoub ; Siqueira, Rodrigo > Subject: Re: [igt-dev] [PATCH i-g-t] tests/kms_flip : skip subtest bo-too= -big > > On Mon, Sep 27, 2021 at 12:07 PM Aurabindo Pillai > wrote: > > > > [Why] > > The rationale of the test does not hold true for AMD hardware. The > > aperture size calculation has an upper bound check which is done throug= h > > i915 specific IOCTL. Hence this part of subtest must be moved > > out of the platform agnostic tests. Moreover, AMD hardware > > supports buffers larger than aperture size. > > > > [How] > > Skip the bo-too-big subtest unless its run on i915 as the test fails on > > AMD, VKMS and VC4 > > > > Signed-off-by: Aurabindo Pillai > > --- > > tests/kms_flip.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/tests/kms_flip.c b/tests/kms_flip.c > > index 54137871..20112de1 100755 > > --- a/tests/kms_flip.c > > +++ b/tests/kms_flip.c > > @@ -1290,6 +1290,7 @@ static void __run_test_on_crtc_set(struct test_ou= tput *o, int *crtc_idxs, > > /* 256 MB is usually the maximum mappable aperture, > > * (make it 4x times that to ensure failure) */ > > if (o->flags & TEST_BO_TOOBIG) { > There is also another place where TEST_BO_TOOBIG is used, should we > skip this one as well or it's only this part? > > + igt_skip_on(!is_i915_device(drm_fd)); > > bo_size =3D 4*gem_mappable_aperture_size(drm_fd); > > igt_require(bo_size < gem_global_aperture_size(drm_fd))= ; > > } > > -- > > 2.30.2 > > --_000_DM4PR12MB529578D5A447184B322867638BA79DM4PR12MB5295namp_ Content-Type: text/html; charset="us-ascii" Content-Transfer-Encoding: quoted-printable

[AMD Official Use Only]


Skipping using the continue statement has the side effect of this subtest b= eing absent from the list of subtests, when running the test executable wit= h paramter --list-subtest. Is that okay ?

--

Regards,
Jay
From: Mark Yacoub <marky= acoub@chromium.org>
Sent: Monday, September 27, 2021 1:34 PM
To: Pillai, Aurabindo <Aurabindo.Pillai@amd.com>
Cc: Development mailing list for IGT GPU Tools <igt-dev@lists.fre= edesktop.org>; Mark Yacoub <markyacoub@google.com>; Siqueira, Rodr= igo <Rodrigo.Siqueira@amd.com>
Subject: Re: [igt-dev] [PATCH i-g-t] tests/kms_flip : skip subtest b= o-too-big
 
so for the sake of consistency, I'm wondering if w= e can skip the test
the same way we skip other tests such as
```
if (tests[i].flags & TEST_NO_2X_OUTPUT)
continue;
```
because we're not failing a certain condition but a whole test, it
might be better not to run the test in the first place unless it's
intel.
So in both main loops that call run_test we can do if
if (tests[i].flags & TEST_BO_TOOBIG && !intel)
    continue;
this way we're not too worried where the flag would be used across the
test and not skip halfway through a test.

On Mon, Sep 27, 2021 at 1:28 PM Pillai, Aurabindo
<Aurabindo.Pillai@amd.com> wrote:
>
> [AMD Official Use Only]
>
>
> Hi Mark,
>
>
> Both bo-too-big and bo-too-big-interruptible shall be skipped with thi= s patch. The other location where TEST_BO_TOOBIG is mentioned is in the sam= e function.
>
> --
>
> Regards,
> Jay
> ________________________________
> From: Mark Yacoub <markyacoub@chromium.org>
> Sent: Monday, September 27, 2021 1:21 PM
> To: Pillai, Aurabindo <Aurabindo.Pillai@amd.com>
> Cc: Development mailing list for IGT GPU Tools <igt-dev@lists.freed= esktop.org>; Mark Yacoub <markyacoub@google.com>; Siqueira, Rodrig= o <Rodrigo.Siqueira@amd.com>
> Subject: Re: [igt-dev] [PATCH i-g-t] tests/kms_flip : skip subtest bo-= too-big
>
> On Mon, Sep 27, 2021 at 12:07 PM Aurabindo Pillai
> <aurabindo.pillai@amd.com> wrote:
> >
> > [Why]
> > The rationale of the test does not hold true for AMD hardware. Th= e
> > aperture size calculation has an upper bound check which is done = through
> > i915 specific IOCTL. Hence this part of subtest must be moved
> > out of the platform agnostic tests. Moreover, AMD hardware
> > supports buffers larger than aperture size.
> >
> > [How]
> > Skip the bo-too-big subtest unless its run on i915 as the test fa= ils on
> > AMD, VKMS and VC4
> >
> > Signed-off-by: Aurabindo Pillai <aurabindo.pillai@amd.com><= br> > > ---
> >  tests/kms_flip.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/tests/kms_flip.c b/tests/kms_flip.c
> > index 54137871..20112de1 100755
> > --- a/tests/kms_flip.c
> > +++ b/tests/kms_flip.c
> > @@ -1290,6 +1290,7 @@ static void __run_test_on_crtc_set(struct t= est_output *o, int *crtc_idxs,
> >         /* 256 MB is usua= lly the maximum mappable aperture,
> >          * (make it = 4x times that to ensure failure) */
> >         if (o->flags &= amp; TEST_BO_TOOBIG) {
> There is also another place where TEST_BO_TOOBIG is used, should we > skip this one as well or it's only this part?
> > +          &nbs= p;    igt_skip_on(!is_i915_device(drm_fd));
> >           =       bo_size =3D 4*gem_mappable_aperture_size(drm= _fd);
> >           =       igt_require(bo_size < gem_global_aperture= _size(drm_fd));
> >         }
> > --
> > 2.30.2
> >
--_000_DM4PR12MB529578D5A447184B322867638BA79DM4PR12MB5295namp_--