From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by gabe.freedesktop.org (Postfix) with ESMTPS id 7472A6E8E2 for ; Fri, 13 Aug 2021 22:32:08 +0000 (UTC) Date: Fri, 13 Aug 2021 15:18:56 -0700 Message-ID: <87y295gf8v.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" In-Reply-To: <20210813074703.18778-5-zbigniew.kempczynski@intel.com> References: <20210813074703.18778-1-zbigniew.kempczynski@intel.com> <20210813074703.18778-5-zbigniew.kempczynski@intel.com> MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=ISO-8859-2 Content-Transfer-Encoding: quoted-printable Subject: Re: [igt-dev] [PATCH i-g-t 4/9] tests/gem_exec_big: Skip relocation part List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: igt-dev-bounces@lists.freedesktop.org Sender: "igt-dev" To: Zbigniew =?ISO-8859-2?Q?Kempczy=F1ski?= Cc: igt-dev@lists.freedesktop.org, Andrzej Turko , Petri Latvala List-ID: On Fri, 13 Aug 2021 00:46:58 -0700, Zbigniew Kempczy=F1ski wrote: > > When running on platforms without relocation support, verification > of their correctness should be skipped. What remains is exercising > submission of large batches. Not sure if there's much point to this file in the absence of relocations since all it would do is bind the batch buffer in the gtt but the first instruction is already MI_BATCH_BUFFER_END. So the patch could possibly just be: igt_require(gem_has_relocations(i915)); as was in the earlier version of the patch. > @@ -70,7 +71,7 @@ static void exec1(int fd, uint32_t handle, uint64_t rel= oc_ofs, unsigned flags, c > gem_reloc[0].presumed_offset =3D 0; > > gem_exec[0].handle =3D handle; > - gem_exec[0].relocation_count =3D 1; > + gem_exec[0].relocation_count =3D has_relocs ? 1 : 0; > gem_exec[0].relocs_ptr =3D to_user_pointer(gem_reloc); > gem_exec[0].alignment =3D 0; > gem_exec[0].offset =3D 0; > @@ -100,6 +101,9 @@ static void exec1(int fd, uint32_t handle, uint64_t r= eloc_ofs, unsigned flags, c > igt_warn_on(gem_reloc[0].presumed_offset =3D=3D -1); > gem_set_domain(fd, gem_exec[0].handle, I915_GEM_DOMAIN_WC, 0); > > + if (!has_relocs) > + return; > + I would move this 2 lines above, right after gem_execbuf(), so that there's no confusion about verifying presumed_offset etc. in the absence of relocations. Otherwise this is: Reviewed-by: Ashutosh Dixit x