alsa-devel.alsa-project.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/7] DisplayPort audio support on Cherrytrail
@ 2017-01-31 21:36 Takashi Iwai
  2017-01-31 21:36 ` [PATCH 1/7] drm/i915: add DP support in LPE audio mode Takashi Iwai
                   ` (7 more replies)
  0 siblings, 8 replies; 18+ messages in thread
From: Takashi Iwai @ 2017-01-31 21:36 UTC (permalink / raw)
  To: alsa-devel, intel-gfx; +Cc: rakesh.a.ughreja

Hi,

the following patches enable DisplayPort Audio on Cherrytrail machines
when applied on top of my topic/intel-lpe-audio branch.  Tests of DP
audio were run on Dell Wyse 3040.  The regression test were performed
on Baytrail (Compute Stick) and Cherrytrail (Zotac PI330) in HDMI
mode.  On Cherrytrail there were no issues changing between HDMI and
DP connectors dynamically.

Could you i915 people review and give ACK if they are OK?
The changes in drm/i915 side are fairly trivial, so I'd like to take
them through sound git tree once after I receive your ACKs.


Changes since RFC:
 - reordered and squashed patches
 - clean-up of register definitions and offsets (based on feedback from
   Jani/Ville)
 - unmute amp for both HDMI and DP unconditionally
 - mute amp on invalid ELD (unplug)
 - remove test for chicken bit which seems to have no effect in hardware
 - cosmetic edits to make checkpatch happy
 - change i915 notification argument to pass the plataform device
   instead


Most of hard work in this patchset has been done by Pierre, so all
credits go to him.


thanks,

Takashi

===

Pierre-Louis Bossart (4):
  drm/i915: add DP support in LPE audio mode
  drm/i915: add DisplayPort amp unmute for LPE audio mode
  ALSA: x86: intel_hdmi: add definitions and logic for DP audio
  ALSA: x86: Use config base depending on the pipe

Takashi Iwai (3):
  drm/i915: Avoid MST pipe handling for LPE audio
  drm/i915: Pass pipe to LPE audio notification
  drm/i915: Pass platform device to LPE audio notifier

 drivers/gpu/drm/i915/i915_drv.h        |   3 +-
 drivers/gpu/drm/i915/i915_reg.h        |  10 ++
 drivers/gpu/drm/i915/intel_audio.c     |  38 +++++---
 drivers/gpu/drm/i915/intel_lpe_audio.c |  28 +++++-
 include/drm/intel_lpe_audio.h          |   7 +-
 sound/x86/intel_hdmi_audio.c           | 173 ++++++++++++++++++++++++++++-----
 sound/x86/intel_hdmi_audio.h           |   8 +-
 sound/x86/intel_hdmi_lpe_audio.c       |  83 ++++++++++++----
 sound/x86/intel_hdmi_lpe_audio.h       |  29 ++++++
 9 files changed, 315 insertions(+), 64 deletions(-)

-- 
2.11.0

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 1/7] drm/i915: add DP support in LPE audio mode
  2017-01-31 21:36 [PATCH 0/7] DisplayPort audio support on Cherrytrail Takashi Iwai
@ 2017-01-31 21:36 ` Takashi Iwai
  2017-01-31 21:36 ` [PATCH 2/7] drm/i915: add DisplayPort amp unmute for " Takashi Iwai
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 18+ messages in thread
From: Takashi Iwai @ 2017-01-31 21:36 UTC (permalink / raw)
  To: alsa-devel, intel-gfx; +Cc: rakesh.a.ughreja

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

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

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 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 3e3102cedc82..836d823d476b 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 364f96207c40..1645ce42b898 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 27d94255872d..245523e14418 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 952de05a9d76..857e0eafed79 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.11.0

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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 2/7] drm/i915: add DisplayPort amp unmute for LPE audio mode
  2017-01-31 21:36 [PATCH 0/7] DisplayPort audio support on Cherrytrail Takashi Iwai
  2017-01-31 21:36 ` [PATCH 1/7] drm/i915: add DP support in LPE audio mode Takashi Iwai
@ 2017-01-31 21:36 ` Takashi Iwai
  2017-02-01 14:45   ` Ville Syrjälä
  2017-02-02  9:57   ` Takashi Iwai
  2017-01-31 21:36 ` [PATCH 3/7] drm/i915: Avoid MST pipe handling for LPE audio Takashi Iwai
                   ` (5 subsequent siblings)
  7 siblings, 2 replies; 18+ messages in thread
From: Takashi Iwai @ 2017-01-31 21:36 UTC (permalink / raw)
  To: alsa-devel, intel-gfx; +Cc: rakesh.a.ughreja

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

Enable unmute/mute amp notification. This doesn't seem to affect
HDMI support so this is done unconditionally.

An earlier version of this patch set a chicken bit at address 0x62F38
prior to the mute/unmute but this register doesn't seem to do anything
so this phase was removed.

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

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index a9ffc8df241b..8fcc80cb864b 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2061,6 +2061,16 @@ 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 VLV_AUD_PORT_EN_B_DBG		(VLV_DISPLAY_BASE + 0x62F20)
+#define VLV_AUD_PORT_EN_C_DBG		(VLV_DISPLAY_BASE + 0x62F30)
+#define VLV_AUD_PORT_EN_D_DBG		(VLV_DISPLAY_BASE + 0x62F34)
+#define VLV_AUD_PORT_EN_DBG(port)	_MMIO_PORT3((port) - PORT_B,	   \
+						    VLV_AUD_PORT_EN_B_DBG, \
+						    VLV_AUD_PORT_EN_C_DBG, \
+						    VLV_AUD_PORT_EN_D_DBG)
+#define VLV_AMP_MUTE		        (1 << 1)
+
 #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 245523e14418..f95624a46f27 100644
--- a/drivers/gpu/drm/i915/intel_lpe_audio.c
+++ b/drivers/gpu/drm/i915/intel_lpe_audio.c
@@ -337,15 +337,25 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv,
 {
 	unsigned long irq_flags;
 	struct intel_hdmi_lpe_audio_pdata *pdata = NULL;
+	u32 audio_enable;
+	i915_reg_t mmio;
 
 	if (!HAS_LPE_AUDIO(dev_priv))
 		return;
 
+	if (port == PORT_A) {
+		DRM_ERROR("PORT_A is not valid for HDMI/DP usages\n");
+		return;
+	}
+
 	pdata = dev_get_platdata(
 		&(dev_priv->lpe_audio.platdev->dev));
 
 	spin_lock_irqsave(&pdata->lpe_audio_slock, irq_flags);
 
+	mmio = VLV_AUD_PORT_EN_DBG(port);
+	audio_enable = I915_READ(mmio);
+
 	if (eld != NULL) {
 		memcpy(pdata->eld.eld_data, eld,
 			HDMI_MAX_ELD_BYTES);
@@ -357,11 +367,18 @@ 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;
+
+		/* Unmute the amp for both DP and HDMI */
+		I915_WRITE(mmio, audio_enable & ~VLV_AMP_MUTE);
+
 	} else {
 		memset(pdata->eld.eld_data, 0,
 			HDMI_MAX_ELD_BYTES);
 		pdata->hdmi_connected = false;
 		pdata->dp_output = false;
+
+		/* Mute the amp for both DP and HDMI */
+		I915_WRITE(mmio, audio_enable | VLV_AMP_MUTE);
 	}
 
 	if (pdata->notify_audio_lpe)
-- 
2.11.0

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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 3/7] drm/i915: Avoid MST pipe handling for LPE audio
  2017-01-31 21:36 [PATCH 0/7] DisplayPort audio support on Cherrytrail Takashi Iwai
  2017-01-31 21:36 ` [PATCH 1/7] drm/i915: add DP support in LPE audio mode Takashi Iwai
  2017-01-31 21:36 ` [PATCH 2/7] drm/i915: add DisplayPort amp unmute for " Takashi Iwai
@ 2017-01-31 21:36 ` Takashi Iwai
  2017-01-31 21:36 ` [PATCH 4/7] drm/i915: Pass pipe to LPE audio notification Takashi Iwai
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 18+ messages in thread
From: Takashi Iwai @ 2017-01-31 21:36 UTC (permalink / raw)
  To: alsa-devel, intel-gfx; +Cc: rakesh.a.ughreja

The pipe gets cleared to -1 for non-MST before the ELD audio
notification due to the MST audio support.  This makes sense for
HD-audio that received the MST handling, but it's useless for LPE
audio.  Handle the MST pipe hack conditionally only for HD-audio.

Reported-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 drivers/gpu/drm/i915/intel_audio.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
index 1645ce42b898..d4e6d1136cfe 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -624,13 +624,14 @@ void intel_audio_codec_enable(struct intel_encoder *intel_encoder,
 	dev_priv->av_enc_map[pipe] = intel_encoder;
 	mutex_unlock(&dev_priv->av_mutex);
 
-	/* audio drivers expect pipe = -1 to indicate Non-MST cases */
-	if (intel_encoder->type != INTEL_OUTPUT_DP_MST)
-		pipe = -1;
-
-	if (acomp && acomp->audio_ops && acomp->audio_ops->pin_eld_notify)
+	if (acomp && acomp->audio_ops && acomp->audio_ops->pin_eld_notify) {
+		/* audio drivers expect pipe = -1 to indicate Non-MST cases */
+		if (intel_encoder->type != INTEL_OUTPUT_DP_MST)
+			pipe = -1;
 		acomp->audio_ops->pin_eld_notify(acomp->audio_ops->audio_ptr,
 						 (int) port, (int) pipe);
+	}
+
 	switch (intel_encoder->type) {
 	case INTEL_OUTPUT_HDMI:
 		intel_lpe_audio_notify(dev_priv, connector->eld, port,
@@ -671,13 +672,13 @@ void intel_audio_codec_disable(struct intel_encoder *intel_encoder)
 	dev_priv->av_enc_map[pipe] = NULL;
 	mutex_unlock(&dev_priv->av_mutex);
 
-	/* audio drivers expect pipe = -1 to indicate Non-MST cases */
-	if (intel_encoder->type != INTEL_OUTPUT_DP_MST)
-		pipe = -1;
-
-	if (acomp && acomp->audio_ops && acomp->audio_ops->pin_eld_notify)
+	if (acomp && acomp->audio_ops && acomp->audio_ops->pin_eld_notify) {
+		/* audio drivers expect pipe = -1 to indicate Non-MST cases */
+		if (intel_encoder->type != INTEL_OUTPUT_DP_MST)
+			pipe = -1;
 		acomp->audio_ops->pin_eld_notify(acomp->audio_ops->audio_ptr,
 						 (int) port, (int) pipe);
+	}
 
 	intel_lpe_audio_notify(dev_priv, NULL, port, 0, false, 0);
 }
-- 
2.11.0

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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 4/7] drm/i915: Pass pipe to LPE audio notification
  2017-01-31 21:36 [PATCH 0/7] DisplayPort audio support on Cherrytrail Takashi Iwai
                   ` (2 preceding siblings ...)
  2017-01-31 21:36 ` [PATCH 3/7] drm/i915: Avoid MST pipe handling for LPE audio Takashi Iwai
@ 2017-01-31 21:36 ` Takashi Iwai
  2017-01-31 21:36 ` [PATCH 5/7] ALSA: x86: intel_hdmi: add definitions and logic for DP audio Takashi Iwai
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 18+ messages in thread
From: Takashi Iwai @ 2017-01-31 21:36 UTC (permalink / raw)
  To: alsa-devel, intel-gfx
  Cc: Pierre-Louis Bossart, Jani Nikula, rakesh.a.ughreja,
	Jerome Anand, Daniel Vetter, Ville Syrjälä

The LPE audio configuration depends on the pipe, thus we need to pass
the currently used pipe.  It's now embedded in struct
intel_hdmi_lpe_audio_eld as well as port id.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 drivers/gpu/drm/i915/i915_drv.h        | 2 +-
 drivers/gpu/drm/i915/intel_audio.c     | 6 +++---
 drivers/gpu/drm/i915/intel_lpe_audio.c | 3 ++-
 include/drm/intel_lpe_audio.h          | 1 +
 4 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 836d823d476b..27311a337e2b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3401,7 +3401,7 @@ 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 pipe, int tmds_clk_speed,
 			    bool dp_output, int link_rate);
 
 /* intel_i2c.c */
diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
index d4e6d1136cfe..892169b7952b 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -634,12 +634,12 @@ void intel_audio_codec_enable(struct intel_encoder *intel_encoder,
 
 	switch (intel_encoder->type) {
 	case INTEL_OUTPUT_HDMI:
-		intel_lpe_audio_notify(dev_priv, connector->eld, port,
+		intel_lpe_audio_notify(dev_priv, connector->eld, port, pipe,
 				       crtc_state->port_clock,
 				       false, 0);
 		break;
 	case INTEL_OUTPUT_DP:
-		intel_lpe_audio_notify(dev_priv, connector->eld, port,
+		intel_lpe_audio_notify(dev_priv, connector->eld, port, pipe,
 				       adjusted_mode->crtc_clock,
 				       true, crtc_state->port_clock);
 		break;
@@ -680,7 +680,7 @@ void intel_audio_codec_disable(struct intel_encoder *intel_encoder)
 						 (int) port, (int) pipe);
 	}
 
-	intel_lpe_audio_notify(dev_priv, NULL, port, 0, false, 0);
+	intel_lpe_audio_notify(dev_priv, NULL, port, pipe, 0, false, 0);
 }
 
 /**
diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c
index f95624a46f27..2ca3c775c6b1 100644
--- a/drivers/gpu/drm/i915/intel_lpe_audio.c
+++ b/drivers/gpu/drm/i915/intel_lpe_audio.c
@@ -332,7 +332,7 @@ 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 pipe, int tmds_clk_speed,
 			    bool dp_output, int link_rate)
 {
 	unsigned long irq_flags;
@@ -360,6 +360,7 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv,
 		memcpy(pdata->eld.eld_data, eld,
 			HDMI_MAX_ELD_BYTES);
 		pdata->eld.port_id = port;
+		pdata->eld.pipe_id = pipe;
 		pdata->hdmi_connected = true;
 
 		pdata->dp_output = dp_output;
diff --git a/include/drm/intel_lpe_audio.h b/include/drm/intel_lpe_audio.h
index 857e0eafed79..410128e4cd70 100644
--- a/include/drm/intel_lpe_audio.h
+++ b/include/drm/intel_lpe_audio.h
@@ -31,6 +31,7 @@
 
 struct intel_hdmi_lpe_audio_eld {
 	int port_id;
+	int pipe_id;
 	unsigned char eld_data[HDMI_MAX_ELD_BYTES];
 };
 
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 5/7] ALSA: x86: intel_hdmi: add definitions and logic for DP audio
  2017-01-31 21:36 [PATCH 0/7] DisplayPort audio support on Cherrytrail Takashi Iwai
                   ` (3 preceding siblings ...)
  2017-01-31 21:36 ` [PATCH 4/7] drm/i915: Pass pipe to LPE audio notification Takashi Iwai
@ 2017-01-31 21:36 ` Takashi Iwai
  2017-01-31 21:36 ` [PATCH 6/7] ALSA: x86: Use config base depending on the pipe Takashi Iwai
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 18+ messages in thread
From: Takashi Iwai @ 2017-01-31 21:36 UTC (permalink / raw)
  To: alsa-devel, intel-gfx; +Cc: rakesh.a.ughreja

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

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>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/x86/intel_hdmi_audio.c     | 173 +++++++++++++++++++++++++++++++++------
 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, 216 insertions(+), 30 deletions(-)

diff --git a/sound/x86/intel_hdmi_audio.c b/sound/x86/intel_hdmi_audio.c
index f30155446117..5ce139c1b21d 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;
 }
@@ -548,6 +550,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 +688,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 +696,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 +725,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 +849,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 +938,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 +964,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 +1066,18 @@ 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 +1450,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 +1495,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 +1503,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 +1618,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 +1639,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 d2015ec84843..034b3873ffa1 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 ead2d3af168c..cea05dfc081a 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 ec4bde50dba7..3aed89af5b45 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.11.0

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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 6/7] ALSA: x86: Use config base depending on the pipe
  2017-01-31 21:36 [PATCH 0/7] DisplayPort audio support on Cherrytrail Takashi Iwai
                   ` (4 preceding siblings ...)
  2017-01-31 21:36 ` [PATCH 5/7] ALSA: x86: intel_hdmi: add definitions and logic for DP audio Takashi Iwai
@ 2017-01-31 21:36 ` Takashi Iwai
  2017-01-31 21:36 ` [PATCH 7/7] drm/i915: Pass platform device to LPE audio notifier Takashi Iwai
  2017-03-13  8:33 ` [PATCH 0/7] DisplayPort audio support on Cherrytrail Daniel Vetter
  7 siblings, 0 replies; 18+ messages in thread
From: Takashi Iwai @ 2017-01-31 21:36 UTC (permalink / raw)
  To: alsa-devel, intel-gfx; +Cc: rakesh.a.ughreja

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

Now the pipe that is being used is passed over i915 notification, we
can re-setup the relevant register offset depending on pipe assignments
during hotplug.
This allows playback on single port machines such Zotac Pi330 or
dual-port machines such as Dell Wyse 3040 box

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/x86/intel_hdmi_lpe_audio.c | 26 +++++++++++++++++++++-----
 1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/sound/x86/intel_hdmi_lpe_audio.c b/sound/x86/intel_hdmi_lpe_audio.c
index cea05dfc081a..6d630f20bca8 100644
--- a/sound/x86/intel_hdmi_lpe_audio.c
+++ b/sound/x86/intel_hdmi_lpe_audio.c
@@ -463,6 +463,22 @@ static void notify_audio_lpe(void *audio_ptr)
 
 	} else if (eld != NULL) {
 
+		switch (eld->pipe_id) {
+		case 0:
+			ctx->had_config_offset = AUDIO_HDMI_CONFIG_A;
+			break;
+		case 1:
+			ctx->had_config_offset = AUDIO_HDMI_CONFIG_B;
+			break;
+		case 2:
+			ctx->had_config_offset = AUDIO_HDMI_CONFIG_C;
+			break;
+		default:
+			dev_dbg(&hlpe_pdev->dev, "Invalid pipe %d\n",
+				eld->pipe_id);
+			break;
+		}
+
 		hdmi_set_eld(eld->eld_data);
 
 		mid_hdmi_audio_signal_event(HAD_EVENT_HOT_PLUG);
@@ -560,15 +576,15 @@ static int hdmi_lpe_audio_probe(struct platform_device *pdev)
 	ctx->mmio_start = mmio_start;
 	ctx->tmds_clock_speed = DIS_SAMPLE_RATE_148_5;
 
-	if (pci_dev_present(cherryview_ids)) {
+	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;
-	} else {
+	else
 		dev_dbg(&hlpe_pdev->dev, "%s: Baytrail LPE - Assume\n",
 				__func__);
-		ctx->had_config_offset = AUDIO_HDMI_CONFIG_A;
-	}
+
+	/* assume pipe A as default */
+	ctx->had_config_offset = AUDIO_HDMI_CONFIG_A;
 
 	pdata = pdev->dev.platform_data;
 
-- 
2.11.0

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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 7/7] drm/i915: Pass platform device to LPE audio notifier
  2017-01-31 21:36 [PATCH 0/7] DisplayPort audio support on Cherrytrail Takashi Iwai
                   ` (5 preceding siblings ...)
  2017-01-31 21:36 ` [PATCH 6/7] ALSA: x86: Use config base depending on the pipe Takashi Iwai
@ 2017-01-31 21:36 ` Takashi Iwai
  2017-03-13  8:33 ` [PATCH 0/7] DisplayPort audio support on Cherrytrail Daniel Vetter
  7 siblings, 0 replies; 18+ messages in thread
From: Takashi Iwai @ 2017-01-31 21:36 UTC (permalink / raw)
  To: alsa-devel, intel-gfx; +Cc: rakesh.a.ughreja

This allows the LPE HDMI driver to clean up its global variable
reference.

Also drop to pass the eld pointer because the connection status and
the ELD bytes can be retrieved from the attached pdata.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 drivers/gpu/drm/i915/intel_lpe_audio.c |  3 +--
 include/drm/intel_lpe_audio.h          |  4 +++-
 sound/x86/intel_hdmi_lpe_audio.c       | 23 +++++++++++------------
 3 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c
index 2ca3c775c6b1..ef0e74830289 100644
--- a/drivers/gpu/drm/i915/intel_lpe_audio.c
+++ b/drivers/gpu/drm/i915/intel_lpe_audio.c
@@ -383,8 +383,7 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv,
 	}
 
 	if (pdata->notify_audio_lpe)
-		pdata->notify_audio_lpe(
-			(eld != NULL) ? &pdata->eld : NULL);
+		pdata->notify_audio_lpe(dev_priv->lpe_audio.platdev);
 	else
 		pdata->notify_pending = true;
 
diff --git a/include/drm/intel_lpe_audio.h b/include/drm/intel_lpe_audio.h
index 410128e4cd70..e9892b4c3af1 100644
--- a/include/drm/intel_lpe_audio.h
+++ b/include/drm/intel_lpe_audio.h
@@ -27,6 +27,8 @@
 #include <linux/types.h>
 #include <linux/spinlock_types.h>
 
