* [PATCH 1/4] hw/dma: sifive_pdma: reset Next* registers when Control.claim is set
2021-09-10 5:56 [PATCH 0/4] hw/dma: Align SiFive PDMA behavior with real hardware frank.chang
@ 2021-09-10 5:56 ` frank.chang
2021-09-11 12:37 ` Bin Meng
2021-09-10 5:56 ` [PATCH 2/4] hw/dma: sifive_pdma: claim bit must be set before DMA transactions frank.chang
` (2 subsequent siblings)
3 siblings, 1 reply; 12+ messages in thread
From: frank.chang @ 2021-09-10 5:56 UTC (permalink / raw)
To: qemu-devel, qemu-riscv
Cc: Frank Chang, Palmer Dabbelt, Bin Meng, Alistair Francis, Max Hsu
From: Frank Chang <frank.chang@sifive.com>
Setting Control.claim clears all of the chanel's Next registers.
This is effective only when Control.claim is set from 0 to 1.
Signed-off-by: Frank Chang <frank.chang@sifive.com>
Tested-by: Max Hsu <max.hsu@sifive.com>
---
hw/dma/sifive_pdma.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/hw/dma/sifive_pdma.c b/hw/dma/sifive_pdma.c
index 9b2ac2017d9..e723db9d700 100644
--- a/hw/dma/sifive_pdma.c
+++ b/hw/dma/sifive_pdma.c
@@ -54,6 +54,9 @@
#define DMA_EXEC_DST 0x110
#define DMA_EXEC_SRC 0x118
+#define CONFIG_WRSZ_DEFAULT 6
+#define CONFIG_RDSZ_DEFAULT 6
+
enum dma_chan_state {
DMA_CHAN_STATE_IDLE,
DMA_CHAN_STATE_STARTED,
@@ -221,6 +224,7 @@ static void sifive_pdma_write(void *opaque, hwaddr offset,
{
SiFivePDMAState *s = opaque;
int ch = SIFIVE_PDMA_CHAN_NO(offset);
+ bool claimed;
if (ch >= SIFIVE_PDMA_CHANS) {
qemu_log_mask(LOG_GUEST_ERROR, "%s: Invalid channel no %d\n",
@@ -231,6 +235,17 @@ static void sifive_pdma_write(void *opaque, hwaddr offset,
offset &= 0xfff;
switch (offset) {
case DMA_CONTROL:
+ claimed = !!s->chan[ch].control & CONTROL_CLAIM;
+
+ if (!claimed && (value & CONTROL_CLAIM)) {
+ /* reset Next* registers */
+ s->chan[ch].next_config = (CONFIG_RDSZ_DEFAULT << CONFIG_RDSZ_SHIFT) |
+ (CONFIG_WRSZ_DEFAULT << CONFIG_WRSZ_SHIFT);
+ s->chan[ch].next_bytes = 0;
+ s->chan[ch].next_dst = 0;
+ s->chan[ch].next_src = 0;
+ }
+
s->chan[ch].control = value;
if (value & CONTROL_RUN) {
--
2.25.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 1/4] hw/dma: sifive_pdma: reset Next* registers when Control.claim is set
2021-09-10 5:56 ` [PATCH 1/4] hw/dma: sifive_pdma: reset Next* registers when Control.claim is set frank.chang
@ 2021-09-11 12:37 ` Bin Meng
2021-09-11 13:12 ` Bin Meng
0 siblings, 1 reply; 12+ messages in thread
From: Bin Meng @ 2021-09-11 12:37 UTC (permalink / raw)
To: frank.chang
Cc: open list:RISC-V, Bin Meng, qemu-devel@nongnu.org Developers,
Max Hsu, Palmer Dabbelt, Alistair Francis
On Fri, Sep 10, 2021 at 1:56 PM <frank.chang@sifive.com> wrote:
>
> From: Frank Chang <frank.chang@sifive.com>
>
> Setting Control.claim clears all of the chanel's Next registers.
> This is effective only when Control.claim is set from 0 to 1.
>
> Signed-off-by: Frank Chang <frank.chang@sifive.com>
> Tested-by: Max Hsu <max.hsu@sifive.com>
> ---
> hw/dma/sifive_pdma.c | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> diff --git a/hw/dma/sifive_pdma.c b/hw/dma/sifive_pdma.c
> index 9b2ac2017d9..e723db9d700 100644
> --- a/hw/dma/sifive_pdma.c
> +++ b/hw/dma/sifive_pdma.c
> @@ -54,6 +54,9 @@
> #define DMA_EXEC_DST 0x110
> #define DMA_EXEC_SRC 0x118
>
> +#define CONFIG_WRSZ_DEFAULT 6
> +#define CONFIG_RDSZ_DEFAULT 6
The FU540 manual says the next config reset value is 0, not 6.
> +
> enum dma_chan_state {
> DMA_CHAN_STATE_IDLE,
> DMA_CHAN_STATE_STARTED,
> @@ -221,6 +224,7 @@ static void sifive_pdma_write(void *opaque, hwaddr offset,
> {
> SiFivePDMAState *s = opaque;
> int ch = SIFIVE_PDMA_CHAN_NO(offset);
> + bool claimed;
>
> if (ch >= SIFIVE_PDMA_CHANS) {
> qemu_log_mask(LOG_GUEST_ERROR, "%s: Invalid channel no %d\n",
> @@ -231,6 +235,17 @@ static void sifive_pdma_write(void *opaque, hwaddr offset,
> offset &= 0xfff;
> switch (offset) {
> case DMA_CONTROL:
> + claimed = !!s->chan[ch].control & CONTROL_CLAIM;
> +
> + if (!claimed && (value & CONTROL_CLAIM)) {
> + /* reset Next* registers */
> + s->chan[ch].next_config = (CONFIG_RDSZ_DEFAULT << CONFIG_RDSZ_SHIFT) |
> + (CONFIG_WRSZ_DEFAULT << CONFIG_WRSZ_SHIFT);
> + s->chan[ch].next_bytes = 0;
> + s->chan[ch].next_dst = 0;
> + s->chan[ch].next_src = 0;
> + }
> +
> s->chan[ch].control = value;
>
> if (value & CONTROL_RUN) {
Regards,
Bin
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 1/4] hw/dma: sifive_pdma: reset Next* registers when Control.claim is set
2021-09-11 12:37 ` Bin Meng
@ 2021-09-11 13:12 ` Bin Meng
2021-09-11 14:48 ` Bin Meng
2021-09-12 12:42 ` Frank Chang
0 siblings, 2 replies; 12+ messages in thread
From: Bin Meng @ 2021-09-11 13:12 UTC (permalink / raw)
To: frank.chang
Cc: open list:RISC-V, Bin Meng, qemu-devel@nongnu.org Developers,
Max Hsu, Palmer Dabbelt, Alistair Francis
On Sat, Sep 11, 2021 at 8:37 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> On Fri, Sep 10, 2021 at 1:56 PM <frank.chang@sifive.com> wrote:
> >
> > From: Frank Chang <frank.chang@sifive.com>
> >
> > Setting Control.claim clears all of the chanel's Next registers.
> > This is effective only when Control.claim is set from 0 to 1.
> >
> > Signed-off-by: Frank Chang <frank.chang@sifive.com>
> > Tested-by: Max Hsu <max.hsu@sifive.com>
> > ---
> > hw/dma/sifive_pdma.c | 15 +++++++++++++++
> > 1 file changed, 15 insertions(+)
> >
> > diff --git a/hw/dma/sifive_pdma.c b/hw/dma/sifive_pdma.c
> > index 9b2ac2017d9..e723db9d700 100644
> > --- a/hw/dma/sifive_pdma.c
> > +++ b/hw/dma/sifive_pdma.c
> > @@ -54,6 +54,9 @@
> > #define DMA_EXEC_DST 0x110
> > #define DMA_EXEC_SRC 0x118
> >
> > +#define CONFIG_WRSZ_DEFAULT 6
> > +#define CONFIG_RDSZ_DEFAULT 6
>
> The FU540 manual says the next config reset value is 0, not 6.
>
From patch#2 's log on Unmatched, I see where the number 6 is coming.
I also validated on Unleashed and observed the same. So there is a
documentation error.
Please add a comment to explain that.
Otherwise,
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/4] hw/dma: sifive_pdma: reset Next* registers when Control.claim is set
2021-09-11 13:12 ` Bin Meng
@ 2021-09-11 14:48 ` Bin Meng
2021-09-12 12:42 ` Frank Chang
1 sibling, 0 replies; 12+ messages in thread
From: Bin Meng @ 2021-09-11 14:48 UTC (permalink / raw)
To: frank.chang
Cc: open list:RISC-V, Bin Meng, qemu-devel@nongnu.org Developers,
Max Hsu, Palmer Dabbelt, Alistair Francis
On Sat, Sep 11, 2021 at 9:12 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> On Sat, Sep 11, 2021 at 8:37 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > On Fri, Sep 10, 2021 at 1:56 PM <frank.chang@sifive.com> wrote:
> > >
> > > From: Frank Chang <frank.chang@sifive.com>
> > >
> > > Setting Control.claim clears all of the chanel's Next registers.
> > > This is effective only when Control.claim is set from 0 to 1.
> > >
> > > Signed-off-by: Frank Chang <frank.chang@sifive.com>
> > > Tested-by: Max Hsu <max.hsu@sifive.com>
> > > ---
> > > hw/dma/sifive_pdma.c | 15 +++++++++++++++
> > > 1 file changed, 15 insertions(+)
> > >
> > > diff --git a/hw/dma/sifive_pdma.c b/hw/dma/sifive_pdma.c
> > > index 9b2ac2017d9..e723db9d700 100644
> > > --- a/hw/dma/sifive_pdma.c
> > > +++ b/hw/dma/sifive_pdma.c
> > > @@ -54,6 +54,9 @@
> > > #define DMA_EXEC_DST 0x110
> > > #define DMA_EXEC_SRC 0x118
> > >
> > > +#define CONFIG_WRSZ_DEFAULT 6
> > > +#define CONFIG_RDSZ_DEFAULT 6
> >
> > The FU540 manual says the next config reset value is 0, not 6.
> >
>
> From patch#2 's log on Unmatched, I see where the number 6 is coming.
> I also validated on Unleashed and observed the same. So there is a
> documentation error.
>
> Please add a comment to explain that.
>
> Otherwise,
> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Tested-by: Bin Meng <bmeng.cn@gmail.com>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/4] hw/dma: sifive_pdma: reset Next* registers when Control.claim is set
2021-09-11 13:12 ` Bin Meng
2021-09-11 14:48 ` Bin Meng
@ 2021-09-12 12:42 ` Frank Chang
1 sibling, 0 replies; 12+ messages in thread
From: Frank Chang @ 2021-09-12 12:42 UTC (permalink / raw)
To: Bin Meng
Cc: open list:RISC-V, Bin Meng, qemu-devel@nongnu.org Developers,
Max Hsu, Palmer Dabbelt, Alistair Francis
[-- Attachment #1: Type: text/plain, Size: 1398 bytes --]
On Sat, Sep 11, 2021 at 9:12 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> On Sat, Sep 11, 2021 at 8:37 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > On Fri, Sep 10, 2021 at 1:56 PM <frank.chang@sifive.com> wrote:
> > >
> > > From: Frank Chang <frank.chang@sifive.com>
> > >
> > > Setting Control.claim clears all of the chanel's Next registers.
> > > This is effective only when Control.claim is set from 0 to 1.
> > >
> > > Signed-off-by: Frank Chang <frank.chang@sifive.com>
> > > Tested-by: Max Hsu <max.hsu@sifive.com>
> > > ---
> > > hw/dma/sifive_pdma.c | 15 +++++++++++++++
> > > 1 file changed, 15 insertions(+)
> > >
> > > diff --git a/hw/dma/sifive_pdma.c b/hw/dma/sifive_pdma.c
> > > index 9b2ac2017d9..e723db9d700 100644
> > > --- a/hw/dma/sifive_pdma.c
> > > +++ b/hw/dma/sifive_pdma.c
> > > @@ -54,6 +54,9 @@
> > > #define DMA_EXEC_DST 0x110
> > > #define DMA_EXEC_SRC 0x118
> > >
> > > +#define CONFIG_WRSZ_DEFAULT 6
> > > +#define CONFIG_RDSZ_DEFAULT 6
> >
> > The FU540 manual says the next config reset value is 0, not 6.
> >
>
> From patch#2 's log on Unmatched, I see where the number 6 is coming.
> I also validated on Unleashed and observed the same. So there is a
> documentation error.
>
> Please add a comment to explain that.
>
Thanks for the review.
Will update it.
Regards,
Frank Chang
>
> Otherwise,
> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
>
[-- Attachment #2: Type: text/html, Size: 2529 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/4] hw/dma: sifive_pdma: claim bit must be set before DMA transactions
2021-09-10 5:56 [PATCH 0/4] hw/dma: Align SiFive PDMA behavior with real hardware frank.chang
2021-09-10 5:56 ` [PATCH 1/4] hw/dma: sifive_pdma: reset Next* registers when Control.claim is set frank.chang
@ 2021-09-10 5:56 ` frank.chang
2021-09-11 14:48 ` Bin Meng
2021-09-10 5:56 ` [PATCH 3/4] hw/dma: sifive_pdma: allow non-multiple transaction size transactions frank.chang
2021-09-10 5:56 ` [PATCH 4/4] hw/dma: sifive_pdma: don't set Control.error if 0 bytes to transfer frank.chang
3 siblings, 1 reply; 12+ messages in thread
From: frank.chang @ 2021-09-10 5:56 UTC (permalink / raw)
To: qemu-devel, qemu-riscv
Cc: Frank Chang, Palmer Dabbelt, Bin Meng, Alistair Francis, Max Hsu
From: Frank Chang <frank.chang@sifive.com>
Real PDMA must have Control.claim bit to be set before
Control.run bit is set to start any DMA transactions.
Otherwise nothing will be transferred.
The following result is PDMA tested in U-boot on Unmatched board:
=> mw.l 0x3000000 0x0 <= Disclaim channel 0
(Channel 0 is not claimed)
=> mw.l 0x3000004 0x55000000 <= wsize = rsize = 5 (2^5 = 32 bytes)
=> mw.q 0x3000008 0x2 <= NextBytes = 2
=> mw.q 0x3000010 0x84000000 <= NextDestination = 0x84000000
=> mw.q 0x3000018 0x84001000 <= NextSource = 0x84001000
=> mw.l 0x84000000 0x87654321 <= Fill test data to dst
=> mw.l 0x84001000 0x12345678 <= Fill test data to src
=> md.l 0x84000000 1; md.l 0x84001000 1 <= Dump src/dst memory contents
84000000: 87654321 !Ce.
84001000: 12345678 xV4.
=> md.l 0x3000000 8 <= Dump PDMA status
03000000: 00000000 55000000 00000002 00000000 .......U........
03000010: 84000000 00000000 84001000 00000000 ................
=> mw.l 0x3000000 0x3 <= Set channel 0 run and claim bits
=> md.l 0x3000000 8 <= Dump PDMA status
03000000: 00000001 66000000 00000000 00000000 .......f........
03000010: 00000000 00000000 00000000 00000000 ................
=> md.l 0x84000000 1; md.l 0x84001000 1 <= Dump src/dst memory contents
84000000: 87654321 !Ce.
84001000: 12345678 xV4.
Signed-off-by: Frank Chang <frank.chang@sifive.com>
Tested-by: Max Hsu <max.hsu@sifive.com>
---
hw/dma/sifive_pdma.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/hw/dma/sifive_pdma.c b/hw/dma/sifive_pdma.c
index e723db9d700..1311b80a5cd 100644
--- a/hw/dma/sifive_pdma.c
+++ b/hw/dma/sifive_pdma.c
@@ -248,6 +248,15 @@ static void sifive_pdma_write(void *opaque, hwaddr offset,
s->chan[ch].control = value;
+ /*
+ * If channel was not claimed before run bit is set,
+ * DMA won't run.
+ */
+ if (!claimed) {
+ s->chan[ch].control &= ~CONTROL_RUN;
+ return;
+ }
+
if (value & CONTROL_RUN) {
sifive_pdma_run(s, ch);
}
--
2.25.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 2/4] hw/dma: sifive_pdma: claim bit must be set before DMA transactions
2021-09-10 5:56 ` [PATCH 2/4] hw/dma: sifive_pdma: claim bit must be set before DMA transactions frank.chang
@ 2021-09-11 14:48 ` Bin Meng
0 siblings, 0 replies; 12+ messages in thread
From: Bin Meng @ 2021-09-11 14:48 UTC (permalink / raw)
To: frank.chang
Cc: open list:RISC-V, Bin Meng, qemu-devel@nongnu.org Developers,
Max Hsu, Palmer Dabbelt, Alistair Francis
On Fri, Sep 10, 2021 at 1:58 PM <frank.chang@sifive.com> wrote:
>
> From: Frank Chang <frank.chang@sifive.com>
>
> Real PDMA must have Control.claim bit to be set before
> Control.run bit is set to start any DMA transactions.
> Otherwise nothing will be transferred.
>
> The following result is PDMA tested in U-boot on Unmatched board:
%s/U-boot/U-Boot
>
> => mw.l 0x3000000 0x0 <= Disclaim channel 0
> (Channel 0 is not claimed)
> => mw.l 0x3000004 0x55000000 <= wsize = rsize = 5 (2^5 = 32 bytes)
> => mw.q 0x3000008 0x2 <= NextBytes = 2
> => mw.q 0x3000010 0x84000000 <= NextDestination = 0x84000000
> => mw.q 0x3000018 0x84001000 <= NextSource = 0x84001000
> => mw.l 0x84000000 0x87654321 <= Fill test data to dst
> => mw.l 0x84001000 0x12345678 <= Fill test data to src
> => md.l 0x84000000 1; md.l 0x84001000 1 <= Dump src/dst memory contents
> 84000000: 87654321 !Ce.
> 84001000: 12345678 xV4.
> => md.l 0x3000000 8 <= Dump PDMA status
> 03000000: 00000000 55000000 00000002 00000000 .......U........
> 03000010: 84000000 00000000 84001000 00000000 ................
> => mw.l 0x3000000 0x3 <= Set channel 0 run and claim bits
> => md.l 0x3000000 8 <= Dump PDMA status
> 03000000: 00000001 66000000 00000000 00000000 .......f........
> 03000010: 00000000 00000000 00000000 00000000 ................
> => md.l 0x84000000 1; md.l 0x84001000 1 <= Dump src/dst memory contents
> 84000000: 87654321 !Ce.
> 84001000: 12345678 xV4.
>
> Signed-off-by: Frank Chang <frank.chang@sifive.com>
> Tested-by: Max Hsu <max.hsu@sifive.com>
> ---
> hw/dma/sifive_pdma.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
Otherwise,
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Tested-by: Bin Meng <bmeng.cn@gmail.com>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 3/4] hw/dma: sifive_pdma: allow non-multiple transaction size transactions
2021-09-10 5:56 [PATCH 0/4] hw/dma: Align SiFive PDMA behavior with real hardware frank.chang
2021-09-10 5:56 ` [PATCH 1/4] hw/dma: sifive_pdma: reset Next* registers when Control.claim is set frank.chang
2021-09-10 5:56 ` [PATCH 2/4] hw/dma: sifive_pdma: claim bit must be set before DMA transactions frank.chang
@ 2021-09-10 5:56 ` frank.chang
2021-09-11 14:48 ` Bin Meng
2021-09-10 5:56 ` [PATCH 4/4] hw/dma: sifive_pdma: don't set Control.error if 0 bytes to transfer frank.chang
3 siblings, 1 reply; 12+ messages in thread
From: frank.chang @ 2021-09-10 5:56 UTC (permalink / raw)
To: qemu-devel, qemu-riscv
Cc: Frank Chang, Bin Meng, Green Wan, Max Hsu, Palmer Dabbelt,
Alistair Francis
From: Green Wan <green.wan@sifive.com>
Real PDMA is able to deal with non-multiple transaction size transactions.
The following result is PDMA tested in U-boot on Unmatched board:
=> mw.l 0x3000000 0x0 <= Disclaim channel 0
=> mw.l 0x3000000 0x1 <= Claim channel 0
=> mw.l 0x3000004 0x11000000 <= wsize = rsize = 1 (2^1 = 2 bytes)
=> mw.q 0x3000008 0x3 <= NextBytes = 3
=> mw.q 0x3000010 0x84000000 <= NextDestination = 0x84000000
=> mw.q 0x3000018 0x84001000 <= NextSource = 0x84001000
=> mw.l 0x84000000 0x87654321 <= Fill test data to dst
=> mw.l 0x84001000 0x12345678 <= Fill test data to src
=> md.l 0x84000000 1; md.l 0x84001000 1 <= Dump src/dst memory contents
84000000: 87654321 !Ce.
84001000: 12345678 xV4.
=> md.l 0x3000000 8 <= Dump PDMA status
03000000: 00000001 11000000 00000003 00000000 ................
03000010: 84000000 00000000 84001000 00000000 ................
=> mw.l 0x3000000 0x3 <= Set channel 0 run and claim bits
=> md.l 0x3000000 8 <= Dump PDMA status
03000000: 40000001 11000000 00000003 00000000 ...@............
03000010: 84000000 00000000 84001000 00000000 ................
=> md.l 0x84000000 1; md.l 0x84001000 1 <= Dump src/dst memory contents
84000000: 87345678 xV4.
84001000: 12345678 xV4.
Signed-off-by: Green Wan <green.wan@sifive.com>
Reviewed-by: Frank Chang <frank.chang@sifive.com>
Tested-by: Max Hsu <max.hsu@sifive.com>
Signed-off-by: Frank Chang <frank.chang@sifive.com>
---
hw/dma/sifive_pdma.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/hw/dma/sifive_pdma.c b/hw/dma/sifive_pdma.c
index 1311b80a5cd..d6980fbbd62 100644
--- a/hw/dma/sifive_pdma.c
+++ b/hw/dma/sifive_pdma.c
@@ -70,7 +70,7 @@ static void sifive_pdma_run(SiFivePDMAState *s, int ch)
uint64_t dst = s->chan[ch].next_dst;
uint64_t src = s->chan[ch].next_src;
uint32_t config = s->chan[ch].next_config;
- int wsize, rsize, size;
+ int wsize, rsize, size, remainder;
uint8_t buf[64];
int n;
@@ -102,11 +102,7 @@ static void sifive_pdma_run(SiFivePDMAState *s, int ch)
size = 6;
}
size = 1 << size;
-
- /* the bytes to transfer should be multiple of transaction size */
- if (bytes % size) {
- goto error;
- }
+ remainder = bytes % size;
/* indicate a DMA transfer is started */
s->chan[ch].state = DMA_CHAN_STATE_STARTED;
@@ -127,6 +123,14 @@ static void sifive_pdma_run(SiFivePDMAState *s, int ch)
s->chan[ch].exec_bytes -= size;
}
+ if (remainder) {
+ cpu_physical_memory_read(s->chan[ch].exec_src, buf, remainder);
+ cpu_physical_memory_write(s->chan[ch].exec_dst, buf, remainder);
+ s->chan[ch].exec_src += remainder;
+ s->chan[ch].exec_dst += remainder;
+ s->chan[ch].exec_bytes -= remainder;
+ }
+
/* indicate a DMA transfer is done */
s->chan[ch].state = DMA_CHAN_STATE_DONE;
s->chan[ch].control &= ~CONTROL_RUN;
--
2.25.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 3/4] hw/dma: sifive_pdma: allow non-multiple transaction size transactions
2021-09-10 5:56 ` [PATCH 3/4] hw/dma: sifive_pdma: allow non-multiple transaction size transactions frank.chang
@ 2021-09-11 14:48 ` Bin Meng
0 siblings, 0 replies; 12+ messages in thread
From: Bin Meng @ 2021-09-11 14:48 UTC (permalink / raw)
To: frank.chang
Cc: open list:RISC-V, Bin Meng, qemu-devel@nongnu.org Developers,
Green Wan, Max Hsu, Palmer Dabbelt, Alistair Francis
On Fri, Sep 10, 2021 at 1:56 PM <frank.chang@sifive.com> wrote:
>
> From: Green Wan <green.wan@sifive.com>
>
> Real PDMA is able to deal with non-multiple transaction size transactions.
>
> The following result is PDMA tested in U-boot on Unmatched board:
%s/U-boot/U-Boot
>
> => mw.l 0x3000000 0x0 <= Disclaim channel 0
> => mw.l 0x3000000 0x1 <= Claim channel 0
> => mw.l 0x3000004 0x11000000 <= wsize = rsize = 1 (2^1 = 2 bytes)
> => mw.q 0x3000008 0x3 <= NextBytes = 3
> => mw.q 0x3000010 0x84000000 <= NextDestination = 0x84000000
> => mw.q 0x3000018 0x84001000 <= NextSource = 0x84001000
> => mw.l 0x84000000 0x87654321 <= Fill test data to dst
> => mw.l 0x84001000 0x12345678 <= Fill test data to src
> => md.l 0x84000000 1; md.l 0x84001000 1 <= Dump src/dst memory contents
> 84000000: 87654321 !Ce.
> 84001000: 12345678 xV4.
> => md.l 0x3000000 8 <= Dump PDMA status
> 03000000: 00000001 11000000 00000003 00000000 ................
> 03000010: 84000000 00000000 84001000 00000000 ................
> => mw.l 0x3000000 0x3 <= Set channel 0 run and claim bits
> => md.l 0x3000000 8 <= Dump PDMA status
> 03000000: 40000001 11000000 00000003 00000000 ...@............
> 03000010: 84000000 00000000 84001000 00000000 ................
> => md.l 0x84000000 1; md.l 0x84001000 1 <= Dump src/dst memory contents
> 84000000: 87345678 xV4.
> 84001000: 12345678 xV4.
>
> Signed-off-by: Green Wan <green.wan@sifive.com>
> Reviewed-by: Frank Chang <frank.chang@sifive.com>
> Tested-by: Max Hsu <max.hsu@sifive.com>
> Signed-off-by: Frank Chang <frank.chang@sifive.com>
> ---
> hw/dma/sifive_pdma.c | 16 ++++++++++------
> 1 file changed, 10 insertions(+), 6 deletions(-)
>
Otherwise,
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Tested-by: Bin Meng <bmeng.cn@gmail.com>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 4/4] hw/dma: sifive_pdma: don't set Control.error if 0 bytes to transfer
2021-09-10 5:56 [PATCH 0/4] hw/dma: Align SiFive PDMA behavior with real hardware frank.chang
` (2 preceding siblings ...)
2021-09-10 5:56 ` [PATCH 3/4] hw/dma: sifive_pdma: allow non-multiple transaction size transactions frank.chang
@ 2021-09-10 5:56 ` frank.chang
2021-09-11 14:48 ` Bin Meng
3 siblings, 1 reply; 12+ messages in thread
From: frank.chang @ 2021-09-10 5:56 UTC (permalink / raw)
To: qemu-devel, qemu-riscv
Cc: Frank Chang, Palmer Dabbelt, Bin Meng, Alistair Francis, Max Hsu
From: Frank Chang <frank.chang@sifive.com>
Real PDMA doesn't set Control.error if there are 0 bytes to be
transferred. The DMA transfer is still success.
The following result is PDMA tested in U-boot on Unmatched board:
=> mw.l 0x3000000 0x0 <= Disclaim channel 0
=> mw.l 0x3000000 0x1 <= Claim channel 0
=> mw.l 0x3000004 0x55000000 <= wsize = rsize = 5 (2^5 = 32 bytes)
=> mw.q 0x3000008 0x0 <= NextBytes = 0
=> mw.q 0x3000010 0x84000000 <= NextDestination = 0x84000000
=> mw.q 0x3000018 0x84001000 <= NextSource = 0x84001000
=> mw.l 0x84000000 0x87654321 <= Fill test data to dst
=> mw.l 0x84001000 0x12345678 <= Fill test data to src
=> md.l 0x84000000 1; md.l 0x84001000 1 <= Dump src/dst memory contents
84000000: 87654321 !Ce.
84001000: 12345678 xV4.
=> md.l 0x3000000 8 <= Dump PDMA status
03000000: 00000001 55000000 00000000 00000000 .......U........
03000010: 84000000 00000000 84001000 00000000 ................
=> mw.l 0x3000000 0x3 <= Set channel 0 run and claim bits
=> md.l 0x3000000 8 <= Dump PDMA status
03000000: 40000001 55000000 00000000 00000000 ...@...U........
03000010: 84000000 00000000 84001000 00000000 ................
=> md.l 0x84000000 1; md.l 0x84001000 1 <= Dump src/dst memory contents
84000000: 87654321 !Ce.
84001000: 12345678 xV4.
Signed-off-by: Frank Chang <frank.chang@sifive.com>
Tested-by: Max Hsu <max.hsu@sifive.com>
---
hw/dma/sifive_pdma.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/hw/dma/sifive_pdma.c b/hw/dma/sifive_pdma.c
index d6980fbbd62..133db817dc7 100644
--- a/hw/dma/sifive_pdma.c
+++ b/hw/dma/sifive_pdma.c
@@ -76,7 +76,7 @@ static void sifive_pdma_run(SiFivePDMAState *s, int ch)
/* do nothing if bytes to transfer is zero */
if (!bytes) {
- goto error;
+ goto done;
}
/*
@@ -131,11 +131,6 @@ static void sifive_pdma_run(SiFivePDMAState *s, int ch)
s->chan[ch].exec_bytes -= remainder;
}
- /* indicate a DMA transfer is done */
- s->chan[ch].state = DMA_CHAN_STATE_DONE;
- s->chan[ch].control &= ~CONTROL_RUN;
- s->chan[ch].control |= CONTROL_DONE;
-
/* reload exec_ registers if repeat is required */
if (s->chan[ch].next_config & CONFIG_REPEAT) {
s->chan[ch].exec_bytes = bytes;
@@ -143,6 +138,11 @@ static void sifive_pdma_run(SiFivePDMAState *s, int ch)
s->chan[ch].exec_src = src;
}
+done:
+ /* indicate a DMA transfer is done */
+ s->chan[ch].state = DMA_CHAN_STATE_DONE;
+ s->chan[ch].control &= ~CONTROL_RUN;
+ s->chan[ch].control |= CONTROL_DONE;
return;
error:
--
2.25.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 4/4] hw/dma: sifive_pdma: don't set Control.error if 0 bytes to transfer
2021-09-10 5:56 ` [PATCH 4/4] hw/dma: sifive_pdma: don't set Control.error if 0 bytes to transfer frank.chang
@ 2021-09-11 14:48 ` Bin Meng
0 siblings, 0 replies; 12+ messages in thread
From: Bin Meng @ 2021-09-11 14:48 UTC (permalink / raw)
To: frank.chang
Cc: open list:RISC-V, Bin Meng, qemu-devel@nongnu.org Developers,
Max Hsu, Palmer Dabbelt, Alistair Francis
On Fri, Sep 10, 2021 at 1:59 PM <frank.chang@sifive.com> wrote:
>
> From: Frank Chang <frank.chang@sifive.com>
>
> Real PDMA doesn't set Control.error if there are 0 bytes to be
nits: 0 byte
Please fix the commit title as well
> transferred. The DMA transfer is still success.
>
> The following result is PDMA tested in U-boot on Unmatched board:
%s/U-boot/U-Boot
>
> => mw.l 0x3000000 0x0 <= Disclaim channel 0
> => mw.l 0x3000000 0x1 <= Claim channel 0
> => mw.l 0x3000004 0x55000000 <= wsize = rsize = 5 (2^5 = 32 bytes)
> => mw.q 0x3000008 0x0 <= NextBytes = 0
> => mw.q 0x3000010 0x84000000 <= NextDestination = 0x84000000
> => mw.q 0x3000018 0x84001000 <= NextSource = 0x84001000
> => mw.l 0x84000000 0x87654321 <= Fill test data to dst
> => mw.l 0x84001000 0x12345678 <= Fill test data to src
> => md.l 0x84000000 1; md.l 0x84001000 1 <= Dump src/dst memory contents
> 84000000: 87654321 !Ce.
> 84001000: 12345678 xV4.
> => md.l 0x3000000 8 <= Dump PDMA status
> 03000000: 00000001 55000000 00000000 00000000 .......U........
> 03000010: 84000000 00000000 84001000 00000000 ................
> => mw.l 0x3000000 0x3 <= Set channel 0 run and claim bits
> => md.l 0x3000000 8 <= Dump PDMA status
> 03000000: 40000001 55000000 00000000 00000000 ...@...U........
> 03000010: 84000000 00000000 84001000 00000000 ................
> => md.l 0x84000000 1; md.l 0x84001000 1 <= Dump src/dst memory contents
> 84000000: 87654321 !Ce.
> 84001000: 12345678 xV4.
>
> Signed-off-by: Frank Chang <frank.chang@sifive.com>
> Tested-by: Max Hsu <max.hsu@sifive.com>
> ---
> hw/dma/sifive_pdma.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
Otherwise,
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Tested-by: Bin Meng <bmeng.cn@gmail.com>
^ permalink raw reply [flat|nested] 12+ messages in thread