* [PATCH v2 0/2] mmc: renesas_sdhi: improve performance by using IOMMU @ 2019-05-13 4:29 Yoshihiro Shimoda 2019-05-13 4:29 ` [PATCH v2 1/2] mmc: tmio: No memory size limitation if runs on IOMMU Yoshihiro Shimoda 2019-05-13 4:29 ` [PATCH v2 2/2] mmc: renesas_sdhi: use multiple segments if possible Yoshihiro Shimoda 0 siblings, 2 replies; 10+ messages in thread From: Yoshihiro Shimoda @ 2019-05-13 4:29 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-05-07-v5.1 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 following URL about the performance. (I beleive if the performance is improved, the CPU load is also increased.) https://marc.info/?l=linux-renesas-soc&m=155625619320165&w=2 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 v1 [1]: - 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=110485 Yoshihiro Shimoda (2): mmc: tmio: No memory size limitation if runs on IOMMU mmc: renesas_sdhi: use multiple segments if possible drivers/mmc/host/renesas_sdhi_core.c | 13 +++++++++++++ drivers/mmc/host/tmio_mmc_core.c | 5 +++-- 2 files changed, 16 insertions(+), 2 deletions(-) -- 2.7.4 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 1/2] mmc: tmio: No memory size limitation if runs on IOMMU 2019-05-13 4:29 [PATCH v2 0/2] mmc: renesas_sdhi: improve performance by using IOMMU Yoshihiro Shimoda @ 2019-05-13 4:29 ` Yoshihiro Shimoda 2019-05-13 4:29 ` [PATCH v2 2/2] mmc: renesas_sdhi: use multiple segments if possible Yoshihiro Shimoda 1 sibling, 0 replies; 10+ messages in thread From: Yoshihiro Shimoda @ 2019-05-13 4:29 UTC (permalink / raw) To: ulf.hansson, wsa+renesas; +Cc: linux-mmc, linux-renesas-soc, 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> --- 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..7c76ab0 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() && !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] 10+ messages in thread
* [PATCH v2 2/2] mmc: renesas_sdhi: use multiple segments if possible 2019-05-13 4:29 [PATCH v2 0/2] mmc: renesas_sdhi: improve performance by using IOMMU Yoshihiro Shimoda 2019-05-13 4:29 ` [PATCH v2 1/2] mmc: tmio: No memory size limitation if runs on IOMMU Yoshihiro Shimoda @ 2019-05-13 4:29 ` Yoshihiro Shimoda 2019-05-13 9:00 ` Wolfram Sang 1 sibling, 1 reply; 10+ messages in thread From: Yoshihiro Shimoda @ 2019-05-13 4:29 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 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_core.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/drivers/mmc/host/renesas_sdhi_core.c b/drivers/mmc/host/renesas_sdhi_core.c index 5e9e36e..29a7d66 100644 --- a/drivers/mmc/host/renesas_sdhi_core.c +++ b/drivers/mmc/host/renesas_sdhi_core.c @@ -203,6 +203,17 @@ 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); + + 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 int renesas_sdhi_card_busy(struct mmc_host *mmc) { struct tmio_mmc_host *host = mmc_priv(mmc); @@ -726,6 +737,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 = -- 2.7.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] mmc: renesas_sdhi: use multiple segments if possible 2019-05-13 4:29 ` [PATCH v2 2/2] mmc: renesas_sdhi: use multiple segments if possible Yoshihiro Shimoda @ 2019-05-13 9:00 ` Wolfram Sang 2019-05-13 9:46 ` Yoshihiro Shimoda 0 siblings, 1 reply; 10+ messages in thread From: Wolfram Sang @ 2019-05-13 9:00 UTC (permalink / raw) To: Yoshihiro Shimoda; +Cc: ulf.hansson, wsa+renesas, linux-mmc, linux-renesas-soc [-- Attachment #1: Type: text/plain, Size: 965 bytes --] Hi Shimoda-san, thank you for this update! > +static void renesas_sdhi_init_card(struct mmc_host *mmc, struct mmc_card *card) > +{ > + struct tmio_mmc_host *host = mmc_priv(mmc); > + > + if (host->pdev->dev.iommu_group && I wonder if I am too cautious, but maybe we should have another condition here to be checked first, namely "host->mmc->max_segs < 512"? > + (mmc_card_mmc(card) || mmc_card_sd(card))) > + host->mmc->max_segs = 512; > + else > + host->mmc->max_segs = host->pdata->max_segs; max_segs can be 0, so we should probably have: + host->mmc->max_segs = host->pdata->max_segs ?: 32; That also means, for the sys-dmac and Gen2, we then use 512 for the IOMMU case and 32 (default TMIO value) for the non IOMMU case. My understanding is that SYS DMAC can handle 512 in both cases. Maybe it makes sense then to make an incremental patch setting the max_segs value explicitly to 512 in the sys-dmac driver for Gen2? Kind regards, Wolfram [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH v2 2/2] mmc: renesas_sdhi: use multiple segments if possible 2019-05-13 9:00 ` Wolfram Sang @ 2019-05-13 9:46 ` Yoshihiro Shimoda 2019-05-13 11:34 ` Yoshihiro Shimoda 2019-05-14 6:05 ` Yoshihiro Shimoda 0 siblings, 2 replies; 10+ messages in thread From: Yoshihiro Shimoda @ 2019-05-13 9:46 UTC (permalink / raw) To: Wolfram Sang; +Cc: ulf.hansson, wsa+renesas, linux-mmc, linux-renesas-soc Hi Wolfram-san, > From: Wolfram Sang, Sent: Monday, May 13, 2019 6:01 PM > > Hi Shimoda-san, > > thank you for this update! > > > +static void renesas_sdhi_init_card(struct mmc_host *mmc, struct mmc_card *card) > > +{ > > + struct tmio_mmc_host *host = mmc_priv(mmc); > > + > > + if (host->pdev->dev.iommu_group && > > I wonder if I am too cautious, but maybe we should have another > condition here to be checked first, namely "host->mmc->max_segs < 512"? I got it. I'll fix it on v3 patch. > > + (mmc_card_mmc(card) || mmc_card_sd(card))) > > + host->mmc->max_segs = 512; > > + else > > + host->mmc->max_segs = host->pdata->max_segs; > > max_segs can be 0, so we should probably have: > > + host->mmc->max_segs = host->pdata->max_segs ?: 32; Thank you for the point! I'll fix it on v3 patch. > That also means, for the sys-dmac and Gen2, we then use 512 for the > IOMMU case and 32 (default TMIO value) for the non IOMMU case. My > understanding is that SYS DMAC can handle 512 in both cases. Maybe it > makes sense then to make an incremental patch setting the max_segs value > explicitly to 512 in the sys-dmac driver for Gen2? I also think SYS DMAC can handle 512 segments. However, I'm not sure it can improve the performance or not though. Anyway, an incremental patch makes sense if needed, I think. Best regards, Yoshihiro Shimoda > Kind regards, > > Wolfram ^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH v2 2/2] mmc: renesas_sdhi: use multiple segments if possible 2019-05-13 9:46 ` Yoshihiro Shimoda @ 2019-05-13 11:34 ` Yoshihiro Shimoda 2019-05-13 13:54 ` Wolfram Sang 2019-05-14 6:05 ` Yoshihiro Shimoda 1 sibling, 1 reply; 10+ messages in thread From: Yoshihiro Shimoda @ 2019-05-13 11:34 UTC (permalink / raw) To: Wolfram Sang; +Cc: ulf.hansson, wsa+renesas, linux-mmc, linux-renesas-soc Hi Wolfram-san, > From: Yoshihiro Shimoda, Sent: Monday, May 13, 2019 6:46 PM <snip> > > That also means, for the sys-dmac and Gen2, we then use 512 for the > > IOMMU case and 32 (default TMIO value) for the non IOMMU case. My > > understanding is that SYS DMAC can handle 512 in both cases. Maybe it > > makes sense then to make an incremental patch setting the max_segs value > > explicitly to 512 in the sys-dmac driver for Gen2? > > I also think SYS DMAC can handle 512 segments. However, I'm not sure > it can improve the performance or not though. Anyway, an incremental patch > makes sense if needed, I think. I measured the performance on R-Car H2 Lager. It seems 512 segments improve the sequential input to 5%. May I make an incremental patch on the patch series? What do you think? Best regards, Yoshihiro Shimoda -- kernel v5.1-rc7 + local patches + sdr104,,,,,,,,,,,,,,,,,,,,,,,,,, Buildroot 2019.02.1,,,,,,,,,,,,,,,,,,,,,,,,,, Bonnie++ 1.03e : bonnie\+\+ -d ./ -s 2048 -r 1024 -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 %) max_segs_32_sys_dmac,2G,19651,45,18122,7,11612,5,31417,56,34344,6,20.9,0,16,183,1,+++++,+++,195,1,198,2,+++++,+++,195,2 max_segs_512_sys_dmac,2G,18728,43,18273,8,12405,5,33524,61,34158,6,21.0,0,16,184,1,+++++,+++,189,1,198,2,+++++,+++,194,2 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] mmc: renesas_sdhi: use multiple segments if possible 2019-05-13 11:34 ` Yoshihiro Shimoda @ 2019-05-13 13:54 ` Wolfram Sang 0 siblings, 0 replies; 10+ messages in thread From: Wolfram Sang @ 2019-05-13 13:54 UTC (permalink / raw) To: Yoshihiro Shimoda; +Cc: ulf.hansson, wsa+renesas, linux-mmc, linux-renesas-soc [-- Attachment #1: Type: text/plain, Size: 1096 bytes --] Hi Shimoda-san, > > > That also means, for the sys-dmac and Gen2, we then use 512 for the > > > IOMMU case and 32 (default TMIO value) for the non IOMMU case. My > > > understanding is that SYS DMAC can handle 512 in both cases. Maybe it > > > makes sense then to make an incremental patch setting the max_segs value > > > explicitly to 512 in the sys-dmac driver for Gen2? > > > > I also think SYS DMAC can handle 512 segments. However, I'm not sure > > it can improve the performance or not though. Anyway, an incremental patch > > makes sense if needed, I think. > > I measured the performance on R-Car H2 Lager. It seems 512 segments improve > the sequential input to 5%. May I make an incremental patch on the patch series? > What do you think? Cool! I didn't expect that much of a performance improvement. My main concern was understandable code because there was no "real reason" that we once use 32 and once 512. But if it causes even a performance improvement, even better :) Yes, an incremental patch is a good idea. Thanks for the work, Wolfram [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH v2 2/2] mmc: renesas_sdhi: use multiple segments if possible 2019-05-13 9:46 ` Yoshihiro Shimoda 2019-05-13 11:34 ` Yoshihiro Shimoda @ 2019-05-14 6:05 ` Yoshihiro Shimoda 2019-05-21 21:57 ` Wolfram Sang 1 sibling, 1 reply; 10+ messages in thread From: Yoshihiro Shimoda @ 2019-05-14 6:05 UTC (permalink / raw) To: Wolfram Sang; +Cc: ulf.hansson, wsa+renesas, linux-mmc, linux-renesas-soc Hi Wolfram-san again, > From: Yoshihiro Shimoda, Sent: Monday, May 13, 2019 6:46 PM > > Hi Wolfram-san, > > > From: Wolfram Sang, Sent: Monday, May 13, 2019 6:01 PM > > > > Hi Shimoda-san, > > > > thank you for this update! > > > > > +static void renesas_sdhi_init_card(struct mmc_host *mmc, struct mmc_card *card) > > > +{ > > > + struct tmio_mmc_host *host = mmc_priv(mmc); > > > + > > > + if (host->pdev->dev.iommu_group && > > > > I wonder if I am too cautious, but maybe we should have another > > condition here to be checked first, namely "host->mmc->max_segs < 512"? > > I got it. I'll fix it on v3 patch. I'm afraid but I misunderstood this condition is "host->pdata->max_segs", not "host->mmc->max_segs", to avoid small max_segs value than pdata->max_segs? (No one has such max_segs value at the moment though.) If we use "host->mmc->max_segs", the max_segments value will be toggled by connecting/disconnecting a card like below: (a card is connected) # cat /sys/block/mmcblk0/queue/max_segments 512 (a card is disconnected and connected again) # cat /sys/block/mmcblk0/queue/max_segments 1 (a card is disconnected and connected again) # cat /sys/block/mmcblk0/queue/max_segments 512 ... Best regards, Yoshihiro Shimoda > > > + (mmc_card_mmc(card) || mmc_card_sd(card))) > > > + host->mmc->max_segs = 512; > > > + else > > > + host->mmc->max_segs = host->pdata->max_segs; > > > > max_segs can be 0, so we should probably have: > > > > + host->mmc->max_segs = host->pdata->max_segs ?: 32; > > Thank you for the point! I'll fix it on v3 patch. > > > That also means, for the sys-dmac and Gen2, we then use 512 for the > > IOMMU case and 32 (default TMIO value) for the non IOMMU case. My > > understanding is that SYS DMAC can handle 512 in both cases. Maybe it > > makes sense then to make an incremental patch setting the max_segs value > > explicitly to 512 in the sys-dmac driver for Gen2? > > I also think SYS DMAC can handle 512 segments. However, I'm not sure > it can improve the performance or not though. Anyway, an incremental patch > makes sense if needed, I think. > > Best regards, > Yoshihiro Shimoda > > > Kind regards, > > > > Wolfram ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] mmc: renesas_sdhi: use multiple segments if possible 2019-05-14 6:05 ` Yoshihiro Shimoda @ 2019-05-21 21:57 ` Wolfram Sang 2019-05-22 5:11 ` Yoshihiro Shimoda 0 siblings, 1 reply; 10+ messages in thread From: Wolfram Sang @ 2019-05-21 21:57 UTC (permalink / raw) To: Yoshihiro Shimoda; +Cc: ulf.hansson, wsa+renesas, linux-mmc, linux-renesas-soc [-- Attachment #1: Type: text/plain, Size: 1249 bytes --] Hi Shimoda-san, > > > > + if (host->pdev->dev.iommu_group && > > > > > > I wonder if I am too cautious, but maybe we should have another > > > condition here to be checked first, namely "host->mmc->max_segs < 512"? > > > > I got it. I'll fix it on v3 patch. > > I'm afraid but I misunderstood this condition is > "host->pdata->max_segs", not "host->mmc->max_segs", to avoid small > max_segs value than pdata->max_segs? You are right. I was in deed thinking mmc->max_segs because it will be set at probe time, so it would work with values > 512. But I missed the case you described, I am sorry. But using pdata->max_segs should work. > (No one has such max_segs value at the moment though.) Yes. I want to be future-proof here. Just to avoid that the value might be "magically" decreased if the value might be bigger than 512. It would be hard to find then because it is kinda deep in the driver. Two more remarks: * We should probably use a define for the magic value 512. * Maybe you could add a comment to the init_card function describing why we can increase max_segs in that case. Basically, a short summary of your patch description. Does this make sense to you? Kind regards, Wolfram [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH v2 2/2] mmc: renesas_sdhi: use multiple segments if possible 2019-05-21 21:57 ` Wolfram Sang @ 2019-05-22 5:11 ` Yoshihiro Shimoda 0 siblings, 0 replies; 10+ messages in thread From: Yoshihiro Shimoda @ 2019-05-22 5:11 UTC (permalink / raw) To: Wolfram Sang; +Cc: ulf.hansson, wsa+renesas, linux-mmc, linux-renesas-soc Hi Wolfram-san, > From: Wolfram Sang, Sent: Wednesday, May 22, 2019 6:57 AM > > Hi Shimoda-san, > > > > > > + if (host->pdev->dev.iommu_group && > > > > > > > > I wonder if I am too cautious, but maybe we should have another > > > > condition here to be checked first, namely "host->mmc->max_segs < 512"? > > > > > > I got it. I'll fix it on v3 patch. > > > > I'm afraid but I misunderstood this condition is > > "host->pdata->max_segs", not "host->mmc->max_segs", to avoid small > > max_segs value than pdata->max_segs? > > You are right. I was in deed thinking mmc->max_segs because it will be > set at probe time, so it would work with values > 512. But I missed the > case you described, I am sorry. But using pdata->max_segs should work. Thank you for the reply. I got it! > > (No one has such max_segs value at the moment though.) > > Yes. I want to be future-proof here. Just to avoid that the value might > be "magically" decreased if the value might be bigger than 512. It would > be hard to find then because it is kinda deep in the driver. I got it. > Two more remarks: > > * We should probably use a define for the magic value 512. I think so. I also would like to use a define for a magic value 32. > * Maybe you could add a comment to the init_card function describing why > we can increase max_segs in that case. Basically, a short summary of > your patch description. It's a good idea! I'll add such a short summary. > Does this make sense to you? Yes. Thank you for your comments. I'll make v3 patch later. Best regards, Yoshihiro Shimoda > Kind regards, > > Wolfram ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2019-05-22 5:11 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-05-13 4:29 [PATCH v2 0/2] mmc: renesas_sdhi: improve performance by using IOMMU Yoshihiro Shimoda 2019-05-13 4:29 ` [PATCH v2 1/2] mmc: tmio: No memory size limitation if runs on IOMMU Yoshihiro Shimoda 2019-05-13 4:29 ` [PATCH v2 2/2] mmc: renesas_sdhi: use multiple segments if possible Yoshihiro Shimoda 2019-05-13 9:00 ` Wolfram Sang 2019-05-13 9:46 ` Yoshihiro Shimoda 2019-05-13 11:34 ` Yoshihiro Shimoda 2019-05-13 13:54 ` Wolfram Sang 2019-05-14 6:05 ` Yoshihiro Shimoda 2019-05-21 21:57 ` Wolfram Sang 2019-05-22 5:11 ` 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).