* [PATCH] dw_mmc: add support for pre_req and post_req
@ 2011-04-07 8:04 Jaehoon Chung
2011-04-16 16:16 ` Shawn Guo
0 siblings, 1 reply; 5+ messages in thread
From: Jaehoon Chung @ 2011-04-07 8:04 UTC (permalink / raw)
To: linux-mmc@vger.kernel.org
Cc: Chris Ball, Per Forlin, will.newton, Kyungmin Park
This patch is based on Per Forlin's patches.
([PATCH v2 00/12]mmc: use nonblock mmc requests to minimize latency)
After applied Per Forlin's patches, this patch must be apply.
In dw_mmc.c, support for pre/post_reqeust().
Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
Signed-off-by: kyungmin Park <kyungmin.park@samsung.com>
---
drivers/mmc/host/dw_mmc.c | 84 ++++++++++++++++++++++++++++++++++++++++----
include/linux/mmc/dw_mmc.h | 7 ++++
2 files changed, 84 insertions(+), 7 deletions(-)
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 87e1f57..67eeb85 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -417,6 +417,42 @@ static int dw_mci_idmac_init(struct dw_mci *host)
return 0;
}
+static unsigned int dw_mci_pre_dma_transfer(struct dw_mci *host,
+ struct mmc_data *data, struct dw_mci_next *next)
+{
+ unsigned int sg_len;
+
+ BUG_ON(next && data->host_cookie);
+ BUG_ON(!next && data->host_cookie &&
+ data->host_cookie != host->next_data.cookie);
+
+ if (!next && data->host_cookie &&
+ data->host_cookie != host->next_data.cookie) {
+ data->host_cookie = 0;
+ }
+
+ if (next ||
+ (!next && data->host_cookie != host->next_data.cookie)) {
+ sg_len = dma_map_sg(&host->pdev->dev, data->sg,
+ data->sg_len, ((data->flags & MMC_DATA_WRITE)
+ ? DMA_TO_DEVICE : DMA_FROM_DEVICE));
+ } else {
+ sg_len = host->next_data.sg_len;
+ host->next_data.sg_len = 0;
+ }
+
+ if (sg_len == 0)
+ return -EINVAL;
+
+ if (next) {
+ next->sg_len = sg_len;
+ data->host_cookie = ++next->cookie < 0 ? 1 : next->cookie;
+ } else
+ data->sg_len = sg_len;
+
+ return sg_len;
+}
+
static struct dw_mci_dma_ops dw_mci_idmac_ops = {
.init = dw_mci_idmac_init,
.start = dw_mci_idmac_start_dma,
@@ -451,13 +487,9 @@ static int dw_mci_submit_data_dma(struct dw_mci *host, struct mmc_data *data)
return -EINVAL;
}
- if (data->flags & MMC_DATA_READ)
- direction = DMA_FROM_DEVICE;
- else
- direction = DMA_TO_DEVICE;
-
- sg_len = dma_map_sg(&host->pdev->dev, data->sg, data->sg_len,
- direction);
+ sg_len = dw_mci_pre_dma_transfer(host, data, NULL);
+ if (sg_len < 0)
+ return sg_len;
dev_vdbg(&host->pdev->dev,
"sd sg_cpu: %#lx sg_dma: %#lx sg_len: %d\n",
@@ -643,6 +675,42 @@ static void dw_mci_queue_request(struct dw_mci *host, struct dw_mci_slot *slot,
spin_unlock_bh(&host->lock);
}
+static void dw_mci_post_request(struct mmc_host *mmc, struct mmc_request *mrq,
+ int err)
+{
+ struct dw_mci_slot *slot = mmc_priv(mmc);
+ struct mmc_data *data = mrq->data;
+
+ if (!data)
+ return;
+
+ if (slot->host->use_dma) {
+ dma_unmap_sg(&slot->host->pdev->dev, data->sg, data->sg_len,
+ ((data->flags & MMC_DATA_WRITE)
+ ? DMA_TO_DEVICE : DMA_FROM_DEVICE));
+
+ data->host_cookie = 0;
+ }
+}
+
+static void dw_mci_pre_request(struct mmc_host *mmc, struct mmc_request *mrq,
+ bool is_first_req)
+{
+ struct dw_mci_slot *slot = mmc_priv(mmc);
+ struct mmc_data *data = mrq->data;
+
+ if (!data)
+ return;
+
+ BUG_ON(mrq->data->host_cookie);
+
+ if (slot->host->use_dma) {
+ if (dw_mci_pre_dma_transfer(slot->host, mrq->data,
+ &slot->host->next_data))
+ mrq->data->host_cookie = 0;
+ }
+}
+
static void dw_mci_request(struct mmc_host *mmc, struct mmc_request *mrq)
{
struct dw_mci_slot *slot = mmc_priv(mmc);
@@ -748,6 +816,8 @@ static int dw_mci_get_cd(struct mmc_host *mmc)
static const struct mmc_host_ops dw_mci_ops = {
.request = dw_mci_request,
+ .pre_req = dw_mci_pre_request,
+ .post_req = dw_mci_post_request,
.set_ios = dw_mci_set_ios,
.get_ro = dw_mci_get_ro,
.get_cd = dw_mci_get_cd,
diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h
index c0207a7..dca82ee 100644
--- a/include/linux/mmc/dw_mmc.h
+++ b/include/linux/mmc/dw_mmc.h
@@ -35,6 +35,11 @@ enum {
struct mmc_data;
+struct dw_mci_next {
+ unsigned int sg_len;
+ s32 cookie;
+};
+
/**
* struct dw_mci - MMC controller state shared between all slots
* @lock: Spinlock protecting the queue and associated data.
@@ -154,6 +159,8 @@ struct dw_mci {
u32 quirks;
struct regulator *vmmc; /* Power regulator */
+
+ struct dw_mci_next next_data;
};
/* DMA ops for Internal/External DMAC interface */
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] dw_mmc: add support for pre_req and post_req
2011-04-07 8:04 [PATCH] dw_mmc: add support for pre_req and post_req Jaehoon Chung
@ 2011-04-16 16:16 ` Shawn Guo
2011-04-18 4:55 ` Jaehoon Chung
0 siblings, 1 reply; 5+ messages in thread
From: Shawn Guo @ 2011-04-16 16:16 UTC (permalink / raw)
To: Jaehoon Chung
Cc: linux-mmc@vger.kernel.org, Chris Ball, Per Forlin, will.newton,
Kyungmin Park
Hi Jaehoon,
On Thu, Apr 07, 2011 at 05:04:52PM +0900, Jaehoon Chung wrote:
[...]
> +static unsigned int dw_mci_pre_dma_transfer(struct dw_mci *host,
> + struct mmc_data *data, struct dw_mci_next *next)
> +{
> + unsigned int sg_len;
> +
> + BUG_ON(next && data->host_cookie);
> + BUG_ON(!next && data->host_cookie &&
> + data->host_cookie != host->next_data.cookie);
> +
> + if (!next && data->host_cookie &&
> + data->host_cookie != host->next_data.cookie) {
> + data->host_cookie = 0;
> + }
> +
I'm unsure if the 'if' statement makes any sense here, since the
exactly same conditions have been caught by the BUG_ON just above
it.
> + if (next ||
> + (!next && data->host_cookie != host->next_data.cookie)) {
> + sg_len = dma_map_sg(&host->pdev->dev, data->sg,
> + data->sg_len, ((data->flags & MMC_DATA_WRITE)
> + ? DMA_TO_DEVICE : DMA_FROM_DEVICE));
> + } else {
> + sg_len = host->next_data.sg_len;
> + host->next_data.sg_len = 0;
> + }
> +
> + if (sg_len == 0)
> + return -EINVAL;
> +
> + if (next) {
> + next->sg_len = sg_len;
> + data->host_cookie = ++next->cookie < 0 ? 1 : next->cookie;
> + } else
> + data->sg_len = sg_len;
> +
> + return sg_len;
> +}
> +
Function dw_mci_pre_dma_transfer() returns non-zero value anyway,
either -EINVAL or sg_len ...
[...]
> +static void dw_mci_pre_request(struct mmc_host *mmc, struct mmc_request *mrq,
> + bool is_first_req)
> +{
> + struct dw_mci_slot *slot = mmc_priv(mmc);
> + struct mmc_data *data = mrq->data;
> +
> + if (!data)
> + return;
> +
> + BUG_ON(mrq->data->host_cookie);
> +
> + if (slot->host->use_dma) {
> + if (dw_mci_pre_dma_transfer(slot->host, mrq->data,
> + &slot->host->next_data))
> + mrq->data->host_cookie = 0;
... while it steps back to old blocking way by setting
data->host_cookie 0 when dw_mci_pre_dma_transfer returns non-zero.
Per my understanding, it means the non-blocking optimization will
always get bypassed anyway, so I doubt the patch can really gain
performance improvement. Did you get the chance to measure?
--
Regards,
Shawn
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] dw_mmc: add support for pre_req and post_req
2011-04-16 16:16 ` Shawn Guo
@ 2011-04-18 4:55 ` Jaehoon Chung
2011-04-18 5:10 ` Shawn Guo
0 siblings, 1 reply; 5+ messages in thread
From: Jaehoon Chung @ 2011-04-18 4:55 UTC (permalink / raw)
To: Shawn Guo
Cc: Jaehoon Chung, linux-mmc@vger.kernel.org, Chris Ball, Per Forlin,
will.newton, Kyungmin Park
Hi Shawn..
Shawn Guo wrote:
> Hi Jaehoon,
>
> On Thu, Apr 07, 2011 at 05:04:52PM +0900, Jaehoon Chung wrote:
> [...]
>> +static unsigned int dw_mci_pre_dma_transfer(struct dw_mci *host,
>> + struct mmc_data *data, struct dw_mci_next *next)
>> +{
>> + unsigned int sg_len;
>> +
>> + BUG_ON(next && data->host_cookie);
>> + BUG_ON(!next && data->host_cookie &&
>> + data->host_cookie != host->next_data.cookie);
>> +
>> + if (!next && data->host_cookie &&
>> + data->host_cookie != host->next_data.cookie) {
>> + data->host_cookie = 0;
>> + }
>> +
> I'm unsure if the 'if' statement makes any sense here, since the
> exactly same conditions have been caught by the BUG_ON just above
> it.
>
You're right..i'll modify this..
>> + if (next ||
>> + (!next && data->host_cookie != host->next_data.cookie)) {
>> + sg_len = dma_map_sg(&host->pdev->dev, data->sg,
>> + data->sg_len, ((data->flags & MMC_DATA_WRITE)
>> + ? DMA_TO_DEVICE : DMA_FROM_DEVICE));
>> + } else {
>> + sg_len = host->next_data.sg_len;
>> + host->next_data.sg_len = 0;
>> + }
>> +
>> + if (sg_len == 0)
>> + return -EINVAL;
>> +
>> + if (next) {
>> + next->sg_len = sg_len;
>> + data->host_cookie = ++next->cookie < 0 ? 1 : next->cookie;
>> + } else
>> + data->sg_len = sg_len;
>> +
>> + return sg_len;
>> +}
>> +
> Function dw_mci_pre_dma_transfer() returns non-zero value anyway,
> either -EINVAL or sg_len ...
>
Sorry,, i didn't understand this your comments..
> [...]
>> +static void dw_mci_pre_request(struct mmc_host *mmc, struct mmc_request *mrq,
>> + bool is_first_req)
>> +{
>> + struct dw_mci_slot *slot = mmc_priv(mmc);
>> + struct mmc_data *data = mrq->data;
>> +
>> + if (!data)
>> + return;
>> +
>> + BUG_ON(mrq->data->host_cookie);
>> +
>> + if (slot->host->use_dma) {
>> + if (dw_mci_pre_dma_transfer(slot->host, mrq->data,
>> + &slot->host->next_data))
>> + mrq->data->host_cookie = 0;
> ... while it steps back to old blocking way by setting
> data->host_cookie 0 when dw_mci_pre_dma_transfer returns non-zero.
>
> Per my understanding, it means the non-blocking optimization will
> always get bypassed anyway, so I doubt the patch can really gain
> performance improvement. Did you get the chance to measure?
>
Actually, i didn't get performance improvement, but didn't fully affect by CPU_FREQ.
Somebody get performance improvement?
Regards,
Jaehoon Chung
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] dw_mmc: add support for pre_req and post_req
2011-04-18 4:55 ` Jaehoon Chung
@ 2011-04-18 5:10 ` Shawn Guo
2011-04-18 5:10 ` Jaehoon Chung
0 siblings, 1 reply; 5+ messages in thread
From: Shawn Guo @ 2011-04-18 5:10 UTC (permalink / raw)
To: Jaehoon Chung
Cc: linux-mmc@vger.kernel.org, Chris Ball, Per Forlin, will.newton,
Kyungmin Park
On Mon, Apr 18, 2011 at 01:55:11PM +0900, Jaehoon Chung wrote:
> Hi Shawn..
>
> Shawn Guo wrote:
> > Hi Jaehoon,
> >
> > On Thu, Apr 07, 2011 at 05:04:52PM +0900, Jaehoon Chung wrote:
> > [...]
> >> +static unsigned int dw_mci_pre_dma_transfer(struct dw_mci *host,
> >> + struct mmc_data *data, struct dw_mci_next *next)
> >> +{
> >> + unsigned int sg_len;
> >> +
> >> + BUG_ON(next && data->host_cookie);
> >> + BUG_ON(!next && data->host_cookie &&
> >> + data->host_cookie != host->next_data.cookie);
> >> +
> >> + if (!next && data->host_cookie &&
> >> + data->host_cookie != host->next_data.cookie) {
> >> + data->host_cookie = 0;
> >> + }
> >> +
> > I'm unsure if the 'if' statement makes any sense here, since the
> > exactly same conditions have been caught by the BUG_ON just above
> > it.
> >
> You're right..i'll modify this..
>
> >> + if (next ||
> >> + (!next && data->host_cookie != host->next_data.cookie)) {
> >> + sg_len = dma_map_sg(&host->pdev->dev, data->sg,
> >> + data->sg_len, ((data->flags & MMC_DATA_WRITE)
> >> + ? DMA_TO_DEVICE : DMA_FROM_DEVICE));
> >> + } else {
> >> + sg_len = host->next_data.sg_len;
> >> + host->next_data.sg_len = 0;
> >> + }
> >> +
> >> + if (sg_len == 0)
> >> + return -EINVAL;
> >> +
> >> + if (next) {
> >> + next->sg_len = sg_len;
> >> + data->host_cookie = ++next->cookie < 0 ? 1 : next->cookie;
> >> + } else
> >> + data->sg_len = sg_len;
> >> +
> >> + return sg_len;
> >> +}
> >> +
> > Function dw_mci_pre_dma_transfer() returns non-zero value anyway,
> > either -EINVAL or sg_len ...
> >
> Sorry,, i didn't understand this your comments..
>
> > [...]
> >> +static void dw_mci_pre_request(struct mmc_host *mmc, struct mmc_request *mrq,
> >> + bool is_first_req)
> >> +{
> >> + struct dw_mci_slot *slot = mmc_priv(mmc);
> >> + struct mmc_data *data = mrq->data;
> >> +
> >> + if (!data)
> >> + return;
> >> +
> >> + BUG_ON(mrq->data->host_cookie);
> >> +
> >> + if (slot->host->use_dma) {
> >> + if (dw_mci_pre_dma_transfer(slot->host, mrq->data,
> >> + &slot->host->next_data))
> >> + mrq->data->host_cookie = 0;
> > ... while it steps back to old blocking way by setting
> > data->host_cookie 0 when dw_mci_pre_dma_transfer returns non-zero.
> >
> > Per my understanding, it means the non-blocking optimization will
> > always get bypassed anyway, so I doubt the patch can really gain
> > performance improvement. Did you get the chance to measure?
> >
> Actually, i didn't get performance improvement, but didn't fully affect by CPU_FREQ.
> Somebody get performance improvement?
>
Though the performance improvement is limited, I did see nearly 4%
improvement with best case on mxs-mmc.
Try the following change in dw_mci_pre_request to see if you can see
any performance difference.
if (slot->host->use_dma) {
if (dw_mci_pre_dma_transfer(slot->host, mrq->data,
&slot->host->next_data) < 0)
mrq->data->host_cookie = 0;
--
Regards,
Shawn
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] dw_mmc: add support for pre_req and post_req
2011-04-18 5:10 ` Shawn Guo
@ 2011-04-18 5:10 ` Jaehoon Chung
0 siblings, 0 replies; 5+ messages in thread
From: Jaehoon Chung @ 2011-04-18 5:10 UTC (permalink / raw)
To: Shawn Guo
Cc: Jaehoon Chung, linux-mmc@vger.kernel.org, Chris Ball, Per Forlin,
will.newton, Kyungmin Park
Hi Shawn,
Shawn Guo wrote:
>
> if (slot->host->use_dma) {
> if (dw_mci_pre_dma_transfer(slot->host, mrq->data,
> &slot->host->next_data) < 0)
> mrq->data->host_cookie = 0;
>
Ok..I understood yours..:) i'll check and test..
Thanks for your comment.
Regards,
Jaehoon Chung
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2011-04-18 5:13 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-04-07 8:04 [PATCH] dw_mmc: add support for pre_req and post_req Jaehoon Chung
2011-04-16 16:16 ` Shawn Guo
2011-04-18 4:55 ` Jaehoon Chung
2011-04-18 5:10 ` Shawn Guo
2011-04-18 5:10 ` Jaehoon Chung
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).