+struct platform_device;
+
 #define HDMI_MAX_ELD_BYTES	128
 
 struct intel_hdmi_lpe_audio_eld {
@@ -42,7 +44,7 @@ struct intel_hdmi_lpe_audio_pdata {
 	bool dp_output;
 	int link_rate;
 	struct intel_hdmi_lpe_audio_eld eld;
-	void (*notify_audio_lpe)(void *audio_ptr);
+	void (*notify_audio_lpe)(struct platform_device *pdev);
 	spinlock_t lpe_audio_slock;
 };
 
diff --git a/sound/x86/intel_hdmi_lpe_audio.c b/sound/x86/intel_hdmi_lpe_audio.c
index 6d630f20bca8..3cb0f642575c 100644
--- a/sound/x86/intel_hdmi_lpe_audio.c
+++ b/sound/x86/intel_hdmi_lpe_audio.c
@@ -439,15 +439,14 @@ static irqreturn_t display_pipe_interrupt_handler(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
-static void notify_audio_lpe(void *audio_ptr)
+static void notify_audio_lpe(struct platform_device *pdev)
 {
-	struct hdmi_lpe_audio_ctx *ctx = get_hdmi_context();
-	struct intel_hdmi_lpe_audio_pdata *pdata = hlpe_pdev->dev.platform_data;
-	struct intel_hdmi_lpe_audio_eld *eld = audio_ptr;
+	struct hdmi_lpe_audio_ctx *ctx = platform_get_drvdata(pdev);
+	struct intel_hdmi_lpe_audio_pdata *pdata = pdev->dev.platform_data;
 
 	if (pdata->hdmi_connected != true) {
 
-		dev_dbg(&hlpe_pdev->dev, "%s: Event: HAD_NOTIFY_HOT_UNPLUG\n",
+		dev_dbg(&pdev->dev, "%s: Event: HAD_NOTIFY_HOT_UNPLUG\n",
 			__func__);
 
 		if (hlpe_state == hdmi_connector_status_connected) {
@@ -458,10 +457,11 @@ static void notify_audio_lpe(void *audio_ptr)
 			mid_hdmi_audio_signal_event(
 				HAD_EVENT_HOT_UNPLUG);
 		} else
-			dev_dbg(&hlpe_pdev->dev, "%s: Already Unplugged!\n",
+			dev_dbg(&pdev->dev, "%s: Already Unplugged!\n",
 				__func__);
 
-	} else if (eld != NULL) {
+	} else {
+		struct intel_hdmi_lpe_audio_eld *eld = &pdata->eld;
 
 		switch (eld->pipe_id) {
 		case 0:
@@ -474,7 +474,7 @@ static void notify_audio_lpe(void *audio_ptr)
 			ctx->had_config_offset = AUDIO_HDMI_CONFIG_C;
 			break;
 		default:
-			dev_dbg(&hlpe_pdev->dev, "Invalid pipe %d\n",
+			dev_dbg(&pdev->dev, "Invalid pipe %d\n",
 				eld->pipe_id);
 			break;
 		}
@@ -485,7 +485,7 @@ static void notify_audio_lpe(void *audio_ptr)
 
 		hlpe_state = hdmi_connector_status_connected;
 
-		dev_dbg(&hlpe_pdev->dev, "%s: HAD_NOTIFY_ELD : port = %d, tmds = %d\n",
+		dev_dbg(&pdev->dev, "%s: HAD_NOTIFY_ELD : port = %d, tmds = %d\n",
 			__func__, eld->port_id,	pdata->tmds_clock_speed);
 
 		if (pdata->tmds_clock_speed) {
@@ -494,8 +494,7 @@ static void notify_audio_lpe(void *audio_ptr)
 			ctx->link_rate = pdata->link_rate;
 			mid_hdmi_audio_signal_event(HAD_EVENT_MODE_CHANGING);
 		}
-	} else
-		dev_dbg(&hlpe_pdev->dev, "%s: Event: NULL EDID!!\n", __func__);
+	}
 }
 
 /**
@@ -606,7 +605,7 @@ static int hdmi_lpe_audio_probe(struct platform_device *pdev)
 	if (pdata->notify_pending) {
 
 		dev_dbg(&hlpe_pdev->dev, "%s: handle pending notification\n", __func__);
-		notify_audio_lpe(&pdata->eld);
+		notify_audio_lpe(pdev);
 		pdata->notify_pending = false;
 	}
 	spin_unlock_irqrestore(&pdata->lpe_audio_slock, flag_irq);
-- 
2.11.0

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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/7] drm/i915: add DisplayPort amp unmute for LPE audio mode
  2017-01-31 21:36 ` [PATCH 2/7] drm/i915: add DisplayPort amp unmute for " Takashi Iwai
@ 2017-02-01 14:45   ` Ville Syrjälä
  2017-02-01 14:53     ` Takashi Iwai
  2017-02-02  9:57   ` Takashi Iwai
  1 sibling, 1 reply; 18+ messages in thread
From: Ville Syrjälä @ 2017-02-01 14:45 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: alsa-devel, intel-gfx, rakesh.a.ughreja

On Tue, Jan 31, 2017 at 10:36:44PM +0100, Takashi Iwai wrote:
> From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> 
> Enable unmute/mute amp notification. This doesn't seem to affect
> HDMI support so this is done unconditionally.
> 
> An earlier version of this patch set a chicken bit at address 0x62F38
> prior to the mute/unmute but this register doesn't seem to do anything
> so this phase was removed.
> 
> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
>  drivers/gpu/drm/i915/i915_reg.h        | 10 ++++++++++
>  drivers/gpu/drm/i915/intel_lpe_audio.c | 17 +++++++++++++++++
>  2 files changed, 27 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index a9ffc8df241b..8fcc80cb864b 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2061,6 +2061,16 @@ 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 VLV_AUD_PORT_EN_B_DBG		(VLV_DISPLAY_BASE + 0x62F20)
> +#define VLV_AUD_PORT_EN_C_DBG		(VLV_DISPLAY_BASE + 0x62F30)
> +#define VLV_AUD_PORT_EN_D_DBG		(VLV_DISPLAY_BASE + 0x62F34)
> +#define VLV_AUD_PORT_EN_DBG(port)	_MMIO_PORT3((port) - PORT_B,	   \
> +						    VLV_AUD_PORT_EN_B_DBG, \
> +						    VLV_AUD_PORT_EN_C_DBG, \
> +						    VLV_AUD_PORT_EN_D_DBG)
> +#define VLV_AMP_MUTE		        (1 << 1)
> +
>  #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 245523e14418..f95624a46f27 100644
> --- a/drivers/gpu/drm/i915/intel_lpe_audio.c
> +++ b/drivers/gpu/drm/i915/intel_lpe_audio.c
> @@ -337,15 +337,25 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv,
>  {
>  	unsigned long irq_flags;
>  	struct intel_hdmi_lpe_audio_pdata *pdata = NULL;
> +	u32 audio_enable;
> +	i915_reg_t mmio;
>  
>  	if (!HAS_LPE_AUDIO(dev_priv))
>  		return;
>  
> +	if (port == PORT_A) {
> +		DRM_ERROR("PORT_A is not valid for HDMI/DP usages\n");
> +		return;
> +	}

Port A doesn't exists on these platforms at all. So this is
just dead code.

> +
>  	pdata = dev_get_platdata(
>  		&(dev_priv->lpe_audio.platdev->dev));
>  
>  	spin_lock_irqsave(&pdata->lpe_audio_slock, irq_flags);
>  
> +	mmio = VLV_AUD_PORT_EN_DBG(port);

I'm not a fan of these temporary reg offset variables. Makes it hard to
figure out later which register is being frobbed, especially when the
variable name doesn't say what it actually is (like in this case). So
I'd like to see this killed off.

> +	audio_enable = I915_READ(mmio);
> +
>  	if (eld != NULL) {
>  		memcpy(pdata->eld.eld_data, eld,
>  			HDMI_MAX_ELD_BYTES);
> @@ -357,11 +367,18 @@ 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;
> +
> +		/* Unmute the amp for both DP and HDMI */
> +		I915_WRITE(mmio, audio_enable & ~VLV_AMP_MUTE);
> +
>  	} else {
>  		memset(pdata->eld.eld_data, 0,
>  			HDMI_MAX_ELD_BYTES);
>  		pdata->hdmi_connected = false;
>  		pdata->dp_output = false;
> +
> +		/* Mute the amp for both DP and HDMI */
> +		I915_WRITE(mmio, audio_enable | VLV_AMP_MUTE);
>  	}
>  
>  	if (pdata->notify_audio_lpe)
> -- 
> 2.11.0

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/7] drm/i915: add DisplayPort amp unmute for LPE audio mode
  2017-02-01 14:45   ` Ville Syrjälä
@ 2017-02-01 14:53     ` Takashi Iwai
  2017-02-01 15:11       ` Ville Syrjälä
  0 siblings, 1 reply; 18+ messages in thread
From: Takashi Iwai @ 2017-02-01 14:53 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: alsa-devel, intel-gfx, rakesh.a.ughreja

On Wed, 01 Feb 2017 15:45:24 +0100,
Ville Syrjälä wrote:
> 
> On Tue, Jan 31, 2017 at 10:36:44PM +0100, Takashi Iwai wrote:
> > From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> > 
> > Enable unmute/mute amp notification. This doesn't seem to affect
> > HDMI support so this is done unconditionally.
> > 
> > An earlier version of this patch set a chicken bit at address 0x62F38
> > prior to the mute/unmute but this register doesn't seem to do anything
> > so this phase was removed.
> > 
> > Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h        | 10 ++++++++++
> >  drivers/gpu/drm/i915/intel_lpe_audio.c | 17 +++++++++++++++++
> >  2 files changed, 27 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index a9ffc8df241b..8fcc80cb864b 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -2061,6 +2061,16 @@ 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 VLV_AUD_PORT_EN_B_DBG		(VLV_DISPLAY_BASE + 0x62F20)
> > +#define VLV_AUD_PORT_EN_C_DBG		(VLV_DISPLAY_BASE + 0x62F30)
> > +#define VLV_AUD_PORT_EN_D_DBG		(VLV_DISPLAY_BASE + 0x62F34)
> > +#define VLV_AUD_PORT_EN_DBG(port)	_MMIO_PORT3((port) - PORT_B,	   \
> > +						    VLV_AUD_PORT_EN_B_DBG, \
> > +						    VLV_AUD_PORT_EN_C_DBG, \
> > +						    VLV_AUD_PORT_EN_D_DBG)
> > +#define VLV_AMP_MUTE		        (1 << 1)
> > +
> >  #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 245523e14418..f95624a46f27 100644
> > --- a/drivers/gpu/drm/i915/intel_lpe_audio.c
> > +++ b/drivers/gpu/drm/i915/intel_lpe_audio.c
> > @@ -337,15 +337,25 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv,
> >  {
> >  	unsigned long irq_flags;
> >  	struct intel_hdmi_lpe_audio_pdata *pdata = NULL;
> > +	u32 audio_enable;
> > +	i915_reg_t mmio;
> >  
> >  	if (!HAS_LPE_AUDIO(dev_priv))
> >  		return;
> >  
> > +	if (port == PORT_A) {
> > +		DRM_ERROR("PORT_A is not valid for HDMI/DP usages\n");
> > +		return;
> > +	}
> 
> Port A doesn't exists on these platforms at all. So this is
> just dead code.
> 
> > +
> >  	pdata = dev_get_platdata(
> >  		&(dev_priv->lpe_audio.platdev->dev));
> >  
> >  	spin_lock_irqsave(&pdata->lpe_audio_slock, irq_flags);
> >  
> > +	mmio = VLV_AUD_PORT_EN_DBG(port);
> 
> I'm not a fan of these temporary reg offset variables. Makes it hard to
> figure out later which register is being frobbed, especially when the
> variable name doesn't say what it actually is (like in this case). So
> I'd like to see this killed off.

How about something like below?


thanks,

Takashi

-- 8< --
From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Subject: [PATCH v2] drm/i915: add DisplayPort amp unmute for LPE audio mode

Enable unmute/mute amp notification. This doesn't seem to affect
HDMI support so this is done unconditionally.

An earlier version of this patch set a chicken bit at address 0x62F38
prior to the mute/unmute but this register doesn't seem to do anything
so this phase was removed.

v1->v2: Drop needless pipe A check, avoid temporary reg offset variable.

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

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index a9ffc8df241b..8fcc80cb864b 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2061,6 +2061,16 @@ 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 VLV_AUD_PORT_EN_B_DBG		(VLV_DISPLAY_BASE + 0x62F20)
+#define VLV_AUD_PORT_EN_C_DBG		(VLV_DISPLAY_BASE + 0x62F30)
+#define VLV_AUD_PORT_EN_D_DBG		(VLV_DISPLAY_BASE + 0x62F34)
+#define VLV_AUD_PORT_EN_DBG(port)	_MMIO_PORT3((port) - PORT_B,	   \
+						    VLV_AUD_PORT_EN_B_DBG, \
+						    VLV_AUD_PORT_EN_C_DBG, \
+						    VLV_AUD_PORT_EN_D_DBG)
+#define VLV_AMP_MUTE		        (1 << 1)
+
 #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 245523e14418..5da14f40f94a 100644
--- a/drivers/gpu/drm/i915/intel_lpe_audio.c
+++ b/drivers/gpu/drm/i915/intel_lpe_audio.c
@@ -337,6 +337,7 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv,
 {
 	unsigned long irq_flags;
 	struct intel_hdmi_lpe_audio_pdata *pdata = NULL;
+	u32 audio_enable;
 
 	if (!HAS_LPE_AUDIO(dev_priv))
 		return;
@@ -346,6 +347,8 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv,
 
 	spin_lock_irqsave(&pdata->lpe_audio_slock, irq_flags);
 
+	audio_enable = I915_READ(VLV_AUD_PORT_EN_DBG(port));
+
 	if (eld != NULL) {
 		memcpy(pdata->eld.eld_data, eld,
 			HDMI_MAX_ELD_BYTES);
@@ -357,11 +360,20 @@ 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;
+
+		/* Unmute the amp for both DP and HDMI */
+		I915_WRITE(VLV_AUD_PORT_EN_DBG(port),
+			   audio_enable & ~VLV_AMP_MUTE);
+
 	} else {
 		memset(pdata->eld.eld_data, 0,
 			HDMI_MAX_ELD_BYTES);
 		pdata->hdmi_connected = false;
 		pdata->dp_output = false;
+
+		/* Mute the amp for both DP and HDMI */
+		I915_WRITE(VLV_AUD_PORT_EN_DBG(port),
+			   audio_enable | VLV_AMP_MUTE);
 	}
 
 	if (pdata->notify_audio_lpe)
-- 
2.11.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/7] drm/i915: add DisplayPort amp unmute for LPE audio mode
  2017-02-01 14:53     ` Takashi Iwai
@ 2017-02-01 15:11       ` Ville Syrjälä
  2017-02-01 15:24         ` Takashi Iwai
  0 siblings, 1 reply; 18+ messages in thread
From: Ville Syrjälä @ 2017-02-01 15:11 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: alsa-devel, intel-gfx, rakesh.a.ughreja

On Wed, Feb 01, 2017 at 03:53:49PM +0100, Takashi Iwai wrote:
> On Wed, 01 Feb 2017 15:45:24 +0100,
> Ville Syrjälä wrote:
> > 
> > On Tue, Jan 31, 2017 at 10:36:44PM +0100, Takashi Iwai wrote:
> > > From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> > > 
> > > Enable unmute/mute amp notification. This doesn't seem to affect
> > > HDMI support so this is done unconditionally.
> > > 
> > > An earlier version of this patch set a chicken bit at address 0x62F38
> > > prior to the mute/unmute but this register doesn't seem to do anything
> > > so this phase was removed.
> > > 
> > > Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> > > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> > > ---
> > >  drivers/gpu/drm/i915/i915_reg.h        | 10 ++++++++++
> > >  drivers/gpu/drm/i915/intel_lpe_audio.c | 17 +++++++++++++++++
> > >  2 files changed, 27 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > > index a9ffc8df241b..8fcc80cb864b 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -2061,6 +2061,16 @@ 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 VLV_AUD_PORT_EN_B_DBG		(VLV_DISPLAY_BASE + 0x62F20)
> > > +#define VLV_AUD_PORT_EN_C_DBG		(VLV_DISPLAY_BASE + 0x62F30)
> > > +#define VLV_AUD_PORT_EN_D_DBG		(VLV_DISPLAY_BASE + 0x62F34)
> > > +#define VLV_AUD_PORT_EN_DBG(port)	_MMIO_PORT3((port) - PORT_B,	   \
> > > +						    VLV_AUD_PORT_EN_B_DBG, \
> > > +						    VLV_AUD_PORT_EN_C_DBG, \
> > > +						    VLV_AUD_PORT_EN_D_DBG)
> > > +#define VLV_AMP_MUTE		        (1 << 1)
> > > +
> > >  #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 245523e14418..f95624a46f27 100644
> > > --- a/drivers/gpu/drm/i915/intel_lpe_audio.c
> > > +++ b/drivers/gpu/drm/i915/intel_lpe_audio.c
> > > @@ -337,15 +337,25 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv,
> > >  {
> > >  	unsigned long irq_flags;
> > >  	struct intel_hdmi_lpe_audio_pdata *pdata = NULL;
> > > +	u32 audio_enable;
> > > +	i915_reg_t mmio;
> > >  
> > >  	if (!HAS_LPE_AUDIO(dev_priv))
> > >  		return;
> > >  
> > > +	if (port == PORT_A) {
> > > +		DRM_ERROR("PORT_A is not valid for HDMI/DP usages\n");
> > > +		return;
> > > +	}
> > 
> > Port A doesn't exists on these platforms at all. So this is
> > just dead code.
> > 
> > > +
> > >  	pdata = dev_get_platdata(
> > >  		&(dev_priv->lpe_audio.platdev->dev));
> > >  
> > >  	spin_lock_irqsave(&pdata->lpe_audio_slock, irq_flags);
> > >  
> > > +	mmio = VLV_AUD_PORT_EN_DBG(port);
> > 
> > I'm not a fan of these temporary reg offset variables. Makes it hard to
> > figure out later which register is being frobbed, especially when the
> > variable name doesn't say what it actually is (like in this case). So
> > I'd like to see this killed off.
> 
> How about something like below?
> 
> 
> thanks,
> 
> Takashi
> 
> -- 8< --
> From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> Subject: [PATCH v2] drm/i915: add DisplayPort amp unmute for LPE audio mode
> 
> Enable unmute/mute amp notification. This doesn't seem to affect
> HDMI support so this is done unconditionally.
> 
> An earlier version of this patch set a chicken bit at address 0x62F38
> prior to the mute/unmute but this register doesn't seem to do anything
> so this phase was removed.
> 
> v1->v2: Drop needless pipe A check, avoid temporary reg offset variable.
> 
> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
>  drivers/gpu/drm/i915/i915_reg.h        | 10 ++++++++++
>  drivers/gpu/drm/i915/intel_lpe_audio.c | 12 ++++++++++++
>  2 files changed, 22 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index a9ffc8df241b..8fcc80cb864b 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2061,6 +2061,16 @@ 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 VLV_AUD_PORT_EN_B_DBG		(VLV_DISPLAY_BASE + 0x62F20)
> +#define VLV_AUD_PORT_EN_C_DBG		(VLV_DISPLAY_BASE + 0x62F30)
> +#define VLV_AUD_PORT_EN_D_DBG		(VLV_DISPLAY_BASE + 0x62F34)

We generally like to prefix the raw register offsets with '_' to make
it clear they're not to be used directly.

With that fixed the i915 patches are
Acked-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> +#define VLV_AUD_PORT_EN_DBG(port)	_MMIO_PORT3((port) - PORT_B,	   \
> +						    VLV_AUD_PORT_EN_B_DBG, \
> +						    VLV_AUD_PORT_EN_C_DBG, \
> +						    VLV_AUD_PORT_EN_D_DBG)
> +#define VLV_AMP_MUTE		        (1 << 1)
> +
>  #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 245523e14418..5da14f40f94a 100644
> --- a/drivers/gpu/drm/i915/intel_lpe_audio.c
> +++ b/drivers/gpu/drm/i915/intel_lpe_audio.c
> @@ -337,6 +337,7 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv,
>  {
>  	unsigned long irq_flags;
>  	struct intel_hdmi_lpe_audio_pdata *pdata = NULL;
> +	u32 audio_enable;
>  
>  	if (!HAS_LPE_AUDIO(dev_priv))
>  		return;
> @@ -346,6 +347,8 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv,
>  
>  	spin_lock_irqsave(&pdata->lpe_audio_slock, irq_flags);
>  
> +	audio_enable = I915_READ(VLV_AUD_PORT_EN_DBG(port));
> +
>  	if (eld != NULL) {
>  		memcpy(pdata->eld.eld_data, eld,
>  			HDMI_MAX_ELD_BYTES);
> @@ -357,11 +360,20 @@ 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;
> +
> +		/* Unmute the amp for both DP and HDMI */
> +		I915_WRITE(VLV_AUD_PORT_EN_DBG(port),
> +			   audio_enable & ~VLV_AMP_MUTE);
> +
>  	} else {
>  		memset(pdata->eld.eld_data, 0,
>  			HDMI_MAX_ELD_BYTES);
>  		pdata->hdmi_connected = false;
>  		pdata->dp_output = false;
> +
> +		/* Mute the amp for both DP and HDMI */
> +		I915_WRITE(VLV_AUD_PORT_EN_DBG(port),
> +			   audio_enable | VLV_AMP_MUTE);
>  	}
>  
>  	if (pdata->notify_audio_lpe)
> -- 
> 2.11.0

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/7] drm/i915: add DisplayPort amp unmute for LPE audio mode
  2017-02-01 15:11       ` Ville Syrjälä
@ 2017-02-01 15:24         ` Takashi Iwai
  0 siblings, 0 replies; 18+ messages in thread
From: Takashi Iwai @ 2017-02-01 15:24 UTC (permalink / raw)
  To: Ville Syrjälä
  Cc: alsa-devel, intel-gfx, Pierre-Louis Bossart, Jani Nikula,
	rakesh.a.ughreja, Jerome Anand, Daniel Vetter

On Wed, 01 Feb 2017 16:11:37 +0100,
Ville Syrjälä wrote:
> 
> > +/* DisplayPort Audio w/ LPE */
> > +#define VLV_AUD_PORT_EN_B_DBG		(VLV_DISPLAY_BASE + 0x62F20)
> > +#define VLV_AUD_PORT_EN_C_DBG		(VLV_DISPLAY_BASE + 0x62F30)
> > +#define VLV_AUD_PORT_EN_D_DBG		(VLV_DISPLAY_BASE + 0x62F34)
> 
> We generally like to prefix the raw register offsets with '_' to make
> it clear they're not to be used directly.
> 
> With that fixed the i915 patches are
> Acked-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Alright, below is the respinned one.


Thanks!

Takashi

-- 8< --
From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Subject: [PATCH v3] drm/i915: add DisplayPort amp unmute for LPE audio mode

Enable unmute/mute amp notification. This doesn't seem to affect
HDMI support so this is done unconditionally.

An earlier version of this patch set a chicken bit at address 0x62F38
prior to the mute/unmute but this register doesn't seem to do anything
so this phase was removed.

v1->v2: Drop needless pipe A check, avoid temporary reg offset variable.
v2->v3: Add "_" prefix to VLV_AUD_PORT_EN_X_DBG as they are internal.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Acked-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 drivers/gpu/drm/i915/i915_reg.h        | 10 ++++++++++
 drivers/gpu/drm/i915/intel_lpe_audio.c | 12 ++++++++++++
 2 files changed, 22 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index a9ffc8df241b..4e24ba0cdbe8 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2061,6 +2061,16 @@ 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 _VLV_AUD_PORT_EN_B_DBG		(VLV_DISPLAY_BASE + 0x62F20)
+#define _VLV_AUD_PORT_EN_C_DBG		(VLV_DISPLAY_BASE + 0x62F30)
+#define _VLV_AUD_PORT_EN_D_DBG		(VLV_DISPLAY_BASE + 0x62F34)
+#define VLV_AUD_PORT_EN_DBG(port)	_MMIO_PORT3((port) - PORT_B,	   \
+						    _VLV_AUD_PORT_EN_B_DBG, \
+						    _VLV_AUD_PORT_EN_C_DBG, \
+						    _VLV_AUD_PORT_EN_D_DBG)
+#define VLV_AMP_MUTE		        (1 << 1)
+
 #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 245523e14418..5da14f40f94a 100644
--- a/drivers/gpu/drm/i915/intel_lpe_audio.c
+++ b/drivers/gpu/drm/i915/intel_lpe_audio.c
@@ -337,6 +337,7 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv,
 {
 	unsigned long irq_flags;
 	struct intel_hdmi_lpe_audio_pdata *pdata = NULL;
+	u32 audio_enable;
 
 	if (!HAS_LPE_AUDIO(dev_priv))
 		return;
@@ -346,6 +347,8 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv,
 
 	spin_lock_irqsave(&pdata->lpe_audio_slock, irq_flags);
 
+	audio_enable = I915_READ(VLV_AUD_PORT_EN_DBG(port));
+
 	if (eld != NULL) {
 		memcpy(pdata->eld.eld_data, eld,
 			HDMI_MAX_ELD_BYTES);
@@ -357,11 +360,20 @@ 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;
+
+		/* Unmute the amp for both DP and HDMI */
+		I915_WRITE(VLV_AUD_PORT_EN_DBG(port),
+			   audio_enable & ~VLV_AMP_MUTE);
+
 	} else {
 		memset(pdata->eld.eld_data, 0,
 			HDMI_MAX_ELD_BYTES);
 		pdata->hdmi_connected = false;
 		pdata->dp_output = false;
+
+		/* Mute the amp for both DP and HDMI */
+		I915_WRITE(VLV_AUD_PORT_EN_DBG(port),
+			   audio_enable | VLV_AMP_MUTE);
 	}
 
 	if (pdata->notify_audio_lpe)
-- 
2.11.0

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/7] drm/i915: add DisplayPort amp unmute for LPE audio mode
  2017-01-31 21:36 ` [PATCH 2/7] drm/i915: add DisplayPort amp unmute for " Takashi Iwai
  2017-02-01 14:45   ` Ville Syrjälä
@ 2017-02-02  9:57   ` Takashi Iwai
  2017-02-02 10:06     ` Ville Syrjälä
  1 sibling, 1 reply; 18+ messages in thread
From: Takashi Iwai @ 2017-02-02  9:57 UTC (permalink / raw)
  To: Pierre-Louis Bossart; +Cc: alsa-devel, intel-gfx, rakesh.a.ughreja

On Tue, 31 Jan 2017 22:36:44 +0100,
Takashi Iwai wrote:
> 
> From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> 
> Enable unmute/mute amp notification. This doesn't seem to affect
> HDMI support so this is done unconditionally.
> 
> An earlier version of this patch set a chicken bit at address 0x62F38
> prior to the mute/unmute but this register doesn't seem to do anything
> so this phase was removed.

Sorry, this seems like a wrong result.  It worked yesterday, but when
I retested the latest code today, it didn't work any longer.
Then, after setting the chicken bit, it starts working again.

But now I can't make it broken again.  Turn off the monitor, turn off
the machine, DPMS off, suspend/resume...  All still works without
chicken bit set.

Also, the chicken bit of the register 0x62f38 couldn't be read back.
The register always returns zero when read.

So the chicken bit helps definitely something on a device here, but
it's still mystery how it works.

I'll send an additional patch to re-add the audio chicken bit stuff.


thanks,

Takashi

> 
> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
>  drivers/gpu/drm/i915/i915_reg.h        | 10 ++++++++++
>  drivers/gpu/drm/i915/intel_lpe_audio.c | 17 +++++++++++++++++
>  2 files changed, 27 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index a9ffc8df241b..8fcc80cb864b 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2061,6 +2061,16 @@ 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 VLV_AUD_PORT_EN_B_DBG		(VLV_DISPLAY_BASE + 0x62F20)
> +#define VLV_AUD_PORT_EN_C_DBG		(VLV_DISPLAY_BASE + 0x62F30)
> +#define VLV_AUD_PORT_EN_D_DBG		(VLV_DISPLAY_BASE + 0x62F34)
> +#define VLV_AUD_PORT_EN_DBG(port)	_MMIO_PORT3((port) - PORT_B,	   \
> +						    VLV_AUD_PORT_EN_B_DBG, \
> +						    VLV_AUD_PORT_EN_C_DBG, \
> +						    VLV_AUD_PORT_EN_D_DBG)
> +#define VLV_AMP_MUTE		        (1 << 1)
> +
>  #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 245523e14418..f95624a46f27 100644
> --- a/drivers/gpu/drm/i915/intel_lpe_audio.c
> +++ b/drivers/gpu/drm/i915/intel_lpe_audio.c
> @@ -337,15 +337,25 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv,
>  {
>  	unsigned long irq_flags;
>  	struct intel_hdmi_lpe_audio_pdata *pdata = NULL;
> +	u32 audio_enable;
> +	i915_reg_t mmio;
>  
>  	if (!HAS_LPE_AUDIO(dev_priv))
>  		return;
>  
> +	if (port == PORT_A) {
> +		DRM_ERROR("PORT_A is not valid for HDMI/DP usages\n");
> +		return;
> +	}
> +
>  	pdata = dev_get_platdata(
>  		&(dev_priv->lpe_audio.platdev->dev));
>  
>  	spin_lock_irqsave(&pdata->lpe_audio_slock, irq_flags);
>  
> +	mmio = VLV_AUD_PORT_EN_DBG(port);
> +	audio_enable = I915_READ(mmio);
> +
>  	if (eld != NULL) {
>  		memcpy(pdata->eld.eld_data, eld,
>  			HDMI_MAX_ELD_BYTES);
> @@ -357,11 +367,18 @@ 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;
> +
> +		/* Unmute the amp for both DP and HDMI */
> +		I915_WRITE(mmio, audio_enable & ~VLV_AMP_MUTE);
> +
>  	} else {
>  		memset(pdata->eld.eld_data, 0,
>  			HDMI_MAX_ELD_BYTES);
>  		pdata->hdmi_connected = false;
>  		pdata->dp_output = false;
> +
> +		/* Mute the amp for both DP and HDMI */
> +		I915_WRITE(mmio, audio_enable | VLV_AMP_MUTE);
>  	}
>  
>  	if (pdata->notify_audio_lpe)
> -- 
> 2.11.0
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/7] drm/i915: add DisplayPort amp unmute for LPE audio mode
  2017-02-02  9:57   ` Takashi Iwai
@ 2017-02-02 10:06     ` Ville Syrjälä
  2017-02-02 10:13       ` Takashi Iwai
  0 siblings, 1 reply; 18+ messages in thread
From: Ville Syrjälä @ 2017-02-02 10:06 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: alsa-devel, intel-gfx, rakesh.a.ughreja

On Thu, Feb 02, 2017 at 10:57:30AM +0100, Takashi Iwai wrote:
> On Tue, 31 Jan 2017 22:36:44 +0100,
> Takashi Iwai wrote:
> > 
> > From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> > 
> > Enable unmute/mute amp notification. This doesn't seem to affect
> > HDMI support so this is done unconditionally.
> > 
> > An earlier version of this patch set a chicken bit at address 0x62F38
> > prior to the mute/unmute but this register doesn't seem to do anything
> > so this phase was removed.
> 
> Sorry, this seems like a wrong result.  It worked yesterday, but when
> I retested the latest code today, it didn't work any longer.
> Then, after setting the chicken bit, it starts working again.
> 
> But now I can't make it broken again.  Turn off the monitor, turn off
> the machine, DPMS off, suspend/resume...  All still works without
> chicken bit set.
> 
> Also, the chicken bit of the register 0x62f38 couldn't be read back.
> The register always returns zero when read.

The docs do say it's write-only actually.

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/7] drm/i915: add DisplayPort amp unmute for LPE audio mode
  2017-02-02 10:06     ` Ville Syrjälä
