[RFC PATCH 0/5] DisplayPort Audio on Cherrytrail

[RFC PATCH 0/5] DisplayPort Audio on Cherrytrail

[Pierre-Louis Bossart]

The following patches enable DisplayPort Audio on Cherrytrail machines
when applied on top of Takashi's topic/intel-lpe-audio branch (tested
on Zotac PI330)

There are a couple of opens where I could use some help:
 - is it necessary to set a valid_bit which is used only for DP audio?
 - is the sequence to set the chicken bits and unmute the amplifier ok or
can it be improved by being moved somewhere else in the i915 driver?
 - the register offset to be used by the audio driver depends on a
combination of port/pipe/output type. Do we need to get access to the
pipe information and when is it available (initial trials showed the
pipe is still invalid when the audio notification happens)

Feedback welcome!

Pierre-Louis Bossart (5):
  drm: i915: add DP support in LPE audio mode
  ALSA: x86: intel_hdmi: add definitions and logic for DP audio
  ALSA: x86: intel_hdmi: set config bitfields for DP mode
  drm: i915: add DisplayPort amp unmute for LPE audio mode
  ALSA: x86: hdmi: hack to enable DP audio on CHT

 drivers/gpu/drm/i915/i915_drv.h        |   3 +-
 drivers/gpu/drm/i915/i915_reg.h        |  12 +++
 drivers/gpu/drm/i915/intel_audio.c     |  19 +++-
 drivers/gpu/drm/i915/intel_lpe_audio.c |  34 ++++++-
 include/drm/intel_lpe_audio.h          |   2 +
 sound/x86/intel_hdmi_audio.c           | 174 ++++++++++++++++++++++++++++-----
 sound/x86/intel_hdmi_audio.h           |   8 +-
 sound/x86/intel_hdmi_lpe_audio.c       |  44 ++++++++-
 sound/x86/intel_hdmi_lpe_audio.h       |  29 ++++++
 9 files changed, 288 insertions(+), 37 deletions(-)

-- 
2.7.4

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

[RFC PATCH 1/5] drm: i915: add DP support in LPE audio mode

[Pierre-Louis Bossart]

If DisplayPort is detected, pass flag and link rate to audio driver

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h        |  3 ++-
 drivers/gpu/drm/i915/intel_audio.c     | 19 +++++++++++++++----
 drivers/gpu/drm/i915/intel_lpe_audio.c |  7 ++++++-
 include/drm/intel_lpe_audio.h          |  2 ++
 4 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 3e3102c..836d823 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3401,7 +3401,8 @@ int  intel_lpe_audio_init(struct drm_i915_private *dev_priv);
 void intel_lpe_audio_teardown(struct drm_i915_private *dev_priv);
 void intel_lpe_audio_irq_handler(struct drm_i915_private *dev_priv);
 void intel_lpe_audio_notify(struct drm_i915_private *dev_priv,
-			void *eld, int port, int tmds_clk_speed);
+			    void *eld, int port, int tmds_clk_speed,
+			    bool dp_output, int link_rate);
 
 /* intel_i2c.c */
 extern int intel_setup_gmbus(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
index 364f962..1645ce4 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -631,9 +631,20 @@ void intel_audio_codec_enable(struct intel_encoder *intel_encoder,
 	if (acomp && acomp->audio_ops && acomp->audio_ops->pin_eld_notify)
 		acomp->audio_ops->pin_eld_notify(acomp->audio_ops->audio_ptr,
 						 (int) port, (int) pipe);
-
-	intel_lpe_audio_notify(dev_priv, connector->eld, port,
-			crtc_state->port_clock);
+	switch (intel_encoder->type) {
+	case INTEL_OUTPUT_HDMI:
+		intel_lpe_audio_notify(dev_priv, connector->eld, port,
+				       crtc_state->port_clock,
+				       false, 0);
+		break;
+	case INTEL_OUTPUT_DP:
+		intel_lpe_audio_notify(dev_priv, connector->eld, port,
+				       adjusted_mode->crtc_clock,
+				       true, crtc_state->port_clock);
+		break;
+	default:
+		break;
+	}
 }
 
 /**
@@ -668,7 +679,7 @@ void intel_audio_codec_disable(struct intel_encoder *intel_encoder)
 		acomp->audio_ops->pin_eld_notify(acomp->audio_ops->audio_ptr,
 						 (int) port, (int) pipe);
 
-	intel_lpe_audio_notify(dev_priv, NULL, port, 0);
+	intel_lpe_audio_notify(dev_priv, NULL, port, 0, false, 0);
 }
 
 /**
diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c
index 27d9425..245523e 100644
--- a/drivers/gpu/drm/i915/intel_lpe_audio.c
+++ b/drivers/gpu/drm/i915/intel_lpe_audio.c
@@ -332,7 +332,8 @@ void intel_lpe_audio_teardown(struct drm_i915_private *dev_priv)
  * Notify lpe audio driver of eld change.
  */
 void intel_lpe_audio_notify(struct drm_i915_private *dev_priv,
-			void *eld, int port, int tmds_clk_speed)
+			    void *eld, int port, int tmds_clk_speed,
+			    bool dp_output, int link_rate)
 {
 	unsigned long irq_flags;
 	struct intel_hdmi_lpe_audio_pdata *pdata = NULL;
@@ -351,12 +352,16 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv,
 		pdata->eld.port_id = port;
 		pdata->hdmi_connected = true;
 
+		pdata->dp_output = dp_output;
 		if (tmds_clk_speed)
 			pdata->tmds_clock_speed = tmds_clk_speed;
+		if (link_rate)
+			pdata->link_rate = link_rate;
 	} else {
 		memset(pdata->eld.eld_data, 0,
 			HDMI_MAX_ELD_BYTES);
 		pdata->hdmi_connected = false;
+		pdata->dp_output = false;
 	}
 
 	if (pdata->notify_audio_lpe)
diff --git a/include/drm/intel_lpe_audio.h b/include/drm/intel_lpe_audio.h
index 952de05..857e0ea 100644
--- a/include/drm/intel_lpe_audio.h
+++ b/include/drm/intel_lpe_audio.h
@@ -38,6 +38,8 @@ struct intel_hdmi_lpe_audio_pdata {
 	bool notify_pending;
 	int tmds_clock_speed;
 	bool hdmi_connected;
+	bool dp_output;
+	int link_rate;
 	struct intel_hdmi_lpe_audio_eld eld;
 	void (*notify_audio_lpe)(void *audio_ptr);
 	spinlock_t lpe_audio_slock;
-- 
2.7.4

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

[RFC PATCH 2/5] ALSA: x86: intel_hdmi: add definitions and logic for DP audio

[Pierre-Louis Bossart]

Imported from legacy patches

Note: the new code doesn't assume a modified ELD but
an explicit notification that DP is present. It appears
that the i915 code does change the ELD so we could use
the ELD-based tests to check for DP audio

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/x86/intel_hdmi_audio.c     | 172 +++++++++++++++++++++++++++++++++------
 sound/x86/intel_hdmi_audio.h     |   8 +-
 sound/x86/intel_hdmi_lpe_audio.c |  36 +++++++-
 sound/x86/intel_hdmi_lpe_audio.h |  29 +++++++
 4 files changed, 215 insertions(+), 30 deletions(-)

diff --git a/sound/x86/intel_hdmi_audio.c b/sound/x86/intel_hdmi_audio.c
index f301554..4155b38 100644
--- a/sound/x86/intel_hdmi_audio.c
+++ b/sound/x86/intel_hdmi_audio.c
@@ -548,6 +548,7 @@ void had_build_channel_allocation_map(struct snd_intelhad *intelhaddata)
 	}
 
 	had_get_caps(HAD_GET_ELD, &intelhaddata->eeld);
+	had_get_caps(HAD_GET_DP_OUTPUT, &intelhaddata->dp_output);
 
 	pr_debug("eeld.speaker_allocation_block = %x\n",
 			intelhaddata->eeld.speaker_allocation_block);
@@ -685,7 +686,7 @@ static void snd_intelhad_prog_dip_v1(struct snd_pcm_substream *substream,
 
 	/*Calculte the byte wide checksum for all valid DIP words*/
 	for (i = 0; i < BYTES_PER_WORD; i++)
-		checksum += (INFO_FRAME_WORD1 >> i*BITS_PER_BYTE) & MASK_BYTE0;
+		checksum += (HDMI_INFO_FRAME_WORD1 >> i*BITS_PER_BYTE) & MASK_BYTE0;
 	for (i = 0; i < BYTES_PER_WORD; i++)
 		checksum += (frame2.fr2_val >> i*BITS_PER_BYTE) & MASK_BYTE0;
 	for (i = 0; i < BYTES_PER_WORD; i++)
@@ -693,7 +694,7 @@ static void snd_intelhad_prog_dip_v1(struct snd_pcm_substream *substream,
 
 	frame2.fr2_regx.chksum = -(checksum);
 
-	had_write_register(AUD_HDMIW_INFOFR, INFO_FRAME_WORD1);
+	had_write_register(AUD_HDMIW_INFOFR, HDMI_INFO_FRAME_WORD1);
 	had_write_register(AUD_HDMIW_INFOFR, frame2.fr2_val);
 	had_write_register(AUD_HDMIW_INFOFR, frame3.fr3_val);
 
@@ -722,28 +723,35 @@ static void snd_intelhad_prog_dip_v2(struct snd_pcm_substream *substream,
 	union aud_info_frame2 frame2 = {.fr2_val = 0};
 	union aud_info_frame3 frame3 = {.fr3_val = 0};
 	u8 checksum = 0;
+	u32 info_frame;
 	int channels;
 
 	channels = substream->runtime->channels;
 
 	had_write_register(AUD_CNTL_ST, ctrl_state.ctrl_val);
 
-	frame2.fr2_regx.chnl_cnt = substream->runtime->channels - 1;
+	if (intelhaddata->dp_output) {
+		info_frame = DP_INFO_FRAME_WORD1;
+		frame2.fr2_val = 1;
+	} else {
+		info_frame = HDMI_INFO_FRAME_WORD1;
+		frame2.fr2_regx.chnl_cnt = substream->runtime->channels - 1;
 
-	frame3.fr3_regx.chnl_alloc = snd_intelhad_channel_allocation(
-					intelhaddata, channels);
+		frame3.fr3_regx.chnl_alloc = snd_intelhad_channel_allocation(
+			intelhaddata, channels);
 
-	/*Calculte the byte wide checksum for all valid DIP words*/
-	for (i = 0; i < BYTES_PER_WORD; i++)
-		checksum += (INFO_FRAME_WORD1 >> i*BITS_PER_BYTE) & MASK_BYTE0;
-	for (i = 0; i < BYTES_PER_WORD; i++)
-		checksum += (frame2.fr2_val >> i*BITS_PER_BYTE) & MASK_BYTE0;
-	for (i = 0; i < BYTES_PER_WORD; i++)
-		checksum += (frame3.fr3_val >> i*BITS_PER_BYTE) & MASK_BYTE0;
+		/*Calculte the byte wide checksum for all valid DIP words*/
+		for (i = 0; i < BYTES_PER_WORD; i++)
+			checksum += (info_frame >> i*BITS_PER_BYTE) & MASK_BYTE0;
+		for (i = 0; i < BYTES_PER_WORD; i++)
+			checksum += (frame2.fr2_val >> i*BITS_PER_BYTE) & MASK_BYTE0;
+		for (i = 0; i < BYTES_PER_WORD; i++)
+			checksum += (frame3.fr3_val >> i*BITS_PER_BYTE) & MASK_BYTE0;
 
-	frame2.fr2_regx.chksum = -(checksum);
+		frame2.fr2_regx.chksum = -(checksum);
+	}
 
-	had_write_register(AUD_HDMIW_INFOFR_v2, INFO_FRAME_WORD1);
+	had_write_register(AUD_HDMIW_INFOFR_v2, info_frame);
 	had_write_register(AUD_HDMIW_INFOFR_v2, frame2.fr2_val);
 	had_write_register(AUD_HDMIW_INFOFR_v2, frame3.fr3_val);
 
@@ -839,6 +847,85 @@ int snd_intelhad_read_len(struct snd_intelhad *intelhaddata)
 	return retval;
 }
 
+static int had_calculate_maud_value(u32 aud_samp_freq, u32 link_rate)
+{
+	u32 maud_val;
+
+	/* Select maud according to DP 1.2 spec*/
+	if (link_rate == DP_2_7_GHZ) {
+		switch (aud_samp_freq) {
+		case AUD_SAMPLE_RATE_32:
+			maud_val = AUD_SAMPLE_RATE_32_DP_2_7_MAUD_VAL;
+			break;
+
+		case AUD_SAMPLE_RATE_44_1:
+			maud_val = AUD_SAMPLE_RATE_44_1_DP_2_7_MAUD_VAL;
+			break;
+
+		case AUD_SAMPLE_RATE_48:
+			maud_val = AUD_SAMPLE_RATE_48_DP_2_7_MAUD_VAL;
+			break;
+
+		case AUD_SAMPLE_RATE_88_2:
+			maud_val = AUD_SAMPLE_RATE_88_2_DP_2_7_MAUD_VAL;
+			break;
+
+		case AUD_SAMPLE_RATE_96:
+			maud_val = AUD_SAMPLE_RATE_96_DP_2_7_MAUD_VAL;
+			break;
+
+		case AUD_SAMPLE_RATE_176_4:
+			maud_val = AUD_SAMPLE_RATE_176_4_DP_2_7_MAUD_VAL;
+			break;
+
+		case HAD_MAX_RATE:
+			maud_val = HAD_MAX_RATE_DP_2_7_MAUD_VAL;
+			break;
+
+		default:
+			maud_val = -EINVAL;
+			break;
+		}
+	} else if (link_rate == DP_1_62_GHZ) {
+		switch (aud_samp_freq) {
+		case AUD_SAMPLE_RATE_32:
+			maud_val = AUD_SAMPLE_RATE_32_DP_1_62_MAUD_VAL;
+			break;
+
+		case AUD_SAMPLE_RATE_44_1:
+			maud_val = AUD_SAMPLE_RATE_44_1_DP_1_62_MAUD_VAL;
+			break;
+
+		case AUD_SAMPLE_RATE_48:
+			maud_val = AUD_SAMPLE_RATE_48_DP_1_62_MAUD_VAL;
+			break;
+
+		case AUD_SAMPLE_RATE_88_2:
+			maud_val = AUD_SAMPLE_RATE_88_2_DP_1_62_MAUD_VAL;
+			break;
+
+		case AUD_SAMPLE_RATE_96:
+			maud_val = AUD_SAMPLE_RATE_96_DP_1_62_MAUD_VAL;
+			break;
+
+		case AUD_SAMPLE_RATE_176_4:
+			maud_val = AUD_SAMPLE_RATE_176_4_DP_1_62_MAUD_VAL;
+			break;
+
+		case HAD_MAX_RATE:
+			maud_val = HAD_MAX_RATE_DP_1_62_MAUD_VAL;
+			break;
+
+		default:
+			maud_val = -EINVAL;
+			break;
+		}
+	} else
+		maud_val = -EINVAL;
+
+	return maud_val;
+}
+
 /**
  * snd_intelhad_prog_cts_v1 - Program HDMI audio CTS value
  *
@@ -849,8 +936,9 @@ int snd_intelhad_read_len(struct snd_intelhad *intelhaddata)
  *
  * Program CTS register based on the audio and display sampling frequency
  */
-static void snd_intelhad_prog_cts_v1(u32 aud_samp_freq, u32 tmds, u32 n_param,
-				struct snd_intelhad *intelhaddata)
+static void snd_intelhad_prog_cts_v1(u32 aud_samp_freq, u32 tmds,
+				     u32 link_rate, u32 n_param,
+				     struct snd_intelhad *intelhaddata)
 {
 	u32 cts_val;
 	u64 dividend, divisor;
@@ -874,18 +962,24 @@ static void snd_intelhad_prog_cts_v1(u32 aud_samp_freq, u32 tmds, u32 n_param,
  *
  * Program CTS register based on the audio and display sampling frequency
  */
-static void snd_intelhad_prog_cts_v2(u32 aud_samp_freq, u32 tmds, u32 n_param,
-				struct snd_intelhad *intelhaddata)
+static void snd_intelhad_prog_cts_v2(u32 aud_samp_freq, u32 tmds,
+				     u32 link_rate, u32 n_param,
+				     struct snd_intelhad *intelhaddata)
 {
 	u32 cts_val;
 	u64 dividend, divisor;
 
-	/* Calculate CTS according to HDMI 1.3a spec*/
-	dividend = (u64)tmds * n_param*1000;
-	divisor = 128 * aud_samp_freq;
-	cts_val = div64_u64(dividend, divisor);
+	if (intelhaddata->dp_output) {
+		/* Substitute cts_val with Maud according to DP 1.2 spec*/
+		cts_val = had_calculate_maud_value(aud_samp_freq, link_rate);
+	} else {
+		/* Calculate CTS according to HDMI 1.3a spec*/
+		dividend = (u64)tmds * n_param*1000;
+		divisor = 128 * aud_samp_freq;
+		cts_val = div64_u64(dividend, divisor);
+	}
 	pr_debug("TMDS value=%d, N value=%d, CTS Value=%d\n",
-			tmds, n_param, cts_val);
+		 tmds, n_param, cts_val);
 	had_write_register(AUD_HDMI_CTS, (BIT(24) | cts_val));
 }
 
@@ -970,7 +1064,19 @@ static int snd_intelhad_prog_n_v2(u32 aud_samp_freq, u32 *n_param,
 {
 	s32 n_val;
 
-	n_val =	had_calculate_n_value(aud_samp_freq);
+	if (intelhaddata->dp_output) {
+		/*
+		 * According to DP specs, Maud and Naud values hold
+		 * a relationship, which is stated as:
+		 * Maud/Naud = 512 * fs / f_LS_Clk
+		 * where, fs is the sampling frequency of the audio stream
+		 * and Naud is 32768 for Async clock.
+		*/
+
+		n_val = DP_NAUD_VAL;
+	}
+	else
+		n_val =	had_calculate_n_value(aud_samp_freq);
 
 	if (n_val < 0)
 		return n_val;
@@ -1343,6 +1449,7 @@ static int snd_intelhad_pcm_prepare(struct snd_pcm_substream *substream)
 {
 	int retval;
 	u32 disp_samp_freq, n_param;
+	u32 link_rate = 0;
 	struct snd_intelhad *intelhaddata;
 	struct snd_pcm_runtime *runtime;
 	struct had_pvt_data *had_stream;
@@ -1387,6 +1494,7 @@ static int snd_intelhad_pcm_prepare(struct snd_pcm_substream *substream)
 	}
 
 	had_get_caps(HAD_GET_ELD, &intelhaddata->eeld);
+	had_get_caps(HAD_GET_DP_OUTPUT, &intelhaddata->dp_output);
 
 	retval = intelhaddata->ops->prog_n(substream->runtime->rate, &n_param,
 								intelhaddata);
@@ -1394,8 +1502,14 @@ static int snd_intelhad_pcm_prepare(struct snd_pcm_substream *substream)
 		pr_err("programming N value failed %#x\n", retval);
 		goto prep_end;
 	}
+
+	if (intelhaddata->dp_output)
+		had_get_caps(HAD_GET_LINK_RATE, &link_rate);
+
+
 	intelhaddata->ops->prog_cts(substream->runtime->rate,
-					disp_samp_freq, n_param, intelhaddata);
+				    disp_samp_freq, link_rate,
+				    n_param, intelhaddata);
 
 	intelhaddata->ops->prog_dip(substream, intelhaddata);
 
@@ -1503,6 +1617,7 @@ int hdmi_audio_mode_change(struct snd_pcm_substream *substream)
 {
 	int retval = 0;
 	u32 disp_samp_freq, n_param;
+	u32 link_rate = 0;
 	struct snd_intelhad *intelhaddata;
 
 	intelhaddata = snd_pcm_substream_chip(substream);
@@ -1523,8 +1638,13 @@ int hdmi_audio_mode_change(struct snd_pcm_substream *substream)
 		pr_err("programming N value failed %#x\n", retval);
 		goto out;
 	}
+
+	if (intelhaddata->dp_output)
+		had_get_caps(HAD_GET_LINK_RATE, &link_rate);
+
 	intelhaddata->ops->prog_cts(substream->runtime->rate,
-					disp_samp_freq, n_param, intelhaddata);
+				    disp_samp_freq, link_rate,
+				    n_param, intelhaddata);
 
 	/* Enable Audio */
 	intelhaddata->ops->enable_audio(substream, 1);
diff --git a/sound/x86/intel_hdmi_audio.h b/sound/x86/intel_hdmi_audio.h
index d2015ec..034b387 100644
--- a/sound/x86/intel_hdmi_audio.h
+++ b/sound/x86/intel_hdmi_audio.h
@@ -44,7 +44,8 @@
 #define MAX_CAP_STREAMS		0
 #define HDMI_AUDIO_DRIVER	"hdmi-audio"
 
-#define INFO_FRAME_WORD1	0x000a0184
+#define HDMI_INFO_FRAME_WORD1	0x000a0184
+#define DP_INFO_FRAME_WORD1	0x00441b84
 #define FIFO_THRESHOLD		0xFE
 #define DMA_FIFO_THRESHOLD	0x7
 #define BYTES_PER_WORD		0x4
@@ -134,6 +135,7 @@ struct snd_intelhad {
 	struct		ring_buf_info buf_info[HAD_NUM_OF_RING_BUFS];
 	struct		pcm_stream_info stream_info;
 	union otm_hdmi_eld_t	eeld;
+	bool dp_output;
 	enum		intel_had_aud_buf_type curr_buf;
 	int		valid_buf_cnt;
 	unsigned int	aes_bits;
@@ -156,8 +158,8 @@ struct had_ops {
 	void (*reset_audio)(u8 reset);
 	int (*prog_n)(u32 aud_samp_freq, u32 *n_param,
 			struct snd_intelhad *intelhaddata);
-	void (*prog_cts)(u32 aud_samp_freq, u32 tmds, u32 n_param,
-			struct snd_intelhad *intelhaddata);
+	void (*prog_cts)(u32 aud_samp_freq, u32 tmds, u32 link_rate,
+			 u32 n_param, struct snd_intelhad *intelhaddata);
 	int (*audio_ctrl)(struct snd_pcm_substream *substream,
 				struct snd_intelhad *intelhaddata);
 	void (*prog_dip)(struct snd_pcm_substream *substream,
diff --git a/sound/x86/intel_hdmi_lpe_audio.c b/sound/x86/intel_hdmi_lpe_audio.c
index ead2d3a..6ef0ff8 100644
--- a/sound/x86/intel_hdmi_lpe_audio.c
+++ b/sound/x86/intel_hdmi_lpe_audio.c
@@ -48,6 +48,8 @@ struct hdmi_lpe_audio_ctx {
 	struct snd_intel_had_interface *had_interface;
 	void *had_pvt_data;
 	int tmds_clock_speed;
+	bool dp_output;
+	int link_rate;
 	unsigned int had_config_offset;
 	int hdmi_audio_interrupt_mask;
 	struct work_struct hdmi_audio_wq;
@@ -187,6 +189,15 @@ static int hdmi_audio_write(u32 reg, u32 val)
 
 	dev_dbg(&hlpe_pdev->dev, "%s: reg[0x%x] = 0x%x\n", __func__, reg, val);
 
+	if (ctx->dp_output) {
+		if ((reg == AUDIO_HDMI_CONFIG_A ) ||
+		    (reg == AUDIO_HDMI_CONFIG_B ) ||
+		    (reg == AUDIO_HDMI_CONFIG_C )) {
+			if (val & AUD_CONFIG_VALID_BIT)
+				val = val | AUD_CONFIG_DP_MODE |
+					AUD_CONFIG_BLOCK_BIT;
+		}
+	}
 	iowrite32(val, (ctx->mmio_start+reg));
 
 	return 0;
@@ -220,6 +231,16 @@ static int hdmi_audio_rmw(u32 reg, u32 val, u32 mask)
 	val_tmp = (val & mask) |
 			((ioread32(ctx->mmio_start + reg)) & ~mask);
 
+	if (ctx->dp_output) {
+		if ((reg == AUDIO_HDMI_CONFIG_A ) ||
+		    (reg == AUDIO_HDMI_CONFIG_B ) ||
+		    (reg == AUDIO_HDMI_CONFIG_C )) {
+			if (val_tmp & AUD_CONFIG_VALID_BIT)
+				val_tmp = val_tmp | AUD_CONFIG_DP_MODE |
+					AUD_CONFIG_BLOCK_BIT;
+		}
+	}
+
 	iowrite32(val_tmp, (ctx->mmio_start+reg));
 	dev_dbg(&hlpe_pdev->dev, "%s: reg[0x%x] = 0x%x\n", __func__,
 				reg, val_tmp);
@@ -249,7 +270,18 @@ static int hdmi_audio_get_caps(enum had_caps_list get_element,
 		/* ToDo: Verify if sampling freq logic is correct */
 		*(u32 *)capabilities = ctx->tmds_clock_speed;
 		dev_dbg(&hlpe_pdev->dev, "%s: tmds_clock_speed = 0x%x\n",
-				__func__, ctx->tmds_clock_speed);
+			__func__, ctx->tmds_clock_speed);
+		break;
+	case HAD_GET_LINK_RATE:
+		/* ToDo: Verify if sampling freq logic is correct */
+		*(u32 *)capabilities = ctx->link_rate;
+		dev_dbg(&hlpe_pdev->dev, "%s: link rate = 0x%x\n",
+			__func__, ctx->link_rate);
+		break;
+	case HAD_GET_DP_OUTPUT:
+		*(u32 *)capabilities = ctx->dp_output;
+		dev_dbg(&hlpe_pdev->dev, "%s: dp_output = %d\n",
+			__func__, ctx->dp_output);
 		break;
 	default:
 		break;
@@ -442,6 +474,8 @@ static void notify_audio_lpe(void *audio_ptr)
 
 		if (pdata->tmds_clock_speed) {
 			ctx->tmds_clock_speed = pdata->tmds_clock_speed;
+			ctx->dp_output = pdata->dp_output;
+			ctx->link_rate = pdata->link_rate;
 			mid_hdmi_audio_signal_event(HAD_EVENT_MODE_CHANGING);
 		}
 	} else
diff --git a/sound/x86/intel_hdmi_lpe_audio.h b/sound/x86/intel_hdmi_lpe_audio.h
index ec4bde5..3aed89a 100644
--- a/sound/x86/intel_hdmi_lpe_audio.h
+++ b/sound/x86/intel_hdmi_lpe_audio.h
@@ -31,6 +31,10 @@
 #include <sound/control.h>
 #include <sound/pcm.h>
 
+#define AUD_CONFIG_VALID_BIT			(1<<9)
+#define AUD_CONFIG_DP_MODE			(1<<15)
+#define AUD_CONFIG_BLOCK_BIT			(1<<7)
+
 #define HMDI_LPE_AUDIO_DRIVER_NAME		"intel-hdmi-lpe-audio"
 #define HAD_MAX_DEVICES		1
 #define HAD_MIN_CHANNEL		2
@@ -68,6 +72,29 @@
 #define HAD_MAX_DIP_WORDS		16
 #define INTEL_HAD		"IntelHdmiLpeAudio"
 
+/* DP Link Rates */
+#define DP_2_7_GHZ			270000
+#define DP_1_62_GHZ			162000
+
+/* Maud Values */
+#define AUD_SAMPLE_RATE_32_DP_2_7_MAUD_VAL		1988
+#define AUD_SAMPLE_RATE_44_1_DP_2_7_MAUD_VAL		2740
+#define AUD_SAMPLE_RATE_48_DP_2_7_MAUD_VAL		2982
+#define AUD_SAMPLE_RATE_88_2_DP_2_7_MAUD_VAL		5480
+#define AUD_SAMPLE_RATE_96_DP_2_7_MAUD_VAL		5965
+#define AUD_SAMPLE_RATE_176_4_DP_2_7_MAUD_VAL		10961
+#define HAD_MAX_RATE_DP_2_7_MAUD_VAL			11930
+#define AUD_SAMPLE_RATE_32_DP_1_62_MAUD_VAL		3314
+#define AUD_SAMPLE_RATE_44_1_DP_1_62_MAUD_VAL		4567
+#define AUD_SAMPLE_RATE_48_DP_1_62_MAUD_VAL		4971
+#define AUD_SAMPLE_RATE_88_2_DP_1_62_MAUD_VAL		9134
+#define AUD_SAMPLE_RATE_96_DP_1_62_MAUD_VAL		9942
+#define AUD_SAMPLE_RATE_176_4_DP_1_62_MAUD_VAL		18268
+#define HAD_MAX_RATE_DP_1_62_MAUD_VAL			19884
+
+/* Naud Value */
+#define DP_NAUD_VAL					32768
+
 /* _AUD_CONFIG register MASK */
 #define AUD_CONFIG_MASK_UNDERRUN	0xC0000000
 #define AUD_CONFIG_MASK_SRDBG		0x00000002
@@ -618,6 +645,8 @@ enum hdmi_connector_status {
 enum had_caps_list {
 	HAD_GET_ELD = 1,
 	HAD_GET_DISPLAY_RATE,
+	HAD_GET_DP_OUTPUT,
+	HAD_GET_LINK_RATE,
 	HAD_SET_ENABLE_AUDIO,
 	HAD_SET_DISABLE_AUDIO,
 	HAD_SET_ENABLE_AUDIO_INT,
-- 
2.7.4

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

[RFC PATCH 3/5] ALSA: x86: intel_hdmi: set config bitfields for DP mode

[Pierre-Louis Bossart]

These bits were set in legacy and not in upstream code, and are
apparently tested for when writing a config in DP mode

FIXME: is this even needed?

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/x86/intel_hdmi_audio.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/sound/x86/intel_hdmi_audio.c b/sound/x86/intel_hdmi_audio.c
index 4155b38..768e6e3 100644
--- a/sound/x86/intel_hdmi_audio.c
+++ b/sound/x86/intel_hdmi_audio.c
@@ -396,6 +396,7 @@ static int snd_intelhad_prog_audio_ctrl_v2(struct snd_pcm_substream *substream,
 	else
 		cfg_val.cfg_regx_v2.layout = LAYOUT1;
 
+	cfg_val.cfg_regx_v2.val_bit = 1;
 	had_write_register(AUD_CONFIG, cfg_val.cfg_regval);
 	return 0;
 }
@@ -447,6 +448,7 @@ static int snd_intelhad_prog_audio_ctrl_v1(struct snd_pcm_substream *substream,
 
 	}
 
+	cfg_val.cfg_regx.val_bit = 1;
 	had_write_register(AUD_CONFIG, cfg_val.cfg_regval);
 	return 0;
 }
-- 
2.7.4

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

[RFC PATCH 4/5] drm: i915: add DisplayPort amp unmute for LPE audio mode

[Pierre-Louis Bossart]

Enable chicken bit on LPE mode setup and unmute amp on
notification

FIXME: should these two phases done somewhere else?

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h        | 12 ++++++++++++
 drivers/gpu/drm/i915/intel_lpe_audio.c | 27 +++++++++++++++++++++++++++
 2 files changed, 39 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index a9ffc8d..ee90f64 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2061,6 +2061,18 @@ enum skl_disp_power_wells {
 #define I915_HDMI_LPE_AUDIO_BASE	(VLV_DISPLAY_BASE + 0x65000)
 #define I915_HDMI_LPE_AUDIO_SIZE	0x1000
 
+/* DisplayPort Audio w/ LPE */
+#define CHICKEN_BIT_DBG_ENABLE		(1 << 0)
+#define AMP_UNMUTE		        (1 << 1)
+#define AUD_CHICKEN_BIT_REG		0x62F38
+#define AUD_PORT_EN_B_DBG		0x62F20
+#define AUD_PORT_EN_C_DBG		0x62F28
+#define AUD_PORT_EN_D_DBG		0x62F2C
+#define VLV_AUD_CHICKEN_BIT_REG		_MMIO(VLV_DISPLAY_BASE + AUD_CHICKEN_BIT_REG)
+#define VLV_AUD_PORT_EN_B_DBG		_MMIO(VLV_DISPLAY_BASE + AUD_PORT_EN_B_DBG)
+#define VLV_AUD_PORT_EN_C_DBG		_MMIO(VLV_DISPLAY_BASE + AUD_PORT_EN_C_DBG)
+#define VLV_AUD_PORT_EN_D_DBG		_MMIO(VLV_DISPLAY_BASE + AUD_PORT_EN_D_DBG)
+
 #define GEN6_BSD_RNCID			_MMIO(0x12198)
 
 #define GEN7_FF_THREAD_MODE		_MMIO(0x20a0)
diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c
index 245523e..b3134ef 100644
--- a/drivers/gpu/drm/i915/intel_lpe_audio.c
+++ b/drivers/gpu/drm/i915/intel_lpe_audio.c
@@ -248,6 +248,15 @@ static int lpe_audio_setup(struct drm_i915_private *dev_priv)
 		goto err_free_irq;
 	}
 
+	/* Enable DPAudio debug bits by default */
+	if (IS_CHERRYVIEW(dev_priv)) {
+		u32 chicken_bit;
+
+		chicken_bit = I915_READ(VLV_AUD_CHICKEN_BIT_REG);
+		I915_WRITE(VLV_AUD_CHICKEN_BIT_REG,
+			   chicken_bit | CHICKEN_BIT_DBG_ENABLE);
+	}
+
 	return 0;
 err_free_irq:
 	irq_free_desc(dev_priv->lpe_audio.irq);
@@ -357,6 +366,24 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv,
 			pdata->tmds_clock_speed = tmds_clk_speed;
 		if (link_rate)
 			pdata->link_rate = link_rate;
+
+		if (dp_output) { /* unmute the amp */
+			u32 audio_enable;
+
+			if (port == PORT_B) {
+				audio_enable = I915_READ(VLV_AUD_PORT_EN_B_DBG);
+				I915_WRITE(VLV_AUD_PORT_EN_B_DBG,
+					   audio_enable & ~AMP_UNMUTE);
+			} else if (port == PORT_C) {
+				audio_enable = I915_READ(VLV_AUD_PORT_EN_C_DBG);
+				I915_WRITE(VLV_AUD_PORT_EN_C_DBG,
+					   audio_enable & ~AMP_UNMUTE);
+			} else if (port == PORT_D) {
+				audio_enable = I915_READ(VLV_AUD_PORT_EN_D_DBG);
+				I915_WRITE(VLV_AUD_PORT_EN_D_DBG,
+					   audio_enable & ~AMP_UNMUTE);
+			}
+		}
 	} else {
 		memset(pdata->eld.eld_data, 0,
 			HDMI_MAX_ELD_BYTES);
-- 
2.7.4

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

[RFC PATCH 5/5] ALSA: x86: hdmi: hack to enable DP audio on CHT

[Pierre-Louis Bossart]

Somehow the configuration register that the audio driver needs to use
depends on the port/pipe combination.
Adding the pipe information to the notify() call isn't helping, the
pipe value is -1 (illegal).

FIXME: does this require a change in the pdata or a handshake with i915?
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/x86/intel_hdmi_lpe_audio.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/sound/x86/intel_hdmi_lpe_audio.c b/sound/x86/intel_hdmi_lpe_audio.c
index 6ef0ff8..23e5b34 100644
--- a/sound/x86/intel_hdmi_lpe_audio.c
+++ b/sound/x86/intel_hdmi_lpe_audio.c
@@ -563,7 +563,13 @@ static int hdmi_lpe_audio_probe(struct platform_device *pdev)
 	if (pci_dev_present(cherryview_ids)) {
 		dev_dbg(&hlpe_pdev->dev, "%s: Cherrytrail LPE - Detected\n",
 				__func__);
-		ctx->had_config_offset = AUDIO_HDMI_CONFIG_C;
+		//ctx->had_config_offset = AUDIO_HDMI_CONFIG_C;
+		/* FIXME: hard-coding to CONFIG_A enables DP audio on CHT,
+		 *  how do I find out which config to use ?
+		 * the pipe is -1 (invalid) when the notify function is called,
+		 * so not sure how to go about this
+		 */
+		ctx->had_config_offset = AUDIO_HDMI_CONFIG_A;
 	} else {
 		dev_dbg(&hlpe_pdev->dev, "%s: Baytrail LPE - Assume\n",
 				__func__);
-- 
2.7.4

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

✗ Fi.CI.BAT: failure for DisplayPort Audio on Cherrytrail

[Patchwork]

== Series Details ==

Series: DisplayPort Audio on Cherrytrail
URL   : https://patchwork.freedesktop.org/series/18639/
State : failure

== Summary ==

Series 18639v1 DisplayPort Audio on Cherrytrail
https://patchwork.freedesktop.org/api/1.0/series/18639/revisions/1/mbox/

Test gem_close_race:
        Subgroup basic-threads:
                incomplete -> PASS       (fi-bxt-j4205)
Test gem_exec_reloc:
        Subgroup basic-gtt:
                pass       -> INCOMPLETE (fi-byt-j1900)
Test kms_pipe_crc_basic:
        Subgroup read-crc-pipe-b:
                dmesg-warn -> PASS       (fi-skl-6700k)
        Subgroup suspend-read-crc-pipe-c:
                pass       -> DMESG-WARN (fi-skl-6700hq)

fi-bdw-5557u     total:247  pass:233  dwarn:0   dfail:0   fail:0   skip:14 
fi-bsw-n3050     total:247  pass:208  dwarn:0   dfail:0   fail:0   skip:39 
fi-bxt-j4205     total:247  pass:225  dwarn:0   dfail:0   fail:0   skip:22 
fi-bxt-t5700     total:79   pass:66   dwarn:0   dfail:0   fail:0   skip:12 
fi-byt-j1900     total:67   pass:57   dwarn:0   dfail:0   fail:0   skip:9  
fi-byt-n2820     total:247  pass:216  dwarn:0   dfail:0   fail:0   skip:31 
fi-hsw-4770      total:247  pass:228  dwarn:0   dfail:0   fail:0   skip:19 
fi-hsw-4770r     total:247  pass:228  dwarn:0   dfail:0   fail:0   skip:19 
fi-ivb-3520m     total:247  pass:226  dwarn:0   dfail:0   fail:0   skip:21 
fi-ivb-3770      total:247  pass:226  dwarn:0   dfail:0   fail:0   skip:21 
fi-kbl-7500u     total:247  pass:226  dwarn:0   dfail:0   fail:0   skip:21 
fi-skl-6260u     total:247  pass:234  dwarn:0   dfail:0   fail:0   skip:13 
fi-skl-6700hq    total:247  pass:226  dwarn:1   dfail:0   fail:0   skip:20 
fi-skl-6700k     total:247  pass:222  dwarn:4   dfail:0   fail:0   skip:21 
fi-snb-2520m     total:247  pass:216  dwarn:0   dfail:0   fail:0   skip:31 
fi-snb-2600      total:247  pass:215  dwarn:0   dfail:0   fail:0   skip:32 

48cf644b4781ce8345c7506b719601c563982802 drm-tip: 2017y-01m-26d-18h-59m-50s UTC integration manifest
d221f49 ALSA: x86: hdmi: hack to enable DP audio on CHT
f3f0075 drm: i915: add DisplayPort amp unmute for LPE audio mode
90a15d25 ALSA: x86: intel_hdmi: set config bitfields for DP mode
d079636 ALSA: x86: intel_hdmi: add definitions and logic for DP audio
7fa2fcf drm: i915: add DP support in LPE audio mode

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3616/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC PATCH 4/5] drm: i915: add DisplayPort amp unmute for LPE audio mode

[Jani Nikula]

On Thu, 26 Jan 2017, Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> wrote:
> Enable chicken bit on LPE mode setup and unmute amp on
> notification
>
> FIXME: should these two phases done somewhere else?
>
> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h        | 12 ++++++++++++
>  drivers/gpu/drm/i915/intel_lpe_audio.c | 27 +++++++++++++++++++++++++++
>  2 files changed, 39 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index a9ffc8d..ee90f64 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2061,6 +2061,18 @@ enum skl_disp_power_wells {
>  #define I915_HDMI_LPE_AUDIO_BASE	(VLV_DISPLAY_BASE + 0x65000)
>  #define I915_HDMI_LPE_AUDIO_SIZE	0x1000
>  
> +/* DisplayPort Audio w/ LPE */
> +#define CHICKEN_BIT_DBG_ENABLE		(1 << 0)
> +#define AMP_UNMUTE		        (1 << 1)

The convention is to define registers first and the contents/bits for
each register immedialy below. For groups of registers (like
PORT_EN_B/C/D below) define all registers first and bits immediately
below. (But note that the chicken register is not part of the group.)

> +#define AUD_CHICKEN_BIT_REG		0x62F38
> +#define AUD_PORT_EN_B_DBG		0x62F20
> +#define AUD_PORT_EN_C_DBG		0x62F28
> +#define AUD_PORT_EN_D_DBG		0x62F2C

Please don't define these separately without the display base, use
(VLV_DISPLAY_BASE + 0x62f38) style instead. All the other uses of
VLV_DISPLAY_BASE in the file follow the same convention.

> +#define VLV_AUD_CHICKEN_BIT_REG		_MMIO(VLV_DISPLAY_BASE + AUD_CHICKEN_BIT_REG)
> +#define VLV_AUD_PORT_EN_B_DBG		_MMIO(VLV_DISPLAY_BASE + AUD_PORT_EN_B_DBG)
> +#define VLV_AUD_PORT_EN_C_DBG		_MMIO(VLV_DISPLAY_BASE + AUD_PORT_EN_C_DBG)
> +#define VLV_AUD_PORT_EN_D_DBG		_MMIO(VLV_DISPLAY_BASE + AUD_PORT_EN_D_DBG)
> +

Would be nice to have a macro VLV_AUD_PORT_EN_DBG(port), but damn those
reg offsets don't make it easy...


>  #define GEN6_BSD_RNCID			_MMIO(0x12198)
>  
>  #define GEN7_FF_THREAD_MODE		_MMIO(0x20a0)
> diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c
> index 245523e..b3134ef 100644
> --- a/drivers/gpu/drm/i915/intel_lpe_audio.c
> +++ b/drivers/gpu/drm/i915/intel_lpe_audio.c
> @@ -248,6 +248,15 @@ static int lpe_audio_setup(struct drm_i915_private *dev_priv)
>  		goto err_free_irq;
>  	}
>  
> +	/* Enable DPAudio debug bits by default */
> +	if (IS_CHERRYVIEW(dev_priv)) {
> +		u32 chicken_bit;
> +
> +		chicken_bit = I915_READ(VLV_AUD_CHICKEN_BIT_REG);
> +		I915_WRITE(VLV_AUD_CHICKEN_BIT_REG,
> +			   chicken_bit | CHICKEN_BIT_DBG_ENABLE);
> +	}
> +
>  	return 0;
>  err_free_irq:
>  	irq_free_desc(dev_priv->lpe_audio.irq);
> @@ -357,6 +366,24 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv,
>  			pdata->tmds_clock_speed = tmds_clk_speed;
>  		if (link_rate)
>  			pdata->link_rate = link_rate;
> +
> +		if (dp_output) { /* unmute the amp */
> +			u32 audio_enable;
> +
> +			if (port == PORT_B) {
> +				audio_enable = I915_READ(VLV_AUD_PORT_EN_B_DBG);
> +				I915_WRITE(VLV_AUD_PORT_EN_B_DBG,
> +					   audio_enable & ~AMP_UNMUTE);
> +			} else if (port == PORT_C) {
> +				audio_enable = I915_READ(VLV_AUD_PORT_EN_C_DBG);
> +				I915_WRITE(VLV_AUD_PORT_EN_C_DBG,
> +					   audio_enable & ~AMP_UNMUTE);
> +			} else if (port == PORT_D) {
> +				audio_enable = I915_READ(VLV_AUD_PORT_EN_D_DBG);
> +				I915_WRITE(VLV_AUD_PORT_EN_D_DBG,
> +					   audio_enable & ~AMP_UNMUTE);
> +			}
> +		}
>  	} else {
>  		memset(pdata->eld.eld_data, 0,
>  			HDMI_MAX_ELD_BYTES);

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

Re: [RFC PATCH 0/5] DisplayPort Audio on Cherrytrail

[Takashi Iwai]

On Thu, 26 Jan 2017 21:05:36 +0100,
Pierre-Louis Bossart wrote:
> 
> The following patches enable DisplayPort Audio on Cherrytrail machines
> when applied on top of Takashi's topic/intel-lpe-audio branch (tested
> on Zotac PI330)
> 
> There are a couple of opens where I could use some help:
>  - is it necessary to set a valid_bit which is used only for DP audio?
>  - is the sequence to set the chicken bits and unmute the amplifier ok or
> can it be improved by being moved somewhere else in the i915 driver?
>  - the register offset to be used by the audio driver depends on a
> combination of port/pipe/output type. Do we need to get access to the
> pipe information and when is it available (initial trials showed the
> pipe is still invalid when the audio notification happens)
> 
> Feedback welcome!

This makes indeed the audio on DP1 working on Dell Wyse 3040.
But the output on DP3 is still broken.  Now the transfer stalls.
No wonder, as you changed the config base to A from C.

I could make the config base changing per pipe, and now the stream
flows on DP3, too.  Alas, still no audio out.

I'll send out my patchset on top of yours.


thanks,

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

Re: [RFC PATCH 4/5] drm: i915: add DisplayPort amp unmute for LPE audio mode

[Ville Syrjälä]

On Fri, Jan 27, 2017 at 12:08:58PM +0200, Jani Nikula wrote:
> On Thu, 26 Jan 2017, Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> wrote:
> > Enable chicken bit on LPE mode setup and unmute amp on
> > notification
> >
> > FIXME: should these two phases done somewhere else?
> >
> > Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h        | 12 ++++++++++++
> >  drivers/gpu/drm/i915/intel_lpe_audio.c | 27 +++++++++++++++++++++++++++
> >  2 files changed, 39 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index a9ffc8d..ee90f64 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -2061,6 +2061,18 @@ enum skl_disp_power_wells {
> >  #define I915_HDMI_LPE_AUDIO_BASE	(VLV_DISPLAY_BASE + 0x65000)
> >  #define I915_HDMI_LPE_AUDIO_SIZE	0x1000
> >  
> > +/* DisplayPort Audio w/ LPE */
> > +#define CHICKEN_BIT_DBG_ENABLE		(1 << 0)
> > +#define AMP_UNMUTE		        (1 << 1)

That should be called AMP_MUTE I think,

> 
> The convention is to define registers first and the contents/bits for
> each register immedialy below. For groups of registers (like
> PORT_EN_B/C/D below) define all registers first and bits immediately
> below. (But note that the chicken register is not part of the group.)
> 
> > +#define AUD_CHICKEN_BIT_REG		0x62F38

Spec calls this AUD_CHICKENBIT_REG. Might as well follow it to the
letter.

> > +#define AUD_PORT_EN_B_DBG		0x62F20
> > +#define AUD_PORT_EN_C_DBG		0x62F28
> > +#define AUD_PORT_EN_D_DBG		0x62F2C

These match the spec. But to match the standard i915 convention they
should be called _AUD_PORT_EN_B_DBG etc. Same forthe chicken bit
register.

> 
> Please don't define these separately without the display base, use
> (VLV_DISPLAY_BASE + 0x62f38) style instead. All the other uses of
> VLV_DISPLAY_BASE in the file follow the same convention.
> 
> > +#define VLV_AUD_CHICKEN_BIT_REG		_MMIO(VLV_DISPLAY_BASE + AUD_CHICKEN_BIT_REG)
> > +#define VLV_AUD_PORT_EN_B_DBG		_MMIO(VLV_DISPLAY_BASE + AUD_PORT_EN_B_DBG)
> > +#define VLV_AUD_PORT_EN_C_DBG		_MMIO(VLV_DISPLAY_BASE + AUD_PORT_EN_C_DBG)
> > +#define VLV_AUD_PORT_EN_D_DBG		_MMIO(VLV_DISPLAY_BASE + AUD_PORT_EN_D_DBG)
> > +
> 
> Would be nice to have a macro VLV_AUD_PORT_EN_DBG(port), but damn those
> reg offsets don't make it easy...

_MMIO_PORT3().

> 
> 
> >  #define GEN6_BSD_RNCID			_MMIO(0x12198)
> >  
> >  #define GEN7_FF_THREAD_MODE		_MMIO(0x20a0)
> > diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c
> > index 245523e..b3134ef 100644
> > --- a/drivers/gpu/drm/i915/intel_lpe_audio.c
> > +++ b/drivers/gpu/drm/i915/intel_lpe_audio.c
> > @@ -248,6 +248,15 @@ static int lpe_audio_setup(struct drm_i915_private *dev_priv)
> >  		goto err_free_irq;
> >  	}
> >  
> > +	/* Enable DPAudio debug bits by default */
> > +	if (IS_CHERRYVIEW(dev_priv)) {

VLV too. And like I said we might need this in the powerwell code as
well. You should make a test to see if the register value is retained
across the display power well being turned off. Eg. simply disable all
displays, check the log to make sure it really did turn off the display
power well, then re-enable some displays, and finally check if the
register value was retained or not).

> > +		u32 chicken_bit;
> > +
> > +		chicken_bit = I915_READ(VLV_AUD_CHICKEN_BIT_REG);
> > +		I915_WRITE(VLV_AUD_CHICKEN_BIT_REG,
> > +			   chicken_bit | CHICKEN_BIT_DBG_ENABLE);
> > +	}
> > +
> >  	return 0;
> >  err_free_irq:
> >  	irq_free_desc(dev_priv->lpe_audio.irq);
> > @@ -357,6 +366,24 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv,
> >  			pdata->tmds_clock_speed = tmds_clk_speed;
> >  		if (link_rate)
> >  			pdata->link_rate = link_rate;
> > +
> > +		if (dp_output) { /* unmute the amp */

The spec doesn't distinquish DP vs. HDMI here. So I presume we should be
able to do this always.

And I think we might want to mute things again when disabling audio.

> > +			u32 audio_enable;
> > +
> > +			if (port == PORT_B) {
> > +				audio_enable = I915_READ(VLV_AUD_PORT_EN_B_DBG);
> > +				I915_WRITE(VLV_AUD_PORT_EN_B_DBG,
> > +					   audio_enable & ~AMP_UNMUTE);
> > +			} else if (port == PORT_C) {
> > +				audio_enable = I915_READ(VLV_AUD_PORT_EN_C_DBG);
> > +				I915_WRITE(VLV_AUD_PORT_EN_C_DBG,
> > +					   audio_enable & ~AMP_UNMUTE);
> > +			} else if (port == PORT_D) {
> > +				audio_enable = I915_READ(VLV_AUD_PORT_EN_D_DBG);
> > +				I915_WRITE(VLV_AUD_PORT_EN_D_DBG,
> > +					   audio_enable & ~AMP_UNMUTE);
> > +			}
> > +		}
> >  	} else {
> >  		memset(pdata->eld.eld_data, 0,
> >  			HDMI_MAX_ELD_BYTES);
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC PATCH 4/5] drm: i915: add DisplayPort amp unmute for LPE audio mode

[Jani Nikula]

On Fri, 27 Jan 2017, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Fri, Jan 27, 2017 at 12:08:58PM +0200, Jani Nikula wrote:
>> On Thu, 26 Jan 2017, Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> wrote:
>> > Enable chicken bit on LPE mode setup and unmute amp on
>> > notification
>> >
>> > FIXME: should these two phases done somewhere else?
>> >
>> > Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
>> > ---
>> >  drivers/gpu/drm/i915/i915_reg.h        | 12 ++++++++++++
>> >  drivers/gpu/drm/i915/intel_lpe_audio.c | 27 +++++++++++++++++++++++++++
>> >  2 files changed, 39 insertions(+)
>> >
>> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> > index a9ffc8d..ee90f64 100644
>> > --- a/drivers/gpu/drm/i915/i915_reg.h
>> > +++ b/drivers/gpu/drm/i915/i915_reg.h
>> > @@ -2061,6 +2061,18 @@ enum skl_disp_power_wells {
>> >  #define I915_HDMI_LPE_AUDIO_BASE	(VLV_DISPLAY_BASE + 0x65000)
>> >  #define I915_HDMI_LPE_AUDIO_SIZE	0x1000
>> >  
>> > +/* DisplayPort Audio w/ LPE */
>> > +#define CHICKEN_BIT_DBG_ENABLE		(1 << 0)
>> > +#define AMP_UNMUTE		        (1 << 1)
>
> That should be called AMP_MUTE I think,
>
>> 
>> The convention is to define registers first and the contents/bits for
>> each register immedialy below. For groups of registers (like
>> PORT_EN_B/C/D below) define all registers first and bits immediately
>> below. (But note that the chicken register is not part of the group.)
>> 
>> > +#define AUD_CHICKEN_BIT_REG		0x62F38
>
> Spec calls this AUD_CHICKENBIT_REG. Might as well follow it to the
> letter.
>
>> > +#define AUD_PORT_EN_B_DBG		0x62F20
>> > +#define AUD_PORT_EN_C_DBG		0x62F28
>> > +#define AUD_PORT_EN_D_DBG		0x62F2C
>
> These match the spec. But to match the standard i915 convention they
> should be called _AUD_PORT_EN_B_DBG etc. Same forthe chicken bit
> register.
>
>> 
>> Please don't define these separately without the display base, use
>> (VLV_DISPLAY_BASE + 0x62f38) style instead. All the other uses of
>> VLV_DISPLAY_BASE in the file follow the same convention.
>> 
>> > +#define VLV_AUD_CHICKEN_BIT_REG		_MMIO(VLV_DISPLAY_BASE + AUD_CHICKEN_BIT_REG)
>> > +#define VLV_AUD_PORT_EN_B_DBG		_MMIO(VLV_DISPLAY_BASE + AUD_PORT_EN_B_DBG)
>> > +#define VLV_AUD_PORT_EN_C_DBG		_MMIO(VLV_DISPLAY_BASE + AUD_PORT_EN_C_DBG)
>> > +#define VLV_AUD_PORT_EN_D_DBG		_MMIO(VLV_DISPLAY_BASE + AUD_PORT_EN_D_DBG)
>> > +
>> 
>> Would be nice to have a macro VLV_AUD_PORT_EN_DBG(port), but damn those
>> reg offsets don't make it easy...
>
> _MMIO_PORT3().

Works for ports A, B, C, but here we have ports B, C, D.

BR,
Jani.


>
>> 
>> 
>> >  #define GEN6_BSD_RNCID			_MMIO(0x12198)
>> >  
>> >  #define GEN7_FF_THREAD_MODE		_MMIO(0x20a0)
>> > diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c
>> > index 245523e..b3134ef 100644
>> > --- a/drivers/gpu/drm/i915/intel_lpe_audio.c
>> > +++ b/drivers/gpu/drm/i915/intel_lpe_audio.c
>> > @@ -248,6 +248,15 @@ static int lpe_audio_setup(struct drm_i915_private *dev_priv)
>> >  		goto err_free_irq;
>> >  	}
>> >  
>> > +	/* Enable DPAudio debug bits by default */
>> > +	if (IS_CHERRYVIEW(dev_priv)) {
>
> VLV too. And like I said we might need this in the powerwell code as
> well. You should make a test to see if the register value is retained
> across the display power well being turned off. Eg. simply disable all
> displays, check the log to make sure it really did turn off the display
> power well, then re-enable some displays, and finally check if the
> register value was retained or not).
>
>> > +		u32 chicken_bit;
>> > +
>> > +		chicken_bit = I915_READ(VLV_AUD_CHICKEN_BIT_REG);
>> > +		I915_WRITE(VLV_AUD_CHICKEN_BIT_REG,
>> > +			   chicken_bit | CHICKEN_BIT_DBG_ENABLE);
>> > +	}
>> > +
>> >  	return 0;
>> >  err_free_irq:
>> >  	irq_free_desc(dev_priv->lpe_audio.irq);
>> > @@ -357,6 +366,24 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv,
>> >  			pdata->tmds_clock_speed = tmds_clk_speed;
>> >  		if (link_rate)
>> >  			pdata->link_rate = link_rate;
>> > +
>> > +		if (dp_output) { /* unmute the amp */
>
> The spec doesn't distinquish DP vs. HDMI here. So I presume we should be
> able to do this always.
>
> And I think we might want to mute things again when disabling audio.
>
>> > +			u32 audio_enable;
>> > +
>> > +			if (port == PORT_B) {
>> > +				audio_enable = I915_READ(VLV_AUD_PORT_EN_B_DBG);
>> > +				I915_WRITE(VLV_AUD_PORT_EN_B_DBG,
>> > +					   audio_enable & ~AMP_UNMUTE);
>> > +			} else if (port == PORT_C) {
>> > +				audio_enable = I915_READ(VLV_AUD_PORT_EN_C_DBG);
>> > +				I915_WRITE(VLV_AUD_PORT_EN_C_DBG,
>> > +					   audio_enable & ~AMP_UNMUTE);
>> > +			} else if (port == PORT_D) {
>> > +				audio_enable = I915_READ(VLV_AUD_PORT_EN_D_DBG);
>> > +				I915_WRITE(VLV_AUD_PORT_EN_D_DBG,
>> > +					   audio_enable & ~AMP_UNMUTE);
>> > +			}
>> > +		}
>> >  	} else {
>> >  		memset(pdata->eld.eld_data, 0,
>> >  			HDMI_MAX_ELD_BYTES);
>> 
>> -- 
>> Jani Nikula, Intel Open Source Technology Center

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

Re: [RFC PATCH 4/5] drm: i915: add DisplayPort amp unmute for LPE audio mode

[Ville Syrjälä]

On Fri, Jan 27, 2017 at 03:51:34PM +0200, Jani Nikula wrote:
> On Fri, 27 Jan 2017, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> > On Fri, Jan 27, 2017 at 12:08:58PM +0200, Jani Nikula wrote:
> >> On Thu, 26 Jan 2017, Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> wrote:
> >> > Enable chicken bit on LPE mode setup and unmute amp on
> >> > notification
> >> >
> >> > FIXME: should these two phases done somewhere else?
> >> >
> >> > Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> >> > ---
> >> >  drivers/gpu/drm/i915/i915_reg.h        | 12 ++++++++++++
> >> >  drivers/gpu/drm/i915/intel_lpe_audio.c | 27 +++++++++++++++++++++++++++
> >> >  2 files changed, 39 insertions(+)
> >> >
> >> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> >> > index a9ffc8d..ee90f64 100644
> >> > --- a/drivers/gpu/drm/i915/i915_reg.h
> >> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> >> > @@ -2061,6 +2061,18 @@ enum skl_disp_power_wells {
> >> >  #define I915_HDMI_LPE_AUDIO_BASE	(VLV_DISPLAY_BASE + 0x65000)
> >> >  #define I915_HDMI_LPE_AUDIO_SIZE	0x1000
> >> >  
> >> > +/* DisplayPort Audio w/ LPE */
> >> > +#define CHICKEN_BIT_DBG_ENABLE		(1 << 0)
> >> > +#define AMP_UNMUTE		        (1 << 1)
> >
> > That should be called AMP_MUTE I think,
> >
> >> 
> >> The convention is to define registers first and the contents/bits for
> >> each register immedialy below. For groups of registers (like
> >> PORT_EN_B/C/D below) define all registers first and bits immediately
> >> below. (But note that the chicken register is not part of the group.)
> >> 
> >> > +#define AUD_CHICKEN_BIT_REG		0x62F38
> >
> > Spec calls this AUD_CHICKENBIT_REG. Might as well follow it to the
> > letter.
> >
> >> > +#define AUD_PORT_EN_B_DBG		0x62F20
> >> > +#define AUD_PORT_EN_C_DBG		0x62F28
> >> > +#define AUD_PORT_EN_D_DBG		0x62F2C
> >
> > These match the spec. But to match the standard i915 convention they
> > should be called _AUD_PORT_EN_B_DBG etc. Same forthe chicken bit
> > register.
> >
> >> 
> >> Please don't define these separately without the display base, use
> >> (VLV_DISPLAY_BASE + 0x62f38) style instead. All the other uses of
> >> VLV_DISPLAY_BASE in the file follow the same convention.
> >> 
> >> > +#define VLV_AUD_CHICKEN_BIT_REG		_MMIO(VLV_DISPLAY_BASE + AUD_CHICKEN_BIT_REG)
> >> > +#define VLV_AUD_PORT_EN_B_DBG		_MMIO(VLV_DISPLAY_BASE + AUD_PORT_EN_B_DBG)
> >> > +#define VLV_AUD_PORT_EN_C_DBG		_MMIO(VLV_DISPLAY_BASE + AUD_PORT_EN_C_DBG)
> >> > +#define VLV_AUD_PORT_EN_D_DBG		_MMIO(VLV_DISPLAY_BASE + AUD_PORT_EN_D_DBG)
> >> > +
> >> 
> >> Would be nice to have a macro VLV_AUD_PORT_EN_DBG(port), but damn those
> >> reg offsets don't make it easy...
> >
> > _MMIO_PORT3().
> 
> Works for ports A, B, C, but here we have ports B, C, D.

(port)-PORT_B is easy enough to stick in there ;)

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC PATCH 4/5] drm: i915: add DisplayPort amp unmute for LPE audio mode

[Ville Syrjälä]

On Fri, Jan 27, 2017 at 03:17:34PM +0200, Ville Syrjälä wrote:
> On Fri, Jan 27, 2017 at 12:08:58PM +0200, Jani Nikula wrote:
> > On Thu, 26 Jan 2017, Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> wrote:
> > > Enable chicken bit on LPE mode setup and unmute amp on
> > > notification
> > >
> > > FIXME: should these two phases done somewhere else?
> > >
> > > Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/i915_reg.h        | 12 ++++++++++++
> > >  drivers/gpu/drm/i915/intel_lpe_audio.c | 27 +++++++++++++++++++++++++++
> > >  2 files changed, 39 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > > index a9ffc8d..ee90f64 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -2061,6 +2061,18 @@ enum skl_disp_power_wells {
> > >  #define I915_HDMI_LPE_AUDIO_BASE	(VLV_DISPLAY_BASE + 0x65000)
> > >  #define I915_HDMI_LPE_AUDIO_SIZE	0x1000
> > >  
> > > +/* DisplayPort Audio w/ LPE */
> > > +#define CHICKEN_BIT_DBG_ENABLE		(1 << 0)
> > > +#define AMP_UNMUTE		        (1 << 1)
> 
> That should be called AMP_MUTE I think,
> 
> > 
> > The convention is to define registers first and the contents/bits for
> > each register immedialy below. For groups of registers (like
> > PORT_EN_B/C/D below) define all registers first and bits immediately
> > below. (But note that the chicken register is not part of the group.)
> > 
> > > +#define AUD_CHICKEN_BIT_REG		0x62F38
> 
> Spec calls this AUD_CHICKENBIT_REG. Might as well follow it to the
> letter.
> 
> > > +#define AUD_PORT_EN_B_DBG		0x62F20
> > > +#define AUD_PORT_EN_C_DBG		0x62F28
> > > +#define AUD_PORT_EN_D_DBG		0x62F2C
> 
> These match the spec. But to match the standard i915 convention they
> should be called _AUD_PORT_EN_B_DBG etc. Same forthe chicken bit
> register.

Actually they just match one version of the spec I had lying around.
Another versions says:

AUD_PORT_EN_B_DBG 0x62F20
AUD_PORT_EN_C_DBG 0x62F30
AUD_PORT_EN_D_DBG 0x62F34

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC PATCH 4/5] drm: i915: add DisplayPort amp unmute for LPE audio mode

[Takashi Iwai]

On Fri, 27 Jan 2017 15:35:47 +0100,
Ville Syrjälä wrote:
> 
> On Fri, Jan 27, 2017 at 03:17:34PM +0200, Ville Syrjälä wrote:
> > On Fri, Jan 27, 2017 at 12:08:58PM +0200, Jani Nikula wrote:
> > > On Thu, 26 Jan 2017, Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> wrote:
> > > > Enable chicken bit on LPE mode setup and unmute amp on
> > > > notification
> > > >
> > > > FIXME: should these two phases done somewhere else?
> > > >
> > > > Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/i915_reg.h        | 12 ++++++++++++
> > > >  drivers/gpu/drm/i915/intel_lpe_audio.c | 27 +++++++++++++++++++++++++++
> > > >  2 files changed, 39 insertions(+)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > > > index a9ffc8d..ee90f64 100644
> > > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > > @@ -2061,6 +2061,18 @@ enum skl_disp_power_wells {
> > > >  #define I915_HDMI_LPE_AUDIO_BASE	(VLV_DISPLAY_BASE + 0x65000)
> > > >  #define I915_HDMI_LPE_AUDIO_SIZE	0x1000
> > > >  
> > > > +/* DisplayPort Audio w/ LPE */
> > > > +#define CHICKEN_BIT_DBG_ENABLE		(1 << 0)
> > > > +#define AMP_UNMUTE		        (1 << 1)
> > 
> > That should be called AMP_MUTE I think,
> > 
> > > 
> > > The convention is to define registers first and the contents/bits for
> > > each register immedialy below. For groups of registers (like
> > > PORT_EN_B/C/D below) define all registers first and bits immediately
> > > below. (But note that the chicken register is not part of the group.)
> > > 
> > > > +#define AUD_CHICKEN_BIT_REG		0x62F38
> > 
> > Spec calls this AUD_CHICKENBIT_REG. Might as well follow it to the
> > letter.
> > 
> > > > +#define AUD_PORT_EN_B_DBG		0x62F20
> > > > +#define AUD_PORT_EN_C_DBG		0x62F28
> > > > +#define AUD_PORT_EN_D_DBG		0x62F2C
> > 
> > These match the spec. But to match the standard i915 convention they
> > should be called _AUD_PORT_EN_B_DBG etc. Same forthe chicken bit
> > register.
> 
> Actually they just match one version of the spec I had lying around.
> Another versions says:
> 
> AUD_PORT_EN_B_DBG 0x62F20
> AUD_PORT_EN_C_DBG 0x62F30
> AUD_PORT_EN_D_DBG 0x62F34

That's it!  Now finally I can hear the audio from DP3 with the
additional patch below.


thanks,

Takashi

---
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index ee90f64b89e8..5c577d242078 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2066,8 +2066,8 @@ enum skl_disp_power_wells {
 #define AMP_UNMUTE		        (1 << 1)
 #define AUD_CHICKEN_BIT_REG		0x62F38
 #define AUD_PORT_EN_B_DBG		0x62F20
-#define AUD_PORT_EN_C_DBG		0x62F28
-#define AUD_PORT_EN_D_DBG		0x62F2C
+#define AUD_PORT_EN_C_DBG		0x62F30
+#define AUD_PORT_EN_D_DBG		0x62F34
 #define VLV_AUD_CHICKEN_BIT_REG		_MMIO(VLV_DISPLAY_BASE + AUD_CHICKEN_BIT_REG)
 #define VLV_AUD_PORT_EN_B_DBG		_MMIO(VLV_DISPLAY_BASE + AUD_PORT_EN_B_DBG)
 #define VLV_AUD_PORT_EN_C_DBG		_MMIO(VLV_DISPLAY_BASE + AUD_PORT_EN_C_DBG)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC PATCH 4/5] drm: i915: add DisplayPort amp unmute for LPE audio mode

[Pierre-Louis Bossart]

Thanks Jani and Ville for the comments. Couple of precisions needed below:
>>
>>>   #define GEN6_BSD_RNCID			_MMIO(0x12198)
>>>   
>>>   #define GEN7_FF_THREAD_MODE		_MMIO(0x20a0)
>>> diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c
>>> index 245523e..b3134ef 100644
>>> --- a/drivers/gpu/drm/i915/intel_lpe_audio.c
>>> +++ b/drivers/gpu/drm/i915/intel_lpe_audio.c
>>> @@ -248,6 +248,15 @@ static int lpe_audio_setup(struct drm_i915_private *dev_priv)
>>>   		goto err_free_irq;
>>>   	}
>>>   
>>> +	/* Enable DPAudio debug bits by default */
>>> +	if (IS_CHERRYVIEW(dev_priv)) {
> VLV too. And like I said we might need this in the powerwell code as
> well. You should make a test to see if the register value is retained
> across the display power well being turned off. Eg. simply disable all
> displays, check the log to make sure it really did turn off the display
> power well, then re-enable some displays, and finally check if the
> register value was retained or not).
VLV has DisplayPort? I thought this was an addition on CHT, and I really 
don't know of any devices with this combination of HDaudio disabled+DP. 
I'd rather keep this CHT-only until we find a device where this can be 
tested.

On the powerwell, I could use more guidance. i tried this first solution 
to see if streaming worked (and it did :-)). I don't mind moving the 
code somewhere else but I have no idea where.
>
>>> +		u32 chicken_bit;
>>> +
>>> +		chicken_bit = I915_READ(VLV_AUD_CHICKEN_BIT_REG);
>>> +		I915_WRITE(VLV_AUD_CHICKEN_BIT_REG,
>>> +			   chicken_bit | CHICKEN_BIT_DBG_ENABLE);
>>> +	}
>>> +
>>>   	return 0;
>>>   err_free_irq:
>>>   	irq_free_desc(dev_priv->lpe_audio.irq);
>>> @@ -357,6 +366,24 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv,
>>>   			pdata->tmds_clock_speed = tmds_clk_speed;
>>>   		if (link_rate)
>>>   			pdata->link_rate = link_rate;
>>> +
>>> +		if (dp_output) { /* unmute the amp */
> The spec doesn't distinquish DP vs. HDMI here. So I presume we should be
> able to do this always.

I'll try to see if HDMI still works with this. We could tentatively add 
unmute in all cases but I'll need to add a test for Baytrail (no PORT_D) 
so in the end it's the same number of tests.

>
> And I think we might want to mute things again when disabling audio.
I was wondering if there would be side effects of writing to a register 
controlling a port if that port is not connected any longer. I'll give 
it a try.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC PATCH 4/5] drm: i915: add DisplayPort amp unmute for LPE audio mode

