[alsa-devel] FW: move eapd coef function before ACT_PRE_PROBE state
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
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)>]
-----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.
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.
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);
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.
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
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.
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);
participants (2)
-
Kailang -
Takashi Iwai