[Intel-gfx] [PATCH v2] Displayport compliance testing

[Jesse Barnes]

On Tue, 22 Jul 2014 08:41:11 +0200
Daniel Vetter <daniel at ffwll.ch> 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?

-- 
Jesse Barnes, Intel Open Source Technology Center