[Intel-gfx] [PATCH igt] igt/gem_softpin: Remove false dependencies on esoteric features

[Belgaumkar, Vinay]

Hi Dan,

I was not dismissing Chris's patch, in fact, I was trying to see if we could keep the combined userptr/softpin tests in a separate file. Apologize if it came across as dismissive.

I agree that it is better to keep the tests simple and effective, however, we thought it would be worthwhile to test the stack as well at the time. We used these tests during the development phase to find/reproduce some scenarios. We can maintain these type of tests in our internal framework if they are not appropriate here.  

Thanks,
Vinay.

-----Original Message-----
From: Daniel Vetter [mailto:daniel.vetter at ffwll.ch] On Behalf Of Daniel Vetter
Sent: Tuesday, January 26, 2016 2:06 AM
To: Barnes, Jesse
Cc: Daniel Vetter; Belgaumkar, Vinay; Chris Wilson; intel-gfx at lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH igt] igt/gem_softpin: Remove false dependencies on esoteric features

On Mon, Jan 25, 2016 at 12:04:43PM -0800, Jesse Barnes wrote:
> On 01/21/2016 12:08 AM, Daniel Vetter wrote:
> > On Wed, Jan 20, 2016 at 06:49:49PM +0000, Belgaumkar, Vinay wrote:
> >> Hi Chris,
> >>     These tests were developed for testing buffered SVM(using userptr
> >>     and soft pinning API). I think Dan wanted me to rename the tests to
> >>     gem_softpin, since they were being checked in as tests which
> >>     validated the softpin kernel patches. Can we rename the existing
> >>     tests to gem_buffered_svm or something similar, and then push any
> >>     targeted softpin only tests as gem_softpin? We were hoping to use
> >>     these userptr+softpin tests as GFT tests for SVM(Android) as well,
> >>     since buffered SVM is POR for BXT Android. 
> > 
> > I agree with Chris, there's no need to unecessarily mix together features.
> > When the api is designed in an orthogonal way, so should be the testing.
> > i915.ko is already a mindboggling complex beast, no need to make our 
> > lives harder by making the tests use features that aren't strictly needed.
> > 
> > In the end applications and UMDs will of course use all these 
> > features together, but that's why we do integration testing on top 
> > of just running igt.
> > 
> > Can you please review Chris' patch?
> 
> So what's the actual request here?  Chris rewrote Vinay's test, but 
> does it cover all the same stuff Vinay did?  If not, it would be nice 
> to include those, maybe in a separate file, since Vinay did work with 
> lots of people to make sure the coverage was complete for the SVM use 
> cases...  I definitely like the sound of the new stuff Chris added 
> though; no-reloc in particular is an important use case for upcoming 
> APIs.

Afaict Chris' patch doesn't reduce the coverage of the existing/merged testcase in any way at all, but makes it a bit simpler to ensure we test features more orthogonally. I didn't check in detail, but the combination tests should still be there (and would be something reviewers can/should check).

I was only replying to (what seemed to me) Vinay's outright dismissal of Chris' patch, since I think Chris' idea to make feature testing as orthogonal as possible is sound. Details would be up to the review, like I said.

Another topic related to this is whether igt should do integration testing of the entire stack (i.e. combining all the features exactly as the UMD would use them). In my opinion it's better to do such integration testing with the actual UMD and UMD-specific testsuites, and leave testing of corner-cases and low-level functionality to igt. That of course doesn't exclude igt tests that exercise corner-cases when different features interact.

Thanks, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch