[PATCH v2 2/2] ASoC: Intel: avs: Disconnect substream if suspend or resume fails

[Takashi Iwai]

On Mon, 14 Nov 2022 15:08:17 +0100,
Cezary Rojewski wrote:
> 
> 
> 
> On 2022-11-14 2:02 PM, Takashi Iwai wrote:
> > On Mon, 14 Nov 2022 12:37:29 +0100,
> > Cezary Rojewski wrote:
> >> 
> >> To improve performance and overall system stability, suspend/resume
> >> operations for ASoC cards always return success status and defer the
> >> actual work.
> >> 
> >> Because of that, if a substream fails to resume, userspace may still
> >> attempt to invoke commands on it as from their perspective the operation
> >> completed successfully. Set substream's state to DISCONNECTED to ensure
> >> no further commands are attempted.
> 
> ...
> 
> > Hm, that might work, but note that, once when the stream is set with
> > the disconnected state, it won't be taken back to the normal state any
> > longer on most sound backends.  That is, it'll be gone and won't
> > revive unless you completely unload and reload the whole stuff.  If
> > that's the intended behavior, that's fine, of course.  So just to make
> > sure.
> 
> Good point.
> 
> Our intention: if we fail e.g.: on resume, we would like the framework
> to invoke ->hw_free() and close us. Right now, if we pretend that
> everything is okay, invalid actions can be performed on our
> streams. This all comes down to userspace calling "just"
> snd_pcm_resume(). If we had an option to opt-in to a _hw_params() +
> _prepare() + _resume() path, then things would look differently.

So you'd rather like to make the stream appearing and working after
re-opening the stream again?  Then DISCONNECTED state might be too
strong.

If the broken state could be recovered by the PCM PREPARE phase, then
we may (ab-)use XRUN state, instead.  Then application shall
re-setup via PCM prepare call.  But if hw_free/hw_params is required,
it won't work.

Other than that, there is no such PCM state that forces to close and
re-open, I'm afraid.  You can have an internal error state and let the
stream returning an error at each operation, instead, too.

And, creating a new PCM state is very difficult.  It would influence
way too much, IMO, as each application code has to be modified to
handle the new PCM state.


Takashi