[alsa-devel] hp_pin was NULL value

Takashi Iwai tiwai at suse.de
Tue Jan 29 09:28:32 CET 2019


On Tue, 29 Jan 2019 09:05:48 +0100,
Kailang wrote:
> 
> Hi Takashi,
> 
> I ask our AE for this function.
> We also prefer to run it only one time on boot time. S3 resume back not need to run this function.
> Because it will take about 650ms times to initial.
> 
> Attach was the new patch.
> Reject commit or use attach patch to commit. Which is better?

This won't work with S4, unfortunately.

The point of S4 problem is that the switch happens from the
uninitialized state to the already running system via a single resume
callback.  That is, the driver was already initialized in the restored
image although the chip is still uninitialized on the booted image
before the switch.  Then switch happens, and since it keeps
done_hp_init=1 at S4 resume call, the whole trick won't be applied at
all.

I'll check whether we have some indicator showing that the call is in
thaw/restore, or we'd have to implement a separate thaw/restore
callback.


thanks,

Takashi

> 
> BR,
> Kailang
> 
> -----Original Message-----
> From: Takashi Iwai <tiwai at suse.de> 
> Sent: Tuesday, January 15, 2019 5:17 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 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 at suse.de>
> > Sent: Tuesday, January 15, 2019 4:58 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 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 at suse.de>
> > > Sent: Tuesday, January 15, 2019 3:54 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 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 at suse.de>
> > > > Sent: Tuesday, January 15, 2019 2:37 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 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 at suse.de>
> > > > > Cc: (alsa-devel at alsa-project.org) <alsa-devel at 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.
> > > > 
> > > 
> > 
> [2 0000-alc294-hp-init-again.patch <application/octet-stream (base64)>]
> 


More information about the Alsa-devel mailing list