ASoC: rt711-sdca: add GE selected mode control #4969

shumingfan · 2024-05-03T04:54:57Z

ASoC: rt711-sdca: add GE selected mode control
This patch adds the 'selected mode' control which overrides the detected_mode.

plbossart

Thanks @shumingfan, I am afraid I don't fully understand the logic behind this patch, see comments below.

plbossart · 2024-05-03T12:33:30Z

sound/soc/codecs/rt711-sdca.c

@@ -258,6 +258,8 @@ static int rt711_sdca_headset_detect(struct rt711_sdca_priv *rt711)
 	switch (det_mode) {
 	case 0x00:
 		rt711->jack_type = 0;
+		regmap_write(rt711->regmap,
+			SDW_SDCA_CTL(FUNC_NUM_JACK_CODEC, RT711_SDCA_ENT_GE49, RT711_SDCA_CTL_SELECTED_MODE, 0), 0);


i am a bit confused by this patch @shumingfan

Shouldn't the code override det_mode and change the value so that the updated selected_mode is written line 276?

I am also confused by the rest of the switch case where the jack_type would be set based on the det_mode, not selected_mode.

plbossart · 2024-05-03T12:36:32Z

sound/soc/codecs/rt711-sdca.c

+		break;
+	}
+
+	return snd_soc_put_enum_double(kcontrol, ucontrol);


still not following. Even if we used a changed in selected_mode as the trigger to update the configuration, the jack_type wouldn't change.

My maybe naive proposal was that the jack detection would be set with the alsa control when the card is created, and whenever an actual jack is detected we would use the value of the alsa control to force the configuration and ignore the detected mode.

@plbossart Could we use the module parameter to force the jack type?
I updated the patch. Please take a look.

plbossart

Thanks @shumingfan. I had the idea of using an alsa control but the module parameter is fine to start testing.

@fredoh9 @marc-hb can you try to see if this patch improve the alsa-bat results with the option
options snd_soc_rt711_sdca detected_mode=5

plbossart · 2024-05-06T13:13:04Z

sound/soc/codecs/rt711-sdca.c

+			det_mode = ge_mode_override;
+			rt711->jack_type =
+				(rt711->jack_type == SND_JACK_HEADPHONE) ? SND_JACK_HEADSET : SND_JACK_HEADPHONE;
+			rt711_sdca_ge_force_jack_type(rt711, det_mode);


optimization: I think you could move this line 297 after the if, if det_mode is zero then using rt711_sdca_ge_force_jack_type(rt711, det_mode); would work just fine.

plbossart · 2024-05-06T13:13:31Z

sound/soc/codecs/rt711-sdca.c

+			rt711_sdca_ge_force_jack_type(rt711, det_mode);
+		}
+		if (!det_mode)
+			rt711_sdca_ge_force_jack_type(rt711, 0);


these two lines can be removed and replaced by rt711_sdca_ge_force_jack_type(rt711, det_mode);

fredoh9 · 2024-05-06T16:47:37Z

@fredoh9 @marc-hb can you try to see if this patch improve the alsa-bat results with the option
options snd_soc_rt711_sdca detected_mode=5

I can quickly test with this today. Will update soon.

fredoh9 · 2024-05-06T18:38:13Z

I was able to test with this PR and the module param.

options snd_soc_rt711_sdca detected_mode=5

Looks detected_mode is consistent throughout multiple tests, however test results have more failure than pass. Captured wave file has very low volume, not even tell whether it's sinewave. Still looking at it. i.e increasing gain. Will update.

Headset Mic Jack also stays ON.

numid=37,iface=CARD,name='Headset Mic Jack'
  ; type=BOOLEAN,access=r-------,values=1
  : values=on

