From 1b11d579eab1eff6248dfc41416c6a5aeca34e39 Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Tue, 3 Oct 2017 14:21:27 -0700 Subject: [PATCH 1/2] Add read buffer to bitbang, improving performance. 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: I7dc774e1e0f5752905ac4318fd9b85b930374a05 --- src/jtag/drivers/bitbang.c | 21 +++++-- src/jtag/drivers/bitbang.h | 14 +++++ src/jtag/drivers/remote_bitbang.c | 100 ++++++++++++++++++++++++------ 3 files changed, 111 insertions(+), 24 deletions(-) diff --git a/src/jtag/drivers/bitbang.c b/src/jtag/drivers/bitbang.c index c9ec9c9d6..bde48e0dc 100644 --- a/src/jtag/drivers/bitbang.c +++ b/src/jtag/drivers/bitbang.c @@ -203,7 +203,6 @@ static void bitbang_scan(bool ir_scan, enum scan_type type, uint8_t *buffer, int } 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,12 +218,26 @@ 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->sample) { + bitbang_interface->sample(); + } 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 (type != SCAN_OUT && bitbang_interface->sample) { + for (bit_cnt = 0; bit_cnt < scan_size; bit_cnt++) { + int bytec = bit_cnt/8; + int bcval = 1 << (bit_cnt % 8); + int val = bitbang_interface->read_sample(); if (val) buffer[bytec] |= bcval; else diff --git a/src/jtag/drivers/bitbang.h b/src/jtag/drivers/bitbang.h index c5b44bfd5..563ea104c 100644 --- a/src/jtag/drivers/bitbang.h +++ b/src/jtag/drivers/bitbang.h @@ -27,7 +27,21 @@ 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. */ + /* 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 b898e957d..3907a077a 100644 --- a/src/jtag/drivers/remote_bitbang.c +++ b/src/jtag/drivers/remote_bitbang.c @@ -40,11 +40,44 @@ 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; + +static char remote_bitbang_buf[64]; +static unsigned remote_bitbang_start; +static unsigned remote_bitbang_end; + +/* 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 (1) { + ssize_t count = read(remote_bitbang_fd, + remote_bitbang_buf + remote_bitbang_end, + sizeof(remote_bitbang_buf) - remote_bitbang_end); + 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) { + remote_bitbang_fill_buf(); if (EOF == fputc(c, remote_bitbang_out)) REMOTE_BITBANG_RAISE_ERROR("remote_bitbang_putc: %s", strerror(errno)); } @@ -75,15 +108,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 +122,40 @@ 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_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 +178,8 @@ static void remote_bitbang_blink(int on) } static struct bitbang_interface remote_bitbang_bitbang = { - .read = &remote_bitbang_read, + .sample = &remote_bitbang_sample, + .read_sample = &remote_bitbang_read_sample, .write = &remote_bitbang_write, .reset = &remote_bitbang_reset, .blink = &remote_bitbang_blink, @@ -199,26 +257,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); From 28eb10f43d513770224967fe7a73e98b498aa1a9 Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Wed, 4 Oct 2017 15:23:14 -0700 Subject: [PATCH 2/2] Ensure the buffer doesn't overflow. Tested with a variety of prime buffer sizes. Change-Id: I2b4835d46adf4c971111da88e8de4b46eb8dad41 --- src/jtag/drivers/bitbang.c | 40 +++++++++++++++++-------------- src/jtag/drivers/bitbang.h | 3 +++ src/jtag/drivers/remote_bitbang.c | 26 +++++++++++++++++--- 3 files changed, 48 insertions(+), 21 deletions(-) diff --git a/src/jtag/drivers/bitbang.c b/src/jtag/drivers/bitbang.c index bde48e0dc..4fdca7980 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,6 +203,7 @@ 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 tms = (bit_cnt == scan_size-1) ? 1 : 0; int tdi; @@ -219,8 +221,9 @@ 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) { - if (bitbang_interface->sample) { + if (bitbang_interface->buf_size) { bitbang_interface->sample(); + buffered++; } else { int val = bitbang_interface->read(); if (val) @@ -231,17 +234,17 @@ static void bitbang_scan(bool ir_scan, enum scan_type type, uint8_t *buffer, int } bitbang_interface->write(1, tms, tdi); - } - if (type != SCAN_OUT && bitbang_interface->sample) { - for (bit_cnt = 0; bit_cnt < scan_size; bit_cnt++) { - int bytec = bit_cnt/8; - int bcval = 1 << (bit_cnt % 8); - int val = bitbang_interface->read_sample(); - 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; } } @@ -322,13 +325,14 @@ int bitbang_execute_queue(void) bitbang_path_move(cmd->cmd.pathmove); break; case JTAG_SCAN: -#ifdef _DEBUG_JTAG_IO_ - LOG_DEBUG("%s scan end in %s", - (cmd->cmd.scan->ir_scan) ? "IR" : "DR", - 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); +#ifdef _DEBUG_JTAG_IO_ + 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 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 563ea104c..f0b9263c1 100644 --- a/src/jtag/drivers/bitbang.h +++ b/src/jtag/drivers/bitbang.h @@ -36,6 +36,9 @@ struct bitbang_interface { /* 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. */ diff --git a/src/jtag/drivers/remote_bitbang.c b/src/jtag/drivers/remote_bitbang.c index 3907a077a..5e78ccb45 100644 --- a/src/jtag/drivers/remote_bitbang.c +++ b/src/jtag/drivers/remote_bitbang.c @@ -44,18 +44,36 @@ 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 (1) { + 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, - sizeof(remote_bitbang_buf) - remote_bitbang_end); + contiguous_available_space); if (count > 0) { remote_bitbang_end += count; // TODO: check for overflow. @@ -77,7 +95,6 @@ static void remote_bitbang_fill_buf(void) static void remote_bitbang_putc(int c) { - remote_bitbang_fill_buf(); if (EOF == fputc(c, remote_bitbang_out)) REMOTE_BITBANG_RAISE_ERROR("remote_bitbang_putc: %s", strerror(errno)); } @@ -145,6 +162,8 @@ static int remote_bitbang_rread(void) static void remote_bitbang_sample(void) { + remote_bitbang_fill_buf(); + assert(!remote_bitbang_buf_full()); remote_bitbang_putc('R'); } @@ -178,6 +197,7 @@ static void remote_bitbang_blink(int on) } static struct bitbang_interface remote_bitbang_bitbang = { + .buf_size = sizeof(remote_bitbang_buf) - 1, .sample = &remote_bitbang_sample, .read_sample = &remote_bitbang_read_sample, .write = &remote_bitbang_write,