On Tue, Sep 22, 2009 at 10:20 PM, Pandita, Vikram <vikram.pandita@xxxxxx> wrote: > > >>-----Original Message----- >>From: linux-omap-owner@xxxxxxxxxxxxxxx [mailto:linux-omap-owner@xxxxxxxxxxxxxxx] On Behalf Of hari n >>Sent: Tuesday, September 22, 2009 9:38 PM >>To: linux-omap@xxxxxxxxxxxxxxx >>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 majordomo@xxxxxxxxxxxxxxx >>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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
