mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Satyam Gupta <sgsatyam@google.com>
To: Benson Leung <bleung@chromium.org>, Tzung-Bi Shih <tzungbi@kernel.org>
Cc: Guenter Roeck <groeck@chromium.org>,
	Gwendal Grignou <gwendal@chromium.org>,
	 Scott Collyer <scollyer@google.com>,
	chrome-platform@lists.linux.dev,  linux-kernel@vger.kernel.org,
	Satyam Gupta <sgsatyam@google.com>
Subject: [PATCH v4] platform/chrome: cros_ec_sensorhub: Modify clock drift estimation
Date: Tue, 13 Oct 2026 10:57:53 +0000	[thread overview]
Message-ID: <20261013105753.1370713-1-sgsatyam@google.com> (raw)

From: Scott Collyer <scollyer@google.com>

Update the clock drift estimation and timestamp filter to prevent
monotonic backward skews and Out Of Order (OOO) timestamp errors during
sensor batching.

To keep the timestamp filter active across normal batching flush
intervals (which can be 1 to 2 seconds long), increase
TS_HISTORY_BORED_US from 0.5s to 2.0s. Because transitions between
active streaming and low-frequency batching may now fall within that 2.0s
window, detect Output Data Rate (ODR) or mode changes where the time
delta (dx) changes by more than 10x and reset the filter history so
streaming and batched samples are not mixed. Additionally, fix the
history shift loop starting index using min_t() so state->x_history[1] is
properly populated as soon as history_len reaches 2, and clamp the
calculated drift slope (m) within +/- MAX_DRIFT_PPM Parts Per Million
(PPM) so temporary jitter spikes cannot corrupt the median filter.

Finally, when a sample timestamp is strictly earlier than the newest
event seen for that sensor, drop the Out Of Order sample instead of
clamping its timestamp to last_ts, which previously caused downstream
batch spreading logic to misinterpret stale samples as a batch of
identical timestamps.

Signed-off-by: Scott Collyer <scollyer@google.com>
Signed-off-by: Satyam Gupta <sgsatyam@google.com>
---
Changes in v4:
- Reword commit description to clarify why the batching filter changes form a
  single logical change, and spell out Out Of Order (OOO) and Parts Per
  Million (PPM).
- Fix Signed-off-by capitalization.
- Drop redundant inline comment on TS_HISTORY_BORED_US.
- Use min_t(int, state->history_len, CROS_EC_SENSORHUB_TS_HISTORY_SIZE - 1)
  for history shift loop start index.
- Rename cros_ec_sensor_ring_check_for_past_timestamp() to
  cros_ec_sensor_ring_is_past_timestamp() and return true when the sample has
  a past timestamp.
 .../platform/chrome/cros_ec_sensorhub_ring.c  | 73 ++++++++++++++-----
 1 file changed, 55 insertions(+), 18 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec_sensorhub_ring.c b/drivers/platform/chrome/cros_ec_sensorhub_ring.c
index cc038af59218..20fa47cbee32 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
 
 /* 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 = min_t(int, state->history_len,
+		      CROS_EC_SENSORHUB_TS_HISTORY_SIZE - 1);
+	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;
 
@@ -398,23 +427,29 @@ cros_ec_sensor_ring_fix_overflow(s64 *ts,
 	state->last = *ts;
 }
 
-static void
-cros_ec_sensor_ring_check_for_past_timestamp(struct cros_ec_sensorhub
-					     *sensorhub,
-					     struct cros_ec_sensors_ring_sample
-					     *sample)
+static bool
+cros_ec_sensor_ring_is_past_timestamp(struct cros_ec_sensorhub *sensorhub,
+				      struct cros_ec_sensors_ring_sample *sample)
 {
 	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;
+		return true;
+
+	sensorhub->batch_state[sensor_id].newest_sensor_event =
+		sample->timestamp;
+	return false;
 }
 
 /**
@@ -547,8 +582,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_is_past_timestamp(sensorhub, out))
+			return false;
+	}
 	return true;
 }
 
-- 
2.56.0.385.gd3acb90ef8-goog


             reply	other threads:[~2026-10-13 10:57 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-13 10:57 Satyam Gupta [this message]
2026-10-14  6:49 ` Tzung-Bi Shih

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=20261013105753.1370713-1-sgsatyam@google.com \
    --to=sgsatyam@google.com \
    --cc=bleung@chromium.org \
    --cc=chrome-platform@lists.linux.dev \
    --cc=groeck@chromium.org \
    --cc=gwendal@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=scollyer@google.com \
    --cc=tzungbi@kernel.org \
    /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

all inboxes | Powered by JetHome®