[alsa-devel] [PATCH 14/31] HDA patch_via.c: Add Jack detect feature for VT1708.

Takashi Iwai tiwai at suse.de
Tue Oct 6 08:01:40 CEST 2009


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 at 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 at 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;
> >>  }
> >>
> >
> 


More information about the Alsa-devel mailing list