[RFC v2] dmaengine: omap-dma: Start DMA without delay for cyclic channels

[RFC v2] dmaengine: omap-dma: Start DMA without delay for cyclic channels

[Peter Ujfalusi]

cyclic DMA is only used by audio which needs DMA to be started without a
delay.
If the DMA for audio is started using the tasklet we experience random
channel switch (to be more precise: channel shift).

Reported-by: Peter Meerwald <pmeerw@pmeerw.net>
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
Hi Russell,

Instead of removing the tasklet we can identify the DMA channel used by audio
based on the cyclic flag of the channel.
I think this can be used as a short term solution to fix the audio channel shift
issue and later when we have the dynamic DMA channel allocation we can adjust
the code.

Regards,
Peter

 drivers/dma/omap-dma.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/drivers/dma/omap-dma.c b/drivers/dma/omap-dma.c
index 2ea3d7e..ec3fc4f 100644
--- a/drivers/dma/omap-dma.c
+++ b/drivers/dma/omap-dma.c
@@ -282,12 +282,20 @@ static void omap_dma_issue_pending(struct dma_chan *chan)
 
 	spin_lock_irqsave(&c->vc.lock, flags);
 	if (vchan_issue_pending(&c->vc) && !c->desc) {
-		struct omap_dmadev *d = to_omap_dma_dev(chan->device);
-		spin_lock(&d->lock);
-		if (list_empty(&c->node))
-			list_add_tail(&c->node, &d->pending);
-		spin_unlock(&d->lock);
-		tasklet_schedule(&d->task);
+		/*
+		 * c->cyclic is used only by audio and in this case the DMA need
+		 * to be started without delay.
+		 */
+		if (!c->cyclic) {
+			struct omap_dmadev *d = to_omap_dma_dev(chan->device);
+			spin_lock(&d->lock);
+			if (list_empty(&c->node))
+				list_add_tail(&c->node, &d->pending);
+			spin_unlock(&d->lock);
+			tasklet_schedule(&d->task);
+		} else {
+			omap_dma_start_desc(c);
+		}
 	}
 	spin_unlock_irqrestore(&c->vc.lock, flags);
 }
-- 
1.8.1.5

Re: [RFC v2] dmaengine: omap-dma: Start DMA without delay for cyclic channels

[Santosh Shilimkar]

On Wednesday 03 April 2013 04:47 PM, Peter Ujfalusi wrote:
> cyclic DMA is only used by audio which needs DMA to be started without a
> delay.
> If the DMA for audio is started using the tasklet we experience random
> channel switch (to be more precise: channel shift).
> 
> Reported-by: Peter Meerwald <pmeerw@pmeerw.net>
> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> ---
> Hi Russell,
> 
> Instead of removing the tasklet we can identify the DMA channel used by audio
> based on the cyclic flag of the channel.
> I think this can be used as a short term solution to fix the audio channel shift
> issue and later when we have the dynamic DMA channel allocation we can adjust
> the code.
> 
> Regards,
> Peter
> 
>  drivers/dma/omap-dma.c | 20 ++++++++++++++------
>  1 file changed, 14 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/dma/omap-dma.c b/drivers/dma/omap-dma.c
> index 2ea3d7e..ec3fc4f 100644
> --- a/drivers/dma/omap-dma.c
> +++ b/drivers/dma/omap-dma.c
> @@ -282,12 +282,20 @@ static void omap_dma_issue_pending(struct dma_chan *chan)
>  
>  	spin_lock_irqsave(&c->vc.lock, flags);
>  	if (vchan_issue_pending(&c->vc) && !c->desc) {
If you add "!c->cyclic" in above if then you can avoid
indentation change and just have else for cyclic case.

> -		struct omap_dmadev *d = to_omap_dma_dev(chan->device);
> -		spin_lock(&d->lock);
> -		if (list_empty(&c->node))
> -			list_add_tail(&c->node, &d->pending);
> -		spin_unlock(&d->lock);
> -		tasklet_schedule(&d->task);
> +		/*
> +		 * c->cyclic is used only by audio and in this case the DMA need
> +		 * to be started without delay.
> +		 */
> +		if (!c->cyclic) {
> +			struct omap_dmadev *d = to_omap_dma_dev(chan->device);
> +			spin_lock(&d->lock);
> +			if (list_empty(&c->node))
> +				list_add_tail(&c->node, &d->pending);
> +			spin_unlock(&d->lock);
> +			tasklet_schedule(&d->task);
> +		} else {
> +			omap_dma_start_desc(c);
> +		}
>  	}
>  	spin_unlock_irqrestore(&c->vc.lock, flags);
>  }
> 

