Skip to content

ASoC: SOF: ipc4-pcm: support multiple configs in BE DAIs #4137

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
plbossart merged 2 commits into from
Feb 16, 2023
Merged

ASoC: SOF: ipc4-pcm: support multiple configs in BE DAIs #4137

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6df6e28
ASoC: SOF: ipc4-pcm: support multiple configs for BE DAIs
kv2019i Feb 9, 2023
f37e1d2
ASoC: SOF: ipc4-pcm: use common helper function in copier prepare
kv2019i Jan 25, 2023
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
71 changes: 60 additions & 11 deletions sound/soc/sof/ipc4-pcm.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -457,13 +457,65 @@ static void ipc4_ssp_dai_config_pcm_params_match(struct snd_sof_dev *sdev, const
}
}

/*
* Fixup DAI link parameters for sampling rate based on
* DAI copier configuration.
*/
static int sof_ipc4_pcm_dai_link_fixup_rate(struct snd_sof_dev *sdev,
struct snd_pcm_hw_params *params,
struct sof_ipc4_copier *ipc4_copier)
{
struct sof_ipc4_pin_format *pin_fmts = ipc4_copier->available_fmt.input_pin_fmts;
struct snd_interval *rate = hw_param_interval(params, SNDRV_PCM_HW_PARAM_RATE);
int num_input_formats = ipc4_copier->available_fmt.num_input_formats;
unsigned int fe_rate = params_rate(params);
bool fe_be_rate_match = false;
bool single_be_rate = true;
unsigned int be_rate;
int i;

/*
* Copier does not change sampling rate, so we
* need to only consider the input pin information.
*/
for (i = 0; i < num_input_formats; i++) {
unsigned int val = pin_fmts[i].audio_fmt.sampling_frequency;

if (i == 0)
be_rate = val;
else if (val != be_rate)
single_be_rate = false;

if (val == fe_rate) {
fe_be_rate_match = true;
break;
}
}

/*
* If rate is different than FE rate, topology must
* contain an SRC. But we do require topology to
* define a single rate in the DAI copier config in
* this case (FE rate may be variable).
*/
if (!fe_be_rate_match) {
if (!single_be_rate) {
dev_err(sdev->dev, "Unable to select sampling rate for DAI link\n");
return -EINVAL;
}

rate->min = be_rate;
rate->max = rate->min;
}

return 0;
}

static int sof_ipc4_pcm_dai_link_fixup(struct snd_soc_pcm_runtime *rtd,
struct snd_pcm_hw_params *params)
{
struct snd_soc_component *component = snd_soc_rtdcom_lookup(rtd, SOF_AUDIO_PCM_DRV_NAME);
struct snd_sof_dai *dai = snd_sof_find_dai(component, rtd->dai_link->name);
struct snd_interval *rate = hw_param_interval(params, SNDRV_PCM_HW_PARAM_RATE);
struct snd_mask *fmt = hw_param_mask(params, SNDRV_PCM_HW_PARAM_FORMAT);
struct snd_sof_dev *sdev = snd_soc_component_get_drvdata(component);
struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0);
struct sof_ipc4_copier *ipc4_copier;
Expand Down Expand Up @@ -495,17 +547,14 @@ static int sof_ipc4_pcm_dai_link_fixup(struct snd_soc_pcm_runtime *rtd,
use_chain_dma = true;
}
}
/*
* Always set BE format to 32-bits for both playback and capture, except if chained
* DMA is in use. In chained DMA case the back-end and front-end format must match.
*/

/* Chain DMA does not use copiers, so no fixup needed */
if (!use_chain_dma) {
Copy link

@RanderWang RanderWang Feb 15, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

so now if (use_chain_dma) we do nothing but the original code set rate->min and rate->max. If we don't need to set rate min and max, we can do it as

if (use_chain_dma)
     return 0

Copy link
Collaborator

@ujfalusi ujfalusi Feb 15, 2023

Choose a reason for hiding this comment

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

We have a switch for dai_type later on, yes it is only handling INTEL_SSP and the ChainDMA is only supported (for now) via HDA, but that can change, I think. Probably a goto then?

Copy link
Collaborator Author

@kv2019i kv2019i Feb 15, 2023

Choose a reason for hiding this comment

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

@RanderWang I was wondering about this but in theory we could use chain-dma for SSP as well, and in that case, we may need to run the DAI-type specific logic later in the function. But yeah, this code could be more reabable, let me split this segment to a separate subfunction in next version.

Copy link
Collaborator

@ranj063 ranj063 Feb 15, 2023

Choose a reason for hiding this comment

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

We cannot use chain DMA for SSP. It uses gpdma

Copy link
Collaborator

@ujfalusi ujfalusi Feb 15, 2023

Choose a reason for hiding this comment

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

@ranj063, #4158 gives a glimpse into the future ;)
But the ChainDMA FW implementation as it is does not rule out use of GPDMA on the peripheral side, with some effort it can be made working.

Copy link
Collaborator

@ujfalusi ujfalusi Feb 15, 2023

Choose a reason for hiding this comment

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

Having said that, for now a simple return 0 in case of chain_dma would make the code nicer. If there will be a need for a change, it can be done when it is due.

Copy link
Collaborator Author

@kv2019i kv2019i Feb 16, 2023

Choose a reason for hiding this comment

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

@ranj063 @ujfalusi As it's more a FW side development whether chain_dma can work with SSP, I'd rather keep the kernel code neutral on this.

snd_mask_none(fmt);
snd_mask_set_format(fmt, SNDRV_PCM_FORMAT_S32_LE);
}
int ret = sof_ipc4_pcm_dai_link_fixup_rate(sdev, params, ipc4_copier);

