* [PATCH] drm/dp: Do not busy-loop during link training @ 2015-12-14 13:21 Thierry Reding 2015-12-14 14:48 ` Ville Syrjälä 0 siblings, 1 reply; 6+ messages in thread From: Thierry Reding @ 2015-12-14 13:21 UTC (permalink / raw) To: dri-devel From: Thierry Reding <treding@nvidia.com> Use microsecond sleeps for the clock recovery and channel equalization delays during link training. The duration of these delays can be from 100 us up to 16 ms. It is rude to busy-loop for that amount of time. While at it, also convert to standard coding style by putting the opening braces in a function definition on a new line. Signed-off-by: Thierry Reding <treding@nvidia.com> --- drivers/gpu/drm/drm_dp_helper.c | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c index 73a3e0544f36..2dcc4efcd34c 100644 --- a/drivers/gpu/drm/drm_dp_helper.c +++ b/drivers/gpu/drm/drm_dp_helper.c @@ -125,19 +125,29 @@ u8 drm_dp_get_adjust_request_post_cursor(const u8 link_status[DP_LINK_STATUS_SIZ } EXPORT_SYMBOL(drm_dp_get_adjust_request_post_cursor); -void drm_dp_link_train_clock_recovery_delay(const u8 dpcd[DP_RECEIVER_CAP_SIZE]) { - if (dpcd[DP_TRAINING_AUX_RD_INTERVAL] == 0) - udelay(100); +void drm_dp_link_train_clock_recovery_delay(const u8 dpcd[DP_RECEIVER_CAP_SIZE]) +{ + unsigned int min; + + if (dpcd[DP_TRAINING_AUX_RD_INTERVAL] != 0) + min = dpcd[DP_TRAINING_AUX_RD_INTERVAL] * 4000; else - mdelay(dpcd[DP_TRAINING_AUX_RD_INTERVAL] * 4); + min = 100; + + usleep_range(min, min * 2); } EXPORT_SYMBOL(drm_dp_link_train_clock_recovery_delay); -void drm_dp_link_train_channel_eq_delay(const u8 dpcd[DP_RECEIVER_CAP_SIZE]) { - if (dpcd[DP_TRAINING_AUX_RD_INTERVAL] == 0) - udelay(400); +void drm_dp_link_train_channel_eq_delay(const u8 dpcd[DP_RECEIVER_CAP_SIZE]) +{ + unsigned int min; + + if (dpcd[DP_TRAINING_AUX_RD_INTERVAL] != 0) + min = dpcd[DP_TRAINING_AUX_RD_INTERVAL] * 4000; else - mdelay(dpcd[DP_TRAINING_AUX_RD_INTERVAL] * 4); + min = 400; + + usleep_range(min, min * 2); } EXPORT_SYMBOL(drm_dp_link_train_channel_eq_delay); -- 2.5.0 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/dp: Do not busy-loop during link training 2015-12-14 13:21 [PATCH] drm/dp: Do not busy-loop during link training Thierry Reding @ 2015-12-14 14:48 ` Ville Syrjälä 2015-12-14 15:23 ` Thierry Reding 0 siblings, 1 reply; 6+ messages in thread From: Ville Syrjälä @ 2015-12-14 14:48 UTC (permalink / raw) To: Thierry Reding; +Cc: dri-devel On Mon, Dec 14, 2015 at 02:21:56PM +0100, Thierry Reding wrote: > From: Thierry Reding <treding@nvidia.com> > > Use microsecond sleeps for the clock recovery and channel equalization > delays during link training. The duration of these delays can be from > 100 us up to 16 ms. It is rude to busy-loop for that amount of time. Do you have some numbers on how this affects a typical link training cycle? > > While at it, also convert to standard coding style by putting the > opening braces in a function definition on a new line. > > Signed-off-by: Thierry Reding <treding@nvidia.com> > --- > drivers/gpu/drm/drm_dp_helper.c | 26 ++++++++++++++++++-------- > 1 file changed, 18 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c > index 73a3e0544f36..2dcc4efcd34c 100644 > --- a/drivers/gpu/drm/drm_dp_helper.c > +++ b/drivers/gpu/drm/drm_dp_helper.c > @@ -125,19 +125,29 @@ u8 drm_dp_get_adjust_request_post_cursor(const u8 link_status[DP_LINK_STATUS_SIZ > } > EXPORT_SYMBOL(drm_dp_get_adjust_request_post_cursor); > > -void drm_dp_link_train_clock_recovery_delay(const u8 dpcd[DP_RECEIVER_CAP_SIZE]) { > - if (dpcd[DP_TRAINING_AUX_RD_INTERVAL] == 0) > - udelay(100); > +void drm_dp_link_train_clock_recovery_delay(const u8 dpcd[DP_RECEIVER_CAP_SIZE]) > +{ > + unsigned int min; > + > + if (dpcd[DP_TRAINING_AUX_RD_INTERVAL] != 0) > + min = dpcd[DP_TRAINING_AUX_RD_INTERVAL] * 4000; > else > - mdelay(dpcd[DP_TRAINING_AUX_RD_INTERVAL] * 4); > + min = 100; > + > + usleep_range(min, min * 2); > } > EXPORT_SYMBOL(drm_dp_link_train_clock_recovery_delay); > > -void drm_dp_link_train_channel_eq_delay(const u8 dpcd[DP_RECEIVER_CAP_SIZE]) { > - if (dpcd[DP_TRAINING_AUX_RD_INTERVAL] == 0) > - udelay(400); > +void drm_dp_link_train_channel_eq_delay(const u8 dpcd[DP_RECEIVER_CAP_SIZE]) > +{ > + unsigned int min; > + > + if (dpcd[DP_TRAINING_AUX_RD_INTERVAL] != 0) > + min = dpcd[DP_TRAINING_AUX_RD_INTERVAL] * 4000; > else > - mdelay(dpcd[DP_TRAINING_AUX_RD_INTERVAL] * 4); > + min = 400; > + > + usleep_range(min, min * 2); > } > EXPORT_SYMBOL(drm_dp_link_train_channel_eq_delay); > > -- > 2.5.0 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- Ville Syrjälä Intel OTC _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/dp: Do not busy-loop during link training 2015-12-14 14:48 ` Ville Syrjälä @ 2015-12-14 15:23 ` Thierry Reding 2015-12-14 16:30 ` Ville Syrjälä 0 siblings, 1 reply; 6+ messages in thread From: Thierry Reding @ 2015-12-14 15:23 UTC (permalink / raw) To: Ville Syrjälä; +Cc: dri-devel [-- Attachment #1.1: Type: text/plain, Size: 1347 bytes --] On Mon, Dec 14, 2015 at 04:48:09PM +0200, Ville Syrjälä wrote: > On Mon, Dec 14, 2015 at 02:21:56PM +0100, Thierry Reding wrote: > > From: Thierry Reding <treding@nvidia.com> > > > > Use microsecond sleeps for the clock recovery and channel equalization > > delays during link training. The duration of these delays can be from > > 100 us up to 16 ms. It is rude to busy-loop for that amount of time. > > Do you have some numbers on how this affects a typical link training > cycle? Not really. Sinks aren't required to provide a value here, in which case the specification says that a default of 100 us and 400 us should be used for clock recovery and channel equalization, respectively. If the sink provides an AUX_RD_INTERVAL value, it is used for both CR and CE (and is in units of 4 ms). Best case a typical link training cycle would therefore take something like 0.5 ms and worst case, since the number of retries should be limited to 5, it'd be around 5 * 16 ms = 80 ms. That's not counting the actual AUX transactions, though they should be pretty fast. Since this patch uses usleep_range(min, min * 2) the worst case now becomes ~ 160 ms. That's still not very much from a responsiveness point of view, but the upside is of course that there are no busy loops that could potentially hog the CPU. Thierry [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 819 bytes --] [-- Attachment #2: Type: text/plain, Size: 159 bytes --] _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/dp: Do not busy-loop during link training 2015-12-14 15:23 ` Thierry Reding @ 2015-12-14 16:30 ` Ville Syrjälä 2016-01-11 8:36 ` Thierry Reding 0 siblings, 1 reply; 6+ messages in thread From: Ville Syrjälä @ 2015-12-14 16:30 UTC (permalink / raw) To: Thierry Reding; +Cc: dri-devel On Mon, Dec 14, 2015 at 04:23:41PM +0100, Thierry Reding wrote: > On Mon, Dec 14, 2015 at 04:48:09PM +0200, Ville Syrjälä wrote: > > On Mon, Dec 14, 2015 at 02:21:56PM +0100, Thierry Reding wrote: > > > From: Thierry Reding <treding@nvidia.com> > > > > > > Use microsecond sleeps for the clock recovery and channel equalization > > > delays during link training. The duration of these delays can be from > > > 100 us up to 16 ms. It is rude to busy-loop for that amount of time. > > > > Do you have some numbers on how this affects a typical link training > > cycle? > > Not really. Sinks aren't required to provide a value here, in which case > the specification says that a default of 100 us and 400 us should be > used for clock recovery and channel equalization, respectively. If the > sink provides an AUX_RD_INTERVAL value, it is used for both CR and CE > (and is in units of 4 ms). Best case a typical link training cycle would > therefore take something like 0.5 ms and worst case, since the number of > retries should be limited to 5, it'd be around 5 * 16 ms = 80 ms. That's > not counting the actual AUX transactions, though they should be pretty > fast. > > Since this patch uses usleep_range(min, min * 2) the worst case now > becomes ~ 160 ms. Would be nice to have some *actual* numbers in the commit message, otherwise it's all just guesswork. > That's still not very much from a responsiveness point > of view, but the upside is of course that there are no busy loops that > could potentially hog the CPU. > > Thierry -- Ville Syrjälä Intel OTC _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/dp: Do not busy-loop during link training 2015-12-14 16:30 ` Ville Syrjälä @ 2016-01-11 8:36 ` Thierry Reding 2016-01-11 19:18 ` Ville Syrjälä 0 siblings, 1 reply; 6+ messages in thread From: Thierry Reding @ 2016-01-11 8:36 UTC (permalink / raw) To: Ville Syrjälä; +Cc: dri-devel [-- Attachment #1.1: Type: text/plain, Size: 1788 bytes --] On Mon, Dec 14, 2015 at 06:30:28PM +0200, Ville Syrjälä wrote: > On Mon, Dec 14, 2015 at 04:23:41PM +0100, Thierry Reding wrote: > > On Mon, Dec 14, 2015 at 04:48:09PM +0200, Ville Syrjälä wrote: > > > On Mon, Dec 14, 2015 at 02:21:56PM +0100, Thierry Reding wrote: > > > > From: Thierry Reding <treding@nvidia.com> > > > > > > > > Use microsecond sleeps for the clock recovery and channel equalization > > > > delays during link training. The duration of these delays can be from > > > > 100 us up to 16 ms. It is rude to busy-loop for that amount of time. > > > > > > Do you have some numbers on how this affects a typical link training > > > cycle? > > > > Not really. Sinks aren't required to provide a value here, in which case > > the specification says that a default of 100 us and 400 us should be > > used for clock recovery and channel equalization, respectively. If the > > sink provides an AUX_RD_INTERVAL value, it is used for both CR and CE > > (and is in units of 4 ms). Best case a typical link training cycle would > > therefore take something like 0.5 ms and worst case, since the number of > > retries should be limited to 5, it'd be around 5 * 16 ms = 80 ms. That's > > not counting the actual AUX transactions, though they should be pretty > > fast. > > > > Since this patch uses usleep_range(min, min * 2) the worst case now > > becomes ~ 160 ms. > > Would be nice to have some *actual* numbers in the commit message, > otherwise it's all just guesswork. I only have a limited range of test equipment. In the primary test-case, which is an eDP panel, the difference was ~4.5 ms for udelay()/mdelay() and ~5 ms for the usleep_range() case. I'll see if I can get one more test setup running for better comparison. Thierry [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 819 bytes --] [-- Attachment #2: Type: text/plain, Size: 159 bytes --] _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/dp: Do not busy-loop during link training 2016-01-11 8:36 ` Thierry Reding @ 2016-01-11 19:18 ` Ville Syrjälä 0 siblings, 0 replies; 6+ messages in thread From: Ville Syrjälä @ 2016-01-11 19:18 UTC (permalink / raw) To: Thierry Reding; +Cc: dri-devel On Mon, Jan 11, 2016 at 09:36:04AM +0100, Thierry Reding wrote: > On Mon, Dec 14, 2015 at 06:30:28PM +0200, Ville Syrjälä wrote: > > On Mon, Dec 14, 2015 at 04:23:41PM +0100, Thierry Reding wrote: > > > On Mon, Dec 14, 2015 at 04:48:09PM +0200, Ville Syrjälä wrote: > > > > On Mon, Dec 14, 2015 at 02:21:56PM +0100, Thierry Reding wrote: > > > > > From: Thierry Reding <treding@nvidia.com> > > > > > > > > > > Use microsecond sleeps for the clock recovery and channel equalization > > > > > delays during link training. The duration of these delays can be from > > > > > 100 us up to 16 ms. It is rude to busy-loop for that amount of time. > > > > > > > > Do you have some numbers on how this affects a typical link training > > > > cycle? > > > > > > Not really. Sinks aren't required to provide a value here, in which case > > > the specification says that a default of 100 us and 400 us should be > > > used for clock recovery and channel equalization, respectively. If the > > > sink provides an AUX_RD_INTERVAL value, it is used for both CR and CE > > > (and is in units of 4 ms). Best case a typical link training cycle would > > > therefore take something like 0.5 ms and worst case, since the number of > > > retries should be limited to 5, it'd be around 5 * 16 ms = 80 ms. That's > > > not counting the actual AUX transactions, though they should be pretty > > > fast. > > > > > > Since this patch uses usleep_range(min, min * 2) the worst case now > > > becomes ~ 160 ms. > > > > Would be nice to have some *actual* numbers in the commit message, > > otherwise it's all just guesswork. > > I only have a limited range of test equipment. In the primary test-case, > which is an eDP panel, the difference was ~4.5 ms for udelay()/mdelay() > and ~5 ms for the usleep_range() case. I'll see if I can get one more > test setup running for better comparison. Hmm. Depending on how many iterations it took 5ms could be quite a bit, or not much at all. I think ideally link training shouldn't take very many milliseconds if you don't have iterate the settings too much. I've not measured this myself in a while though, and I'm sure we're much worse at least on some platforms currently. But at least a few sample numbers in the commit message could then help people if they later have to look into why link training is taking as long as it is. -- Ville Syrjälä Intel OTC _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-01-11 19:18 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-12-14 13:21 [PATCH] drm/dp: Do not busy-loop during link training Thierry Reding 2015-12-14 14:48 ` Ville Syrjälä 2015-12-14 15:23 ` Thierry Reding 2015-12-14 16:30 ` Ville Syrjälä 2016-01-11 8:36 ` Thierry Reding 2016-01-11 19:18 ` Ville Syrjälä
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.