mbox series

[0/7] ALSA: add virtio sound driver

Message ID 20210120003638.3339987-1-anton.yakovlev@opensynergy.com
Headers show
Series ALSA: add virtio sound driver | expand

Message

Anton Yakovlev Jan. 20, 2021, 12:36 a.m. UTC
This series implements a driver part of the virtio sound device
specification v8 [1].

The driver supports PCM playback and capture substreams, jack and
channel map controls. A message-based transport is used to write/read
PCM frames to/from a device.

The series is based (and was actually tested) on Linus's master
branch [2], on top of

commit 1e2a199f6ccd ("Merge tag 'spi-fix-v5.11-rc4' of ...")

As a device part was used OpenSynergy proprietary implementation.

Any comments are very welcome.

[1] https://lists.oasis-open.org/archives/virtio-dev/202003/msg00185.html
[2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git


Anton Yakovlev (7):
  uapi: virtio_ids: add a sound device type ID from OASIS spec
  uapi: virtio_snd: add the sound device header file
  ALSA: virtio: add virtio sound driver
  ALSA: virtio: handling control and I/O messages for the PCM device
  ALSA: virtio: PCM substream operators
  ALSA: virtio: introduce jack support
  ALSA: virtio: introduce device suspend/resume support

 MAINTAINERS                     |   8 +
 include/uapi/linux/virtio_ids.h |   1 +
 include/uapi/linux/virtio_snd.h | 361 +++++++++++++++++++
 sound/Kconfig                   |   2 +
 sound/Makefile                  |   3 +-
 sound/virtio/Kconfig            |  10 +
 sound/virtio/Makefile           |  13 +
 sound/virtio/virtio_card.c      | 593 ++++++++++++++++++++++++++++++++
 sound/virtio/virtio_card.h      | 121 +++++++
 sound/virtio/virtio_chmap.c     | 237 +++++++++++++
 sound/virtio/virtio_ctl_msg.c   | 293 ++++++++++++++++
 sound/virtio/virtio_ctl_msg.h   | 122 +++++++
 sound/virtio/virtio_jack.c      | 255 ++++++++++++++
 sound/virtio/virtio_pcm.c       | 582 +++++++++++++++++++++++++++++++
 sound/virtio/virtio_pcm.h       | 132 +++++++
 sound/virtio/virtio_pcm_msg.c   | 317 +++++++++++++++++
 sound/virtio/virtio_pcm_ops.c   | 524 ++++++++++++++++++++++++++++
 17 files changed, 3573 insertions(+), 1 deletion(-)
 create mode 100644 include/uapi/linux/virtio_snd.h
 create mode 100644 sound/virtio/Kconfig
 create mode 100644 sound/virtio/Makefile
 create mode 100644 sound/virtio/virtio_card.c
 create mode 100644 sound/virtio/virtio_card.h
 create mode 100644 sound/virtio/virtio_chmap.c
 create mode 100644 sound/virtio/virtio_ctl_msg.c
 create mode 100644 sound/virtio/virtio_ctl_msg.h
 create mode 100644 sound/virtio/virtio_jack.c
 create mode 100644 sound/virtio/virtio_pcm.c
 create mode 100644 sound/virtio/virtio_pcm.h
 create mode 100644 sound/virtio/virtio_pcm_msg.c
 create mode 100644 sound/virtio/virtio_pcm_ops.c

Comments

Michael S. Tsirkin Jan. 20, 2021, 8:36 a.m. UTC | #1
On Wed, Jan 20, 2021 at 01:36:33AM +0100, Anton Yakovlev wrote:
> Introduce the operators required for the operation of substreams.
> 
> Signed-off-by: Anton Yakovlev <anton.yakovlev@opensynergy.com>
> ---
>  sound/virtio/Makefile         |   3 +-
>  sound/virtio/virtio_pcm.c     |   5 +-
>  sound/virtio/virtio_pcm.h     |   2 +
>  sound/virtio/virtio_pcm_ops.c | 509 ++++++++++++++++++++++++++++++++++
>  4 files changed, 517 insertions(+), 2 deletions(-)
>  create mode 100644 sound/virtio/virtio_pcm_ops.c
> 
> diff --git a/sound/virtio/Makefile b/sound/virtio/Makefile
> index 626af3cc3ed7..34493226793f 100644
> --- a/sound/virtio/Makefile
> +++ b/sound/virtio/Makefile
> @@ -6,5 +6,6 @@ virtio_snd-objs := \
>  	virtio_card.o \
>  	virtio_ctl_msg.o \
>  	virtio_pcm.o \
> -	virtio_pcm_msg.o
> +	virtio_pcm_msg.o \
> +	virtio_pcm_ops.o
>  
> diff --git a/sound/virtio/virtio_pcm.c b/sound/virtio/virtio_pcm.c
> index 1ab50dcc88c8..6a1ca6b2c3ca 100644
> --- a/sound/virtio/virtio_pcm.c
> +++ b/sound/virtio/virtio_pcm.c
> @@ -121,7 +121,8 @@ static int virtsnd_pcm_build_hw(struct virtio_pcm_substream *substream,
>  		SNDRV_PCM_INFO_MMAP_VALID |
>  		SNDRV_PCM_INFO_BATCH |
>  		SNDRV_PCM_INFO_BLOCK_TRANSFER |
> -		SNDRV_PCM_INFO_INTERLEAVED;
> +		SNDRV_PCM_INFO_INTERLEAVED |
> +		SNDRV_PCM_INFO_PAUSE;
>  
>  	if (!info->channels_min || info->channels_min > info->channels_max) {
>  		dev_err(&vdev->dev,
> @@ -503,6 +504,8 @@ int virtsnd_pcm_build_devs(struct virtio_snd *snd)
>  				if (rc)
>  					return rc;
>  			}
> +
> +			snd_pcm_set_ops(pcm->pcm, i, &virtsnd_pcm_ops);
>  		}
>  
>  	return 0;
> diff --git a/sound/virtio/virtio_pcm.h b/sound/virtio/virtio_pcm.h
> index d011b7e1d18d..fe467bc05d8b 100644
> --- a/sound/virtio/virtio_pcm.h
> +++ b/sound/virtio/virtio_pcm.h
> @@ -90,6 +90,8 @@ struct virtio_pcm {
>  	struct virtio_pcm_stream streams[SNDRV_PCM_STREAM_LAST + 1];
>  };
>  
> +extern const struct snd_pcm_ops virtsnd_pcm_ops;
> +
>  int virtsnd_pcm_validate(struct virtio_device *vdev);
>  
>  int virtsnd_pcm_parse_cfg(struct virtio_snd *snd);
> diff --git a/sound/virtio/virtio_pcm_ops.c b/sound/virtio/virtio_pcm_ops.c
> new file mode 100644
> index 000000000000..8d26c1144ad6
> --- /dev/null
> +++ b/sound/virtio/virtio_pcm_ops.c
> @@ -0,0 +1,509 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Sound card driver for virtio
> + * Copyright (C) 2020  OpenSynergy GmbH
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +#include <sound/pcm_params.h>
> +
> +#include "virtio_card.h"
> +
> +/*
> + * Our main concern here is maintaining the correct state of the underlying I/O
> + * virtqueues. Thus, operators are implemented to support all of the following
> + * possible control paths (excluding all trivial ones):
> + *
> + *                        +---------+
> + *                        | open()  |<------------------+
> + *                        +----+----+                   |
> + *                             v                        |
> + *                      +------+------+                 |
> + *       +------------->| hw_params() |<-------------+  |
> + *       |              +-------------+              |  |
> + *       |                     v                     |  |
> + *       |               +-----------+               |  |
> + *       |               | prepare() |<-----------+  |  |
> + *       |               +-----------+            |  |  |
> + *       |                     v                  |  |  |
> + *       |        +-------------------------+     |  |  |
> + * +-----------+  | trigger(START/          |     |  |  |
> + * | restore() |  |         PAUSE_RELEASE/  |<-+  |  |  |
> + * +-----------+  |         RESUME)         |  |  |  |  |
> + *       ^        +-------------------------+  |  |  |  |
> + *       |                     v               |  |  |  |
> + *       |               +-----------+         |  |  |  |
> + *       |               | pointer() |         |  |  |  |
> + *       |               +-----------+         |  |  |  |
> + *       |                     v               |  |  |  |
> + *       |          +---------------------+    |  |  |  |
> + * +-----------+    | trigger(STOP/       |----+  |  |  |
> + * | freeze()  |<---|         PAUSE_PUSH/ |-------+  |  |
> + * +-----------+    |         SUSPEND)    |          |  |
> + *                  +---------------------+          |  |
> + *                             v                     |  |
> + *                       +-----------+               |  |
> + *                       | hw_free() |---------------+  |
> + *                       +-----------+                  |
> + *                             v                        |
> + *                        +---------+                   |
> + *                        | close() |-------------------+
> + *                        +---------+
> + */
> +
> +/* Map for converting ALSA format to VirtIO format. */
> +struct virtsnd_a2v_format {
> +	unsigned int alsa_bit;
> +	unsigned int vio_bit;
> +};
> +
> +static const struct virtsnd_a2v_format g_a2v_format_map[] = {
> +	{ SNDRV_PCM_FORMAT_IMA_ADPCM, VIRTIO_SND_PCM_FMT_IMA_ADPCM },
> +	{ SNDRV_PCM_FORMAT_MU_LAW, VIRTIO_SND_PCM_FMT_MU_LAW },
> +	{ SNDRV_PCM_FORMAT_A_LAW, VIRTIO_SND_PCM_FMT_A_LAW },
> +	{ SNDRV_PCM_FORMAT_S8, VIRTIO_SND_PCM_FMT_S8 },
> +	{ SNDRV_PCM_FORMAT_U8, VIRTIO_SND_PCM_FMT_U8 },
> +	{ SNDRV_PCM_FORMAT_S16_LE, VIRTIO_SND_PCM_FMT_S16 },
> +	{ SNDRV_PCM_FORMAT_U16_LE, VIRTIO_SND_PCM_FMT_U16 },
> +	{ SNDRV_PCM_FORMAT_S18_3LE, VIRTIO_SND_PCM_FMT_S18_3 },
> +	{ SNDRV_PCM_FORMAT_U18_3LE, VIRTIO_SND_PCM_FMT_U18_3 },
> +	{ SNDRV_PCM_FORMAT_S20_3LE, VIRTIO_SND_PCM_FMT_S20_3 },
> +	{ SNDRV_PCM_FORMAT_U20_3LE, VIRTIO_SND_PCM_FMT_U20_3 },
> +	{ SNDRV_PCM_FORMAT_S24_3LE, VIRTIO_SND_PCM_FMT_S24_3 },
> +	{ SNDRV_PCM_FORMAT_U24_3LE, VIRTIO_SND_PCM_FMT_U24_3 },
> +	{ SNDRV_PCM_FORMAT_S20_LE, VIRTIO_SND_PCM_FMT_S20 },
> +	{ SNDRV_PCM_FORMAT_U20_LE, VIRTIO_SND_PCM_FMT_U20 },
> +	{ SNDRV_PCM_FORMAT_S24_LE, VIRTIO_SND_PCM_FMT_S24 },
> +	{ SNDRV_PCM_FORMAT_U24_LE, VIRTIO_SND_PCM_FMT_U24 },
> +	{ SNDRV_PCM_FORMAT_S32_LE, VIRTIO_SND_PCM_FMT_S32 },
> +	{ SNDRV_PCM_FORMAT_U32_LE, VIRTIO_SND_PCM_FMT_U32 },
> +	{ SNDRV_PCM_FORMAT_FLOAT_LE, VIRTIO_SND_PCM_FMT_FLOAT },
> +	{ SNDRV_PCM_FORMAT_FLOAT64_LE, VIRTIO_SND_PCM_FMT_FLOAT64 },
> +	{ SNDRV_PCM_FORMAT_DSD_U8, VIRTIO_SND_PCM_FMT_DSD_U8 },
> +	{ SNDRV_PCM_FORMAT_DSD_U16_LE, VIRTIO_SND_PCM_FMT_DSD_U16 },
> +	{ SNDRV_PCM_FORMAT_DSD_U32_LE, VIRTIO_SND_PCM_FMT_DSD_U32 },
> +	{ SNDRV_PCM_FORMAT_IEC958_SUBFRAME_LE,
> +	  VIRTIO_SND_PCM_FMT_IEC958_SUBFRAME }
> +};
> +
> +/* Map for converting ALSA frame rate to VirtIO frame rate. */
> +struct virtsnd_a2v_rate {
> +	unsigned int rate;
> +	unsigned int vio_bit;
> +};
> +
> +static const struct virtsnd_a2v_rate g_a2v_rate_map[] = {
> +	{ 5512, VIRTIO_SND_PCM_RATE_5512 },
> +	{ 8000, VIRTIO_SND_PCM_RATE_8000 },
> +	{ 11025, VIRTIO_SND_PCM_RATE_11025 },
> +	{ 16000, VIRTIO_SND_PCM_RATE_16000 },
> +	{ 22050, VIRTIO_SND_PCM_RATE_22050 },
> +	{ 32000, VIRTIO_SND_PCM_RATE_32000 },
> +	{ 44100, VIRTIO_SND_PCM_RATE_44100 },
> +	{ 48000, VIRTIO_SND_PCM_RATE_48000 },
> +	{ 64000, VIRTIO_SND_PCM_RATE_64000 },
> +	{ 88200, VIRTIO_SND_PCM_RATE_88200 },
> +	{ 96000, VIRTIO_SND_PCM_RATE_96000 },
> +	{ 176400, VIRTIO_SND_PCM_RATE_176400 },
> +	{ 192000, VIRTIO_SND_PCM_RATE_192000 }
> +};
> +
> +/**
> + * virtsnd_pcm_release() - Release the PCM substream on the device side.
> + * @substream: VirtIO substream.
> + *
> + * The function waits for all pending I/O messages to complete.
> + *
> + * Context: Any context that permits to sleep.
> + * Return: 0 on success, -errno on failure.
> + */
> +static inline bool virtsnd_pcm_released(struct virtio_pcm_substream *substream)
> +{
> +	return atomic_read(&substream->msg_count) == 0;

All this use of atomic_read/atomic_set all over the place makes me pause.
Could you add documentation explaining the use rules for these atomic fields?

> +}
> +
> +static int virtsnd_pcm_release(struct virtio_pcm_substream *substream)
> +{
> +	struct virtio_snd *snd = substream->snd;
> +	struct virtio_snd_msg *msg;
> +	unsigned int js = msecs_to_jiffies(msg_timeout_ms);
> +	int rc;
> +
> +	msg = virtsnd_pcm_ctl_msg_alloc(substream, VIRTIO_SND_R_PCM_RELEASE,
> +					GFP_KERNEL);
> +	if (IS_ERR(msg))
> +		return PTR_ERR(msg);
> +
> +	rc = virtsnd_ctl_msg_send_sync(snd, msg);
> +	if (rc)
> +		return rc;
> +
> +	return wait_event_interruptible_timeout(substream->msg_empty,
> +						virtsnd_pcm_released(substream),
> +						js);
> +}
> +
> +/**
> + * virtsnd_pcm_open() - Open the PCM substream.
> + * @substream: Kernel ALSA substream.
> + *
> + * Context: Any context.
> + * Return: 0 on success, -errno on failure.
> + */
> +static int virtsnd_pcm_open(struct snd_pcm_substream *substream)
> +{
> +	struct virtio_pcm *pcm = snd_pcm_substream_chip(substream);
> +	struct virtio_pcm_substream *ss = NULL;
> +
> +	if (pcm) {
> +		switch (substream->stream) {
> +		case SNDRV_PCM_STREAM_PLAYBACK:
> +		case SNDRV_PCM_STREAM_CAPTURE: {
> +			struct virtio_pcm_stream *stream =
> +				&pcm->streams[substream->stream];
> +
> +			if (substream->number < stream->nsubstreams)
> +				ss = stream->substreams[substream->number];
> +			break;
> +		}
> +		}
> +	}
> +
> +	if (!ss)
> +		return -EBADFD;
> +
> +	substream->runtime->hw = ss->hw;
> +	substream->private_data = ss;
> +
> +	return 0;
> +}
> +
> +/**
> + * virtsnd_pcm_close() - Close the PCM substream.
> + * @substream: Kernel ALSA substream.
> + *
> + * Context: Any context.
> + * Return: 0.
> + */
> +static int virtsnd_pcm_close(struct snd_pcm_substream *substream)
> +{
> +	return 0;
> +}
> +
> +/**
> + * virtsnd_pcm_hw_params() - Set the parameters of the PCM substream.
> + * @substream: Kernel ALSA substream.
> + * @hw_params: Hardware parameters (can be NULL).
> + *
> + * The function can be called both from the upper level (in this case,
> + * @hw_params is not NULL) or from the driver itself (in this case, @hw_params
> + * is NULL, and the parameter values are taken from the runtime structure).
> + *
> + * In all cases, the function:
> + *   1. checks the state of the virtqueue and, if necessary, tries to fix it,
> + *   2. sets the parameters on the device side,
> + *   3. allocates a hardware buffer and I/O messages.
> + *
> + * Context: Any context that permits to sleep.
> + * Return: 0 on success, -errno on failure.
> + */
> +static int virtsnd_pcm_hw_params(struct snd_pcm_substream *substream,
> +				 struct snd_pcm_hw_params *hw_params)
> +{
> +	struct snd_pcm_runtime *runtime = substream->runtime;
> +	struct virtio_pcm_substream *ss = snd_pcm_substream_chip(substream);
> +	struct virtio_device *vdev = ss->snd->vdev;
> +	struct virtio_snd_msg *msg;
> +	struct virtio_snd_pcm_set_params *request;
> +	snd_pcm_format_t format;
> +	unsigned int channels;
> +	unsigned int rate;
> +	unsigned int buffer_bytes;
> +	unsigned int period_bytes;
> +	unsigned int periods;
> +	unsigned int i;
> +	int vformat = -1;
> +	int vrate = -1;
> +	int rc;
> +
> +	/*
> +	 * If we got here after ops->trigger() was called, the queue may
> +	 * still contain messages. In this case, we need to release the
> +	 * substream first.
> +	 */
> +	if (atomic_read(&ss->msg_count)) {
> +		rc = virtsnd_pcm_release(ss);
> +		if (rc) {
> +			dev_err(&vdev->dev,
> +				"SID %u: invalid I/O queue state\n",
> +				ss->sid);
> +			return rc;
> +		}
> +	}
> +
> +	/* Set hardware parameters in device */
> +	if (hw_params) {
> +		format = params_format(hw_params);
> +		channels = params_channels(hw_params);
> +		rate = params_rate(hw_params);
> +		buffer_bytes = params_buffer_bytes(hw_params);
> +		period_bytes = params_period_bytes(hw_params);
> +		periods = params_periods(hw_params);
> +	} else {
> +		format = runtime->format;
> +		channels = runtime->channels;
> +		rate = runtime->rate;
> +		buffer_bytes = frames_to_bytes(runtime, runtime->buffer_size);
> +		period_bytes = frames_to_bytes(runtime, runtime->period_size);
> +		periods = runtime->periods;
> +	}
> +
> +	for (i = 0; i < ARRAY_SIZE(g_a2v_format_map); ++i)
> +		if (g_a2v_format_map[i].alsa_bit == format) {
> +			vformat = g_a2v_format_map[i].vio_bit;
> +
> +			break;
> +		}
> +
> +	for (i = 0; i < ARRAY_SIZE(g_a2v_rate_map); ++i)
> +		if (g_a2v_rate_map[i].rate == rate) {
> +			vrate = g_a2v_rate_map[i].vio_bit;
> +
> +			break;
> +		}
> +
> +	if (vformat == -1 || vrate == -1)
> +		return -EINVAL;
> +
> +	msg = virtsnd_pcm_ctl_msg_alloc(ss, VIRTIO_SND_R_PCM_SET_PARAMS,
> +					GFP_KERNEL);
> +	if (IS_ERR(msg))
> +		return PTR_ERR(msg);
> +
> +	request = sg_virt(&msg->sg_request);
> +
> +	request->buffer_bytes = cpu_to_virtio32(vdev, buffer_bytes);
> +	request->period_bytes = cpu_to_virtio32(vdev, period_bytes);
> +	request->channels = channels;
> +	request->format = vformat;
> +	request->rate = vrate;
> +
> +	if (ss->features & (1U << VIRTIO_SND_PCM_F_MSG_POLLING))
> +		request->features |=
> +			cpu_to_virtio32(vdev,
> +					1U << VIRTIO_SND_PCM_F_MSG_POLLING);
> +
> +	if (ss->features & (1U << VIRTIO_SND_PCM_F_EVT_XRUNS))
> +		request->features |=
> +			cpu_to_virtio32(vdev,
> +					1U << VIRTIO_SND_PCM_F_EVT_XRUNS);
> +
> +	rc = virtsnd_ctl_msg_send_sync(ss->snd, msg);
> +	if (rc)
> +		return rc;
> +
> +	/* If the buffer was already allocated earlier, do nothing. */
> +	if (runtime->dma_area)
> +		return 0;
> +
> +	/* Allocate hardware buffer */
> +	rc = snd_pcm_lib_malloc_pages(substream, buffer_bytes);
> +	if (rc < 0)
> +		return rc;
> +
> +	/* Allocate and initialize I/O messages */
> +	rc = virtsnd_pcm_msg_alloc(ss, periods, runtime->dma_area,
> +				   period_bytes);
> +	if (rc)
> +		snd_pcm_lib_free_pages(substream);
> +
> +	return rc;
> +}
> +
> +/**
> + * virtsnd_pcm_hw_free() - Reset the parameters of the PCM substream.
> + * @substream: Kernel ALSA substream.
> + *
> + * The function does the following:
> + *   1. tries to release the PCM substream on the device side,
> + *   2. frees the hardware buffer.
> + *
> + * Context: Any context that permits to sleep.
> + * Return: 0 on success, -errno on failure.
> + */
> +static int virtsnd_pcm_hw_free(struct snd_pcm_substream *substream)
> +{
> +	struct virtio_pcm_substream *ss = snd_pcm_substream_chip(substream);
> +	int rc;
> +
> +	rc = virtsnd_pcm_release(ss);
> +
> +	/*
> +	 * Even if we failed to send the RELEASE message or wait for the queue
> +	 * flush to complete, we can safely delete the buffer. Because after
> +	 * receiving the STOP command, the device must stop all I/O message
> +	 * processing. If there are still pending messages in the queue, the
> +	 * next ops->hw_params() call should deal with this.
> +	 */
> +	snd_pcm_lib_free_pages(substream);
> +
> +	return rc;
> +}
> +
> +/**
> + * virtsnd_pcm_hw_params() - Prepare the PCM substream.
> + * @substream: Kernel ALSA substream.
> + *
> + * The function can be called both from the upper level or from the driver
> + * itself.
> + *
> + * In all cases, the function:
> + *   1. checks the state of the virtqueue and, if necessary, tries to fix it,
> + *   2. prepares the substream on the device side.
> + *
> + * Context: Any context that permits to sleep. May take and release the tx/rx
> + *          queue spinlock.
> + * Return: 0 on success, -errno on failure.
> + */
> +static int virtsnd_pcm_prepare(struct snd_pcm_substream *substream)
> +{
> +	struct virtio_pcm_substream *ss = snd_pcm_substream_chip(substream);
> +	struct virtio_snd_queue *queue = virtsnd_pcm_queue(ss);
> +	struct virtio_snd_msg *msg;
> +	unsigned long flags;
> +	int rc;
> +
> +	/*
> +	 * If we got here after ops->trigger() was called, the queue may
> +	 * still contain messages. In this case, we need to reset the
> +	 * substream first.
> +	 */
> +	if (atomic_read(&ss->msg_count)) {
> +		rc = virtsnd_pcm_hw_params(substream, NULL);
> +		if (rc)
> +			return rc;
> +	}
> +
> +	spin_lock_irqsave(&queue->lock, flags);
> +	ss->msg_last_enqueued = -1;
> +	spin_unlock_irqrestore(&queue->lock, flags);
> +
> +	/*
> +	 * Since I/O messages are asynchronous, they can be completed
> +	 * when the runtime structure no longer exists. Since each
> +	 * completion implies incrementing the hw_ptr, we cache all the
> +	 * current values needed to compute the new hw_ptr value.
> +	 */
> +	ss->frame_bytes = substream->runtime->frame_bits >> 3;
> +	ss->period_size = substream->runtime->period_size;
> +	ss->buffer_size = substream->runtime->buffer_size;
> +
> +	atomic_set(&ss->hw_ptr, 0);
> +	atomic_set(&ss->xfer_xrun, 0);
> +	atomic_set(&ss->msg_count, 0);
> +
> +	msg = virtsnd_pcm_ctl_msg_alloc(ss, VIRTIO_SND_R_PCM_PREPARE,
> +					GFP_KERNEL);
> +	if (IS_ERR(msg))
> +		return PTR_ERR(msg);
> +
> +	return virtsnd_ctl_msg_send_sync(ss->snd, msg);
> +}
> +
> +/**
> + * virtsnd_pcm_trigger() - Process command for the PCM substream.
> + * @substream: Kernel ALSA substream.
> + * @command: Substream command (SNDRV_PCM_TRIGGER_XXX).
> + *
> + * Depending on the command, the function does the following:
> + *   1. enables/disables data transmission,
> + *   2. starts/stops the substream on the device side.
> + *
> + * Context: Atomic context. May take and release the tx/rx queue spinlock.
> + * Return: 0 on success, -errno on failure.
> + */
> +static int virtsnd_pcm_trigger(struct snd_pcm_substream *substream, int command)
> +{
> +	struct virtio_pcm_substream *ss = snd_pcm_substream_chip(substream);
> +	struct virtio_snd *snd = ss->snd;
> +	struct virtio_snd_queue *queue = virtsnd_pcm_queue(ss);
> +	struct virtio_snd_msg *msg;
> +
> +	switch (command) {
> +	case SNDRV_PCM_TRIGGER_START:
> +	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: {
> +		int rc;
> +
> +		spin_lock(&queue->lock);
> +		rc = virtsnd_pcm_msg_send(ss);
> +		spin_unlock(&queue->lock);
> +		if (rc)
> +			return rc;
> +
> +		atomic_set(&ss->xfer_enabled, 1);
> +
> +		msg = virtsnd_pcm_ctl_msg_alloc(ss, VIRTIO_SND_R_PCM_START,
> +						GFP_ATOMIC);
> +		if (IS_ERR(msg))
> +			return PTR_ERR(msg);
> +
> +		return virtsnd_ctl_msg_send(snd, msg);
> +	}
> +	case SNDRV_PCM_TRIGGER_STOP:
> +	case SNDRV_PCM_TRIGGER_PAUSE_PUSH: {
> +		atomic_set(&ss->xfer_enabled, 0);
> +
> +		msg = virtsnd_pcm_ctl_msg_alloc(ss, VIRTIO_SND_R_PCM_STOP,
> +						GFP_ATOMIC);
> +		if (IS_ERR(msg))
> +			return PTR_ERR(msg);
> +
> +		return virtsnd_ctl_msg_send(snd, msg);
> +	}
> +	default: {
> +		return -EINVAL;
> +	}
> +	}
> +}
> +
> +/**
> + * virtsnd_pcm_pointer() - Get the current hardware position for the PCM
> + *                         substream.
> + * @substream: Kernel ALSA substream.
> + *
> + * Context: Atomic context.
> + * Return: Hardware position in frames inside [0 ... buffer_size) range.
> + */
> +static snd_pcm_uframes_t
> +virtsnd_pcm_pointer(struct snd_pcm_substream *substream)
> +{
> +	struct virtio_pcm_substream *ss = snd_pcm_substream_chip(substream);
> +
> +	if (atomic_read(&ss->xfer_xrun))
> +		return SNDRV_PCM_POS_XRUN;
> +
> +	return (snd_pcm_uframes_t)atomic_read(&ss->hw_ptr);
> +}
> +
> +/* PCM substream operators map. */
> +const struct snd_pcm_ops virtsnd_pcm_ops = {
> +	.open = virtsnd_pcm_open,
> +	.close = virtsnd_pcm_close,
> +	.ioctl = snd_pcm_lib_ioctl,
> +	.hw_params = virtsnd_pcm_hw_params,
> +	.hw_free = virtsnd_pcm_hw_free,
> +	.prepare = virtsnd_pcm_prepare,
> +	.trigger = virtsnd_pcm_trigger,
> +	.pointer = virtsnd_pcm_pointer,
> +};
> -- 
> 2.30.0
> 
>
Anton Yakovlev Jan. 24, 2021, 4:49 p.m. UTC | #2
On 20.01.2021 09:29, Michael S. Tsirkin wrote:
> CAUTION: This email originated from outside of the organization.
> Do not click links or open attachments unless you recognize the sender and know the content is safe.
> 
> 
> On Wed, Jan 20, 2021 at 01:36:32AM +0100, Anton Yakovlev wrote:
>> The driver implements a message-based transport for I/O substream
>> operations. Before the start of the substream, the hardware buffer is
>> sliced into I/O messages, the number of which is equal to the current
>> number of periods. The size of each message is equal to the current
>> size of one period.
>>
>> I/O messages are organized in an ordered queue. The completion of the
>> I/O message indicates an expired period (the only exception is the end
>> of the stream for the capture substream). Upon completion, the message
>> is automatically re-added to the end of the queue.
>>
>> When an I/O message is completed, the hw_ptr value is incremented
>> unconditionally (to ensure that the hw_ptr always correctly reflects
>> the state of the messages in the virtqueue). Due to its asynchronous
>> nature, a message can be completed when the runtime structure no longer
>> exists. For this reason, the values from this structure are cached in
>> the virtio substream, which are required to calculate the new value of
>> the hw_ptr.
>>
>> Signed-off-by: Anton Yakovlev <anton.yakovlev@opensynergy.com>
>> ---
>>   sound/virtio/Makefile         |   3 +-
>>   sound/virtio/virtio_card.c    |  33 ++++
>>   sound/virtio/virtio_card.h    |   9 +
>>   sound/virtio/virtio_pcm.c     |   3 +
>>   sound/virtio/virtio_pcm.h     |  31 ++++
>>   sound/virtio/virtio_pcm_msg.c | 317 ++++++++++++++++++++++++++++++++++
>>   6 files changed, 395 insertions(+), 1 deletion(-)
>>   create mode 100644 sound/virtio/virtio_pcm_msg.c
>>
>> diff --git a/sound/virtio/Makefile b/sound/virtio/Makefile
>> index 69162a545a41..626af3cc3ed7 100644
>> --- a/sound/virtio/Makefile
>> +++ b/sound/virtio/Makefile
>> @@ -5,5 +5,6 @@ obj-$(CONFIG_SND_VIRTIO) += virtio_snd.o
>>   virtio_snd-objs := \
>>        virtio_card.o \
>>        virtio_ctl_msg.o \
>> -     virtio_pcm.o
>> +     virtio_pcm.o \
>> +     virtio_pcm_msg.o
>>
>> diff --git a/sound/virtio/virtio_card.c b/sound/virtio/virtio_card.c
>> index 293d497f24e7..dc703fc662f5 100644
>> --- a/sound/virtio/virtio_card.c
>> +++ b/sound/virtio/virtio_card.c
>> @@ -145,6 +145,12 @@ static int virtsnd_find_vqs(struct virtio_snd *snd)
>>        callbacks[VIRTIO_SND_VQ_CONTROL] = virtsnd_ctl_notify_cb;
>>        callbacks[VIRTIO_SND_VQ_EVENT] = virtsnd_event_notify_cb;
>>
>> +     virtio_cread(vdev, struct virtio_snd_config, streams, &n);
>> +     if (n) {
>> +             callbacks[VIRTIO_SND_VQ_TX] = virtsnd_pcm_tx_notify_cb;
>> +             callbacks[VIRTIO_SND_VQ_RX] = virtsnd_pcm_rx_notify_cb;
>> +     }
>> +
>>        rc = virtio_find_vqs(vdev, VIRTIO_SND_VQ_MAX, vqs, callbacks, names,
>>                             NULL);
>>        if (rc) {
>> @@ -186,15 +192,42 @@ static int virtsnd_find_vqs(struct virtio_snd *snd)
>>    * virtsnd_enable_vqs() - Enable the event, tx and rx virtqueues.
>>    * @snd: VirtIO sound device.
>>    *
>> + * The tx queue is enabled only if the device supports playback stream(s).
>> + *
>> + * The rx queue is enabled only if the device supports capture stream(s).
>> + *
>>    * Context: Any context.
>>    */
>>   static void virtsnd_enable_vqs(struct virtio_snd *snd)
>>   {
>> +     struct virtio_device *vdev = snd->vdev;
>>        struct virtqueue *vqueue;
>> +     struct virtio_pcm *pcm;
>> +     unsigned int npbs = 0;
>> +     unsigned int ncps = 0;
>>
>>        vqueue = snd->queues[VIRTIO_SND_VQ_EVENT].vqueue;
>>        if (!virtqueue_enable_cb(vqueue))
>>                virtsnd_event_notify_cb(vqueue);
>> +
>> +     list_for_each_entry(pcm, &snd->pcm_list, list) {
>> +             npbs += pcm->streams[SNDRV_PCM_STREAM_PLAYBACK].nsubstreams;
>> +             ncps += pcm->streams[SNDRV_PCM_STREAM_CAPTURE].nsubstreams;
>> +     }
>> +
>> +     if (npbs) {
>> +             vqueue = snd->queues[VIRTIO_SND_VQ_TX].vqueue;
>> +             if (!virtqueue_enable_cb(vqueue))
>> +                     dev_warn(&vdev->dev,
>> +                              "suspicious notification in the TX queue\n");
>> +     }
>> +
>> +     if (ncps) {
>> +             vqueue = snd->queues[VIRTIO_SND_VQ_RX].vqueue;
>> +             if (!virtqueue_enable_cb(vqueue))
>> +                     dev_warn(&vdev->dev,
>> +                              "suspicious notification in the RX queue\n");
>> +     }
> 
> Not sure how all this prevents use of same vq from multiple threads ...
> And why are we sure there are no buffers yet?  If that is because
> nothing yet happened, then I'd also like to point out that a vq starts
> out with callbacks enabled, so you don't need to do that first thing ...

Yes, I redone that logic in v2.


>>   }
>>
>>   /**
>> diff --git a/sound/virtio/virtio_card.h b/sound/virtio/virtio_card.h
>> index be6651a6aaf8..b11c09984882 100644
>> --- a/sound/virtio/virtio_card.h
>> +++ b/sound/virtio/virtio_card.h
>> @@ -89,4 +89,13 @@ virtsnd_rx_queue(struct virtio_snd *snd)
>>        return &snd->queues[VIRTIO_SND_VQ_RX];
>>   }
>>
>> +static inline struct virtio_snd_queue *
>> +virtsnd_pcm_queue(struct virtio_pcm_substream *substream)
>> +{
>> +     if (substream->direction == SNDRV_PCM_STREAM_PLAYBACK)
>> +             return virtsnd_tx_queue(substream->snd);
>> +     else
>> +             return virtsnd_rx_queue(substream->snd);
>> +}
>> +
>>   #endif /* VIRTIO_SND_CARD_H */
>> diff --git a/sound/virtio/virtio_pcm.c b/sound/virtio/virtio_pcm.c
>> index 036990b7b78a..1ab50dcc88c8 100644
>> --- a/sound/virtio/virtio_pcm.c
>> +++ b/sound/virtio/virtio_pcm.c
>> @@ -376,6 +376,7 @@ int virtsnd_pcm_parse_cfg(struct virtio_snd *snd)
>>
>>                substream->snd = snd;
>>                substream->sid = i;
>> +             init_waitqueue_head(&substream->msg_empty);
>>
>>                rc = virtsnd_pcm_build_hw(substream, &info[i]);
>>                if (rc)
>> @@ -530,6 +531,8 @@ void virtsnd_pcm_event(struct virtio_snd *snd, struct virtio_snd_event *event)
>>                break;
>>        }
>>        case VIRTIO_SND_EVT_PCM_XRUN: {
>> +             if (atomic_read(&substream->xfer_enabled))
>> +                     atomic_set(&substream->xfer_xrun, 1);
>>                break;
>>        }
>>        }
>> diff --git a/sound/virtio/virtio_pcm.h b/sound/virtio/virtio_pcm.h
>> index 73fb4d9dc524..d011b7e1d18d 100644
>> --- a/sound/virtio/virtio_pcm.h
>> +++ b/sound/virtio/virtio_pcm.h
>> @@ -24,6 +24,7 @@
>>   #include <sound/pcm.h>
>>
>>   struct virtio_pcm;
>> +struct virtio_pcm_msg;
>>
>>   /**
>>    * struct virtio_pcm_substream - VirtIO PCM substream.
>> @@ -34,6 +35,16 @@ struct virtio_pcm;
>>    * @features: Stream VirtIO feature bit map (1 << VIRTIO_SND_PCM_F_XXX).
>>    * @substream: Kernel ALSA substream.
>>    * @hw: Kernel ALSA substream hardware descriptor.
>> + * @frame_bytes: Current frame size in bytes.
>> + * @period_size: Current period size in frames.
>> + * @buffer_size: Current buffer size in frames.
>> + * @hw_ptr: Substream hardware pointer value in frames [0 ... buffer_size).
>> + * @xfer_enabled: Data transfer state (0 - off, 1 - on).
>> + * @xfer_xrun: Data underflow/overflow state (0 - no xrun, 1 - xrun).
>> + * @msgs: I/O messages.
>> + * @msg_last_enqueued: Index of the last I/O message added to the virtqueue.
>> + * @msg_count: Number of pending I/O messages in the virtqueue.
>> + * @msg_empty: Notify when msg_count is zero.
>>    */
>>   struct virtio_pcm_substream {
>>        struct virtio_snd *snd;
>> @@ -43,6 +54,16 @@ struct virtio_pcm_substream {
>>        u32 features;
>>        struct snd_pcm_substream *substream;
>>        struct snd_pcm_hardware hw;
>> +     unsigned int frame_bytes;
>> +     snd_pcm_uframes_t period_size;
>> +     snd_pcm_uframes_t buffer_size;
>> +     atomic_t hw_ptr;
>> +     atomic_t xfer_enabled;
>> +     atomic_t xfer_xrun;
>> +     struct virtio_pcm_msg *msgs;
>> +     int msg_last_enqueued;
>> +     atomic_t msg_count;
>> +     wait_queue_head_t msg_empty;
>>   };
>>
>>   /**
>> @@ -86,4 +107,14 @@ struct virtio_pcm *virtsnd_pcm_find(struct virtio_snd *snd, unsigned int nid);
>>   struct virtio_pcm *virtsnd_pcm_find_or_create(struct virtio_snd *snd,
>>                                              unsigned int nid);
>>
>> +struct virtio_snd_msg *
>> +virtsnd_pcm_ctl_msg_alloc(struct virtio_pcm_substream *substream,
>> +                       unsigned int command, gfp_t gfp);
>> +
>> +int virtsnd_pcm_msg_alloc(struct virtio_pcm_substream *substream,
>> +                       unsigned int nmsg, u8 *dma_area,
>> +                       unsigned int period_bytes);
>> +
>> +int virtsnd_pcm_msg_send(struct virtio_pcm_substream *substream);
>> +
>>   #endif /* VIRTIO_SND_PCM_H */
>> diff --git a/sound/virtio/virtio_pcm_msg.c b/sound/virtio/virtio_pcm_msg.c
>> new file mode 100644
>> index 000000000000..cfbe5935527a
>> --- /dev/null
>> +++ b/sound/virtio/virtio_pcm_msg.c
>> @@ -0,0 +1,317 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * Sound card driver for virtio
>> + * Copyright (C) 2020  OpenSynergy GmbH
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program; if not, see <http://www.gnu.org/licenses/>.
>> + */
>> +#include <sound/pcm_params.h>
>> +
>> +#include "virtio_card.h"
>> +
>> +/**
>> + * enum pcm_msg_sg_index - Scatter-gather element indexes for an I/O message.
>> + * @PCM_MSG_SG_XFER: Element containing a virtio_snd_pcm_xfer structure.
>> + * @PCM_MSG_SG_DATA: Element containing a data buffer.
>> + * @PCM_MSG_SG_STATUS: Element containing a virtio_snd_pcm_status structure.
>> + * @PCM_MSG_SG_MAX: The maximum number of elements in the scatter-gather table.
>> + *
>> + * These values are used as the index of the payload scatter-gather table.
>> + */
>> +enum pcm_msg_sg_index {
>> +     PCM_MSG_SG_XFER = 0,
>> +     PCM_MSG_SG_DATA,
>> +     PCM_MSG_SG_STATUS,
>> +     PCM_MSG_SG_MAX
>> +};
>> +
>> +/**
>> + * struct virtio_pcm_msg - VirtIO I/O message.
>> + * @substream: VirtIO PCM substream.
>> + * @xfer: Request header payload.
>> + * @status: Response header payload.
>> + * @sgs: Payload scatter-gather table.
>> + */
>> +struct virtio_pcm_msg {
>> +     struct virtio_pcm_substream *substream;
>> +     struct virtio_snd_pcm_xfer xfer;
>> +     struct virtio_snd_pcm_status status;
>> +     struct scatterlist sgs[PCM_MSG_SG_MAX];
>> +};
>> +
>> +/**
>> + * virtsnd_pcm_msg_alloc() - Allocate I/O messages.
>> + * @substream: VirtIO PCM substream.
>> + * @nmsg: Number of messages (equal to the number of periods).
>> + * @dma_area: Pointer to used audio buffer.
>> + * @period_bytes: Period (message payload) size.
>> + *
>> + * The function slices the buffer into nmsg parts (each with the size of
>> + * period_bytes), and creates nmsg corresponding I/O messages.
>> + *
>> + * Context: Any context that permits to sleep.
>> + * Return: 0 on success, -ENOMEM on failure.
>> + */
>> +int virtsnd_pcm_msg_alloc(struct virtio_pcm_substream *substream,
>> +                       unsigned int nmsg, u8 *dma_area,
>> +                       unsigned int period_bytes)
>> +{
>> +     struct virtio_device *vdev = substream->snd->vdev;
>> +     unsigned int i;
>> +
>> +     if (substream->msgs)
>> +             devm_kfree(&vdev->dev, substream->msgs);
>> +
>> +     substream->msgs = devm_kcalloc(&vdev->dev, nmsg,
>> +                                    sizeof(*substream->msgs), GFP_KERNEL);
>> +     if (!substream->msgs)
>> +             return -ENOMEM;
>> +
>> +     for (i = 0; i < nmsg; ++i) {
>> +             struct virtio_pcm_msg *msg = &substream->msgs[i];
>> +
>> +             msg->substream = substream;
>> +
>> +             sg_init_table(msg->sgs, PCM_MSG_SG_MAX);
>> +             sg_init_one(&msg->sgs[PCM_MSG_SG_XFER], &msg->xfer,
>> +                         sizeof(msg->xfer));
>> +             sg_init_one(&msg->sgs[PCM_MSG_SG_DATA],
>> +                         dma_area + period_bytes * i, period_bytes);
>> +             sg_init_one(&msg->sgs[PCM_MSG_SG_STATUS], &msg->status,
>> +                         sizeof(msg->status));
>> +     }
>> +
>> +     return 0;
>> +}
>> +
>> +/**
>> + * virtsnd_pcm_msg_send() - Send asynchronous I/O messages.
>> + * @substream: VirtIO PCM substream.
>> + *
>> + * All messages are organized in an ordered circular list. Each time the
>> + * function is called, all currently non-enqueued messages are added to the
>> + * virtqueue. For this, the function keeps track of two values:
>> + *
>> + *   msg_last_enqueued = index of the last enqueued message,
>> + *   msg_count = # of pending messages in the virtqueue.
>> + *
>> + * Context: Any context.
>> + * Return: 0 on success, -EIO on failure.
>> + */
>> +int virtsnd_pcm_msg_send(struct virtio_pcm_substream *substream)
>> +{
>> +     struct snd_pcm_runtime *runtime = substream->substream->runtime;
>> +     struct virtio_snd *snd = substream->snd;
>> +     struct virtio_device *vdev = snd->vdev;
>> +     struct virtqueue *vqueue = virtsnd_pcm_queue(substream)->vqueue;
>> +     int i;
>> +     int n;
>> +     bool notify = false;
>> +
>> +     if (!vqueue)
>> +             return -EIO;
>> +
>> +     i = (substream->msg_last_enqueued + 1) % runtime->periods;
>> +     n = runtime->periods - atomic_read(&substream->msg_count);
>> +
>> +     for (; n; --n, i = (i + 1) % runtime->periods) {
>> +             struct virtio_pcm_msg *msg = &substream->msgs[i];
>> +             struct scatterlist *psgs[PCM_MSG_SG_MAX] = {
>> +                     [PCM_MSG_SG_XFER] = &msg->sgs[PCM_MSG_SG_XFER],
>> +                     [PCM_MSG_SG_DATA] = &msg->sgs[PCM_MSG_SG_DATA],
>> +                     [PCM_MSG_SG_STATUS] = &msg->sgs[PCM_MSG_SG_STATUS]
>> +             };
>> +             int rc;
>> +
>> +             msg->xfer.stream_id = cpu_to_virtio32(vdev, substream->sid);
>> +             memset(&msg->status, 0, sizeof(msg->status));
>> +
>> +             atomic_inc(&substream->msg_count);
>> +
>> +             if (substream->direction == SNDRV_PCM_STREAM_PLAYBACK)
>> +                     rc = virtqueue_add_sgs(vqueue, psgs, 2, 1, msg,
>> +                                            GFP_ATOMIC);
>> +             else
>> +                     rc = virtqueue_add_sgs(vqueue, psgs, 1, 2, msg,
>> +                                            GFP_ATOMIC);
>> +
>> +             if (rc) {
>> +                     atomic_dec(&substream->msg_count);
>> +                     return -EIO;
>> +             }
>> +
>> +             substream->msg_last_enqueued = i;
>> +     }
>> +
>> +     if (!(substream->features & (1U << VIRTIO_SND_PCM_F_MSG_POLLING)))
>> +             notify = virtqueue_kick_prepare(vqueue);
>> +
>> +     if (notify)
>> +             if (!virtqueue_notify(vqueue))
>> +                     return -EIO;
>> +
>> +     return 0;
>> +}
>> +
>> +/**
>> + * virtsnd_pcm_msg_complete() - Complete an I/O message.
>> + * @msg: I/O message.
>> + * @size: Number of bytes written.
>> + *
>> + * Completion of the message means the elapsed period:
>> + *   - update hardware pointer,
>> + *   - update latency value,
>> + *   - kick the upper layer.
>> + *
>> + * Context: Interrupt context.
>> + */
>> +static void virtsnd_pcm_msg_complete(struct virtio_pcm_msg *msg, size_t size)
>> +{
>> +     struct virtio_pcm_substream *substream = msg->substream;
>> +     snd_pcm_uframes_t hw_ptr;
>> +     unsigned int msg_count;
>> +
>> +     hw_ptr = (snd_pcm_uframes_t)atomic_read(&substream->hw_ptr);
>> +
>> +     /*
>> +      * If the capture substream returned an incorrect status, then just
>> +      * increase the hw_ptr by the period size.
>> +      */
>> +     if (substream->direction == SNDRV_PCM_STREAM_PLAYBACK ||
>> +         size <= sizeof(msg->status)) {
>> +             hw_ptr += substream->period_size;
>> +     } else {
>> +             size -= sizeof(msg->status);
>> +             hw_ptr += size / substream->frame_bytes;
>> +     }
>> +
>> +     atomic_set(&substream->hw_ptr, (u32)(hw_ptr % substream->buffer_size));
>> +     atomic_set(&substream->xfer_xrun, 0);
>> +
>> +     msg_count = atomic_dec_return(&substream->msg_count);
>> +
>> +     if (atomic_read(&substream->xfer_enabled)) {
>> +             struct snd_pcm_runtime *runtime = substream->substream->runtime;
>> +
>> +             runtime->delay =
>> +                     bytes_to_frames(runtime,
>> +                                     le32_to_cpu(msg->status.latency_bytes));
>> +
>> +             snd_pcm_period_elapsed(substream->substream);
>> +
>> +             virtsnd_pcm_msg_send(substream);
>> +     } else if (!msg_count) {
>> +             wake_up_all(&substream->msg_empty);
>> +     }
>> +}
>> +
>> +/**
>> + * virtsnd_pcm_notify_cb() - Process all completed I/O messages.
>> + * @vqueue: Underlying tx/rx virtqueue.
>> + *
>> + * If transmission is allowed, then each completed message is immediately placed
>> + * back at the end of the queue.
>> + *
>> + * Context: Interrupt context. Takes and releases the tx/rx queue spinlock.
>> + */
>> +static inline void virtsnd_pcm_notify_cb(struct virtio_snd_queue *queue)
>> +{
>> +     unsigned long flags;
>> +
>> +     spin_lock_irqsave(&queue->lock, flags);
>> +     while (queue->vqueue) {
>> +             virtqueue_disable_cb(queue->vqueue);
>> +
>> +             for (;;) {
>> +                     struct virtio_pcm_msg *msg;
>> +                     u32 length;
>> +
>> +                     msg = virtqueue_get_buf(queue->vqueue, &length);
>> +                     if (!msg)
>> +                             break;
>> +
>> +                     virtsnd_pcm_msg_complete(msg, length);
>> +             }
>> +
>> +             if (unlikely(virtqueue_is_broken(queue->vqueue)))
>> +                     break;
>> +
>> +             if (virtqueue_enable_cb(queue->vqueue))
>> +                     break;
>> +     }
>> +     spin_unlock_irqrestore(&queue->lock, flags);
>> +}
>> +
>> +/**
>> + * virtsnd_pcm_tx_notify_cb() - Process all completed TX messages.
>> + * @vqueue: Underlying tx virtqueue.
>> + *
>> + * Context: Interrupt context.
>> + */
>> +void virtsnd_pcm_tx_notify_cb(struct virtqueue *vqueue)
>> +{
>> +     struct virtio_snd *snd = vqueue->vdev->priv;
>> +
>> +     virtsnd_pcm_notify_cb(virtsnd_tx_queue(snd));
>> +}
>> +
>> +/**
>> + * virtsnd_pcm_rx_notify_cb() - Process all completed RX messages.
>> + * @vqueue: Underlying rx virtqueue.
>> + *
>> + * Context: Interrupt context.
>> + */
>> +void virtsnd_pcm_rx_notify_cb(struct virtqueue *vqueue)
>> +{
>> +     struct virtio_snd *snd = vqueue->vdev->priv;
>> +
>> +     virtsnd_pcm_notify_cb(virtsnd_rx_queue(snd));
>> +}
>> +
>> +/**
>> + * virtsnd_pcm_ctl_msg_alloc() - Allocate and initialize the PCM device control
>> + *                               message for the specified substream.
>> + * @substream: VirtIO PCM substream.
>> + * @command: Control request code (VIRTIO_SND_R_PCM_XXX).
>> + * @gfp: Kernel flags for memory allocation.
>> + *
>> + * Context: Any context. May sleep if @gfp flags permit.
>> + * Return: Allocated message on success, ERR_PTR(-errno) on failure.
>> + */
>> +struct virtio_snd_msg *
>> +virtsnd_pcm_ctl_msg_alloc(struct virtio_pcm_substream *substream,
>> +                       unsigned int command, gfp_t gfp)
>> +{
>> +     struct virtio_device *vdev = substream->snd->vdev;
>> +     size_t request_size = sizeof(struct virtio_snd_pcm_hdr);
>> +     size_t response_size = sizeof(struct virtio_snd_hdr);
>> +     struct virtio_snd_msg *msg;
>> +
>> +     switch (command) {
>> +     case VIRTIO_SND_R_PCM_SET_PARAMS: {
>> +             request_size = sizeof(struct virtio_snd_pcm_set_params);
>> +             break;
>> +     }
>> +     }
>> +
>> +     msg = virtsnd_ctl_msg_alloc(vdev, request_size, response_size, gfp);
>> +     if (!IS_ERR(msg)) {
>> +             struct virtio_snd_pcm_hdr *hdr = sg_virt(&msg->sg_request);
>> +
>> +             hdr->hdr.code = cpu_to_virtio32(vdev, command);
>> +             hdr->stream_id = cpu_to_virtio32(vdev, substream->sid);
>> +     }
>> +
>> +     return msg;
>> +}
>> --
>> 2.30.0
>>
>>
> 
>
Anton Yakovlev Jan. 24, 2021, 4:53 p.m. UTC | #3
Hi, Liam!


On 20.01.2021 11:10, Girdwood, Liam R wrote:
> CAUTION: This email originated from outside of the organization.
> Do not click links or open attachments unless you recognize the sender and know the content is safe.
> 
> 
> Hi Anton,
> 
> On Wed, 2021-01-20 at 01:36 +0100, Anton Yakovlev wrote:
>> This series implements a driver part of the virtio sound device
>> specification v8 [1].
>>
>> The driver supports PCM playback and capture substreams, jack and
>> channel map controls. A message-based transport is used to write/read
>> PCM frames to/from a device.
>>
>> The series is based (and was actually tested) on Linus's master
>> branch [2], on top of
>>
>> commit 1e2a199f6ccd ("Merge tag 'spi-fix-v5.11-rc4' of ...")
>>
>> As a device part was used OpenSynergy proprietary implementation.
>>
>> Any comments are very welcome.
>>
> 
> This just looks like the guest front end here, do you have a follow up
> series for the host backend ?

As I mentioned in the cover message, as a device part was used our own
proprietary implementation. And there are no plans to upstream that
part.


> Thanks
> 
> Liam
> ---------------------------------------------------------------------
> Intel Corporation (UK) Limited
> Registered No. 1134945 (England)
> Registered Office: Pipers Way, Swindon SN3 1RJ
> VAT No: 860 2173 47
> 
> This e-mail and any attachments may contain confidential material for
> the sole use of the intended recipient(s). Any review or distribution
> by others is strictly prohibited. If you are not the intended
> recipient, please contact the sender and delete all copies.
>