[igt-dev] [PATCH] drm/doc: Make igts for cross-driver stuff mandatory

Thu Jan 17 15:59:49 UTC 2019

On Thu, Jan 17, 2019 at 3:54 PM Liviu Dudau <liviu.dudau at arm.com> wrote:
>
> On Thu, Jan 17, 2019 at 01:32:15PM +0100, Daniel Vetter wrote:
> > On Thu, Jan 17, 2019 at 1:26 PM Liviu Dudau <liviu.dudau at arm.com> wrote:
> > >
> > > On Thu, Jan 17, 2019 at 12:52:16PM +0100, Daniel Vetter wrote:
> > > > On Thu, Jan 17, 2019 at 12:38 PM Liviu Dudau <liviu.dudau at arm.com> wrote:
> > > > >
> > > > > On Wed, Jan 16, 2019 at 05:39:03PM +0100, Daniel Vetter wrote:
> > > > > > Compared to the RFC[1] no changes to the patch itself, but igt moved
> > > > > > forward a lot:
> > > > > >
> > > > > > - gitlab CI builds with: reduced configs/libraries, arm cross build
> > > > > >   and a sysroot build (should address all the build/cross platform
> > > > > >   concerns raised in the RFC discussions).
> > > > > >
> > > > > > - tests reorganized into subdirectories so that the i915-gem tests
> > > > > >   don't clog the main/shared tests directory anymore
> > > > > >
> > > > > > - quite a few more non-intel people contributing/reviewing/committing
> > > > > >   igt tests patches.
> > > > > >
> > > > > > I think this addresses all the concerns raised in the RFC discussions,
> > > > > > and assuming there's enough Acks and no new issues that pop up, we can
> > > > > > go ahead with this.
> > > > > >
> > > > > > 1: https://patchwork.kernel.org/patch/10648851/
> > > > > > Cc: Petri Latvala <petri.latvala at intel.com>
> > > > > > Cc: Arkadiusz Hiler <arkadiusz.hiler at intel.com>
> > > > > > Cc: Liviu Dudau <liviu.dudau at arm.com>
> > > > > > Cc: Sean Paul <sean at poorly.run>
> > > > > > Cc: Eric Anholt <eric at anholt.net>
> > > > > > Cc: Alex Deucher <alexander.deucher at amd.com>
> > > > > > Cc: Dave Airlie <airlied at redhat.com>
> > > > > > Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>
> > > > > > ---
> > > > > >  Documentation/gpu/drm-uapi.rst | 7 +++++++
> > > > > >  1 file changed, 7 insertions(+)
> > > > > >
> > > > > > diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst
> > > > > > index a752aa561ea4..413915d6b7d2 100644
> > > > > > --- a/Documentation/gpu/drm-uapi.rst
> > > > > > +++ b/Documentation/gpu/drm-uapi.rst
> > > > > > @@ -238,6 +238,13 @@ DRM specific patterns. Note that ENOTTY has the slightly unintuitive meaning of
> > > > > >  Testing and validation
> > > > > >  ======================
> > > > > >
> > > > > > +Testing Requirements for userspace API
> > > > > > +--------------------------------------
> > > > > > +
> > > > > > +New cross-driver userspace interface extensions, like new IOCTL, new KMS
> > > > > > +properties, new files in sysfs or anything else that constitutes an API change
> > > > > > +need to have driver-agnostic testcases in IGT for that feature.
> > > > >
> > > > > From an aspirational point of view I am fine with this and you can have
> > > > > my Acked-by: Liviu Dudau <liviu.dudau at arm.com>.
> > > > >
> > > > > From a practical point of view I would like to see a matrix of KMS APIs
> > > > > that are being validated and the drivers that have been tested. Otherwise,
> > > > > the next person that comes and tries to add a new IOCTL, KMS property or new
> > > > > file in sysfs is going to discover that he has subscribed to a much bigger
> > > > > task of getting enough KMS drivers testable in the first place.
> > > >
> > > > This is what the _new_ features is about, no expectation to write
> > > > tests for all the existing stuff.
> > >
> > > Yeah, but if the "tests for all the existing stuff" doesn't exist, your
> > > _new_ feature tests are not going to mean much, doesn't it?
> >
> > Why? There's still good test coverage for the new stuff at least. And
> > eventually someone gets bored and fills in the gaps. This is how we've
> > created all the current igt testcases, so it works - mandatory igt for
> > new features is the rule for anything intel does since well over 5
> > years.
>
> Because if you can't test basic stuff with igt how did you test your new
> feature? What I'm saying is: if developer X is having to use igt for
> a new feature then he is either: a) expecting that basic testing works for
> his driver (or he has bootstrapping isssues) or that b) if the feature
> doesn't get tested on driver A because that driver doesn't even have basic
> tests passing for it then the feature is acceptable. And it will be helpful
> to publish the list of igt tests that pass for each driver in KMS.

