All of lore.kernel.org
 help / color / mirror / Atom feed
* FW: move eapd coef function before ACT_PRE_PROBE state
@ 2019-05-07  9:17 Kailang
  2019-05-07  9:22 ` Takashi Iwai
  0 siblings, 1 reply; 9+ messages in thread
From: Kailang @ 2019-05-07  9:17 UTC (permalink / raw)
  To: Takashi Iwai (tiwai@suse.de); +Cc:  (alsa-devel@alsa-project.org)

[-- Attachment #1: Type: text/plain, Size: 991 bytes --]

Hi Takashi,

This one.

-----Original Message-----
From: Kailang 
Sent: Monday, May 6, 2019 2:46 PM
To: Takashi Iwai (tiwai@suse.de) <tiwai@suse.de>
Cc: (alsa-devel@alsa-project.org) <alsa-devel@alsa-project.org>
Subject: RE: move eapd coef function before ACT_PRE_PROBE state

Hi Takashi,

Are you available for apply this ?
Thanks.

BR,
Kailang

> -----Original Message-----
> From: Kailang
> Sent: Tuesday, April 30, 2019 3:41 PM
> To: Takashi Iwai (tiwai@suse.de) <tiwai@suse.de>
> Cc: (alsa-devel@alsa-project.org) <alsa-devel@alsa-project.org>
> Subject: move eapd coef function before ACT_PRE_PROBE state
> 
> Hi Takashi,
> 
> alc_fill_eapd_coef(),this function was change EAPD control to default.
> Default was set EAPD by verb control.
> This function was run in ACT_INIT state.
> Move it to ACT_PRE_PROBE above. It will have a chance to change EAPD 
> control on ACT_PRE_PROBE state. It could change control by 0x20 coef bit.
> 
> BR,
> Kailang

[-- Attachment #2: 0000-move-eapd-coef-func.patch --]
[-- Type: application/octet-stream, Size: 1327 bytes --]

From 23c9a1c3288c4e78255f133a4d9c48f5842e03e6 Mon Sep 17 00:00:00 2001
From: Kailang Yang <kailang@realtek.com>
Date: Fri, 26 Apr 2019 17:16:48 +0800
Subject: [PATCH] ALSA: hda/realtek - Move EPAD by verb function

alc_fill_eapd_coef(),this function was change EAPD control to default.
Default was set EAPD by verb control.
This function was run in ACT_INIT state.
Move it to ACT_PRE_PROBE above. It will have a chance to change EAPD control
on ACT_PRE_PROBE state. It could change control by 0x20 coef bit.

Signed-off-by: Kailang Yang <kailang@realtek.com>

diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
index 1d0e0f2e6526..7553f704cb7f 100644
--- a/sound/pci/hda/patch_realtek.c
+++ b/sound/pci/hda/patch_realtek.c
@@ -501,7 +501,6 @@ static void alc_eapd_shutup(struct hda_codec *codec)
 /* generic EAPD initialization */
 static void alc_auto_init_amp(struct hda_codec *codec, int type)
 {
-	alc_fill_eapd_coef(codec);
 	alc_auto_setup_eapd(codec, true);
 	alc_write_gpio(codec);
 	switch (type) {
@@ -1085,6 +1084,7 @@ static int alc_alloc_spec(struct hda_codec *codec, hda_nid_t mixer_nid)
 		return -ENOMEM;
 	codec->spec = spec;
 	snd_hda_gen_spec_init(&spec->gen);
+	alc_fill_eapd_coef(codec);
 	spec->gen.mixer_nid = mixer_nid;
 	spec->gen.own_eapd_ctl = 1;
 	codec->single_adc_amp = 1;

[-- Attachment #3: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: FW: move eapd coef function before ACT_PRE_PROBE state
  2019-05-07  9:17 FW: move eapd coef function before ACT_PRE_PROBE state Kailang
@ 2019-05-07  9:22 ` Takashi Iwai
  2019-05-07  9:25   ` Kailang
  2019-05-08  6:59   ` Kailang
  0 siblings, 2 replies; 9+ messages in thread
From: Takashi Iwai @ 2019-05-07  9:22 UTC (permalink / raw)
  To: Kailang; +Cc:  (alsa-devel@alsa-project.org)

On Tue, 07 May 2019 11:17:15 +0200,
Kailang wrote:
> 
> Hi Takashi,
> 
> This one.

I already replied twice.  The patch needs rewrite.
Didn't you get the post?


thanks,

Takashi

