Re: hp_pin was NULL value

From: Kailang <kailang@realtek.com>
To: Takashi Iwai <tiwai@suse.de>
Cc: " (alsa-devel@alsa-project.org)" <alsa-devel@alsa-project.org>
Subject: Re: hp_pin was NULL value
Date: Tue, 15 Jan 2019 02:31:33 +0000	[thread overview]
Message-ID: <6FAB7C47BCF00940BB0999A99BE3547A184210B1@RTITMBSV02.realtek.com.tw> (raw)
In-Reply-To: s5hlg3ux9x9.wl-tiwai@suse.de

Hi Takashi,

Could you receive this email?

BR,
Kailang

-----Original Message-----
From: Kailang 
Sent: Thursday, January 10, 2019 11:14 AM
To: 'Takashi Iwai' <tiwai@suse.de>
Cc: (alsa-devel@alsa-project.org) <alsa-devel@alsa-project.org>
Subject: RE: hp_pin was NULL value

Hi Takashi,

I simulate test your prefer function.
I put alc225_hp_init() in alc225_init().
If system boot ready, I write 1 to power_save of /sys/module/....
The print message as below. It called two times until boot ready.
If System enter to power save, play stream will wake up codec. It also call alc225_hp_init().

[   21.497524] alc225_init hp_pin=0x21
[   21.497526] alc225_hp_init-s hp=0x21
[   22.140076] alc225_hp_init-e hp=0x21
[   22.183496] snd_hda_codec_realtek hdaudioC0D2: Headset jack set to unplugged mode.
[   22.184681] snd_hda_codec_realtek hdaudioC0D2: Headset jack set to unplugged mode.
[   22.186846] alc225_shutup hp_pin=0x21
[   22.187831] input: HDA Intel Line Out as /devices/pci0000:00/0000:00:1b.0/sound/card0/input5
[   22.188055] input: HDA Intel Headphone as /devices/pci0000:00/0000:00:1b.0/sound/card0/input6
[   22.205104] alc225_init hp_pin=0x21
[   22.205108] alc225_hp_init-s hp=0x21
[   22.852123] alc225_hp_init-e hp=0x21
[   22.894772] snd_hda_codec_realtek hdaudioC0D2: Headset jack set to unplugged mode.
############  Boot ready ##############
[  116.832084] alc225_shutup hp_pin=0x21  ==> echo 1 to power_save
[  120.002582] alc225_init hp_pin=0x21     ==> play system sound
[  120.002586] alc225_hp_init-s hp=0x21
[  120.644128] alc225_hp_init-e hp=0x21

Many thanks.
BR,
Kailang

-----Original Message-----
From: Takashi Iwai <tiwai@suse.de>
Sent: Wednesday, January 9, 2019 9:01 PM
To: Kailang <kailang@realtek.com>
Cc: (alsa-devel@alsa-project.org) <alsa-devel@alsa-project.org>
Subject: Re: hp_pin was NULL value

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@suse.de]
> 寄件日期: 2019年1月9日 下午 07:29
> 至: Kailang
> 副本:  (alsa-devel@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@suse.de]
> > 寄件日期: 2019年1月9日 下午 05:56
> > 至: Kailang
> > 副本:  (alsa-devel@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@suse.de>
> > > Sent: Wednesday, January 9, 2019 5:43 PM
> > > To: Kailang <kailang@realtek.com>
> > > Cc: (alsa-devel@alsa-project.org) <alsa-devel@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.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel