mmci: restrict DMA usage to large, even multiblock transfers
Subsystems (2)
PATCH 1/1 mmci: restrict DMA usage to large, even multiblock transfers
This will restrict the use of DMA to only cover multi-block transfers of blocks that are evenly divisible with the FIFO halfsize (8 4-byte words, 32 bytes). This will be true for any normal, large MMC file transfer, falling back to PIO mode for any smallish reads.
---
With this and the previous DMA end sync code the latest MMCI patches work fine on U8500. On external cards the first read is an 8-byte single block, which would fail otherwise.
I believe the earlier 1-byte-burst code would circumvent this restriction, this solution is more elegant. Whether it is also more efficient to avoid DMA on small packets needs to be confirmed by measurements but I suspect it is.
--- drivers/mmc/host/mmci.c | 10 +++++++--- 1 files changed, 7 insertions(+), 3 deletions(-)
drivers/mmc/host/mmci.c +7 -3
@@ -484,10 +484,14 @@ static void mmci_start_data(struct mmci_host *host, struct mmc_data *data) datactrl |=MCI_DPSM_DIRECTION ;/* -* Attempt to use DMA operation mode, if this -* should fail, fall back to PIO mode +* If the transfer is more than one block, and the block size is +* evenly dividable by the half FIFO so it can be chopped into +* burst packets, then attempt to use DMA operation mode, if this +* should fail, fall back to PIO mode. */ -if (!mmci_dma_start_data (host, datactrl)) +if (data->blocks >1 && + (data->blksz % variant->fifohalfsize ==0 ) && + !mmci_dma_start_data (host, datactrl))return ;/* IRQ mode, map the SG list for CPU reading/writing */
Signature · 1 line
1.7.3.2
PATCH 1/1 mmci: restrict DMA usage to large, even multiblock transfers
Quoted 5 lines · Linus Walleij
This will restrict the use of DMA to only cover multi-block transfers of blocks that are evenly divisible with the FIFO halfsize (8 4-byte words, 32 bytes). This will be true for any normal, large MMC file transfer, falling back to PIO mode for any smallish reads.
That's a very bad idea. If it's an ARM primecell, and we (by some magical reason) are using DMA, we want to always use DMA for transfers larger than the FIFO depth. If we switch to PIO mode, we will see FIFO overruns/ underruns if the CPU can't keep up with the data rate.
So I think you actually want:
if (host->size >= variant->fifosize && !mmci_dma_start_data(host, datactrl)) return;
PATCH 1/1 mmci: restrict DMA usage to large, even multiblock transfers
Quoted 14 lines
Quoted 6 lines · Linus Walleij
This will restrict the use of DMA to only cover multi-block transfers of blocks that are evenly divisible with the FIFO halfsize (8 4-byte words, 32 bytes). This will be true for any normal, large MMC file transfer, falling back to PIO mode for any smallish reads.
Russell King - ARM Linux wrote:
That's a very bad idea. If it's an ARM primecell, and we (by some magical reason) are using DMA, we want to always use DMA for transfers larger than the FIFO depth. If we switch to PIO mode, we will see FIFO overruns/ underruns if the CPU can't keep up with the data rate.
Quoted 5 lines
So I think you actually want:
if (host->size >= variant->fifosize && !mmci_dma_start_data(host, datactrl)) return;
Getting FIFO overrun/underruns is then as of today already the case when running PIO for ARM primecell. To prevent this; you probably now that already, hardware flow control is implemented in ST version of the ARM primecell.
For performance gain I think the threshold should probably not be less than a 512 bytes block, but of course performance test needs to verify this.
Maybe a better option would be to specify a dma_threshold value in the variant struct. Thus the value can differ between what version of the ARM primcell we are using.
Finally I would prefer to move this "threshold" check into the mmci_dma_start_data function instead, since there anyway can be other reasons to why we want to fall back to PIO mode. Better to do that from one place.
/Ulf Hansson
PATCH 1/1 mmci: restrict DMA usage to large, even multiblock transfers
Quoted 4 lines · Ulf Hansson
Getting FIFO overrun/underruns is then as of today already the case when running PIO for ARM primecell. To prevent this; you probably now that already, hardware flow control is implemented in ST version of the ARM primecell.
That really doesn't matter. This driver is not specific to the ST version. It has to work on other versions as well.
I would like to see some further discussion on the unresolved issue of the burst size setting - the discussion between myself and Linus died without any apparant conclusion. Linus tried to get some knowledgable people from ST involved but no one ever followed up.
What I gathered from the discussion was that the additional DMA enable bit found in ST variants should not be set as this changes the behaviour of the DMA requests to be incompatible with the DMA controller requirements. With that bit clear, I see no problem with leaving the burst threshold set at the half FIFO size.
That then makes avoiding DMA for requests below a specific size an optimization issue and nothing more.
PATCH 1/1 mmci: restrict DMA usage to large, even multiblock transfers
Quoted 14 lines · Russell King - ARM Linux
Quoted 4 lines · Ulf Hansson
Getting FIFO overrun/underruns is then as of today already the case when running PIO for ARM primecell. To prevent this; you probably now that already, hardware flow control is implemented in ST version of the ARM primecell.
That really doesn't matter. This driver is not specific to the ST version. It has to work on other versions as well.
I would like to see some further discussion on the unresolved issue of the burst size setting - the discussion between myself and Linus died without any apparant conclusion. Linus tried to get some knowledgable people from ST involved but no one ever followed up.
Quoted 9 lines
What I gathered from the discussion was that the additional DMA enable bit found in ST variants should not be set as this changes the behaviour of the DMA requests to be incompatible with the DMA controller requirements. With that bit clear, I see no problem with leaving the burst threshold set at the half FIFO size.
That then makes avoiding DMA for requests below a specific size an optimization issue and nothing more.
OK, I see your point - I agree. This should then be done as a two step approach. One for making sure DMA transfers is setup on data transfers that is not too small to handle by any reasons. Second as an performance optimization, which most likely means an increased threshold value that could "override" the earlier one.
/Ulf Hansson
PATCH 1/1 mmci: restrict DMA usage to large, even multiblock transfers
In reply to Russell King - ARM LinuxQuoted 14 lines · Russell King - ARM Linux
Quoted 4 lines · Ulf Hansson
Getting FIFO overrun/underruns is then as of today already the case when running PIO for ARM primecell. To prevent this; you probably now that already, hardware flow control is implemented in ST version of the ARM primecell.
That really doesn't matter. This driver is not specific to the ST version. It has to work on other versions as well.
I would like to see some further discussion on the unresolved issue of the burst size setting - the discussion between myself and Linus died without any apparant conclusion. Linus tried to get some knowledgable people from ST involved but no one ever followed up.
Quoted 8 lines
What I gathered from the discussion was that the additional DMA enable bit found in ST variants should not be set as this changes the behaviour of the DMA requests to be incompatible with the DMA controller requirements. With that bit clear, I see no problem with leaving the burst threshold set at the half FIFO size.
That then makes avoiding DMA for requests below a specific size an optimization issue and nothing more.
So... I think this patch will work fine on ST variants, returning the DMA request signals back to their classic meaning. That being LSREQ is only activated on the _final_ transfer rather than the last 8 transfers, which as Linus describes makes your DMA controller complete on the 8th-to-last transfer.
drivers/mmc/host/mmci.c +4 -4
@@ -44,7 +44,6 @@ static unsigned int fmax = 515633;* struct variant_data - MMCI variant-specific quirks * @clkreg: default value for MCICLOCK register * @clkreg_enable: enable value for MMCICLOCK register -* @dmareg_enable: enable value for MMCIDATACTRL register * @datalength_bits: number of bits in the MMCIDATALENGTH register * @fifosize: number of bytes that can be written when MMCI_TXFIFOEMPTY * is asserted (likewise for RX)
@@ -56,7 +55,6 @@ static unsigned int fmax = 515633;struct variant_data {unsigned int clkreg;unsigned int clkreg_enable; -unsigned int dmareg_enable;unsigned int datalength_bits;unsigned int fifosize;unsigned int fifohalfsize;
@@ -83,7 +81,6 @@ static struct variant_data variant_ux500 = { .fifohalfsize = 8 * 4, .clkreg = MCI_CLK_ENABLE, .clkreg_enable = 1 << 14, /* HWFCEN */ - .dmareg_enable = 1 << 12, /* DMAREQCTRL */ .datalength_bits = 24, .sdio = true, .st_clkdiv = true,
@@ -367,6 +364,10 @@ static int mmci_dma_start_data(struct mmci_host *host, unsigned int datactrl)if (!chan)return -EINVAL ; +/* If less than or equal to the fifo size, don't bother with DMA */ +if (host->size <= variant->fifosize) +return -EINVAL ; + device = chan->device; nr_sg =dma_map_sg (device->dev, data->sg, data->sg_len, conf.direction);if (nr_sg ==0 )
@@ -388,7 +389,6 @@ static int mmci_dma_start_data(struct mmci_host *host, unsigned int datactrl)dma_async_issue_pending (chan); datactrl |=MCI_DPSM_DMAENABLE ; - datactrl |= variant->dmareg_enable;/* Trigger the DMA transfer */ writel (datactrl, host->base +MMCIDATACTRL );
PATCH 1/1 mmci: restrict DMA usage to large, even multiblock transfers
2011/2/1 Russell King - ARM Linux <[email protected]>:
Quoted 5 lines
So... I think this patch will work fine on ST variants, returning the DMA request signals back to their classic meaning. ?That being LSREQ is only activated on the _final_ transfer rather than the last 8 transfers, which as Linus describes makes your DMA controller complete on the 8th-to-last transfer.
I've tested this patch on MMC and SD on U8500 and it works fine, so
However I think this weird bit turning off all single requests is there for some reason and I still try to find out what the usecase really is. I highly suspect that it's related to either doing some SDIO usecase or pleasing some buggy DMA controller.
Having it deactivated most likely works for all MMC/SD cards and then we have to think about having (ST-Specific) if (mmc_card_sdio()) clauses for SDIO usecases, which we can defer until we know what's going on.
Yours,
Linus Walleij
PATCH 1/1 mmci: restrict DMA usage to large, even multiblock transfers
Quoted 10 lines · Linus Walleij
2011/2/1 Russell King - ARM Linux <[email protected]>:
Quoted 5 lines
So... I think this patch will work fine on ST variants, returning the DMA request signals back to their classic meaning. ?That being LSREQ is only activated on the _final_ transfer rather than the last 8 transfers, which as Linus describes makes your DMA controller complete on the 8th-to-last transfer.
I've tested this patch on MMC and SD on U8500 and it works fine, so
Quoted 7 lines
Tested-by Linus Walleij <[email protected]>However I think this weird bit turning off all single requests is there for some reason and I still try to find out what the usecase really is. I highly suspect that it's related to either doing some SDIO usecase or pleasing some buggy DMA controller.
We aren't turning off single requests - by clearing DMAREQCTL, we're actually turning them back on.
What I think is going on is someone had an idea of using LBREQ to transfer the last one to seven transfers as a burst, rather than one to seven separate single transfers as an optimization. However, this can only work if the DMA controller obeys the transfer count and doesn't try to transfer more than the remainder of the transfer in any burst. IOW, the DMAC has to truncate bursts to the smaller of the burst size and remainder. If it can't, DMAREQCTL causes problems.
I assume that the U8500/U300 documentation isn't publically available... A quick google doesn't seem to help.
PATCH 1/1 mmci: restrict DMA usage to large, even multiblock transfers
2011/2/1 Russell King - ARM Linux <[email protected]>:
Quoted 9 lines · Linus Walleij
Quoted 5 lines
However I think this weird bit turning off all single requests is there for some reason and I still try to find out what the usecase really is. I highly suspect that it's related to either doing some SDIO usecase or pleasing some buggy DMA controller.
We aren't turning off single requests - by clearing DMAREQCTL, we're actually turning them back on.
Yes that's what I meant, sorry for the confusion.
Quoted 7 lines
What I think is going on is someone had an idea of using LBREQ to transfer the last one to seven transfers as a burst, rather than one to seven separate single transfers as an optimization. ?However, this can only work if the DMA controller obeys the transfer count and doesn't try to transfer more than the remainder of the transfer in any burst. IOW, the DMAC has to truncate bursts to the smaller of the burst size and remainder. ?If it can't, DMAREQCTL causes problems.
Seems plausible. I'll try to look into it.
Quoted 2 lines
I assume that the U8500/U300 documentation isn't publically available... A quick google doesn't seem to help.
In this case it doesn't help... U300 does not have this peculiar feature BTW, it's a pure ux500 thing.
If it gives clear performance improvements (need measurements) we can always patch it in later. For now much is already achieved by this patch.
Yours,
Linus Walleij
PATCH 1/1 mmci: restrict DMA usage to large, even multiblock transfers
In reply to Linus WalleijQuoted 11 lines · Linus Walleij
2011/2/1 Russell King - ARM Linux <[email protected]>:
Quoted 5 lines
So... I think this patch will work fine on ST variants, returning the DMA request signals back to their classic meaning. ?That being LSREQ is only activated on the _final_ transfer rather than the last 8 transfers, which as Linus describes makes your DMA controller complete on the 8th-to-last transfer.
I've tested this patch on MMC and SD on U8500 and it works fine, so
Tested-by Linus Walleij <[email protected]>
Thanks. Can I apply that to the DMA patch too (and combine this with the main MMCI DMA patch) ?
PATCH 1/1 mmci: restrict DMA usage to large, even multiblock transfers
2011/2/1 Russell King - ARM Linux <[email protected]>:
Quoted 9 lines · Linus Walleij
Quoted 8 lines
2011/2/1 Russell King - ARM Linux <[email protected]>:
Quoted 5 lines
So... I think this patch will work fine on ST variants, returning the DMA request signals back to their classic meaning. ?That being LSREQ is only activated on the _final_ transfer rather than the last 8 transfers, which as Linus describes makes your DMA controller complete on the 8th-to-last transfer.
Quoted 6 lines
Quoted 3 lines
I've tested this patch on MMC and SD on U8500 and it works fine, so
Tested-by Linus Walleij <[email protected]>Thanks. ?Can I apply that to the DMA patch too (and combine this with the main MMCI DMA patch) ?
Yes I've tested it and it works for MMC/SD cards with the above additions.
Thanks,
Linus Walleij