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: [PATCH v3] virtio-snd: add virtio sound device specification


On 18.12.2019 12:52, Gerd Hoffmann wrote:
+/* supported PCM stream features */
+enum {
+    VIRTIO_SND_PCM_F_HOST_MEM = 0,
+    VIRTIO_SND_PCM_F_GUEST_MEM,

Is this useful as stream property?  I would expect when supported by a
device it would work for all streams.

Since we allowed different capabilities for different streams, now it's
possible to have different backends for them. Iâm not sure how much this can
be a real scenario, but it is theoretically possible to have one backend that
is able to work with shared memory, and the other is not. The same can be said
for other stream feature bits.

Hmm, ok.  Point.

Independant from that they should either be a note that these two are
nor (yet) specified and are just placeholders.  Or simply be deleted and
re-added with the actual specification for guestmem + hostmem.

I think having a note is better.


+    VIRTIO_SND_PCM_F_EVT_PERIOD,

I think this is only useful for hostmem/guestmem.

Using message-based transport, an application can send the entire playback
buffer at the beginning, and then send period-sized parts on each period
notification.

The driver can still split the data from the application into a set of
smaller virtio buffers.  And this is how I would write a driver.  Create
a bunch of buffers, period_bytes each, enough to cover buffer_bytes.
Then go submit them, re-use them robin-round (each time the device
completes a buffer re-fill and re-submit it), without a special case
with one big buffer for the (playback) stream start.

Yes, but what about device side? Most likely, it will copy the contents of the
buffer to another location and immediately complete the request (otherwise it
is not clear when to complete). In addition, the driver must advance the
hardware position, and completing the request is the only possible place to do
this. In most cases, all this will not coincide with the period notification.
This means that the notification itself is an optional feature that may or may
not be enabled for message-based transport.

Of course, the benefits of such notifications largely depend on the
implementation of the driver. But at least people here claimed that they could
be useful.


I still think this is not needed for message-based transport.

Maybe this is not needed for hostmem/guestmem either.  The point of
using shared memory is to avoid system calls and vmexits for buffer
management, so VIRTIO_SND_PCM_F_EVT_PERIOD doesn't look that useful
here too.  But maybe it is nice to have that as option.

Earlier there was a big discussion on this topic, which resulted in having
this feature. The main point was it reflects the state of the hardware device,
not the virtual device itself. But they may not always be useful or necessary,
so this feature is optional.


+\subsubsection{PCM I/O Messages}\label{sec:Device Types / Sound Device / Device Operation / PCM IO Messages}
+
+An I/O message consists of the header part, followed by the buffer, and then
+the status part.

Each buffer MUST (or SHOULD?) be period_bytes in size.

The driver SHOULD queue (buffer_bytes / period_bytes) virtio buffers, so
the device has buffer_bytes total buffer space available.

I think, it's better not to set queued buffer size restrictions.

Lets make it SHOULD then.

Again, if periods are not used, then there's no period_bytes. And they are not
used, for example, in most low-latency applications.

(Well, technically speaking, low-latency applications in Linux must configure
period size for ALSA. But then they usually suppress period notifications and
send very small chunks of data, which usually do not correspond to the size of
the period at all.)

Periods are an optional feature. If the driver is not interested in
period notifications, it's unclear what period_bytes means here.

Well, the driver has to manage buffers, so I can't see how a driver is
not interested in completion notifications.  If the driver isn't
interested in keeping the latency low by using lots of small buffers it
still needs to queue at least two (larger) buffers, so the device still
has data to continue playback when it completed a buffer.  And the
driver needs to know when the device is done with a buffer so it can
either recycle the buffer or free the pages.

It seems that you have mixed up two different things: sending/completing a
request containing a buffer, and period notifications. They are not the same.


--
Anton Yakovlev
Senior Software Engineer

OpenSynergy GmbH
Rotherstr. 20, 10245 Berlin

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



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