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. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