Re: [RFC v2] dmaengine: omap-dma: Start DMA without delay for cyclic channels

[Peter Ujfalusi]

On 04/03/2013 01:24 PM, Santosh Shilimkar wrote:
> On Wednesday 03 April 2013 04:47 PM, Peter Ujfalusi wrote:
>> cyclic DMA is only used by audio which needs DMA to be started without a
>> delay.
>> If the DMA for audio is started using the tasklet we experience random
>> channel switch (to be more precise: channel shift).
>>
>> Reported-by: Peter Meerwald <pmeerw@pmeerw.net>
>> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
>> ---
>> Hi Russell,
>>
>> Instead of removing the tasklet we can identify the DMA channel used by audio
>> based on the cyclic flag of the channel.
>> I think this can be used as a short term solution to fix the audio channel shift
>> issue and later when we have the dynamic DMA channel allocation we can adjust
>> the code.
>>
>> Regards,
>> Peter
>>
>>  drivers/dma/omap-dma.c | 20 ++++++++++++++------
>>  1 file changed, 14 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/dma/omap-dma.c b/drivers/dma/omap-dma.c
>> index 2ea3d7e..ec3fc4f 100644
>> --- a/drivers/dma/omap-dma.c
>> +++ b/drivers/dma/omap-dma.c
>> @@ -282,12 +282,20 @@ static void omap_dma_issue_pending(struct dma_chan *chan)
>>  
>>  	spin_lock_irqsave(&c->vc.lock, flags);
>>  	if (vchan_issue_pending(&c->vc) && !c->desc) {
> If you add "!c->cyclic" in above if then you can avoid
> indentation change and just have else for cyclic case.

It can not be embedded there because of the existing tests. How would we
handle the case when c->desc is _not_ NULL and c->cyclic is false? We would
need to test again in else, but we can not do this for the
vchan_issue_pending(&c->vc).

> 
>> -		struct omap_dmadev *d = to_omap_dma_dev(chan->device);
>> -		spin_lock(&d->lock);
>> -		if (list_empty(&c->node))
>> -			list_add_tail(&c->node, &d->pending);
>> -		spin_unlock(&d->lock);
>> -		tasklet_schedule(&d->task);
>> +		/*
>> +		 * c->cyclic is used only by audio and in this case the DMA need
>> +		 * to be started without delay.
>> +		 */
>> +		if (!c->cyclic) {
>> +			struct omap_dmadev *d = to_omap_dma_dev(chan->device);
>> +			spin_lock(&d->lock);
>> +			if (list_empty(&c->node))
>> +				list_add_tail(&c->node, &d->pending);
>> +			spin_unlock(&d->lock);
>> +			tasklet_schedule(&d->task);
>> +		} else {
>> +			omap_dma_start_desc(c);
>> +		}
>>  	}
>>  	spin_unlock_irqrestore(&c->vc.lock, flags);
>>  }
>>
> 


-- 
Péter

Re: [RFC v2] dmaengine: omap-dma: Start DMA without delay for cyclic channels

[Santosh Shilimkar]

