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

Tue Jan 22 16:28:19 UTC 2019

On Tue, Jan 22, 2019 at 5:11 PM Sean Paul <sean at poorly.run> wrote:
> On Tue, Jan 22, 2019 at 04:17:25PM +0100, Daniel Vetter wrote:
> > On Tue, Jan 22, 2019 at 4:08 PM Brian Starkey <Brian.Starkey at arm.com> wrote:
> > >
> > > On Tue, Jan 22, 2019 at 03:03:59PM +0100, Daniel Vetter wrote:
> > > > On Tue, Jan 22, 2019 at 2:27 PM Brian Starkey <Brian.Starkey at arm.com> wrote:
> > >
>
> /snip
>
> >
> > > > > > > > That seems a bit out of order to me. It would be like me saying "all
> > > > > > > > KMS drivers must use Arm's test suite, which uses writeback and pixel
> > > > > > > > checking", and you'd be in a pickle because you don't have writeback.
> > > > > > > >
> > > > > > > > In a similar vein, I remember having to fix igt on devices which
> > > > > > > > didn't have cursor planes, before I could even think about writing
> > > > > > > > tests.
> > > > > > > >
> > > > > > > > If you can prove that issues like these are all resolved now in igt,
> > > > > > > > then great! Otherwise, I don't think igt is "ready" to be used as a
> > > > > > > > requirement for merging KMS kernel API.
> > > > > >
> > > > > > With a night's worth of sleep, this here is what annoys me - you
> > > > > > demand I "prove" that igt works everywhere. That's not realistic.
> > > > > > Intel is not going to pay the bill to get a CI farm for every drm
> > > > > > driver existing up&running, including fixing all the bugs that will
> > > > > > uncover (both in igt and even more so in drivers). Especially not if
> > > > > > mere mortals can't even buy the hardware. Nor is anyone else going to
> > > > > > do that. If there are some fundamental overall issues with igt then
> > > > > > I'm ofc happy to get them addressed (like the build issues raised a
> > > > > > few months ago).
> > > > >
> > > > > I'm really not asking you to. I'm sorry that I've annoyed you, and I'm
> > > > > not being deliberately awkward.
> > > > >
> > > > > I genuinely don't know what the current state of "not-i915" testing is
> > > > > in igt. Do you really not think it's a good idea to have that known
> > > > > and documented before you make igt a mandatory part of the DRM tree?
> > > > >
> > > > > Sure, there's commits from non-intel folks - heck there's even a few
> > > > > from me. But I can tell you right away that doesn't mean that igt
> > > > > works in any meaningful way on my platform. Does it work well enough
> > > > > for me to add new test cases? Honestly I don't know.
> > > > >
> > > > > Maybe you can suggest some suites which are expected to already be
> > > > > fully generic and should run anywhere which we can use as
> > > > > confirmation? To me, having some reasonable subset of the active
> > > > > driver devs building and running that on their platforms and reporting
> > > > > back before you merge this patch wouldn't be unreasonable or
> > > > > outlandish.
> > > >
> > > > So my Grand Plan is to get vkms up to par, and even get that
> > > > integrated into igt and drm autobuilds we'll run on gitlab CI. But
> > > > vkms is still a few internships away from being useful, atm it has one
> > > > primary plane and one non-plane cursor (it's not even a universal
> > > > cursor plane yet). Will give you _lots_ of skips.
> > > >
> > > > But I guess we could fairly easily add vkms as one of the machines
> > > > we're testing to intel's CI farm, which means every patch and every
> > > > commit gets at least tested on non-i915, and you'll get tons of data
> > > > of which tests blow up, and even more which will just skip (due to
> > > > lack of features on vkms' side). We did have that situation a while
> > > > back with amdgpu and kbl-g (the intel+amd gpu's chip), but only by
> > > > accident (we do still run i915+amdgpu buffer sharing tests, since we
> > > > care about that). Took us a while to realize that igt preferred
> > > > running on amdgpu somehow. Running random non-i915 chips in our CI
> > > > farm isn't something that'll happen, but vkms should not be hard to
> > > > set up and get going.
> > > >
> > > > Would that be useful if we get that done first?
> > >
> > > Sure, that would be useful. I'm not even saying I want it in CI
> > > though, just for some people who aren't @intel.com to try it on their
> > > HW and say "works for me".
> >
> > There's a pretty huge difference between "I want acks from other
> > people" and "you need to prove this works everywhere" ... The prove
> > thing pretty much requires CI, or it's imo not really anywhere near
> > proving.
>
> My $0.02 since my interests are:
> - Make sure all of our (CrOS) platforms are well-tested, and
> - Ensure new features can be merged upstream without too much overhead
>
> My initial reaction was the same as Brian's. I've "brought up" igt on a non-i915
> device and it was a bit of a horror show, but I managed to get the test I wanted
> running and lit the device on fire afterwards. Since then at least the toolchain
> has been fixed and igt builds, so that's nice.
>
> That said, the counterarguments presented have been pretty hypothetical. I get
> that crc is a pain for ARM (Arm? arm? aRm?), but it seems like a worthwhile
> investment to get it working so that they can leverage igt. Even if crc
> generation is laggy. If, hypothetically, ARM has a new feature that requires an
> igt test using crc, it seems like a good time to get crc working so the new
> feature can be proven out.
>
> I think we should at least have a solid plan for crc generation on ARM (that ARM
> is happy with) before requiring igt across the board. That can be translated to
> a TODO, and perhaps someone will be interested in picking that up.

One thing to keep in mind is that this is only about _new_ features.
In my experience (and I killed a fair share of internal test suites)
that's the only way to get out of the chicken&egg problem if you have
a large project without a test suite. The second you start requiring
that the tests work even on old stuff, and have good coverage on
existing features your test suite is dead:
- no one is going to abandon their investement into existing test suites
- no one is going to be able to afford rewriting their existing tests in one go
- no one is going to fix all the bugs this will uncover

So requiring that igt works for everyone kills this idea of having a
shared test suite for kms. Which I think wouldn't be great, but I'm
also not going to push for this if this is how the community overall
feels like.

The only thing that works is limiting yourself on new stuff only, and
pretending the the existing dumpster fire of untested features, broken
drivers and not-generic enough tests doesn't exist. And I think igt
for non-i915 drivers is in a much better shape than igt was 5+ years
ago when we made it mandatory for anything intel does in the kernel.
Over the course of the past 5 years we've managed to mostly address
the gaps in igt from an intel pov. I think that's about the same time
I expect anyone else switching over to igt will need to fully get rid
of their existing test suite and integrated it all into igt. So again:
Requiring that work first just means we perpetually postpone having a
shared test suite by 5 years, forever (there's going to be new drivers
which then again wont work with igt). And most likely it won't happen,
because there's no incentive to switch over at all.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch