linux-renesas-soc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] mmc: renesas_sdhi_internal_dmac: improve performance by using IOMMU
@ 2019-04-26  5:18 Yoshihiro Shimoda
  2019-04-26  5:18 ` [PATCH 1/3] mmc: tmio: add init_card ops Yoshihiro Shimoda
                   ` (3 more replies)
  0 siblings, 4 replies; 18+ messages in thread
From: Yoshihiro Shimoda @ 2019-04-26  5:18 UTC (permalink / raw)
  To: ulf.hansson, wsa+renesas; +Cc: linux-mmc, linux-renesas-soc, 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


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 1/3] mmc: tmio: add init_card ops
  2019-04-26  5:18 [PATCH 0/3] mmc: renesas_sdhi_internal_dmac: improve performance by using IOMMU Yoshihiro Shimoda
@ 2019-04-26  5:18 ` Yoshihiro Shimoda
  2019-04-26  9:15   ` Wolfram Sang
  2019-04-26  9:45   ` Simon Horman
  2019-04-26  5:18 ` [PATCH 2/3] mmc: tmio: No memory size limitation if runs on IOMMU Yoshihiro Shimoda
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 18+ messages in thread
From: Yoshihiro Shimoda @ 2019-04-26  5:18 UTC (permalink / raw)
  To: ulf.hansson, wsa+renesas; +Cc: linux-mmc, linux-renesas-soc, 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


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 2/3] mmc: tmio: No memory size limitation if runs on IOMMU
  2019-04-26  5:18 [PATCH 0/3] mmc: renesas_sdhi_internal_dmac: improve performance by using IOMMU Yoshihiro Shimoda
  2019-04-26  5:18 ` [PATCH 1/3] mmc: tmio: add init_card ops Yoshihiro Shimoda
@ 2019-04-26  5:18 ` Yoshihiro Shimoda
  2019-04-26  8:09   ` Sergei Shtylyov
  2019-04-26  9:45   ` Simon Horman
  2019-04-26  5:18 ` [PATCH 3/3] mmc: renesas_sdhi_internal_dmac: use multiple segments if possible Yoshihiro Shimoda
  2019-04-26  9:46 ` [PATCH 0/3] mmc: renesas_sdhi_internal_dmac: improve performance by using IOMMU Wolfram Sang
  3 siblings, 2 replies; 18+ messages in thread
From: Yoshihiro Shimoda @ 2019-04-26  5:18 UTC (permalink / raw)
  To: ulf.hansson, wsa+renesas; +Cc: linux-mmc, linux-renesas-soc, 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


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 3/3] mmc: renesas_sdhi_internal_dmac: use multiple segments if possible
  2019-04-26  5:18 [PATCH 0/3] mmc: renesas_sdhi_internal_dmac: improve performance by using IOMMU Yoshihiro Shimoda
  2019-04-26  5:18 ` [PATCH 1/3] mmc: tmio: add init_card ops Yoshihiro Shimoda
  2019-04-26  5:18 ` [PATCH 2/3] mmc: tmio: No memory size limitation if runs on IOMMU Yoshihiro Shimoda
@ 2019-04-26  5:18 ` Yoshihiro Shimoda
  2019-04-26  9:37   ` Wolfram Sang
  2019-04-26  9:45   ` Simon Horman
  2019-04-26  9:46 ` [PATCH 0/3] mmc: renesas_sdhi_internal_dmac: improve performance by using IOMMU Wolfram Sang
  3 siblings, 2 replies; 18+ messages in thread
From: Yoshihiro Shimoda @ 2019-04-26  5:18 UTC (permalink / raw)
  To: ulf.hansson, wsa+renesas; +Cc: linux-mmc, linux-renesas-soc, 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


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/3] mmc: tmio: No memory size limitation if runs on IOMMU
  2019-04-26  5:18 ` [PATCH 2/3] mmc: tmio: No memory size limitation if runs on IOMMU Yoshihiro Shimoda
@ 2019-04-26  8:09   ` Sergei Shtylyov
  2019-04-26  9:17     ` Wolfram Sang
  2019-04-26  9:45   ` Simon Horman
  1 sibling, 1 reply; 18+ messages in thread
From: Sergei Shtylyov @ 2019-04-26  8:09 UTC (permalink / raw)
  To: Yoshihiro Shimoda, ulf.hansson, wsa+renesas; +Cc: linux-mmc, linux-renesas-soc

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/3] mmc: tmio: add init_card ops
  2019-04-26  5:18 ` [PATCH 1/3] mmc: tmio: add init_card ops Yoshihiro Shimoda
@ 2019-04-26  9:15   ` Wolfram Sang
  2019-04-26  9:45   ` Simon Horman
  1 sibling, 0 replies; 18+ messages in thread
From: Wolfram Sang @ 2019-04-26  9:15 UTC (permalink / raw)
  To: Yoshihiro Shimoda; +Cc: ulf.hansson, wsa+renesas, linux-mmc, linux-renesas-soc

[-- 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 --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/3] mmc: tmio: No memory size limitation if runs on IOMMU
  2019-04-26  8:09   ` Sergei Shtylyov
@ 2019-04-26  9:17     ` Wolfram Sang
  2019-05-07  7:13       ` Yoshihiro Shimoda
  0 siblings, 1 reply; 18+ messages in thread
From: Wolfram Sang @ 2019-04-26  9:17 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: Yoshihiro Shimoda, ulf.hansson, wsa+renesas, linux-mmc,
	linux-renesas-soc

[-- 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 --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/3] mmc: renesas_sdhi_internal_dmac: use multiple segments if possible
  2019-04-26  5:18 ` [PATCH 3/3] mmc: renesas_sdhi_internal_dmac: use multiple segments if possible Yoshihiro Shimoda
@ 2019-04-26  9:37   ` Wolfram Sang
  2019-05-07  7:27     ` Yoshihiro Shimoda
  2019-04-26  9:45   ` Simon Horman
  1 sibling, 1 reply; 18+ messages in thread
From: Wolfram Sang @ 2019-04-26  9:37 UTC (permalink / raw)
  To: Yoshihiro Shimoda; +Cc: ulf.hansson, wsa+renesas, linux-mmc, linux-renesas-soc

[-- 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 --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/3] mmc: tmio: add init_card ops
  2019-04-26  5:18 ` [PATCH 1/3] mmc: tmio: add init_card ops Yoshihiro Shimoda
  2019-04-26  9:15   ` Wolfram Sang
@ 2019-04-26  9:45   ` Simon Horman
  2019-04-26  9:56     ` Wolfram Sang
  1 sibling, 1 reply; 18+ messages in thread
From: Simon Horman @ 2019-04-26  9:45 UTC (permalink / raw)
  To: Yoshihiro Shimoda; +Cc: ulf.hansson, wsa+renesas, linux-mmc, linux-renesas-soc

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>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/3] mmc: tmio: No memory size limitation if runs on IOMMU
  2019-04-26  5:18 ` [PATCH 2/3] mmc: tmio: No memory size limitation if runs on IOMMU Yoshihiro Shimoda
  2019-04-26  8:09   ` Sergei Shtylyov
@ 2019-04-26  9:45   ` Simon Horman
  1 sibling, 0 replies; 18+ messages in thread
From: Simon Horman @ 2019-04-26  9:45 UTC (permalink / raw)
  To: Yoshihiro Shimoda; +Cc: ulf.hansson, wsa+renesas, linux-mmc, linux-renesas-soc

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>


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/3] mmc: renesas_sdhi_internal_dmac: use multiple segments if possible
  2019-04-26  5:18 ` [PATCH 3/3] mmc: renesas_sdhi_internal_dmac: use multiple segments if possible Yoshihiro Shimoda
  2019-04-26  9:37   ` Wolfram Sang
@ 2019-04-26  9:45   ` Simon Horman
  1 sibling, 0 replies; 18+ messages in thread
From: Simon Horman @ 2019-04-26  9:45 UTC (permalink / raw)
  To: Yoshihiro Shimoda; +Cc: ulf.hansson, wsa+renesas, linux-mmc, linux-renesas-soc

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>


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/3] mmc: renesas_sdhi_internal_dmac: improve performance by using IOMMU
  2019-04-26  5:18 [PATCH 0/3] mmc: renesas_sdhi_internal_dmac: improve performance by using IOMMU Yoshihiro Shimoda
                   ` (2 preceding siblings ...)
  2019-04-26  5:18 ` [PATCH 3/3] mmc: renesas_sdhi_internal_dmac: use multiple segments if possible Yoshihiro Shimoda
@ 2019-04-26  9:46 ` Wolfram Sang
  2019-05-07  8:58   ` Yoshihiro Shimoda
  3 siblings, 1 reply; 18+ messages in thread
From: Wolfram Sang @ 2019-04-26  9:46 UTC (permalink / raw)
  To: Yoshihiro Shimoda; +Cc: ulf.hansson, wsa+renesas, linux-mmc, linux-renesas-soc

