[Intel-gfx] [PATCH 5/5] drm/i915: Add support for DP Video pattern compliance tests

[Ville Syrjälä]

On Wed, Nov 23, 2016 at 03:17:53PM +0100, Daniel Vetter wrote:
> On Wed, Nov 23, 2016 at 03:58:34PM +0200, Jani Nikula wrote:
> > On Wed, 23 Nov 2016, Ville Syrjälä <ville.syrjala at linux.intel.com> wrote:
> > > On Wed, Nov 23, 2016 at 03:37:24PM +0200, Jani Nikula wrote:
> > >> On Tue, 22 Nov 2016, Manasi Navare <manasi.d.navare at intel.com> wrote:
> > >> > The intel_dp_autotest_video_pattern() function gets invoked through the
> > >> > compliance test handler on a HPD short pulse if the test type is
> > >> > set to DP_TEST_VIDEO_PATTERN. This performs the DPCD registers
> > >> > reads to read the requested test pattern, video pattern resolution,
> > >> > frame rate and bits per color value. The results of this analysis
> > >> > are handed off to userspace so that the userspace app can set the
> > >> > video pattern mode appropriately for the test result/response.
> > >> >
> > >> > The compliance_test_active flag is set at the end of the individual
> > >> > test handling functions. This is so that the kernel-side operations
> > >> > can be completed without the risk of interruption from the userspace
> > >> > app that is polling on that flag.
> > >> 
> > >> I've brought this up before, but I think for this stuff the way to go is
> > >> to have the userspace read the DPCD directly. We have the dev node for
> > >> it.
> > >> 
> > >> With the approach in this patch, we'll just end up reading a bunch of
> > >> stuff from DPCD in kernel, doing error handling for that, decoding and
> > >> sanity checking the values, putting them in debugfs for the userspace to
> > >> read, having userspace code read debugfs, doing error handling for that,
> > >> decoding and sanity checking the data, finally doing something based on
> > >> the data.
> > >> 
> > >> You'll also get a *much* faster turnaround for getting your userspace
> > >> code done than getting all of this in kernel first, then tweaking your
> > >> userspace, having to update both of those in lockstep, etc. When this is
> > >> based on reading DPCD directly, you can just add new stuff quickly in
> > >> userspace, with no kernel dependency.
> > >> 
> > >> The easiest way would be to have an indication in debugfs for userspace
> > >> that there's something interesting in DPCD. Just a simple thing.
> > >
> > > Or just have the kernel fire off an uevent...
> > 
> > I know, I know, I had that written in my mail already, but swallowed it
> > not to add another dependency on another new UABI... we can do that
> > later.
> 
> In principle I'd agree, but the trouble with dp compliance is that the CTS
> expects us to do stupid shit sometimes, like that test pattern forced as
> 6bpc. So if we decode&handle everything in userspace then the question is
> how to feed all those special cases back into the kernel, which means more
> debugfs interfaces. Just going in the other direction.
> 
> Given that it's a bit a toss-up unfortunately, and the current approach
> seems reasonable.

The kernel could of course parse whatever is needed from the test
request as well, but leave it up to userspace to initiate the actual
test and ack/nack the test request. So I don't see a real need for
a debugfs interface in either direction.

-- 
Ville Syrjälä
Intel OTC