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


Hi,

On Mon, Nov 11, 2019 at 03:20:55PM +0000, Liam Girdwood wrote:
> > > > +struct virtio_snd_ctl_msg {
> > > > +    /* device-read-only data */
> > > > +    le32 request_code;
> > > > +    u8 request_payload[];
> > > 
> > > How do I know how much data to read here ? Is size embedded in
> > > request_code ?
> > 
> > Yes, you are right. Actual request/response length can be easily
> > derived from 
> > a request_code.
> > 
> 
> Nak, please use a separate size and type code otherwise we will have
> real problems in the future when we come to add new features (with new
> codes or bigger sizes).

Other virtio devices rely on the buffer length already provided by virtio
for this. For example see the virtio-net control virtqueue
(virtio_net_ctrl*), which is extended with new commands from time to time.
Most recently, the "receive-side scaling" feature introduces a large
structure as payload to virtio_net_ctrl.

[...]
> > > > +\begin{lstlisting}
> > > > +struct virtio_snd_pcm_xfer {
> > > > +    le32 stream;
> > > > +    u8 data[];
> > > > +    le32 status;
> > > > +    le32 actual_length;
> > > 
> > > Not following this, is actual_length the size of data[]. If so, it
> > > must
> > > preceed data[].
> > 
> > No, the actual_length field is supposed to be used by device side to
> > report
> > actual amount of bytes read from/written to a buffer.

By the way the written size is already provided by virtio (field len in
the used descriptor), but not the read size.

> > In real world
> > scenario, 
> > if an I/O request contains N bytes, a device can play/capture *up to*
> > N bytes. 
> > Thus, it's required to report this length back to a driver.
> > 
> 
> This is not how PCM audio buffering works. I don't copy extra padding
> if I dont need to, I only copy when my buffer is full. See the need for
> period size in earlier comments.
[...]
> > > 2) Zero copy of audio data and stream positions. Essential for any
> > > low
> > > latency audio use cases (if supported by HW, hypervisor, VM
> > > framework).
> > 
> > What do you mean by zero-copy? Shared memory between device and
> > driver sides?
> 
> Yes, but I see this is now a separate thread as it used by others too.

The virtio transport already permits zero-copy, because one buffer can be
described with a chain of descriptors, each pointing to a different area
in memory. The driver can split the virtio_snd_pcm_xfer into header and
data:

    virtqueue
    desc ring
    |       |
    +-------+
    |       |---------> header (copied)
    +- - - -+
    |       |---------> PCM data (mapped)
    +-------+
    |       |

So it might be a good idea to put all the metadata (stream, status,
actual_length) at the beginning of the virtio_snd_pcm_xfer struct,
otherwise the driver will need three descriptors instead of two to achieve
this.

Thanks,
Jean


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