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

[Daniel Vetter]

On Wed, Jan 23, 2019 at 09:40:34AM +0000, Brian Starkey wrote:
> On Tue, Jan 22, 2019 at 08:19:10PM +0100, Daniel Vetter wrote:
> > On Tue, Jan 22, 2019 at 8:00 PM Wentland, Harry <Harry.Wentland at amd.com> wrote:
> > > On 2019-01-16 11:39 a.m., 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>
> > >
> > > I'm all for anything resembling TDD and standardizing on one test framework. IGT works quite well for us for testing display stuff. We still have a way to go but have been trying to adopt this requirement lately anyways for the DC driver. Can't really comment on anything beyond display, though, for AMD.
> > >
> > > No matter how much I want this to be mandatory, seeing the discussions with ARM and the comment about lack of CRC on Nouveau makes me think that we might not be quite ready to go there. Implementing DWB is non-trivial. VKMS knows how to compute a CRC from a framebuffer, but that's the trivial part. Setting up the HW and SW to do DWB is the hard part.
> > 
> > We also discussed a bit writeback implementations on irc, and it looks
> > like you can't really use writeback to accurately test that your
> > compositing engine is programmed correctly, since on at least vc4,
> > malidp and msm (not yet merged upstream) the writeback engine can't be
> > shared with any other outputs, often it even needs a
> > dedicated/special-purpose CRTC (at least vc4 from what I can tell).
> > That means if you botch your programming and e.g. cause an underrun
> > scanning out continous-update outputs, then the writeback won't show
> > that to you, since it's composited separately. I guess we could teach
> > igt to run these tests on the special crtc->writeback pipeline only,
> > but essentially that's a new testcase, and not really testing the
> > actual display: It tests writeback, not hdmi/dp/panels/whatever real
> > outputs you have.
> 
> That doesn't sound right for mali-dp at least. Our writeback is
> synchronous with the normal output. If the normal output underruns
> then the writeback shows that, and if the writeback overruns, then the
> normal output dies too.
> 
> Our writeback encoder/connector is a "possible_clone" of the normal
> one. So, to integrate it in igt I was intending to set up a cloned
> output on the pipe being tested.

Hm, I grepped for that, didn't find the assignment anywhere. If you can do
possible_clones writeback, then I think doing the writeback-as-fake CRC in
the kernel is probably the best, since it's closest to testing the real
thing. Of course if the writeback connector is already used by igt (for a
writeback test) then the magic "auto" crc tap point would need to fail.
But I don't think that would be a problem for igt, since if you're using
both writeback _and_ crc I have no idea what you're trying to do :-)

I also wouldn't tie this crc writeback into atomic (except with a special
"crc writeback flag" to make sure atomic_check rejects any other use of
the writeback connector while it's used for crc generation), but just a
vblank driven worker that flips between 2 buffers and checksums the other
one.
-Daniel

> 
> Cheers,
> -Brian
> 
> > 
> > I'd say we'll shrug these cases off as "can't be reasonable tested,
> > won't have an igt". First driver team with hw that can be validated
> > gets to fill the gaps :-) In practice still going to be a lot better
> > than no tests at all, just exercising the feature will be useful, and
> > will make it a lot easier for the next team to add the crc based tests
> > on top.
> > 
> > -Daniel
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch