diff mbox series

[3/3] mmc: renesas_sdhi_internal_dmac: use multiple segments if possible

Message ID 1556255930-18188-4-git-send-email-yoshihiro.shimoda.uh@renesas.com (mailing list archive)
State New, archived
Headers show
Series mmc: renesas_sdhi_internal_dmac: improve performance by using IOMMU | expand

Commit Message

Yoshihiro Shimoda April 26, 2019, 5:18 a.m. UTC
In IOMMU environment, since it's possible to merge scatter gather
buffers of memory requests to one iova, this patch changes
the max_segs value when init_card of mmc_host timing. Notes that
an sdio card may be possible to use scatter gather buffers with
non page aligned size, so that this driver will not use multiple
segments to avoid any trouble.

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 drivers/mmc/host/renesas_sdhi_core.c          |  8 ++++++++
 drivers/mmc/host/renesas_sdhi_internal_dmac.c | 12 ++++++++++++
 2 files changed, 20 insertions(+)

Comments

Wolfram Sang April 26, 2019, 9:37 a.m. UTC | #1
> +static void
> +renesas_sdhi_internal_dmac_init_card(struct tmio_mmc_host *host,
> +				     struct mmc_card *card)
> +{
> +	if (host->pdev->dev.iommu_group &&
> +	    (mmc_card_mmc(card) || mmc_card_sd(card)))
> +		host->mmc->max_segs = 512;
> +	else
> +		host->mmc->max_segs = host->pdata->max_segs;
> +}
> +

Will this work with Gen2 as well if we explicitly set max_segs in
of_rcar_gen2_compatible (renesas_sdhi_sys_dmac.c)? Then we could simply
move the above chunk to renesas_sdhi_core.c and use this minimal diff.