> 
> -----Original Message-----
> From: Kailang 
> Sent: Monday, May 6, 2019 2:46 PM
> To: Takashi Iwai (tiwai@suse.de) <tiwai@suse.de>
> Cc: (alsa-devel@alsa-project.org) <alsa-devel@alsa-project.org>
> Subject: RE: move eapd coef function before ACT_PRE_PROBE state
> 
> Hi Takashi,
> 
> Are you available for apply this ?
> Thanks.
> 
> BR,
> Kailang
> 
> > -----Original Message-----
> > From: Kailang
> > Sent: Tuesday, April 30, 2019 3:41 PM
> > To: Takashi Iwai (tiwai@suse.de) <tiwai@suse.de>
> > Cc: (alsa-devel@alsa-project.org) <alsa-devel@alsa-project.org>
> > Subject: move eapd coef function before ACT_PRE_PROBE state
> > 
> > Hi Takashi,
> > 
> > alc_fill_eapd_coef(),this function was change EAPD control to default.
> > Default was set EAPD by verb control.
> > This function was run in ACT_INIT state.
> > Move it to ACT_PRE_PROBE above. It will have a chance to change EAPD 
> > control on ACT_PRE_PROBE state. It could change control by 0x20 coef bit.
> > 
> > BR,
> > Kailang
> [2 0000-move-eapd-coef-func.patch <application/octet-stream (base64)>]
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: FW: move eapd coef function before ACT_PRE_PROBE state
  2019-05-07  9:22 ` Takashi Iwai
@ 2019-05-07  9:25   ` Kailang
  2019-05-08  6:59   ` Kailang
  1 sibling, 0 replies; 9+ messages in thread
From: Kailang @ 2019-05-07  9:25 UTC (permalink / raw)
  To: Takashi Iwai; +Cc:  (alsa-devel@alsa-project.org)



> -----Original Message-----
> From: Takashi Iwai <tiwai@suse.de>
> Sent: Tuesday, May 7, 2019 5:23 PM
> To: Kailang <kailang@realtek.com>
> Cc: (alsa-devel@alsa-project.org) <alsa-devel@alsa-project.org>
> Subject: Re: FW: move eapd coef function before ACT_PRE_PROBE state
> 
> On Tue, 07 May 2019 11:17:15 +0200,
> Kailang wrote:
> >
> > Hi Takashi,
> >
> > This one.
> 
> I already replied twice.  The patch needs rewrite.
> Didn't you get the post?

Yes, I didn't get.
Ok! I rewrite patch again.

> 
> 
> thanks,
> 
> Takashi
> 
> >
> > -----Original Message-----
> > From: Kailang
> > Sent: Monday, May 6, 2019 2:46 PM
> > To: Takashi Iwai (tiwai@suse.de) <tiwai@suse.de>
> > Cc: (alsa-devel@alsa-project.org) <alsa-devel@alsa-project.org>
> > Subject: RE: move eapd coef function before ACT_PRE_PROBE state
> >
> > Hi Takashi,
> >
> > Are you available for apply this ?
> > Thanks.
> >
> > BR,
> > Kailang
> >
> > > -----Original Message-----
> > > From: Kailang
> > > Sent: Tuesday, April 30, 2019 3:41 PM
> > > To: Takashi Iwai (tiwai@suse.de) <tiwai@suse.de>
> > > Cc: (alsa-devel@alsa-project.org) <alsa-devel@alsa-project.org>
> > > Subject: move eapd coef function before ACT_PRE_PROBE state
> > >
> > > Hi Takashi,
> > >
> > > alc_fill_eapd_coef(),this function was change EAPD control to default.
> > > Default was set EAPD by verb control.
> > > This function was run in ACT_INIT state.
> > > Move it to ACT_PRE_PROBE above. It will have a chance to change EAPD
> > > control on ACT_PRE_PROBE state. It could change control by 0x20 coef bit.
> > >
> > > BR,
> > > Kailang
> > [2 0000-move-eapd-coef-func.patch <application/octet-stream (base64)>]
> >
> 
> ------Please consider the environment before printing this e-mail.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: FW: move eapd coef function before ACT_PRE_PROBE state
  2019-05-07  9:22 ` Takashi Iwai
  2019-05-07  9:25   ` Kailang
@ 2019-05-08  6:59   ` Kailang
  2019-05-08  7:28     ` Takashi Iwai
  1 sibling, 1 reply; 9+ messages in thread
From: Kailang @ 2019-05-08  6:59 UTC (permalink / raw)
  To: Takashi Iwai; +Cc:  (alsa-devel@alsa-project.org)

[-- Attachment #1: Type: text/plain, Size: 1821 bytes --]

Hi Takashi,

I recreate patch as attach.
Thanks.

BR,
Kailang

> -----Original Message-----
> From: Takashi Iwai <tiwai@suse.de>
> Sent: Tuesday, May 7, 2019 5:23 PM
> To: Kailang <kailang@realtek.com>
> Cc: (alsa-devel@alsa-project.org) <alsa-devel@alsa-project.org>
> Subject: Re: FW: move eapd coef function before ACT_PRE_PROBE state
> 
> On Tue, 07 May 2019 11:17:15 +0200,
> Kailang wrote:
> >
> > Hi Takashi,
> >
> > This one.
> 
> I already replied twice.  The patch needs rewrite.
> Didn't you get the post?
> 
> 
> thanks,
> 
> Takashi
> 
> >
> > -----Original Message-----
> > From: Kailang
> > Sent: Monday, May 6, 2019 2:46 PM
> > To: Takashi Iwai (tiwai@suse.de) <tiwai@suse.de>
> > Cc: (alsa-devel@alsa-project.org) <alsa-devel@alsa-project.org>
> > Subject: RE: move eapd coef function before ACT_PRE_PROBE state
> >
> > Hi Takashi,
> >
> > Are you available for apply this ?
> > Thanks.
> >
> > BR,
> > Kailang
> >
> > > -----Original Message-----
> > > From: Kailang
> > > Sent: Tuesday, April 30, 2019 3:41 PM
> > > To: Takashi Iwai (tiwai@suse.de) <tiwai@suse.de>
> > > Cc: (alsa-devel@alsa-project.org) <alsa-devel@alsa-project.org>
> > > Subject: move eapd coef function before ACT_PRE_PROBE state
> > >
> > > Hi Takashi,
> > >
> > > alc_fill_eapd_coef(),this function was change EAPD control to default.
> > > Default was set EAPD by verb control.
> > > This function was run in ACT_INIT state.
> > > Move it to ACT_PRE_PROBE above. It will have a chance to change EAPD
> > > control on ACT_PRE_PROBE state. It could change control by 0x20 coef bit.
> > >
> > > BR,
> > > Kailang
> > [2 0000-move-eapd-coef-func.patch <application/octet-stream (base64)>]
> >
> 
> ------Please consider the environment before printing this e-mail.

[-- Attachment #2: 0001-move-eapd-coef-func.patch --]
[-- Type: application/octet-stream, Size: 1326 bytes --]

From 835219a02a6ae1a07f6f6e028fc4641edc7a3631 Mon Sep 17 00:00:00 2001
From: Kailang Yang <kailang@realtek.com>
Date: Wed, 8 May 2019 14:54:51 +0800
Subject: [PATCH] ALSA: hda/realtek - Move EPAD by verb function

alc_fill_eapd_coef(),this function was change EAPD control to default.
Default was set EAPD by verb control.
This function was run in ACT_INIT state.
Move it to ACT_PRE_PROBE above. It will have a chance to change EAPD control
on ACT_PRE_PROBE state. It could change control by 0x20 coef bit.

Signed-off-by: Kailang Yang <kailang@realtek.com>

diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
index dacccacb5fe0..de23b8ad932e 100644
--- a/sound/pci/hda/patch_realtek.c
+++ b/sound/pci/hda/patch_realtek.c
@@ -502,7 +502,6 @@ static void alc_eapd_shutup(struct hda_codec *codec)
 /* generic EAPD initialization */
 static void alc_auto_init_amp(struct hda_codec *codec, int type)
 {
-	alc_fill_eapd_coef(codec);
 	alc_auto_setup_eapd(codec, true);
 	alc_write_gpio(codec);
 	switch (type) {
@@ -1086,6 +1085,7 @@ static int alc_alloc_spec(struct hda_codec *codec, hda_nid_t mixer_nid)
 		return -ENOMEM;
 	codec->spec = spec;
 	snd_hda_gen_spec_init(&spec->gen);
+	alc_fill_eapd_coef(codec);
 	spec->gen.mixer_nid = mixer_nid;
 	spec->gen.own_eapd_ctl = 1;
 	codec->single_adc_amp = 1;

[-- Attachment #3: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: FW: move eapd coef function before ACT_PRE_PROBE state
  2019-05-08  6:59   ` Kailang
@ 2019-05-08  7:28     ` Takashi Iwai
  2019-05-08  9:17       ` Kailang
  0 siblings, 1 reply; 9+ messages in thread
From: Takashi Iwai @ 2019-05-08  7:28 UTC (permalink / raw)
  To: Kailang; +Cc:  (alsa-devel@alsa-project.org)

On Wed, 08 May 2019 08:59:02 +0200,
Kailang wrote:
> 
> Hi Takashi,
> 
> I recreate patch as attach.

No, no, it's not what I meant.  I already *reviewed* and replied your
patch.

I copy my previous reply once again.  Please read and test it.

===

Unfortuantely, moving this doesn't suffice.  There is the hibernation
resume that needs the explicit initialization again.

Also, calling this in alc_alloc_spec() isn't intuitive.  Although it'd
become a larger patch, I prefer making it more explicit, e.g. creating
alc_pre_init() function handling the pre-init procedure and call it
from appropriate places.

So I can imagine a patch like below.  Does it work for you?


thanks,

Takashi

--- a/sound/pci/hda/patch_realtek.c
+++ b/sound/pci/hda/patch_realtek.c
@@ -501,7 +501,6 @@ static void alc_eapd_shutup(struct hda_codec *codec)
 /* generic EAPD initialization */
 static void alc_auto_init_amp(struct hda_codec *codec, int type)
 {
-	alc_fill_eapd_coef(codec);
 	alc_auto_setup_eapd(codec, true);
 	alc_write_gpio(codec);
 	switch (type) {
@@ -796,10 +795,22 @@ static int alc_build_controls(struct hda_codec *codec)
  * Common callbacks
  */
 
+static void alc_pre_init(struct hda_codec *codec)
+{
+	alc_fill_eapd_coef(codec);
+}
+
+#define is_s4_resume(codec) \
+	((codec)->core.dev.power.power_state.event == PM_EVENT_RESTORE)
+
 static int alc_init(struct hda_codec *codec)
 {
 	struct alc_spec *spec = codec->spec;
 
+	/* hibernation resume needs the full chip initialization */
+	if (is_s4_resume(codec))
+		alc_pre_init(codec);
+
 	if (spec->init_hook)
 		spec->init_hook(codec);
 
@@ -1537,6 +1548,8 @@ static int patch_alc880(struct hda_codec *codec)
 
 	codec->patch_ops.unsol_event = alc880_unsol_event;
 
+	alc_pre_init(codec);
+
 	snd_hda_pick_fixup(codec, alc880_fixup_models, alc880_fixup_tbl,
 		       alc880_fixups);
 	snd_hda_apply_fixup(codec, HDA_FIXUP_ACT_PRE_PROBE);
@@ -1788,6 +1801,8 @@ static int patch_alc260(struct hda_codec *codec)
 
 	spec->shutup = alc_eapd_shutup;
 
+	alc_pre_init(codec);
+
 	snd_hda_pick_fixup(codec, alc260_fixup_models, alc260_fixup_tbl,
 			   alc260_fixups);
 	snd_hda_apply_fixup(codec, HDA_FIXUP_ACT_PRE_PROBE);
@@ -2491,6 +2506,8 @@ static int patch_alc882(struct hda_codec *codec)
 		break;
 	}
 
+	alc_pre_init(codec);
+
 	snd_hda_pick_fixup(codec, alc882_fixup_models, alc882_fixup_tbl,
 		       alc882_fixups);
 	snd_hda_apply_fixup(codec, HDA_FIXUP_ACT_PRE_PROBE);
@@ -2665,6 +2682,8 @@ static int patch_alc262(struct hda_codec *codec)
 #endif
 	alc_fix_pll_init(codec, 0x20, 0x0a, 10);
 
+	alc_pre_init(codec);
+
 	snd_hda_pick_fixup(codec, alc262_fixup_models, alc262_fixup_tbl,
 		       alc262_fixups);
 	snd_hda_apply_fixup(codec, HDA_FIXUP_ACT_PRE_PROBE);
@@ -2809,6 +2828,8 @@ static int patch_alc268(struct hda_codec *codec)
 
 	spec->shutup = alc_eapd_shutup;
 
+	alc_pre_init(codec);
+
 	snd_hda_pick_fixup(codec, alc268_fixup_models, alc268_fixup_tbl, alc268_fixups);
 	snd_hda_apply_fixup(codec, HDA_FIXUP_ACT_PRE_PROBE);
 
@@ -7768,6 +7789,8 @@ static int patch_alc269(struct hda_codec *codec)
 		spec->init_hook = alc5505_dsp_init;
 	}
 
+	alc_pre_init(codec);
+
 	snd_hda_pick_fixup(codec, alc269_fixup_models,
 		       alc269_fixup_tbl, alc269_fixups);
 	snd_hda_pick_pin_fixup(codec, alc269_pin_fixup_tbl, alc269_fixups);
@@ -7910,6 +7933,8 @@ static int patch_alc861(struct hda_codec *codec)
 	spec->power_hook = alc_power_eapd;
 #endif
 
+	alc_pre_init(codec);
+
 	snd_hda_pick_fixup(codec, NULL, alc861_fixup_tbl, alc861_fixups);
 	snd_hda_apply_fixup(codec, HDA_FIXUP_ACT_PRE_PROBE);
 
@@ -8007,6 +8032,8 @@ static int patch_alc861vd(struct hda_codec *codec)
 
 	spec->shutup = alc_eapd_shutup;
 
+	alc_pre_init(codec);
+
 	snd_hda_pick_fixup(codec, NULL, alc861vd_fixup_tbl, alc861vd_fixups);
 	snd_hda_apply_fixup(codec, HDA_FIXUP_ACT_PRE_PROBE);
 
@@ -8742,6 +8769,8 @@ static int patch_alc662(struct hda_codec *codec)
 		break;
 	}
 
+	alc_pre_init(codec);
+
 	snd_hda_pick_fixup(codec, alc662_fixup_models,
 		       alc662_fixup_tbl, alc662_fixups);
 	snd_hda_pick_pin_fixup(codec, alc662_pin_fixup_tbl, alc662_fixups);

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: FW: move eapd coef function before ACT_PRE_PROBE state
  2019-05-08  7:28     ` Takashi Iwai
@ 2019-05-08  9:17       ` Kailang
  2019-05-08  9:20         ` Takashi Iwai
  0 siblings, 1 reply; 9+ messages in thread
From: Kailang @ 2019-05-08  9:17 UTC (permalink / raw)
  To: Takashi Iwai; +Cc:  (alsa-devel@alsa-project.org)

Sorry!! I didn't get the mail.

> -----Original Message-----
> From: Takashi Iwai <tiwai@suse.de>
> Sent: Wednesday, May 8, 2019 3:28 PM
> To: Kailang <kailang@realtek.com>
> Cc: (alsa-devel@alsa-project.org) <alsa-devel@alsa-project.org>
> Subject: Re: FW: move eapd coef function before ACT_PRE_PROBE state
> 
> On Wed, 08 May 2019 08:59:02 +0200,
> Kailang wrote:
> >
> > Hi Takashi,
> >
> > I recreate patch as attach.
> 
> No, no, it's not what I meant.  I already *reviewed* and replied your patch.
> 
> I copy my previous reply once again.  Please read and test it.

Patch fail.

patch: **** malformed patch at line 10: @@ -796,10 +795,22 @@ static int alc_build_controls(struct hda_codec *codec)

> 
> ===
> 
> Unfortuantely, moving this doesn't suffice.  There is the hibernation resume
> that needs the explicit initialization again.
> 
> Also, calling this in alc_alloc_spec() isn't intuitive.  Although it'd become a
> larger patch, I prefer making it more explicit, e.g. creating
> alc_pre_init() function handling the pre-init procedure and call it from
> appropriate places.
> 
> So I can imagine a patch like below.  Does it work for you?
> 
> 
> thanks,
> 
> Takashi
> 
> --- a/sound/pci/hda/patch_realtek.c
> +++ b/sound/pci/hda/patch_realtek.c
> @@ -501,7 +501,6 @@ static void alc_eapd_shutup(struct hda_codec *codec)
>  /* generic EAPD initialization */
>  static void alc_auto_init_amp(struct hda_codec *codec, int type)  {
> -	alc_fill_eapd_coef(codec);
>  	alc_auto_setup_eapd(codec, true);
>  	alc_write_gpio(codec);
>  	switch (type) {
> @@ -796,10 +795,22 @@ static int alc_build_controls(struct hda_codec
> *codec)
>   * Common callbacks
>   */
> 
> +static void alc_pre_init(struct hda_codec *codec) {
> +	alc_fill_eapd_coef(codec);
> +}
> +
> +#define is_s4_resume(codec) \
> +	((codec)->core.dev.power.power_state.event == PM_EVENT_RESTORE)
> +
>  static int alc_init(struct hda_codec *codec)  {
>  	struct alc_spec *spec = codec->spec;
> 
> +	/* hibernation resume needs the full chip initialization */
> +	if (is_s4_resume(codec))
> +		alc_pre_init(codec);
> +
>  	if (spec->init_hook)
>  		spec->init_hook(codec);
> 
> @@ -1537,6 +1548,8 @@ static int patch_alc880(struct hda_codec *codec)
> 
>  	codec->patch_ops.unsol_event = alc880_unsol_event;
> 
> +	alc_pre_init(codec);
> +
>  	snd_hda_pick_fixup(codec, alc880_fixup_models, alc880_fixup_tbl,
>  		       alc880_fixups);
>  	snd_hda_apply_fixup(codec, HDA_FIXUP_ACT_PRE_PROBE); @@ -1788,6
> +1801,8 @@ static int patch_alc260(struct hda_codec *codec)
> 
>  	spec->shutup = alc_eapd_shutup;
> 
> +	alc_pre_init(codec);
> +
>  	snd_hda_pick_fixup(codec, alc260_fixup_models, alc260_fixup_tbl,
>  			   alc260_fixups);
>  	snd_hda_apply_fixup(codec, HDA_FIXUP_ACT_PRE_PROBE); @@ -2491,6
> +2506,8 @@ static int patch_alc882(struct hda_codec *codec)
>  		break;
>  	}
> 
> +	alc_pre_init(codec);
> +
>  	snd_hda_pick_fixup(codec, alc882_fixup_models, alc882_fixup_tbl,
>  		       alc882_fixups);
>  	snd_hda_apply_fixup(codec, HDA_FIXUP_ACT_PRE_PROBE); @@ -2665,6
> +2682,8 @@ static int patch_alc262(struct hda_codec *codec)  #endif
>  	alc_fix_pll_init(codec, 0x20, 0x0a, 10);
> 
> +	alc_pre_init(codec);
> +
>  	snd_hda_pick_fixup(codec, alc262_fixup_models, alc262_fixup_tbl,
>  		       alc262_fixups);
>  	snd_hda_apply_fixup(codec, HDA_FIXUP_ACT_PRE_PROBE); @@ -2809,6
> +2828,8 @@ static int patch_alc268(struct hda_codec *codec)
> 
>  	spec->shutup = alc_eapd_shutup;
> 
> +	alc_pre_init(codec);
> +
>  	snd_hda_pick_fixup(codec, alc268_fixup_models, alc268_fixup_tbl,
> alc268_fixups);
>  	snd_hda_apply_fixup(codec, HDA_FIXUP_ACT_PRE_PROBE);
> 
> @@ -7768,6 +7789,8 @@ static int patch_alc269(struct hda_codec *codec)
>  		spec->init_hook = alc5505_dsp_init;
>  	}
> 
> +	alc_pre_init(codec);
> +
>  	snd_hda_pick_fixup(codec, alc269_fixup_models,
>  		       alc269_fixup_tbl, alc269_fixups);
>  	snd_hda_pick_pin_fixup(codec, alc269_pin_fixup_tbl, alc269_fixups); @@
> -7910,6 +7933,8 @@ static int patch_alc861(struct hda_codec *codec)
>  	spec->power_hook = alc_power_eapd;
>  #endif
> 
> +	alc_pre_init(codec);
> +
>  	snd_hda_pick_fixup(codec, NULL, alc861_fixup_tbl, alc861_fixups);
>  	snd_hda_apply_fixup(codec, HDA_FIXUP_ACT_PRE_PROBE);
> 
> @@ -8007,6 +8032,8 @@ static int patch_alc861vd(struct hda_codec *codec)
> 
>  	spec->shutup = alc_eapd_shutup;
> 
> +	alc_pre_init(codec);
> +
>  	snd_hda_pick_fixup(codec, NULL, alc861vd_fixup_tbl, alc861vd_fixups);
>  	snd_hda_apply_fixup(codec, HDA_FIXUP_ACT_PRE_PROBE);
> 
> @@ -8742,6 +8769,8 @@ static int patch_alc662(struct hda_codec *codec)
>  		break;
>  	}
> 
> +	alc_pre_init(codec);
> +
>  	snd_hda_pick_fixup(codec, alc662_fixup_models,
>  		       alc662_fixup_tbl, alc662_fixups);
>  	snd_hda_pick_pin_fixup(codec, alc662_pin_fixup_tbl, alc662_fixups);
> 
> 
> ------Please consider the environment before printing this e-mail.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: FW: move eapd coef function before ACT_PRE_PROBE state
  2019-05-08  9:17       ` Kailang
