This is an automated email from Gerrit. "Tomas Vanek <van...@fbl.cz>" just uploaded a new patch set to Gerrit, which you can find at https://review.openocd.org/c/openocd/+/7364
-- gerrit commit d4fa0e707596abdfe718b3a00e34f308b91fd375 Author: Tomas Vanek <van...@fbl.cz> Date: Sat Nov 19 07:21:10 2022 +0100 jtag/drivers/cmsis_dap: improve USB packets filling DAP write transaction occupies 5 bytes of a command packet. DAP read transaction needs just one byte in a command packet and expect 4 bytes in a response. The fixed maximal number of transactions in a packet caused packet filling less than optimal. Compute both command and expected response sizes based on read or write direction of each transaction. Run the queue if one of sizes does not fit into a packet. The change increases the speed of the mostly read transfer from 36 KiB/s to almost 40 KiB/s @ USB FS, adapter speed 1000 due to reduction of adapter inserted RDBUFF reads. Signed-off-by: Tomas Vanek <van...@fbl.cz> Change-Id: Ib70812600eaae0403b8ee8673b6f897348496569 diff --git a/src/jtag/drivers/cmsis_dap.c b/src/jtag/drivers/cmsis_dap.c index 2b76f5b77a..b12415c7e2 100644 --- a/src/jtag/drivers/cmsis_dap.c +++ b/src/jtag/drivers/cmsis_dap.c @@ -223,6 +223,10 @@ struct pending_scan_result { /* Pending requests are organized as a FIFO - circular buffer */ /* Each block in FIFO can contain up to pending_queue_len transfers */ static int pending_queue_len; +static unsigned int tfer_command_size; +static unsigned int tfer_max_command_size; +static unsigned int tfer_response_size; +static unsigned int tfer_max_response_size; /* pointers to buffers that will receive jtag scan results on the next flush */ #define MAX_PENDING_SCAN_RESULTS 256 @@ -748,6 +752,10 @@ static void cmsis_dap_swd_write_from_queue(struct cmsis_dap *dap) uint8_t *command = dap->command; struct pending_request_block *block = &dap->pending_fifo[dap->pending_fifo_put_idx]; + /* Reset size counters for the next packet */ + tfer_command_size = 0; + tfer_response_size = 0; + LOG_DEBUG_IO("Executing %d queued transactions from FIFO index %u", block->transfer_count, dap->pending_fifo_put_idx); @@ -918,7 +926,15 @@ static void cmsis_dap_swd_queue_cmd(uint8_t cmd, uint32_t *dst, uint32_t data) { bool targetsel_cmd = swd_cmd(false, false, DP_TARGETSEL) == cmd; - if (cmsis_dap_handle->pending_fifo[cmsis_dap_handle->pending_fifo_put_idx].transfer_count == pending_queue_len + unsigned int this_rw_command_size = 5; + unsigned int this_rw_response_size = 0; + if (cmd & SWD_CMD_RNW) { + this_rw_command_size = 1; + this_rw_response_size = 4; + } + + if (tfer_command_size + this_rw_command_size > tfer_max_command_size + || tfer_response_size + this_rw_response_size > tfer_max_response_size || targetsel_cmd) { if (cmsis_dap_handle->pending_fifo_block_count) cmsis_dap_swd_read_process(cmsis_dap_handle, 0); @@ -930,6 +946,8 @@ static void cmsis_dap_swd_queue_cmd(uint8_t cmd, uint32_t *dst, uint32_t data) cmsis_dap_swd_read_process(cmsis_dap_handle, LIBUSB_TIMEOUT_MS); } + assert(cmsis_dap_handle->pending_fifo[cmsis_dap_handle->pending_fifo_put_idx].transfer_count < pending_queue_len); + if (queued_retval != ERROR_OK) return; @@ -947,6 +965,8 @@ static void cmsis_dap_swd_queue_cmd(uint8_t cmd, uint32_t *dst, uint32_t data) transfer->buffer = dst; } block->transfer_count++; + tfer_command_size += this_rw_command_size; + tfer_response_size += this_rw_response_size; } static void cmsis_dap_swd_write_reg(uint8_t cmd, uint32_t value, uint32_t ap_delay_clk) @@ -1184,7 +1204,6 @@ static int cmsis_dap_init(void) /* Be conservative and suppress submitting multiple HID requests * until we get packet count info from the adaptor */ cmsis_dap_handle->packet_count = 1; - pending_queue_len = 12; /* INFO_ID_PKT_SZ - short */ retval = cmsis_dap_cmd_dap_info(INFO_ID_PKT_SZ, &data); @@ -1194,12 +1213,6 @@ static int cmsis_dap_init(void) if (data[0] == 2) { /* short */ uint16_t pkt_sz = data[1] + (data[2] << 8); if (pkt_sz != cmsis_dap_handle->packet_size) { - - /* 4 bytes of command header + 5 bytes per register - * write. For bulk read sequences just 4 bytes are - * needed per transfer, so this is suboptimal. */ - pending_queue_len = (pkt_sz - 4) / 5; - free(cmsis_dap_handle->packet_buffer); retval = cmsis_dap_handle->backend->packet_buffer_alloc(cmsis_dap_handle, pkt_sz); if (retval != ERROR_OK) @@ -1209,6 +1222,16 @@ static int cmsis_dap_init(void) } } + /* Maximal number of transfers which fit to one packet: + * Limited by response size: 3 bytes of response header + 4 per read + * Plus writes to full command size: 3 bytes cmd header + 1 per read + 5 per write */ + tfer_max_command_size = cmsis_dap_handle->packet_usable_size - 3; + tfer_max_response_size = cmsis_dap_handle->packet_usable_size - 3; + unsigned int max_reads = tfer_max_response_size / 4; + pending_queue_len = max_reads + (tfer_max_command_size - max_reads) / 5; + tfer_command_size = 0; + tfer_response_size = 0; + /* INFO_ID_PKT_CNT - byte */ retval = cmsis_dap_cmd_dap_info(INFO_ID_PKT_CNT, &data); if (retval != ERROR_OK) --