diff mbox

[1/3] dmaengine: pl330: Set residue in tx_status callback.

Message ID 1378879685-5352-2-git-send-email-padma.v@samsung.com (mailing list archive)
State New, archived
Headers show

Commit Message

Padmavathi Venna Sept. 11, 2013, 6:08 a.m. UTC
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(-)

Comments

Chanho Park Sept. 12, 2013, 11:40 a.m. UTC | #1
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
padma venkat Sept. 13, 2013, 3:03 a.m. UTC | #2
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
Vinod Koul Sept. 17, 2013, 7:01 a.m. UTC | #3
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
Dylan Reid Oct. 2, 2013, 4:33 a.m. UTC | #4
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
>
Chanho Park Oct. 7, 2013, 1:39 a.m. UTC | #5
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
> >
Vinod Koul Oct. 7, 2013, 3:48 a.m. UTC | #6
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 :)
Dylan Reid Oct. 9, 2013, 8:37 p.m. UTC | #7
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
Vinod Koul Oct. 10, 2013, 3:58 p.m. UTC | #8
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 mbox

Patch

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)