At Wed, 30 Oct 2013 20:28:11 +0530, Vinod Koul wrote:
On Wed, Oct 30, 2013 at 03:47:45PM +0100, Takashi Iwai wrote:
At Thu, 24 Oct 2013 16:37:31 +0530, Vinod Koul wrote:
The drain and drain_notify callback were blocked by low level driver untill the draining was complete. Due to this being invoked with big fat mutex held, others ops like reading timestamp, calling pause, drop were blocked.
So to fix this we add a new snd_compr_drain_notify() API. This would be required to be invoked by low level driver when drain or partial drain has been completed by the DSP. Thus we make the drain and partial_drain callback as non blocking and driver returns immediately after notifying DSP. The waiting is done while relasing the lock so that other ops can go ahead.
Signed-off-by: Vinod Koul vinod.koul@intel.com CC: stable@vger.kernel.org
v3: call snd_compr_drain_notify from compress_stop() rename draining -> drain_wake add some comments on state transistion after drain
v2: fix the 80 line warn move the state change to compress_drain()
include/sound/compress_driver.h | 12 +++++++++++ sound/core/compress_offload.c | 41 ++++++++++++++++++++++++++++++++++++-- 2 files changed, 50 insertions(+), 3 deletions(-)
diff --git a/include/sound/compress_driver.h b/include/sound/compress_driver.h index 9031a26..175ab32 100644 --- a/include/sound/compress_driver.h +++ b/include/sound/compress_driver.h @@ -48,6 +48,8 @@ struct snd_compr_ops;
- the ring buffer
- @total_bytes_transferred: cumulative bytes transferred by offload DSP
- @sleep: poll sleep
- @wait: drain wait queue
*/
- @drain_wake: condition for drain wake
struct snd_compr_runtime { snd_pcm_state_t state; @@ -59,6 +61,8 @@ struct snd_compr_runtime { u64 total_bytes_available; u64 total_bytes_transferred; wait_queue_head_t sleep;
- wait_queue_head_t wait;
- unsigned int drain_wake; void *private_data;
};
@@ -171,4 +175,12 @@ static inline void snd_compr_fragment_elapsed(struct snd_compr_stream *stream) wake_up(&stream->runtime->sleep); }
+static inline void snd_compr_drain_notify(struct snd_compr_stream *stream) +{
- snd_BUG_ON(!stream);
Should return, otherwise you'll get Oops:
if (snd_BUG_ON(!stream)) return;
ah, missed that!
- stream->runtime->drain_wake = 1;
- wake_up(&stream->runtime->wait);
+}
#endif diff --git a/sound/core/compress_offload.c b/sound/core/compress_offload.c index bea523a..3eb47d0 100644 --- a/sound/core/compress_offload.c +++ b/sound/core/compress_offload.c @@ -123,6 +123,7 @@ static int snd_compr_open(struct inode *inode, struct file *f) } runtime->state = SNDRV_PCM_STATE_OPEN; init_waitqueue_head(&runtime->sleep);
- init_waitqueue_head(&runtime->wait); data->stream.runtime = runtime; f->private_data = (void *)data; mutex_lock(&compr->lock);
@@ -682,12 +683,34 @@ static int snd_compr_stop(struct snd_compr_stream *stream) if (!retval) { stream->runtime->state = SNDRV_PCM_STATE_SETUP; wake_up(&stream->runtime->sleep);
snd_compr_drain_notify(stream);}
+static int snd_compress_wait_for_drain(struct snd_compr_stream *stream) +{
- /*
* We are called with lock held. So drop the lock while we wait for* drain complete notfication from the driver** It is expected that driver will notify the drain completion and then* stream will be moved to SETUP state, even if draining resulted in an* error. We can trigger next track after this.*/- stream->runtime->state = SNDRV_PCM_STATE_DRAINING;
- mutex_unlock(&stream->device->lock);
- wait_event(stream->runtime->wait, stream->runtime->drain_wake);
Don't you really want the interruption? Or use wait_event_interruptible() and return error appropriately.
any interruption from usermode should trigger the compress_stop/compress_free
... but how?
which will unblock this. I dont see the need to have interruptible here
Suppose you're running a single thread program. Then who triggers the stop?
Takashi