Message ID | 20170919222505.28654-1-pierre-louis.bossart@linux.intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On Wed, 20 Sep 2017 00:25:05 +0200, Pierre-Louis Bossart wrote: > > From: Sriram Periyasamy <sriramx.periyasamy@intel.com> > > On recent Intel platforms (Haswell, Broadwell, Skylake, ApolloLake, > KabyLake, ...), the IEC Coding Type (ICT) bitfield in the Digital > Converter Control #3 needs to be set explicitly for HDMI/DisplayPort > High Bit Rate (HBR) audio playback to work. This was not required in > earlier platforms when HBR was first introduced. The ICT bits are > defined in Section 7.3.3.9 of the HDaudio 1.0a specification. > > Since the ICT bitfield was not specified for HDAudio 1.0 devices > (before 2009), we only program it on machines more recent than > Haswell. > > We tested that this fix is not needed on Baytrail-I (MinnowBoard > Turbot) and believe by extension it also does not apply to Braswell. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98797 > > Signed-off-by: Sriram Periyasamy <sriramx.periyasamy@intel.com> > Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> > Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com> > --- > v2: move digi3 verb in Set section, only set ICT for haswell and newer Applied now with a slight fix (moving *_SET_* definition to the right place). thanks, Takashi
On 9/20/17 5:00 AM, Takashi Iwai wrote: > On Wed, 20 Sep 2017 00:25:05 +0200, > Pierre-Louis Bossart wrote: >> >> From: Sriram Periyasamy <sriramx.periyasamy@intel.com> >> >> On recent Intel platforms (Haswell, Broadwell, Skylake, ApolloLake, >> KabyLake, ...), the IEC Coding Type (ICT) bitfield in the Digital >> Converter Control #3 needs to be set explicitly for HDMI/DisplayPort >> High Bit Rate (HBR) audio playback to work. This was not required in >> earlier platforms when HBR was first introduced. The ICT bits are >> defined in Section 7.3.3.9 of the HDaudio 1.0a specification. >> >> Since the ICT bitfield was not specified for HDAudio 1.0 devices >> (before 2009), we only program it on machines more recent than >> Haswell. >> >> We tested that this fix is not needed on Baytrail-I (MinnowBoard >> Turbot) and believe by extension it also does not apply to Braswell. >> >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98797 >> >> Signed-off-by: Sriram Periyasamy <sriramx.periyasamy@intel.com> >> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> >> Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com> >> --- >> v2: move digi3 verb in Set section, only set ICT for haswell and newer > > Applied now with a slight fix (moving *_SET_* definition to the right > place). Thanks Takashi. Not sure what happened since I have a clear memory of doing that change and making a note of it...
On Wed, Sep 20, 2017 at 08:12:36AM -0500, Pierre-Louis Bossart wrote: > On 9/20/17 5:00 AM, Takashi Iwai wrote: > >On Wed, 20 Sep 2017 00:25:05 +0200, > >Pierre-Louis Bossart wrote: > >> > >>From: Sriram Periyasamy <sriramx.periyasamy@intel.com> > >> > >>On recent Intel platforms (Haswell, Broadwell, Skylake, ApolloLake, > >>KabyLake, ...), the IEC Coding Type (ICT) bitfield in the Digital > >>Converter Control #3 needs to be set explicitly for HDMI/DisplayPort > >>High Bit Rate (HBR) audio playback to work. This was not required in > >>earlier platforms when HBR was first introduced. The ICT bits are > >>defined in Section 7.3.3.9 of the HDaudio 1.0a specification. > >> > >>Since the ICT bitfield was not specified for HDAudio 1.0 devices > >>(before 2009), we only program it on machines more recent than > >>Haswell. > >> > >>We tested that this fix is not needed on Baytrail-I (MinnowBoard > >>Turbot) and believe by extension it also does not apply to Braswell. > >> > >>Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98797 > >> > >>Signed-off-by: Sriram Periyasamy <sriramx.periyasamy@intel.com> > >>Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> > >>Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com> > >>--- > >>v2: move digi3 verb in Set section, only set ICT for haswell and newer > > > >Applied now with a slight fix (moving *_SET_* definition to the right > >place). > > Thanks Takashi. Not sure what happened since I have a clear memory > of doing that change and making a note of it... Hi Takashi, I think this patch should have also gone to stable. We didn't cc. Please suggest whether we should submit it separately for stable. Regards, Subhransu > _______________________________________________ > Alsa-devel mailing list > Alsa-devel@alsa-project.org > http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
On Mon, 25 Sep 2017 16:56:53 +0200, Subhransu S. Prusty wrote: > > On Wed, Sep 20, 2017 at 08:12:36AM -0500, Pierre-Louis Bossart wrote: > > On 9/20/17 5:00 AM, Takashi Iwai wrote: > > >On Wed, 20 Sep 2017 00:25:05 +0200, > > >Pierre-Louis Bossart wrote: > > >> > > >>From: Sriram Periyasamy <sriramx.periyasamy@intel.com> > > >> > > >>On recent Intel platforms (Haswell, Broadwell, Skylake, ApolloLake, > > >>KabyLake, ...), the IEC Coding Type (ICT) bitfield in the Digital > > >>Converter Control #3 needs to be set explicitly for HDMI/DisplayPort > > >>High Bit Rate (HBR) audio playback to work. This was not required in > > >>earlier platforms when HBR was first introduced. The ICT bits are > > >>defined in Section 7.3.3.9 of the HDaudio 1.0a specification. > > >> > > >>Since the ICT bitfield was not specified for HDAudio 1.0 devices > > >>(before 2009), we only program it on machines more recent than > > >>Haswell. > > >> > > >>We tested that this fix is not needed on Baytrail-I (MinnowBoard > > >>Turbot) and believe by extension it also does not apply to Braswell. > > >> > > >>Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98797 > > >> > > >>Signed-off-by: Sriram Periyasamy <sriramx.periyasamy@intel.com> > > >>Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> > > >>Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com> > > >>--- > > >>v2: move digi3 verb in Set section, only set ICT for haswell and newer > > > > > >Applied now with a slight fix (moving *_SET_* definition to the right > > >place). > > > > Thanks Takashi. Not sure what happened since I have a clear memory > > of doing that change and making a note of it... > > Hi Takashi, > > I think this patch should have also gone to stable. We didn't cc. Please > suggest whether we should submit it separately for stable. In such a case, please send a mail to Greg KH once after the patch gets merged to Linus tree and tell him the upstream git commit id to cherry-pick for stable kernels. thanks, Takashi
diff --git a/include/sound/hda_verbs.h b/include/sound/hda_verbs.h index d0509db6d0ec..d2004d7feb2b 100644 --- a/include/sound/hda_verbs.h +++ b/include/sound/hda_verbs.h @@ -54,6 +54,7 @@ enum { #define AC_VERB_GET_EAPD_BTLENABLE 0x0f0c #define AC_VERB_GET_DIGI_CONVERT_1 0x0f0d #define AC_VERB_GET_DIGI_CONVERT_2 0x0f0e /* unused */ +#define AC_VERB_SET_DIGI_CONVERT_3 0x73e #define AC_VERB_GET_VOLUME_KNOB_CONTROL 0x0f0f /* f10-f1a: GPIO */ #define AC_VERB_GET_GPIO_DATA 0x0f15 diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 53f9311370de..8566de298ea3 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -906,6 +906,7 @@ static int hdmi_setup_stream(struct hda_codec *codec, hda_nid_t cvt_nid, hda_nid_t pin_nid, u32 stream_tag, int format) { struct hdmi_spec *spec = codec->spec; + unsigned int param; int err; err = spec->ops.pin_hbr_setup(codec, pin_nid, is_hbr_format(format)); @@ -915,6 +916,26 @@ static int hdmi_setup_stream(struct hda_codec *codec, hda_nid_t cvt_nid, return err; } + if (is_haswell_plus(codec)) { + + /* + * on recent platforms IEC Coding Type is required for HBR + * support, read current Digital Converter settings and set + * ICT bitfield if needed. + */ + param = snd_hda_codec_read(codec, cvt_nid, 0, + AC_VERB_GET_DIGI_CONVERT_1, 0); + + param = (param >> 16) & ~(AC_DIG3_ICT); + + /* on recent platforms ICT mode is required for HBR support */ + if (is_hbr_format(format)) + param |= 0x1; + + snd_hda_codec_write(codec, cvt_nid, 0, + AC_VERB_SET_DIGI_CONVERT_3, param); + } + snd_hda_codec_setup_stream(codec, cvt_nid, stream_tag, 0, format); return 0; }