[Sound-open-firmware] [PATCH 0/2] ASoC: SOF: initialise work immediately
Fix uninitialised work errors by moving initialisation to directly after allocation.
Guennadi Liakhovetski (2): ASoC: SOF: (cosmetic) use for_each_pcm_streams() in sof_dai_load() ASoC: SOF: fix uninitialised "work" with VirtIO
sound/soc/sof/pcm.c | 4 +--- sound/soc/sof/sof-audio.h | 3 +++ sound/soc/sof/topology.c | 17 ++++++++++++----- 3 files changed, 16 insertions(+), 8 deletions(-)
Use for_each_pcm_streams() to enumerate streams in sof_dai_load() instead of doing that manually.
Signed-off-by: Guennadi Liakhovetski guennadi.liakhovetski@linux.intel.com --- sound/soc/sof/topology.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/sound/soc/sof/topology.c b/sound/soc/sof/topology.c index 058de94..54437ca 100644 --- a/sound/soc/sof/topology.c +++ b/sound/soc/sof/topology.c @@ -2448,7 +2448,7 @@ static int sof_dai_load(struct snd_soc_component *scomp, int index, struct snd_soc_tplg_stream_caps *caps; struct snd_soc_tplg_private *private = &pcm->priv; struct snd_sof_pcm *spcm; - int stream = SNDRV_PCM_STREAM_PLAYBACK; + int stream; int ret = 0;
/* nothing to do for BEs atm */ @@ -2460,8 +2460,9 @@ static int sof_dai_load(struct snd_soc_component *scomp, int index, return -ENOMEM;
spcm->scomp = scomp; - spcm->stream[SNDRV_PCM_STREAM_PLAYBACK].comp_id = COMP_ID_UNASSIGNED; - spcm->stream[SNDRV_PCM_STREAM_CAPTURE].comp_id = COMP_ID_UNASSIGNED; + + for_each_pcm_streams(stream) + spcm->stream[stream].comp_id = COMP_ID_UNASSIGNED;
spcm->pcm = *pcm; dev_dbg(scomp->dev, "tplg: load pcm %s\n", pcm->dai_name); @@ -2482,8 +2483,10 @@ static int sof_dai_load(struct snd_soc_component *scomp, int index, if (!spcm->pcm.playback) goto capture;
+ stream = SNDRV_PCM_STREAM_PLAYBACK; + dev_vdbg(scomp->dev, "tplg: pcm %s stream tokens: playback d0i3:%d\n", - spcm->pcm.pcm_name, spcm->stream[0].d0i3_compatible); + spcm->pcm.pcm_name, spcm->stream[stream].d0i3_compatible);
caps = &spcm->pcm.caps[stream];
@@ -2513,7 +2516,7 @@ static int sof_dai_load(struct snd_soc_component *scomp, int index, return ret;
dev_vdbg(scomp->dev, "tplg: pcm %s stream tokens: capture d0i3:%d\n", - spcm->pcm.pcm_name, spcm->stream[1].d0i3_compatible); + spcm->pcm.pcm_name, spcm->stream[stream].d0i3_compatible);
caps = &spcm->pcm.caps[stream];
Guennadi, who are the intended reviewers/maintainers here and which tree is this supposed to be merged on?
Patch0 has everyone copied, patch 1/2 only target mailing lists, and I haven't seen any PR/CI for this?
I am flexible with processes, I am just lost here on what you had in mind.
Thanks -Pierre
On 3/24/20 7:29 AM, Guennadi Liakhovetski wrote:
Use for_each_pcm_streams() to enumerate streams in sof_dai_load() instead of doing that manually.
Signed-off-by: Guennadi Liakhovetski guennadi.liakhovetski@linux.intel.com
sound/soc/sof/topology.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/sound/soc/sof/topology.c b/sound/soc/sof/topology.c index 058de94..54437ca 100644 --- a/sound/soc/sof/topology.c +++ b/sound/soc/sof/topology.c @@ -2448,7 +2448,7 @@ static int sof_dai_load(struct snd_soc_component *scomp, int index, struct snd_soc_tplg_stream_caps *caps; struct snd_soc_tplg_private *private = &pcm->priv; struct snd_sof_pcm *spcm;
- int stream = SNDRV_PCM_STREAM_PLAYBACK;
int stream; int ret = 0;
/* nothing to do for BEs atm */
@@ -2460,8 +2460,9 @@ static int sof_dai_load(struct snd_soc_component *scomp, int index, return -ENOMEM;
spcm->scomp = scomp;
- spcm->stream[SNDRV_PCM_STREAM_PLAYBACK].comp_id = COMP_ID_UNASSIGNED;
- spcm->stream[SNDRV_PCM_STREAM_CAPTURE].comp_id = COMP_ID_UNASSIGNED;
for_each_pcm_streams(stream)
spcm->stream[stream].comp_id = COMP_ID_UNASSIGNED;spcm->pcm = *pcm; dev_dbg(scomp->dev, "tplg: load pcm %s\n", pcm->dai_name);
@@ -2482,8 +2483,10 @@ static int sof_dai_load(struct snd_soc_component *scomp, int index, if (!spcm->pcm.playback) goto capture;
- stream = SNDRV_PCM_STREAM_PLAYBACK;
- dev_vdbg(scomp->dev, "tplg: pcm %s stream tokens: playback d0i3:%d\n",
spcm->pcm.pcm_name, spcm->stream[0].d0i3_compatible);
spcm->pcm.pcm_name, spcm->stream[stream].d0i3_compatible);caps = &spcm->pcm.caps[stream];
@@ -2513,7 +2516,7 @@ static int sof_dai_load(struct snd_soc_component *scomp, int index, return ret;
dev_vdbg(scomp->dev, "tplg: pcm %s stream tokens: capture d0i3:%d\n",
spcm->pcm.pcm_name, spcm->stream[1].d0i3_compatible);
spcm->pcm.pcm_name, spcm->stream[stream].d0i3_compatible);caps = &spcm->pcm.caps[stream];
Hi Pierre,
On Tue, Mar 24, 2020 at 08:29:43AM -0500, Pierre-Louis Bossart wrote:
Guennadi, who are the intended reviewers/maintainers here and which tree is this supposed to be merged on?
Patch0 has everyone copied, patch 1/2 only target mailing lists, and I haven't seen any PR/CI for this?
Yes, as explained in my reply to Mark, I didn't want to spam everybody's mail boxes with all the patches, so I only copied the "patch 0" to maintainers and sent the patches themselves to lists only.
I am flexible with processes, I am just lost here on what you had in mind.
I was advised to make this a separate patch from my VirtIO patch series, so I did that and sent it again directly to the mailing lists. But we can take it back to the local review process, if you prefer.
Thanks Guennadi
Thanks -Pierre
On 3/24/20 7:29 AM, Guennadi Liakhovetski wrote:
Use for_each_pcm_streams() to enumerate streams in sof_dai_load() instead of doing that manually.
Signed-off-by: Guennadi Liakhovetski guennadi.liakhovetski@linux.intel.com
sound/soc/sof/topology.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/sound/soc/sof/topology.c b/sound/soc/sof/topology.c index 058de94..54437ca 100644 --- a/sound/soc/sof/topology.c +++ b/sound/soc/sof/topology.c @@ -2448,7 +2448,7 @@ static int sof_dai_load(struct snd_soc_component *scomp, int index, struct snd_soc_tplg_stream_caps *caps; struct snd_soc_tplg_private *private = &pcm->priv; struct snd_sof_pcm *spcm;
- int stream = SNDRV_PCM_STREAM_PLAYBACK;
- int stream; int ret = 0; /* nothing to do for BEs atm */
@@ -2460,8 +2460,9 @@ static int sof_dai_load(struct snd_soc_component *scomp, int index, return -ENOMEM; spcm->scomp = scomp;
- spcm->stream[SNDRV_PCM_STREAM_PLAYBACK].comp_id = COMP_ID_UNASSIGNED;
- spcm->stream[SNDRV_PCM_STREAM_CAPTURE].comp_id = COMP_ID_UNASSIGNED;
- for_each_pcm_streams(stream)
spcm->stream[stream].comp_id = COMP_ID_UNASSIGNED;@@ -2482,8 +2483,10 @@ static int sof_dai_load(struct snd_soc_component *scomp, int index, if (!spcm->pcm.playback) goto capture;
- stream = SNDRV_PCM_STREAM_PLAYBACK;
- dev_vdbg(scomp->dev, "tplg: pcm %s stream tokens: playback d0i3:%d\n",
spcm->pcm.pcm_name, spcm->stream[0].d0i3_compatible);
spcm->pcm.pcm_name, spcm->stream[stream].d0i3_compatible);@@ -2513,7 +2516,7 @@ static int sof_dai_load(struct snd_soc_component *scomp, int index, return ret; dev_vdbg(scomp->dev, "tplg: pcm %s stream tokens: capture d0i3:%d\n",
spcm->pcm.pcm_name, spcm->stream[1].d0i3_compatible);
spcm->pcm.pcm_name, spcm->stream[stream].d0i3_compatible);
In the VirtIO case the sof_pcm_open() function isn't called on the host during guest streaming, which then leaves "work" structures uninitialised. However it is then used to handle position update messages from the DSP. Move their initialisation to immediately after allocation of the containing structure.
Signed-off-by: Guennadi Liakhovetski guennadi.liakhovetski@linux.intel.com --- sound/soc/sof/pcm.c | 4 +--- sound/soc/sof/sof-audio.h | 3 +++ sound/soc/sof/topology.c | 6 +++++- 3 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/sound/soc/sof/pcm.c b/sound/soc/sof/pcm.c index f4769e1..47cd741 100644 --- a/sound/soc/sof/pcm.c +++ b/sound/soc/sof/pcm.c @@ -57,7 +57,7 @@ static int sof_pcm_dsp_params(struct snd_sof_pcm *spcm, struct snd_pcm_substream /* * sof pcm period elapse work */ -static void sof_pcm_period_elapsed_work(struct work_struct *work) +void snd_sof_pcm_period_elapsed_work(struct work_struct *work) { struct snd_sof_pcm_stream *sps = container_of(work, struct snd_sof_pcm_stream, @@ -475,8 +475,6 @@ static int sof_pcm_open(struct snd_soc_component *component, dev_dbg(component->dev, "pcm: open stream %d dir %d\n", spcm->pcm.pcm_id, substream->stream);
- INIT_WORK(&spcm->stream[substream->stream].period_elapsed_work, - sof_pcm_period_elapsed_work);
caps = &spcm->pcm.caps[substream->stream];
diff --git a/sound/soc/sof/sof-audio.h b/sound/soc/sof/sof-audio.h index eacd10e..bf65f31a 100644 --- a/sound/soc/sof/sof-audio.h +++ b/sound/soc/sof/sof-audio.h @@ -11,6 +11,8 @@ #ifndef __SOUND_SOC_SOF_AUDIO_H #define __SOUND_SOC_SOF_AUDIO_H
+#include <linux/workqueue.h> + #include <sound/soc.h> #include <sound/control.h> #include <sound/sof/stream.h> /* needs to be included before control.h */ @@ -189,6 +191,7 @@ struct snd_sof_pcm *snd_sof_find_spcm_comp(struct snd_soc_component *scomp, struct snd_sof_pcm *snd_sof_find_spcm_pcm_id(struct snd_soc_component *scomp, unsigned int pcm_id); void snd_sof_pcm_period_elapsed(struct snd_pcm_substream *substream); +void snd_sof_pcm_period_elapsed_work(struct work_struct *work);
/* * Mixer IPC diff --git a/sound/soc/sof/topology.c b/sound/soc/sof/topology.c index 54437ca..fe8ba3e 100644 --- a/sound/soc/sof/topology.c +++ b/sound/soc/sof/topology.c @@ -9,6 +9,7 @@ //
#include <linux/firmware.h> +#include <linux/workqueue.h> #include <sound/tlv.h> #include <sound/pcm_params.h> #include <uapi/sound/sof/tokens.h> @@ -2461,8 +2462,11 @@ static int sof_dai_load(struct snd_soc_component *scomp, int index,
spcm->scomp = scomp;
- for_each_pcm_streams(stream) + for_each_pcm_streams(stream) { spcm->stream[stream].comp_id = COMP_ID_UNASSIGNED; + INIT_WORK(&spcm->stream[stream].period_elapsed_work, + snd_sof_pcm_period_elapsed_work); + }
spcm->pcm = *pcm; dev_dbg(scomp->dev, "tplg: load pcm %s\n", pcm->dai_name);
participants (2)
-
Guennadi Liakhovetski -
Pierre-Louis Bossart