[alsa-devel] hp_pin was NULL value

[Takashi Iwai]

On Wed, 09 Jan 2019 12:19:14 +0100,
Kailang wrote:
> 
> Hi Takashi,
> 
> But spec->init_hook will call at resume back.
> alc294_hp_init() function just need to call at one time.

As already mentioned, even if it has to be called only once and does
something for the hardware, it'll be missing if the machine goes
hibernate (S4) and resume.  In S4, the machine boots from scratch and
switches immediately to the saved image that calls resume callback.
What's worse for us is that this switch happens usually in initrd,
i.e. before the sound driver gets loaded.  So, the machine goes to
resume without processing alc294_hp_init().

IOW, even if it's needed only once, you'll have to call it at resume
unless it has a significant drawback.

(Strictly speaking, we may use thaw PM callback, but the HD-audio
 codec driver doesn't use it, so there is only resume callback for
 now.)

> And it need to call it before call spec->init_hook.

But ALC294 (and ALC700) have no init_hook, so far?

> ALC225 also need to call similar procedure. I will post it later.

OK, thanks.


Takashi

> 
> BR,
> Kailang
> ________________________________________
> 從: Takashi Iwai [tiwai at suse.de]
> 寄件日期: 2019年1月9日 下午 05:56
> 至: Kailang
> 副本:  (alsa-devel at alsa-project.org)
> 主旨: Re: hp_pin was NULL value
> 
> On Wed, 09 Jan 2019 10:45:25 +0100,
> Kailang wrote:
> >
> > >>But this made me wonder whether we don't need to call this function at resume as well?  Currently it's called only at probing.
> > Yes, it only call at probing. Just need to call it at boot time.
> >
> > -----Original Message-----
> > From: Takashi Iwai <tiwai at suse.de>
> > Sent: Wednesday, January 9, 2019 5:43 PM
> > To: Kailang <kailang at realtek.com>
> > Cc: (alsa-devel at alsa-project.org) <alsa-devel at alsa-project.org>
> > Subject: Re: hp_pin was NULL value
> >
> > On Wed, 09 Jan 2019 10:31:33 +0100,
> > Kailang wrote:
> > >
> > > Hi Takashi,
> > >
> > > Could I move the alc294_hp_init(codec) to below line.
> > > Because hp_pin = spec->gen.autocfg.hp_pins[0] was null value when alc294_hp_init(codec) at original line.
> > > Or move alc269_parse_auto_config() upward.
> >
> > It looks OK to me.  But this made me wonder whether we don't need to call this function at resume as well?  Currently it's called only at probing.
> 
> Hrm, but it modifies many COEFs, and are these all preserved with
> suspend?  For example, there is also hibernation (S4), which is a
> switch from the boot without the audio driver initialization.  Then
> we'll resume from the uninitialized state.  I guess other COEF changes
> in that area should be moved as the additional init hook as well.
> 
> Actually, if we do call this at resume, the change would be easier, a
> patch like below.
> 
> 
> thanks,
> 
> Takashi
> 
> --- a/sound/pci/hda/patch_realtek.c
> +++ b/sound/pci/hda/patch_realtek.c
> @@ -7404,6 +7404,20 @@ static void alc294_hp_init(struct hda_codec *codec)
>         msleep(50);
>  }
> 
> +static void alc294_init(struct hda_codec *codec)
> +{
> +       /* UAJ MIC Vref control by verb */
> +       alc_update_coef_idx(codec, 0x6b, 0x0018, (1<<4) | (1<<3));
> +       alc294_hp_init(codec);
> +}
> +
> +static void alc700_init(struct hda_codec *codec)
> +{
> +       /* Combo jack auto trigger control */
> +       alc_update_coef_idx(codec, 0x4a, 1 << 15, 0);
> +       alc294_hp_init(codec);
> +}
> +
>  /*
>   */
>  static int patch_alc269(struct hda_codec *codec)
> @@ -7528,8 +7542,7 @@ static int patch_alc269(struct hda_codec *codec)
>         case 0x10ec0294:
>                 spec->codec_variant = ALC269_TYPE_ALC294;
>                 spec->gen.mixer_nid = 0; /* ALC2x4 does not have any loopback mixer path */
> -               alc_update_coef_idx(codec, 0x6b, 0x0018, (1<<4) | (1<<3)); /* UAJ MIC Vref control by verb */
> -               alc294_hp_init(codec);
> +               spec->init_hook = alc294_init;
>                 break;
>         case 0x10ec0300:
>                 spec->codec_variant = ALC269_TYPE_ALC300;
> @@ -7540,8 +7553,7 @@ static int patch_alc269(struct hda_codec *codec)
>         case 0x10ec0703:
>                 spec->codec_variant = ALC269_TYPE_ALC700;
>                 spec->gen.mixer_nid = 0; /* ALC700 does not have any loopback mixer path */
> -               alc_update_coef_idx(codec, 0x4a, 1 << 15, 0); /* Combo jack auto trigger control */
> -               alc294_hp_init(codec);
> +               spec->init_hook = alc700_init;
>                 break;
> 
>         }
> 
> 
> ------Please consider the environment before printing this e-mail.