| Message ID | 1574403231-18512-5-git-send-email-yoshihiro.shimoda.uh@renesas.com (mailing list archive) |
|---|---|
| State | Under Review |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | mmc: host: renesas_sdhi_sys_dmac: change dma_buswidth | expand |
> On November 22, 2019 at 7:13 AM Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> wrote: > > > To improve performance, this patch sets dma_buswidth value to 32 > when transfer size is multiples of 32. In other words, a sd card > transfer's size if not multiples of 32, this driver uses PIO > and then the performance will be down. > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > --- > drivers/mmc/host/renesas_sdhi_sys_dmac.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/mmc/host/renesas_sdhi_sys_dmac.c b/drivers/mmc/host/renesas_sdhi_sys_dmac.c > index 09137cc..65e71b6 100644 > --- a/drivers/mmc/host/renesas_sdhi_sys_dmac.c > +++ b/drivers/mmc/host/renesas_sdhi_sys_dmac.c > @@ -58,7 +58,7 @@ static const struct renesas_sdhi_of_data of_rcar_gen2_compatible = { > .capabilities = MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ | > MMC_CAP_CMD23, > .capabilities2 = MMC_CAP2_NO_WRITE_PROTECT, > - .dma_buswidth = DMA_SLAVE_BUSWIDTH_4_BYTES, > + .dma_buswidth = DMA_SLAVE_BUSWIDTH_32_BYTES, > .dma_rx_offset = 0x2000, > .scc_offset = 0x0300, > .taps = rcar_gen2_scc_taps, > -- > 2.7.4 > Reviewed-by: Ulrich Hecht <uli+renesas@fpond.eu> CU Uli
Hi Shimoda-san, Interesting series! > - .dma_buswidth = DMA_SLAVE_BUSWIDTH_4_BYTES, > + .dma_buswidth = DMA_SLAVE_BUSWIDTH_32_BYTES, Two very high level questions: 1) can't we set dma_priv->dma_buswidth at runtime when we know what the card is capable of? Either DMA_SLAVE_BUSWIDTH_32_BYTES or DMA_SLAVE_BUSWIDTH_4_BYTES? Then we don't need to fallback to PIO. AFAIS, we only Gen2 sets .dma_buswidth in of_data, so we could even remove it from of_data entirely? 2) Just by grepping in mmc/hosts, I see that no driver uses DMA_SLAVE_BUSWIDTH_32_BYTES. Do you have an idea why? Because it is the convenient setting which works for all cards? Thanks and kind regards, Wolfram
Hi Wolfram-san, Thank you for your comments! > From: Wolfram Sang, Sent: Friday, November 29, 2019 6:07 AM > > Hi Shimoda-san, > > Interesting series! > > > - .dma_buswidth = DMA_SLAVE_BUSWIDTH_4_BYTES, > > + .dma_buswidth = DMA_SLAVE_BUSWIDTH_32_BYTES, > > Two very high level questions: > > 1) can't we set dma_priv->dma_buswidth at runtime when we know what the > card is capable of? Either DMA_SLAVE_BUSWIDTH_32_BYTES or > DMA_SLAVE_BUSWIDTH_4_BYTES? Then we don't need to fallback to PIO. > AFAIS, we only Gen2 sets .dma_buswidth in of_data, so we could even > remove it from of_data entirely? As I replied to Ulrich-san on other email thread, for now, rcar-dmac has a limitation on dmaengine_slave_config(), we should not call it at runtime. But, I don't think any sd card have such a limitation. In other words, if rcar-dmac doesn't have the limitation, I think we can change the buswidth at runtime and then we can remove the .dma_buswidth from of_data. > 2) Just by grepping in mmc/hosts, I see that no driver uses > DMA_SLAVE_BUSWIDTH_32_BYTES. Do you have an idea why? Because it is the > convenient setting which works for all cards? I also grepped in drivers/dma, and all dmaengine drivers except Renesas related SoCs don't support DMA_SLAVE_BUSWIDTH_32_BYTES. So, I think no driver uses the 32 bytes on mmc/hosts :) Best regards, Yoshihiro Shimoda > Thanks and kind regards, > > Wolfram
Hi Shimoda-san, > > 1) can't we set dma_priv->dma_buswidth at runtime when we know what the > > card is capable of? Either DMA_SLAVE_BUSWIDTH_32_BYTES or > > DMA_SLAVE_BUSWIDTH_4_BYTES? Then we don't need to fallback to PIO. > > AFAIS, we only Gen2 sets .dma_buswidth in of_data, so we could even > > remove it from of_data entirely? > > As I replied to Ulrich-san on other email thread, for now, rcar-dmac has a limitation > on dmaengine_slave_config(), we should not call it at runtime. But, I don't think > any sd card have such a limitation. In other words, if rcar-dmac doesn't have > the limitation, I think we can change the buswidth at runtime and then we can > remove the .dma_buswidth from of_data. So, that I understand correctly: The DMAC limitation is because of the driver and not because of the HW? If so, is it hard/planned to be fixed? > I also grepped in drivers/dma, and all dmaengine drivers except Renesas related > SoCs don't support DMA_SLAVE_BUSWIDTH_32_BYTES. So, I think no driver uses > the 32 bytes on mmc/hosts :) Wow, we are bleeding edge with this? :) Thanks, Wolfram
diff --git a/drivers/mmc/host/renesas_sdhi_sys_dmac.c b/drivers/mmc/host/renesas_sdhi_sys_dmac.c index 09137cc..65e71b6 100644 --- a/drivers/mmc/host/renesas_sdhi_sys_dmac.c +++ b/drivers/mmc/host/renesas_sdhi_sys_dmac.c @@ -58,7 +58,7 @@ static const struct renesas_sdhi_of_data of_rcar_gen2_compatible = { .capabilities = MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ | MMC_CAP_CMD23, .capabilities2 = MMC_CAP2_NO_WRITE_PROTECT, - .dma_buswidth = DMA_SLAVE_BUSWIDTH_4_BYTES, + .dma_buswidth = DMA_SLAVE_BUSWIDTH_32_BYTES, .dma_rx_offset = 0x2000, .scc_offset = 0x0300, .taps = rcar_gen2_scc_taps,
To improve performance, this patch sets dma_buswidth value to 32 when transfer size is multiples of 32. In other words, a sd card transfer's size if not multiples of 32, this driver uses PIO and then the performance will be down. Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> --- drivers/mmc/host/renesas_sdhi_sys_dmac.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)