<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<div class="moz-cite-prefix">On 12/17/14 10:57 AM, Paulo Zanoni
wrote:<br>
</div>
<blockquote
cite="mid:CA+gsUGReWtqf+OxCKTQB=bf=0yfUBb5vfCEQuXCnyAsfZ5jG7A@mail.gmail.com"
type="cite">
<pre wrap="">2014-12-10 21:53 GMT-02:00 Todd Previte <a class="moz-txt-link-rfc2396E" href="mailto:tprevite@gmail.com"><tprevite@gmail.com></a>:
</pre>
<blockquote type="cite">
<pre wrap="">Moves the non-MST case out of the if-statement and places it at the beginning
of the function to handle HPD events for SST mode. The reasoning behind this
is to accommodate link status checks for compliance testing. Some test devices
use long pulses to perform test requests so link status must be checked
regardless of the pulse width for the SST operational mode.
</pre>
</blockquote>
<pre wrap="">Can you please elaborate a little more on what do you see on these
devices? The test spec is very clear about short vs long HPD pulses,
so it's hard to believe a test device would get this wrong. We have
some registers on the PCH that allow us to redefine short vs long
durations. Have you tried to play with them?
More below:
</pre>
</blockquote>
<meta http-equiv="content-type" content="text/html; charset=utf-8">
The issue is not in differentiating between the two pulse widths.
The problem is that compliance testing mixes the two together, i.e.
some tests are IRQ events where the source has to "service" the sink
while other tests are examining the hot plug detection and response
functionality. So when it comes to compliance testing, they both
have to be handled and checked to see if there's a test request
coming in or if it's a real HPD event of some kind. The current
implementation only checks SST mode for the short pulse case.<br>
<br>
In light of Daniel's comment, though, that makes this code even more
broken. The best place to handle this (as I mention in my response
to Daniel) is to place the SST case after the mst_fail tag. That
way, once it is determined that MST mode is not in use, the SST
handler can be invoked and events responded to appropriately.<br>
<br>
<br>
<blockquote
cite="mid:CA+gsUGReWtqf+OxCKTQB=bf=0yfUBb5vfCEQuXCnyAsfZ5jG7A@mail.gmail.com"
type="cite">
<blockquote type="cite">
<pre wrap="">This patch replaces [PATCH 10/10] drm/i915: Fix intel_dp_hot_plug() in the
previous compliance testing patch sequence. Review feedback on that patch
indicated that updating intel_dp_hot_plug() was not the correct place for
the test handler.
Signed-off-by: Todd Previte <a class="moz-txt-link-rfc2396E" href="mailto:tprevite@gmail.com"><tprevite@gmail.com></a>
---
drivers/gpu/drm/i915/intel_dp.c | 22 ++++++++++++----------
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 4a55ca6..73014d8 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4613,6 +4613,18 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
power_domain = intel_display_port_power_domain(intel_encoder);
intel_display_power_get(dev_priv, power_domain);
+ if (!intel_dp->is_mst) {
+ /*
+ * Pulse width doesn't matter for SST mode
+ * Handle the HPD event now
+ */
+ drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
+ intel_dp_check_link_status(intel_dp);
</pre>
</blockquote>
<pre wrap="">The very first thing intel_dp_check_link_status() does is to return in
case "connector->base.status != connected". If we're getting a long
HPD, it doesn't seem make sense to check this field because the status
might be changing due to the long HPD.
</pre>
</blockquote>
Long pulses are either connect OR disconnect events. In the case of
the test device, what happens is that it's already connected, so HPD
is asserted. It pulses the HPD line low for > 2ms and then
reasserts it sometime later once the source device has disabled the
main link. So it becomes a 2-fold event for us - the initial
connected->disconnected transition and the following
disconnected->connected transition. Both of those invoke our IRQ
handler and cause neat and exciting things to happen. For compliance
testing, the disconnect event needs to be ignored (by the test code,
anyways) and the connect event needs to be checked for
TEST_REQUEST=1. Thus, it absolutely makes sense to check this here.<br>
<br>
<blockquote
cite="mid:CA+gsUGReWtqf+OxCKTQB=bf=0yfUBb5vfCEQuXCnyAsfZ5jG7A@mail.gmail.com"
type="cite">
<blockquote type="cite">
<pre wrap="">+ drm_modeset_unlock(&dev->mode_config.connection_mutex);
+ ret = false;
+ goto put_power;
+ }
+
if (long_hpd) {
if (HAS_PCH_SPLIT(dev)) {
@@ -4637,16 +4649,6 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
if (intel_dp_check_mst_status(intel_dp) == -EINVAL)
goto mst_fail;
}
-
- if (!intel_dp->is_mst) {
- /*
- * we'll check the link status via the normal hot plug path later -
- * but for short hpds we should check it now
- */
- drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
- intel_dp_check_link_status(intel_dp);
- drm_modeset_unlock(&dev->mode_config.connection_mutex);
- }
}
ret = false;
goto put_power;
--
1.9.1
_______________________________________________
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>
</blockquote>
<br>
</body>
</html>