From mboxrd@z Thu Jan 1 00:00:00 1970 From: Chanho Park Subject: RE: [PATCH 1/3] dmaengine: pl330: Set residue in tx_status callback. Date: Mon, 07 Oct 2013 10:39:34 +0900 Message-ID: <026501cec2fe$1387c630$3a975290$@samsung.com> References: <1378879685-5352-1-git-send-email-padma.v@samsung.com> <1378879685-5352-2-git-send-email-padma.v@samsung.com> <00b401ceafac$e3a88170$aaf98450$@samsung.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Return-path: Received: from mailout3.samsung.com ([203.254.224.33]:10321 "EHLO mailout3.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751769Ab3JGBjc (ORCPT ); Sun, 6 Oct 2013 21:39:32 -0400 Received: from epcpsbgr5.samsung.com (u145.gpu120.samsung.co.kr [203.254.230.145]) by mailout3.samsung.com (Oracle Communications Messaging Server 7u4-24.01 (7.0.4.24.0) 64bit (built Nov 17 2011)) with ESMTP id <0MU9002GOZ9N2890@mailout3.samsung.com> for linux-samsung-soc@vger.kernel.org; Mon, 07 Oct 2013 10:39:30 +0900 (KST) In-reply-to: Content-language: ko Sender: linux-samsung-soc-owner@vger.kernel.org List-Id: linux-samsung-soc@vger.kernel.org To: 'Dylan Reid' Cc: 'Padmavathi Venna' , linux-samsung-soc@vger.kernel.org, linux-arm-kernel@lists.infradead.org, padma.kvr@gmail.com, kgene.kim@samsung.com, arnd@arndb.de, 'Sangbeom Kim' , vinod.koul@intel.com, 'Mark Brown' , 'Olof Johansson' Hi Dylan, > -----Original Message----- > From: dgreid@google.com [mailto:dgreid@google.com] On Behalf Of Dylan > Reid > Sent: Wednesday, October 02, 2013 1:34 PM > To: Chanho Park > Cc: Padmavathi Venna; linux-samsung-soc@vger.kernel.org; linux-arm- > kernel@lists.infradead.org; padma.kvr@gmail.com; kgene.kim@samsung.com; > arnd@arndb.de; Sangbeom Kim; vinod.koul@intel.com; Mark Brown; Olof > Johansson > Subject: Re: [PATCH 1/3] dmaengine: pl330: Set residue in tx_status > callback. > > On Thu, Sep 12, 2013 at 4:40 AM, Chanho Park > wrote: > > Hi Padmavathi, > > > >> -----Original Message----- > >> From: linux-arm-kernel [mailto:linux-arm-kernel- > >> bounces@lists.infradead.org] On Behalf Of Padmavathi Venna > >> Sent: Wednesday, September 11, 2013 3:08 PM > >> To: linux-samsung-soc@vger.kernel.org; linux-arm- > >> kernel@lists.infradead.org; padma.v@samsung.com; padma.kvr@gmail.com > >> Cc: kgene.kim@samsung.com; arnd@arndb.de; sbkim73@samsung.com; > >> vinod.koul@intel.com; broonie@kernel.org; dgreid@chromium.org; > >> olofj@chromium.org > >> Subject: [PATCH 1/3] dmaengine: pl330: Set residue in tx_status > callback. > >> > >> From: Dylan Reid > >> > >> Fill txstate.residue with the amount of bytes remaining in the > >> current transfer if the transfer is not complete. This will be of > >> particular use to i2s DMA transfers, providing more accurate hw_ptr > values to ASoC. > >> > >> Signed-off-by: Dylan Reid > >> Reviewed-by: Olof Johansson > >> Signed-off-by: Padmavathi Venna > >> --- > >> drivers/dma/pl330.c | 55 > >> ++++++++++++++++++++++++++++++++++++++++++++++++++- > >> 1 files changed, 54 insertions(+), 1 deletions(-) > >> > >> diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c index > >> 593827b..7ab9136 100644 > >> --- a/drivers/dma/pl330.c > >> +++ b/drivers/dma/pl330.c > >> @@ -2476,11 +2476,64 @@ static void pl330_free_chan_resources(struct > >> dma_chan *chan) > >> spin_unlock_irqrestore(&pch->lock, flags); } > >> > >> +static inline int > >> +pl330_src_addr_in_desc(struct dma_pl330_desc *desc, unsigned int sar) > { > >> + return ((desc->px.src_addr <= sar) && > >> + (sar <= (desc->px.src_addr + desc->px.bytes))); } > >> + > >> +static inline int > >> +pl330_dst_addr_in_desc(struct dma_pl330_desc *desc, unsigned int dar) > { > >> + return ((desc->px.dst_addr <= dar) && > >> + (dar <= (desc->px.dst_addr + desc->px.bytes))); } > >> + > >> +static unsigned int pl330_tx_residue(struct dma_chan *chan) { > >> + struct dma_pl330_chan *pch = to_pchan(chan); > >> + void __iomem *regs = pch->dmac->pif.base; > >> + struct pl330_thread *thrd = pch->pl330_chid; > >> + struct dma_pl330_desc *desc; > >> + unsigned int sar, dar; > >> + unsigned int residue = 0; > >> + unsigned long flags; > >> + > >> + sar = readl(regs + SA(thrd->id)); > >> + dar = readl(regs + DA(thrd->id)); > >> + > >> + spin_lock_irqsave(&pch->lock, flags); > >> + > >> + /* Find the desc related to the current buffer. */ > >> + list_for_each_entry(desc, &pch->work_list, node) { > >> + if (desc->rqcfg.src_inc && pl330_src_addr_in_desc(desc, > >> sar)) { > >> + residue = desc->px.bytes - (sar - > > desc->px.src_addr); > >> + goto found_unlock; > >> + } > >> + if (desc->rqcfg.dst_inc && pl330_dst_addr_in_desc(desc, > >> dar)) { > >> + residue = desc->px.bytes - (dar - > > desc->px.dst_addr); > >> + goto found_unlock; > >> + } > >> + } > >> + > >> +found_unlock: > >> + spin_unlock_irqrestore(&pch->lock, flags); > >> + > >> + return residue; > >> +} > >> + > >> static enum dma_status > >> pl330_tx_status(struct dma_chan *chan, dma_cookie_t cookie, > >> struct dma_tx_state *txstate) { > >> - return dma_cookie_status(chan, cookie, txstate); > >> + enum dma_status ret; > >> + > >> + ret = dma_cookie_status(chan, cookie, txstate); > >> + if (ret != DMA_SUCCESS) /* Not complete, check amount left. */ > >> + dma_set_residue(txstate, pl330_tx_residue(chan)); > >> + > >> + return ret; > > > > Why didn't you use a cookie value to track the request? > > The cookie is assigned when each transfer is submitted. > > If you save the value in the desc, we can find the request easily. > > If there are several cyclic desc in the work list, is there a better way > to find the "current" one? The chan struct tracks the last completed and > last submitted cookies, but these will be the first and last > respectively as long as the cyclic transfer is active. Is there an > "active" cookie stored somewhere that I missed? Assume there are three cookies. If you want to get the second cookie not latest cookie, your way can be also correct in such case? I think tx_status API is to get dma status of the given cookie. You are only considering a cyclic case. > > Looking for the first buffer with status == BUSY is an improvement I'll > make. Any way to avoid looking through the list? > > Thanks, > > Dylan > > > > > Thanks, > > > > Best Regards, > > Chanho Park > > From mboxrd@z Thu Jan 1 00:00:00 1970 From: chanho61.park@samsung.com (Chanho Park) Date: Mon, 07 Oct 2013 10:39:34 +0900 Subject: [PATCH 1/3] dmaengine: pl330: Set residue in tx_status callback. In-Reply-To: References: <1378879685-5352-1-git-send-email-padma.v@samsung.com> <1378879685-5352-2-git-send-email-padma.v@samsung.com> <00b401ceafac$e3a88170$aaf98450$@samsung.com> Message-ID: <026501cec2fe$1387c630$3a975290$@samsung.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi Dylan, > -----Original Message----- > From: dgreid at google.com [mailto:dgreid at google.com] On Behalf Of Dylan > Reid > Sent: Wednesday, October 02, 2013 1:34 PM > To: Chanho Park > Cc: Padmavathi Venna; linux-samsung-soc at vger.kernel.org; linux-arm- > kernel at lists.infradead.org; padma.kvr at gmail.com; kgene.kim at samsung.com; > arnd at arndb.de; Sangbeom Kim; vinod.koul at intel.com; Mark Brown; Olof > Johansson > Subject: Re: [PATCH 1/3] dmaengine: pl330: Set residue in tx_status > callback. > > On Thu, Sep 12, 2013 at 4:40 AM, Chanho Park > wrote: > > Hi Padmavathi, > > > >> -----Original Message----- > >> From: linux-arm-kernel [mailto:linux-arm-kernel- > >> bounces at lists.infradead.org] On Behalf Of Padmavathi Venna > >> Sent: Wednesday, September 11, 2013 3:08 PM > >> To: linux-samsung-soc at vger.kernel.org; linux-arm- > >> kernel at lists.infradead.org; padma.v at samsung.com; padma.kvr at gmail.com > >> Cc: kgene.kim at samsung.com; arnd at arndb.de; sbkim73 at samsung.com; > >> vinod.koul at intel.com; broonie at kernel.org; dgreid at chromium.org; > >> olofj at chromium.org > >> Subject: [PATCH 1/3] dmaengine: pl330: Set residue in tx_status > callback. > >> > >> From: Dylan Reid > >> > >> Fill txstate.residue with the amount of bytes remaining in the > >> current transfer if the transfer is not complete. This will be of > >> particular use to i2s DMA transfers, providing more accurate hw_ptr > values to ASoC. > >> > >> Signed-off-by: Dylan Reid > >> Reviewed-by: Olof Johansson > >> Signed-off-by: Padmavathi Venna > >> --- > >> drivers/dma/pl330.c | 55 > >> ++++++++++++++++++++++++++++++++++++++++++++++++++- > >> 1 files changed, 54 insertions(+), 1 deletions(-) > >> > >> diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c index > >> 593827b..7ab9136 100644 > >> --- a/drivers/dma/pl330.c > >> +++ b/drivers/dma/pl330.c > >> @@ -2476,11 +2476,64 @@ static void pl330_free_chan_resources(struct > >> dma_chan *chan) > >> spin_unlock_irqrestore(&pch->lock, flags); } > >> > >> +static inline int > >> +pl330_src_addr_in_desc(struct dma_pl330_desc *desc, unsigned int sar) > { > >> + return ((desc->px.src_addr <= sar) && > >> + (sar <= (desc->px.src_addr + desc->px.bytes))); } > >> + > >> +static inline int > >> +pl330_dst_addr_in_desc(struct dma_pl330_desc *desc, unsigned int dar) > { > >> + return ((desc->px.dst_addr <= dar) && > >> + (dar <= (desc->px.dst_addr + desc->px.bytes))); } > >> + > >> +static unsigned int pl330_tx_residue(struct dma_chan *chan) { > >> + struct dma_pl330_chan *pch = to_pchan(chan); > >> + void __iomem *regs = pch->dmac->pif.base; > >> + struct pl330_thread *thrd = pch->pl330_chid; > >> + struct dma_pl330_desc *desc; > >> + unsigned int sar, dar; > >> + unsigned int residue = 0; > >> + unsigned long flags; > >> + > >> + sar = readl(regs + SA(thrd->id)); > >> + dar = readl(regs + DA(thrd->id)); > >> + > >> + spin_lock_irqsave(&pch->lock, flags); > >> + > >> + /* Find the desc related to the current buffer. */ > >> + list_for_each_entry(desc, &pch->work_list, node) { > >> + if (desc->rqcfg.src_inc && pl330_src_addr_in_desc(desc, > >> sar)) { > >> + residue = desc->px.bytes - (sar - > > desc->px.src_addr); > >> + goto found_unlock; > >> + } > >> + if (desc->rqcfg.dst_inc && pl330_dst_addr_in_desc(desc, > >> dar)) { > >> + residue = desc->px.bytes - (dar - > > desc->px.dst_addr); > >> + goto found_unlock; > >> + } > >> + } > >> + > >> +found_unlock: > >> + spin_unlock_irqrestore(&pch->lock, flags); > >> + > >> + return residue; > >> +} > >> + > >> static enum dma_status > >> pl330_tx_status(struct dma_chan *chan, dma_cookie_t cookie, > >> struct dma_tx_state *txstate) { > >> - return dma_cookie_status(chan, cookie, txstate); > >> + enum dma_status ret; > >> + > >> + ret = dma_cookie_status(chan, cookie, txstate); > >> + if (ret != DMA_SUCCESS) /* Not complete, check amount left. */ > >> + dma_set_residue(txstate, pl330_tx_residue(chan)); > >> + > >> + return ret; > > > > Why didn't you use a cookie value to track the request? > > The cookie is assigned when each transfer is submitted. > > If you save the value in the desc, we can find the request easily. > > If there are several cyclic desc in the work list, is there a better way > to find the "current" one? The chan struct tracks the last completed and > last submitted cookies, but these will be the first and last > respectively as long as the cyclic transfer is active. Is there an > "active" cookie stored somewhere that I missed? Assume there are three cookies. If you want to get the second cookie not latest cookie, your way can be also correct in such case? I think tx_status API is to get dma status of the given cookie. You are only considering a cyclic case. > > Looking for the first buffer with status == BUSY is an improvement I'll > make. Any way to avoid looking through the list? > > Thanks, > > Dylan > > > > > Thanks, > > > > Best Regards, > > Chanho Park > >