diff mbox series

[Xen-devel,v3,1/2] arm/xen: vpl011: Fix the slow early console SBSA UART output

Message ID 20171024170922.17207-2-andre.przywara@linaro.org
State New
Headers show
Series Fix SBSA UART emulation | expand

Commit Message

Andre Przywara Oct. 24, 2017, 5:09 p.m. UTC
From: Bhupinder Thakur <bhupinder.thakur@linaro.org>

The early console output uses pl011_early_write() to write data. This
function waits for BUSY bit to get cleared before writing the next byte.

In the SBSA UART emulation logic, the BUSY bit was set as soon one
byte was written in the FIFO and it remained set until the FIFO was
emptied. This meant that the output was delayed as each character needed
the BUSY to get cleared.

Since the SBSA UART is getting emulated in Xen using ring buffers, it
ensures that once the data is enqueued in the FIFO, it will be received
by xenconsole so it is safe to set the BUSY bit only when FIFO becomes
full. This will ensure that pl011_early_write() is not delayed unduly
to write the data.

Signed-off-by: Bhupinder Thakur <bhupinder.thakur@linaro.org>
Reviewed-by: Andre Przywara <andre.przywara@linaro.org>
Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
---
 xen/arch/arm/vpl011.c | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

Comments

Stefano Stabellini Oct. 25, 2017, 11:35 p.m. UTC | #1
On Tue, 24 Oct 2017, Andre Przywara wrote:
> From: Bhupinder Thakur <bhupinder.thakur@linaro.org>
> 
> The early console output uses pl011_early_write() to write data. This
> function waits for BUSY bit to get cleared before writing the next byte.
> 
> In the SBSA UART emulation logic, the BUSY bit was set as soon one
> byte was written in the FIFO and it remained set until the FIFO was
> emptied. This meant that the output was delayed as each character needed
> the BUSY to get cleared.
> 
> Since the SBSA UART is getting emulated in Xen using ring buffers, it
> ensures that once the data is enqueued in the FIFO, it will be received
> by xenconsole so it is safe to set the BUSY bit only when FIFO becomes
> full. This will ensure that pl011_early_write() is not delayed unduly
> to write the data.
> 
> Signed-off-by: Bhupinder Thakur <bhupinder.thakur@linaro.org>
> Reviewed-by: Andre Przywara <andre.przywara@linaro.org>
> Signed-off-by: Andre Przywara <andre.przywara@linaro.org>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
>  xen/arch/arm/vpl011.c | 21 ++++++++++++++++-----
>  1 file changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c
> index f7ddccb42a..0b0743679f 100644
> --- a/xen/arch/arm/vpl011.c
> +++ b/xen/arch/arm/vpl011.c
> @@ -159,9 +159,15 @@ static void vpl011_write_data(struct domain *d, uint8_t data)
>      {
>          vpl011->uartfr |= TXFF;
>          vpl011->uartris &= ~TXI;
> -    }
>  
> -    vpl011->uartfr |= BUSY;
> +        /*
> +         * This bit is set only when FIFO becomes full. This ensures that
> +         * the SBSA UART driver can write the early console data as fast as
> +         * possible, without waiting for the BUSY bit to get cleared before
> +         * writing each byte.
> +         */
> +        vpl011->uartfr |= BUSY;
> +    }
>  
>      vpl011->uartfr &= ~TXFE;
>  
> @@ -371,11 +377,16 @@ static void vpl011_data_avail(struct domain *d)
>      {
>          vpl011->uartfr &= ~TXFF;
>          vpl011->uartris |= TXI;
> +
> +        /*
> +         * Clear the BUSY bit as soon as space becomes available
> +         * so that the SBSA UART driver can start writing more data
> +         * without any further delay.
> +         */
> +        vpl011->uartfr &= ~BUSY;
> +
>          if ( out_ring_qsize == 0 )
> -        {
> -            vpl011->uartfr &= ~BUSY;
>              vpl011->uartfr |= TXFE;
> -        }
>      }
>  
>      vpl011_update_interrupt_status(d);
> -- 
> 2.14.1
>
diff mbox series

Patch

diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c
index f7ddccb42a..0b0743679f 100644
--- a/xen/arch/arm/vpl011.c
+++ b/xen/arch/arm/vpl011.c
@@ -159,9 +159,15 @@  static void vpl011_write_data(struct domain *d, uint8_t data)
     {
         vpl011->uartfr |= TXFF;
         vpl011->uartris &= ~TXI;
-    }
 
-    vpl011->uartfr |= BUSY;
+        /*
+         * This bit is set only when FIFO becomes full. This ensures that
+         * the SBSA UART driver can write the early console data as fast as
+         * possible, without waiting for the BUSY bit to get cleared before
+         * writing each byte.
+         */
+        vpl011->uartfr |= BUSY;
+    }
 
     vpl011->uartfr &= ~TXFE;
 
@@ -371,11 +377,16 @@  static void vpl011_data_avail(struct domain *d)
     {
         vpl011->uartfr &= ~TXFF;
         vpl011->uartris |= TXI;
+
+        /*
+         * Clear the BUSY bit as soon as space becomes available
+         * so that the SBSA UART driver can start writing more data
+         * without any further delay.
+         */
+        vpl011->uartfr &= ~BUSY;
+
         if ( out_ring_qsize == 0 )
-        {
-            vpl011->uartfr &= ~BUSY;
             vpl011->uartfr |= TXFE;
-        }
     }
 
     vpl011_update_interrupt_status(d);