Re: [PATCH v1 7/9] hw/dma/xilinx_axidma: mm2s: Stream descriptor by descriptor

From: Alistair Francis <alistair23@gmail.com>
To: "Edgar E. Iglesias" <edgar.iglesias@gmail.com>
Cc: "Damien Hedde" <damien.hedde@greensocs.com>,
	"Peter Maydell" <peter.maydell@linaro.org>,
	"Edgar Iglesias" <edgar.iglesias@xilinx.com>,
	"Sai Pavan Boddu" <sai.pavan.boddu@xilinx.com>,
	"Francisco Iglesias" <frasse.iglesias@gmail.com>,
	"Jason Wang" <jasowang@redhat.com>,
	"Alistair Francis" <alistair@alistair23.me>,
	"qemu-devel@nongnu.org Developers" <qemu-devel@nongnu.org>,
	"KONRAD Frederic" <frederic.konrad@adacore.com>,
	"Stefano Stabellini" <sstabellini@kernel.org>,
	qemu-arm <qemu-arm@nongnu.org>,
	"Philippe Mathieu-Daudé" <philmd@redhat.com>,
	"Luc Michel" <luc.michel@greensocs.com>,
	figlesia@xilinx.com
Subject: Re: [PATCH v1 7/9] hw/dma/xilinx_axidma: mm2s: Stream descriptor by descriptor
Date: Tue, 5 May 2020 12:57:17 -0700	[thread overview]
Message-ID: <CAKmqyKPSUzqQxvKEvu3TYJao4sVhFXz62OyVkoW2cpNpt8jFFQ@mail.gmail.com> (raw)
In-Reply-To: <20200430162439.2659-8-edgar.iglesias@gmail.com>

On Thu, Apr 30, 2020 at 9:31 AM Edgar E. Iglesias
<edgar.iglesias@gmail.com> wrote:
>
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Stream descriptor by descriptor from memory instead of
> buffering entire packets before pushing. This enables
> non-packet streaming clients to work and also lifts the
> limitation that our internal DMA buffer needs to be able
> to hold entire packets.
>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/dma/xilinx_axidma.c | 31 +++++++++++++++++--------------
>  1 file changed, 17 insertions(+), 14 deletions(-)
>
> diff --git a/hw/dma/xilinx_axidma.c b/hw/dma/xilinx_axidma.c
> index a770e12c96..101d32a965 100644
> --- a/hw/dma/xilinx_axidma.c
> +++ b/hw/dma/xilinx_axidma.c
> @@ -111,7 +111,6 @@ struct Stream {
>      int nr;
>
>      struct SDesc desc;
> -    int pos;
>      unsigned int complete_cnt;
>      uint32_t regs[R_MAX];
>      uint8_t app[20];
> @@ -267,7 +266,9 @@ static void stream_process_mem2s(struct Stream *s, StreamSlave *tx_data_dev,
>                                   StreamSlave *tx_control_dev)
>  {
>      uint32_t prev_d;
> -    unsigned int txlen;
> +    uint32_t txlen;
> +    uint64_t addr;
> +    bool eop;
>
>      if (!stream_running(s) || stream_idle(s)) {
>          return;
> @@ -282,24 +283,26 @@ static void stream_process_mem2s(struct Stream *s, StreamSlave *tx_data_dev,
>          }
>
>          if (stream_desc_sof(&s->desc)) {
> -            s->pos = 0;
>              stream_push(tx_control_dev, s->desc.app, sizeof(s->desc.app), true);
>          }
>
>          txlen = s->desc.control & SDESC_CTRL_LEN_MASK;
> -        if ((txlen + s->pos) > sizeof s->txbuf) {
> -            hw_error("%s: too small internal txbuf! %d\n", __func__,
> -                     txlen + s->pos);
> -        }
>
> -        address_space_read(&s->dma->as, s->desc.buffer_address,
> -                           MEMTXATTRS_UNSPECIFIED,
> -                           s->txbuf + s->pos, txlen);
> -        s->pos += txlen;
> +        eop = stream_desc_eof(&s->desc);
> +        addr = s->desc.buffer_address;
> +        while (txlen) {
> +            unsigned int len;
> +
> +            len = txlen > sizeof s->txbuf ? sizeof s->txbuf : txlen;
> +            address_space_read(&s->dma->as, addr,
> +                               MEMTXATTRS_UNSPECIFIED,
> +                               s->txbuf, len);
> +            stream_push(tx_data_dev, s->txbuf, len, eop && len == txlen);
> +            txlen -= len;
> +            addr += len;
> +        }
>
> -        if (stream_desc_eof(&s->desc)) {
> -            stream_push(tx_data_dev, s->txbuf, s->pos, true);
> -            s->pos = 0;
> +        if (eop) {
>              stream_complete(s);
>          }
>
> --
> 2.20.1
>
>