@ 2017-02-02 10:13       ` Takashi Iwai
  0 siblings, 0 replies; 18+ messages in thread
From: Takashi Iwai @ 2017-02-02 10:13 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: alsa-devel, intel-gfx, rakesh.a.ughreja

On Thu, 02 Feb 2017 11:06:05 +0100,
Ville Syrjälä wrote:
> 
> On Thu, Feb 02, 2017 at 10:57:30AM +0100, Takashi Iwai wrote:
> > On Tue, 31 Jan 2017 22:36:44 +0100,
> > Takashi Iwai wrote:
> > > 
> > > From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> > > 
> > > Enable unmute/mute amp notification. This doesn't seem to affect
> > > HDMI support so this is done unconditionally.
> > > 
> > > An earlier version of this patch set a chicken bit at address 0x62F38
> > > prior to the mute/unmute but this register doesn't seem to do anything
> > > so this phase was removed.
> > 
> > Sorry, this seems like a wrong result.  It worked yesterday, but when
> > I retested the latest code today, it didn't work any longer.
> > Then, after setting the chicken bit, it starts working again.
> > 
> > But now I can't make it broken again.  Turn off the monitor, turn off
> > the machine, DPMS off, suspend/resume...  All still works without
> > chicken bit set.
> > 
> > Also, the chicken bit of the register 0x62f38 couldn't be read back.
> > The register always returns zero when read.
> 
> The docs do say it's write-only actually.

OK, thanks for confirmation!


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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/7] DisplayPort audio support on Cherrytrail
  2017-01-31 21:36 [PATCH 0/7] DisplayPort audio support on Cherrytrail Takashi Iwai
                   ` (6 preceding siblings ...)
  2017-01-31 21:36 ` [PATCH 7/7] drm/i915: Pass platform device to LPE audio notifier Takashi Iwai
@ 2017-03-13  8:33 ` Daniel Vetter
  2017-03-31  6:29   ` Daniel Vetter
  7 siblings, 1 reply; 18+ messages in thread
From: Daniel Vetter @ 2017-03-13  8:33 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: alsa-devel, intel-gfx, Pierre-Louis Bossart, Jani Nikula,
	rakesh.a.ughreja, Jerome Anand, Daniel Vetter,
	Ville Syrjälä

On Tue, Jan 31, 2017 at 10:36:42PM +0100, Takashi Iwai wrote:
> Hi,
> 
> the following patches enable DisplayPort Audio on Cherrytrail machines
> when applied on top of my topic/intel-lpe-audio branch.  Tests of DP
> audio were run on Dell Wyse 3040.  The regression test were performed
> on Baytrail (Compute Stick) and Cherrytrail (Zotac PI330) in HDMI
> mode.  On Cherrytrail there were no issues changing between HDMI and
> DP connectors dynamically.
> 
> Could you i915 people review and give ACK if they are OK?
> The changes in drm/i915 side are fairly trivial, so I'd like to take
> them through sound git tree once after I receive your ACKs.
> 
> 
> Changes since RFC:
>  - reordered and squashed patches
>  - clean-up of register definitions and offsets (based on feedback from
>    Jani/Ville)
>  - unmute amp for both HDMI and DP unconditionally
>  - mute amp on invalid ELD (unplug)
>  - remove test for chicken bit which seems to have no effect in hardware
>  - cosmetic edits to make checkpatch happy
>  - change i915 notification argument to pass the plataform device
>    instead
> 
> 
> Most of hard work in this patchset has been done by Pierre, so all
> credits go to him.

Please build the htmldocs and fix the new fallout these patches create:

$ make   DOCBOOKS="" htmldocs

0day should be reporting these (if it scans your mailing list), but
there's been a hiccup recently.

Good docs matter and all that ...

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/7] DisplayPort audio support on Cherrytrail
  2017-03-13  8:33 ` [PATCH 0/7] DisplayPort audio support on Cherrytrail Daniel Vetter