@ 2019-05-08  9:20         ` Takashi Iwai
  2019-05-10  7:24           ` Kailang
  0 siblings, 1 reply; 9+ messages in thread
From: Takashi Iwai @ 2019-05-08  9:20 UTC (permalink / raw)
  To: Kailang; +Cc:  (alsa-devel@alsa-project.org)

[-- Attachment #1: Type: text/plain, Size: 866 bytes --]

On Wed, 08 May 2019 11:17:30 +0200,
Kailang wrote:
> 
> Sorry!! I didn't get the mail.
> 
> > -----Original Message-----
> > From: Takashi Iwai <tiwai@suse.de>
> > Sent: Wednesday, May 8, 2019 3:28 PM
> > To: Kailang <kailang@realtek.com>
> > Cc: (alsa-devel@alsa-project.org) <alsa-devel@alsa-project.org>
> > Subject: Re: FW: move eapd coef function before ACT_PRE_PROBE state
> > 
> > On Wed, 08 May 2019 08:59:02 +0200,
> > Kailang wrote:
> > >
> > > Hi Takashi,
> > >
> > > I recreate patch as attach.
> > 
> > No, no, it's not what I meant.  I already *reviewed* and replied your patch.
> > 
> > I copy my previous reply once again.  Please read and test it.
> 
> Patch fail.
> 
> patch: **** malformed patch at line 10: @@ -796,10 +795,22 @@ static int alc_build_controls(struct hda_codec *codec)

Some mailer problem...  Use the attachment below.


Takashi


[-- Attachment #2: realtek-pre-init.diff --]
[-- Type: application/octet-stream, Size: 3544 bytes --]

diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
index c53ca589c930..138d448a58dc 100644
--- a/sound/pci/hda/patch_realtek.c
+++ b/sound/pci/hda/patch_realtek.c
@@ -502,7 +502,6 @@ static void alc_eapd_shutup(struct hda_codec *codec)
 /* generic EAPD initialization */
 static void alc_auto_init_amp(struct hda_codec *codec, int type)
 {
-	alc_fill_eapd_coef(codec);
 	alc_auto_setup_eapd(codec, true);
 	alc_write_gpio(codec);
 	switch (type) {
@@ -797,10 +796,22 @@ static int alc_build_controls(struct hda_codec *codec)
  * Common callbacks
  */
 
+static void alc_pre_init(struct hda_codec *codec)
+{
+	alc_fill_eapd_coef(codec);
+}
+
+#define is_s4_resume(codec) \
+	((codec)->core.dev.power.power_state.event == PM_EVENT_RESTORE)
+
 static int alc_init(struct hda_codec *codec)
 {
 	struct alc_spec *spec = codec->spec;
 
+	/* hibernation resume needs the full chip initialization */
+	if (is_s4_resume(codec))
+		alc_pre_init(codec);
+
 	if (spec->init_hook)
 		spec->init_hook(codec);
 
@@ -1538,6 +1549,8 @@ static int patch_alc880(struct hda_codec *codec)
 
 	codec->patch_ops.unsol_event = alc880_unsol_event;
 
+	alc_pre_init(codec);
+
 	snd_hda_pick_fixup(codec, alc880_fixup_models, alc880_fixup_tbl,
 		       alc880_fixups);
 	snd_hda_apply_fixup(codec, HDA_FIXUP_ACT_PRE_PROBE);
@@ -1789,6 +1802,8 @@ static int patch_alc260(struct hda_codec *codec)
 
 	spec->shutup = alc_eapd_shutup;
 
+	alc_pre_init(codec);
+
 	snd_hda_pick_fixup(codec, alc260_fixup_models, alc260_fixup_tbl,
 			   alc260_fixups);
 	snd_hda_apply_fixup(codec, HDA_FIXUP_ACT_PRE_PROBE);
@@ -2492,6 +2507,8 @@ static int patch_alc882(struct hda_codec *codec)
 		break;
 	}
 
+	alc_pre_init(codec);
+
 	snd_hda_pick_fixup(codec, alc882_fixup_models, alc882_fixup_tbl,
 		       alc882_fixups);
 	snd_hda_apply_fixup(codec, HDA_FIXUP_ACT_PRE_PROBE);
@@ -2666,6 +2683,8 @@ static int patch_alc262(struct hda_codec *codec)
 #endif
 	alc_fix_pll_init(codec, 0x20, 0x0a, 10);
 
+	alc_pre_init(codec);
+
 	snd_hda_pick_fixup(codec, alc262_fixup_models, alc262_fixup_tbl,
 		       alc262_fixups);
 	snd_hda_apply_fixup(codec, HDA_FIXUP_ACT_PRE_PROBE);
@@ -2810,6 +2829,8 @@ static int patch_alc268(struct hda_codec *codec)
 
 	spec->shutup = alc_eapd_shutup;
 
+	alc_pre_init(codec);
+
 	snd_hda_pick_fixup(codec, alc268_fixup_models, alc268_fixup_tbl, alc268_fixups);
 	snd_hda_apply_fixup(codec, HDA_FIXUP_ACT_PRE_PROBE);
 
@@ -7805,6 +7826,8 @@ static int patch_alc269(struct hda_codec *codec)
 		spec->init_hook = alc5505_dsp_init;
 	}
 
+	alc_pre_init(codec);
+
 	snd_hda_pick_fixup(codec, alc269_fixup_models,
 		       alc269_fixup_tbl, alc269_fixups);
 	snd_hda_pick_pin_fixup(codec, alc269_pin_fixup_tbl, alc269_fixups);
@@ -7947,6 +7970,8 @@ static int patch_alc861(struct hda_codec *codec)
 	spec->power_hook = alc_power_eapd;
 #endif
 
+	alc_pre_init(codec);
+
 	snd_hda_pick_fixup(codec, NULL, alc861_fixup_tbl, alc861_fixups);
 	snd_hda_apply_fixup(codec, HDA_FIXUP_ACT_PRE_PROBE);
 
@@ -8044,6 +8069,8 @@ static int patch_alc861vd(struct hda_codec *codec)
 
 	spec->shutup = alc_eapd_shutup;
 
+	alc_pre_init(codec);
+
 	snd_hda_pick_fixup(codec, NULL, alc861vd_fixup_tbl, alc861vd_fixups);
 	snd_hda_apply_fixup(codec, HDA_FIXUP_ACT_PRE_PROBE);
 
@@ -8779,6 +8806,8 @@ static int patch_alc662(struct hda_codec *codec)
 		break;
 	}
 
+	alc_pre_init(codec);
+
 	snd_hda_pick_fixup(codec, alc662_fixup_models,
 		       alc662_fixup_tbl, alc662_fixups);
 	snd_hda_pick_pin_fixup(codec, alc662_pin_fixup_tbl, alc662_fixups);

[-- Attachment #3: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: FW: move eapd coef function before ACT_PRE_PROBE state
  2019-05-08  9:20         ` Takashi Iwai
@ 2019-05-10  7:24           ` Kailang
  2019-05-10  9:20             ` Takashi Iwai
  0 siblings, 1 reply; 9+ messages in thread
From: Kailang @ 2019-05-10  7:24 UTC (permalink / raw)
  To: Takashi Iwai; +Cc:  (alsa-devel@alsa-project.org)

Hi Takashi,

The patch was work for me.
Thanks.

BR,
Kailang

> -----Original Message-----
> From: Takashi Iwai <tiwai@suse.de>
> Sent: Wednesday, May 8, 2019 5:20 PM
> To: Kailang <kailang@realtek.com>
> Cc: (alsa-devel@alsa-project.org) <alsa-devel@alsa-project.org>
> Subject: Re: FW: move eapd coef function before ACT_PRE_PROBE state
> 
> On Wed, 08 May 2019 11:17:30 +0200,
> Kailang wrote:
> >
> > Sorry!! I didn't get the mail.
> >
> > > -----Original Message-----
> > > From: Takashi Iwai <tiwai@suse.de>
> > > Sent: Wednesday, May 8, 2019 3:28 PM
> > > To: Kailang <kailang@realtek.com>
> > > Cc: (alsa-devel@alsa-project.org) <alsa-devel@alsa-project.org>
> > > Subject: Re: FW: move eapd coef function before ACT_PRE_PROBE state
> > >
> > > On Wed, 08 May 2019 08:59:02 +0200,
> > > Kailang wrote:
> > > >
> > > > Hi Takashi,
> > > >
> > > > I recreate patch as attach.
> > >
> > > No, no, it's not what I meant.  I already *reviewed* and replied your
> patch.
> > >
> > > I copy my previous reply once again.  Please read and test it.
> >
> > Patch fail.
> >
> > patch: **** malformed patch at line 10: @@ -796,10 +795,22 @@ static int
> alc_build_controls(struct hda_codec *codec)
> 
> Some mailer problem...  Use the attachment below.
> 
> 
> Takashi
> 
> 
> ------Please consider the environment before printing this e-mail.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: FW: move eapd coef function before ACT_PRE_PROBE state
  2019-05-10  7:24           ` Kailang
@ 2019-05-10  9:20             ` Takashi Iwai
  0 siblings, 0 replies; 9+ messages in thread
From: Takashi Iwai @ 2019-05-10  9:20 UTC (permalink / raw)
  To: Kailang; +Cc:  (alsa-devel@alsa-project.org)

On Fri, 10 May 2019 09:24:04 +0200,
Kailang wrote:
> 
> Hi Takashi,
> 
> The patch was work for me.

OK, I'm going to apply the patch below.


thanks,

Takashi

-- 8< --
From: Takashi Iwai <tiwai@suse.de>
Subject: [PATCH] ALSA: hda/realtek - Avoid superfluous COEF EAPD setups

Realtek codec driver applied the COEF setups to change the EAPD
control to the default mode (i.e. control by EPAD verbs) at the init
callback.  It works, but this is too excessive at the same time, since
it's called at each runtime PM resume.  That is, the initialization
should be done only once after the probe.  One may think that moving
this to the probe should be OK, but no -- there is a catch; when a
system resumes from S4 (hibernation), we need to re-initialize this
again manually, because it's out of regcache restoration.

This patch addresses the issue by introducing alc_pre_init() function
that performs such a task.  This is called from each codec probe
function, and it's called from the resume callback conditionally only
from S4 resume.

Reported-and-tested-by: Kailang Yang <kailang@realtek.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/pci/hda/patch_realtek.c | 31 ++++++++++++++++++++++++++++++-
 1 file changed, 30 insertions(+), 1 deletion(-)

diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
index c39f48e02ee9..2a50e580aa56 100644
--- a/sound/pci/hda/patch_realtek.c
+++ b/sound/pci/hda/patch_realtek.c
@@ -535,7 +535,6 @@ static void alc_eapd_shutup(struct hda_codec *codec)
 /* generic EAPD initialization */
 static void alc_auto_init_amp(struct hda_codec *codec, int type)
 {
-	alc_fill_eapd_coef(codec);
 	alc_auto_setup_eapd(codec, true);
 	alc_write_gpio(codec);
 	switch (type) {
@@ -830,10 +829,22 @@ static int alc_build_controls(struct hda_codec *codec)
  * Common callbacks
  */
 
+static void alc_pre_init(struct hda_codec *codec)
+{
+	alc_fill_eapd_coef(codec);
+}
+
+#define is_s4_resume(codec) \
+	((codec)->core.dev.power.power_state.event == PM_EVENT_RESTORE)
+
 static int alc_init(struct hda_codec *codec)
 {
 	struct alc_spec *spec = codec->spec;
 
+	/* hibernation resume needs the full chip initialization */
+	if (is_s4_resume(codec))
+		alc_pre_init(codec);
+
 	if (spec->init_hook)
 		spec->init_hook(codec);
 
@@ -1571,6 +1582,8 @@ static int patch_alc880(struct hda_codec *codec)
 
 	codec->patch_ops.unsol_event = alc880_unsol_event;
 
+	alc_pre_init(codec);
+
 	snd_hda_pick_fixup(codec, alc880_fixup_models, alc880_fixup_tbl,
 		       alc880_fixups);
 	snd_hda_apply_fixup(codec, HDA_FIXUP_ACT_PRE_PROBE);
@@ -1822,6 +1835,8 @@ static int patch_alc260(struct hda_codec *codec)
 
 	spec->shutup = alc_eapd_shutup;
 
+	alc_pre_init(codec);
+
 	snd_hda_pick_fixup(codec, alc260_fixup_models, alc260_fixup_tbl,
 			   alc260_fixups);
 	snd_hda_apply_fixup(codec, HDA_FIXUP_ACT_PRE_PROBE);
@@ -2525,6 +2540,8 @@ static int patch_alc882(struct hda_codec *codec)
 		break;
 	}
 
+	alc_pre_init(codec);
+
 	snd_hda_pick_fixup(codec, alc882_fixup_models, alc882_fixup_tbl,
 		       alc882_fixups);
 	snd_hda_apply_fixup(codec, HDA_FIXUP_ACT_PRE_PROBE);
@@ -2699,6 +2716,8 @@ static int patch_alc262(struct hda_codec *codec)
 #endif
 	alc_fix_pll_init(codec, 0x20, 0x0a, 10);
 
+	alc_pre_init(codec);
+
 	snd_hda_pick_fixup(codec, alc262_fixup_models, alc262_fixup_tbl,
 		       alc262_fixups);
 	snd_hda_apply_fixup(codec, HDA_FIXUP_ACT_PRE_PROBE);
@@ -2843,6 +2862,8 @@ static int patch_alc268(struct hda_codec *codec)
 
 	spec->shutup = alc_eapd_shutup;
 
+	alc_pre_init(codec);
+
 	snd_hda_pick_fixup(codec, alc268_fixup_models, alc268_fixup_tbl, alc268_fixups);
 	snd_hda_apply_fixup(codec, HDA_FIXUP_ACT_PRE_PROBE);
 
@@ -7816,6 +7837,8 @@ static int patch_alc269(struct hda_codec *codec)
 		spec->init_hook = alc5505_dsp_init;
 	}
 
+	alc_pre_init(codec);
+
 	snd_hda_pick_fixup(codec, alc269_fixup_models,
 		       alc269_fixup_tbl, alc269_fixups);
 	snd_hda_pick_pin_fixup(codec, alc269_pin_fixup_tbl, alc269_fixups);
@@ -7958,6 +7981,8 @@ static int patch_alc861(struct hda_codec *codec)
 	spec->power_hook = alc_power_eapd;
 #endif
 
+	alc_pre_init(codec);
+
 	snd_hda_pick_fixup(codec, NULL, alc861_fixup_tbl, alc861_fixups);
 	snd_hda_apply_fixup(codec, HDA_FIXUP_ACT_PRE_PROBE);
 
@@ -8055,6 +8080,8 @@ static int patch_alc861vd(struct hda_codec *codec)
 
 	spec->shutup = alc_eapd_shutup;
 
+	alc_pre_init(codec);
+
 	snd_hda_pick_fixup(codec, NULL, alc861vd_fixup_tbl, alc861vd_fixups);
 	snd_hda_apply_fixup(codec, HDA_FIXUP_ACT_PRE_PROBE);
 
@@ -8790,6 +8817,8 @@ static int patch_alc662(struct hda_codec *codec)
 		break;
 	}
 
+	alc_pre_init(codec);
+
 	snd_hda_pick_fixup(codec, alc662_fixup_models,
 		       alc662_fixup_tbl, alc662_fixups);
 	snd_hda_pick_pin_fixup(codec, alc662_pin_fixup_tbl, alc662_fixups);
-- 
2.16.4

^ permalink raw reply related	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2019-05-10  9:20 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-07  9:17 FW: move eapd coef function before ACT_PRE_PROBE state Kailang
2019-05-07  9:22 ` Takashi Iwai
2019-05-07  9:25   ` Kailang
2019-05-08  6:59   ` Kailang
2019-05-08  7:28     ` Takashi Iwai
2019-05-08  9:17       ` Kailang
2019-05-08  9:20         ` Takashi Iwai
2019-05-10  7:24           ` Kailang
2019-05-10  9:20             ` Takashi Iwai

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.