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 09:25:40 +0000	[thread overview]
Message-ID: <6FAB7C47BCF00940BB0999A99BE3547A18421148@RTITMBSV02.realtek.com.tw> (raw)
In-Reply-To: <s5hwon6ffg9.wl-tiwai@suse.de>

Our Windows code just called it at boot time only. SD's programing guide point just need to call one time.
So, I don't know it's harmful or not. I will check with our AE and SD.
I could also test it with S3 resume state.

Many Thanks. Please wait.

-----Original Message-----
From: Takashi Iwai <tiwai@suse.de> 
Sent: Tuesday, January 15, 2019 5:17 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 Tue, 15 Jan 2019 10:06:58 +0100,
Kailang wrote:
> 
> >>>Is it about that alc225_hp_init() gets called at resume in addition to the boot time?
> 
> alc225_hp_init() ==> it only need to call one time at boot time.
> This HP initial code doesn't need to call at resume.
> If system boot, HP initial code just need to call it at boot time.
> S3 state was not need to call this function.

But S4 resume must call this function: that's my point.

So the question is whether calling alc225_hp_init() at S3 resume is significantly harmful or not.  If not, we can move it there.  If it's harmful, we need to distinguish S3 and S4, and introduce either some flag or a new callback, which will be a much more work than simply patching the codec code.

thanks,

Takashi

> Sorry!! Maybe I describe it confuse you.
> 
> -----Original Message-----
> From: Takashi Iwai <tiwai@suse.de>
> Sent: Tuesday, January 15, 2019 4:58 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 Tue, 15 Jan 2019 09:17:24 +0100,
> Kailang wrote:
> > 
> > 
> > Hi Takashi,
> > 
> > Sorry!! Maybe I don't understand what you mean.	
> > Please see below.
> > 
> > --- patch_realtek.c.orig	2019-01-11 16:28:43.745957854 +0800
> > +++ patch_realtek.c	2019-01-15 16:09:56.017873567 +0800
> > @@ -3221,12 +3221,29 @@ static void alc256_shutup(struct hda_cod
> >  	snd_hda_shutup_pins(codec);
> >  }
> >  
> > +static void alc225_hp_init(struct hda_codec *codec) {
> > +	struct alc_spec *spec = codec->spec;
> > +	hda_nid_t hp_pin = spec->gen.autocfg.hp_pins[0];
> > +	int i, val;
> > +	int coef38, coef0d, coef36;
> > +	printk("%s-start hp=0x%x\n",__func__,hp_pin);
> > +	if (!hp_pin)
> > +		return;
> > +
> > +	alc_update_coef_idx(codec, 0x4a, 1<<15, 1<<15);
> > +	msleep(50);
> > +	alc_update_coef_idx(codec, 0x4a, 1<<15, 0);
> > +	printk("%s-end hp=0x%x\n",__func__,hp_pin); }
> > +
> >  static void alc225_init(struct hda_codec *codec)  {
> >  	struct alc_spec *spec = codec->spec;
> >  	hda_nid_t hp_pin = spec->gen.autocfg.hp_pins[0];
> >  	bool hp1_pin_sense, hp2_pin_sense;
> >  
> > +	alc225_hp_init(codec);
> >  	if (!hp_pin)
> >  		return;
> >  
> > @@ -3262,6 +3279,7 @@ static void alc225_init(struct hda_codec
> >  
> >  	alc_update_coef_idx(codec, 0x4a, 3 << 10, 0);
> >  	alc_update_coefex_idx(codec, 0x57, 0x04, 0x0007, 0x4); /* Hight 
> > power */
> > +	printk("%s hp=0x%x\n",__func__,hp_pin);
> >  }
> 
> ... and I don't understand what's your problem.
> 
> Is it about that alc255_hp_init() gets called at resume in addition to the boot time?  If so, this is the intended behavior.
> 
> 
> thanks,
> 
> Takashi
> 
> > 
> > -----Original Message-----
> > From: Takashi Iwai <tiwai@suse.de>
> > Sent: Tuesday, January 15, 2019 3:54 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 Tue, 15 Jan 2019 08:43:40 +0100,
> > Kailang wrote:
> > > 
> > > 
> > > No, it not expected.
> > > It will enter two times after boot ready.
> > > I think it could add a flag in alc225_hp_init(). 
> > > To make sure it only excute one time. How do you think about?
> > 
> > Where was it called, actually?  The call of patch_ops.init is found in several places: once in snd_hda_codec_build_controls() just before creating controls (which is the boot time init), and at resume, alc269_resume().
> > 
> > And, as already explained, we have to call this at resume, because it's not guaranteed that it's been already called in the case of hibernation.
> > 
> > 
> > thanks,
> > 
> > Takashi
> > 
> > > 
> > > -----Original Message-----
> > > From: Takashi Iwai <tiwai@suse.de>
> > > Sent: Tuesday, January 15, 2019 2:37 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 Tue, 15 Jan 2019 03:31:33 +0100, Kailang wrote:
> > > > 
> > > > 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
> > > 
> > > It wasn't clear to me whether you meant this as a success or a failure...  Did the patch work as expected, no?
> > > 
> > > 
> > > thanks,
> > > 
> > > Takashi
> > > 
> > > ------Please consider the environment before printing this e-mail.
> > > 
> > 
> 