$ grep detected_mode 20240506_dmesg01_pr4969_detected_mode_add_alsabat_tests.txt
[    6.041556] snd_soc_rt711_sdca:rt711_sdca_headset_detect: rt711-sdca sdw:0:0:025d:0711:01: rt711_sdca_headset_detect, detected_mode=0x0
[    6.440514] snd_soc_rt711_sdca:rt711_sdca_headset_detect: rt711-sdca sdw:0:0:025d:0711:01: rt711_sdca_headset_detect, detected_mode=0x5
[   43.739315] snd_soc_rt711_sdca:rt711_sdca_headset_detect: rt711-sdca sdw:0:0:025d:0711:01: rt711_sdca_headset_detect, detected_mode=0x5
[   43.774153] snd_soc_rt711_sdca:rt711_sdca_headset_detect: rt711-sdca sdw:0:0:025d:0711:01: rt711_sdca_headset_detect, detected_mode=0x5
[  177.001368] snd_soc_rt711_sdca:rt711_sdca_headset_detect: rt711-sdca sdw:0:0:025d:0711:01: rt711_sdca_headset_detect, detected_mode=0x5
[  204.427909] snd_soc_rt711_sdca:rt711_sdca_headset_detect: rt711-sdca sdw:0:0:025d:0711:01: rt711_sdca_headset_detect, detected_mode=0x5
[  964.759599] snd_soc_rt711_sdca:rt711_sdca_headset_detect: rt711-sdca sdw:0:0:025d:0711:01: rt711_sdca_headset_detect, detected_mode=0x5

marc-hb · 2024-05-06T19:21:05Z

Headset Mic Jack also stays ON.

From the narrow perspective of this PR, this is a PASS! We can discuss and debug alsabat and other issues elsewhere.

Could you test basic, manual hotplug with a sample of headphones and headsets? evtest is the most convenient for this.

marc-hb · 2024-05-06T21:12:39Z

I had the idea of using an alsa control but the module parameter is fine to start testing.

A module parameter will be less flexible and less convenient for testing automation.

shumingfan · 2024-05-07T04:59:55Z

@marc-hb @plbossart I will update the patch, creating an alsa control to change the jack type.

plbossart

Looks good to me @shumingfan, two questions below:

plbossart · 2024-05-07T13:11:47Z

sound/soc/codecs/rt711-sdca.c

+		return -EINVAL;
+	val = snd_soc_enum_item_to_val(e, item[0]) << e->shift_l;
+	rt711->ge_mode_override = val;
+	return 0;


isn't there something about the return value of the 'put' that informs upper layers that the value was indeed changed?

OK, will fix

sound/soc/codecs/rt711-sdca.c

This patch adds the 'selected mode' control which overrides the detected_mode. Signed-off-by: Shuming Fan <[email protected]>

plbossart

LGTM @shumingfan

Can you send to Mark Brown, maybe with an updated commit message such as:

The SDCA spec defines a 'selected_mode' control which can override the 'detected_mode' reported by hardware. This is useful for platform integration as well as in cases where the hardware is not able to accurately detect the jack type.

fredoh9

I was able test test with LNL RVP, the overriding mode worked fine.

shumingfan · 2024-05-09T02:03:36Z

Can you send to Mark Brown, maybe with an updated commit message such as:

Sure, will send the patch soon.

plbossart · 2024-05-09T15:47:55Z

shared upstream, thanks @shumingfan

marc-hb · 2024-05-10T14:28:04Z

@plbossart don't we need this in this branch already to unblock investigation of other capture issues?

plbossart · 2024-05-10T14:34:06Z

good point @marc-hb. I was planning to pick it up at the next merge, but that merge might be a ways off since we have nothing to send. I'll merge it to topic/sof-dev.

marc-hb · 2024-06-18T17:04:26Z

Not sure this override is enough:

LNL SDW "signal too weak" alsabat capture failure #5076

shumingfan force-pushed the add-ge-select-mode-ctl branch 2 times, most recently from c2bf13e to 19357b8 Compare May 3, 2024 09:01

plbossart reviewed May 3, 2024

View reviewed changes

fredoh9 self-requested a review May 3, 2024 18:26

plbossart mentioned this pull request May 3, 2024

[BUG][LNL] alsabat capture test sometimes fails when headset capture silent -> flaky MIC jack detect thesofproject/sof#9018

Closed

shumingfan force-pushed the add-ge-select-mode-ctl branch from 19357b8 to e9cfe95 Compare May 6, 2024 07:49

shumingfan changed the title ~~ASoC: rt711-sdca: add GE selected mode control~~ ASoC: rt711-sdca: add module parameter to set jack type May 6, 2024

