Linux-Block Archive on lore.kernel.org
 help / color / Atom feed
* Re: [PATCH v3 3/3] mmc: renesas_sdhi: use multiple segments if possible
       [not found] ` <1558520319-16452-4-git-send-email-yoshihiro.shimoda.uh@renesas.com>
@ 2019-05-22 12:29   ` Christoph Hellwig
  2019-05-23  4:15     ` Yoshihiro Shimoda
  0 siblings, 1 reply; 2+ messages in thread
From: Christoph Hellwig @ 2019-05-22 12:29 UTC (permalink / raw)
  To: Yoshihiro Shimoda
  Cc: ulf.hansson, wsa+renesas, linux-mmc, linux-renesas-soc, iommu,
	linux-block

On Wed, May 22, 2019 at 07:18:39PM +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 to improve the transfer
> performance on renesas_sdhi_internal_dmac.

Well, you can't merge everything with an IOMMU.  For one not every
IOMMU can merge multiple scatterlist segments, second even it can merge
segements the segments need to be aligned to the IOMMU page size.  And
then of course we might have an upper limit on the total mapping.

> +	if (host->pdata->max_segs < SDHI_MAX_SEGS_IN_IOMMU &&
> +	    host->pdev->dev.iommu_group &&
> +	    (mmc_card_mmc(card) || mmc_card_sd(card)))
> +		host->mmc->max_segs = SDHI_MAX_SEGS_IN_IOMMU;

This is way to magic.  We'll need a proper DMA layer API to expose
this information, and preferably a block layer helper to increase
max_segs instead of hacking that up in the driver.

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

* RE: [PATCH v3 3/3] mmc: renesas_sdhi: use multiple segments if possible
  2019-05-22 12:29   ` [PATCH v3 3/3] mmc: renesas_sdhi: use multiple segments if possible Christoph Hellwig
@ 2019-05-23  4:15     ` Yoshihiro Shimoda
  0 siblings, 0 replies; 2+ messages in thread
From: Yoshihiro Shimoda @ 2019-05-23  4:15 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: ulf.hansson, wsa+renesas, linux-mmc, linux-renesas-soc, iommu,
	linux-block

Hi Christoph,

Thank you for your review!

> From: Christoph Hellwig, Sent: Wednesday, May 22, 2019 9:29 PM
> 
> On Wed, May 22, 2019 at 07:18:39PM +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 to improve the transfer
> > performance on renesas_sdhi_internal_dmac.
> 
> Well, you can't merge everything with an IOMMU.  For one not every
> IOMMU can merge multiple scatterlist segments,

I didn't know such IOMMU exists. But, since R-Car Gen3 IOMMU device
(handled by ipmmu-vmsa.c) can merge multiple scatterlist segments,
should this mmc driver check whether the IOMMU device is used or not somehow?

> second even it can merge
> segements the segments need to be aligned to the IOMMU page size.

If this driver checks whether the segments are aligned to the IOMMU
page size before DMA API is called every time, is it acceptable?
If one of the segments is not aligned, this driver should not use
the DMAC.

>  And
> then of course we might have an upper limit on the total mapping.

IIUC, if such a case, DMA API will fail. What do you think?

> > +	if (host->pdata->max_segs < SDHI_MAX_SEGS_IN_IOMMU &&
> > +	    host->pdev->dev.iommu_group &&
> > +	    (mmc_card_mmc(card) || mmc_card_sd(card)))
> > +		host->mmc->max_segs = SDHI_MAX_SEGS_IN_IOMMU;
> 
> This is way to magic.  We'll need a proper DMA layer API to expose
> this information, and preferably a block layer helper to increase
> max_segs instead of hacking that up in the driver.

I think I should have described the detail somewhere. This can expose
this information 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. Is this acceptable if
such a comment is described here?

Best regards,
Yoshihiro Shimoda


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

end of thread, back to index

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <1558520319-16452-1-git-send-email-yoshihiro.shimoda.uh@renesas.com>
     [not found] ` <1558520319-16452-4-git-send-email-yoshihiro.shimoda.uh@renesas.com>
2019-05-22 12:29   ` [PATCH v3 3/3] mmc: renesas_sdhi: use multiple segments if possible Christoph Hellwig
2019-05-23  4:15     ` Yoshihiro Shimoda

Linux-Block Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-block/0 linux-block/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-block linux-block/ https://lore.kernel.org/linux-block \
		linux-block@vger.kernel.org linux-block@archiver.kernel.org
	public-inbox-index linux-block


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-block


AGPL code for this site: git clone https://public-inbox.org/ public-inbox