[Intel-gfx] [PATCH V2 4/4] ALSA HDA: Force HDMI pins enabled

[Wang, Xingchao]

Hi Takashi,

> -----Original Message-----
> From: Takashi Iwai [mailto:tiwai at suse.de]
> Sent: Tuesday, July 31, 2012 7:18 PM
> To: Wang, Xingchao
> Cc: intel-gfx at lists.freedesktop.org; Zanoni, Paulo R
> Subject: Re: [Intel-gfx] [PATCH V2 4/4] ALSA HDA: Force HDMI pins enabled
> 
> At Tue, 31 Jul 2012 10:47:05 +0800,
> Wang Xingchao wrote:
> >
> > There's one issue for HDMI pins, even the related pin will be enabled
> > when the stream is active but the GPU registers show the PIN is not in
> > active state, so we force all pins in active state and donot close it
> > when the stream is closed.
> >
> > Signed-off-by: Wang Xingchao <xingchao.wang at intel.com>
> 
> Oh no, this is a bad workaround.
> 
> Please be more exact at which timing the pin must be set.
> Setting all the time is just a wrong approach.

Sorry for that, I am still trying to find out the root cause and the patch was intended for test only.
Basically the HDA verb made the change but GPU side does not reflect the correct state, did you ever meet same phenomenon?
There's one similar bug:
HBR sound passthrough (DTS-HS-MA, Dolby-Turuhd) not working on Intel Sandy Bridge
https://bugs.freedesktop.org/show_bug.cgi?id=49055

> 
> Also, at the next time, please Cc to alsa-devel and other developers.
> I don't want that such a patch is merged secretly through drm tree :)

Hmm, sure. i will cc to alsa-devel when send out the final version for review.

Thanks your clarification. I will take more care when send out alsa patches. :)

--xingchao

> 
> > ---
> >  sound/pci/hda/patch_hdmi.c |   15 ++++++++-------
> >  1 file changed, 8 insertions(+), 7 deletions(-)
> >
> > diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
> > index ad319d4..64cc9e0 100644
> > --- a/sound/pci/hda/patch_hdmi.c
> > +++ b/sound/pci/hda/patch_hdmi.c
> > @@ -421,13 +421,17 @@ static void hdmi_write_dip_byte(struct hda_codec
> > *codec, hda_nid_t pin_nid,
> >
> >  static void hdmi_init_pin(struct hda_codec *codec, hda_nid_t pin_nid)
> > {
> > +	/* Force all output Pins enabled */
> > +	snd_printk(KERN_DEBUG "HDMI: enable all output\n");
> > +	snd_hda_codec_write(codec, pin_nid, 0,
> > +			    AC_VERB_SET_PIN_WIDGET_CONTROL, 0x40);
> 
> Don't put debug prints unconditionally.
> 
> > +
> >  	/* Unmute */
> > -	if (get_wcaps(codec, pin_nid) & AC_WCAP_OUT_AMP)
> > +	if (get_wcaps(codec, pin_nid) & AC_WCAP_OUT_AMP) {
> > +		snd_printk(KERN_DEBUG "HDMI: unmute pin %d\n", pin_nid);
> 
> Ditto.
> 
> 
> thanks,
> 
> Takashi
> 
> 
> >  		snd_hda_codec_write(codec, pin_nid, 0,
> >  				AC_VERB_SET_AMP_GAIN_MUTE, AMP_OUT_UNMUTE);
> > -	/* Disable pin out until stream is active*/
> > -	snd_hda_codec_write(codec, pin_nid, 0,
> > -			    AC_VERB_SET_PIN_WIDGET_CONTROL, 0);
> > +	}
> >  }
> >
> >  static int hdmi_get_channel_count(struct hda_codec *codec, hda_nid_t
> > cvt_nid) @@ -1190,9 +1194,6 @@ static int
> > generic_hdmi_playback_pcm_cleanup(struct hda_pcm_stream *hinfo,
> >
> >  		pinctl = snd_hda_codec_read(codec, per_pin->pin_nid, 0,
> >  					    AC_VERB_GET_PIN_WIDGET_CONTROL, 0);
> > -		snd_hda_codec_write(codec, per_pin->pin_nid, 0,
> > -				    AC_VERB_SET_PIN_WIDGET_CONTROL,
> > -				    pinctl & ~PIN_OUT);
> >  		snd_hda_spdif_ctls_unassign(codec, pin_idx);
> >  	}
> >
> > --
> > 1.7.9.5
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >