On Thu, 21 May 2015, Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> wrote:
> On Wed, May 20, 2015 at 04:07:37PM -0700, Todd Previte wrote:
>> Hi Jim,
>>
>> I checked the BSpec as well and there's nothing indicating that these
>> two bits are mutually exclusive. They are both sticky though, and if the
>> loop times out 5 times, both _DONE and _TIMEOUT may very well be set. In
>> that case the current code would just exit and never bother to change
>> clock dividers. So I think your code here is valid.
>
> Shouldn't we we checking receive error as well then?
In other words,
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 7cf3fd43071a..179af62d803d 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -893,10 +893,8 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
continue;
}
if (status & DP_AUX_CH_CTL_DONE)
- break;
+ goto done;
}
- if (status & DP_AUX_CH_CTL_DONE)
- break;
}
if ((status & DP_AUX_CH_CTL_DONE) == 0) {
@@ -921,7 +919,7 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
ret = -ETIMEDOUT;
goto out;
}
-
+done:
/* Unload any bytes sent back from the other side */
recv_bytes = ((status & DP_AUX_CH_CTL_MESSAGE_SIZE_MASK) >>
DP_AUX_CH_CTL_MESSAGE_SIZE_SHIFT);
>
> In fact looking at our code after the loop, it's expecting DONE to be
> set before it'll even report receive/timeout errors to the user as
> EIO/ETIMEDOUT, otherwise it'll just return EBUSY.
>
>>
>> The only thing you may want to do is capture some of the IRC discussion
>> though. In particular, whether or not this is really HSW-specific, how
>> this affects other platforms and and the additional delays incurred by
>> running through the loop again after changing AUX channel clock dividers
>> would be good information to put in there. That's probably more of a
>> bikeshed though.
>>
>> Reviewed-by: Todd Previte <tprevite@xxxxxxxxx>
>>
>> -T
>>
>> On 5/19/2015 9:13 AM, jim.bride@xxxxxxxxxxxxxxx wrote:
>> > From: Jim Bride <jim.bride@xxxxxxxxxxxxxxx>
>> >
>> > According to the HSW b-spec we need to try clock divisors of 63
>> > and 72, each 3 or more times, when attempting DP AUX channel
>> > communication on a server chipset. This actually wasn't happening
>> > due to a short-circuit that only checked the DP_AUX_CH_CTL_DONE bit
>> > in status rather than checking that the operation was done and
>> > that DP_AUX_CH_CTL_TIME_OUT_ERROR was not set.
>> >
>> > Signed-off-by: Jim Bride <jim.bride@xxxxxxxxxxxxxxx>
>> > ---
>> > drivers/gpu/drm/i915/intel_dp.c | 3 ++-
>> > 1 file changed, 2 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> > index 0edc305..c01a3f9 100644
>> > --- a/drivers/gpu/drm/i915/intel_dp.c
>> > +++ b/drivers/gpu/drm/i915/intel_dp.c
>> > @@ -895,7 +895,8 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
>> > if (status & DP_AUX_CH_CTL_DONE)
>> > break;
>> > }
>> > - if (status & DP_AUX_CH_CTL_DONE)
>> > + if ((status & DP_AUX_CH_CTL_DONE) &&
>> > + !(status & DP_AUX_CH_CTL_TIME_OUT_ERROR))
>> > break;
>> > }
>> >
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
