Re: [PATCH] ASoC: omap-mcbsp: Add PM QoS support for McBSP to prevent glitches

[Tony Lindgren <tony@atomide.com>]

* Peter Ujfalusi <peter.ujfalusi@ti.com> [160908 03:49]:
> > 
> > I managed to get time for this and it looks like that we only need to block
> > core OFF for audio. I don't know why C6 ("MPU OFF + CORE RET") with it's 3000
> > + 8500 exit latency not causing issues when we have less time from the DMA
> > request to empty FIFO...

Right, the wakeup event should be the fifo threshold and the dma trigger
line.. But there must some other thing we're not accounting for.

> > My findings so far:
> > w/o the QoS patch we will hit core OFF even if we are in element mode in McBSP:
> > # cat /sys/devices/platform/68000000.ocp/49022000.mcbsp/dma_op_mode
> > [element] threshold
> > 
> > If I start the playback we will hit core OFF even if the we have DMA request
> > coming at every sample (every 0.02ms).

I think that's because we still have windows without pm qos where dma
is not active and we hit idle and there's nothing in hardware blocking
off mode.

> > We hit C6, but when the DMA request comes we have 16 words in the FIFO (8
> > samples, 0.16ms of audio). C6 should take 11.5ms to exit...

So maybe we have the C6 latency for 36xx wrong, and it's much faster than
for 34xx? I wonder what happens on the original beagleboard, not the
xm variant.. I don't have one though.

> > Based on my experiments the FIFO threshold does not matter as even if we
> > should not be able to leave from a C state in time, we do leave from the state :o

Yes that's a mystery :)

> > The patch generates the following warning when the playback is stopped:
...
> > [  115.996002] [<c0155b74>] (__cancel_work_timer) from [<c0199ad8>]
> > (pm_qos_remove_request+0x28/0x1c8)
> > [  116.005523] [<c0199ad8>] (pm_qos_remove_request) from [<c0684634>]
> > (omap_mcbsp_dai_trigger+0x70/0x8c)
> > [  116.015197] [<c0684634>] (omap_mcbsp_dai_trigger) from [<c067baa8>]
> > (soc_pcm_trigger+0xd0/0x11c)
> > [  116.024414] [<c067baa8>] (soc_pcm_trigger) from [<c0663be0>]
> > (snd_pcm_do_stop+0x50/0x54)
> > [  116.032928] [<c0663be0>] (snd_pcm_do_stop) from [<c06639c4>]
> > (snd_pcm_action_single+0x38/0x80)

Hmm OK I wonder why have not seen that one, I've been just hitting ctrl-c
to stop the playback.

> > There is one more issue: if we have playback and capture running at the same
> > time and you stop one of them. It will remove the QoS and the remaining stream
> > might break.
> 
> it is better to place the QoS in omap_mcbsp_request() and remove it in
> omap_mcbsp_free(). This way we retain the QoS for the time the McBSP is in
> active use.

OK makes sense.

Regards,

Tony