[Pierre-Louis Bossart]

>>>>> +#define AUD_PORT_EN_B_DBG		0x62F20
>>>>> +#define AUD_PORT_EN_C_DBG		0x62F28
>>>>> +#define AUD_PORT_EN_D_DBG		0x62F2C
>>> These match the spec. But to match the standard i915 convention they
>>> should be called _AUD_PORT_EN_B_DBG etc. Same forthe chicken bit
>>> register.
>> Actually they just match one version of the spec I had lying around.
>> Another versions says:
>>
>> AUD_PORT_EN_B_DBG 0x62F20
>> AUD_PORT_EN_C_DBG 0x62F30
>> AUD_PORT_EN_D_DBG 0x62F34
> That's it!  Now finally I can hear the audio from DP3 with the
> additional patch below.
Wow. Thanks Ville for looking into this, we could have lost a lot of 
time. Do you happen to know if those previous values are due to poor 
documentation or a different skew we'd need to support, e.g. with a 
PCI-Id quirk?
At any rate, 2 days to get DP audio working is pretty nice, this was a 
good week.

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

Re: [RFC PATCH 4/5] drm: i915: add DisplayPort amp unmute for LPE audio mode

[Ville Syrjälä]

On Fri, Jan 27, 2017 at 08:47:50AM -0600, Pierre-Louis Bossart wrote:
> Thanks Jani and Ville for the comments. Couple of precisions needed below:
> >>
> >>>   #define GEN6_BSD_RNCID			_MMIO(0x12198)
> >>>   
> >>>   #define GEN7_FF_THREAD_MODE		_MMIO(0x20a0)
> >>> diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c
> >>> index 245523e..b3134ef 100644
> >>> --- a/drivers/gpu/drm/i915/intel_lpe_audio.c
> >>> +++ b/drivers/gpu/drm/i915/intel_lpe_audio.c
> >>> @@ -248,6 +248,15 @@ static int lpe_audio_setup(struct drm_i915_private *dev_priv)
> >>>   		goto err_free_irq;
> >>>   	}
> >>>   
> >>> +	/* Enable DPAudio debug bits by default */
> >>> +	if (IS_CHERRYVIEW(dev_priv)) {
> > VLV too. And like I said we might need this in the powerwell code as
> > well. You should make a test to see if the register value is retained
> > across the display power well being turned off. Eg. simply disable all
> > displays, check the log to make sure it really did turn off the display
> > power well, then re-enable some displays, and finally check if the
> > register value was retained or not).
> VLV has DisplayPort?

