[edk2-devel] VirtIO Sound Driver (GSoC 2021)

Fri Apr 16 04:29:10 UTC 2021

Thanks, Andrew, that explains a lot. I've gotten the EDK2 build
process working successfully (which makes me really happy) so I can,
in theory, begin development on the protocol and driver at any time,
though I'm not sure if starting development now would be a good idea.
As noted in the EDK2 driver development guide I'm working on the
latest validated branch (edk2-stable202102). I guess the base
requirements for an audio protocol that we can all agree on are these:
- The audio protocol should allow for muting and volume control.
- The protocol should not be overly complicated to implement,
especially if it is to be standardized in future. (I think we can all
agree that it also shouldn't be hard to use, either.)
- The protocol should be multiple protocols, one for each audio device
supported. (I just use the singular to refer to the entire set of
them.).
Looking at the idea of my simple audio protocol that I outlined
previously, I don't think that's necessarily going to be possible or
easy -- not in the context of providing a single simple audio protocol
that manages the underlying device(s). In particular, if that's
something that would be desirable, we'd need to develop the
device-specific protocols first and then the simple audio protocol,
which would consume all of the device-specific ones and produce the
simple one. Since I'll only be working on VirtIO to start with,
though, that shouldn't be too complex, I don't think. I'd need to add
volume control and muting to my simple audio protocol idea though --
which is easy to do (apparently VirtIO makes this really easy; all you
need to do is set the channel mapping to VIRTIO_SND_PCM_CH_NA).
But backing up a way and focusing solely on VirtIO, here's what I'll
definitely have:
- A Start method that begins playback/recording (though I'll just
implement playback for now), and a Stop() method to stop a stream.
- A GetVolume() and SetVolume() function for volume control.
- A Mute()/Unmute() for muting and unmuting the stream.
- (Maybe?) A LoadBuffer()/UnloadBuffer() function for loading samples
into the appropriate queues (unless we want Start() to handle that).
The rest of the VirtIO device functionality can probably be ignored,
e.g.: jack remapping and such.
The only problem I see is the period_bytes parameter. In order for a
stream to be set up we have to send the VIRTIO_SND_R_PCM_SET_PARAMS
request, and period_bytes has to be a divisor of buffer_bytes (that
is, period_bytes % buffer_bytes == 0). If it doesn't violate GSoC
program rules and you guys are okay with it, I might develop a
preliminary protocol and a driver that uses it in order to figure out
exactly what the specification means in certain paragraphs, e.g.: in
sec. 5.14.6.6 where it seems to conflict with itself and says that all
buffers are of period_bytes bytes, but earlier in sec. 5.14.6.4.3 it
says that buffer_bytes is the size of the hardware buffer used by the
driver. I'm also going to see if I can glean anything from the Linux
kernel source code, as the kernel has a VirtIO sound driver that was
recently implemented. (I could also just write a driver for my hobby
OS kernel as well, but my kernel has no VirtIO support at the moment,
so I'd be implementing it from scratch.) If I can't start now and
develop when I have time, I'll just learn what I can from the Linux
sources.

On 4/15/21, Andrew Fish via groups.io <afish=apple.com at groups.io> wrote:
>
>
>> On Apr 15, 2021, at 6:03 PM, Michael Brown <mcb30 at ipxe.org> wrote:
>>
>> On 16/04/2021 01:59, Ethin Probst wrote:
>>> Also, I'm a bit confused. I've looked at several VirtIO devices now
>>> and have seen things like this:
>>> #define VIRTIO_PCI_DEVICE_SIGNATURE   SIGNATURE_32 ('V', 'P', 'C', 'I')
>>> // ...
>>>   UINT32                 Signature;
>>> I'm quite confused because I can't seem to find this anywhere in the
>>> VirtIO specification. The spec says nothing about signature values
>>> anywhere, just the magic value of 0x74726976. So where does this come
>>> from?
>>
>> From a quick look at the code, it seems to be a private signature value
>> applied to a software-only data structure used by the UEFI driver, and is
>> used solely for sanity checking that a pointer is pointing to the right
>> kind of object.
>>
>
> Ethin,
>
> This is a common edk2 code pattern. We have something called a Containment
> Record (CR) macro [1] and the DEBUG version of the CR macro [2] checks the
> signature. What happens is the public API is a Field in the private data
> structure, thus you can convert the This pointer to the private context of
> the driver for that protocol instances. The driver publishes the protocol
> from the Start() function so that is why the This pointer is always going to
> be a member of the larger private context structure.
>
> This pattern is also used for double linked lists as the data structure to
> represent the link list is generic and the data is attached to it via the
> Containment Record scheme by making that linked list data structure part of
> the larger data structure.
>
> [1] #define BASE_CR(Record, TYPE, Field)  ((TYPE *) ((CHAR8 *) (Record) -
> OFFSET_OF (TYPE, Field)))
>
> [2]
>   #define CR(Record, TYPE, Field, TestSignature)
>                  \
>     (DebugAssertEnabled () && (BASE_CR (Record, TYPE, Field)->Signature !=
> TestSignature)) ?  \
>     (TYPE *) (_ASSERT (CR has Bad Signature), Record) :
>                  \
>     BASE_CR (Record, TYPE, Field)
>
> Thanks,
>
> Andrew Fish
>
>
>
> 
>
>
>


-- 
Signed,
Ethin D. Probst


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#74192): https://edk2.groups.io/g/devel/message/74192
Mute This Topic: https://groups.io/mt/81710286/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-