[PATCH 0/3] mmc: renesas_sdhi_internal_dmac: improve performance by using IOMMU

[PATCH 0/3] mmc: renesas_sdhi_internal_dmac: improve performance by using IOMMU

[Yoshihiro Shimoda]

This patch set is based on renesas-drivers.git /
renesas-drivers-2019-04-23-v5.1-rc6 tag.

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 all platforms (r8a7795, r8a77965 and r8a77990).
Please refer to the end of this email about the performance.
(I beleive if the performance is improved, the CPU load is also increased.)

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.

---
kernel v5.1-rc6 + local patches + eMMC ext4 format,,,,,,,,,,,,,,,,,,,,,,,,,,
Buildroot 2019.02.1,,,,,,,,,,,,,,,,,,,,,,,,,,
Bonnie++ 1.03e : bonnie\+\+ -d ./ -s 8192 -r 4096 -b -u root,,,,,,,,,,,,,,,,,,,,,,,,,,
,,,,,,,,,,,,,,,,,,,,,,,,,,
environment,Size,Sequential Output - per char (K/sec),<- (CPU %),Sequential Output - block (K/sec),<- (CPU %),Sequential Output - rewrite (K/sec),<- (CPU %),Sequential Input - per char (K/sec),<- (CPU %),Sequential Input - block (K/sec),<- (CPU %),Random seeks,<- (CPU %),files,Sequential Create,<- (CPU %),Sequential Read,<- (CPU %),Sequential Delete,<- (CPU %),Random Create,<- (CPU %),Random Read,<- (CPU %),Random Delete,<- (CPU %)
H3_No_IPMMU,8G,80621,97,117133,28,58619,15,70679,94,118682,16,4068.2,12,16,673,3,+++++,+++,597,3,642,3,+++++,+++,588,3
H3_IPMMU,8G,68183,97,130482,31,80730,20,74719,98,195727,25,4326.4,12,16,859,4,+++++,+++,809,4,796,4,+++++,+++,781,4
M3-N_No_IPMMU,8G,59031,96,121806,32,59500,17,54025,95,118384,17,3245.2,14,16,688,4,+++++,+++,641,3,679,4,+++++,+++,641,4
M3-N_IPMMU,8G,57414,93,136734,35,79095,22,56235,98,196351,27,3438.8,15,16,846,5,+++++,+++,809,4,830,5,+++++,+++,815,5
E3_No_IPMMU,8G,32136,96,99390,42,40603,27,28733,94,76958,26,2638.6,32,16,485,15,+++++,+++,485,11,490,15,+++++,+++,491,11
E3_IPMMU,8G,31712,95,119053,48,61360,36,30075,97,138801,44,2714.4,35,16,552,17,+++++,+++,588,13,573,17,+++++,+++,590,13
---

Yoshihiro Shimoda (3):
  mmc: tmio: add init_card ops
  mmc: tmio: No memory size limitation if runs on IOMMU
  mmc: renesas_sdhi_internal_dmac: use multiple segments if possible

 drivers/mmc/host/renesas_sdhi_core.c          |  8 ++++++++
 drivers/mmc/host/renesas_sdhi_internal_dmac.c | 12 ++++++++++++
 drivers/mmc/host/tmio_mmc.h                   |  2 ++
 drivers/mmc/host/tmio_mmc_core.c              | 14 ++++++++++++--
 4 files changed, 34 insertions(+), 2 deletions(-)

-- 
2.7.4

[PATCH 1/3] mmc: tmio: add init_card ops

[Yoshihiro Shimoda]

Since renesas_sdhi_internal_dmac driver would like to use
the init_card() ops in the future, this patch adds init_card ops
into the struct tmio_mmc_host and struct tmio_mmc_dma_ops.

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 drivers/mmc/host/tmio_mmc.h      | 2 ++
 drivers/mmc/host/tmio_mmc_core.c | 9 +++++++++
 2 files changed, 11 insertions(+)

diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index c5ba13f..cb2c42b 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -118,6 +118,7 @@ struct tmio_mmc_dma_ops {
 	void (*release)(struct tmio_mmc_host *host);
 	void (*abort)(struct tmio_mmc_host *host);
 	void (*dataend)(struct tmio_mmc_host *host);
+	void (*init_card)(struct tmio_mmc_host *host, struct mmc_card *card);
 };
 
 struct tmio_mmc_host {
@@ -178,6 +179,7 @@ struct tmio_mmc_host {
 	void (*hw_reset)(struct tmio_mmc_host *host);
 	void (*prepare_tuning)(struct tmio_mmc_host *host, unsigned long tap);
 	bool (*check_scc_error)(struct tmio_mmc_host *host);
+	void (*init_card)(struct tmio_mmc_host *host, struct mmc_card *card);
 
 	/*
 	 * Mandatory callback for tuning to occur which is optional for SDR50
diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c
index 130b91c..6019628 100644
--- a/drivers/mmc/host/tmio_mmc_core.c
+++ b/drivers/mmc/host/tmio_mmc_core.c
@@ -1047,6 +1047,14 @@ static void tmio_mmc_hs400_complete(struct mmc_host *mmc)
 		host->hs400_complete(host);
 }
 
+static void tmio_mmc_init_card(struct mmc_host *mmc, struct mmc_card *card)
+{
+	struct tmio_mmc_host *host = mmc_priv(mmc);
+
+	if (host->init_card)
+		host->init_card(host, card);
+}
+
 static const struct mmc_host_ops tmio_mmc_ops = {
 	.request	= tmio_mmc_request,
 	.set_ios	= tmio_mmc_set_ios,
@@ -1059,6 +1067,7 @@ static const struct mmc_host_ops tmio_mmc_ops = {
 	.prepare_hs400_tuning = tmio_mmc_prepare_hs400_tuning,
 	.hs400_downgrade = tmio_mmc_hs400_downgrade,
 	.hs400_complete	= tmio_mmc_hs400_complete,
+	.init_card	= tmio_mmc_init_card,
 };
 
 static int tmio_mmc_init_ocr(struct tmio_mmc_host *host)
-- 
2.7.4

Re: [PATCH 1/3] mmc: tmio: add init_card ops

[Wolfram Sang]

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

On Fri, Apr 26, 2019 at 02:18:48PM +0900, Yoshihiro Shimoda wrote:
> Since renesas_sdhi_internal_dmac driver would like to use
> the init_card() ops in the future, this patch adds init_card ops
> into the struct tmio_mmc_host and struct tmio_mmc_dma_ops.
> 
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>

Yamada-san refactored the code, so we don't need such hooks in the TMIO
part anymore. Check 2aaa3c5193db ("mmc: tmio, renesas_sdhi: set
mmc_host_ops hooks directly") for an example. I will comment further in
patch 3.


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

Re: [PATCH 1/3] mmc: tmio: add init_card ops

[Simon Horman]

On Fri, Apr 26, 2019 at 02:18:48PM +0900, Yoshihiro Shimoda wrote:
> Since renesas_sdhi_internal_dmac driver would like to use
> the init_card() ops in the future, this patch adds init_card ops
> into the struct tmio_mmc_host and struct tmio_mmc_dma_ops.
> 
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>

Reviewed-by: Simon Horman <horms+renesas@verge.net.au>

Re: [PATCH 1/3] mmc: tmio: add init_card ops

[Wolfram Sang]

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

On Fri, Apr 26, 2019 at 11:45:01AM +0200, Simon Horman wrote:
> On Fri, Apr 26, 2019 at 02:18:48PM +0900, Yoshihiro Shimoda wrote:
> > Since renesas_sdhi_internal_dmac driver would like to use
> > the init_card() ops in the future, this patch adds init_card ops
> > into the struct tmio_mmc_host and struct tmio_mmc_dma_ops.
> > 
> > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> 
> Reviewed-by: Simon Horman <horms+renesas@verge.net.au>

Don't you think this not needed anymore in context with all the
refactoring around 2aaa3c5193db ("mmc: tmio, renesas_sdhi: set
mmc_host_ops hooks directly")?


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

Re: [PATCH 1/3] mmc: tmio: add init_card ops

[Simon Horman]

On Fri, Apr 26, 2019 at 11:56:00AM +0200, Wolfram Sang wrote:
> On Fri, Apr 26, 2019 at 11:45:01AM +0200, Simon Horman wrote:
> > On Fri, Apr 26, 2019 at 02:18:48PM +0900, Yoshihiro Shimoda wrote:
> > > Since renesas_sdhi_internal_dmac driver would like to use
> > > the init_card() ops in the future, this patch adds init_card ops
> > > into the struct tmio_mmc_host and struct tmio_mmc_dma_ops.
> > > 
> > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > 
> > Reviewed-by: Simon Horman <horms+renesas@verge.net.au>
> 
> Don't you think this not needed anymore in context with all the
> refactoring around 2aaa3c5193db ("mmc: tmio, renesas_sdhi: set
> mmc_host_ops hooks directly")?

Yes, true.

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

[Yoshihiro Shimoda]

This patch adds a condition to avoid memory size limitaion 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>
---
 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 6019628..0474337 100644
--- a/drivers/mmc/host/tmio_mmc_core.c
+++ b/drivers/mmc/host/tmio_mmc_core.c
@@ -1203,9 +1203,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 doesn't need.
 	 */
-	if (swiotlb_max_segment()) {
+	if (swiotlb_max_segment() && !pdev->dev.iommu_group) {
 		unsigned int max_size = (1 << IO_TLB_SHIFT) * IO_TLB_SEGSIZE;
 
 		if (mmc->max_req_size > max_size)
-- 
2.7.4

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

[Sergei Shtylyov]

On 26.04.2019 8:18, Yoshihiro Shimoda wrote:

> This patch adds a condition to avoid memory size limitaion of

    Limitation.

> 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>
> ---
>   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 6019628..0474337 100644
> --- a/drivers/mmc/host/tmio_mmc_core.c
> +++ b/drivers/mmc/host/tmio_mmc_core.c
> @@ -1203,9 +1203,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 doesn't need.

    Isn't needed, maybe?

[...]

MBR, Sergei

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

[Wolfram Sang]

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

> > This patch adds a condition to avoid memory size limitaion of
> 
>    Limitation.

"limitation" :) And maybe "a memory size limitation"?

> > +	 * the max_req_size if needed as a workaround. However, if the driver
> > +	 * runs on IOMMU, this workaround doesn't need.
> 
>    Isn't needed, maybe?

Ack, "isn't needed".

Patch itself looks good to me.


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

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

[Yoshihiro Shimoda]

Hi Sergei-san, Wolfram-san,

Thank you for your review and sorry for the delayed response...

> From: Wolfram Sang, Sent: Friday, April 26, 2019 6:18 PM
> 
> > > This patch adds a condition to avoid memory size limitaion of
> >
> >    Limitation.
> 
> "limitation" :) And maybe "a memory size limitation"?

I think so. I'll fix it on v2 patch.

> > > +	 * the max_req_size if needed as a workaround. However, if the driver
> > > +	 * runs on IOMMU, this workaround doesn't need.
> >
> >    Isn't needed, maybe?
> 
> Ack, "isn't needed".

I'll fix it.

> Patch itself looks good to me.

Thank you for your comment!

Best regards,
Yoshihiro Shimoda

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

[Simon Horman]

On Fri, Apr 26, 2019 at 02:18:49PM +0900, Yoshihiro Shimoda wrote:
> This patch adds a condition to avoid memory size limitaion 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>

[PATCH 3/3] mmc: renesas_sdhi_internal_dmac: 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. 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(+)

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,
 };
 
 /*
-- 
2.7.4

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

[Wolfram Sang]

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


> +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.


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

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

[Yoshihiro Shimoda]

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

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

[Simon Horman]

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>

Re: [PATCH 0/3] mmc: renesas_sdhi_internal_dmac: improve performance by using IOMMU

[Wolfram Sang]

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

Hi Shimoda-san,

thanks for working on this!

> Please refer to the end of this email about the performance.

Yes, nice improvements, great!

> (I beleive if the performance is improved, the CPU load is also increased.)

I do wonder about this a bit, though. IPMMU and DMA shouldn't be that
much expensive for the CPU, or? Am I overlooking something?

Kind regards,

   Wolfram


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

RE: [PATCH 0/3] mmc: renesas_sdhi_internal_dmac: improve performance by using IOMMU

[Yoshihiro Shimoda]

Hi Wolfram-san,

> From: Wolfram Sang, Sent: Friday, April 26, 2019 6:46 PM
> 
> Hi Shimoda-san,
> 
> thanks for working on this!
> 
> > Please refer to the end of this email about the performance.
> 
> Yes, nice improvements, great!

Thanks!

> > (I beleive if the performance is improved, the CPU load is also increased.)
> 
> I do wonder about this a bit, though. IPMMU and DMA shouldn't be that
> much expensive for the CPU, or? Am I overlooking something?

I'm guessing that a user land app (in this case bonnie++) consumes CPU load for some reason.
I'll experiment whether my guess is correct or not by using usb 3.0 host like below tomorrow:
 - case 1: usb 3.0 host + usb SSD as SuperSpeed (IOMMU is disabled).
 - case 2: usb 3.0 host + usb SSD via a usb2.0 hub as high-speed (IOMMU is disabled).

Best regards,
Yoshihiro Shimoda

> Kind regards,
> 
>    Wolfram

RE: [PATCH 0/3] mmc: renesas_sdhi_internal_dmac: improve performance by using IOMMU

[Yoshihiro Shimoda]

Hi Wolfram-san again,

> From: Yoshihiro Shimoda, Sent: Tuesday, May 7, 2019 5:59 PM
> 
> Hi Wolfram-san,
> 
> > From: Wolfram Sang, Sent: Friday, April 26, 2019 6:46 PM
> >
> > Hi Shimoda-san,
> >
> > thanks for working on this!
> >
> > > Please refer to the end of this email about the performance.
> >
> > Yes, nice improvements, great!
> 
> Thanks!
> 
> > > (I beleive if the performance is improved, the CPU load is also increased.)
> >
> > I do wonder about this a bit, though. IPMMU and DMA shouldn't be that
> > much expensive for the CPU, or? Am I overlooking something?
> 
> I'm guessing that a user land app (in this case bonnie++) consumes CPU load for some reason.
> I'll experiment whether my guess is correct or not by using usb 3.0 host like below tomorrow:
>  - case 1: usb 3.0 host + usb SSD as SuperSpeed (IOMMU is disabled).
>  - case 2: usb 3.0 host + usb SSD via a usb2.0 hub as high-speed (IOMMU is disabled).

I have measured them + IOMMU enabled environment. It seems my guess is correct.

  - case 1: usb 3.0 host + usb SSD as SuperSpeed (IOMMU is disabled).
  - case 2: usb 3.0 host + usb SSD via a usb2.0 hub as high-speed (IOMMU is disabled).
  - case 3: usb 3.0 host + usb SSD as SuperSpeed (IOMMU is enabled).
  - case 4: usb 3.0 host + usb SSD via a usb2.0 hub as high-speed (IOMMU is enabled).

---
kernel v5.1-rc7 + local patches + USB SSD ext4 format,,,,,,,,,,,,,,,,,,,,,,,,,,
Buildroot 2019.02.1,,,,,,,,,,,,,,,,,,,,,,,,,,
Bonnie++ 1.03e : bonnie\+\+ -d ./ -s 8192 -r 4096 -b -u root,,,,,,,,,,,,,,,,,,,,,,,,,,
,,,,,,,,,,,,,,,,,,,,,,,,,,
environment,Size,Sequential Output - per char (K/sec),<- (CPU %),Sequential Output - block (K/sec),<- (CPU %),Sequential Output - rewrite (K/sec),<- (CPU %),Sequential Input - per char (K/sec),<- (CPU %),Sequential Input ? block (K/sec),<- (CPU %),Random seeks,<- (CPU %),files,Sequential Create,<- (CPU %),Sequential Read,<- (CPU %),Sequential Delete,<- (CPU %),Random Create,<- (CPU %),Random Read,<- (CPU %),Random Delete,<- (CPU %)
H3_SuperSpeed_No_IOMMU,8G,82598,99,242161,58,102489,25,73719,98,254089,32,2133.3,6,16,382,2,+++++,+++,385,1,380,1,+++++,+++,387,2
H3_HighSpeed_No_IOMMU,8G,41971,53,39983,9,16459,4,37900,51,37833,5,1585.7,5,16,304,1,+++++,+++,295,1,302,1,+++++,+++,294,1
H3_SuperSpeed_IOMMU,8G,66139,99,276686,65,132297,33,69732,99,293396,37,2099.2,7,16,389,2,+++++,+++,391,1,382,1,+++++,+++,391,2
H3_HighSpeed_IOMMU,8G,43191,50,40446,9,17432,4,38541,51,38481,5,1619.4,4,16,302,1,+++++,+++,296,1,303,1,+++++,+++,294,1
---

Best regards,
Yoshihiro Shimoda