Skip to content

Fix IPC4 pipeline trigger#3917

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 5 commits into from
Oct 20, 2022
Merged

Fix IPC4 pipeline trigger #3917

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
64d7cf4
ASoC: SOF: Introduce PCM setup/free PCM IPC ops
ranj063 Oct 18, 2022
1b3379e
ASoC: SOF: ipc4-pcm: Define pcm_setup/free ops
ranj063 Oct 18, 2022
55b013f
ASoC: SOF: ipc4: Add flag to skip triggering pipelines during FE DAI …
ranj063 Oct 18, 2022
479e68d
ASoC: SOF: sof-audio: Populate the PCM stream pipeline_info
ranj063 Oct 18, 2022
4abe644
ASoC: SOF: ipc4-pcm: Use the PCM stream's pipeline_info during trigger
ranj063 Oct 17, 2022
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
3 changes: 3 additions & 0 deletions include/sound/sof/ipc4/header.h
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -185,6 +185,9 @@ enum sof_ipc4_pipeline_state {
#define SOF_IPC4_GLB_PIPE_STATE_MASK GENMASK(15, 0)
#define SOF_IPC4_GLB_PIPE_STATE(x) ((x) << SOF_IPC4_GLB_PIPE_STATE_SHIFT)

/* pipeline set state IPC msg extension */
#define SOF_IPC4_GLB_PIPE_STATE_EXT_MULTI BIT(0)

/* load library ipc msg */
#define SOF_IPC4_GLB_LOAD_LIBRARY_LIB_ID_SHIFT 16
#define SOF_IPC4_GLB_LOAD_LIBRARY_LIB_ID(x) ((x) << SOF_IPC4_GLB_LOAD_LIBRARY_LIB_ID_SHIFT)
Expand Down
92 changes: 17 additions & 75 deletions sound/soc/sof/intel/hda-dai.c
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -450,6 +450,8 @@ static int ipc4_hda_dai_trigger(struct snd_pcm_substream *substream,
{
struct hdac_ext_stream *hext_stream = snd_soc_dai_get_dma_data(dai, substream);
struct snd_sof_dev *sdev = snd_soc_component_get_drvdata(dai->component);
struct snd_sof_widget *pipe_widget;
struct sof_ipc4_pipeline *pipeline;
struct snd_soc_pcm_runtime *rtd;
struct snd_sof_widget *swidget;
struct snd_soc_dapm_widget *w;
Expand All@@ -466,18 +468,30 @@ static int ipc4_hda_dai_trigger(struct snd_pcm_substream *substream,

w = snd_soc_dai_get_widget(dai, substream->stream);
swidget = w->dobj.private;
pipe_widget = swidget->pipe_widget;
pipeline = pipe_widget->private;
Comment thread
ranj063 marked this conversation as resolved.

switch (cmd) {
case SNDRV_PCM_TRIGGER_START:
case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
snd_hdac_ext_stream_start(hext_stream);
if (pipeline->state != SOF_IPC4_PIPE_PAUSED) {
Comment thread
plbossart marked this conversation as resolved.
ret = sof_ipc4_set_pipeline_state(sdev, pipe_widget->instance_id,
SOF_IPC4_PIPE_PAUSED);
if (ret < 0)
return ret;
pipeline->state = SOF_IPC4_PIPE_PAUSED;
}

ret = sof_ipc4_set_pipeline_state(sdev, pipe_widget->instance_id,
SOF_IPC4_PIPE_RUNNING);
if (ret < 0)
return ret;
pipeline->state = SOF_IPC4_PIPE_RUNNING;
break;
case SNDRV_PCM_TRIGGER_SUSPEND:
case SNDRV_PCM_TRIGGER_STOP:
{
struct snd_sof_widget *pipe_widget = swidget->pipe_widget;
struct sof_ipc4_pipeline *pipeline = pipe_widget->private;

ret = sof_ipc4_set_pipeline_state(sdev, pipe_widget->instance_id,
SOF_IPC4_PIPE_PAUSED);
if (ret < 0)
Expand All@@ -503,9 +517,6 @@ static int ipc4_hda_dai_trigger(struct snd_pcm_substream *substream,
}
case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
{
struct snd_sof_widget *pipe_widget = swidget->pipe_widget;
struct sof_ipc4_pipeline *pipeline = pipe_widget->private;

ret = sof_ipc4_set_pipeline_state(sdev, pipe_widget->instance_id,
SOF_IPC4_PIPE_PAUSED);
if (ret < 0)
Expand DownExpand Up@@ -703,64 +714,6 @@ static const struct snd_soc_dai_ops ipc3_ssp_dai_ops = {
.shutdown = ssp_dai_shutdown,
};

static int ipc4_be_dai_common_trigger(struct snd_soc_dai *dai, int cmd, int stream)
{
struct snd_sof_widget *pipe_widget;
struct sof_ipc4_pipeline *pipeline;
struct snd_sof_widget *swidget;
struct snd_soc_dapm_widget *w;
struct snd_sof_dev *sdev;
int ret;

w = snd_soc_dai_get_widget(dai, stream);
swidget = w->dobj.private;
pipe_widget = swidget->pipe_widget;
pipeline = pipe_widget->private;
sdev = snd_soc_component_get_drvdata(swidget->scomp);

switch (cmd) {
case SNDRV_PCM_TRIGGER_SUSPEND:
case SNDRV_PCM_TRIGGER_STOP:
ret = sof_ipc4_set_pipeline_state(sdev, pipe_widget->instance_id,
SOF_IPC4_PIPE_PAUSED);
if (ret < 0)
return ret;
pipeline->state = SOF_IPC4_PIPE_PAUSED;

ret = sof_ipc4_set_pipeline_state(sdev, pipe_widget->instance_id,
SOF_IPC4_PIPE_RESET);
if (ret < 0)
return ret;
pipeline->state = SOF_IPC4_PIPE_RESET;
break;
case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
ret = sof_ipc4_set_pipeline_state(sdev, pipe_widget->instance_id,
SOF_IPC4_PIPE_PAUSED);
if (ret < 0)
return ret;
pipeline->state = SOF_IPC4_PIPE_PAUSED;
break;
default:
break;
}

return 0;
}

static int ipc4_be_dai_trigger(struct snd_pcm_substream *substream,
int cmd, struct snd_soc_dai *dai)
{
return ipc4_be_dai_common_trigger(dai, cmd, substream->stream);
}

static const struct snd_soc_dai_ops ipc4_dmic_dai_ops = {
.trigger = ipc4_be_dai_trigger,
};

static const struct snd_soc_dai_ops ipc4_ssp_dai_ops = {
.trigger = ipc4_be_dai_trigger,
};

void hda_set_dai_drv_ops(struct snd_sof_dev *sdev, struct snd_sof_dsp_ops *ops)
{
int i;
Expand All@@ -785,14 +738,6 @@ void hda_set_dai_drv_ops(struct snd_sof_dev *sdev, struct snd_sof_dsp_ops *ops)
struct sof_ipc4_fw_data *ipc4_data = sdev->private;

for (i = 0; i < ops->num_drv; i++) {
if (strstr(ops->drv[i].name, "DMIC")) {
ops->drv[i].ops = &ipc4_dmic_dai_ops;
continue;
}
if (strstr(ops->drv[i].name, "SSP")) {
ops->drv[i].ops = &ipc4_ssp_dai_ops;
continue;
}
#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA_AUDIO_CODEC)
if (strstr(ops->drv[i].name, "iDisp") ||
strstr(ops->drv[i].name, "Analog") ||
Expand All@@ -804,9 +749,6 @@ void hda_set_dai_drv_ops(struct snd_sof_dev *sdev, struct snd_sof_dsp_ops *ops)
if (!hda_use_tplg_nhlt)
ipc4_data->nhlt = intel_nhlt_init(sdev->dev);

if (IS_ENABLED(CONFIG_SND_SOC_SOF_INTEL_SOUNDWIRE))
sdw_callback.trigger = ipc4_be_dai_common_trigger;

break;
}
default:
Expand Down
193 changes: 143 additions & 50 deletions sound/soc/sof/ipc4-pcm.c
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -13,6 +13,33 @@
#include "ipc4-priv.h"
#include "ipc4-topology.h"

static int sof_ipc4_set_multi_pipeline_state(struct snd_sof_dev *sdev, u32 state,
struct ipc4_pipeline_set_state_data *data)
{
struct sof_ipc4_msg msg = {{ 0 }};
u32 primary, ipc_size;

Comment thread
ranj063 marked this conversation as resolved.
/* trigger a single pipeline */
if (data->count == 1)
return sof_ipc4_set_pipeline_state(sdev, data->pipeline_ids[0], state);

primary = state;
primary |= SOF_IPC4_MSG_TYPE_SET(SOF_IPC4_GLB_SET_PIPELINE_STATE);
primary |= SOF_IPC4_MSG_DIR(SOF_IPC4_MSG_REQUEST);
primary |= SOF_IPC4_MSG_TARGET(SOF_IPC4_FW_GEN_MSG);
msg.primary = primary;

/* trigger multiple pipelines with a single IPC */
msg.extension = SOF_IPC4_GLB_PIPE_STATE_EXT_MULTI;

/* ipc_size includes the count and the pipeline IDs for the number of pipelines */
ipc_size = sizeof(u32) * (data->count + 1);
msg.data_size = ipc_size;
msg.data_ptr = data;

return sof_ipc_tx_message(sdev->ipc, &msg, ipc_size, NULL, 0);
}

int sof_ipc4_set_pipeline_state(struct snd_sof_dev *sdev, u32 id, u32 state)
{
struct sof_ipc4_msg msg = {{ 0 }};
Expand All@@ -37,75 +64,100 @@ static int sof_ipc4_trigger_pipelines(struct snd_soc_component *component,
{
struct snd_sof_dev *sdev = snd_soc_component_get_drvdata(component);
struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream);
struct snd_sof_widget *pipeline_widget;
struct snd_soc_dapm_widget_list *list;
struct snd_soc_dapm_widget *widget;
struct snd_sof_pcm_stream_trigger_info *trigger_info;
struct ipc4_pipeline_set_state_data *data;
struct snd_sof_widget *pipe_widget;
struct sof_ipc4_pipeline *pipeline;
struct snd_sof_widget *swidget;
struct snd_sof_pcm *spcm;
int ret = 0;
int num_widgets;
int ret;
int i, j;

spcm = snd_sof_find_spcm_dai(component, rtd);
if (!spcm)
return -EINVAL;

list = spcm->stream[substream->stream].list;
trigger_info = &spcm->stream[substream->stream].trigger_info;

/* nothing to trigger if the list is empty */
if (!trigger_info->pipeline_list)
return 0;

/* allocate data for the pipeline ID's and the count of pipelines to be triggered */
data = kzalloc(sizeof(u32) * (trigger_info->count + 1), GFP_KERNEL);
if (!data)
return -ENOMEM;

/*
* IPC4 requires pipelines to be triggered in order starting at the sink and
* walking all the way to the source. So traverse the pipeline_list in the reverse order.
Comment thread
ranj063 marked this conversation as resolved.
* Skip the pipelines that have their skip_during_fe_trigger flag set or if they're already
* in the requested state. If there is a fork in the pipeline, the order of triggering
* between the left/right paths will be indeterministic. But the sink->source trigger order
* sink->source would still be guaranteed for each fork independently.
*/
for (i = trigger_info->count - 1; i >= 0; i--) {
pipe_widget = trigger_info->pipeline_list[i];
pipeline = pipe_widget->private;
if (pipeline->state != state && !pipeline->skip_during_fe_trigger)
data->pipeline_ids[data->count++] = pipe_widget->instance_id;
}

for_each_dapm_widgets(list, num_widgets, widget) {
swidget = widget->dobj.private;
/* return if all pipelines are in the requested state already */
if (!data->count) {
kfree(data);
return 0;
}

if (!swidget)
continue;
/*
* Pause all pipelines. This could result in an extra IPC to pause all pipelines even if
* they are already paused. But it helps keep the logic simpler and the firmware handles
* the repeated pause gracefully. This can be optimized in the future if needed.
*/
ret = sof_ipc4_set_multi_pipeline_state(sdev, SOF_IPC4_PIPE_PAUSED, data);
Comment thread
ranj063 marked this conversation as resolved.
if (ret < 0) {
dev_err(sdev->dev, "failed to pause all pipelines\n");
goto free;
}

/*
* set pipeline state for both FE and BE pipelines for RUNNING state.
* For PAUSE/RESET, set the pipeline state only for the FE pipeline.
*/
switch (state) {
case SOF_IPC4_PIPE_PAUSED:
case SOF_IPC4_PIPE_RESET:
if (!WIDGET_IS_AIF(swidget->id))
continue;
break;
default:
break;
}
/* update PAUSED state for all pipelines that were just triggered */
for (i = 0; i < data->count; i++) {
for (j = 0; j < trigger_info->count; j++) {
pipe_widget = trigger_info->pipeline_list[j];
pipeline = pipe_widget->private;

/* find pipeline widget for the pipeline that this widget belongs to */
pipeline_widget = swidget->pipe_widget;
pipeline = (struct sof_ipc4_pipeline *)pipeline_widget->private;

if (pipeline->state == state)
continue;

/* first set the pipeline to PAUSED state */
if (pipeline->state != SOF_IPC4_PIPE_PAUSED) {
ret = sof_ipc4_set_pipeline_state(sdev, pipeline_widget->instance_id,
SOF_IPC4_PIPE_PAUSED);
if (ret < 0) {
dev_err(sdev->dev, "failed to pause pipeline %d\n",
swidget->pipeline_id);
return ret;
if (data->pipeline_ids[i] == pipe_widget->instance_id) {
pipeline->state = SOF_IPC4_PIPE_PAUSED;
break;
}
}
}

pipeline->state = SOF_IPC4_PIPE_PAUSED;
/* return if this is the final state */
if (state == SOF_IPC4_PIPE_PAUSED)
goto free;

if (pipeline->state == state)
continue;
/* else set the final state in the DSP */
ret = sof_ipc4_set_multi_pipeline_state(sdev, state, data);
if (ret < 0) {
dev_err(sdev->dev, "failed to set final state %d for all pipelines\n", state);
goto free;
}

/* then set the final state */
ret = sof_ipc4_set_pipeline_state(sdev, pipeline_widget->instance_id, state);
if (ret < 0) {
dev_err(sdev->dev, "failed to set state %d for pipeline %d\n",
state, swidget->pipeline_id);
break;
}
/* update final state for all pipelines that were just triggered */
for (i = 0; i < data->count; i++) {
for (j = 0; j < trigger_info->count; j++) {
pipe_widget = trigger_info->pipeline_list[j];
pipeline = pipe_widget->private;

pipeline->state = state;
if (data->pipeline_ids[i] == pipe_widget->instance_id) {
pipeline->state = state;
break;
}
}
}

free:
kfree(data);
return ret;
}

Expand DownExpand Up@@ -215,8 +267,49 @@ static int sof_ipc4_pcm_dai_link_fixup(struct snd_soc_pcm_runtime *rtd,
return 0;
}

static int sof_ipc4_pcm_setup(struct snd_sof_dev *sdev, struct snd_sof_pcm *spcm)
{
struct snd_sof_pcm_stream_trigger_info *trigger_info;
struct sof_ipc4_fw_data *ipc4_data = sdev->private;
int stream;

for_each_pcm_streams(stream) {
trigger_info = &spcm->stream[stream].trigger_info;

/* allocate memory for max number of pipeline IDs */
trigger_info->pipeline_list = kcalloc(ipc4_data->max_num_pipelines,
sizeof(struct snd_sof_widget *),
GFP_KERNEL);
if (!trigger_info->pipeline_list)
goto err;
}

return 0;
err:
for_each_pcm_streams(stream) {
trigger_info = &spcm->stream[stream].trigger_info;
kfree(trigger_info->pipeline_list);
trigger_info->pipeline_list = NULL;
}
Comment thread
ranj063 marked this conversation as resolved.
return -ENOMEM;
}

static void sof_ipc4_pcm_free(struct snd_sof_dev *sdev, struct snd_sof_pcm *spcm)
{
struct snd_sof_pcm_stream_trigger_info *trigger_info;
int stream;

for_each_pcm_streams(stream) {
trigger_info = &spcm->stream[stream].trigger_info;
kfree(trigger_info->pipeline_list);
trigger_info->pipeline_list = NULL;
}
}

const struct sof_ipc_pcm_ops ipc4_pcm_ops = {
.trigger = sof_ipc4_pcm_trigger,
.hw_free = sof_ipc4_pcm_hw_free,
.dai_link_fixup = sof_ipc4_pcm_dai_link_fixup,
.pcm_setup = sof_ipc4_pcm_setup,
.pcm_free = sof_ipc4_pcm_free,
};
1 change: 1 addition & 0 deletions sound/soc/sof/ipc4-topology.c
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -1801,6 +1801,7 @@ static int sof_ipc4_dai_config(struct snd_sof_dev *sdev, struct snd_sof_widget *
case SOF_DAI_INTEL_HDA:
gtw_attr = ipc4_copier->gtw_attr;
gtw_attr->lp_buffer_alloc = pipeline->lp_mode;
pipeline->skip_during_fe_trigger = true;

@plbossartplbossartOct 19, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I am still in the dark on why we need to skip only HDAudio pipelines?

@ranj063ranj063Oct 19, 2022

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

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

because they're the only ones left with BE DAI trigger ops

@plbossartplbossartOct 19, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can't we detect this rather than add a flag that duplicates information that can be discovered?

@ranj063ranj063Oct 19, 2022•
edited
Loading

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

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

I can think of doing this with a very convoluted check by accessing the snd_soc_dai from the snd_sof_widget using the sname and then checking if the dai->drv-ops are set. But the flag is a lot simpler I think

@plbossartplbossartOct 19, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Don't we have a snd_sof_dai -> snd_soc_dai helper? I don't really understand why we don't have a 1:1 mapping.

@ranj063ranj063Oct 20, 2022

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

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

we don't have a 1:1 mapping, Pierre. I can add this in a follow up maybe

@plbossartplbossartOct 20, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

That would be good indeed

fallthrough;
case SOF_DAI_INTEL_ALH:
copier_data->gtw_cfg.node_id &= ~SOF_IPC4_NODE_INDEX_MASK;
Expand Down
Loading