Skip to content
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

Refactor pipeline management #5262

Open
wants to merge 7 commits into
base: topic/sof-dev
Choose a base branch
from
Open

Refactor pipeline management #5262

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
6ff3766
ASoC: SOF: sof-audio: Add a new op in struct sof_ipc_tplg_ops
ranj063 Dec 7, 2024
8f21468
ASoC: SOF: pcm: Split up widget prepare and setup
ranj063 Dec 3, 2024
285fe36
ASoC: SOF: Intel: hda-dai-ops: Add new BE DAI DMA ops
ranj063 Dec 4, 2024
5ccfeac
ASoC: SOF: sof-audio: expose sof_widget_free()
ranj063 Dec 4, 2024
c6ece6d
drivers: soundwire: Add new prepare op in struct sdw_intel_ops
ranj063 Dec 5, 2024
a1d073c
ASoC: SOF: Refactor pipeline management
ranj063 Dec 4, 2024
32588f6
ASoC: SOF: ipc4-topology: Remove the skip_during_fe_trigger flag
ranj063 Dec 4, 2024
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
12 changes: 8 additions & 4 deletions sound/soc/sof/intel/hda-dai-ops.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -416,10 +416,11 @@ static int hda_ipc4_post_trigger(struct snd_sof_dev *sdev, struct snd_soc_dai *c
break;
case SNDRV_PCM_TRIGGER_SUSPEND:
case SNDRV_PCM_TRIGGER_STOP:
/*
* STOP/SUSPEND trigger is invoked only once when all users of this pipeline have
* been stopped. So, clear the started_count so that the pipeline can be reset
*/
ret = sof_ipc4_set_pipeline_state(sdev, pipe_widget->instance_id,
SOF_IPC4_PIPE_RESET);
if (ret < 0)
goto out;
pipeline->state = SOF_IPC4_PIPE_RESET;
swidget->spipe->started_count = 0;
break;
case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
Expand Down Expand Up @@ -638,6 +639,9 @@ static int hda_ipc4_set_up_be_pipeline(struct snd_soc_dapm_widget *w, int dir)
struct snd_sof_dev *sdev = widget_to_sdev(w);
int ret;

/* set the be_pipeline flag true for the pipeline */
swidget->spipe->be_pipeline = true;

/* set up the widgets in the BE pipeline */
ret = hda_dai_set_up_widgets_in_pipeline(w, swidget->spipe, dir);
if (ret < 0) {
Expand Down
118 changes: 113 additions & 5 deletions sound/soc/sof/intel/hda-dai.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,9 +198,12 @@ static int hda_link_dma_hw_params(struct snd_pcm_substream *substream,
static int __maybe_unused hda_dai_hw_free(struct snd_pcm_substream *substream,
struct snd_soc_dai *cpu_dai)
{
struct snd_soc_dapm_widget *w = snd_soc_dai_get_widget(cpu_dai, substream->stream);
const struct hda_dai_widget_dma_ops *ops = hda_dai_get_ops(substream, cpu_dai);
struct hdac_ext_stream *hext_stream;
struct snd_sof_dev *sdev = dai_to_sdev(substream, cpu_dai);
struct snd_sof_widget *swidget = w->dobj.private;
int ret;

if (!ops) {
dev_err(cpu_dai->dev, "DAI widget ops not set\n");
Expand All @@ -209,9 +212,23 @@ static int __maybe_unused hda_dai_hw_free(struct snd_pcm_substream *substream,

hext_stream = ops->get_hext_stream(sdev, cpu_dai, substream);
if (!hext_stream)
return 0;
goto free;

ret = hda_link_dma_cleanup(substream, hext_stream, cpu_dai);
if (ret < 0)
return ret;

free:
if (ops->free_be_pipeline) {
ret = ops->free_be_pipeline(w, substream->stream);
if (ret < 0)
return ret;
}

if (ops->unprepare_be_pipeline)
ops->unprepare_be_pipeline(w, swidget->spipe, substream->stream);

return hda_link_dma_cleanup(substream, hext_stream, cpu_dai);
return 0;
}

static int __maybe_unused hda_dai_hw_params_data(struct snd_pcm_substream *substream,
Expand Down Expand Up @@ -303,13 +320,29 @@ static int __maybe_unused hda_dai_trigger(struct snd_pcm_substream *substream, i

switch (cmd) {
case SNDRV_PCM_TRIGGER_STOP:
ret = hda_link_dma_cleanup(substream, hext_stream, dai);
if (ret < 0) {
dev_err(sdev->dev, "%s: failed to clean up link DMA\n", __func__);
return ret;
}
break;
case SNDRV_PCM_TRIGGER_SUSPEND:
{
struct snd_soc_dapm_widget *w = snd_soc_dai_get_widget(dai, substream->stream);

ret = hda_link_dma_cleanup(substream, hext_stream, dai);
if (ret < 0) {
dev_err(sdev->dev, "%s: failed to clean up link DMA\n", __func__);
return ret;
}

if (ops->free_be_pipeline) {
ret = ops->free_be_pipeline(w, substream->stream);
if (ret < 0)
return ret;
}
break;
}
default:
break;
}
Expand All @@ -322,9 +355,33 @@ static int __maybe_unused hda_dai_trigger(struct snd_pcm_substream *substream, i
static int hda_dai_prepare(struct snd_pcm_substream *substream, struct snd_soc_dai *dai)
{
struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream);
const struct hda_dai_widget_dma_ops *ops = hda_dai_get_ops(substream, dai);
struct snd_soc_dapm_widget *w = snd_soc_dai_get_widget(dai, substream->stream);
struct snd_sof_widget *swidget = w->dobj.private;
struct snd_sof_dev *sdev = widget_to_sdev(w);
int stream = substream->stream;
int ret;

return hda_dai_hw_params(substream, &rtd->dpcm[stream].hw_params, dai);
if (!ops) {
dev_err(sdev->dev, "DAI widget ops not set\n");
return -EINVAL;
}

/* if this is a prepare without a hw_free or a suspend, free the BE pipeline */
if (swidget->spipe->complete && ops->free_be_pipeline) {
ret = ops->free_be_pipeline(w, substream->stream);
if (ret < 0)
return ret;
}

ret = hda_dai_hw_params(substream, &rtd->dpcm[stream].hw_params, dai);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there any specific reason that we always call hw_params in .prepare?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

because sometimes we dont have a hw_free like in the case of xruns and we'll need to do everything again.

Copy link
Member

Choose a reason for hiding this comment

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

prepare() should be fast. We should call this out in comments - we need to do X,Y out of cycle because Z

if (ret < 0)
return ret;

if (ops && ops->set_up_be_pipeline)
return ops->set_up_be_pipeline(w, substream->stream);

return ret;
}

static const struct snd_soc_dai_ops hda_dai_ops = {
Expand Down Expand Up @@ -446,10 +503,28 @@ static int non_hda_dai_hw_params(struct snd_pcm_substream *substream,
static int non_hda_dai_prepare(struct snd_pcm_substream *substream,
struct snd_soc_dai *cpu_dai)
{
struct snd_soc_dapm_widget *w = snd_soc_dai_get_widget(cpu_dai, substream->stream);
const struct hda_dai_widget_dma_ops *ops = hda_dai_get_ops(substream, cpu_dai);
struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream);
struct snd_sof_widget *swidget = w->dobj.private;
int stream = substream->stream;
int ret;

/* if this is a prepare without a hw_free or a suspend, free the BE pipeline */
if (swidget->spipe->complete && ops->free_be_pipeline) {
ret = ops->free_be_pipeline(w, stream);
if (ret < 0)
return ret;
}

ret = non_hda_dai_hw_params(substream, &rtd->dpcm[stream].hw_params, cpu_dai);
if (ret < 0)
return ret;

if (ops && ops->set_up_be_pipeline)
return ops->set_up_be_pipeline(w, stream);

return non_hda_dai_hw_params(substream, &rtd->dpcm[stream].hw_params, cpu_dai);
return ret;
}

static const struct snd_soc_dai_ops ssp_dai_ops = {
Expand Down Expand Up @@ -612,10 +687,36 @@ int sdw_hda_dai_trigger(struct snd_pcm_substream *substream, int cmd,
}
EXPORT_SYMBOL_NS(sdw_hda_dai_trigger, SND_SOC_SOF_INTEL_HDA_COMMON);


int sdw_hda_dai_prepare(struct snd_pcm_substream *substream, struct snd_pcm_hw_params *params,
struct snd_soc_dai *cpu_dai, int link_id, int intel_alh_id)
{
return sdw_hda_dai_hw_params(substream, params, cpu_dai, link_id, intel_alh_id);
struct snd_soc_dapm_widget *w = snd_soc_dai_get_widget(cpu_dai, substream->stream);
const struct hda_dai_widget_dma_ops *ops = hda_dai_get_ops(substream, cpu_dai);
struct snd_sof_widget *swidget = w->dobj.private;
int stream = substream->stream;
int ret;

if (!ops) {
dev_err(cpu_dai->dev, "DAI widget ops not set\n");
return -EINVAL;
}

/* if this is a prepare without a hw_free or a suspend, free the BE pipeline */
if (swidget->spipe->complete && ops->free_be_pipeline) {
ret = ops->free_be_pipeline(w, stream);
if (ret < 0)
return ret;
}

ret = sdw_hda_dai_hw_params(substream, params, cpu_dai, link_id, intel_alh_id);
if (ret < 0)
return ret;

if (ops->set_up_be_pipeline)
return ops->set_up_be_pipeline(w, stream);

return ret;
}
EXPORT_SYMBOL_NS(sdw_hda_dai_prepare, SND_SOC_SOF_INTEL_HDA_COMMON);

Expand Down Expand Up @@ -671,6 +772,13 @@ static int hda_dai_suspend(struct hdac_bus *bus)
cpu_dai);
if (ret < 0)
return ret;

if (ops->free_be_pipeline) {
ret = ops->free_be_pipeline(w,
hdac_stream(hext_stream)->direction);
if (ret < 0)
return ret;
}
}
}

Expand Down
4 changes: 2 additions & 2 deletions sound/soc/sof/ipc4-pcm.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ sof_ipc4_add_pipeline_to_trigger_list(struct snd_sof_dev *sdev, int state,
struct snd_sof_widget *pipe_widget = spipe->pipe_widget;
struct sof_ipc4_pipeline *pipeline = pipe_widget->private;

if (pipeline->skip_during_fe_trigger && state != SOF_IPC4_PIPE_RESET)
if (pipeline->skip_during_fe_trigger)
return;

switch (state) {
Expand Down Expand Up @@ -177,7 +177,7 @@ sof_ipc4_update_pipeline_state(struct snd_sof_dev *sdev, int state, int cmd,
struct sof_ipc4_pipeline *pipeline = pipe_widget->private;
int i;

if (pipeline->skip_during_fe_trigger && state != SOF_IPC4_PIPE_RESET)
if (pipeline->skip_during_fe_trigger)
return;

/* set state for pipeline if it was just triggered */
Expand Down
1 change: 0 additions & 1 deletion sound/soc/sof/ipc4-topology.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1445,7 +1445,6 @@ static void sof_ipc4_unprepare_copier_module(struct snd_sof_widget *swidget)
/* reset pipeline memory usage */
pipe_widget = swidget->spipe->pipe_widget;
pipeline = pipe_widget->private;
pipeline->mem_usage = 0;

if (WIDGET_IS_AIF(swidget->id) || swidget->id == snd_soc_dapm_buffer) {
if (pipeline->use_chain_dma) {
Expand Down
39 changes: 34 additions & 5 deletions sound/soc/sof/sof-audio.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,17 @@ int sof_route_setup(struct snd_sof_dev *sdev, struct snd_soc_dapm_widget *wsourc
}
EXPORT_SYMBOL(sof_route_setup);
Copy link
Collaborator

Choose a reason for hiding this comment

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

You should have done this earlier, the previous patch uses the function.


/* helper function to check if the widget belongs to a BE pipeline */
static bool sof_is_be_pipeline_widget(struct snd_soc_dapm_widget *w)
{
struct snd_sof_widget *swidget = w->dobj.private;

if (swidget->spipe->be_pipeline)
return true;

return false;
}

static int sof_setup_pipeline_connections(struct snd_sof_dev *sdev,
struct snd_soc_dapm_widget_list *list, int dir)
{
Expand All @@ -319,6 +330,12 @@ static int sof_setup_pipeline_connections(struct snd_sof_dev *sdev,
continue;

if (p->sink->dobj.private) {
/*
* skip routes between widgets belonging to the BE pipeline
*/
if (sof_is_be_pipeline_widget(widget) &&
sof_is_be_pipeline_widget(p->sink))
continue;
ret = sof_route_setup(sdev, widget, p->sink);
if (ret < 0)
return ret;
Expand All @@ -335,6 +352,12 @@ static int sof_setup_pipeline_connections(struct snd_sof_dev *sdev,
continue;

if (p->source->dobj.private) {
/*
* skip routes between widgets belonging to the BE pipeline
*/
if (sof_is_be_pipeline_widget(widget) &&
sof_is_be_pipeline_widget(p->source))
continue;
ret = sof_route_setup(sdev, p->source, widget);
if (ret < 0)
return ret;
Expand Down Expand Up @@ -415,8 +438,12 @@ sof_unprepare_widgets_in_path(struct snd_sof_dev *sdev, struct snd_soc_dapm_widg
if (is_virtual_widget(sdev, widget, __func__))
return;

/* skip if the widget is in use or if it is already unprepared */
if (!swidget || !swidget->prepared || swidget->use_count > 0)
/*
* skip if the widget is in use or if it is already unprepared or
* if it belongs to a BE pipeline.
*/
if (!swidget || !swidget->prepared || swidget->use_count > 0 ||
swidget->spipe->be_pipeline)
Copy link
Collaborator

Choose a reason for hiding this comment

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

In some places use use sof_is_be_pipeline_widget() in some swidget->spipe->be_pipeline

goto sink_unprepare;

widget_ops = tplg_ops ? tplg_ops->widget : NULL;
Expand Down Expand Up @@ -506,14 +533,16 @@ static int sof_free_widgets_in_path(struct snd_sof_dev *sdev, struct snd_soc_dap
int dir, struct snd_sof_pcm *spcm)
{
struct snd_soc_dapm_widget_list *list = spcm->stream[dir].list;
struct snd_sof_widget *swidget = widget->dobj.private;
struct snd_soc_dapm_path *p;
int err;
int ret = 0;

if (is_virtual_widget(sdev, widget, __func__))
return 0;

if (widget->dobj.private) {
/* only free widgets that aren't part of the BE pipeline */
if (swidget && !sof_is_be_pipeline_widget(widget)) {
err = sof_widget_free(sdev, widget->dobj.private);
if (err < 0)
ret = err;
Expand Down Expand Up @@ -555,7 +584,7 @@ static int sof_set_up_widgets_in_path(struct snd_sof_dev *sdev, struct snd_soc_d
if (is_virtual_widget(sdev, widget, __func__))
return 0;

if (swidget) {
if (swidget && !sof_is_be_pipeline_widget(widget)) {
int i;

ret = sof_widget_setup(sdev, widget->dobj.private);
Expand Down Expand Up @@ -594,7 +623,7 @@ static int sof_set_up_widgets_in_path(struct snd_sof_dev *sdev, struct snd_soc_d
ret = sof_set_up_widgets_in_path(sdev, p->sink, dir, spcm);
p->walking = false;
if (ret < 0) {
if (swidget)
if (swidget && !sof_is_be_pipeline_widget(widget))
sof_widget_free(sdev, swidget);
return ret;
}
Expand Down
2 changes: 2 additions & 0 deletions sound/soc/sof/sof-audio.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -502,6 +502,7 @@ struct snd_sof_widget {
pipeline
* @complete: flag used to indicate that pipeline set up is complete.
* @core_mask: Mask containing target cores for all modules in the pipeline
* @be_pipeline: Flag indicating that the pipeline contains a BE DAI widget
* @list: List item in sdev pipeline_list
*/
struct snd_sof_pipeline {
Expand All @@ -510,6 +511,7 @@ struct snd_sof_pipeline {
int paused_count;
int complete;
unsigned long core_mask;
bool be_pipeline;
struct list_head list;
};

Expand Down