@ 2017-03-31  6:29   ` Daniel Vetter
  2017-03-31  6:40     ` Takashi Iwai
  0 siblings, 1 reply; 18+ messages in thread
From: Daniel Vetter @ 2017-03-31  6:29 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: alsa-devel, intel-gfx, Rakesh A Ughreja

On Mon, Mar 13, 2017 at 9:33 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Tue, Jan 31, 2017 at 10:36:42PM +0100, Takashi Iwai wrote:
>> Hi,
>>
>> the following patches enable DisplayPort Audio on Cherrytrail machines
>> when applied on top of my topic/intel-lpe-audio branch.  Tests of DP
>> audio were run on Dell Wyse 3040.  The regression test were performed
>> on Baytrail (Compute Stick) and Cherrytrail (Zotac PI330) in HDMI
>> mode.  On Cherrytrail there were no issues changing between HDMI and
>> DP connectors dynamically.
>>
>> Could you i915 people review and give ACK if they are OK?
>> The changes in drm/i915 side are fairly trivial, so I'd like to take
>> them through sound git tree once after I receive your ACKs.
>>
>>
>> Changes since RFC:
>>  - reordered and squashed patches
>>  - clean-up of register definitions and offsets (based on feedback from
>>    Jani/Ville)
>>  - unmute amp for both HDMI and DP unconditionally
>>  - mute amp on invalid ELD (unplug)
>>  - remove test for chicken bit which seems to have no effect in hardware
>>  - cosmetic edits to make checkpatch happy
>>  - change i915 notification argument to pass the plataform device
>>    instead
>>
>>
>> Most of hard work in this patchset has been done by Pierre, so all
>> credits go to him.
>
> Please build the htmldocs and fix the new fallout these patches create:
>
> $ make   DOCBOOKS="" htmldocs
>
> 0day should be reporting these (if it scans your mailing list), but
> there's been a hiccup recently.
>
> Good docs matter and all that ...

