Re: [PATCH v2 8/9] ALSA: virtio: introduce PCM channel map support

On Sun, 24 Jan 2021, Anton Yakovlev wrote:

Enumerate all available PCM channel maps and create ALSA controls.

Signed-off-by: Anton Yakovlev anton.yakovlev@opensynergy.com

sound/virtio/Makefile | 1 + sound/virtio/virtio_card.c | 15 +++ sound/virtio/virtio_card.h | 8 ++ sound/virtio/virtio_chmap.c | 237 ++++++++++++++++++++++++++++++++++++ sound/virtio/virtio_pcm.h | 4 + 5 files changed, 265 insertions(+) create mode 100644 sound/virtio/virtio_chmap.c

[snip]

diff --git a/sound/virtio/virtio_chmap.c b/sound/virtio/virtio_chmap.c new file mode 100644 index 000000000000..8a2ddc4dcffb --- /dev/null +++ b/sound/virtio/virtio_chmap.c @@ -0,0 +1,237 @@

[snip]

+/**

• • virtsnd_chmap_parse_cfg() - Parse the channel map configuration.
• • @snd: VirtIO sound device.
• • This function is called during initial device initialization.
• • Context: Any context that permits to sleep.
• • Return: 0 on success, -errno on failure.
• */

+int virtsnd_chmap_parse_cfg(struct virtio_snd *snd) +{

• struct virtio_device *vdev = snd->vdev;
• unsigned int i;
• int rc;
• virtio_cread(vdev, struct virtio_snd_config, chmaps, &snd->nchmaps);
• if (!snd->nchmaps)

• return 0;
  

• snd->chmaps = devm_kcalloc(&vdev->dev, snd->nchmaps,

• 		   sizeof(*snd->chmaps), GFP_KERNEL);
  

• if (!snd->chmaps)

• return -ENOMEM;
  

• rc = virtsnd_ctl_query_info(snd, VIRTIO_SND_R_CHMAP_INFO, 0,

• 		    snd->nchmaps, sizeof(*snd->chmaps),
  

• 		    snd->chmaps);
  

• if (rc)

• return rc;
  

• /* Count the number of channel maps per each PCM device/stream. */
• for (i = 0; i < snd->nchmaps; ++i) {

• struct virtio_snd_chmap_info *info = &snd->chmaps[i];
  

• unsigned int nid = le32_to_cpu(info->hdr.hda_fn_nid);
  

• struct virtio_pcm *pcm;
  

• struct virtio_pcm_stream *stream;
  

• pcm = virtsnd_pcm_find_or_create(snd, nid);
  

• if (IS_ERR(pcm))
  

• 	return PTR_ERR(pcm);
  

• switch (info->direction) {
  

• case VIRTIO_SND_D_OUTPUT: {
  

• 	stream = &pcm->streams[SNDRV_PCM_STREAM_PLAYBACK];
  

• 	break;
  

• }
  

• case VIRTIO_SND_D_INPUT: {
  

• 	stream = &pcm->streams[SNDRV_PCM_STREAM_CAPTURE];
  

• 	break;
  

• }
  

• default: {
  

• 	dev_err(&vdev->dev,
  

• 		"chmap #%u: unknown direction (%u)\n", i,
  

• 		info->direction);
  

• 	return -EINVAL;
  

• }
  

• }
  

• stream->nchmaps++;
  

• }
• return 0;

+}

+/**

• • virtsnd_chmap_add_ctls() - Create an ALSA control for channel maps.
• • @pcm: ALSA PCM device.
• • @direction: PCM stream direction (SNDRV_PCM_STREAM_XXX).
• • @stream: VirtIO PCM stream.
• • Context: Any context.
• • Return: 0 on success, -errno on failure.
• */

+static int virtsnd_chmap_add_ctls(struct snd_pcm *pcm, int direction,

• 		  struct virtio_pcm_stream *stream)
  

+{

• unsigned int i;
• int max_channels = 0;
• for (i = 0; i < stream->nchmaps; i++)

• if (max_channels < stream->chmaps[i].channels)
  

• 	max_channels = stream->chmaps[i].channels;
  

• return snd_pcm_add_chmap_ctls(pcm, direction, stream->chmaps,

• 		      max_channels, 0, NULL);
  

+}

+/**

• • virtsnd_chmap_build_devs() - Build ALSA controls for channel maps.
• • @snd: VirtIO sound device.
• • Context: Any context.
• • Return: 0 on success, -errno on failure.
• */

+int virtsnd_chmap_build_devs(struct virtio_snd *snd) +{

• struct virtio_device *vdev = snd->vdev;
• struct virtio_pcm *pcm;
• struct virtio_pcm_stream *stream;
• unsigned int i;
• int rc;
• /* Allocate channel map elements per each PCM device/stream. */
• list_for_each_entry(pcm, &snd->pcm_list, list) {

• for (i = 0; i < ARRAY_SIZE(pcm->streams); ++i) {
  

• 	stream = &pcm->streams[i];
  

• 	if (!stream->nchmaps)
  

• 		continue;
  

• 	stream->chmaps = devm_kcalloc(&vdev->dev,
  

• 				      stream->nchmaps + 1,
  

• 				      sizeof(*stream->chmaps),
  

• 				      GFP_KERNEL);
  

• 	if (!stream->chmaps)
  

• 		return -ENOMEM;
  

• 	stream->nchmaps = 0;
  

• }
  

• }
• /* Initialize channel maps per each PCM device/stream. */
• for (i = 0; i < snd->nchmaps; ++i) {

• struct virtio_snd_chmap_info *info = &snd->chmaps[i];
  

• unsigned int nid = le32_to_cpu(info->hdr.hda_fn_nid);
  

• unsigned int channels = info->channels;
  

• unsigned int ch;
  

• struct snd_pcm_chmap_elem *chmap;
  

• pcm = virtsnd_pcm_find(snd, nid);
  

• if (IS_ERR(pcm))
  

• 	return PTR_ERR(pcm);
  

• if (info->direction == VIRTIO_SND_D_OUTPUT)
  

• 	stream = &pcm->streams[SNDRV_PCM_STREAM_PLAYBACK];
  

• else
  

• 	stream = &pcm->streams[SNDRV_PCM_STREAM_CAPTURE];
  

• chmap = &stream->chmaps[stream->nchmaps++];
  

• if (channels > ARRAY_SIZE(chmap->map))
  

• 	channels = ARRAY_SIZE(chmap->map);
  

• chmap->channels = channels;
  

• for (ch = 0; ch < channels; ++ch) {
  

• 	u8 position = info->positions[ch];
  

• 	if (position >= ARRAY_SIZE(g_v2a_position_map))
  

• 		return -EINVAL;
  

• 	chmap->map[ch] = g_v2a_position_map[position];
  

• }
  

• }

You enter this function after virtsnd_chmap_parse_cfg() has run. And virtsnd_chmap_parse_cfg() has already found or created all the PCMs and counted channel maps - the same way as you do in the above loop. Wouldn't it be enough to reuse the result of that counting and avoid re-counting here?

• /* Create an ALSA control per each PCM device/stream. */
• list_for_each_entry(pcm, &snd->pcm_list, list) {

• if (!pcm->pcm)
  

• 	continue;
  

• for (i = 0; i < ARRAY_SIZE(pcm->streams); ++i) {
  

• 	stream = &pcm->streams[i];
  

• 	if (!stream->nchmaps)
  

• 		continue;
  

• 	rc = virtsnd_chmap_add_ctls(pcm->pcm, i, stream);
  

• 	if (rc)
  

• 		return rc;
  

• }
  

• }
• return 0;

+}