On 09/16/2011 01:07 PM, Takashi Iwai wrote:
At Fri, 16 Sep 2011 12:45:06 +0200, David Henningsson wrote:
On 09/16/2011 12:03 PM, Takashi Iwai wrote:
At Fri, 16 Sep 2011 11:35:31 +0200, Takashi Iwai wrote:
At Fri, 16 Sep 2011 11:22:01 +0200, David Henningsson wrote:
[1<text/plain; ISO-8859-1 (7bit)>] On 09/16/2011 10:31 AM, Takashi Iwai wrote:
At Fri, 16 Sep 2011 10:01:48 +0200, David Henningsson wrote: > > The ~6 months old functionality for configurable automuting is broken > for the case that the user has only HP and LO (no speakers) - basically > there is an "Automute mode" control, but enabling it does nothing. While > fixing that, I also took the liberty of refactoring/rewriting some of > that code, in order to make it easier to understand and maintain. > > Takashi, given you approve/like these changes, I'll go ahead and fix up > the quirk files to fit the new variable names.
I'm fine with renames but let's split patches: first fix the bug of non-working auto-mute control, then refactoring. The former should go to 3.1 while the latter is queued to 3.2.
Ok. If it also should go to 3.0, feel free to add cc: stable@kernel.org (hmm, is that working even though kernel.org is down?)
Here comes the 3.1 part of the patch. If you're happy with it, commit it and I'll continue with the 3.2 renames.
Hm, it's more complicated than I wanted.
Could you have an example codec proc or alsa-info.sh file to simulate here? The logic is already there, so a few lines of corrections should suffice to fix the behavior.
I guess the oneliner below should fix the bug. Could you check it?
thanks,
Takashi
diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c index 70ba45e..5c4625e 100644 --- a/sound/pci/hda/patch_realtek.c +++ b/sound/pci/hda/patch_realtek.c @@ -556,7 +556,7 @@ static void update_speakers(struct hda_codec *codec) if (spec->autocfg.line_out_pins[0] == spec->autocfg.hp_pins[0] || spec->autocfg.line_out_pins[0] == spec->autocfg.speaker_pins[0]) return;
- if (!spec->automute_lines || !spec->automute)
- if ((spec->automute_hp_lo&& !spec->automute_lines) || !spec->automute) on = 0; else on = spec->jack_present;
It does in this particular case, but it's very confusing that spec->automute_lines can be 0 while the lines are still automuted. To avoid such confusion, a more complicated patch is needed.
Yes, needed for 3.2, but not for 3.0/3.1 kernels.
Honestly, would you have accepted such a patch if I were the one proposing it? :-)
Yes, certainly I would. And I asked for it.
Of course, a later rewrite would be needed, but as a fix at this late stage, smaller is more beautiful.
We'll have to disagree on that, then. For me, this is confusing, so I have a hard time ensuring that the one-liner isn't causing problems in some other use case. But you wrote the code, so you probably have a better overview than I have.