From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from NAM04-MW2-obe.outbound.protection.outlook.com (mail-mw2nam08on2059.outbound.protection.outlook.com [40.107.101.59]) by gabe.freedesktop.org (Postfix) with ESMTPS id 856866E03C for ; Mon, 27 Sep 2021 19:23:10 +0000 (UTC) From: "Pillai, Aurabindo" Date: Mon, 27 Sep 2021 19:23:07 +0000 Message-ID: References: <20210927160700.1570885-1-aurabindo.pillai@amd.com> In-Reply-To: Content-Language: en-US Content-Type: multipart/alternative; boundary="_000_DM4PR12MB52958E8DC8ED99397D2176D78BA79DM4PR12MB5295namp_" 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_DM4PR12MB52958E8DC8ED99397D2176D78BA79DM4PR12MB5295namp_ Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable [AMD Official Use Only] Sure, that makes sense. Will send a v2 -- Regards, Jay ________________________________ From: Mark Yacoub Sent: Monday, September 27, 2021 3:12 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 I'm not 100% sure but personally if I'm running the test on an AMD device, I don't really care about seeing a subtest that I'm not interested in. On Mon, Sep 27, 2021 at 3:10 PM Pillai, Aurabindo wrote: > > [AMD Official Use Only] > > > Skipping using the continue statement has the side effect of this subtest= being absent from the list of subtests, when running the test executable w= ith 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= -big > > 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= patch. The other location where TEST_BO_TOOBIG is mentioned is in the same= function. > > > > -- > > > > 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-t= oo-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 thro= ugh > > > 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_= output *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_DM4PR12MB52958E8DC8ED99397D2176D78BA79DM4PR12MB5295namp_ Content-Type: text/html; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable

[AMD Official Use Only]


Sure, that makes sense. Will send a v2

--

Regards,
Jay

From: Mark Yacoub <mar= kyacoub@chromium.org>
Sent: Monday, September 27, 2021 3:12 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
 
I'm not 100% sure but personally if I'm running th= e test on an AMD
device, I don't really care about seeing a subtest that I'm not
interested in.

On Mon, Sep 27, 2021 at 3:10 PM Pillai, Aurabindo
<Aurabindo.Pillai@amd.com> wrote:
>
> [AMD Official Use Only]
>
>
> Skipping using the continue statement has the side effect of this subt= est being absent from the list of subtests, when running the test executabl= e with paramter --list-subtest. Is that okay ?
>
> --
>
> Regards,
> Jay
> ________________________________
> From: Mark Yacoub <markyacoub@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.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
>
> so for the sake of consistency, I'm wondering if we can skip the test<= br> > 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 wit= h this patch. The other location where TEST_BO_TOOBIG is mentioned is in th= e same 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.= freedesktop.org>; Mark Yacoub <markyacoub@google.com>; Siqueira, R= odrigo <Rodrigo.Siqueira@amd.com>
> > Subject: Re: [igt-dev] [PATCH i-g-t] tests/kms_flip : skip subtes= t 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 hardwar= e. The
> > > aperture size calculation has an upper bound check which is = done through
> > > i915 specific IOCTL. Hence this part of subtest must be move= d
> > > 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 te= st fails on
> > > AMD, VKMS and VC4
> > >
> > > Signed-off-by: Aurabindo Pillai <aurabindo.pillai@amd.com= >
> > > ---
> > >  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(str= uct test_output *o, int *crtc_idxs,
> > >         /* 256 MB is= usually the maximum mappable aperture,
> > >          * (mak= e it 4x times that to ensure failure) */
> > >         if (o->fl= ags & 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));
> > >          &= nbsp;      bo_size =3D 4*gem_mappable_aperture_siz= e(drm_fd);
> > >          &= nbsp;      igt_require(bo_size < gem_global_ape= rture_size(drm_fd));
> > >         }
> > > --
> > > 2.30.2
> > >
--_000_DM4PR12MB52958E8DC8ED99397D2176D78BA79DM4PR12MB5295namp_--