I think this is an entirely different discussion, something along the
lines of "new drivers/new features implemented on existing drivers"
must come with igt test results to prove it's working. Not what I'm
proposing here at all. It would be a good discussion to have, but a
different one I think.

The proposal here is only for new uapi: Thus far the requirement is
"driver + userspace", not it would be "driver + userspace + igt, if
generic feature".

> Alternatively, you need to explain better what "driver-agnostic testcase" is
> (my reading: a testcase that can be run on any driver). Or point to some igt
> testcases that are considered driver-agnostic and known to run on different
> (read: non-x86) platforms.

If this is just about clarifying "driver-agnostic testcase" then that
boils down to drm_open(DRIVER_ANY). Plus reasonable engineering calls
on what exactly agnostic means, because in practice I expect that
"works on first driver, doesn't on 2nd" is fairly common, no matter
which driver is the first one, and which is the second.

Now for a list of guaranteed-to-be-generic tests, I'm slowly pushing
vkms forward to get there. But it's going to take a lot of internships
to make vkms featureful enough to be useful in such a role.

If you're just looking for i915 test results, we publish them all. And
afaik no one else does so, so not really possible to point at anything
else.

> > > > Although I think there's not really
> > > > any big gaps in igt anymore, we do have at least some (rather rough
> > > > and coarse in some case) test coverage for everything I think.
> > >
> > > I would like to see some proof of that in the form of ....
> > >
> > >
> > > > Should this be clarified further?
> > >
> > > an URL that points to a page similar to Mesa's GL supported features
> > > would be nice to add here, from my point of view.
> >
> > There isn't even a list of upstream drm features ...
>
> Yeah, I know that! :)
>
> > how exactly should that look like?
>
> What about a list of igt tests then? OR we could put in the igt TODO file
> the kind of tests we would like to have at some moment?

I'm still not clear what your concern is and what you want to achieve
here. There's lots of good ideas we can push for around igt, so
arbitrarily filling a todo file isn't hard. But I also don't see the
point (we do have some todo items btw on gitlab I think).
-Daniel

>
> Best regards,
> Liviu
>
>
> > Only thing I can tell you is that we had huge
> > amounts of todo for missing igt tests, and afaik we've worked them all
> > down. We = intel here. It's not perfect, but good enough that we've
> > essentially stopped doing any validation except by running igt. Of
> > course you can always write more tests, same way you can always
> > bikeshed a patch a few more times. Eventually there's dimishing
> > returns.
> > -Daniel
> >
> > > Best regards,
> > > Liviu
> > >
> > > > -Daniel
> > > >
> > > > >
> > > > > Best regards,
> > > > > Liviu
> > > > >
> > > > >
> > > > > > +
> > > > > >  Validating changes with IGT
> > > > > >  ---------------------------
> > > > > >
> > > > > > --
> > > > > > 2.20.1
> > > > > >
> > > > >
> > > >
> > > > --
> > > > Daniel Vetter
> > > > Software Engineer, Intel Corporation
> > > > +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> > >
> > > --
> > > ====================
> > > | I would like to |
> > > | fix the world,  |
> > > | but they're not |
> > > | giving me the   |
> > >  \ source code!  /
> > >   ---------------
> > >     ¯\_(ツ)_/¯
> >
> >
> >
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > +41 (0) 79 365 57 48 - http://blog.ffwll.ch
>
> --
> ====================
> | I would like to |
> | fix the world,  |
> | but they're not |
> | giving me the   |
>  \ source code!  /
>   ---------------
>     ¯\_(ツ)_/¯

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch