On Tue, Sep 22, 2009 at 10:20 PM, Pandita, Vikram <vikram.pand...@ti.com> wrote:
>
>
>>-----Original Message-----
>>From: linux-omap-ow...@vger.kernel.org
>>[mailto:linux-omap-ow...@vger.kernel.org] On Behalf Of hari n
>>Sent: Tuesday, September 22, 2009 9:38 PM
>>To: linux-omap@vger.kernel.org
>>Subject: [OMAP3] ALSA driver 'suspend/resume' handlers
>>
>>Hi,
>>
>>It appears OMAP ALSA driver does not seem to disable and idle the SDMA
>>channel on a 'suspend' call. The problem seems to be with the
>>'omap_stop_dma()' call under 'SNDRV_PCM_TRIGGER_SUSPEND' in
>>'omap_pcm_trigger()'. ALSA audio driver uses self linking and the
>>function 'omap_stop_dma()', only unlinks the channels, but DOES NOT
>>disable an active channel for linked channels.
>>
>> memset(dma_chan_link_map, 0, sizeof(dma_chan_link_map));
>> do {
>> /* The loop case: we've been here already */
>> if (dma_chan_link_map[cur_lch])
>> break;
>> /* Mark the current channel */
>> dma_chan_link_map[cur_lch] = 1;
>>
>> disable_lnk(cur_lch);
>
> What if we introduce this code here?
> /* Stop the Channel transmission */
> l = dma_read(CCR(cur_lch));
> l &= ~(1 << OMAP_DMA_CCR_EN);
> dma_write(l, CCR(cur_lch));
>
This may result in disabling an active DMA channel.. Which can lead to
un-drained FIFOs, if we do not wait to check for 'RD_ACTIVE',
'WR_ACTIVE' bits (refer to 'Disabling a channel during transfer' and
'FIFO draining mechanism'. In addition, as i mentioned below we may
also lose data..
>>
>> next_lch = dma_chan[cur_lch].next_lch;
>> cur_lch = next_lch;
>> } while (next_lch != -1);
>>
>> return; <---
>> }
>>
>>This means after a return from the 'omap_stop_dma()', there can still
>>be an active DMA transaction on the channel. Is this the intent Or a
>>bug? I can think of situations, where in, we may want to complete the
>>DMA transfer and then disable the channel. In such a case, we need to
>>wait until the channel is inactive.
>>The problem with the current implementation is that after the audio
>>platform trigger (suspend) is called, SOC core calls the CPU DAI
>>trigger (suspend) and this stops the McBSP. Depending on the current
>>DMA pointer, this may leave the DMA channel in active state, since DMA
>>is configured for DEST HW Synchronization and and the this never gets
>>asserted after the McBSP is stopped..
>
> If you check the chaining DMA api: omap_stop_dma_chain_transfers() has this
> kind of code:
>
> /* Stop the Channel transmission */
> l = dma_read(CCR(channels[i]));
> l &= ~(1 << 7);
> dma_write(l, CCR(channels[i]));
>
> Should audio be using chaining api's?
> Not sure how linking and chaining are related though.
> Is self-linking a case of chaining with two same elements?
>
Here again, we have the same issue of disabling the channels, while
some data may be in the FIFO/flight..
>
>>
>>I see two ways to resolve this issue:
>>a) Wait after the 'omap_stop_dma()' in audio platform trigger(suspend)
>>until the DMA channel is inactive (i.e buffer transfer complete). But,
>>i believe 'trigger' is supposed to be an atomic operation, so we may
>>need to wait in a worker thread. And we need to flag the McBSP stop
>>based on DMA transfer completion.
>>b) Explicitly disable the DMA channel in 'omap_stop_dma().
>>
>>Option 'b' above may mean losing some data on resume, if we hit 'OFF'
>>state during suspend. This may not be a big deal for audio (loss of
>>few secs audio), but for other drivers (ex: USB data file transfer?),
>>this may not be acceptable. 'Option 'a' means more rework in audio
>>driver, but no changes in DAM driver and no loss of data.
>>Current Audio driver also does not seem to support 'OFF' mode during
>>suspend. It seems to assume that all DMA and McBSP configurations are
>>retained in a suspend to resume transition.
>>
>>Thanks
>>--
>>To unsubscribe from this list: send the line "unsubscribe linux-omap" in
>>the body of a message to majord...@vger.kernel.org
>>More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
