All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 14/31] HDA patch_via.c: Add Jack detect feature for VT1708.
@ 2009-10-05 14:27 Li Bo
  2009-10-05 15:12 ` Takashi Iwai
  0 siblings, 1 reply; 12+ messages in thread
From: Li Bo @ 2009-10-05 14:27 UTC (permalink / raw)
  To: alsa-devel; +Cc: tiwai, LoganLi, HaraldWelte, lydiawang

[ALSA] HDA VIA: Add Jack detect feature for VT1708.

Signed-off-by: Lydia Wang <lydiawang@viatech.com.cn>

Index: sound-2.6/sound/pci/hda/patch_via.c
===================================================================
--- sound-2.6.orig/sound/pci/hda/patch_via.c	2009-10-05 15:10:22.000000000 +0800
+++ sound-2.6/sound/pci/hda/patch_via.c	2009-10-05 15:10:36.000000000 +0800
@@ -257,6 +257,11 @@
 	unsigned int smart51_enabled;
 	enum VIA_HDA_CODEC codec_type;

+	/* work to check hp jack state */
+	struct hda_codec *codec;
+	struct delayed_work hp_jack_work;
+	int vt1708_jack_detectect;
+
 #ifdef CONFIG_SND_HDA_POWER_SAVE
 	struct hda_loopback_check loopback;
 #endif
@@ -966,6 +971,8 @@
 	{0x20, AC_VERB_SET_CONNECT_SEL, 0x1},
 	/* PW9 Output enable */
 	{0x25, AC_VERB_SET_PIN_WIDGET_CONTROL, 0x40},
+	/* power down jack detect function */
+	{0x1, 0xf81, 0x1},
 	{ }
 };

@@ -1340,6 +1347,10 @@
 		return;

 	via_free_kctls(codec);
+	if (spec->codec_type == VT1708) {
+		cancel_delayed_work(&spec->hp_jack_work);
+		flush_scheduled_work();
+	}
 	kfree(codec->spec);
 }

@@ -1724,6 +1735,52 @@
 	return;
 }

+static int vt1708_jack_detectect_get(struct snd_kcontrol *kcontrol,
+				     struct snd_ctl_elem_value *ucontrol)
+{
+	struct hda_codec *codec = snd_kcontrol_chip(kcontrol);
+	struct via_spec *spec = codec->spec;
+
+	if (spec->codec_type != VT1708)
+		return 0;
+	spec->vt1708_jack_detectect =
+		!((snd_hda_codec_read(codec, 0x1, 0, 0xf84, 0) >> 8) & 0x1);
+	ucontrol->value.integer.value[0] = spec->vt1708_jack_detectect;
+	return 0;
+}
+
+static int vt1708_jack_detectect_put(struct snd_kcontrol *kcontrol,
+				     struct snd_ctl_elem_value *ucontrol)
+{
+	struct hda_codec *codec = snd_kcontrol_chip(kcontrol);
+	struct via_spec *spec = codec->spec;
+	int change;
+
+	if (spec->codec_type != VT1708)
+		return 0;
+	spec->vt1708_jack_detectect = ucontrol->value.integer.value[0];
+	snd_hda_codec_write(codec, 0x1, 0, 0xf81, !spec->vt1708_jack_detectect);
+	change = (0x1 & (snd_hda_codec_read(codec, 0x1, 0, 0xf84, 0) >> 8))
+		== !spec->vt1708_jack_detectect;
+	if (spec->vt1708_jack_detectect) {
+		mute_aa_path(codec, 1);
+		notify_aa_path_ctls(codec);
+	}
+	return change;
+}
+
+static struct snd_kcontrol_new vt1708_jack_detectect[] = {
+	{
+		.iface = SNDRV_CTL_ELEM_IFACE_MIXER,
+		.name = "Jack Detect",
+		.count = 1,
+		.info = snd_ctl_boolean_mono_info,
+		.get = vt1708_jack_detectect_get,
+		.put = vt1708_jack_detectect_put,
+	},
+	{} /* end */
+};
+
 static int vt1708_parse_auto_config(struct hda_codec *codec)
 {
 	struct via_spec *spec = codec->spec;
@@ -1751,6 +1808,10 @@
 	err = vt1708_auto_create_analog_input_ctls(spec, &spec->autocfg);
 	if (err < 0)
 		return err;
+	/* add jack detect on/off control */
+	err = snd_hda_add_new_ctls(codec, vt1708_jack_detectect);
+	if (err < 0)
+		return err;

 	spec->multiout.max_channels = spec->multiout.num_dacs * 2;

@@ -1784,6 +1845,24 @@
 	return 0;
 }

+static void update_hp_jack_state(struct work_struct *work)
+{
+	struct via_spec *spec = container_of(work, struct via_spec,
+					     hp_jack_work.work);
+	static int present = 1;
+
+	if (spec->codec_type != VT1708)
+		return;
+	/* if jack state toggled */
+	if (present
+	    != (snd_hda_codec_read(spec->codec, spec->autocfg.hp_pins[0], 0,
+				   AC_VERB_GET_PIN_SENSE, 0) >> 31)) {
+		present ^= 1;
+		via_hp_automute(spec->codec);
+	}
+	schedule_delayed_work(&spec->hp_jack_work, msecs_to_jiffies(100));
+}
+
 static int get_mux_nids(struct hda_codec *codec)
 {
 	struct via_spec *spec = codec->spec;
@@ -1860,7 +1939,9 @@
 #ifdef CONFIG_SND_HDA_POWER_SAVE
 	spec->loopback.amplist = vt1708_loopbacks;
 #endif
-
+	spec->codec = codec;
+	INIT_DELAYED_WORK(&spec->hp_jack_work, update_hp_jack_state);
+	schedule_delayed_work(&spec->hp_jack_work, msecs_to_jiffies(100));
 	return 0;
 }

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

* Re: [PATCH 14/31] HDA patch_via.c: Add Jack detect feature for VT1708.
  2009-10-05 14:27 [PATCH 14/31] HDA patch_via.c: Add Jack detect feature for VT1708 Li Bo
@ 2009-10-05 15:12 ` Takashi Iwai
  2009-10-06  5:06   ` Li Bo
  0 siblings, 1 reply; 12+ messages in thread
From: Takashi Iwai @ 2009-10-05 15:12 UTC (permalink / raw)
  To: Li Bo; +Cc: LoganLi, alsa-devel, HaraldWelte, lydiawang

At Mon, 5 Oct 2009 22:27:20 +0800,
Li Bo wrote:
> 
> [ALSA] HDA VIA: Add Jack detect feature for VT1708.

Why is it needed?  Isn't the unsolicited event handled properly at
HP plugging?

Also, this implementation doesn't look good.  If I understand
correctly, it invokes a workqueue which re-schedules itself at each
100ms...


Takashi

> Signed-off-by: Lydia Wang <lydiawang@viatech.com.cn>
> 
> Index: sound-2.6/sound/pci/hda/patch_via.c
> ===================================================================
> --- sound-2.6.orig/sound/pci/hda/patch_via.c	2009-10-05 15:10:22.000000000 +0800
> +++ sound-2.6/sound/pci/hda/patch_via.c	2009-10-05 15:10:36.000000000 +0800
> @@ -257,6 +257,11 @@
>  	unsigned int smart51_enabled;
>  	enum VIA_HDA_CODEC codec_type;
> 
> +	/* work to check hp jack state */
> +	struct hda_codec *codec;
> +	struct delayed_work hp_jack_work;
> +	int vt1708_jack_detectect;
> +
>  #ifdef CONFIG_SND_HDA_POWER_SAVE
>  	struct hda_loopback_check loopback;
>  #endif
> @@ -966,6 +971,8 @@
>  	{0x20, AC_VERB_SET_CONNECT_SEL, 0x1},
>  	/* PW9 Output enable */
>  	{0x25, AC_VERB_SET_PIN_WIDGET_CONTROL, 0x40},
> +	/* power down jack detect function */
> +	{0x1, 0xf81, 0x1},
>  	{ }
>  };
> 
> @@ -1340,6 +1347,10 @@
>  		return;
> 
>  	via_free_kctls(codec);
> +	if (spec->codec_type == VT1708) {
> +		cancel_delayed_work(&spec->hp_jack_work);
> +		flush_scheduled_work();
> +	}
>  	kfree(codec->spec);
>  }
> 
> @@ -1724,6 +1735,52 @@
>  	return;
>  }
> 
> +static int vt1708_jack_detectect_get(struct snd_kcontrol *kcontrol,
> +				     struct snd_ctl_elem_value *ucontrol)
> +{
> +	struct hda_codec *codec = snd_kcontrol_chip(kcontrol);
> +	struct via_spec *spec = codec->spec;
> +
> +	if (spec->codec_type != VT1708)
> +		return 0;
> +	spec->vt1708_jack_detectect =
> +		!((snd_hda_codec_read(codec, 0x1, 0, 0xf84, 0) >> 8) & 0x1);
> +	ucontrol->value.integer.value[0] = spec->vt1708_jack_detectect;
> +	return 0;
> +}
> +
> +static int vt1708_jack_detectect_put(struct snd_kcontrol *kcontrol,
> +				     struct snd_ctl_elem_value *ucontrol)
> +{
> +	struct hda_codec *codec = snd_kcontrol_chip(kcontrol);
> +	struct via_spec *spec = codec->spec;
> +	int change;
> +
> +	if (spec->codec_type != VT1708)
> +		return 0;
> +	spec->vt1708_jack_detectect = ucontrol->value.integer.value[0];
> +	snd_hda_codec_write(codec, 0x1, 0, 0xf81, !spec->vt1708_jack_detectect);
> +	change = (0x1 & (snd_hda_codec_read(codec, 0x1, 0, 0xf84, 0) >> 8))
> +		== !spec->vt1708_jack_detectect;
> +	if (spec->vt1708_jack_detectect) {
> +		mute_aa_path(codec, 1);
> +		notify_aa_path_ctls(codec);
> +	}
> +	return change;
> +}
> +
> +static struct snd_kcontrol_new vt1708_jack_detectect[] = {
> +	{
> +		.iface = SNDRV_CTL_ELEM_IFACE_MIXER,
> +		.name = "Jack Detect",
> +		.count = 1,
> +		.info = snd_ctl_boolean_mono_info,
> +		.get = vt1708_jack_detectect_get,
> +		.put = vt1708_jack_detectect_put,
> +	},
> +	{} /* end */
> +};
> +
>  static int vt1708_parse_auto_config(struct hda_codec *codec)
>  {
>  	struct via_spec *spec = codec->spec;
> @@ -1751,6 +1808,10 @@
>  	err = vt1708_auto_create_analog_input_ctls(spec, &spec->autocfg);
>  	if (err < 0)
>  		return err;
> +	/* add jack detect on/off control */
> +	err = snd_hda_add_new_ctls(codec, vt1708_jack_detectect);
> +	if (err < 0)
> +		return err;
> 
>  	spec->multiout.max_channels = spec->multiout.num_dacs * 2;
> 
> @@ -1784,6 +1845,24 @@
>  	return 0;
>  }
> 
> +static void update_hp_jack_state(struct work_struct *work)
> +{
> +	struct via_spec *spec = container_of(work, struct via_spec,
> +					     hp_jack_work.work);
> +	static int present = 1;
> +
> +	if (spec->codec_type != VT1708)
> +		return;
> +	/* if jack state toggled */
> +	if (present
> +	    != (snd_hda_codec_read(spec->codec, spec->autocfg.hp_pins[0], 0,
> +				   AC_VERB_GET_PIN_SENSE, 0) >> 31)) {
> +		present ^= 1;
> +		via_hp_automute(spec->codec);
> +	}
> +	schedule_delayed_work(&spec->hp_jack_work, msecs_to_jiffies(100));
> +}
> +
>  static int get_mux_nids(struct hda_codec *codec)
>  {
>  	struct via_spec *spec = codec->spec;
> @@ -1860,7 +1939,9 @@
>  #ifdef CONFIG_SND_HDA_POWER_SAVE
>  	spec->loopback.amplist = vt1708_loopbacks;
>  #endif
> -
> +	spec->codec = codec;
> +	INIT_DELAYED_WORK(&spec->hp_jack_work, update_hp_jack_state);
> +	schedule_delayed_work(&spec->hp_jack_work, msecs_to_jiffies(100));
>  	return 0;
>  }
> 

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

* Re: [PATCH 14/31] HDA patch_via.c: Add Jack detect feature for VT1708.
  2009-10-05 15:12 ` Takashi Iwai
@ 2009-10-06  5:06   ` Li Bo
  2009-10-06  6:01     ` Takashi Iwai
  0 siblings, 1 reply; 12+ messages in thread
From: Li Bo @ 2009-10-06  5:06 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: LoganLi, alsa-devel, HaraldWelte, lydiawang

VT1708 HW doesn't support unsolicited response, so we polling with jack
detect function instead.
As you describe, we implement it with self-reschedule workqueue, acting
like a timer.
Is there some advice to implement timer-like task, I tested  with timer and
it will crash the driver.

On Mon, Oct 5, 2009 at 11:12 PM, Takashi Iwai <tiwai@suse.de> wrote:
> At Mon, 5 Oct 2009 22:27:20 +0800,
> Li Bo wrote:
>>
>> [ALSA] HDA VIA: Add Jack detect feature for VT1708.
>
> Why is it needed?  Isn't the unsolicited event handled properly at
> HP plugging?
>
> Also, this implementation doesn't look good.  If I understand
> correctly, it invokes a workqueue which re-schedules itself at each
> 100ms...
>
>
> Takashi
>
>> Signed-off-by: Lydia Wang <lydiawang@viatech.com.cn>
>>
>> Index: sound-2.6/sound/pci/hda/patch_via.c
>> ===================================================================
>> --- sound-2.6.orig/sound/pci/hda/patch_via.c  2009-10-05 15:10:22.000000000 +0800
>> +++ sound-2.6/sound/pci/hda/patch_via.c       2009-10-05 15:10:36.000000000 +0800
>> @@ -257,6 +257,11 @@
>>       unsigned int smart51_enabled;
>>       enum VIA_HDA_CODEC codec_type;
>>
>> +     /* work to check hp jack state */
>> +     struct hda_codec *codec;
>> +     struct delayed_work hp_jack_work;
>> +     int vt1708_jack_detectect;
>> +
>>  #ifdef CONFIG_SND_HDA_POWER_SAVE
>>       struct hda_loopback_check loopback;
>>  #endif
>> @@ -966,6 +971,8 @@
>>       {0x20, AC_VERB_SET_CONNECT_SEL, 0x1},
>>       /* PW9 Output enable */
>>       {0x25, AC_VERB_SET_PIN_WIDGET_CONTROL, 0x40},
>> +     /* power down jack detect function */
>> +     {0x1, 0xf81, 0x1},
>>       { }
>>  };
>>
>> @@ -1340,6 +1347,10 @@
>>               return;
>>
>>       via_free_kctls(codec);
>> +     if (spec->codec_type == VT1708) {
>> +             cancel_delayed_work(&spec->hp_jack_work);
>> +             flush_scheduled_work();
>> +     }
>>       kfree(codec->spec);
>>  }
>>
>> @@ -1724,6 +1735,52 @@
>>       return;
>>  }
>>
>> +static int vt1708_jack_detectect_get(struct snd_kcontrol *kcontrol,
>> +                                  struct snd_ctl_elem_value *ucontrol)
>> +{
>> +     struct hda_codec *codec = snd_kcontrol_chip(kcontrol);
>> +     struct via_spec *spec = codec->spec;
>> +
>> +     if (spec->codec_type != VT1708)
>> +             return 0;
>> +     spec->vt1708_jack_detectect =
>> +             !((snd_hda_codec_read(codec, 0x1, 0, 0xf84, 0) >> 8) & 0x1);
>> +     ucontrol->value.integer.value[0] = spec->vt1708_jack_detectect;
>> +     return 0;
>> +}
>> +
>> +static int vt1708_jack_detectect_put(struct snd_kcontrol *kcontrol,
>> +                                  struct snd_ctl_elem_value *ucontrol)
>> +{
>> +     struct hda_codec *codec = snd_kcontrol_chip(kcontrol);
>> +     struct via_spec *spec = codec->spec;
>> +     int change;
>> +
>> +     if (spec->codec_type != VT1708)
>> +             return 0;
>> +     spec->vt1708_jack_detectect = ucontrol->value.integer.value[0];
>> +     snd_hda_codec_write(codec, 0x1, 0, 0xf81, !spec->vt1708_jack_detectect);
>> +     change = (0x1 & (snd_hda_codec_read(codec, 0x1, 0, 0xf84, 0) >> 8))
>> +             == !spec->vt1708_jack_detectect;
>> +     if (spec->vt1708_jack_detectect) {
>> +             mute_aa_path(codec, 1);
>> +             notify_aa_path_ctls(codec);
>> +     }
>> +     return change;
>> +}
>> +
>> +static struct snd_kcontrol_new vt1708_jack_detectect[] = {
>> +     {
>> +             .iface = SNDRV_CTL_ELEM_IFACE_MIXER,
>> +             .name = "Jack Detect",
>> +             .count = 1,
>> +             .info = snd_ctl_boolean_mono_info,
>> +             .get = vt1708_jack_detectect_get,
>> +             .put = vt1708_jack_detectect_put,
>> +     },
>> +     {} /* end */
>> +};
>> +
>>  static int vt1708_parse_auto_config(struct hda_codec *codec)
>>  {
>>       struct via_spec *spec = codec->spec;
>> @@ -1751,6 +1808,10 @@
>>       err = vt1708_auto_create_analog_input_ctls(spec, &spec->autocfg);
>>       if (err < 0)
>>               return err;
>> +     /* add jack detect on/off control */
>> +     err = snd_hda_add_new_ctls(codec, vt1708_jack_detectect);
>> +     if (err < 0)
>> +             return err;
>>
>>       spec->multiout.max_channels = spec->multiout.num_dacs * 2;
>>
>> @@ -1784,6 +1845,24 @@
>>       return 0;
>>  }
>>
>> +static void update_hp_jack_state(struct work_struct *work)
>> +{
>> +     struct via_spec *spec = container_of(work, struct via_spec,
>> +                                          hp_jack_work.work);
>> +     static int present = 1;
>> +
>> +     if (spec->codec_type != VT1708)
>> +             return;
>> +     /* if jack state toggled */
>> +     if (present
>> +         != (snd_hda_codec_read(spec->codec, spec->autocfg.hp_pins[0], 0,
>> +                                AC_VERB_GET_PIN_SENSE, 0) >> 31)) {
>> +             present ^= 1;
>> +             via_hp_automute(spec->codec);
>> +     }
>> +     schedule_delayed_work(&spec->hp_jack_work, msecs_to_jiffies(100));
>> +}
>> +
>>  static int get_mux_nids(struct hda_codec *codec)
>>  {
>>       struct via_spec *spec = codec->spec;
>> @@ -1860,7 +1939,9 @@
>>  #ifdef CONFIG_SND_HDA_POWER_SAVE
>>       spec->loopback.amplist = vt1708_loopbacks;
>>  #endif
>> -
>> +     spec->codec = codec;
>> +     INIT_DELAYED_WORK(&spec->hp_jack_work, update_hp_jack_state);
>> +     schedule_delayed_work(&spec->hp_jack_work, msecs_to_jiffies(100));
>>       return 0;
>>  }
>>
>

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

