On 2022-11-14 3:26 PM, Takashi Iwai wrote:
On Mon, 14 Nov 2022 15:08:17 +0100,
Cezary Rojewski wrote:
On 2022-11-14 2:02 PM, Takashi Iwai wrote:
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.
To resume properly an AudioDSP stream, operations typically found in
hw_params() and prepare() need to be re-executed _before_ actual
_TRIGGER_RESUME can be called on a stream.
Since implementations found in userspace apps such as aplay and
pulseaudio first invoke snd_pcm_resume() and then snd_pcm_prepare() if
the former fails, one could abuse this flow by doing hw_params-related
operations in _resume() and returning a specific error code e.g.:
-ESTRPIPE when they succeed (if they fail, the internal error code would
be returned instead of course). Prepare tasks are left to
snd_pcm_prepare() just like in standard case.
I believe such code is a good example of _code smell_ though.
First we abuse the error path, second we basically drop the
_TRIGGER_RESUME entirely - after all, in such approach it mimics
hw_params() and will always fail, either with an internal error code or
hardcoded -ESTRPIPE. _TRIGGER_START/STOP would be reused once
snd_pcm_prepare() succeeds causing another problem - no differentiation
between start and resume and thus lack of information when to or when
not to poll DRSM.
Thus, we ended up with 'state = DISCONNECTED' if we encounter a firmware
issue during PCM-pm. Keeps things sane.
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.
Agree, this basically screams not to follow that path.
Regards,
Czarek
