On Thu, 13 Jan 2022 12:31:30 +0100, Jaroslav Kysela wrote:
It may be useful to completely invalidate streaming under some circumstances like the USB gadget detach. This case is a bit different than the complete disconnection. The applications should be notified that the PCM streaming is no longer available, but the recovery may be expected.
This patch adds support for SNDRV_PCM_STATE_OPEN passed to snd_pcm_stop() function. Only the hw_free ioctl is allowed to free the buffers in this state for a full recovery operation or the PCM file handle must be closed.
In the OPEN state, ioctls return EBADFD error (with the added hw_free exception above). The applications which are not aware about this new state transition (and recovery) will fail with an error. This operation is expected.
Link: https://lore.kernel.org/alsa-devel/4e17c99b-1d8a-8ebe-972c-9b1299952ff8@ivit... Cc: Pavel Hofman pavel.hofman@ivitera.com Signed-off-by: Jaroslav Kysela perex@perex.cz
I find the idea neat, but I'm afraid that it's a bit confusing from the user POV as is. Namely, the state is in OPEN, but you'd have to perform hw_free manually at first for moving forward; how can user know it? Maybe PCM core should do hw_free by itself when hw_params is called with hw_free_queued.
Also, do_hw_free() will skip the actual free because it's in OPEN state, no?
In anyway, this doesn't look like a 5.17 material, so we still have some time to stew more.
thanks,
Takashi
include/sound/pcm.h | 1 + sound/core/pcm.c | 1 + sound/core/pcm_native.c | 12 +++++++++++- 3 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/include/sound/pcm.h b/include/sound/pcm.h index 33451f8ff755..4de1c2c91109 100644 --- a/include/sound/pcm.h +++ b/include/sound/pcm.h @@ -467,6 +467,7 @@ struct snd_pcm_substream { /* -- assigned files -- */ int ref_count; atomic_t mmap_count;
- atomic_t queued_hw_free; unsigned int f_flags; void (*pcm_release)(struct snd_pcm_substream *); struct pid *pid;
diff --git a/sound/core/pcm.c b/sound/core/pcm.c index 6fd3677685d7..8dc7e99b2113 100644 --- a/sound/core/pcm.c +++ b/sound/core/pcm.c @@ -694,6 +694,7 @@ int snd_pcm_new_stream(struct snd_pcm *pcm, int stream, int substream_count) snd_pcm_group_init(&substream->self_group); list_add_tail(&substream->link_list, &substream->self_group.substreams); atomic_set(&substream->mmap_count, 0);
atomic_set(&substream->queued_hw_free, 0);diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index 621883e71194..118ad3f26f4a 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -686,10 +686,13 @@ static int snd_pcm_hw_params(struct snd_pcm_substream *substream, snd_pcm_stream_lock_irq(substream); switch (runtime->status->state) { case SNDRV_PCM_STATE_OPEN:
if (atomic_read(&substream->queued_hw_free))goto __badfd;+__badfd: snd_pcm_stream_unlock_irq(substream); return -EBADFD; } @@ -829,6 +832,7 @@ static int do_hw_free(struct snd_pcm_substream *substream) result = substream->ops->hw_free(substream); if (substream->managed_buffer_alloc) snd_pcm_lib_free_pages(substream);
- atomic_set(&substream->queued_hw_free, 0); return result;
}
@@ -1454,6 +1458,8 @@ static void snd_pcm_post_stop(struct snd_pcm_substream *substream, } wake_up(&runtime->sleep); wake_up(&runtime->tsleep);
- if (state == SNDRV_PCM_STATE_OPEN)
atomic_set(&substream->queued_hw_free, 1);}
static const struct action_ops snd_pcm_action_stop = { @@ -1469,6 +1475,9 @@ static const struct action_ops snd_pcm_action_stop = {
- The state of each stream is then changed to the given state unconditionally.
- If the requested state is OPEN, the stream is invalidated and
- the application must call hw_free to recover the operation.
*/
- Return: Zero if successful, or a negative error code.
int snd_pcm_stop(struct snd_pcm_substream *substream, snd_pcm_state_t state) @@ -2637,7 +2646,8 @@ void snd_pcm_release_substream(struct snd_pcm_substream *substream)
snd_pcm_drop(substream); if (substream->hw_opened) {
if (substream->runtime->status->state != SNDRV_PCM_STATE_OPEN)
if (substream->runtime->status->state != SNDRV_PCM_STATE_OPEN ||atomic_read(&substream->queued_hw_free)) do_hw_free(substream);-- 2.33.1