All of lore.kernel.org
 help / color / mirror / Atom feed
From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
To: alsa-devel@alsa-project.org
Cc: tiwai@suse.de, broonie@kernel.org,
	Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>,
	Kai Vehmanen <kai.vehmanen@linux.intel.com>
Subject: [alsa-devel] [PATCH 6/8] ASoC: SOF: fix PCM playback through ALSA OSS emulation
Date: Fri, 10 Jan 2020 17:57:49 -0600	[thread overview]
Message-ID: <20200110235751.3404-7-pierre-louis.bossart@linux.intel.com> (raw)
In-Reply-To: <20200110235751.3404-1-pierre-louis.bossart@linux.intel.com>

From: Kai Vehmanen <kai.vehmanen@linux.intel.com>

Any app using ALSA OSS emulation on top of SOF will fail
to error from OSS SNDCTL_DSP_SETFMT ioctl. Reported initially
as an issue with xournalpp (application using PortAudio with
an OSS backend), but applies more generally to other apps
using OSS as well.

Problem is caused by SOF PCM not supporting repeated calls
to hw_params(), without matching calls to pcm_free(). This
is however exactly what the ALSA OSS PCM code is doing when
it is handling the OSS ioctls.

The problem will lead to leaking of DSP resources and eventual
failure of DSP PCM_PARAMS IPC.

BugLink: https://github.com/thesofproject/linux/issues/1510
Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/sof/pcm.c | 53 ++++++++++++++++++++++++++-------------------
 1 file changed, 31 insertions(+), 22 deletions(-)

diff --git a/sound/soc/sof/pcm.c b/sound/soc/sof/pcm.c
index 9bb6388742e1..314f3095c12f 100644
--- a/sound/soc/sof/pcm.c
+++ b/sound/soc/sof/pcm.c
@@ -92,7 +92,27 @@ void snd_sof_pcm_period_elapsed(struct snd_pcm_substream *substream)
 }
 EXPORT_SYMBOL(snd_sof_pcm_period_elapsed);
 
-/* this may get called several times by oss emulation */
+static int sof_pcm_dsp_pcm_free(struct snd_pcm_substream *substream,
+				struct snd_sof_dev *sdev,
+				struct snd_sof_pcm *spcm)
+{
+	struct sof_ipc_stream stream;
+	struct sof_ipc_reply reply;
+	int ret;
+
+	stream.hdr.size = sizeof(stream);
+	stream.hdr.cmd = SOF_IPC_GLB_STREAM_MSG | SOF_IPC_STREAM_PCM_FREE;
+	stream.comp_id = spcm->stream[substream->stream].comp_id;
+
+	/* send IPC to the DSP */
+	ret = sof_ipc_tx_message(sdev->ipc, stream.hdr.cmd, &stream,
+				 sizeof(stream), &reply, sizeof(reply));
+	if (!ret)
+		spcm->prepared[substream->stream] = false;
+
+	return ret;
+}
+
 static int sof_pcm_hw_params(struct snd_soc_component *component,
 			     struct snd_pcm_substream *substream,
 			     struct snd_pcm_hw_params *params)
