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.wentl...@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.latv...@intel.com> > > > > Cc: Arkadiusz Hiler <arkadiusz.hi...@intel.com> > > > > Cc: Liviu Dudau <liviu.du...@arm.com> > > > > Cc: Sean Paul <s...@poorly.run> > > > > Cc: Eric Anholt <e...@anholt.net> > > > > Cc: Alex Deucher <alexander.deuc...@amd.com> > > > > Cc: Dave Airlie <airl...@redhat.com> > > > > Signed-off-by: Daniel Vetter <daniel.vet...@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@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
