[alsa-devel] hp_pin was NULL value

[Takashi Iwai]

On Wed, 09 Jan 2019 13:32:51 +0100,
Kailang wrote:
> 
> 
> Hi Takashi,
> 
> 
> OK! I will test it. 
> So, if ALC225 need to do this. I will assigned it to alc225_init(). Right?
> 
> >>But ALC294 (and ALC700) have no init_hook, so far?
> 
> spec->init_hook = alc_default_init;
> It has default init_hook. I use this to do standard headphone depop function.

Ah, OK.  Then we need a call chain from alc294_init(),

static void alc294_init()
{
	coef();
	alc294_hp_init();
	alc_default_init();
}


thanks,

Takashi

> 
> BR,
> Kailang
> ________________________________________
> 從: Takashi Iwai [tiwai at suse.de]
> 寄件日期: 2019年1月9日 下午 07:29
> 至: Kailang
> 副本:  (alsa-devel at alsa-project.org)
> 主旨: Re: hp_pin was NULL value
> 
> 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.