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

Mon Jan 28 14:03:54 UTC 2019

On Tue, Jan 22, 2019 at 05:28:19PM +0100, Daniel Vetter wrote:
> 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.

Sorry for resurecting this thread, I was on holiday for a week but I
want to keep my promise of keeping Daniel on an honest path as a DRM
co-maintainer:

Daniel, if you had that previous experience and if you know/knew that
"requiring that igt works for everyone would kill the idea", I think you
should have been more clear and forthcoming with the information and
also made it more explicit that you're trying to get *something* in place
and that means being forgiving with the past but less so with the future
APIs. It would have cut tremendously on the number of messages exchanged
between Arm-side and you.

Without being judgemental, from my side I'm seeing a disconnect between
what is being claimed that igt can do today and what I know I can extract
from igt on my driver. As a result, it makes me wonder if it is just the
Arm-developed drivers' problem or is it true for most of the non-PC drivers.
That was the reason for asking for results from other drivers: so that we
know where we are and plan for change. I obviously don't want to be left
behind but I also face the reality that being one of the first to bend igt
to work on my platforms requires effort that I need to justify to my management.
And I can only do so when I have information readily at my disposal so that
I can point them to some test runs that show that others are doing better than
us, or that we are failing in important areas. We're not trying to hold
anyone hostage, nor trying to kill the idea. We just need collaboration
and fair grounds.

That being said, if you feel that this thread has dragged on for too long,
please ignore this reply even if I will use it in the future for reference ;)

I would also like to add that I'm happy with the resolution presented last
week, when it was agreed that the change should say "should" and also contain
some blurb about the igt tests being created for API changes "when appropriate".

Best regards,
Liviu

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

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