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 <johngro@google.com>
Change-Id: Ibfd361fe1cc2cbd4909489e3317fb12e005c6a75

7 files changed, 251 insertions, 58 deletions

diff --git a/services/common_time/Android.mk b/services/common_time/Android.mk
index e534d4987c45..0606ab4e8eb6 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 4a1b57e36ca1..6e914e590630 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<int32_t>(N);
+    time_to_cur_slew_.a_to_b_denom = static_cast<uint32_t>(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<int16_t>(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 b7362be33b14..20fbf96602c3 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<DiagThread> diag_thread_;
 #endif
diff --git a/services/common_time/common_time_server.cpp b/services/common_time/common_time_server.cpp
index 59576aa245e6..faf1b5fc71ab 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<uint32_t>(mCurTimeout.msecTillTimeout());
+        t2 = static_cast<uint32_t>(mClockRecovery.applyRateLimitedSlew());
+        timeout = static_cast<int>(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())
                 LOGE("handleTimeout failed");
             continue;
@@ -1325,29 +1333,6 @@ bool CommonTimeServer::sockaddrMatch(const sockaddr_storage& a1,
     }
 }
 
-void CommonTimeServer::TimeoutHelper::setTimeout(int msec) {
-    mTimeoutValid = (msec >= 0);
-    if (mTimeoutValid)
-        mEndTime = systemTime() +
-                   (static_cast<nsecs_t>(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<uint64_t>(std::numeric_limits<int>::max()))
-        return std::numeric_limits<int>::max();
-
-    return static_cast<int>(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 2aa2b4c43971..2401b77a34c9 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 000000000000..3ed259944cf1
--- /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<nsecs_t>(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<int>(delta);
+}
+
+}  // namespace android
diff --git a/services/common_time/utils.h b/services/common_time/utils.h
new file mode 100644
index 000000000000..d3545c972b07
--- /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 <stdint.h>
+#include <unistd.h>
+
+#include <utils/Timers.h>
+
+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__