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

Zhu Ning
2023-03-24T01:54:58Z

> > 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;
+}