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

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to