[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <a8dc2a3a-27a2-40d6-9e67-6ea475701e44@linux.intel.com>
Date: Tue, 17 Oct 2023 17:37:20 -0500
From: Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>
To: Wesley Cheng <quic_wcheng@...cinc.com>, mathias.nyman@...el.com,
gregkh@...uxfoundation.org, lgirdwood@...il.com,
broonie@...nel.org, perex@...ex.cz, tiwai@...e.com,
agross@...nel.org, andersson@...nel.org, konrad.dybcio@...aro.org,
robh+dt@...nel.org, krzysztof.kozlowski+dt@...aro.org,
conor+dt@...nel.org, srinivas.kandagatla@...aro.org,
bgoswami@...cinc.com, Thinh.Nguyen@...opsys.com
Cc: linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org,
alsa-devel@...a-project.org, linux-arm-msm@...r.kernel.org,
devicetree@...r.kernel.org
Subject: Re: [PATCH v9 23/34] ALSA: usb-audio: Prevent starting of audio
stream if in use
On 10/17/23 15:00, Wesley Cheng wrote:
> With USB audio offloading, an audio session is started from the ASoC
> platform sound card and PCM devices. Likewise, the USB SND path is still
> readily available for use, in case the non-offload path is desired. In
> order to prevent the two entities from attempting to use the USB bus,
> introduce a flag that determines when either paths are in use.
>
> If a PCM device is already in use, the check will return an error to
> userspace notifying that the stream is currently busy. This ensures that
> only one path is using the USB substream.
>
> Signed-off-by: Wesley Cheng <quic_wcheng@...cinc.com>
> ---
> sound/usb/card.h | 1 +
> sound/usb/pcm.c | 19 +++++++++++++++++--
> sound/usb/qcom/qc_audio_offload.c | 15 ++++++++++++++-
should this be split in a generic part and a more specific qcom patch?
> 3 files changed, 32 insertions(+), 3 deletions(-)
>
> diff --git a/sound/usb/card.h b/sound/usb/card.h
> index e26292363cf0..01f7e10f30f4 100644
> --- a/sound/usb/card.h
> +++ b/sound/usb/card.h
> @@ -164,6 +164,7 @@ struct snd_usb_substream {
> unsigned int pkt_offset_adj; /* Bytes to drop from beginning of packets (for non-compliant devices) */
> unsigned int stream_offset_adj; /* Bytes to drop from beginning of stream (for non-compliant devices) */
>
> + unsigned int opened:1; /* pcm device opened */
> unsigned int running: 1; /* running status */
> unsigned int period_elapsed_pending; /* delay period handling */
>
> diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c
> index 3adb09ce1702..c2cb52cd5d23 100644
> --- a/sound/usb/pcm.c
> +++ b/sound/usb/pcm.c
> @@ -1241,8 +1241,15 @@ static int snd_usb_pcm_open(struct snd_pcm_substream *substream)
> struct snd_usb_stream *as = snd_pcm_substream_chip(substream);
> struct snd_pcm_runtime *runtime = substream->runtime;
> struct snd_usb_substream *subs = &as->substream[direction];
> + struct snd_usb_audio *chip = subs->stream->chip;
> int ret;
>
> + mutex_lock(&chip->mutex);
> + if (subs->opened) {
> + mutex_unlock(&chip->mutex);
> + return -EBUSY;
> + }
> +
> runtime->hw = snd_usb_hardware;
> /* need an explicit sync to catch applptr update in low-latency mode */
> if (direction == SNDRV_PCM_STREAM_PLAYBACK &&
> @@ -1259,13 +1266,17 @@ static int snd_usb_pcm_open(struct snd_pcm_substream *substream)
>
> ret = setup_hw_info(runtime, subs);
> if (ret < 0)
> - return ret;
> + goto out;
> ret = snd_usb_autoresume(subs->stream->chip);
> if (ret < 0)
> - return ret;
> + goto out;
> ret = snd_media_stream_init(subs, as->pcm, direction);
> if (ret < 0)
> snd_usb_autosuspend(subs->stream->chip);
> + subs->opened = 1;
> +out:
> + mutex_unlock(&chip->mutex);
> +
> return ret;
> }
>
> @@ -1274,6 +1285,7 @@ static int snd_usb_pcm_close(struct snd_pcm_substream *substream)
> int direction = substream->stream;
> struct snd_usb_stream *as = snd_pcm_substream_chip(substream);
> struct snd_usb_substream *subs = &as->substream[direction];
> + struct snd_usb_audio *chip = subs->stream->chip;
> int ret;
>
> snd_media_stop_pipeline(subs);
> @@ -1287,6 +1299,9 @@ static int snd_usb_pcm_close(struct snd_pcm_substream *substream)
>
> subs->pcm_substream = NULL;
> snd_usb_autosuspend(subs->stream->chip);
> + mutex_lock(&chip->mutex);
> + subs->opened = 0;
> + mutex_unlock(&chip->mutex);
>
> return 0;
> }
> diff --git a/sound/usb/qcom/qc_audio_offload.c b/sound/usb/qcom/qc_audio_offload.c
> index 320ce3a6688f..bd6b84f72c74 100644
> --- a/sound/usb/qcom/qc_audio_offload.c
> +++ b/sound/usb/qcom/qc_audio_offload.c
> @@ -1413,12 +1413,17 @@ static void handle_uaudio_stream_req(struct qmi_handle *handle,
> goto response;
> }
>
> + mutex_lock(&chip->mutex);
> if (req_msg->enable) {
> - if (info_idx < 0 || chip->system_suspend) {
> + if (info_idx < 0 || chip->system_suspend || subs->opened) {
> ret = -EBUSY;
> + mutex_unlock(&chip->mutex);
> +
> goto response;
> }
> + subs->opened = 1;
> }
> + mutex_unlock(&chip->mutex);
>
> if (req_msg->service_interval_valid) {
> ret = get_data_interval_from_si(subs,
> @@ -1440,6 +1445,11 @@ static void handle_uaudio_stream_req(struct qmi_handle *handle,
> if (!ret)
> ret = prepare_qmi_response(subs, req_msg, &resp,
> info_idx);
> + if (ret < 0) {
> + mutex_lock(&chip->mutex);
> + subs->opened = 0;
> + mutex_unlock(&chip->mutex);
> + }
> } else {
> info = &uadev[pcm_card_num].info[info_idx];
> if (info->data_ep_pipe) {
> @@ -1463,6 +1473,9 @@ static void handle_uaudio_stream_req(struct qmi_handle *handle,
> }
>
> disable_audio_stream(subs);
> + mutex_lock(&chip->mutex);
> + subs->opened = 0;
> + mutex_unlock(&chip->mutex);
> }
>
> response:
Powered by blists - more mailing lists