From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) by gabe.freedesktop.org (Postfix) with ESMTPS id 9B11089BAB for ; Tue, 28 Sep 2021 09:12:32 +0000 (UTC) Date: Tue, 28 Sep 2021 12:15:54 +0300 From: Petri Latvala Message-ID: References: <20210927160700.1570885-1-aurabindo.pillai@amd.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: 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: "Pillai, Aurabindo" Cc: Mark Yacoub , Development mailing list for IGT GPU Tools , Mark Yacoub , "Siqueira, Rodrigo" List-ID: On Mon, Sep 27, 2021 at 07:10:45PM +0000, Pillai, Aurabindo wrote: > Skipping using the continue statement has the side effect of this > subtest being absent from the list of subtests, when running the > test executable with paramter --list-subtest. Is that okay ? It's not okay btw. When you're not in an igt_fixture or an igt_subtest, you cannot use anything that touches the hardware or the running system in any way. -- Petri Latvala > > -- > > 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-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 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 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 = 4*gem_mappable_aperture_size(drm_fd); > > > igt_require(bo_size < gem_global_aperture_size(drm_fd)); > > > } > > > -- > > > 2.30.2 > > >