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

[Jesse Barnes]

On Tue, 22 Jul 2014 13:48:45 -0700
Jesse Barnes <jbarnes at virtuousgeek.org> wrote:

> 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?

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