Message ID | 1438843977-4269-3-git-send-email-libin.yang@intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On Thu, 06 Aug 2015 08:52:56 +0200, libin.yang@intel.com wrote: > > From: Libin Yang <libin.yang@intel.com> > > On some Intel platforms, display audio need set N/CTS > manually at some TMDS frequencies. > > Signed-off-by: Libin Yang <libin.yang@intel.com> > --- > sound/pci/hda/patch_hdmi.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c > index a97db5f..4bd11ff 100644 > --- a/sound/pci/hda/patch_hdmi.c > +++ b/sound/pci/hda/patch_hdmi.c > @@ -1786,6 +1786,8 @@ static int generic_hdmi_playback_pcm_prepare(struct hda_pcm_stream *hinfo, > int pin_idx = hinfo_to_pin_index(codec, hinfo); > struct hdmi_spec_per_pin *per_pin = get_pin(spec, pin_idx); > hda_nid_t pin_nid = per_pin->pin_nid; > + struct snd_pcm_runtime *runtime = substream->runtime; > + struct i915_audio_component *acomp = codec->bus->core.audio_component; > bool non_pcm; > int pinctl; > > @@ -1802,6 +1804,11 @@ static int generic_hdmi_playback_pcm_prepare(struct hda_pcm_stream *hinfo, > intel_not_share_assigned_cvt(codec, pin_nid, per_pin->mux_idx); > } > > + if (is_haswell_plus(codec)) { > + if (acomp && acomp->ops && acomp->ops->set_ncts) > + acomp->ops->set_ncts(acomp->dev, per_pin->pin_nid - 4, Please describe more how "pin_nid - 4" is supposed to work. Also... > + 0, runtime->rate); ... this implies that no MST support included? Overall, it'd be appreciated if you put more information text in changelog or comment. it series looks like a black magic to me unless clearly explained. thanks, Takashi
Hi Takashi, > -----Original Message----- > From: Takashi Iwai [mailto:tiwai@suse.de] > Sent: Thursday, August 06, 2015 6:03 PM > To: Yang, Libin > Cc: alsa-devel@alsa-project.org; intel-gfx@lists.freedesktop.org; Lin, > Mengdong > Subject: Re: [PATCH 3/4] ALSA: hda - display audio call ncts callback > > On Thu, 06 Aug 2015 08:52:56 +0200, > libin.yang@intel.com wrote: > > > > From: Libin Yang <libin.yang@intel.com> > > > > On some Intel platforms, display audio need set N/CTS > > manually at some TMDS frequencies. > > > > Signed-off-by: Libin Yang <libin.yang@intel.com> > > --- > > sound/pci/hda/patch_hdmi.c | 7 +++++++ > > 1 file changed, 7 insertions(+) > > > > diff --git a/sound/pci/hda/patch_hdmi.c > b/sound/pci/hda/patch_hdmi.c > > index a97db5f..4bd11ff 100644 > > --- a/sound/pci/hda/patch_hdmi.c > > +++ b/sound/pci/hda/patch_hdmi.c > > @@ -1786,6 +1786,8 @@ static int > generic_hdmi_playback_pcm_prepare(struct hda_pcm_stream *hinfo, > > int pin_idx = hinfo_to_pin_index(codec, hinfo); > > struct hdmi_spec_per_pin *per_pin = get_pin(spec, pin_idx); > > hda_nid_t pin_nid = per_pin->pin_nid; > > + struct snd_pcm_runtime *runtime = substream->runtime; > > + struct i915_audio_component *acomp = codec->bus- > >core.audio_component; > > bool non_pcm; > > int pinctl; > > > > @@ -1802,6 +1804,11 @@ static int > generic_hdmi_playback_pcm_prepare(struct hda_pcm_stream *hinfo, > > intel_not_share_assigned_cvt(codec, pin_nid, per_pin- > >mux_idx); > > } > > > > + if (is_haswell_plus(codec)) { > > + if (acomp && acomp->ops && acomp->ops->set_ncts) > > + acomp->ops->set_ncts(acomp->dev, per_pin- > >pin_nid - 4, > > Please describe more how "pin_nid - 4" is supposed to work. Also... OK, I see. > > > + 0, runtime->rate); > > ... this implies that no MST support included? We will support MST later. Currently I just add the interface to support MST, but not implementing it. After we enabled MST, I will submit another patch to support MST. Currently, it seems the display audio driver need do some change to support MST. > > Overall, it'd be appreciated if you put more information text in > changelog or comment. it series looks like a black magic to me unless > clearly explained. OK, I will add the comments about the details. > > > thanks, > > Takashi
> > > -----Original Message----- > > From: Takashi Iwai [mailto:tiwai@suse.de] > > Sent: Thursday, August 06, 2015 6:03 PM > > To: Yang, Libin > > Cc: alsa-devel@alsa-project.org; intel-gfx@lists.freedesktop.org; Lin, > > Mengdong > > Subject: Re: [PATCH 3/4] ALSA: hda - display audio call ncts callback > > > > On Thu, 06 Aug 2015 08:52:56 +0200, > > libin.yang@intel.com wrote: > > > > > > From: Libin Yang <libin.yang@intel.com> > > > > > > On some Intel platforms, display audio need set N/CTS > > > manually at some TMDS frequencies. > > > > > > Signed-off-by: Libin Yang <libin.yang@intel.com> > > > --- > > > sound/pci/hda/patch_hdmi.c | 7 +++++++ > > > 1 file changed, 7 insertions(+) > > > > > > diff --git a/sound/pci/hda/patch_hdmi.c > > b/sound/pci/hda/patch_hdmi.c > > > index a97db5f..4bd11ff 100644 > > > --- a/sound/pci/hda/patch_hdmi.c > > > +++ b/sound/pci/hda/patch_hdmi.c > > > @@ -1786,6 +1786,8 @@ static int > > generic_hdmi_playback_pcm_prepare(struct hda_pcm_stream *hinfo, > > > int pin_idx = hinfo_to_pin_index(codec, hinfo); > > > struct hdmi_spec_per_pin *per_pin = get_pin(spec, pin_idx); > > > hda_nid_t pin_nid = per_pin->pin_nid; > > > + struct snd_pcm_runtime *runtime = substream->runtime; > > > + struct i915_audio_component *acomp = codec->bus- > > >core.audio_component; > > > bool non_pcm; > > > int pinctl; > > > > > > @@ -1802,6 +1804,11 @@ static int > > generic_hdmi_playback_pcm_prepare(struct hda_pcm_stream *hinfo, > > > intel_not_share_assigned_cvt(codec, pin_nid, per_pin- > > >mux_idx); > > > } > > > > > > + if (is_haswell_plus(codec)) { > > > + if (acomp && acomp->ops && acomp->ops->set_ncts) > > > + acomp->ops->set_ncts(acomp->dev, per_pin- > > >pin_nid - 4, > > > > Please describe more how "pin_nid - 4" is supposed to work. Also... > > OK, I see. > > > > > > + 0, runtime->rate); > > > > ... this implies that no MST support included? > > We will support MST later. Currently I just add the > interface to support MST, but not implementing it. Refer to DCN HDA040-A Multi-stream over Single Display Port Can the driver use subdevices for those display port support multi streaming ? The specification allow up to 64 device entries This mean the number of subdevices is equal to the device list length More than one audio output /converters can be connected to the multi stream displayport pin widget but different device entry while only one audio output can be dynamically allocated to other HDMI pin widget 7.3.3.42 Device Select For Digital Display Pin Widget that is multi stream capable, the Device Select control determines which Device Entry is currently selected and accessible by the Pin Widget verbs which are controlling the sink device operations. This control verb is only required if it is a Digital Display Pin Widget and multi stream capable. The index is in relation to the Device List associated with the widget. The index is a zero-based offset into the Device List. Once the Device Entry is selected by the Set index, all subsequent Pin Widget verbs controlling the sink device operations will be directed to the selected Device Entry, until the Device Select verb get updated with a new value. Device Entry: Indicate the index of Device Entry (0 63) which the UR bit of is generated for a multi stream capable Digital Display Pin Widget. Not valid for non Digital Display Pin Widget or Digital Display Pin Widget that is not multi stream capable Device List Length is a 0 based integer value indicating the number of sink device that a multi stream capable Digital Display Pin Widget can support. If Device List Length is value is 0, there is only one sink device connection possible indicating the Pin Widget is not multi stream capable, and there is no Device Select control (see Section 7.3.3.42). If the Device List Length value is 1 – 63, it indicates the Pin Widget is multi stream capable, and 2 – 64 Device Entries are supported in the Pin Widget. >
Hi Raymond, > > > > } > > > > > > + if (is_haswell_plus(codec)) { > > > + if (acomp && acomp->ops && acomp->ops->set_ncts) > > > + acomp->ops->set_ncts(acomp->dev, per_pin- > > >pin_nid - 4, > > > > Please describe more how "pin_nid - 4" is supposed to work. Also... > > OK, I see. > > > > > > + 0, runtime->rate); > > > > ... this implies that no MST support included? > > We will support MST later. Currently I just add the > interface to support MST, but not implementing it. Refer to DCN HDA040-A Multi-stream over Single Display Port Can the driver use subdevices for those display port support multi streaming ? [Libin] What do you mean subdevice here, using a struct device to represent a dev_entry or an int type? The specification allow up to 64 device entries This mean the number of subdevices is equal to the device list length More than one audio output /converters can be connected to the multi stream displayport pin widget but different device entry while only one audio output can be dynamically allocated to other HDMI pin widget [Libin] Yes, Pin widget can have multiple device entry and connecting different converters. The audio output will be based on device entry. 7.3.3.42 Device Select For Digital Display Pin Widget that is multi stream capable, the Device Select control determines which Device Entry is currently selected and accessible by the Pin Widget verbs which are controlling the sink device operations. This control verb is only required if it is a Digital Display Pin Widget and multi stream capable. The index is in relation to the Device List associated with the widget. The index is a zero-based offset into the Device List. Once the Device Entry is selected by the Set index, all subsequent Pin Widget verbs controlling the sink device operations will be directed to the selected Device Entry, until the Device Select verb get updated with a new value. Device Entry: Indicate the index of Device Entry (0 63) which the UR bit of is generated for a multi stream capable Digital Display Pin Widget. Not valid for non Digital Display Pin Widget or Digital Display Pin Widget that is not multi stream capable Device List Length is a 0 based integer value indicating the number of sink device that a multi stream capable Digital Display Pin Widget can support. If Device List Length is value is 0, there is only one sink device connection possible indicating the Pin Widget is not multi stream capable, and there is no Device Select control (see Section 7.3.3.42). If the Device List Length value is 1 – 63, it indicates the Pin Widget is multi stream capable, and 2 – 64 Device Entries are supported in the Pin Widget. > Regards, Libin
2015-8-10 ??11:15? "Yang, Libin" <libin.yang@intel.com>??? > > Hi Raymond, > > > > > > > } > > > > > > > > + if (is_haswell_plus(codec)) { > > > > + if (acomp && acomp->ops && acomp->ops->set_ncts) > > > > + acomp->ops->set_ncts(acomp->dev, per_pin- > > > >pin_nid - 4, > > > > > > Please describe more how "pin_nid - 4" is supposed to work. Also... > > > > OK, I see. > > > > > > > > > + 0, runtime->rate); > > > > > > ... this implies that no MST support included? > > > > We will support MST later. Currently I just add the > > interface to support MST, but not implementing it. > Refer to DCN HDA040-A > Multi-stream over Single Display Port > Can the driver use subdevices for those display port support multi streaming ? > > [Libin] What do you mean subdevice here, > using a struct device to represent a dev_entry or an int type? http://git.kernel.org/cgit/linux/kernel/git/tiwai/hda-emu.git/tree/codecs/stac9227-intel-d946gzis-mobo?id=HEAD When HDA codecs have three Audio Input widgets, the driver create three subdevices for those desktop which have three or more input sources in the past ARECORD **** List of CAPTURE Hardware Devices **** card 0: Intel [HDA Intel], device 0: STAC92xx Analog [STAC92xx Analog] Subdevices: 3/3 Subdevice #0: subdevice #0 Subdevice #1: subdevice #1 Subdevice #2: subdevice #2 With the auto generic parser , the driver create one subdevice for Analog two subdevices for Alt Analog **** List of CAPTURE Hardware Devices **** card 0: Intel [HDA Intel], device 0: STAC92xx Analog [STAC92xx Analog] Subdevices: 1/1 Subdevice #0: subdevice #0 card 0: Intel [HDA Intel], device 2: STAC92xx Alt Analog [STAC92xx Alt Analog] Subdevices: 2/2 Subdevice #0: subdevice #0 Subdevice #1: subdevice #1 > > The specification allow up to 64 device entries > This mean the number of subdevices is equal to the device list length > More than one audio output /converters can be connected to the multi stream displayport pin widget but different device entry while only one audio output can be dynamically allocated to other HDMI pin widget > > [Libin] Yes, Pin widget can have multiple device entry and connecting different converters. The audio output will be based on device entry.
Hi Raymond, From: Raymond Yau [mailto:superquad.vortex2@gmail.com] Sent: Monday, August 10, 2015 12:23 PM To: Yang, Libin Cc: alsa-devel@alsa-project.org; Takashi Iwai; Lin, Mengdong; intel-gfx@lists.freedesktop.org Subject: RE: [alsa-devel] [PATCH 3/4] ALSA: hda - display audio call ncts callback 2015-8-10 ??11:15? "Yang, Libin" <libin.yang@intel.com<mailto:libin.yang@intel.com>>??? > > Hi Raymond, > > > > > > > } > > > > > > > > + if (is_haswell_plus(codec)) { > > > > + if (acomp && acomp->ops && acomp->ops->set_ncts) > > > > + acomp->ops->set_ncts(acomp->dev, per_pin- > > > >pin_nid - 4, > > > > > > Please describe more how "pin_nid - 4" is supposed to work. Also... > > > > OK, I see. > > > > > > > > > + 0, runtime->rate); > > > > > > ... this implies that no MST support included? > > > > We will support MST later. Currently I just add the > > interface to support MST, but not implementing it. > Refer to DCN HDA040-A > Multi-stream over Single Display Port > Can the driver use subdevices for those display port support multi streaming ? > > [Libin] What do you mean subdevice here, > using a struct device to represent a dev_entry or an int type? http://git.kernel.org/cgit/linux/kernel/git/tiwai/hda-emu.git/tree/codecs/stac9227-intel-d946gzis-mobo?id=HEAD When HDA codecs have three Audio Input widgets, the driver create three subdevices for those desktop which have three or more input sources in the past This is what we are thinking currently. Different companies have different implementation. On currently Intel platforms, it may show several pin widgets and each pin widget has several device entry. But it actually only support 3 streams. Mengdong is thinking to use dynamic PCM to implement it, and so we don’t need each subdevice for each device entry. I’m not sure we will use what solution. It seems it is a good open question to discuss. Regards, Libin ARECORD **** List of CAPTURE Hardware Devices **** card 0: Intel [HDA Intel], device 0: STAC92xx Analog [STAC92xx Analog] Subdevices: 3/3 Subdevice #0: subdevice #0 Subdevice #1: subdevice #1 Subdevice #2: subdevice #2 With the auto generic parser , the driver create one subdevice for Analog two subdevices for Alt Analog **** List of CAPTURE Hardware Devices **** card 0: Intel [HDA Intel], device 0: STAC92xx Analog [STAC92xx Analog] Subdevices: 1/1 Subdevice #0: subdevice #0 card 0: Intel [HDA Intel], device 2: STAC92xx Alt Analog [STAC92xx Alt Analog] Subdevices: 2/2 Subdevice #0: subdevice #0 Subdevice #1: subdevice #1 > > The specification allow up to 64 device entries > This mean the number of subdevices is equal to the device list length > More than one audio output /converters can be connected to the multi stream displayport pin widget but different device entry while only one audio output can be dynamically allocated to other HDMI pin widget > > [Libin] Yes, Pin widget can have multiple device entry and connecting different converters. The audio output will be based on device entry.
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index a97db5f..4bd11ff 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -1786,6 +1786,8 @@ static int generic_hdmi_playback_pcm_prepare(struct hda_pcm_stream *hinfo, int pin_idx = hinfo_to_pin_index(codec, hinfo); struct hdmi_spec_per_pin *per_pin = get_pin(spec, pin_idx); hda_nid_t pin_nid = per_pin->pin_nid; + struct snd_pcm_runtime *runtime = substream->runtime; + struct i915_audio_component *acomp = codec->bus->core.audio_component; bool non_pcm; int pinctl; @@ -1802,6 +1804,11 @@ static int generic_hdmi_playback_pcm_prepare(struct hda_pcm_stream *hinfo, intel_not_share_assigned_cvt(codec, pin_nid, per_pin->mux_idx); } + if (is_haswell_plus(codec)) { + if (acomp && acomp->ops && acomp->ops->set_ncts) + acomp->ops->set_ncts(acomp->dev, per_pin->pin_nid - 4, + 0, runtime->rate); + } non_pcm = check_non_pcm_per_cvt(codec, cvt_nid); mutex_lock(&per_pin->lock); per_pin->channels = substream->runtime->channels;