ASoC: SOF: topology: send ipc for all found DAIs in sof_set_dai_config by bardliao · Pull Request #1982 · thesofproject/linux

bardliao · 2020-04-06T07:37:55Z

We may find more than one DAIs in sof_set_dai_config(). All the DAIs will share the same config and we have to send ipc for each of them.

kv2019i

First patch looks good and is a nicer reduction of code overall.
The second patch is a bit harder to digest. Maybe this is aligned with the overall approach we have taken with multiple DAIs on one link. Some questions inline.

plbossart · 2020-04-06T15:37:09Z

I've personally never quite understood the topology and dai/dailink management, @ranj063 this is for you really to comment on...

ranj063 · 2020-04-06T15:45:46Z

@bardliao @plbossart the direction is correct here. The only thing is we need one more step to remove sof_link_hda_process() and replace it call with sof_set_dai_config() as well and then the code will look clean and possibly even look a bit self-explanatory

bardliao · 2020-04-07T01:51:48Z

@bardliao @plbossart the direction is correct here. The only thing is we need one more step to remove sof_link_hda_process() and replace it call with sof_set_dai_config() as well and then the code will look clean and possibly even look a bit self-explanatory

@ranj063 Good suggestion. Updated. @RanderWang Can you review the HDA change?

ranj063

Thanks @bardliao. LGTM

kv2019i

Thanks, looks good now!

plbossart · 2020-04-07T18:58:38Z

I am a bit nervous about the last patch. IIRC there were some tricks between playback and capture and something about a mask not shared. We should make sure @RanderWang reviews and tests.

Also CI reports consistent failure on Up2-Nocodec, that's odd. We should verify this.

plbossart · 2020-04-07T19:00:04Z

oh and I forget, those changes are at the core level so our friends at NXP should chime in. @dbaluta @paulstelian97 please check if those patches have any adverse effect on your solution

RanderWang · 2020-04-08T03:05:45Z

@bardliao LGTM. you can get help from QA to test it on HDA codec.

ClarexZhou · 2020-04-08T06:37:47Z

Verified this PR on Lenovo X1 Yoga with ALC285 in HDA mode. All works ok. No regression issue.

RanderWang

LGTM

lyakh · 2020-04-09T06:36:46Z

 			      struct snd_soc_dai_link *link,
 			      struct sof_ipc_dai_config *config)
 {
+	struct sof_ipc_reply reply;


I still prefer keeping the scope as small as possible. In this case it would mean defining both reply and ret in the code block where they're really needed.

I still prefer keeping the scope as small as possible. In this case it would mean defining both reply and ret in the code block where they're really needed.

Thanks @lyakh Updated.

bardliao · 2020-04-10T02:45:08Z

oh and I forget, those changes are at the core level so our friends at NXP should chime in. @dbaluta @paulstelian97 please check if those patches have any adverse effect on your solution

@dbaluta @paulstelian97 Do you have any concern about this PR?

kv2019i · 2020-04-14T15:19:34Z

@dbaluta @paulstelian97 -- I'll give you some time to comment on this (see question above in comment)

We may find more than one DAIs in sof_set_dai_config() and we should send message to DSP for each DAI we found. Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>

The ipc dai config dai_index is from topology. However, the same dai config will be applied to all DAIs in the same dai link. We have to ensure that the ipc dai config's dai_index match to the component's dai_index. Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>

Now we send ipc in sof_set_dai_config(), too. Therefore, we can use sof_set_dai_config() in sof_link_hda_load() just like other link load functions do. Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>

bardliao · 2020-04-20T01:56:48Z

rebased to fix the conflict in topology.c

kv2019i · 2020-04-21T07:55:31Z

Ok, this been open for more comments for two weeks and no further findings, so time to move ahead. Tests look ok (ICL failure is a boot retry failure which is known, GLK load/unload is known as well), so proceeding with merge.

dbaluta · 2020-04-21T08:25:44Z

@plbossart sorry, this got lost under the tons of notifications. This looks good to me, if there is any problem we will send a fix.

bardliao requested review from plbossart and ranj063 April 6, 2020 07:37

bardliao requested review from juimonen and singalsu as code owners April 6, 2020 07:37

bardliao mentioned this pull request Apr 6, 2020

ASoC: SOF: topology: config ALH dai in sof_widget_load_dai #1976

Closed

kv2019i reviewed Apr 6, 2020

View reviewed changes

Comment thread sound/soc/sof/topology.c

bardliao requested a review from RanderWang April 7, 2020 01:52

ranj063 reviewed Apr 7, 2020

View reviewed changes

Comment thread sound/soc/sof/topology.c Outdated

bardliao force-pushed the dai-config branch from b0cfffe to c9f4723 Compare April 7, 2020 05:47

bardliao changed the title ~~[RFC]: ASoC: SOF: topology: send ipc for all found DAIs in sof_set_dai_config~~ ASoC: SOF: topology: send ipc for all found DAIs in sof_set_dai_config Apr 7, 2020

ranj063 previously approved these changes Apr 7, 2020

View reviewed changes

sinahuang mentioned this pull request Apr 7, 2020

[BUG][CML][tplg]"error: ipc error for 0x60010000 size 20" happened after booting up successfully. thesofproject/sof#2690

Closed

kv2019i previously approved these changes Apr 7, 2020

View reviewed changes

Comment thread sound/soc/sof/topology.c

plbossart requested review from dbaluta and paulstelian97 April 7, 2020 19:00

RanderWang previously approved these changes Apr 8, 2020

View reviewed changes

YvonneYang2 mentioned this pull request Apr 8, 2020

[BUG][CML]DSP Panic occurred when pause/resume on aplay0,2(RT1308) thesofproject/sof#2658

Closed

sinahuang mentioned this pull request Apr 9, 2020

[BUG][CML-H]" error: ipc error for 0x60010000 size 20" display when aplay via speaker0,2. thesofproject/sof#2633

Closed

lyakh reviewed Apr 9, 2020

View reviewed changes

bardliao dismissed stale reviews from RanderWang, kv2019i, and ranj063 via 39d0f29 April 9, 2020 07:36

bardliao force-pushed the dai-config branch from c9f4723 to 39d0f29 Compare April 9, 2020 07:36

bardliao force-pushed the dai-config branch from 39d0f29 to 28fe1b5 Compare April 9, 2020 08:47

bardliao requested review from RanderWang, kv2019i, lyakh and ranj063 April 10, 2020 02:43

RanderWang previously approved these changes Apr 10, 2020

View reviewed changes

ranj063 previously approved these changes Apr 10, 2020

View reviewed changes

plbossart previously approved these changes Apr 10, 2020

View reviewed changes

lyakh previously approved these changes Apr 14, 2020

View reviewed changes

bardliao added 3 commits April 20, 2020 09:52

ASoC: SOF: topology: send ipc for all found DAIs in sof_set_dai_config

eb8a5f6

We may find more than one DAIs in sof_set_dai_config() and we should send message to DSP for each DAI we found. Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>

ASoC: SOF: topology: replace sof_link_hda_process by sof_set_dai_config

1fd7b97

Now we send ipc in sof_set_dai_config(), too. Therefore, we can use sof_set_dai_config() in sof_link_hda_load() just like other link load functions do. Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>

bardliao dismissed stale reviews from lyakh, plbossart, ranj063, and RanderWang via 1fd7b97 April 20, 2020 01:54

bardliao force-pushed the dai-config branch from 28fe1b5 to 1fd7b97 Compare April 20, 2020 01:54

lyakh approved these changes Apr 20, 2020

View reviewed changes

kv2019i approved these changes Apr 21, 2020

View reviewed changes

kv2019i merged commit 7136b5a into thesofproject:topic/sof-dev Apr 21, 2020

Conversation

bardliao commented Apr 6, 2020

Uh oh!

kv2019i left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

plbossart commented Apr 6, 2020

Uh oh!

ranj063 commented Apr 6, 2020

Uh oh!

bardliao commented Apr 7, 2020

Uh oh!

Uh oh!

ranj063 left a comment

Choose a reason for hiding this comment

Uh oh!

kv2019i left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

plbossart commented Apr 7, 2020

Uh oh!

plbossart commented Apr 7, 2020

Uh oh!

RanderWang commented Apr 8, 2020

Uh oh!

ClarexZhou commented Apr 8, 2020

Uh oh!

RanderWang left a comment

Choose a reason for hiding this comment

Uh oh!

lyakh Apr 9, 2020

Choose a reason for hiding this comment

Uh oh!

bardliao Apr 9, 2020

Choose a reason for hiding this comment

Uh oh!

bardliao commented Apr 10, 2020

Uh oh!

kv2019i commented Apr 14, 2020

Uh oh!

bardliao commented Apr 20, 2020

Uh oh!

kv2019i commented Apr 21, 2020

Uh oh!

dbaluta commented Apr 21, 2020

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants