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 <[email protected]>
Signed-off-by: Peter Ujfalusi <[email protected]>
---
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
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 <[email protected]>
> Signed-off-by: Peter Ujfalusi <[email protected]>
> ---
> 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);
> }
>
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 <[email protected]>
>> Signed-off-by: Peter Ujfalusi <[email protected]>
>> ---
>> 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
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 <[email protected]>
>>> Signed-off-by: Peter Ujfalusi <[email protected]>
>>> ---
>>> 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
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 <[email protected]>
> Signed-off-by: Peter Ujfalusi <[email protected]>
> ---
> 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);
> }
>
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 <[email protected]>
> > Signed-off-by: Peter Ujfalusi <[email protected]>
> > ---
> > 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?
* Russell King - ARM Linux <[email protected]> [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 <[email protected]>
> > > Signed-off-by: Peter Ujfalusi <[email protected]>
> > > ---
> > > 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
On Monday 08 April 2013 10:45 PM, Tony Lindgren wrote:
> * Russell King - ARM Linux <[email protected]> [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 <[email protected]>
>>>> Signed-off-by: Peter Ujfalusi <[email protected]>
>>>> ---
>>>> 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
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
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
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
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
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
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.