Apparently this is still not fixed, I applied a fixup patch now from
someone else. Tsk.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/7] DisplayPort audio support on Cherrytrail
  2017-03-31  6:29   ` Daniel Vetter
@ 2017-03-31  6:40     ` Takashi Iwai
  0 siblings, 0 replies; 18+ messages in thread
From: Takashi Iwai @ 2017-03-31  6:40 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: alsa-devel, intel-gfx, Rakesh A Ughreja

On Fri, 31 Mar 2017 08:29:55 +0200,
Daniel Vetter wrote:
> 
> On Mon, Mar 13, 2017 at 9:33 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
> > On Tue, Jan 31, 2017 at 10:36:42PM +0100, Takashi Iwai wrote:
> >> Hi,
> >>
> >> the following patches enable DisplayPort Audio on Cherrytrail machines
> >> when applied on top of my topic/intel-lpe-audio branch.  Tests of DP
> >> audio were run on Dell Wyse 3040.  The regression test were performed
> >> on Baytrail (Compute Stick) and Cherrytrail (Zotac PI330) in HDMI
> >> mode.  On Cherrytrail there were no issues changing between HDMI and
> >> DP connectors dynamically.
> >>
> >> Could you i915 people review and give ACK if they are OK?
> >> The changes in drm/i915 side are fairly trivial, so I'd like to take
> >> them through sound git tree once after I receive your ACKs.
> >>
> >>
> >> Changes since RFC:
> >>  - reordered and squashed patches
> >>  - clean-up of register definitions and offsets (based on feedback from
> >>    Jani/Ville)
> >>  - unmute amp for both HDMI and DP unconditionally
> >>  - mute amp on invalid ELD (unplug)
> >>  - remove test for chicken bit which seems to have no effect in hardware
> >>  - cosmetic edits to make checkpatch happy
> >>  - change i915 notification argument to pass the plataform device
> >>    instead
> >>
> >>
> >> Most of hard work in this patchset has been done by Pierre, so all
> >> credits go to him.
> >
> > Please build the htmldocs and fix the new fallout these patches create:
> >
> > $ make   DOCBOOKS="" htmldocs
> >
> > 0day should be reporting these (if it scans your mailing list), but
> > there's been a hiccup recently.
> >
> > Good docs matter and all that ...
> 
> Apparently this is still not fixed, I applied a fixup patch now from
> someone else. Tsk.

Ah, sorry, this felt into crack during my vacation.
Good that it was already fixed.  Thanks.


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

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2017-03-31  6:40 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-31 21:36 [PATCH 0/7] DisplayPort audio support on Cherrytrail Takashi Iwai
2017-01-31 21:36 ` [PATCH 1/7] drm/i915: add DP support in LPE audio mode Takashi Iwai
2017-01-31 21:36 ` [PATCH 2/7] drm/i915: add DisplayPort amp unmute for " Takashi Iwai
2017-02-01 14:45   ` Ville Syrjälä
2017-02-01 14:53     ` Takashi Iwai
2017-02-01 15:11       ` Ville Syrjälä
2017-02-01 15:24         ` Takashi Iwai
2017-02-02  9:57   ` Takashi Iwai
2017-02-02 10:06     ` Ville Syrjälä
2017-02-02 10:13       ` Takashi Iwai
2017-01-31 21:36 ` [PATCH 3/7] drm/i915: Avoid MST pipe handling for LPE audio Takashi Iwai
2017-01-31 21:36 ` [PATCH 4/7] drm/i915: Pass pipe to LPE audio notification Takashi Iwai
2017-01-31 21:36 ` [PATCH 5/7] ALSA: x86: intel_hdmi: add definitions and logic for DP audio Takashi Iwai
2017-01-31 21:36 ` [PATCH 6/7] ALSA: x86: Use config base depending on the pipe Takashi Iwai
2017-01-31 21:36 ` [PATCH 7/7] drm/i915: Pass platform device to LPE audio notifier Takashi Iwai
2017-03-13  8:33 ` [PATCH 0/7] DisplayPort audio support on Cherrytrail Daniel Vetter
2017-03-31  6:29   ` Daniel Vetter
2017-03-31  6:40     ` Takashi Iwai

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).