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

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

[libin.yang]

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

Add the sync_audio_rate callback.

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

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

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

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

[PATCH v3 2/4] drm/i915: implement sync_audio_rate callback

[libin.yang]

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

HDMI 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/intel_audio.c | 117 +++++++++++++++++++++++++++++++++++++
 1 file changed, 117 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
index dc32cf4..27add6a 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -68,6 +68,31 @@ static const struct {
 	{ 148500, AUD_CONFIG_PIXEL_CLOCK_HDMI_148500 },
 };
 
+/* HDMI N/CTS table */
+#define TMDS_297M 297000
+#define TMDS_296M DIV_ROUND_UP(297000 * 1000, 1001)
+static const struct {
+	int sample_rate;
+	int clock;
+	int n;
+	int cts;
+} aud_ncts[] = {
+	{ 44100, TMDS_296M, 4459, 234375 },
+	{ 44100, TMDS_297M, 4704, 247500 },
+	{ 48000, TMDS_296M, 5824, 281250 },
+	{ 48000, TMDS_297M, 5120, 247500 },
+	{ 32000, TMDS_296M, 5824, 421875 },
+	{ 32000, TMDS_297M, 3072, 222750 },
+	{ 88200, TMDS_296M, 8918, 234375 },
+	{ 88200, TMDS_297M, 9408, 247500 },
+	{ 96000, TMDS_296M, 11648, 281250 },
+	{ 96000, TMDS_297M, 10240, 247500 },
+	{ 176400, TMDS_296M, 17836, 234375 },
+	{ 176400, TMDS_297M, 18816, 247500 },
+	{ 44100, TMDS_296M, 23296, 281250 },
+	{ 44100, TMDS_297M, 20480, 247500 },
+};
+
 /* get AUD_CONFIG_PIXEL_CLOCK_HDMI_* value for mode */
 static u32 audio_config_hdmi_pixel_clock(struct drm_display_mode *mode)
 {
@@ -90,6 +115,31 @@ static u32 audio_config_hdmi_pixel_clock(struct drm_display_mode *mode)
 	return hdmi_audio_clock[i].config;
 }
 
+static int audio_config_get_n(struct drm_display_mode *mode, int rate)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(aud_ncts); i++) {
+		if ((rate == aud_ncts[i].sample_rate) &&
+			(mode->clock == aud_ncts[i].clock)) {
+			return aud_ncts[i].n;
+		}
+	}
+	return 0;
+}
+
+/* check whether N/CTS/M need be set manually */
+static bool audio_rate_need_prog(struct intel_crtc *crtc,
+					struct drm_display_mode *mode)
+{
+	if (((mode->clock == TMDS_297M) ||
+		 (mode->clock == TMDS_296M)) &&
+		intel_pipe_has_type(crtc, INTEL_OUTPUT_HDMI))
+		return true;
+	else
+		return false;
+}
+
 static bool intel_eld_uptodate(struct drm_connector *connector,
 			       int reg_eldv, uint32_t bits_eldv,
 			       int reg_elda, uint32_t bits_elda,
@@ -514,12 +564,79 @@ static int i915_audio_component_get_cdclk_freq(struct device *dev)
 	return ret;
 }
 
+static int i915_audio_component_sync_audio_rate(struct device *dev,
+						int port, 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 n_low, n_up, n;
+
+	/* 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 == INVALID_PIPE) {
+		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. check whether to set the N/CTS/M manually or not */
+	if (!audio_rate_need_prog(crtc, mode)) {
+		tmp = I915_READ(HSW_AUD_CFG(pipe));
+		tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
+		I915_WRITE(HSW_AUD_CFG(pipe), tmp);
+		return 0;
+	}
+
+	n = audio_config_get_n(mode, rate);
+	if (n == 0) {
+		DRM_DEBUG_KMS("Using automatic mode for N value on port %c\n",
+					  port_name(port));
+		tmp = I915_READ(HSW_AUD_CFG(pipe));
+		tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
+		I915_WRITE(HSW_AUD_CFG(pipe), tmp);
+		return 0;
+	}
+	n_low = n & 0xfff;
+	n_up = (n >> 12) & 0xff;
+
+	/* 4. set the N/CTS/M */
+	tmp = I915_READ(HSW_AUD_CFG(pipe));
+	tmp &= ~(AUD_CONFIG_UPPER_N_MASK | AUD_CONFIG_LOWER_N_MASK);
+	tmp |= ((n_up << AUD_CONFIG_UPPER_N_SHIFT) |
+			(n_low << AUD_CONFIG_LOWER_N_SHIFT) |
+			AUD_CONFIG_N_PROG_ENABLE);
+	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,
+	.sync_audio_rate = i915_audio_component_sync_audio_rate,
 };
 
 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 v3 3/4] ALSA: hda - display audio call sync_audio_rate callback

[libin.yang]

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

For display audio, call the sync_audio_rate callback function
to do the synchronization between gfx driver and audio driver.

Signed-off-by: Libin Yang <libin.yang@intel.com>
---
 sound/pci/hda/patch_hdmi.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index a97db5f..1668868 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -1770,6 +1770,16 @@ static bool check_non_pcm_per_cvt(struct hda_codec *codec, hda_nid_t cvt_nid)
 	return non_pcm;
 }
 
+/* There is a fixed mapping between audio pin node and display port
+ * on current Intel platforms:
+ * Pin Widget 5 - PORT B (port = 1 in i915 driver)
+ * Pin Widget 6 - PORT C (port = 2 in i915 driver)
+ * Pin Widget 7 - PORT D (port = 3 in i915 driver)
+ */
+static int intel_pin2port(hda_nid_t pin_nid)
+{
+	return pin_nid - 4;
+}
 
 /*
  * HDMI callbacks
@@ -1786,6 +1796,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 +1814,13 @@ static int generic_hdmi_playback_pcm_prepare(struct hda_pcm_stream *hinfo,
 		intel_not_share_assigned_cvt(codec, pin_nid, per_pin->mux_idx);
 	}
 
+	/* Call sync_audio_rate to set the N/CTS/M manually if necessary */
+	/* Todo: add DP1.2 MST audio support later */
+	if (acomp && acomp->ops && acomp->ops->sync_audio_rate)
+		acomp->ops->sync_audio_rate(acomp->dev,
+				intel_pin2port(pin_nid),
+				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 v3 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 values, 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    |  8 ++++++++
 drivers/gpu/drm/i915/intel_audio.c | 40 +++++++++++++++++++++++++++++++++++++-
 2 files changed, 47 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 6786e94..122b5bd 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7035,6 +7035,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, \
@@ -7049,6 +7051,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 27add6a..0a54ac0 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -140,6 +140,27 @@ static bool audio_rate_need_prog(struct intel_crtc *crtc,
 		return false;
 }
 
+static int audio_config_get_rate(struct drm_i915_private *dev_priv,
+						enum pipe pipe)
+{
+	uint32_t tmp;
+	int cvt_idx;
+	int base_rate, mul, div, rate;
+
+	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;
+	return rate;
+}
+
 static bool intel_eld_uptodate(struct drm_connector *connector,
 			       int reg_eldv, uint32_t bits_eldv,
 			       int reg_elda, uint32_t bits_elda,
@@ -261,6 +282,8 @@ static void hsw_audio_codec_enable(struct drm_connector *connector,
 	const uint8_t *eld = connector->eld;
 	uint32_t tmp;
 	int len, i;
+	int n_low, n_up, n;
+	int rate;
 
 	DRM_DEBUG_KMS("Enable audio codec on pipe %c, %u bytes ELD\n",
 		      pipe_name(pipe), drm_eld_size(eld));
@@ -296,12 +319,27 @@ static void hsw_audio_codec_enable(struct drm_connector *connector,
 	/* Enable timestamps */
 	tmp = I915_READ(HSW_AUD_CFG(pipe));
 	tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
-	tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
 	tmp &= ~AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK;
 	if (intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DISPLAYPORT))
 		tmp |= AUD_CONFIG_N_VALUE_INDEX;
 	else
 		tmp |= audio_config_hdmi_pixel_clock(mode);
+
+	tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
+	if (audio_rate_need_prog(intel_crtc, mode)) {
+		rate = audio_config_get_rate(dev_priv, pipe);
+		n = audio_config_get_n(mode, rate);
+		if (n != 0) {
+			n_low = n & 0xfff;
+			n_up = (n >> 12) & 0xff;
+			tmp &= ~(AUD_CONFIG_UPPER_N_MASK |
+					 AUD_CONFIG_LOWER_N_MASK);
+			tmp |= ((n_up << AUD_CONFIG_UPPER_N_SHIFT) |
+					(n_low << AUD_CONFIG_LOWER_N_SHIFT) |
+					AUD_CONFIG_N_PROG_ENABLE);
+		}
+	}
+
 	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: [PATCH v3 2/4] drm/i915: implement sync_audio_rate callback

[Jani Nikula]

On Mon, 17 Aug 2015, libin.yang@intel.com wrote:
> From: Libin Yang <libin.yang@intel.com>
>
> HDMI 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/intel_audio.c | 117 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 117 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
> index dc32cf4..27add6a 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -68,6 +68,31 @@ static const struct {
>  	{ 148500, AUD_CONFIG_PIXEL_CLOCK_HDMI_148500 },
>  };
>  
> +/* HDMI N/CTS table */
> +#define TMDS_297M 297000
> +#define TMDS_296M DIV_ROUND_UP(297000 * 1000, 1001)
> +static const struct {
> +	int sample_rate;
> +	int clock;
> +	int n;
> +	int cts;
> +} aud_ncts[] = {
> +	{ 44100, TMDS_296M, 4459, 234375 },
> +	{ 44100, TMDS_297M, 4704, 247500 },
> +	{ 48000, TMDS_296M, 5824, 281250 },
> +	{ 48000, TMDS_297M, 5120, 247500 },
> +	{ 32000, TMDS_296M, 5824, 421875 },
> +	{ 32000, TMDS_297M, 3072, 222750 },
> +	{ 88200, TMDS_296M, 8918, 234375 },
> +	{ 88200, TMDS_297M, 9408, 247500 },
> +	{ 96000, TMDS_296M, 11648, 281250 },
> +	{ 96000, TMDS_297M, 10240, 247500 },
> +	{ 176400, TMDS_296M, 17836, 234375 },
> +	{ 176400, TMDS_297M, 18816, 247500 },
> +	{ 44100, TMDS_296M, 23296, 281250 },
> +	{ 44100, TMDS_297M, 20480, 247500 },
> +};
> +
>  /* get AUD_CONFIG_PIXEL_CLOCK_HDMI_* value for mode */
>  static u32 audio_config_hdmi_pixel_clock(struct drm_display_mode *mode)
>  {
> @@ -90,6 +115,31 @@ static u32 audio_config_hdmi_pixel_clock(struct drm_display_mode *mode)
>  	return hdmi_audio_clock[i].config;
>  }
>  
> +static int audio_config_get_n(struct drm_display_mode *mode, int rate)
> +{
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(aud_ncts); i++) {
> +		if ((rate == aud_ncts[i].sample_rate) &&
> +			(mode->clock == aud_ncts[i].clock)) {
> +			return aud_ncts[i].n;
> +		}
> +	}
> +	return 0;
> +}
> +
> +/* check whether N/CTS/M need be set manually */
> +static bool audio_rate_need_prog(struct intel_crtc *crtc,
> +					struct drm_display_mode *mode)
> +{
> +	if (((mode->clock == TMDS_297M) ||
> +		 (mode->clock == TMDS_296M)) &&
> +		intel_pipe_has_type(crtc, INTEL_OUTPUT_HDMI))
> +		return true;
> +	else
> +		return false;
> +}
> +
>  static bool intel_eld_uptodate(struct drm_connector *connector,
>  			       int reg_eldv, uint32_t bits_eldv,
>  			       int reg_elda, uint32_t bits_elda,
> @@ -514,12 +564,79 @@ static int i915_audio_component_get_cdclk_freq(struct device *dev)
>  	return ret;
>  }
>  
> +static int i915_audio_component_sync_audio_rate(struct device *dev,
> +						int port, 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 n_low, n_up, n;
> +
> +	/* 1. get the pipe */
> +	for_each_intel_encoder(drm_dev, intel_encoder) {
> +		intel_dig_port = enc_to_dig_port(&intel_encoder->base);

Mmmh, not all encoders are digital ports. enc_to_dig_port does a
container_of, so you can't just check for NULL either. IOW, you need to
check the encoder->type (and you only care about HDMI here).

> +		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 == INVALID_PIPE) {
> +		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. check whether to set the N/CTS/M manually or not */
> +	if (!audio_rate_need_prog(crtc, mode)) {
> +		tmp = I915_READ(HSW_AUD_CFG(pipe));
> +		tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
> +		I915_WRITE(HSW_AUD_CFG(pipe), tmp);
> +		return 0;
> +	}
> +
> +	n = audio_config_get_n(mode, rate);
> +	if (n == 0) {
> +		DRM_DEBUG_KMS("Using automatic mode for N value on port %c\n",
> +					  port_name(port));
> +		tmp = I915_READ(HSW_AUD_CFG(pipe));
> +		tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
> +		I915_WRITE(HSW_AUD_CFG(pipe), tmp);
> +		return 0;
> +	}
> +	n_low = n & 0xfff;
> +	n_up = (n >> 12) & 0xff;
> +
> +	/* 4. set the N/CTS/M */
> +	tmp = I915_READ(HSW_AUD_CFG(pipe));
> +	tmp &= ~(AUD_CONFIG_UPPER_N_MASK | AUD_CONFIG_LOWER_N_MASK);
> +	tmp |= ((n_up << AUD_CONFIG_UPPER_N_SHIFT) |
> +			(n_low << AUD_CONFIG_LOWER_N_SHIFT) |
> +			AUD_CONFIG_N_PROG_ENABLE);
> +	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,
> +	.sync_audio_rate = i915_audio_component_sync_audio_rate,
>  };
>  
>  static int i915_audio_component_bind(struct device *i915_dev,
> -- 
> 1.9.1
>

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Takashi Iwai]

On Mon, 17 Aug 2015 12:40:00 +0200,
libin.yang@intel.com wrote:
> 
> From: Libin Yang <libin.yang@intel.com>
> 
> For display audio, call the sync_audio_rate callback function
> to do the synchronization between gfx driver and audio driver.
> 
> Signed-off-by: Libin Yang <libin.yang@intel.com>

I guess it would be easier to take this series from drm tree.
Feel free to my ack
  Reviewed-by: Takashi Iwai <tiwai@suse.de>


thanks,

Takashi


> ---
>  sound/pci/hda/patch_hdmi.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
> index a97db5f..1668868 100644
> --- a/sound/pci/hda/patch_hdmi.c
> +++ b/sound/pci/hda/patch_hdmi.c
> @@ -1770,6 +1770,16 @@ static bool check_non_pcm_per_cvt(struct hda_codec *codec, hda_nid_t cvt_nid)
>  	return non_pcm;
>  }
>  
> +/* There is a fixed mapping between audio pin node and display port
> + * on current Intel platforms:
> + * Pin Widget 5 - PORT B (port = 1 in i915 driver)
> + * Pin Widget 6 - PORT C (port = 2 in i915 driver)
> + * Pin Widget 7 - PORT D (port = 3 in i915 driver)
> + */
> +static int intel_pin2port(hda_nid_t pin_nid)
> +{
> +	return pin_nid - 4;
> +}
>  
>  /*
>   * HDMI callbacks
> @@ -1786,6 +1796,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 +1814,13 @@ static int generic_hdmi_playback_pcm_prepare(struct hda_pcm_stream *hinfo,
>  		intel_not_share_assigned_cvt(codec, pin_nid, per_pin->mux_idx);
>  	}
>  
> +	/* Call sync_audio_rate to set the N/CTS/M manually if necessary */
> +	/* Todo: add DP1.2 MST audio support later */
> +	if (acomp && acomp->ops && acomp->ops->sync_audio_rate)
> +		acomp->ops->sync_audio_rate(acomp->dev,
> +				intel_pin2port(pin_nid),
> +				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

Re: [PATCH v3 2/4] drm/i915: implement sync_audio_rate callback

[Yang, Libin]

Hi Jani

> -----Original Message-----
> From: Jani Nikula [mailto:jani.nikula@linux.intel.com]
> Sent: Monday, August 17, 2015 8:21 PM
> To: Yang, Libin; alsa-devel@alsa-project.org; tiwai@suse.de; intel-
> gfx@lists.freedesktop.org; daniel.vetter@ffwll.ch
> Cc: Yang, Libin
> Subject: Re: [PATCH v3 2/4] drm/i915: implement sync_audio_rate
> callback
> 
> On Mon, 17 Aug 2015, libin.yang@intel.com wrote:
> > From: Libin Yang <libin.yang@intel.com>
> >
> > HDMI 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/intel_audio.c | 117
> +++++++++++++++++++++++++++++++++++++
> >  1 file changed, 117 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_audio.c
> b/drivers/gpu/drm/i915/intel_audio.c
> > index dc32cf4..27add6a 100644
> > --- a/drivers/gpu/drm/i915/intel_audio.c
> > +++ b/drivers/gpu/drm/i915/intel_audio.c
> > @@ -68,6 +68,31 @@ static const struct {
> >  	{ 148500, AUD_CONFIG_PIXEL_CLOCK_HDMI_148500 },
> >  };
> >
> > +/* HDMI N/CTS table */
> > +#define TMDS_297M 297000
> > +#define TMDS_296M DIV_ROUND_UP(297000 * 1000, 1001)
> > +static const struct {
> > +	int sample_rate;
> > +	int clock;
> > +	int n;
> > +	int cts;
> > +} aud_ncts[] = {
> > +	{ 44100, TMDS_296M, 4459, 234375 },
> > +	{ 44100, TMDS_297M, 4704, 247500 },
> > +	{ 48000, TMDS_296M, 5824, 281250 },
> > +	{ 48000, TMDS_297M, 5120, 247500 },
> > +	{ 32000, TMDS_296M, 5824, 421875 },
> > +	{ 32000, TMDS_297M, 3072, 222750 },
> > +	{ 88200, TMDS_296M, 8918, 234375 },
> > +	{ 88200, TMDS_297M, 9408, 247500 },
> > +	{ 96000, TMDS_296M, 11648, 281250 },
> > +	{ 96000, TMDS_297M, 10240, 247500 },
> > +	{ 176400, TMDS_296M, 17836, 234375 },
> > +	{ 176400, TMDS_297M, 18816, 247500 },
> > +	{ 44100, TMDS_296M, 23296, 281250 },
> > +	{ 44100, TMDS_297M, 20480, 247500 },
> > +};
> > +
> >  /* get AUD_CONFIG_PIXEL_CLOCK_HDMI_* value for mode */
> >  static u32 audio_config_hdmi_pixel_clock(struct drm_display_mode
> *mode)
> >  {
> > @@ -90,6 +115,31 @@ static u32
> audio_config_hdmi_pixel_clock(struct drm_display_mode *mode)
> >  	return hdmi_audio_clock[i].config;
> >  }
> >
> > +static int audio_config_get_n(struct drm_display_mode *mode, int
> rate)
> > +{
> > +	int i;
> > +
> > +	for (i = 0; i < ARRAY_SIZE(aud_ncts); i++) {
> > +		if ((rate == aud_ncts[i].sample_rate) &&
> > +			(mode->clock == aud_ncts[i].clock)) {
> > +			return aud_ncts[i].n;
> > +		}
> > +	}
> > +	return 0;
> > +}
> > +
> > +/* check whether N/CTS/M need be set manually */
> > +static bool audio_rate_need_prog(struct intel_crtc *crtc,
> > +					struct drm_display_mode
> *mode)
> > +{
> > +	if (((mode->clock == TMDS_297M) ||
> > +		 (mode->clock == TMDS_296M)) &&
> > +		intel_pipe_has_type(crtc, INTEL_OUTPUT_HDMI))
> > +		return true;
> > +	else
> > +		return false;
> > +}
> > +
> >  static bool intel_eld_uptodate(struct drm_connector *connector,
> >  			       int reg_eldv, uint32_t bits_eldv,
> >  			       int reg_elda, uint32_t bits_elda,
> > @@ -514,12 +564,79 @@ static int
> i915_audio_component_get_cdclk_freq(struct device *dev)
> >  	return ret;
> >  }
> >
> > +static int i915_audio_component_sync_audio_rate(struct device
> *dev,
> > +						int port, 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 n_low, n_up, n;
> > +
> > +	/* 1. get the pipe */
> > +	for_each_intel_encoder(drm_dev, intel_encoder) {
> > +		intel_dig_port = enc_to_dig_port(&intel_encoder-
> >base);
> 
> Mmmh, not all encoders are digital ports. enc_to_dig_port does a
> container_of, so you can't just check for NULL either. IOW, you need
> to
> check the encoder->type (and you only care about HDMI here).

Do you mean it should be:
if (!crtc || !intel_pipe_has_type(crtc, INTEL_OUTPUT_HDMI))

regards,
Libin

> 
> > +		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 == INVALID_PIPE) {
> > +		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. check whether to set the N/CTS/M manually or not */
> > +	if (!audio_rate_need_prog(crtc, mode)) {
> > +		tmp = I915_READ(HSW_AUD_CFG(pipe));
> > +		tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
> > +		I915_WRITE(HSW_AUD_CFG(pipe), tmp);
> > +		return 0;
> > +	}
> > +
> > +	n = audio_config_get_n(mode, rate);
> > +	if (n == 0) {
> > +		DRM_DEBUG_KMS("Using automatic mode for N value
> on port %c\n",
> > +					  port_name(port));
> > +		tmp = I915_READ(HSW_AUD_CFG(pipe));
> > +		tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
> > +		I915_WRITE(HSW_AUD_CFG(pipe), tmp);
> > +		return 0;
> > +	}
> > +	n_low = n & 0xfff;
> > +	n_up = (n >> 12) & 0xff;
> > +
> > +	/* 4. set the N/CTS/M */
> > +	tmp = I915_READ(HSW_AUD_CFG(pipe));
> > +	tmp &= ~(AUD_CONFIG_UPPER_N_MASK |
> AUD_CONFIG_LOWER_N_MASK);
> > +	tmp |= ((n_up << AUD_CONFIG_UPPER_N_SHIFT) |
> > +			(n_low << AUD_CONFIG_LOWER_N_SHIFT) |
> > +			AUD_CONFIG_N_PROG_ENABLE);
> > +	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,
> > +	.sync_audio_rate = i915_audio_component_sync_audio_rate,
> >  };
> >
> >  static int i915_audio_component_bind(struct device *i915_dev,
> > --
> > 1.9.1
> >
> 
> --
> Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Yang, Libin]

Hi Takashi,

> -----Original Message-----
> From: Takashi Iwai [mailto:tiwai@suse.de]
> Sent: Monday, August 17, 2015 8:26 PM
> To: Yang, Libin
> Cc: alsa-devel@alsa-project.org; intel-gfx@lists.freedesktop.org;
> daniel.vetter@ffwll.ch; jani.nikula@linux.intel.com
> Subject: Re: [PATCH v3 3/4] ALSA: hda - display audio call
> sync_audio_rate callback
> 
> On Mon, 17 Aug 2015 12:40:00 +0200,
> libin.yang@intel.com wrote:
> >
> > From: Libin Yang <libin.yang@intel.com>
> >
> > For display audio, call the sync_audio_rate callback function
> > to do the synchronization between gfx driver and audio driver.
> >
> > Signed-off-by: Libin Yang <libin.yang@intel.com>
> 
> I guess it would be easier to take this series from drm tree.
> Feel free to my ack
>   Reviewed-by: Takashi Iwai <tiwai@suse.de>

Get it. Thanks.

Regards,
Libin

> 
> 
> thanks,
> 
> Takashi
> 
> 
> > ---
> >  sound/pci/hda/patch_hdmi.c | 19 +++++++++++++++++++
> >  1 file changed, 19 insertions(+)
> >
> > diff --git a/sound/pci/hda/patch_hdmi.c
> b/sound/pci/hda/patch_hdmi.c
> > index a97db5f..1668868 100644
> > --- a/sound/pci/hda/patch_hdmi.c
> > +++ b/sound/pci/hda/patch_hdmi.c
> > @@ -1770,6 +1770,16 @@ static bool
> check_non_pcm_per_cvt(struct hda_codec *codec, hda_nid_t cvt_nid)
> >  	return non_pcm;
> >  }
> >
> > +/* There is a fixed mapping between audio pin node and display
> port
> > + * on current Intel platforms:
> > + * Pin Widget 5 - PORT B (port = 1 in i915 driver)
> > + * Pin Widget 6 - PORT C (port = 2 in i915 driver)
> > + * Pin Widget 7 - PORT D (port = 3 in i915 driver)
> > + */
> > +static int intel_pin2port(hda_nid_t pin_nid)
> > +{
> > +	return pin_nid - 4;
> > +}
> >
> >  /*
> >   * HDMI callbacks
> > @@ -1786,6 +1796,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 +1814,13 @@ static int
> generic_hdmi_playback_pcm_prepare(struct hda_pcm_stream *hinfo,
> >  		intel_not_share_assigned_cvt(codec, pin_nid, per_pin-
> >mux_idx);
> >  	}
> >
> > +	/* Call sync_audio_rate to set the N/CTS/M manually if
> necessary */
> > +	/* Todo: add DP1.2 MST audio support later */
> > +	if (acomp && acomp->ops && acomp->ops->sync_audio_rate)
> > +		acomp->ops->sync_audio_rate(acomp->dev,
> > +				intel_pin2port(pin_nid),
> > +				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

Re: [PATCH v3 2/4] drm/i915: implement sync_audio_rate callback

[Jani Nikula]

On Mon, 17 Aug 2015, "Yang, Libin" <libin.yang@intel.com> wrote:
> Hi Jani
>
>> -----Original Message-----
>> From: Jani Nikula [mailto:jani.nikula@linux.intel.com]
>> Sent: Monday, August 17, 2015 8:21 PM
>> To: Yang, Libin; alsa-devel@alsa-project.org; tiwai@suse.de; intel-
>> gfx@lists.freedesktop.org; daniel.vetter@ffwll.ch
>> Cc: Yang, Libin
>> Subject: Re: [PATCH v3 2/4] drm/i915: implement sync_audio_rate
>> callback
>> 
>> On Mon, 17 Aug 2015, libin.yang@intel.com wrote:
>> > From: Libin Yang <libin.yang@intel.com>
>> >
>> > HDMI 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/intel_audio.c | 117
>> +++++++++++++++++++++++++++++++++++++
>> >  1 file changed, 117 insertions(+)
>> >
>> > diff --git a/drivers/gpu/drm/i915/intel_audio.c
>> b/drivers/gpu/drm/i915/intel_audio.c
>> > index dc32cf4..27add6a 100644
>> > --- a/drivers/gpu/drm/i915/intel_audio.c
>> > +++ b/drivers/gpu/drm/i915/intel_audio.c
>> > @@ -68,6 +68,31 @@ static const struct {
>> >  	{ 148500, AUD_CONFIG_PIXEL_CLOCK_HDMI_148500 },
>> >  };
>> >
>> > +/* HDMI N/CTS table */
>> > +#define TMDS_297M 297000
>> > +#define TMDS_296M DIV_ROUND_UP(297000 * 1000, 1001)
>> > +static const struct {
>> > +	int sample_rate;
>> > +	int clock;
>> > +	int n;
>> > +	int cts;
>> > +} aud_ncts[] = {
>> > +	{ 44100, TMDS_296M, 4459, 234375 },
>> > +	{ 44100, TMDS_297M, 4704, 247500 },
>> > +	{ 48000, TMDS_296M, 5824, 281250 },
>> > +	{ 48000, TMDS_297M, 5120, 247500 },
>> > +	{ 32000, TMDS_296M, 5824, 421875 },
>> > +	{ 32000, TMDS_297M, 3072, 222750 },
>> > +	{ 88200, TMDS_296M, 8918, 234375 },
>> > +	{ 88200, TMDS_297M, 9408, 247500 },
>> > +	{ 96000, TMDS_296M, 11648, 281250 },
>> > +	{ 96000, TMDS_297M, 10240, 247500 },
>> > +	{ 176400, TMDS_296M, 17836, 234375 },
>> > +	{ 176400, TMDS_297M, 18816, 247500 },
>> > +	{ 44100, TMDS_296M, 23296, 281250 },
>> > +	{ 44100, TMDS_297M, 20480, 247500 },
>> > +};
>> > +
>> >  /* get AUD_CONFIG_PIXEL_CLOCK_HDMI_* value for mode */
>> >  static u32 audio_config_hdmi_pixel_clock(struct drm_display_mode
>> *mode)
>> >  {
>> > @@ -90,6 +115,31 @@ static u32
>> audio_config_hdmi_pixel_clock(struct drm_display_mode *mode)
>> >  	return hdmi_audio_clock[i].config;
>> >  }
>> >
>> > +static int audio_config_get_n(struct drm_display_mode *mode, int
>> rate)
>> > +{
>> > +	int i;
>> > +
>> > +	for (i = 0; i < ARRAY_SIZE(aud_ncts); i++) {
>> > +		if ((rate == aud_ncts[i].sample_rate) &&
>> > +			(mode->clock == aud_ncts[i].clock)) {
>> > +			return aud_ncts[i].n;
>> > +		}
>> > +	}
>> > +	return 0;
>> > +}
>> > +
>> > +/* check whether N/CTS/M need be set manually */
>> > +static bool audio_rate_need_prog(struct intel_crtc *crtc,
>> > +					struct drm_display_mode
>> *mode)
>> > +{
>> > +	if (((mode->clock == TMDS_297M) ||
>> > +		 (mode->clock == TMDS_296M)) &&
>> > +		intel_pipe_has_type(crtc, INTEL_OUTPUT_HDMI))
>> > +		return true;
>> > +	else
>> > +		return false;
>> > +}
>> > +
>> >  static bool intel_eld_uptodate(struct drm_connector *connector,
>> >  			       int reg_eldv, uint32_t bits_eldv,
>> >  			       int reg_elda, uint32_t bits_elda,
>> > @@ -514,12 +564,79 @@ static int
>> i915_audio_component_get_cdclk_freq(struct device *dev)
>> >  	return ret;
>> >  }
>> >
>> > +static int i915_audio_component_sync_audio_rate(struct device
>> *dev,
>> > +						int port, 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 n_low, n_up, n;
>> > +
>> > +	/* 1. get the pipe */
>> > +	for_each_intel_encoder(drm_dev, intel_encoder) {
>> > +		intel_dig_port = enc_to_dig_port(&intel_encoder-
>> >base);
>> 
>> Mmmh, not all encoders are digital ports. enc_to_dig_port does a
>> container_of, so you can't just check for NULL either. IOW, you need
>> to
>> check the encoder->type (and you only care about HDMI here).
>
> Do you mean it should be:
> if (!crtc || !intel_pipe_has_type(crtc, INTEL_OUTPUT_HDMI))

No, just check for intel_encoder->type as the first thing in the loop,
and continue if it isn't HDMI.

	if (intel_encoder->type != INTEL_OUTPUT_HDMI)
        	continue;

>
> regards,
> Libin
>
>> 
>> > +		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 == INVALID_PIPE) {
>> > +		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. check whether to set the N/CTS/M manually or not */
>> > +	if (!audio_rate_need_prog(crtc, mode)) {
>> > +		tmp = I915_READ(HSW_AUD_CFG(pipe));
>> > +		tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
>> > +		I915_WRITE(HSW_AUD_CFG(pipe), tmp);
>> > +		return 0;
>> > +	}
>> > +
>> > +	n = audio_config_get_n(mode, rate);
>> > +	if (n == 0) {
>> > +		DRM_DEBUG_KMS("Using automatic mode for N value
>> on port %c\n",
>> > +					  port_name(port));
>> > +		tmp = I915_READ(HSW_AUD_CFG(pipe));
>> > +		tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
>> > +		I915_WRITE(HSW_AUD_CFG(pipe), tmp);
>> > +		return 0;
>> > +	}
>> > +	n_low = n & 0xfff;
>> > +	n_up = (n >> 12) & 0xff;
>> > +
>> > +	/* 4. set the N/CTS/M */
>> > +	tmp = I915_READ(HSW_AUD_CFG(pipe));
>> > +	tmp &= ~(AUD_CONFIG_UPPER_N_MASK |
>> AUD_CONFIG_LOWER_N_MASK);
>> > +	tmp |= ((n_up << AUD_CONFIG_UPPER_N_SHIFT) |
>> > +			(n_low << AUD_CONFIG_LOWER_N_SHIFT) |
>> > +			AUD_CONFIG_N_PROG_ENABLE);
>> > +	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,
>> > +	.sync_audio_rate = i915_audio_component_sync_audio_rate,
>> >  };
>> >
>> >  static int i915_audio_component_bind(struct device *i915_dev,
>> > --
>> > 1.9.1
>> >
>> 
>> --
>> Jani Nikula, Intel Open Source Technology Center

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3 2/4] drm/i915: implement sync_audio_rate callback

[Daniel Vetter]

On Mon, Aug 17, 2015 at 03:20:38PM +0300, Jani Nikula wrote:
> On Mon, 17 Aug 2015, libin.yang@intel.com wrote:
> > From: Libin Yang <libin.yang@intel.com>
> >
> > HDMI 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/intel_audio.c | 117 +++++++++++++++++++++++++++++++++++++
> >  1 file changed, 117 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
> > index dc32cf4..27add6a 100644
> > --- a/drivers/gpu/drm/i915/intel_audio.c
> > +++ b/drivers/gpu/drm/i915/intel_audio.c
> > @@ -68,6 +68,31 @@ static const struct {
> >  	{ 148500, AUD_CONFIG_PIXEL_CLOCK_HDMI_148500 },
> >  };
> >  
> > +/* HDMI N/CTS table */
> > +#define TMDS_297M 297000
> > +#define TMDS_296M DIV_ROUND_UP(297000 * 1000, 1001)
> > +static const struct {
> > +	int sample_rate;
> > +	int clock;
> > +	int n;
> > +	int cts;
> > +} aud_ncts[] = {
> > +	{ 44100, TMDS_296M, 4459, 234375 },
> > +	{ 44100, TMDS_297M, 4704, 247500 },
> > +	{ 48000, TMDS_296M, 5824, 281250 },
> > +	{ 48000, TMDS_297M, 5120, 247500 },
> > +	{ 32000, TMDS_296M, 5824, 421875 },
> > +	{ 32000, TMDS_297M, 3072, 222750 },
> > +	{ 88200, TMDS_296M, 8918, 234375 },
> > +	{ 88200, TMDS_297M, 9408, 247500 },
> > +	{ 96000, TMDS_296M, 11648, 281250 },
> > +	{ 96000, TMDS_297M, 10240, 247500 },
> > +	{ 176400, TMDS_296M, 17836, 234375 },
> > +	{ 176400, TMDS_297M, 18816, 247500 },
> > +	{ 44100, TMDS_296M, 23296, 281250 },
> > +	{ 44100, TMDS_297M, 20480, 247500 },
> > +};
> > +
> >  /* get AUD_CONFIG_PIXEL_CLOCK_HDMI_* value for mode */
> >  static u32 audio_config_hdmi_pixel_clock(struct drm_display_mode *mode)
> >  {
> > @@ -90,6 +115,31 @@ static u32 audio_config_hdmi_pixel_clock(struct drm_display_mode *mode)
> >  	return hdmi_audio_clock[i].config;
> >  }
> >  
> > +static int audio_config_get_n(struct drm_display_mode *mode, int rate)
> > +{
> > +	int i;
> > +
> > +	for (i = 0; i < ARRAY_SIZE(aud_ncts); i++) {
> > +		if ((rate == aud_ncts[i].sample_rate) &&
> > +			(mode->clock == aud_ncts[i].clock)) {
> > +			return aud_ncts[i].n;
> > +		}
> > +	}
> > +	return 0;
> > +}
> > +
> > +/* check whether N/CTS/M need be set manually */
> > +static bool audio_rate_need_prog(struct intel_crtc *crtc,
> > +					struct drm_display_mode *mode)
> > +{
> > +	if (((mode->clock == TMDS_297M) ||
> > +		 (mode->clock == TMDS_296M)) &&
> > +		intel_pipe_has_type(crtc, INTEL_OUTPUT_HDMI))
> > +		return true;
> > +	else
> > +		return false;
> > +}
> > +
> >  static bool intel_eld_uptodate(struct drm_connector *connector,
> >  			       int reg_eldv, uint32_t bits_eldv,
> >  			       int reg_elda, uint32_t bits_elda,
> > @@ -514,12 +564,79 @@ static int i915_audio_component_get_cdclk_freq(struct device *dev)
> >  	return ret;
> >  }
> >  
> > +static int i915_audio_component_sync_audio_rate(struct device *dev,
> > +						int port, 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 n_low, n_up, n;
> > +
> > +	/* 1. get the pipe */
> > +	for_each_intel_encoder(drm_dev, intel_encoder) {
> > +		intel_dig_port = enc_to_dig_port(&intel_encoder->base);
> 
> Mmmh, not all encoders are digital ports. enc_to_dig_port does a
> container_of, so you can't just check for NULL either. IOW, you need to
> check the encoder->type (and you only care about HDMI here).

encoder->type is changed at runtime thanks to ddi encoders. Also shouldn't
we care about dp audio too or am I confused?
-Daniel

> 
> > +		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 == INVALID_PIPE) {
> > +		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. check whether to set the N/CTS/M manually or not */
> > +	if (!audio_rate_need_prog(crtc, mode)) {
> > +		tmp = I915_READ(HSW_AUD_CFG(pipe));
> > +		tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
> > +		I915_WRITE(HSW_AUD_CFG(pipe), tmp);
> > +		return 0;
> > +	}
> > +
> > +	n = audio_config_get_n(mode, rate);
> > +	if (n == 0) {
> > +		DRM_DEBUG_KMS("Using automatic mode for N value on port %c\n",
> > +					  port_name(port));
> > +		tmp = I915_READ(HSW_AUD_CFG(pipe));
> > +		tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
> > +		I915_WRITE(HSW_AUD_CFG(pipe), tmp);
> > +		return 0;
> > +	}
> > +	n_low = n & 0xfff;
> > +	n_up = (n >> 12) & 0xff;
> > +
> > +	/* 4. set the N/CTS/M */
> > +	tmp = I915_READ(HSW_AUD_CFG(pipe));
> > +	tmp &= ~(AUD_CONFIG_UPPER_N_MASK | AUD_CONFIG_LOWER_N_MASK);
> > +	tmp |= ((n_up << AUD_CONFIG_UPPER_N_SHIFT) |
> > +			(n_low << AUD_CONFIG_LOWER_N_SHIFT) |
> > +			AUD_CONFIG_N_PROG_ENABLE);
> > +	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,
> > +	.sync_audio_rate = i915_audio_component_sync_audio_rate,
> >  };
> >  
> >  static int i915_audio_component_bind(struct device *i915_dev,
> > -- 
> > 1.9.1
> >
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3 2/4] drm/i915: implement sync_audio_rate callback

[Yang, Libin]

Hi Daniel,

> -----Original Message-----
> From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of
> Daniel Vetter
> Sent: Wednesday, August 26, 2015 3:53 PM
> To: Jani Nikula
> Cc: Yang, Libin; alsa-devel@alsa-project.org; tiwai@suse.de; intel-
> gfx@lists.freedesktop.org; daniel.vetter@ffwll.ch
> Subject: Re: [Intel-gfx] [PATCH v3 2/4] drm/i915: implement
> sync_audio_rate callback
> 
> On Mon, Aug 17, 2015 at 03:20:38PM +0300, Jani Nikula wrote:
> > On Mon, 17 Aug 2015, libin.yang@intel.com wrote:
> > > From: Libin Yang <libin.yang@intel.com>
> > >
> > > HDMI 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/intel_audio.c | 117
> +++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 117 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_audio.c
> b/drivers/gpu/drm/i915/intel_audio.c
> > > index dc32cf4..27add6a 100644
> > > --- a/drivers/gpu/drm/i915/intel_audio.c
> > > +++ b/drivers/gpu/drm/i915/intel_audio.c
> > > @@ -68,6 +68,31 @@ static const struct {
> > >  	{ 148500, AUD_CONFIG_PIXEL_CLOCK_HDMI_148500 },
> > >  };
> > >
> > > +/* HDMI N/CTS table */
> > > +#define TMDS_297M 297000
> > > +#define TMDS_296M DIV_ROUND_UP(297000 * 1000, 1001)
> > > +static const struct {
> > > +	int sample_rate;
> > > +	int clock;
> > > +	int n;
> > > +	int cts;
> > > +} aud_ncts[] = {
> > > +	{ 44100, TMDS_296M, 4459, 234375 },
> > > +	{ 44100, TMDS_297M, 4704, 247500 },
> > > +	{ 48000, TMDS_296M, 5824, 281250 },
> > > +	{ 48000, TMDS_297M, 5120, 247500 },
> > > +	{ 32000, TMDS_296M, 5824, 421875 },
> > > +	{ 32000, TMDS_297M, 3072, 222750 },
> > > +	{ 88200, TMDS_296M, 8918, 234375 },
> > > +	{ 88200, TMDS_297M, 9408, 247500 },
> > > +	{ 96000, TMDS_296M, 11648, 281250 },
> > > +	{ 96000, TMDS_297M, 10240, 247500 },
> > > +	{ 176400, TMDS_296M, 17836, 234375 },
> > > +	{ 176400, TMDS_297M, 18816, 247500 },
> > > +	{ 44100, TMDS_296M, 23296, 281250 },
> > > +	{ 44100, TMDS_297M, 20480, 247500 },
> > > +};
> > > +
> > >  /* get AUD_CONFIG_PIXEL_CLOCK_HDMI_* value for mode */
> > >  static u32 audio_config_hdmi_pixel_clock(struct
> drm_display_mode *mode)
> > >  {
> > > @@ -90,6 +115,31 @@ static u32
> audio_config_hdmi_pixel_clock(struct drm_display_mode *mode)
> > >  	return hdmi_audio_clock[i].config;
> > >  }
> > >
> > > +static int audio_config_get_n(struct drm_display_mode *mode,
> int rate)
> > > +{
> > > +	int i;
> > > +
> > > +	for (i = 0; i < ARRAY_SIZE(aud_ncts); i++) {
> > > +		if ((rate == aud_ncts[i].sample_rate) &&
> > > +			(mode->clock == aud_ncts[i].clock)) {
> > > +			return aud_ncts[i].n;
> > > +		}
> > > +	}
> > > +	return 0;
> > > +}
> > > +
> > > +/* check whether N/CTS/M need be set manually */
> > > +static bool audio_rate_need_prog(struct intel_crtc *crtc,
> > > +					struct drm_display_mode
> *mode)
> > > +{
> > > +	if (((mode->clock == TMDS_297M) ||
> > > +		 (mode->clock == TMDS_296M)) &&
> > > +		intel_pipe_has_type(crtc, INTEL_OUTPUT_HDMI))
> > > +		return true;
> > > +	else
> > > +		return false;
> > > +}
> > > +
> > >  static bool intel_eld_uptodate(struct drm_connector *connector,
> > >  			       int reg_eldv, uint32_t bits_eldv,
> > >  			       int reg_elda, uint32_t bits_elda,
> > > @@ -514,12 +564,79 @@ static int
> i915_audio_component_get_cdclk_freq(struct device *dev)
> > >  	return ret;
> > >  }
> > >
> > > +static int i915_audio_component_sync_audio_rate(struct device
> *dev,
> > > +						int port, 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 n_low, n_up, n;
> > > +
> > > +	/* 1. get the pipe */
> > > +	for_each_intel_encoder(drm_dev, intel_encoder) {
> > > +		intel_dig_port = enc_to_dig_port(&intel_encoder-
> >base);
> >
> > Mmmh, not all encoders are digital ports. enc_to_dig_port does a
> > container_of, so you can't just check for NULL either. IOW, you need
> to
> > check the encoder->type (and you only care about HDMI here).
> 
> encoder->type is changed at runtime thanks to ddi encoders. Also
> shouldn't
> we care about dp audio too or am I confused?

Do you have suggestion how to handle it as it is changed
at runtime? Need a lock or not?

For DP, because the 4K DP display is not stable, so we didn't
do the full test on DP and no patch is ready for DP currently.

Based on the very simple test on DP, it seems DP audio is
OK. When DP is stable, we will do the full test and decide
to make the patch or not based on the test result.

Regards,
Libin

> -Daniel
> 
> >
> > > +		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 == INVALID_PIPE) {
> > > +		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. check whether to set the N/CTS/M manually or not */
> > > +	if (!audio_rate_need_prog(crtc, mode)) {
> > > +		tmp = I915_READ(HSW_AUD_CFG(pipe));
> > > +		tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
> > > +		I915_WRITE(HSW_AUD_CFG(pipe), tmp);
> > > +		return 0;
> > > +	}
> > > +
> > > +	n = audio_config_get_n(mode, rate);
> > > +	if (n == 0) {
> > > +		DRM_DEBUG_KMS("Using automatic mode for N value
> on port %c\n",
> > > +					  port_name(port));
> > > +		tmp = I915_READ(HSW_AUD_CFG(pipe));
> > > +		tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
> > > +		I915_WRITE(HSW_AUD_CFG(pipe), tmp);
> > > +		return 0;
> > > +	}
> > > +	n_low = n & 0xfff;
> > > +	n_up = (n >> 12) & 0xff;
> > > +
> > > +	/* 4. set the N/CTS/M */
> > > +	tmp = I915_READ(HSW_AUD_CFG(pipe));
> > > +	tmp &= ~(AUD_CONFIG_UPPER_N_MASK |
> AUD_CONFIG_LOWER_N_MASK);
> > > +	tmp |= ((n_up << AUD_CONFIG_UPPER_N_SHIFT) |
> > > +			(n_low << AUD_CONFIG_LOWER_N_SHIFT) |
> > > +			AUD_CONFIG_N_PROG_ENABLE);
> > > +	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,
> > > +	.sync_audio_rate = i915_audio_component_sync_audio_rate,
> > >  };
> > >
> > >  static int i915_audio_component_bind(struct device *i915_dev,
> > > --
> > > 1.9.1
> > >
> >
> > --
> > Jani Nikula, Intel Open Source Technology Center
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3 2/4] drm/i915: implement sync_audio_rate callback

[Jani Nikula]

On Wed, 26 Aug 2015, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Mon, Aug 17, 2015 at 03:20:38PM +0300, Jani Nikula wrote:
>> On Mon, 17 Aug 2015, libin.yang@intel.com wrote:
>> > From: Libin Yang <libin.yang@intel.com>
>> >
>> > HDMI 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/intel_audio.c | 117 +++++++++++++++++++++++++++++++++++++
>> >  1 file changed, 117 insertions(+)
>> >
>> > diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
>> > index dc32cf4..27add6a 100644
>> > --- a/drivers/gpu/drm/i915/intel_audio.c
>> > +++ b/drivers/gpu/drm/i915/intel_audio.c
>> > @@ -68,6 +68,31 @@ static const struct {
>> >  	{ 148500, AUD_CONFIG_PIXEL_CLOCK_HDMI_148500 },
>> >  };
>> >  
>> > +/* HDMI N/CTS table */
>> > +#define TMDS_297M 297000
>> > +#define TMDS_296M DIV_ROUND_UP(297000 * 1000, 1001)
>> > +static const struct {
>> > +	int sample_rate;
>> > +	int clock;
>> > +	int n;
>> > +	int cts;
>> > +} aud_ncts[] = {
>> > +	{ 44100, TMDS_296M, 4459, 234375 },
>> > +	{ 44100, TMDS_297M, 4704, 247500 },
>> > +	{ 48000, TMDS_296M, 5824, 281250 },
>> > +	{ 48000, TMDS_297M, 5120, 247500 },
>> > +	{ 32000, TMDS_296M, 5824, 421875 },
>> > +	{ 32000, TMDS_297M, 3072, 222750 },
>> > +	{ 88200, TMDS_296M, 8918, 234375 },
>> > +	{ 88200, TMDS_297M, 9408, 247500 },
>> > +	{ 96000, TMDS_296M, 11648, 281250 },
>> > +	{ 96000, TMDS_297M, 10240, 247500 },
>> > +	{ 176400, TMDS_296M, 17836, 234375 },
>> > +	{ 176400, TMDS_297M, 18816, 247500 },
>> > +	{ 44100, TMDS_296M, 23296, 281250 },
>> > +	{ 44100, TMDS_297M, 20480, 247500 },
>> > +};
>> > +
>> >  /* get AUD_CONFIG_PIXEL_CLOCK_HDMI_* value for mode */
>> >  static u32 audio_config_hdmi_pixel_clock(struct drm_display_mode *mode)
>> >  {
>> > @@ -90,6 +115,31 @@ static u32 audio_config_hdmi_pixel_clock(struct drm_display_mode *mode)
>> >  	return hdmi_audio_clock[i].config;
>> >  }
>> >  
>> > +static int audio_config_get_n(struct drm_display_mode *mode, int rate)
>> > +{
>> > +	int i;
>> > +
>> > +	for (i = 0; i < ARRAY_SIZE(aud_ncts); i++) {
>> > +		if ((rate == aud_ncts[i].sample_rate) &&
>> > +			(mode->clock == aud_ncts[i].clock)) {
>> > +			return aud_ncts[i].n;
>> > +		}
>> > +	}
>> > +	return 0;
>> > +}
>> > +
>> > +/* check whether N/CTS/M need be set manually */
>> > +static bool audio_rate_need_prog(struct intel_crtc *crtc,
>> > +					struct drm_display_mode *mode)
>> > +{
>> > +	if (((mode->clock == TMDS_297M) ||
>> > +		 (mode->clock == TMDS_296M)) &&
>> > +		intel_pipe_has_type(crtc, INTEL_OUTPUT_HDMI))
>> > +		return true;
>> > +	else
>> > +		return false;
>> > +}
>> > +
>> >  static bool intel_eld_uptodate(struct drm_connector *connector,
>> >  			       int reg_eldv, uint32_t bits_eldv,
>> >  			       int reg_elda, uint32_t bits_elda,
>> > @@ -514,12 +564,79 @@ static int i915_audio_component_get_cdclk_freq(struct device *dev)
>> >  	return ret;
>> >  }
>> >  
>> > +static int i915_audio_component_sync_audio_rate(struct device *dev,
>> > +						int port, 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 n_low, n_up, n;
>> > +
>> > +	/* 1. get the pipe */
>> > +	for_each_intel_encoder(drm_dev, intel_encoder) {
>> > +		intel_dig_port = enc_to_dig_port(&intel_encoder->base);
>> 
>> Mmmh, not all encoders are digital ports. enc_to_dig_port does a
>> container_of, so you can't just check for NULL either. IOW, you need to
>> check the encoder->type (and you only care about HDMI here).
>
> encoder->type is changed at runtime thanks to ddi encoders. Also shouldn't
> we care about dp audio too or am I confused?

IIUC DP can take care of itself, we need the whole callback here to be
able to program HDMI N/CTS parameters.

BR,
Jani.




> -Daniel
>
>> 
>> > +		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 == INVALID_PIPE) {
>> > +		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. check whether to set the N/CTS/M manually or not */
>> > +	if (!audio_rate_need_prog(crtc, mode)) {
>> > +		tmp = I915_READ(HSW_AUD_CFG(pipe));
>> > +		tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
>> > +		I915_WRITE(HSW_AUD_CFG(pipe), tmp);
>> > +		return 0;
>> > +	}
>> > +
>> > +	n = audio_config_get_n(mode, rate);
>> > +	if (n == 0) {
>> > +		DRM_DEBUG_KMS("Using automatic mode for N value on port %c\n",
>> > +					  port_name(port));
>> > +		tmp = I915_READ(HSW_AUD_CFG(pipe));
>> > +		tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
>> > +		I915_WRITE(HSW_AUD_CFG(pipe), tmp);
>> > +		return 0;
>> > +	}
>> > +	n_low = n & 0xfff;
>> > +	n_up = (n >> 12) & 0xff;
>> > +
>> > +	/* 4. set the N/CTS/M */
>> > +	tmp = I915_READ(HSW_AUD_CFG(pipe));
>> > +	tmp &= ~(AUD_CONFIG_UPPER_N_MASK | AUD_CONFIG_LOWER_N_MASK);
>> > +	tmp |= ((n_up << AUD_CONFIG_UPPER_N_SHIFT) |
>> > +			(n_low << AUD_CONFIG_LOWER_N_SHIFT) |
>> > +			AUD_CONFIG_N_PROG_ENABLE);
>> > +	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,
>> > +	.sync_audio_rate = i915_audio_component_sync_audio_rate,
>> >  };
>> >  
>> >  static int i915_audio_component_bind(struct device *i915_dev,
>> > -- 
>> > 1.9.1
>> >
>> 
>> -- 
>> Jani Nikula, Intel Open Source Technology Center
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx