From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by gabe.freedesktop.org (Postfix) with ESMTPS id 2195B6FE9A for ; Thu, 23 Jan 2020 23:07:01 +0000 (UTC) Date: Thu, 23 Jan 2020 15:07:00 -0800 Message-ID: <87muaddb6z.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" In-Reply-To: <20200122233326.29131-1-vinay.belgaumkar@intel.com> References: <20200122233326.29131-1-vinay.belgaumkar@intel.com> MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Subject: Re: [igt-dev] [PATCH] tests/i915/gem_mmap_wc: Update test to use mmap_wc IOCTL List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: igt-dev-bounces@lists.freedesktop.org Sender: "igt-dev" To: Vinay Belgaumkar Cc: igt-dev@lists.freedesktop.org List-ID: On Wed, 22 Jan 2020 15:33:26 -0800, Vinay Belgaumkar wrote: > > Purpose of this test is to validate the mmap_wc ioctl. After recent > additions to library wrappers, the local wrapper ended up calling > the unintended mmap. Also check for presence of mappable GGTT before > testing it. > > Signed-off-by: Vinay Belgaumkar > Cc: Antonio Argenziano > Cc: Chris Wilson > --- > tests/i915/gem_mmap_wc.c | 59 ++++++++++++++++++++++++---------------- > 1 file changed, 35 insertions(+), 24 deletions(-) > > diff --git a/tests/i915/gem_mmap_wc.c b/tests/i915/gem_mmap_wc.c > index 375a9b50..27334b6d 100644 > --- a/tests/i915/gem_mmap_wc.c > +++ b/tests/i915/gem_mmap_wc.c > @@ -51,19 +51,36 @@ struct local_i915_gem_mmap_v2 { > > static int OBJECT_SIZE = 16*1024*1024; > > +static int mmap_ioctl(int i915, struct drm_i915_gem_mmap *arg) > +{ > + int err = 0; > + > + if (igt_ioctl(i915, DRM_IOCTL_I915_GEM_MMAP, arg)) > + err = -errno; > + > + errno = 0; > + return err; > +} > + > /* > * Local WC mmap wrapper. This is used to make sure we go through > * the GEM_MMAP IOCTL. > * */ > static void * > -local_gem_mmap__wc(int fd, uint32_t handle, uint64_t offset, uint64_t size, unsigned prot) > +local_gem_mmap__wc(int fd, uint32_t handle, uint64_t offset, uint64_t size) > { > - void *ptr; > > - ptr = __gem_mmap__wc(fd, handle, 0, OBJECT_SIZE, PROT_READ | PROT_WRITE); > - igt_assert(ptr); > + struct drm_i915_gem_mmap arg = { > + .handle = handle, > + .offset = offset, > + .size = size, > + .flags = I915_MMAP_WC, > + }; > > - return ptr; > + igt_assert_eq(mmap_ioctl(fd, &arg), 0); Sorry maybe I am missing something but could you please explain which unintended mmap is being called, at least at this point it seems __gem_mmap__wc() is also calling the DRM_IOCTL_I915_GEM_MMAP ioctl? I agree the protection flags are unused and can be removed. > @@ -593,14 +600,18 @@ igt_main > run_without_prefault(fd, test_read); > igt_subtest("write-no-prefault") > run_without_prefault(fd, test_write); > - igt_subtest("write-gtt-no-prefault") > + igt_subtest("write-gtt-no-prefault") { > + gem_require_mappable_ggtt(fd); > run_without_prefault(fd, test_write_gtt); test_write_gtt() is actually named incorrectly, it is actually setting up a WC mapping so no need to skip? > + } > igt_subtest("write-cpu-read-wc") > test_write_cpu_read_wc(fd, 1); > igt_subtest("write-cpu-read-wc-unflushed") > test_write_cpu_read_wc(fd, 0); > - igt_subtest("write-gtt-read-wc") > + igt_subtest("write-gtt-read-wc") { > + gem_require_mappable_ggtt(fd); > test_write_gtt_read_wc(fd); Here it is ok to skip I think. _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev