Detect SSP port number from NHLT table #4492
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1297,8 +1297,9 @@ static int snd_sof_get_nhlt_endpoint_data(struct snd_sof_dev *sdev, struct snd_s | |
| struct sof_ipc4_fw_data *ipc4_data = sdev->private; | ||
| struct nhlt_specific_cfg *cfg; | ||
| int sample_rate, channel_count; | ||
|
There was a problem hiding this comment. commit message s/direcion/direction |
||
| int bit_depth, ret, i; | ||
| u32 nhlt_type; | ||
| static const u8 nhlt_devices[] = {NHLT_DEVICE_I2S, NHLT_DEVICE_BT}; | ||
|
|
||
| /* convert to NHLT type */ | ||
| switch (linktype) { | ||
|
|
@@ -1323,9 +1324,15 @@ static int snd_sof_get_nhlt_endpoint_data(struct snd_sof_dev *sdev, struct snd_s | |
| dai_index, nhlt_type, dir); | ||
|
|
||
| /* find NHLT blob with matching params */ | ||
| for (i = 0; i < ARRAY_SIZE(nhlt_devices); i++) { | ||
| cfg = intel_nhlt_get_endpoint_blob(sdev->dev, ipc4_data->nhlt, | ||
| dai_index, nhlt_type, bit_depth, | ||
| bit_depth, channel_count, sample_rate, | ||
| dir, nhlt_devices[i]); | ||
| if (cfg || (linktype == SOF_DAI_INTEL_DMIC)) | ||
| goto cfg_found; | ||
| } | ||
| cfg_found: | ||
|
There was a problem hiding this comment. the main issue I have is that this is not backwards compatible. You need a new topology to get this to work. Is this not too late already? @ranj063 please chime in here. There was a problem hiding this comment. It's backward compatible. Because current implementation set the device type of all endpoints to 0 (BT), and use Render(0) and Capture(1) in direction field. So current topologies could work with the code. |
||
|
|
||
| if (!cfg) { | ||
| dev_err(sdev->dev, | ||
|
|
@@ -2725,6 +2732,78 @@ static int sof_ipc4_dai_config(struct snd_sof_dev *sdev, struct snd_sof_widget * | |
| return 0; | ||
| } | ||
|
|
||
| static int sof_ipc4_nhlt_register_dai_links(struct snd_soc_component *scomp) | ||
|
There was a problem hiding this comment. adding dai links in the SOF ipc4 code is a major layering violation. This belongs in machine drivers IMHO. The SOF driver should only add CPU dais, not muck with dai links. There was a problem hiding this comment. The problem is topology is loaded after machine driver registers soundcard. If we want to extract ssp numbers from NHLT, then we have to register the dai links in sof-topology layer... There was a problem hiding this comment. it's a bit too much for me... I am not hot on hard-coding such names in a generic layer. need to think a bit more on all this. |
||
| { | ||
| struct snd_sof_dev *sdev = snd_soc_component_get_drvdata(scomp); | ||
| struct sof_ipc4_fw_data *ipc4_data = sdev->private; | ||
| struct snd_soc_card *card = scomp->card; | ||
| struct snd_soc_dai_link *dai_link = card->dai_link; | ||
| int capture_mask, render_mask; | ||
| int ssp_mask, ssp_num; | ||
| int i, is_bt, ret; | ||
|
|
||
| capture_mask = intel_nhlt_ssp_dir_mask(scomp->dev, ipc4_data->nhlt, NHLT_DIRECTION_CAPTURE); | ||
| render_mask = intel_nhlt_ssp_dir_mask(scomp->dev, ipc4_data->nhlt, NHLT_DIRECTION_RENDER); | ||
|
|
||
| for (i = 0; i < card->num_links; i++, dai_link++) { | ||
| if (!strcmp(dai_link->name, "NHLT-RENDER-CAPTURE")) { | ||
| /* headset codec */ | ||
| ssp_mask = render_mask & capture_mask; | ||
| is_bt = 0; | ||
| } else if (!strcmp(dai_link->name, "NHLT-RENDER-ONLY")) { | ||
| /* speaker amplifier */ | ||
| ssp_mask = render_mask & ~capture_mask; | ||
| is_bt = 0; | ||
| } else if (!strcmp(dai_link->name, "NHLT-CAPTURE-ONLY")) { | ||
| /* HDMI in */ | ||
| ssp_mask = ~render_mask & capture_mask; | ||
| is_bt = 0; | ||
| } else if (!strcmp(dai_link->name, "NHLT-BT")) { | ||
| /* bluetooth sideband */ | ||
| ssp_mask = intel_nhlt_ssp_endpoint_mask(ipc4_data->nhlt, | ||
| NHLT_DEVICE_BT); | ||
| is_bt = 1; | ||
| } else | ||
| continue; | ||
|
|
||
| if (hweight32(ssp_mask) != 1) { | ||
| dev_warn(scomp->dev, "Invalid mask 0x%x for dai link %s\n", | ||
| ssp_mask, dai_link->name); | ||
| continue; | ||
| } | ||
|
|
||
| ssp_num = ffs(ssp_mask) - 1; /* ffs returns 1-based result */ | ||
|
|
||
| if (is_bt) | ||
| dai_link->name = devm_kasprintf(card->dev, GFP_KERNEL, | ||
| "SSP%d-BT", ssp_num); | ||
| else | ||
| dai_link->name = devm_kasprintf(card->dev, GFP_KERNEL, | ||
| "SSP%d-Codec", ssp_num); | ||
| if (!dai_link->name) | ||
| return -ENOMEM; | ||
|
|
||
| /* stream_name is initialized in soc_check_tplg_fes() function | ||
| * with dai link name */ | ||
| dai_link->stream_name = devm_kstrdup(card->dev, dai_link->name, | ||
| GFP_KERNEL); | ||
| if (!dai_link->stream_name) | ||
| return -ENOMEM; | ||
|
|
||
| dai_link->cpus->dai_name = devm_kasprintf(card->dev, GFP_KERNEL, | ||
| "SSP%d Pin", ssp_num); | ||
| if (!dai_link->cpus->dai_name) | ||
| return -ENOMEM; | ||
|
|
||
| dai_link->ignore = false; | ||
| ret = snd_soc_add_pcm_runtimes(card, dai_link, 1); | ||
| if (ret < 0) | ||
| return ret; | ||
| } | ||
|
|
||
| return 0; | ||
| } | ||
|
|
||
| static int sof_ipc4_parse_manifest(struct snd_soc_component *scomp, int index, | ||
| struct snd_soc_tplg_manifest *man) | ||
| { | ||
|
|
@@ -2735,7 +2814,7 @@ static int sof_ipc4_parse_manifest(struct snd_soc_component *scomp, int index, | |
| u32 size = le32_to_cpu(man->priv.size); | ||
| u8 *man_ptr = man->priv.data; | ||
| u32 len_check; | ||
| int i, ret; | ||
|
|
||
| if (!size || size < SOF_IPC4_TPLG_ABI_SIZE) { | ||
| dev_err(scomp->dev, "%s: Invalid topology ABI size: %u\n", | ||
|
|
@@ -2773,6 +2852,13 @@ static int sof_ipc4_parse_manifest(struct snd_soc_component *scomp, int index, | |
| le32_to_cpu(manifest_tlv->size), GFP_KERNEL); | ||
| if (!ipc4_data->nhlt) | ||
| return -ENOMEM; | ||
|
|
||
| /* register 'ignored' dai links which link name and | ||
| * codec dai name need fixup with info from NHLT table | ||
| */ | ||
| ret = sof_ipc4_nhlt_register_dai_links(scomp); | ||
| if (ret) | ||
| return ret; | ||
| break; | ||
| default: | ||
| dev_warn(scomp->dev, "Skipping unknown manifest data type %d\n", | ||
|
|
||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know what this 'ignore' means but it certainly does not immediately translate as 'fixup and register later'.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
soc-core(soc_check_tplg_fes() function) uses this flag to skip FE dai links in machine driver. Here we use this flag to make snd_soc_add_pcm_runtime() function skipping this dai link.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"borrow this flag" really means overloading a concept used for FE and extend it for BEs. That's not so good, this will add confusions left and right. We should be really careful with such moves.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I checked the soc.h and the flag means "do not create pcm". Maybe the reason to use this flag could be different? soc-core uses this flag to skip FE links and we use it to skip links which link/stream names need to be resolved later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's best to avoid using the same flag for two completely different purposes, which means adding a new one with a clearly defined behavior.
I am still in the dark on what the problem statement is and why the SOF driver is involved.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree we cannot repurpose an existing flag for a compltely different purpose