<html><head>
<meta content="text/html; charset=ISO-8859-1" http-equiv="Content-Type">
</head><body bgcolor="#FFFFFF" text="#000000"><br>
<br>
<blockquote style="border: 0px none;"
cite="mid:CA+gsUGSKPpEGtKOnneNVqMNRrNTi_O6kR=vRq=z8nb0MDJ=fNg@mail.gmail.com"
type="cite">
<div style="margin:30px 25px 10px 25px;" class="__pbConvHr"><div
style="display:table;width:100%;border-top:1px solid
#EDEEF0;padding-top:5px"> <div
style="display:table-cell;vertical-align:middle;padding-right:6px;"><img
photoaddress="przanoni@gmail.com" photoname="Paulo Zanoni"
src="cid:part1.03030605.02040006@gmail.com" name="postbox-contact.jpg"
height="25px" width="25px"></div> <div
style="display:table-cell;white-space:nowrap;vertical-align:middle;width:100%">
<a moz-do-not-send="true" href="mailto:przanoni@gmail.com"
style="color:#737F92
!important;padding-right:6px;font-weight:bold;text-decoration:none
!important;">Paulo Zanoni</a></div> <div
style="display:table-cell;white-space:nowrap;vertical-align:middle;">
<font color="#9FA2A5"><span style="padding-left:6px">Tuesday, July 29,
2014 2:53 PM</span></font></div></div></div>
<div style="color: rgb(136, 136, 136); margin-left: 24px;
margin-right: 24px;" __pbrmquotes="true" class="__pbConvBody"><pre wrap="">2014-07-22 18:11 GMT-03:00 Jesse Barnes <a class="moz-txt-link-rfc2396E" href="mailto:jbarnes@virtuousgeek.org"><jbarnes@virtuousgeek.org></a>:
</pre><blockquote type="cite"><pre wrap="">On Tue, 22 Jul 2014 22:53:44 +0200
Daniel Vetter <a class="moz-txt-link-rfc2396E" href="mailto:daniel@ffwll.ch"><daniel@ffwll.ch></a> wrote:
</pre><blockquote type="cite"><pre wrap="">On Tue, Jul 22, 2014 at 10:48 PM, Jesse Barnes <a class="moz-txt-link-rfc2396E" href="mailto:jbarnes@virtuousgeek.org"><jbarnes@virtuousgeek.org></a> wrote:
</pre><blockquote type="cite"><pre wrap="">Are you saying
you'll reject this approach entirely?
</pre></blockquote><pre wrap="">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.
</pre></blockquote><pre wrap="">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...
</pre></blockquote><pre wrap=""><!---->
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.</pre></div>
</blockquote>
There's no question that this is going to require considerable support
within the kernel. The tests that can be placed inside the kernel are
mostly going to be ones that simply can't be accomplished from userspace
(the 400us AUX transaction retry, for instance) or are elements that
are necessary to enable the userspace tests to execute properly. The
tests themselves though should definitely be handled out in userspace
where possible. <br>
<blockquote style="border: 0px none;"
cite="mid:CA+gsUGSKPpEGtKOnneNVqMNRrNTi_O6kR=vRq=z8nb0MDJ=fNg@mail.gmail.com"
type="cite">
<div style="color: rgb(136, 136, 136); margin-left: 24px;
margin-right: 24px;" __pbrmquotes="true" class="__pbConvBody">
<pre wrap="">
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.</pre>
</div>
</blockquote>
There's a couple options here that I've considered. This is a perfectly
valid option but it's also possible to just use one debugfs file for the
compliance work. It's unlikely that multiple Displayport connectors
will ever be testing simultaneously, so the need to adjust the
parameters on a per-connector basis is arguable. Only the connector for
the sink that issued the test request would be required to read the
debugfs file and adhere to its parameters. <br>
<br>
For general debugging of Displayport though, this is a better option as
it allows for considerably more flexibility and utility. Personally I
prefer the individual file approach, even though it might end up being
somewhat more complicated to implement. The single-file approach does
have merit though, so if anyone has a solid argument in favor of that,
it's worth the discussion.<br>
<blockquote style="border: 0px none;"
cite="mid:CA+gsUGSKPpEGtKOnneNVqMNRrNTi_O6kR=vRq=z8nb0MDJ=fNg@mail.gmail.com"
type="cite">
<div style="color: rgb(136, 136, 136); margin-left: 24px;
margin-right: 24px;" __pbrmquotes="true" class="__pbConvBody">
<pre wrap="">
- 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 :)</pre>
</div>
</blockquote>
Probably better to use clearly-defined constants instead of strings, but
yes, that's the idea. :)<br>
<blockquote style="border: 0px none;"
cite="mid:CA+gsUGSKPpEGtKOnneNVqMNRrNTi_O6kR=vRq=z8nb0MDJ=fNg@mail.gmail.com"
type="cite">
<div style="color: rgb(136, 136, 136); margin-left: 24px;
margin-right: 24px;" __pbrmquotes="true" class="__pbConvBody">
<pre wrap="">
- 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.</pre>
</div>
</blockquote>
Agreed.<br>
<blockquote style="border: 0px none;"
cite="mid:CA+gsUGSKPpEGtKOnneNVqMNRrNTi_O6kR=vRq=z8nb0MDJ=fNg@mail.gmail.com"
type="cite">
<div style="color: rgb(136, 136, 136); margin-left: 24px;
margin-right: 24px;" __pbrmquotes="true" class="__pbConvBody">
<pre wrap="">
- 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.</pre>
</div>
</blockquote>
I would argue here that it's better to leave as much of the active
configuration undisturbed as possible and only update the fields
necessary to complete the test(s). Additionally, any fields that are
changed should be saved and subsequently restored upon completion of the
test(s)<br>
<blockquote style="border: 0px none;"
cite="mid:CA+gsUGSKPpEGtKOnneNVqMNRrNTi_O6kR=vRq=z8nb0MDJ=fNg@mail.gmail.com"
type="cite">
<div style="color: rgb(136, 136, 136); margin-left: 24px;
margin-right: 24px;" __pbrmquotes="true" class="__pbConvBody">
<pre wrap="">
- The user-space app could be part of intel-gpu-tools.</pre>
</div>
</blockquote>
Cool, this is what I was planning on doing. One thing I was looking into
was potentially launching the test app from a uevent in the kernel. But
none of the solutions I could find were all that good and some were
downright scary. So having the app as a separate entity that needs to be
launched by the tester isn't such a bad idea.<br>
<div style="color: rgb(136, 136, 136); margin-left: 24px; margin-right:
24px;" __pbrmquotes="true" class="__pbConvBody">
<pre wrap="">
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?
I will continue the review of the patches we currently have, since it
seems we didn't decide what we're gonna merge.</pre>
</div>
<blockquote style="border: 0px none;"
cite="mid:CA+gsUGSKPpEGtKOnneNVqMNRrNTi_O6kR=vRq=z8nb0MDJ=fNg@mail.gmail.com"
type="cite">
<div style="color:#888888;margin-left:24px;margin-right:24px;"
__pbrmquotes="true" class="__pbConvBody">
<pre wrap="">
</pre>
<blockquote type="cite"><pre wrap="">--
Jesse Barnes, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
<a class="moz-txt-link-abbreviated" href="mailto:Intel-gfx@lists.freedesktop.org">Intel-gfx@lists.freedesktop.org</a>
<a class="moz-txt-link-freetext" href="http://lists.freedesktop.org/mailman/listinfo/intel-gfx">http://lists.freedesktop.org/mailman/listinfo/intel-gfx</a>
</pre></blockquote><pre wrap=""><!---->
</pre></div>
<div style="margin:30px 25px 10px 25px;" class="__pbConvHr"><div
style="display:table;width:100%;border-top:1px solid
#EDEEF0;padding-top:5px"> <div
style="display:table-cell;vertical-align:middle;padding-right:6px;"><img
photoaddress="jbarnes@virtuousgeek.org" photoname="Jesse Barnes"
src="cid:part2.08080906.01030409@gmail.com"
name="compose-unknown-contact.jpg" height="25px" width="25px"></div> <div
style="display:table-cell;white-space:nowrap;vertical-align:middle;width:100%">
<a moz-do-not-send="true" href="mailto:jbarnes@virtuousgeek.org"
style="color:#737F92
!important;padding-right:6px;font-weight:bold;text-decoration:none
!important;">Jesse Barnes</a></div> <div
style="display:table-cell;white-space:nowrap;vertical-align:middle;">
<font color="#9FA2A5"><span style="padding-left:6px">Tuesday, July 22,
2014 2:11 PM</span></font></div></div></div>
<div style="color:#888888;margin-left:24px;margin-right:24px;"
__pbrmquotes="true" class="__pbConvBody"><div>On Tue, 22 Jul 2014
22:53:44 +0200<br></div><div><!----><br>Yeah I think it depends on the
test. We're supposed to go through<br>existing paths for testing e.g.
link training with different params<br>(though with a fixed fb and
mode), so getting coverage there is<br>something we want regardless.
But getting something like probing<br>covered as part of the compliance
testing may be something else<br>entirely...<br><br></div></div>
<div style="margin:30px 25px 10px 25px;" class="__pbConvHr"><div
style="display:table;width:100%;border-top:1px solid
#EDEEF0;padding-top:5px"> <div
style="display:table-cell;vertical-align:middle;padding-right:6px;"><img
photoaddress="daniel@ffwll.ch" photoname="Daniel Vetter"
src="cid:part3.02030704.01000309@gmail.com" name="postbox-contact.jpg"
height="25px" width="25px"></div> <div
style="display:table-cell;white-space:nowrap;vertical-align:middle;width:100%">
<a moz-do-not-send="true" href="mailto:daniel@ffwll.ch"
style="color:#737F92
!important;padding-right:6px;font-weight:bold;text-decoration:none
!important;">Daniel Vetter</a></div> <div
style="display:table-cell;white-space:nowrap;vertical-align:middle;">
<font color="#9FA2A5"><span style="padding-left:6px">Tuesday, July 22,
2014 1:53 PM</span></font></div></div></div>
<div style="color:#888888;margin-left:24px;margin-right:24px;"
__pbrmquotes="true" class="__pbConvBody"><div><!----><br>I'm saying that
I don't see terrible lot of value in adding a bunch of<br>code for a
sticker, and that we should look into making it actually<br>useful by
testing the paths that end-users end up using. And we have<br>to keep
this working once it's merged.<br><br>But if it doesn't make sense to
make this sticker useful while still<br>being able to get it then I'll
reconsider.<br>-Daniel<br></div></div>
<div style="margin:30px 25px 10px 25px;" class="__pbConvHr"><div
style="display:table;width:100%;border-top:1px solid
#EDEEF0;padding-top:5px"> <div
style="display:table-cell;vertical-align:middle;padding-right:6px;"><img
photoaddress="jbarnes@virtuousgeek.org" photoname="Jesse Barnes"
src="cid:part2.08080906.01030409@gmail.com"
name="compose-unknown-contact.jpg" height="25px" width="25px"></div> <div
style="display:table-cell;white-space:nowrap;vertical-align:middle;width:100%">
<a moz-do-not-send="true" href="mailto:jbarnes@virtuousgeek.org"
style="color:#737F92
!important;padding-right:6px;font-weight:bold;text-decoration:none
!important;">Jesse Barnes</a></div> <div
style="display:table-cell;white-space:nowrap;vertical-align:middle;">
<font color="#9FA2A5"><span style="padding-left:6px">Tuesday, July 22,
2014 1:48 PM</span></font></div></div></div>
<div style="color:#888888;margin-left:24px;margin-right:24px;"
__pbrmquotes="true" class="__pbConvBody"><div>On Tue, 22 Jul 2014
08:41:11 +0200<br></div><div><!----><br>This amounts to a complete
rewrite with a different goal. In this<br>series, I only see two big
new test functions added: one for EDID<br>testing and one for link
training. The EDID one could probably share<br>some more code with the
core, but for link training it seems pretty<br>simple, unless we need a
full mode & fb for some reason (I'm presuming<br>Todd has tested
this with the equipment). Paulo had some good<br>feedback, but overall
this is a small addition to get the compliance<br>tests working and get
some coverage on the link training paths which we<br>desperately need.<br><br>Having
some additional user level tests would be great, but that's a<br>much
bigger and different task than simply implementing what's required<br>for
the DP compliance test. Asking Todd to take on a huge new task<br>just
because he posted this series is a big request. Are you saying<br>you'll
reject this approach entirely?<br><br></div></div>
<div style="margin:30px 25px 10px 25px;" class="__pbConvHr"><div
style="display:table;width:100%;border-top:1px solid
#EDEEF0;padding-top:5px"> <div
style="display:table-cell;vertical-align:middle;padding-right:6px;"><img
photoaddress="daniel@ffwll.ch" photoname="Daniel Vetter"
src="cid:part3.02030704.01000309@gmail.com" name="postbox-contact.jpg"
height="25px" width="25px"></div> <div
style="display:table-cell;white-space:nowrap;vertical-align:middle;width:100%">
<a moz-do-not-send="true" href="mailto:daniel@ffwll.ch"
style="color:#737F92
!important;padding-right:6px;font-weight:bold;text-decoration:none
!important;">Daniel Vetter</a></div> <div
style="display:table-cell;white-space:nowrap;vertical-align:middle;">
<font color="#9FA2A5"><span style="padding-left:6px">Monday, July 21,
2014 11:41 PM</span></font></div></div></div>
<div style="color:#888888;margin-left:24px;margin-right:24px;"
__pbrmquotes="true" class="__pbConvBody"><div><!----><br>Ok, high level
review of the overall approach.<br><br>So your design is to implement
special functions for each test procedure.<br>This has two issues:<br>-
We have duplicated code we test, which has a really high chance to be<br>
different from what users actually run on their systems. And so dp<br>
validation won't actually validate the real code. Example would be the<br>
fallback edid reading tests using defer/nacks. The drm i2c helpers<br>
already have logic for this (including fallback modes), but very
likely<br> it doesn't quite match the dp requirements. So we need to
adjust that<br> (presuming the dp standard is sane).<br>- Even if we
can fix this there's still lots of important code in the<br> probe
paths we can't test with this. Userspace updates edids through the<br>
get_connector ioctl, and there's lots of additional logic in-between<br>
until we reach the dp connector implementation in intel_dp.c.<br><br>So
what I'd like to see is that the test procedures are implemented in<br>userspace,
using nothing more than the standard kms interfaces.<br><br>Afaics we
need three steps overall:<br>- Get a shot hpd pulse and notice that we
should do a validation test<br> sequence. We need to get this
information to userspace, and the best<br> approach would be a uevent
on the connector in sysfs. We can supply any<br> additional metadata
(like the test mode) userspace needs with uevent<br> attributes.<br><br>-
Do the test from userspace. Depending upon what exactly needs to be
done<br> we might need to extend the properties exposed to userspace,
e.g. dp<br> link parameters.<br><br>- Give the result (edid checksum,
...) back to the dp sink/validator. A<br> generic dp aux transaction
interface for userspace in the dp helpers,<br> maybe even as a real
/dev node should fit the bill.<br><br>dp validation has the potential to
automatically test a pile of code for<br>which we currently have 0
automated test coverage at all. I want to fully<br>seize this
opportunity instead of doing the bare minimum to get a (rather<br>useless)
certification.<br>-Daniel<br></div></div>
</blockquote>
<br>
<div class="moz-signature">-- <br>
<div>Sent with <a href="http://www.getpostbox.com"><span style="color:
rgb(51, 102, 153);">Postbox</span></a></div></div>
</body></html>