[-- 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 --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/3] mmc: tmio: add init_card ops
  2019-04-26  9:45   ` Simon Horman
@ 2019-04-26  9:56     ` Wolfram Sang
  2019-04-26 10:17       ` Simon Horman
  0 siblings, 1 reply; 18+ messages in thread
From: Wolfram Sang @ 2019-04-26  9:56 UTC (permalink / raw)
  To: Simon Horman
  Cc: Yoshihiro Shimoda, ulf.hansson, wsa+renesas, linux-mmc,
	linux-renesas-soc

[-- 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 --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/3] mmc: tmio: add init_card ops
  2019-04-26  9:56     ` Wolfram Sang
@ 2019-04-26 10:17       ` Simon Horman
  0 siblings, 0 replies; 18+ messages in thread
From: Simon Horman @ 2019-04-26 10:17 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Yoshihiro Shimoda, ulf.hansson, wsa+renesas, linux-mmc,
	linux-renesas-soc

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.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH 2/3] mmc: tmio: No memory size limitation if runs on IOMMU
  2019-04-26  9:17     ` Wolfram Sang
@ 2019-05-07  7:13       ` Yoshihiro Shimoda
  0 siblings, 0 replies; 18+ messages in thread
From: Yoshihiro Shimoda @ 2019-05-07  7:13 UTC (permalink / raw)
  To: Wolfram Sang, Sergei Shtylyov
  Cc: ulf.hansson, wsa+renesas, linux-mmc, linux-renesas-soc

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


^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH 3/3] mmc: renesas_sdhi_internal_dmac: use multiple segments if possible
  2019-04-26  9:37   ` Wolfram Sang
@ 2019-05-07  7:27     ` Yoshihiro Shimoda
  0 siblings, 0 replies; 18+ messages in thread
From: Yoshihiro Shimoda @ 2019-05-07  7:27 UTC (permalink / raw)
  To: Wolfram Sang; +Cc: ulf.hansson, wsa+renesas, linux-mmc, linux-renesas-soc

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


^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH 0/3] mmc: renesas_sdhi_internal_dmac: improve performance by using IOMMU
  2019-04-26  9:46 ` [PATCH 0/3] mmc: renesas_sdhi_internal_dmac: improve performance by using IOMMU Wolfram Sang
@ 2019-05-07  8:58   ` Yoshihiro Shimoda
  2019-05-08  4:28     ` Yoshihiro Shimoda
  0 siblings, 1 reply; 18+ messages in thread
From: Yoshihiro Shimoda @ 2019-05-07  8:58 UTC (permalink / raw)
  To: Wolfram Sang; +Cc: ulf.hansson, wsa+renesas, linux-mmc, linux-renesas-soc

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


^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH 0/3] mmc: renesas_sdhi_internal_dmac: improve performance by using IOMMU
  2019-05-07  8:58   ` Yoshihiro Shimoda
@ 2019-05-08  4:28     ` Yoshihiro Shimoda
  0 siblings, 0 replies; 18+ messages in thread
From: Yoshihiro Shimoda @ 2019-05-08  4:28 UTC (permalink / raw)
  To: Wolfram Sang; +Cc: ulf.hansson, wsa+renesas, linux-mmc, linux-renesas-soc

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


^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2019-05-08  4:29 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-26  5:18 [PATCH 0/3] mmc: renesas_sdhi_internal_dmac: improve performance by using IOMMU Yoshihiro Shimoda
2019-04-26  5:18 ` [PATCH 1/3] mmc: tmio: add init_card ops Yoshihiro Shimoda
2019-04-26  9:15   ` Wolfram Sang
2019-04-26  9:45   ` Simon Horman
2019-04-26  9:56     ` Wolfram Sang
2019-04-26 10:17       ` Simon Horman
2019-04-26  5:18 ` [PATCH 2/3] mmc: tmio: No memory size limitation if runs on IOMMU Yoshihiro Shimoda
2019-04-26  8:09   ` Sergei Shtylyov
2019-04-26  9:17     ` Wolfram Sang
2019-05-07  7:13       ` Yoshihiro Shimoda
2019-04-26  9:45   ` Simon Horman
2019-04-26  5:18 ` [PATCH 3/3] mmc: renesas_sdhi_internal_dmac: use multiple segments if possible Yoshihiro Shimoda
2019-04-26  9:37   ` Wolfram Sang
2019-05-07  7:27     ` Yoshihiro Shimoda
2019-04-26  9:45   ` Simon Horman
2019-04-26  9:46 ` [PATCH 0/3] mmc: renesas_sdhi_internal_dmac: improve performance by using IOMMU Wolfram Sang
2019-05-07  8:58   ` Yoshihiro Shimoda
2019-05-08  4:28     ` Yoshihiro Shimoda

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).