OASIS Mailing List ArchivesView the OASIS mailing list archive below
or browse/search using MarkMail.


Help: OASIS Mailing Lists Help | MarkMail Help

virtio-dev message

[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]

Subject: Re: [virtio-dev] [PATCH] snd: Add virtio sound device specification

On 22.11.2019 08:19, Gerd Hoffmann wrote:
Feature bits


and new ones would be

VIRTIO_SND_F_PCM_EVT_PERIOD - support elapsed period notifications
VIRTIO_SND_F_PCM_EVT_XRUN - support underrun/overrun notifications
VIRTIO_SND_F_PCM_MSG - support message-based data transport
VIRTIO_SND_F_PCM_HOST_MEM - support sharing host memory with guest
VIRTIO_SND_F_PCM_GUEST_MEM - support sharing guest memory with host

Not sure about correct naming here.

Hmm, I think at least parts of this should be device configuration not
feature bits ...

Instead of PCM_OUTPUT and PCM_INPUT I would put the number of input and
output streams into the device config space (and explicitly allow count
being zero).

There are several options for the number of streams:

1. The device can provide one input and/or output stream.
2. The device can provide several input and/or output streams, and all of them
share the same capabilities.
3. The device can provide several input and/or output streams, and they can
have different set of capabilities.

Overall design depends on chosen option. Current draft is based on p.1. But we
have never discussed what is the best solution here.

PCM_EVT_* probably makes sense to be a per-stream configuration bit.

Yes, it definitely makes sense.

PCM_MSG -- I would drop the feature bit and make that mandatory, so we
have a common baseline on which all drivers and devices can agree on.

Then we need to inform device what "transport" will be in use (I assumed it
would be feature negotiation).

By the way, Liam mentioned something about using PCI BARs for mapping host
memory. Does it mean, that some features will be transport-specific?

In the latest spec revision virtio got support for shared memory
regions.  This should be used for VIRTIO_SND_F_PCM_HOST_MEM.

What about virtio mmio?

Right now this is defined for the pci transport only.

And now we only declare a feature bit, right?


As Gerd explained, we need 4 virtqueues in total:

1. control queue (guest->host: commands)
2. events queue (host->guest: notifications)
3. tx queue (guest->host: playback)
4. rx queue (host->guest: capture)

The first one always exists, but all other depend on feature bits. Should we
check and assign these in some pre-defined order?

I'd suggest to make them all mandatory.

Yes, it would be the simplest solution.

Configuration space

We talked about PCM formats defined in ALSA. In Linux kernel v5.4-rc8 we have

Do we really need all of them? And I skipped endianess, but should we care
about it as well?

I'd keep the list as short as possible.  The formats listed in the patch
looked like a good start.

It was proposed to have all non-compressed formats.

And... do we care about cross-endian cases?

It also affects how large configuration space should be.

I'd make the format bitfield 64bit no matter what.  It doesn't cost much
and it leaves room to extend things without hassle if needed.

Good, then it will be le64.

Common control message structure will be explicitly splitted into two

struct virtio_snd_ctl_req {
     le32 type;
       le32 stream_id;
     u8 payload[];

and the SET_FORMAT request will be replaced with the SET_PARAMS

struct virtio_snd_pcm_set_params {
     struct virtio_snd_pcm_hdr hdr;
     le32 channels;
     le32 format;
     le32 rate;
     le32 period_size;

We only need to decide units for the period_size field (frames or bytes).

I'd suggest bytes (and rename the field to period_bytes).


PCM I/O requests

As we get rid of multiplexing, we don't need a header anymore.

What if a device wants support two playback streams (possibly even with
different features, say one stereo and one 5:1)?

We need to make a design choice (see comment above).

And a message
will have only status

struct virtio_snd_pcm_status {
     le32 status;
     le32 actual_length;

Hmm.  Why actual_length?  Do we want allow short transfers?  Why?
Wouldn't it be more useful to just fill the buffer completely?

In current design we have no buffer size requirements. It means, that in
theory the device and the driver could have buffers with different sizes.

Also, the capture stream is a special case. Now we don't state explicitly
whether read request is blockable or not. And if the device complete it immediately, the request could contain less frames than was expected by the


Anton Yakovlev
Senior Software Engineer

OpenSynergy GmbH
Rotherstr. 20, 10245 Berlin

Phone: +49 30 60 98 54 0
E-Mail: anton.yakovlev@opensynergy.com


Handelsregister/Commercial Registry: Amtsgericht Charlottenburg, HRB 108616B
Geschäftsführer/Managing Director: Regis Adjamah

[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]