--- a/drivers/mmc/host/renesas_sdhi_core.c
+++ b/drivers/mmc/host/renesas_sdhi_core.c
@@ -726,6 +726,8 @@ int renesas_sdhi_probe(struct platform_device *pdev,
 
 	/* SDR speeds are only available on Gen2+ */
 	if (mmc_data->flags & TMIO_MMC_MIN_RCAR2) {
+		host->ops.init_card = renesas_sdhi_init_card;
+
 		/* card_busy caused issues on r8a73a4 (pre-Gen2) CD-less SDHI */
 		host->ops.card_busy = renesas_sdhi_card_busy;
 		host->ops.start_signal_voltage_switch =

What do you think, Shimoda-san? Otherwise, we probably need to keep the
init_card in the dma_ops struct and could do something like

+		host->ops.init_card = dma_ops->init_card;

?

Need to think about the latter a bit more, though.
Simon Horman April 26, 2019, 9:45 a.m. UTC | #2
On Fri, Apr 26, 2019 at 02:18:50PM +0900, Yoshihiro Shimoda wrote:
> In IOMMU environment, since it's possible to merge scatter gather
> buffers of memory requests to one iova, this patch changes
> the max_segs value when init_card of mmc_host timing. Notes that
> an sdio card may be possible to use scatter gather buffers with
> non page aligned size, so that this driver will not use multiple
> segments to avoid any trouble.
> 
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>

Reviewed-by: Simon Horman <horms+renesas@verge.net.au>
Yoshihiro Shimoda May 7, 2019, 7:27 a.m. UTC | #3
Hi Wolfram-san,

> From: Wolfram Sang, Sent: Friday, April 26, 2019 6:37 PM
> 
> 
> > +static void
> > +renesas_sdhi_internal_dmac_init_card(struct tmio_mmc_host *host,
> > +				     struct mmc_card *card)
> > +{
> > +	if (host->pdev->dev.iommu_group &&
> > +	    (mmc_card_mmc(card) || mmc_card_sd(card)))
> > +		host->mmc->max_segs = 512;
> > +	else
> > +		host->mmc->max_segs = host->pdata->max_segs;
> > +}
> > +
> 
> Will this work with Gen2 as well if we explicitly set max_segs in
> of_rcar_gen2_compatible (renesas_sdhi_sys_dmac.c)?

Yes, Gen2 (renesas_sdhi_sys_dmac.c) can work with max_segs = 512.
# Enabling IPMMU on Gen2 might cause some troubles anyway regardless the max_segs value though.

> Then we could simply
> move the above chunk to renesas_sdhi_core.c and use this minimal diff.
> 
> --- a/drivers/mmc/host/renesas_sdhi_core.c
> +++ b/drivers/mmc/host/renesas_sdhi_core.c
> @@ -726,6 +726,8 @@ int renesas_sdhi_probe(struct platform_device *pdev,
> 
>  	/* SDR speeds are only available on Gen2+ */
>  	if (mmc_data->flags & TMIO_MMC_MIN_RCAR2) {
> +		host->ops.init_card = renesas_sdhi_init_card;
> +
>  		/* card_busy caused issues on r8a73a4 (pre-Gen2) CD-less SDHI */
>  		host->ops.card_busy = renesas_sdhi_card_busy;
>  		host->ops.start_signal_voltage_switch =
> 
> What do you think, Shimoda-san?

I think it's a nice idea. So, I'll modify this patch on v2.

Best regards,
Yoshihiro Shimoda
diff mbox series

Patch

diff --git a/drivers/mmc/host/renesas_sdhi_core.c b/drivers/mmc/host/renesas_sdhi_core.c
index 5e9e36e..e27cc24 100644
--- a/drivers/mmc/host/renesas_sdhi_core.c
+++ b/drivers/mmc/host/renesas_sdhi_core.c
@@ -601,6 +601,13 @@  static int renesas_sdhi_multi_io_quirk(struct mmc_card *card,
 	return blk_size;
 }
 
+static void renesas_sdhi_init_card(struct tmio_mmc_host *host,
+				   struct mmc_card *card)
+{
+	if (host->dma_ops->init_card)
+		host->dma_ops->init_card(host, card);
+}
+
 static void renesas_sdhi_enable_dma(struct tmio_mmc_host *host, bool enable)
 {
 	/* Iff regs are 8 byte apart, sdbuf is 64 bit. Otherwise always 32. */
@@ -712,6 +719,7 @@  int renesas_sdhi_probe(struct platform_device *pdev,
 	host->clk_disable	= renesas_sdhi_clk_disable;
 	host->set_clock		= renesas_sdhi_set_clock;
 	host->multi_io_quirk	= renesas_sdhi_multi_io_quirk;
+	host->init_card		= renesas_sdhi_init_card;
 	host->dma_ops		= dma_ops;
 
 	if (quirks && quirks->hs400_disabled)
diff --git a/drivers/mmc/host/renesas_sdhi_internal_dmac.c b/drivers/mmc/host/renesas_sdhi_internal_dmac.c
index 751fe91..0cf14c9 100644
--- a/drivers/mmc/host/renesas_sdhi_internal_dmac.c
+++ b/drivers/mmc/host/renesas_sdhi_internal_dmac.c
@@ -285,6 +285,17 @@  renesas_sdhi_internal_dmac_release_dma(struct tmio_mmc_host *host)
 	host->chan_rx = host->chan_tx = NULL;
 }
 
+static void
+renesas_sdhi_internal_dmac_init_card(struct tmio_mmc_host *host,
+				     struct mmc_card *card)
+{
+	if (host->pdev->dev.iommu_group &&
+	    (mmc_card_mmc(card) || mmc_card_sd(card)))
+		host->mmc->max_segs = 512;
+	else
+		host->mmc->max_segs = host->pdata->max_segs;
+}
+
 static const struct tmio_mmc_dma_ops renesas_sdhi_internal_dmac_dma_ops = {
 	.start = renesas_sdhi_internal_dmac_start_dma,
 	.enable = renesas_sdhi_internal_dmac_enable_dma,
@@ -292,6 +303,7 @@  static const struct tmio_mmc_dma_ops renesas_sdhi_internal_dmac_dma_ops = {
 	.release = renesas_sdhi_internal_dmac_release_dma,
 	.abort = renesas_sdhi_internal_dmac_abort_dma,
 	.dataend = renesas_sdhi_internal_dmac_dataend_dma,
+	.init_card = renesas_sdhi_internal_dmac_init_card,
 };
 
 /*