* Re: [PATCH 14/31] HDA patch_via.c: Add Jack detect feature for VT1708.
  2009-10-06  5:06   ` Li Bo
@ 2009-10-06  6:01     ` Takashi Iwai
  2009-10-06 12:17       ` Harald Welte
  0 siblings, 1 reply; 12+ messages in thread
From: Takashi Iwai @ 2009-10-06  6:01 UTC (permalink / raw)
  To: Li Bo; +Cc: LoganLi, alsa-devel, HaraldWelte, lydiawang

At Tue, 6 Oct 2009 13:06:40 +0800,
Li Bo wrote:
> 
> VT1708 HW doesn't support unsolicited response, so we polling with jack
> detect function instead.

Ah OK.  Please write this essential information in the changelog text!

> As you describe, we implement it with self-reschedule workqueue, acting
> like a timer.
> Is there some advice to implement timer-like task, I tested  with timer and
> it will crash the driver.

Right, you can't call it in a softirq.

A few problems, however:
- present variable should be in struct via_spec, instead of a static
  variable in update_hp_jack_state().
- This mechanism is unconditionally invoked on VT1708 although, in
  theory, you can have a hardware that doesn't need HP jack detection
- Waking up each 100ms for the *whole* time is really bad regarding
  the power-saving; can't it be optimized?
- You need to cancel/trigger the work at suspend/resume, at least


Takashi


> On Mon, Oct 5, 2009 at 11:12 PM, Takashi Iwai <tiwai@suse.de> wrote:
> > At Mon, 5 Oct 2009 22:27:20 +0800,
> > Li Bo wrote:
> >>
> >> [ALSA] HDA VIA: Add Jack detect feature for VT1708.
> >
> > Why is it needed?  Isn't the unsolicited event handled properly at
> > HP plugging?
> >
> > Also, this implementation doesn't look good.  If I understand
> > correctly, it invokes a workqueue which re-schedules itself at each
> > 100ms...
> >
> >
> > Takashi
> >
> >> Signed-off-by: Lydia Wang <lydiawang@viatech.com.cn>
> >>
> >> Index: sound-2.6/sound/pci/hda/patch_via.c
> >> ===================================================================
> >> --- sound-2.6.orig/sound/pci/hda/patch_via.c  2009-10-05 15:10:22.000000000 +0800
> >> +++ sound-2.6/sound/pci/hda/patch_via.c       2009-10-05 15:10:36.000000000 +0800
> >> @@ -257,6 +257,11 @@
> >>       unsigned int smart51_enabled;
> >>       enum VIA_HDA_CODEC codec_type;
> >>
> >> +     /* work to check hp jack state */
> >> +     struct hda_codec *codec;
> >> +     struct delayed_work hp_jack_work;
> >> +     int vt1708_jack_detectect;
> >> +
> >>  #ifdef CONFIG_SND_HDA_POWER_SAVE
> >>       struct hda_loopback_check loopback;
> >>  #endif
> >> @@ -966,6 +971,8 @@
> >>       {0x20, AC_VERB_SET_CONNECT_SEL, 0x1},
> >>       /* PW9 Output enable */
> >>       {0x25, AC_VERB_SET_PIN_WIDGET_CONTROL, 0x40},
> >> +     /* power down jack detect function */
> >> +     {0x1, 0xf81, 0x1},
> >>       { }
> >>  };
> >>
> >> @@ -1340,6 +1347,10 @@
> >>               return;
> >>
> >>       via_free_kctls(codec);
> >> +     if (spec->codec_type == VT1708) {
> >> +             cancel_delayed_work(&spec->hp_jack_work);
> >> +             flush_scheduled_work();
> >> +     }
> >>       kfree(codec->spec);
> >>  }
> >>
> >> @@ -1724,6 +1735,52 @@
> >>       return;
> >>  }
> >>
> >> +static int vt1708_jack_detectect_get(struct snd_kcontrol *kcontrol,
> >> +                                  struct snd_ctl_elem_value *ucontrol)
> >> +{
> >> +     struct hda_codec *codec = snd_kcontrol_chip(kcontrol);
> >> +     struct via_spec *spec = codec->spec;
> >> +
> >> +     if (spec->codec_type != VT1708)
> >> +             return 0;
> >> +     spec->vt1708_jack_detectect =
> >> +             !((snd_hda_codec_read(codec, 0x1, 0, 0xf84, 0) >> 8) & 0x1);
> >> +     ucontrol->value.integer.value[0] = spec->vt1708_jack_detectect;
> >> +     return 0;
> >> +}
> >> +
> >> +static int vt1708_jack_detectect_put(struct snd_kcontrol *kcontrol,
> >> +                                  struct snd_ctl_elem_value *ucontrol)
> >> +{
> >> +     struct hda_codec *codec = snd_kcontrol_chip(kcontrol);
> >> +     struct via_spec *spec = codec->spec;
> >> +     int change;
> >> +
> >> +     if (spec->codec_type != VT1708)
> >> +             return 0;
> >> +     spec->vt1708_jack_detectect = ucontrol->value.integer.value[0];
> >> +     snd_hda_codec_write(codec, 0x1, 0, 0xf81, !spec->vt1708_jack_detectect);
> >> +     change = (0x1 & (snd_hda_codec_read(codec, 0x1, 0, 0xf84, 0) >> 8))
> >> +             == !spec->vt1708_jack_detectect;
> >> +     if (spec->vt1708_jack_detectect) {
> >> +             mute_aa_path(codec, 1);
> >> +             notify_aa_path_ctls(codec);
> >> +     }
> >> +     return change;
> >> +}
> >> +
> >> +static struct snd_kcontrol_new vt1708_jack_detectect[] = {
> >> +     {
> >> +             .iface = SNDRV_CTL_ELEM_IFACE_MIXER,
> >> +             .name = "Jack Detect",
> >> +             .count = 1,
> >> +             .info = snd_ctl_boolean_mono_info,
> >> +             .get = vt1708_jack_detectect_get,
> >> +             .put = vt1708_jack_detectect_put,
> >> +     },
> >> +     {} /* end */
> >> +};
> >> +
> >>  static int vt1708_parse_auto_config(struct hda_codec *codec)
> >>  {
> >>       struct via_spec *spec = codec->spec;
> >> @@ -1751,6 +1808,10 @@
> >>       err = vt1708_auto_create_analog_input_ctls(spec, &spec->autocfg);
> >>       if (err < 0)
> >>               return err;
> >> +     /* add jack detect on/off control */
> >> +     err = snd_hda_add_new_ctls(codec, vt1708_jack_detectect);
> >> +     if (err < 0)
> >> +             return err;
> >>
> >>       spec->multiout.max_channels = spec->multiout.num_dacs * 2;
> >>
> >> @@ -1784,6 +1845,24 @@
> >>       return 0;
> >>  }
> >>
> >> +static void update_hp_jack_state(struct work_struct *work)
> >> +{
> >> +     struct via_spec *spec = container_of(work, struct via_spec,
> >> +                                          hp_jack_work.work);
> >> +     static int present = 1;
> >> +
> >> +     if (spec->codec_type != VT1708)
> >> +             return;
> >> +     /* if jack state toggled */
> >> +     if (present
> >> +         != (snd_hda_codec_read(spec->codec, spec->autocfg.hp_pins[0], 0,
> >> +                                AC_VERB_GET_PIN_SENSE, 0) >> 31)) {
> >> +             present ^= 1;
> >> +             via_hp_automute(spec->codec);
> >> +     }
> >> +     schedule_delayed_work(&spec->hp_jack_work, msecs_to_jiffies(100));
> >> +}
> >> +
> >>  static int get_mux_nids(struct hda_codec *codec)
> >>  {
> >>       struct via_spec *spec = codec->spec;
> >> @@ -1860,7 +1939,9 @@
> >>  #ifdef CONFIG_SND_HDA_POWER_SAVE
> >>       spec->loopback.amplist = vt1708_loopbacks;
> >>  #endif
> >> -
> >> +     spec->codec = codec;
> >> +     INIT_DELAYED_WORK(&spec->hp_jack_work, update_hp_jack_state);
> >> +     schedule_delayed_work(&spec->hp_jack_work, msecs_to_jiffies(100));
> >>       return 0;
> >>  }
> >>
> >
> 
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

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

* Re: [PATCH 14/31] HDA patch_via.c: Add Jack detect feature for VT1708.
  2009-10-06  6:01     ` Takashi Iwai
