Messages in this thread | | | Date | Fri, 8 Apr 2022 23:00:52 +0200 | Subject | Re: [PATCH] staging: bcm2835-audio: remove compat ALSA card | From | Stefan Wahren <> |
| |
Hi Adrien,
[ add Maxime and Takashi ]
Am 08.04.22 um 17:03 schrieb Adrien Thierry: > Remove compat ALSA card, which has overlapping functionality with the > two other cards described by the driver (HDMI and headphones) > > This handles TODO item "Revisit multi-cards options and PCM route mixer > control". > > Move the S/PDIF device that was part of the compat ALSA card to the HDMI > card. > > Only enable headphones card by default, because HDMI breaks when using > both vc4 and bcm2835-audio with HDMI card enabled. > > Signed-off-by: Adrien Thierry <athierry@redhat.com> > --- > > I was able to test PCM output on 3.5mm and HDMI on a Raspberry Pi 4. > However, I couldn't test the HDMI S/PDIF device since I don't own a S/PDIF > capable speaker.
thanks for taking care of this issue. Unfortunately i cannot give a technical review to this patch. So hoping Maxime or Takashi can do ...
Best regards
> > .../vc04_services/bcm2835-audio/bcm2835-ctl.c | 86 ++----------------- > .../vc04_services/bcm2835-audio/bcm2835.c | 33 ++----- > 2 files changed, 15 insertions(+), 104 deletions(-) > > diff --git a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-ctl.c b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-ctl.c > index 3703409715da..1c1f040122d7 100644 > --- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-ctl.c > +++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-ctl.c > @@ -117,15 +117,6 @@ static const struct snd_kcontrol_new snd_bcm2835_ctl[] = { > .get = snd_bcm2835_ctl_get, > .put = snd_bcm2835_ctl_put, > }, > - { > - .iface = SNDRV_CTL_ELEM_IFACE_MIXER, > - .name = "PCM Playback Route", > - .access = SNDRV_CTL_ELEM_ACCESS_READWRITE, > - .private_value = PCM_PLAYBACK_DEVICE, > - .info = snd_bcm2835_ctl_info, > - .get = snd_bcm2835_ctl_get, > - .put = snd_bcm2835_ctl_put, > - }, > }; > > static int snd_bcm2835_spdif_default_info(struct snd_kcontrol *kcontrol, > @@ -220,7 +211,14 @@ static int create_ctls(struct bcm2835_chip *chip, size_t size, > return 0; > } > > -int snd_bcm2835_new_ctl(struct bcm2835_chip *chip) > +int snd_bcm2835_new_headphones_ctl(struct bcm2835_chip *chip) > +{ > + strscpy(chip->card->mixername, "Broadcom Mixer", sizeof(chip->card->mixername)); > + return create_ctls(chip, ARRAY_SIZE(snd_bcm2835_ctl), > + snd_bcm2835_ctl); > +} > + > +int snd_bcm2835_new_hdmi_ctl(struct bcm2835_chip *chip) > { > int err; > > @@ -232,71 +230,3 @@ int snd_bcm2835_new_ctl(struct bcm2835_chip *chip) > snd_bcm2835_spdif); > } > > -static const struct snd_kcontrol_new snd_bcm2835_headphones_ctl[] = { > - { > - .iface = SNDRV_CTL_ELEM_IFACE_MIXER, > - .name = "Headphone Playback Volume", > - .index = 0, > - .access = SNDRV_CTL_ELEM_ACCESS_READWRITE | > - SNDRV_CTL_ELEM_ACCESS_TLV_READ, > - .private_value = PCM_PLAYBACK_VOLUME, > - .info = snd_bcm2835_ctl_info, > - .get = snd_bcm2835_ctl_get, > - .put = snd_bcm2835_ctl_put, > - .count = 1, > - .tlv = {.p = snd_bcm2835_db_scale} > - }, > - { > - .iface = SNDRV_CTL_ELEM_IFACE_MIXER, > - .name = "Headphone Playback Switch", > - .index = 0, > - .access = SNDRV_CTL_ELEM_ACCESS_READWRITE, > - .private_value = PCM_PLAYBACK_MUTE, > - .info = snd_bcm2835_ctl_info, > - .get = snd_bcm2835_ctl_get, > - .put = snd_bcm2835_ctl_put, > - .count = 1, > - } > -}; > - > -int snd_bcm2835_new_headphones_ctl(struct bcm2835_chip *chip) > -{ > - strscpy(chip->card->mixername, "Broadcom Mixer", sizeof(chip->card->mixername)); > - return create_ctls(chip, ARRAY_SIZE(snd_bcm2835_headphones_ctl), > - snd_bcm2835_headphones_ctl); > -} > - > -static const struct snd_kcontrol_new snd_bcm2835_hdmi[] = { > - { > - .iface = SNDRV_CTL_ELEM_IFACE_MIXER, > - .name = "HDMI Playback Volume", > - .index = 0, > - .access = SNDRV_CTL_ELEM_ACCESS_READWRITE | > - SNDRV_CTL_ELEM_ACCESS_TLV_READ, > - .private_value = PCM_PLAYBACK_VOLUME, > - .info = snd_bcm2835_ctl_info, > - .get = snd_bcm2835_ctl_get, > - .put = snd_bcm2835_ctl_put, > - .count = 1, > - .tlv = {.p = snd_bcm2835_db_scale} > - }, > - { > - .iface = SNDRV_CTL_ELEM_IFACE_MIXER, > - .name = "HDMI Playback Switch", > - .index = 0, > - .access = SNDRV_CTL_ELEM_ACCESS_READWRITE, > - .private_value = PCM_PLAYBACK_MUTE, > - .info = snd_bcm2835_ctl_info, > - .get = snd_bcm2835_ctl_get, > - .put = snd_bcm2835_ctl_put, > - .count = 1, > - } > -}; > - > -int snd_bcm2835_new_hdmi_ctl(struct bcm2835_chip *chip) > -{ > - strscpy(chip->card->mixername, "Broadcom Mixer", sizeof(chip->card->mixername)); > - return create_ctls(chip, ARRAY_SIZE(snd_bcm2835_hdmi), > - snd_bcm2835_hdmi); > -} > - > diff --git a/drivers/staging/vc04_services/bcm2835-audio/bcm2835.c b/drivers/staging/vc04_services/bcm2835-audio/bcm2835.c > index 628732d7bf6a..00bc898b0189 100644 > --- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835.c > +++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835.c > @@ -10,17 +10,13 @@ > #include "bcm2835.h" > > static bool enable_hdmi; > -static bool enable_headphones; > -static bool enable_compat_alsa = true; > +static bool enable_headphones = true; > static int num_channels = MAX_SUBSTREAMS; > > module_param(enable_hdmi, bool, 0444); > MODULE_PARM_DESC(enable_hdmi, "Enables HDMI virtual audio device"); > module_param(enable_headphones, bool, 0444); > MODULE_PARM_DESC(enable_headphones, "Enables Headphones virtual audio device"); > -module_param(enable_compat_alsa, bool, 0444); > -MODULE_PARM_DESC(enable_compat_alsa, > - "Enables ALSA compatibility virtual audio device"); > module_param(num_channels, int, 0644); > MODULE_PARM_DESC(num_channels, "Number of audio channels (default: 8)"); > > @@ -63,19 +59,20 @@ struct bcm2835_audio_driver { > enum snd_bcm2835_route route; > }; > > -static int bcm2835_audio_alsa_newpcm(struct bcm2835_chip *chip, > +static int bcm2835_audio_dual_newpcm(struct bcm2835_chip *chip, > const char *name, > enum snd_bcm2835_route route, > u32 numchannels) > { > int err; > > - err = snd_bcm2835_new_pcm(chip, "bcm2835 ALSA", 0, AUDIO_DEST_AUTO, > - numchannels - 1, false); > + err = snd_bcm2835_new_pcm(chip, name, 0, route, > + numchannels, false); > + > if (err) > return err; > > - err = snd_bcm2835_new_pcm(chip, "bcm2835 IEC958/HDMI", 1, 0, 1, true); > + err = snd_bcm2835_new_pcm(chip, "IEC958", 1, route, 1, true); > if (err) > return err; > > @@ -90,18 +87,6 @@ static int bcm2835_audio_simple_newpcm(struct bcm2835_chip *chip, > return snd_bcm2835_new_pcm(chip, name, 0, route, numchannels, false); > } > > -static struct bcm2835_audio_driver bcm2835_audio_alsa = { > - .driver = { > - .name = "bcm2835_alsa", > - .owner = THIS_MODULE, > - }, > - .shortname = "bcm2835 ALSA", > - .longname = "bcm2835 ALSA", > - .minchannels = 2, > - .newpcm = bcm2835_audio_alsa_newpcm, > - .newctl = snd_bcm2835_new_ctl, > -}; > - > static struct bcm2835_audio_driver bcm2835_audio_hdmi = { > .driver = { > .name = "bcm2835_hdmi", > @@ -110,7 +95,7 @@ static struct bcm2835_audio_driver bcm2835_audio_hdmi = { > .shortname = "bcm2835 HDMI", > .longname = "bcm2835 HDMI", > .minchannels = 1, > - .newpcm = bcm2835_audio_simple_newpcm, > + .newpcm = bcm2835_audio_dual_newpcm, > .newctl = snd_bcm2835_new_hdmi_ctl, > .route = AUDIO_DEST_HDMI > }; > @@ -134,10 +119,6 @@ struct bcm2835_audio_drivers { > }; > > static struct bcm2835_audio_drivers children_devices[] = { > - { > - .audio_driver = &bcm2835_audio_alsa, > - .is_enabled = &enable_compat_alsa, > - }, > { > .audio_driver = &bcm2835_audio_hdmi, > .is_enabled = &enable_hdmi, > > base-commit: 1831fed559732b132aef0ea8261ac77e73f7eadf
| |