Re: [PATCH] spi: spi-rspi: Add force_dma variable to spi_ops

[Vinod Koul <vkoul@xxxxxxxxxx>]

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@xxxxxxxxxxxxxx>
> > > > 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@xxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxx/
> 
> > 
> > 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@xxxxxxxxxxxxxx/

Thanks for the explanation Biju. But why do we need .force_dma flag? 

> 
> Cheers,
> Biju
> 
> 
> > 
> > > > > > >
> > > > > > > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> > > > > >
> > > > > > 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

-- 
~Vinod