Menu
▾
▴
[OpenOCD-devel] [PATCH]: a904a0c Add read buffer to bitbang, improving performance.
|
From: <ge...@op...> - 2017-12-13 21:17:06
|
This is an automated email from Gerrit. Tim Newsome (ti...@si...) just uploaded a new patch set to Gerrit, which you can find at http://openocd.zylin.com/4312 -- gerrit commit a904a0c0beff244690da74769eaa9b15bb1e8b9f Author: Tim Newsome <ti...@si...> Date: Wed Dec 13 13:13:22 2017 -0800 Add read buffer to bitbang, improving performance. Previously for every bit scanned OpenOCD would write the bit, wait for that bit to be scanned, and then read the result. This involves at least 2 context switches. Most of the time the next bit scanned does not depend on the last bit we read, so with a buffer we now write a bunch of bits to be scanned all at once, and then we wait for them all to be scanned and have a result. This reduces the time for one testcase where OpenOCD connects to a simulator from 12.30s to 5.35s! Running all our tests went from 13m13s to 3m55s. Change-Id: Ie9fcea043ac1d7877a521125334ed47d4b3e1615 Signed-off-by: Tim Newsome <ti...@si...> diff --git a/src/jtag/drivers/bitbang.c b/src/jtag/drivers/bitbang.c index c9ec9c9..4fdca79 100644 --- a/src/jtag/drivers/bitbang.c +++ b/src/jtag/drivers/bitbang.c @@ -185,10 +185,11 @@ static void bitbang_stableclocks(int num_cycles) } } -static void bitbang_scan(bool ir_scan, enum scan_type type, uint8_t *buffer, int scan_size) +static void bitbang_scan(bool ir_scan, enum scan_type type, uint8_t *buffer, + unsigned scan_size) { tap_state_t saved_end_state = tap_get_end_state(); - int bit_cnt; + unsigned bit_cnt; if (!((!ir_scan && (tap_get_state() == TAP_DRSHIFT)) || @@ -202,8 +203,8 @@ static void bitbang_scan(bool ir_scan, enum scan_type type, uint8_t *buffer, int bitbang_end_state(saved_end_state); } + size_t buffered = 0; for (bit_cnt = 0; bit_cnt < scan_size; bit_cnt++) { - int val = 0; int tms = (bit_cnt == scan_size-1) ? 1 : 0; int tdi; int bytec = bit_cnt/8; @@ -219,16 +220,31 @@ static void bitbang_scan(bool ir_scan, enum scan_type type, uint8_t *buffer, int bitbang_interface->write(0, tms, tdi); - if (type != SCAN_OUT) - val = bitbang_interface->read(); + if (type != SCAN_OUT) { + if (bitbang_interface->buf_size) { + bitbang_interface->sample(); + buffered++; + } else { + int val = bitbang_interface->read(); + if (val) + buffer[bytec] |= bcval; + else + buffer[bytec] &= ~bcval; + } + } bitbang_interface->write(1, tms, tdi); - if (type != SCAN_OUT) { - if (val) - buffer[bytec] |= bcval; - else - buffer[bytec] &= ~bcval; + if (type != SCAN_OUT && bitbang_interface->buf_size && + (buffered == bitbang_interface->buf_size || + bit_cnt == scan_size - 1)) { + for (unsigned i = bit_cnt + 1 - buffered; i <= bit_cnt; i++) { + if (bitbang_interface->read_sample()) + buffer[i/8] |= 1 << (i % 8); + else + buffer[i/8] &= ~(1 << (i % 8)); + } + buffered = 0; } } @@ -309,13 +325,14 @@ int bitbang_execute_queue(void) bitbang_path_move(cmd->cmd.pathmove); break; case JTAG_SCAN: + bitbang_end_state(cmd->cmd.scan->end_state); + scan_size = jtag_build_buffer(cmd->cmd.scan, &buffer); #ifdef _DEBUG_JTAG_IO_ - LOG_DEBUG("%s scan end in %s", + LOG_DEBUG("%s scan %d bits; end in %s", (cmd->cmd.scan->ir_scan) ? "IR" : "DR", + scan_size, tap_state_name(cmd->cmd.scan->end_state)); #endif - bitbang_end_state(cmd->cmd.scan->end_state); - scan_size = jtag_build_buffer(cmd->cmd.scan, &buffer); type = jtag_scan_type(cmd->cmd.scan); bitbang_scan(cmd->cmd.scan->ir_scan, type, buffer, scan_size); if (jtag_read_buffer(buffer, cmd->cmd.scan) != ERROR_OK) diff --git a/src/jtag/drivers/bitbang.h b/src/jtag/drivers/bitbang.h index c5b44bf..f0b9263 100644 --- a/src/jtag/drivers/bitbang.h +++ b/src/jtag/drivers/bitbang.h @@ -27,7 +27,24 @@ struct bitbang_interface { /* low level callbacks (for bitbang) */ + + /* Either read() or sample()/read_sample() must be implemented. */ + + /* Sample TDO and return 0 or 1. */ int (*read)(void); + + /* The sample functions allow an interface to batch a number of writes and + * sample requests together. Not waiting for a value to come back can + * greatly increase throughput. */ + /* The number of TDO samples that can be buffered up before the caller has + * to call read_sample. */ + size_t buf_size; + /* Sample TDO and put the result in a buffer. */ + void (*sample)(void); + /* Return the next unread value from the buffer. */ + int (*read_sample)(void); + + /* Set TCK, TMS, and TDI to the given values. */ void (*write)(int tck, int tms, int tdi); void (*reset)(int trst, int srst); void (*blink)(int on); diff --git a/src/jtag/drivers/remote_bitbang.c b/src/jtag/drivers/remote_bitbang.c index c8d0136..5e78ccb 100644 --- a/src/jtag/drivers/remote_bitbang.c +++ b/src/jtag/drivers/remote_bitbang.c @@ -40,8 +40,58 @@ static char *remote_bitbang_host; static char *remote_bitbang_port; -FILE *remote_bitbang_in; -FILE *remote_bitbang_out; +static FILE *remote_bitbang_in; +static FILE *remote_bitbang_out; +static int remote_bitbang_fd; + +/* Circular buffer. When start == end, the buffer is empty. */ +static char remote_bitbang_buf[64]; +static unsigned remote_bitbang_start; +static unsigned remote_bitbang_end; + +static int remote_bitbang_buf_full(void) +{ + return remote_bitbang_end == + ((remote_bitbang_start + sizeof(remote_bitbang_buf) - 1) % + sizeof(remote_bitbang_buf)); +} + +/* Read any incoming data, placing it into the buffer. */ +static void remote_bitbang_fill_buf(void) +{ + fcntl(remote_bitbang_fd, F_SETFL, O_NONBLOCK); + while (!remote_bitbang_buf_full()) { + unsigned contiguous_available_space; + if (remote_bitbang_end >= remote_bitbang_start) { + contiguous_available_space = sizeof(remote_bitbang_buf) - + remote_bitbang_end; + if (remote_bitbang_start == 0) + contiguous_available_space -= 1; + } else { + contiguous_available_space = remote_bitbang_start - + remote_bitbang_end - 1; + } + ssize_t count = read(remote_bitbang_fd, + remote_bitbang_buf + remote_bitbang_end, + contiguous_available_space); + if (count > 0) { + remote_bitbang_end += count; + // TODO: check for overflow. + if (remote_bitbang_end == sizeof(remote_bitbang_buf)) { + remote_bitbang_end = 0; + } + } else if (count == 0) { + return; + } else if (count < 0) { + if (errno == EAGAIN) { + return; + } else { + REMOTE_BITBANG_RAISE_ERROR("remote_bitbang_fill_buf: %s (%d)", + strerror(errno), errno); + } + } + } +} static void remote_bitbang_putc(int c) { @@ -75,15 +125,8 @@ static int remote_bitbang_quit(void) return ERROR_OK; } -/* Get the next read response. */ -static int remote_bitbang_rread(void) +static int char_to_int(int c) { - if (EOF == fflush(remote_bitbang_out)) { - remote_bitbang_quit(); - REMOTE_BITBANG_RAISE_ERROR("fflush: %s", strerror(errno)); - } - - int c = fgetc(remote_bitbang_in); switch (c) { case '0': return 0; @@ -96,9 +139,42 @@ static int remote_bitbang_rread(void) } } -static int remote_bitbang_read(void) +/* Get the next read response. */ +static int remote_bitbang_rread(void) { + if (EOF == fflush(remote_bitbang_out)) { + remote_bitbang_quit(); + REMOTE_BITBANG_RAISE_ERROR("fflush: %s", strerror(errno)); + } + + /* Enable blocking access. */ + fcntl(remote_bitbang_fd, F_SETFL, 0); + char c; + ssize_t count = read(remote_bitbang_fd, &c, 1); + if (count == 1) { + return char_to_int(c); + } else { + remote_bitbang_quit(); + REMOTE_BITBANG_RAISE_ERROR("read: count=%d, error=%s", (int) count, + strerror(errno)); + } +} + +static void remote_bitbang_sample(void) +{ + remote_bitbang_fill_buf(); + assert(!remote_bitbang_buf_full()); remote_bitbang_putc('R'); +} + +static int remote_bitbang_read_sample(void) +{ + if (remote_bitbang_start != remote_bitbang_end) { + int c = remote_bitbang_buf[remote_bitbang_start]; + remote_bitbang_start = + (remote_bitbang_start + 1) % sizeof(remote_bitbang_buf); + return char_to_int(c); + } return remote_bitbang_rread(); } @@ -121,7 +197,9 @@ static void remote_bitbang_blink(int on) } static struct bitbang_interface remote_bitbang_bitbang = { - .read = &remote_bitbang_read, + .buf_size = sizeof(remote_bitbang_buf) - 1, + .sample = &remote_bitbang_sample, + .read_sample = &remote_bitbang_read_sample, .write = &remote_bitbang_write, .reset = &remote_bitbang_reset, .blink = &remote_bitbang_blink, @@ -131,7 +209,7 @@ static int remote_bitbang_init_tcp(void) { struct addrinfo hints = { .ai_family = AF_UNSPEC, .ai_socktype = SOCK_STREAM }; struct addrinfo *result, *rp; - int fd; + int fd = 0; LOG_INFO("Connecting to %s:%s", remote_bitbang_host ? remote_bitbang_host : "localhost", @@ -199,26 +277,28 @@ static int remote_bitbang_init_unix(void) static int remote_bitbang_init(void) { - int fd; bitbang_interface = &remote_bitbang_bitbang; + remote_bitbang_start = 0; + remote_bitbang_end = 0; + LOG_INFO("Initializing remote_bitbang driver"); if (remote_bitbang_port == NULL) - fd = remote_bitbang_init_unix(); + remote_bitbang_fd = remote_bitbang_init_unix(); else - fd = remote_bitbang_init_tcp(); + remote_bitbang_fd = remote_bitbang_init_tcp(); - if (fd < 0) - return fd; + if (remote_bitbang_fd < 0) + return remote_bitbang_fd; - remote_bitbang_in = fdopen(fd, "r"); + remote_bitbang_in = fdopen(remote_bitbang_fd, "r"); if (remote_bitbang_in == NULL) { LOG_ERROR("fdopen: failed to open read stream"); - close(fd); + close(remote_bitbang_fd); return ERROR_FAIL; } - remote_bitbang_out = fdopen(fd, "w"); + remote_bitbang_out = fdopen(remote_bitbang_fd, "w"); if (remote_bitbang_out == NULL) { LOG_ERROR("fdopen: failed to open write stream"); fclose(remote_bitbang_in); -- |
