diff mbox series

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

Message ID 20230802175737.263412-5-alpernebiyasak@gmail.com (mailing list archive)
State Accepted
Commit 00c7ee820895c9563b7780f84f1b9420cbfbe6c7
Headers show
Series ASoC: Map missing jack kcontrols | expand

Commit Message

Alper Nebi Yasak Aug. 2, 2023, 5:57 p.m. UTC
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?

 sound/soc/amd/acp3x-rt5682-max9836.c | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

Comments

Akihiko Odaki Aug. 4, 2023, 8:04 a.m. UTC | #1
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;
Mark Brown Aug. 4, 2023, 1:11 p.m. UTC | #2
On Fri, Aug 04, 2023 at 05:04:52PM +0900, Akihiko Odaki wrote:

> 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?

Sure.
Akihiko Odaki Aug. 4, 2023, 1:19 p.m. UTC | #3
On 2023/08/04 22:11, Mark Brown wrote:
> On Fri, Aug 04, 2023 at 05:04:52PM +0900, Akihiko Odaki wrote:
> 
>> 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?
> 
> Sure.

I see. Alper, can you add a patch to remove SND_JACK_LINEOUT to your series?

Regards,
Akihiko Odaki
Mark Brown Aug. 4, 2023, 1:22 p.m. UTC | #4
On Fri, Aug 04, 2023 at 10:19:43PM +0900, Akihiko Odaki wrote:

> I see. Alper, can you add a patch to remove SND_JACK_LINEOUT to your series?

Please send an incremental patch on top of the series rather than
resending the full thing given how big the series is.
Akihiko Odaki Aug. 8, 2023, 7:47 a.m. UTC | #5
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>

Reviewed-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Akihiko Odaki Aug. 8, 2023, 7:49 a.m. UTC | #6
On 2023/08/08 16:47, Akihiko Odaki wrote:
> 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>
> 
> Reviewed-by: Akihiko Odaki <akihiko.odaki@daynix.com>

Tested-by: Akihiko Odaki <akihiko.odaki@daynix.com>
diff mbox series

Patch

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;