[PATCH 1/4] drm/i915: Add audio set_ncts callback

[PATCH 1/4] drm/i915: Add audio set_ncts callback

[libin.yang]

From: Libin Yang <libin.yang@intel.com>

Add the set_ncts callback.

With the callback, audio driver can trigger
i915 driver to set the proper N/CTS
based on different sample rates.

Signed-off-by: Libin Yang <libin.yang@intel.com>
---
 include/drm/i915_component.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/drm/i915_component.h b/include/drm/i915_component.h
index c9a8b64..7305881 100644
--- a/include/drm/i915_component.h
+++ b/include/drm/i915_component.h
@@ -33,6 +33,8 @@ struct i915_audio_component {
 		void (*put_power)(struct device *);
 		void (*codec_wake_override)(struct device *, bool enable);
 		int (*get_cdclk_freq)(struct device *);
+		int (*set_ncts)(struct device *, int port, int dev_entry,
+					int rate);
 	} *ops;
 };
 
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/4] drm/i915: implement set_ncts callback

[libin.yang]

From: Libin Yang <libin.yang@intel.com>

Display audio may not work at some frequencies
with the HW provided N/CTS.

This patch sets the proper N value for the
given audio sample rate at the impacted frequencies.
At other frequencies, it will use the N/CTS value
which HW provides.

Signed-off-by: Libin Yang <libin.yang@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h    |  2 +
 drivers/gpu/drm/i915/intel_audio.c | 93 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 95 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 3a77678..0b1cd72 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7010,6 +7010,8 @@ enum skl_disp_power_wells {
 					_HSW_AUD_MISC_CTRL_A, \
 					_HSW_AUD_MISC_CTRL_B)
 
+#define HSW_AUD_PIPE_CONN_SEL_CTRL  0x650ac
+
 #define _HSW_AUD_DIP_ELD_CTRL_ST_A	0x650b4
 #define _HSW_AUD_DIP_ELD_CTRL_ST_B	0x651b4
 #define HSW_AUD_DIP_ELD_CTRL(pipe) _PIPE(pipe, \
diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
index dc32cf4..f1148cd 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -68,6 +68,28 @@ static const struct {
 	{ 148500, AUD_CONFIG_PIXEL_CLOCK_HDMI_148500 },
 };
 
+static const struct {
+	int sample_rate;
+	int clock;
+	int n;
+	int cts;
+} aud_ncts[] = {
+	{ 44100, DIV_ROUND_UP(297000 * 1000, 1001), 4459, 234375 },
+	{ 44100, 297000, 4704, 247500 },
+	{ 48000, DIV_ROUND_UP(297000 * 1000, 1001), 5824, 281250 },
+	{ 48000, 297000, 5120, 247500 },
+	{ 32000, DIV_ROUND_UP(297000 * 1000, 1001), 5824, 421875 },
+	{ 32000, 297000, 3072, 222750 },
+	{ 88200, DIV_ROUND_UP(297000 * 1000, 1001), 8918, 234375 },
+	{ 88200, 297000, 9408, 247500 },
+	{ 96000, DIV_ROUND_UP(297000 * 1000, 1001), 11648, 281250 },
+	{ 96000, 297000, 10240, 247500 },
+	{ 176400, DIV_ROUND_UP(297000 * 1000, 1001), 17836, 234375 },
+	{ 176400, 297000, 18816, 247500 },
+	{ 44100, DIV_ROUND_UP(297000 * 1000, 1001), 23296, 281250 },
+	{ 44100, 297000, 20480, 247500 },
+};
+
 /* get AUD_CONFIG_PIXEL_CLOCK_HDMI_* value for mode */
 static u32 audio_config_hdmi_pixel_clock(struct drm_display_mode *mode)
 {
@@ -514,12 +536,83 @@ static int i915_audio_component_get_cdclk_freq(struct device *dev)
 	return ret;
 }
 
