| Message ID | ae7e1c58c756e7149bc8b93f648bc56fd7af73d1.1395291484.git.mengdong.lin@intel.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
At Thu, 20 Mar 2014 13:01:06 +0800, mengdong.lin@intel.com wrote: > > From: Mengdong Lin <mengdong.lin@intel.com> > > This is a temporary fix for some Intel HDMI codecs to avoid no sound output for > a resuming playback after S3. > > After S3, the audio driver restores pin:cvt connection selections by > snd_hda_codec_resume_cache(). However this can happen before the gfx side is > ready and such connect selection is overlooked by HW. After gfx is ready, the > pins make the default selection again. And this will cause multiple pins share > a same convertor and mute control will affect each other. Thus a resumed audio > playback become silent after S3. > > This patch verifies pin:cvt connection on preparing a stream, to assure the pin > selects the right convetor and an assigned convertor is not shared by other > unused pins. Apply this fix-up on Haswell, Broadwell and Valleyview (Baytrail). > > We need this temporary fix before a reliable software communication channel is > established between audio and gfx, to sync audio/gfx operations. > > Signed-off-by: Mengdong Lin <mengdong.lin@intel.com> Thanks, applied. Takashi > > diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c > index 3ab7063..585c271 100644 > --- a/sound/pci/hda/patch_hdmi.c > +++ b/sound/pci/hda/patch_hdmi.c > @@ -68,6 +68,7 @@ struct hdmi_spec_per_pin { > hda_nid_t pin_nid; > int num_mux_nids; > hda_nid_t mux_nids[HDA_MAX_CONNECTIONS]; > + int mux_idx; > hda_nid_t cvt_nid; > > struct hda_codec *codec; > @@ -1342,6 +1343,8 @@ static int hdmi_choose_cvt(struct hda_codec *codec, > if (cvt_idx == spec->num_cvts) > return -ENODEV; > > + per_pin->mux_idx = mux_idx; > + > if (cvt_id) > *cvt_id = cvt_idx; > if (mux_id) > @@ -1350,6 +1353,22 @@ static int hdmi_choose_cvt(struct hda_codec *codec, > return 0; > } > > +/* Assure the pin select the right convetor */ > +static void intel_verify_pin_cvt_connect(struct hda_codec *codec, > + struct hdmi_spec_per_pin *per_pin) > +{ > + hda_nid_t pin_nid = per_pin->pin_nid; > + int mux_idx, curr; > + > + mux_idx = per_pin->mux_idx; > + curr = snd_hda_codec_read(codec, pin_nid, 0, > + AC_VERB_GET_CONNECT_SEL, 0); > + if (curr != mux_idx) > + snd_hda_codec_write_cache(codec, pin_nid, 0, > + AC_VERB_SET_CONNECT_SEL, > + mux_idx); > +} > + > /* Intel HDMI workaround to fix audio routing issue: > * For some Intel display codecs, pins share the same connection list. > * So a conveter can be selected by multiple pins and playback on any of these > @@ -1751,6 +1770,19 @@ static int generic_hdmi_playback_pcm_prepare(struct hda_pcm_stream *hinfo, > bool non_pcm; > int pinctl; > > + if (is_haswell_plus(codec) || is_valleyview(codec)) { > + /* Verify pin:cvt selections to avoid silent audio after S3. > + * After S3, the audio driver restores pin:cvt selections > + * but this can happen before gfx is ready and such selection > + * is overlooked by HW. Thus multiple pins can share a same > + * default convertor and mute control will affect each other, > + * which can cause a resumed audio playback become silent > + * after S3. > + */ > + intel_verify_pin_cvt_connect(codec, per_pin); > + intel_not_share_assigned_cvt(codec, pin_nid, per_pin->mux_idx); > + } > + > non_pcm = check_non_pcm_per_cvt(codec, cvt_nid); > mutex_lock(&per_pin->lock); > per_pin->channels = substream->runtime->channels; > -- > 1.8.1.2 >
On 03/20/2014 07:37 AM, Takashi Iwai wrote: > At Thu, 20 Mar 2014 13:01:06 +0800, > mengdong.lin@intel.com wrote: >> >> From: Mengdong Lin <mengdong.lin@intel.com> >> >> This is a temporary fix for some Intel HDMI codecs to avoid no sound output for >> a resuming playback after S3. >> >> After S3, the audio driver restores pin:cvt connection selections by >> snd_hda_codec_resume_cache(). However this can happen before the gfx side is >> ready and such connect selection is overlooked by HW. After gfx is ready, the >> pins make the default selection again. And this will cause multiple pins share >> a same convertor and mute control will affect each other. Thus a resumed audio >> playback become silent after S3. >> >> This patch verifies pin:cvt connection on preparing a stream, to assure the pin >> selects the right convetor and an assigned convertor is not shared by other >> unused pins. Apply this fix-up on Haswell, Broadwell and Valleyview (Baytrail). >> >> We need this temporary fix before a reliable software communication channel is >> established between audio and gfx, to sync audio/gfx operations. >> >> Signed-off-by: Mengdong Lin <mengdong.lin@intel.com> > > Thanks, applied. Hi, Any reason this commit isn't sent to stable? Looks like a candidate. > > > Takashi > >> >> diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c >> index 3ab7063..585c271 100644 >> --- a/sound/pci/hda/patch_hdmi.c >> +++ b/sound/pci/hda/patch_hdmi.c >> @@ -68,6 +68,7 @@ struct hdmi_spec_per_pin { >> hda_nid_t pin_nid; >> int num_mux_nids; >> hda_nid_t mux_nids[HDA_MAX_CONNECTIONS]; >> + int mux_idx; >> hda_nid_t cvt_nid; >> >> struct hda_codec *codec; >> @@ -1342,6 +1343,8 @@ static int hdmi_choose_cvt(struct hda_codec *codec, >> if (cvt_idx == spec->num_cvts) >> return -ENODEV; >> >> + per_pin->mux_idx = mux_idx; >> + >> if (cvt_id) >> *cvt_id = cvt_idx; >> if (mux_id) >> @@ -1350,6 +1353,22 @@ static int hdmi_choose_cvt(struct hda_codec *codec, >> return 0; >> } >> >> +/* Assure the pin select the right convetor */ >> +static void intel_verify_pin_cvt_connect(struct hda_codec *codec, >> + struct hdmi_spec_per_pin *per_pin) >> +{ >> + hda_nid_t pin_nid = per_pin->pin_nid; >> + int mux_idx, curr; >> + >> + mux_idx = per_pin->mux_idx; >> + curr = snd_hda_codec_read(codec, pin_nid, 0, >> + AC_VERB_GET_CONNECT_SEL, 0); >> + if (curr != mux_idx) >> + snd_hda_codec_write_cache(codec, pin_nid, 0, >> + AC_VERB_SET_CONNECT_SEL, >> + mux_idx); >> +} >> + >> /* Intel HDMI workaround to fix audio routing issue: >> * For some Intel display codecs, pins share the same connection list. >> * So a conveter can be selected by multiple pins and playback on any of these >> @@ -1751,6 +1770,19 @@ static int generic_hdmi_playback_pcm_prepare(struct hda_pcm_stream *hinfo, >> bool non_pcm; >> int pinctl; >> >> + if (is_haswell_plus(codec) || is_valleyview(codec)) { >> + /* Verify pin:cvt selections to avoid silent audio after S3. >> + * After S3, the audio driver restores pin:cvt selections >> + * but this can happen before gfx is ready and such selection >> + * is overlooked by HW. Thus multiple pins can share a same >> + * default convertor and mute control will affect each other, >> + * which can cause a resumed audio playback become silent >> + * after S3. >> + */ >> + intel_verify_pin_cvt_connect(codec, per_pin); >> + intel_not_share_assigned_cvt(codec, pin_nid, per_pin->mux_idx); >> + } >> + >> non_pcm = check_non_pcm_per_cvt(codec, cvt_nid); >> mutex_lock(&per_pin->lock); >> per_pin->channels = substream->runtime->channels; >> -- >> 1.8.1.2 >> > _______________________________________________ > Alsa-devel mailing list > Alsa-devel@alsa-project.org > http://mailman.alsa-project.org/mailman/listinfo/alsa-devel >
At Fri, 28 Mar 2014 09:24:53 +0100, David Henningsson wrote: > > On 03/20/2014 07:37 AM, Takashi Iwai wrote: > > At Thu, 20 Mar 2014 13:01:06 +0800, > > mengdong.lin@intel.com wrote: > >> > >> From: Mengdong Lin <mengdong.lin@intel.com> > >> > >> This is a temporary fix for some Intel HDMI codecs to avoid no sound output for > >> a resuming playback after S3. > >> > >> After S3, the audio driver restores pin:cvt connection selections by > >> snd_hda_codec_resume_cache(). However this can happen before the gfx side is > >> ready and such connect selection is overlooked by HW. After gfx is ready, the > >> pins make the default selection again. And this will cause multiple pins share > >> a same convertor and mute control will affect each other. Thus a resumed audio > >> playback become silent after S3. > >> > >> This patch verifies pin:cvt connection on preparing a stream, to assure the pin > >> selects the right convetor and an assigned convertor is not shared by other > >> unused pins. Apply this fix-up on Haswell, Broadwell and Valleyview (Baytrail). > >> > >> We need this temporary fix before a reliable software communication channel is > >> established between audio and gfx, to sync audio/gfx operations. > >> > >> Signed-off-by: Mengdong Lin <mengdong.lin@intel.com> > > > > Thanks, applied. > > Hi, > > Any reason this commit isn't sent to stable? Looks like a candidate. No particular reason from my side. Feel free to ask for stable once when the patch hits to Linus tree. thanks, Takashi
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 3ab7063..585c271 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -68,6 +68,7 @@ struct hdmi_spec_per_pin { hda_nid_t pin_nid; int num_mux_nids; hda_nid_t mux_nids[HDA_MAX_CONNECTIONS]; + int mux_idx; hda_nid_t cvt_nid; struct hda_codec *codec; @@ -1342,6 +1343,8 @@ static int hdmi_choose_cvt(struct hda_codec *codec, if (cvt_idx == spec->num_cvts) return -ENODEV; + per_pin->mux_idx = mux_idx; + if (cvt_id) *cvt_id = cvt_idx; if (mux_id) @@ -1350,6 +1353,22 @@ static int hdmi_choose_cvt(struct hda_codec *codec, return 0; } +/* Assure the pin select the right convetor */ +static void intel_verify_pin_cvt_connect(struct hda_codec *codec, + struct hdmi_spec_per_pin *per_pin) +{ + hda_nid_t pin_nid = per_pin->pin_nid; + int mux_idx, curr; + + mux_idx = per_pin->mux_idx; + curr = snd_hda_codec_read(codec, pin_nid, 0, + AC_VERB_GET_CONNECT_SEL, 0); + if (curr != mux_idx) + snd_hda_codec_write_cache(codec, pin_nid, 0, + AC_VERB_SET_CONNECT_SEL, + mux_idx); +} + /* Intel HDMI workaround to fix audio routing issue: * For some Intel display codecs, pins share the same connection list. * So a conveter can be selected by multiple pins and playback on any of these @@ -1751,6 +1770,19 @@ static int generic_hdmi_playback_pcm_prepare(struct hda_pcm_stream *hinfo, bool non_pcm; int pinctl; + if (is_haswell_plus(codec) || is_valleyview(codec)) { + /* Verify pin:cvt selections to avoid silent audio after S3. + * After S3, the audio driver restores pin:cvt selections + * but this can happen before gfx is ready and such selection + * is overlooked by HW. Thus multiple pins can share a same + * default convertor and mute control will affect each other, + * which can cause a resumed audio playback become silent + * after S3. + */ + intel_verify_pin_cvt_connect(codec, per_pin); + intel_not_share_assigned_cvt(codec, pin_nid, per_pin->mux_idx); + } + non_pcm = check_non_pcm_per_cvt(codec, cvt_nid); mutex_lock(&per_pin->lock); per_pin->channels = substream->runtime->channels;