lkml.org 
[lkml]   [2021]   [Jan]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [RFC PATCH 1/2] soundwire: add support for static port mapping
From
Date


On 1/22/21 1:05 AM, Srinivas Kandagatla wrote:
>
>
> On 21/01/2021 21:30, Pierre-Louis Bossart wrote:
>>>
>>> Am looking at intel_hw_params(). Isn't sdw_stream_add_master() called
>>> for every dai in the dai link.
>>
>> Yes, that's correct, but again a dai may use one or more ports.
>>
>> if you defined each port as a dai, and want to call
>> sdw_stream_add_master() for each port you are doing something the API
>> was not designed for. There is a 'num_ports' argument for a reason :-)
>>
>>>> per master, and that master_rt deals with one or more ports - your
>>>> choice. >
>>>> A 'stream' is an abstract data transport which can be split across
>>>> multiple masters/sales and for each master/slave use multiple ports.
>>>> When calling sdw_stream_add_master/slave, you need to provide a
>>>> port_config/num_ports to state which ports will be used on that
>>>> master/slave when using the stream. That's how we e.g. deal with 4ch
>>>> streams that are handled by two ports on each side.
>>>>
>>>> To up-level a bit, the notion of 'stream' is actually very very
>>>> similar to the notion of dailink. And in fact, the 'stream' is
>>>> actually created for Intel in the dailink .startup callback, so I am
>>>> quite in the dark on what you are trying to accomplish.
>>> In qcom case stream is also allocated for in dai startup().
>>>
>>> I think we are talking about two different issues,
>>>
>>> 1>one is the failure I see in sdw_stream_add_master() when I try to
>>> use dai-link dai-id style approach suggested. I will dig this bit
>>> more and collect more details!
>>>
>>> 2>(Main issue) Ability for slave to select different combination of
>>> ports at runtime based on the mixer setting or active dapm.
>>>
>>> All this patch is trying do is the pass this *CURRENT/ACTIVE* static
>>> port mapping between slave and master while setting up the stream.
>>> With the dailink approach number of ports are pretty much static and
>>> may not be required for particular use case. As above example if we
>>> have a headset with button click suppression we would need 2 Ports
>>> and similarly without we only need one port.
>>
>> As I said above you cannot enable the button click suppression
>> dynamically *after* the headset capture hw_params/prepare.
>
> That is not true, the ports in this case are selected based on mixer
> setting or register state even before stream is setup/started in
> hw_params/prepare.
> WSA881x codec has pretty much similar setup.

we are saying the same thing, the configuration provided is only taken
into account when setting-up the stream in hw_params. mixer or
configuration changes after that step are ignored.

If you follow what we've done at Intel with the sdw_stream_add_master()
called in the .hw_params phase, and conversely call
sdw_stream_remove_master() in .hw_free, you should be good to go.

You will note that we have a notification to the DSP, so you can manage
resources in your firmware, there is no need to oversubscribe but only
allocate what is required for a given use case.

>>> This is not possible with dai-link approach, unless we create two
>>> different dai links for the above example usecase!
>>
>> The current approach is a worst-case one, where you would create a
>> single 'headset capture' dailink.
>>
>
> Are you suggesting that we have dailink for each usecase like:
>
> "headset capture"
> "Analog MIC1 capture"
> "Analog MIC2 Capture"
>
> ...
>
> "Analog MIC4 Capture"
>
> ...
>
> "DMIC0 capture"
> "DMIC1 Capture"
> "DMIC2 Capture"
>
> ...
>
> "DMIC7 Capture"
> ..
> "Headset Playback"
> "Ear Playback"
> ..
> "Aux Playback"
> ...
>
> this is not really doable!

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).

> 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.



\
 
 \ /
  Last update: 2021-01-22 16:37    [W:0.084 / U:2.964 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site