[PATCH] Make top/rear speaker, mute and micmute leds work on HP x360 14-ea000 laptops that use Realtek 245 codec

[PATCH] Make top/rear speaker, mute and micmute leds work on HP x360 14-ea000 laptops that use Realtek 245 codec

[Jonathan Clarke]

BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=210633
Signed-off-by: Jonathan Clarke <jonathan.a.clarke@gmail.com>
---
 sound/pci/hda/patch_realtek.c | 46 +++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
index 22d27b12c..e3c6d17ea 100644
--- a/sound/pci/hda/patch_realtek.c
+++ b/sound/pci/hda/patch_realtek.c
@@ -4409,6 +4409,13 @@ static void alc245_fixup_hp_x360_amp(struct hda_codec *codec,
 	case HDA_FIXUP_ACT_PRE_PROBE:
 		spec->gpio_mask |= 0x01;
 		spec->gpio_dir |= 0x01;
+
+		/* use only amp at 0x02 for bottom(front) speaker,
+		 * otherwise it is set to use 0x02,0x03,0x06 and when used in conjunction
+		 * with top(rear) speaker 0x14, gets locked at full volume */
+		static const hda_nid_t conn1[] = { 0x02 };
+		snd_hda_override_conn_list(codec, 0x17, ARRAY_SIZE(conn1), conn1);
+
 		break;
 	case HDA_FIXUP_ACT_INIT:
 		/* need to toggle GPIO to enable the amp */
@@ -4503,6 +4510,26 @@ static void alc236_fixup_hp_mute_led_coefbit(struct hda_codec *codec,
 	}
 }
 
+static void alc245_fixup_hp_x360_mute_leds(struct hda_codec *codec,
+				const struct hda_fixup *fix, int action)
+{
+	struct alc_spec *spec = codec->spec;
+	if (action == HDA_FIXUP_ACT_PRE_PROBE) {
+		/* mic mute is set via gpio 0x04 */
+		spec->micmute_led_polarity = 1;
+		codec->power_filter = led_power_filter;
+		alc_fixup_hp_gpio_led(codec, action, 0x00, 0x04);
+
+		/* output mute is set via SET_COEF_INDEX,SET_PROC_COEF */
+		spec->mute_led_polarity = 0;
+		spec->mute_led_coef.idx = 0x0b;
+		spec->mute_led_coef.mask = 0xffff;
+		spec->mute_led_coef.on = 0xa02f;
+		spec->mute_led_coef.off = 0x7774;
+		snd_hda_gen_add_mute_led_cdev(codec, coef_mute_led_set);
+	}
+}
+
 /* turn on/off mic-mute LED per capture hook by coef bit */
 static int coef_micmute_led_set(struct led_classdev *led_cdev,
 				enum led_brightness brightness)
