[PATCH v2 2/2] bcm2835_dma: Re-initialize xlen in TD mode

[PATCH v2 2/2] bcm2835_dma: Re-initialize xlen in TD mode

[Rene Stange]

TD (two dimensions) DMA mode did not work, because the xlen variable
has not been re-initialized before each additional ylen run through
in bcm2835_dma_update(), which has been fixed.

Signed-off-by: Rene Stange <rsta2@o2online.de>
---
 hw/dma/bcm2835_dma.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/dma/bcm2835_dma.c b/hw/dma/bcm2835_dma.c
index 667d951a6f..ccff5ed55b 100644
--- a/hw/dma/bcm2835_dma.c
+++ b/hw/dma/bcm2835_dma.c
@@ -54,7 +54,7 @@
 static void bcm2835_dma_update(BCM2835DMAState *s, unsigned c)
 {
     BCM2835DMAChan *ch = &s->chan[c];
-    uint32_t data, xlen, ylen;
+    uint32_t data, xlen, xlen_td, ylen;
     int16_t dst_stride, src_stride;
 
     if (!(s->enable & (1 << c))) {
@@ -82,6 +82,7 @@ static void bcm2835_dma_update(BCM2835DMAState *s, unsigned c)
             dst_stride = 0;
             src_stride = 0;
         }
+        xlen_td = xlen;
 
         while (ylen != 0) {
             /* Normal transfer mode */
@@ -117,6 +118,7 @@ static void bcm2835_dma_update(BCM2835DMAState *s, unsigned c)
             if (--ylen != 0) {
                 ch->source_ad += src_stride;
                 ch->dest_ad += dst_stride;
+                xlen = xlen_td;
             }
         }
         ch->cs |= BCM2708_DMA_END;
-- 
2.16.4

Re: [PATCH v2 2/2] bcm2835_dma: Re-initialize xlen in TD mode

[Philippe Mathieu-Daudé]

On 2/3/20 4:40 PM, Rene Stange wrote:
> TD (two dimensions) DMA mode did not work, because the xlen variable
> has not been re-initialized before each additional ylen run through
> in bcm2835_dma_update(), which has been fixed.

"which has been fixed" confused me, because this current patch is fixing 
it. Using present tense makes it easier to understand for non-native 
English speakers IMHO:

   TD (two dimensions) DMA mode does not work, because the xlen
   variable is not re-initialized before each additional ylen
   run through in bcm2835_dma_update(). Fix it.

If you agree, maybe Peter (the maintainer who will take your patch) can 
make the change for you.

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> 
> Signed-off-by: Rene Stange <rsta2@o2online.de>
> ---
>   hw/dma/bcm2835_dma.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/dma/bcm2835_dma.c b/hw/dma/bcm2835_dma.c
> index 667d951a6f..ccff5ed55b 100644
> --- a/hw/dma/bcm2835_dma.c
> +++ b/hw/dma/bcm2835_dma.c
> @@ -54,7 +54,7 @@
>   static void bcm2835_dma_update(BCM2835DMAState *s, unsigned c)
>   {
>       BCM2835DMAChan *ch = &s->chan[c];
> -    uint32_t data, xlen, ylen;
> +    uint32_t data, xlen, xlen_td, ylen;
>       int16_t dst_stride, src_stride;
>   
>       if (!(s->enable & (1 << c))) {
> @@ -82,6 +82,7 @@ static void bcm2835_dma_update(BCM2835DMAState *s, unsigned c)
>               dst_stride = 0;
>               src_stride = 0;
>           }
> +        xlen_td = xlen;
>   
>           while (ylen != 0) {
>               /* Normal transfer mode */
> @@ -117,6 +118,7 @@ static void bcm2835_dma_update(BCM2835DMAState *s, unsigned c)
>               if (--ylen != 0) {
>                   ch->source_ad += src_stride;
>                   ch->dest_ad += dst_stride;
> +                xlen = xlen_td;
>               }
>           }
>           ch->cs |= BCM2708_DMA_END;
> 

Re: [PATCH v2 2/2] bcm2835_dma: Re-initialize xlen in TD mode

[Rene Stange]

Philippe, of course you are right. I understand, what you mean. I'm a non-native
English speaker and I'm still learning. :)

Yes, I agree. Peter, please make the change, if you agree with the patch.

Thanks,

Rene


On Monday, 3 February 2020, 17:27:08 CET, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> On 2/3/20 4:40 PM, Rene Stange wrote:
> > TD (two dimensions) DMA mode did not work, because the xlen variable
> > has not been re-initialized before each additional ylen run through
> > in bcm2835_dma_update(), which has been fixed.
> 
> "which has been fixed" confused me, because this current patch is fixing 
> it. Using present tense makes it easier to understand for non-native 
> English speakers IMHO:
> 
>    TD (two dimensions) DMA mode does not work, because the xlen
>    variable is not re-initialized before each additional ylen
>    run through in bcm2835_dma_update(). Fix it.
> 
> If you agree, maybe Peter (the maintainer who will take your patch) can 
> make the change for you.
> 
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> 
> > 
> > Signed-off-by: Rene Stange <rsta2@o2online.de>
> > ---
> >   hw/dma/bcm2835_dma.c | 4 +++-
> >   1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/hw/dma/bcm2835_dma.c b/hw/dma/bcm2835_dma.c
> > index 667d951a6f..ccff5ed55b 100644
> > --- a/hw/dma/bcm2835_dma.c
> > +++ b/hw/dma/bcm2835_dma.c
> > @@ -54,7 +54,7 @@
> >   static void bcm2835_dma_update(BCM2835DMAState *s, unsigned c)
> >   {
> >       BCM2835DMAChan *ch = &s->chan[c];
> > -    uint32_t data, xlen, ylen;
> > +    uint32_t data, xlen, xlen_td, ylen;
> >       int16_t dst_stride, src_stride;
> >   
> >       if (!(s->enable & (1 << c))) {
> > @@ -82,6 +82,7 @@ static void bcm2835_dma_update(BCM2835DMAState *s, unsigned c)
> >               dst_stride = 0;
> >               src_stride = 0;
> >           }
> > +        xlen_td = xlen;
> >   
> >           while (ylen != 0) {
> >               /* Normal transfer mode */
> > @@ -117,6 +118,7 @@ static void bcm2835_dma_update(BCM2835DMAState *s, unsigned c)
> >               if (--ylen != 0) {
> >                   ch->source_ad += src_stride;
> >                   ch->dest_ad += dst_stride;
> > +                xlen = xlen_td;
> >               }
> >           }
> >           ch->cs |= BCM2708_DMA_END;
> > 
> 
> 
>