plbossart reviewed May 6, 2024

View reviewed changes

shumingfan force-pushed the add-ge-select-mode-ctl branch from e9cfe95 to c568b60 Compare May 7, 2024 05:03

shumingfan changed the title ~~ASoC: rt711-sdca: add module parameter to set jack type~~ ASoC: rt711-sdca: add GE selected mode control May 7, 2024

shumingfan force-pushed the add-ge-select-mode-ctl branch from c568b60 to ff0b669 Compare May 7, 2024 06:31

plbossart reviewed May 7, 2024

View reviewed changes

marc-hb reviewed May 7, 2024

View reviewed changes

sound/soc/codecs/rt711-sdca.c Show resolved Hide resolved

ASoC: rt711-sdca: add GE selected mode control

b81cf53

This patch adds the 'selected mode' control which overrides the detected_mode. Signed-off-by: Shuming Fan <[email protected]>

shumingfan force-pushed the add-ge-select-mode-ctl branch from ff0b669 to b81cf53 Compare May 8, 2024 02:43

plbossart approved these changes May 8, 2024

View reviewed changes

fredoh9 mentioned this pull request May 8, 2024

LNLM_SDW_AIOC.sh: overrides the headset detected_mode thesofproject/sof-test#1190

Merged

fredoh9 approved these changes May 8, 2024

View reviewed changes

shumingfan closed this May 9, 2024

shumingfan reopened this May 9, 2024

plbossart closed this May 10, 2024

plbossart reopened this May 10, 2024

plbossart merged commit 193bd3d into thesofproject:topic/sof-dev May 10, 2024
28 of 33 checks passed

marc-hb mentioned this pull request May 17, 2024

[BUG] Gen4.1 AIOC rt711 : jack detection and alsa-bat capture fail #4681

Closed

marc-hb mentioned this pull request Jun 18, 2024

alsa_settings: MTLP_RVP_SDW.sh: add rt711 GE control thesofproject/sof-test#1206

Merged

shumingfan deleted the add-ge-select-mode-ctl branch June 20, 2024 03:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ASoC: rt711-sdca: add GE selected mode control #4969

ASoC: rt711-sdca: add GE selected mode control #4969

shumingfan commented May 3, 2024 •

edited

Loading

plbossart left a comment

plbossart May 3, 2024

plbossart May 3, 2024

plbossart May 3, 2024

shumingfan May 6, 2024

plbossart left a comment

plbossart May 6, 2024

plbossart May 6, 2024

fredoh9 commented May 6, 2024

fredoh9 commented May 6, 2024 •

edited

Loading

marc-hb commented May 6, 2024

marc-hb commented May 6, 2024 •

edited

Loading

shumingfan commented May 7, 2024

plbossart left a comment

plbossart May 7, 2024

shumingfan May 8, 2024

plbossart left a comment

fredoh9 left a comment

shumingfan commented May 9, 2024 •

edited

Loading

plbossart commented May 9, 2024

marc-hb commented May 10, 2024

plbossart commented May 10, 2024

marc-hb commented Jun 18, 2024

ASoC: rt711-sdca: add GE selected mode control #4969

ASoC: rt711-sdca: add GE selected mode control #4969

Conversation

shumingfan commented May 3, 2024 • edited Loading

plbossart left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

plbossart left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fredoh9 commented May 6, 2024

fredoh9 commented May 6, 2024 • edited Loading

marc-hb commented May 6, 2024

marc-hb commented May 6, 2024 • edited Loading

shumingfan commented May 7, 2024

plbossart left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

plbossart left a comment

Choose a reason for hiding this comment

fredoh9 left a comment

Choose a reason for hiding this comment

shumingfan commented May 9, 2024 • edited Loading

plbossart commented May 9, 2024

marc-hb commented May 10, 2024

plbossart commented May 10, 2024

marc-hb commented Jun 18, 2024

shumingfan commented May 3, 2024 •

edited

Loading

fredoh9 commented May 6, 2024 •

edited

Loading

marc-hb commented May 6, 2024 •

edited

Loading

shumingfan commented May 9, 2024 •

edited

Loading