@@ -6557,6 +6584,8 @@ enum {
 	ALC269_FIXUP_HP_DOCK_GPIO_MIC1_LED,
 	ALC280_FIXUP_HP_9480M,
 	ALC245_FIXUP_HP_X360_AMP,
+	ALC245_FIXUP_HP_X360_MUTE_LEDS,
+	ALC245_FIXUP_HP_X360_GPIO_TOP_SPEAKER,
 	ALC288_FIXUP_DELL_HEADSET_MODE,
 	ALC288_FIXUP_DELL1_MIC_NO_PRESENCE,
 	ALC288_FIXUP_DELL_XPS_13,
@@ -7293,6 +7322,21 @@ static const struct hda_fixup alc269_fixups[] = {
 	[ALC245_FIXUP_HP_X360_AMP] = {
 		.type = HDA_FIXUP_FUNC,
 		.v.func = alc245_fixup_hp_x360_amp,
+		.chained = true,
+		.chain_id = ALC245_FIXUP_HP_X360_MUTE_LEDS
+	},
+	[ALC245_FIXUP_HP_X360_MUTE_LEDS] = {
+		.type = HDA_FIXUP_FUNC,
+		.v.func = alc245_fixup_hp_x360_mute_leds,
+		.chained = true,
+		.chain_id = ALC245_FIXUP_HP_X360_GPIO_TOP_SPEAKER
+	},
+	[ALC245_FIXUP_HP_X360_GPIO_TOP_SPEAKER] = {
+		.type = HDA_FIXUP_PINS,
+		.v.pins = (const struct hda_pintbl[]) {
+			{ 0x14, 0x90170110 }, /* enable top(back) speaker in addition to bottom(front) speaker at 0x17 */
+			{ },
+		}
 	},
 	[ALC288_FIXUP_DELL_HEADSET_MODE] = {
 		.type = HDA_FIXUP_FUNC,
@@ -9003,6 +9047,8 @@ static const struct hda_model_fixup alc269_fixup_models[] = {
 	{.id = ALC255_FIXUP_XIAOMI_HEADSET_MIC, .name = "alc255-xiaomi-headset"},
 	{.id = ALC274_FIXUP_HP_MIC, .name = "alc274-hp-mic-detect"},
 	{.id = ALC245_FIXUP_HP_X360_AMP, .name = "alc245-hp-x360-amp"},
+	{.id = ALC245_FIXUP_HP_X360_MUTE_LEDS, .name = "alc245-hp-x360-mute-leds"},
+	{.id = ALC245_FIXUP_HP_X360_GPIO_TOP_SPEAKER, .name = "alc245-hp-x360-gpio-top-speaker"},
 	{.id = ALC295_FIXUP_HP_OMEN, .name = "alc295-hp-omen"},
 	{.id = ALC285_FIXUP_HP_SPECTRE_X360, .name = "alc285-hp-spectre-x360"},
 	{.id = ALC287_FIXUP_IDEAPAD_BASS_SPK_AMP, .name = "alc287-ideapad-bass-spk-amp"},
-- 
2.25.1

Re: [PATCH] Make top/rear speaker, mute and micmute leds work on HP x360 14-ea000 laptops that use Realtek 245 codec

[Takashi Iwai]

On Fri, 29 Oct 2021 17:43:13 +0200,
Jonathan Clarke wrote:
> 
> BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=210633
> Signed-off-by: Jonathan Clarke <jonathan.a.clarke@gmail.com>

Could you give more descriptions?  The patch isn't trivial at all, and
it needs more explanations.

> ---
>  sound/pci/hda/patch_realtek.c | 46 +++++++++++++++++++++++++++++++++++
>  1 file changed, 46 insertions(+)
> 
> diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
> index 22d27b12c..e3c6d17ea 100644
> --- a/sound/pci/hda/patch_realtek.c
> +++ b/sound/pci/hda/patch_realtek.c
> @@ -4409,6 +4409,13 @@ static void alc245_fixup_hp_x360_amp(struct hda_codec *codec,
>  	case HDA_FIXUP_ACT_PRE_PROBE:
>  		spec->gpio_mask |= 0x01;
>  		spec->gpio_dir |= 0x01;
> +
> +		/* use only amp at 0x02 for bottom(front) speaker,
> +		 * otherwise it is set to use 0x02,0x03,0x06 and when used in conjunction
> +		 * with top(rear) speaker 0x14, gets locked at full volume */
> +		static const hda_nid_t conn1[] = { 0x02 };
> +		snd_hda_override_conn_list(codec, 0x17, ARRAY_SIZE(conn1), conn1);
> +
>  		break;
>  	case HDA_FIXUP_ACT_INIT:
>  		/* need to toggle GPIO to enable the amp */
> @@ -4503,6 +4510,26 @@ static void alc236_fixup_hp_mute_led_coefbit(struct hda_codec *codec,
>  	}
>  }
>  
> +static void alc245_fixup_hp_x360_mute_leds(struct hda_codec *codec,
> +				const struct hda_fixup *fix, int action)
> +{
> +	struct alc_spec *spec = codec->spec;
> +	if (action == HDA_FIXUP_ACT_PRE_PROBE) {
> +		/* mic mute is set via gpio 0x04 */
> +		spec->micmute_led_polarity = 1;
> +		codec->power_filter = led_power_filter;
> +		alc_fixup_hp_gpio_led(codec, action, 0x00, 0x04);
> +
> +		/* output mute is set via SET_COEF_INDEX,SET_PROC_COEF */
> +		spec->mute_led_polarity = 0;
> +		spec->mute_led_coef.idx = 0x0b;
> +		spec->mute_led_coef.mask = 0xffff;
> +		spec->mute_led_coef.on = 0xa02f;
> +		spec->mute_led_coef.off = 0x7774;
> +		snd_hda_gen_add_mute_led_cdev(codec, coef_mute_led_set);

I guess this COEF isn't only about mute-LED but actually does mute the
output?  IIRC, the bit 0x08 corresponds to the LED.  If so, it's
better to split.  Basically this snd_hda_gen_add_mute_led_cdev() and
mute_led_coef stuff are only for the mute LED control.  e.g. you can
change the mute LED independently via sysfs.


thanks,

Takashi

Re: [PATCH] Make top/rear speaker, mute and micmute leds work on HP x360 14-ea000 laptops that use Realtek 245 codec

[kernel test robot]

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

Hi Jonathan,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on tiwai-sound/for-next]
[cannot apply to v5.15-rc7 next-20211029]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Jonathan-Clarke/Make-top-rear-speaker-mute-and-micmute-leds-work-on-HP-x360-14-ea000-laptops-that-use-Realtek-245-codec/20211029-234508
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git for-next
config: nds32-randconfig-r033-20211031 (attached as .config)
compiler: nds32le-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/a4b221979b51a8d3256056869efc55d4dc80a4ad
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Jonathan-Clarke/Make-top-rear-speaker-mute-and-micmute-leds-work-on-HP-x360-14-ea000-laptops-that-use-Realtek-245-codec/20211029-234508
        git checkout a4b221979b51a8d3256056869efc55d4dc80a4ad
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross ARCH=nds32 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   sound/pci/hda/patch_realtek.c: In function 'alc245_fixup_hp_x360_amp':
>> sound/pci/hda/patch_realtek.c:4417:17: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
    4417 |                 static const hda_nid_t conn1[] = { 0x02 };
         |                 ^~~~~~


vim +4417 sound/pci/hda/patch_realtek.c

  4400	
  4401	/* HP Spectre x360 14 model needs a unique workaround for enabling the amp;
  4402	 * it needs to toggle the GPIO0 once on and off at each time (bko#210633)
  4403	 */
  4404	static void alc245_fixup_hp_x360_amp(struct hda_codec *codec,
  4405					     const struct hda_fixup *fix, int action)
  4406	{
  4407		struct alc_spec *spec = codec->spec;
  4408	
  4409		switch (action) {
  4410		case HDA_FIXUP_ACT_PRE_PROBE:
  4411			spec->gpio_mask |= 0x01;
  4412			spec->gpio_dir |= 0x01;
  4413	
  4414			/* use only amp at 0x02 for bottom(front) speaker,
  4415			 * otherwise it is set to use 0x02,0x03,0x06 and when used in conjunction
  4416			 * with top(rear) speaker 0x14, gets locked at full volume */
> 4417			static const hda_nid_t conn1[] = { 0x02 };
  4418			snd_hda_override_conn_list(codec, 0x17, ARRAY_SIZE(conn1), conn1);
  4419	
  4420			break;
  4421		case HDA_FIXUP_ACT_INIT:
  4422			/* need to toggle GPIO to enable the amp */
  4423			alc_update_gpio_data(codec, 0x01, true);
  4424			msleep(100);
  4425			alc_update_gpio_data(codec, 0x01, false);
  4426			break;
  4427		}
  4428	}
  4429	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 32681 bytes --]

Re: [PATCH] Make top/rear speaker, mute and micmute leds work on HP x360 14-ea000 laptops that use Realtek 245 codec

[kernel test robot]

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

Hi Jonathan,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on tiwai-sound/for-next]
[cannot apply to v5.15-rc7 next-20211029]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Jonathan-Clarke/Make-top-rear-speaker-mute-and-micmute-leds-work-on-HP-x360-14-ea000-laptops-that-use-Realtek-245-codec/20211029-234508
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git for-next
config: x86_64-allyesconfig (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
        # https://github.com/0day-ci/linux/commit/a4b221979b51a8d3256056869efc55d4dc80a4ad
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Jonathan-Clarke/Make-top-rear-speaker-mute-and-micmute-leds-work-on-HP-x360-14-ea000-laptops-that-use-Realtek-245-codec/20211029-234508
        git checkout a4b221979b51a8d3256056869efc55d4dc80a4ad
        # save the attached .config to linux build tree
        make W=1 ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   sound/pci/hda/patch_realtek.c: In function 'alc245_fixup_hp_x360_amp':
>> sound/pci/hda/patch_realtek.c:4417:3: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement]
    4417 |   static const hda_nid_t conn1[] = { 0x02 };
         |   ^~~~~~
   cc1: all warnings being treated as errors


vim +4417 sound/pci/hda/patch_realtek.c

  4400	
  4401	/* HP Spectre x360 14 model needs a unique workaround for enabling the amp;
  4402	 * it needs to toggle the GPIO0 once on and off at each time (bko#210633)
  4403	 */
  4404	static void alc245_fixup_hp_x360_amp(struct hda_codec *codec,
  4405					     const struct hda_fixup *fix, int action)
  4406	{
  4407		struct alc_spec *spec = codec->spec;
  4408	
  4409		switch (action) {
  4410		case HDA_FIXUP_ACT_PRE_PROBE:
  4411			spec->gpio_mask |= 0x01;
  4412			spec->gpio_dir |= 0x01;
  4413	
  4414			/* use only amp at 0x02 for bottom(front) speaker,
  4415			 * otherwise it is set to use 0x02,0x03,0x06 and when used in conjunction
  4416			 * with top(rear) speaker 0x14, gets locked at full volume */
> 4417			static const hda_nid_t conn1[] = { 0x02 };
  4418			snd_hda_override_conn_list(codec, 0x17, ARRAY_SIZE(conn1), conn1);
  4419	
  4420			break;
  4421		case HDA_FIXUP_ACT_INIT:
  4422			/* need to toggle GPIO to enable the amp */
  4423			alc_update_gpio_data(codec, 0x01, true);
  4424			msleep(100);
  4425			alc_update_gpio_data(codec, 0x01, false);
  4426			break;
  4427		}
  4428	}
  4429	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 66443 bytes --]

Re: [PATCH] Make top/rear speaker, mute and micmute leds work on HP x360 14-ea000 laptops that use Realtek 245 codec

[Jonathan Clarke]

Thanks for taking a look at this patch so quickly, Takashi.

On Sat, Oct 30, 2021 at 12:01:03PM +0200, Takashi Iwai wrote:
> Could you give more descriptions?  The patch isn't trivial at all, and
> it needs more explanations.

Yes, will do.

> > +           /* output mute is set via SET_COEF_INDEX,SET_PROC_COEF */
> > +           spec->mute_led_polarity = 0;
> > +           spec->mute_led_coef.idx = 0x0b;
> > +           spec->mute_led_coef.mask = 0xffff;
> > +           spec->mute_led_coef.on = 0xa02f;
> > +           spec->mute_led_coef.off = 0x7774;
> > +           snd_hda_gen_add_mute_led_cdev(codec, coef_mute_led_set);
>
> I guess this COEF isn't only about mute-LED but actually does mute the
> output?  IIRC, the bit 0x08 corresponds to the LED.  If so, it's
> better to split.  Basically this snd_hda_gen_add_mute_led_cdev() and
> mute_led_coef stuff are only for the mute LED control.  e.g. you can
> change the mute LED independently via sysfs.

Thanks for suggesting this.

Having tested, I can confirm that setting this coef only affects
the output mute LED, and does not affect output.

I will therefore assume that current implementation in my patch is OK,
but let me know if it still needs changing (maybe I've misunderstood).

For reference to other users, the commands to test are:
# output LED on
hda-verb /dev/snd/hwC0D0 0x20 SET_COEF_INDEX 0x0b
hda-verb /dev/snd/hwC0D0 0x20 SET_PROC_COEF 0xa02f

# output LED off
hda-verb /dev/snd/hwC0D0 0x20 SET_COEF_INDEX 0x0b
hda-verb /dev/snd/hwC0D0 0x20 SET_PROC_COEF 0x7774

Many thanks, Jonathan

Re: [PATCH] Make top/rear speaker, mute and micmute leds work on HP x360 14-ea000 laptops that use Realtek 245 codec

[Takashi Iwai]

On Mon, 01 Nov 2021 11:34:45 +0100,
Jonathan Clarke wrote:
> 
> Thanks for taking a look at this patch so quickly, Takashi.
> 
> On Sat, Oct 30, 2021 at 12:01:03PM +0200, Takashi Iwai wrote:
> > Could you give more descriptions?  The patch isn't trivial at all, and
> > it needs more explanations.
> 
> Yes, will do.
> 
> > > +           /* output mute is set via SET_COEF_INDEX,SET_PROC_COEF */
> > > +           spec->mute_led_polarity = 0;
> > > +           spec->mute_led_coef.idx = 0x0b;
> > > +           spec->mute_led_coef.mask = 0xffff;
> > > +           spec->mute_led_coef.on = 0xa02f;
> > > +           spec->mute_led_coef.off = 0x7774;
> > > +           snd_hda_gen_add_mute_led_cdev(codec, coef_mute_led_set);
> >
> > I guess this COEF isn't only about mute-LED but actually does mute the
> > output?  IIRC, the bit 0x08 corresponds to the LED.  If so, it's
> > better to split.  Basically this snd_hda_gen_add_mute_led_cdev() and
> > mute_led_coef stuff are only for the mute LED control.  e.g. you can
> > change the mute LED independently via sysfs.
> 
> Thanks for suggesting this.
> 
> Having tested, I can confirm that setting this coef only affects
> the output mute LED, and does not affect output.
> 
> I will therefore assume that current implementation in my patch is OK,
> but let me know if it still needs changing (maybe I've misunderstood).
> 
> For reference to other users, the commands to test are:
> # output LED on
> hda-verb /dev/snd/hwC0D0 0x20 SET_COEF_INDEX 0x0b
> hda-verb /dev/snd/hwC0D0 0x20 SET_PROC_COEF 0xa02f
> 
> # output LED off
> hda-verb /dev/snd/hwC0D0 0x20 SET_COEF_INDEX 0x0b
> hda-verb /dev/snd/hwC0D0 0x20 SET_PROC_COEF 0x7774

Could you try just to flip the bit 0x08?  At LED off state?

% hda-verb /dev/snd/hwC0D0 0x20 SET_COEF_INDEX 0x0b
% hda-verb /dev/snd/hwC0D0 0x20 SET_PROC_COEF 0x77f4

That is, the implementation in alc286_fixup_hp_mute_led_coefbit(),
which is used by many other HP laptops.


Takashi

Re: [PATCH] Make top/rear speaker, mute and micmute leds work on HP x360 14-ea000 laptops that use Realtek 245 codec

[Jonathan Clarke]

On Tue, Nov 02, 2021 at 09:19:25AM +0100, Takashi Iwai wrote:
> Could you try just to flip the bit 0x08?  At LED off state?
> 
> % hda-verb /dev/snd/hwC0D0 0x20 SET_COEF_INDEX 0x0b
> % hda-verb /dev/snd/hwC0D0 0x20 SET_PROC_COEF 0x77f4
> 
> That is, the implementation in alc286_fixup_hp_mute_led_coefbit(),
> which is used by many other HP laptops.

Ah, I see what you mean.  

Yes, you're right, changing from 0x77f4 to 0x77fc makes the mute led light up.
The changes to the other proc_coef bits in my patch are not needed.

I will revise and revert in next few days.  Thanks