On Wednesday 03 April 2013 05:22 PM, Peter Ujfalusi wrote:
> On 04/03/2013 01:24 PM, Santosh Shilimkar wrote:
>> On Wednesday 03 April 2013 04:47 PM, Peter Ujfalusi wrote:
>>> cyclic DMA is only used by audio which needs DMA to be started without a
>>> delay.
>>> If the DMA for audio is started using the tasklet we experience random
>>> channel switch (to be more precise: channel shift).
>>>
>>> Reported-by: Peter Meerwald <pmeerw@pmeerw.net>
>>> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
>>> ---
>>> Hi Russell,
>>>
>>> Instead of removing the tasklet we can identify the DMA channel used by audio
>>> based on the cyclic flag of the channel.
>>> I think this can be used as a short term solution to fix the audio channel shift
>>> issue and later when we have the dynamic DMA channel allocation we can adjust
>>> the code.
>>>
>>> Regards,
>>> Peter
>>>
>>>  drivers/dma/omap-dma.c | 20 ++++++++++++++------
>>>  1 file changed, 14 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/dma/omap-dma.c b/drivers/dma/omap-dma.c
>>> index 2ea3d7e..ec3fc4f 100644
>>> --- a/drivers/dma/omap-dma.c
>>> +++ b/drivers/dma/omap-dma.c
>>> @@ -282,12 +282,20 @@ static void omap_dma_issue_pending(struct dma_chan *chan)
>>>  
>>>  	spin_lock_irqsave(&c->vc.lock, flags);
>>>  	if (vchan_issue_pending(&c->vc) && !c->desc) {
>> If you add "!c->cyclic" in above if then you can avoid
>> indentation change and just have else for cyclic case.
> 
> It can not be embedded there because of the existing tests. How would we
> handle the case when c->desc is _not_ NULL and c->cyclic is false? We would
> need to test again in else, but we can not do this for the
> vchan_issue_pending(&c->vc).
> 
right. Thanks for clarifying it.

Regards,
Santosh

Re: [RFC v2] dmaengine: omap-dma: Start DMA without delay for cyclic channels

[Peter Ujfalusi]

Russell,

On 04/03/2013 01:17 PM, Peter Ujfalusi wrote:
> cyclic DMA is only used by audio which needs DMA to be started without a
> delay.
> If the DMA for audio is started using the tasklet we experience random
> channel switch (to be more precise: channel shift).
> 
> Reported-by: Peter Meerwald <pmeerw@pmeerw.net>
> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> ---
> Hi Russell,
> 
> Instead of removing the tasklet we can identify the DMA channel used by audio
> based on the cyclic flag of the channel.
> I think this can be used as a short term solution to fix the audio channel shift
> issue and later when we have the dynamic DMA channel allocation we can adjust
> the code.

Could you, please look at this patch?

Thank you,
Péter

> Regards,
> Peter
> 
>  drivers/dma/omap-dma.c | 20 ++++++++++++++------
>  1 file changed, 14 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/dma/omap-dma.c b/drivers/dma/omap-dma.c
> index 2ea3d7e..ec3fc4f 100644
> --- a/drivers/dma/omap-dma.c
> +++ b/drivers/dma/omap-dma.c
> @@ -282,12 +282,20 @@ static void omap_dma_issue_pending(struct dma_chan *chan)
>  
>  	spin_lock_irqsave(&c->vc.lock, flags);
>  	if (vchan_issue_pending(&c->vc) && !c->desc) {
> -		struct omap_dmadev *d = to_omap_dma_dev(chan->device);
> -		spin_lock(&d->lock);
> -		if (list_empty(&c->node))
> -			list_add_tail(&c->node, &d->pending);
> -		spin_unlock(&d->lock);
> -		tasklet_schedule(&d->task);
> +		/*
> +		 * c->cyclic is used only by audio and in this case the DMA need
> +		 * to be started without delay.
> +		 */
> +		if (!c->cyclic) {
> +			struct omap_dmadev *d = to_omap_dma_dev(chan->device);
> +			spin_lock(&d->lock);
> +			if (list_empty(&c->node))
> +				list_add_tail(&c->node, &d->pending);
> +			spin_unlock(&d->lock);
> +			tasklet_schedule(&d->task);
> +		} else {
> +			omap_dma_start_desc(c);
> +		}
>  	}
>  	spin_unlock_irqrestore(&c->vc.lock, flags);
>  }
> 

Re: [RFC v2] dmaengine: omap-dma: Start DMA without delay for cyclic channels

[Russell King - ARM Linux]

On Mon, Apr 08, 2013 at 09:11:04AM +0200, Peter Ujfalusi wrote:
> Russell,
> 
> On 04/03/2013 01:17 PM, Peter Ujfalusi wrote:
> > cyclic DMA is only used by audio which needs DMA to be started without a
> > delay.
> > If the DMA for audio is started using the tasklet we experience random
> > channel switch (to be more precise: channel shift).
> > 
> > Reported-by: Peter Meerwald <pmeerw@pmeerw.net>
> > Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> > ---
> > Hi Russell,
> > 
> > Instead of removing the tasklet we can identify the DMA channel used by audio
> > based on the cyclic flag of the channel.
> > I think this can be used as a short term solution to fix the audio channel shift
> > issue and later when we have the dynamic DMA channel allocation we can adjust
> > the code.
> 
> Could you, please look at this patch?

Now that I'm back from a short 4 day break, then yes, and the answer is
that it's fine.  Who's handling the patch?

Re: [RFC v2] dmaengine: omap-dma: Start DMA without delay for cyclic channels

[Tony Lindgren]

* Russell King - ARM Linux <linux@arm.linux.org.uk> [130408 10:15]:
> On Mon, Apr 08, 2013 at 09:11:04AM +0200, Peter Ujfalusi wrote:
> > Russell,
> > 
> > On 04/03/2013 01:17 PM, Peter Ujfalusi wrote:
> > > cyclic DMA is only used by audio which needs DMA to be started without a
> > > delay.
> > > If the DMA for audio is started using the tasklet we experience random
> > > channel switch (to be more precise: channel shift).
> > > 
> > > Reported-by: Peter Meerwald <pmeerw@pmeerw.net>
> > > Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> > > ---
> > > Hi Russell,
> > > 
> > > Instead of removing the tasklet we can identify the DMA channel used by audio
> > > based on the cyclic flag of the channel.
> > > I think this can be used as a short term solution to fix the audio channel shift
> > > issue and later when we have the dynamic DMA channel allocation we can adjust
> > > the code.
> > 
> > Could you, please look at this patch?
> 
> Now that I'm back from a short 4 day break, then yes, and the answer is
> that it's fine.  Who's handling the patch?

I suggest Peter resend the patch with also Grant + Linus W cc:d so
they can queue it unless there are other related patches pending
somewhere else.

Regards,

Tony

Re: [RFC v2] dmaengine: omap-dma: Start DMA without delay for cyclic channels

[Santosh Shilimkar]

On Monday 08 April 2013 10:45 PM, Tony Lindgren wrote:
> * Russell King - ARM Linux <linux@arm.linux.org.uk> [130408 10:15]:
>> On Mon, Apr 08, 2013 at 09:11:04AM +0200, Peter Ujfalusi wrote:
>>> Russell,
>>>
>>> On 04/03/2013 01:17 PM, Peter Ujfalusi wrote:
>>>> cyclic DMA is only used by audio which needs DMA to be started without a
>>>> delay.
>>>> If the DMA for audio is started using the tasklet we experience random
>>>> channel switch (to be more precise: channel shift).
>>>>
>>>> Reported-by: Peter Meerwald <pmeerw@pmeerw.net>
>>>> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
>>>> ---
>>>> Hi Russell,
>>>>
>>>> Instead of removing the tasklet we can identify the DMA channel used by audio
>>>> based on the cyclic flag of the channel.
>>>> I think this can be used as a short term solution to fix the audio channel shift
>>>> issue and later when we have the dynamic DMA channel allocation we can adjust
>>>> the code.
>>>
>>> Could you, please look at this patch?
>>
>> Now that I'm back from a short 4 day break, then yes, and the answer is
>> that it's fine.  Who's handling the patch?
> 
> I suggest Peter resend the patch with also Grant + Linus W cc:d so
> they can queue it unless there are other related patches pending
> somewhere else.
> 
Am curious on your suggestion. DMA engine patches are going via Vinod
Koul's tree so I think the $subject patch should follow the same
tree, No ?

