Skip to content

Commit

Permalink
ASoC: SOF: ipc4-topology: Fix nhlt configuration blob
Browse files Browse the repository at this point in the history 
Merge series from Peter Ujfalusi <peter.ujfalusi@linux.intel.com>:

The existing logic to pick a DMIC blob is based on several historical
assumptions that the NHLT in BIOS always contains 32-bits per sample
type (first patch, [1]).

The other issue with the existing logic is that it was designed to care only
about the bit depth of the format and fails to find the existing and correct
blob when rate/channels are different on the FE side compared to what we should
be using on the DAI side (we have components in path which can change
rate/channel count).

These issues have not been observed in past but with new MTL based (Windows)
laptops and new topologies to enhance the audio quality, we started to see weird
issues around how our assumptions of vendors failed.

Since some NHLT blob handling cleanup has been done for 6.10, this series will
complete that work to cover even cases that we don't anticipate to see.

[1] thesofproject#4973
  • Loading branch information
@broonie
broonie committed May 30, 2024
2 parents ba2e832 + b65456b commit c85578e
Show file tree
Hide file tree
Showing 3 changed files with 115 additions and 58 deletions.
12 changes: 6 additions & 6 deletions sound/soc/sof/ipc4-pcm.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -650,7 +650,7 @@ static int sof_ipc4_pcm_dai_link_fixup(struct snd_soc_pcm_runtime *rtd,
struct snd_soc_dai *cpu_dai = snd_soc_rtd_to_cpu(rtd, 0);
struct sof_ipc4_audio_format *ipc4_fmt;
struct sof_ipc4_copier *ipc4_copier;
bool single_fmt = false;
bool single_bitdepth = false;
u32 valid_bits = 0;
int dir, ret;

Expand Down Expand Up @@ -682,18 +682,18 @@ static int sof_ipc4_pcm_dai_link_fixup(struct snd_soc_pcm_runtime *rtd,
return 0;

if (dir == SNDRV_PCM_STREAM_PLAYBACK) {
if (sof_ipc4_copier_is_single_format(sdev,
if (sof_ipc4_copier_is_single_bitdepth(sdev,
available_fmt->output_pin_fmts,
available_fmt->num_output_formats)) {
ipc4_fmt = &available_fmt->output_pin_fmts->audio_fmt;
single_fmt = true;
single_bitdepth = true;
}
} else {
if (sof_ipc4_copier_is_single_format(sdev,
if (sof_ipc4_copier_is_single_bitdepth(sdev,
available_fmt->input_pin_fmts,
available_fmt->num_input_formats)) {
ipc4_fmt = &available_fmt->input_pin_fmts->audio_fmt;
single_fmt = true;
single_bitdepth = true;
}
}
}
Expand All @@ -703,7 +703,7 @@ static int sof_ipc4_pcm_dai_link_fixup(struct snd_soc_pcm_runtime *rtd,
if (ret)
return ret;

if (single_fmt) {
if (single_bitdepth) {
snd_mask_none(fmt);
valid_bits = SOF_IPC4_AUDIO_FORMAT_CFG_V_BIT_DEPTH(ipc4_fmt->fmt_cfg);
dev_dbg(component->dev, "Set %s to %d bit format\n", dai->name, valid_bits);
Expand Down
155 changes: 106 additions & 49 deletions sound/soc/sof/ipc4-topology.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -195,9 +195,10 @@ static void sof_ipc4_dbg_audio_format(struct device *dev, struct sof_ipc4_pin_fo
for (i = 0; i < num_formats; i++) {
struct sof_ipc4_audio_format *fmt = &pin_fmt[i].audio_fmt;
dev_dbg(dev,
"Pin index #%d: %uHz, %ubit (ch_map %#x ch_cfg %u interleaving_style %u fmt_cfg %#x) buffer size %d\n",
pin_fmt[i].pin_index, fmt->sampling_frequency, fmt->bit_depth, fmt->ch_map,
fmt->ch_cfg, fmt->interleaving_style, fmt->fmt_cfg,
"Pin index #%d: %uHz, %ubit, %luch (ch_map %#x ch_cfg %u interleaving_style %u fmt_cfg %#x) buffer size %d\n",
pin_fmt[i].pin_index, fmt->sampling_frequency, fmt->bit_depth,
SOF_IPC4_AUDIO_FORMAT_CFG_CHANNELS_COUNT(fmt->fmt_cfg),
fmt->ch_map, fmt->ch_cfg, fmt->interleaving_style, fmt->fmt_cfg,
pin_fmt[i].buffer_size);
}
}
Expand Down Expand Up @@ -1430,7 +1431,7 @@ static int snd_sof_get_hw_config_params(struct snd_sof_dev *sdev, struct snd_sof

static int
snd_sof_get_nhlt_endpoint_data(struct snd_sof_dev *sdev, struct snd_sof_dai *dai,
bool single_format,
bool single_bitdepth,
struct snd_pcm_hw_params *params, u32 dai_index,
u32 linktype, u8 dir, u32 **dst, u32 *len)
{
Expand All @@ -1453,7 +1454,7 @@ snd_sof_get_nhlt_endpoint_data(struct snd_sof_dev *sdev, struct snd_sof_dai *dai
* Look for 32-bit blob first instead of 16-bit if copier
* supports multiple formats
*/
if (bit_depth == 16 && !single_format) {
if (bit_depth == 16 && !single_bitdepth) {
dev_dbg(sdev->dev, "Looking for 32-bit blob first for DMIC\n");
format_change = true;
bit_depth = 32;
Expand Down Expand Up @@ -1491,14 +1492,29 @@ snd_sof_get_nhlt_endpoint_data(struct snd_sof_dev *sdev, struct snd_sof_dai *dai
dir, dev_type);

if (!cfg) {
bool get_new_blob = false;

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;
get_new_blob = true;
} else if (linktype == SOF_DAI_INTEL_DMIC && !single_bitdepth) {
/*
* The requested 32-bit blob (no format change for the
* blob request) was not found in NHLT table, try to
* look for 16-bit blob if the copier supports multiple
* formats
*/
bit_depth = 16;
format_change = true;
get_new_blob = true;
}

if (get_new_blob) {
cfg = intel_nhlt_get_endpoint_blob(sdev->dev, ipc4_data->nhlt,
dai_index, nhlt_type,
bit_depth, bit_depth,
Expand All @@ -1521,34 +1537,38 @@ snd_sof_get_nhlt_endpoint_data(struct snd_sof_dev *sdev, struct snd_sof_dai *dai

if (format_change) {
/*
* Update the params to reflect that we have loaded 32-bit blob
* instead of the 16-bit.
* Update the params to reflect that different blob was loaded
* instead of the requested bit depth (16 -> 32 or 32 -> 16).
* 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);
if (bit_depth == 16)
snd_mask_set_format(m, SNDRV_PCM_FORMAT_S16_LE);
else
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,
bool single_format,
bool single_bitdepth,
struct snd_pcm_hw_params *params, u32 dai_index,
u32 linktype, u8 dir, u32 **dst, u32 *len)
{
return 0;
}
#endif

bool sof_ipc4_copier_is_single_format(struct snd_sof_dev *sdev,
struct sof_ipc4_pin_format *pin_fmts,
u32 pin_fmts_size)
bool sof_ipc4_copier_is_single_bitdepth(struct snd_sof_dev *sdev,
struct sof_ipc4_pin_format *pin_fmts,
u32 pin_fmts_size)
{
struct sof_ipc4_audio_format *fmt;
u32 valid_bits;
Expand All @@ -1571,56 +1591,93 @@ bool sof_ipc4_copier_is_single_format(struct snd_sof_dev *sdev,
return true;
}

static int
sof_ipc4_adjust_params_to_dai_format(struct snd_sof_dev *sdev,
struct snd_pcm_hw_params *params,
struct sof_ipc4_pin_format *pin_fmts,
u32 pin_fmts_size)
{
u32 params_mask = BIT(SNDRV_PCM_HW_PARAM_RATE) |
BIT(SNDRV_PCM_HW_PARAM_CHANNELS) |
BIT(SNDRV_PCM_HW_PARAM_FORMAT);
struct sof_ipc4_audio_format *fmt;
u32 rate, channels, valid_bits;
int i;

fmt = &pin_fmts[0].audio_fmt;
rate = fmt->sampling_frequency;
channels = SOF_IPC4_AUDIO_FORMAT_CFG_CHANNELS_COUNT(fmt->fmt_cfg);
valid_bits = SOF_IPC4_AUDIO_FORMAT_CFG_V_BIT_DEPTH(fmt->fmt_cfg);

/* check if parameters in topology defined formats are the same */
for (i = 1; i < pin_fmts_size; i++) {
u32 val;

fmt = &pin_fmts[i].audio_fmt;

if (params_mask & BIT(SNDRV_PCM_HW_PARAM_RATE)) {
val = fmt->sampling_frequency;
if (val != rate)
params_mask &= ~BIT(SNDRV_PCM_HW_PARAM_RATE);
}
if (params_mask & BIT(SNDRV_PCM_HW_PARAM_CHANNELS)) {
val = SOF_IPC4_AUDIO_FORMAT_CFG_CHANNELS_COUNT(fmt->fmt_cfg);
if (val != channels)
params_mask &= ~BIT(SNDRV_PCM_HW_PARAM_CHANNELS);
}
if (params_mask & BIT(SNDRV_PCM_HW_PARAM_FORMAT)) {
val = SOF_IPC4_AUDIO_FORMAT_CFG_V_BIT_DEPTH(fmt->fmt_cfg);
if (val != valid_bits)
params_mask &= ~BIT(SNDRV_PCM_HW_PARAM_FORMAT);
}
}

if (params_mask)
return sof_ipc4_update_hw_params(sdev, params,
&pin_fmts[0].audio_fmt,
params_mask);

return 0;
}

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_pin_format *pin_fmts;
struct sof_ipc4_copier *ipc4_copier;
bool single_format;
bool single_bitdepth;
u32 num_pin_fmts;
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
* Fixup the params based on the format parameters of the DAI. If any
* of the RATE, CHANNELS, bit depth is static among the formats then
* narrow the params to only allow that specific parameter value.
*/
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;
}
pin_fmts = available_fmt->output_pin_fmts;
num_pin_fmts = available_fmt->num_output_formats;
} 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;
}
pin_fmts = available_fmt->input_pin_fmts;
num_pin_fmts = available_fmt->num_input_formats;
}

ret = snd_sof_get_nhlt_endpoint_data(sdev, dai, single_format,
ret = sof_ipc4_adjust_params_to_dai_format(sdev, &dai_params, pin_fmts,
num_pin_fmts);
if (ret)
return ret;

single_bitdepth = sof_ipc4_copier_is_single_bitdepth(sdev, pin_fmts,
num_pin_fmts);
ret = snd_sof_get_nhlt_endpoint_data(sdev, dai, single_bitdepth,
&dai_params,
ipc4_copier->dai_index,
ipc4_copier->dai_type, dir,
Expand Down Expand Up @@ -1655,7 +1712,7 @@ sof_ipc4_prepare_copier_module(struct snd_sof_widget *swidget,
u32 out_ref_rate, out_ref_channels;
u32 deep_buffer_dma_ms = 0;
int output_fmt_index;
bool single_output_format;
bool single_output_bitdepth;
int i;

dev_dbg(sdev->dev, "copier %s, type %d", swidget->widget->name, swidget->id);
Expand Down Expand Up @@ -1792,9 +1849,9 @@ sof_ipc4_prepare_copier_module(struct snd_sof_widget *swidget,
return ret;

/* set the reference params for output format selection */
single_output_format = sof_ipc4_copier_is_single_format(sdev,
available_fmt->output_pin_fmts,
available_fmt->num_output_formats);
single_output_bitdepth = sof_ipc4_copier_is_single_bitdepth(sdev,
available_fmt->output_pin_fmts,
available_fmt->num_output_formats);
switch (swidget->id) {
case snd_soc_dapm_aif_in:
case snd_soc_dapm_dai_out:
Expand All @@ -1806,7 +1863,7 @@ sof_ipc4_prepare_copier_module(struct snd_sof_widget *swidget,
out_ref_rate = in_fmt->sampling_frequency;
out_ref_channels = SOF_IPC4_AUDIO_FORMAT_CFG_CHANNELS_COUNT(in_fmt->fmt_cfg);

if (!single_output_format)
if (!single_output_bitdepth)
out_ref_valid_bits =
SOF_IPC4_AUDIO_FORMAT_CFG_V_BIT_DEPTH(in_fmt->fmt_cfg);
break;
Expand All @@ -1815,7 +1872,7 @@ sof_ipc4_prepare_copier_module(struct snd_sof_widget *swidget,
case snd_soc_dapm_dai_in:
out_ref_rate = params_rate(fe_params);
out_ref_channels = params_channels(fe_params);
if (!single_output_format) {
if (!single_output_bitdepth) {
out_ref_valid_bits = sof_ipc4_get_valid_bits(sdev, fe_params);
if (out_ref_valid_bits < 0)
return out_ref_valid_bits;
Expand All @@ -1833,7 +1890,7 @@ sof_ipc4_prepare_copier_module(struct snd_sof_widget *swidget,
* if the output format is the same across all available output formats, choose
* that as the reference.
*/
if (single_output_format) {
if (single_output_bitdepth) {
struct sof_ipc4_audio_format *out_fmt;

out_fmt = &available_fmt->output_pin_fmts[0].audio_fmt;
Expand Down
6 changes: 3 additions & 3 deletions sound/soc/sof/ipc4-topology.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -476,7 +476,7 @@ struct sof_ipc4_process {
u32 init_config;
};

bool sof_ipc4_copier_is_single_format(struct snd_sof_dev *sdev,
struct sof_ipc4_pin_format *pin_fmts,
u32 pin_fmts_size);
bool sof_ipc4_copier_is_single_bitdepth(struct snd_sof_dev *sdev,
struct sof_ipc4_pin_format *pin_fmts,
u32 pin_fmts_size);
#endif

0 comments on commit c85578e

Please sign in to comment.