On Tue, 22 Jul 2014 13:48:45 -0700 Jesse Barnes <jbarnes@xxxxxxxxxxxxxxxx> wrote: > On Tue, 22 Jul 2014 08:41:11 +0200 > Daniel Vetter <daniel@xxxxxxxx> wrote: > > > On Mon, Jul 14, 2014 at 12:10:35PM -0700, Todd Previte wrote: > > > >This patch set adds the foundational support for Displayport compliance testing in the > > > >i915 driver. It implements the framework for automated test support that preclude the > > > >need (most) for operator input during testing. Tests for AUX transactions, EDID reads > > > >and basic link training have also been included, along with any support and utility > > > >functions required. Some changes have also been made to existing code to accommodate > > > >compliance testing. > > > > > > V2: > > > - Addressed review feedback from the mailing list > > > - Broke up patches into smaller, easily managed chunks > > > - Reordered the patches such that they can be applied in order > > > - Fixed checkpatch.pl errors across the patchset > > > - Updated and enhanced functionality for the EDID test function > > > - Completely revamped the mode set operations for compliance testing > > > > Ok, high level review of the overall approach. > > > > So your design is to implement special functions for each test procedure. > > This has two issues: > > - We have duplicated code we test, which has a really high chance to be > > different from what users actually run on their systems. And so dp > > validation won't actually validate the real code. Example would be the > > fallback edid reading tests using defer/nacks. The drm i2c helpers > > already have logic for this (including fallback modes), but very likely > > it doesn't quite match the dp requirements. So we need to adjust that > > (presuming the dp standard is sane). > > - Even if we can fix this there's still lots of important code in the > > probe paths we can't test with this. Userspace updates edids through the > > get_connector ioctl, and there's lots of additional logic in-between > > until we reach the dp connector implementation in intel_dp.c. > > > > So what I'd like to see is that the test procedures are implemented in > > userspace, using nothing more than the standard kms interfaces. > > > > Afaics we need three steps overall: > > - Get a shot hpd pulse and notice that we should do a validation test > > sequence. We need to get this information to userspace, and the best > > approach would be a uevent on the connector in sysfs. We can supply any > > additional metadata (like the test mode) userspace needs with uevent > > attributes. > > > > - Do the test from userspace. Depending upon what exactly needs to be done > > we might need to extend the properties exposed to userspace, e.g. dp > > link parameters. > > > > - Give the result (edid checksum, ...) back to the dp sink/validator. A > > generic dp aux transaction interface for userspace in the dp helpers, > > maybe even as a real /dev node should fit the bill. > > > > dp validation has the potential to automatically test a pile of code for > > which we currently have 0 automated test coverage at all. I want to fully > > seize this opportunity instead of doing the bare minimum to get a (rather > > useless) certification. > > This amounts to a complete rewrite with a different goal. In this > series, I only see two big new test functions added: one for EDID > testing and one for link training. The EDID one could probably share > some more code with the core, but for link training it seems pretty > simple, unless we need a full mode & fb for some reason (I'm presuming > Todd has tested this with the equipment). Paulo had some good > feedback, but overall this is a small addition to get the compliance > tests working and get some coverage on the link training paths which we > desperately need. > > Having some additional user level tests would be great, but that's a > much bigger and different task than simply implementing what's required > for the DP compliance test. Asking Todd to take on a huge new task > just because he posted this series is a big request. Are you saying > you'll reject this approach entirely? I'd have to check the spec, maybe the best approach is a hybrid; some tests handled directly in the kernel to easily re-use our existing bits, and some in userspace to push through a full mode set path more easily. -- Jesse Barnes, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