Peter, if you plan to re-send, feel free to add my ack.


Regards,
Santosh

Re: [RFC v2] dmaengine: omap-dma: Start DMA without delay for cyclic channels

[Peter Ujfalusi]

On 04/09/2013 08:52 AM, Santosh Shilimkar wrote:
>> I suggest Peter resend the patch with also Grant + Linus W cc:d so
>> they can queue it unless there are other related patches pending
>> somewhere else.
>>
> Am curious on your suggestion. DMA engine patches are going via Vinod
> Koul's tree so I think the $subject patch should follow the same
> tree, No ?

AFAIK Vinod should be the correct person to pick it up, but I can CC Grant and
Linus W as well.

> Peter, if you plan to re-send, feel free to add my ack.

I'll figure out which stable release should have this applied and will resend.

-- 
Péter

Re: [RFC v2] dmaengine: omap-dma: Start DMA without delay for cyclic channels

[Peter Ujfalusi]

On 04/08/2013 07:09 PM, Russell King - ARM Linux wrote:
> Now that I'm back from a short 4 day break, then yes, and the answer is
> that it's fine.  Who's handling the patch?

Thank you,
Péter

Re: [RFC v2] dmaengine: omap-dma: Start DMA without delay for cyclic channels

[Peter Ujfalusi]

Russell,

On 04/09/2013 09:19 AM, Peter Ujfalusi wrote:
> On 04/08/2013 07:09 PM, Russell King - ARM Linux wrote:
>> Now that I'm back from a short 4 day break, then yes, and the answer is
>> that it's fine.  Who's handling the patch?
> 
> Thank you,
> Péter

Can I add you acked-by to the patch?

-- 
Péter

Re: [RFC v2] dmaengine: omap-dma: Start DMA without delay for cyclic channels

[Vinod Koul]

On Tue, Apr 09, 2013 at 09:19:31AM +0200, Peter Ujfalusi wrote:
> On 04/09/2013 08:52 AM, Santosh Shilimkar wrote:
> >> I suggest Peter resend the patch with also Grant + Linus W cc:d so
> >> they can queue it unless there are other related patches pending
> >> somewhere else.
> >>
> > Am curious on your suggestion. DMA engine patches are going via Vinod
> > Koul's tree so I think the $subject patch should follow the same
> > tree, No ?
> 
> AFAIK Vinod should be the correct person to pick it up, but I can CC Grant and
> Linus W as well.
Yes it should go thru dmaengine tree, sorry was travelling hence the delay, pls
resend the patch and I will do the needful
> 
> > Peter, if you plan to re-send, feel free to add my ack.
> 
> I'll figure out which stable release should have this applied and will resend.
> 
> -- 
> Péter

Re: [RFC v2] dmaengine: omap-dma: Start DMA without delay for cyclic channels

[Peter Ujfalusi]

On 04/09/2013 09:26 AM, Vinod Koul wrote:
> Yes it should go thru dmaengine tree, sorry was travelling hence the delay, pls
> resend the patch and I will do the needful

I already have the patch rebased on today's linux-next, just waiting for
Russell to confirm that I can add his Acked-by to the patch.
I take this from Russell as ACK: "Now that I'm back from a short 4 day break,
then yes, and the answer is that it's fine.  Who's handling the patch?"

But I want to be sure I can add that line...

-- 
Péter

Re: [RFC v2] dmaengine: omap-dma: Start DMA without delay for cyclic channels

[Russell King - ARM Linux]

On Tue, Apr 09, 2013 at 09:21:40AM +0200, Peter Ujfalusi wrote:
> Russell,
> 
> On 04/09/2013 09:19 AM, Peter Ujfalusi wrote:
> > On 04/08/2013 07:09 PM, Russell King - ARM Linux wrote:
> >> Now that I'm back from a short 4 day break, then yes, and the answer is
> >> that it's fine.  Who's handling the patch?
> > 
> > Thank you,
> > Péter
> 
> Can I add you acked-by to the patch?

You may, using the rmk+kernel address, not this one.  Thanks.