[alsa-devel] [PATCH] ALSA: hda - Add snd_hda_jack_detect_state() helper function

[Takashi Iwai]

At Sat, 20 Jul 2013 20:39:56 +0200,
David Henningsson wrote:
> 
> On 07/19/2013 05:06 PM, Takashi Iwai wrote:
> > snd_hda_jack_detect() function returns a boolean value for a jack
> > plugged in or not, but it also returns always true when the
> > corresponding pin is phantom (i.e. fixed).  This is OK in most cases,
> > but it makes the generic parser misbehaving about the auto-mute or
> > auto-mic switching, e.g. when one of headphone pins is a fixed.
> > Namely, the driver decides whether to mute the speaker or not, just
> > depending on the headphone plug state: if one of the headphone jacks
> > is seen as active, then the speaker is muted.  Thus this will result
> > always in the muted speaker output.
> >
> > So, the problem is the function returns a boolean, after all, although
> > we need to think of "phantom" jack.  Now a new function,
> > snd_hda_jack_detect_state() is introduced to return these tristates.
> > The generic parser uses this function for checking the headphone or
> > mic jack states.
> >
> > Meanwhile, the behavior of snd_hda_jack_detect() is kept as is, for
> > keeping compatibility in other driver codes.
> >
> > Signed-off-by: Takashi Iwai <tiwai at suse.de>
> 
> Acked-by: David Henningsson <david.henningsson at canonical.com>
> 
> Although I doubt this is needed on the input side. If we have more than 
> one input source that is fixed/phantom, we should always have a capture 
> switch rather than auto-mic switch, right?

It'd be better to use the explicit HDA_JACK_PRESENT check, yes, but
in theory this shouldn't be an issue for the current auto-mic
switching scheme because the capture source is mutual exclusive while
the output can be multiplexing.


thanks,

Takashi

> 
> > ---
> >   sound/pci/hda/hda_generic.c |  8 +++++---
> >   sound/pci/hda/hda_jack.c    | 18 ++++++++++++------
> >   sound/pci/hda/hda_jack.h    | 13 ++++++++++++-
> >   3 files changed, 29 insertions(+), 10 deletions(-)
> >
> > diff --git a/sound/pci/hda/hda_generic.c b/sound/pci/hda/hda_generic.c
> > index 8e77cbb..f5c2d1f 100644
> > --- a/sound/pci/hda/hda_generic.c
> > +++ b/sound/pci/hda/hda_generic.c
> > @@ -3724,7 +3724,8 @@ static int mux_select(struct hda_codec *codec, unsigned int adc_idx,
> >   /* check each pin in the given array; returns true if any of them is plugged */
> >   static bool detect_jacks(struct hda_codec *codec, int num_pins, hda_nid_t *pins)
> >   {
> > -	int i, present = 0;
> > +	int i;
> > +	bool present = false;
> >
> >   	for (i = 0; i < num_pins; i++) {
> >   		hda_nid_t nid = pins[i];
> > @@ -3733,7 +3734,8 @@ static bool detect_jacks(struct hda_codec *codec, int num_pins, hda_nid_t *pins)
> >   		/* don't detect pins retasked as inputs */
> >   		if (snd_hda_codec_get_pin_target(codec, nid) & AC_PINCTL_IN_EN)
> >   			continue;
> > -		present |= snd_hda_jack_detect(codec, nid);
> > +		if (snd_hda_jack_detect_state(codec, nid) == HDA_JACK_PRESENT)
> > +			present = true;
> >   	}
> >   	return present;
> >   }
> > @@ -3887,7 +3889,7 @@ void snd_hda_gen_mic_autoswitch(struct hda_codec *codec, struct hda_jack_tbl *ja
> >   		/* don't detect pins retasked as outputs */
> >   		if (snd_hda_codec_get_pin_target(codec, pin) & AC_PINCTL_OUT_EN)
> >   			continue;
> > -		if (snd_hda_jack_detect(codec, pin)) {
> > +		if (snd_hda_jack_detect_state(codec, pin) == HDA_JACK_PRESENT) {
> >   			mux_select(codec, 0, spec->am_entry[i].idx);
> >   			return;
> >   		}
> > diff --git a/sound/pci/hda/hda_jack.c b/sound/pci/hda/hda_jack.c
> > index 3fd2973..dc93761 100644
> > --- a/sound/pci/hda/hda_jack.c
> > +++ b/sound/pci/hda/hda_jack.c
> > @@ -194,18 +194,24 @@ u32 snd_hda_pin_sense(struct hda_codec *codec, hda_nid_t nid)
> >   EXPORT_SYMBOL_HDA(snd_hda_pin_sense);
> >
> >   /**
> > - * snd_hda_jack_detect - query pin Presence Detect status
> > + * snd_hda_jack_detect_state - query pin Presence Detect status
> >    * @codec: the CODEC to sense
> >    * @nid: the pin NID to sense
> >    *
> > - * Query and return the pin's Presence Detect status.
> > + * Query and return the pin's Presence Detect status, as either
> > + * HDA_JACK_NOT_PRESENT, HDA_JACK_PRESENT or HDA_JACK_PHANTOM.
> >    */
> > -int snd_hda_jack_detect(struct hda_codec *codec, hda_nid_t nid)
> > +int snd_hda_jack_detect_state(struct hda_codec *codec, hda_nid_t nid)
> >   {
> > -	u32 sense = snd_hda_pin_sense(codec, nid);
> > -	return get_jack_plug_state(sense);
> > +	struct hda_jack_tbl *jack = snd_hda_jack_tbl_get(codec, nid);
> > +	if (jack && jack->phantom_jack)
> > +		return HDA_JACK_PHANTOM;
> > +	else if (snd_hda_pin_sense(codec, nid) & AC_PINSENSE_PRESENCE)
> > +		return HDA_JACK_PRESENT;
> > +	else
> > +		return HDA_JACK_NOT_PRESENT;
> >   }
> > -EXPORT_SYMBOL_HDA(snd_hda_jack_detect);
> > +EXPORT_SYMBOL_HDA(snd_hda_jack_detect_state);
> >
> >   /**
> >    * snd_hda_jack_detect_enable - enable the jack-detection
> > diff --git a/sound/pci/hda/hda_jack.h b/sound/pci/hda/hda_jack.h
> > index ec12abd..379420c 100644
> > --- a/sound/pci/hda/hda_jack.h
> > +++ b/sound/pci/hda/hda_jack.h
> > @@ -75,7 +75,18 @@ int snd_hda_jack_set_gating_jack(struct hda_codec *codec, hda_nid_t gated_nid,
> >   				 hda_nid_t gating_nid);
> >
> >   u32 snd_hda_pin_sense(struct hda_codec *codec, hda_nid_t nid);
> > -int snd_hda_jack_detect(struct hda_codec *codec, hda_nid_t nid);
> > +
> > +/* the jack state returned from snd_hda_jack_detect_state() */
> > +enum {
> > +	HDA_JACK_NOT_PRESENT, HDA_JACK_PRESENT, HDA_JACK_PHANTOM,
> > +};
> > +
> > +int snd_hda_jack_detect_state(struct hda_codec *codec, hda_nid_t nid);
> > +
> > +static inline bool snd_hda_jack_detect(struct hda_codec *codec, hda_nid_t nid)
> > +{
> > +	return snd_hda_jack_detect_state(codec, nid) != HDA_JACK_NOT_PRESENT;
> > +}
> >
> >   bool is_jack_detectable(struct hda_codec *codec, hda_nid_t nid);
> >
> >
> 
> 
> 
> -- 
> David Henningsson, Canonical Ltd.
> https://launchpad.net/~diwic
>