<div dir="ltr">Hi Jani,<div><br></div><div>Is this version ok now?</div><div>Do you believe we could go only with this patch for now and continue the retry and kill read_wake function later?</div><div><br></div><div>Thanks,</div><div>Rodrigo.</div><div><br></div><div><div class="gmail_quote"><div dir="ltr">On Wed, Nov 25, 2015 at 4:04 PM Rodrigo Vivi <<a href="mailto:rodrigo.vivi@intel.com">rodrigo.vivi@intel.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Mainly aux communications on sink_crc<br>
were failing a lot randomly on recent platforms.<br>
The first solution was to try to use intel_dp_dpcd_read_wake, but then<br>
it was suggested to move retries to drm level.<br>
<br>
Since drm level was already taking care of retries and didn't want<br>
to through random retries on that level the second solution was to<br>
put the retries at aux_transfer layer what was nacked.<br>
<br>
So I realized we had so many retries in different places and<br>
started to organize that a bit. During this organization I noticed<br>
that we weren't handing at all the case were the message size was<br>
zeroed. And this was exactly the case that was affecting sink_crc.<br>
<br>
Also we weren't respect BSPec who says this size message = 0 or > 20<br>
are forbidden.<br>
<br>
It is a fact that we still have no clue why we are getting this<br>
forbidden value there. But anyway we need to handle that for now<br>
so we return -EBUSY and drm level takes care of the retries that<br>
are already in place.<br>
<br>
v2: Print debug messsage when this case is reached as suggested<br>
    by Jani.<br>
<br>
Cc: Jani Nikula <<a href="mailto:jani.nikula@intel.com" target="_blank">jani.nikula@intel.com</a>><br>
Cc: Daniel Vetter <<a href="mailto:daniel.vetter@ffwll.ch" target="_blank">daniel.vetter@ffwll.ch</a>><br>
Signed-off-by: Rodrigo Vivi <<a href="mailto:rodrigo.vivi@intel.com" target="_blank">rodrigo.vivi@intel.com</a>><br>
Tested-by: Daniel Stone <<a href="mailto:daniels@collabora.com" target="_blank">daniels@collabora.com</a>> # SKL<br>
---<br>
 drivers/gpu/drm/i915/intel_dp.c | 13 +++++++++++++<br>
 1 file changed, 13 insertions(+)<br>
<br>
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c<br>
index 54e85f5..a02bfa1 100644<br>
--- a/drivers/gpu/drm/i915/intel_dp.c<br>
+++ b/drivers/gpu/drm/i915/intel_dp.c<br>
@@ -899,6 +899,19 @@ done:<br>
        /* Unload any bytes sent back from the other side */<br>
        recv_bytes = ((status & DP_AUX_CH_CTL_MESSAGE_SIZE_MASK) >><br>
                      DP_AUX_CH_CTL_MESSAGE_SIZE_SHIFT);<br>
+<br>
+       /*<br>
+        * By BSpec: "Message sizes of 0 or >20 are not allowed."<br>
+        * We have no idea of what happened so we return -EBUSY so<br>
+        * drm layer takes care for the necessary retries.<br>
+        */<br>
+       if (recv_bytes == 0 || recv_bytes > 20) {<br>
+               DRM_DEBUG_KMS("Forbidden recv_bytes = %d on aux transaction\n",<br>
+                             recv_bytes);<br>
+               ret = -EBUSY;<br>
+               goto out;<br>
+       }<br>
+<br>
        if (recv_bytes > recv_size)<br>
                recv_bytes = recv_size;<br>
<br>
--<br>
2.4.3<br>
<br>
_______________________________________________<br>
dri-devel mailing list<br>
<a href="mailto:dri-devel@lists.freedesktop.org" target="_blank">dri-devel@lists.freedesktop.org</a><br>
<a href="http://lists.freedesktop.org/mailman/listinfo/dri-devel" rel="noreferrer" target="_blank">http://lists.freedesktop.org/mailman/listinfo/dri-devel</a><br>
</blockquote></div></div></div>