Re: [PATCH i-g-t 4/3] tests/gem_ctx_param_basic: Expand ctx_param tests

From: David Weinehall <david.weinehall@linux.intel.com>
To: Jesse Barnes <jbarnes@virtuousgeek.org>
Cc: intel-gfx <intel-gfx@lists.freedesktop.org>
Subject: Re: [PATCH i-g-t 4/3] tests/gem_ctx_param_basic: Expand ctx_param tests
Date: Mon, 10 Aug 2015 17:15:44 +0300	[thread overview]
Message-ID: <20150810141544.GJ6150@boom> (raw)
In-Reply-To: <55C3D2AB.10802@virtuousgeek.org>

On Thu, Aug 06, 2015 at 02:33:31PM -0700, Jesse Barnes wrote:
> On 08/06/2015 02:30 PM, Daniel Vetter wrote:
> > On Fri, May 29, 2015 at 09:52:52AM +0200, Daniel Vetter wrote:
> >> On Thu, May 28, 2015 at 05:53:17PM +0300, David Weinehall wrote:
> >>> On Wed, May 27, 2015 at 01:32:10PM +0200, Daniel Vetter wrote:
> >>>> A simple functional test here which does:
> >>>> a) an execbuf with just 1 batch. With full ppgtt you should get that one
> >>>> at offset 0. If not, skip the testcase.
> >>>> b) set the NO_ZEROMAP property.
> >>>> c) re-run the same batch, assert that now the buffer is relocated to
> >>>> something non-0.
> >>>>
> >>>> Just to make sure we have a bare minimal testcase to make sure we don't
> >>>> break this.
> >>>
> >>> Maybe this should be added to another test rather than here?  This test
> >>> is described as a:
> >>>
> >>> "Basic test for context set/get param input validation."
> >>>
> >>> Somehow I feel that testing whether the *functionality* is correct
> >>> does not belong in this test, but rather in some test case that's
> >>> already related to execbufs, or even a dedicated test case.
> >>>
> >>> But that might be over-engineering.  Opinions?
> >>
> >> Yeah separate testcase would fit better, agreed.
> > 
> > Update version of this patch is still missing. I'll need to revert the
> > kernel side if this one doesn't show up soonish.
> > 
> > Also you're breaking the invalid-flags testcase (did you bother to run
> > them all and check for regressions?) which Jesse spotted, and with the new
> > basic set this will be a P1 "I'm going to block everything" bug.
> 
> We really need man pages for new ioctls as well in libdrm.

Hmmm, this isn't a new ioctl, just a context parameter that can be
set/queried using a pre-existing ioctl, but I can modify the existing
manual page (if there is one?) to include information about the new
parameter.

Kind regards, David
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx