Re: [PATCH] bcm2835_dma: Fix TD mode

[Rene Stange <rsta2@o2online.de>]

Hi Philippe,

I'm running an example program for my Circle bare metal framework for the
Raspberry Pi using the LittlevGL graphics library. It uses the TD DMA mode to
transfer pixel data to the screen buffer (10 lines at a time). Without the
given patch applied to QEMU only the first pixel line of each transfer is
shown in TigerVNC viewer, after applying it, a solid image is shown.

You can reproduce the problem on a 64-bit Linux machine as follows. The "sed"
command modifies the example program, so that it doesn't try to access the
(not available) USB HCI controller of the Raspberry Pi 3.

Regards,

Rene


cd
mkdir dma-test
cd dma-test/
wget https://developer.arm.com/-/media/Files/downloads/gnu-a/8.3-2019.03/binrel/gcc-arm-8.3-2019.03-x86_64-aarch64-elf.tar.xz
tar xJf gcc-arm-8.3-2019.03-x86_64-aarch64-elf.tar.xz
git clone https://github.com/rsta2/circle.git
cd circle
git submodule update --init
echo "AARCH = 64" > Config.mk
echo "RASPPI = 3" >> Config.mk
echo "PREFIX64 = ~/dma-test/gcc-arm-8.3-2019.03-x86_64-aarch64-elf/bin/aarch64-elf-" >> Config.mk
./makeall
cd addon/littlevgl/
make
cd sample/
sed -i -e "s/bOK = m_USBHCI/\/\/bOK = m_USBHCI/" kernel.cpp
make
qemu-system-aarch64 -M raspi3 -kernel kernel8.img


On Monday, 27 January 2020, 09:29:59 CET, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> Hi Rene,
> 
> On 1/24/20 6:55 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(). Furthermore ylen has to be increased by one after
> > reading it from the TXFR_LEN register, because a value of zero has to
> > result in one run through of the ylen loop. Both issues have been fixed.
> 
> What were you running, how can we reproduce?
> 
> > 
> > Signed-off-by: Rene Stange <rsta2@o2online.de>
> > ---
> >   hw/dma/bcm2835_dma.c | 9 +++++----
> >   1 file changed, 5 insertions(+), 4 deletions(-)
> > 
> > diff --git a/hw/dma/bcm2835_dma.c b/hw/dma/bcm2835_dma.c
> > index 1e458d7fba..0881c9506e 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))) {
> > @@ -72,13 +72,13 @@ static void bcm2835_dma_update(BCM2835DMAState *s, unsigned c)
> >   
> >           if (ch->ti & BCM2708_DMA_TDMODE) {
> >               /* 2D transfer mode */
> > -            ylen = (ch->txfr_len >> 16) & 0x3fff;
> > -            xlen = ch->txfr_len & 0xffff;
> > +            ylen = ((ch->txfr_len >> 16) & 0x3fff) + 1;
> > +            xlen_td = xlen = ch->txfr_len & 0xffff;
> >               dst_stride = ch->stride >> 16;
> >               src_stride = ch->stride & 0xffff;
> >           } else {
> >               ylen = 1;
> > -            xlen = ch->txfr_len;
> > +            xlen_td = xlen = ch->txfr_len;
> >               dst_stride = 0;
> >               src_stride = 0;
> >           }
> > @@ -117,6 +117,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;
> > 
> 
>