Yes. And as I said earlier the docs don't make any distinciton between
HDMI and DP in the audio programming sequence. So based on the docs we
should always unmute the amp manually.

>I thought this was an addition on CHT, and I really 
> don't know of any devices with this combination of HDaudio disabled+DP. 
> I'd rather keep this CHT-only until we find a device where this can be 
> tested.
> 
> On the powerwell, I could use more guidance. i tried this first solution 
> to see if streaming worked (and it did :-)). I don't mind moving the 
> code somewhere else but I have no idea where.

1. boot with drm.debug=0xe
1. 'intel_reg read 0x62f38' just after boot, to see that the
   chicken bit is as it should
2. disable all displays (eg. xset dpms force off)
3. grep 'power_well.*display' (make sure the last thing it says is
   "disabling"
4. turn on the displays (eg. xset dpms force on)
5. 'intel_reg read 0x62f38' to check the chicken bit again

If it didn't survive then we need to set it in either in
vlv_display_power_well_init(), or we could just set it whenever
we set the AMP_MUTE bit for any of the ports.

>
> >>> +		u32 chicken_bit;
> >>> +
> >>> +		chicken_bit = I915_READ(VLV_AUD_CHICKEN_BIT_REG);
> >>> +		I915_WRITE(VLV_AUD_CHICKEN_BIT_REG,
> >>> +			   chicken_bit | CHICKEN_BIT_DBG_ENABLE);
> >>> +	}
> >>> +
> >>>   	return 0;
> >>>   err_free_irq:
> >>>   	irq_free_desc(dev_priv->lpe_audio.irq);
> >>> @@ -357,6 +366,24 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv,
> >>>   			pdata->tmds_clock_speed = tmds_clk_speed;
> >>>   		if (link_rate)
> >>>   			pdata->link_rate = link_rate;
> >>> +
> >>> +		if (dp_output) { /* unmute the amp */
> > The spec doesn't distinquish DP vs. HDMI here. So I presume we should be
> > able to do this always.
> 
> I'll try to see if HDMI still works with this. We could tentatively add 
> unmute in all cases but I'll need to add a test for Baytrail (no PORT_D) 
> so in the end it's the same number of tests.
> 
> >
> > And I think we might want to mute things again when disabling audio.
> I was wondering if there would be side effects of writing to a register 
> controlling a port if that port is not connected any longer. I'll give 
> it a try.

Nothing apart from HPD circuitry really cares if there's anything
connectect or not.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC PATCH 4/5] drm: i915: add DisplayPort amp unmute for LPE audio mode

