ASoC: SOF: intel: hda-dai: Remove stream name from dai driver by bardliao · Pull Request #1874 · thesofproject/linux

bardliao · 2020-03-09T05:38:37Z

snd_soc_dapm_new_dai_widgets() will create a dai and link it to
dai->playback_widget/capture_widget if stream name is defined.
However, we want to make sure playback_widget/capture_widget will
be created by topology.

Signed-off-by: Bard Liao yung-chuan.liao@linux.intel.com

snd_soc_dapm_new_dai_widgets() will create a dai and link it to dai->playback_widget/capture_widget if stream name is defined. However, we want to make sure playback_widget/capture_widget will be created by topology. Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>

kv2019i · 2020-03-09T11:22:05Z

sound/soc/sof/intel/hda-dai.c

 	.name = "SSP0 Pin",
 	.playback = {
-		.stream_name = "ssp0 Tx",
 		.channels_min = 1,


@bardliao The patch is good, but I'd like a bit more about the problem this is solving in commit message. I.e. why do we want to make sure the widgets are created by topology?

plbossart

I am with @kv2019i I don't quite get the issue.

We've lived with stream names for a very long time, so if there a change in the ASoC core that creates a problem? Or is this the Soundwire-related changes that force this change as well?

ranj063 · 2020-03-09T15:55:54Z

I am with @kv2019i I don't quite get the issue.

We've lived with stream names for a very long time, so if there a change in the ASoC core that creates a problem? Or is this the Soundwire-related changes that force this change as well?

@plbossart this change is similar to what we did for SDW in 8046453

ranj063

LGTM but wondering if we need a fixes tag? or does this need to be squashed before sending upstream?

plbossart · 2020-03-09T16:12:16Z

I am with @kv2019i I don't quite get the issue.
We've lived with stream names for a very long time, so if there a change in the ASoC core that creates a problem? Or is this the Soundwire-related changes that force this change as well?

@plbossart this change is similar to what we did for SDW in 8046453

Right, but is it necessary for SSP? What's broken at the moment?

ranj063 · 2020-03-09T16:47:01Z

I am with @kv2019i I don't quite get the issue.
We've lived with stream names for a very long time, so if there a change in the ASoC core that creates a problem? Or is this the Soundwire-related changes that force this change as well?

@plbossart this change is similar to what we did for SDW in 8046453

Right, but is it necessary for SSP? What's broken at the moment?

@plbossart snd_soc_dapm_new_dai_widgets() will overwrite the playback/capture widget with a newly created widget with the stream name in the DAI driver, if the stream name is set. If not, the capture/playback widgets set by sof_connect_dai_widget() during topology loading will be retained and in our case, this is the right thing to do I think.

ranj063 · 2020-03-09T17:14:57Z

I am with @kv2019i I don't quite get the issue.
We've lived with stream names for a very long time, so if there a change in the ASoC core that creates a problem? Or is this the Soundwire-related changes that force this change as well?

@plbossart this change is similar to what we did for SDW in 8046453

Right, but is it necessary for SSP? What's broken at the moment?

@plbossart snd_soc_dapm_new_dai_widgets() will overwrite the playback/capture widget with a newly created widget with the stream name in the DAI driver, if the stream name is set. If not, the capture/playback widgets set by sof_connect_dai_widget() during topology loading will be retained and in our case, this is the right thing to do I think.

And oh to answer your question fully, I think @bardliao trying to fix the debugfs error seen as below:

"debugfs: File 'iDisp3 TX' in directory 'dapm' already present!"

plbossart · 2020-03-09T19:41:50Z

I am with @kv2019i I don't quite get the issue.
We've lived with stream names for a very long time, so if there a change in the ASoC core that creates a problem? Or is this the Soundwire-related changes that force this change as well?

@plbossart this change is similar to what we did for SDW in 8046453

Right, but is it necessary for SSP? What's broken at the moment?

@plbossart snd_soc_dapm_new_dai_widgets() will overwrite the playback/capture widget with a newly created widget with the stream name in the DAI driver, if the stream name is set. If not, the capture/playback widgets set by sof_connect_dai_widget() during topology loading will be retained and in our case, this is the right thing to do I think.

And oh to answer your question fully, I think @bardliao trying to fix the debugfs error seen as below:

"debugfs: File 'iDisp3 TX' in directory 'dapm' already present!"

I can't recall having seen this error, or it was a very long time ago?

Let's @bardliao answer and see tomorrow

bardliao · 2020-03-10T02:27:19Z

Thanks @kv2019i @plbossart @ranj063 Please see my answers below.

We've lived with stream names for a very long time, so if there a change in the ASoC core that creates a problem? Or is this the Soundwire-related changes that force this change as well?

We didn't have stream name until c12e84e.

LGTM but wondering if we need a fixes tag? or does this need to be squashed before sending upstream?

So, yes, this need to be squashed before sending upstream.

Right, but is it necessary for SSP? What's broken at the moment?

#1863 will test if cpu_dai->playback_widget or cpu_dai->capture_widget exist. We won't assign the widget which we created from topology to cpu_dai->playback_widget or cpu_dai->capture_widget if cpu_dai->playback_widget or cpu_dai->capture_widget already exists. That's why we need this PR.

ranj063 · 2020-03-10T04:45:03Z

debugfs: File 'iDisp3 TX' in directory 'dapm' already present

@bardliao I'm seeing these on my mantis. Does your patch fix these?

bardliao · 2020-03-10T05:41:53Z

debugfs: File 'iDisp3 TX' in directory 'dapm' already present

@bardliao I'm seeing these on my mantis. Does your patch fix these?

@ranj063 Sorry, I don't see this log on my side.

ranj063 · 2020-03-10T16:30:04Z

debugfs: File 'iDisp3 TX' in directory 'dapm' already present

@bardliao I'm seeing these on my mantis. Does your patch fix these?

@ranj063 Sorry, I don't see this log on my side.

@bardliao I cant be the only one seeing these debugfs errors and I can confirm that your patch does fix these errors. @kv2019i could you please check on your side?

ranj063 · 2020-03-11T15:31:40Z

@kv2019i are you OK with this one now?

lyakh · 2020-03-12T08:00:02Z

debugfs: File 'iDisp3 TX' in directory 'dapm' already present

@bardliao I'm seeing these on my mantis. Does your patch fix these?

@ranj063 Sorry, I don't see this log on my side.

@bardliao I cant be the only one seeing these debugfs errors and I can confirm that your patch does fix these errors. @kv2019i could you please check on your side?

@ranj063 fixes them for me too

kv2019i

Thanks for clarifications @bardliao and @ranj063 . We should probably squash this patch before sending upstream.

bardliao requested review from kv2019i, plbossart and ranj063 March 9, 2020 05:38

bardliao requested review from dbaluta and lgirdwood as code owners March 9, 2020 05:38

kv2019i requested changes Mar 9, 2020

View reviewed changes

plbossart requested changes Mar 9, 2020

View reviewed changes

ranj063 approved these changes Mar 9, 2020

View reviewed changes

plbossart approved these changes Mar 10, 2020

View reviewed changes

lyakh approved these changes Mar 12, 2020

View reviewed changes

kv2019i approved these changes Mar 12, 2020

View reviewed changes

kv2019i merged commit 27926d2 into thesofproject:topic/sof-dev Mar 12, 2020

Conversation

bardliao commented Mar 9, 2020

Uh oh!

kv2019i Mar 9, 2020

Choose a reason for hiding this comment

Uh oh!

plbossart left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ranj063 commented Mar 9, 2020

Uh oh!

ranj063 left a comment

Choose a reason for hiding this comment

Uh oh!

plbossart commented Mar 9, 2020

Uh oh!

ranj063 commented Mar 9, 2020

Uh oh!

ranj063 commented Mar 9, 2020

Uh oh!

plbossart commented Mar 9, 2020

Uh oh!

bardliao commented Mar 10, 2020

Uh oh!

ranj063 commented Mar 10, 2020

Uh oh!

bardliao commented Mar 10, 2020

Uh oh!

ranj063 commented Mar 10, 2020

Uh oh!

ranj063 commented Mar 11, 2020

Uh oh!

lyakh commented Mar 12, 2020

Uh oh!

kv2019i left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

plbossart left a comment •

edited

Loading