@ 2009-10-06 12:17       ` Harald Welte
  2009-10-06 12:38         ` Takashi Iwai
  0 siblings, 1 reply; 12+ messages in thread
From: Harald Welte @ 2009-10-06 12:17 UTC (permalink / raw)
  To: Li Bo; +Cc: Takashi Iwai, alsa-devel, lydiawang, LoganLi

Hi all,

On Tue, Oct 06, 2009 at 08:01:40AM +0200, Takashi Iwai wrote:

> > As you describe, we implement it with self-reschedule workqueue, acting
> > like a timer.
> > Is there some advice to implement timer-like task, I tested  with timer and
> > it will crash the driver.
> 
> Right, you can't call it in a softirq.
> 
> A few problems, however:
> - present variable should be in struct via_spec, instead of a static
>   variable in update_hp_jack_state().

I agree
> - This mechanism is unconditionally invoked on VT1708 although, in
>   theory, you can have a hardware that doesn't need HP jack detection

Yes, Logan/Li/Lydia: If we generate this kind of workaround with a timer,
it should only be used on those particular chips that absolutely need it.

> - Waking up each 100ms for the *whole* time is really bad regarding
>   the power-saving; can't it be optimized?

I would personally actually suggest to simply not support headphone plug events
in case you are using such a [buggy?] chipset that does not support them.

I mean, what do you gain from that event? It's not something super-important.

waking up unconditionally every 100ms is really a too high burden on the
battery life.

Another option might be to make it a module load time parameter, so people
can choose if they actually want to pay the high power/batyery price for this
small feature.

-- 
- Harald Welte <laforge@gnumonks.org>           http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
                                                  (ETSI EN 300 175-7 Ch. A6)

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