[Ville Syrjälä]

On Fri, Jan 27, 2017 at 08:55:19AM -0600, Pierre-Louis Bossart wrote:
> 
> 
> >>>>> +#define AUD_PORT_EN_B_DBG		0x62F20
> >>>>> +#define AUD_PORT_EN_C_DBG		0x62F28
> >>>>> +#define AUD_PORT_EN_D_DBG		0x62F2C
> >>> These match the spec. But to match the standard i915 convention they
> >>> should be called _AUD_PORT_EN_B_DBG etc. Same forthe chicken bit
> >>> register.
> >> Actually they just match one version of the spec I had lying around.
> >> Another versions says:
> >>
> >> AUD_PORT_EN_B_DBG 0x62F20
> >> AUD_PORT_EN_C_DBG 0x62F30
> >> AUD_PORT_EN_D_DBG 0x62F34
> > That's it!  Now finally I can hear the audio from DP3 with the
> > additional patch below.
> Wow. Thanks Ville for looking into this, we could have lost a lot of 
> time. Do you happen to know if those previous values are due to poor 
> documentation or a different skew we'd need to support, e.g. with a 
> PCI-Id quirk?

No idea really. You should really test this on both CHV and VLV with all
possible port/pipe combinations to make sure we got it right. I trust
these VLV/CHV docs about as much as I trust most politicians.

Alternatively you could just read all those regs on both platforms and
see if the values you get from them conform to any visible pattern that
could tell us which offsets are the correct ones. They might not, in
which case actual testing is the best bet.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx