| Message ID | cover.1698062525.git.manos.pitsidianakis@linaro.org |
|---|---|
| Headers | show |
| Series | Add VIRTIO sound card | expand |
On Mon, Oct 23, 2023 at 03:03:17PM +0300, Manos Pitsidianakis wrote: > This patch series adds an audio device implementing the recent virtio > sound spec (1.2) and a corresponding PCI wrapper device. > > v13 can be found online at: > > https://gitlab.com/epilys/qemu/-/tree/virtio-snd-v13 > > Ref e7fb941cf7636fdff40cbdcdcd660dec5f15ca3c > > Main differences with v12 patch series [^v12]: > > - Fixed device aliases (thanks Volker Rümelin <vr_qemu@t-online.de>) Acked-by: Michael S. Tsirkin <mst@redhat.com> to merge through the audio tree. > v11[^v11] -> v12[^v12] > ====================== > > - Moved devices from hw/virtio under hw/audio. > > v10[^v10] -> v11[^v11] > ====================== > > - Rebased against upstream, which has minor changes to the AUD_* API. > - Fixed noise in playback because of invalid bounds when accessing the > audio data in the VirtQueueElement. > - Refactor invalid I/O message queue flushing into separate function. > - Removed attempt to write unwritten bytes to QEMU sound backend when > flushing: it should only happen when the stream STARTs. > - Set latency_bytes to buffer size when returning TX I/O message because > it happens immediately after writing the last bytes to the QEMU > backend, therefore there might be up to <buffer size> bytes to be > played before all the buffer data has finished playing. > - Addressed [^v10] review comments: > - Refactored VirtIOSoundPCMBuffer return code into a function instead > of using goto labels in output/input audio callbacks. (Suggested by > <philmd@linaro.org>) > > v9[^v9] -> v10[^v10] > ==================== > > - Addressed [^v9] review comments. > - Copy buffer data just before playing it on the host instead of when > the IO message arrives. This in most cases takes care of the buffer > not being populated with data from the guest application when it > firsts arrives. > > v8[^v8] -> v9[^v9] > ================== > > - Addressed [^v8] review comments. > - Add cpu_to_le32(_) and le32_to_cpu(_) conversions for messages from/to > the guest according to the virtio spec. > - Inlined some functions and types to reduce review complexity. > - Corrected the replies to IO messages; now both Playback and Capture > work correctly for me. (If you hear cracks in pulseaudio+guest, try > pipewire+guest). > > v7[^v7] -> v8[^v8] > ================== > > - Addressed [^v7] review comments. > Functions that were called from more than one place for code re-use > are not created until they are actually needed. > - Fixed cases where block->offset was not respected in Playback > > v6[^v6] -> v7[^v7] > ================== > > - Removed minor stale/duplicate code. > - Addressed [^v6] review comments. > Notably, the audio driver name is now `virtio` instead of > `virtio-sound`. > - Fixed some invalid pointer logic. > - Fixed minor typos and updated documentation. > > v5[^v5] -> v6[^v6] > ================== > > - Free any existing PCM stream resources before allocating a new one. > - Add docs. > > v4[^v4] -> v5[^v5] > ================== > > - Use ERRP_GUARD() to propagate errors. > - Use virtio_add_feature() instead of XORing constants. > - Use %zu format specifier for size_t. > > v3[^v3] -> v4[^v4] > ================== > > - Addressed review style comments. > - Split patches for easier review. > > v2[^v2] -> v3[^v3] > ================== > > - Addressed review comments. > > v1[^v1] -> v2[^v2] > ================== > > - Split virtio-snd and virtio-snd-pci devices to two commits > - Added audio capture support > > Previously: > > [^v12]: > https://lore.kernel.org/qemu-devel/cover.1697709630.git.manos.pitsidianakis@linaro.org/ > [^v11]: > https://lore.kernel.org/qemu-devel/cover.1696935992.git.manos.pitsidianakis@linaro.org/ > [^v10]: > https://lore.kernel.org/qemu-devel/cover.1695996196.git.manos.pitsidianakis@linaro.org/ > [^v9]: > https://lore.kernel.org/qemu-devel/cover.1694588927.git.manos.pitsidianakis@linaro.org/ > [^v8]: > https://lore.kernel.org/qemu-devel/cover.1693252037.git.manos.pitsidianakis@linaro.org/ > [^v7]: > https://lore.kernel.org/qemu-devel/cover.1692731646.git.manos.pitsidianakis@linaro.org/ > [^v6]: > https://lore.kernel.org/qemu-devel/cover.1692089917.git.manos.pitsidianakis@linaro.org/ > [^v5]: > https://lore.kernel.org/qemu-devel/cover.1690626150.git.manos.pitsidianakis@linaro.org/ > [^v4]: > https://lore.kernel.org/qemu-devel/cover.1689857559.git.manos.pitsidianakis@linaro.org/ > [^v3]: > https://lore.kernel.org/qemu-devel/cover.1689692765.git.manos.pitsidianakis@linaro.org/ > [^v2]: > https://lore.kernel.org/qemu-devel/cover.1686238728.git.manos.pitsidianakis@linaro.org/ > [^v1]: > https://lore.kernel.org/qemu-devel/20230526204845.673031-1-manos.pitsidianakis@linaro.org/ > > Manos Pitsidianakis (11): > Add virtio-sound device stub > Add virtio-sound-pci device > virtio-sound: handle control messages and streams > virtio-sound: handle VIRTIO_SND_R_PCM_INFO request > virtio-sound: handle VIRTIO_SND_R_PCM_{START,STOP} > virtio-sound: handle VIRTIO_SND_R_PCM_SET_PARAMS > virtio-sound: handle VIRTIO_SND_R_PCM_PREPARE > virtio-sound: handle VIRTIO_SND_R_PCM_RELEASE > virtio-sound: implement audio output (TX) > virtio-sound: implement audio capture (RX) > docs/system: add basic virtio-snd documentation > > MAINTAINERS | 9 + > docs/system/device-emulation.rst | 1 + > docs/system/devices/virtio-snd.rst | 49 + > hw/audio/Kconfig | 5 + > hw/audio/meson.build | 2 + > hw/audio/trace-events | 20 + > hw/audio/virtio-snd-pci.c | 93 ++ > hw/audio/virtio-snd.c | 1409 ++++++++++++++++++++++++++++ > include/hw/audio/virtio-snd.h | 235 +++++ > system/qdev-monitor.c | 2 + > 10 files changed, 1825 insertions(+) > create mode 100644 docs/system/devices/virtio-snd.rst > create mode 100644 hw/audio/virtio-snd-pci.c > create mode 100644 hw/audio/virtio-snd.c > create mode 100644 include/hw/audio/virtio-snd.h > > > base-commit: 384dbdda94c0bba55bf186cccd3714bbb9b737e9 > -- > 2.39.2
"Michael S. Tsirkin" <mst@redhat.com> writes: > On Mon, Oct 23, 2023 at 03:03:17PM +0300, Manos Pitsidianakis wrote: >> This patch series adds an audio device implementing the recent virtio >> sound spec (1.2) and a corresponding PCI wrapper device. >> >> v13 can be found online at: >> >> https://gitlab.com/epilys/qemu/-/tree/virtio-snd-v13 >> >> Ref e7fb941cf7636fdff40cbdcdcd660dec5f15ca3c >> >> Main differences with v12 patch series [^v12]: >> >> - Fixed device aliases (thanks Volker Rümelin <vr_qemu@t-online.de>) > > Acked-by: Michael S. Tsirkin <mst@redhat.com> > > to merge through the audio tree. This hasn't been picked up yet and I'm worried we'll miss the soft-freeze. I guess this is the reason the entire audio stack is currently on "Odd Fixes". Should we just re-spin with us as maintainers for virtio-sound and merge it through my tree?
On Mon, Nov 06, 2023 at 11:17:16AM +0000, Alex Bennée wrote: > "Michael S. Tsirkin" <mst@redhat.com> writes: > > > On Mon, Oct 23, 2023 at 03:03:17PM +0300, Manos Pitsidianakis wrote: > >> This patch series adds an audio device implementing the recent virtio > >> sound spec (1.2) and a corresponding PCI wrapper device. > >> > >> v13 can be found online at: > >> > >> https://gitlab.com/epilys/qemu/-/tree/virtio-snd-v13 > >> > >> Ref e7fb941cf7636fdff40cbdcdcd660dec5f15ca3c > >> > >> Main differences with v12 patch series [^v12]: > >> > >> - Fixed device aliases (thanks Volker Rümelin <vr_qemu@t-online.de>) > > > > Acked-by: Michael S. Tsirkin <mst@redhat.com> > > > > to merge through the audio tree. > > This hasn't been picked up yet and I'm worried we'll miss the > soft-freeze. I guess this is the reason the entire audio stack is > currently on "Odd Fixes". > > Should we just re-spin with us as maintainers for virtio-sound and merge > it through my tree? Since there's no response I'll pick it up, don't worry.
"Michael S. Tsirkin" <mst@redhat.com> writes: > On Mon, Nov 06, 2023 at 11:17:16AM +0000, Alex Bennée wrote: >> "Michael S. Tsirkin" <mst@redhat.com> writes: >> >> > On Mon, Oct 23, 2023 at 03:03:17PM +0300, Manos Pitsidianakis wrote: >> >> This patch series adds an audio device implementing the recent virtio >> >> sound spec (1.2) and a corresponding PCI wrapper device. >> >> >> >> v13 can be found online at: >> >> >> >> https://gitlab.com/epilys/qemu/-/tree/virtio-snd-v13 >> >> >> >> Ref e7fb941cf7636fdff40cbdcdcd660dec5f15ca3c >> >> >> >> Main differences with v12 patch series [^v12]: >> >> >> >> - Fixed device aliases (thanks Volker Rümelin <vr_qemu@t-online.de>) >> > >> > Acked-by: Michael S. Tsirkin <mst@redhat.com> >> > >> > to merge through the audio tree. >> >> This hasn't been picked up yet and I'm worried we'll miss the >> soft-freeze. I guess this is the reason the entire audio stack is >> currently on "Odd Fixes". >> >> Should we just re-spin with us as maintainers for virtio-sound and merge >> it through my tree? > > Since there's no response I'll pick it up, don't worry. Thanks :-)
On Mon, Oct 23, 2023 at 03:03:17PM +0300, Manos Pitsidianakis wrote: > This patch series adds an audio device implementing the recent virtio > sound spec (1.2) and a corresponding PCI wrapper device. Pls send patches on top to fix checkpatch warnings. https://gitlab.com/mstredhat/qemu/-/jobs/5470533893 > v13 can be found online at: > > https://gitlab.com/epilys/qemu/-/tree/virtio-snd-v13 > > Ref e7fb941cf7636fdff40cbdcdcd660dec5f15ca3c > > Main differences with v12 patch series [^v12]: > > - Fixed device aliases (thanks Volker Rümelin <vr_qemu@t-online.de>) > > v11[^v11] -> v12[^v12] > ====================== > > - Moved devices from hw/virtio under hw/audio. > > v10[^v10] -> v11[^v11] > ====================== > > - Rebased against upstream, which has minor changes to the AUD_* API. > - Fixed noise in playback because of invalid bounds when accessing the > audio data in the VirtQueueElement. > - Refactor invalid I/O message queue flushing into separate function. > - Removed attempt to write unwritten bytes to QEMU sound backend when > flushing: it should only happen when the stream STARTs. > - Set latency_bytes to buffer size when returning TX I/O message because > it happens immediately after writing the last bytes to the QEMU > backend, therefore there might be up to <buffer size> bytes to be > played before all the buffer data has finished playing. > - Addressed [^v10] review comments: > - Refactored VirtIOSoundPCMBuffer return code into a function instead > of using goto labels in output/input audio callbacks. (Suggested by > <philmd@linaro.org>) > > v9[^v9] -> v10[^v10] > ==================== > > - Addressed [^v9] review comments. > - Copy buffer data just before playing it on the host instead of when > the IO message arrives. This in most cases takes care of the buffer > not being populated with data from the guest application when it > firsts arrives. > > v8[^v8] -> v9[^v9] > ================== > > - Addressed [^v8] review comments. > - Add cpu_to_le32(_) and le32_to_cpu(_) conversions for messages from/to > the guest according to the virtio spec. > - Inlined some functions and types to reduce review complexity. > - Corrected the replies to IO messages; now both Playback and Capture > work correctly for me. (If you hear cracks in pulseaudio+guest, try > pipewire+guest). > > v7[^v7] -> v8[^v8] > ================== > > - Addressed [^v7] review comments. > Functions that were called from more than one place for code re-use > are not created until they are actually needed. > - Fixed cases where block->offset was not respected in Playback > > v6[^v6] -> v7[^v7] > ================== > > - Removed minor stale/duplicate code. > - Addressed [^v6] review comments. > Notably, the audio driver name is now `virtio` instead of > `virtio-sound`. > - Fixed some invalid pointer logic. > - Fixed minor typos and updated documentation. > > v5[^v5] -> v6[^v6] > ================== > > - Free any existing PCM stream resources before allocating a new one. > - Add docs. > > v4[^v4] -> v5[^v5] > ================== > > - Use ERRP_GUARD() to propagate errors. > - Use virtio_add_feature() instead of XORing constants. > - Use %zu format specifier for size_t. > > v3[^v3] -> v4[^v4] > ================== > > - Addressed review style comments. > - Split patches for easier review. > > v2[^v2] -> v3[^v3] > ================== > > - Addressed review comments. > > v1[^v1] -> v2[^v2] > ================== > > - Split virtio-snd and virtio-snd-pci devices to two commits > - Added audio capture support > > Previously: > > [^v12]: > https://lore.kernel.org/qemu-devel/cover.1697709630.git.manos.pitsidianakis@linaro.org/ > [^v11]: > https://lore.kernel.org/qemu-devel/cover.1696935992.git.manos.pitsidianakis@linaro.org/ > [^v10]: > https://lore.kernel.org/qemu-devel/cover.1695996196.git.manos.pitsidianakis@linaro.org/ > [^v9]: > https://lore.kernel.org/qemu-devel/cover.1694588927.git.manos.pitsidianakis@linaro.org/ > [^v8]: > https://lore.kernel.org/qemu-devel/cover.1693252037.git.manos.pitsidianakis@linaro.org/ > [^v7]: > https://lore.kernel.org/qemu-devel/cover.1692731646.git.manos.pitsidianakis@linaro.org/ > [^v6]: > https://lore.kernel.org/qemu-devel/cover.1692089917.git.manos.pitsidianakis@linaro.org/ > [^v5]: > https://lore.kernel.org/qemu-devel/cover.1690626150.git.manos.pitsidianakis@linaro.org/ > [^v4]: > https://lore.kernel.org/qemu-devel/cover.1689857559.git.manos.pitsidianakis@linaro.org/ > [^v3]: > https://lore.kernel.org/qemu-devel/cover.1689692765.git.manos.pitsidianakis@linaro.org/ > [^v2]: > https://lore.kernel.org/qemu-devel/cover.1686238728.git.manos.pitsidianakis@linaro.org/ > [^v1]: > https://lore.kernel.org/qemu-devel/20230526204845.673031-1-manos.pitsidianakis@linaro.org/ > > Manos Pitsidianakis (11): > Add virtio-sound device stub > Add virtio-sound-pci device > virtio-sound: handle control messages and streams > virtio-sound: handle VIRTIO_SND_R_PCM_INFO request > virtio-sound: handle VIRTIO_SND_R_PCM_{START,STOP} > virtio-sound: handle VIRTIO_SND_R_PCM_SET_PARAMS > virtio-sound: handle VIRTIO_SND_R_PCM_PREPARE > virtio-sound: handle VIRTIO_SND_R_PCM_RELEASE > virtio-sound: implement audio output (TX) > virtio-sound: implement audio capture (RX) > docs/system: add basic virtio-snd documentation > > MAINTAINERS | 9 + > docs/system/device-emulation.rst | 1 + > docs/system/devices/virtio-snd.rst | 49 + > hw/audio/Kconfig | 5 + > hw/audio/meson.build | 2 + > hw/audio/trace-events | 20 + > hw/audio/virtio-snd-pci.c | 93 ++ > hw/audio/virtio-snd.c | 1409 ++++++++++++++++++++++++++++ > include/hw/audio/virtio-snd.h | 235 +++++ > system/qdev-monitor.c | 2 + > 10 files changed, 1825 insertions(+) > create mode 100644 docs/system/devices/virtio-snd.rst > create mode 100644 hw/audio/virtio-snd-pci.c > create mode 100644 hw/audio/virtio-snd.c > create mode 100644 include/hw/audio/virtio-snd.h > > > base-commit: 384dbdda94c0bba55bf186cccd3714bbb9b737e9 > -- > 2.39.2
On Tue, 7 Nov 2023 at 02:44, Michael S. Tsirkin <mst@redhat.com> wrote: > > On Mon, Oct 23, 2023 at 03:03:17PM +0300, Manos Pitsidianakis wrote: > > This patch series adds an audio device implementing the recent virtio > > sound spec (1.2) and a corresponding PCI wrapper device. > > > Pls send patches on top to fix checkpatch warnings. > > https://gitlab.com/mstredhat/qemu/-/jobs/5470533893 Do you mean this one? 6583087d531f2e37a2e2ba2cef7d123e21f8b0ad:214: ERROR: spaces required around that ':' (ctx:VxE) total: 1 errors, 0 warnings, 674 lines checked If yes, it's a false warning. It's a switch statement with a macro and it confuses it with the ternary operator.
This patch series adds an audio device implementing the recent virtio sound spec (1.2) and a corresponding PCI wrapper device. v13 can be found online at: https://gitlab.com/epilys/qemu/-/tree/virtio-snd-v13 Ref e7fb941cf7636fdff40cbdcdcd660dec5f15ca3c Main differences with v12 patch series [^v12]: - Fixed device aliases (thanks Volker Rümelin <vr_qemu@t-online.de>) v11[^v11] -> v12[^v12] ====================== - Moved devices from hw/virtio under hw/audio. v10[^v10] -> v11[^v11] ====================== - Rebased against upstream, which has minor changes to the AUD_* API. - Fixed noise in playback because of invalid bounds when accessing the audio data in the VirtQueueElement. - Refactor invalid I/O message queue flushing into separate function. - Removed attempt to write unwritten bytes to QEMU sound backend when flushing: it should only happen when the stream STARTs. - Set latency_bytes to buffer size when returning TX I/O message because it happens immediately after writing the last bytes to the QEMU backend, therefore there might be up to <buffer size> bytes to be played before all the buffer data has finished playing. - Addressed [^v10] review comments: - Refactored VirtIOSoundPCMBuffer return code into a function instead of using goto labels in output/input audio callbacks. (Suggested by <philmd@linaro.org>) v9[^v9] -> v10[^v10] ==================== - Addressed [^v9] review comments. - Copy buffer data just before playing it on the host instead of when the IO message arrives. This in most cases takes care of the buffer not being populated with data from the guest application when it firsts arrives. v8[^v8] -> v9[^v9] ================== - Addressed [^v8] review comments. - Add cpu_to_le32(_) and le32_to_cpu(_) conversions for messages from/to the guest according to the virtio spec. - Inlined some functions and types to reduce review complexity. - Corrected the replies to IO messages; now both Playback and Capture work correctly for me. (If you hear cracks in pulseaudio+guest, try pipewire+guest). v7[^v7] -> v8[^v8] ================== - Addressed [^v7] review comments. Functions that were called from more than one place for code re-use are not created until they are actually needed. - Fixed cases where block->offset was not respected in Playback v6[^v6] -> v7[^v7] ================== - Removed minor stale/duplicate code. - Addressed [^v6] review comments. Notably, the audio driver name is now `virtio` instead of `virtio-sound`. - Fixed some invalid pointer logic. - Fixed minor typos and updated documentation. v5[^v5] -> v6[^v6] ================== - Free any existing PCM stream resources before allocating a new one. - Add docs. v4[^v4] -> v5[^v5] ================== - Use ERRP_GUARD() to propagate errors. - Use virtio_add_feature() instead of XORing constants. - Use %zu format specifier for size_t. v3[^v3] -> v4[^v4] ================== - Addressed review style comments. - Split patches for easier review. v2[^v2] -> v3[^v3] ================== - Addressed review comments. v1[^v1] -> v2[^v2] ================== - Split virtio-snd and virtio-snd-pci devices to two commits - Added audio capture support Previously: [^v12]: https://lore.kernel.org/qemu-devel/cover.1697709630.git.manos.pitsidianakis@linaro.org/ [^v11]: https://lore.kernel.org/qemu-devel/cover.1696935992.git.manos.pitsidianakis@linaro.org/ [^v10]: https://lore.kernel.org/qemu-devel/cover.1695996196.git.manos.pitsidianakis@linaro.org/ [^v9]: https://lore.kernel.org/qemu-devel/cover.1694588927.git.manos.pitsidianakis@linaro.org/ [^v8]: https://lore.kernel.org/qemu-devel/cover.1693252037.git.manos.pitsidianakis@linaro.org/ [^v7]: https://lore.kernel.org/qemu-devel/cover.1692731646.git.manos.pitsidianakis@linaro.org/ [^v6]: https://lore.kernel.org/qemu-devel/cover.1692089917.git.manos.pitsidianakis@linaro.org/ [^v5]: https://lore.kernel.org/qemu-devel/cover.1690626150.git.manos.pitsidianakis@linaro.org/ [^v4]: https://lore.kernel.org/qemu-devel/cover.1689857559.git.manos.pitsidianakis@linaro.org/ [^v3]: https://lore.kernel.org/qemu-devel/cover.1689692765.git.manos.pitsidianakis@linaro.org/ [^v2]: https://lore.kernel.org/qemu-devel/cover.1686238728.git.manos.pitsidianakis@linaro.org/ [^v1]: https://lore.kernel.org/qemu-devel/20230526204845.673031-1-manos.pitsidianakis@linaro.org/ Manos Pitsidianakis (11): Add virtio-sound device stub Add virtio-sound-pci device virtio-sound: handle control messages and streams virtio-sound: handle VIRTIO_SND_R_PCM_INFO request virtio-sound: handle VIRTIO_SND_R_PCM_{START,STOP} virtio-sound: handle VIRTIO_SND_R_PCM_SET_PARAMS virtio-sound: handle VIRTIO_SND_R_PCM_PREPARE virtio-sound: handle VIRTIO_SND_R_PCM_RELEASE virtio-sound: implement audio output (TX) virtio-sound: implement audio capture (RX) docs/system: add basic virtio-snd documentation MAINTAINERS | 9 + docs/system/device-emulation.rst | 1 + docs/system/devices/virtio-snd.rst | 49 + hw/audio/Kconfig | 5 + hw/audio/meson.build | 2 + hw/audio/trace-events | 20 + hw/audio/virtio-snd-pci.c | 93 ++ hw/audio/virtio-snd.c | 1409 ++++++++++++++++++++++++++++ include/hw/audio/virtio-snd.h | 235 +++++ system/qdev-monitor.c | 2 + 10 files changed, 1825 insertions(+) create mode 100644 docs/system/devices/virtio-snd.rst create mode 100644 hw/audio/virtio-snd-pci.c create mode 100644 hw/audio/virtio-snd.c create mode 100644 include/hw/audio/virtio-snd.h base-commit: 384dbdda94c0bba55bf186cccd3714bbb9b737e9