| Message ID | 20220716153934.292311-1-biju.das.jz@bp.renesas.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | spi: spi-rspi: Add force_dma variable to spi_ops | expand |
On Sat, Jul 16, 2022 at 04:39:34PM +0100, Biju Das wrote: > On RZ/G2L SoCs switching from DMA to interrupt mode, causes timeout > issue as we are not getting Rx interrupt even though SPRF bit is set in > the status register. > > But there is no issue if we don't switch between interrupt to DMA mode > or vice versa. So we need this for correctness until someone can figure out what goes wrong :/ > Performance comparison between interrupt and DMA mode on RZ/Five SMARC > platform connected to a display module shows that performance and > CPU utilization is much better with DMA mode compared to interrupt mode > (1->65 fps) and (98->8%). How does that look for something doing lots of short transfers (like register I/O for example), and what's the throughput like? DMA probably is the most sensible default even so since the impact of doing PIO for large transfers tends to be so bad, you have to be doing a *lot* of small I/O before it gets to be a problem whereas basically any large transfer will notice PIO. > This patch introduces a variable force_dma to avoid switching between > DMA to interrupt mode for RZ platforms. Not that it really matters but it's not a variable, it's a flag in the device configuration. I'm wondering if we might want a way to override this at runtime in case someone does have an application that suffers badly from being forced into DMA (eg, some IIO thing), that could be done incrementally though.
Hi Mark, Thanks for the feedback. > Subject: Re: [PATCH] spi: spi-rspi: Add force_dma variable to spi_ops > > On Sat, Jul 16, 2022 at 04:39:34PM +0100, Biju Das wrote: > > On RZ/G2L SoCs switching from DMA to interrupt mode, causes timeout > > issue as we are not getting Rx interrupt even though SPRF bit is set > > in the status register. > > > > But there is no issue if we don't switch between interrupt to DMA mode > > or vice versa. > > So we need this for correctness until someone can figure out what goes > wrong :/ Yes, I am in touch with our HW engineers for the missing interrupt when we switch from DMA to interrupt mode. I will update you once, I receive the feedback from them. > > > Performance comparison between interrupt and DMA mode on RZ/Five SMARC > > platform connected to a display module shows that performance and CPU > > utilization is much better with DMA mode compared to interrupt mode > > (1->65 fps) and (98->8%). > > How does that look for something doing lots of short transfers (like > register I/O for example), and what's the throughput like? DMA probably > is the most sensible default even so since the impact of doing PIO for > large transfers tends to be so bad, you have to be doing a *lot* of small > I/O before it gets to be a problem whereas basically any large transfer > will notice PIO. Basically, this performance measurement done by running LVGL on RZ/Five Connected to display module ili9341 by SPI interface. Currently we are switch to DMA mode for transfer length greater than 8 bytes. Since we have an issue with DMA to interrupt mode switching, initially we tried with PIO mode and fps was very low 1 fps. So we switched to DMA mode and it shoots up the fps to 65. Similar experiment done on RZ/G2UL connected to PMOD spi flash. In this case 1) The number of interrupts with PIO mode is 6 times greater than that of DMA. 2) The rd/wr test with DMA mode takes 3 minutes to finish whereas with PIO mode it takes 4 minutes to finish. > > > This patch introduces a variable force_dma to avoid switching between > > DMA to interrupt mode for RZ platforms. > > Not that it really matters but it's not a variable, it's a flag in the > device configuration. OK. > I'm wondering if we might want a way to override > this at runtime in case someone does have an application that suffers > badly from being forced into DMA (eg, some IIO thing), that could be done > incrementally though. OK. Sure. Cheers, Biju
Hi Biju, On Sat, Jul 16, 2022 at 5:39 PM Biju Das <biju.das.jz@bp.renesas.com> wrote: > On RZ/G2L SoCs switching from DMA to interrupt mode, causes timeout > issue as we are not getting Rx interrupt even though SPRF bit is set in > the status register. > > But there is no issue if we don't switch between interrupt to DMA mode > or vice versa. > > Performance comparison between interrupt and DMA mode on RZ/Five SMARC > platform connected to a display module shows that performance and > CPU utilization is much better with DMA mode compared to interrupt mode > (1->65 fps) and (98->8%). > > This patch introduces a variable force_dma to avoid switching between > DMA to interrupt mode for RZ platforms. > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> Thanks for your patch! > @@ -1196,6 +1197,7 @@ static const struct spi_ops rspi_rz_ops = { > .flags = SPI_CONTROLLER_MUST_RX | SPI_CONTROLLER_MUST_TX, > .fifo_size = 8, /* 8 for TX, 32 for RX */ > .num_hw_ss = 1, > + .force_dma = true, > }; Do you know if this is needed on RZ/A series, too? I know upstream does not have DMA support for RZ/A yet, but the BSP has? Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Hi Geert, + Chris, > Subject: Re: [PATCH] spi: spi-rspi: Add force_dma variable to spi_ops > > Hi Biju, > > On Sat, Jul 16, 2022 at 5:39 PM Biju Das <biju.das.jz@bp.renesas.com> wrote: > > On RZ/G2L SoCs switching from DMA to interrupt mode, causes timeout > > issue as we are not getting Rx interrupt even though SPRF bit is set > > in the status register. > > > > But there is no issue if we don't switch between interrupt to DMA mode > > or vice versa. > > > > Performance comparison between interrupt and DMA mode on RZ/Five SMARC > > platform connected to a display module shows that performance and CPU > > utilization is much better with DMA mode compared to interrupt mode > > (1->65 fps) and (98->8%). > > > > This patch introduces a variable force_dma to avoid switching between > > DMA to interrupt mode for RZ platforms. > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > Thanks for your patch! > > > @@ -1196,6 +1197,7 @@ static const struct spi_ops rspi_rz_ops = { > > .flags = SPI_CONTROLLER_MUST_RX | > SPI_CONTROLLER_MUST_TX, > > .fifo_size = 8, /* 8 for TX, 32 for RX */ > > .num_hw_ss = 1, > > + .force_dma = true, > > }; > > Do you know if this is needed on RZ/A series, too? I guess it is needed?? I may be wrong. I got a link from Chris [1]. As per this still We haven't found a solution. May be the priority is changed for this activity and no one looked after this. [1] https://lore.kernel.org/linux-renesas-soc/?q=spi-rspi+mixes+DMA+and+PIO+transfers+causing+PIO+transfer+to+fail > I knowupstream does not have DMA support for RZ/A yet, but the BSP has? RZ/G2L DMA driver is derived from RZ/A, with small changes it can support RZ/A. I am not sure is there any plan to upstream DMA support for RZ/A? I do not have RZ/A board. Cheers, Biju
Hi Biju, On Tue, Jul 19, 2022 at 10:29 AM Biju Das <biju.das.jz@bp.renesas.com> wrote: > > Subject: Re: [PATCH] spi: spi-rspi: Add force_dma variable to spi_ops > > On Sat, Jul 16, 2022 at 5:39 PM Biju Das <biju.das.jz@bp.renesas.com> wrote: > > > On RZ/G2L SoCs switching from DMA to interrupt mode, causes timeout > > > issue as we are not getting Rx interrupt even though SPRF bit is set > > > in the status register. > > > > > > But there is no issue if we don't switch between interrupt to DMA mode > > > or vice versa. > > > > > > Performance comparison between interrupt and DMA mode on RZ/Five SMARC > > > platform connected to a display module shows that performance and CPU > > > utilization is much better with DMA mode compared to interrupt mode > > > (1->65 fps) and (98->8%). > > > > > > This patch introduces a variable force_dma to avoid switching between > > > DMA to interrupt mode for RZ platforms. > > > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > > > Thanks for your patch! > > > > > @@ -1196,6 +1197,7 @@ static const struct spi_ops rspi_rz_ops = { > > > .flags = SPI_CONTROLLER_MUST_RX | > > SPI_CONTROLLER_MUST_TX, > > > .fifo_size = 8, /* 8 for TX, 32 for RX */ > > > .num_hw_ss = 1, > > > + .force_dma = true, > > > }; > > > > Do you know if this is needed on RZ/A series, too? > > I guess it is needed?? I may be wrong. I got a link from Chris [1]. As per this still > We haven't found a solution. May be the priority is changed for this activity and > no one looked after this. > > [1] https://lore.kernel.org/linux-renesas-soc/?q=spi-rspi+mixes+DMA+and+PIO+transfers+causing+PIO+transfer+to+fail Daniel said he found the issue, i.e. the dmac driver never resetting DMARS? > > I knowupstream does not have DMA support for RZ/A yet, but the BSP has? > > RZ/G2L DMA driver is derived from RZ/A, with small changes it can support RZ/A. > I am not sure is there any plan to upstream DMA support for RZ/A? > I do not have RZ/A board. I have, so it might happen, one day... Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Hi Geert, +Vinod > Subject: Re: [PATCH] spi: spi-rspi: Add force_dma variable to spi_ops > > Hi Biju, > > On Tue, Jul 19, 2022 at 10:29 AM Biju Das <biju.das.jz@bp.renesas.com> > wrote: > > > Subject: Re: [PATCH] spi: spi-rspi: Add force_dma variable to > > > spi_ops On Sat, Jul 16, 2022 at 5:39 PM Biju Das > <biju.das.jz@bp.renesas.com> wrote: > > > > On RZ/G2L SoCs switching from DMA to interrupt mode, causes > > > > timeout issue as we are not getting Rx interrupt even though SPRF > > > > bit is set in the status register. > > > > > > > > But there is no issue if we don't switch between interrupt to DMA > > > > mode or vice versa. > > > > > > > > Performance comparison between interrupt and DMA mode on RZ/Five > > > > SMARC platform connected to a display module shows that > > > > performance and CPU utilization is much better with DMA mode > > > > compared to interrupt mode > > > > (1->65 fps) and (98->8%). > > > > > > > > This patch introduces a variable force_dma to avoid switching > > > > between DMA to interrupt mode for RZ platforms. > > > > > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > > > > > Thanks for your patch! > > > > > > > @@ -1196,6 +1197,7 @@ static const struct spi_ops rspi_rz_ops = { > > > > .flags = SPI_CONTROLLER_MUST_RX | > > > SPI_CONTROLLER_MUST_TX, > > > > .fifo_size = 8, /* 8 for TX, 32 for RX */ > > > > .num_hw_ss = 1, > > > > + .force_dma = true, > > > > }; > > > > > > Do you know if this is needed on RZ/A series, too? > > > > I guess it is needed?? I may be wrong. I got a link from Chris [1]. As > > per this still We haven't found a solution. May be the priority is > > changed for this activity and no one looked after this. > > > > [1] > > Daniel said he found the issue, i.e. the dmac driver never resetting DMARS? Currently DMARS set during prepare and it never cleared. So I added device_synchronize callback in DMA driver to clear the DMARS. RSPI client driver after synchronizing DMA callback, calls dmaengine_synchronize which clears DMARS. With this DMA to interrupt mode switching is working fine. Cheers, Biju
On 19-07-22, 11:28, Biju Das wrote: > Hi Geert, > > +Vinod > > > Subject: Re: [PATCH] spi: spi-rspi: Add force_dma variable to spi_ops > > > > Hi Biju, > > > > On Tue, Jul 19, 2022 at 10:29 AM Biju Das <biju.das.jz@bp.renesas.com> > > wrote: > > > > Subject: Re: [PATCH] spi: spi-rspi: Add force_dma variable to > > > > spi_ops On Sat, Jul 16, 2022 at 5:39 PM Biju Das > > <biju.das.jz@bp.renesas.com> wrote: > > > > > On RZ/G2L SoCs switching from DMA to interrupt mode, causes > > > > > timeout issue as we are not getting Rx interrupt even though SPRF > > > > > bit is set in the status register. > > > > > > > > > > But there is no issue if we don't switch between interrupt to DMA > > > > > mode or vice versa. > > > > > > > > > > Performance comparison between interrupt and DMA mode on RZ/Five > > > > > SMARC platform connected to a display module shows that > > > > > performance and CPU utilization is much better with DMA mode > > > > > compared to interrupt mode > > > > > (1->65 fps) and (98->8%). > > > > > > > > > > This patch introduces a variable force_dma to avoid switching > > > > > between DMA to interrupt mode for RZ platforms. Why do you need a variable for that, if DMA is availble (you were able to allocate channels) then use DMA, otherwise fall back to PIO.. Or anything missing from context which I am not aware of? > > > > > > > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > > > > > > > Thanks for your patch! > > > > > > > > > @@ -1196,6 +1197,7 @@ static const struct spi_ops rspi_rz_ops = { > > > > > .flags = SPI_CONTROLLER_MUST_RX | > > > > SPI_CONTROLLER_MUST_TX, > > > > > .fifo_size = 8, /* 8 for TX, 32 for RX */ > > > > > .num_hw_ss = 1, > > > > > + .force_dma = true, > > > > > }; > > > > > > > > Do you know if this is needed on RZ/A series, too? > > > > > > I guess it is needed?? I may be wrong. I got a link from Chris [1]. As > > > per this still We haven't found a solution. May be the priority is > > > changed for this activity and no one looked after this. > > > > > > [1] > > > > Daniel said he found the issue, i.e. the dmac driver never resetting DMARS? > > Currently DMARS set during prepare and it never cleared. So I added device_synchronize > callback in DMA driver to clear the DMARS. RSPI client driver after synchronizing > DMA callback, calls dmaengine_synchronize which clears DMARS. > > With this DMA to interrupt mode switching is working fine. > > Cheers, > Biju >
Hi Vinod, > Subject: Re: [PATCH] spi: spi-rspi: Add force_dma variable to spi_ops > > On 19-07-22, 11:28, Biju Das wrote: > > Hi Geert, > > > > +Vinod > > > > > Subject: Re: [PATCH] spi: spi-rspi: Add force_dma variable to > > > spi_ops > > > > > > Hi Biju, > > > > > > On Tue, Jul 19, 2022 at 10:29 AM Biju Das > > > <biju.das.jz@bp.renesas.com> > > > wrote: > > > > > Subject: Re: [PATCH] spi: spi-rspi: Add force_dma variable to > > > > > spi_ops On Sat, Jul 16, 2022 at 5:39 PM Biju Das > > > <biju.das.jz@bp.renesas.com> wrote: > > > > > > On RZ/G2L SoCs switching from DMA to interrupt mode, causes > > > > > > timeout issue as we are not getting Rx interrupt even though > > > > > > SPRF bit is set in the status register. > > > > > > > > > > > > But there is no issue if we don't switch between interrupt to > > > > > > DMA mode or vice versa. > > > > > > > > > > > > Performance comparison between interrupt and DMA mode on > > > > > > RZ/Five SMARC platform connected to a display module shows > > > > > > that performance and CPU utilization is much better with DMA > > > > > > mode compared to interrupt mode > > > > > > (1->65 fps) and (98->8%). > > > > > > > > > > > > This patch introduces a variable force_dma to avoid switching > > > > > > between DMA to interrupt mode for RZ platforms. > > Why do you need a variable for that, if DMA is availble (you were able to > allocate channels) then use DMA, otherwise fall back to PIO.. I was using DMA. We are not getting rspi interrupts after the DMA to PIO switch because of [1]. ie, we are not clearing DMAR in DMA driver and interrupt requests to the interrupt controller are masked. [1] https://patchwork.kernel.org/project/linux-renesas-soc/patch/20220719150000.383722-2-biju.das.jz@bp.renesas.com/ > > Or anything missing from context which I am not aware of? After this discussion, I have posted [1] and [2] to fix this issue. [2] https://patchwork.kernel.org/project/linux-renesas-soc/patch/20220719150000.383722-1-biju.das.jz@bp.renesas.com/ Cheers, Biju > > > > > > > > > > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > > > > > > > > > Thanks for your patch! > > > > > > > > > > > @@ -1196,6 +1197,7 @@ static const struct spi_ops rspi_rz_ops = { > > > > > > .flags = SPI_CONTROLLER_MUST_RX | > > > > > SPI_CONTROLLER_MUST_TX, > > > > > > .fifo_size = 8, /* 8 for TX, 32 for RX */ > > > > > > .num_hw_ss = 1, > > > > > > + .force_dma = true, > > > > > > }; > > > > > > > > > > Do you know if this is needed on RZ/A series, too? > > > > > > > > I guess it is needed?? I may be wrong. I got a link from Chris > > > > [1]. As per this still We haven't found a solution. May be the > > > > priority is changed for this activity and no one looked after this. > > > > > > > > [1] > > > > > > Daniel said he found the issue, i.e. the dmac driver never resetting > DMARS? > > > > Currently DMARS set during prepare and it never cleared. So I added > > device_synchronize callback in DMA driver to clear the DMARS. RSPI > > client driver after synchronizing DMA callback, calls > dmaengine_synchronize which clears DMARS. > > > > With this DMA to interrupt mode switching is working fine. > > > > Cheers, > > Biju > > > > -- > ~Vinod
On 20-07-22, 05:13, Biju Das wrote: > Hi Vinod, > > > Subject: Re: [PATCH] spi: spi-rspi: Add force_dma variable to spi_ops > > > > On 19-07-22, 11:28, Biju Das wrote: > > > Hi Geert, > > > > > > +Vinod > > > > > > > Subject: Re: [PATCH] spi: spi-rspi: Add force_dma variable to > > > > spi_ops > > > > > > > > Hi Biju, > > > > > > > > On Tue, Jul 19, 2022 at 10:29 AM Biju Das > > > > <biju.das.jz@bp.renesas.com> > > > > wrote: > > > > > > Subject: Re: [PATCH] spi: spi-rspi: Add force_dma variable to > > > > > > spi_ops On Sat, Jul 16, 2022 at 5:39 PM Biju Das > > > > <biju.das.jz@bp.renesas.com> wrote: > > > > > > > On RZ/G2L SoCs switching from DMA to interrupt mode, causes > > > > > > > timeout issue as we are not getting Rx interrupt even though > > > > > > > SPRF bit is set in the status register. > > > > > > > > > > > > > > But there is no issue if we don't switch between interrupt to > > > > > > > DMA mode or vice versa. > > > > > > > > > > > > > > Performance comparison between interrupt and DMA mode on > > > > > > > RZ/Five SMARC platform connected to a display module shows > > > > > > > that performance and CPU utilization is much better with DMA > > > > > > > mode compared to interrupt mode > > > > > > > (1->65 fps) and (98->8%). > > > > > > > > > > > > > > This patch introduces a variable force_dma to avoid switching > > > > > > > between DMA to interrupt mode for RZ platforms. > > > > Why do you need a variable for that, if DMA is availble (you were able to > > allocate channels) then use DMA, otherwise fall back to PIO.. > > I was using DMA. We are not getting rspi interrupts after the DMA to PIO switch > because of [1]. ie, we are not clearing DMAR in DMA driver > and interrupt requests to the interrupt controller are masked. > > [1] https://patchwork.kernel.org/project/linux-renesas-soc/patch/20220719150000.383722-2-biju.das.jz@bp.renesas.com/ > > > > > Or anything missing from context which I am not aware of? > > After this discussion, I have posted [1] and [2] to fix this issue. > > [2] https://patchwork.kernel.org/project/linux-renesas-soc/patch/20220719150000.383722-1-biju.das.jz@bp.renesas.com/ Thanks for the explanation Biju. But why do we need .force_dma flag? > > Cheers, > Biju > > > > > > > > > > > > > > > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > > > > > > > > > > > Thanks for your patch! > > > > > > > > > > > > > @@ -1196,6 +1197,7 @@ static const struct spi_ops rspi_rz_ops = { > > > > > > > .flags = SPI_CONTROLLER_MUST_RX | > > > > > > SPI_CONTROLLER_MUST_TX, > > > > > > > .fifo_size = 8, /* 8 for TX, 32 for RX */ > > > > > > > .num_hw_ss = 1, > > > > > > > + .force_dma = true, > > > > > > > }; > > > > > > > > > > > > Do you know if this is needed on RZ/A series, too? > > > > > > > > > > I guess it is needed?? I may be wrong. I got a link from Chris > > > > > [1]. As per this still We haven't found a solution. May be the > > > > > priority is changed for this activity and no one looked after this. > > > > > > > > > > [1] > > > > > > > > Daniel said he found the issue, i.e. the dmac driver never resetting > > DMARS? > > > > > > Currently DMARS set during prepare and it never cleared. So I added > > > device_synchronize callback in DMA driver to clear the DMARS. RSPI > > > client driver after synchronizing DMA callback, calls > > dmaengine_synchronize which clears DMARS. > > > > > > With this DMA to interrupt mode switching is working fine. > > > > > > Cheers, > > > Biju > > > > > > > -- > > ~Vinod
Hi Vinod, > Subject: Re: [PATCH] spi: spi-rspi: Add force_dma variable to spi_ops > > On 20-07-22, 05:13, Biju Das wrote: > > Hi Vinod, > > > > > Subject: Re: [PATCH] spi: spi-rspi: Add force_dma variable to > > > spi_ops > > > > > > On 19-07-22, 11:28, Biju Das wrote: > > > > Hi Geert, > > > > > > > > +Vinod > > > > > > > > > Subject: Re: [PATCH] spi: spi-rspi: Add force_dma variable to > > > > > spi_ops > > > > > > > > > > Hi Biju, > > > > > > > > > > On Tue, Jul 19, 2022 at 10:29 AM Biju Das > > > > > <biju.das.jz@bp.renesas.com> > > > > > wrote: > > > > > > > Subject: Re: [PATCH] spi: spi-rspi: Add force_dma variable > > > > > > > to spi_ops On Sat, Jul 16, 2022 at 5:39 PM Biju Das > > > > > <biju.das.jz@bp.renesas.com> wrote: > > > > > > > > On RZ/G2L SoCs switching from DMA to interrupt mode, > > > > > > > > causes timeout issue as we are not getting Rx interrupt > > > > > > > > even though SPRF bit is set in the status register. > > > > > > > > > > > > > > > > But there is no issue if we don't switch between interrupt > > > > > > > > to DMA mode or vice versa. > > > > > > > > > > > > > > > > Performance comparison between interrupt and DMA mode on > > > > > > > > RZ/Five SMARC platform connected to a display module shows > > > > > > > > that performance and CPU utilization is much better with > > > > > > > > DMA mode compared to interrupt mode > > > > > > > > (1->65 fps) and (98->8%). > > > > > > > > > > > > > > > > This patch introduces a variable force_dma to avoid > > > > > > > > switching between DMA to interrupt mode for RZ platforms. > > > > > > Why do you need a variable for that, if DMA is availble (you were > > > able to allocate channels) then use DMA, otherwise fall back to PIO.. > > > > I was using DMA. We are not getting rspi interrupts after the DMA to > > PIO switch because of [1]. ie, we are not clearing DMAR in DMA driver > > and interrupt requests to the interrupt controller are masked. > > > > [1] > > > > > > > > Or anything missing from context which I am not aware of? > > > > After this discussion, I have posted [1] and [2] to fix this issue. > > > > [2] > > Thanks for the explanation Biju. But why do we need .force_dma flag? It is not required. This patch is not valid anymore. Initially I met with an issue(PIO fallback does not work). So posted this patch to make all transfer DMA by using .force_dma flag. Then Mark suggested that we should find the root cause. After that, Geert mentioned we are not clearing DMARS, that is the reason for interrupt miss. During DMA prepare, we set RSPI DMARS and signal is set for DMA transfer request signal and it masks rspi interrupts. When we do PIO mode, still DMARS is set, and we won't get rspi interrupt. The new patches which I posted[1] and [2] clears DMARS in dmaengine_synchronize() in dma callback after synchronizing with wait_event and PIO fallback works as expected. [1] https://patchwork.kernel.org/project/linux-renesas-soc/patch/20220719150000.383722-1-biju.das.jz@bp.renesas.com/ [2] https://patchwork.kernel.org/project/linux-renesas-soc/patch/20220719150000.383722-2-biju.das.jz@bp.renesas.com/ Cheers, Biju
On 20-07-22, 10:54, Biju Das wrote: > Hi Vinod, > > > Subject: Re: [PATCH] spi: spi-rspi: Add force_dma variable to spi_ops > > > > On 20-07-22, 05:13, Biju Das wrote: > > > Hi Vinod, > > > > > > > Subject: Re: [PATCH] spi: spi-rspi: Add force_dma variable to > > > > spi_ops > > > > > > > > On 19-07-22, 11:28, Biju Das wrote: > > > > > Hi Geert, > > > > > > > > > > +Vinod > > > > > > > > > > > Subject: Re: [PATCH] spi: spi-rspi: Add force_dma variable to > > > > > > spi_ops > > > > > > > > > > > > Hi Biju, > > > > > > > > > > > > On Tue, Jul 19, 2022 at 10:29 AM Biju Das > > > > > > <biju.das.jz@bp.renesas.com> > > > > > > wrote: > > > > > > > > Subject: Re: [PATCH] spi: spi-rspi: Add force_dma variable > > > > > > > > to spi_ops On Sat, Jul 16, 2022 at 5:39 PM Biju Das > > > > > > <biju.das.jz@bp.renesas.com> wrote: > > > > > > > > > On RZ/G2L SoCs switching from DMA to interrupt mode, > > > > > > > > > causes timeout issue as we are not getting Rx interrupt > > > > > > > > > even though SPRF bit is set in the status register. > > > > > > > > > > > > > > > > > > But there is no issue if we don't switch between interrupt > > > > > > > > > to DMA mode or vice versa. > > > > > > > > > > > > > > > > > > Performance comparison between interrupt and DMA mode on > > > > > > > > > RZ/Five SMARC platform connected to a display module shows > > > > > > > > > that performance and CPU utilization is much better with > > > > > > > > > DMA mode compared to interrupt mode > > > > > > > > > (1->65 fps) and (98->8%). > > > > > > > > > > > > > > > > > > This patch introduces a variable force_dma to avoid > > > > > > > > > switching between DMA to interrupt mode for RZ platforms. > > > > > > > > Why do you need a variable for that, if DMA is availble (you were > > > > able to allocate channels) then use DMA, otherwise fall back to PIO.. > > > > > > I was using DMA. We are not getting rspi interrupts after the DMA to > > > PIO switch because of [1]. ie, we are not clearing DMAR in DMA driver > > > and interrupt requests to the interrupt controller are masked. > > > > > > [1] > > > > > > > > > > > Or anything missing from context which I am not aware of? > > > > > > After this discussion, I have posted [1] and [2] to fix this issue. > > > > > > [2] > > > > Thanks for the explanation Biju. But why do we need .force_dma flag? > > It is not required. This patch is not valid anymore. okay, that sounds about right! > Initially I met with an issue(PIO fallback does not work). So posted this patch to make all transfer DMA by using .force_dma flag. > > Then Mark suggested that we should find the root cause. After that, Geert mentioned > we are not clearing DMARS, that is the reason for interrupt miss. > > During DMA prepare, we set RSPI DMARS and signal is set for DMA transfer request signal > and it masks rspi interrupts. When we do PIO mode, still DMARS is set, and we won't get rspi > interrupt. > > The new patches which I posted[1] and [2] clears DMARS in dmaengine_synchronize() in dma callback > after synchronizing with wait_event and PIO fallback works as expected. > > [1] https://patchwork.kernel.org/project/linux-renesas-soc/patch/20220719150000.383722-1-biju.das.jz@bp.renesas.com/ > [2] https://patchwork.kernel.org/project/linux-renesas-soc/patch/20220719150000.383722-2-biju.das.jz@bp.renesas.com/ > > Cheers, > Biju
diff --git a/drivers/spi/spi-rspi.c b/drivers/spi/spi-rspi.c index 7a014eeec2d0..f86b7b53288f 100644 --- a/drivers/spi/spi-rspi.c +++ b/drivers/spi/spi-rspi.c @@ -249,6 +249,7 @@ struct spi_ops { u16 flags; u16 fifo_size; u8 num_hw_ss; + bool force_dma; }; static void rspi_set_rate(struct rspi_data *rspi) @@ -677,7 +678,7 @@ static void qspi_receive_init(const struct rspi_data *rspi) static bool __rspi_can_dma(const struct rspi_data *rspi, const struct spi_transfer *xfer) { - return xfer->len > rspi->ops->fifo_size; + return rspi->ops->force_dma || (xfer->len > rspi->ops->fifo_size); } static bool rspi_can_dma(struct spi_controller *ctlr, struct spi_device *spi, @@ -1196,6 +1197,7 @@ static const struct spi_ops rspi_rz_ops = { .flags = SPI_CONTROLLER_MUST_RX | SPI_CONTROLLER_MUST_TX, .fifo_size = 8, /* 8 for TX, 32 for RX */ .num_hw_ss = 1, + .force_dma = true, }; static const struct spi_ops qspi_ops = {
On RZ/G2L SoCs switching from DMA to interrupt mode, causes timeout issue as we are not getting Rx interrupt even though SPRF bit is set in the status register. But there is no issue if we don't switch between interrupt to DMA mode or vice versa. Performance comparison between interrupt and DMA mode on RZ/Five SMARC platform connected to a display module shows that performance and CPU utilization is much better with DMA mode compared to interrupt mode (1->65 fps) and (98->8%). This patch introduces a variable force_dma to avoid switching between DMA to interrupt mode for RZ platforms. Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> --- drivers/spi/spi-rspi.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)