rate->min = ipc4_copier->available_fmt.input_pin_fmts->audio_fmt.sampling_frequency;
rate->max = rate->min;
if (ret)
return ret;
}

switch (ipc4_copier->dai_type) {
case SOF_DAI_INTEL_SSP:
Expand Down
92 changes: 70 additions & 22 deletions sound/soc/sof/ipc4-topology.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1224,6 +1224,69 @@ static int snd_sof_get_nhlt_endpoint_data(struct snd_sof_dev *sdev, struct snd_s
}
#endif

static int ipc4_set_fmt_mask(struct snd_mask *fmt, unsigned int bit_depth)
{
switch (bit_depth) {
case 16:
snd_mask_set_format(fmt, SNDRV_PCM_FORMAT_S16_LE);
break;
case 24:
snd_mask_set_format(fmt, SNDRV_PCM_FORMAT_S24_LE);
break;
case 32:
snd_mask_set_format(fmt, SNDRV_PCM_FORMAT_S32_LE);
break;
default:
return -EINVAL;
}

return 0;
}

static int ipc4_copier_set_capture_fmt(struct snd_sof_dev *sdev,
struct snd_pcm_hw_params *pipeline_params,
struct snd_pcm_hw_params *fe_params,
struct sof_ipc4_available_audio_format *available_fmt)
{
struct sof_ipc4_audio_format *audio_fmt;
unsigned int sample_valid_bits;
bool multiple_formats = false;
bool fe_format_match = false;
struct snd_mask *fmt;
int i;

for (i = 0; i < available_fmt->num_output_formats; i++) {
unsigned int val;

audio_fmt = &available_fmt->output_pin_fmts[i].audio_fmt;
val = SOF_IPC4_AUDIO_FORMAT_CFG_V_BIT_DEPTH(audio_fmt->fmt_cfg);

if (i == 0)
sample_valid_bits = val;
else if (sample_valid_bits != val)
multiple_formats = true;

if (snd_pcm_format_width(params_format(fe_params)) == val)
fe_format_match = true;
}

fmt = hw_param_mask(pipeline_params, SNDRV_PCM_HW_PARAM_FORMAT);
snd_mask_none(fmt);

if (multiple_formats) {
if (fe_format_match) {
Copy link
Collaborator

@ranj063 ranj063 Feb 13, 2023

Choose a reason for hiding this comment

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

@kv2019i if multiple_formats is true and fe_format_match is untrue, we should return an error instead of returning an arbitrary format with a warning

Copy link
Collaborator Author

@kv2019i kv2019i Feb 14, 2023

Choose a reason for hiding this comment

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

@ranj063 Ack. We'll need to rework the tplgs before we can do that in kernel. I started this work now in thesofproject/sof#7091

Copy link
Collaborator

@ujfalusi ujfalusi Feb 15, 2023

Choose a reason for hiding this comment

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

multiple_formats == false && fe_format_match == false is a possibility? The we will set the first bit_depth, right?

/* multiple formats defined and one matches FE */
snd_mask_set_format(fmt, params_format(fe_params));
return 0;
}

dev_err(sdev->dev, "Multiple audio formats for single dai_out not supported\n");
return -EINVAL;
}

return ipc4_set_fmt_mask(fmt, sample_valid_bits);
}

static int
sof_ipc4_prepare_copier_module(struct snd_sof_widget *swidget,
struct snd_pcm_hw_params *fe_params,
Expand Down Expand Up @@ -1344,13 +1407,10 @@ sof_ipc4_prepare_copier_module(struct snd_sof_widget *swidget,
format_list_to_search = available_fmt->output_pin_fmts;
format_list_count = available_fmt->num_output_formats;

/*
* modify the input params for the dai copier as it only supports
* 32-bit always
*/
fmt = hw_param_mask(pipeline_params, SNDRV_PCM_HW_PARAM_FORMAT);
snd_mask_none(fmt);
snd_mask_set_format(fmt, SNDRV_PCM_FORMAT_S32_LE);
ret = ipc4_copier_set_capture_fmt(sdev, pipeline_params, fe_params,
available_fmt);
if (ret < 0)
return ret;
} else {
format_list_to_search = available_fmt->input_pin_fmts;
format_list_count = available_fmt->num_input_formats;
Expand Down Expand Up @@ -1495,21 +1555,9 @@ sof_ipc4_prepare_copier_module(struct snd_sof_widget *swidget,
out_sample_valid_bits =
SOF_IPC4_AUDIO_FORMAT_CFG_V_BIT_DEPTH(copier_data->out_format.fmt_cfg);
snd_mask_none(fmt);
switch (out_sample_valid_bits) {
case 16:
snd_mask_set_format(fmt, SNDRV_PCM_FORMAT_S16_LE);
break;
case 24:
snd_mask_set_format(fmt, SNDRV_PCM_FORMAT_S24_LE);
break;
case 32:
snd_mask_set_format(fmt, SNDRV_PCM_FORMAT_S32_LE);
break;
default:
dev_err(sdev->dev, "invalid sample frame format %d\n",
params_format(pipeline_params));
return -EINVAL;
}
ret = ipc4_set_fmt_mask(fmt, out_sample_valid_bits);
if (ret)
return ret;

/*
* Set the gateway dma_buffer_size to 2ms buffer size to meet the FW expectation. In the
Expand Down