Message ID | 1378879685-5352-2-git-send-email-padma.v@samsung.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
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 <dgreid@chromium.org> > > 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 <dgreid@chromium.org> > Reviewed-by: Olof Johansson <olofj@chromium.org> > Signed-off-by: Padmavathi Venna <padma.v@samsung.com> > --- > 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. Thanks, Best Regards, Chanho Park
Hi Chanho, On Thu, Sep 12, 2013 at 5:10 PM, Chanho Park <chanho61.park@samsung.com> 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 <dgreid@chromium.org> >> >> 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 <dgreid@chromium.org> >> Reviewed-by: Olof Johansson <olofj@chromium.org> >> Signed-off-by: Padmavathi Venna <padma.v@samsung.com> >> --- >> 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. Ok. I will check this and modify the code in the next version of patches. Thanks for the suggestion. > > Thanks, > > Best Regards, > Chanho Park > Best Regards Padma
On Wed, Sep 11, 2013 at 11:38:03AM +0530, Padmavathi Venna wrote: > From: Dylan Reid <dgreid@chromium.org> > > 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 <dgreid@chromium.org> > Reviewed-by: Olof Johansson <olofj@chromium.org> > Signed-off-by: Padmavathi Venna <padma.v@samsung.com> > --- > 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. */ if will return DMA_IN_PROGRESS for two cases a) when the descriptor is in queue b) when the descriptor is submitted and running You need to take different paths for these two cases. For former just return residue as complete size of descriptor. For latter you cna read sar/dar and check remaining bytes in sg_list. Hint: use the cookie values to find the state > + dma_set_residue(txstate, pl330_tx_residue(chan)); > + > + return ret; > } > > static void pl330_issue_pending(struct dma_chan *chan) ~Vinod
On Thu, Sep 12, 2013 at 4:40 AM, Chanho Park <chanho61.park@samsung.com> 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 <dgreid@chromium.org> >> >> 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 <dgreid@chromium.org> >> Reviewed-by: Olof Johansson <olofj@chromium.org> >> Signed-off-by: Padmavathi Venna <padma.v@samsung.com> >> --- >> 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? 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 >
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 <chanho61.park@samsung.com> > 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 <dgreid@chromium.org> > >> > >> 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 <dgreid@chromium.org> > >> Reviewed-by: Olof Johansson <olofj@chromium.org> > >> Signed-off-by: Padmavathi Venna <padma.v@samsung.com> > >> --- > >> 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 > >
On Mon, Oct 07, 2013 at 10:39:34AM +0900, Chanho Park wrote: > > > 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. For cyclic case you would have possible same descriptor running till you terminate. For non cyclic, if you have 3 descriptors submitted, the cookie value can be, say 7, 8 and 9. If you query the status of any descriptor and pass the last optional txstate arg then you will know the last cookie completed. so if txstate->last is 7, then 7th was completed and 8 should be running and 9 in queue! > > Looking for the first buffer with status == BUSY is an improvement I'll > > make. Any way to avoid looking through the list? Its already there :)
On Sun, Oct 6, 2013 at 8:48 PM, Vinod Koul <vinod.koul@intel.com> wrote: > On Mon, Oct 07, 2013 at 10:39:34AM +0900, Chanho Park wrote: >> > > 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. > For cyclic case you would have possible same descriptor running till you > terminate. The cyclic case that makes this interesting is when there are multiple cyclic descriptors in the list. The cookie and completed_cookie markers don't help to determine which of the descriptors in the list is currently active. dma_cookie_complete isn't called for a cyclic desc, the desc is just pushed to the end of the list. > > For non cyclic, if you have 3 descriptors submitted, the cookie value can be, say > 7, 8 and 9. If you query the status of any descriptor and pass the last optional > txstate arg then you will know the last cookie completed. so if txstate->last is > 7, then 7th was completed and 8 should be running and 9 in queue! Got it, but the correct desc for cookie 8 will still have to be searched for, correct? Thanks for the advise. Dylan >> > Looking for the first buffer with status == BUSY is an improvement I'll >> > make. Any way to avoid looking through the list? > Its already there :) > > -- > ~Vinod
On Wed, Oct 09, 2013 at 01:37:56PM -0700, Dylan Reid wrote: > On Sun, Oct 6, 2013 at 8:48 PM, Vinod Koul <vinod.koul@intel.com> wrote: > > On Mon, Oct 07, 2013 at 10:39:34AM +0900, Chanho Park wrote: > >> > > 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. > > For cyclic case you would have possible same descriptor running till you > > terminate. > > The cyclic case that makes this interesting is when there are multiple > cyclic descriptors in the list. The cookie and completed_cookie > markers don't help to determine which of the descriptors in the list > is currently active. dma_cookie_complete isn't called for a cyclic > desc, the desc is just pushed to the end of the list. Yes the cyclic is a very different case. I think driver can still return for cyclic case which was txstate->last and that will give clue to client which is getting processed now! > > For non cyclic, if you have 3 descriptors submitted, the cookie value can be, say > > 7, 8 and 9. If you query the status of any descriptor and pass the last optional > > txstate arg then you will know the last cookie completed. so if txstate->last is > > 7, then 7th was completed and 8 should be running and 9 in queue! > > Got it, but the correct desc for cookie 8 will still have to be > searched for, correct? Yes
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; } static void pl330_issue_pending(struct dma_chan *chan)