* [PATCH] bcm2835_dma: Fix TD mode @ 2020-01-24 17:55 Rene Stange 2020-01-27 8:29 ` Philippe Mathieu-Daudé 0 siblings, 1 reply; 4+ messages in thread From: Rene Stange @ 2020-01-24 17:55 UTC (permalink / raw) To: qemu-devel Cc: Peter Maydell, qemu-arm, Philippe Mathieu-Daudé, Andrew Baumann 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. 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; -- 2.16.4 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] bcm2835_dma: Fix TD mode 2020-01-24 17:55 [PATCH] bcm2835_dma: Fix TD mode Rene Stange @ 2020-01-27 8:29 ` Philippe Mathieu-Daudé 2020-01-27 11:20 ` Rene Stange 0 siblings, 1 reply; 4+ messages in thread From: Philippe Mathieu-Daudé @ 2020-01-27 8:29 UTC (permalink / raw) To: Rene Stange, qemu-devel; +Cc: Peter Maydell, qemu-arm, Andrew Baumann 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; > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] bcm2835_dma: Fix TD mode 2020-01-27 8:29 ` Philippe Mathieu-Daudé @ 2020-01-27 11:20 ` Rene Stange 2020-02-03 12:09 ` Philippe Mathieu-Daudé 0 siblings, 1 reply; 4+ messages in thread From: Rene Stange @ 2020-01-27 11:20 UTC (permalink / raw) To: Philippe Mathieu-Daudé Cc: Peter Maydell, qemu-arm, qemu-devel, Andrew Baumann 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; > > > > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] bcm2835_dma: Fix TD mode 2020-01-27 11:20 ` Rene Stange @ 2020-02-03 12:09 ` Philippe Mathieu-Daudé 0 siblings, 0 replies; 4+ messages in thread From: Philippe Mathieu-Daudé @ 2020-02-03 12:09 UTC (permalink / raw) To: Rene Stange; +Cc: Peter Maydell, qemu-arm, qemu-devel, Andrew Baumann Hi Rene, On 1/27/20 12:20 PM, Rene Stange wrote: > 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. So we have 2 fixes in 1 patch. I'd rather see 2 different commits: 1: Fix the ylen loop -- >8 -- --- a/hw/dma/bcm2835_dma.c +++ b/hw/dma/bcm2835_dma.c @@ -70,14 +70,14 @@ static void bcm2835_dma_update(BCM2835DMAState *s, unsigned c) ch->stride = ldl_le_phys(&s->dma_as, ch->conblk_ad + 16); ch->nextconbk = ldl_le_phys(&s->dma_as, ch->conblk_ad + 20); + ylen = 1; if (ch->ti & BCM2708_DMA_TDMODE) { /* 2D transfer mode */ - ylen = (ch->txfr_len >> 16) & 0x3fff; + ylen += (ch->txfr_len >> 16) & 0x3fff; xlen = ch->txfr_len & 0xffff; dst_stride = ch->stride >> 16; src_stride = ch->stride & 0xffff; } else { - ylen = 1; xlen = ch->txfr_len; dst_stride = 0; src_stride = 0; --- 2: re-initialized xlen: -- >8 -- --- 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; --- What do you think? If this sounds good to you, do you mind sending a v2 with the 2 patches? Thanks, Phil. >> >> 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; >>> >> >> > > > ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-02-03 12:10 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-01-24 17:55 [PATCH] bcm2835_dma: Fix TD mode Rene Stange 2020-01-27 8:29 ` Philippe Mathieu-Daudé 2020-01-27 11:20 ` Rene Stange 2020-02-03 12:09 ` Philippe Mathieu-Daudé
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).