[PATCH v0 2/2] HID: nintendo: fix rumble rate limiter

["Daniel J. Ogorchock" <djogorchock@gmail.com>]

It's been discovered that BT controller disconnect events correlate to
erratic input report timestamp deltas.

In experimentation, it's been found that ensuring that multiple
timestamp deltas are consistent prior to transmitting a rumble packet
drastically reduces the occurence rate of BT disconnects.

Alter the joycon_enforce_subcmd_rate() function to use this new
approach.

Signed-off-by: Daniel J. Ogorchock <djogorchock@gmail.com>
---
 drivers/hid/hid-nintendo.c | 75 +++++++++++++++++++++++++++++++++++---
 1 file changed, 69 insertions(+), 6 deletions(-)

diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c
index 2b781cc9082b4..250f5d2f888ab 100644
--- a/drivers/hid/hid-nintendo.c
+++ b/drivers/hid/hid-nintendo.c
@@ -433,7 +433,9 @@ struct joycon_ctlr {
 	u8 usb_ack_match;
 	u8 subcmd_ack_match;
 	bool received_input_report;
+	unsigned int last_input_report_msecs;
 	unsigned int last_subcmd_sent_msecs;
+	unsigned int consecutive_valid_report_deltas;
 
 	/* factory calibration data */
 	struct joycon_stick_cal left_stick_cal_x;
@@ -543,19 +545,54 @@ static void joycon_wait_for_input_report(struct joycon_ctlr *ctlr)
  * Sending subcommands and/or rumble data at too high a rate can cause bluetooth
  * controller disconnections.
  */
+#define JC_INPUT_REPORT_MIN_DELTA	8
+#define JC_INPUT_REPORT_MAX_DELTA	17
+#define JC_SUBCMD_TX_OFFSET_MS		4
+#define JC_SUBCMD_VALID_DELTA_REQ	3
+#define JC_SUBCMD_RATE_MAX_ATTEMPTS	500
+#define JC_SUBCMD_RATE_LIMITER_USB_MS	20
+#define JC_SUBCMD_RATE_LIMITER_BT_MS	60
+#define JC_SUBCMD_RATE_LIMITER_MS(ctlr)	((ctlr)->hdev->bus == BUS_USB ? JC_SUBCMD_RATE_LIMITER_USB_MS : JC_SUBCMD_RATE_LIMITER_BT_MS)
 static void joycon_enforce_subcmd_rate(struct joycon_ctlr *ctlr)
 {
-	static const unsigned int max_subcmd_rate_ms = 25;
-	unsigned int current_ms = jiffies_to_msecs(jiffies);
-	unsigned int delta_ms = current_ms - ctlr->last_subcmd_sent_msecs;
+	unsigned int current_ms;
+	unsigned long subcmd_delta;
+	int consecutive_valid_deltas = 0;
+	int attempts = 0;
+	unsigned long flags;
+
+	if (unlikely(ctlr->ctlr_state != JOYCON_CTLR_STATE_READ))
+		return;
 
-	while (delta_ms < max_subcmd_rate_ms &&
-	       ctlr->ctlr_state == JOYCON_CTLR_STATE_READ) {
+	do {
 		joycon_wait_for_input_report(ctlr);
 		current_ms = jiffies_to_msecs(jiffies);
-		delta_ms = current_ms - ctlr->last_subcmd_sent_msecs;
+		subcmd_delta = current_ms - ctlr->last_subcmd_sent_msecs;
+
+		spin_lock_irqsave(&ctlr->lock, flags);
+		consecutive_valid_deltas = ctlr->consecutive_valid_report_deltas;
+		spin_unlock_irqrestore(&ctlr->lock, flags);
+
+		attempts++;
+	} while ((consecutive_valid_deltas < JC_SUBCMD_VALID_DELTA_REQ ||
+		  subcmd_delta < JC_SUBCMD_RATE_LIMITER_MS(ctlr)) &&
+		 ctlr->ctlr_state == JOYCON_CTLR_STATE_READ &&
+		 attempts < JC_SUBCMD_RATE_MAX_ATTEMPTS);
+
+	if (attempts >= JC_SUBCMD_RATE_MAX_ATTEMPTS) {
+		hid_warn(ctlr->hdev, "%s: exceeded max attempts", __func__);
+		return;
 	}
+
 	ctlr->last_subcmd_sent_msecs = current_ms;
+
+	/*
+	 * Wait a short time after receiving an input report before
+	 * transmitting. This should reduce odds of a TX coinciding with an RX.
+	 * Minimizing concurrent BT traffic with the controller seems to lower
+	 * the rate of disconnections.
+	 */
+	msleep(JC_SUBCMD_TX_OFFSET_MS);
 }
 
 static int joycon_hid_send_sync(struct joycon_ctlr *ctlr, u8 *data, size_t len,
@@ -1223,6 +1260,7 @@ static void joycon_parse_report(struct joycon_ctlr *ctlr,
 	u8 tmp;
 	u32 btns;
 	unsigned long msecs = jiffies_to_msecs(jiffies);
+	unsigned long report_delta_ms = msecs - ctlr->last_input_report_msecs;
 
 	spin_lock_irqsave(&ctlr->lock, flags);
 	if (IS_ENABLED(CONFIG_NINTENDO_FF) && rep->vibrator_report &&
@@ -1364,6 +1402,31 @@ static void joycon_parse_report(struct joycon_ctlr *ctlr,
 
 	input_sync(dev);
 
+	spin_lock_irqsave(&ctlr->lock, flags);
+	ctlr->last_input_report_msecs = msecs;
+	/*
+	 * Was this input report a reasonable time delta compared to the prior
+	 * report? We use this information to decide when a safe time is to send
+	 * rumble packets or subcommand packets.
+	 */
+	if (report_delta_ms >= JC_INPUT_REPORT_MIN_DELTA &&
+	    report_delta_ms <= JC_INPUT_REPORT_MAX_DELTA) {
+		if (ctlr->consecutive_valid_report_deltas < JC_SUBCMD_VALID_DELTA_REQ)
+			ctlr->consecutive_valid_report_deltas++;
+	} else {
+		ctlr->consecutive_valid_report_deltas = 0;
+	}
+	/*
+	 * Our consecutive valid report tracking is only relevant for
+	 * bluetooth-connected controllers. For USB devices, we're beholden to
+	 * USB's underlying polling rate anyway. Always set to the consecutive
+	 * delta requirement.
+	 */
+	if (ctlr->hdev->bus == BUS_USB)
+		ctlr->consecutive_valid_report_deltas = JC_SUBCMD_VALID_DELTA_REQ;
+
+	spin_unlock_irqrestore(&ctlr->lock, flags);
+
 	/*
 	 * Immediately after receiving a report is the most reliable time to
 	 * send a subcommand to the controller. Wake any subcommand senders
-- 
2.39.1