diff mbox series

[v1,19/19] hid-ft260: uart: improve write performance

Message ID 20240210215147.77629-20-michael.zaidman@gmail.com (mailing list archive)
State New
Delegated to: Jiri Kosina
Headers show
Series hid-ft260: Fixes for serial driver patch v4 | expand

Commit Message

Michael Zaidman Feb. 10, 2024, 9:51 p.m. UTC
Tx performance with the current buffer size of 256 bytes is lower when
the data length exceeds the xmit buf size.

[134331.147978] ft260_uart_write: count: 288, len: 256
[134331.157945] ft260_uart_write: count: 32, len: 32
[134331.159977] ft260_uart_write: count: 288, len: 256
[134331.169990] ft260_uart_write: count: 32, len: 32

1. Increase the xmit buffer size to page size as used in the serial core
and other tty drivers.

2. Remove the xmit buffer fulness against the watermark checking and the
tty_wakeup calling in the ft260_uart_transmit_chars routine. This code is
taken from other drivers, but other drivers may call the routine from the
interrupt context. In our case, this condition is always True since xmit
buffer filling and emptying are serialized and done synchronously.

Tested with picocom ASCII file transfer by 288-byte chunks at 921600
bauds rate with above 20% performance improvement.

Before:
2821.7 Kbytes transferred at 47367 CPS... Done.

After:
2821.7 Kbytes transferred at 57788 CPS... Done.

Signed-off-by: Michael Zaidman <michael.zaidman@gmail.com>
---
 drivers/hid/hid-ft260.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

Comments

Michael Zaidman Feb. 23, 2024, 9:22 p.m. UTC | #1
On Sat, Feb 10, 2024 at 11:51:47PM +0200, Michael Zaidman wrote:
> Tx performance with the current buffer size of 256 bytes is lower when
> the data length exceeds the xmit buf size.
> 
> [134331.147978] ft260_uart_write: count: 288, len: 256
> [134331.157945] ft260_uart_write: count: 32, len: 32
> [134331.159977] ft260_uart_write: count: 288, len: 256
> [134331.169990] ft260_uart_write: count: 32, len: 32
> 
> 1. Increase the xmit buffer size to page size as used in the serial core
> and other tty drivers.
> 
> 2. Remove the xmit buffer fulness against the watermark checking and the
> tty_wakeup calling in the ft260_uart_transmit_chars routine. This code is
> taken from other drivers, but other drivers may call the routine from the
> interrupt context. In our case, this condition is always True since xmit
> buffer filling and emptying are serialized and done synchronously.
> 
> Tested with picocom ASCII file transfer by 288-byte chunks at 921600
> bauds rate with above 20% performance improvement.
> 
> Before:
> 2821.7 Kbytes transferred at 47367 CPS... Done.
> 
> After:
> 2821.7 Kbytes transferred at 57788 CPS... Done.
> 

Besides the performance improvement, it fixes the bug of outputting
characters to the local terminal when local echo is disabled and
printing every character twice with local echo enabled.

> Signed-off-by: Michael Zaidman <michael.zaidman@gmail.com>
> ---
>  drivers/hid/hid-ft260.c | 11 +++--------
>  1 file changed, 3 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
> index 6b172bfa4f98..1188b8e09938 100644
> --- a/drivers/hid/hid-ft260.c
> +++ b/drivers/hid/hid-ft260.c
> @@ -308,8 +308,7 @@ enum {
>  #define FT260_UART_EN_PW_SAVE_BAUD	(4800)
>  
>  #define UART_COUNT_MAX (4) /* Number of supported UARTs */
> -#define XMIT_FIFO_SIZE (256)
> -#define TTY_WAKEUP_WATERMARK (XMIT_FIFO_SIZE / 2)
> +#define XMIT_FIFO_SIZE (PAGE_SIZE)
>  
>  static const struct hid_device_id ft260_devices[] = {
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_FUTURE_TECHNOLOGY,
> @@ -1211,7 +1210,7 @@ static int ft260_uart_transmit_chars(struct ft260_device *port)
>  
>  		len = kfifo_out_spinlocked(xmit, rep->data, len, &port->xmit_fifo_lock);
>  
> -		ret = ft260_hid_output_report(hdev, (u8 *)rep, len + sizeof(*rep));
> +		ret = ft260_hid_output_report(hdev, (u8 *)rep, len + 2);
>  		if (ret < 0)
>  			goto tty_out;
>  
> @@ -1219,10 +1218,6 @@ static int ft260_uart_transmit_chars(struct ft260_device *port)
>  		port->icount.tx += len;
>  	} while (data_len > 0);
>  
> -	len = kfifo_len(xmit);
> -	if ((XMIT_FIFO_SIZE - len) > TTY_WAKEUP_WATERMARK)
> -		tty_wakeup(tty);
> -
>  	ret = 0;
>  
>  tty_out:
diff mbox series

Patch

diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
index 6b172bfa4f98..1188b8e09938 100644
--- a/drivers/hid/hid-ft260.c
+++ b/drivers/hid/hid-ft260.c
@@ -308,8 +308,7 @@  enum {
 #define FT260_UART_EN_PW_SAVE_BAUD	(4800)
 
 #define UART_COUNT_MAX (4) /* Number of supported UARTs */
-#define XMIT_FIFO_SIZE (256)
-#define TTY_WAKEUP_WATERMARK (XMIT_FIFO_SIZE / 2)
+#define XMIT_FIFO_SIZE (PAGE_SIZE)
 
 static const struct hid_device_id ft260_devices[] = {
 	{ HID_USB_DEVICE(USB_VENDOR_ID_FUTURE_TECHNOLOGY,
@@ -1211,7 +1210,7 @@  static int ft260_uart_transmit_chars(struct ft260_device *port)
 
 		len = kfifo_out_spinlocked(xmit, rep->data, len, &port->xmit_fifo_lock);
 
-		ret = ft260_hid_output_report(hdev, (u8 *)rep, len + sizeof(*rep));
+		ret = ft260_hid_output_report(hdev, (u8 *)rep, len + 2);
 		if (ret < 0)
 			goto tty_out;
 
@@ -1219,10 +1218,6 @@  static int ft260_uart_transmit_chars(struct ft260_device *port)
 		port->icount.tx += len;
 	} while (data_len > 0);
 
-	len = kfifo_len(xmit);
-	if ((XMIT_FIFO_SIZE - len) > TTY_WAKEUP_WATERMARK)
-		tty_wakeup(tty);
-
 	ret = 0;
 
 tty_out:
@@ -1546,7 +1541,7 @@  static int ft260_uart_port_activate(struct tty_port *tport, struct tty_struct *t
 	if (baudrate > FT260_UART_EN_PW_SAVE_BAUD)
 		ft260_uart_wakeup_workaraund_enable(port, true);
 
-	ft260_dbg("configurd baudrate = %d", baudrate);
+	ft260_dbg("configured baudrate = %d", baudrate);
 
 	mod_timer(&port->wakeup_timer, jiffies +
 		  msecs_to_jiffies(FT260_WAKEUP_NEEDED_AFTER_MS));