[PATCH v4 0/3] mmc: renesas_sdhi: improve performance by changing max_segs

[PATCH v4 0/3] mmc: renesas_sdhi: improve performance by changing max_segs

[Yoshihiro Shimoda]

This patch series is based on iommu.git / next branch.

Since SDHI host internal DMAC of the R-Car Gen3 cannot handle two or more
segments, the performance rate (especially, eMMC HS400 reading) is not good.
However, if IOMMU is enabled on the DMAC, since IOMMU will map multiple
scatter gather buffers as one contignous iova, the DMAC can handle the iova
as well and then the performance rate is possible to improve. In fact,
I have measured the performance by using bonnie++, "Sequential Input - block"
rate was improved on r8a7795.

However, in case of a sdio card (especiialy some WiFi cards/drivers),
scatter gather buffers are possible to be not contiguous iova because
each scatter gather buffer has only about 1500 bytes, the DMAC cannot
handle it. So, this patch set adds init_card() ops to detect the card
type, and then the driver changes the max_segs if the DMAC is under
IOMMU environment and an sd card/mmc is detected.

Changes from v3 [1]:
 - Use a helper function device_iommu_mapped on patch 1 and 3.
 - Check if R-Car Gen3 IPMMU is used or not on patch 3.
 - Check if all multiple segment buffers are aligned to PAGE_SIZE on patch 3.
 - Add Reviewed-by Wolfram-san on patch 1 and 2. Note that I also got his
   Reviewed-by on patch 3, but I changed it from v2. So, I didn't add
   his Reviewed-by at this time.

Changes from v2 [2]:
 - Add some conditions in the init_card().
 - Add a comment in the init_card().
 - Add definitions for some "MAX_SEGS".

Changes from v1 [3]:
 - Remove adding init_card ops into struct tmio_mmc_dma_ops and
   tmio_mmc_host and just set init_card on renesas_sdhi_core.c.
 - Revise typos on "mmc: tmio: No memory size limitation if runs on IOMMU".
 - Add Simon-san's Reviewed-by on a tmio patch.

[1]
https://patchwork.kernel.org/project/linux-renesas-soc/list/?series=120985

[2]
https://patchwork.kernel.org/project/linux-renesas-soc/list/?series=116729

[3]
https://patchwork.kernel.org/project/linux-renesas-soc/list/?series=110485


Yoshihiro Shimoda (3):
  mmc: tmio: No memory size limitation if runs on IOMMU
  mmc: tmio: Add a definition for default max_segs
  mmc: renesas_sdhi: use multiple segments if possible

 drivers/mmc/host/renesas_sdhi.h               |  1 +
 drivers/mmc/host/renesas_sdhi_core.c          | 60 +++++++++++++++++++++++++++
 drivers/mmc/host/renesas_sdhi_internal_dmac.c | 19 +++++++++
 drivers/mmc/host/tmio_mmc.h                   |  1 +
 drivers/mmc/host/tmio_mmc_core.c              |  7 ++--
 5 files changed, 85 insertions(+), 3 deletions(-)

-- 
2.7.4

[PATCH v4 1/3] mmc: tmio: No memory size limitation if runs on IOMMU

[Yoshihiro Shimoda]

This patch adds a condition to avoid a memory size limitation of
swiotlb if the driver runs on IOMMU.

