Re: [PATCH v2] Displayport compliance testing

[Daniel Vetter <daniel@ffwll.ch>]

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