[alsa-devel] [PATCH 0/6] ALSA: hda/ca0132: Various cleanups and fixes
The ca0132 codec support is fairly crufty, and the new R3Di and SBZ support hasn't exactly improved that situation. This patch series tries to clean up a small amount of longstanding cruft, reverse some behavioral changes to other systems made by the recent R3Di/SBZ patches, and make the DMic work properly on the Alienware M17x R4.
For two of these patches (the QUIRK_ALIENWARE patch and the DMic patch), it may be that the correct behavior is the exact OPPOSITE of what the patch does as far as quirk testing goes (that is, it should apply globally, rather than limited to some specific quirk or set of quirks), but I have no way to make such a determination at this time, so erring on the side of the status quo ante seems appropriate.
Alastair Bridgewater (6): ALSA: hda/ca0132: Delete pointless assignments to struct auto_pin_cfg fields ALSA: hda/ca0132: Delete redundant UNSOL event requests ALSA: hda/ca0132: Restore behavior of QUIRK_ALIENWARE ALSA: hda/ca0132: Don't test for QUIRK_NONE ALSA: hda/ca0132: Restore PCM Analog Mic-In2 ALSA: hda/ca0132: Fix DMic data rate for Alienware M17x R4
sound/pci/hda/patch_ca0132.c | 62 ++++++++++++++++---------------------------- 1 file changed, 22 insertions(+), 40 deletions(-)
ca0132_config() was setting some values in the auto_pin_cfg for the codec... but it is called prior to snd_hda_parse_pin_defcfg(), which does a memset() to clear the entire structure as one of its first actions, making the entire exercise pointless.
Kill all use of struct auto_pin_cfg from ca0132_config().
Signed-off-by: Alastair Bridgewater alastair.bridgewater@gmail.com
diff --git a/sound/pci/hda/patch_ca0132.c b/sound/pci/hda/patch_ca0132.c index 292e2c592c17..e03461aab68c 100644 --- a/sound/pci/hda/patch_ca0132.c +++ b/sound/pci/hda/patch_ca0132.c @@ -7343,7 +7343,6 @@ static const struct hda_codec_ops ca0132_patch_ops = { static void ca0132_config(struct hda_codec *codec) { struct ca0132_spec *spec = codec->spec; - struct auto_pin_cfg *cfg = &spec->autocfg;
spec->dacs[0] = 0x2; spec->dacs[1] = 0x3; @@ -7405,12 +7404,7 @@ static void ca0132_config(struct hda_codec *codec) /* SPDIF I/O */ spec->dig_out = 0x05; spec->multiout.dig_out_nid = spec->dig_out; - cfg->dig_out_pins[0] = 0x0c; - cfg->dig_outs = 1; - cfg->dig_out_type[0] = HDA_PCM_TYPE_SPDIF; spec->dig_in = 0x09; - cfg->dig_in_pin = 0x0e; - cfg->dig_in_type = HDA_PCM_TYPE_SPDIF; break; case QUIRK_R3DI: codec_dbg(codec, "%s: QUIRK_R3DI applied.\n", __func__); @@ -7438,9 +7432,6 @@ static void ca0132_config(struct hda_codec *codec) /* SPDIF I/O */ spec->dig_out = 0x05; spec->multiout.dig_out_nid = spec->dig_out; - cfg->dig_out_pins[0] = 0x0c; - cfg->dig_outs = 1; - cfg->dig_out_type[0] = HDA_PCM_TYPE_SPDIF; break; default: spec->num_outputs = 2; @@ -7463,12 +7454,7 @@ static void ca0132_config(struct hda_codec *codec) /* SPDIF I/O */ spec->dig_out = 0x05; spec->multiout.dig_out_nid = spec->dig_out; - cfg->dig_out_pins[0] = 0x0c; - cfg->dig_outs = 1; - cfg->dig_out_type[0] = HDA_PCM_TYPE_SPDIF; spec->dig_in = 0x09; - cfg->dig_in_pin = 0x0e; - cfg->dig_in_type = HDA_PCM_TYPE_SPDIF; break; } }
During ca0132_init(), ca0132_init_unsol() is run before the spec->spec_init_verbs are written. ca0132_init_unsol() calls snd_hda_jack_detect_enable_callback(), which requests UNSOL events for three or four nodes, two of which were also (redundantly) requested by spec_init_verbs.
Kill the redundant AC_VERB_SET_UNSOLICITED_ENABLE verbs.
Signed-off-by: Alastair Bridgewater alastair.bridgewater@gmail.com
diff --git a/sound/pci/hda/patch_ca0132.c b/sound/pci/hda/patch_ca0132.c index e03461aab68c..bdd626bde267 100644 --- a/sound/pci/hda/patch_ca0132.c +++ b/sound/pci/hda/patch_ca0132.c @@ -7462,7 +7462,7 @@ static void ca0132_config(struct hda_codec *codec) static int ca0132_prepare_verbs(struct hda_codec *codec) { /* Verbs + terminator (an empty element) */ -#define NUM_SPEC_VERBS 4 +#define NUM_SPEC_VERBS 2 struct ca0132_spec *spec = codec->spec;
spec->chip_init_verbs = ca0132_init_verbs0; @@ -7472,34 +7472,24 @@ static int ca0132_prepare_verbs(struct hda_codec *codec) if (!spec->spec_init_verbs) return -ENOMEM;
- /* HP jack autodetection */ - spec->spec_init_verbs[0].nid = spec->unsol_tag_hp; - spec->spec_init_verbs[0].param = AC_VERB_SET_UNSOLICITED_ENABLE; - spec->spec_init_verbs[0].verb = AC_USRSP_EN | spec->unsol_tag_hp; - - /* MIC1 jack autodetection */ - spec->spec_init_verbs[1].nid = spec->unsol_tag_amic1; - spec->spec_init_verbs[1].param = AC_VERB_SET_UNSOLICITED_ENABLE; - spec->spec_init_verbs[1].verb = AC_USRSP_EN | spec->unsol_tag_amic1; - /* config EAPD */ - spec->spec_init_verbs[2].nid = 0x0b; - spec->spec_init_verbs[2].param = 0x78D; - spec->spec_init_verbs[2].verb = 0x00; + spec->spec_init_verbs[0].nid = 0x0b; + spec->spec_init_verbs[0].param = 0x78D; + spec->spec_init_verbs[0].verb = 0x00;
/* Previously commented configuration */ /* - spec->spec_init_verbs[3].nid = 0x0b; - spec->spec_init_verbs[3].param = AC_VERB_SET_EAPD_BTLENABLE; + spec->spec_init_verbs[2].nid = 0x0b; + spec->spec_init_verbs[2].param = AC_VERB_SET_EAPD_BTLENABLE; + spec->spec_init_verbs[2].verb = 0x02; + + spec->spec_init_verbs[3].nid = 0x10; + spec->spec_init_verbs[3].param = 0x78D; spec->spec_init_verbs[3].verb = 0x02;
spec->spec_init_verbs[4].nid = 0x10; - spec->spec_init_verbs[4].param = 0x78D; + spec->spec_init_verbs[4].param = AC_VERB_SET_EAPD_BTLENABLE; spec->spec_init_verbs[4].verb = 0x02; - - spec->spec_init_verbs[5].nid = 0x10; - spec->spec_init_verbs[5].param = AC_VERB_SET_EAPD_BTLENABLE; - spec->spec_init_verbs[5].verb = 0x02; */
/* Terminator: spec->spec_init_verbs[NUM_SPEC_VERBS-1] */
Commit e93ac30a32a6ba7ac3b4b2a4379af1dadb91e505 (ALSA: HDA/ca0132: add extra init functions for r3di + sbz) introduced an extra initialization function that was improperly guarded, taking effect on systems with QUIRK_ALIENWARE, even though such systems were supposedly not affected.
It may be that this piece of initialization should be done for all systems, but that's not a call that I can make.
Signed-off-by: Alastair Bridgewater alastair.bridgewater@gmail.com
diff --git a/sound/pci/hda/patch_ca0132.c b/sound/pci/hda/patch_ca0132.c index bdd626bde267..ca7375e48138 100644 --- a/sound/pci/hda/patch_ca0132.c +++ b/sound/pci/hda/patch_ca0132.c @@ -7223,7 +7223,7 @@ static int ca0132_init(struct hda_codec *codec)
snd_hda_sequence_write(codec, spec->base_init_verbs);
- if (spec->quirk != QUIRK_NONE) + if (spec->use_alt_functions) ca0132_alt_init(codec);
ca0132_download_dsp(codec);
I agree with this one too, must have been one I missed. I added use_alt_functions explicitly to get away from using spec->quirk != QUIRK_NONE.
On Fri, Jun 15, 2018 at 9:56 PM, Alastair Bridgewater alastair.bridgewater@gmail.com wrote:
Commit e93ac30a32a6ba7ac3b4b2a4379af1dadb91e505 (ALSA: HDA/ca0132: add extra init functions for r3di + sbz) introduced an extra initialization function that was improperly guarded, taking effect on systems with QUIRK_ALIENWARE, even though such systems were supposedly not affected.
It may be that this piece of initialization should be done for all systems, but that's not a call that I can make.
Signed-off-by: Alastair Bridgewater alastair.bridgewater@gmail.com
diff --git a/sound/pci/hda/patch_ca0132.c b/sound/pci/hda/patch_ca0132.c index bdd626bde267..ca7375e48138 100644 --- a/sound/pci/hda/patch_ca0132.c +++ b/sound/pci/hda/patch_ca0132.c @@ -7223,7 +7223,7 @@ static int ca0132_init(struct hda_codec *codec)
snd_hda_sequence_write(codec, spec->base_init_verbs);
if (spec->quirk != QUIRK_NONE)
if (spec->use_alt_functions) ca0132_alt_init(codec); ca0132_download_dsp(codec);-- 2.16.1
Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
Going through the code again today, I realized why I did this. I don't believe the Chromebook will ever go into Windows, so I don't think I have to set the 0x797 and 0x798 audio volume verbs to 0x00 on it. I did this so that the Alienware laptops, which probably have booted into Windows, would have those values set to 0 if they hadn't been already.
On Fri, Jun 15, 2018 at 9:56 PM, Alastair Bridgewater alastair.bridgewater@gmail.com wrote:
Commit e93ac30a32a6ba7ac3b4b2a4379af1dadb91e505 (ALSA: HDA/ca0132: add extra init functions for r3di + sbz) introduced an extra initialization function that was improperly guarded, taking effect on systems with QUIRK_ALIENWARE, even though such systems were supposedly not affected.
It may be that this piece of initialization should be done for all systems, but that's not a call that I can make.
Signed-off-by: Alastair Bridgewater alastair.bridgewater@gmail.com
diff --git a/sound/pci/hda/patch_ca0132.c b/sound/pci/hda/patch_ca0132.c index bdd626bde267..ca7375e48138 100644 --- a/sound/pci/hda/patch_ca0132.c +++ b/sound/pci/hda/patch_ca0132.c @@ -7223,7 +7223,7 @@ static int ca0132_init(struct hda_codec *codec)
snd_hda_sequence_write(codec, spec->base_init_verbs);
if (spec->quirk != QUIRK_NONE)
if (spec->use_alt_functions) ca0132_alt_init(codec); ca0132_download_dsp(codec);-- 2.16.1
Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
That makes a certain amount of sense, but there are a few things that would invalidate that reasoning. First, the contrived scenario: Running Linux on the Chromebook with the sound card routed to a Qemu VM running Some Other Operating System. Second, there's no quirk matching for the Chromebook, which means that every unknown card (and these unknown cards would presumably have the opportunity to be dual-boot windows devices) would be denied the benefit of having the extra volume bits cleared. Third, if Linux starts to use those extra bits, and someone is dual-booting between kernel versions, we get the same effect.
At this point, unless there is reason to expect that clearing these volume parameters could be harmful, I think that doing so globally is the right thing. I can prep a patch for that in a day or so.
On Sat, Jun 16, 2018 at 11:37 AM, Connor McAdams conmanx360@gmail.com wrote:
Going through the code again today, I realized why I did this. I don't believe the Chromebook will ever go into Windows, so I don't think I have to set the 0x797 and 0x798 audio volume verbs to 0x00 on it. I did this so that the Alienware laptops, which probably have booted into Windows, would have those values set to 0 if they hadn't been already.
On Fri, Jun 15, 2018 at 9:56 PM, Alastair Bridgewater alastair.bridgewater@gmail.com wrote:
Commit e93ac30a32a6ba7ac3b4b2a4379af1dadb91e505 (ALSA: HDA/ca0132: add extra init functions for r3di + sbz) introduced an extra initialization function that was improperly guarded, taking effect on systems with QUIRK_ALIENWARE, even though such systems were supposedly not affected.
It may be that this piece of initialization should be done for all systems, but that's not a call that I can make.
Signed-off-by: Alastair Bridgewater alastair.bridgewater@gmail.com
diff --git a/sound/pci/hda/patch_ca0132.c b/sound/pci/hda/patch_ca0132.c index bdd626bde267..ca7375e48138 100644 --- a/sound/pci/hda/patch_ca0132.c +++ b/sound/pci/hda/patch_ca0132.c @@ -7223,7 +7223,7 @@ static int ca0132_init(struct hda_codec *codec)
snd_hda_sequence_write(codec, spec->base_init_verbs);
if (spec->quirk != QUIRK_NONE)
if (spec->use_alt_functions) ca0132_alt_init(codec); ca0132_download_dsp(codec);-- 2.16.1
Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
Well, here's how I've been treating this. The ca0132 driver was made for the Chromebook's ca0132 chip. So in my mind, QUIRK_NONE is the Chromebook, and any added support for other cards is something that has been tacked on top of it's base. I don't know of any Core3D based cards that work just fine by going through the default setup without some sort of QUIRK related fixes. I don't envision ALSA getting floating point decibel volume, but I guess if that were to happen, maybe this could become an issue.
In my mind, there is probably a reason Creative didn't make any reference to that verb on the driver that they, the people with the actual documentation, wrote. I try not to make any changes to things that I have no real reason to make a change to. It worked fine before without it, so I see no reason to begin doing that. But, if you believe that it's a necessary thing to do, then I guess you can. I just don't like touching things that I don't need to.
On Sat, Jun 16, 2018 at 8:56 PM, Alastair Bridgewater alastair.bridgewater@gmail.com wrote:
That makes a certain amount of sense, but there are a few things that would invalidate that reasoning. First, the contrived scenario: Running Linux on the Chromebook with the sound card routed to a Qemu VM running Some Other Operating System. Second, there's no quirk matching for the Chromebook, which means that every unknown card (and these unknown cards would presumably have the opportunity to be dual-boot windows devices) would be denied the benefit of having the extra volume bits cleared. Third, if Linux starts to use those extra bits, and someone is dual-booting between kernel versions, we get the same effect.
At this point, unless there is reason to expect that clearing these volume parameters could be harmful, I think that doing so globally is the right thing. I can prep a patch for that in a day or so.
On Sat, Jun 16, 2018 at 11:37 AM, Connor McAdams conmanx360@gmail.com wrote:
Going through the code again today, I realized why I did this. I don't believe the Chromebook will ever go into Windows, so I don't think I have to set the 0x797 and 0x798 audio volume verbs to 0x00 on it. I did this so that the Alienware laptops, which probably have booted into Windows, would have those values set to 0 if they hadn't been already.
On Fri, Jun 15, 2018 at 9:56 PM, Alastair Bridgewater alastair.bridgewater@gmail.com wrote:
Commit e93ac30a32a6ba7ac3b4b2a4379af1dadb91e505 (ALSA: HDA/ca0132: add extra init functions for r3di + sbz) introduced an extra initialization function that was improperly guarded, taking effect on systems with QUIRK_ALIENWARE, even though such systems were supposedly not affected.
It may be that this piece of initialization should be done for all systems, but that's not a call that I can make.
Signed-off-by: Alastair Bridgewater alastair.bridgewater@gmail.com
diff --git a/sound/pci/hda/patch_ca0132.c b/sound/pci/hda/patch_ca0132.c index bdd626bde267..ca7375e48138 100644 --- a/sound/pci/hda/patch_ca0132.c +++ b/sound/pci/hda/patch_ca0132.c @@ -7223,7 +7223,7 @@ static int ca0132_init(struct hda_codec *codec)
snd_hda_sequence_write(codec, spec->base_init_verbs);
if (spec->quirk != QUIRK_NONE)
if (spec->use_alt_functions) ca0132_alt_init(codec); ca0132_download_dsp(codec);-- 2.16.1
Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
QUIRK_NONE is, quite explicitly, the default case. The entire point of a quirks system is to allow "programming by difference" from a given base case, which requires that merely defining a new quirk for some piece of hardware should not change the behavior of the driver for that hardware. In turn, this means that testing for QUIRK_NONE explicitly is a violation of that implicit contract.
Change a test for QUIRK_NONE and QUIRK_ALIENWARE to default, and add a test for QUIRK_SBZ to disable the default behavior in that instance.
Signed-off-by: Alastair Bridgewater alastair.bridgewater@gmail.com
diff --git a/sound/pci/hda/patch_ca0132.c b/sound/pci/hda/patch_ca0132.c index ca7375e48138..71487b82eef5 100644 --- a/sound/pci/hda/patch_ca0132.c +++ b/sound/pci/hda/patch_ca0132.c @@ -7237,8 +7237,9 @@ static int ca0132_init(struct hda_codec *codec) case QUIRK_R3DI: r3di_setup_defaults(codec); break; - case QUIRK_NONE: - case QUIRK_ALIENWARE: + case QUIRK_SBZ: + break; + default: ca0132_setup_defaults(codec); ca0132_init_analog_mic2(codec); ca0132_init_dmic(codec);
Good call. Please understand that I'm fairly new to all of this, so mistakes like this are more due to being ignorant than anything else. I was afraid certain things may end up broken by my changes, and I hope it didn't inconvenience you.
On Fri, Jun 15, 2018 at 9:56 PM, Alastair Bridgewater alastair.bridgewater@gmail.com wrote:
QUIRK_NONE is, quite explicitly, the default case. The entire point of a quirks system is to allow "programming by difference" from a given base case, which requires that merely defining a new quirk for some piece of hardware should not change the behavior of the driver for that hardware. In turn, this means that testing for QUIRK_NONE explicitly is a violation of that implicit contract.
Change a test for QUIRK_NONE and QUIRK_ALIENWARE to default, and add a test for QUIRK_SBZ to disable the default behavior in that instance.
Signed-off-by: Alastair Bridgewater alastair.bridgewater@gmail.com
diff --git a/sound/pci/hda/patch_ca0132.c b/sound/pci/hda/patch_ca0132.c index ca7375e48138..71487b82eef5 100644 --- a/sound/pci/hda/patch_ca0132.c +++ b/sound/pci/hda/patch_ca0132.c @@ -7237,8 +7237,9 @@ static int ca0132_init(struct hda_codec *codec) case QUIRK_R3DI: r3di_setup_defaults(codec); break;
case QUIRK_NONE:case QUIRK_ALIENWARE:
case QUIRK_SBZ:break;default: ca0132_setup_defaults(codec); ca0132_init_analog_mic2(codec); ca0132_init_dmic(codec);-- 2.16.1
Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
My inconvenience from the R3Di/SBZ changes has been minor at worst.
Actually, the timing on this has been very good. Finding the R3Di patches and QemuHDADump two weeks ago, just as I was starting to look again into trying to improve sound support on my M17x R4 was a pleasant surprise. Yes, there are rough edges, but they're the sort of thing that should get caught by careful code review or from testing various -rc kernels. And you'll get better at this if you keep at it.
Speaking of rough edges, QUIRK_SBZ and QUIRK_R3DI have code paths that use both ca0132_alt_select_in() (from ca0132_init() and the input source control) and ca0132_select_mic() (from amic_callback(), tied to a jack detect). I'm fairly certain that this is wrong.
On Fri, Jun 15, 2018 at 10:20 PM, Connor McAdams conmanx360@gmail.com wrote:
Good call. Please understand that I'm fairly new to all of this, so mistakes like this are more due to being ignorant than anything else. I was afraid certain things may end up broken by my changes, and I hope it didn't inconvenience you.
On Fri, Jun 15, 2018 at 9:56 PM, Alastair Bridgewater alastair.bridgewater@gmail.com wrote:
QUIRK_NONE is, quite explicitly, the default case. The entire point of a quirks system is to allow "programming by difference" from a given base case, which requires that merely defining a new quirk for some piece of hardware should not change the behavior of the driver for that hardware. In turn, this means that testing for QUIRK_NONE explicitly is a violation of that implicit contract.
Change a test for QUIRK_NONE and QUIRK_ALIENWARE to default, and add a test for QUIRK_SBZ to disable the default behavior in that instance.
Signed-off-by: Alastair Bridgewater alastair.bridgewater@gmail.com
diff --git a/sound/pci/hda/patch_ca0132.c b/sound/pci/hda/patch_ca0132.c index ca7375e48138..71487b82eef5 100644 --- a/sound/pci/hda/patch_ca0132.c +++ b/sound/pci/hda/patch_ca0132.c @@ -7237,8 +7237,9 @@ static int ca0132_init(struct hda_codec *codec) case QUIRK_R3DI: r3di_setup_defaults(codec); break;
case QUIRK_NONE:case QUIRK_ALIENWARE:
case QUIRK_SBZ:break;default: ca0132_setup_defaults(codec); ca0132_init_analog_mic2(codec); ca0132_init_dmic(codec);-- 2.16.1
Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
On Sat, 16 Jun 2018 04:20:18 +0200, Connor McAdams wrote:
Good call. Please understand that I'm fairly new to all of this, so mistakes like this are more due to being ignorant than anything else. I was afraid certain things may end up broken by my changes, and I hope it didn't inconvenience you.
Connor, if the change looks good and it doesn't break your device either, just give your Reviewed-by, Acked-by and/or Tested-by tag, so that I can merge them quickly.
thanks,
Takashi
Commit 009b8f979bf8cb5f7ec6d3dd7683585122ed10f8 conditionalized adding the "CA0132 Analog Mic-In2" PCM with a comment to the effect that, "desktops don't use this ADC", but the test was set up such that the ADC was only created for desktops. Invert the test.
Signed-off-by: Alastair Bridgewater alastair.bridgewater@gmail.com
diff --git a/sound/pci/hda/patch_ca0132.c b/sound/pci/hda/patch_ca0132.c index 71487b82eef5..cef22c60d7ee 100644 --- a/sound/pci/hda/patch_ca0132.c +++ b/sound/pci/hda/patch_ca0132.c @@ -5966,7 +5966,7 @@ static int ca0132_build_pcms(struct hda_codec *codec) info->stream[SNDRV_PCM_STREAM_CAPTURE].nid = spec->adcs[0];
/* With the DSP enabled, desktops don't use this ADC. */ - if (spec->use_alt_functions) { + if (!spec->use_alt_functions) { info = snd_hda_codec_pcm_new(codec, "CA0132 Analog Mic-In2"); if (!info) return -ENOMEM;
The commentary says to use various parameters, and lays out what the mapping is... The code used a 32KHz rate when the comment says that it needs to use a 48KHz rate. And this has been the case since day one.
On the Alienware M17x R4, the DMic used to have exceptionally quiet pickup and a lot of noise. Changing the data rate fixes both of these issues.
Searching the kernel bug tracker for ca0132-related issues shows no mention of this being an issue for other hardware, and I have no other hardware to test with, so a quirk is used to limit the effect to just the M17x R4.
Signed-off-by: Alastair Bridgewater alastair.bridgewater@gmail.com
diff --git a/sound/pci/hda/patch_ca0132.c b/sound/pci/hda/patch_ca0132.c index cef22c60d7ee..614f69407c9f 100644 --- a/sound/pci/hda/patch_ca0132.c +++ b/sound/pci/hda/patch_ca0132.c @@ -991,6 +991,7 @@ struct ca0132_spec { enum { QUIRK_NONE, QUIRK_ALIENWARE, + QUIRK_ALIENWARE_M17XR4, QUIRK_SBZ, QUIRK_R3DI, }; @@ -1040,6 +1041,7 @@ static const struct hda_pintbl r3di_pincfgs[] = { };
static const struct snd_pci_quirk ca0132_quirks[] = { + SND_PCI_QUIRK(0x1028, 0x057b, "Alienware M17x R4", QUIRK_ALIENWARE_M17XR4), SND_PCI_QUIRK(0x1028, 0x0685, "Alienware 15 2015", QUIRK_ALIENWARE), SND_PCI_QUIRK(0x1028, 0x0688, "Alienware 17 2015", QUIRK_ALIENWARE), SND_PCI_QUIRK(0x1028, 0x0708, "Alienware 15 R2 2016", QUIRK_ALIENWARE), @@ -6130,7 +6132,10 @@ static void ca0132_init_dmic(struct hda_codec *codec) * Bit 6: set to select Data2, clear for Data1 * Bit 7: set to enable DMic, clear for AMic */ - val = 0x23; + if (spec->quirk == QUIRK_ALIENWARE_M17XR4) + val = 0x33; + else + val = 0x23; /* keep a copy of dmic ctl val for enable/disable dmic purpuse */ spec->dmic_ctl = val; snd_hda_codec_write(codec, spec->input_pins[0], 0,
In my experience, the Dmic is always set to have the voice focus effect enabled on the Chromebook. The reason it drops the sample rate is to ease the load on the DSP. On the Recon3Di and the Sound Blaster Z, if effects are enabled, it drops to 16khz when effects in general are enabled. I don't work for Creative so I have no clue about the actual details, but I do know if input effects are on on R3Di or SBZ, the card becomes laggy and less responsive to commands.
So, it may depend upon the implementation. You could probably figure out definitively by using my program QemuHDADump to dump the data if you really wanted to find out for sure what sample rate the mic is set to by default, and the other parameters. I would be willing to look through the data dump to help you determine it. Let me know.
On Fri, Jun 15, 2018 at 9:56 PM, Alastair Bridgewater alastair.bridgewater@gmail.com wrote:
The commentary says to use various parameters, and lays out what the mapping is... The code used a 32KHz rate when the comment says that it needs to use a 48KHz rate. And this has been the case since day one.
On the Alienware M17x R4, the DMic used to have exceptionally quiet pickup and a lot of noise. Changing the data rate fixes both of these issues.
Searching the kernel bug tracker for ca0132-related issues shows no mention of this being an issue for other hardware, and I have no other hardware to test with, so a quirk is used to limit the effect to just the M17x R4.
Signed-off-by: Alastair Bridgewater alastair.bridgewater@gmail.com
diff --git a/sound/pci/hda/patch_ca0132.c b/sound/pci/hda/patch_ca0132.c index cef22c60d7ee..614f69407c9f 100644 --- a/sound/pci/hda/patch_ca0132.c +++ b/sound/pci/hda/patch_ca0132.c @@ -991,6 +991,7 @@ struct ca0132_spec { enum { QUIRK_NONE, QUIRK_ALIENWARE,
QUIRK_ALIENWARE_M17XR4, QUIRK_SBZ, QUIRK_R3DI,}; @@ -1040,6 +1041,7 @@ static const struct hda_pintbl r3di_pincfgs[] = { };
static const struct snd_pci_quirk ca0132_quirks[] = {
SND_PCI_QUIRK(0x1028, 0x057b, "Alienware M17x R4", QUIRK_ALIENWARE_M17XR4), SND_PCI_QUIRK(0x1028, 0x0685, "Alienware 15 2015", QUIRK_ALIENWARE), SND_PCI_QUIRK(0x1028, 0x0688, "Alienware 17 2015", QUIRK_ALIENWARE), SND_PCI_QUIRK(0x1028, 0x0708, "Alienware 15 R2 2016", QUIRK_ALIENWARE),@@ -6130,7 +6132,10 @@ static void ca0132_init_dmic(struct hda_codec *codec) * Bit 6: set to select Data2, clear for Data1 * Bit 7: set to enable DMic, clear for AMic */
val = 0x23;
if (spec->quirk == QUIRK_ALIENWARE_M17XR4)val = 0x33;elseval = 0x23; /* keep a copy of dmic ctl val for enable/disable dmic purpuse */ spec->dmic_ctl = val; snd_hda_codec_write(codec, spec->input_pins[0], 0,-- 2.16.1
Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
On Sat, 16 Jun 2018 03:56:14 +0200, Alastair Bridgewater wrote:
The ca0132 codec support is fairly crufty, and the new R3Di and SBZ support hasn't exactly improved that situation. This patch series tries to clean up a small amount of longstanding cruft, reverse some behavioral changes to other systems made by the recent R3Di/SBZ patches, and make the DMic work properly on the Alienware M17x R4.
For two of these patches (the QUIRK_ALIENWARE patch and the DMic patch), it may be that the correct behavior is the exact OPPOSITE of what the patch does as far as quirk testing goes (that is, it should apply globally, rather than limited to some specific quirk or set of quirks), but I have no way to make such a determination at this time, so erring on the side of the status quo ante seems appropriate.
Alastair Bridgewater (6): ALSA: hda/ca0132: Delete pointless assignments to struct auto_pin_cfg fields ALSA: hda/ca0132: Delete redundant UNSOL event requests ALSA: hda/ca0132: Restore behavior of QUIRK_ALIENWARE ALSA: hda/ca0132: Don't test for QUIRK_NONE ALSA: hda/ca0132: Restore PCM Analog Mic-In2 ALSA: hda/ca0132: Fix DMic data rate for Alienware M17x R4
Since these look like reasonable fixes, I applied all six patches as is now.
If any further fixes are needed, please make on top of these.
thanks,
Takashi
On Sun, Jun 17, 2018 at 3:22 AM, Takashi Iwai tiwai@suse.de wrote:
On Sat, 16 Jun 2018 03:56:14 +0200, Alastair Bridgewater wrote:
The ca0132 codec support is fairly crufty, and the new R3Di and SBZ support hasn't exactly improved that situation. This patch series tries to clean up a small amount of longstanding cruft, reverse some behavioral changes to other systems made by the recent R3Di/SBZ patches, and make the DMic work properly on the Alienware M17x R4.
For two of these patches (the QUIRK_ALIENWARE patch and the DMic patch), it may be that the correct behavior is the exact OPPOSITE of what the patch does as far as quirk testing goes (that is, it should apply globally, rather than limited to some specific quirk or set of quirks), but I have no way to make such a determination at this time, so erring on the side of the status quo ante seems appropriate.
Alastair Bridgewater (6): ALSA: hda/ca0132: Delete pointless assignments to struct auto_pin_cfg fields ALSA: hda/ca0132: Delete redundant UNSOL event requests ALSA: hda/ca0132: Restore behavior of QUIRK_ALIENWARE ALSA: hda/ca0132: Don't test for QUIRK_NONE ALSA: hda/ca0132: Restore PCM Analog Mic-In2 ALSA: hda/ca0132: Fix DMic data rate for Alienware M17x R4
Since these look like reasonable fixes, I applied all six patches as is now.
If any further fixes are needed, please make on top of these.
Wonderful! Thank you. About the only other patch that I can see making at this point is to call ca0132_alt_vol_setup() on all systems, and I could go either way on that.
With a bit of luck, I may have another patch series in a month or so. Not suitable for 4.18, but maybe for 4.19 or so?
thanks,
Takashi
On Tue, 19 Jun 2018 15:18:41 +0200, Alastair Bridgewater wrote:
On Sun, Jun 17, 2018 at 3:22 AM, Takashi Iwai tiwai@suse.de wrote:
On Sat, 16 Jun 2018 03:56:14 +0200, Alastair Bridgewater wrote:
The ca0132 codec support is fairly crufty, and the new R3Di and SBZ support hasn't exactly improved that situation. This patch series tries to clean up a small amount of longstanding cruft, reverse some behavioral changes to other systems made by the recent R3Di/SBZ patches, and make the DMic work properly on the Alienware M17x R4.
For two of these patches (the QUIRK_ALIENWARE patch and the DMic patch), it may be that the correct behavior is the exact OPPOSITE of what the patch does as far as quirk testing goes (that is, it should apply globally, rather than limited to some specific quirk or set of quirks), but I have no way to make such a determination at this time, so erring on the side of the status quo ante seems appropriate.
Alastair Bridgewater (6): ALSA: hda/ca0132: Delete pointless assignments to struct auto_pin_cfg fields ALSA: hda/ca0132: Delete redundant UNSOL event requests ALSA: hda/ca0132: Restore behavior of QUIRK_ALIENWARE ALSA: hda/ca0132: Don't test for QUIRK_NONE ALSA: hda/ca0132: Restore PCM Analog Mic-In2 ALSA: hda/ca0132: Fix DMic data rate for Alienware M17x R4
Since these look like reasonable fixes, I applied all six patches as is now.
If any further fixes are needed, please make on top of these.
Wonderful! Thank you. About the only other patch that I can see making at this point is to call ca0132_alt_vol_setup() on all systems, and I could go either way on that.
With a bit of luck, I may have another patch series in a month or so. Not suitable for 4.18, but maybe for 4.19 or so?
Yes, sounds like a plan.
thanks,
Takashi
participants (3)
-
Alastair Bridgewater -
Connor McAdams -
Takashi Iwai