[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]
Subject: Re: [virtio-dev] [RFC] snd: Add virtio sound device specification
On Fri, Aug 16, 2019 at 10:26:28PM +0200, Mikhail Golubev wrote:
> From: Anton Yakovlev <Anton.Yakovlev@opensynergy.com>
>
> The virtio sound device is an extendable virtual audio device. It
> provides playback and capture PCM streams to a guest.
>
> The device is implemented as a collection of a device-specific functions
> that provide dedicated audio-related functionality. At the moment it
> supports the following:
>
> 1. Base function (generic device management);
> 2. PCM function (a playback/capture PCM stream).
>
> Signed-off-by: Anton Yakovlev <Anton.Yakovlev@opensynergy.com>
> Signed-off-by: Mikhail Golubev <Mikhail.Golubev@opensynergy.com>
> ---
> content.tex | 1 +
> virtio-sound.tex | 432 +++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 433 insertions(+)
> create mode 100644 virtio-sound.tex
>
> diff --git a/content.tex b/content.tex
> index ee0d7c9..6a7f9a3 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -5677,6 +5677,7 @@ \subsubsection{Legacy Interface: Framing Requirements}\label{sec:Device
> \input{virtio-input.tex}
> \input{virtio-crypto.tex}
> \input{virtio-vsock.tex}
> +\input{virtio-sound.tex}
>
> \chapter{Reserved Feature Bits}\label{sec:Reserved Feature Bits}
>
> diff --git a/virtio-sound.tex b/virtio-sound.tex
> new file mode 100644
> index 0000000..4039d64
> --- /dev/null
> +++ b/virtio-sound.tex
> @@ -0,0 +1,432 @@
> +\section{Sound Device}\label{sec:Device Types / Sound Device}
> +
> +The virtio sound card is an extendable virtual audio device. It provides its
> +functionality in a form of functions that can be configured and managed in an
> +independent way.
> +
> +\subsection{Device ID}\label{sec:Device Types / Sound Device / Device ID}
> +
> +25
> +
> +\subsection{Virtqueues}\label{sec:Device Types / Sound Device / Virtqueues}
> +
> +Depending on provided functionality, a device can use one or more virtqueues
> +described in a configuration space. A virtqueue assigned to the base function
> +is mandatory and referred as a control queue.
Pls look at other devices such as e.g. serial for examples of how
other devices list virtqueues.
> +
> +\subsection{Feature bits}\label{sec:Device Types / Sound Device / Feature bits}
> +
> +There are currently no feature bits defined for this device.
I see at least PCM as an optional feature.
Pls make that a feature bit so we can have
reasonable forward and backward compatibility.
> +
> +\subsection{Device configuration layout}\label{sec:Device Types / Sound Device / Device configuration layout}
> +
> +Configuration space provides information about available virtqueues and their
> +assignments to enabled functions using following layout structure and definitions:
> +
> +\begin{lstlisting}
> +enum {
> + /* function identifiers for virtqueue assignments */
> + VIRTIO_SND_FN_BASE = 0,
> + VIRTIO_SND_FN_PCM
Please document that PCM is 1, and add comments.
> +};
So I think we should have PCM as a feature bit.
> +
> +struct virtio_snd_queue_info {
> + /* VIRTIO_SND_FN_* */
> + le16 function;
> + /* a function device id */
> + u8 device_id;
> + /* a function subdevice id */
> + u8 subdevice_id;
Are all fields read-only?
For each config field you need to explain where it's read only, write only,
read/write.
> +};
> +
> +struct virtio_snd_config {
> + /* maximum # of available virtqueues */
> + le32 nqueues;
> +};
> +\end{lstlisting}
> +
> +The \field{nqueues} field contains a maximum amount of available virtqueues and
> +is followed by an \field{nqueues}-sized array of \field{struct virtio_snd_queue_info}
> +structures.
This will require lots of config space. Do we need fast access to this?
If not we could add a queue selector, or even a config virtqueue.
But I really think there should just be a fixed queue for each
function.
> \field{struct virtio_snd_queue_info} with index \field{i} describes
> +a device function assigned to a virtqueue with index \field{i}.
i is not a field that appears in the above text.
> \field{device_id}
> +and \field{subdevice_id} fields are used to distinguish different instances of
> +the same function type.
How are they used? I see no explanation anywhere.
> +
> +\subsection{Device Initialization}\label{sec:Device Types / Sound Device / Device Initialization}
> +
> +\begin{enumerate}
> +\item Initialize all available virtqueues.
> +\item Retrieve a device configuration.
> +\item Initialize all functions enabled in the device configuration.
> +\end{enumerate}
> +
> +A device configuration is represented with a set of descriptors having
> +a fixed header using the following layout structure and definitions:
Do you mean this appears in config space? Or sent on some queue?
Also please avoid using the term "descriptor" since it's already
used for ring operation descriptions.
I also wonder why is all this in the configuration space.
> +
> +\begin{lstlisting}
> +enum {
> + /* the PCM function descriptor types */
> + VIRTIO_SND_DESC_PCM = 0,
> + VIRTIO_SND_DESC_PCM_STREAM
> +};
> +
> +struct virtio_snd_desc {
> + u8 length;
> + u8 type;
> +
> + u16 padding;
> +};
> +\end{lstlisting}
> +
> +The fixed header \field{struct virtio_snd_desc} in each
> +descriptor includes the following fields:
> +
> +\begin{description}
> +\item[\field{length}] specifies the total number of bytes in the descriptor.
why do we need the length? buffer length not enough?
> +\item[\field{type}] identifies the descriptor type (VIRTIO_SND_DESC_*).
> +\end{description}
> +
> +With the exception of the base function, all implemented functions MUST add
> +their descriptors in configuration only if a particular function (or part of
> +its functionality) is enabled in the device.
This kind of ad-hoc feature negotiation goes against the
idea of using virtio for unifying device operation.
Please add each optional feature with a feature bit,
and dedicate a vq to it. This way you will have
fixed 2 vqs right now (I think?) and no need for
the complex discovery mechanism.
> +
> +\subsection{Device Operation}\label{sec:Device Types / Sound Device / Device Operation}
> +
> +All control messages are placed into a single control queue.
> +
> +All requests and responses on the queue consist of or are preceded by
> +a header using the following layout structure and definitions:
> +
> +\begin{lstlisting}
> +enum {
> + /* the base function request types */
> + VIRTIO_SND_R_BASE_GET_CFG = 0,
> +
> + /* the PCM function request types */
> + VIRTIO_SND_R_PCM_SET_FORMAT = 0x0100,
> + VIRTIO_SND_R_PCM_PREPARE,
> + VIRTIO_SND_R_PCM_START,
> + VIRTIO_SND_R_PCM_STOP,
> + VIRTIO_SND_R_PCM_PAUSE,
> + VIRTIO_SND_R_PCM_UNPAUSE,
> + VIRTIO_SND_R_PCM_QUERY_CHMAP,
> + VIRTIO_SND_R_PCM_USE_CHMAP,
Where are these documented?
> +
> + /* response status codes */
> + VIRTIO_SND_S_OK = 0x8000,
> + VIRTIO_SND_S_ENOTSUPPORTED,
> + VIRTIO_SND_S_EINVALID,
> + VIRTIO_SND_S_EIO
where are these used?
> +};
> +
> +struct virtio_snd_hdr {
> + le32 code;
> +};
> +\end{lstlisting}
> +
> +The generic header \field{struct virtio_snd_hdr} contains the only field:
> +
> +\begin{description}
> +\item[\field{code}] specifies a type of the driver request
> +(VIRTIO_SND_R_*) or a device response status code (VIRTIO_SND_S_*).
> +\end{description}
> +
> +\subsubsection{Device Operation: Base function}
> +
> +The base function is responsible for operations having a scope of or that
> +can affect the entire device.
> +
> +\begin{description}
> +
> +\item[VIRTIO_SND_R_BASE_GET_CFG]
> +Retrieve the current device configuration, response is \field{struct virtio_snd_base_configuration}
> +containing a set of function descriptors.
> +
> +\begin{lstlisting}
> +/* a maximum possible configuration data size (in bytes) */
> +#define VIRTIO_SND_BASE_CFG_MAX_SIZE 1024
> +
> +/* a response containing device configuration */
> +struct virtio_snd_base_configuration {
> + struct virtio_snd_hdr hdr;
> + /* size in bytes of configuration data */
> + le32 length;
> + /* configuration data */
> + u8 data[VIRTIO_SND_BASE_CFG_MAX_SIZE];
> +};
> +\end{lstlisting}
> +
> +\end{description}
> +
> +\subsubsection{Device Operation: PCM function}
> +
> +The PCM function provides up to one playback and up to one capture PCM stream.
> +If the device supports more than one PCM stream of the same type, it MUST
> +provide them as separate PCM functions.
Please put all conformance statements (including MAY/MUST) in the normative sections.
> +
> +The function puts a PCM function descriptor to a device configuration.
what does this mean? "put to" is not in my English dictionary. Typo?
> The descriptor
> +is followed by up to two PCM stream descriptors. The data is stored according to
> +the following layout structure and definitions:
You don't document which parts of the buffer are read and which write
(VIRTQ_DESC_F_WRITE).
> +
> +\begin{lstlisting}
> +/* supported PCM stream types */
> +enum {
> + VIRTIO_SND_PCM_T_PLAYBACK = 0,
> + VIRTIO_SND_PCM_T_CAPTURE
> +};
> +
> +/* supported PCM sample formats */
> +enum {
> + VIRTIO_SND_PCM_FMT_S8 = 0,
> + VIRTIO_SND_PCM_FMT_U8,
> + VIRTIO_SND_PCM_FMT_S16,
> + VIRTIO_SND_PCM_FMT_U16,
> + VIRTIO_SND_PCM_FMT_S32,
> + VIRTIO_SND_PCM_FMT_U32,
> + VIRTIO_SND_PCM_FMT_FLOAT,
> + VIRTIO_SND_PCM_FMT_FLOAT64
> +};
> +
> +/* supported PCM frame rates */
> +enum {
> + VIRTIO_SND_PCM_RATE_8000 = 0,
> + VIRTIO_SND_PCM_RATE_11025,
> + VIRTIO_SND_PCM_RATE_16000,
> + VIRTIO_SND_PCM_RATE_22050,
> + VIRTIO_SND_PCM_RATE_32000,
> + VIRTIO_SND_PCM_RATE_44100,
> + VIRTIO_SND_PCM_RATE_48000,
> + VIRTIO_SND_PCM_RATE_64000,
> + VIRTIO_SND_PCM_RATE_88200,
> + VIRTIO_SND_PCM_RATE_96000,
> + VIRTIO_SND_PCM_RATE_176400,
> + VIRTIO_SND_PCM_RATE_192000
> +};
> +
> +/* PCM function descriptor */
> +struct virtio_snd_pcm_desc {
> + /* sizeof(struct virtio_snd_pcm_desc) */
> + u8 length;
So if it's a fixed value, why do we need it? Same in below types.
> + /* VIRTIO_SND_DESC_PCM */
> + u8 type;
> + /* a PCM function ID (assigned by the device) */
> + u8 pcm_id;
> + /* # of PCM stream descriptors in the configuration (one per supported
> + * PCM stream type)
> + */
> + u8 nstreams;
> +};
> +
> +/* PCM stream descriptor */
> +struct virtio_snd_pcm_stream_desc {
> + /* sizeof(struct virtio_snd_pcm_stream_desc) */
> + u8 length;
> + /* VIRTIO_SND_DESC_PCM_STREAM */
> + u8 type;
> + /* a PCM stream type (VIRTIO_SND_PCM_T_*) */
> + u8 stream_type;
> + /* # of supported channel maps */
> + u8 nchmaps;
> + /* minimum # of supported channels */
> + le16 channels_min;
> + /* maximum # of supported channels */
> + le16 channels_max;
> + /* supported sample format bit mask (1 << VIRTIO_SND_PCM_FMT_*) */
> + le32 formats;
> + /* supported frame rate bit mask (1 << VIRTIO_SND_PCM_RATE_*) */
> + le32 rates;
> +};
> +\end{lstlisting}
> +
> +The function assumes the following command lifecycle:
> +
> +\begin{enumerate}
> +\item Set format
> +\item Prepare
> +\item Playback: transfer data for prebuffing.
> +\item Start
> +\item Transfer data to/from the PCM device.
> +\begin{enumerate}
> + \item Pause
> + \item Unpause
> +\end{enumerate}
> +\item Stop
> +\end{enumerate}
> +
> +PCM control requests have or consist of a fixed header with the following
> +layout structure:
> +
> +\begin{lstlisting}
> +struct virtio_snd_pcm_hdr {
> + /* a PCM request type (VIRTIO_SND_R_PCM_*) */
> + struct virtio_snd_hdr hdr;
> + /* a PCM identifier (assigned in configuration) */
> + u8 pcm_id;
> + /* a PCM stream type (VIRTIO_SND_PCM_T_*) */
> + u8 stream_type;
> +
> + u16 padding;
> +};
> +\end{lstlisting}
> +
> +\begin{description}
> +
> +\item[VIRTIO_SND_R_PCM_SET_FORMAT]
> +Set selected PCM format.
> +
> +\begin{lstlisting}
> +struct virtio_snd_pcm_set_format {
> + struct virtio_snd_pcm_hdr hdr;
> + /* # of channels */
> + le16 channels;
> + /* a PCM sample format (VIRTIO_SND_PCM_FMT_*) */
> + le16 format;
> + /* a PCM frame rate (VIRTIO_SND_PCM_RATE_*) */
> + le16 rate;
> +
> + u16 padding;
> +};
> +\end{lstlisting}
> +
> +\item[VIRTIO_SND_R_PCM_PREPARE]
> +Prepare the PCM device.
> +
> +\item[VIRTIO_SND_R_PCM_START]
> +Start the PCM device.
> +
> +\item[VIRTIO_SND_R_PCM_STOP]
> +Stop the PCM device.
> +
> +\item[VIRTIO_SND_R_PCM_PAUSE]
> +Set the PCM device on pause.
> +
> +\item[VIRTIO_SND_R_PCM_UNPAUSE]
> +Unset the PCM device from pause.
> +
> +\item[VIRTIO_SND_R_PCM_QUERY_CHMAP]
> +Query PCM channel map information.
> +
> +The function reports an amount of available channel maps in the PCM stream
> +configuration descriptor. The driver can enumerate these using the following
> +request and response layout structure and definitions:
> +
> +\begin{lstlisting}
> +enum {
> + /* All channels have fixed channel positions */
> + VIRTIO_SND_PCM_CHMAP_FIXED = 0,
> + /* All channels are swappable (e.g. {FL/FR/RL/RR} -> {RR/RL/FR/FL}) */
> + VIRTIO_SND_PCM_CHMAP_VARIABLE,
> + /* Only pair-wise channels are swappable (e.g. {FL/FR/RL/RR} -> {RL/RR/FL/FR}) */
> + VIRTIO_SND_PCM_CHMAP_PAIRED
> +};
> +
> +/* Standard channel position definitions */
> +enum {
> + VIRTIO_SND_PCM_CH_NONE = 0, /* undefined */
> + VIRTIO_SND_PCM_CH_NA, /* silent */
> + VIRTIO_SND_PCM_CH_MONO, /* mono stream */
> + VIRTIO_SND_PCM_CH_FL, /* front left */
> + VIRTIO_SND_PCM_CH_FR, /* front right */
> + VIRTIO_SND_PCM_CH_RL, /* rear left */
> + VIRTIO_SND_PCM_CH_RR, /* rear right */
> + VIRTIO_SND_PCM_CH_FC, /* front center */
> + VIRTIO_SND_PCM_CH_LFE, /* low frequency (LFE) */
> + VIRTIO_SND_PCM_CH_SL, /* side left */
> + VIRTIO_SND_PCM_CH_SR, /* side right */
> + VIRTIO_SND_PCM_CH_RC, /* rear center */
> + VIRTIO_SND_PCM_CH_FLC, /* front left center */
> + VIRTIO_SND_PCM_CH_FRC, /* front right center */
> + VIRTIO_SND_PCM_CH_RLC, /* rear left center */
> + VIRTIO_SND_PCM_CH_RRC, /* rear right center */
> + VIRTIO_SND_PCM_CH_FLW, /* front left wide */
> + VIRTIO_SND_PCM_CH_FRW, /* front right wide */
> + VIRTIO_SND_PCM_CH_FLH, /* front left high */
> + VIRTIO_SND_PCM_CH_FCH, /* front center high */
> + VIRTIO_SND_PCM_CH_FRH, /* front right high */
> + VIRTIO_SND_PCM_CH_TC, /* top center */
> + VIRTIO_SND_PCM_CH_TFL, /* top front left */
> + VIRTIO_SND_PCM_CH_TFR, /* top front right */
> + VIRTIO_SND_PCM_CH_TFC, /* top front center */
> + VIRTIO_SND_PCM_CH_TRL, /* top rear left */
> + VIRTIO_SND_PCM_CH_TRR, /* top rear right */
> + VIRTIO_SND_PCM_CH_TRC, /* top rear center */
> + VIRTIO_SND_PCM_CH_TFLC, /* top front left center */
> + VIRTIO_SND_PCM_CH_TFRC, /* top front right center */
> + VIRTIO_SND_PCM_CH_TSL, /* top side left */
> + VIRTIO_SND_PCM_CH_TSR, /* top side right */
> + VIRTIO_SND_PCM_CH_LLFE, /* left LFE */
> + VIRTIO_SND_PCM_CH_RLFE, /* right LFE */
> + VIRTIO_SND_PCM_CH_BC, /* bottom center */
> + VIRTIO_SND_PCM_CH_BLC, /* bottom left center */
> + VIRTIO_SND_PCM_CH_BRC /* bottom right center */
> +};
> +
> +#define VIRTIO_SND_PCM_CH_MAX 256
> +
> +struct virtio_snd_pcm_query_chmap {
> + struct virtio_snd_hdr hdr;
> + /* a PCM identifier (assigned in configuration) */
> + u8 pcm_id;
> + /* a PCM stream type (VIRTIO_SND_PCM_T_*) */
> + u8 stream_type;
> + /* a PCM channel map identifier [0 .. virtio_snd_pcm_stream_desc::nchmaps - 1] */
> + u8 chmap_id;
> +
> + u8 padding;
> +};
> +
> +/* a response containing PCM channel map information */
> +struct virtio_snd_pcm_chmap_info {
> + /* a response status code (VIRTIO_SND_S_*) */
> + struct virtio_snd_hdr hdr;
> + /* unused */
> + u8 pcm_id;
> + /* a PCM channel map type (VIRTIO_SND_PCM_CHMAP_*) */
> + u8 type;
> + /* # of valid entries in the positions array */
> + le16 nchannels;
> + /* PCM channel positions */
> + u8 positions[VIRTIO_SND_PCM_CH_MAX];
> +};
> +\end{lstlisting}
> +
> +\item[VIRTIO_SND_R_PCM_USE_CHMAP]
> +Use modified PCM channel map.
> +
> +In case of a channel map has swappable positions, the driver can set a modified
> +channel map using the same \field{struct virtio_snd_pcm_chmap_info} structure as
> +request.
> +
> +\begin{lstlisting}
> +/* a request containing PCM channel map information */
> +struct virtio_snd_pcm_chmap_info {
> + /* VIRTIO_SND_R_PCM_USE_CHMAP */
> + struct virtio_snd_hdr hdr;
> + /* a PCM identifier (assigned in configuration) */
> + u8 pcm_id;
> + /* a PCM stream type (VIRTIO_SND_PCM_T_*) */
> + u8 type;
> + /* # of valid entries in the positions array */
> + le16 nchannels;
> + /* PCM channel positions */
> + u8 positions[VIRTIO_SND_PCM_CH_MAX];
> +};
> +\end{lstlisting}
> +
> +\end{description}
> +
> +All IO requests are placed into a single virtqueue assigned for PCM data
> +transport. Each request is of form:
> +
> +\begin{lstlisting}
> +struct virtio_snd_pcm_xfer {
> + /* a PCM stream type (VIRTIO_SND_PCM_T_*) */
> + u8 stream_type;
> + /* a PCM frame buffer */
> + u8 data[];
> + /* VIRTIO_SND_S_* */
> + le32 status;
> +};
> +\end{lstlisting}
> +
> +The device returns an actual amount of bytes read from/written to data buffer.
So are there two buffers? read and write?
> --
> 2.22.1
>
>
> Please mind our privacy notice<https://www.opensynergy.com/datenschutzerklaerung/privacy-notice-for-business-partners-pursuant-to-article-13-of-the-general-data-protection-regulation-gdpr/> pursuant to Art. 13 GDPR. // Unsere Hinweise zum Datenschutz gem. Art. 13 DSGVO finden Sie hier.<https://www.opensynergy.com/de/datenschutzerklaerung/datenschutzhinweise-fuer-geschaeftspartner-gem-art-13-dsgvo/>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
> For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]