+static int i915_audio_component_set_ncts(struct device *dev, int port,
+			int dev_entry, int rate)
+{
+	struct drm_i915_private *dev_priv = dev_to_i915(dev);
+	struct drm_device *drm_dev = dev_priv->dev;
+	struct intel_encoder *intel_encoder;
+	struct intel_digital_port *intel_dig_port;
+	struct intel_crtc *crtc;
+	struct drm_display_mode *mode;
+	enum pipe pipe = -1;
+	u32 tmp;
+	int i;
+	int n_low, n_up, n = 0;
+
+	/* 1. get the pipe */
+	for_each_intel_encoder(drm_dev, intel_encoder) {
+		intel_dig_port = enc_to_dig_port(&intel_encoder->base);
+		if (port == intel_dig_port->port) {
+			crtc = to_intel_crtc(intel_encoder->base.crtc);
+			if (!crtc) {
+				DRM_DEBUG_KMS("%s: crtc is NULL\n", __func__);
+				continue;
+			}
+			pipe = crtc->pipe;
+			break;
+		}
+	}
+
+	if (pipe == -1) {
+		DRM_DEBUG_KMS("no pipe for the port %c\n", port_name(port));
+		return -ENODEV;
+	}
+	DRM_DEBUG_KMS("pipe %c connects port %c\n",
+				  pipe_name(pipe), port_name(port));
+	mode = &crtc->config->base.adjusted_mode;
+
+	/* 2. Need set the N/CTS manually at some frequencies */
+	if ((mode->clock != 297000) &&
+		(mode->clock != DIV_ROUND_UP(297000 * 1000, 1001))) {
+		tmp = I915_READ(HSW_AUD_CFG(pipe));
+		tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
+		I915_WRITE(HSW_AUD_CFG(pipe), tmp);
+		return 0;
+	}
+
+	/* 3. calculate the N/CTS */
+	for (i = 0; i < ARRAY_SIZE(aud_ncts); i++) {
+		if ((rate == aud_ncts[i].sample_rate) &&
+			(mode->clock == aud_ncts[i].clock)) {
+			n = aud_ncts[i].n;
+			break;
+		}
+	}
+	if (n == 0)
+		return 0;
+	n_low = n & 0xfff;
+	n_up = (n >> 12) & 0xff;
+
+	/* 4. set the N/CTS */
+	tmp = I915_READ(HSW_AUD_CFG(pipe));
+	tmp |= AUD_CONFIG_N_PROG_ENABLE;
+	tmp &= ~AUD_CONFIG_UPPER_N_MASK;
+	tmp |= (n_up << AUD_CONFIG_UPPER_N_SHIFT);
+	tmp &= ~AUD_CONFIG_LOWER_N_MASK;
+	tmp |= (n_low << AUD_CONFIG_LOWER_N_SHIFT);
+	I915_WRITE(HSW_AUD_CFG(pipe), tmp);
+
+	return 0;
+}
+
 static const struct i915_audio_component_ops i915_audio_component_ops = {
 	.owner		= THIS_MODULE,
 	.get_power	= i915_audio_component_get_power,
 	.put_power	= i915_audio_component_put_power,
 	.codec_wake_override = i915_audio_component_codec_wake_override,
 	.get_cdclk_freq	= i915_audio_component_get_cdclk_freq,
+	.set_ncts = i915_audio_component_set_ncts,
 };
 
 static int i915_audio_component_bind(struct device *i915_dev,
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 3/4] ALSA: hda - display audio call ncts callback

[libin.yang]

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,
+				0, runtime->rate);
+	}
 	non_pcm = check_non_pcm_per_cvt(codec, cvt_nid);
 	mutex_lock(&per_pin->lock);
 	per_pin->channels = substream->runtime->channels;
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 4/4] drm/i915: set proper N/CTS in modeset

[libin.yang]

From: Libin Yang <libin.yang@intel.com>

When modeset occurs and the TMDS frequency is set to some
speical value, the N/CTS need to be set manually if audio
is playing.

Signed-off-by: Libin Yang <libin.yang@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h    |  6 ++++++
 drivers/gpu/drm/i915/intel_audio.c | 42 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 48 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 0b1cd72..94f7f8b 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7026,6 +7026,12 @@ enum skl_disp_power_wells {
 					_HSW_AUD_DIG_CNVT_2)
 #define DIP_PORT_SEL_MASK		0x3
 
+#define _HSW_AUD_STR_DESC_1		0x65084
+#define _HSW_AUD_STR_DESC_2		0x65184
+#define AUD_STR_DESC(pipe)	_PIPE(pipe, \
+					 _HSW_AUD_STR_DESC_1,	\
+					 _HSW_AUD_STR_DESC_2)
+
 #define _HSW_AUD_EDID_DATA_A		0x65050
 #define _HSW_AUD_EDID_DATA_B		0x65150
 #define HSW_AUD_EDID_DATA(pipe) _PIPE(pipe, \
diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
index f1148cd..df514b7 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -233,6 +233,9 @@ static void hsw_audio_codec_enable(struct drm_connector *connector,
 	const uint8_t *eld = connector->eld;
 	uint32_t tmp;
 	int len, i;
+	int cvt_idx;
+	int n_low, n_up, n;
+	int base_rate, mul, div, rate;
 
 	DRM_DEBUG_KMS("Enable audio codec on pipe %c, %u bytes ELD\n",
 		      pipe_name(pipe), drm_eld_size(eld));
@@ -265,6 +268,21 @@ static void hsw_audio_codec_enable(struct drm_connector *connector,
 	tmp |= AUDIO_ELD_VALID(pipe);
 	I915_WRITE(HSW_AUD_PIN_ELD_CP_VLD, tmp);
 
+	if ((mode->clock == 297000) ||
+		(mode->clock == DIV_ROUND_UP(297000 * 1000, 1001))) {
+		tmp = I915_READ(HSW_AUD_PIPE_CONN_SEL_CTRL);
+		cvt_idx = (tmp >> pipe*8) & 0xff;
+		tmp = I915_READ(AUD_STR_DESC(cvt_idx));
+		base_rate = tmp & (1 << 14);
+		if (base_rate == 0)
+			rate = 48000;
+		else
+			rate = 44100;
+		mul = (tmp & (0x7 << 11)) + 1;
+		div = (tmp & (0x7 << 8)) + 1;
+		rate = rate * mul / div;
+	}
+
 	/* Enable timestamps */
 	tmp = I915_READ(HSW_AUD_CFG(pipe));
 	tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
@@ -274,6 +292,30 @@ static void hsw_audio_codec_enable(struct drm_connector *connector,
 		tmp |= AUD_CONFIG_N_VALUE_INDEX;
 	else
 		tmp |= audio_config_hdmi_pixel_clock(mode);
+
+	if ((mode->clock != 297000) &&
+		(mode->clock != DIV_ROUND_UP(297000 * 1000, 1001))) {
+		tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
+	} else {
+		for (i = 0; i < ARRAY_SIZE(aud_ncts); i++) {
+			if ((rate == aud_ncts[i].sample_rate) &&
+				(mode->clock == aud_ncts[i].clock)) {
+				n = aud_ncts[i].n;
+				break;
+			}
+		}
+		if (n != 0) {
+			tmp |= AUD_CONFIG_N_PROG_ENABLE;
+			n_low = n & 0xfff;
+			n_up = (n >> 12) & 0xff;
+			tmp |= AUD_CONFIG_N_PROG_ENABLE;
+			tmp &= ~AUD_CONFIG_UPPER_N_MASK;
+			tmp |= (n_up << AUD_CONFIG_UPPER_N_SHIFT);
+			tmp &= ~AUD_CONFIG_LOWER_N_MASK;
+			tmp |= (n_low << AUD_CONFIG_LOWER_N_SHIFT);
+		}
+	}
+
 	I915_WRITE(HSW_AUD_CFG(pipe), tmp);
 }
 
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [alsa-devel] [PATCH 2/4] drm/i915: implement set_ncts callback

[Takashi Iwai]

On Thu, 06 Aug 2015 08:52:55 +0200,
libin.yang@intel.com wrote:
> 
> From: Libin Yang <libin.yang@intel.com>
> 
> Display audio may not work at some frequencies
> with the HW provided N/CTS.
> 
> This patch sets the proper N value for the
> given audio sample rate at the impacted frequencies.
> At other frequencies, it will use the N/CTS value
> which HW provides.
> 
> Signed-off-by: Libin Yang <libin.yang@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h    |  2 +
>  drivers/gpu/drm/i915/intel_audio.c | 93 ++++++++++++++++++++++++++++++++++++++
>  2 files changed, 95 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 3a77678..0b1cd72 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7010,6 +7010,8 @@ enum skl_disp_power_wells {
>  					_HSW_AUD_MISC_CTRL_A, \
>  					_HSW_AUD_MISC_CTRL_B)
>  
> +#define HSW_AUD_PIPE_CONN_SEL_CTRL  0x650ac
> +
>  #define _HSW_AUD_DIP_ELD_CTRL_ST_A	0x650b4
>  #define _HSW_AUD_DIP_ELD_CTRL_ST_B	0x651b4
>  #define HSW_AUD_DIP_ELD_CTRL(pipe) _PIPE(pipe, \
> diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
> index dc32cf4..f1148cd 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -68,6 +68,28 @@ static const struct {
>  	{ 148500, AUD_CONFIG_PIXEL_CLOCK_HDMI_148500 },
>  };
>  
> +static const struct {
> +	int sample_rate;
> +	int clock;
> +	int n;
> +	int cts;
> +} aud_ncts[] = {
> +	{ 44100, DIV_ROUND_UP(297000 * 1000, 1001), 4459, 234375 },
> +	{ 44100, 297000, 4704, 247500 },

As these two clock values are referred repeatedly in other places,
it'd be better to define constants.


Takashi
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/4] ALSA: hda - display audio call ncts callback

[Takashi Iwai]

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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/4] ALSA: hda - display audio call ncts callback

[Yang, Libin]

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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [alsa-devel] [PATCH 2/4] drm/i915: implement set_ncts callback

[Yang, Libin]

Hi Takashi,

> -----Original Message-----
> From: Takashi Iwai [mailto:tiwai@suse.de]
> Sent: Thursday, August 06, 2015 5:21 PM
> To: Yang, Libin
> Cc: alsa-devel@alsa-project.org; intel-gfx@lists.freedesktop.org; Lin,
> Mengdong
> Subject: Re: [alsa-devel] [PATCH 2/4] drm/i915: implement set_ncts
> callback
> 
> On Thu, 06 Aug 2015 08:52:55 +0200,
> libin.yang@intel.com wrote:
> >
> > From: Libin Yang <libin.yang@intel.com>
> >
> > Display audio may not work at some frequencies
> > with the HW provided N/CTS.
> >
> > This patch sets the proper N value for the
> > given audio sample rate at the impacted frequencies.
> > At other frequencies, it will use the N/CTS value
> > which HW provides.
> >
> > Signed-off-by: Libin Yang <libin.yang@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h    |  2 +
> >  drivers/gpu/drm/i915/intel_audio.c | 93
> ++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 95 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> b/drivers/gpu/drm/i915/i915_reg.h
> > index 3a77678..0b1cd72 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -7010,6 +7010,8 @@ enum skl_disp_power_wells {
> >  					_HSW_AUD_MISC_CTRL_A, \
> >  					_HSW_AUD_MISC_CTRL_B)
> >
> > +#define HSW_AUD_PIPE_CONN_SEL_CTRL  0x650ac
> > +
> >  #define _HSW_AUD_DIP_ELD_CTRL_ST_A	0x650b4
> >  #define _HSW_AUD_DIP_ELD_CTRL_ST_B	0x651b4
> >  #define HSW_AUD_DIP_ELD_CTRL(pipe) _PIPE(pipe, \
> > diff --git a/drivers/gpu/drm/i915/intel_audio.c
> b/drivers/gpu/drm/i915/intel_audio.c
> > index dc32cf4..f1148cd 100644
> > --- a/drivers/gpu/drm/i915/intel_audio.c
> > +++ b/drivers/gpu/drm/i915/intel_audio.c
> > @@ -68,6 +68,28 @@ static const struct {
> >  	{ 148500, AUD_CONFIG_PIXEL_CLOCK_HDMI_148500 },
> >  };
> >
> > +static const struct {
> > +	int sample_rate;
> > +	int clock;
> > +	int n;
> > +	int cts;
> > +} aud_ncts[] = {
> > +	{ 44100, DIV_ROUND_UP(297000 * 1000, 1001), 4459, 234375 },
> > +	{ 44100, 297000, 4704, 247500 },
> 
> As these two clock values are referred repeatedly in other places,
> it'd be better to define constants.

Do you mean use Macro such as:
#define 297MHZ 297000
#define 296MHZ DIV_ROUND_UP(297000 * 1000, 1001)

> 
> 
> Takashi
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [alsa-devel] [PATCH 3/4] ALSA: hda - display audio call ncts callback

[Raymond Yau]

[-- Attachment #1.1: Type: text/plain, Size: 4159 bytes --]

>
> > -----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.
>

[-- Attachment #1.2: Type: text/html, Size: 5313 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [alsa-devel] [PATCH 3/4] ALSA: hda - display audio call ncts callback

[Yang, Libin]

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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [alsa-devel] [PATCH 3/4] ALSA: hda - display audio call ncts callback

[Raymond Yau]

[-- Attachment #1.1: Type: text/plain, Size: 2374 bytes --]

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.

[-- Attachment #1.2: Type: text/html, Size: 3102 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [alsa-devel] [PATCH 3/4] ALSA: hda - display audio call ncts callback

[Yang, Libin]

[-- Attachment #1.1: Type: text/plain, Size: 3201 bytes --]

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.

[-- Attachment #1.2: Type: text/html, Size: 8734 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx