Skip to content

ASoC: SOF: Intel: test w before processing the params_stream callback #5250

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ranj063 merged 1 commit into from
Nov 26, 2024
Merged

ASoC: SOF: Intel: test w before processing the params_stream callback #5250

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions sound/soc/sof/intel/hda-dai.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -489,6 +489,12 @@ int sdw_hda_dai_hw_params(struct snd_pcm_substream *substream,
int ret;
int i;

if (!w) {
dev_err(cpu_dai->dev, "%s widget not found, check amp link num in the topology\n",
cpu_dai->name);
return -EINVAL;
}

ops = hda_dai_get_ops(substream, cpu_dai);
if (!ops) {
dev_err(cpu_dai->dev, "DAI widget ops not set\n");
Expand Down Expand Up @@ -568,6 +574,12 @@ int sdw_hda_dai_hw_params(struct snd_pcm_substream *substream,
*/
for_each_rtd_cpu_dais(rtd, i, dai) {
w = snd_soc_dai_get_widget(dai, substream->stream);
if (!w) {
dev_err(cpu_dai->dev,
"%s widget not found, check amp link num in the topology\n",
dai->name);
return -EINVAL;
Copy link
Collaborator

@ujfalusi ujfalusi Nov 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is no way to catch this during probe?

Copy link
Collaborator Author

@bardliao bardliao Nov 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we can catch this during probe since we need substream->stream. What do you think? @ranj063

Copy link
Collaborator

@ranj063 ranj063 Nov 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's one possibility of making sure that all cpu DAI's associated with all runtimes in the card have their respective stream widgets set. What we can do is in the topology complete op, we can iterate though all of the rtd's and check if they're set and fail if they aren't. Worth a try. Something like this in sof_complete()

struct snd_soc_card *card = scomp->card;
list_for_each_entry(rtd, &card->rtd_list, list)
		for_each_rtd_cpu_dais(rtd, i, cpu_dai)
			if (!snd_soc_dai_get_widget(cpu_dai, stream))
                              return -EINVAL;

A CPU DAI must have atleast the playback/capture widget set. so you'd have to check both directions above

Copy link
Collaborator Author

@bardliao bardliao Nov 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, I can add

        list_for_each_entry(rtd, &card->rtd_list, list)
                for_each_rtd_cpu_dais(rtd, i, cpu_dai)
                        if (!snd_soc_dai_get_widget(cpu_dai, SNDRV_PCM_STREAM_PLAYBACK) &&
                            !snd_soc_dai_get_widget(cpu_dai, SNDRV_PCM_STREAM_CAPTURE)) {
                                dev_err(sdev->dev, "no associated widgets for dai %s\n",
                                        cpu_dai->name);
                                return -EINVAL;
                        }

in sof_complete().
However, it will not return error if a dai has both directions and one direction isn't associated with a widget.

Copy link
Collaborator

@ranj063 ranj063 Nov 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so if we know what directions the CPU DAI has set, can we not check both or either based on that?

Copy link
Collaborator Author

@bardliao bardliao Nov 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The main issue is that the dai links may not always be used by the topology. I.e. topology could be a subset of the dai links. In that case the cpu dai will not associate with any widget and it is still valid.

Copy link
Collaborator

@ranj063 ranj063 Nov 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fair enough. this change looks good then

}
ipc4_copier = widget_to_copier(w);
memcpy(&ipc4_copier->dma_config_tlv[cpu_dai_id], dma_config_tlv,
sizeof(*dma_config_tlv));
Expand Down
5 changes: 5 additions & 0 deletions sound/soc/sof/intel/hda.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,11 @@ static int sdw_params_stream(struct device *dev,
struct snd_soc_dapm_widget *w = snd_soc_dai_get_widget(d, params_data->substream->stream);
struct snd_sof_dai_config_data data = { 0 };

if (!w) {
dev_err(dev, "%s widget not found, check amp link num in the topology\n",
d->name);
return -EINVAL;
}
data.dai_index = (params_data->link_id << 8) | d->id;
data.dai_data = params_data->alh_stream_id;
data.dai_node_id = data.dai_data;
Expand Down
Loading