All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Ujfalusi <peter.ujfalusi@ti.com>
To: Santosh Shilimkar <santosh.shilimkar@ti.com>
Cc: Russell King - ARM Linux <linux@arm.linux.org.uk>,
	Vinod Koul <vinod.koul@intel.com>, Dan Williams <djbw@fb.com>,
	Tony Lindgren <tony@atomide.com>,
	Jarkko Nikula <jarkko.nikula@bitmer.com>,
	Felipe Balbi <balbi@ti.com>,
	linux-kernel@vger.kernel.org, linux-omap@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	Peter Meerwald <pmeerw@pmeerw.net>
Subject: Re: [RFC v2] dmaengine: omap-dma: Start DMA without delay for cyclic channels
Date: Wed, 3 Apr 2013 13:52:25 +0200	[thread overview]
Message-ID: <515C17F9.4040300@ti.com> (raw)
In-Reply-To: <515C117A.6060700@ti.com>

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

WARNING: multiple messages have this Message-ID (diff)
From: peter.ujfalusi@ti.com (Peter Ujfalusi)
To: linux-arm-kernel@lists.infradead.org
Subject: [RFC v2] dmaengine: omap-dma: Start DMA without delay for cyclic channels
Date: Wed, 3 Apr 2013 13:52:25 +0200	[thread overview]
Message-ID: <515C17F9.4040300@ti.com> (raw)
In-Reply-To: <515C117A.6060700@ti.com>

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

WARNING: multiple messages have this Message-ID (diff)
From: Peter Ujfalusi <peter.ujfalusi@ti.com>
To: Santosh Shilimkar <santosh.shilimkar@ti.com>
Cc: Russell King - ARM Linux <linux@arm.linux.org.uk>,
	Vinod Koul <vinod.koul@intel.com>, Dan Williams <djbw@fb.com>,
	Tony Lindgren <tony@atomide.com>,
	Jarkko Nikula <jarkko.nikula@bitmer.com>,
	Felipe Balbi <balbi@ti.com>, <linux-kernel@vger.kernel.org>,
	<linux-omap@vger.kernel.org>,
	<linux-arm-kernel@lists.infradead.org>,
	Peter Meerwald <pmeerw@pmeerw.net>
Subject: Re: [RFC v2] dmaengine: omap-dma: Start DMA without delay for cyclic channels
Date: Wed, 3 Apr 2013 13:52:25 +0200	[thread overview]
Message-ID: <515C17F9.4040300@ti.com> (raw)
In-Reply-To: <515C117A.6060700@ti.com>

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

  reply	other threads:[~2013-04-03 11:52 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-04-03 11:17 [RFC v2] dmaengine: omap-dma: Start DMA without delay for cyclic channels Peter Ujfalusi
2013-04-03 11:17 ` Peter Ujfalusi
2013-04-03 11:17 ` Peter Ujfalusi
2013-04-03 11:24 ` Santosh Shilimkar
2013-04-03 11:24   ` Santosh Shilimkar
2013-04-03 11:24   ` Santosh Shilimkar
2013-04-03 11:52   ` Peter Ujfalusi [this message]
2013-04-03 11:52     ` Peter Ujfalusi
2013-04-03 11:52     ` Peter Ujfalusi
2013-04-03 12:00     ` Santosh Shilimkar
2013-04-03 12:00       ` Santosh Shilimkar
2013-04-03 12:00       ` Santosh Shilimkar
2013-04-08  7:11 ` Peter Ujfalusi
2013-04-08  7:11   ` Peter Ujfalusi
2013-04-08  7:11   ` Peter Ujfalusi
2013-04-08 17:09   ` Russell King - ARM Linux
2013-04-08 17:09     ` Russell King - ARM Linux
2013-04-08 17:15     ` Tony Lindgren
2013-04-08 17:15       ` Tony Lindgren
2013-04-09  6:52       ` Santosh Shilimkar
2013-04-09  6:52         ` Santosh Shilimkar
2013-04-09  6:52         ` Santosh Shilimkar
2013-04-09  7:19         ` Peter Ujfalusi
2013-04-09  7:19           ` Peter Ujfalusi
2013-04-09  7:19           ` Peter Ujfalusi
2013-04-09  7:26           ` Vinod Koul
2013-04-09  7:26             ` Vinod Koul
2013-04-09 10:20             ` Peter Ujfalusi
2013-04-09 10:20               ` Peter Ujfalusi
2013-04-09 10:20               ` Peter Ujfalusi
2013-04-09  7:19     ` Peter Ujfalusi
2013-04-09  7:19       ` Peter Ujfalusi
2013-04-09  7:19       ` Peter Ujfalusi
2013-04-09  7:21       ` Peter Ujfalusi
2013-04-09  7:21         ` Peter Ujfalusi
2013-04-09  7:21         ` Peter Ujfalusi
2013-04-09 13:54         ` Russell King - ARM Linux
2013-04-09 13:54           ` Russell King - ARM Linux

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=515C17F9.4040300@ti.com \
    --to=peter.ujfalusi@ti.com \
    --cc=balbi@ti.com \
    --cc=djbw@fb.com \
    --cc=jarkko.nikula@bitmer.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=linux@arm.linux.org.uk \
    --cc=pmeerw@pmeerw.net \
    --cc=santosh.shilimkar@ti.com \
    --cc=tony@atomide.com \
    --cc=vinod.koul@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.