From: Todd Previte <tprevite@gmail.com>
To: jim.bride@linux.intel.com, intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/i915/hsw: Fix workaround for server AUX channel clock divisor
Date: Wed, 20 May 2015 16:07:37 -0700 [thread overview]
Message-ID: <555D13B9.7030004@gmail.com> (raw)
In-Reply-To: <1432052010-13412-1-git-send-email-jim.bride@linux.intel.com>
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.
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@gmail.com>
-T
On 5/19/2015 9:13 AM, jim.bride@linux.intel.com wrote:
> From: Jim Bride <jim.bride@linux.intel.com>
>
> 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@linux.intel.com>
> ---
> 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@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2015-05-20 23:07 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-05-19 16:13 [PATCH] drm/i915/hsw: Fix workaround for server AUX channel clock divisor jim.bride
2015-05-20 23:07 ` Todd Previte [this message]
2015-05-21 8:20 ` Ville Syrjälä
2015-05-21 12:04 ` Jani Nikula
2015-05-21 18:34 ` Jim Bride
2015-05-22 4:43 ` Jani Nikula
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=555D13B9.7030004@gmail.com \
--to=tprevite@gmail.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=jim.bride@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox