> -----Original Message----- > From: svenkatr@xxxxxxxxx [mailto:svenkatr@xxxxxxxxx] On Behalf Of Venkatraman S > Sent: Thursday, April 29, 2010 11:05 PM > To: linux-omap@xxxxxxxxxxxxxxx; linux-mmc@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxxxxx > Cc: Chikkature Rajashekar, Madhusudhan; Adrian Hunter; Kadiyala, Kishore; Shilimkar, Santosh; Tony > Lindgren > Subject: [PATCH v8 2/2] omap hsmmc: adaptation of sdma descriptor autoloading feature > > From 1c63dd42fc6c563c931168779ce73401332faa52 Mon Sep 17 00:00:00 2001 > From: Venkatraman S <svenkatr@xxxxxx> > Date: Thu, 29 Apr 2010 22:43:31 +0530 > Subject: [PATCH 2/2] omap hsmmc: adaptation of sdma descriptor > autoloading feature > > Start to use the sDMA descriptor autoloading feature. > For large datablocks, the MMC driver has to repeatedly setup, > program and teardown the dma channel for each element of the > sglist received in omap_hsmmc_request. > > By using descriptor autoloading, transfers from / to each element of > the sglist is pre programmed into a linked list. The sDMA driver > completes the entire transaction and provides a single interrupt. > > Due to this, number of dma interrupts for a typical 100MB transfer on the MMC is > reduced from 25000 to about 400 (approximate). Transfer speeds are > improved by ~5%. > (Though it varies on the size of read / write & improves on huge transfers) > > Descriptor autoloading is available only in 3630 and 4430 (as of now). > Hence normal DMA mode is also retained. > > Signed-off-by: Venkatraman S <svenkatr@xxxxxx> > CC: Adrian Hunter <adrian.hunter@xxxxxxxxx> > CC: Madhusudhan C <madhu.cr@xxxxxx> > CC: Shilimkar Santosh <santosh.shilimkar@xxxxxx> > CC: Tony Lindgren <tony@xxxxxxxxxxx> > --- > Changes since previous version > * Rebased to Adrian Hunter's interrupt syncronisation patch > https://patchwork.kernel.org/patch/94670/ > * Rename ICR name definition > drivers/mmc/host/omap_hsmmc.c | 148 ++++++++++++++++++++++++++++++++++------ > 1 files changed, 125 insertions(+), 23 deletions(-) > > diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c > index 65093d4..095fd94 100644 > --- a/drivers/mmc/host/omap_hsmmc.c > +++ b/drivers/mmc/host/omap_hsmmc.c > @@ -102,6 +102,8 @@ > #define SRD (1 << 26) > #define SOFTRESET (1 << 1) > #define RESETDONE (1 << 0) > +/* End of superblock indicator for sglist transfers */ > +#define DMA_ICR_SGLIST_END 0x4D00 > > /* > * FIXME: Most likely all the data using these _DEVID defines should come > @@ -118,6 +120,12 @@ > #define OMAP_MMC_MASTER_CLOCK 96000000 > #define DRIVER_NAME "mmci-omap-hs" > > +#define DMA_TYPE_NODMA 0 " DMA_TYPE_NODMA" doesn't make sense > +#define DMA_TYPE_SDMA 1 > +#define DMA_TYPE_SDMA_DLOAD 2 How about ?? #define TRANSFER_NODMA 0 #define TRANSFER_SDMA_NORMAL 1 #define TRANSFER_SDMA_DESCRIPTOR 2 I think ADMA is also in pipeline, so it can become then #define TRANSFER_ADMA_DESCRIPTOR 3 > + > +#define DMA_CTRL_BUF_SIZE (PAGE_SIZE * 3) > + > /* Timeouts for entering power saving states on inactivity, msec */ > #define OMAP_MMC_DISABLED_TIMEOUT 100 > #define OMAP_MMC_SLEEP_TIMEOUT 1000 > @@ -170,7 +178,11 @@ struct omap_hsmmc_host { > u32 bytesleft; > int suspended; > int irq; > - int use_dma, dma_ch; > + int dma_caps; > + int dma_in_use; This flag isn't necessary. What you are doing is just renaming it. Can't you avoid that ?? Inudertand the intent behind "dma_in_use " instead of "use_dma", but you can surely avoid this change. > + int dma_ch; > + void *dma_ctrl_buf; > + dma_addr_t dma_ctrl_buf_phy; > int dma_line_tx, dma_line_rx; > int slot_id; > int got_dbclk; > @@ -527,7 +539,7 @@ static void omap_hsmmc_enable_irq(struct > omap_hsmmc_host *host) > { > unsigned int irq_mask; > > - if (host->use_dma) > + if (host->dma_in_use) This will go with no flag change. > irq_mask = INT_EN_MASK & ~(BRR_ENABLE | BWR_ENABLE); > else > irq_mask = INT_EN_MASK; > @@ -813,7 +825,7 @@ omap_hsmmc_start_command(struct omap_hsmmc_host > *host, struct mmc_command *cmd, > cmdreg &= ~(DDIR); > } > > - if (host->use_dma) > + if (host->dma_in_use) ditto > cmdreg |= DMA_EN; > > host->req_in_progress = 1; > @@ -842,7 +854,7 @@ static void omap_hsmmc_request_done(struct > omap_hsmmc_host *host, struct mmc_req > > omap_hsmmc_disable_irq(host); > /* Do not complete the request if DMA is still in progress */ > - if (mrq->data && host->use_dma && dma_ch != -1) > + if (mrq->data && host->dma_in_use && dma_ch != -1) ditto > return; > host->mrq = NULL; > mmc_request_done(host->mmc, mrq); > @@ -920,7 +932,7 @@ static void omap_hsmmc_dma_cleanup(struct > omap_hsmmc_host *host, int errno) > host->dma_ch = -1; > spin_unlock(&host->irq_lock); > > - if (host->use_dma && dma_ch != -1) { > + if (host->dma_in_use && dma_ch != -1) { ditto > dma_unmap_sg(mmc_dev(host->mmc), host->data->sg, host->dma_len, > omap_hsmmc_get_dma_dir(host, host->data)); > omap_free_dma(dma_ch); > @@ -1266,7 +1278,6 @@ static void omap_hsmmc_config_dma_params(struct > omap_hsmmc_host *host, > omap_hsmmc_get_dma_sync_dev(host, data), > !(data->flags & MMC_DATA_WRITE)); > > - omap_start_dma(dma_ch); > } > > /* > @@ -1279,21 +1290,23 @@ static void omap_hsmmc_dma_cb(int lch, u16 > ch_status, void *cb_data) > int dma_ch, req_in_progress; > > if (ch_status & OMAP2_DMA_MISALIGNED_ERR_IRQ) > - dev_dbg(mmc_dev(host->mmc), "MISALIGNED_ADRS_ERR\n"); > + dev_dbg(mmc_dev(host->mmc), "Misaligned address error\n"); > > spin_lock(&host->irq_lock); > if (host->dma_ch < 0) { > spin_unlock(&host->irq_lock); > return; > } > - > - host->dma_sg_idx++; > - if (host->dma_sg_idx < host->dma_len) { > - /* Fire up the next transfer. */ > - omap_hsmmc_config_dma_params(host, data, > + if (host->dma_in_use == DMA_TYPE_SDMA) { > + host->dma_sg_idx++; > + if (host->dma_sg_idx < host->dma_len) { > + /* Fire up the next transfer. */ > + omap_hsmmc_config_dma_params(host, data, > data->sg + host->dma_sg_idx); > - spin_unlock(&host->irq_lock); > - return; > + omap_start_dma(host->dma_ch); > + spin_unlock(&host->irq_lock); > + return; > + } > } > > dma_unmap_sg(mmc_dev(host->mmc), data->sg, host->dma_len, > @@ -1315,10 +1328,19 @@ static void omap_hsmmc_dma_cb(int lch, u16 > ch_status, void *cb_data) > } > } > > +static int omap_hsmmc_start_dma_transfer(struct omap_hsmmc_host *host) > +{ > + if (host->dma_in_use == DMA_TYPE_SDMA) > + omap_start_dma(host->dma_ch); > + else if (host->dma_in_use == DMA_TYPE_SDMA_DLOAD) > + return omap_start_dma_sglist_transfers(host->dma_ch, -1); Instead of "-1" , some macro for "NO_PAUSE" would have been better. > + > + return 0; > +} > /* > - * Routine to configure and start DMA for the MMC card > + * Routine to configure DMA for the MMC card > */ > -static int omap_hsmmc_start_dma_transfer(struct omap_hsmmc_host *host, > +static int omap_hsmmc_configure_sdma(struct omap_hsmmc_host *host, > struct mmc_request *req) > { > int dma_ch = 0, ret = 0, i; > @@ -1359,6 +1381,56 @@ static int omap_hsmmc_start_dma_transfer(struct > omap_hsmmc_host *host, > return 0; > } > > +static int omap_hsmmc_configure_sdma_sglist(struct omap_hsmmc_host *host, > + struct mmc_request *req) > +{ > + int i; > + struct omap_dma_sglist_node *sglist, *snode; > + struct mmc_data *data = req->data; > + int blksz; > + int dmadir = omap_hsmmc_get_dma_dir(host, data); > + struct omap_dma_sglist_type2a_params *t2p; > + > + sglist = (struct omap_dma_sglist_node *) host->dma_ctrl_buf; > + snode = sglist; > + blksz = host->data->blksz; > + > + if ((host->dma_len * sizeof(*snode)) > DMA_CTRL_BUF_SIZE) { > + dev_err(mmc_dev(host->mmc), "not enough sglist memory %d\n", > + host->dma_len); > + return -ENOMEM; > + } > + for (i = 0; i < host->dma_len; snode++, i++) { > + snode->desc_type = OMAP_DMA_SGLIST_DESCRIPTOR_TYPE2a; > + snode->num_of_elem = blksz / 4; > + t2p = &snode->sg_node.t2a; > + > + if (dmadir == DMA_FROM_DEVICE) { > + t2p->src_addr = host->mapbase + OMAP_HSMMC_DATA; > + t2p->dst_addr = sg_dma_address(data->sg + i); > + } else { > + t2p->dst_addr = host->mapbase + OMAP_HSMMC_DATA; > + t2p->src_addr = sg_dma_address(data->sg + i); > + } > + snode->flags = > + OMAP_DMA_LIST_DST_VALID | OMAP_DMA_LIST_SRC_VALID; > + > + t2p->cfn_fn = sg_dma_len(data->sg + i) / host->data->blksz; > + t2p->cicr = DMA_ICR_SGLIST_END; > + > + t2p->dst_frame_idx_or_pkt_size = 0; > + t2p->src_frame_idx_or_pkt_size = 0; > + t2p->dst_elem_idx = 0; > + t2p->src_elem_idx = 0; > + } > + dev_dbg(mmc_dev(host->mmc), "new sglist %x len =%d\n", > + host->dma_ctrl_buf_phy, i); > + omap_set_dma_sglist_mode(host->dma_ch, sglist, > + host->dma_ctrl_buf_phy, i, NULL); > + omap_dma_set_sglist_fastmode(host->dma_ch, 1); You should check for return value of above two. If any one of above fails the transfer will fail BADLY > + return 0; > +} > + > static void set_data_timeout(struct omap_hsmmc_host *host, > unsigned int timeout_ns, > unsigned int timeout_clks) > @@ -1420,14 +1492,23 @@ omap_hsmmc_prepare_data(struct omap_hsmmc_host > *host, struct mmc_request *req) > | (req->data->blocks << 16)); > set_data_timeout(host, req->data->timeout_ns, req->data->timeout_clks); > > - if (host->use_dma) { > - ret = omap_hsmmc_start_dma_transfer(host, req); > - if (ret != 0) { > - dev_dbg(mmc_dev(host->mmc), "MMC start dma failure\n"); > + if (host->dma_caps & DMA_TYPE_SDMA) { > + ret = omap_hsmmc_configure_sdma(host, req); > + if (ret) > return ret; > - } > + host->dma_in_use = DMA_TYPE_SDMA; > } > - return 0; > + if ((host->dma_caps & DMA_TYPE_SDMA_DLOAD) && > + host->data->sg_len > 4) { > + ret = omap_hsmmc_configure_sdma_sglist(host, req); > + if (ret) > + return ret; > + host->dma_in_use = DMA_TYPE_SDMA_DLOAD; > + > + } > + ret = omap_hsmmc_start_dma_transfer(host); > + return ret; > + > } > > /* > @@ -2007,7 +2088,9 @@ static int __init omap_hsmmc_probe(struct > platform_device *pdev) > host->mmc = mmc; > host->pdata = pdata; > host->dev = &pdev->dev; > - host->use_dma = 1; > + host->dma_caps = DMA_TYPE_SDMA; > + host->dma_in_use = DMA_TYPE_NODMA; > + host->dma_ctrl_buf = NULL; > host->dev->dma_mask = &pdata->dma_mask; > host->dma_ch = -1; > host->irq = irq; > @@ -2088,6 +2171,15 @@ static int __init omap_hsmmc_probe(struct > platform_device *pdev) > " clk failed\n"); > } > > + if (cpu_is_omap44xx() || cpu_is_omap3630()) { Can we avoid above by passing this part of platform data?? devices.c > + host->dma_ctrl_buf = dma_alloc_coherent(NULL, > + DMA_CTRL_BUF_SIZE, > + &host->dma_ctrl_buf_phy, > + 0); > + if (host->dma_ctrl_buf != NULL) > + host->dma_caps |= DMA_TYPE_SDMA_DLOAD; > + } > + > /* Since we do only SG emulation, we can have as many segs > * as we want. */ Not your patch but above commenting style isn't right > mmc->max_phys_segs = 1024; > @@ -2213,6 +2305,10 @@ err_reg: > err_irq_cd_init: > free_irq(host->irq, host); > err_irq: > + if (host->dma_ctrl_buf) > + dma_free_coherent(NULL, DMA_CTRL_BUF_SIZE, > + host->dma_ctrl_buf, > + host->dma_ctrl_buf_phy); > mmc_host_disable(host->mmc); > clk_disable(host->iclk); > clk_put(host->fclk); > @@ -2240,6 +2336,12 @@ static int omap_hsmmc_remove(struct > platform_device *pdev) > if (host) { > mmc_host_enable(host->mmc); > mmc_remove_host(host->mmc); > + > + if (host->dma_ctrl_buf != NULL) { > + dma_free_coherent(NULL, DMA_CTRL_BUF_SIZE, > + host->dma_ctrl_buf, > + host->dma_ctrl_buf_phy); > + } > if (host->use_reg) > omap_hsmmc_reg_put(host); > if (host->pdata->cleanup) > -- > 1.6.3.3 -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
