Re: [RFC PATCH 1/2] soundwire: add support for static port mapping - Pierre-Louis Bossart

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
To: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>,
	vkoul@kernel.org, yung-chuan.liao@linux.intel.com
Cc: gregkh@linuxfoundation.org, sanyog.r.kale@intel.com,
	alsa-devel@alsa-project.org, linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH 1/2] soundwire: add support for static port mapping
Date: Fri, 22 Jan 2021 10:42:36 -0600	[thread overview]
Message-ID: <adb91730-35db-db7a-75b3-4771723de945@linux.intel.com> (raw)
In-Reply-To: <e962caa8-89c3-4a22-5932-4498c406e8f8@linaro.org>

>> No, what I was saying is that you need to define multiple streams e.g.
>> - headset capture (configured with or without click suppression)
>> - mic capture (configured with AMICs or DMICs)
>> - playback (or possibly different endpoint specific streams depending 
>> on whether concurrency between endpoint is possible)
>>
>> if you change the configuration, you have to tear down the stream and 
>> reconfigure it - and for this we already have the required API and you 
>> can guarantee that the configuration for that stream is consistent 
>> between master and slave(s).
> 
> Yes, we make sure that new configuration is only applied before the 
> stream is started, and not in middle of already started stream.

ok, we are almost in agreement but...

>>> All am saying is that codec can decide which ports it has to select 
>>> based on mixer setting before the stream is setup/started. This 
>>> updated mapping between slv port and master ports is passed as part 
>>> of the port_config in sdw_stream_add_slave().
>>
>> if you completely remove the stream and re-add it with updated 
>> configuration things should work.
> 
> That's exactly what we do currently!
> 
> The updated ports due to new configuration ex: for "mic capture" dailink 
> needs to be communicated from slave(codec) to master so that it can 
> allocate correct ports. That is what this patch is trying to do (share 
> current port map information).

.. we have a disconnect on how to do this configuration update.

The 'stream' support was designed so that a stream can be split across 
multiple devices (both masters and slaves). With this design we need to 
have a central configuration and distribute the information to all 
devices taking part of the stream.

It seems you are in a different solution-space, where the codec driver 
needs to notify the master of which ports it needs to use?

I also don't see where the mapping is actually set. Patch 2 uses a 
mapping but there's no codec driver change that defines the mapping?

Do you actually call sdw_stream_add_slave() with a new mapping?

It feels we are missing the codec part to really see what you are trying 
to do?