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

[Liviu Dudau <liviu.dudau@arm.com>]

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@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@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@intel.com>
> > > > > Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> > > > > Cc: Liviu Dudau <liviu.dudau@arm.com>
> > > > > Cc: Sean Paul <sean@poorly.run>
> > > > > Cc: Eric Anholt <eric@anholt.net>
> > > > > Cc: Alex Deucher <alexander.deucher@amd.com>
> > > > > Cc: Dave Airlie <airlied@redhat.com>
> > > > > Signed-off-by: Daniel Vetter <daniel.vetter@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@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.

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.

> 
> > > 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?

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!  /
  ---------------
    ¯\_(ツ)_/¯
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev