[alsa-devel] [PATCH] ALSA: hda - unmute pin amplifier in infoframe setup for Haswell

Lin, Mengdong mengdong.lin at intel.com
Thu Sep 5 03:41:10 CEST 2013


Hi Takashi and David,

Thanks for your comments! I'll split the patch.

Broadwell has the same HW behavior as Haswell. Maybe in the future, is_haswell() can be renamed to is_haswell_plus(),
'plus' means including the successor processors. This is the naming convention used in our documents.

"is_haswell_or_broadwell_or_somethingwell_or_verywell()" really sounds like "is_everywhere", considering these specific fixings :-)

Thanks
Mengdong

> -----Original Message-----
> From: Takashi Iwai [mailto:tiwai at suse.de]
> Sent: Wednesday, September 04, 2013 6:04 PM
> To: David Henningsson
> Cc: Lin, Mengdong; alsa-devel at alsa-project.org
> Subject: Re: [PATCH] ALSA: hda - unmute pin amplifier in infoframe setup for
> Haswell
> 
> At Wed, 04 Sep 2013 11:55:27 +0200,
> David Henningsson wrote:
> >
> > On 09/03/2013 10:38 PM, mengdong.lin at intel.com wrote:
> > > From: Mengdong Lin <mengdong.lin at intel.com>
> > >
> > > When Gfx driver reconnects a port and transcoder, the pin amplifier
> > > will be muted. To enable sound, the pin amp need to be unmuted.
> > >
> > > This patch
> > > - moves pin amp unmuting from stream preparing to
> hdmi_setup_audio_infoframe().
> > >   So if port:transcoder reconnection happens during stream playback, the
> ELDV
> > >   unsol event can stil trigger pin's amp unmuting when re-setting up audio
> > >   info frame.
> > >
> > > - remove reading pin amp status before unmuting for speed-up, since pin
> amp
> > >   should always be unmuted.
> > >
> > > - rename haswell_verify_pin_D0() to haswell_verify_pin_cvt_D0(), since the
> > >   convertor power state is also fixed here.
> >
> > Or just haswell_verify_D0. Anyway, looks good to me.
> >
> > > - define is_haswell() to replace checking Haswell vendor ID everywhere.
> >
> > I agree with this, but Takashi might want refactoring and behavioural
> > changes in different patches.
> 
> Have no patches right now.  Feel free to go forward.
> 
> > Side note: One can also wonder what happens in future generations of
> > Intel hardware (Broadwell etc), if we end up with a
> > is_haswell_or_broadwell_or_somethingwell_or_verywell() after a while
> > :-)
> 
> Yeah, but we'll keep maintaining the driver code, so changing this would be
> trivial ;)
> 
> Though, it'd be cleaner to split this patch: one for defining and replacing with
> is_hawell() (or whatever), and another actually changes
> haswell_veriy_pin_D0().
> 
> 
> Takashi
> 
> > > This patch is mostly based on suggestion of David Henningsson.
> > >
> > > Cc: David Henningsson <david.henningsson at canonical.com>
> > > Signed-off-by: Mengdong Lin <mengdong.lin at intel.com>
> > >
> > > diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
> > > index 9a58893..471c158 100644
> > > --- a/sound/pci/hda/patch_hdmi.c
> > > +++ b/sound/pci/hda/patch_hdmi.c
> > > @@ -44,6 +44,8 @@ static bool static_hdmi_pcm;
> > > module_param(static_hdmi_pcm, bool, 0644);
> > > MODULE_PARM_DESC(static_hdmi_pcm, "Don't restrict PCM parameters
> per
> > > ELD info");
> > >
> > > +#define is_haswell(codec)  ((codec)->vendor_id == 0x80862807)
> > > +
> > >  struct hdmi_spec_per_cvt {
> > >  	hda_nid_t cvt_nid;
> > >  	int assigned;
> > > @@ -894,6 +896,11 @@ static void hdmi_setup_audio_infoframe(struct
> hda_codec *codec,
> > >  	if (!channels)
> > >  		return;
> > >
> > > +	if (is_haswell(codec))
> > > +		snd_hda_codec_write(codec, pin_nid, 0,
> > > +					    AC_VERB_SET_AMP_GAIN_MUTE,
> > > +					    AMP_OUT_UNMUTE);
> > > +
> > >  	eld = &per_pin->sink_eld;
> > >  	if (!eld->monitor_present)
> > >  		return;
> > > @@ -1033,10 +1040,10 @@ static void hdmi_unsol_event(struct
> hda_codec *codec, unsigned int res)
> > >  		hdmi_non_intrinsic_event(codec, res);  }
> > >
> > > -static void haswell_verify_pin_D0(struct hda_codec *codec,
> > > +static void haswell_verify_pin_cvt_D0(struct hda_codec *codec,
> > >  		hda_nid_t cvt_nid, hda_nid_t nid)  {
> > > -	int pwr, lamp, ramp;
> > > +	int pwr;
> > >
> > >  	/* For Haswell, the converter 1/2 may keep in D3 state after bootup,
> > >  	 * thus pins could only choose converter 0 for use. Make sure the
> > > @@ -1052,25 +1059,6 @@ static void haswell_verify_pin_D0(struct
> hda_codec *codec,
> > >  		pwr = (pwr & AC_PWRST_ACTUAL) >>
> AC_PWRST_ACTUAL_SHIFT;
> > >  		snd_printd("Haswell HDMI audio: Power for pin 0x%x is now
> D%d\n", nid, pwr);
> > >  	}
> > > -
> > > -	lamp = snd_hda_codec_read(codec, nid, 0,
> > > -				  AC_VERB_GET_AMP_GAIN_MUTE,
> > > -				  AC_AMP_GET_LEFT | AC_AMP_GET_OUTPUT);
> > > -	ramp = snd_hda_codec_read(codec, nid, 0,
> > > -				  AC_VERB_GET_AMP_GAIN_MUTE,
> > > -				  AC_AMP_GET_RIGHT | AC_AMP_GET_OUTPUT);
> > > -	if (lamp != ramp) {
> > > -		snd_hda_codec_write(codec, nid, 0,
> AC_VERB_SET_AMP_GAIN_MUTE,
> > > -				    AC_AMP_SET_RIGHT | AC_AMP_SET_OUTPUT |
> lamp);
> > > -
> > > -		lamp = snd_hda_codec_read(codec, nid, 0,
> > > -				  AC_VERB_GET_AMP_GAIN_MUTE,
> > > -				  AC_AMP_GET_LEFT | AC_AMP_GET_OUTPUT);
> > > -		ramp = snd_hda_codec_read(codec, nid, 0,
> > > -				  AC_VERB_GET_AMP_GAIN_MUTE,
> > > -				  AC_AMP_GET_RIGHT | AC_AMP_GET_OUTPUT);
> > > -		snd_printd("Haswell HDMI audio: Mute after set on pin 0x%x: [0x%x
> 0x%x]\n", nid, lamp, ramp);
> > > -	}
> > >  }
> > >
> > >  /*
> > > @@ -1087,8 +1075,8 @@ static int hdmi_setup_stream(struct hda_codec
> *codec, hda_nid_t cvt_nid,
> > >  	int pinctl;
> > >  	int new_pinctl = 0;
> > >
> > > -	if (codec->vendor_id == 0x80862807)
> > > -		haswell_verify_pin_D0(codec, cvt_nid, pin_nid);
> > > +	if (is_haswell(codec))
> > > +		haswell_verify_pin_cvt_D0(codec, cvt_nid, pin_nid);
> > >
> > >  	if (snd_hda_query_pin_caps(codec, pin_nid) & AC_PINCAP_HBR) {
> > >  		pinctl = snd_hda_codec_read(codec, pin_nid, 0, @@ -1227,7
> +1215,7
> > > @@ static int hdmi_pcm_open(struct hda_pcm_stream *hinfo,
> > >  			    mux_idx);
> > >
> > >  	/* configure unused pins to choose other converters */
> > > -	if (codec->vendor_id == 0x80862807)
> > > +	if (is_haswell(codec))
> > >  		haswell_config_cvts(codec, pin_idx, mux_idx);
> > >
> > >  	snd_hda_spdif_ctls_assign(codec, pin_idx, per_cvt->cvt_nid); @@
> > > -1358,14 +1346,10 @@ static void hdmi_present_sense(struct
> hdmi_spec_per_pin *per_pin, int repoll)
> > >  		/* Haswell-specific workaround: re-setup when the transcoder is
> > >  		 * changed during the stream playback
> > >  		 */
> > > -		if (codec->vendor_id == 0x80862807 &&
> > > -		    eld->eld_valid && !old_eld_valid && per_pin->setup) {
> > > -			snd_hda_codec_write(codec, pin_nid, 0,
> > > -					    AC_VERB_SET_AMP_GAIN_MUTE,
> > > -					    AMP_OUT_UNMUTE);
> > > +		if (is_haswell(codec) &&
> > > +		    eld->eld_valid && !old_eld_valid && per_pin->setup)
> > >  			hdmi_setup_audio_infoframe(codec, per_pin,
> > >  						   per_pin->non_pcm);
> > > -		}
> > >  	}
> > >  	mutex_unlock(&pin_eld->lock);
> > >
> > > @@ -1405,7 +1389,7 @@ static int hdmi_add_pin(struct hda_codec
> *codec, hda_nid_t pin_nid)
> > >  	if (get_defcfg_connect(config) == AC_JACK_PORT_NONE)
> > >  		return 0;
> > >
> > > -	if (codec->vendor_id == 0x80862807)
> > > +	if (is_haswell(codec))
> > >  		intel_haswell_fixup_connect_list(codec, pin_nid);
> > >
> > >  	pin_idx = spec->num_pins;
> > > @@ -2014,7 +1998,7 @@ static int patch_generic_hdmi(struct hda_codec
> *codec)
> > >  	codec->spec = spec;
> > >  	hdmi_array_init(spec, 4);
> > >
> > > -	if (codec->vendor_id == 0x80862807) {
> > > +	if (is_haswell(codec)) {
> > >  		intel_haswell_enable_all_pins(codec, true);
> > >  		intel_haswell_fixup_enable_dp12(codec);
> > >  	}
> > > @@ -2025,7 +2009,7 @@ static int patch_generic_hdmi(struct hda_codec
> *codec)
> > >  		return -EINVAL;
> > >  	}
> > >  	codec->patch_ops = generic_hdmi_patch_ops;
> > > -	if (codec->vendor_id == 0x80862807) {
> > > +	if (is_haswell(codec)) {
> > >  		codec->patch_ops.set_power_state =
> haswell_set_power_state;
> > >  		codec->dp_mst = true;
> > >  	}
> > >
> >
> >
> >
> > --
> > David Henningsson, Canonical Ltd.
> > https://launchpad.net/~diwic
> >


More information about the Alsa-devel mailing list