@@ -113,6 +133,16 @@ static int sof_pcm_hw_params(struct snd_soc_component *component,
 	if (!spcm)
 		return -EINVAL;
 
+	/*
+	 * Handle repeated calls to hw_params() without free_pcm() in
+	 * between. At least ALSA OSS emulation depends on this.
+	 */
+	if (spcm->prepared[substream->stream]) {
+		ret = sof_pcm_dsp_pcm_free(substream, sdev, spcm);
+		if (ret < 0)
+			return ret;
+	}
+
 	dev_dbg(component->dev, "pcm: hw params stream %d dir %d\n",
 		spcm->pcm.pcm_id, substream->stream);
 
@@ -201,27 +231,6 @@ static int sof_pcm_hw_params(struct snd_soc_component *component,
 	return ret;
 }
 
-static int sof_pcm_dsp_pcm_free(struct snd_pcm_substream *substream,
-				struct snd_sof_dev *sdev,
-				struct snd_sof_pcm *spcm)
-{
-	struct sof_ipc_stream stream;
-	struct sof_ipc_reply reply;
-	int ret;
-
-	stream.hdr.size = sizeof(stream);
-	stream.hdr.cmd = SOF_IPC_GLB_STREAM_MSG | SOF_IPC_STREAM_PCM_FREE;
-	stream.comp_id = spcm->stream[substream->stream].comp_id;
-
-	/* send IPC to the DSP */
-	ret = sof_ipc_tx_message(sdev->ipc, stream.hdr.cmd, &stream,
-				 sizeof(stream), &reply, sizeof(reply));
-	if (!ret)
-		spcm->prepared[substream->stream] = false;
-
-	return ret;
-}
-
 static int sof_pcm_hw_free(struct snd_soc_component *component,
 			   struct snd_pcm_substream *substream)
 {
-- 
2.20.1

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

  parent reply	other threads:[~2020-01-11  0:04 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-10 23:57 [alsa-devel] [PATCH 0/8] ASOC: SOF: Intel: improvements for Broadwell, HDA, OSS Pierre-Louis Bossart
2020-01-10 23:57 ` [alsa-devel] [PATCH 1/8] ASoC: Intel: bdw-rt5677: fix Kconfig dependencies Pierre-Louis Bossart
2020-01-13 15:13   ` [alsa-devel] Applied "ASoC: Intel: bdw-rt5677: fix Kconfig dependencies" to the asoc tree Mark Brown
2020-01-10 23:57 ` [alsa-devel] [PATCH 2/8] ASoC: Intel: bdw-rt5677: change cpu_dai and platform components for SOF Pierre-Louis Bossart
2020-01-13 15:13   ` [alsa-devel] Applied "ASoC: Intel: bdw-rt5677: change cpu_dai and platform components for SOF" to the asoc tree Mark Brown
2020-01-10 23:57 ` [alsa-devel] [PATCH 3/8] ASoC: Intel: broadwell: change cpu_dai and platform components for SOF Pierre-Louis Bossart
2020-01-13 15:13   ` [alsa-devel] Applied "ASoC: Intel: broadwell: change cpu_dai and platform components for SOF" to the asoc tree Mark Brown
2020-01-10 23:57 ` [alsa-devel] [PATCH 4/8] ASoC: Intel: bdw-rt5650: change cpu_dai and platform components for SOF Pierre-Louis Bossart
2020-01-13 15:13   ` [alsa-devel] Applied "ASoC: Intel: bdw-rt5650: change cpu_dai and platform components for SOF" to the asoc tree Mark Brown
2020-01-10 23:57 ` [alsa-devel] [PATCH 5/8] ASoC: SOF: Intel: lower print level to dbg if we will reinit DSP Pierre-Louis Bossart
2020-01-13 15:13   ` [alsa-devel] Applied "ASoC: SOF: Intel: lower print level to dbg if we will reinit DSP" to the asoc tree Mark Brown
2020-01-10 23:57 ` Pierre-Louis Bossart [this message]
2020-01-13 15:13   ` [alsa-devel] Applied "ASoC: SOF: fix PCM playback through ALSA OSS emulation" " Mark Brown
2020-01-10 23:57 ` [alsa-devel] [PATCH 7/8] ASoC: SOF: Intel: fix HDA codec driver probe with multiple controllers Pierre-Louis Bossart
2020-01-10 23:57 ` [alsa-devel] [PATCH 8/8] ASoC: hdac_hda: Fix error in driver removal after failed probe Pierre-Louis Bossart
2020-01-13 15:13   ` [alsa-devel] Applied "ASoC: hdac_hda: Fix error in driver removal after failed probe" to the asoc tree Mark Brown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200110235751.3404-7-pierre-louis.bossart@linux.intel.com \
    --to=pierre-louis.bossart@linux.intel.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=broonie@kernel.org \
    --cc=kai.vehmanen@linux.intel.com \
    --cc=tiwai@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.