lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Sat, 16 Aug 2008 14:37:37 +0100
From:	"Daniel J Blueman" <daniel.blueman@...il.com>
To:	"Takashi Iwai" <tiwai@...e.de>
Cc:	"Vegard Nossum" <vegard.nossum@...il.com>,
	"Romano Giannetti" <romano@....icai.upcomillas.es>,
	"Linux Kernel" <linux-kernel@...r.kernel.org>
Subject: Re: ALC883 recording troubles...

Hi Takashi and thanks,

On Sat, Aug 16, 2008 at 9:38 AM, Takashi Iwai <tiwai@...e.de> wrote:
> At Fri, 15 Aug 2008 23:01:42 +0100,
> Daniel J Blueman wrote:
>>
>> Hi Takashi,
>>
>> On Fri, Aug 15, 2008 at 3:42 PM, Takashi Iwai <tiwai@...e.de> wrote:
>> > At Thu, 14 Aug 2008 20:10:03 +0100,
>> > Daniel J Blueman wrote:
>> >>
>> >> Hi Takashi,
>> >>
>> >> I am still finding this recording +ve saturation on my ALC883 Intel
>> >> HDA sound device with 2.6.27-rc3 (ie ALSA 1.0.17 drivers) and ALSA
>> >> 1.0.16-1 libraries on Ubuntu Intrepid.
>> >>
>> >> The previous workaround still 'gets us out of jail':
>> >>
>> >> # ./hda-verb /dev/snd/hwC0D0 0x23 SET_AMP_GAIN_MUTE 0x7180
>> >> nid = 0x23, verb = 0x300, param = 0x7180
>> >> value = 0x0
>> >
>> > Could you try the patch below?  With this patch, unused connections
>> > will be disabled.  Let me know if it really works so that I can merge
>> > it to git tree.
>> >
>> > Takashi
>> >
>> > ---
>> > diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
>> > index 9473abb..6f0485a 100644
>> > --- a/sound/pci/hda/patch_realtek.c
>> > +++ b/sound/pci/hda/patch_realtek.c
>> > @@ -6442,6 +6442,40 @@ static void alc882_auto_init_analog_input(struct hda_codec *codec)
>> >        }
>> >  }
>> >
>> > +static void alc882_auto_init_input_src(struct hda_codec *codec)
>> > +{
>> > +       struct alc_spec *spec = codec->spec;
>> > +       const struct hda_input_mux *imux = spec->input_mux;
>> > +       int c;
>> > +
>> > +       for (c = 0; c < spec->num_adc_nids; c++) {
>> > +               hda_nid_t conn_list[10];
>> > +               hda_nid_t nid = spec->capsrc_nids[c];
>> > +               int conns, mute, idx, item;
>> > +
>> > +               conns = snd_hda_get_connections(codec, nid, conn_list,
>> > +                                               ARRAY_SIZE(conn_list));
>> > +               if (conns < 0)
>> > +                       continue;
>> > +               mute = HDA_AMP_MUTE;
>> > +               for (idx = 0; idx < conns; idx++) {
>> > +                       /* if the current connection is the selected one,
>> > +                        * unmute it as default - otherwise mute it
>> > +                        */
>> > +                       for (item = 0; item < imux->num_items; item++) {
>> > +                               if (imux->items[item].index == idx) {
>> > +                                       if (spec->cur_mux[c] == item)
>> > +                                               mute = 0;
>> > +                                       break;
>> > +                               }
>> > +                       }
>> > +                       snd_hda_codec_amp_stereo(codec, nid, HDA_INPUT,
>> > +                                                idx, HDA_AMP_MUTE, mute);
>> > +               }
>> > +
>> > +       }
>> > +}
>> > +
>> >  /* add mic boosts if needed */
>> >  static int alc_auto_add_mic_boost(struct hda_codec *codec)
>> >  {
>> > @@ -6496,6 +6530,7 @@ static void alc882_auto_init(struct hda_codec *codec)
>> >        alc882_auto_init_multi_out(codec);
>> >        alc882_auto_init_hp_out(codec);
>> >        alc882_auto_init_analog_input(codec);
>> > +       alc882_auto_init_input_src(codec);
>> >        if (spec->unsol_event)
>> >                alc_sku_automute(codec);
>> >  }
>> > @@ -8290,6 +8325,8 @@ static void alc883_auto_init_analog_input(struct hda_codec *codec)
>> >        }
>> >  }
>> >
>> > +#define alc883_auto_init_input_src     alc882_auto_init_input_src
>> > +
>> >  /* almost identical with ALC880 parser... */
>> >  static int alc883_parse_auto_config(struct hda_codec *codec)
>> >  {
>> > @@ -8320,6 +8357,7 @@ static void alc883_auto_init(struct hda_codec *codec)
>> >        alc883_auto_init_multi_out(codec);
>> >        alc883_auto_init_hp_out(codec);
>> >        alc883_auto_init_analog_input(codec);
>> > +       alc883_auto_init_input_src(codec);
>> >        if (spec->unsol_event)
>> >                alc_sku_automute(codec);
>> >  }
>> > @@ -9703,6 +9741,7 @@ static int alc262_parse_auto_config(struct hda_codec *codec)
>> >  #define alc262_auto_init_multi_out     alc882_auto_init_multi_out
>> >  #define alc262_auto_init_hp_out                alc882_auto_init_hp_out
>> >  #define alc262_auto_init_analog_input  alc882_auto_init_analog_input
>> > +#define alc262_auto_init_input_src     alc882_auto_init_input_src
>> >
>> >
>> >  /* init callback for auto-configuration model -- overriding the default init */
>> > @@ -9712,6 +9751,7 @@ static void alc262_auto_init(struct hda_codec *codec)
>> >        alc262_auto_init_multi_out(codec);
>> >        alc262_auto_init_hp_out(codec);
>> >        alc262_auto_init_analog_input(codec);
>> > +       alc262_auto_init_input_src(codec);
>> >        if (spec->unsol_event)
>> >                alc_sku_automute(codec);
>> >  }
>> > @@ -13382,6 +13422,8 @@ static void alc861vd_auto_init_analog_input(struct hda_codec *codec)
>> >        }
>> >  }
>> >
>> > +#define alc861vd_auto_init_input_src   alc882_auto_init_input_src
>> > +
>> >  #define alc861vd_idx_to_mixer_vol(nid)         ((nid) + 0x02)
>> >  #define alc861vd_idx_to_mixer_switch(nid)      ((nid) + 0x0c)
>> >
>> > @@ -13564,6 +13606,7 @@ static void alc861vd_auto_init(struct hda_codec *codec)
>> >        alc861vd_auto_init_multi_out(codec);
>> >        alc861vd_auto_init_hp_out(codec);
>> >        alc861vd_auto_init_analog_input(codec);
>> > +       alc861vd_auto_init_input_src(codec);
>> >        if (spec->unsol_event)
>> >                alc_sku_automute(codec);
>> >  }
>> > @@ -14729,6 +14772,8 @@ static void alc662_auto_init_analog_input(struct hda_codec *codec)
>> >        }
>> >  }
>> >
>> > +#define alc662_auto_init_input_src     alc882_auto_init_input_src
>> > +
>> >  static int alc662_parse_auto_config(struct hda_codec *codec)
>> >  {
>> >        struct alc_spec *spec = codec->spec;
>> > @@ -14785,6 +14830,7 @@ static void alc662_auto_init(struct hda_codec *codec)
>> >        alc662_auto_init_multi_out(codec);
>> >        alc662_auto_init_hp_out(codec);
>> >        alc662_auto_init_analog_input(codec);
>> > +       alc662_auto_init_input_src(codec);
>> >        if (spec->unsol_event)
>> >                alc_sku_automute(codec);
>> >  }
>>
>> There were a couple of issues: getting the mute logic right from
>> moving the assignment and increasing the number of inputs beyond the
>> 11 required here to avoid the 'Too many connections' message and
>> fixing the last input being skipped.
>
> Oh I should have more carefully checked.
>
>> With the updated patch, the problem is fully addressed; no resulting
>> DC offset, so recording quality is great. This seems logically safe
>> and a hopeful for 2.6.27-rc4...
>>
>> The updated patch is against 2.6.27-rc3.
>
> Thanks.  There is another problem in the current patch.
> It should use snd_hda_codec_write() without caching.  Otherwise
> it breaks the suspend/resume.
>
> The revised patch is below.  Please test and report back if it works
> so that I can push it to the upstream.
>
>
> thanks,
>
> Takashi
>
> ===
> commit 340c6e7d8bc26354313fe8bacc636ca18f71995e
> Author: Takashi Iwai <tiwai@...e.de>
> Date:   Fri Aug 15 16:46:42 2008 +0200
>
>    ALSA: hda - Fix capture source widgets on ALC codecs
>
>    On some Realtek codecs like ALC882 or ALC883, the capture source is
>    no mux but sum widget.  We have to initialize all channels properly
>    for this type, otherwise noises may come in from the unused route.
>
>    The patch assures to mute unused routes, and unmute the currently
>    selected route.
>
>    Signed-off-by: Takashi Iwai <tiwai@...e.de>
>
> diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
> index add4e87..56d899f 100644
> --- a/sound/pci/hda/patch_realtek.c
> +++ b/sound/pci/hda/patch_realtek.c
> @@ -6437,6 +6437,40 @@ static void alc882_auto_init_analog_input(struct hda_codec *codec)
>        }
>  }
>
> +static void alc882_auto_init_input_src(struct hda_codec *codec)
> +{
> +       struct alc_spec *spec = codec->spec;
> +       const struct hda_input_mux *imux = spec->input_mux;
> +       int c;
> +
> +       for (c = 0; c < spec->num_adc_nids; c++) {
> +               hda_nid_t conn_list[10];
> +               hda_nid_t nid = spec->capsrc_nids[c];
> +               int conns, mute, idx, item;
> +
> +               conns = snd_hda_get_connections(codec, nid, conn_list,
> +                                               ARRAY_SIZE(conn_list));
> +               if (conns < 0)
> +                       continue;
> +               mute = HDA_AMP_MUTE;
> +               for (idx = 0; idx < conns; idx++) {
> +                       /* if the current connection is the selected one,
> +                        * unmute it as default - otherwise mute it
> +                        */
> +                       for (item = 0; item < imux->num_items; item++) {
> +                               if (imux->items[item].index == idx) {
> +                                       if (spec->cur_mux[c] == item)
> +                                               mute = 0;
> +                                       break;
> +                               }
> +                       }
> +                       snd_hda_codec_amp_stereo(codec, nid, HDA_INPUT,
> +                                                idx, HDA_AMP_MUTE, mute);
> +               }
> +
> +       }
> +}
> +
>  /* add mic boosts if needed */
>  static int alc_auto_add_mic_boost(struct hda_codec *codec)
>  {
> @@ -6491,6 +6525,7 @@ static void alc882_auto_init(struct hda_codec *codec)
>        alc882_auto_init_multi_out(codec);
>        alc882_auto_init_hp_out(codec);
>        alc882_auto_init_analog_input(codec);
> +       alc882_auto_init_input_src(codec);
>        if (spec->unsol_event)
>                alc_sku_automute(codec);
>  }
> @@ -8285,6 +8320,8 @@ static void alc883_auto_init_analog_input(struct hda_codec *codec)
>        }
>  }
>
> +#define alc883_auto_init_input_src     alc882_auto_init_input_src
> +
>  /* almost identical with ALC880 parser... */
>  static int alc883_parse_auto_config(struct hda_codec *codec)
>  {
> @@ -8315,6 +8352,7 @@ static void alc883_auto_init(struct hda_codec *codec)
>        alc883_auto_init_multi_out(codec);
>        alc883_auto_init_hp_out(codec);
>        alc883_auto_init_analog_input(codec);
> +       alc883_auto_init_input_src(codec);
>        if (spec->unsol_event)
>                alc_sku_automute(codec);
>  }
> @@ -9663,6 +9701,7 @@ static int alc262_parse_auto_config(struct hda_codec *codec)
>  #define alc262_auto_init_multi_out     alc882_auto_init_multi_out
>  #define alc262_auto_init_hp_out                alc882_auto_init_hp_out
>  #define alc262_auto_init_analog_input  alc882_auto_init_analog_input
> +#define alc262_auto_init_input_src     alc882_auto_init_input_src
>
>
>  /* init callback for auto-configuration model -- overriding the default init */
> @@ -9672,6 +9711,7 @@ static void alc262_auto_init(struct hda_codec *codec)
>        alc262_auto_init_multi_out(codec);
>        alc262_auto_init_hp_out(codec);
>        alc262_auto_init_analog_input(codec);
> +       alc262_auto_init_input_src(codec);
>        if (spec->unsol_event)
>                alc_sku_automute(codec);
>  }
> @@ -13330,6 +13370,8 @@ static void alc861vd_auto_init_analog_input(struct hda_codec *codec)
>        }
>  }
>
> +#define alc861vd_auto_init_input_src   alc882_auto_init_input_src
> +
>  #define alc861vd_idx_to_mixer_vol(nid)         ((nid) + 0x02)
>  #define alc861vd_idx_to_mixer_switch(nid)      ((nid) + 0x0c)
>
> @@ -13512,6 +13554,7 @@ static void alc861vd_auto_init(struct hda_codec *codec)
>        alc861vd_auto_init_multi_out(codec);
>        alc861vd_auto_init_hp_out(codec);
>        alc861vd_auto_init_analog_input(codec);
> +       alc861vd_auto_init_input_src(codec);
>        if (spec->unsol_event)
>                alc_sku_automute(codec);
>  }
> @@ -14677,6 +14720,8 @@ static void alc662_auto_init_analog_input(struct hda_codec *codec)
>        }
>  }
>
> +#define alc662_auto_init_input_src     alc882_auto_init_input_src
> +
>  static int alc662_parse_auto_config(struct hda_codec *codec)
>  {
>        struct alc_spec *spec = codec->spec;
> @@ -14733,6 +14778,7 @@ static void alc662_auto_init(struct hda_codec *codec)
>        alc662_auto_init_multi_out(codec);
>        alc662_auto_init_hp_out(codec);
>        alc662_auto_init_analog_input(codec);
> +       alc662_auto_init_input_src(codec);
>        if (spec->unsol_event)
>                alc_sku_automute(codec);
>  }

This patch is first one you sent, except differing line numbers
(rediffed against 2.6.27-rc3?).

You could try attaching the patch to the mail too, to avoid the line
wrapping, silent whitespace conversion and any gmail/mailer 'hide
quoted text' mangling...

Thanks,
  Daniel
-- 
Daniel J Blueman
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