Re: [PATCH 04/27] ASoC: amd: acp3x-rt5682-max9836: Map missing jack kcontrols

[Akihiko Odaki <akihiko.odaki@gmail.com>]

On 2023/08/03 2:57, Alper Nebi Yasak wrote:
> This driver does not properly map jack pins to kcontrols that PulseAudio
> and PipeWire need to handle jack detection events. The RT5682, RT1015
> and RT1015p codecs used here can detect Headphone and Headset Mic
> connections. Expose the former two to userspace as kcontrols.
> 
> Signed-off-by: Alper Nebi Yasak <alpernebiyasak@gmail.com>
> ---
> Should the SND_JACK_LINEOUT be removed from the jack_new_pins calls?

I had patches to remove SND_JACK_LINEOUT and to add separate kcontrols, 
but I forgot them.

Mark Brown has a raised question if the jack is really not driven as a 
line output[1], but I never answered him (sorry for that!) I don't know 
if the hardware is capable, but I think we better to remove it for now 
since the codec driver does not handle it.

Mark, does it sound reasonable?

Regards,
Akihiko Odaki

[1] https://www.spinics.net/lists/alsa-devel/msg147970.html

> 
>   sound/soc/amd/acp3x-rt5682-max9836.c | 23 ++++++++++++++++++-----
>   1 file changed, 18 insertions(+), 5 deletions(-)
> 
> diff --git a/sound/soc/amd/acp3x-rt5682-max9836.c b/sound/soc/amd/acp3x-rt5682-max9836.c
> index 0543dda75b99..3bf7e1e83c36 100644
> --- a/sound/soc/amd/acp3x-rt5682-max9836.c
> +++ b/sound/soc/amd/acp3x-rt5682-max9836.c
> @@ -28,6 +28,17 @@
>   #define DUAL_CHANNEL		2
>   
>   static struct snd_soc_jack pco_jack;
> +static struct snd_soc_jack_pin pco_jack_pins[] = {
> +	{
> +		.pin = "Headphone Jack",
> +		.mask = SND_JACK_HEADPHONE,
> +	},
> +	{
> +		.pin = "Headset Mic",
> +		.mask = SND_JACK_MICROPHONE,
> +	},
> +};
> +
>   static struct clk *rt5682_dai_wclk;
>   static struct clk *rt5682_dai_bclk;
>   static struct gpio_desc *dmic_sel;
> @@ -86,11 +97,13 @@ static int acp3x_5682_init(struct snd_soc_pcm_runtime *rtd)
>   	rt5682_dai_wclk = clk_get(component->dev, "rt5682-dai-wclk");
>   	rt5682_dai_bclk = clk_get(component->dev, "rt5682-dai-bclk");
>   
> -	ret = snd_soc_card_jack_new(card, "Headset Jack",
> -				SND_JACK_HEADSET | SND_JACK_LINEOUT |
> -				SND_JACK_BTN_0 | SND_JACK_BTN_1 |
> -				SND_JACK_BTN_2 | SND_JACK_BTN_3,
> -				&pco_jack);
> +	ret = snd_soc_card_jack_new_pins(card, "Headset Jack",
> +					 SND_JACK_HEADSET | SND_JACK_LINEOUT |
> +					 SND_JACK_BTN_0 | SND_JACK_BTN_1 |
> +					 SND_JACK_BTN_2 | SND_JACK_BTN_3,
> +					 &pco_jack,
> +					 pco_jack_pins,
> +					 ARRAY_SIZE(pco_jack_pins));
>   	if (ret) {
>   		dev_err(card->dev, "HP jack creation failed %d\n", ret);
>   		return ret;