* Re: [PATCH 14/31] HDA patch_via.c: Add Jack detect feature for VT1708.
  2009-10-06 12:17       ` Harald Welte
@ 2009-10-06 12:38         ` Takashi Iwai
  2009-10-06 15:27           ` Li Bo
  0 siblings, 1 reply; 12+ messages in thread
From: Takashi Iwai @ 2009-10-06 12:38 UTC (permalink / raw)
  To: Harald Welte; +Cc: Li Bo, alsa-devel, lydiawang, LoganLi

At Tue, 6 Oct 2009 14:17:40 +0200,
Harald Welte wrote:
> 
> Hi all,
> 
> On Tue, Oct 06, 2009 at 08:01:40AM +0200, Takashi Iwai wrote:
> 
> > > As you describe, we implement it with self-reschedule workqueue, acting
> > > like a timer.
> > > Is there some advice to implement timer-like task, I tested  with timer and
> > > it will crash the driver.
> > 
> > Right, you can't call it in a softirq.
> > 
> > A few problems, however:
> > - present variable should be in struct via_spec, instead of a static
> >   variable in update_hp_jack_state().
> 
> I agree
> > - This mechanism is unconditionally invoked on VT1708 although, in
> >   theory, you can have a hardware that doesn't need HP jack detection
> 
> Yes, Logan/Li/Lydia: If we generate this kind of workaround with a timer,
> it should only be used on those particular chips that absolutely need it.

As far as I see, the code is only for VT1708, so it's already specific
to the codec chip.  But, even with this chip, you might have a pin
configuration that doesn't need the HP jack detection.

> > - Waking up each 100ms for the *whole* time is really bad regarding
> >   the power-saving; can't it be optimized?
> 
> I would personally actually suggest to simply not support headphone plug events
> in case you are using such a [buggy?] chipset that does not support them.
> 
> I mean, what do you gain from that event? It's not something super-important.
> 
> waking up unconditionally every 100ms is really a too high burden on the
> battery life.
> 
> Another option might be to make it a module load time parameter, so people
> can choose if they actually want to pay the high power/batyery price for this
> small feature.

Yes.

Basically, if we can drop the analog-loopback feature, the HP jack
probing can be simplified very much.  The probing is needed only when
the PCM is opened/prepared and running.  It can be implemented in a
similar way for the volume-update procedure at PCM trigger.
And, during PCM running, (relatively) high-frequent polling isn't so
critical.


thanks,

Takashi

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

* Re: [PATCH 14/31] HDA patch_via.c: Add Jack detect feature for VT1708.
  2009-10-06 12:38         ` Takashi Iwai
@ 2009-10-06 15:27           ` Li Bo
  2009-10-06 15:34             ` Takashi Iwai
  0 siblings, 1 reply; 12+ messages in thread
From: Li Bo @ 2009-10-06 15:27 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: alsa-devel, LoganLi, Harald Welte, lydiawang

On Tue, Oct 6, 2009 at 8:38 PM, Takashi Iwai <tiwai@suse.de> wrote:
> At Tue, 6 Oct 2009 14:17:40 +0200,
> Harald Welte wrote:
>>
>> Hi all,
>>
>> On Tue, Oct 06, 2009 at 08:01:40AM +0200, Takashi Iwai wrote:
>>
>> > > As you describe, we implement it with self-reschedule workqueue, acting
>> > > like a timer.
>> > > Is there some advice to implement timer-like task, I tested  with timer and
>> > > it will crash the driver.
>> >
>> > Right, you can't call it in a softirq.
>> >
>> > A few problems, however:
>> > - present variable should be in struct via_spec, instead of a static
>> >   variable in update_hp_jack_state().
>>
>> I agree
>> > - This mechanism is unconditionally invoked on VT1708 although, in
>> >   theory, you can have a hardware that doesn't need HP jack detection
>>
>> Yes, Logan/Li/Lydia: If we generate this kind of workaround with a timer,
>> it should only be used on those particular chips that absolutely need it.
>
> As far as I see, the code is only for VT1708, so it's already specific
> to the codec chip.  But, even with this chip, you might have a pin
> configuration that doesn't need the HP jack detection.
>

Yes, it's only for VT1708, and HP exists on most cases, and so HP jack detection
are needed.
And present will put into via_spec.

>> > - Waking up each 100ms for the *whole* time is really bad regarding
>> >   the power-saving; can't it be optimized?
>>
>> I would personally actually suggest to simply not support headphone plug events
>> in case you are using such a [buggy?] chipset that does not support them.
>>
>> I mean, what do you gain from that event? It's not something super-important.
>>
>> waking up unconditionally every 100ms is really a too high burden on the
>> battery life.
>>
>> Another option might be to make it a module load time parameter, so people
>> can choose if they actually want to pay the high power/batyery price for this
>> small feature.
>
> Yes.
>
> Basically, if we can drop the analog-loopback feature, the HP jack
> probing can be simplified very much.  The probing is needed only when
> the PCM is opened/prepared and running.  It can be implemented in a
> similar way for the volume-update procedure at PCM trigger.
> And, during PCM running, (relatively) high-frequent polling isn't so
> critical.
>
>
> thanks,
>
> Takashi
>

I know that jack detect on VT1708 that do not support unsolicited
response is weird, but
 it's for customer request (and so are some other patches:), we just
have to do something.

I think it's good in open/close callback, I'll update.

About self-reschedule workqueue, maybe there are some other choices,
or should we keep
using it?

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

* Re: [PATCH 14/31] HDA patch_via.c: Add Jack detect feature for VT1708.
  2009-10-06 15:27           ` Li Bo
@ 2009-10-06 15:34             ` Takashi Iwai
  2009-10-07  5:56               ` Li Bo
  0 siblings, 1 reply; 12+ messages in thread
From: Takashi Iwai @ 2009-10-06 15:34 UTC (permalink / raw)
  To: Li Bo; +Cc: alsa-devel, LoganLi, Harald Welte, lydiawang

At Tue, 6 Oct 2009 23:27:35 +0800,
Li Bo wrote:
> 
> On Tue, Oct 6, 2009 at 8:38 PM, Takashi Iwai <tiwai@suse.de> wrote:
> > At Tue, 6 Oct 2009 14:17:40 +0200,
> > Harald Welte wrote:
> >>
> >> Hi all,
> >>
> >> On Tue, Oct 06, 2009 at 08:01:40AM +0200, Takashi Iwai wrote:
> >>
> >> > > As you describe, we implement it with self-reschedule workqueue, acting
> >> > > like a timer.
> >> > > Is there some advice to implement timer-like task, I tested  with timer and
> >> > > it will crash the driver.
> >> >
> >> > Right, you can't call it in a softirq.
> >> >
> >> > A few problems, however:
> >> > - present variable should be in struct via_spec, instead of a static
> >> >   variable in update_hp_jack_state().
> >>
> >> I agree
> >> > - This mechanism is unconditionally invoked on VT1708 although, in
> >> >   theory, you can have a hardware that doesn't need HP jack detection
> >>
> >> Yes, Logan/Li/Lydia: If we generate this kind of workaround with a timer,
> >> it should only be used on those particular chips that absolutely need it.
> >
> > As far as I see, the code is only for VT1708, so it's already specific
> > to the codec chip.  But, even with this chip, you might have a pin
> > configuration that doesn't need the HP jack detection.
> >
> 
> Yes, it's only for VT1708, and HP exists on most cases, and so HP jack detection
> are needed.
> And present will put into via_spec.

If it's restricted only during the opened (better running) PCM stream,
it's not too bad.  But, it'd be even better to check the availability
of HP jack.


> >> > - Waking up each 100ms for the *whole* time is really bad regarding
> >> >   the power-saving; can't it be optimized?
> >>
> >> I would personally actually suggest to simply not support headphone plug events
> >> in case you are using such a [buggy?] chipset that does not support them.
> >>
> >> I mean, what do you gain from that event? It's not something super-important.
> >>
> >> waking up unconditionally every 100ms is really a too high burden on the
> >> battery life.
> >>
> >> Another option might be to make it a module load time parameter, so people
> >> can choose if they actually want to pay the high power/batyery price for this
> >> small feature.
> >
> > Yes.
> >
> > Basically, if we can drop the analog-loopback feature, the HP jack
> > probing can be simplified very much.  The probing is needed only when
> > the PCM is opened/prepared and running.  It can be implemented in a
> > similar way for the volume-update procedure at PCM trigger.
> > And, during PCM running, (relatively) high-frequent polling isn't so
> > critical.
> >
> >
> > thanks,
> >
> > Takashi
> >
> 
> I know that jack detect on VT1708 that do not support unsolicited
> response is weird, but
>  it's for customer request (and so are some other patches:), we just
> have to do something.
> 
> I think it's good in open/close callback, I'll update.
> 
> About self-reschedule workqueue, maybe there are some other choices,
> or should we keep
> using it?

A delayed workqueue is OK.  But, it should check some flag before
rescheduling itself.  Also, don't forget about suspend/resume.


thanks,

Takashi
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

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

* Re: [PATCH 14/31] HDA patch_via.c: Add Jack detect feature for VT1708.
  2009-10-06 15:34             ` Takashi Iwai
@ 2009-10-07  5:56               ` Li Bo
  2009-10-07  7:03                 ` Takashi Iwai
  0 siblings, 1 reply; 12+ messages in thread
From: Li Bo @ 2009-10-07  5:56 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: alsa-devel, LoganLi, Harald Welte, lydiawang

On Tue, Oct 6, 2009 at 11:34 PM, Takashi Iwai <tiwai@suse.de> wrote:
> At Tue, 6 Oct 2009 23:27:35 +0800,
> Li Bo wrote:
>>
>> On Tue, Oct 6, 2009 at 8:38 PM, Takashi Iwai <tiwai@suse.de> wrote:
>> > At Tue, 6 Oct 2009 14:17:40 +0200,
>> > Harald Welte wrote:
>> >>
>> >> Hi all,
>> >>
>> >> On Tue, Oct 06, 2009 at 08:01:40AM +0200, Takashi Iwai wrote:
>> >>
>> >> > > As you describe, we implement it with self-reschedule workqueue, acting
>> >> > > like a timer.
>> >> > > Is there some advice to implement timer-like task, I tested  with timer and
>> >> > > it will crash the driver.
>> >> >
>> >> > Right, you can't call it in a softirq.
>> >> >
>> >> > A few problems, however:
>> >> > - present variable should be in struct via_spec, instead of a static
>> >> >   variable in update_hp_jack_state().
>> >>
>> >> I agree
>> >> > - This mechanism is unconditionally invoked on VT1708 although, in
>> >> >   theory, you can have a hardware that doesn't need HP jack detection
>> >>
>> >> Yes, Logan/Li/Lydia: If we generate this kind of workaround with a timer,
>> >> it should only be used on those particular chips that absolutely need it.
>> >
>> > As far as I see, the code is only for VT1708, so it's already specific
>> > to the codec chip.  But, even with this chip, you might have a pin
>> > configuration that doesn't need the HP jack detection.
>> >
>>
>> Yes, it's only for VT1708, and HP exists on most cases, and so HP jack detection
>> are needed.
>> And present will put into via_spec.
>
> If it's restricted only during the opened (better running) PCM stream,
> it's not too bad.  But, it'd be even better to check the availability
> of HP jack.
>
Sorry about analog loopback is not considered, I think analog loopback
mute/unmute
callback should also be added, is that OK?

>> >> > - Waking up each 100ms for the *whole* time is really bad regarding
>> >> >   the power-saving; can't it be optimized?
>> >>
>> >> I would personally actually suggest to simply not support headphone plug events
>> >> in case you are using such a [buggy?] chipset that does not support them.
>> >>
>> >> I mean, what do you gain from that event? It's not something super-important.
>> >>
>> >> waking up unconditionally every 100ms is really a too high burden on the
>> >> battery life.
>> >>
>> >> Another option might be to make it a module load time parameter, so people
>> >> can choose if they actually want to pay the high power/batyery price for this
>> >> small feature.
>> >
>> > Yes.
>> >
>> > Basically, if we can drop the analog-loopback feature, the HP jack
>> > probing can be simplified very much.  The probing is needed only when
>> > the PCM is opened/prepared and running.  It can be implemented in a
>> > similar way for the volume-update procedure at PCM trigger.
>> > And, during PCM running, (relatively) high-frequent polling isn't so
>> > critical.
>> >
>> >
>> > thanks,
>> >
>> > Takashi
>> >
>>
>> I know that jack detect on VT1708 that do not support unsolicited
>> response is weird, but
>>  it's for customer request (and so are some other patches:), we just
>> have to do something.
>>
>> I think it's good in open/close callback, I'll update.
>>
>> About self-reschedule workqueue, maybe there are some other choices,
>> or should we keep
>> using it?
>
> A delayed workqueue is OK.  But, it should check some flag before
> rescheduling itself.  Also, don't forget about suspend/resume.
>
>
> thanks,
>
> Takashi
>
Thank you, I will rewrite like this.

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

* Re: [PATCH 14/31] HDA patch_via.c: Add Jack detect feature for VT1708.
  2009-10-07  5:56               ` Li Bo
@ 2009-10-07  7:03                 ` Takashi Iwai
  2009-10-08  5:26                   ` Li Bo
  0 siblings, 1 reply; 12+ messages in thread
From: Takashi Iwai @ 2009-10-07  7:03 UTC (permalink / raw)
  To: Li Bo; +Cc: alsa-devel, LoganLi, Harald Welte, lydiawang

At Wed, 7 Oct 2009 13:56:35 +0800,
Li Bo wrote:
> 
> On Tue, Oct 6, 2009 at 11:34 PM, Takashi Iwai <tiwai@suse.de> wrote:
> > At Tue, 6 Oct 2009 23:27:35 +0800,
> > Li Bo wrote:
> >>
> >> On Tue, Oct 6, 2009 at 8:38 PM, Takashi Iwai <tiwai@suse.de> wrote:
> >> > At Tue, 6 Oct 2009 14:17:40 +0200,
> >> > Harald Welte wrote:
> >> >>
> >> >> Hi all,
> >> >>
> >> >> On Tue, Oct 06, 2009 at 08:01:40AM +0200, Takashi Iwai wrote:
> >> >>
> >> >> > > As you describe, we implement it with self-reschedule workqueue, acting
> >> >> > > like a timer.
> >> >> > > Is there some advice to implement timer-like task, I tested  with timer and
> >> >> > > it will crash the driver.
> >> >> >
> >> >> > Right, you can't call it in a softirq.
> >> >> >
> >> >> > A few problems, however:
> >> >> > - present variable should be in struct via_spec, instead of a static
> >> >> >   variable in update_hp_jack_state().
> >> >>
> >> >> I agree
> >> >> > - This mechanism is unconditionally invoked on VT1708 although, in
> >> >> >   theory, you can have a hardware that doesn't need HP jack detection
> >> >>
> >> >> Yes, Logan/Li/Lydia: If we generate this kind of workaround with a timer,
> >> >> it should only be used on those particular chips that absolutely need it.
> >> >
> >> > As far as I see, the code is only for VT1708, so it's already specific
> >> > to the codec chip.  But, even with this chip, you might have a pin
> >> > configuration that doesn't need the HP jack detection.
> >> >
> >>
> >> Yes, it's only for VT1708, and HP exists on most cases, and so HP jack detection
> >> are needed.
> >> And present will put into via_spec.
> >
> > If it's restricted only during the opened (better running) PCM stream,
> > it's not too bad.  But, it'd be even better to check the availability
> > of HP jack.
> >
> Sorry about analog loopback is not considered, I think analog loopback
> mute/unmute
> callback should also be added, is that OK?

Honestly, I think we can remove the analog-loopback part completely
for VT1708 + HP-jack.  If the loopback is inevitably needed, you can add
a check of codec "hint" interface to determine whether to build the
HP-jack detection or not at probing.
About hints, look at snd_hda_get_bool_hint() and HD-Audio.txt.


thanks,

Takashi
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

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

* Re: [PATCH 14/31] HDA patch_via.c: Add Jack detect feature for VT1708.
  2009-10-07  7:03                 ` Takashi Iwai
@ 2009-10-08  5:26                   ` Li Bo
  2009-10-08  5:28                     ` Li Bo
  0 siblings, 1 reply; 12+ messages in thread
From: Li Bo @ 2009-10-08  5:26 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: alsa-devel, LoganLi, Harald Welte, lydiawang

On Wed, Oct 7, 2009 at 3:03 PM, Takashi Iwai <tiwai@suse.de> wrote:
> At Wed, 7 Oct 2009 13:56:35 +0800,
> Li Bo wrote:
>>
>> On Tue, Oct 6, 2009 at 11:34 PM, Takashi Iwai <tiwai@suse.de> wrote:
>> > At Tue, 6 Oct 2009 23:27:35 +0800,
>> > Li Bo wrote:
>> >>
>> >> On Tue, Oct 6, 2009 at 8:38 PM, Takashi Iwai <tiwai@suse.de> wrote:
>> >> > At Tue, 6 Oct 2009 14:17:40 +0200,
>> >> > Harald Welte wrote:
>> >> >>
>> >> >> Hi all,
>> >> >>
>> >> >> On Tue, Oct 06, 2009 at 08:01:40AM +0200, Takashi Iwai wrote:
>> >> >>
>> >> >> > > As you describe, we implement it with self-reschedule workqueue, acting
>> >> >> > > like a timer.
>> >> >> > > Is there some advice to implement timer-like task, I tested  with timer and
>> >> >> > > it will crash the driver.
>> >> >> >
>> >> >> > Right, you can't call it in a softirq.
>> >> >> >
>> >> >> > A few problems, however:
>> >> >> > - present variable should be in struct via_spec, instead of a static
>> >> >> >   variable in update_hp_jack_state().
>> >> >>
>> >> >> I agree
>> >> >> > - This mechanism is unconditionally invoked on VT1708 although, in
>> >> >> >   theory, you can have a hardware that doesn't need HP jack detection
>> >> >>
>> >> >> Yes, Logan/Li/Lydia: If we generate this kind of workaround with a timer,
>> >> >> it should only be used on those particular chips that absolutely need it.
>> >> >
>> >> > As far as I see, the code is only for VT1708, so it's already specific
>> >> > to the codec chip.  But, even with this chip, you might have a pin
>> >> > configuration that doesn't need the HP jack detection.
>> >> >
>> >>
>> >> Yes, it's only for VT1708, and HP exists on most cases, and so HP jack detection
>> >> are needed.
>> >> And present will put into via_spec.
>> >
>> > If it's restricted only during the opened (better running) PCM stream,
>> > it's not too bad.  But, it'd be even better to check the availability
>> > of HP jack.
>> >
>> Sorry about analog loopback is not considered, I think analog loopback
>> mute/unmute
>> callback should also be added, is that OK?
>
> Honestly, I think we can remove the analog-loopback part completely
> for VT1708 + HP-jack.  If the loopback is inevitably needed, you can add
> a check of codec "hint" interface to determine whether to build the
> HP-jack detection or not at probing.
> About hints, look at snd_hda_get_bool_hint() and HD-Audio.txt.
>
>
> thanks,
>
> Takashi
>
OK, I add hp detect to prepare/cleanup, and check hint
"analog_loopback_hp_detect"
before adding it to analog mute/unmute.
Following will be the update patch

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

* Re: [PATCH 14/31] HDA patch_via.c: Add Jack detect feature for VT1708.
  2009-10-08  5:26                   ` Li Bo
@ 2009-10-08  5:28                     ` Li Bo
  0 siblings, 0 replies; 12+ messages in thread
From: Li Bo @ 2009-10-08  5:28 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: alsa-devel, LoganLi, Harald Welte, lydiawang

[ALSA] HDA VIA: Add Jack detect feature for VT1708.

VT1708 does not support unsolicited response, but we need hp detect to
automute speaker. Implemented in workqueue.

Signed-off-by: Lydia Wang <lydiawang@viatech.com.cn>

---
 sound/pci/hda/patch_via.c |  224 ++++++++++++++++++++++++++++++++++------------
 1 file changed, 168 insertions(+), 56 deletions(-)

--- a/sound/pci/hda/patch_via.c
+++ b/sound/pci/hda/patch_via.c
@@ -89,6 +89,64 @@
 	CODEC_TYPES,
 };

+struct via_spec {
+	/* codec parameterization */
+	struct snd_kcontrol_new *mixers[4];
+	unsigned int num_mixers;
+
+	struct hda_verb *init_verbs[5];
+	unsigned int num_iverbs;
+
+	char *stream_name_analog;
+	struct hda_pcm_stream *stream_analog_playback;
+	struct hda_pcm_stream *stream_analog_capture;
+
+	char *stream_name_digital;
+	struct hda_pcm_stream *stream_digital_playback;
+	struct hda_pcm_stream *stream_digital_capture;
+
+	/* playback */
+	struct hda_multi_out multiout;
+	hda_nid_t slave_dig_outs[2];
+
+	/* capture */
+	unsigned int num_adc_nids;
+	hda_nid_t *adc_nids;
+	hda_nid_t mux_nids[3];
+	hda_nid_t dig_in_nid;
+	hda_nid_t dig_in_pin;
+
+	/* capture source */
+	const struct hda_input_mux *input_mux;
+	unsigned int cur_mux[3];
+
+	/* PCM information */
+	struct hda_pcm pcm_rec[3];
+
+	/* dynamic controls, init_verbs and input_mux */
+	struct auto_pin_cfg autocfg;
+	struct snd_array kctls;
+	struct hda_input_mux private_imux[2];
+	hda_nid_t private_dac_nids[AUTO_CFG_MAX_OUTS];
+
+	/* HP mode source */
+	const struct hda_input_mux *hp_mux;
+	unsigned int hp_independent_mode;
+	unsigned int hp_independent_mode_index;
+	unsigned int smart51_enabled;
+
+	enum VIA_HDA_CODEC codec_type;
+
+	/* work to check hp jack state */
+	struct hda_codec *codec;
+	struct delayed_work vt1708_hp_work;
+	int vt1708_jack_detectect;
+	int vt1708_hp_present;
+#ifdef CONFIG_SND_HDA_POWER_SAVE
+	struct hda_loopback_check loopback;
+#endif
+};
+
 static enum VIA_HDA_CODEC get_codec_type(struct hda_codec *codec)
 {
 	u32 vendor_id = codec->vendor_id;
@@ -181,6 +239,28 @@

 static void analog_low_current_mode(struct hda_codec *codec, int stream_idle);
 static void set_jack_power_state(struct hda_codec *codec);
+static int is_aa_path_mute(struct hda_codec *codec);
+
+static void vt1708_start_hp_work(struct via_spec *spec)
+{
+	if (spec->codec_type != VT1708)
+		return;
+	snd_hda_codec_write(spec->codec, 0x1, 0, 0xf81,
+			    !spec->vt1708_jack_detectect);
+	if (!delayed_work_pending(&spec->vt1708_hp_work))
+		schedule_delayed_work(&spec->vt1708_hp_work,
+				      msecs_to_jiffies(100));
+}
+
+static void vt1708_stop_hp_work(struct via_spec *spec)
+{
+	if (spec->codec_type != VT1708)
+		return;
+	snd_hda_codec_write(spec->codec, 0x1, 0, 0xf81,
+			    !spec->vt1708_jack_detectect);
+	cancel_delayed_work(&spec->vt1708_hp_work);
+	flush_scheduled_work();
+}

 static int analog_input_switch_put(struct snd_kcontrol *kcontrol,
 				   struct snd_ctl_elem_value *ucontrol)
@@ -190,6 +270,12 @@

 	set_jack_power_state(codec);
 	analog_low_current_mode(snd_kcontrol_chip(kcontrol), -1);
+	if (snd_hda_get_bool_hint(codec, "analog_loopback_hp_detect") == 1) {
+		if (is_aa_path_mute(codec))
+			vt1708_start_hp_work(codec->spec);
+		else
+			vt1708_stop_hp_work(codec->spec);
+	}
 	return change;
 }

@@ -210,59 +296,6 @@
 };


-struct via_spec {
-	/* codec parameterization */
-	struct snd_kcontrol_new *mixers[4];
-	unsigned int num_mixers;
-
-	struct hda_verb *init_verbs[5];
-	unsigned int num_iverbs;
-
-	char *stream_name_analog;
-	struct hda_pcm_stream *stream_analog_playback;
-	struct hda_pcm_stream *stream_analog_capture;
-
-	char *stream_name_digital;
-	struct hda_pcm_stream *stream_digital_playback;
-	struct hda_pcm_stream *stream_digital_capture;
-
-	/* playback */
-	struct hda_multi_out multiout;
-	hda_nid_t slave_dig_outs[2];
-
-	/* capture */
-	unsigned int num_adc_nids;
-	hda_nid_t *adc_nids;
-	hda_nid_t mux_nids[3];
-	hda_nid_t dig_in_nid;
-	hda_nid_t dig_in_pin;
-
-	/* capture source */
-	const struct hda_input_mux *input_mux;
-	unsigned int cur_mux[3];
-
-	/* PCM information */
-	struct hda_pcm pcm_rec[3];
-
-	/* dynamic controls, init_verbs and input_mux */
-	struct auto_pin_cfg autocfg;
-	struct snd_array kctls;
-	struct hda_input_mux private_imux[2];
-	hda_nid_t private_dac_nids[AUTO_CFG_MAX_OUTS];
-
-	/* HP mode source */
-	const struct hda_input_mux *hp_mux;
-	unsigned int hp_independent_mode;
-	unsigned int hp_independent_mode_index;
-	unsigned int smart51_enabled;
-
-	enum VIA_HDA_CODEC codec_type;
-
-#ifdef CONFIG_SND_HDA_POWER_SAVE
-	struct hda_loopback_check loopback;
-#endif
-};
-
 static hda_nid_t vt1708_adc_nids[2] = {
 	/* ADC1-2 */
 	0x15, 0x27
@@ -1094,7 +1127,7 @@
 			snd_hda_codec_setup_stream(codec, mout->hp_nid,
 						   stream_tag, 0, format);
 	}
-
+	vt1708_start_hp_work(spec);
 	return 0;
 }

@@ -1134,7 +1167,7 @@
 			snd_hda_codec_setup_stream(codec, mout->hp_nid,
 						   0, 0, 0);
 	}
-
+	vt1708_stop_hp_work(spec);
 	return 0;
 }

@@ -1345,6 +1378,7 @@
 		return;

 	via_free_kctls(codec);
+	vt1708_stop_hp_work(spec);
 	kfree(codec->spec);
 }

@@ -1464,6 +1498,15 @@
  	return 0;
 }

+#ifdef SND_HDA_NEEDS_RESUME
+static int via_suspend(struct hda_codec *codec, pm_message_t state)
+{
+	struct via_spec *spec = codec->spec;
+	vt1708_stop_hp_work(spec);
+	return 0;
+}
+#endif
+
 #ifdef CONFIG_SND_HDA_POWER_SAVE
 static int via_check_power_status(struct hda_codec *codec, hda_nid_t nid)
 {
@@ -1479,6 +1522,9 @@
 	.build_pcms = via_build_pcms,
 	.init = via_init,
 	.free = via_free,
+#ifdef SND_HDA_NEEDS_RESUME
+	.suspend = via_suspend,
+#endif
 #ifdef CONFIG_SND_HDA_POWER_SAVE
 	.check_power_status = via_check_power_status,
 #endif
@@ -1728,6 +1774,51 @@
 	return;
 }

+static int vt1708_jack_detectect_get(struct snd_kcontrol *kcontrol,
+				     struct snd_ctl_elem_value *ucontrol)
+{
+	struct hda_codec *codec = snd_kcontrol_chip(kcontrol);
+	struct via_spec *spec = codec->spec;
+
+	if (spec->codec_type != VT1708)
+		return 0;
+	spec->vt1708_jack_detectect =
+		!((snd_hda_codec_read(codec, 0x1, 0, 0xf84, 0) >> 8) & 0x1);
+	ucontrol->value.integer.value[0] = spec->vt1708_jack_detectect;
+	return 0;
+}
+
+static int vt1708_jack_detectect_put(struct snd_kcontrol *kcontrol,
+				     struct snd_ctl_elem_value *ucontrol)
+{
+	struct hda_codec *codec = snd_kcontrol_chip(kcontrol);
+	struct via_spec *spec = codec->spec;
+	int change;
+
+	if (spec->codec_type != VT1708)
+		return 0;
+	spec->vt1708_jack_detectect = ucontrol->value.integer.value[0];
+	change = (0x1 & (snd_hda_codec_read(codec, 0x1, 0, 0xf84, 0) >> 8))
+		== !spec->vt1708_jack_detectect;
+	if (spec->vt1708_jack_detectect) {
+		mute_aa_path(codec, 1);
+		notify_aa_path_ctls(codec);
+	}
+	return change;
+}
+
+static struct snd_kcontrol_new vt1708_jack_detectect[] = {
+	{
+		.iface = SNDRV_CTL_ELEM_IFACE_MIXER,
+		.name = "Jack Detect",
+		.count = 1,
+		.info = snd_ctl_boolean_mono_info,
+		.get = vt1708_jack_detectect_get,
+		.put = vt1708_jack_detectect_put,
+	},
+	{} /* end */
+};
+
 static int vt1708_parse_auto_config(struct hda_codec *codec)
 {
 	struct via_spec *spec = codec->spec;
@@ -1755,6 +1846,10 @@
 	err = vt1708_auto_create_analog_input_ctls(spec, &spec->autocfg);
 	if (err < 0)
 		return err;
+	/* add jack detect on/off control */
+	err = snd_hda_add_new_ctls(codec, vt1708_jack_detectect);
+	if (err < 0)
+		return err;

 	spec->multiout.max_channels = spec->multiout.num_dacs * 2;

@@ -1788,6 +1883,22 @@
 	return 0;
 }

+static void vt1708_update_hp_jack_state(struct work_struct *work)
+{
+	struct via_spec *spec = container_of(work, struct via_spec,
+					     vt1708_hp_work.work);
+	if (spec->codec_type != VT1708)
+		return;
+	/* if jack state toggled */
+	if (spec->vt1708_hp_present
+	    != (snd_hda_codec_read(spec->codec, spec->autocfg.hp_pins[0], 0,
+				   AC_VERB_GET_PIN_SENSE, 0) >> 31)) {
+		spec->vt1708_hp_present ^= 1;
+		via_hp_automute(spec->codec);
+	}
+	vt1708_start_hp_work(spec);
+}
+
 static int get_mux_nids(struct hda_codec *codec)
 {
 	struct via_spec *spec = codec->spec;
@@ -1864,7 +1975,8 @@
 #ifdef CONFIG_SND_HDA_POWER_SAVE
 	spec->loopback.amplist = vt1708_loopbacks;
 #endif
-
+	spec->codec = codec;
+	INIT_DELAYED_WORK(&spec->vt1708_hp_work, vt1708_update_hp_jack_state);
 	return 0;
 }

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

end of thread, other threads:[~2009-10-08  5:28 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-10-05 14:27 [PATCH 14/31] HDA patch_via.c: Add Jack detect feature for VT1708 Li Bo
2009-10-05 15:12 ` Takashi Iwai
2009-10-06  5:06   ` Li Bo
2009-10-06  6:01     ` Takashi Iwai
2009-10-06 12:17       ` Harald Welte
2009-10-06 12:38         ` Takashi Iwai
2009-10-06 15:27           ` Li Bo
2009-10-06 15:34             ` Takashi Iwai
2009-10-07  5:56               ` Li Bo
2009-10-07  7:03                 ` Takashi Iwai
2009-10-08  5:26                   ` Li Bo
2009-10-08  5:28                     ` Li Bo

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.