From 2d363a23cb9e825200e772f973eab921a69e0646 Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Wed, 25 Oct 2023 11:04:54 +1100 Subject: [PATCH 1/3] shared/tinyusb: Schedule TinyUSB task function from dcd_event_handler. dcd_event_handler() is called from the IRQ when a new DCD event is queued for processing by the TinyUSB thread mode task. This lets us queue the handler to run immediately when MicroPython resumes. Currently this relies on a linker --wrap hack to work, but a PR has been submitted to TinyUSB to allow the function to be called inline from dcd_event_handler() itself. This work was funded through GitHub Sponsors. Signed-off-by: Angus Gratton --- shared/tinyusb/mp_usbd.c | 23 +++++++++++++++++++++++ shared/tinyusb/mp_usbd.h | 3 --- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/shared/tinyusb/mp_usbd.c b/shared/tinyusb/mp_usbd.c index ea1de674bc1b4..87c10310f7bea 100644 --- a/shared/tinyusb/mp_usbd.c +++ b/shared/tinyusb/mp_usbd.c @@ -27,17 +27,40 @@ #include #include "py/mpconfig.h" +#include "py/runtime.h" #if MICROPY_HW_ENABLE_USBDEV #ifndef NO_QSTR #include "tusb.h" // TinyUSB is not available when running the string preprocessor +#include "device/dcd.h" #include "device/usbd.h" #include "device/usbd_pvt.h" #endif +// Legacy TinyUSB task function wrapper, called by some ports from the interpreter hook void usbd_task(void) { tud_task_ext(0, false); } +// TinyUSB task function wrapper, as scheduled from the USB IRQ +static void mp_usbd_task(mp_sched_node_t *node); + +extern void __real_dcd_event_handler(dcd_event_t const *event, bool in_isr); + +// If -Wl,--wrap=dcd_event_handler is passed to the linker, then this wrapper +// will be called and allows MicroPython to schedule the TinyUSB task when +// dcd_event_handler() is called from an ISR. +TU_ATTR_FAST_FUNC void __wrap_dcd_event_handler(dcd_event_t const *event, bool in_isr) { + static mp_sched_node_t usbd_task_node; + + __real_dcd_event_handler(event, in_isr); + mp_sched_schedule_node(&usbd_task_node, mp_usbd_task); +} + +static void mp_usbd_task(mp_sched_node_t *node) { + (void)node; + tud_task_ext(0, false); +} + #endif diff --git a/shared/tinyusb/mp_usbd.h b/shared/tinyusb/mp_usbd.h index 3a93b929c586d..2e4feaca9fa19 100644 --- a/shared/tinyusb/mp_usbd.h +++ b/shared/tinyusb/mp_usbd.h @@ -29,9 +29,6 @@ #include "py/obj.h" -// Call instead of tud_task() -void mp_usbd_task(void); - // Function to be implemented in port code. // Can write a string up to MICROPY_HW_USB_DESC_STR_MAX characters long, plus terminating byte. extern void mp_usbd_port_get_serial_number(char *buf); From bcbdee235719d459a4cd60d51021454fba54cd0f Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Wed, 25 Oct 2023 13:50:53 +1100 Subject: [PATCH 2/3] rp2: Change to use TinyUSB dcd_event_handler hook. This change: - Has a small code size reduction. - Should slightly improve overall performance. The old hook code seemed to use between 0.1% and 1.6% of the total CPU time doing no-op calls even when no USB work was required. - USB performance is mostly the same, there is a small increase in latency for some workloads that seems to because sometimes the hook usbd_task() is called at the right time to line up with the next USB host request. This only happened semi-randomly due to the timing of the hook. Improving the wakeup latency by switching rp2 to tickless WFE allows the usbd_task() to run in time for the next USB host request almost always, improving performance and more than offsetting this impact. This work was funded through GitHub Sponsors. Signed-off-by: Angus Gratton --- ports/rp2/CMakeLists.txt | 1 + ports/rp2/mpconfigport.h | 15 --------------- 2 files changed, 1 insertion(+), 15 deletions(-) diff --git a/ports/rp2/CMakeLists.txt b/ports/rp2/CMakeLists.txt index 4f76c5864fc2a..15f6f6ca1c288 100644 --- a/ports/rp2/CMakeLists.txt +++ b/ports/rp2/CMakeLists.txt @@ -406,6 +406,7 @@ target_compile_options(${MICROPY_TARGET} PRIVATE target_link_options(${MICROPY_TARGET} PRIVATE -Wl,--defsym=__micropy_c_heap_size__=${MICROPY_C_HEAP_SIZE} + -Wl,--wrap=dcd_event_handler ) set_source_files_properties( diff --git a/ports/rp2/mpconfigport.h b/ports/rp2/mpconfigport.h index 84a961db43e8a..61749f108aafc 100644 --- a/ports/rp2/mpconfigport.h +++ b/ports/rp2/mpconfigport.h @@ -250,23 +250,8 @@ extern void mp_thread_end_atomic_section(uint32_t); #define MICROPY_PY_LWIP_REENTER lwip_lock_acquire(); #define MICROPY_PY_LWIP_EXIT lwip_lock_release(); -#if MICROPY_HW_ENABLE_USBDEV -#define MICROPY_HW_USBDEV_TASK_HOOK extern void usbd_task(void); usbd_task(); -#define MICROPY_VM_HOOK_COUNT (10) -#define MICROPY_VM_HOOK_INIT static uint vm_hook_divisor = MICROPY_VM_HOOK_COUNT; -#define MICROPY_VM_HOOK_POLL if (get_core_num() == 0 && --vm_hook_divisor == 0) { \ - vm_hook_divisor = MICROPY_VM_HOOK_COUNT; \ - MICROPY_HW_USBDEV_TASK_HOOK \ -} -#define MICROPY_VM_HOOK_LOOP MICROPY_VM_HOOK_POLL -#define MICROPY_VM_HOOK_RETURN MICROPY_VM_HOOK_POLL -#else -#define MICROPY_HW_USBDEV_TASK_HOOK -#endif - #define MICROPY_EVENT_POLL_HOOK_FAST \ do { \ - if (get_core_num() == 0) { MICROPY_HW_USBDEV_TASK_HOOK } \ extern void mp_handle_pending(bool); \ mp_handle_pending(true); \ } while (0) From 26d5032980335ccdfe81234ad7a24030a580f6d3 Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Wed, 1 Nov 2023 15:30:05 +1100 Subject: [PATCH 3/3] samd: Switch TinyUSB to run via a scheduled task. Previously the TinyUSB task was run in the ISR immediately after the interrupt handler. This approach gives very similar performance (no change in CDC throughput tests) but reduces the amount of time spent in the ISR, and allows TinyUSB callbacks to run in thread mode. This work was funded through GitHub Sponsors. Signed-off-by: Angus Gratton --- ports/samd/Makefile | 3 +++ ports/samd/samd_isr.c | 8 ++++---- ports/samd/samd_soc.h | 4 ---- ports/samd/tusb_port.c | 27 --------------------------- 4 files changed, 7 insertions(+), 35 deletions(-) diff --git a/ports/samd/Makefile b/ports/samd/Makefile index 291a894ab7b07..12223e6ded8f4 100644 --- a/ports/samd/Makefile +++ b/ports/samd/Makefile @@ -93,6 +93,8 @@ LIBSTDCPP_FILE_NAME = "$(shell $(CXX) $(CXXFLAGS) -print-file-name=libstdc++.a)" LDFLAGS += -L"$(shell dirname $(LIBSTDCPP_FILE_NAME))" endif +LDFLAGS += --wrap=dcd_event_handler + MPY_CROSS_FLAGS += -march=$(MPY_CROSS_MCU_ARCH) SRC_C += \ @@ -131,6 +133,7 @@ SHARED_SRC_C += \ shared/runtime/sys_stdio_mphal.c \ shared/timeutils/timeutils.c \ shared/tinyusb/mp_cdc_common.c \ + shared/tinyusb/mp_usbd.c ASF4_SRC_C += $(addprefix lib/asf4/$(MCU_SERIES_LOWER)/,\ hal/src/hal_atomic.c \ diff --git a/ports/samd/samd_isr.c b/ports/samd/samd_isr.c index a12140313b24c..7c4c1d060f768 100644 --- a/ports/samd/samd_isr.c +++ b/ports/samd/samd_isr.c @@ -310,10 +310,10 @@ const ISR isr_vector[] __attribute__((section(".isr_vector"))) = { &Sercom7_Handler, // 77 Serial Communication Interface 7 (SERCOM7): SERCOM7_3 - 6 0, // 78 Control Area Network 0 (CAN0) 0, // 79 Control Area Network 1 (CAN1) - &USB_0_Handler_wrapper, // 80 Universal Serial Bus (USB): USB_EORSM_DNRS, ... - &USB_1_Handler_wrapper, // 81 Universal Serial Bus (USB): USB_SOF_HSOF - &USB_2_Handler_wrapper, // 82 Universal Serial Bus (USB): USB_TRCPT0_0 - _7 - &USB_3_Handler_wrapper, // 83 Universal Serial Bus (USB): USB_TRCPT1_0 - _7 + &USB_Handler_wrapper, // 80 Universal Serial Bus (USB): USB_EORSM_DNRS, ... + &USB_Handler_wrapper, // 81 Universal Serial Bus (USB): USB_SOF_HSOF + &USB_Handler_wrapper, // 82 Universal Serial Bus (USB): USB_TRCPT0_0 - _7 + &USB_Handler_wrapper, // 83 Universal Serial Bus (USB): USB_TRCPT1_0 - _7 0, // 84 Ethernet MAC (GMAC) 0, // 85 Timer Counter Control 0 (TCC0): TCC0_CNT_A ... 0, // 86 Timer Counter Control 0 (TCC0): TCC0_MC_0 diff --git a/ports/samd/samd_soc.h b/ports/samd/samd_soc.h index 1848cf7db198a..90a5a57ffa14e 100644 --- a/ports/samd/samd_soc.h +++ b/ports/samd/samd_soc.h @@ -36,10 +36,6 @@ void samd_init(void); void samd_main(void); void USB_Handler_wrapper(void); -void USB_0_Handler_wrapper(void); -void USB_1_Handler_wrapper(void); -void USB_2_Handler_wrapper(void); -void USB_3_Handler_wrapper(void); void sercom_enable(Sercom *spi, int state); void sercom_register_irq(int sercom_id, void (*sercom_irq_handler)); diff --git a/ports/samd/tusb_port.c b/ports/samd/tusb_port.c index 2098334fbae61..e56ef0fd6af00 100644 --- a/ports/samd/tusb_port.c +++ b/ports/samd/tusb_port.c @@ -117,33 +117,6 @@ const uint16_t *tud_descriptor_string_cb(uint8_t index, uint16_t langid) { return desc_str; } -#if defined(MCU_SAMD21) - void USB_Handler_wrapper(void) { tud_int_handler(0); - tud_task(); -} - -#elif defined(MCU_SAMD51) - -void USB_0_Handler_wrapper(void) { - tud_int_handler(0); - tud_task(); -} - -void USB_1_Handler_wrapper(void) { - tud_int_handler(0); - tud_task(); } - -void USB_2_Handler_wrapper(void) { - tud_int_handler(0); - tud_task(); -} - -void USB_3_Handler_wrapper(void) { - tud_int_handler(0); - tud_task(); -} - -#endif