1. Patchew
  2. QEMU
  3. View series

[RFC PATCH 00/27] Virtio sound card implementation

Shreyansh Chouhan posted 27 patches 3 years ago
Test checkpatch failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20210429120445.694420-1-chouhan.shreyansh2702@gmail.com
Maintainers: Gerd Hoffmann <kraxel@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>
hw/audio/Kconfig               |    5 +
hw/audio/meson.build           |    1 +
hw/audio/virtio-snd.c          | 1168 ++++++++++++++++++++++++++++++++
hw/virtio/meson.build          |    1 +
hw/virtio/virtio-snd-pci.c     |   72 ++
include/hw/virtio/virtio-snd.h |  393 +++++++++++
6 files changed, 1640 insertions(+)
create mode 100644 hw/audio/virtio-snd.c
create mode 100644 hw/virtio/virtio-snd-pci.c
create mode 100644 include/hw/virtio/virtio-snd.h
[RFC PATCH 00/27] Virtio sound card implementation
Posted by Shreyansh Chouhan 3 years ago 
This patch series aims to implement the virtio sound card
as defined in the virtio specs (v8). The specs can be found
at the following github repo:
https://github.com/oasis-tcs/virtio-spec

This patch series is not complete yet, but here is what's
already been done:

    - The device is initialized properly and is recognized
      by the guest as a sound card device.
    - Output stream works but the output is very noisy. (Which
      is what I wanted coments on.)

What remains to be done:

    - Input streams yet to be done.
    - The jacks are initialized with a default config, but
      they are not mapped to any streams for now.
    - Channel maps are yet to be implemented.

I'd like to request some comments on the following points:

    - The output from the sound card is accompanied by periodic
      white noise. I do not know why this is happening. I tried
      debugging it by writing the buffers to a new wav file and
      sure enough the contents of the file were different at
      some places, but I couldn't find what must be causing it.
      (Relevant patches: #19, #20, #21 and #25.) What steps should
      I take for debugging this?

    - If I try and output a wav file of a different size, that
      sets the period_bytes to 4004, I get an assert failure in
      the object_unref function defined in qom/object.c. (Function
      defined on line #681, assert on line #690.)
                assert(obj->parent == NULL);
      I tried taking a look at the stack trace for when this failure
      happens. In the stacktrace I found out that this happened
      when QEMU was trying to unmap the out_sg from the VirtQueue
      element. This failure doesn't happen if I am using a different
      wav file, that sets the period_bytes to something else.
      (Relevant patches: #19, #20, #21 and #25.)
      What could be causing this problem?

    - What is the suggested way of waiting? When the driver issues
      the VIRTIO_SND_PCM_STOP ctrl command I want to wait for the
      buffers existing in tx vq to be consumed before closing the
      stream.

Shreyansh Chouhan (27):
  virtio-snd: Add virtio sound header file
  virtio-snd: Add jack control structures
  virtio-snd: Add PCM control structures
  virtio-snd: Add chmap control structures
  virtio-snd: Add device implementation structures
  virtio-snd: Add PCI wrapper code for VirtIOSound
  virtio-snd: Add properties for class init
  virtio-snd: Add code for get config function
  virtio-snd: Add code for set config function
  virtio-snd: Add code for the realize function
  virtio-snd: Add macros for logging
  virtio-snd: Add control virtqueue handler
  virtio-snd: Add VIRTIO_SND_R_JACK_INFO handler
  virtio-snd: Add stub for VIRTIO_SND_R_JACK_REMAP handler
  virtio-snd: Add VIRTIO_SND_R_PCM_INFO handler
  virtio-snd: Add VIRITO_SND_R_PCM_SET_PARAMS handle
  virtio-snd: Add VIRTIO_SND_R_PCM_PREPARE handler
  virtio-snd: Add default configs to realize fn
  virtio-snd: Add callback for SWVoiceOut
  virtio-snd: Add VIRITO_SND_R_PCM_START handler
  virtio-snd: Add VIRTIO_SND_R_PCM_STOP handler
  virtio-snd: Add VIRTIO_SND_R_PCM_RELEASE handler
  virtio-snd: Replaced goto with if else
  virtio-snd: Add code to device unrealize function
  virtio-snd: Add tx vq and handler
  virtio-snd: Add event vq and a handler stub
  virtio-snd: Add rx vq and stub handler

 hw/audio/Kconfig               |    5 +
 hw/audio/meson.build           |    1 +
 hw/audio/virtio-snd.c          | 1168 ++++++++++++++++++++++++++++++++
 hw/virtio/meson.build          |    1 +
 hw/virtio/virtio-snd-pci.c     |   72 ++
 include/hw/virtio/virtio-snd.h |  393 +++++++++++
 6 files changed, 1640 insertions(+)
 create mode 100644 hw/audio/virtio-snd.c
 create mode 100644 hw/virtio/virtio-snd-pci.c
 create mode 100644 include/hw/virtio/virtio-snd.h

-- 
2.25.1
Re: [RFC PATCH 00/27] Virtio sound card implementation
Posted by no-reply@patchew.org 3 years ago 
Patchew URL: https://patchew.org/QEMU/20210429120445.694420-1-chouhan.shreyansh2702@gmail.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20210429120445.694420-1-chouhan.shreyansh2702@gmail.com
Subject: [RFC PATCH 00/27] Virtio sound card implementation

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]         patchew/20210429120445.694420-1-chouhan.shreyansh2702@gmail.com -> patchew/20210429120445.694420-1-chouhan.shreyansh2702@gmail.com
Switched to a new branch 'test'
28acae8 virtio-snd: Add rx vq and stub handler
f130077 virtio-snd: Add event vq and a handler stub
ba43c58 virtio-snd: Add tx vq and handler
d1fae3c virtio-snd: Add code to device unrealize function
5633fc1 virtio-snd: Replaced goto with if else
df2aca8 virtio-snd: Add VIRTIO_SND_R_PCM_RELEASE handler
4d5b976 virtio-snd: Add VIRTIO_SND_R_PCM_STOP handler
9c792b2 virtio-snd: Add VIRITO_SND_R_PCM_START handler
ad07e2c virtio-snd: Add callback for SWVoiceOut
6a9e47d virtio-snd: Add default configs to realize fn
75c62c8 virtio-snd: Add VIRTIO_SND_R_PCM_PREPARE handler
64043bd virtio-snd: Add VIRITO_SND_R_PCM_SET_PARAMS handle
bb5fe5d virtio-snd: Add VIRTIO_SND_R_PCM_INFO handler
858de0e virtio-snd: Add stub for VIRTIO_SND_R_JACK_REMAP handler
a1596e5 virtio-snd: Add VIRTIO_SND_R_JACK_INFO handler
d303365 virtio-snd: Add control virtqueue handler
4c49f92 virtio-snd: Add macros for logging
504942a virtio-snd: Add code for the realize function
d7fad34 virtio-snd: Add code for set config function
f1d51f3 virtio-snd: Add code for get config function
cfc404f virtio-snd: Add properties for class init
b0b4fa2 virtio-snd: Add PCI wrapper code for VirtIOSound
0a0606a virtio-snd: Add device implementation structures
e57488a virtio-snd: Add chmap control structures
f8faf7e virtio-snd: Add PCM control structures
ba1cc8b virtio-snd: Add jack control structures
cc2413a virtio-snd: Add virtio sound header file

=== OUTPUT BEGIN ===
1/27 Checking commit cc2413a852b8 (virtio-snd: Add virtio sound header file)
Use of uninitialized value $acpi_testexpected in string eq at ./scripts/checkpatch.pl line 1529.
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#17: 
new file mode 100644

total: 0 errors, 1 warnings, 97 lines checked

Patch 1/27 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
2/27 Checking commit ba1cc8ba3399 (virtio-snd: Add jack control structures)
3/27 Checking commit f8faf7ec9a71 (virtio-snd: Add PCM control structures)
4/27 Checking commit e57488ae6767 (virtio-snd: Add chmap control structures)
5/27 Checking commit 0a0606abe96c (virtio-snd: Add device implementation structures)
6/27 Checking commit b0b4fa241458 (virtio-snd: Add PCI wrapper code for VirtIOSound)
Use of uninitialized value $acpi_testexpected in string eq at ./scripts/checkpatch.pl line 1529.
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#30: 
new file mode 100644

total: 0 errors, 1 warnings, 79 lines checked

Patch 6/27 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
7/27 Checking commit cfc404f0d2ff (virtio-snd: Add properties for class init)
Use of uninitialized value $acpi_testexpected in string eq at ./scripts/checkpatch.pl line 1529.
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#41: 
new file mode 100644

total: 0 errors, 1 warnings, 138 lines checked

Patch 7/27 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
8/27 Checking commit f1d51f3d3a6d (virtio-snd: Add code for get config function)
9/27 Checking commit d7fad34dacf1 (virtio-snd: Add code for set config function)
10/27 Checking commit 504942a2f235 (virtio-snd: Add code for the realize function)
11/27 Checking commit 4c49f923a5dd (virtio-snd: Add macros for logging)
12/27 Checking commit d3033655589b (virtio-snd: Add control virtqueue handler)
WARNING: Block comments use a leading /* on a separate line
#23: FILE: hw/audio/virtio-snd.c:104:
+/* The control queue handler. Pops an element from the control virtqueue,

total: 0 errors, 1 warnings, 83 lines checked

Patch 12/27 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
13/27 Checking commit a1596e59b05f (virtio-snd: Add VIRTIO_SND_R_JACK_INFO handler)
14/27 Checking commit 858de0ed4483 (virtio-snd: Add stub for VIRTIO_SND_R_JACK_REMAP handler)
15/27 Checking commit bb5fe5d45c23 (virtio-snd: Add VIRTIO_SND_R_PCM_INFO handler)
16/27 Checking commit 64043bdef02c (virtio-snd: Add VIRITO_SND_R_PCM_SET_PARAMS handle)
WARNING: line over 80 characters
#35: FILE: hw/audio/virtio-snd.c:294:
+                                               virtio_snd_pcm_set_params *params)

total: 0 errors, 1 warnings, 112 lines checked

Patch 16/27 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
17/27 Checking commit 75c62c857884 (virtio-snd: Add VIRTIO_SND_R_PCM_PREPARE handler)
WARNING: Block comments use a leading /* on a separate line
#168: FILE: hw/audio/virtio-snd.c:526:
+        /* st->voice.out = AUD_open_out(&s->card,

WARNING: Block comments use a leading /* on a separate line
#175: FILE: hw/audio/virtio-snd.c:533:
+        /* st->voice.in = AUD_open_in(&s->card,

total: 0 errors, 2 warnings, 210 lines checked

Patch 17/27 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
18/27 Checking commit 6a9e47df0a16 (virtio-snd: Add default configs to realize fn)
Argument "m" isn't numeric in numeric eq (==) at ./scripts/checkpatch.pl line 2830.
Argument "m" isn't numeric in numeric eq (==) at ./scripts/checkpatch.pl line 2830.
Use of uninitialized value $1 in concatenation (.) or string at ./scripts/checkpatch.pl line 2831.
ERROR: do not use C99 // comments
#24: FILE: hw/audio/virtio-snd.c:686:
+    // set default params for all streams

ERROR: Error messages should not contain newlines
#44: FILE: hw/audio/virtio-snd.c:706:
+            error_setg(errp, "Can't initalize stream params.\n");

ERROR: Error messages should not contain newlines
#49: FILE: hw/audio/virtio-snd.c:711:
+            error_setg(errp, "Can't prepare streams.\n");

ERROR: do not use C99 // comments
#55: FILE: hw/audio/virtio-snd.c:717:
+        // TODO: For now the hda_fn_nid connects the starting streams to these

ERROR: do not use C99 // comments
#56: FILE: hw/audio/virtio-snd.c:718:
+        // jacks. This isn't working for now since the directions will be wrong

ERROR: do not use C99 // comments
#57: FILE: hw/audio/virtio-snd.c:719:
+        // for a few jacks. Similarly the capabilities are just placeholders.

ERROR: unnecessary cast may hide bugs, use g_new0 instead
#58: FILE: hw/audio/virtio-snd.c:720:
+        s->jacks[i] = (virtio_snd_jack *)g_malloc0(sizeof(virtio_snd_jack));

ERROR: line over 90 characters
#61: FILE: hw/audio/virtio-snd.c:723:
+        s->jacks[i]->hda_reg_defconf = ((AC_JACK_PORT_COMPLEX << AC_DEFCFG_PORT_CONN_SHIFT) |

ERROR: line over 90 characters
#62: FILE: hw/audio/virtio-snd.c:724:
+                                       (AC_JACK_LINE_OUT     << AC_DEFCFG_DEVICE_SHIFT)    |

ERROR: line over 90 characters
#63: FILE: hw/audio/virtio-snd.c:725:
+                                       (AC_JACK_CONN_1_8     << AC_DEFCFG_CONN_TYPE_SHIFT) |

ERROR: line over 90 characters
#64: FILE: hw/audio/virtio-snd.c:726:
+                                       (AC_JACK_COLOR_GREEN  << AC_DEFCFG_COLOR_SHIFT)     |

total: 11 errors, 0 warnings, 51 lines checked

Patch 18/27 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

19/27 Checking commit ad07e2c375df (virtio-snd: Add callback for SWVoiceOut)
WARNING: line over 80 characters
#129: FILE: hw/audio/virtio-snd.c:575:
+        int curr_elem_size = iov_size(st->elems[i]->out_sg, st->elems[i]->out_num)

total: 0 errors, 1 warnings, 183 lines checked

Patch 19/27 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
20/27 Checking commit 9c792b226bf4 (virtio-snd: Add VIRITO_SND_R_PCM_START handler)
21/27 Checking commit 4d5b9762480e (virtio-snd: Add VIRTIO_SND_R_PCM_STOP handler)
22/27 Checking commit df2aca848dfb (virtio-snd: Add VIRTIO_SND_R_PCM_RELEASE handler)
23/27 Checking commit 5633fc14cfe7 (virtio-snd: Replaced goto with if else)
24/27 Checking commit d1fae3cd628b (virtio-snd: Add code to device unrealize function)
25/27 Checking commit ba43c5895b33 (virtio-snd: Add tx vq and handler)
WARNING: line over 80 characters
#70: FILE: hw/audio/virtio-snd.c:973:
+        if (iov_size(elem->in_sg, elem->in_num) < sizeof(virtio_snd_pcm_status) ||

WARNING: line over 80 characters
#71: FILE: hw/audio/virtio-snd.c:974:
+            iov_size(elem->out_sg, elem->out_num) < sizeof(virtio_snd_pcm_xfer)) {

total: 0 errors, 2 warnings, 80 lines checked

Patch 25/27 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
26/27 Checking commit f13007720379 (virtio-snd: Add event vq and a handler stub)
27/27 Checking commit 28acae83382f (virtio-snd: Add rx vq and stub handler)
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20210429120445.694420-1-chouhan.shreyansh2702@gmail.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
Re: [RFC PATCH 00/27] Virtio sound card implementation
Posted by Gerd Hoffmann 3 years ago 
  Hi,

>     - The output from the sound card is accompanied by periodic
>       white noise. I do not know why this is happening. I tried
>       debugging it by writing the buffers to a new wav file and
>       sure enough the contents of the file were different at
>       some places, but I couldn't find what must be causing it.
>       (Relevant patches: #19, #20, #21 and #25.) What steps should
>       I take for debugging this?

Hmm, I'd try to simplify the code.  The two helper functions
virtio_snd_pcm_get_buf() + virtio_snd_pcm_handle_buf_written() look
rather complex to me.  I'd suggest to make them handle a single
virtqueue element only.  That should make the logic simpler.
The loop in virtio_snd_output_cb() will need to take a few more rounds
then, and virtio_snd_pcm_get_buf() would probably have to return the
number of bytes it actually placed in the buffer so you can pass on that
value to AUD_write().

For actual debugging I typically use trace points or temporary debug
printfs or a combination of both.  I'd suggest logging the buffer
handling, filling them from virt queue, writing to AUD, also log the
offset, maybe something goes wrong with partial writes.

>     - What is the suggested way of waiting? When the driver issues
>       the VIRTIO_SND_PCM_STOP ctrl command I want to wait for the
>       buffers existing in tx vq to be consumed before closing the
>       stream.

Store a pointer to the virtqueue element, then complete it when
virtio_snd_output_cb() processed all pending buffers.

take care,
  Gerd
[RFC PATCH v2 00/25] Virtio Sound card Implementation
Posted by Shreyansh Chouhan 2 years, 2 months ago 
The second RFC for implementing the VirtIO Sound card as described in
the virtio specs. Sorry for the absence of activity on this.

The output from the sound card works.

What remains to be done:
- Features defined in PCM features. (Eg message polling)
- Channel maps
- Jack remaps
- Input

I will work on the input after I have implemented the output
along with all the features since at that point it should just be a
matter of reversing a few things in the code that writes the audio.

I can work on this patchset mostly on weekends now but I will try to be
more regular with this.

Reviews are welcome :)

Shreyansh Chouhan (25):
  virtio-snd: Add virtio sound header file
  virtio-snd: Add jack control structures
  virtio-snd: Add PCM control structures
  virtio-snd: Add chmap control structures
  virtio-snd: Add device implementation structures
  virtio-snd: Add PCI wrapper code for VirtIOSound
  virtio-snd: Add properties for class init
  virtio-snd: Add code for get config function
  virtio-snd: Add code for the realize function
  virtio-snd: Add macros for logging
  virtio-snd: Add control virtqueue handler
  virtio-snd: Add VIRTIO_SND_R_JACK_INFO handler
  virtio-snd: Add stub for VIRTIO_SND_R_JACK_REMAP handler
  virtio-snd: Add VIRTIO_SND_R_PCM_INFO handler
  virtio-snd: Add VIRITO_SND_R_PCM_SET_PARAMS handle
  virtio-snd: Add VIRTIO_SND_R_PCM_PREPARE handler
  virtio-snd: Add default configs to realize fn
  virtio-snd: Add callback for SWVoiceOut
  virtio-snd: Add start/stop handler
  virtio-snd: Add VIRTIO_SND_R_PCM_RELEASE handler
  virtio-snd: Replaced goto with if else
  virtio-snd: Add code to device unrealize function
  virtio-snd: Add xfer handler
  virtio-snd: Add event vq and a handler stub
  virtio-snd: Replaced AUD_log with tracepoints

 hw/audio/Kconfig               |    5 +
 hw/audio/meson.build           |    1 +
 hw/audio/trace-events          |   14 +
 hw/audio/virtio-snd.c          | 1241 ++++++++++++++++++++++++++++++++
 hw/virtio/meson.build          |    1 +
 hw/virtio/virtio-snd-pci.c     |   72 ++
 include/hw/virtio/virtio-snd.h |  383 ++++++++++
 7 files changed, 1717 insertions(+)
 create mode 100644 hw/audio/virtio-snd.c
 create mode 100644 hw/virtio/virtio-snd-pci.c
 create mode 100644 include/hw/virtio/virtio-snd.h

-- 
2.31.1
Re: [RFC PATCH v2 00/25] Virtio Sound card Implementation
Posted by Stefano Garzarella 1 year, 2 months ago 
Hi Shreyansh,

On Fri, Feb 11, 2022 at 11:18 PM Shreyansh Chouhan
<chouhan.shreyansh2702@gmail.com> wrote:
>
> The second RFC for implementing the VirtIO Sound card as described in
> the virtio specs. Sorry for the absence of activity on this.

Thanks for starting working on virtio-sound device for QEMU!
I'm interested in completing this work, but first I wanted to know if
you are still working on it or have any new updates.

Thanks,
Stefano

>
> The output from the sound card works.
>
> What remains to be done:
> - Features defined in PCM features. (Eg message polling)
> - Channel maps
> - Jack remaps
> - Input
>
> I will work on the input after I have implemented the output
> along with all the features since at that point it should just be a
> matter of reversing a few things in the code that writes the audio.
>
> I can work on this patchset mostly on weekends now but I will try to be
> more regular with this.
>
> Reviews are welcome :)
>
> Shreyansh Chouhan (25):
>   virtio-snd: Add virtio sound header file
>   virtio-snd: Add jack control structures
>   virtio-snd: Add PCM control structures
>   virtio-snd: Add chmap control structures
>   virtio-snd: Add device implementation structures
>   virtio-snd: Add PCI wrapper code for VirtIOSound
>   virtio-snd: Add properties for class init
>   virtio-snd: Add code for get config function
>   virtio-snd: Add code for the realize function
>   virtio-snd: Add macros for logging
>   virtio-snd: Add control virtqueue handler
>   virtio-snd: Add VIRTIO_SND_R_JACK_INFO handler
>   virtio-snd: Add stub for VIRTIO_SND_R_JACK_REMAP handler
>   virtio-snd: Add VIRTIO_SND_R_PCM_INFO handler
>   virtio-snd: Add VIRITO_SND_R_PCM_SET_PARAMS handle
>   virtio-snd: Add VIRTIO_SND_R_PCM_PREPARE handler
>   virtio-snd: Add default configs to realize fn
>   virtio-snd: Add callback for SWVoiceOut
>   virtio-snd: Add start/stop handler
>   virtio-snd: Add VIRTIO_SND_R_PCM_RELEASE handler
>   virtio-snd: Replaced goto with if else
>   virtio-snd: Add code to device unrealize function
>   virtio-snd: Add xfer handler
>   virtio-snd: Add event vq and a handler stub
>   virtio-snd: Replaced AUD_log with tracepoints
>
>  hw/audio/Kconfig               |    5 +
>  hw/audio/meson.build           |    1 +
>  hw/audio/trace-events          |   14 +
>  hw/audio/virtio-snd.c          | 1241 ++++++++++++++++++++++++++++++++
>  hw/virtio/meson.build          |    1 +
>  hw/virtio/virtio-snd-pci.c     |   72 ++
>  include/hw/virtio/virtio-snd.h |  383 ++++++++++
>  7 files changed, 1717 insertions(+)
>  create mode 100644 hw/audio/virtio-snd.c
>  create mode 100644 hw/virtio/virtio-snd-pci.c
>  create mode 100644 include/hw/virtio/virtio-snd.h
>
> --
> 2.31.1
>
>
Re: [RFC PATCH v2 00/25] Virtio Sound card Implementation
Posted by Laurent Vivier 2 years, 2 months ago 
Le 11/02/2022 à 23:12, Shreyansh Chouhan a écrit :
> The second RFC for implementing the VirtIO Sound card as described in
> the virtio specs. Sorry for the absence of activity on this.
> 
> The output from the sound card works.
> 
> What remains to be done:
> - Features defined in PCM features. (Eg message polling)
> - Channel maps
> - Jack remaps
> - Input
> 
> I will work on the input after I have implemented the output
> along with all the features since at that point it should just be a
> matter of reversing a few things in the code that writes the audio.
> 
> I can work on this patchset mostly on weekends now but I will try to be
> more regular with this.
> 
> Reviews are welcome :)
> 
> Shreyansh Chouhan (25):
>    virtio-snd: Add virtio sound header file
>    virtio-snd: Add jack control structures
>    virtio-snd: Add PCM control structures
>    virtio-snd: Add chmap control structures
>    virtio-snd: Add device implementation structures
>    virtio-snd: Add PCI wrapper code for VirtIOSound
>    virtio-snd: Add properties for class init
>    virtio-snd: Add code for get config function
>    virtio-snd: Add code for the realize function
>    virtio-snd: Add macros for logging
>    virtio-snd: Add control virtqueue handler
>    virtio-snd: Add VIRTIO_SND_R_JACK_INFO handler
>    virtio-snd: Add stub for VIRTIO_SND_R_JACK_REMAP handler
>    virtio-snd: Add VIRTIO_SND_R_PCM_INFO handler
>    virtio-snd: Add VIRITO_SND_R_PCM_SET_PARAMS handle
>    virtio-snd: Add VIRTIO_SND_R_PCM_PREPARE handler
>    virtio-snd: Add default configs to realize fn
>    virtio-snd: Add callback for SWVoiceOut
>    virtio-snd: Add start/stop handler
>    virtio-snd: Add VIRTIO_SND_R_PCM_RELEASE handler
>    virtio-snd: Replaced goto with if else
>    virtio-snd: Add code to device unrealize function
>    virtio-snd: Add xfer handler
>    virtio-snd: Add event vq and a handler stub
>    virtio-snd: Replaced AUD_log with tracepoints
> 
>   hw/audio/Kconfig               |    5 +
>   hw/audio/meson.build           |    1 +
>   hw/audio/trace-events          |   14 +
>   hw/audio/virtio-snd.c          | 1241 ++++++++++++++++++++++++++++++++
>   hw/virtio/meson.build          |    1 +
>   hw/virtio/virtio-snd-pci.c     |   72 ++
>   include/hw/virtio/virtio-snd.h |  383 ++++++++++
>   7 files changed, 1717 insertions(+)
>   create mode 100644 hw/audio/virtio-snd.c
>   create mode 100644 hw/virtio/virtio-snd-pci.c
>   create mode 100644 include/hw/virtio/virtio-snd.h
> 

Thank you for your work.

IMHO, all your patches can be merged in only one. Morever it would help for review as some patches 
remove code done in previous patches.

The "v2" tag is missing in the subject of the patches of your series.

And don't send a series as a reply of a previous one.

You can use "git-publish" it helps a lot when we have to send several versions of a series.

Thanks,
Laurent
Re: [RFC PATCH v2 00/25] Virtio Sound card Implementation
Posted by Gerd Hoffmann 2 years, 2 months ago 
  Hi,

> IMHO, all your patches can be merged in only one.

For the most part yes.  I'd keep the pci wrapper (aka -device
virtio-snd-pci) separate though.  Possibly also patches adding
significant functionality in the future (i.e. one patch with all
basics and playback support, one patch adding recording
functionality, ...).

> Morever it would help for
> review as some patches remove code done in previous patches.

Yes, squashing the incremental fixes at the end of the series makes
sense.

take care,
  Gerd
Re: [RFC PATCH v2 00/25] Virtio Sound card Implementation
Posted by Laurent Vivier 2 years, 2 months ago 
Le 14/02/2022 à 11:44, Gerd Hoffmann a écrit :
>    Hi,
> 
>> IMHO, all your patches can be merged in only one.
> 
> For the most part yes.  I'd keep the pci wrapper (aka -device
> virtio-snd-pci) separate though.  Possibly also patches adding
> significant functionality in the future (i.e. one patch with all
> basics and playback support, one patch adding recording
> functionality, ...).
> 

I agree.

Laurent

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.