From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 1/3] tests/gem_userptr_blits: Expanded userptr test cases Date: Wed, 23 Apr 2014 19:53:38 +0200 Message-ID: <20140423175338.GD10722@phenom.ffwll.local> References: <1393431465-31630-1-git-send-email-tvrtko.ursulin@linux.intel.com> <1395227586-18623-1-git-send-email-tvrtko.ursulin@linux.intel.com> <1395227586-18623-2-git-send-email-tvrtko.ursulin@linux.intel.com> <20140418171059.GA30178@bdvolkin-ubuntu-desktop> <5357C134.7000700@linux.intel.com> <20140423153227.GB8947@bdvolkin-ubuntu-desktop> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-ee0-f54.google.com (mail-ee0-f54.google.com [74.125.83.54]) by gabe.freedesktop.org (Postfix) with ESMTP id 2CE7F6E26F for ; Wed, 23 Apr 2014 10:53:43 -0700 (PDT) Received: by mail-ee0-f54.google.com with SMTP id d49so1052309eek.13 for ; Wed, 23 Apr 2014 10:53:42 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20140423153227.GB8947@bdvolkin-ubuntu-desktop> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: "Volkin, Bradley D" Cc: "Intel-gfx@lists.freedesktop.org" List-Id: intel-gfx@lists.freedesktop.org On Wed, Apr 23, 2014 at 08:32:27AM -0700, Volkin, Bradley D wrote: > On Wed, Apr 23, 2014 at 06:33:40AM -0700, Tvrtko Ursulin wrote: > > > > On 04/18/2014 06:10 PM, Volkin, Bradley D wrote: > > > On Wed, Mar 19, 2014 at 04:13:04AM -0700, Tvrtko Ursulin wrote: > > >> From: Tvrtko Ursulin > > >> > > >> A set of userptr test cases to support the new feature. > > >> > > >> For the eviction and swapping stress testing I have extracted > > >> some common behaviour from gem_evict_everything and made both > > >> test cases use it to avoid duplicating the code. > > >> > > >> Both unsynchronized and synchronized userptr objects are > > >> tested but the latter set of tests will be skipped if kernel > > >> is compiled without MMU_NOTIFIERS. > > >> > > >> Also, with 32-bit userspace swapping tests are skipped if > > >> the system has a lot more RAM than process address space. > > >> Forking swapping tests are not skipped since they can still > > >> trigger swapping by cumulative effect. > > >> > > >> v2: > > >> * Fixed dmabuf test. > > >> * Added test for rejecting read-only. > > >> * Fixed ioctl detection for latest kernel patch. > > >> > > >> v3: > > >> * Updated copy() for Gen8+. > > >> * Fixed ioctl detection on kernels without MMU_NOTIFIERs. > > >> > > >> Signed-off-by: Tvrtko Ursulin > > > > > > A number of the comments I made on patch 3 apply here as well. > > > The sizeof(linear) thing is more prevalent in this test, though > > > it looks like linear is at least used. Other than those comments > > > this looks good to me. > > > > Believe it or not that sizeof(linear) "idiom" I inherited from other > > blitter tests. Personally I don't care one way or another. But since it > > makes sense to get rid of it for the benchmark part, perhaps I should > > change it here as well to be consistent. How strongly do you feel > > strongly about this? > > I think changing it would be slightly more readable, but if it's > consistent with other blit tests then I don't feel too strongly > about it. In fact, consistency with the other tests might be the > better approach. I'm fine with whichever approach you prefer. Some of the igt tests are so Gross Hacks that justifying ugliness in new tests with consistency is ill-advised ;-) If you find some spare cycles to clean up existing tests that would be awesome, but I don't mind if they keep being ugly. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch