From 5a7543d0ca01d68d992f480d151efd693807e0ce Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Fri, 3 May 2024 08:32:51 -0500 Subject: [PATCH 1/3] ASoC: SOF: Intel: discard SoundWire configuration if HDaudio codec is reported MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The machine driver and topology selection starts with I2S, then SoundWire and last uses HDaudio as a fallback. That assumes that the ACPI information is correct but there are of course exceptions to the rule. On a Lenovo platform, an external HDaudio codec is detected, but the ACPI tables expose TWO RT711 jack codecs. This patch skips the SoundWire selection in case an external HDaudio codec is detected - which only works with the additional assumption that no one will mix HDaudio and SoundWire. Closes: https://github.com/thesofproject/linux/issues/4962 Reviewed-by: Bard Liao Reviewed-by: Péter Ujfalusi Reviewed-by: Kai Vehmanen Reviewed-by: Ranjani Sridharan Signed-off-by: Pierre-Louis Bossart Link: https://lore.kernel.org/r/20240503133253.108201-2-pierre-louis.bossart@linux.intel.com Signed-off-by: Mark Brown --- sound/soc/sof/intel/hda.c | 9 +++++++-- sound/soc/sof/intel/hda.h | 2 ++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/sound/soc/sof/intel/hda.c b/sound/soc/sof/intel/hda.c index d509cfa38a64..6c10eb2c8241 100644 --- a/sound/soc/sof/intel/hda.c +++ b/sound/soc/sof/intel/hda.c @@ -1809,6 +1809,7 @@ struct snd_soc_acpi_mach *hda_machine_select(struct snd_sof_dev *sdev) u32 interface_mask = hda_get_interface_mask(sdev); struct snd_sof_pdata *sof_pdata = sdev->pdata; const struct sof_dev_desc *desc = sof_pdata->desc; + struct hdac_bus *bus = sof_to_bus(sdev); struct snd_soc_acpi_mach *mach = NULL; enum snd_soc_acpi_intel_codec codec_type; const char *tplg_filename; @@ -1981,8 +1982,12 @@ struct snd_soc_acpi_mach *hda_machine_select(struct snd_sof_dev *sdev) } } - /* If I2S fails, try SoundWire if it is supported */ - if (!mach && (interface_mask & BIT(SOF_DAI_INTEL_ALH))) + /* + * If I2S fails and no external HDaudio codec is detected, + * try SoundWire if it is supported + */ + if (!mach && !HDA_EXT_CODEC(bus->codec_mask) && + (interface_mask & BIT(SOF_DAI_INTEL_ALH))) mach = hda_sdw_machine_select(sdev); /* diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h index 7804c5b50c26..8b495264f23d 100644 --- a/sound/soc/sof/intel/hda.h +++ b/sound/soc/sof/intel/hda.h @@ -454,6 +454,8 @@ #define SSP_SET_SFRM_CONSUMER BIT(24) #define SSP_SET_CBP_CFP (SSP_SET_SCLK_CONSUMER | SSP_SET_SFRM_CONSUMER) +#define HDA_EXT_ADDR 0 +#define HDA_EXT_CODEC(x) ((x) & BIT(HDA_EXT_ADDR)) #define HDA_IDISP_ADDR 2 #define HDA_IDISP_CODEC(x) ((x) & BIT(HDA_IDISP_ADDR)) From b679302526d637700e998da85ed1e06d8f7d3674 Mon Sep 17 00:00:00 2001 From: Peter Ujfalusi Date: Fri, 3 May 2024 08:32:52 -0500 Subject: [PATCH 2/3] ASoC: SOF: ipc4-topology: Allow selective update in sof_ipc4_update_hw_params Add a bitmask parameter to sof_ipc4_update_hw_params() to be able to select the param to be updated. This feature can be used when not all params should be updated, for example if caller only wants to update the format in the params, leaving the channels and rates untouched. Reviewed-by: Seppo Ingalsuo Reviewed-by: Ranjani Sridharan Signed-off-by: Peter Ujfalusi Signed-off-by: Pierre-Louis Bossart Link: https://lore.kernel.org/r/20240503133253.108201-3-pierre-louis.bossart@linux.intel.com Signed-off-by: Mark Brown --- sound/soc/sof/ipc4-topology.c | 83 +++++++++++++++++++++-------------- 1 file changed, 51 insertions(+), 32 deletions(-) diff --git a/sound/soc/sof/ipc4-topology.c b/sound/soc/sof/ipc4-topology.c index a972b2b5122a..f56b7fbbe10f 100644 --- a/sound/soc/sof/ipc4-topology.c +++ b/sound/soc/sof/ipc4-topology.c @@ -1119,42 +1119,50 @@ static int sof_ipc4_widget_assign_instance_id(struct snd_sof_dev *sdev, /* update hw_params based on the audio stream format */ static int sof_ipc4_update_hw_params(struct snd_sof_dev *sdev, struct snd_pcm_hw_params *params, - struct sof_ipc4_audio_format *fmt) + struct sof_ipc4_audio_format *fmt, u32 param_to_update) { - snd_pcm_format_t snd_fmt; struct snd_interval *i; - struct snd_mask *m; - int valid_bits = SOF_IPC4_AUDIO_FORMAT_CFG_V_BIT_DEPTH(fmt->fmt_cfg); - unsigned int channels, rate; - switch (valid_bits) { - case 16: - snd_fmt = SNDRV_PCM_FORMAT_S16_LE; - break; - case 24: - snd_fmt = SNDRV_PCM_FORMAT_S24_LE; - break; - case 32: - snd_fmt = SNDRV_PCM_FORMAT_S32_LE; - break; - default: - dev_err(sdev->dev, "invalid PCM valid_bits %d\n", valid_bits); - return -EINVAL; + if (param_to_update & BIT(SNDRV_PCM_HW_PARAM_FORMAT)) { + int valid_bits = SOF_IPC4_AUDIO_FORMAT_CFG_V_BIT_DEPTH(fmt->fmt_cfg); + snd_pcm_format_t snd_fmt; + struct snd_mask *m; + + switch (valid_bits) { + case 16: + snd_fmt = SNDRV_PCM_FORMAT_S16_LE; + break; + case 24: + snd_fmt = SNDRV_PCM_FORMAT_S24_LE; + break; + case 32: + snd_fmt = SNDRV_PCM_FORMAT_S32_LE; + break; + default: + dev_err(sdev->dev, "invalid PCM valid_bits %d\n", valid_bits); + return -EINVAL; + } + + m = hw_param_mask(params, SNDRV_PCM_HW_PARAM_FORMAT); + snd_mask_none(m); + snd_mask_set_format(m, snd_fmt); } - m = hw_param_mask(params, SNDRV_PCM_HW_PARAM_FORMAT); - snd_mask_none(m); - snd_mask_set_format(m, snd_fmt); + if (param_to_update & BIT(SNDRV_PCM_HW_PARAM_RATE)) { + unsigned int rate = fmt->sampling_frequency; - rate = fmt->sampling_frequency; - i = hw_param_interval(params, SNDRV_PCM_HW_PARAM_RATE); - i->min = rate; - i->max = rate; + i = hw_param_interval(params, SNDRV_PCM_HW_PARAM_RATE); + i->min = rate; + i->max = rate; + } - channels = SOF_IPC4_AUDIO_FORMAT_CFG_CHANNELS_COUNT(fmt->fmt_cfg); - i = hw_param_interval(params, SNDRV_PCM_HW_PARAM_CHANNELS); - i->min = channels; - i->max = channels; + if (param_to_update & BIT(SNDRV_PCM_HW_PARAM_CHANNELS)) { + unsigned int channels = SOF_IPC4_AUDIO_FORMAT_CFG_CHANNELS_COUNT(fmt->fmt_cfg); + + i = hw_param_interval(params, SNDRV_PCM_HW_PARAM_CHANNELS); + i->min = channels; + i->max = channels; + } return 0; } @@ -1844,7 +1852,11 @@ sof_ipc4_prepare_copier_module(struct snd_sof_widget *swidget, } /* modify the input params for the next widget */ - ret = sof_ipc4_update_hw_params(sdev, pipeline_params, &copier_data->out_format); + ret = sof_ipc4_update_hw_params(sdev, pipeline_params, + &copier_data->out_format, + BIT(SNDRV_PCM_HW_PARAM_FORMAT) | + BIT(SNDRV_PCM_HW_PARAM_CHANNELS) | + BIT(SNDRV_PCM_HW_PARAM_RATE)); if (ret) return ret; @@ -2069,7 +2081,10 @@ static int sof_ipc4_prepare_src_module(struct snd_sof_widget *swidget, src->data.sink_rate = out_audio_fmt->sampling_frequency; /* update pipeline_params for sink widgets */ - return sof_ipc4_update_hw_params(sdev, pipeline_params, out_audio_fmt); + return sof_ipc4_update_hw_params(sdev, pipeline_params, out_audio_fmt, + BIT(SNDRV_PCM_HW_PARAM_FORMAT) | + BIT(SNDRV_PCM_HW_PARAM_CHANNELS) | + BIT(SNDRV_PCM_HW_PARAM_RATE)); } static int @@ -2193,7 +2208,11 @@ static int sof_ipc4_prepare_process_module(struct snd_sof_widget *swidget, sizeof(struct sof_ipc4_audio_format)); /* modify the pipeline params with the pin 0 output format */ - ret = sof_ipc4_update_hw_params(sdev, pipeline_params, &process->output_format); + ret = sof_ipc4_update_hw_params(sdev, pipeline_params, + &process->output_format, + BIT(SNDRV_PCM_HW_PARAM_FORMAT) | + BIT(SNDRV_PCM_HW_PARAM_CHANNELS) | + BIT(SNDRV_PCM_HW_PARAM_RATE)); if (ret) return ret; } From f9209644ae7688e82f629e737417bc8916db7b57 Mon Sep 17 00:00:00 2001 From: Peter Ujfalusi Date: Fri, 3 May 2024 08:32:53 -0500 Subject: [PATCH 3/3] ASoC: SOF: ipc4-topology: Correct DAI copier config and NHLT blob request In case of capture and when the DAI copier have single bit depth supported on it's input side we should use this format instead of the one in fe_params. Regardless of the stream direction for the NHLT blob lookup when the DAI copier only supports single bit depth on the DAI side we should only look for a blob which matches with this single configuration. For DMIC if the DAI copier supports multiple bit depths, try to request 32-bit blob first if the requested bit depth is 16-bit. If the 32-bit blob is available then look for marching (32-bit) copier format to make sure that both the blob and copier have correct parameters. Reviewed-by: Seppo Ingalsuo Reviewed-by: Ranjani Sridharan Signed-off-by: Peter Ujfalusi Signed-off-by: Pierre-Louis Bossart Link: https://lore.kernel.org/r/20240503133253.108201-4-pierre-louis.bossart@linux.intel.com Signed-off-by: Mark Brown --- sound/soc/sof/ipc4-topology.c | 162 ++++++++++++++++++++++++++++------ 1 file changed, 136 insertions(+), 26 deletions(-) diff --git a/sound/soc/sof/ipc4-topology.c b/sound/soc/sof/ipc4-topology.c index f56b7fbbe10f..d0125fd1c09f 100644 --- a/sound/soc/sof/ipc4-topology.c +++ b/sound/soc/sof/ipc4-topology.c @@ -1420,13 +1420,16 @@ static int snd_sof_get_hw_config_params(struct snd_sof_dev *sdev, struct snd_sof return 0; } -static int snd_sof_get_nhlt_endpoint_data(struct snd_sof_dev *sdev, struct snd_sof_dai *dai, - struct snd_pcm_hw_params *params, u32 dai_index, - u32 linktype, u8 dir, u32 **dst, u32 *len) +static int +snd_sof_get_nhlt_endpoint_data(struct snd_sof_dev *sdev, struct snd_sof_dai *dai, + bool single_format, + struct snd_pcm_hw_params *params, u32 dai_index, + u32 linktype, u8 dir, u32 **dst, u32 *len) { struct sof_ipc4_fw_data *ipc4_data = sdev->private; struct nhlt_specific_cfg *cfg; int sample_rate, channel_count; + bool format_change = false; int bit_depth, ret; u32 nhlt_type; int dev_type = 0; @@ -1435,9 +1438,18 @@ static int snd_sof_get_nhlt_endpoint_data(struct snd_sof_dev *sdev, struct snd_s switch (linktype) { case SOF_DAI_INTEL_DMIC: nhlt_type = NHLT_LINK_DMIC; - bit_depth = params_width(params); channel_count = params_channels(params); sample_rate = params_rate(params); + bit_depth = params_width(params); + /* + * Look for 32-bit blob first instead of 16-bit if copier + * supports multiple formats + */ + if (bit_depth == 16 && !single_format) { + dev_dbg(sdev->dev, "Looking for 32-bit blob first for DMIC\n"); + format_change = true; + bit_depth = 32; + } break; case SOF_DAI_INTEL_SSP: nhlt_type = NHLT_LINK_SSP; @@ -1471,22 +1483,56 @@ static int snd_sof_get_nhlt_endpoint_data(struct snd_sof_dev *sdev, struct snd_s dir, dev_type); if (!cfg) { + if (format_change) { + /* + * The 32-bit blob was not found in NHLT table, try to + * look for one based on the params + */ + bit_depth = params_width(params); + format_change = false; + + cfg = intel_nhlt_get_endpoint_blob(sdev->dev, ipc4_data->nhlt, + dai_index, nhlt_type, + bit_depth, bit_depth, + channel_count, sample_rate, + dir, dev_type); + if (cfg) + goto out; + } + dev_err(sdev->dev, "no matching blob for sample rate: %d sample width: %d channels: %d\n", sample_rate, bit_depth, channel_count); return -EINVAL; } +out: /* config length should be in dwords */ *len = cfg->size >> 2; *dst = (u32 *)cfg->caps; + if (format_change) { + /* + * Update the params to reflect that we have loaded 32-bit blob + * instead of the 16-bit. + * This information is going to be used by the caller to find + * matching copier format on the dai side. + */ + struct snd_mask *m; + + m = hw_param_mask(params, SNDRV_PCM_HW_PARAM_FORMAT); + snd_mask_none(m); + snd_mask_set_format(m, SNDRV_PCM_FORMAT_S32_LE); + } + return 0; } #else -static int snd_sof_get_nhlt_endpoint_data(struct snd_sof_dev *sdev, struct snd_sof_dai *dai, - struct snd_pcm_hw_params *params, u32 dai_index, - u32 linktype, u8 dir, u32 **dst, u32 *len) +static int +snd_sof_get_nhlt_endpoint_data(struct snd_sof_dev *sdev, struct snd_sof_dai *dai, + bool single_format, + struct snd_pcm_hw_params *params, u32 dai_index, + u32 linktype, u8 dir, u32 **dst, u32 *len) { return 0; } @@ -1517,6 +1563,68 @@ bool sof_ipc4_copier_is_single_format(struct snd_sof_dev *sdev, return true; } +static int +sof_ipc4_prepare_dai_copier(struct snd_sof_dev *sdev, struct snd_sof_dai *dai, + struct snd_pcm_hw_params *params, int dir) +{ + struct sof_ipc4_available_audio_format *available_fmt; + struct snd_pcm_hw_params dai_params = *params; + struct sof_ipc4_copier_data *copier_data; + struct sof_ipc4_copier *ipc4_copier; + bool single_format; + int ret; + + ipc4_copier = dai->private; + copier_data = &ipc4_copier->data; + available_fmt = &ipc4_copier->available_fmt; + + /* + * If the copier on the DAI side supports only single bit depth then + * this depth (format) should be used to look for the NHLT blob (if + * needed) and in case of capture this should be used for the input + * format lookup + */ + if (dir == SNDRV_PCM_STREAM_PLAYBACK) { + single_format = sof_ipc4_copier_is_single_format(sdev, + available_fmt->output_pin_fmts, + available_fmt->num_output_formats); + + /* Update the dai_params with the only supported format */ + if (single_format) { + ret = sof_ipc4_update_hw_params(sdev, &dai_params, + &available_fmt->output_pin_fmts[0].audio_fmt, + BIT(SNDRV_PCM_HW_PARAM_FORMAT)); + if (ret) + return ret; + } + } else { + single_format = sof_ipc4_copier_is_single_format(sdev, + available_fmt->input_pin_fmts, + available_fmt->num_input_formats); + + /* Update the dai_params with the only supported format */ + if (single_format) { + ret = sof_ipc4_update_hw_params(sdev, &dai_params, + &available_fmt->input_pin_fmts[0].audio_fmt, + BIT(SNDRV_PCM_HW_PARAM_FORMAT)); + if (ret) + return ret; + } + } + + ret = snd_sof_get_nhlt_endpoint_data(sdev, dai, single_format, + &dai_params, + ipc4_copier->dai_index, + ipc4_copier->dai_type, dir, + &ipc4_copier->copier_config, + &copier_data->gtw_cfg.config_length); + /* Update the params to reflect the changes made in this function */ + if (!ret) + *params = dai_params; + + return ret; +} + static int sof_ipc4_prepare_copier_module(struct snd_sof_widget *swidget, struct snd_pcm_hw_params *fe_params, @@ -1527,7 +1635,7 @@ sof_ipc4_prepare_copier_module(struct snd_sof_widget *swidget, struct snd_soc_component *scomp = swidget->scomp; struct snd_sof_dev *sdev = snd_soc_component_get_drvdata(scomp); struct sof_ipc4_copier_data *copier_data; - struct snd_pcm_hw_params *ref_params; + struct snd_pcm_hw_params ref_params; struct sof_ipc4_copier *ipc4_copier; struct snd_sof_dai *dai; u32 gtw_cfg_config_length; @@ -1605,9 +1713,9 @@ sof_ipc4_prepare_copier_module(struct snd_sof_widget *swidget, * for capture. */ if (dir == SNDRV_PCM_STREAM_PLAYBACK) - ref_params = fe_params; + ref_params = *fe_params; else - ref_params = pipeline_params; + ref_params = *pipeline_params; copier_data->gtw_cfg.node_id &= ~SOF_IPC4_NODE_INDEX_MASK; copier_data->gtw_cfg.node_id |= @@ -1633,23 +1741,25 @@ sof_ipc4_prepare_copier_module(struct snd_sof_widget *swidget, available_fmt = &ipc4_copier->available_fmt; /* - * When there is format conversion within a pipeline, the number of supported - * output formats is typically limited to just 1 for the DAI copiers. But when there - * is no format conversion, the DAI copiers input format must match that of the - * FE hw_params for capture and the pipeline params for playback. + * Use the fe_params as a base for the copier configuration. + * The ref_params might get updated to reflect what format is + * supported by the copier on the DAI side. + * + * In case of capture the ref_params returned will be used to + * find the input configuration of the copier. */ - if (dir == SNDRV_PCM_STREAM_PLAYBACK) - ref_params = pipeline_params; - else - ref_params = fe_params; - - ret = snd_sof_get_nhlt_endpoint_data(sdev, dai, fe_params, ipc4_copier->dai_index, - ipc4_copier->dai_type, dir, - &ipc4_copier->copier_config, - &copier_data->gtw_cfg.config_length); + ref_params = *fe_params; + ret = sof_ipc4_prepare_dai_copier(sdev, dai, &ref_params, dir); if (ret < 0) return ret; + /* + * For playback the pipeline_params needs to be used to find the + * input configuration of the copier. + */ + if (dir == SNDRV_PCM_STREAM_PLAYBACK) + ref_params = *pipeline_params; + break; } case snd_soc_dapm_buffer: @@ -1657,7 +1767,7 @@ sof_ipc4_prepare_copier_module(struct snd_sof_widget *swidget, ipc4_copier = (struct sof_ipc4_copier *)swidget->private; copier_data = &ipc4_copier->data; available_fmt = &ipc4_copier->available_fmt; - ref_params = pipeline_params; + ref_params = *pipeline_params; break; } @@ -1668,8 +1778,8 @@ sof_ipc4_prepare_copier_module(struct snd_sof_widget *swidget, } /* set input and output audio formats */ - ret = sof_ipc4_init_input_audio_fmt(sdev, swidget, &copier_data->base_config, ref_params, - available_fmt); + ret = sof_ipc4_init_input_audio_fmt(sdev, swidget, &copier_data->base_config, + &ref_params, available_fmt); if (ret < 0) return ret;