applications: nrf_desktop: Submit HID report sent event on USB SOF

Delay submitting hid_report_sent_event until subsequent USB Start of
Frame. Synchronizing providing HID data to USB SOF helps to avoid
jitter related to USB polls.

Jira: NCSDK-28224

Signed-off-by: Marek Pieta <[email protected]>

applications/nrf_desktop/src/modules/Kconfig.usb_state

@@ -47,6 +47,15 @@ config DESKTOP_USB_PM_ENABLE
 	help
 	  This enables small module that blocks power down if the USB is active.
 
+config DESKTOP_USB_HID_REPORT_SENT_ON_SOF
+	bool "Submit HID report sent event on USB Start of Frame (SOF) [EXPERIMENTAL]"
+	select EXPERIMENTAL
+	help
+	  Delay submitting hid_report_sent_event until subsequent USB Start of
+	  Frame (SOF). Synchronizing submitting HID data to USB stack to USB SOF
+	  helps to avoid jitter related to USB polls, but it also increases HID
+	  data latency as HID report pipeline with length of 2 needs to be used.
+
 config DESKTOP_USB_INIT_THREAD
 	bool
 	default y if SOC_SERIES_NRF54HX
@@ -94,6 +103,7 @@ config DESKTOP_USB_STACK_LEGACY
 	select USB_DEVICE_STACK
 	select USB_DEVICE_HID
 	select USB_DEVICE_REMOTE_WAKEUP if DESKTOP_USB_REMOTE_WAKEUP
+	select USB_DEVICE_SOF if DESKTOP_USB_HID_REPORT_SENT_ON_SOF
 
 endchoice
 
@@ -173,10 +183,17 @@ endif # DESKTOP_USB_STACK_LEGACY
 if DESKTOP_USB_STACK_NEXT
 
 config USBD_HID_IN_BUF_COUNT
+	default 2 if DESKTOP_USB_HID_REPORT_SENT_ON_SOF
 	default 1
 	help
-	  nRF Desktop queues HID reports at the source. There is no need to use
-	  multiple buffers in the IN pool per HID instance.
+	  nRF Desktop queues HID reports at the source. Generally there is no
+	  need to use multiple buffers in the IN pool per HID instance.
+
+	  An exception is use-case where HID reports are submitted from USB sent
+	  callback (when synchronizing hid_report_sent to USB Start of Frame
+	  (SOF) enforces using pipeline size of 2). An extra buffer must be
+	  allocated to allow providing data to the USB next stack before the
+	  previously allocated buffer is freed.
 
 config DESKTOP_USB_STACK_NEXT_DISABLE_ON_VBUS_REMOVAL
 	bool

applications/nrf_desktop/src/modules/usb_state.c

@@ -47,8 +47,8 @@ LOG_MODULE_REGISTER(MODULE, CONFIG_DESKTOP_USB_STATE_LOG_LEVEL);
 #define REPORT_TYPE_FEATURE	0x03
 
 #define USB_SUBSCRIBER_PRIORITY      CONFIG_DESKTOP_USB_SUBSCRIBER_REPORT_PRIORITY
-#define USB_SUBSCRIBER_PIPELINE_SIZE 0x01
-#define USB_SUBSCRIBER_REPORT_MAX    0x01
+#define USB_SUBSCRIBER_PIPELINE_SIZE (IS_ENABLED(CONFIG_DESKTOP_USB_HID_REPORT_SENT_ON_SOF) ? 2 : 1)
+#define USB_SUBSCRIBER_REPORT_MAX    USB_SUBSCRIBER_PIPELINE_SIZE
 
 /* The definitions are available and used only for USB legacy stack.
  * Define them explicitly if needed to allow compiling code without USB legacy stack.
@@ -77,9 +77,10 @@ struct usb_hid_buf {
 struct usb_hid_device {
 	const struct device *dev;
 	uint32_t report_bm;
-	uint8_t hid_protocol;
 	struct usb_hid_buf report_bufs[USB_SUBSCRIBER_REPORT_MAX];
+	atomic_ptr_t report_sent_on_sof;
 	uint32_t idle_duration[REPORT_ID_COUNT];
+	uint8_t hid_protocol;
 	bool report_enabled[REPORT_ID_COUNT];
 	bool enabled;
 };
@@ -398,6 +399,15 @@ static int set_report(const struct device *dev, uint8_t report_type, uint8_t rep
 	return err;
 }
 
+static void report_sent_sof(struct usb_hid_device *usb_hid)
+{
+	struct hid_report_sent_event *event = atomic_ptr_set(&usb_hid->report_sent_on_sof, NULL);
+
+	if (event) {
+		APP_EVENT_SUBMIT(event);
+	}
+}
+
 static void report_sent(struct usb_hid_device *usb_hid, struct usb_hid_buf *buf, bool error)
 {
 	__ASSERT_NO_MSG(buf);
@@ -410,7 +420,28 @@ static void report_sent(struct usb_hid_device *usb_hid, struct usb_hid_buf *buf,
 	event->report_id = report_id;
 	event->subscriber = usb_hid;
 	event->error = error;
-	APP_EVENT_SUBMIT(event);
+
+	if (!IS_ENABLED(CONFIG_DESKTOP_USB_HID_REPORT_SENT_ON_SOF)) {
+		APP_EVENT_SUBMIT(event);
+	} else {
+		if (error) {
+			/* Synchronization to USB SOF is not used on send error. Instantly send
+			 * enqueued report waiting for USB SOF to ensure proper HID report sent
+			 * event order.
+			 */
+			report_sent_sof(usb_hid);
+			APP_EVENT_SUBMIT(event);
+		} else {
+			if (!atomic_ptr_cas(&usb_hid->report_sent_on_sof, NULL, event)) {
+				/* Instantly submit previous event to ensure proper HID report sent
+				 * event order.
+				 */
+				LOG_WRN("Missing USB SOF between HID report sent callbacks");
+				report_sent_sof(usb_hid);
+				(void)atomic_ptr_set(&usb_hid->report_sent_on_sof, event);
+			}
+		}
+	}
 
 	usb_hid_buf_free(buf);
 
@@ -601,6 +632,11 @@ static void update_usb_hid(struct usb_hid_device *usb_hid, bool enabled)
 
 	usb_hid->enabled = enabled;
 
+	/* Inform about the sent HID report if queued. */
+	if (IS_ENABLED(CONFIG_DESKTOP_USB_HID_REPORT_SENT_ON_SOF) && !enabled) {
+		report_sent_sof(usb_hid);
+	}
+
 	/* USB legacy stack does not notify app about sent report when no longer configured.
 	 * Pending report reset needs to be done after usb_hid->enabled field update to prevent
 	 * module from submitting HID report to stack and before broadcast of new USB HID subscriber
@@ -909,6 +945,16 @@ static void usb_init_legacy_status_cb(enum usb_dc_status_code cb_status, const u
 		}
 		break;
 
+	case USB_DC_SOF:
+		if (IS_ENABLED(CONFIG_DESKTOP_USB_HID_REPORT_SENT_ON_SOF)) {
+			for (size_t i = 0; i < ARRAY_SIZE(usb_hid_device); i++) {
+				struct usb_hid_device *usb_hid = &usb_hid_device[i];
+
+				report_sent_sof(usb_hid);
+			}
+		}
+		/* Fall-through. */
+
 	case USB_DC_SET_HALT:
 	case USB_DC_CLEAR_HALT:
 		/* Ignore */
@@ -1076,6 +1122,11 @@ static void report_sent_cb_next(const struct device *dev)
 	report_sent(usb_hid, buf, error);
 }
 
+static void sof_next(const struct device *dev)
+{
+	report_sent_sof(dev_to_usb_hid(dev));
+}
+
 static int usb_init_next_hid_device_init(struct usb_hid_device *usb_hid_dev, uint32_t report_bm)
 {
 	static const struct hid_device_ops hid_ops = {
@@ -1086,6 +1137,7 @@ static int usb_init_next_hid_device_init(struct usb_hid_device *usb_hid_dev, uin
 		.get_idle = get_idle_next,
 		.set_protocol = protocol_change,
 		.input_report_done = report_sent_cb_next,
+		.sof = sof_next,
 	};
 
 	usb_hid_dev->report_bm = report_bm;