[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <69de07fb-efdb-5296-f48b-e3af2ac5ef49@i2se.com>
Date: Fri, 8 Apr 2022 23:00:52 +0200
From: Stefan Wahren <stefan.wahren@...e.com>
To: Adrien Thierry <athierry@...hat.com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Florian Fainelli <f.fainelli@...il.com>,
Ray Jui <rjui@...adcom.com>,
Scott Branden <sbranden@...adcom.com>,
bcm-kernel-feedback-list@...adcom.com,
Nicolas Saenz Julienne <nsaenz@...nel.org>,
Takashi Iwai <tiwai@...e.com>,
Maxime Ripard <mripard@...nel.org>
Cc: linux-staging@...ts.linux.dev,
linux-rpi-kernel@...ts.infradead.org,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] staging: bcm2835-audio: remove compat ALSA card
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@...hat.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
Powered by blists - more mailing lists