.../platform/chrome/cros_ec_sensorhub_ring.c | 70 +++++++++++++++---- 1 file changed, 55 insertions(+), 15 deletions(-)
From: Scott Collyer <scollyer@google.com>
This CL makes 5 specific changes to the clock drift estimation algorithm
and timestamp filtering to resolve monotonic backward skews and OOO errors
during batching:
1. Clamp the calculated drift slope (m) to a realistic maximum bound
(+/- 200 PPM). Huge jitter spikes previously produced astronomical
slopes that corrupted the median filter history.
2. Detect ODR (Output Data Rate) or Mode changes (where dx changes
by more than 10x) and actively reset the history. This prevents
mixing noisy active streaming samples with larger batched samples.
3. Increase the TS_HISTORY_BORED_US gap threshold from 0.5s to 2.0s
to accommodate normal batching flush intervals (which can be 1-2
seconds long) without constantly tearing down the filter.
4. Correctly bound the array index logic during history propagation
so we cleanly drop the oldest elements without array overrun.
5. Fully drop out-of-order samples to guarantee monotonicity instead of
clamping them to the last timestamp. Clamped samples previously caused
stale artifacts downstream during identical timestamp spreading.
Signed-off-by: Scott Collyer <scollyer@google.com>
Signed-off-by: Satyam Gupta <sgsatyam@google.com>
---
.../platform/chrome/cros_ec_sensorhub_ring.c | 70 +++++++++++++++----
1 file changed, 55 insertions(+), 15 deletions(-)
diff --git a/drivers/platform/chrome/cros_ec_sensorhub_ring.c b/drivers/platform/chrome/cros_ec_sensorhub_ring.c
index 1205219515d6..9e1d430eb9d4 100644
--- a/drivers/platform/chrome/cros_ec_sensorhub_ring.c
+++ b/drivers/platform/chrome/cros_ec_sensorhub_ring.c
@@ -22,15 +22,21 @@
/* Precision of fixed point for the m values from the filter */
#define M_PRECISION BIT(23)
+#define MAX_DRIFT_PPM 200LL
+#define MAX_M ((s64)(MAX_DRIFT_PPM * M_PRECISION / 1000000))
/* Only activate the filter once we have at least this many elements. */
#define TS_HISTORY_THRESHOLD 8
/*
* If we don't have any history entries for this long, empty the filter to
- * make sure there are no big discontinuities.
+ * make sure there are no big discontinuities (e.g. after suspend or long idle).
+ *
+ * This is set to 2 seconds to accommodate normal batching flush intervals
+ * (which can be up to 1-2 seconds) without constantly resetting the filter,
+ * while still ensuring a reset occurs during actual long gaps or suspend.
*/
-#define TS_HISTORY_BORED_US 500000
+#define TS_HISTORY_BORED_US 2000000 // 2 seconds
/* To measure by how much the filter is overshooting, if it happens. */
#define FUTURE_TS_ANALYTICS_COUNT_MAX 100
@@ -259,7 +265,7 @@ cros_ec_sensor_ring_ts_filter_update(struct cros_ec_sensors_ts_filter_state
s64 m; /* stored as *M_PRECISION */
s64 *m_history_copy = state->temp_buf;
s64 *error = state->temp_buf;
- int i;
+ int i, start;
/* we trust b the most, that'll be our independent variable */
x = b;
@@ -271,13 +277,35 @@ cros_ec_sensor_ring_ts_filter_update(struct cros_ec_sensors_ts_filter_state
return; /* we already have this irq in the history */
dy = (state->y_history[0] + state->y_offset) - y;
m = div64_s64(dy * M_PRECISION, dx);
+ /*
+ * Clamp the calculated slope to a realistic range (+/- 200 ppm).
+ * Temporary large jitter spikes can produce huge slopes (e.g. +/- 20000 ppm)
+ * that would otherwise corrupt the median filter history.
+ */
+ m = clamp(m, -MAX_M, MAX_M);
+
+ /*
+ * Detect ODR (Output Data Rate) or Mode changes (e.g., transition from
+ * active streaming to low-frequency batching). If the time delta (dx)
+ * changes by more than 10x in either direction, reset the filter history
+ * to prevent mixing noisy active samples with batching samples.
+ */
+ if (state->history_len > 1) {
+ s64 prev_dx = -state->x_history[1];
+ if (prev_dx > 0 && (abs(dx) > 10 * prev_dx || prev_dx > 10 * abs(dx))) {
+ state->history_len = 0;
+ m = 0;
+ }
+ }
/* Empty filter if we haven't seen any action in a while. */
if (-dx > TS_HISTORY_BORED_US)
state->history_len = 0;
/* Move everything over, also update offset to all absolute coords .*/
- for (i = state->history_len - 1; i >= 1; i--) {
+ start = (state->history_len == CROS_EC_SENSORHUB_TS_HISTORY_SIZE) ?
+ state->history_len - 1 : state->history_len;
+ for (i = start; i >= 1; i--) {
state->x_history[i] = state->x_history[i - 1] + dx;
state->y_history[i] = state->y_history[i - 1] + dy;
@@ -380,7 +408,7 @@ cros_ec_sensor_ring_fix_overflow(s64 *ts,
state->last = *ts;
}
-static void
+static bool
cros_ec_sensor_ring_check_for_past_timestamp(struct cros_ec_sensorhub
*sensorhub,
struct cros_ec_sensors_ring_sample
@@ -388,17 +416,27 @@ cros_ec_sensor_ring_check_for_past_timestamp(struct cros_ec_sensorhub
{
const u8 sensor_id = sample->sensor_id;
- /* If this event is earlier than one we saw before... */
+ /*
+ * If this event is strictly earlier than one we saw before, it is truly
+ * out-of-order and must be dropped to preserve monotonicity.
+ *
+ * We intentionally allow duplicate timestamps (==) to pass through here
+ * because the EC/ISH may report a batch of samples with the same raw
+ * timestamp. These duplicates are required by the downstream spreading
+ * logic (cros_ec_sensor_ring_spread_add) to interpolate unique,
+ * monotonic timestamps for each sample in the batch.
+ */
if (sensorhub->batch_state[sensor_id].newest_sensor_event >
- sample->timestamp)
- /* mark it for spreading. */
- sample->timestamp =
- sensorhub->batch_state[sensor_id].last_ts;
- else
- sensorhub->batch_state[sensor_id].newest_sensor_event =
- sample->timestamp;
+ sample->timestamp) {
+ return false;
+ }
+
+ sensorhub->batch_state[sensor_id].newest_sensor_event =
+ sample->timestamp;
+ return true;
}
+
/**
* cros_ec_sensor_ring_process_event() - Process one EC FIFO event
*
@@ -529,8 +567,10 @@ cros_ec_sensor_ring_process_event(struct cros_ec_sensorhub *sensorhub,
for (axis = 0; axis < 3; axis++)
out->vector[axis] = in->data[axis];
- if (sensorhub->tight_timestamps)
- cros_ec_sensor_ring_check_for_past_timestamp(sensorhub, out);
+ if (sensorhub->tight_timestamps) {
+ if (!cros_ec_sensor_ring_check_for_past_timestamp(sensorhub, out))
+ return false;
+ }
return true;
}
--
2.55.0.229.g6434b31f56-googFrom: Scott Collyer <scollyer@google.com>
This CL makes 5 specific changes to the clock drift estimation algorithm
and timestamp filtering to resolve monotonic backward skews and OOO errors
during batching:
1. Clamp the calculated drift slope (m) to a realistic maximum bound
(+/- 200 PPM). Huge jitter spikes previously produced astronomical
slopes that corrupted the median filter history.
2. Detect ODR (Output Data Rate) or Mode changes (where dx changes
by more than 10x) and actively reset the history. This prevents
mixing noisy active streaming samples with larger batched samples.
3. Increase the TS_HISTORY_BORED_US gap threshold from 0.5s to 2.0s
to accommodate normal batching flush intervals (which can be 1-2
seconds long) without constantly tearing down the filter.
4. Correctly bound the array index logic during history propagation
so we cleanly drop the oldest elements without array overrun.
5. Fully drop out-of-order samples to guarantee monotonicity instead of
clamping them to the last timestamp. Clamped samples previously caused
stale artifacts downstream during identical timestamp spreading.
Signed-off-by: Scott Collyer <scollyer@google.com>
Signed-off-by: SATYAM GUPTA <sgsatyam@google.com>
---
.../platform/chrome/cros_ec_sensorhub_ring.c | 71 +++++++++++++++----
1 file changed, 56 insertions(+), 15 deletions(-)
diff --git a/drivers/platform/chrome/cros_ec_sensorhub_ring.c b/drivers/platform/chrome/cros_ec_sensorhub_ring.c
index 1205219515d6..3f57513b4d5f 100644
--- a/drivers/platform/chrome/cros_ec_sensorhub_ring.c
+++ b/drivers/platform/chrome/cros_ec_sensorhub_ring.c
@@ -22,15 +22,21 @@
/* Precision of fixed point for the m values from the filter */
#define M_PRECISION BIT(23)
+#define MAX_DRIFT_PPM 200LL
+#define MAX_M ((s64)(MAX_DRIFT_PPM * M_PRECISION / 1000000))
/* Only activate the filter once we have at least this many elements. */
#define TS_HISTORY_THRESHOLD 8
/*
* If we don't have any history entries for this long, empty the filter to
- * make sure there are no big discontinuities.
+ * make sure there are no big discontinuities (e.g. after suspend or long idle).
+ *
+ * This is set to 2 seconds to accommodate normal batching flush intervals
+ * (which can be up to 1-2 seconds) without constantly resetting the filter,
+ * while still ensuring a reset occurs during actual long gaps or suspend.
*/
-#define TS_HISTORY_BORED_US 500000
+#define TS_HISTORY_BORED_US 2000000 // 2 seconds
/* To measure by how much the filter is overshooting, if it happens. */
#define FUTURE_TS_ANALYTICS_COUNT_MAX 100
@@ -259,7 +265,7 @@ cros_ec_sensor_ring_ts_filter_update(struct cros_ec_sensors_ts_filter_state
s64 m; /* stored as *M_PRECISION */
s64 *m_history_copy = state->temp_buf;
s64 *error = state->temp_buf;
- int i;
+ int i, start;
/* we trust b the most, that'll be our independent variable */
x = b;
@@ -271,13 +277,36 @@ cros_ec_sensor_ring_ts_filter_update(struct cros_ec_sensors_ts_filter_state
return; /* we already have this irq in the history */
dy = (state->y_history[0] + state->y_offset) - y;
m = div64_s64(dy * M_PRECISION, dx);
+ /*
+ * Clamp the calculated slope to a realistic range (+/- 200 ppm).
+ * Temporary large jitter spikes can produce huge slopes (e.g. +/- 20000 ppm)
+ * that would otherwise corrupt the median filter history.
+ */
+ m = clamp(m, -MAX_M, MAX_M);
+
+ /*
+ * Detect ODR (Output Data Rate) or Mode changes (e.g., transition from
+ * active streaming to low-frequency batching). If the time delta (dx)
+ * changes by more than 10x in either direction, reset the filter history
+ * to prevent mixing noisy active samples with batching samples.
+ */
+ if (state->history_len > 1) {
+ s64 prev_dx = -state->x_history[1];
+
+ if (prev_dx > 0 && (abs(dx) > 10 * prev_dx || prev_dx > 10 * abs(dx))) {
+ state->history_len = 0;
+ m = 0;
+ }
+ }
/* Empty filter if we haven't seen any action in a while. */
if (-dx > TS_HISTORY_BORED_US)
state->history_len = 0;
/* Move everything over, also update offset to all absolute coords .*/
- for (i = state->history_len - 1; i >= 1; i--) {
+ start = (state->history_len == CROS_EC_SENSORHUB_TS_HISTORY_SIZE) ?
+ state->history_len - 1 : state->history_len;
+ for (i = start; i >= 1; i--) {
state->x_history[i] = state->x_history[i - 1] + dx;
state->y_history[i] = state->y_history[i - 1] + dy;
@@ -380,7 +409,7 @@ cros_ec_sensor_ring_fix_overflow(s64 *ts,
state->last = *ts;
}
-static void
+static bool
cros_ec_sensor_ring_check_for_past_timestamp(struct cros_ec_sensorhub
*sensorhub,
struct cros_ec_sensors_ring_sample
@@ -388,17 +417,27 @@ cros_ec_sensor_ring_check_for_past_timestamp(struct cros_ec_sensorhub
{
const u8 sensor_id = sample->sensor_id;
- /* If this event is earlier than one we saw before... */
+ /*
+ * If this event is strictly earlier than one we saw before, it is truly
+ * out-of-order and must be dropped to preserve monotonicity.
+ *
+ * We intentionally allow duplicate timestamps (==) to pass through here
+ * because the EC/ISH may report a batch of samples with the same raw
+ * timestamp. These duplicates are required by the downstream spreading
+ * logic (cros_ec_sensor_ring_spread_add) to interpolate unique,
+ * monotonic timestamps for each sample in the batch.
+ */
if (sensorhub->batch_state[sensor_id].newest_sensor_event >
- sample->timestamp)
- /* mark it for spreading. */
- sample->timestamp =
- sensorhub->batch_state[sensor_id].last_ts;
- else
- sensorhub->batch_state[sensor_id].newest_sensor_event =
- sample->timestamp;
+ sample->timestamp) {
+ return false;
+ }
+
+ sensorhub->batch_state[sensor_id].newest_sensor_event =
+ sample->timestamp;
+ return true;
}
+
/**
* cros_ec_sensor_ring_process_event() - Process one EC FIFO event
*
@@ -529,8 +568,10 @@ cros_ec_sensor_ring_process_event(struct cros_ec_sensorhub *sensorhub,
for (axis = 0; axis < 3; axis++)
out->vector[axis] = in->data[axis];
- if (sensorhub->tight_timestamps)
- cros_ec_sensor_ring_check_for_past_timestamp(sensorhub, out);
+ if (sensorhub->tight_timestamps) {
+ if (!cros_ec_sensor_ring_check_for_past_timestamp(sensorhub, out))
+ return false;
+ }
return true;
}
--
2.55.0.229.g6434b31f56-googFrom: Scott Collyer <scollyer@google.com>
This CL makes 5 specific changes to the clock drift estimation algorithm
and timestamp filtering to resolve monotonic backward skews and OOO errors
during batching:
1. Clamp the calculated drift slope (m) to a realistic maximum bound
(+/- 200 PPM). Huge jitter spikes previously produced astronomical
slopes that corrupted the median filter history.
2. Detect ODR (Output Data Rate) or Mode changes (where dx changes
by more than 10x) and actively reset the history. This prevents
mixing noisy active streaming samples with larger batched samples.
3. Increase the TS_HISTORY_BORED_US gap threshold from 0.5s to 2.0s
to accommodate normal batching flush intervals (which can be 1-2
seconds long) without constantly tearing down the filter.
4. Correctly bound the array index logic during history propagation
so we cleanly drop the oldest elements without array overrun.
5. Fully drop out-of-order samples to guarantee monotonicity instead of
clamping them to the last timestamp. Clamped samples previously caused
stale artifacts downstream during identical timestamp spreading.
Signed-off-by: Scott Collyer <scollyer@google.com>
Signed-off-by: SATYAM GUPTA <sgsatyam@google.com>
---
.../platform/chrome/cros_ec_sensorhub_ring.c | 71 +++++++++++++++----
1 file changed, 56 insertions(+), 15 deletions(-)
diff --git a/drivers/platform/chrome/cros_ec_sensorhub_ring.c b/drivers/platform/chrome/cros_ec_sensorhub_ring.c
index 1205219515d6..3f57513b4d5f 100644
--- a/drivers/platform/chrome/cros_ec_sensorhub_ring.c
+++ b/drivers/platform/chrome/cros_ec_sensorhub_ring.c
@@ -22,15 +22,21 @@
/* Precision of fixed point for the m values from the filter */
#define M_PRECISION BIT(23)
+#define MAX_DRIFT_PPM 200LL
+#define MAX_M ((s64)(MAX_DRIFT_PPM * M_PRECISION / 1000000))
/* Only activate the filter once we have at least this many elements. */
#define TS_HISTORY_THRESHOLD 8
/*
* If we don't have any history entries for this long, empty the filter to
- * make sure there are no big discontinuities.
+ * make sure there are no big discontinuities (e.g. after suspend or long idle).
+ *
+ * This is set to 2 seconds to accommodate normal batching flush intervals
+ * (which can be up to 1-2 seconds) without constantly resetting the filter,
+ * while still ensuring a reset occurs during actual long gaps or suspend.
*/
-#define TS_HISTORY_BORED_US 500000
+#define TS_HISTORY_BORED_US 2000000 // 2 seconds
/* To measure by how much the filter is overshooting, if it happens. */
#define FUTURE_TS_ANALYTICS_COUNT_MAX 100
@@ -259,7 +265,7 @@ cros_ec_sensor_ring_ts_filter_update(struct cros_ec_sensors_ts_filter_state
s64 m; /* stored as *M_PRECISION */
s64 *m_history_copy = state->temp_buf;
s64 *error = state->temp_buf;
- int i;
+ int i, start;
/* we trust b the most, that'll be our independent variable */
x = b;
@@ -271,13 +277,36 @@ cros_ec_sensor_ring_ts_filter_update(struct cros_ec_sensors_ts_filter_state
return; /* we already have this irq in the history */
dy = (state->y_history[0] + state->y_offset) - y;
m = div64_s64(dy * M_PRECISION, dx);
+ /*
+ * Clamp the calculated slope to a realistic range (+/- 200 ppm).
+ * Temporary large jitter spikes can produce huge slopes (e.g. +/- 20000 ppm)
+ * that would otherwise corrupt the median filter history.
+ */
+ m = clamp(m, -MAX_M, MAX_M);
+
+ /*
+ * Detect ODR (Output Data Rate) or Mode changes (e.g., transition from
+ * active streaming to low-frequency batching). If the time delta (dx)
+ * changes by more than 10x in either direction, reset the filter history
+ * to prevent mixing noisy active samples with batching samples.
+ */
+ if (state->history_len > 1) {
+ s64 prev_dx = -state->x_history[1];
+
+ if (prev_dx > 0 && (abs(dx) > 10 * prev_dx || prev_dx > 10 * abs(dx))) {
+ state->history_len = 0;
+ m = 0;
+ }
+ }
/* Empty filter if we haven't seen any action in a while. */
if (-dx > TS_HISTORY_BORED_US)
state->history_len = 0;
/* Move everything over, also update offset to all absolute coords .*/
- for (i = state->history_len - 1; i >= 1; i--) {
+ start = (state->history_len == CROS_EC_SENSORHUB_TS_HISTORY_SIZE) ?
+ state->history_len - 1 : state->history_len;
+ for (i = start; i >= 1; i--) {
state->x_history[i] = state->x_history[i - 1] + dx;
state->y_history[i] = state->y_history[i - 1] + dy;
@@ -380,7 +409,7 @@ cros_ec_sensor_ring_fix_overflow(s64 *ts,
state->last = *ts;
}
-static void
+static bool
cros_ec_sensor_ring_check_for_past_timestamp(struct cros_ec_sensorhub
*sensorhub,
struct cros_ec_sensors_ring_sample
@@ -388,17 +417,27 @@ cros_ec_sensor_ring_check_for_past_timestamp(struct cros_ec_sensorhub
{
const u8 sensor_id = sample->sensor_id;
- /* If this event is earlier than one we saw before... */
+ /*
+ * If this event is strictly earlier than one we saw before, it is truly
+ * out-of-order and must be dropped to preserve monotonicity.
+ *
+ * We intentionally allow duplicate timestamps (==) to pass through here
+ * because the EC/ISH may report a batch of samples with the same raw
+ * timestamp. These duplicates are required by the downstream spreading
+ * logic (cros_ec_sensor_ring_spread_add) to interpolate unique,
+ * monotonic timestamps for each sample in the batch.
+ */
if (sensorhub->batch_state[sensor_id].newest_sensor_event >
- sample->timestamp)
- /* mark it for spreading. */
- sample->timestamp =
- sensorhub->batch_state[sensor_id].last_ts;
- else
- sensorhub->batch_state[sensor_id].newest_sensor_event =
- sample->timestamp;
+ sample->timestamp) {
+ return false;
+ }
+
+ sensorhub->batch_state[sensor_id].newest_sensor_event =
+ sample->timestamp;
+ return true;
}
+
/**
* cros_ec_sensor_ring_process_event() - Process one EC FIFO event
*
@@ -529,8 +568,10 @@ cros_ec_sensor_ring_process_event(struct cros_ec_sensorhub *sensorhub,
for (axis = 0; axis < 3; axis++)
out->vector[axis] = in->data[axis];
- if (sensorhub->tight_timestamps)
- cros_ec_sensor_ring_check_for_past_timestamp(sensorhub, out);
+ if (sensorhub->tight_timestamps) {
+ if (!cros_ec_sensor_ring_check_for_past_timestamp(sensorhub, out))
+ return false;
+ }
return true;
}
--
2.55.0.229.g6434b31f56-goog© 2016 - 2026 Red Hat, Inc.