Tested-by: Takeshi Saito <takeshi.saito.xv@renesas.com>
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Reviewed-by: Simon Horman <horms+renesas@verge.net.au>
Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/mmc/host/tmio_mmc_core.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c
index 130b91c..c9f6a59 100644
--- a/drivers/mmc/host/tmio_mmc_core.c
+++ b/drivers/mmc/host/tmio_mmc_core.c
@@ -1194,9 +1194,10 @@ int tmio_mmc_host_probe(struct tmio_mmc_host *_host)
 	 * Since swiotlb has memory size limitation, this will calculate
 	 * the maximum size locally (because we don't have any APIs for it now)
 	 * and check the current max_req_size. And then, this will update
-	 * the max_req_size if needed as a workaround.
+	 * the max_req_size if needed as a workaround. However, if the driver
+	 * runs on IOMMU, this workaround isn't needed.
 	 */
-	if (swiotlb_max_segment()) {
+	if (swiotlb_max_segment() && !device_iommu_mapped(&pdev->dev)) {
 		unsigned int max_size = (1 << IO_TLB_SHIFT) * IO_TLB_SEGSIZE;
 
 		if (mmc->max_req_size > max_size)
-- 
2.7.4

[PATCH v4 2/3] mmc: tmio: Add a definition for default max_segs

[Yoshihiro Shimoda]

This patch adds a definition for default max_segs to be used by other
driver (renesas_sdhi) in the future.

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/mmc/host/tmio_mmc.h      | 1 +
 drivers/mmc/host/tmio_mmc_core.c | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index c5ba13f..9e387be 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -106,6 +106,7 @@
 #define TMIO_MASK_IRQ     (TMIO_MASK_READOP | TMIO_MASK_WRITEOP | TMIO_MASK_CMD)
 
 #define TMIO_MAX_BLK_SIZE 512
+#define TMIO_DEFAULT_MAX_SEGS 32
 
 struct tmio_mmc_data;
 struct tmio_mmc_host;
diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c
index c9f6a59..af1343e 100644
--- a/drivers/mmc/host/tmio_mmc_core.c
+++ b/drivers/mmc/host/tmio_mmc_core.c
@@ -1185,7 +1185,7 @@ int tmio_mmc_host_probe(struct tmio_mmc_host *_host)
 
 	mmc->caps |= MMC_CAP_4_BIT_DATA | pdata->capabilities;
 	mmc->caps2 |= pdata->capabilities2;
-	mmc->max_segs = pdata->max_segs ? : 32;
+	mmc->max_segs = pdata->max_segs ? : TMIO_DEFAULT_MAX_SEGS;
 	mmc->max_blk_size = TMIO_MAX_BLK_SIZE;
 	mmc->max_blk_count = pdata->max_blk_count ? :
 		(PAGE_SIZE / mmc->max_blk_size) * mmc->max_segs;
-- 
2.7.4

[PATCH v4 3/3] mmc: renesas_sdhi: use multiple segments if possible

[Yoshihiro Shimoda]

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 to improve the transfer
performance on renesas_sdhi_internal_dmac.

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. Also, on renesas_sdhi_sys_dmac,
the max_segs value will change from 32 to 512, but the sys_dmac
can handle 512 segments, so that this init_card ops is added on
"TMIO_MMC_MIN_RCAR2" environment.

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 drivers/mmc/host/renesas_sdhi.h               |  1 +
 drivers/mmc/host/renesas_sdhi_core.c          | 60 +++++++++++++++++++++++++++
 drivers/mmc/host/renesas_sdhi_internal_dmac.c | 19 +++++++++
 3 files changed, 80 insertions(+)

diff --git a/drivers/mmc/host/renesas_sdhi.h b/drivers/mmc/host/renesas_sdhi.h
index c0504aa..d53dda5 100644
--- a/drivers/mmc/host/renesas_sdhi.h
+++ b/drivers/mmc/host/renesas_sdhi.h
@@ -51,6 +51,7 @@ struct renesas_sdhi {
 	void __iomem *scc_ctl;
 	u32 scc_tappos;
 	u32 scc_tappos_hs400;
+	bool ipmmu_mapped;
 };
 
 #define host_to_priv(host) \
diff --git a/drivers/mmc/host/renesas_sdhi_core.c b/drivers/mmc/host/renesas_sdhi_core.c
index 5e9e36e..e9d9541 100644
--- a/drivers/mmc/host/renesas_sdhi_core.c
+++ b/drivers/mmc/host/renesas_sdhi_core.c
@@ -46,6 +46,8 @@
 #define SDHI_VER_GEN3_SD	0xcc10
 #define SDHI_VER_GEN3_SDMMC	0xcd10
 
+#define SDHI_MAX_SEGS_IN_IOMMU	512
+
 struct renesas_sdhi_quirks {
 	bool hs400_disabled;
 	bool hs400_4taps;
@@ -203,6 +205,32 @@ static void renesas_sdhi_clk_disable(struct tmio_mmc_host *host)
 	clk_disable_unprepare(priv->clk_cd);
 }
 
+static void renesas_sdhi_init_card(struct mmc_host *mmc, struct mmc_card *card)
+{
+	struct tmio_mmc_host *host = mmc_priv(mmc);
+	struct renesas_sdhi *priv = host_to_priv(host);
+
+	/*
+	 * In IPMMU environment that some R-Car SoCs have, it's possible to
+	 * merge scatter gather buffers of memory requests to one iova so that
+	 * this code changes the max_segs 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 even if IPMMU environment.
+	 *
+	 * This can expose the host->mmc->max_segs to a block layer by using
+	 * blk_queue_max_segments() that mmc_setup_queue() calls. In other
+	 * words, this init_card() ops is called before a block device is
+	 * created.
+	 */
+	if (host->pdata->max_segs < SDHI_MAX_SEGS_IN_IOMMU &&
+	    priv->ipmmu_mapped && (mmc_card_mmc(card) || mmc_card_sd(card)))
+		host->mmc->max_segs = SDHI_MAX_SEGS_IN_IOMMU;
+	else
+		host->mmc->max_segs = host->pdata->max_segs ? :
+				      TMIO_DEFAULT_MAX_SEGS;
+}
+
 static int renesas_sdhi_card_busy(struct mmc_host *mmc)
 {
 	struct tmio_mmc_host *host = mmc_priv(mmc);
@@ -610,6 +638,35 @@ static void renesas_sdhi_enable_dma(struct tmio_mmc_host *host, bool enable)
 	renesas_sdhi_sdbuf_width(host, enable ? width : 16);
 }
 
+static bool renesas_sdhi_ipmmu_mapped(struct device *dev)
+{
+	struct device_node *iommu_np;
+	static const char * const compatibles[] = {
+		"renesas,ipmmu-r8a7795",
+		"renesas,ipmmu-r8a7796",
+		"renesas,ipmmu-r8a77965",
+		"renesas,ipmmu-r8a77970",
+		"renesas,ipmmu-r8a77980",
+		"renesas,ipmmu-r8a77990",
+		"renesas,ipmmu-r8a77995",
+	};
+	int i;
+
+	if (!device_iommu_mapped(dev))
+		return false;
+
+	iommu_np = of_parse_phandle(dev->of_node, "iommus", 0);
+	if (!iommu_np)
+		return false;
+
+	for (i = 0; i < ARRAY_SIZE(compatibles); i++) {
+		if (of_device_is_compatible(iommu_np, compatibles[i]))
+			return true;
+	}
+
+	return false;
+}
+
 static const struct renesas_sdhi_quirks sdhi_quirks_h3_m3w_es1 = {
 	.hs400_disabled = true,
 	.hs400_4taps = true,
@@ -726,6 +783,9 @@ 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;
+		priv->ipmmu_mapped = renesas_sdhi_ipmmu_mapped(&pdev->dev);
+
 		/* 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 =
diff --git a/drivers/mmc/host/renesas_sdhi_internal_dmac.c b/drivers/mmc/host/renesas_sdhi_internal_dmac.c
index 751fe91..2b83b43 100644
--- a/drivers/mmc/host/renesas_sdhi_internal_dmac.c
+++ b/drivers/mmc/host/renesas_sdhi_internal_dmac.c
@@ -177,11 +177,30 @@ renesas_sdhi_internal_dmac_start_dma(struct tmio_mmc_host *host,
 				     struct mmc_data *data)
 {
 	struct scatterlist *sg = host->sg_ptr;
+	struct renesas_sdhi *priv = host_to_priv(host);
 	u32 dtran_mode = DTRAN_MODE_BUS_WIDTH;
 
 	if (!test_bit(SDHI_INTERNAL_DMAC_ADDR_MODE_FIXED_ONLY, &global_flags))
 		dtran_mode |= DTRAN_MODE_ADDR_MODE;
 
+	/*
+	 * If this driver uses multiple segments on IPMMU, all segment buffers
+	 * boundary except the end of buffer should be aligned to IPMMU page
+	 * size. Note that the IPMMU page size will be the same as (or less
+	 * than) CPU page size.
+	 */
+	if (priv->ipmmu_mapped && host->sg_len > 1) {
+		int i;
+		struct scatterlist *s;
+
+		for_each_sg(sg, s, host->sg_len, i) {
+			if (!PAGE_ALIGNED(sg_virt(s)) ||
+			    ((i < host->sg_len - 1) &&
+			     !PAGE_ALIGNED(s->length)))
+				goto force_pio;
+		}
+	}
+
 	if (!dma_map_sg(&host->pdev->dev, sg, host->sg_len,
 			mmc_get_dma_dir(data)))
 		goto force_pio;
-- 
2.7.4

Re: [PATCH v4 0/3] mmc: renesas_sdhi: improve performance by changing max_segs

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 3123 bytes --]

On Fri, May 31, 2019 at 08:16:08PM +0900, Yoshihiro Shimoda wrote:
> This patch series is based on iommu.git / next branch.
> 
> Since SDHI host internal DMAC of the R-Car Gen3 cannot handle two or more
> segments, the performance rate (especially, eMMC HS400 reading) is not good.
> However, if IOMMU is enabled on the DMAC, since IOMMU will map multiple
> scatter gather buffers as one contignous iova, the DMAC can handle the iova
> as well and then the performance rate is possible to improve. In fact,
> I have measured the performance by using bonnie++, "Sequential Input - block"
> rate was improved on r8a7795.
> 
> However, in case of a sdio card (especiialy some WiFi cards/drivers),
> scatter gather buffers are possible to be not contiguous iova because
> each scatter gather buffer has only about 1500 bytes, the DMAC cannot
> handle it. So, this patch set adds init_card() ops to detect the card
> type, and then the driver changes the max_segs if the DMAC is under
> IOMMU environment and an sd card/mmc is detected.
> 
> Changes from v3 [1]:
>  - Use a helper function device_iommu_mapped on patch 1 and 3.
>  - Check if R-Car Gen3 IPMMU is used or not on patch 3.
>  - Check if all multiple segment buffers are aligned to PAGE_SIZE on patch 3.
>  - Add Reviewed-by Wolfram-san on patch 1 and 2. Note that I also got his
>    Reviewed-by on patch 3, but I changed it from v2. So, I didn't add
>    his Reviewed-by at this time.

Yes, dropping my rev on patch 3 is a good thing to do. I added Christoph
to the CC list because he gave valuable input last time.

@hch: If you didn't get the original mails from some list, I can bounce
them to you or you can find them here:

https://patchwork.kernel.org/project/linux-renesas-soc/list/?series=125593

> 
> Changes from v2 [2]:
>  - Add some conditions in the init_card().
>  - Add a comment in the init_card().
>  - Add definitions for some "MAX_SEGS".
> 
> Changes from v1 [3]:
>  - Remove adding init_card ops into struct tmio_mmc_dma_ops and
>    tmio_mmc_host and just set init_card on renesas_sdhi_core.c.
>  - Revise typos on "mmc: tmio: No memory size limitation if runs on IOMMU".
>  - Add Simon-san's Reviewed-by on a tmio patch.
> 
> [1]
> https://patchwork.kernel.org/project/linux-renesas-soc/list/?series=120985
> 
> [2]
> https://patchwork.kernel.org/project/linux-renesas-soc/list/?series=116729
> 
> [3]
> https://patchwork.kernel.org/project/linux-renesas-soc/list/?series=110485
> 
> 
> Yoshihiro Shimoda (3):
>   mmc: tmio: No memory size limitation if runs on IOMMU
>   mmc: tmio: Add a definition for default max_segs
>   mmc: renesas_sdhi: use multiple segments if possible
> 
>  drivers/mmc/host/renesas_sdhi.h               |  1 +
>  drivers/mmc/host/renesas_sdhi_core.c          | 60 +++++++++++++++++++++++++++
>  drivers/mmc/host/renesas_sdhi_internal_dmac.c | 19 +++++++++
>  drivers/mmc/host/tmio_mmc.h                   |  1 +
>  drivers/mmc/host/tmio_mmc_core.c              |  7 ++--
>  5 files changed, 85 insertions(+), 3 deletions(-)
> 
> -- 
> 2.7.4
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v4 0/3] mmc: renesas_sdhi: improve performance by changing max_segs

[Christoph Hellwig]

On Mon, Jun 03, 2019 at 02:57:01PM +0200, Wolfram Sang wrote:
> Yes, dropping my rev on patch 3 is a good thing to do. I added Christoph
> to the CC list because he gave valuable input last time.

Assuming iommu merging in a driver using the DMA API is always bogus
as mentioned last time.  As this cover letter don't seem to include
any higher level DMA subsystem or block changes I'll stick to my NAK.

RE: [PATCH v4 0/3] mmc: renesas_sdhi: improve performance by changing max_segs

[Yoshihiro Shimoda]

Hi Christoph, Wolfram,

> From: Christoph Hellwig, Sent: Monday, June 3, 2019 10:12 PM
> 
> On Mon, Jun 03, 2019 at 02:57:01PM +0200, Wolfram Sang wrote:
> > Yes, dropping my rev on patch 3 is a good thing to do. I added Christoph
> > to the CC list because he gave valuable input last time.
> 
> Assuming iommu merging in a driver using the DMA API is always bogus
> as mentioned last time.  As this cover letter don't seem to include
> any higher level DMA subsystem or block changes I'll stick to my NAK.

Thank you very much for your reply again. I understood this patch series is NAK.
I'll continue to investigate DMA or block subsystem to achieve this. (I'm not sure
I can succeed or not though...)

Best regards,
Yoshihiro Shimoda