From c7f57c6f9289d0e3aaecc0bca4ae7b6eed1c93d7 Mon Sep 17 00:00:00 2001 From: John Grossman Date: Tue, 26 Jun 2012 12:50:28 -0700 Subject: Fix for bug 6691452 Hand merge from ics-aah > Fix for bug 6691452 : DO NOT MERGE > > As it so happens, there seem to be panels out there who disapprove of > sudden changes in their HDMI clock rate. In particular, Sony LCD > panels made from around 2010-2011 (including the Sony GTV panel) seem > to dislike this behavior. When exposed to a large jump in the clock > rate (say from -100pmm to +100ppm in about 30mSec), they seem to > panic, blank their audio and video, and then resync. The whole > panic process takes about 2 seconds. > > The HDMI spec says that its clock jitter requirements are defined by > their differential signalling eye diagram requirements relative to an > "Ideal Recovery Clock" (see section 4.2.3.1 of the HDMI 1.3a spec). > Basically, if you pass the eye diagram tests, you pass the clock > jitter requirements. We have determined in lab that even being > extremely aggressive in our VCXO rate changes does not come even close > to violating the HDMI eye diagrams. Its just this era of Sony panels > which seem to be upset by this behavior. > > One way or the other, experiments which the GTV devices have seemed to > indicate that a full range sweep of the VCXO done in 10mSec steps over > anything faster than 190mSec can cause trouble. Adding a healthy > degree of margin to this finding, the fix is to limit the rate of VCXO > control change such that it never goes at a rate faster than > FullRange/300mSec. > > Change flagged as do not merge due to the code structure changes to master. > This will need to be merged by hand. > > Signed-off-by: John Grossman > Change-Id: Ibfd361fe1cc2cbd4909489e3317fb12e005c6a75 Change-Id: If62f791c826f1145262a6b546b1dc1f9776c37d8 Signed-off-by: John Grossman --- services/common_time/Android.mk | 3 +- services/common_time/clock_recovery.cpp | 124 +++++++++++++++++++++++++--- services/common_time/clock_recovery.h | 25 +++++- services/common_time/common_time_server.cpp | 45 ++++------ services/common_time/common_time_server.h | 15 +--- services/common_time/utils.cpp | 49 +++++++++++ services/common_time/utils.h | 48 +++++++++++ 7 files changed, 251 insertions(+), 58 deletions(-) create mode 100644 services/common_time/utils.cpp create mode 100644 services/common_time/utils.h (limited to 'services/common_time') diff --git a/services/common_time/Android.mk b/services/common_time/Android.mk index e534d49..0606ab4 100644 --- a/services/common_time/Android.mk +++ b/services/common_time/Android.mk @@ -14,7 +14,8 @@ LOCAL_SRC_FILES := \ common_time_server_packets.cpp \ clock_recovery.cpp \ common_clock.cpp \ - main.cpp + main.cpp \ + utils.cpp # Uncomment to enable vesbose logging and debug service. #TIME_SERVICE_DEBUG=true diff --git a/services/common_time/clock_recovery.cpp b/services/common_time/clock_recovery.cpp index 6c98d32..4a5b2ef 100644 --- a/services/common_time/clock_recovery.cpp +++ b/services/common_time/clock_recovery.cpp @@ -53,6 +53,21 @@ ClockRecoveryLoop::ClockRecoveryLoop(LocalClock* local_clock, local_clock_can_slew_ = local_clock_->initCheck() && (local_clock_->setLocalSlew(0) == OK); + tgt_correction_ = 0; + cur_correction_ = 0; + + // Precompute the max rate at which we are allowed to change the VCXO + // control. + uint64_t N = 0x10000ull * 1000ull; + uint64_t D = local_clock_->getLocalFreq() * kMinFullRangeSlewChange_mSec; + LinearTransform::reduce(&N, &D); + while ((N > INT32_MAX) || (D > UINT32_MAX)) { + N >>= 1; + D >>= 1; + LinearTransform::reduce(&N, &D); + } + time_to_cur_slew_.a_to_b_numer = static_cast(N); + time_to_cur_slew_.a_to_b_denom = static_cast(D); reset(true, true); @@ -85,6 +100,9 @@ const int64_t ClockRecoveryLoop::panic_thresh_ = 50000; const int64_t ClockRecoveryLoop::control_thresh_ = 10000; const float ClockRecoveryLoop::COmin = -100.0f; const float ClockRecoveryLoop::COmax = 100.0f; +const uint32_t ClockRecoveryLoop::kMinFullRangeSlewChange_mSec = 300; +const int ClockRecoveryLoop::kSlewChangeStepPeriod_mSec = 10; + void ClockRecoveryLoop::reset(bool position, bool frequency) { Mutex::Autolock lock(&lock_); @@ -144,7 +162,7 @@ bool ClockRecoveryLoop::pushDisciplineEvent(int64_t local_time, int64_t observed_common; int64_t delta; float delta_f, dCO; - int32_t correction_cur; + int32_t tgt_correction; if (OK != common_clock_->localToCommon(local_time, &observed_common)) { // Since we just checked to make certain that this conversion was valid, @@ -254,23 +272,20 @@ bool ClockRecoveryLoop::pushDisciplineEvent(int64_t local_time, // Convert PPM to 16-bit int range. Add some guard band (-0.01) so we // don't get fp weirdness. - correction_cur = CO * 327.66; + tgt_correction = CO * 327.66; // If there was a change in the amt of correction to use, update the // system. - if (correction_cur_ != correction_cur) { - correction_cur_ = correction_cur; - applySlew(); - } + setTargetCorrection_l(tgt_correction); - LOG_TS("clock_loop %lld %f %f %f %d\n", raw_delta, delta_f, CO, CObias, correction_cur); + LOG_TS("clock_loop %lld %f %f %f %d\n", raw_delta, delta_f, CO, CObias, tgt_correction); #ifdef TIME_SERVICE_DEBUG diag_thread_->pushDisciplineEvent( local_time, observed_common, nominal_common_time, - correction_cur, + tgt_correction, rtt); #endif @@ -298,24 +313,109 @@ void ClockRecoveryLoop::reset_l(bool position, bool frequency) { last_delta_valid_ = false; last_delta_ = 0; last_delta_f_ = 0.0; - correction_cur_ = 0x0; CO = 0.0f; lastCObias = CObias = 0.0f; - applySlew(); + setTargetCorrection_l(0); + applySlew_l(); } filter_wr_ = 0; filter_full_ = false; } -void ClockRecoveryLoop::applySlew() { +void ClockRecoveryLoop::setTargetCorrection_l(int32_t tgt) { + // When we make a change to the slew rate, we need to be careful to not + // change it too quickly as it can anger some HDMI sinks out there, notably + // some Sony panels from the 2010-2011 timeframe. From experimenting with + // some of these sinks, it seems like swinging from one end of the range to + // another in less that 190mSec or so can start to cause trouble. Adding in + // a hefty margin, we limit the system to a full range sweep in no less than + // 300mSec. + if (tgt_correction_ != tgt) { + int64_t now = local_clock_->getLocalTime(); + status_t res; + + tgt_correction_ = tgt; + + // Set up the transformation to figure out what the slew should be at + // any given point in time in the future. + time_to_cur_slew_.a_zero = now; + time_to_cur_slew_.b_zero = cur_correction_; + + // Make sure the sign of the slope is headed in the proper direction. + bool needs_increase = (cur_correction_ < tgt_correction_); + bool is_increasing = (time_to_cur_slew_.a_to_b_numer > 0); + if (( needs_increase && !is_increasing) || + (!needs_increase && is_increasing)) { + time_to_cur_slew_.a_to_b_numer = -time_to_cur_slew_.a_to_b_numer; + } + + // Finally, figure out when the change will be finished and start the + // slew operation. + time_to_cur_slew_.doReverseTransform(tgt_correction_, + &slew_change_end_time_); + + applySlew_l(); + } +} + +bool ClockRecoveryLoop::applySlew_l() { + bool ret = true; + + // If cur == tgt, there is no ongoing sleq rate change and we are already + // finished. + if (cur_correction_ == tgt_correction_) + goto bailout; + if (local_clock_can_slew_) { - local_clock_->setLocalSlew(correction_cur_); + int64_t now = local_clock_->getLocalTime(); + int64_t tmp; + + if (now >= slew_change_end_time_) { + cur_correction_ = tgt_correction_; + next_slew_change_timeout_.setTimeout(-1); + } else { + time_to_cur_slew_.doForwardTransform(now, &tmp); + + if (tmp > INT16_MAX) + cur_correction_ = INT16_MAX; + else if (tmp < INT16_MIN) + cur_correction_ = INT16_MIN; + else + cur_correction_ = static_cast(tmp); + + next_slew_change_timeout_.setTimeout(kSlewChangeStepPeriod_mSec); + ret = false; + } + + local_clock_->setLocalSlew(cur_correction_); } else { + // Since we are not actually changing the rate of a HW clock, we don't + // need to worry to much about changing the slew rate so fast that we + // anger any downstream HDMI devices. + cur_correction_ = tgt_correction_; + next_slew_change_timeout_.setTimeout(-1); + // The SW clock recovery implemented by the common clock class expects // values expressed in PPM. CO is in ppm. common_clock_->setSlew(local_clock_->getLocalTime(), CO); } + +bailout: + return ret; +} + +int ClockRecoveryLoop::applyRateLimitedSlew() { + Mutex::Autolock lock(&lock_); + + int ret = next_slew_change_timeout_.msecTillTimeout(); + if (!ret) { + if (applySlew_l()) + next_slew_change_timeout_.setTimeout(-1); + ret = next_slew_change_timeout_.msecTillTimeout(); + } + + return ret; } } // namespace android diff --git a/services/common_time/clock_recovery.h b/services/common_time/clock_recovery.h index b7362be..20fbf96 100644 --- a/services/common_time/clock_recovery.h +++ b/services/common_time/clock_recovery.h @@ -26,6 +26,8 @@ #include "diag_thread.h" #endif +#include "utils.h" + namespace android { class CommonClock; @@ -42,6 +44,11 @@ class ClockRecoveryLoop { int64_t data_point_rtt); int32_t getLastErrorEstimate(); + // Applies the next step in any ongoing slew change operation. Returns a + // timeout suitable for use with poll/select indicating the number of mSec + // until the next change should be applied. + int applyRateLimitedSlew(); + private: // Tuned using the "Good Gain" method. @@ -87,7 +94,8 @@ class ClockRecoveryLoop { static uint32_t findMinRTTNdx(DisciplineDataPoint* data, uint32_t count); void reset_l(bool position, bool frequency); - void applySlew(); + void setTargetCorrection_l(int32_t tgt); + bool applySlew_l(); // The local clock HW abstraction we use as the basis for common time. LocalClock* local_clock_; @@ -104,7 +112,11 @@ class ClockRecoveryLoop { int32_t last_delta_; float last_delta_f_; int32_t integrated_error_; - int32_t correction_cur_; + int32_t tgt_correction_; + int32_t cur_correction_; + LinearTransform time_to_cur_slew_; + int64_t slew_change_end_time_; + Timeout next_slew_change_timeout_; // Contoller Output. float CO; @@ -128,6 +140,15 @@ class ClockRecoveryLoop { DisciplineDataPoint startup_filter_data_[kStartupFilterSize]; uint32_t startup_filter_wr_; + // Minimum number of milliseconds over which we allow a full range change + // (from rail to rail) of the VCXO control signal. This is the rate + // limiting factor which keeps us from changing the clock rate so fast that + // we get in trouble with certain HDMI sinks. + static const uint32_t kMinFullRangeSlewChange_mSec; + + // How much time (in msec) to wait + static const int kSlewChangeStepPeriod_mSec; + #ifdef TIME_SERVICE_DEBUG sp diag_thread_; #endif diff --git a/services/common_time/common_time_server.cpp b/services/common_time/common_time_server.cpp index 7a4986b..4e5d16e 100644 --- a/services/common_time/common_time_server.cpp +++ b/services/common_time/common_time_server.cpp @@ -202,9 +202,11 @@ bool CommonTimeServer::runStateMachine_l() { // run the state machine while (!exitPending()) { struct pollfd pfds[2]; - int rc; + int rc, timeout; int eventCnt = 0; int64_t wakeupTime; + uint32_t t1, t2; + bool needHandleTimeout = false; // We are always interested in our wakeup FD. pfds[eventCnt].fd = mWakeupThreadFD; @@ -221,10 +223,14 @@ bool CommonTimeServer::runStateMachine_l() { eventCnt++; } + t1 = static_cast(mCurTimeout.msecTillTimeout()); + t2 = static_cast(mClockRecovery.applyRateLimitedSlew()); + timeout = static_cast(t1 < t2 ? t1 : t2); + // Note, we were holding mLock when this function was called. We // release it only while we are blocking and hold it at all other times. mLock.unlock(); - rc = poll(pfds, eventCnt, mCurTimeout.msecTillTimeout()); + rc = poll(pfds, eventCnt, timeout); wakeupTime = mLocalClock.getLocalTime(); mLock.lock(); @@ -238,8 +244,11 @@ bool CommonTimeServer::runStateMachine_l() { return false; } - if (rc == 0) - mCurTimeout.setTimeout(kInfiniteTimeout); + if (rc == 0) { + needHandleTimeout = !mCurTimeout.msecTillTimeout(); + if (needHandleTimeout) + mCurTimeout.setTimeout(kInfiniteTimeout); + } // Were we woken up on purpose? If so, clear the eventfd with a read. if (pfds[0].revents) @@ -336,9 +345,8 @@ bool CommonTimeServer::runStateMachine_l() { continue; } - // Did we wakeup with no signalled events across all of our FDs? If so, - // we must have hit our timeout. - if (rc == 0) { + // Time to handle the timeouts? + if (needHandleTimeout) { if (!handleTimeout()) ALOGE("handleTimeout failed"); continue; @@ -1326,29 +1334,6 @@ bool CommonTimeServer::sockaddrMatch(const sockaddr_storage& a1, } } -void CommonTimeServer::TimeoutHelper::setTimeout(int msec) { - mTimeoutValid = (msec >= 0); - if (mTimeoutValid) - mEndTime = systemTime() + - (static_cast(msec) * 1000000); -} - -int CommonTimeServer::TimeoutHelper::msecTillTimeout() { - if (!mTimeoutValid) - return kInfiniteTimeout; - - nsecs_t now = systemTime(); - if (now >= mEndTime) - return 0; - - uint64_t deltaMsec = (((mEndTime - now) + 999999) / 1000000); - - if (deltaMsec > static_cast(MAX_INT)) - return MAX_INT; - - return static_cast(deltaMsec); -} - bool CommonTimeServer::shouldPanicNotGettingGoodData() { if (mClient_FirstSyncTX) { int64_t now = mLocalClock.getLocalTime(); diff --git a/services/common_time/common_time_server.h b/services/common_time/common_time_server.h index a0f549f..b2ad3f0 100644 --- a/services/common_time/common_time_server.h +++ b/services/common_time/common_time_server.h @@ -28,6 +28,7 @@ #include "clock_recovery.h" #include "common_clock.h" #include "common_time_server_packets.h" +#include "utils.h" #define RTT_LOG_SIZE 30 @@ -104,18 +105,6 @@ class CommonTimeServer : public Thread { int64_t rxTimes[RTT_LOG_SIZE]; }; - class TimeoutHelper { - public: - TimeoutHelper() : mTimeoutValid(false) { } - - void setTimeout(int msec); - int msecTillTimeout(); - - private: - bool mTimeoutValid; - nsecs_t mEndTime; - }; - bool threadLoop(); bool runStateMachine_l(); @@ -194,7 +183,7 @@ class CommonTimeServer : public Thread { bool shouldPanicNotGettingGoodData(); // Helper to keep track of the state machine's current timeout - TimeoutHelper mCurTimeout; + Timeout mCurTimeout; // common clock, local clock abstraction, and clock recovery loop CommonClock mCommonClock; diff --git a/services/common_time/utils.cpp b/services/common_time/utils.cpp new file mode 100644 index 0000000..3ed2599 --- /dev/null +++ b/services/common_time/utils.cpp @@ -0,0 +1,49 @@ +/* + * Copyright (C) 2012 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "utils.h" + +namespace android { + +void Timeout::setTimeout(int msec) { + if (msec < 0) { + mSystemEndTime = 0; + return; + } + + mSystemEndTime = systemTime() + (static_cast(msec) * 1000000); +} + +int Timeout::msecTillTimeout(nsecs_t nowTime) { + if (!mSystemEndTime) { + return -1; + } + + if (mSystemEndTime < nowTime) { + return 0; + } + + nsecs_t delta = mSystemEndTime - nowTime; + delta += 999999; + delta /= 1000000; + if (delta > 0x7FFFFFFF) { + return 0x7FFFFFFF; + } + + return static_cast(delta); +} + +} // namespace android diff --git a/services/common_time/utils.h b/services/common_time/utils.h new file mode 100644 index 0000000..d3545c9 --- /dev/null +++ b/services/common_time/utils.h @@ -0,0 +1,48 @@ +/* + * Copyright (C) 2012 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef __UTILS_H__ +#define __UTILS_H__ + +#include +#include + +#include + +namespace android { + +class Timeout { + public: + Timeout() : mSystemEndTime(0) { } + + // Set a timeout which should occur msec milliseconds from now. + // Negative values will cancel any current timeout; + void setTimeout(int msec); + + // Return the number of milliseconds until the timeout occurs, or -1 if + // no timeout is scheduled. + int msecTillTimeout(nsecs_t nowTime); + int msecTillTimeout() { return msecTillTimeout(systemTime()); } + + private: + // The systemTime() at which the timeout will be complete, or 0 if no + // timeout is currently scheduled. + nsecs_t mSystemEndTime; +}; + +} // namespace android + +#endif // __UTILS_H__ -- cgit v1.1