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

[Daniel Vetter]

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.

> > 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 ... how exactly
should that look like? 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