[PATCH] ALSA: hda/hdmi: Fix the converter reuse for the silent stream

[Jaroslav Kysela]

On 19. 09. 22 13:33, Kai Vehmanen wrote:
> Hi,
> 
> sorry for the late review (as Takashi already applied).
> 
> On Tue, 13 Sep 2022 09:02:16 +0200, Jaroslav Kysela wrote:
>> When the user space pcm stream uses the silent stream converter, it is no
>> longer allocated for the silent stream. Clear the appropriate flag in the
>> hdmi_pcm_open() function. The silent stream setup may be applied in
>> hdmi_pcm_close() (and the error path - open fcn) again.
> 
> Jaroslav, did you hit a real-life problem with a particular sequence? The 
> converter allocation was done in a sticky fashion on purpose. The silent 
> stream setup (clearing KAE and/or letting codec to power down) was only 
> released if the receiver is disconnected from the system. I.e. no actions 
> were done in PCM close (on purpose).

Thank you for your comments.

I just wrote a small test utility which shows the bad converter use:

https://gist.github.com/perexg/4977fcb33101148e578c34bd25db8ce4

For the first use (with one monitor connected) I can open 4 HDMI PCM devices,
but later, only 3 HDMI PCM devices can be opened, because the converter
allocation code (3 converters in my case) does a wrong job.

I though that the silent stream is turned off when new S/PDIF frame parameters 
are applied, but it seems that this setup is independent as you noted.

> Now if there were issues, I wonder whether we should modify the pcm_close 
> instead, and avoid some of the cleanup in this case, as a better fix.

Yes, I think that we should add a new silent_stream flag to struct 
hdmi_spec_per_cvt and do not set assigned in this struct when the silent 
stream is managed. The assigned flag should be used only for open/close calls 
to track the used converters correctly. Does it sound right for you?

> I think your patch will create a new problem: - receiver A connected to CVT
> X - silent stream enabled for X - app opens a PCM, connected to A+X, silent
> stream disabled - receiver A disconnected -> actions in
> silent_stream_disable() not run correctly for CVT X

Definitely. The silent stream setup won't be deactivated with this simple patch.

					Jaroslav



-- 
Jaroslav Kysela <perex at perex.cz>
Linux Sound Maintainer; ALSA Project; Red Hat, Inc.