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

[Chris Wilson]

On Tue, Jan 26, 2016 at 11:06:16AM +0100, Daniel Vetter wrote:
> 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).

It did not, quite the opposite. It also removed *buggy* code. If that is
the quality of the userspace implementation, it needs some urgent review.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre