Re: [PATCH 01/13] ASoC: soc-pcm: Don't reconnect an already active BE

["Péter Ujfalusi" <peter.ujfalusi@linux.intel.com>]

On 29/09/2021 10:43, Sameer Pujar wrote:
> 
> 
> On 9/29/2021 2:55 AM, Pierre-Louis Bossart wrote:
>> On 8/27/21 4:33 AM, Sameer Pujar wrote:
> 
> [...]
> 
>> But in addition we'd need to agree on what an 'active BE' is. Why can't
>> we connect a second stream while the first one is already in HW_PARAMS
>> or PAUSED or STOP? It's perfectly legal in ALSA/ASoC to have multiple
>> HW_PARAMS calls, and when we reach STOP we have to do a prepare again.
>>
>> And more fundamentally, the ability to add a second FE on a 'active' BE
>> in START state is a basic requirement for a mixer, e.g. to play a
>> notification on one FE while listening to music on another. What needs
>> to happen is only to make sure that the FE and BE are compatible in
>> terms of HW_PARAMS and not sending a second TRIGGER_STOP, only checking
>> the BE NEW or CLOSE state is way too restrictive.
> 
> Sorry for the trouble to your system.
> 
> Idea was to avoid reconfiguration of the same BE DAI again, but not to
> stop the provision to add a subsequent FE. In fact I had tested mixing
> of streams coming from 10 different FEs.
> 
> In your case, because of this patch, looks like the subsequent FE is not
> finding a BE DAI since it is already active due to a prior FE. The
> reason it works at my end is because the mixer input and output DAIs are
> separated. Any new FE would just configure the mixer input DAI to which
> it is attached and skip already running/configured output DAI. 

The problem as I see is that with this patch one can not connect a new
FE to a BE which is _not_ in NEW or CLOSE state.

The FE and BE needs to be connected to have DPCM working and this patch
makes the code to skip the dpcm_be_connect().

Consider this simple setup:

FE1 -->|
       | --> BE -->
FE2- ->|

First we start FE1, dpcm_be_connect(FE1, BE, stream) is made.

Later FE2 is started but dpcm_be_connect(FE2, BE, stream) would be not
made because BE is no longer in NEW/CLOSE state.

> I am just
> curious to know, if originally you were reconfiguring the BE DAI again
> with same parameters (for a second FE) or some additional configuration
> is done?
> 
> 
>> I can send a revert with the explanations in the commit message if there
>> is a consensus that this patch needs to be revisited.
> 
> May be this can be revisited since it appears to be a critical problem
> for your system. But I hope this discussion can be alive on following
> points for a better fix.
> 
> 1. The original issue at my end was not just a configuration redundancy.
> I realize now that with more stream addition following error print is seen.
>    "ASoC: too many users playback at open 4"
> 
>    This is because the max DPCM users is capped at 8. Increasing this
> may help (need to see what number is better), but does not address the
> redundancy problem.
> 
> 2. If reconfiguration of the same BE is not necessary for a subsequent
> FE run, shouldn't we avoid the reconfig itself and somehow avoid FE
> failure?

I'm not sure, but it might be possible to just skip the
dpcm_set_be_update_state(be, stream, SND_SOC_DPCM_UPDATE_BE);
call at the end of the loop, but the question is under which condition?
Can a BE asked to be reconfigured when STOP/OPEN/HW_PARAMS?

Skipping the connect does not sound right for a new FE-BE connection.

-- 
Péter