[PATCH] RFC: Make igts for cross-driver stuff mandatory?
Daniel Vetter
daniel.vetter at ffwll.ch
Thu Oct 25 10:31:12 UTC 2018
On Thu, Oct 25, 2018 at 11:58 AM Liviu Dudau <liviu at dudau.co.uk> wrote:
> On Fri, Oct 19, 2018 at 10:50:49AM +0200, Daniel Vetter wrote:
> > Hi all,
>
> Hi,
>
> (Replying from my personal address as the work email seems to have let
> this one go to /dev/null)
>
> >
> > This is just to collect feedback on this idea, and see whether the
> > overall dri-devel community stands on all this. I think the past few
> > cross-vendor uapi extensions all came with igts attached, and
> > personally I think there's lots of value in having them: A
> > cross-vendor interface isn't useful if every driver implements it
> > slightly differently.
>
> I would like to also get some clarification on where we are standing on
> "tests vs 'real code'" stanza. Does making igt tests mandatory replace
> the need for 'real code' or does it add to the list of requirements? If
> the later, then I think the bar rises in terms of showing igts'
> usefulness / benefits.
It would be in addition. The "real code" userspace is for validating
the overall design. Igt (and unit tests) are more for checking all the
details, error handling, and having a regression test suite going
forward.
> > I think there's 2 questions here:
> >
> > - Do we want to make such testcases mandatory?
>
> I'm a bit reluctant to make it so by fiat. I think that showing the
> usefulness of having igts tests to newcomers (by adding with this patch
> some more information about why IGT is a good place to add your testing to)
> and getting more mature drivers to get tested under IGT on a regular basis
> would make adoption of IGT for testing a community standard.
There's 2 motivations here for me:
- Most of the generic features in the past 1-2 did come with igts, but
sometimes those igts have been treated as 2nd class and forgotten. I
think it's at least the right time to discuss whether we should make
them an integral part of uapi development.
- The other bit is that our intel CI is catching a lot of regressions,
and people have started to cc: intel-gfx to get their non-intel
patches validated too. So all these igts we have do seem to provide
real value in keeping things working.
> > - If yes, are we there yet, or is there something crucially missing
> > still?
>
> I'm just getting back into IGT by refreshing the writeback patches, but
> by looking at the commit log I get the impression that there aren't that
> many patches that target drivers other than Intel's. Are all the non-Intel
> patches so generic that one doesn't need to specify a target driver for
> those changes (in which case great, but then why is Intel's so different?),
> or are the others not bothered with IGT support?
So this discussion isn't about igt overall, but just about
cross-driver features. The tests we have in igt for intel are a lot
more:
- We also validate all the intel-specific bits with igts.
- We validate all the interactions between generic stuff and
intel-specific uapi with igts.
- We validate hw features with igt.
And finally we've made igts mandatory for all things intel more than 5
years ago. So there's considerable head start.
The other side is that I'm only suggesting that we make igts mandatory
for cross-driver interfaces. Naturally the tests for those aren't
aimed at a specific driver, but use the DRIVER_ANY flag. And the work
needed to make that work on a specific driver is usually rather small
- e.g. vmwgfx (which doesn't support GEM at all) required very small
changes to get things going. Now we do have some tests for other
drivers (vc4 and a few amdgpu), but not anything else. So I'd say the
lack of non-intel targetted patches is a good thing.
> At the moment I'm a bit on the fence on this. Not having spent too much
> time with IGT in the last 6 months, I'm probably closer to a newcomer in
> my attitude towards IGT and at the moment I'm not clear on how to answer the
> "Why?" and "What is in it for me?" questions.
Well all the usual reasons for testing:
- Only way to make sure different implementations are working correct.
- Only real way to make sure regressions are caught before everything
is on fire.
Of course this gets a lot better if there's also CI running them
(which we do for intel, and are open to anyone submitting stuff to
it), but just having the tests is already a big step.
Cheers, Daniel
> Best regards,
> Liviu
>
> >
> > And of course there's a bunch of details to figure out. Like we
> > probably want to also recommend the selftests/unit-tests in
> > drivers/gpu/drm/selftest, since fairly often that's a much more
> > effective approach to checking the details than from userspace.
> >
> > Feedback and thoughts very much appreciated.
> >
> > Cheers, Daniel
> > ---
> > 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 4b4bf2c5eac5..91cf6e4b6303 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.
> > +
> > Validating changes with IGT
> > ---------------------------
> >
> > --
> > 2.19.1
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> /`\
> / : |
> _.._ | '/
> /` \ | /
> | .-._ '-"` (
> |_/ / o o\
> | == () ==
> \ -- / ______________________________________
> / ---<_ ________| |_______
> | \\ \ | I would like to fix the world but | /
> | | \\__ \ | no one gives me the source code. | /
> / ; |.__) / |______________________________________| \
> (_/.-. ; /__________) (_________\
> { `| \_/
> '-\ / |
> | / |
> / \ '-.
> \__|-----'
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
More information about the amd-gfx
mailing list