Re: [PATCH v2] Displayport compliance testing

[Daniel Vetter <daniel@ffwll.ch>]

On Tue, Jul 29, 2014 at 06:53:57PM -0300, Paulo Zanoni wrote:
> 2014-07-22 18:11 GMT-03:00 Jesse Barnes <jbarnes@virtuousgeek.org>:
> > On Tue, 22 Jul 2014 22:53:44 +0200
> > Daniel Vetter <daniel@ffwll.ch> wrote:
> >
> >> On Tue, Jul 22, 2014 at 10:48 PM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> >> > Are you saying
> >> > you'll reject this approach entirely?
> >>
> >> I'm saying that I don't see terrible lot of value in adding a bunch of
> >> code for a sticker, and that we should look into making it actually
> >> useful by testing the paths that end-users end up using. And we have
> >> to keep this working once it's merged.
> >>
> >> But if it doesn't make sense to make this sticker useful while still
> >> being able to get it then I'll reconsider.
> >
> > Yeah I think it depends on the test.  We're supposed to go through
> > existing paths for testing e.g. link training with different params
> > (though with a fixed fb and mode), so getting coverage there is
> > something we want regardless.  But getting something like probing
> > covered as part of the compliance testing may be something else
> > entirely...
> 
> I was finally able to take some time to read the spec, and I agree
> that the hybrid approach looks like the way to go. Some tests require
> specifically-crafted FBs, while some other tests cause real hotplug
> events to be sent from the sink. If there's an unknown/unspecified
> user-space running when the tests are happening, who knows how it is
> going to react? Of course, for tests that can be implemented directly
> inside the Kernel still using the "standard" code paths, we should do
> it in the Kernel.
> 
> One possible approach that I thought would be the following:
> - Each DP encoder provides its own debugfs file for DP test compiance
> (e.g., /sys/kernel/debug/dri/0/i915_ddi_b_dp_test_compliance).
> - If the file is not open, any requests for tests that require special
> actions from our driver - outside of the normal behavior - will be
> NACKed.

Yeah that sounds like a reasonable safety measure to make sure we don't do
stupid things (e.g. malicious DP connector trying to break into the
kernel).

> - If the file is open, we ACK test requests and print special strings
> to the debugfs file telling the user-space app what it's supposed to
> do. We could use simple strings like "set the preferred mode", "set
> failsafe mode", "set mode using FB test pattern Y", etc. A stringly
> typed protocol :)

We need to check how much work this is, since we'll probably need to
implement polling. Otoh we've just done that for the crc interfaces, so
shouldn't be too much fuzz.

If userspace needs to do special dp aux transactions I still think we
should simply expose a proper dp aux interface, similar to i2c.

> - The user-space app needs to be the DRM master, open the debugfs
> file, parse the operations it prints and act accordingly, and listen
> to the hotplug events sent by the Kernel.
> - If some special corner quirky case needs to be done (e.g., train
> link with a specific number of lanes), the Kernel should store this
> information at struct intel_dp, and then when a modeset is done on
> this encoder, we check if the debugfs file is open (i.e., we're doing
> compliance testing) and then we use the specified configuration. With
> this, we can probably avoid special uevents or debug-only
> connector/encoder properties.
> - The user-space app could be part of intel-gpu-tools.
> 
> Anyway, this is just an alternate idea to Daniel's suggestion, and
> many other possible implementation ideas would work for me. Todd, what
> is your opinion?

Well I've only tossed out a rough idea that we should try to use the same
paths for validation as userspace is using in general. Sounds like you
have some good ideas how to get there in practice.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch