* [PATCH] ALSA: hda - unmute pin amplifier in infoframe setup for Haswell
@ 2013-09-03 20:38 mengdong.lin
2013-09-04 9:55 ` David Henningsson
0 siblings, 1 reply; 4+ messages in thread
From: mengdong.lin @ 2013-09-03 20:38 UTC (permalink / raw)
To: alsa-devel, tiwai; +Cc: Mengdong Lin, David Henningsson
From: Mengdong Lin <mengdong.lin@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.
- define is_haswell() to replace checking Haswell vendor ID everywhere.
This patch is mostly based on suggestion of David Henningsson.
Cc: David Henningsson <david.henningsson@canonical.com>
Signed-off-by: Mengdong Lin <mengdong.lin@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;
}
--
1.8.1.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] ALSA: hda - unmute pin amplifier in infoframe setup for Haswell
2013-09-03 20:38 [PATCH] ALSA: hda - unmute pin amplifier in infoframe setup for Haswell mengdong.lin
@ 2013-09-04 9:55 ` David Henningsson
2013-09-04 10:04 ` Takashi Iwai
0 siblings, 1 reply; 4+ messages in thread
From: David Henningsson @ 2013-09-04 9:55 UTC (permalink / raw)
To: mengdong.lin; +Cc: tiwai, alsa-devel
On 09/03/2013 10:38 PM, mengdong.lin@intel.com wrote:
> From: Mengdong Lin <mengdong.lin@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.
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 :-)
>
> This patch is mostly based on suggestion of David Henningsson.
>
> Cc: David Henningsson <david.henningsson@canonical.com>
> Signed-off-by: Mengdong Lin <mengdong.lin@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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ALSA: hda - unmute pin amplifier in infoframe setup for Haswell
2013-09-04 9:55 ` David Henningsson
@ 2013-09-04 10:04 ` Takashi Iwai
2013-09-05 1:41 ` Lin, Mengdong
0 siblings, 1 reply; 4+ messages in thread
From: Takashi Iwai @ 2013-09-04 10:04 UTC (permalink / raw)
To: David Henningsson; +Cc: mengdong.lin, alsa-devel
At Wed, 04 Sep 2013 11:55:27 +0200,
David Henningsson wrote:
>
> On 09/03/2013 10:38 PM, mengdong.lin@intel.com wrote:
> > From: Mengdong Lin <mengdong.lin@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@canonical.com>
> > Signed-off-by: Mengdong Lin <mengdong.lin@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
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ALSA: hda - unmute pin amplifier in infoframe setup for Haswell
2013-09-04 10:04 ` Takashi Iwai
@ 2013-09-05 1:41 ` Lin, Mengdong
0 siblings, 0 replies; 4+ messages in thread
From: Lin, Mengdong @ 2013-09-05 1:41 UTC (permalink / raw)
To: Takashi Iwai, David Henningsson; +Cc: alsa-devel@alsa-project.org
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@suse.de]
> Sent: Wednesday, September 04, 2013 6:04 PM
> To: David Henningsson
> Cc: Lin, Mengdong; alsa-devel@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@intel.com wrote:
> > > From: Mengdong Lin <mengdong.lin@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@canonical.com>
> > > Signed-off-by: Mengdong Lin <mengdong.lin@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
> >
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-09-05 1:41 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-09-03 20:38 [PATCH] ALSA: hda - unmute pin amplifier in infoframe setup for Haswell mengdong.lin
2013-09-04 9:55 ` David Henningsson
2013-09-04 10:04 ` Takashi Iwai
2013-09-05 1:41 ` Lin, Mengdong
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.