| Message ID | 20180104201045.31139-1-linus.walleij@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | RFT: mmc: sdhci: Implement an SDHCI-specific bounce buffer | expand |
> I would be surprised if this works, which is why i sprinkled a > few extra prints all over the place so we can see what goes > wrong. If it would happen to work, try to comment out the noisy > prints ("bouncing read/write data") so they don't disturb test > results. I can comfort you, it doesn't work :). I tried both, with your print outs and without. After I patched the kernel it doesn't find the root filesystem anymore. Here is the kernel log: sdhci: Secure Digital Host Controller Interface driver sdhci: Copyright(c) Pierre Ossman sdhci-pltfm: SDHCI platform and OF driver helper SDMA with bounce buffer: bounce up to 1024 segments into one, max segment size 524288 bytes mmc0: SDHCI controller on 53fb4000.esdhc [53fb4000.esdhc] using DMA sdhci-esdhc-imx 53fb8000.esdhc: Got CD GPIO sdhci-esdhc-imx 53fb8000.esdhc: Got WP GPIO SDMA with bounce buffer: bounce up to 1024 segments into one, max segment size 524288 bytes mmc1: SDHCI controller on 53fb8000.esdhc [53fb8000.esdhc] using DMA bouncing read data bouncing read data mmc0: new high speed MMC card at address 0001 oprofile: no performance counters mmcblk0: mmc0:0001 002G00 1.83 GiB mmcblk0boot0: mmc0:0001 002G00 partition 1 512 KiB mmcblk0boot1: mmc0:0001 002G00 partition 2 512 KiB mmcblk0rpmb: mmc0:0001 002G00 partition 3 128 KiB bouncing read data bouncing read data bouncing read data bouncing read data bouncing read data bouncing read data bouncing read data mmcblk0: p1 p2 p3 p4 < p5 p6 p7 p8 p9 p10 > oprofile: using timer interrupt. NET: Registered protocol family 10 Segment Routing with IPv6 sit: IPv6, IPv4 and MPLS over IPv4 tunneling driver NET: Registered protocol family 17 can: controller area network core (rev 20170425 abi 9) NET: Registered protocol family 29 can: raw protocol (rev 20170425) can: broadcast manager protocol (rev 20170425 t) can: netlink gateway (rev 20170425) max_hops=1 input: gpio-keys as /devices/platform/gpio-keys/input/input1 imxdi_rtc 53ffc000.dryice: setting system clock to 1970-01-03 00:50:58 UTC (175858) bouncing read data EXT4-fs (mmcblk0p8): couldn't mount as ext3 due to feature incompatibilities bouncing read data EXT4-fs (mmcblk0p8): couldn't mount as ext2 due to feature incompatibilities bouncing read data bouncing read data EXT4-fs (mmcblk0p8): ext4_check_descriptors: Block bitmap for group 6 not in group (block 0)! EXT4-fs (mmcblk0p8): group descriptors corrupted! VFS: Cannot open root device "mmcblk0p8" or unknown-block(259,0): error -117 Please append a correct "root=" boot option; here are the available partitions: b300 1916928 mmcblk0 driver: mmcblk b301 767 mmcblk0p1 00000000-01 b302 128 mmcblk0p2 00000000-02 b303 128 mmcblk0p3 00000000-03 b304 1 mmcblk0p4 b305 16384 mmcblk0p5 00000000-05 b306 409599 mmcblk0p6 00000000-06 b307 16383 mmcblk0p7 00000000-07 103:00000 409599 mmcblk0p8 00000000-08 103:00001 102399 mmcblk0p9 00000000-09 103:00002 961535 mmcblk0p10 00000000-0a b318 128 mmcblk0rpmb (driver?) b310 512 mmcblk0boot1 (driver?) b308 512 mmcblk0boot0 (driver?) Kernel panic - not syncing: VFS: Unable to mount root fs on unknown-block(259,0) ---[ end Kernel panic - not syncing: VFS: Unable to mount root fs on unknown-block(259,0) random: crng init done Suprisingly there is a mmcblk0rpmb partition now. I don't know what that is and if this was there before, but normally it has nothing to do with our system. On our other systems it is not present. After I flashed another kernel without your patch it's booting without error. -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jan 5, 2018 at 8:51 AM, Benjamin Beckmeyer <beckmeyer.b@rittal.de> wrote: > I can comfort you, it doesn't work :). I tried both, with your print > outs and without. Hey it doesn't segfault! :D > After I patched the kernel it doesn't find the root > filesystem anymore. Here is the kernel log: OK I clearly need to find a system with sdhci on it, so I dug up a laptop with SDHCI to experiment on. Now trying to figure out a bit about x86_64 development... > Suprisingly there is a mmcblk0rpmb partition now. I don't know what that is > and if this was there before, but normally it has nothing to do > with our system. On our other systems it is not present. It is a feature of the eMMC card, and the feature list is retrieved with special commands from the card. Usually eMMCs that have boot partitions also have an RPMB partition. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> Hey it doesn't segfault! :D Okay, so I guess it was a successfully test :D. > OK I clearly need to find a system with sdhci on it, so I dug up a laptop > with SDHCI to experiment on. Now trying to figure out a bit about > x86_64 development... Oh, sounds like fun. I'm looking forward to read from you. > > Suprisingly there is a mmcblk0rpmb partition now. I don't know what that is > > and if this was there before, but normally it has nothing to do > > with our system. On our other systems it is not present. > It is a feature of the eMMC card, and the feature list is retrieved with > special commands from the card. Usually eMMCs that have boot partitions > also have an RPMB partition. Thanks for that information, I assume it comes with a newer kernel because with our old 3.7.2 kernel there was nothing like that. Kind regards, Benjamin Beckmeyer -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jan 5, 2018 at 11:03 AM, Benjamin Beckmeyer <beckmeyer.b@rittal.de> wrote: >> Hey it doesn't segfault! :D > > Okay, so I guess it was a successfully test :D. > >> OK I clearly need to find a system with sdhci on it, so I dug up a laptop >> with SDHCI to experiment on. Now trying to figure out a bit about >> x86_64 development... > > Oh, sounds like fun. I'm looking forward to read from you. > >> > Suprisingly there is a mmcblk0rpmb partition now. I don't know what that is >> > and if this was there before, but normally it has nothing to do >> > with our system. On our other systems it is not present. > >> It is a feature of the eMMC card, and the feature list is retrieved with >> special commands from the card. Usually eMMCs that have boot partitions >> also have an RPMB partition. > > Thanks for that information, I assume it comes with a newer kernel because with > our old 3.7.2 kernel there was nothing like that. Actually the patch works (!) with my laptop! It also has only SDMA as it turns out. I'm working on trying to figure out the problem.... Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jan 5, 2018 at 11:45 AM, Linus Walleij <linus.walleij@linaro.org> wrote: > On Fri, Jan 5, 2018 at 11:03 AM, Benjamin Beckmeyer > <beckmeyer.b@rittal.de> wrote: >>> Hey it doesn't segfault! :D >> >> Okay, so I guess it was a successfully test :D. >> >>> OK I clearly need to find a system with sdhci on it, so I dug up a laptop >>> with SDHCI to experiment on. Now trying to figure out a bit about >>> x86_64 development... >> >> Oh, sounds like fun. I'm looking forward to read from you. >> >>> > Suprisingly there is a mmcblk0rpmb partition now. I don't know what that is >>> > and if this was there before, but normally it has nothing to do >>> > with our system. On our other systems it is not present. >> >>> It is a feature of the eMMC card, and the feature list is retrieved with >>> special commands from the card. Usually eMMCs that have boot partitions >>> also have an RPMB partition. >> >> Thanks for that information, I assume it comes with a newer kernel because with >> our old 3.7.2 kernel there was nothing like that. > > Actually the patch works (!) with my laptop! > > It also has only SDMA as it turns out. > > I'm working on trying to figure out the problem.... When you say you flashed "another kernel and it works" did you mean some random kernel, or exactly the same you applied my patch on, but without my patch? Please confirm that you have a booting merge base so I'm not chasing ghosts here :) Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> When you say you flashed "another kernel and it works" did you mean > some random kernel, or exactly the same you applied my patch on, > but without my patch? > Please confirm that you have a booting merge base so I'm not chasing > ghosts here :) Sorry for not be exactly enough. I'm using Yocto to build a kernel with some other patches(which have nothing to do with emmc). This is my base and on this base I test your patches. With your patches it is not booting and without it is booting. So, no ghosts at all. Kind regards, Benjamin Beckmeyer -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jan 5, 2018 at 12:08 PM, Benjamin Beckmeyer <beckmeyer.b@rittal.de> wrote: >> When you say you flashed "another kernel and it works" did you mean >> some random kernel, or exactly the same you applied my patch on, >> but without my patch? > >> Please confirm that you have a booting merge base so I'm not chasing >> ghosts here :) > > Sorry for not be exactly enough. > I'm using Yocto to build a kernel with some other patches(which have nothing > to do with emmc). This is my base and on this base I test your patches. > With your patches it is not booting and without it is booting. OK we're back to guesswork, I guess maybe I need to use dma_alloc_coherent() since the buffer gets bigger than 64K. I'll test that and respin. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index e9290a3439d5..d35e25effe7d 100644 --- a/drivers/mmc/host/sdhci.c +++ b/drivers/mmc/host/sdhci.c @@ -502,8 +502,28 @@ static int sdhci_pre_dma_transfer(struct sdhci_host *host, if (data->host_cookie == COOKIE_PRE_MAPPED) return data->sg_count; - sg_count = dma_map_sg(mmc_dev(host->mmc), data->sg, data->sg_len, - mmc_get_dma_dir(data)); + /* Bounce write requests to the bounce buffer */ + if (host->bounce_buffer) { + if (mmc_get_dma_dir(data) == DMA_TO_DEVICE) { + pr_info("bouncing write data\n"); + + /* Copy the data to the bounce buffer */ + sg_copy_to_buffer(data->sg, data->sg_len, + host->bounce_buffer, host->bounce_buffer_size); + } + + /* Map the bounce buffer to the device for both reads and writes */ + sg_count = dma_map_sg(mmc_dev(host->mmc), host->bounce_sg, + 1, DMA_TO_DEVICE); + + /* Overzealous check */ + if (sg_count != 1) + pr_err("mapped bounce buffer more than one SG entry\n"); + } else { + /* Just access the data directly from memory */ + sg_count = dma_map_sg(mmc_dev(host->mmc), data->sg, data->sg_len, + mmc_get_dma_dir(data)); + } if (sg_count == 0) return -ENOSPC; @@ -858,8 +878,13 @@ static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_command *cmd) SDHCI_ADMA_ADDRESS_HI); } else { WARN_ON(sg_cnt != 1); - sdhci_writel(host, sg_dma_address(data->sg), - SDHCI_DMA_ADDRESS); + /* Bounce buffer goes to work */ + if (host->bounce_buffer) + sdhci_writel(host, sg_dma_address(host->bounce_sg), + SDHCI_DMA_ADDRESS); + else + sdhci_writel(host, sg_dma_address(data->sg), + SDHCI_DMA_ADDRESS); } } @@ -2248,7 +2273,12 @@ static void sdhci_pre_req(struct mmc_host *mmc, struct mmc_request *mrq) mrq->data->host_cookie = COOKIE_UNMAPPED; - if (host->flags & SDHCI_REQ_USE_DMA) + /* + * No pre-mapping in the pre hook if we're using the bounce buffer, + * for that we would need two bounce buffers since one buffer is + * in flight when this is getting called. + */ + if (host->flags & SDHCI_REQ_USE_DMA && !host->bounce_buffer) sdhci_pre_dma_transfer(host, mrq->data, COOKIE_PRE_MAPPED); } @@ -2352,8 +2382,25 @@ static bool sdhci_request_done(struct sdhci_host *host) struct mmc_data *data = mrq->data; if (data && data->host_cookie == COOKIE_MAPPED) { - dma_unmap_sg(mmc_dev(host->mmc), data->sg, data->sg_len, - mmc_get_dma_dir(data)); + if (host->bounce_buffer) { + /* Unmap the bounce buffer */ + dma_unmap_sg(mmc_dev(host->mmc), host->bounce_sg, + 1, mmc_get_dma_dir(data)); + + /* On reads, copy the data back to the sglist */ + if (mmc_get_dma_dir(data) == DMA_FROM_DEVICE) { + pr_info("bouncing read data\n"); + + sg_copy_from_buffer(data->sg, data->sg_len, + host->bounce_buffer, + host->bounce_buffer_size); + } + } else { + /* Unmap the raw data */ + dma_unmap_sg(mmc_dev(host->mmc), data->sg, + data->sg_len, + mmc_get_dma_dir(data)); + } data->host_cookie = COOKIE_UNMAPPED; } } @@ -2636,7 +2683,12 @@ static void sdhci_data_irq(struct sdhci_host *host, u32 intmask) */ if (intmask & SDHCI_INT_DMA_END) { u32 dmastart, dmanow; - dmastart = sg_dma_address(host->data->sg); + + if (host->bounce_buffer) + dmastart = sg_dma_address(host->bounce_sg); + else + dmastart = sg_dma_address(host->data->sg); + dmanow = dmastart + host->data->bytes_xfered; /* * Force update to the next DMA block boundary. @@ -3713,6 +3765,50 @@ int sdhci_setup_host(struct sdhci_host *host) */ mmc->max_blk_count = (host->quirks & SDHCI_QUIRK_NO_MULTIBLOCK) ? 1 : 65535; + if (mmc->max_segs == 1) { + unsigned int max_blocks; + unsigned int max_seg_size; + + max_seg_size = mmc->max_req_size; + max_blocks = max_seg_size / 512; + + /* + * When we just support one segment, we can get significant speedups + * by the help of a bounce buffer to group scattered reads/writes + * together. + * + * TODO: is this too big? Stealing too much memory? The old bounce + * buffer is max 64K. This should be the 512K that SDMA can handle + * if I read the code above right. Anyways let's try this. + * FIXME: use devm_* + */ + host->bounce_buffer = kmalloc(max_seg_size, GFP_KERNEL); + if (!host->bounce_buffer) { + pr_err("failed to allocate %u bytes for bounce buffer\n", + max_seg_size); + return -ENOMEM; + } + host->bounce_buffer_size = max_seg_size; + + /* + * Initialize single-entry sglist to point at the bounce buffer + * FIXME: use devm_* + */ + host->bounce_sg = kmalloc(sizeof(*host->bounce_sg), GFP_KERNEL); + if (!host->bounce_sg) { + pr_err("failed to allocate bounce buffer sglist\n"); + return -ENOMEM; + } + sg_init_one(host->bounce_sg, host->bounce_buffer, max_seg_size); + + /* Lie about this since we're bouncing */ + mmc->max_segs = max_blocks; + mmc->max_seg_size = max_seg_size; + + pr_info("SDMA with bounce buffer: bounce up to %u segments into one, max segment size %u bytes\n", + max_blocks, max_seg_size); + } + return 0; unreg: @@ -3743,6 +3839,12 @@ void sdhci_cleanup_host(struct sdhci_host *host) host->align_addr); host->adma_table = NULL; host->align_buffer = NULL; + /* kfree() on NULL is fine */ + kfree(host->bounce_sg); + kfree(host->bounce_buffer); + /* Overzealous but following the style above */ + host->bounce_sg = NULL; + host->bounce_buffer = NULL; } EXPORT_SYMBOL_GPL(sdhci_cleanup_host); diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h index 54bc444c317f..31fc2302454a 100644 --- a/drivers/mmc/host/sdhci.h +++ b/drivers/mmc/host/sdhci.h @@ -440,6 +440,9 @@ struct sdhci_host { int irq; /* Device IRQ */ void __iomem *ioaddr; /* Mapped address */ + char *bounce_buffer; /* For packing SDMA reads/writes */ + size_t bounce_buffer_size; + struct scatterlist *bounce_sg; const struct sdhci_ops *ops; /* Low level hw interface */
The bounce buffer is gone from the MMC core, and now we found out that there are some (crippled) i.MX boards out there that have broken ADMA (cannot do scatter-gather), and broken PIO so they must use SDMA. SDMA sets down the number of segments to one, so that each segment gets turned into a singular request that ping-pongs to the block layer before the next request/segment is issued. These devices can see major benefits from a bounce buffer, as a fragmented read or write buffer may come in even though the sectors we will be reading or writing to the MMC/SD-card are consecutive. This patch accumulates those fragmented scatterlists in a physically contigous bounce buffer so that we can issue bigger DMA data chunks to/from the card. It is possible to achieve even better speed-ups by adding a second bounce buffer so that the ->pre_req() hook in the driver can do the buffer copying and DMA mapping/flushing while the request is in flight. We save this optimization for later. Signed-off-by: Linus Walleij <linus.walleij@linaro.org> --- I would be surprised if this works, which is why i sprinkled a few extra prints all over the place so we can see what goes wrong. If it would happen to work, try to comment out the noisy prints ("bouncing read/write data") so they don't disturb test results. --- drivers/mmc/host/sdhci.c | 118 +++++++++++++++++++++++++++++++++++++++++++---- drivers/mmc/host/sdhci.h | 3 ++ 2 files changed, 113 insertions(+), 8 deletions(-) -- 2.14.3 -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html