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 D80426E513 for ; Tue, 10 Mar 2020 03:27:44 +0000 (UTC) Date: Mon, 09 Mar 2020 20:27:43 -0700 Message-ID: <87mu8o99e8.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" In-Reply-To: <20200207134527.17205-2-ramalingam.c@intel.com> References: <20200207134527.17205-1-ramalingam.c@intel.com> <20200207134527.17205-2-ramalingam.c@intel.com> MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Subject: Re: [igt-dev] [PATCH i-g-t v2 2/2] tests/i915_pm_rpm: use device coherent mapping instead of mmap_gtt 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: Ashutosh Dixit Cc: igt-dev List-ID: On Fri, 07 Feb 2020 05:45:27 -0800, Ramalingam C wrote: > > Since on new discrete GPUs we dont have the mappable aperture, if that > is acceptable for the test purpose, we should use GEM_MMAP_OFFSET. > Hence using gem_mmap_device_coherent() which wraps the mmap options in > the order of gem_mmap_offset / gem_mmap / mmap_gtt > > Incase of fencing/gtt related tests, we mandate the existance of the > mmap_gtt. > > v2: > 3 tests are skipped based on mappable aperture and tiling support > > Signed-off-by: Ramalingam C > --- > tests/i915/i915_pm_rpm.c | 26 +++++++++++++++++++------- > 1 file changed, 19 insertions(+), 7 deletions(-) > > diff --git a/tests/i915/i915_pm_rpm.c b/tests/i915/i915_pm_rpm.c > index 3d7343240802..5ad2d833d53b 100644 > --- a/tests/i915/i915_pm_rpm.c > +++ b/tests/i915/i915_pm_rpm.c > @@ -1535,7 +1535,7 @@ static void fill_igt_fb(struct igt_fb *fb, uint32_t color) > int i; > uint32_t *ptr; > > - ptr = gem_mmap__gtt(drm_fd, fb->gem_handle, fb->size, PROT_WRITE); > + ptr = gem_mmap__device_coherent(drm_fd, fb->gem_handle, 0, fb->size, PROT_WRITE); My earlier comment on this is still valid: even though fill_igt_fb() is called with tiling set, I think it is still ok > to do this since all we are doing is filling a solid color for which tiling is immaterial. cursor_subtest: fix set_tiling() call. > for (i = 0; i < fb->size/sizeof(uint32_t); i++) > ptr[i] = color; > igt_assert(munmap(ptr, fb->size) == 0); > @@ -2016,16 +2016,20 @@ igt_main_args("", long_options, help_str, opt_handler, NULL) > /* GEM */ > igt_subtest("gem-mmap-cpu") > gem_mmap_subtest(false); > - igt_subtest("gem-mmap-gtt") > + igt_subtest("gem-mmap-gtt") { > + gem_require_mappable_ggtt(drm_fd); OK. > gem_mmap_subtest(true); > + } > igt_subtest("gem-pread") > gem_pread_subtest(); > igt_subtest("gem-execbuf") > gem_execbuf_subtest(); > igt_subtest("gem-idle") > gem_idle_subtest(); > - igt_subtest("gem-evict-pwrite") > + igt_subtest("gem-evict-pwrite") { > + gem_require_mappable_ggtt(drm_fd); Sorry I think I changed my mind on this: for this one it is possible to set num_trash_bos to some constant value when we don't have aperture and use device_coherent. I am not completely sure if there's value in that but maybe let's just go ahead and do it. > gem_evict_pwrite_subtest(); > + } > > /* Planes and cursors */ > igt_subtest("cursor") > @@ -2058,10 +2062,14 @@ igt_main_args("", long_options, help_str, opt_handler, NULL) > dpms_mode_unset_subtest(SCREEN_TYPE_LPSP); > igt_subtest("dpms-mode-unset-non-lpsp") > dpms_mode_unset_subtest(SCREEN_TYPE_NON_LPSP); > - igt_subtest("fences") > + igt_subtest("fences") { > + gem_require_mappable_ggtt(drm_fd); > fences_subtest(false); Now I am thinking if fences_subtest() should be skipped or we just fix tiling and mmap and retain it? > - igt_subtest("fences-dpms") > + } > + igt_subtest("fences-dpms") { > + gem_require_mappable_ggtt(drm_fd); > fences_subtest(true); Ditto. > + } > > /* Modeset stress */ > igt_subtest("modeset-lpsp-stress") > @@ -2102,10 +2110,14 @@ igt_main_args("", long_options, help_str, opt_handler, NULL) > gem_execbuf_stress_subtest(rounds, WAIT_STATUS | WAIT_EXTRA); > > /* power-wake reference tests */ > - igt_subtest("pm-tiling") > + igt_subtest("pm-tiling") { > + gem_require_mappable_ggtt(drm_fd); > pm_test_tiling(); Maybe fix and retain? > - igt_subtest("pm-caching") > + } > + igt_subtest("pm-caching") { > + gem_require_mappable_ggtt(drm_fd); Again maybe fix and retain? _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev