* [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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.