linux-kernel - Re: [PATCH 3/4] ASoC: amd: acp: Add machine driver that enables sound for systems with a ES8336 codec

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <CANPLYpBv82p-Fve-JgM=WDwUFp-VnKfG2yFvcGv8AT2roPEifw@mail.gmail.com>
Date:   Fri, 24 Mar 2023 09:54:41 +0800
From:   Zhu Ning <zhuning0077@...il.com>
To:     broonie@...nel.org
Cc:     alsa-devel@...a-project.org, lgirdwood@...il.com,
        linux-kernel@...r.kernel.org, perex@...ex.cz, posteuca@...ex.one,
        tiwai@...e.com, yangxiaohua <yangxiaohua@...rest-semi.com>,
        Ning Zhu <zhuning0077@...il.com>,
        Zhu Ning <zhuning@...rest-semi.com>
Subject: Re: [PATCH 3/4] ASoC: amd: acp: Add machine driver that enables sound
 for systems with a ES8336 codec

> > The GPIO thing on Huawei Intel platform is too complicated, they use two GPIOs
> > for headphone and speaker, and the headphone GPIO is inverted, which means low
> > means on and high means off. Luckily there should be only one hardware config
> > for the AMD acp3x platform GPIO and you may just choose the correct one.
>
> There being two different GPIOs sounds like it just allows the headphone
> and speaker to be controlled separately - that seems more flexible, not
> a problem?

Yes it's called multi stream in Windows. However, extra GPIO causes
more confusion
in the driver.

> > There is no such situation, and the system doesn't produce sound from speaker
> > when headphones are plugged in. The user may manually open speaker using
> > amixer sset 'Speaker' on or pavucontrol.
>
> Again, you're describing a specific configuration - someone might want
> to do something different.

Hi Marian Postevca you may want to separate the GPIO control by adding Headphone
Power SND_SOC_DAPM_SUPPLY. You may also want to change the gpio
handling function
in the acp3x_es83xx_jack_events function.

static const struct snd_soc_dapm_widget acp3x_es83xx_widgets[] = {
        SND_SOC_DAPM_SPK("Speaker", NULL),
        SND_SOC_DAPM_HP("Headphone", NULL),
        SND_SOC_DAPM_MIC("Headset Mic", NULL),
        SND_SOC_DAPM_MIC("Internal Mic", NULL),

+       SND_SOC_DAPM_SUPPLY("Headphone Power", SND_SOC_NOPM, 0, 0,
+                           acp3x_es83xx_headphone_power_event,
+                           SND_SOC_DAPM_PRE_PMD | SND_SOC_DAPM_POST_PMU),
        SND_SOC_DAPM_SUPPLY("Speaker Power", SND_SOC_NOPM, 0, 0,
                            acp3x_es83xx_speaker_power_event,
                            SND_SOC_DAPM_PRE_PMD | SND_SOC_DAPM_POST_PMU),
};

static const struct snd_soc_dapm_route acp3x_es83xx_audio_map[] = {
        {"Headphone", NULL, "HPOL"},
        {"Headphone", NULL, "HPOR"},
+       {"Headphone", NULL, "Headphone Power"},

        {"Speaker", NULL, "HPOL"},
        {"Speaker", NULL, "HPOR"},
        {"Speaker", NULL, "Speaker Power"},
};

+static int acp3x_es83xx_headphone_power_event(struct snd_soc_dapm_widget *w,
+                                           struct snd_kcontrol
*kcontrol, int event)
+{
+       struct acp3x_es83xx_private *priv = get_mach_priv(w->dapm->card);
+
+       dev_dbg(priv->codec_dev, "speaker power event: %d\n", event);
+       if (SND_SOC_DAPM_EVENT_ON(event))
+               gpiod_set_value_cansleep(priv->gpio_headphone, true);
+       else
+               gpiod_set_value_cansleep(priv->gpio_headphone, false);
+
+       return 0;
+}
+
+static int acp3x_es83xx_speaker_power_event(struct snd_soc_dapm_widget *w,
+                                           struct snd_kcontrol
*kcontrol, int event)
+{
+       struct acp3x_es83xx_private *priv = get_mach_priv(w->dapm->card);
+
+       dev_dbg(priv->codec_dev, "speaker power event: %d\n", event);
+       if (SND_SOC_DAPM_EVENT_ON(event))
+               gpiod_set_value_cansleep(priv->gpio_speakers, true);
+       else
+               gpiod_set_value_cansleep(priv->gpio_speakers, false);
+
+       return 0;
+}