On Thu, 13 Jan 2022 14:32:21 +0100, Jaroslav Kysela wrote: > > On 13. 01. 22 13:56, Takashi Iwai wrote: > > 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@xxxxxxxxxxx/ > >> Cc: Pavel Hofman <pavel.hofman@xxxxxxxxxxx> > >> Signed-off-by: Jaroslav Kysela <perex@xxxxxxxx> > > > > 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? > > Thanks for the feedback. > > The state ioctls are also not blocked, so the PCM state can be checked > when EBADFD is returned for the updated applications. But as noted in > the comment, it's expected that current applications will fail (like > for the disconnect). OK. So this must be for a specific new use case... > The OPEN state can be set only when hw_params fails in the current > code. So the applications can distinguish the hw_params error / > invalidate stream cases. We may also add this info (flag) to the PCM > status structure. > > This extension can be also implemented using a new PCM state, but in > the regard of our discussion a few months ago, it's probably not a > way. Right, that'll become a compatibility headache. > > Maybe PCM core should do hw_free by itself when hw_params is > > called with hw_free_queued. > > Yes, it's a possible optimization, too. I had same idea when I post > the patch. But the mmap is an issue here - applications must do unmap > before hw_params, so I'm not convinced to add this auto-free to > hw_params, because hw_free has the straight semantics (munmap > before). It would be probably clever to keep those steps separated. Hm, right, mmap is messy. > Also ideally, there may be a check in hw_params, if parameters > (buffers) are changed, but the implementation is not so easy. Maybe we > can allow OPEN -> > PREPARE transition for this case, so the applications may just restart > the streaming in the most light way. Hmm. Reading more about those restrictions and requirements, I feel that this might be better implemented in the gadget driver side locally at first. Basically we can handle similarly: add a new local flag, set it at the stream stop, and return an error at prepare until hw_params gets reconfigured. This might be even smaller changes? thanks, Takashi
