[RFC] SoundWire: add rt1308 feedback #2514

plbossart · 2020-10-16T21:31:02Z

rough draft as an example of how to add IV feedback support

I have no idea how to add a capture dailink, might been @RanderWang's help here.

shumingfan · 2020-10-19T01:39:53Z

sound/soc/codecs/rt1308-sdw.c

+capture:
+	direction = SDW_DATA_DIR_TX;
+	num_channels = 2;
+
+	stream_config.frame_rate = params_rate(params);
+	stream_config.ch_count = num_channels;
+	stream_config.bps = snd_pcm_format_width(params_format(params));
+	stream_config.direction = direction;
+
+	port_config_feedback[0].ch_mask = BIT(0);
+	port_config_feedback[0].num = 2; /* DP2 */
+	port_config_feedback[1].ch_mask = BIT(1);
+	port_config_feedback[1].num = 4; /* DP4 */


@plbossart Thanks for this commit to explain how to config the multi data ports with one DAI link.
I think something I don't explain clearly in #2513
The feedback channels are two channels for each port.
Due to RT1308 is a stereo amplifier, it will protect the left and right speakers individually.
Left IV => using DP2 to feedback ch0 (I data) and ch1 (V data) for left speaker.
Right IV => using DP4 to feedback ch0 (I data) and ch1 (V data) for right speaker.
Therefore, the total channels are four channels like below.

num_channels = 4; port_config_feedback[0].ch_mask = BIT(0) | BIT(1); port_config_feedback[0].num = 2; /* DP2 */ port_config_feedback[1].ch_mask = BIT(2) | BIT(3); port_config_feedback[1].num = 4; /* DP4 */

@shumingfan waiting for your result. I also had such idea but I didn't get a answer from cadance's spec.

@shumingfan I get the point for the case with a a single amp, we would get 4 channels. Your change makes sense. I have to see though how we would deal with 4 channels on the Intel side, I believe we would need 2 ports.

Also what happens in the case where there are two amplifiers in the system, and each of them controls a tweeter and woofer. How many feedback channels would we get from each amplifier?

@plbossart It will have 4 feedback channels from each amplifier.
Hence, there are eight feedback channels when using two amplifiers in the system.
The amp protection needs to protect each speaker whether it belongs to the tweeter or woofer.

@shumingfan waiting for your result. I also had such idea but I didn't get a answer from cadance's spec.

@RanderWang I tested the feedback function with the approach of this PR.
First, I tried to record the IV data from DP2 only. I can see the ch0 and ch1 have the captured data.
Then, I tried to record the IV data from DP2/4 both. There are captured data with ch0 and ch1 only.
The ch2 and ch3 are zero.
I checked the MIPI-defined registers of DP2 and DP4 which had been configured while the test.
The DP4_ChannelEN doesn't enable. I think there is a problem there.

@shumingfan if the DP4 registers were configured, but only the DP4_ChannelEN is not set, this points to a missing loop in the bank switch where we only deal with a single port?

@shumingfan if the DP4 registers were configured, but only the DP4_ChannelEN is not set, this points to a missing loop in the bank switch where we only deal with a single port?

@plbossart I found the root cause. The DP2/4_ChannelEn only supports channel1/2.
The driver needs to change the ch_mask to 0x3 for DP2/4 both.
Now, I could get the captured data of four channels.

num_channels = 4; port_config_feedback[0].ch_mask = BIT(0) | BIT(1); port_config_feedback[0].num = 2; /* DP2 */ port_config_feedback[1].ch_mask = BIT(0) | BIT(1); port_config_feedback[1].num = 4; /* DP4 */

thanks @shumingfan, updated the patch and added you as co-author. please check if this is ok now.

sound/soc/codecs/rt1308-sdw.c

The RT1308 can provide feedback for two speakers. Each speaker is handled by a port, and each port transmits two channels for I and V. The data is handled as a single 4ch stream and a single DAI. Co-developed-by: Shuming Fan <shumingf@realtek.com> Signed-off-by: Shuming Fan <shumingf@realtek.com> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

FIXME: for now there is no initialization on capture. Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

shumingfan · 2020-10-23T08:05:40Z

@plbossart @bardliao @RanderWang Now I tried to connect two amplifiers to meet the environment of the customer's product.
The two RT1308 connect to the different SdW link. I think we will enable the aggregation feature for playback.
However, do we have the aggregation feature for capture?
I checked the topology files that there is one sof-smart-amplifier.m4 to support the smart amp which could feedback the IV to smart_amp component.
But, it seems not to support the aggregation feature.
Then, our FW team wants to combine all IV feedback data (4+4 channels) to the smart_amp component for amp protection.
I draw the topology graph below.

# PCM_P -- B0 --> smart_amp -- B1--> Demux(M) -- B2 ----> DAI
#                  ^                     |
#                  |                     pipeline n+1 --> DAI
#                  |
#                  B2
#                  |
#                  |
#                  |
# PCM_C <-- B0 <-- Demux(M) <-- B1 <--- Mux <--- B2 <-- DAI
#                                        ^
#                                        |
#                                        ------- B3 <-- DAI 
#

Sorry, I am not familiar with the topology development. Does this graph make sense or be reasonable?
Could anyone support this or give me the sample? I will try to test it if there is any suggestion or topology sample.

RanderWang · 2020-10-23T09:04:22Z

@plbossart @bardliao @RanderWang Now I tried to connect two amplifiers to meet the environment of the customer's product.
The two RT1308 connect to the different SdW link. I think we will enable the aggregation feature for playback.
However, do we have the aggregation feature for capture?
I checked the topology files that there is one sof-smart-amplifier.m4 to support the smart amp which could feedback the IV to smart_amp component.
But, it seems not to support the aggregation feature.
Then, our FW team wants to combine all IV feedback data (4+4 channels) to the smart_amp component for amp protection.
I draw the topology graph below.
# PCM_P -- B0 --> smart_amp -- B1--> Demux(M) -- B2 ----> DAI
#                  ^                     |
#                  |                     pipeline n+1 --> DAI
#                  |
#                  B2
#                  |
#                  |
#                  |
# PCM_C <-- B0 <-- Demux(M) <-- B1 <--- Mux <--- B2 <-- DAI
#                                        ^
#                                        |
#                                        ------- B3 <-- DAI 
#
Sorry, I am not familiar with the topology development. Does this graph make sense or be reasonable?
Could anyone support this or give me the sample? I will try to test it if there is any suggestion or topology sample.

@shumingfan please make it clear about your DAI in graph. only one BE DAI but two CPU DAIs & codec DAIs. Now as I know, we don't support capture aggregation. @bardliao do we support it ?

plbossart · 2020-10-23T14:54:41Z

@shumingfan you are absolutely right that we need a mux on capture to aggregate the two links. I don't think this exists today, we need to check with @slawblauciak.

However I am not sure about the demux. The idea of inserting a demux was that we could pass an echo reference to the application for acoustic echo cancellation. It's my understanding that we use the demux only to split the stream in two.

Now in your case, the aggregated feedback stream would be made of 8 channels. That seems a bit too much to pass to the application, so we may need to combine channels or select specific ones?

Also note that if you want to develop this functionality, we will be limited by the SOF firmware signature issues. The simplest solution for development might be to use an UpExtreme board, which exposes 4 SoundWire links on its connector and allows for the community key to be used, and tie it to the 3-in-1 board.

If you want to enable the RT1308 IV feedback on devices where the Intel production key is used, we need to solve collectively the signature issues. The Realtek library will probably not go into the Intel base firmware, so we are looking at a 2-key solution similar to Windows, and Realtek releasing the firmware to users. Adding @lgirdwood to make sure this is tracked.

shumingfan · 2020-10-26T10:52:18Z

@plbossart Thanks for your explanation. But, it is bad news to our FW team that there is no mux on capture to aggregate the two links for now.
The main purpose is that the aggregated feedback stream could send to smart_amp component to do the amp protection.
Passing the data to the application may be an option for debugging.

We have the ICL RVP to do the test and algorithm development.
I think this environment doesn't have the SOF firmware signature issue, right?

plbossart · 2020-10-26T18:12:16Z

@plbossart Thanks for your explanation. But, it is bad news to our FW team that there is no mux on capture to aggregate the two links for now.

There is no mux yet on capture, but I think this should be relatively simple to do. @bardliao could do this before breakfast :-)

The main purpose is that the aggregated feedback stream could send to smart_amp component to do the amp protection.
Passing the data to the application may be an option for debugging.

yes, the need/usage is understood.

We have the ICL RVP to do the test and algorithm development.
I think this environment doesn't have the SOF firmware signature issue, right?

RVPs shipped outside of Intel usually use the production key, not the debug key. At any rate, it's not the SOF 'community key' either way.

bardliao · 2020-10-27T14:27:50Z

@plbossart Thanks for your explanation. But, it is bad news to our FW team that there is no mux on capture to aggregate the two links for now.

There is no mux yet on capture, but I think this should be relatively simple to do. @bardliao could do this before breakfast :-)

Oh, that is not easy to me in fact.

plbossart · 2021-11-04T23:08:20Z

this PR is a year old and nothing happened, closing.

plbossart requested review from shumingfan, bardliao and RanderWang October 16, 2020 21:31

shumingfan reviewed Oct 19, 2020

View reviewed changes

shumingfan mentioned this pull request Oct 19, 2020

[FEATURE] connect multi-DAIs with one SdW link in one SdW device driver #2513

Closed

plbossart force-pushed the sdw/rt1308-feedback branch from 6c073fa to 3a12553 Compare October 22, 2020 14:21

plbossart requested a review from shumingfan October 22, 2020 14:22

plbossart commented Oct 22, 2020

View reviewed changes

sound/soc/codecs/rt1308-sdw.c Outdated Show resolved Hide resolved

plbossart added 2 commits October 22, 2020 10:07

ASoC: Intel: sof_sdw: add feedback support for RT1308

d41d1c1

FIXME: for now there is no initialization on capture. Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

plbossart force-pushed the sdw/rt1308-feedback branch from 3a12553 to d41d1c1 Compare October 22, 2020 15:10

plbossart mentioned this pull request Nov 4, 2020

[BUG] Intermittent audio distortion on Dell 9700 / sof-cml-rt711-rt1308-rt715.tplg / firmware 1.6 thesofproject/sof#3499

Closed

mengdonglin mentioned this pull request Nov 25, 2020

SoundWire: Add IV feedback support for RT1308 #2580

Closed

libinyang mentioned this pull request Jan 5, 2021

[BUG] memory allocation failure in mux/demux pipeline thesofproject/sof#3724

Closed

plbossart mentioned this pull request Apr 15, 2021

SoundWire: add mockup codec support #2827

Merged

plbossart closed this Nov 4, 2021

libinyang mentioned this pull request Mar 3, 2022

topology1: add sof-icl-rt711-rt1308-rt715-hdmi NOMIC support thesofproject/sof#5367

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RFC] SoundWire: add rt1308 feedback #2514

[RFC] SoundWire: add rt1308 feedback #2514

plbossart commented Oct 16, 2020

shumingfan Oct 19, 2020

RanderWang Oct 19, 2020

plbossart Oct 19, 2020

shumingfan Oct 20, 2020

shumingfan Oct 21, 2020

plbossart Oct 21, 2020

shumingfan Oct 22, 2020

plbossart Oct 22, 2020

shumingfan commented Oct 23, 2020 •

edited

Loading

RanderWang commented Oct 23, 2020

plbossart commented Oct 23, 2020

shumingfan commented Oct 26, 2020

plbossart commented Oct 26, 2020

bardliao commented Oct 27, 2020

plbossart commented Nov 4, 2021

[RFC] SoundWire: add rt1308 feedback #2514

[RFC] SoundWire: add rt1308 feedback #2514

Conversation

plbossart commented Oct 16, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shumingfan commented Oct 23, 2020 • edited Loading

RanderWang commented Oct 23, 2020

plbossart commented Oct 23, 2020

shumingfan commented Oct 26, 2020

plbossart commented Oct 26, 2020

bardliao commented Oct 27, 2020

plbossart commented Nov 4, 2021

shumingfan commented Oct 23, 2020 •

edited

Loading