[PATCH v2 00/11] drm/i915: LPE audio runtime PM and multipipe (v2)

[PATCH v2 00/11] drm/i915: LPE audio runtime PM and multipipe (v2)

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Okay, here's the second attempt at getting multiple pipes playing back
audio on the VLV/CHV HDMI LPE audio device. The main change from v1 is
that now the PCM devices are associated with ports instead of pipes,
so the audio from one device always gets output on the same display.

I've also tacked on the alsa-lib conf update. No clue whether it's
really correct or not (the config language isn't a close friend
of mine).

BTW I did notice that with LPE audio all the controls say iface=PCM,
whereas on HDA a bunch of them say iface=MIXER. No idea if that's
OK or not, just something I spotted when I was comparing the results
with HDA.

Entire series available here:
git://github.com/vsyrjala/linux.git lpe_audio_multipipe_2

Cc: Takashi Iwai <tiwai@suse.de>
Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

Ville Syrjälä (11):
  drm/i915: Fix runtime PM for LPE audio
  ALSA: x86: Clear the pdata.notify_lpe_audio pointer before teardown
  drm/i915: Stop pretending to mask/unmask LPE audio interrupts
  drm/i915: Remove the unused pending_notify from LPE platform data
  drm/i915: Replace tmds_clock_speed and link_rate with just ls_clock
  drm/i915: Remove hdmi_connected from LPE audio pdata
  drm/i915: Reorganize intel_lpe_audio_notify() arguments
  drm/i915: Clean up the LPE audio platform data
  ALSA: x86: Prepare LPE audio ctls for multiple PCMs
  ALSA: x86: Split snd_intelhad into card and PCM specific structures
  ALSA: x86: Register multiple PCM devices for the LPE audio card

 drivers/gpu/drm/i915/i915_drv.h        |   4 +-
 drivers/gpu/drm/i915/i915_irq.c        |  15 +-
 drivers/gpu/drm/i915/intel_audio.c     |  19 +-
 drivers/gpu/drm/i915/intel_lpe_audio.c |  99 ++++------
 include/drm/intel_lpe_audio.h          |  22 +--
 sound/x86/intel_hdmi_audio.c           | 328 ++++++++++++++++++++-------------
 sound/x86/intel_hdmi_audio.h           |  20 +-
 7 files changed, 271 insertions(+), 236 deletions(-)

-- 
2.10.2

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

[PATCH 01/11] drm/i915: Fix runtime PM for LPE audio

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Not calling pm_runtime_enable() means that runtime PM can't be
enabled at all via sysfs. So we definitely need to call it
from somewhere.

Calling it from the driver seems like a bad idea because it
would have to be paired with a pm_runtime_disable() at driver
unload time, otherwise the core gets upset. Also if there's
no LPE audio driver loaded then we couldn't runtime suspend
i915 either.

So it looks like a better plan is to call it from i915 when
we register the platform device. That seems to match how
pci generally does things. I cargo culted the
pm_runtime_forbid() and pm_runtime_set_active() calls from
pci as well.

The exposed runtime PM API is massive an thorougly misleading, so
I don't actually know if this is how you're supposed to use the API
or not. But it seems to work. I can now runtime suspend i915 again
with or without the LPE audio driver loaded, and reloading the
LPE audio driver also seems to work.

Note that powertop won't auto-tune runtime PM for platform devices,
which is a little annoying. So I'm not sure that leaving runtime
PM in "on" mode by default is the best choice here. But I've left
it like that for now at least.

Also remove the comment about there not being much benefit from
LPE audio runtime PM. Not allowing runtime PM blocks i915 runtime
PM, which will also block s0ix, and that could have a measurable
impact on power consumption.

Cc: Takashi Iwai <tiwai@suse.de>
Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Fixes: 0b6b524f3915 ("ALSA: x86: Don't enable runtime PM as default")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_lpe_audio.c | 5 +++++
 sound/x86/intel_hdmi_audio.c           | 4 ----
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c
index 25d8e76489e4..668f00480d97 100644
--- a/drivers/gpu/drm/i915/intel_lpe_audio.c
+++ b/drivers/gpu/drm/i915/intel_lpe_audio.c
@@ -63,6 +63,7 @@
 #include <linux/acpi.h>
 #include <linux/device.h>
 #include <linux/pci.h>
+#include <linux/pm_runtime.h>
 
 #include "i915_drv.h"
 #include <linux/delay.h>
@@ -121,6 +122,10 @@ lpe_audio_platdev_create(struct drm_i915_private *dev_priv)
 
 	kfree(rsc);
 
+	pm_runtime_forbid(&platdev->dev);
+	pm_runtime_set_active(&platdev->dev);
+	pm_runtime_enable(&platdev->dev);
+
 	return platdev;
 
 err:
diff --git a/sound/x86/intel_hdmi_audio.c b/sound/x86/intel_hdmi_audio.c
index c505b019e09c..bfac6f21ae5e 100644
--- a/sound/x86/intel_hdmi_audio.c
+++ b/sound/x86/intel_hdmi_audio.c
@@ -1809,10 +1809,6 @@ static int hdmi_lpe_audio_probe(struct platform_device *pdev)
 	pdata->notify_pending = false;
 	spin_unlock_irq(&pdata->lpe_audio_slock);
 
-	/* runtime PM isn't enabled as default, since it won't save much on
-	 * BYT/CHT devices; user who want the runtime PM should adjust the
-	 * power/ontrol and power/autosuspend_delay_ms sysfs entries instead
-	 */
 	pm_runtime_use_autosuspend(&pdev->dev);
 	pm_runtime_mark_last_busy(&pdev->dev);
 	pm_runtime_set_active(&pdev->dev);
-- 
2.10.2

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

[PATCH 02/11] ALSA: x86: Clear the pdata.notify_lpe_audio pointer before teardown

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Clear the notify function pointer in the platform data before we tear
down the driver. Otherwise i915 would end up calling a stale function
pointer and possibly explode.

Cc: Takashi Iwai <tiwai@suse.de>
Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 sound/x86/intel_hdmi_audio.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/sound/x86/intel_hdmi_audio.c b/sound/x86/intel_hdmi_audio.c
index bfac6f21ae5e..5b89662493c9 100644
--- a/sound/x86/intel_hdmi_audio.c
+++ b/sound/x86/intel_hdmi_audio.c
@@ -1665,6 +1665,11 @@ static int __maybe_unused hdmi_lpe_audio_resume(struct device *dev)
 static void hdmi_lpe_audio_free(struct snd_card *card)
 {
 	struct snd_intelhad *ctx = card->private_data;
+	struct intel_hdmi_lpe_audio_pdata *pdata = ctx->dev->platform_data;
+
+	spin_lock_irq(&pdata->lpe_audio_slock);
+	pdata->notify_audio_lpe = NULL;
+	spin_unlock_irq(&pdata->lpe_audio_slock);
 
 	cancel_work_sync(&ctx->hdmi_audio_wq);
 
-- 
2.10.2

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

[PATCH 03/11] drm/i915: Stop pretending to mask/unmask LPE audio interrupts

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

vlv_display_irq_postinstall() enables the LPE audio interrupts
regardless of whether the LPE audio irq chip has masked/unmasked
them. Also the irqchip masking/unmasking doesn't consider the state
of the display power well or the device, and hence just leads to
dmesg spew when it tries to access the hardware while it's powered
down.

If the current way works, then we don't need to do anything in the
mask/unmask hooks. If it doesn't work, well, then we'd need to properly
track whether the irqchip has masked/unmasked the interrupts when
we enable display interrupts. And the mask/unmask hooks would need
to check whether display interrupts are even enabled before frobbing
with he registers.

So let's just assume the current way works and neuter the mask/unmask
hooks. Also clean up vlv_display_irq_postinstall() a bit and stop
it from trying to unmask/enable the LPE C interrupt on VLV since it
doesn't exist.

Cc: Takashi Iwai <tiwai@suse.de>
Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c        | 15 ++++++--------
 drivers/gpu/drm/i915/intel_lpe_audio.c | 36 ----------------------------------
 2 files changed, 6 insertions(+), 45 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index fd97fe00cd0d..190f6aa5d15e 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2953,7 +2953,6 @@ static void vlv_display_irq_postinstall(struct drm_i915_private *dev_priv)
 	u32 pipestat_mask;
 	u32 enable_mask;
 	enum pipe pipe;
-	u32 val;
 
 	pipestat_mask = PLANE_FLIP_DONE_INT_STATUS_VLV |
 			PIPE_CRC_DONE_INTERRUPT_STATUS;
@@ -2964,18 +2963,16 @@ static void vlv_display_irq_postinstall(struct drm_i915_private *dev_priv)
 
 	enable_mask = I915_DISPLAY_PORT_INTERRUPT |
 		I915_DISPLAY_PIPE_A_EVENT_INTERRUPT |
-		I915_DISPLAY_PIPE_B_EVENT_INTERRUPT;
+		I915_DISPLAY_PIPE_B_EVENT_INTERRUPT |
+		I915_LPE_PIPE_A_INTERRUPT |
+		I915_LPE_PIPE_B_INTERRUPT;
+
 	if (IS_CHERRYVIEW(dev_priv))
-		enable_mask |= I915_DISPLAY_PIPE_C_EVENT_INTERRUPT;
+		enable_mask |= I915_DISPLAY_PIPE_C_EVENT_INTERRUPT |
+			I915_LPE_PIPE_C_INTERRUPT;
 
 	WARN_ON(dev_priv->irq_mask != ~0);
 
-	val = (I915_LPE_PIPE_A_INTERRUPT |
-		I915_LPE_PIPE_B_INTERRUPT |
-		I915_LPE_PIPE_C_INTERRUPT);
-
-	enable_mask |= val;
-
 	dev_priv->irq_mask = ~enable_mask;
 
 	GEN5_IRQ_INIT(VLV_, dev_priv->irq_mask, enable_mask);
diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c
index 668f00480d97..292fedf30b00 100644
--- a/drivers/gpu/drm/i915/intel_lpe_audio.c
+++ b/drivers/gpu/drm/i915/intel_lpe_audio.c
@@ -149,44 +149,10 @@ static void lpe_audio_platdev_destroy(struct drm_i915_private *dev_priv)
 
 static void lpe_audio_irq_unmask(struct irq_data *d)
 {
-	struct drm_i915_private *dev_priv = d->chip_data;
-	unsigned long irqflags;
-	u32 val = (I915_LPE_PIPE_A_INTERRUPT |
-		I915_LPE_PIPE_B_INTERRUPT);
-
-	if (IS_CHERRYVIEW(dev_priv))
-		val |= I915_LPE_PIPE_C_INTERRUPT;
-
-	spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
-
-	dev_priv->irq_mask &= ~val;
-	I915_WRITE(VLV_IIR, val);
-	I915_WRITE(VLV_IIR, val);
-	I915_WRITE(VLV_IMR, dev_priv->irq_mask);
-	POSTING_READ(VLV_IMR);
-
-	spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
 }
 
 static void lpe_audio_irq_mask(struct irq_data *d)
 {
-	struct drm_i915_private *dev_priv = d->chip_data;
-	unsigned long irqflags;
-	u32 val = (I915_LPE_PIPE_A_INTERRUPT |
-		I915_LPE_PIPE_B_INTERRUPT);
-
-	if (IS_CHERRYVIEW(dev_priv))
-		val |= I915_LPE_PIPE_C_INTERRUPT;
-
-	spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
-
-	dev_priv->irq_mask |= val;
-	I915_WRITE(VLV_IMR, dev_priv->irq_mask);
-	I915_WRITE(VLV_IIR, val);
-	I915_WRITE(VLV_IIR, val);
-	POSTING_READ(VLV_IIR);
-
-	spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
 }
 
 static struct irq_chip lpe_audio_irqchip = {
@@ -330,8 +296,6 @@ void intel_lpe_audio_teardown(struct drm_i915_private *dev_priv)
 
 	desc = irq_to_desc(dev_priv->lpe_audio.irq);
 
-	lpe_audio_irq_mask(&desc->irq_data);
-
 	lpe_audio_platdev_destroy(dev_priv);
 
 	irq_free_desc(dev_priv->lpe_audio.irq);
-- 
2.10.2

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

[PATCH v2 04/11] drm/i915: Remove the unused pending_notify from LPE platform data

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The pending_notify flag in the LPE audio platform data is pointless,
actually unused. So let's kill it off.

v2: Fix typo in patch subject

Cc: Takashi Iwai <tiwai@suse.de>
Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_lpe_audio.c | 2 --
 include/drm/intel_lpe_audio.h          | 1 -
 sound/x86/intel_hdmi_audio.c           | 1 -
 3 files changed, 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c
index 292fedf30b00..79b9dca985ff 100644
--- a/drivers/gpu/drm/i915/intel_lpe_audio.c
+++ b/drivers/gpu/drm/i915/intel_lpe_audio.c
@@ -361,8 +361,6 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv,
 
 	if (pdata->notify_audio_lpe)
 		pdata->notify_audio_lpe(dev_priv->lpe_audio.platdev);
-	else
-		pdata->notify_pending = true;
 
 	spin_unlock_irqrestore(&pdata->lpe_audio_slock,
 			irq_flags);
diff --git a/include/drm/intel_lpe_audio.h b/include/drm/intel_lpe_audio.h
index e9892b4c3af1..c201d39cdfea 100644
--- a/include/drm/intel_lpe_audio.h
+++ b/include/drm/intel_lpe_audio.h
@@ -38,7 +38,6 @@ struct intel_hdmi_lpe_audio_eld {
 };
 
 struct intel_hdmi_lpe_audio_pdata {
-	bool notify_pending;
 	int tmds_clock_speed;
 	bool hdmi_connected;
 	bool dp_output;
diff --git a/sound/x86/intel_hdmi_audio.c b/sound/x86/intel_hdmi_audio.c
index 5b89662493c9..cbba4a78afb5 100644
--- a/sound/x86/intel_hdmi_audio.c
+++ b/sound/x86/intel_hdmi_audio.c
@@ -1811,7 +1811,6 @@ static int hdmi_lpe_audio_probe(struct platform_device *pdev)
 
 	spin_lock_irq(&pdata->lpe_audio_slock);
 	pdata->notify_audio_lpe = notify_audio_lpe;
-	pdata->notify_pending = false;
 	spin_unlock_irq(&pdata->lpe_audio_slock);
 
 	pm_runtime_use_autosuspend(&pdev->dev);
-- 
2.10.2

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

[PATCH 05/11] drm/i915: Replace tmds_clock_speed and link_rate with just ls_clock

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

There's no need to distinguish between the DP link rate and HDMI TMDS
clock for the purposes of the LPE audio. Both are actually the same
thing more or less, which is the link symbol clock. So let's just
call the thing ls_clock and simplify the code.

Cc: Takashi Iwai <tiwai@suse.de>
Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h        |  4 ++--
 drivers/gpu/drm/i915/intel_audio.c     | 19 ++++---------------
 drivers/gpu/drm/i915/intel_lpe_audio.c | 14 ++++++--------
 include/drm/intel_lpe_audio.h          |  3 +--
 sound/x86/intel_hdmi_audio.c           | 11 ++++++++---
 5 files changed, 21 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index d1f7c48e4ae3..8bf72220ee07 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3721,8 +3721,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 pipe, int tmds_clk_speed,
-			    bool dp_output, int link_rate);
+			    void *eld, int port, int pipe, int ls_clock,
+			    bool dp_output);
 
 /* intel_i2c.c */
 extern int intel_setup_gmbus(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
index 52c207e81f41..79eeef25321f 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -632,20 +632,9 @@ void intel_audio_codec_enable(struct intel_encoder *intel_encoder,
 						 (int) port, (int) pipe);
 	}
 
-	switch (intel_encoder->type) {
-	case INTEL_OUTPUT_HDMI:
-		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, pipe,
-				       adjusted_mode->crtc_clock,
-				       true, crtc_state->port_clock);
-		break;
-	default:
-		break;
-	}
+	intel_lpe_audio_notify(dev_priv, connector->eld, port, pipe,
+			       crtc_state->port_clock,
+			       intel_encoder->type == INTEL_OUTPUT_DP);
 }
 
 /**
@@ -680,7 +669,7 @@ void intel_audio_codec_disable(struct intel_encoder *intel_encoder)
 						 (int) port, (int) pipe);
 	}
 
-	intel_lpe_audio_notify(dev_priv, NULL, port, pipe, 0, false, 0);
+	intel_lpe_audio_notify(dev_priv, NULL, port, pipe, 0, false);
 }
 
 /**
diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c
index 79b9dca985ff..5a1a37e963f1 100644
--- a/drivers/gpu/drm/i915/intel_lpe_audio.c
+++ b/drivers/gpu/drm/i915/intel_lpe_audio.c
@@ -309,13 +309,14 @@ void intel_lpe_audio_teardown(struct drm_i915_private *dev_priv)
  * @eld : ELD data
  * @pipe: pipe id
  * @port: port id
- * @tmds_clk_speed: tmds clock frequency in Hz
+ * @ls_clock: Link symbol clock in kHz
+ * @dp_output: Driving a DP output?
  *
  * Notify lpe audio driver of eld change.
  */
 void intel_lpe_audio_notify(struct drm_i915_private *dev_priv,
-			    void *eld, int port, int pipe, int tmds_clk_speed,
-			    bool dp_output, int link_rate)
+			    void *eld, int port, int pipe, int ls_clock,
+			    bool dp_output)
 {
 	unsigned long irq_flags;
 	struct intel_hdmi_lpe_audio_pdata *pdata = NULL;
@@ -337,12 +338,8 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv,
 		pdata->eld.port_id = port;
 		pdata->eld.pipe_id = pipe;
 		pdata->hdmi_connected = true;
-
+		pdata->ls_clock = ls_clock;
 		pdata->dp_output = dp_output;
-		if (tmds_clk_speed)
-			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),
@@ -352,6 +349,7 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv,
 		memset(pdata->eld.eld_data, 0,
 			HDMI_MAX_ELD_BYTES);
 		pdata->hdmi_connected = false;
+		pdata->ls_clock = 0;
 		pdata->dp_output = false;
 
 		/* Mute the amp for both DP and HDMI */
diff --git a/include/drm/intel_lpe_audio.h b/include/drm/intel_lpe_audio.h
index c201d39cdfea..8bf804ce8905 100644
--- a/include/drm/intel_lpe_audio.h
+++ b/include/drm/intel_lpe_audio.h
@@ -38,10 +38,9 @@ struct intel_hdmi_lpe_audio_eld {
 };
 
 struct intel_hdmi_lpe_audio_pdata {
-	int tmds_clock_speed;
+	int ls_clock;
 	bool hdmi_connected;
 	bool dp_output;
-	int link_rate;
 	struct intel_hdmi_lpe_audio_eld eld;
 	void (*notify_audio_lpe)(struct platform_device *pdev);
 	spinlock_t lpe_audio_slock;
diff --git a/sound/x86/intel_hdmi_audio.c b/sound/x86/intel_hdmi_audio.c
index cbba4a78afb5..4eaf5de54f61 100644
--- a/sound/x86/intel_hdmi_audio.c
+++ b/sound/x86/intel_hdmi_audio.c
@@ -1568,7 +1568,7 @@ static void had_audio_wq(struct work_struct *work)
 		struct intel_hdmi_lpe_audio_eld *eld = &pdata->eld;
 
 		dev_dbg(ctx->dev, "%s: HAD_NOTIFY_ELD : port = %d, tmds = %d\n",
-			__func__, eld->port_id,	pdata->tmds_clock_speed);
+			__func__, eld->port_id,	pdata->ls_clock);
 
 		switch (eld->pipe_id) {
 		case 0:
@@ -1589,8 +1589,13 @@ static void had_audio_wq(struct work_struct *work)
 		memcpy(ctx->eld, eld->eld_data, sizeof(ctx->eld));
 
 		ctx->dp_output = pdata->dp_output;
-		ctx->tmds_clock_speed = pdata->tmds_clock_speed;
-		ctx->link_rate = pdata->link_rate;
+		if (ctx->dp_output) {
+			ctx->tmds_clock_speed = 0;
+			ctx->link_rate = pdata->ls_clock;
+		} else {
+			ctx->tmds_clock_speed = pdata->ls_clock;
+			ctx->link_rate = 0;
+		}
 
 		had_process_hot_plug(ctx);
 
-- 
2.10.2

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

[PATCH v2 06/11] drm/i915: Remove hdmi_connected from LPE audio pdata

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

We can determine that the pipe was shut down from pipe<0, so there's
no point in duplicating that information as 'hdmi_connected'.

v2: Use pipe<0 instead of port<0 as we'll want to do per-port
    PCM devices later
    Initialize pipe to -1 to inidicate inactive initial state

Cc: Takashi Iwai <tiwai@suse.de>
Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_lpe_audio.c | 9 +++++----
 include/drm/intel_lpe_audio.h          | 3 +--
 sound/x86/intel_hdmi_audio.c           | 8 ++++----
 3 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c
index 5a1a37e963f1..7fd95733eff5 100644
--- a/drivers/gpu/drm/i915/intel_lpe_audio.c
+++ b/drivers/gpu/drm/i915/intel_lpe_audio.c
@@ -111,6 +111,7 @@ lpe_audio_platdev_create(struct drm_i915_private *dev_priv)
 	pinfo.size_data = sizeof(*pdata);
 	pinfo.dma_mask = DMA_BIT_MASK(32);
 
+	pdata->pipe = -1;
 	spin_lock_init(&pdata->lpe_audio_slock);
 
 	platdev = platform_device_register_full(&pinfo);
@@ -332,12 +333,12 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv,
 
 	audio_enable = I915_READ(VLV_AUD_PORT_EN_DBG(port));
 
+	pdata->eld.port_id = port;
+
 	if (eld != NULL) {
 		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->pipe = pipe;
 		pdata->ls_clock = ls_clock;
 		pdata->dp_output = dp_output;
 
@@ -348,7 +349,7 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv,
 	} else {
 		memset(pdata->eld.eld_data, 0,
 			HDMI_MAX_ELD_BYTES);
-		pdata->hdmi_connected = false;
+		pdata->pipe = -1;
 		pdata->ls_clock = 0;
 		pdata->dp_output = false;
 
diff --git a/include/drm/intel_lpe_audio.h b/include/drm/intel_lpe_audio.h
index 8bf804ce8905..9a5bdf5ad180 100644
--- a/include/drm/intel_lpe_audio.h
+++ b/include/drm/intel_lpe_audio.h
@@ -33,13 +33,12 @@ struct platform_device;
 
 struct intel_hdmi_lpe_audio_eld {
 	int port_id;
-	int pipe_id;
 	unsigned char eld_data[HDMI_MAX_ELD_BYTES];
 };
 
 struct intel_hdmi_lpe_audio_pdata {
+	int pipe;
 	int ls_clock;
-	bool hdmi_connected;
 	bool dp_output;
 	struct intel_hdmi_lpe_audio_eld eld;
 	void (*notify_audio_lpe)(struct platform_device *pdev);
diff --git a/sound/x86/intel_hdmi_audio.c b/sound/x86/intel_hdmi_audio.c
index 4eaf5de54f61..1a095189db83 100644
--- a/sound/x86/intel_hdmi_audio.c
+++ b/sound/x86/intel_hdmi_audio.c
@@ -1559,7 +1559,7 @@ static void had_audio_wq(struct work_struct *work)
 
 	pm_runtime_get_sync(ctx->dev);
 	mutex_lock(&ctx->mutex);
-	if (!pdata->hdmi_connected) {
+	if (pdata->pipe < 0) {
 		dev_dbg(ctx->dev, "%s: Event: HAD_NOTIFY_HOT_UNPLUG\n",
 			__func__);
 		memset(ctx->eld, 0, sizeof(ctx->eld)); /* clear the old ELD */
@@ -1568,9 +1568,9 @@ static void had_audio_wq(struct work_struct *work)
 		struct intel_hdmi_lpe_audio_eld *eld = &pdata->eld;
 
 		dev_dbg(ctx->dev, "%s: HAD_NOTIFY_ELD : port = %d, tmds = %d\n",
-			__func__, eld->port_id,	pdata->ls_clock);
+			__func__, eld->port_id, pdata->ls_clock);
 
-		switch (eld->pipe_id) {
+		switch (pdata->pipe) {
 		case 0:
 			ctx->had_config_offset = AUDIO_HDMI_CONFIG_A;
 			break;
@@ -1582,7 +1582,7 @@ static void had_audio_wq(struct work_struct *work)
 			break;
 		default:
 			dev_dbg(ctx->dev, "Invalid pipe %d\n",
-				eld->pipe_id);
+				pdata->pipe);
 			break;
 		}
 
-- 
2.10.2

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

[PATCH 07/11] drm/i915: Reorganize intel_lpe_audio_notify() arguments

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Shuffle the arguments to intel_lpe_audio_notify() around a bit. Pipe
and port being the most important things, so let's put the first, and
thre rest can come in as is. Also constify the eld argument.

Cc: Takashi Iwai <tiwai@suse.de>
Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h        | 4 ++--
 drivers/gpu/drm/i915/intel_audio.c     | 4 ++--
 drivers/gpu/drm/i915/intel_lpe_audio.c | 8 ++++----
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 8bf72220ee07..9c528209fba7 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3721,8 +3721,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 pipe, int ls_clock,
-			    bool dp_output);
+			    enum pipe pipe, enum port port,
+			    const void *eld, int ls_clock, bool dp_output);
 
 /* intel_i2c.c */
 extern int intel_setup_gmbus(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
index 79eeef25321f..d805b6e6fe71 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -632,7 +632,7 @@ void intel_audio_codec_enable(struct intel_encoder *intel_encoder,
 						 (int) port, (int) pipe);
 	}
 
-	intel_lpe_audio_notify(dev_priv, connector->eld, port, pipe,
+	intel_lpe_audio_notify(dev_priv, pipe, port, connector->eld,
 			       crtc_state->port_clock,
 			       intel_encoder->type == INTEL_OUTPUT_DP);
 }
@@ -669,7 +669,7 @@ void intel_audio_codec_disable(struct intel_encoder *intel_encoder)
 						 (int) port, (int) pipe);
 	}
 
-	intel_lpe_audio_notify(dev_priv, NULL, port, pipe, 0, false);
+	intel_lpe_audio_notify(dev_priv, pipe, port, NULL, 0, false);
 }
 
 /**
diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c
index 7fd95733eff5..4c770d037f23 100644
--- a/drivers/gpu/drm/i915/intel_lpe_audio.c
+++ b/drivers/gpu/drm/i915/intel_lpe_audio.c
@@ -307,17 +307,17 @@ void intel_lpe_audio_teardown(struct drm_i915_private *dev_priv)
  * intel_lpe_audio_notify() - notify lpe audio event
  * audio driver and i915
  * @dev_priv: the i915 drm device private data
+ * @pipe: pipe
+ * @port: port
  * @eld : ELD data
- * @pipe: pipe id
- * @port: port id
  * @ls_clock: Link symbol clock in kHz
  * @dp_output: Driving a DP output?
  *
  * Notify lpe audio driver of eld change.
  */
 void intel_lpe_audio_notify(struct drm_i915_private *dev_priv,
-			    void *eld, int port, int pipe, int ls_clock,
-			    bool dp_output)
+			    enum pipe pipe, enum port port,
+			    const void *eld, int ls_clock, bool dp_output)
 {
 	unsigned long irq_flags;
 	struct intel_hdmi_lpe_audio_pdata *pdata = NULL;
-- 
2.10.2

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

[PATCH v2 08/11] drm/i915: Clean up the LPE audio platform data

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Split the LPE audio platform data into a port specific
chunk and device specific chunk. Eventually we'll have
a port specific chunk for each port, but for now we'll
stick to just one.

We'll also get rid of the intel_hdmi_lpe_audio_eld structure
which doesn't seem to have any real reason to exist.

v2: Organize per port instead of per pipe

Cc: Takashi Iwai <tiwai@suse.de>
Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_lpe_audio.c | 30 ++++++++++++++----------------
 include/drm/intel_lpe_audio.h          | 15 ++++++++-------
 sound/x86/intel_hdmi_audio.c           | 19 +++++++++----------
 3 files changed, 31 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c
index 4c770d037f23..bdbc235141b5 100644
--- a/drivers/gpu/drm/i915/intel_lpe_audio.c
+++ b/drivers/gpu/drm/i915/intel_lpe_audio.c
@@ -111,7 +111,7 @@ lpe_audio_platdev_create(struct drm_i915_private *dev_priv)
 	pinfo.size_data = sizeof(*pdata);
 	pinfo.dma_mask = DMA_BIT_MASK(32);
 
-	pdata->pipe = -1;
+	pdata->port.pipe = -1;
 	spin_lock_init(&pdata->lpe_audio_slock);
 
 	platdev = platform_device_register_full(&pinfo);
@@ -320,38 +320,36 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv,
 			    const void *eld, int ls_clock, bool dp_output)
 {
 	unsigned long irq_flags;
-	struct intel_hdmi_lpe_audio_pdata *pdata = NULL;
+	struct intel_hdmi_lpe_audio_pdata *pdata;
+	struct intel_hdmi_lpe_audio_port_pdata *ppdata;
 	u32 audio_enable;
 
 	if (!HAS_LPE_AUDIO(dev_priv))
 		return;
 
-	pdata = dev_get_platdata(
-		&(dev_priv->lpe_audio.platdev->dev));
+	pdata = dev_get_platdata(&dev_priv->lpe_audio.platdev->dev);
+	ppdata = &pdata->port;
 
 	spin_lock_irqsave(&pdata->lpe_audio_slock, irq_flags);
 
 	audio_enable = I915_READ(VLV_AUD_PORT_EN_DBG(port));
 
-	pdata->eld.port_id = port;
+	ppdata->port = port;
 
 	if (eld != NULL) {
-		memcpy(pdata->eld.eld_data, eld,
-			HDMI_MAX_ELD_BYTES);
-		pdata->pipe = pipe;
-		pdata->ls_clock = ls_clock;
-		pdata->dp_output = dp_output;
+		memcpy(ppdata->eld, eld, HDMI_MAX_ELD_BYTES);
+		ppdata->pipe = pipe;
+		ppdata->ls_clock = ls_clock;
+		ppdata->dp_output = dp_output;
 
 		/* 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->pipe = -1;
-		pdata->ls_clock = 0;
-		pdata->dp_output = false;
+		memset(ppdata->eld, 0, HDMI_MAX_ELD_BYTES);
+		ppdata->pipe = -1;
+		ppdata->ls_clock = 0;
+		ppdata->dp_output = false;
 
 		/* Mute the amp for both DP and HDMI */
 		I915_WRITE(VLV_AUD_PORT_EN_DBG(port),
diff --git a/include/drm/intel_lpe_audio.h b/include/drm/intel_lpe_audio.h
index 9a5bdf5ad180..211f1cd61153 100644
--- a/include/drm/intel_lpe_audio.h
+++ b/include/drm/intel_lpe_audio.h
@@ -31,16 +31,17 @@ struct platform_device;
 
 #define HDMI_MAX_ELD_BYTES	128
 
-struct intel_hdmi_lpe_audio_eld {
-	int port_id;
-	unsigned char eld_data[HDMI_MAX_ELD_BYTES];
-};
-
-struct intel_hdmi_lpe_audio_pdata {
+struct intel_hdmi_lpe_audio_port_pdata {
+	u8 eld[HDMI_MAX_ELD_BYTES];
+	int port;
 	int pipe;
 	int ls_clock;
 	bool dp_output;
-	struct intel_hdmi_lpe_audio_eld eld;
+};
+
+struct intel_hdmi_lpe_audio_pdata {
+	struct intel_hdmi_lpe_audio_port_pdata port;
+
 	void (*notify_audio_lpe)(struct platform_device *pdev);
 	spinlock_t lpe_audio_slock;
 };
diff --git a/sound/x86/intel_hdmi_audio.c b/sound/x86/intel_hdmi_audio.c
index 1a095189db83..c2b78621852e 100644
--- a/sound/x86/intel_hdmi_audio.c
+++ b/sound/x86/intel_hdmi_audio.c
@@ -1556,21 +1556,20 @@ static void had_audio_wq(struct work_struct *work)
 	struct snd_intelhad *ctx =
 		container_of(work, struct snd_intelhad, hdmi_audio_wq);
 	struct intel_hdmi_lpe_audio_pdata *pdata = ctx->dev->platform_data;
+	struct intel_hdmi_lpe_audio_port_pdata *ppdata = &pdata->port;
 
 	pm_runtime_get_sync(ctx->dev);
 	mutex_lock(&ctx->mutex);
-	if (pdata->pipe < 0) {
+	if (ppdata->pipe < 0) {
 		dev_dbg(ctx->dev, "%s: Event: HAD_NOTIFY_HOT_UNPLUG\n",
 			__func__);
 		memset(ctx->eld, 0, sizeof(ctx->eld)); /* clear the old ELD */
 		had_process_hot_unplug(ctx);
 	} else {
-		struct intel_hdmi_lpe_audio_eld *eld = &pdata->eld;
-
 		dev_dbg(ctx->dev, "%s: HAD_NOTIFY_ELD : port = %d, tmds = %d\n",
-			__func__, eld->port_id, pdata->ls_clock);
+			__func__, ppdata->port, ppdata->ls_clock);
 
-		switch (pdata->pipe) {
+		switch (ppdata->pipe) {
 		case 0:
 			ctx->had_config_offset = AUDIO_HDMI_CONFIG_A;
 			break;
@@ -1582,18 +1581,18 @@ static void had_audio_wq(struct work_struct *work)
 			break;
 		default:
 			dev_dbg(ctx->dev, "Invalid pipe %d\n",
-				pdata->pipe);
+				ppdata->pipe);
 			break;
 		}
 
-		memcpy(ctx->eld, eld->eld_data, sizeof(ctx->eld));
+		memcpy(ctx->eld, ppdata->eld, sizeof(ctx->eld));
 
-		ctx->dp_output = pdata->dp_output;
+		ctx->dp_output = ppdata->dp_output;
 		if (ctx->dp_output) {
 			ctx->tmds_clock_speed = 0;
-			ctx->link_rate = pdata->ls_clock;
+			ctx->link_rate = ppdata->ls_clock;
 		} else {
-			ctx->tmds_clock_speed = pdata->ls_clock;
+			ctx->tmds_clock_speed = ppdata->ls_clock;
 			ctx->link_rate = 0;
 		}
 
-- 
2.10.2

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

[PATCH 09/11] ALSA: x86: Prepare LPE audio ctls for multiple PCMs

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

In preparation for register a PCM device for each pipe adjust
link up the ctl elements with the corresponding PCM device.

Cc: Takashi Iwai <tiwai@suse.de>
Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 sound/x86/intel_hdmi_audio.c | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/sound/x86/intel_hdmi_audio.c b/sound/x86/intel_hdmi_audio.c
index c2b78621852e..69e10845633a 100644
--- a/sound/x86/intel_hdmi_audio.c
+++ b/sound/x86/intel_hdmi_audio.c
@@ -1609,11 +1609,16 @@ static void had_audio_wq(struct work_struct *work)
 /*
  * Jack interface
  */
-static int had_create_jack(struct snd_intelhad *ctx)
+static int had_create_jack(struct snd_intelhad *ctx,
+			   struct snd_pcm *pcm)
 {
+	char hdmi_str[32];
 	int err;
 
-	err = snd_jack_new(ctx->card, "HDMI/DP", SND_JACK_AVOUT, &ctx->jack,
+	snprintf(hdmi_str, sizeof(hdmi_str),
+		 "HDMI/DP,pcm=%d", pcm->device);
+
+	err = snd_jack_new(ctx->card, hdmi_str, SND_JACK_AVOUT, &ctx->jack,
 			   true, false);
 	if (err < 0)
 		return err;
@@ -1793,7 +1798,17 @@ static int hdmi_lpe_audio_probe(struct platform_device *pdev)
 
 	/* create controls */
 	for (i = 0; i < ARRAY_SIZE(had_controls); i++) {
-		ret = snd_ctl_add(card, snd_ctl_new1(&had_controls[i], ctx));
+		struct snd_kcontrol *kctl;
+
+		kctl = snd_ctl_new1(&had_controls[i], ctx);
+		if (!kctl) {
+			ret = -ENOMEM;
+			goto err;
+		}
+
+		kctl->id.device = pcm->device;
+
+		ret = snd_ctl_add(card, kctl);
 		if (ret < 0)
 			goto err;
 	}
@@ -1805,7 +1820,7 @@ static int hdmi_lpe_audio_probe(struct platform_device *pdev)
 	if (ret < 0)
 		goto err;
 
-	ret = had_create_jack(ctx);
+	ret = had_create_jack(ctx, pcm);
 	if (ret < 0)
 		goto err;
 
-- 
2.10.2

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

[PATCH v2 10/11] ALSA: x86: Split snd_intelhad into card and PCM specific structures

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

To allow multiple PCM devices to be registered for the LPE audio card,
split the private data into card and PCM specific chunks. For now we'll
stick to just one PCM device as before.

v2: Rework to do a pcm device per port instead of per pipe

Cc: Takashi Iwai <tiwai@suse.de>
Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 sound/x86/intel_hdmi_audio.c | 227 +++++++++++++++++++++++++------------------
 sound/x86/intel_hdmi_audio.h |  15 ++-
 2 files changed, 142 insertions(+), 100 deletions(-)

diff --git a/sound/x86/intel_hdmi_audio.c b/sound/x86/intel_hdmi_audio.c
index 69e10845633a..12fae26e70bb 100644
--- a/sound/x86/intel_hdmi_audio.c
+++ b/sound/x86/intel_hdmi_audio.c
@@ -42,6 +42,9 @@
 #include <drm/intel_lpe_audio.h>
 #include "intel_hdmi_audio.h"
 
+#define for_each_port(card_ctx, port) \
+	for ((port) = 0; (port) < (card_ctx)->num_ports; (port)++)
+
 /*standard module options for ALSA. This module supports only one card*/
 static int hdmi_card_index = SNDRV_DEFAULT_IDX1;
 static char *hdmi_card_id = SNDRV_DEFAULT_STR1;
@@ -192,12 +195,12 @@ static void had_substream_put(struct snd_intelhad *intelhaddata)
 /* Register access functions */
 static u32 had_read_register_raw(struct snd_intelhad *ctx, u32 reg)
 {
-	return ioread32(ctx->mmio_start + ctx->had_config_offset + reg);
+	return ioread32(ctx->card_ctx->mmio_start + ctx->had_config_offset + reg);
 }
 
 static void had_write_register_raw(struct snd_intelhad *ctx, u32 reg, u32 val)
 {
-	iowrite32(val, ctx->mmio_start + ctx->had_config_offset + reg);
+	iowrite32(val, ctx->card_ctx->mmio_start + ctx->had_config_offset + reg);
 }
 
 static void had_read_register(struct snd_intelhad *ctx, u32 reg, u32 *val)
@@ -1519,22 +1522,27 @@ static const struct snd_kcontrol_new had_controls[] = {
  */
 static irqreturn_t display_pipe_interrupt_handler(int irq, void *dev_id)
 {
-	struct snd_intelhad *ctx = dev_id;
-	u32 audio_stat;
+	struct snd_intelhad_card *card_ctx = dev_id;
+	int port;
 
-	/* use raw register access to ack IRQs even while disconnected */
-	audio_stat = had_read_register_raw(ctx, AUD_HDMI_STATUS);
+	for_each_port(card_ctx, port) {
+		struct snd_intelhad *ctx = &card_ctx->pcm_ctx[port];
+		u32 audio_stat;
 
-	if (audio_stat & HDMI_AUDIO_UNDERRUN) {
-		had_write_register_raw(ctx, AUD_HDMI_STATUS,
-				       HDMI_AUDIO_UNDERRUN);
-		had_process_buffer_underrun(ctx);
-	}
+		/* use raw register access to ack IRQs even while disconnected */
+		audio_stat = had_read_register_raw(ctx, AUD_HDMI_STATUS);
+
+		if (audio_stat & HDMI_AUDIO_UNDERRUN) {
+			had_write_register_raw(ctx, AUD_HDMI_STATUS,
+					       HDMI_AUDIO_UNDERRUN);
+			had_process_buffer_underrun(ctx);
+		}
 
-	if (audio_stat & HDMI_AUDIO_BUFFER_DONE) {
-		had_write_register_raw(ctx, AUD_HDMI_STATUS,
-				       HDMI_AUDIO_BUFFER_DONE);
-		had_process_buffer_done(ctx);
+		if (audio_stat & HDMI_AUDIO_BUFFER_DONE) {
+			had_write_register_raw(ctx, AUD_HDMI_STATUS,
+					       HDMI_AUDIO_BUFFER_DONE);
+			had_process_buffer_done(ctx);
+		}
 	}
 
 	return IRQ_HANDLED;
@@ -1545,9 +1553,14 @@ static irqreturn_t display_pipe_interrupt_handler(int irq, void *dev_id)
  */
 static void notify_audio_lpe(struct platform_device *pdev)
 {
-	struct snd_intelhad *ctx = platform_get_drvdata(pdev);
+	struct snd_intelhad_card *card_ctx = platform_get_drvdata(pdev);
+	int port;
+
+	for_each_port(card_ctx, port) {
+		struct snd_intelhad *ctx = &card_ctx->pcm_ctx[port];
 
-	schedule_work(&ctx->hdmi_audio_wq);
+		schedule_work(&ctx->hdmi_audio_wq);
+	}
 }
 
 /* the work to handle monitor hot plug/unplug */
@@ -1618,7 +1631,8 @@ static int had_create_jack(struct snd_intelhad *ctx,
 	snprintf(hdmi_str, sizeof(hdmi_str),
 		 "HDMI/DP,pcm=%d", pcm->device);
 
-	err = snd_jack_new(ctx->card, hdmi_str, SND_JACK_AVOUT, &ctx->jack,
+	err = snd_jack_new(ctx->card_ctx->card, hdmi_str,
+			   SND_JACK_AVOUT, &ctx->jack,
 			   true, false);
 	if (err < 0)
 		return err;
@@ -1632,13 +1646,18 @@ static int had_create_jack(struct snd_intelhad *ctx,
 
 static int hdmi_lpe_audio_runtime_suspend(struct device *dev)
 {
-	struct snd_intelhad *ctx = dev_get_drvdata(dev);
-	struct snd_pcm_substream *substream;
+	struct snd_intelhad_card *card_ctx = dev_get_drvdata(dev);
+	int port;
 
-	substream = had_substream_get(ctx);
-	if (substream) {
-		snd_pcm_suspend(substream);
-		had_substream_put(ctx);
+	for_each_port(card_ctx, port) {
+		struct snd_intelhad *ctx = &card_ctx->pcm_ctx[port];
+		struct snd_pcm_substream *substream;
+
+		substream = had_substream_get(ctx);
+		if (substream) {
+			snd_pcm_suspend(substream);
+			had_substream_put(ctx);
+		}
 	}
 
 	return 0;
@@ -1646,12 +1665,12 @@ static int hdmi_lpe_audio_runtime_suspend(struct device *dev)
 
 static int __maybe_unused hdmi_lpe_audio_suspend(struct device *dev)
 {
-	struct snd_intelhad *ctx = dev_get_drvdata(dev);
+	struct snd_intelhad_card *card_ctx = dev_get_drvdata(dev);
 	int err;
 
 	err = hdmi_lpe_audio_runtime_suspend(dev);
 	if (!err)
-		snd_power_change_state(ctx->card, SNDRV_CTL_POWER_D3hot);
+		snd_power_change_state(card_ctx->card, SNDRV_CTL_POWER_D3hot);
 	return err;
 }
 
@@ -1663,29 +1682,34 @@ static int hdmi_lpe_audio_runtime_resume(struct device *dev)
 
 static int __maybe_unused hdmi_lpe_audio_resume(struct device *dev)
 {
-	struct snd_intelhad *ctx = dev_get_drvdata(dev);
+	struct snd_intelhad_card *card_ctx = dev_get_drvdata(dev);
 
 	hdmi_lpe_audio_runtime_resume(dev);
-	snd_power_change_state(ctx->card, SNDRV_CTL_POWER_D0);
+	snd_power_change_state(card_ctx->card, SNDRV_CTL_POWER_D0);
 	return 0;
 }
 
 /* release resources */
 static void hdmi_lpe_audio_free(struct snd_card *card)
 {
-	struct snd_intelhad *ctx = card->private_data;
-	struct intel_hdmi_lpe_audio_pdata *pdata = ctx->dev->platform_data;
+	struct snd_intelhad_card *card_ctx = card->private_data;
+	struct intel_hdmi_lpe_audio_pdata *pdata = card_ctx->dev->platform_data;
+	int port;
 
 	spin_lock_irq(&pdata->lpe_audio_slock);
 	pdata->notify_audio_lpe = NULL;
 	spin_unlock_irq(&pdata->lpe_audio_slock);
 
-	cancel_work_sync(&ctx->hdmi_audio_wq);
+	for_each_port(card_ctx, port) {
+		struct snd_intelhad *ctx = &card_ctx->pcm_ctx[port];
+
+		cancel_work_sync(&ctx->hdmi_audio_wq);
+	}
 
-	if (ctx->mmio_start)
-		iounmap(ctx->mmio_start);
-	if (ctx->irq >= 0)
-		free_irq(ctx->irq, ctx);
+	if (card_ctx->mmio_start)
+		iounmap(card_ctx->mmio_start);
+	if (card_ctx->irq >= 0)
+		free_irq(card_ctx->irq, card_ctx);
 }
 
 /*
@@ -1697,12 +1721,12 @@ static void hdmi_lpe_audio_free(struct snd_card *card)
 static int hdmi_lpe_audio_probe(struct platform_device *pdev)
 {
 	struct snd_card *card;
-	struct snd_intelhad *ctx;
+	struct snd_intelhad_card *card_ctx;
 	struct snd_pcm *pcm;
 	struct intel_hdmi_lpe_audio_pdata *pdata;
 	int irq;
 	struct resource *res_mmio;
-	int i, ret;
+	int port, ret;
 
 	pdata = pdev->dev.platform_data;
 	if (!pdata) {
@@ -1725,39 +1749,30 @@ static int hdmi_lpe_audio_probe(struct platform_device *pdev)
 
 	/* create a card instance with ALSA framework */
 	ret = snd_card_new(&pdev->dev, hdmi_card_index, hdmi_card_id,
-			   THIS_MODULE, sizeof(*ctx), &card);
+			   THIS_MODULE, sizeof(*card_ctx), &card);
 	if (ret)
 		return ret;
 
-	ctx = card->private_data;
-	spin_lock_init(&ctx->had_spinlock);
-	mutex_init(&ctx->mutex);
-	ctx->connected = false;
-	ctx->dev = &pdev->dev;
-	ctx->card = card;
-	ctx->aes_bits = SNDRV_PCM_DEFAULT_CON_SPDIF;
+	card_ctx = card->private_data;
+	card_ctx->dev = &pdev->dev;
+	card_ctx->card = card;
 	strcpy(card->driver, INTEL_HAD);
 	strcpy(card->shortname, "Intel HDMI/DP LPE Audio");
 	strcpy(card->longname, "Intel HDMI/DP LPE Audio");
 
-	ctx->irq = -1;
-	ctx->tmds_clock_speed = DIS_SAMPLE_RATE_148_5;
-	INIT_WORK(&ctx->hdmi_audio_wq, had_audio_wq);
+	card_ctx->irq = -1;
 
 	card->private_free = hdmi_lpe_audio_free;
 
-	/* assume pipe A as default */
-	ctx->had_config_offset = AUDIO_HDMI_CONFIG_A;
-
-	platform_set_drvdata(pdev, ctx);
+	platform_set_drvdata(pdev, card_ctx);
 
 	dev_dbg(&pdev->dev, "%s: mmio_start = 0x%x, mmio_end = 0x%x\n",
 		__func__, (unsigned int)res_mmio->start,
 		(unsigned int)res_mmio->end);
 
-	ctx->mmio_start = ioremap_nocache(res_mmio->start,
-					  (size_t)(resource_size(res_mmio)));
-	if (!ctx->mmio_start) {
+	card_ctx->mmio_start = ioremap_nocache(res_mmio->start,
+					       (size_t)(resource_size(res_mmio)));
+	if (!card_ctx->mmio_start) {
 		dev_err(&pdev->dev, "Could not get ioremap\n");
 		ret = -EACCES;
 		goto err;
@@ -1765,65 +1780,79 @@ static int hdmi_lpe_audio_probe(struct platform_device *pdev)
 
 	/* setup interrupt handler */
 	ret = request_irq(irq, display_pipe_interrupt_handler, 0,
-			  pdev->name, ctx);
+			  pdev->name, card_ctx);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "request_irq failed\n");
 		goto err;
 	}
 
-	ctx->irq = irq;
-
-	ret = snd_pcm_new(card, INTEL_HAD, PCM_INDEX, MAX_PB_STREAMS,
-			  MAX_CAP_STREAMS, &pcm);
-	if (ret)
-		goto err;
-
-	/* setup private data which can be retrieved when required */
-	pcm->private_data = ctx;
-	pcm->info_flags = 0;
-	strncpy(pcm->name, card->shortname, strlen(card->shortname));
-	/* setup the ops for playabck */
-	snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_PLAYBACK, &had_pcm_ops);
+	card_ctx->irq = irq;
 
 	/* only 32bit addressable */
 	dma_set_mask(&pdev->dev, DMA_BIT_MASK(32));
 	dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(32));
 
-	/* allocate dma pages;
-	 * try to allocate 600k buffer as default which is large enough
-	 */
-	snd_pcm_lib_preallocate_pages_for_all(pcm,
-			SNDRV_DMA_TYPE_DEV, NULL,
-			HAD_DEFAULT_BUFFER, HAD_MAX_BUFFER);
+	init_channel_allocations();
 
-	/* create controls */
-	for (i = 0; i < ARRAY_SIZE(had_controls); i++) {
-		struct snd_kcontrol *kctl;
+	card_ctx->num_ports = 1;
 
-		kctl = snd_ctl_new1(&had_controls[i], ctx);
-		if (!kctl) {
-			ret = -ENOMEM;
+	for_each_port(card_ctx, port) {
+		struct snd_intelhad *ctx = &card_ctx->pcm_ctx[port];
+		int i;
+
+		ctx->card_ctx = card_ctx;
+		ctx->dev = card_ctx->dev;
+
+		INIT_WORK(&ctx->hdmi_audio_wq, had_audio_wq);
+
+		ctx->had_config_offset = AUDIO_HDMI_CONFIG_A;
+
+		ret = snd_pcm_new(card, INTEL_HAD, PCM_INDEX, MAX_PB_STREAMS,
+				  MAX_CAP_STREAMS, &pcm);
+		if (ret)
 			goto err;
+
+		/* setup private data which can be retrieved when required */
+		pcm->private_data = ctx;
+		pcm->info_flags = 0;
+		strncpy(pcm->name, card->shortname, strlen(card->shortname));
+		/* setup the ops for playabck */
+		snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_PLAYBACK, &had_pcm_ops);
+
+		/* allocate dma pages;
+		 * try to allocate 600k buffer as default which is large enough
+		 */
+		snd_pcm_lib_preallocate_pages_for_all(pcm,
+						      SNDRV_DMA_TYPE_DEV, NULL,
+						      HAD_DEFAULT_BUFFER, HAD_MAX_BUFFER);
+
+		/* create controls */
+		for (i = 0; i < ARRAY_SIZE(had_controls); i++) {
+			struct snd_kcontrol *kctl;
+
+			kctl = snd_ctl_new1(&had_controls[i], ctx);
+			if (!kctl) {
+				ret = -ENOMEM;
+				goto err;
+			}
+
+			kctl->id.device = pcm->device;
+
+			ret = snd_ctl_add(card, kctl);
+			if (ret < 0)
+				goto err;
 		}
 
-		kctl->id.device = pcm->device;
+		/* Register channel map controls */
+		ret = had_register_chmap_ctls(ctx, pcm);
+		if (ret < 0)
+			goto err;
 
-		ret = snd_ctl_add(card, kctl);
+		ret = had_create_jack(ctx, pcm);
 		if (ret < 0)
 			goto err;
 	}
 
-	init_channel_allocations();
-
-	/* Register channel map controls */
-	ret = had_register_chmap_ctls(ctx, pcm);
-	if (ret < 0)
-		goto err;
-
-	ret = had_create_jack(ctx, pcm);
-	if (ret < 0)
-		goto err;
-
 	ret = snd_card_register(card);
 	if (ret)
 		goto err;
@@ -1837,7 +1866,11 @@ static int hdmi_lpe_audio_probe(struct platform_device *pdev)
 	pm_runtime_set_active(&pdev->dev);
 
 	dev_dbg(&pdev->dev, "%s: handle pending notification\n", __func__);
-	schedule_work(&ctx->hdmi_audio_wq);
+	for_each_port(card_ctx, port) {
+		struct snd_intelhad *ctx = &card_ctx->pcm_ctx[port];
+
+		schedule_work(&ctx->hdmi_audio_wq);
+	}
 
 	return 0;
 
@@ -1853,9 +1886,9 @@ static int hdmi_lpe_audio_probe(struct platform_device *pdev)
  */
 static int hdmi_lpe_audio_remove(struct platform_device *pdev)
 {
-	struct snd_intelhad *ctx = platform_get_drvdata(pdev);
+	struct snd_intelhad_card *card_ctx = platform_get_drvdata(pdev);
 
-	snd_card_free(ctx->card);
+	snd_card_free(card_ctx->card);
 	return 0;
 }
 
diff --git a/sound/x86/intel_hdmi_audio.h b/sound/x86/intel_hdmi_audio.h
index 2d3e389f76b3..2725964ebc46 100644
--- a/sound/x86/intel_hdmi_audio.h
+++ b/sound/x86/intel_hdmi_audio.h
@@ -101,7 +101,7 @@ struct pcm_stream_info {
  * @chmap: holds channel map info
  */
 struct snd_intelhad {
-	struct snd_card	*card;
+	struct snd_intelhad_card *card_ctx;
 	bool		connected;
 	struct		pcm_stream_info stream_info;
 	unsigned char	eld[HDMI_MAX_ELD_BYTES];
@@ -123,8 +123,6 @@ struct snd_intelhad {
 	unsigned int period_bytes;	/* PCM period size in bytes */
 
 	/* internal stuff */
-	int irq;
-	void __iomem *mmio_start;
 	unsigned int had_config_offset;
 	union aud_cfg aud_config;	/* AUD_CONFIG reg value cache */
 	struct work_struct hdmi_audio_wq;
@@ -133,4 +131,15 @@ struct snd_intelhad {
 	struct snd_jack *jack;
 };
 
+struct snd_intelhad_card {
+	struct snd_card	*card;
+	struct device *dev;
+
+	/* internal stuff */
+	int irq;
+	void __iomem *mmio_start;
+	int num_ports;
+	struct snd_intelhad pcm_ctx[3];
+};
+
 #endif /* _INTEL_HDMI_AUDIO_ */
-- 
2.10.2

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

[PATCH v2 11/11] ALSA: x86: Register multiple PCM devices for the LPE audio card

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Now that everything is in place let's register a PCM device for
each port of the display engine. This will make it possible to
actually output audio to multiple displays at the same time. And
it avoids modesets on unrelated displays from clobbering up the
ELD and whatnot for the display currently doing the playback.

v2: Add a PCM per port instead of per pipe

Cc: Takashi Iwai <tiwai@suse.de>
Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_lpe_audio.c |  19 ++---
 include/drm/intel_lpe_audio.h          |   6 +-
 sound/x86/intel_hdmi_audio.c           | 126 +++++++++++++++++++--------------
 sound/x86/intel_hdmi_audio.h           |   7 +-
 4 files changed, 92 insertions(+), 66 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c
index bdbc235141b5..fa728ed21d1f 100644
--- a/drivers/gpu/drm/i915/intel_lpe_audio.c
+++ b/drivers/gpu/drm/i915/intel_lpe_audio.c
@@ -111,7 +111,11 @@ lpe_audio_platdev_create(struct drm_i915_private *dev_priv)
 	pinfo.size_data = sizeof(*pdata);
 	pinfo.dma_mask = DMA_BIT_MASK(32);
 
-	pdata->port.pipe = -1;
+	pdata->num_pipes = INTEL_INFO(dev_priv)->num_pipes;
+	pdata->num_ports = IS_CHERRYVIEW(dev_priv) ? 3 : 2; /* B,C,D or B,C */
+	pdata->port[0].pipe = -1;
+	pdata->port[1].pipe = -1;
+	pdata->port[2].pipe = -1;
 	spin_lock_init(&pdata->lpe_audio_slock);
 
 	platdev = platform_device_register_full(&pinfo);
@@ -319,7 +323,7 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv,
 			    enum pipe pipe, enum port port,
 			    const void *eld, int ls_clock, bool dp_output)
 {
-	unsigned long irq_flags;
+	unsigned long irqflags;
 	struct intel_hdmi_lpe_audio_pdata *pdata;
 	struct intel_hdmi_lpe_audio_port_pdata *ppdata;
 	u32 audio_enable;
@@ -328,14 +332,12 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv,
 		return;
 
 	pdata = dev_get_platdata(&dev_priv->lpe_audio.platdev->dev);
-	ppdata = &pdata->port;
+	ppdata = &pdata->port[port];
 
-	spin_lock_irqsave(&pdata->lpe_audio_slock, irq_flags);
+	spin_lock_irqsave(&pdata->lpe_audio_slock, irqflags);
 
 	audio_enable = I915_READ(VLV_AUD_PORT_EN_DBG(port));
 
-	ppdata->port = port;
-
 	if (eld != NULL) {
 		memcpy(ppdata->eld, eld, HDMI_MAX_ELD_BYTES);
 		ppdata->pipe = pipe;
@@ -357,8 +359,7 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv,
 	}
 
 	if (pdata->notify_audio_lpe)
-		pdata->notify_audio_lpe(dev_priv->lpe_audio.platdev);
+		pdata->notify_audio_lpe(dev_priv->lpe_audio.platdev, port);
 
-	spin_unlock_irqrestore(&pdata->lpe_audio_slock,
-			irq_flags);
+	spin_unlock_irqrestore(&pdata->lpe_audio_slock, irqflags);
 }
diff --git a/include/drm/intel_lpe_audio.h b/include/drm/intel_lpe_audio.h
index 211f1cd61153..a911530c012e 100644
--- a/include/drm/intel_lpe_audio.h
+++ b/include/drm/intel_lpe_audio.h
@@ -40,9 +40,11 @@ struct intel_hdmi_lpe_audio_port_pdata {
 };
 
 struct intel_hdmi_lpe_audio_pdata {
-	struct intel_hdmi_lpe_audio_port_pdata port;
+	struct intel_hdmi_lpe_audio_port_pdata port[3]; /* ports B,C,D */
+	int num_ports;
+	int num_pipes;
 
-	void (*notify_audio_lpe)(struct platform_device *pdev);
+	void (*notify_audio_lpe)(struct platform_device *pdev, int pipe);
 	spinlock_t lpe_audio_slock;
 };
 
diff --git a/sound/x86/intel_hdmi_audio.c b/sound/x86/intel_hdmi_audio.c
index 12fae26e70bb..909391d5270c 100644
--- a/sound/x86/intel_hdmi_audio.c
+++ b/sound/x86/intel_hdmi_audio.c
@@ -42,6 +42,8 @@
 #include <drm/intel_lpe_audio.h>
 #include "intel_hdmi_audio.h"
 
+#define for_each_pipe(card_ctx, pipe) \
+	for ((pipe) = 0; (pipe) < (card_ctx)->num_pipes; (pipe)++)
 #define for_each_port(card_ctx, port) \
 	for ((port) = 0; (port) < (card_ctx)->num_ports; (port)++)
 
@@ -192,15 +194,30 @@ static void had_substream_put(struct snd_intelhad *intelhaddata)
 	spin_unlock_irqrestore(&intelhaddata->had_spinlock, flags);
 }
 
+static u32 had_config_offset(int pipe)
+{
+	switch (pipe) {
+	default:
+	case 0:
+		return AUDIO_HDMI_CONFIG_A;
+	case 1:
+		return AUDIO_HDMI_CONFIG_B;
+	case 2:
+		return AUDIO_HDMI_CONFIG_C;
+	}
+}
+
 /* Register access functions */
-static u32 had_read_register_raw(struct snd_intelhad *ctx, u32 reg)
+static u32 had_read_register_raw(struct snd_intelhad_card *card_ctx,
+				 int pipe, u32 reg)
 {
-	return ioread32(ctx->card_ctx->mmio_start + ctx->had_config_offset + reg);
+	return ioread32(card_ctx->mmio_start + had_config_offset(pipe) + reg);
 }
 
-static void had_write_register_raw(struct snd_intelhad *ctx, u32 reg, u32 val)
+static void had_write_register_raw(struct snd_intelhad_card *card_ctx,
+				   int pipe, u32 reg, u32 val)
 {
-	iowrite32(val, ctx->card_ctx->mmio_start + ctx->had_config_offset + reg);
+	iowrite32(val, card_ctx->mmio_start + had_config_offset(pipe) + reg);
 }
 
 static void had_read_register(struct snd_intelhad *ctx, u32 reg, u32 *val)
@@ -208,13 +225,13 @@ static void had_read_register(struct snd_intelhad *ctx, u32 reg, u32 *val)
 	if (!ctx->connected)
 		*val = 0;
 	else
-		*val = had_read_register_raw(ctx, reg);
+		*val = had_read_register_raw(ctx->card_ctx, ctx->pipe, reg);
 }
 
 static void had_write_register(struct snd_intelhad *ctx, u32 reg, u32 val)
 {
 	if (ctx->connected)
-		had_write_register_raw(ctx, reg, val);
+		had_write_register_raw(ctx->card_ctx, ctx->pipe, reg, val);
 }
 
 /*
@@ -1361,6 +1378,9 @@ static void had_process_hot_plug(struct snd_intelhad *intelhaddata)
 		return;
 	}
 
+	/* Disable Audio */
+	had_enable_audio(intelhaddata, false);
+
 	intelhaddata->connected = true;
 	dev_dbg(intelhaddata->dev,
 		"%s @ %d:DEBUG PLUG/UNPLUG : HAD_DRV_CONNECTED\n",
@@ -1523,26 +1543,31 @@ static const struct snd_kcontrol_new had_controls[] = {
 static irqreturn_t display_pipe_interrupt_handler(int irq, void *dev_id)
 {
 	struct snd_intelhad_card *card_ctx = dev_id;
-	int port;
+	u32 audio_stat[3] = {};
+	int pipe, port;
+
+	for_each_pipe(card_ctx, pipe) {
+		/* use raw register access to ack IRQs even while disconnected */
+		audio_stat[pipe] = had_read_register_raw(card_ctx, pipe,
+							 AUD_HDMI_STATUS) &
+			(HDMI_AUDIO_UNDERRUN | HDMI_AUDIO_BUFFER_DONE);
+
+		if (audio_stat[pipe])
+			had_write_register_raw(card_ctx, pipe,
+					       AUD_HDMI_STATUS, audio_stat[pipe]);
+	}
 
 	for_each_port(card_ctx, port) {
 		struct snd_intelhad *ctx = &card_ctx->pcm_ctx[port];
-		u32 audio_stat;
+		int pipe = ctx->pipe;
 
-		/* use raw register access to ack IRQs even while disconnected */
-		audio_stat = had_read_register_raw(ctx, AUD_HDMI_STATUS);
-
-		if (audio_stat & HDMI_AUDIO_UNDERRUN) {
-			had_write_register_raw(ctx, AUD_HDMI_STATUS,
-					       HDMI_AUDIO_UNDERRUN);
-			had_process_buffer_underrun(ctx);
-		}
+		if (pipe < 0)
+			continue;
 
-		if (audio_stat & HDMI_AUDIO_BUFFER_DONE) {
-			had_write_register_raw(ctx, AUD_HDMI_STATUS,
-					       HDMI_AUDIO_BUFFER_DONE);
+		if (audio_stat[pipe] & HDMI_AUDIO_BUFFER_DONE)
 			had_process_buffer_done(ctx);
-		}
+		if (audio_stat[pipe] & HDMI_AUDIO_UNDERRUN)
+			had_process_buffer_underrun(ctx);
 	}
 
 	return IRQ_HANDLED;
@@ -1551,16 +1576,12 @@ static irqreturn_t display_pipe_interrupt_handler(int irq, void *dev_id)
 /*
  * monitor plug/unplug notification from i915; just kick off the work
  */
-static void notify_audio_lpe(struct platform_device *pdev)
+static void notify_audio_lpe(struct platform_device *pdev, int port)
 {
 	struct snd_intelhad_card *card_ctx = platform_get_drvdata(pdev);
-	int port;
+	struct snd_intelhad *ctx = &card_ctx->pcm_ctx[port];
 
-	for_each_port(card_ctx, port) {
-		struct snd_intelhad *ctx = &card_ctx->pcm_ctx[port];
-
-		schedule_work(&ctx->hdmi_audio_wq);
-	}
+	schedule_work(&ctx->hdmi_audio_wq);
 }
 
 /* the work to handle monitor hot plug/unplug */
@@ -1569,34 +1590,27 @@ static void had_audio_wq(struct work_struct *work)
 	struct snd_intelhad *ctx =
 		container_of(work, struct snd_intelhad, hdmi_audio_wq);
 	struct intel_hdmi_lpe_audio_pdata *pdata = ctx->dev->platform_data;
-	struct intel_hdmi_lpe_audio_port_pdata *ppdata = &pdata->port;
+	struct intel_hdmi_lpe_audio_port_pdata *ppdata = &pdata->port[ctx->port];
 
 	pm_runtime_get_sync(ctx->dev);
 	mutex_lock(&ctx->mutex);
 	if (ppdata->pipe < 0) {
-		dev_dbg(ctx->dev, "%s: Event: HAD_NOTIFY_HOT_UNPLUG\n",
-			__func__);
+		dev_dbg(ctx->dev, "%s: Event: HAD_NOTIFY_HOT_UNPLUG : port = %d\n",
+			__func__, ctx->port);
+
 		memset(ctx->eld, 0, sizeof(ctx->eld)); /* clear the old ELD */
+
+		ctx->dp_output = false;
+		ctx->tmds_clock_speed = 0;
+		ctx->link_rate = 0;
+
+		/* Shut down the stream */
 		had_process_hot_unplug(ctx);
+
+		ctx->pipe = -1;
 	} else {
 		dev_dbg(ctx->dev, "%s: HAD_NOTIFY_ELD : port = %d, tmds = %d\n",
-			__func__, ppdata->port, ppdata->ls_clock);
-
-		switch (ppdata->pipe) {
-		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(ctx->dev, "Invalid pipe %d\n",
-				ppdata->pipe);
-			break;
-		}
+			__func__, ctx->port, ppdata->ls_clock);
 
 		memcpy(ctx->eld, ppdata->eld, sizeof(ctx->eld));
 
@@ -1609,11 +1623,18 @@ static void had_audio_wq(struct work_struct *work)
 			ctx->link_rate = 0;
 		}
 
+		/*
+		 * Shut down the stream before we change
+		 * the pipe assignment for this pcm device
+		 */
 		had_process_hot_plug(ctx);
 
-		/* Process mode change if stream is active */
+		ctx->pipe = ppdata->pipe;
+
+		/* Restart the stream if necessary */
 		had_process_mode_change(ctx);
 	}
+
 	mutex_unlock(&ctx->mutex);
 	pm_runtime_mark_last_busy(ctx->dev);
 	pm_runtime_put_autosuspend(ctx->dev);
@@ -1794,7 +1815,8 @@ static int hdmi_lpe_audio_probe(struct platform_device *pdev)
 
 	init_channel_allocations();
 
-	card_ctx->num_ports = 1;
+	card_ctx->num_pipes = pdata->num_pipes;
+	card_ctx->num_ports = pdata->num_ports;
 
 	for_each_port(card_ctx, port) {
 		struct snd_intelhad *ctx = &card_ctx->pcm_ctx[port];
@@ -1802,12 +1824,12 @@ static int hdmi_lpe_audio_probe(struct platform_device *pdev)
 
 		ctx->card_ctx = card_ctx;
 		ctx->dev = card_ctx->dev;
+		ctx->port = port;
+		ctx->pipe = -1;
 
 		INIT_WORK(&ctx->hdmi_audio_wq, had_audio_wq);
 
-		ctx->had_config_offset = AUDIO_HDMI_CONFIG_A;
-
-		ret = snd_pcm_new(card, INTEL_HAD, PCM_INDEX, MAX_PB_STREAMS,
+		ret = snd_pcm_new(card, INTEL_HAD, port, MAX_PB_STREAMS,
 				  MAX_CAP_STREAMS, &pcm);
 		if (ret)
 			goto err;
diff --git a/sound/x86/intel_hdmi_audio.h b/sound/x86/intel_hdmi_audio.h
index 2725964ebc46..0d91bb5dbab7 100644
--- a/sound/x86/intel_hdmi_audio.h
+++ b/sound/x86/intel_hdmi_audio.h
@@ -32,7 +32,6 @@
 
 #include "intel_hdmi_lpe_audio.h"
 
-#define PCM_INDEX		0
 #define MAX_PB_STREAMS		1
 #define MAX_CAP_STREAMS		0
 #define BYTES_PER_WORD		0x4
@@ -112,6 +111,8 @@ struct snd_intelhad {
 	struct snd_pcm_chmap *chmap;
 	int tmds_clock_speed;
 	int link_rate;
+	int port; /* fixed */
+	int pipe; /* can change dynamically */
 
 	/* ring buffer (BD) position index */
 	unsigned int bd_head;
@@ -123,7 +124,6 @@ struct snd_intelhad {
 	unsigned int period_bytes;	/* PCM period size in bytes */
 
 	/* internal stuff */
-	unsigned int had_config_offset;
 	union aud_cfg aud_config;	/* AUD_CONFIG reg value cache */
 	struct work_struct hdmi_audio_wq;
 	struct mutex mutex; /* for protecting chmap and eld */
@@ -138,8 +138,9 @@ struct snd_intelhad_card {
 	/* internal stuff */
 	int irq;
 	void __iomem *mmio_start;
+	int num_pipes;
 	int num_ports;
-	struct snd_intelhad pcm_ctx[3];
+	struct snd_intelhad pcm_ctx[3]; /* one for each port */
 };
 
 #endif /* _INTEL_HDMI_AUDIO_ */
-- 
2.10.2

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

[PATCH alsa-lib] conf: Add multiple hdmi pcm definition for Intel LPE audio

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Now that the kernel driver exposes several pcm devices, update
the hdmi pcm definitions to match.

Cc: Takashi Iwai <tiwai@suse.de>
Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 src/conf/cards/HdmiLpeAudio.conf | 74 ++++++++++++++++++++++++++++++++++++++--
 1 file changed, 72 insertions(+), 2 deletions(-)

diff --git a/src/conf/cards/HdmiLpeAudio.conf b/src/conf/cards/HdmiLpeAudio.conf
index 61bdfeae2917..ad174b8ac450 100644
--- a/src/conf/cards/HdmiLpeAudio.conf
+++ b/src/conf/cards/HdmiLpeAudio.conf
@@ -51,11 +51,14 @@ HdmiLpeAudio.pcm.default {
 
 <confdir:pcm/hdmi.conf>
 
-HdmiLpeAudio.pcm.hdmi.0 {
-	@args [ CARD AES0 AES1 AES2 AES3 ]
+HdmiLpeAudio.pcm.hdmi.common {
+	@args [ CARD DEVICE AES0 AES1 AES2 AES3 ]
 	@args.CARD {
 		type string
 	}
+	@args.DEVICE {
+		type integer
+	}
 	@args.AES0 {
 		type integer
 	}
@@ -72,6 +75,7 @@ HdmiLpeAudio.pcm.hdmi.0 {
 	slave.pcm {
 		type hw
 		card $CARD
+		device $DEVICE
 	}
 	hooks.0 {
 		type ctl_elems
@@ -86,3 +90,69 @@ HdmiLpeAudio.pcm.hdmi.0 {
 		]
 	}
 }
+
+HdmiLpeAudio.pcm.hdmi.0 {
+	@args [ CARD AES0 AES1 AES2 AES3 ]
+	@args.CARD { type string }
+	@args.AES0 { type integer }
+	@args.AES1 { type integer }
+	@args.AES2 { type integer }
+	@args.AES3 { type integer }
+	@func refer
+	name {
+		@func concat
+		strings [
+			"cards.HdmiLpeAudio.pcm.hdmi.common:"
+			"CARD=" $CARD ","
+			"DEVICE=0,"
+			"AES0=" $AES0 ","
+			"AES1=" $AES1 ","
+			"AES2=" $AES2 ","
+			"AES3=" $AES3
+		]
+	}
+}
+
+HdmiLpeAudio.pcm.hdmi.1 {
+	@args [ CARD AES0 AES1 AES2 AES3 ]
+	@args.CARD { type string }
+	@args.AES0 { type integer }
+	@args.AES1 { type integer }
+	@args.AES2 { type integer }
+	@args.AES3 { type integer }
+	@func refer
+	name {
+		@func concat
+		strings [
+			"cards.HdmiLpeAudio.pcm.hdmi.common:"
+			"CARD=" $CARD ","
+			"DEVICE=1,"
+			"AES0=" $AES0 ","
+			"AES1=" $AES1 ","
+			"AES2=" $AES2 ","
+			"AES3=" $AES3
+		]
+	}
+}
+
+HdmiLpeAudio.pcm.hdmi.2 {
+	@args [ CARD AES0 AES1 AES2 AES3 ]
+	@args.CARD { type string }
+	@args.AES0 { type integer }
+	@args.AES1 { type integer }
+	@args.AES2 { type integer }
+	@args.AES3 { type integer }
+	@func refer
+	name {
+		@func concat
+		strings [
+			"cards.HdmiLpeAudio.pcm.hdmi.common:"
+			"CARD=" $CARD ","
+			"DEVICE=2,"
+			"AES0=" $AES0 ","
+			"AES1=" $AES1 ","
+			"AES2=" $AES2 ","
+			"AES3=" $AES3
+		]
+	}
+}
-- 
2.10.2

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

✓ Fi.CI.BAT: success for conf: Add multiple hdmi pcm definition for Intel LPE audio

[Patchwork]

== Series Details ==

Series: conf: Add multiple hdmi pcm definition for Intel LPE audio
URL   : https://patchwork.freedesktop.org/series/23639/
State : success

== Summary ==

Series 23639v1 conf: Add multiple hdmi pcm definition for Intel LPE audio
https://patchwork.freedesktop.org/api/1.0/series/23639/revisions/1/mbox/

Test gem_exec_suspend:
        Subgroup basic-s4-devices:
                dmesg-warn -> PASS       (fi-kbl-7560u) fdo#100125

fdo#100125 https://bugs.freedesktop.org/show_bug.cgi?id=100125

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time:435s
fi-bdw-gvtdvm    total:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  time:423s
fi-bsw-n3050     total:278  pass:242  dwarn:0   dfail:0   fail:0   skip:36  time:577s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time:512s
fi-bxt-t5700     total:278  pass:258  dwarn:0   dfail:0   fail:0   skip:20  time:544s
fi-byt-j1900     total:278  pass:254  dwarn:0   dfail:0   fail:0   skip:24  time:487s
fi-byt-n2820     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:483s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:406s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:410s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time:415s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:495s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:480s
fi-kbl-7500u     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:459s
fi-kbl-7560u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:573s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:455s
fi-skl-6700hq    total:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17  time:565s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time:459s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:489s
fi-skl-gvtdvm    total:278  pass:265  dwarn:0   dfail:0   fail:0   skip:13  time:429s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:532s
fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29  time:396s

8b5a41bbd270c3a8db6d48bc1d6d6bafb59e6753 drm-tip: 2017y-04m-27d-13h-10m-59s UTC integration manifest
bc96bd1 ALSA: x86: Register multiple PCM devices for the LPE audio card
e09ae3e ALSA: x86: Split snd_intelhad into card and PCM specific structures
a9f1076 ALSA: x86: Prepare LPE audio ctls for multiple PCMs
ca71392 drm/i915: Clean up the LPE audio platform data
13211f9 drm/i915: Reorganize intel_lpe_audio_notify() arguments
023f2ba drm/i915: Remove hdmi_connected from LPE audio pdata
26eda33 drm/i915: Replace tmds_clock_speed and link_rate with just ls_clock
223ac7f drm/i915: Remove the unused pending_notify from LPE platform data
1df8df6 drm/i915: Stop pretending to mask/unmask LPE audio interrupts
ff5766c ALSA: x86: Clear the pdata.notify_lpe_audio pointer before teardown
ddc7b54 drm/i915: Fix runtime PM for LPE audio

== Logs ==

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

Re: [PATCH 01/11] drm/i915: Fix runtime PM for LPE audio

[Takashi Iwai]

On Thu, 27 Apr 2017 18:02:20 +0200,
ville.syrjala@linux.intel.com wrote:
> 
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Not calling pm_runtime_enable() means that runtime PM can't be
> enabled at all via sysfs. So we definitely need to call it
> from somewhere.
> 
> Calling it from the driver seems like a bad idea because it
> would have to be paired with a pm_runtime_disable() at driver
> unload time, otherwise the core gets upset. Also if there's
> no LPE audio driver loaded then we couldn't runtime suspend
> i915 either.
> 
> So it looks like a better plan is to call it from i915 when
> we register the platform device. That seems to match how
> pci generally does things. I cargo culted the
> pm_runtime_forbid() and pm_runtime_set_active() calls from
> pci as well.
> 
> The exposed runtime PM API is massive an thorougly misleading, so
> I don't actually know if this is how you're supposed to use the API
> or not. But it seems to work. I can now runtime suspend i915 again
> with or without the LPE audio driver loaded, and reloading the
> LPE audio driver also seems to work.
> 
> Note that powertop won't auto-tune runtime PM for platform devices,
> which is a little annoying. So I'm not sure that leaving runtime
> PM in "on" mode by default is the best choice here. But I've left
> it like that for now at least.

The reason I didn't proactively turn on the runtime PM was that it
often caused a few seconds of pause to the A/V receivers before
actually starting playing.

There is a planned feature to keep sending the silent stream even
after stopping the stream, but it's not implemented yet.

> Also remove the comment about there not being much benefit from
> LPE audio runtime PM. Not allowing runtime PM blocks i915 runtime
> PM, which will also block s0ix, and that could have a measurable
> impact on power consumption.
> 
> Cc: Takashi Iwai <tiwai@suse.de>
> Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> Fixes: 0b6b524f3915 ("ALSA: x86: Don't enable runtime PM as default")
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Takashi Iwai <tiwai@suse.de>

IMO, this should be tagged with Cc to stable.


thanks,

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

Re: [PATCH 02/11] ALSA: x86: Clear the pdata.notify_lpe_audio pointer before teardown

[Takashi Iwai]

On Thu, 27 Apr 2017 18:02:21 +0200,
ville.syrjala@linux.intel.com wrote:
> 
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Clear the notify function pointer in the platform data before we tear
> down the driver. Otherwise i915 would end up calling a stale function
> pointer and possibly explode.
> 
> Cc: Takashi Iwai <tiwai@suse.de>
> Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Takashi Iwai <tiwai@suse.de>

This deserves also Cc to stable, IMO.


thanks,

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

Re: [PATCH v2 00/11] drm/i915: LPE audio runtime PM and multipipe (v2)

[Takashi Iwai]

On Thu, 27 Apr 2017 18:02:19 +0200,
ville.syrjala@linux.intel.com wrote:
> 
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Okay, here's the second attempt at getting multiple pipes playing back
> audio on the VLV/CHV HDMI LPE audio device. The main change from v1 is
> that now the PCM devices are associated with ports instead of pipes,
> so the audio from one device always gets output on the same display.
> 
> I've also tacked on the alsa-lib conf update. No clue whether it's
> really correct or not (the config language isn't a close friend
> of mine).
> 
> BTW I did notice that with LPE audio all the controls say iface=PCM,
> whereas on HDA a bunch of them say iface=MIXER. No idea if that's
> OK or not, just something I spotted when I was comparing the results
> with HDA.

We generally accept both iface types for IEC958 stuff, since
historically many drivers have already mixed them up.  So it's no
problem :)


> Entire series available here:
> git://github.com/vsyrjala/linux.git lpe_audio_multipipe_2
> 
> Cc: Takashi Iwai <tiwai@suse.de>
> Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

All look good, and feel free to take my reviewed-by tag
  Reviewed-by: Takashi Iwai <tiwai@suse.de>

As said previously, my only slight concern is the compatibility.
But, in the current situation with PulseAudio, only few people would
use this driver, so it shouldn't be so big impact, I suppose.

BTW, which port is used in general on BYT/CHT?

Oh, also, I suppose you want to carry these over i915 tree?
I don't mind either way, I can take them through sound tree if
preferred, too.


thanks,

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

Re: [alsa-devel] [PATCH v2 00/11] drm/i915: LPE audio runtime PM and multipipe (v2)

[Pierre-Louis Bossart]

On 04/28/2017 03:41 AM, Takashi Iwai wrote:
> On Thu, 27 Apr 2017 18:02:19 +0200,
> ville.syrjala@linux.intel.com wrote:
>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>
>> Okay, here's the second attempt at getting multiple pipes playing back
>> audio on the VLV/CHV HDMI LPE audio device. The main change from v1 is
>> that now the PCM devices are associated with ports instead of pipes,
>> so the audio from one device always gets output on the same display.
>>
>> I've also tacked on the alsa-lib conf update. No clue whether it's
>> really correct or not (the config language isn't a close friend
>> of mine).
>>
>> BTW I did notice that with LPE audio all the controls say iface=PCM,
>> whereas on HDA a bunch of them say iface=MIXER. No idea if that's
>> OK or not, just something I spotted when I was comparing the results
>> with HDA.
> We generally accept both iface types for IEC958 stuff, since
> historically many drivers have already mixed them up.  So it's no
> problem :)
>
>
>> Entire series available here:
>> git://github.com/vsyrjala/linux.git lpe_audio_multipipe_2
>>
>> Cc: Takashi Iwai <tiwai@suse.de>
>> Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> All look good, and feel free to take my reviewed-by tag
>    Reviewed-by: Takashi Iwai <tiwai@suse.de>
>
> As said previously, my only slight concern is the compatibility.
> But, in the current situation with PulseAudio, only few people would
> use this driver, so it shouldn't be so big impact, I suppose.
>
> BTW, which port is used in general on BYT/CHT?
>
> Oh, also, I suppose you want to carry these over i915 tree?
> I don't mind either way, I can take them through sound tree if
> preferred, too.
I see frequent oops on startup with this lpe_audio_multipipe_2 branch 
with my CHT device not booting or no HDMI audio device created.
Not sure if these issues are due to the new patches or to the rest of 
the drm code?

[    5.529023] BUG: unable to handle kernel NULL pointer dereference 
at           (null)
[    5.529143] IP: hdmi_lpe_audio_probe+0x40f/0x650 [snd_hdmi_lpe_audio]
[    5.529202] PGD 0

[    5.529242] Oops: 0000 [#1] SMP
[    5.529274] Modules linked in: snd_soc_sst_atom_hifi2_platform 
snd_soc_sst_match snd_soc_core snd_compress lpc_ich snd_seq 
snd_seq_device shpchp snd_hdmi_lpe_audio(+) snd_pcm snd_timer dw_dmac 
snd soundcore i2c_designware_platform(+) i2c_designware_core 
spi_pxa2xx_platform acpi_pad mac_hid nfsd auth_rpcgss nfs_acl lockd 
grace sunrpc ip_tables x_tables hid_generic mmc_block i2c_hid usbhid hid 
autofs4
[    5.529605] CPU: 2 PID: 512 Comm: systemd-udevd Not tainted 
4.11.0-rc8-test+ #11
[    5.529671] Hardware name: ZOTAC XXXXXX/Cherry Trail FFD, BIOS 5.11 
09/28/2016
[    5.529736] task: ffff88007485b780 task.stack: ffffc90000bfc000
[    5.529793] RIP: 0010:hdmi_lpe_audio_probe+0x40f/0x650 
[snd_hdmi_lpe_audio]
[    5.529855] RSP: 0018:ffffc90000bffaf0 EFLAGS: 00010246
[    5.529904] RAX: 0000000000000000 RBX: ffff880079209898 RCX: 
ffff88007920f078
[    5.529967] RDX: 0000000000000014 RSI: ffffc90000bffb28 RDI: 
0000000000000002
[    5.530031] RBP: ffffc90000bffb70 R08: 0000000000000001 R09: 
0000000000000000
[    5.530094] R10: ffff88007441bf00 R11: ffffc90000bffb36 R12: 
ffff88007920ef20
[    5.530159] R13: ffff88007920ef48 R14: 0000000000005688 R15: 
0000000000000047
[    5.530225] FS:  00007f627c988640(0000) GS:ffff88007b300000(0000) 
knlGS:0000000000000000
[    5.530299] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[    5.530352] CR2: 0000000000000000 CR3: 0000000078cb8000 CR4: 
00000000001006e0
[    5.530416] Call Trace:
[    5.530452]  platform_drv_probe+0x3b/0xa0
[    5.530494]  driver_probe_device+0x2bb/0x460
[    5.530538]  __driver_attach+0xdf/0xf0
[    5.530576]  ? driver_probe_device+0x460/0x460
[    5.530620]  bus_for_each_dev+0x60/0xa0
[    5.530658]  driver_attach+0x1e/0x20
[    5.530693]  bus_add_driver+0x170/0x270
[    5.530731]  driver_register+0x60/0xe0
[    5.530769]  ? 0xffffffffa01cb000
[    5.530803]  __platform_driver_register+0x36/0x40
[    5.530851]  hdmi_lpe_audio_driver_init+0x17/0x1000 [snd_hdmi_lpe_audio]
[    5.530915]  do_one_initcall+0x43/0x180
[    5.530956]  ? __vunmap+0x81/0xd0
[    5.530991]  ? kfree+0x14c/0x160
[    5.531024]  ? kmem_cache_alloc_trace+0x38/0x150
[    5.531070]  do_init_module+0x5f/0x1f8
[    5.531108]  load_module+0x271e/0x2bd0
[    5.531147]  ? kernel_read_file+0x1a3/0x1c0
[    5.531188]  SYSC_finit_module+0xbc/0xf0
[    5.531226]  ? SYSC_finit_module+0xbc/0xf0
[    5.531267]  SyS_finit_module+0xe/0x10
[    5.531305]  do_syscall_64+0x6e/0x180
[    5.531345]  entry_SYSCALL64_slow_path+0x25/0x25
[    5.531389] RIP: 0033:0x7f627b5fbbf9
[    5.531424] RSP: 002b:00007ffe053eee68 EFLAGS: 00000246 ORIG_RAX: 
0000000000000139
[    5.531493] RAX: ffffffffffffffda RBX: 000055d6c745b690 RCX: 
00007f627b5fbbf9
[    5.531558] RDX: 0000000000000000 RSI: 00007f627c134995 RDI: 
0000000000000007
[    5.531622] RBP: 00007f627c134995 R08: 0000000000000000 R09: 
00007ffe053eef80
[    5.531687] R10: 0000000000000007 R11: 0000000000000246 R12: 
0000000000000000
[    5.531751] R13: 000055d6c7459ae0 R14: 0000000000020000 R15: 
000055d6c745b690
[    5.531816] Code: 48 8b 45 b0 8b 48 18 e8 e0 cb 22 e1 49 8b 44 24 28 
4a 8d 8c 33 58 01 00 00 48 8d 75 b8 45 31 c9 41 b8 01 00 00 00 ba 14 00 
00 00 <48> 8b 38 e8 a9 2a ff ff 85 c0 0f 88 09 01 00 00 49 8b 84 24 58
[    5.532026] RIP: hdmi_lpe_audio_probe+0x40f/0x650 
[snd_hdmi_lpe_audio] RSP: ffffc90000bffaf0
[    5.532101] CR2: 0000000000000000
[    5.532168] ---[ end trace e832e97f0e744700 ]---
[    5.534830] i2c i2c-8: i2c read failed
[    5.554395] i2c i2c-8: i2c read failed

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

Re: [alsa-devel] [PATCH v2 00/11] drm/i915: LPE audio runtime PM and multipipe (v2)

[Ville Syrjälä]

On Fri, Apr 28, 2017 at 12:10:31PM -0500, Pierre-Louis Bossart wrote:
> 
> 
> On 04/28/2017 03:41 AM, Takashi Iwai wrote:
> > On Thu, 27 Apr 2017 18:02:19 +0200,
> > ville.syrjala@linux.intel.com wrote:
> >> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>
> >> Okay, here's the second attempt at getting multiple pipes playing back
> >> audio on the VLV/CHV HDMI LPE audio device. The main change from v1 is
> >> that now the PCM devices are associated with ports instead of pipes,
> >> so the audio from one device always gets output on the same display.
> >>
> >> I've also tacked on the alsa-lib conf update. No clue whether it's
> >> really correct or not (the config language isn't a close friend
> >> of mine).
> >>
> >> BTW I did notice that with LPE audio all the controls say iface=PCM,
> >> whereas on HDA a bunch of them say iface=MIXER. No idea if that's
> >> OK or not, just something I spotted when I was comparing the results
> >> with HDA.
> > We generally accept both iface types for IEC958 stuff, since
> > historically many drivers have already mixed them up.  So it's no
> > problem :)
> >
> >
> >> Entire series available here:
> >> git://github.com/vsyrjala/linux.git lpe_audio_multipipe_2
> >>
> >> Cc: Takashi Iwai <tiwai@suse.de>
> >> Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> > All look good, and feel free to take my reviewed-by tag
> >    Reviewed-by: Takashi Iwai <tiwai@suse.de>
> >
> > As said previously, my only slight concern is the compatibility.
> > But, in the current situation with PulseAudio, only few people would
> > use this driver, so it shouldn't be so big impact, I suppose.
> >
> > BTW, which port is used in general on BYT/CHT?
> >
> > Oh, also, I suppose you want to carry these over i915 tree?
> > I don't mind either way, I can take them through sound tree if
> > preferred, too.
> I see frequent oops on startup with this lpe_audio_multipipe_2 branch 
> with my CHT device not booting or no HDMI audio device created.
> Not sure if these issues are due to the new patches or to the rest of 
> the drm code?
> 
> [    5.529023] BUG: unable to handle kernel NULL pointer dereference 
> at           (null)
> [    5.529143] IP: hdmi_lpe_audio_probe+0x40f/0x650 [snd_hdmi_lpe_audio]
> [    5.529202] PGD 0
> 
> [    5.529242] Oops: 0000 [#1] SMP
> [    5.529274] Modules linked in: snd_soc_sst_atom_hifi2_platform 
> snd_soc_sst_match snd_soc_core snd_compress lpc_ich snd_seq 
> snd_seq_device shpchp snd_hdmi_lpe_audio(+) snd_pcm snd_timer dw_dmac 
> snd soundcore i2c_designware_platform(+) i2c_designware_core 
> spi_pxa2xx_platform acpi_pad mac_hid nfsd auth_rpcgss nfs_acl lockd 
> grace sunrpc ip_tables x_tables hid_generic mmc_block i2c_hid usbhid hid 
> autofs4
> [    5.529605] CPU: 2 PID: 512 Comm: systemd-udevd Not tainted 
> 4.11.0-rc8-test+ #11
> [    5.529671] Hardware name: ZOTAC XXXXXX/Cherry Trail FFD, BIOS 5.11 
> 09/28/2016
> [    5.529736] task: ffff88007485b780 task.stack: ffffc90000bfc000
> [    5.529793] RIP: 0010:hdmi_lpe_audio_probe+0x40f/0x650 
> [snd_hdmi_lpe_audio]
> [    5.529855] RSP: 0018:ffffc90000bffaf0 EFLAGS: 00010246
> [    5.529904] RAX: 0000000000000000 RBX: ffff880079209898 RCX: 
> ffff88007920f078
> [    5.529967] RDX: 0000000000000014 RSI: ffffc90000bffb28 RDI: 
> 0000000000000002
> [    5.530031] RBP: ffffc90000bffb70 R08: 0000000000000001 R09: 
> 0000000000000000
> [    5.530094] R10: ffff88007441bf00 R11: ffffc90000bffb36 R12: 
> ffff88007920ef20
> [    5.530159] R13: ffff88007920ef48 R14: 0000000000005688 R15: 
> 0000000000000047
> [    5.530225] FS:  00007f627c988640(0000) GS:ffff88007b300000(0000) 
> knlGS:0000000000000000
> [    5.530299] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [    5.530352] CR2: 0000000000000000 CR3: 0000000078cb8000 CR4: 
> 00000000001006e0
> [    5.530416] Call Trace:
> [    5.530452]  platform_drv_probe+0x3b/0xa0
> [    5.530494]  driver_probe_device+0x2bb/0x460
> [    5.530538]  __driver_attach+0xdf/0xf0
> [    5.530576]  ? driver_probe_device+0x460/0x460
> [    5.530620]  bus_for_each_dev+0x60/0xa0
> [    5.530658]  driver_attach+0x1e/0x20
> [    5.530693]  bus_add_driver+0x170/0x270
> [    5.530731]  driver_register+0x60/0xe0
> [    5.530769]  ? 0xffffffffa01cb000
> [    5.530803]  __platform_driver_register+0x36/0x40
> [    5.530851]  hdmi_lpe_audio_driver_init+0x17/0x1000 [snd_hdmi_lpe_audio]
> [    5.530915]  do_one_initcall+0x43/0x180
> [    5.530956]  ? __vunmap+0x81/0xd0
> [    5.530991]  ? kfree+0x14c/0x160
> [    5.531024]  ? kmem_cache_alloc_trace+0x38/0x150
> [    5.531070]  do_init_module+0x5f/0x1f8
> [    5.531108]  load_module+0x271e/0x2bd0
> [    5.531147]  ? kernel_read_file+0x1a3/0x1c0
> [    5.531188]  SYSC_finit_module+0xbc/0xf0
> [    5.531226]  ? SYSC_finit_module+0xbc/0xf0
> [    5.531267]  SyS_finit_module+0xe/0x10
> [    5.531305]  do_syscall_64+0x6e/0x180
> [    5.531345]  entry_SYSCALL64_slow_path+0x25/0x25
> [    5.531389] RIP: 0033:0x7f627b5fbbf9
> [    5.531424] RSP: 002b:00007ffe053eee68 EFLAGS: 00000246 ORIG_RAX: 
> 0000000000000139
> [    5.531493] RAX: ffffffffffffffda RBX: 000055d6c745b690 RCX: 
> 00007f627b5fbbf9
> [    5.531558] RDX: 0000000000000000 RSI: 00007f627c134995 RDI: 
> 0000000000000007
> [    5.531622] RBP: 00007f627c134995 R08: 0000000000000000 R09: 
> 00007ffe053eef80
> [    5.531687] R10: 0000000000000007 R11: 0000000000000246 R12: 
> 0000000000000000
> [    5.531751] R13: 000055d6c7459ae0 R14: 0000000000020000 R15: 
> 000055d6c745b690
> [    5.531816] Code: 48 8b 45 b0 8b 48 18 e8 e0 cb 22 e1 49 8b 44 24 28 
> 4a 8d 8c 33 58 01 00 00 48 8d 75 b8 45 31 c9 41 b8 01 00 00 00 ba 14 00 
> 00 00 <48> 8b 38 e8 a9 2a ff ff 85 c0 0f 88 09 01 00 00 49 8b 84 24 58

Disassembling that I get:

  4005a0:       48 8b 45 b0             mov    -0x50(%rbp),%rax
  4005a4:       8b 48 18                mov    0x18(%rax),%ecx
  4005a7:       e8 e0 cb 22 e1          callq  ffffffffe162d18c <_end+0xffffffffe102c14c>
  4005ac:       49 8b 44 24 28          mov    0x28(%r12),%rax
  4005b1:       4a 8d 8c 33 58 01 00    lea    0x158(%rbx,%r14,1),%rcx
  4005b8:       00 
  4005b9:       48 8d 75 b8             lea    -0x48(%rbp),%rsi
  4005bd:       45 31 c9                xor    %r9d,%r9d
  4005c0:       41 b8 01 00 00 00       mov    $0x1,%r8d
  4005c6:       ba 14 00 00 00          mov    $0x14,%edx
  4005cb:       48 8b 38                mov    (%rax),%rdi
  4005ce:       e8 a9 2a ff ff          callq  3f307c <_init-0xd32c>
  4005d3:       85 c0                   test   %eax,%eax
  4005d5:       0f 88 09 01 00 00       js     4006e4 <__GNU_EH_FRAME_HDR+0x100>
  4005db:       49                      rex.WB
  4005dc:       8b                      .byte 0x8b
  4005dd:       84 24 58                test   %ah,(%rax,%rbx,2)

Comparing that with the disassembly from my build, that
first call looks like the snprintf() and the second one would
then be snd_jack_new(). So it seems to blow in the
ctx->card_ctx->card dereference. But I don't really see how
a NULL pointer could sneak 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: [alsa-devel] [PATCH v2 11/11] ALSA: x86: Register multiple PCM devices for the LPE audio card

[Pierre-Louis Bossart]

A quick bisect tells me this last patch looks problematic. I don't have 
time to look further into it today, hope this helps progress in finding 
the issue. This is on a CHT device with HDMI plugged in and DP left out 
unconnected for now.

$ git bisect good
9af5b5732365b8ea29000d1ad14800bb091a0724 is the first bad commit
commit 9af5b5732365b8ea29000d1ad14800bb091a0724
Author: Ville Syrjälä <ville.syrjala@linux.intel.com>
Date:   Tue Apr 25 20:42:40 2017 +0300

     ALSA: x86: Register multiple PCM devices for the LPE audio card

     Now that everything is in place let's register a PCM device for
     each port of the display engine. This will make it possible to
     actually output audio to multiple displays at the same time. And
     it avoids modesets on unrelated displays from clobbering up the
     ELD and whatnot for the display currently doing the playback.

     v2: Add a PCM per port instead of per pipe

     Cc: Takashi Iwai <tiwai@suse.de>
     Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
     Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

:040000 040000 17f94eecd597b97ccc003e2d27c03eadceb279f5 
271d4c3130f9cc1334e79bf60b7fdc7337192655 M    drivers
:040000 040000 6806ba942f3c0844dcf6ffdfdd751c2007e5680f 
8b9e1d1f82a12febe705a771654fadc08c02c90f M    include
:040000 040000 e1f46a21e1beaf9535b3e807f4cfeea5ad7dbe47 
afd86621214380c86769c30d6405d9335143cf6b M    sound

$ git bisect log
git bisect start
# bad: [9af5b5732365b8ea29000d1ad14800bb091a0724] ALSA: x86: Register 
multiple PCM devices for the LPE audio card
git bisect bad 9af5b5732365b8ea29000d1ad14800bb091a0724
# good: [8b5a41bbd270c3a8db6d48bc1d6d6bafb59e6753] drm-tip: 
2017y-04m-27d-13h-10m-59s UTC integration manifest
git bisect good 8b5a41bbd270c3a8db6d48bc1d6d6bafb59e6753
# good: [63e34e5d2e8aaf85dfe48085e5dbbb7215da80ba] drm/i915: Replace 
tmds_clock_speed and link_rate with just ls_clock
git bisect good 63e34e5d2e8aaf85dfe48085e5dbbb7215da80ba
# good: [dc0ae8e9c2f395b24cba7a404f2ff49e7272bf4b] drm/i915: Clean up 
the LPE audio platform data
git bisect good dc0ae8e9c2f395b24cba7a404f2ff49e7272bf4b
# good: [7d80cfe6f7eafe6cddf36cd6e227d54a45c6f175] ALSA: x86: Split 
snd_intelhad into card and PCM specific structures
git bisect good 7d80cfe6f7eafe6cddf36cd6e227d54a45c6f175
# first bad commit: [9af5b5732365b8ea29000d1ad14800bb091a0724] ALSA: 
x86: Register multiple PCM devices for the LPE audio card



On 04/27/2017 11:02 AM, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Now that everything is in place let's register a PCM device for
> each port of the display engine. This will make it possible to
> actually output audio to multiple displays at the same time. And
> it avoids modesets on unrelated displays from clobbering up the
> ELD and whatnot for the display currently doing the playback.
>
> v2: Add a PCM per port instead of per pipe
>
> Cc: Takashi Iwai <tiwai@suse.de>
> Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/intel_lpe_audio.c |  19 ++---
>   include/drm/intel_lpe_audio.h          |   6 +-
>   sound/x86/intel_hdmi_audio.c           | 126 +++++++++++++++++++--------------
>   sound/x86/intel_hdmi_audio.h           |   7 +-
>   4 files changed, 92 insertions(+), 66 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c
> index bdbc235141b5..fa728ed21d1f 100644
> --- a/drivers/gpu/drm/i915/intel_lpe_audio.c
> +++ b/drivers/gpu/drm/i915/intel_lpe_audio.c
> @@ -111,7 +111,11 @@ lpe_audio_platdev_create(struct drm_i915_private *dev_priv)
>   	pinfo.size_data = sizeof(*pdata);
>   	pinfo.dma_mask = DMA_BIT_MASK(32);
>   
> -	pdata->port.pipe = -1;
> +	pdata->num_pipes = INTEL_INFO(dev_priv)->num_pipes;
> +	pdata->num_ports = IS_CHERRYVIEW(dev_priv) ? 3 : 2; /* B,C,D or B,C */
> +	pdata->port[0].pipe = -1;
> +	pdata->port[1].pipe = -1;
> +	pdata->port[2].pipe = -1;
>   	spin_lock_init(&pdata->lpe_audio_slock);
>   
>   	platdev = platform_device_register_full(&pinfo);
> @@ -319,7 +323,7 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv,
>   			    enum pipe pipe, enum port port,
>   			    const void *eld, int ls_clock, bool dp_output)
>   {
> -	unsigned long irq_flags;
> +	unsigned long irqflags;
>   	struct intel_hdmi_lpe_audio_pdata *pdata;
>   	struct intel_hdmi_lpe_audio_port_pdata *ppdata;
>   	u32 audio_enable;
> @@ -328,14 +332,12 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv,
>   		return;
>   
>   	pdata = dev_get_platdata(&dev_priv->lpe_audio.platdev->dev);
> -	ppdata = &pdata->port;
> +	ppdata = &pdata->port[port];
>   
> -	spin_lock_irqsave(&pdata->lpe_audio_slock, irq_flags);
> +	spin_lock_irqsave(&pdata->lpe_audio_slock, irqflags);
>   
>   	audio_enable = I915_READ(VLV_AUD_PORT_EN_DBG(port));
>   
> -	ppdata->port = port;
> -
>   	if (eld != NULL) {
>   		memcpy(ppdata->eld, eld, HDMI_MAX_ELD_BYTES);
>   		ppdata->pipe = pipe;
> @@ -357,8 +359,7 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv,
>   	}
>   
>   	if (pdata->notify_audio_lpe)
> -		pdata->notify_audio_lpe(dev_priv->lpe_audio.platdev);
> +		pdata->notify_audio_lpe(dev_priv->lpe_audio.platdev, port);
>   
> -	spin_unlock_irqrestore(&pdata->lpe_audio_slock,
> -			irq_flags);
> +	spin_unlock_irqrestore(&pdata->lpe_audio_slock, irqflags);
>   }
> diff --git a/include/drm/intel_lpe_audio.h b/include/drm/intel_lpe_audio.h
> index 211f1cd61153..a911530c012e 100644
> --- a/include/drm/intel_lpe_audio.h
> +++ b/include/drm/intel_lpe_audio.h
> @@ -40,9 +40,11 @@ struct intel_hdmi_lpe_audio_port_pdata {
>   };
>   
>   struct intel_hdmi_lpe_audio_pdata {
> -	struct intel_hdmi_lpe_audio_port_pdata port;
> +	struct intel_hdmi_lpe_audio_port_pdata port[3]; /* ports B,C,D */
> +	int num_ports;
> +	int num_pipes;
>   
> -	void (*notify_audio_lpe)(struct platform_device *pdev);
> +	void (*notify_audio_lpe)(struct platform_device *pdev, int pipe);
>   	spinlock_t lpe_audio_slock;
>   };
>   
> diff --git a/sound/x86/intel_hdmi_audio.c b/sound/x86/intel_hdmi_audio.c
> index 12fae26e70bb..909391d5270c 100644
> --- a/sound/x86/intel_hdmi_audio.c
> +++ b/sound/x86/intel_hdmi_audio.c
> @@ -42,6 +42,8 @@
>   #include <drm/intel_lpe_audio.h>
>   #include "intel_hdmi_audio.h"
>   
> +#define for_each_pipe(card_ctx, pipe) \
> +	for ((pipe) = 0; (pipe) < (card_ctx)->num_pipes; (pipe)++)
>   #define for_each_port(card_ctx, port) \
>   	for ((port) = 0; (port) < (card_ctx)->num_ports; (port)++)
>   
> @@ -192,15 +194,30 @@ static void had_substream_put(struct snd_intelhad *intelhaddata)
>   	spin_unlock_irqrestore(&intelhaddata->had_spinlock, flags);
>   }
>   
> +static u32 had_config_offset(int pipe)
> +{
> +	switch (pipe) {
> +	default:
> +	case 0:
> +		return AUDIO_HDMI_CONFIG_A;
> +	case 1:
> +		return AUDIO_HDMI_CONFIG_B;
> +	case 2:
> +		return AUDIO_HDMI_CONFIG_C;
> +	}
> +}
> +
>   /* Register access functions */
> -static u32 had_read_register_raw(struct snd_intelhad *ctx, u32 reg)
> +static u32 had_read_register_raw(struct snd_intelhad_card *card_ctx,
> +				 int pipe, u32 reg)
>   {
> -	return ioread32(ctx->card_ctx->mmio_start + ctx->had_config_offset + reg);
> +	return ioread32(card_ctx->mmio_start + had_config_offset(pipe) + reg);
>   }
>   
> -static void had_write_register_raw(struct snd_intelhad *ctx, u32 reg, u32 val)
> +static void had_write_register_raw(struct snd_intelhad_card *card_ctx,
> +				   int pipe, u32 reg, u32 val)
>   {
> -	iowrite32(val, ctx->card_ctx->mmio_start + ctx->had_config_offset + reg);
> +	iowrite32(val, card_ctx->mmio_start + had_config_offset(pipe) + reg);
>   }
>   
>   static void had_read_register(struct snd_intelhad *ctx, u32 reg, u32 *val)
> @@ -208,13 +225,13 @@ static void had_read_register(struct snd_intelhad *ctx, u32 reg, u32 *val)
>   	if (!ctx->connected)
>   		*val = 0;
>   	else
> -		*val = had_read_register_raw(ctx, reg);
> +		*val = had_read_register_raw(ctx->card_ctx, ctx->pipe, reg);
>   }
>   
>   static void had_write_register(struct snd_intelhad *ctx, u32 reg, u32 val)
>   {
>   	if (ctx->connected)
> -		had_write_register_raw(ctx, reg, val);
> +		had_write_register_raw(ctx->card_ctx, ctx->pipe, reg, val);
>   }
>   
>   /*
> @@ -1361,6 +1378,9 @@ static void had_process_hot_plug(struct snd_intelhad *intelhaddata)
>   		return;
>   	}
>   
> +	/* Disable Audio */
> +	had_enable_audio(intelhaddata, false);
> +
>   	intelhaddata->connected = true;
>   	dev_dbg(intelhaddata->dev,
>   		"%s @ %d:DEBUG PLUG/UNPLUG : HAD_DRV_CONNECTED\n",
> @@ -1523,26 +1543,31 @@ static const struct snd_kcontrol_new had_controls[] = {
>   static irqreturn_t display_pipe_interrupt_handler(int irq, void *dev_id)
>   {
>   	struct snd_intelhad_card *card_ctx = dev_id;
> -	int port;
> +	u32 audio_stat[3] = {};
> +	int pipe, port;
> +
> +	for_each_pipe(card_ctx, pipe) {
> +		/* use raw register access to ack IRQs even while disconnected */
> +		audio_stat[pipe] = had_read_register_raw(card_ctx, pipe,
> +							 AUD_HDMI_STATUS) &
> +			(HDMI_AUDIO_UNDERRUN | HDMI_AUDIO_BUFFER_DONE);
> +
> +		if (audio_stat[pipe])
> +			had_write_register_raw(card_ctx, pipe,
> +					       AUD_HDMI_STATUS, audio_stat[pipe]);
> +	}
>   
>   	for_each_port(card_ctx, port) {
>   		struct snd_intelhad *ctx = &card_ctx->pcm_ctx[port];
> -		u32 audio_stat;
> +		int pipe = ctx->pipe;
>   
> -		/* use raw register access to ack IRQs even while disconnected */
> -		audio_stat = had_read_register_raw(ctx, AUD_HDMI_STATUS);
> -
> -		if (audio_stat & HDMI_AUDIO_UNDERRUN) {
> -			had_write_register_raw(ctx, AUD_HDMI_STATUS,
> -					       HDMI_AUDIO_UNDERRUN);
> -			had_process_buffer_underrun(ctx);
> -		}
> +		if (pipe < 0)
> +			continue;
>   
> -		if (audio_stat & HDMI_AUDIO_BUFFER_DONE) {
> -			had_write_register_raw(ctx, AUD_HDMI_STATUS,
> -					       HDMI_AUDIO_BUFFER_DONE);
> +		if (audio_stat[pipe] & HDMI_AUDIO_BUFFER_DONE)
>   			had_process_buffer_done(ctx);
> -		}
> +		if (audio_stat[pipe] & HDMI_AUDIO_UNDERRUN)
> +			had_process_buffer_underrun(ctx);
>   	}
>   
>   	return IRQ_HANDLED;
> @@ -1551,16 +1576,12 @@ static irqreturn_t display_pipe_interrupt_handler(int irq, void *dev_id)
>   /*
>    * monitor plug/unplug notification from i915; just kick off the work
>    */
> -static void notify_audio_lpe(struct platform_device *pdev)
> +static void notify_audio_lpe(struct platform_device *pdev, int port)
>   {
>   	struct snd_intelhad_card *card_ctx = platform_get_drvdata(pdev);
> -	int port;
> +	struct snd_intelhad *ctx = &card_ctx->pcm_ctx[port];
>   
> -	for_each_port(card_ctx, port) {
> -		struct snd_intelhad *ctx = &card_ctx->pcm_ctx[port];
> -
> -		schedule_work(&ctx->hdmi_audio_wq);
> -	}
> +	schedule_work(&ctx->hdmi_audio_wq);
>   }
>   
>   /* the work to handle monitor hot plug/unplug */
> @@ -1569,34 +1590,27 @@ static void had_audio_wq(struct work_struct *work)
>   	struct snd_intelhad *ctx =
>   		container_of(work, struct snd_intelhad, hdmi_audio_wq);
>   	struct intel_hdmi_lpe_audio_pdata *pdata = ctx->dev->platform_data;
> -	struct intel_hdmi_lpe_audio_port_pdata *ppdata = &pdata->port;
> +	struct intel_hdmi_lpe_audio_port_pdata *ppdata = &pdata->port[ctx->port];
>   
>   	pm_runtime_get_sync(ctx->dev);
>   	mutex_lock(&ctx->mutex);
>   	if (ppdata->pipe < 0) {
> -		dev_dbg(ctx->dev, "%s: Event: HAD_NOTIFY_HOT_UNPLUG\n",
> -			__func__);
> +		dev_dbg(ctx->dev, "%s: Event: HAD_NOTIFY_HOT_UNPLUG : port = %d\n",
> +			__func__, ctx->port);
> +
>   		memset(ctx->eld, 0, sizeof(ctx->eld)); /* clear the old ELD */
> +
> +		ctx->dp_output = false;
> +		ctx->tmds_clock_speed = 0;
> +		ctx->link_rate = 0;
> +
> +		/* Shut down the stream */
>   		had_process_hot_unplug(ctx);
> +
> +		ctx->pipe = -1;
>   	} else {
>   		dev_dbg(ctx->dev, "%s: HAD_NOTIFY_ELD : port = %d, tmds = %d\n",
> -			__func__, ppdata->port, ppdata->ls_clock);
> -
> -		switch (ppdata->pipe) {
> -		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(ctx->dev, "Invalid pipe %d\n",
> -				ppdata->pipe);
> -			break;
> -		}
> +			__func__, ctx->port, ppdata->ls_clock);
>   
>   		memcpy(ctx->eld, ppdata->eld, sizeof(ctx->eld));
>   
> @@ -1609,11 +1623,18 @@ static void had_audio_wq(struct work_struct *work)
>   			ctx->link_rate = 0;
>   		}
>   
> +		/*
> +		 * Shut down the stream before we change
> +		 * the pipe assignment for this pcm device
> +		 */
>   		had_process_hot_plug(ctx);
>   
> -		/* Process mode change if stream is active */
> +		ctx->pipe = ppdata->pipe;
> +
> +		/* Restart the stream if necessary */
>   		had_process_mode_change(ctx);
>   	}
> +
>   	mutex_unlock(&ctx->mutex);
>   	pm_runtime_mark_last_busy(ctx->dev);
>   	pm_runtime_put_autosuspend(ctx->dev);
> @@ -1794,7 +1815,8 @@ static int hdmi_lpe_audio_probe(struct platform_device *pdev)
>   
>   	init_channel_allocations();
>   
> -	card_ctx->num_ports = 1;
> +	card_ctx->num_pipes = pdata->num_pipes;
> +	card_ctx->num_ports = pdata->num_ports;
>   
>   	for_each_port(card_ctx, port) {
>   		struct snd_intelhad *ctx = &card_ctx->pcm_ctx[port];
> @@ -1802,12 +1824,12 @@ static int hdmi_lpe_audio_probe(struct platform_device *pdev)
>   
>   		ctx->card_ctx = card_ctx;
>   		ctx->dev = card_ctx->dev;
> +		ctx->port = port;
> +		ctx->pipe = -1;
>   
>   		INIT_WORK(&ctx->hdmi_audio_wq, had_audio_wq);
>   
> -		ctx->had_config_offset = AUDIO_HDMI_CONFIG_A;
> -
> -		ret = snd_pcm_new(card, INTEL_HAD, PCM_INDEX, MAX_PB_STREAMS,
> +		ret = snd_pcm_new(card, INTEL_HAD, port, MAX_PB_STREAMS,
>   				  MAX_CAP_STREAMS, &pcm);
>   		if (ret)
>   			goto err;
> diff --git a/sound/x86/intel_hdmi_audio.h b/sound/x86/intel_hdmi_audio.h
> index 2725964ebc46..0d91bb5dbab7 100644
> --- a/sound/x86/intel_hdmi_audio.h
> +++ b/sound/x86/intel_hdmi_audio.h
> @@ -32,7 +32,6 @@
>   
>   #include "intel_hdmi_lpe_audio.h"
>   
> -#define PCM_INDEX		0
>   #define MAX_PB_STREAMS		1
>   #define MAX_CAP_STREAMS		0
>   #define BYTES_PER_WORD		0x4
> @@ -112,6 +111,8 @@ struct snd_intelhad {
>   	struct snd_pcm_chmap *chmap;
>   	int tmds_clock_speed;
>   	int link_rate;
> +	int port; /* fixed */
> +	int pipe; /* can change dynamically */
>   
>   	/* ring buffer (BD) position index */
>   	unsigned int bd_head;
> @@ -123,7 +124,6 @@ struct snd_intelhad {
>   	unsigned int period_bytes;	/* PCM period size in bytes */
>   
>   	/* internal stuff */
> -	unsigned int had_config_offset;
>   	union aud_cfg aud_config;	/* AUD_CONFIG reg value cache */
>   	struct work_struct hdmi_audio_wq;
>   	struct mutex mutex; /* for protecting chmap and eld */
> @@ -138,8 +138,9 @@ struct snd_intelhad_card {
>   	/* internal stuff */
>   	int irq;
>   	void __iomem *mmio_start;
> +	int num_pipes;
>   	int num_ports;
> -	struct snd_intelhad pcm_ctx[3];
> +	struct snd_intelhad pcm_ctx[3]; /* one for each port */
>   };
>   
>   #endif /* _INTEL_HDMI_AUDIO_ */

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

Re: [alsa-devel] [PATCH v2 00/11] drm/i915: LPE audio runtime PM and multipipe (v2)

[Pierre-Louis Bossart]

On 04/28/2017 02:37 PM, Ville Syrjälä wrote:
> On Fri, Apr 28, 2017 at 12:10:31PM -0500, Pierre-Louis Bossart wrote:
>>
>> On 04/28/2017 03:41 AM, Takashi Iwai wrote:
>>> On Thu, 27 Apr 2017 18:02:19 +0200,
>>> ville.syrjala@linux.intel.com wrote:
>>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>>
>>>> Okay, here's the second attempt at getting multiple pipes playing back
>>>> audio on the VLV/CHV HDMI LPE audio device. The main change from v1 is
>>>> that now the PCM devices are associated with ports instead of pipes,
>>>> so the audio from one device always gets output on the same display.
>>>>
>>>> I've also tacked on the alsa-lib conf update. No clue whether it's
>>>> really correct or not (the config language isn't a close friend
>>>> of mine).
>>>>
>>>> BTW I did notice that with LPE audio all the controls say iface=PCM,
>>>> whereas on HDA a bunch of them say iface=MIXER. No idea if that's
>>>> OK or not, just something I spotted when I was comparing the results
>>>> with HDA.
>>> We generally accept both iface types for IEC958 stuff, since
>>> historically many drivers have already mixed them up.  So it's no
>>> problem :)
>>>
>>>
>>>> Entire series available here:
>>>> git://github.com/vsyrjala/linux.git lpe_audio_multipipe_2
>>>>
>>>> Cc: Takashi Iwai <tiwai@suse.de>
>>>> Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
>>> All look good, and feel free to take my reviewed-by tag
>>>     Reviewed-by: Takashi Iwai <tiwai@suse.de>
>>>
>>> As said previously, my only slight concern is the compatibility.
>>> But, in the current situation with PulseAudio, only few people would
>>> use this driver, so it shouldn't be so big impact, I suppose.
>>>
>>> BTW, which port is used in general on BYT/CHT?
>>>
>>> Oh, also, I suppose you want to carry these over i915 tree?
>>> I don't mind either way, I can take them through sound tree if
>>> preferred, too.
>> I see frequent oops on startup with this lpe_audio_multipipe_2 branch
>> with my CHT device not booting or no HDMI audio device created.
>> Not sure if these issues are due to the new patches or to the rest of
>> the drm code?
>>
>> [    5.529023] BUG: unable to handle kernel NULL pointer dereference
>> at           (null)
>> [    5.529143] IP: hdmi_lpe_audio_probe+0x40f/0x650 [snd_hdmi_lpe_audio]
>> [    5.529202] PGD 0
>>
>> [    5.529242] Oops: 0000 [#1] SMP
>> [    5.529274] Modules linked in: snd_soc_sst_atom_hifi2_platform
>> snd_soc_sst_match snd_soc_core snd_compress lpc_ich snd_seq
>> snd_seq_device shpchp snd_hdmi_lpe_audio(+) snd_pcm snd_timer dw_dmac
>> snd soundcore i2c_designware_platform(+) i2c_designware_core
>> spi_pxa2xx_platform acpi_pad mac_hid nfsd auth_rpcgss nfs_acl lockd
>> grace sunrpc ip_tables x_tables hid_generic mmc_block i2c_hid usbhid hid
>> autofs4
>> [    5.529605] CPU: 2 PID: 512 Comm: systemd-udevd Not tainted
>> 4.11.0-rc8-test+ #11
>> [    5.529671] Hardware name: ZOTAC XXXXXX/Cherry Trail FFD, BIOS 5.11
>> 09/28/2016
>> [    5.529736] task: ffff88007485b780 task.stack: ffffc90000bfc000
>> [    5.529793] RIP: 0010:hdmi_lpe_audio_probe+0x40f/0x650
>> [snd_hdmi_lpe_audio]
>> [    5.529855] RSP: 0018:ffffc90000bffaf0 EFLAGS: 00010246
>> [    5.529904] RAX: 0000000000000000 RBX: ffff880079209898 RCX:
>> ffff88007920f078
>> [    5.529967] RDX: 0000000000000014 RSI: ffffc90000bffb28 RDI:
>> 0000000000000002
>> [    5.530031] RBP: ffffc90000bffb70 R08: 0000000000000001 R09:
>> 0000000000000000
>> [    5.530094] R10: ffff88007441bf00 R11: ffffc90000bffb36 R12:
>> ffff88007920ef20
>> [    5.530159] R13: ffff88007920ef48 R14: 0000000000005688 R15:
>> 0000000000000047
>> [    5.530225] FS:  00007f627c988640(0000) GS:ffff88007b300000(0000)
>> knlGS:0000000000000000
>> [    5.530299] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> [    5.530352] CR2: 0000000000000000 CR3: 0000000078cb8000 CR4:
>> 00000000001006e0
>> [    5.530416] Call Trace:
>> [    5.530452]  platform_drv_probe+0x3b/0xa0
>> [    5.530494]  driver_probe_device+0x2bb/0x460
>> [    5.530538]  __driver_attach+0xdf/0xf0
>> [    5.530576]  ? driver_probe_device+0x460/0x460
>> [    5.530620]  bus_for_each_dev+0x60/0xa0
>> [    5.530658]  driver_attach+0x1e/0x20
>> [    5.530693]  bus_add_driver+0x170/0x270
>> [    5.530731]  driver_register+0x60/0xe0
>> [    5.530769]  ? 0xffffffffa01cb000
>> [    5.530803]  __platform_driver_register+0x36/0x40
>> [    5.530851]  hdmi_lpe_audio_driver_init+0x17/0x1000 [snd_hdmi_lpe_audio]
>> [    5.530915]  do_one_initcall+0x43/0x180
>> [    5.530956]  ? __vunmap+0x81/0xd0
>> [    5.530991]  ? kfree+0x14c/0x160
>> [    5.531024]  ? kmem_cache_alloc_trace+0x38/0x150
>> [    5.531070]  do_init_module+0x5f/0x1f8
>> [    5.531108]  load_module+0x271e/0x2bd0
>> [    5.531147]  ? kernel_read_file+0x1a3/0x1c0
>> [    5.531188]  SYSC_finit_module+0xbc/0xf0
>> [    5.531226]  ? SYSC_finit_module+0xbc/0xf0
>> [    5.531267]  SyS_finit_module+0xe/0x10
>> [    5.531305]  do_syscall_64+0x6e/0x180
>> [    5.531345]  entry_SYSCALL64_slow_path+0x25/0x25
>> [    5.531389] RIP: 0033:0x7f627b5fbbf9
>> [    5.531424] RSP: 002b:00007ffe053eee68 EFLAGS: 00000246 ORIG_RAX:
>> 0000000000000139
>> [    5.531493] RAX: ffffffffffffffda RBX: 000055d6c745b690 RCX:
>> 00007f627b5fbbf9
>> [    5.531558] RDX: 0000000000000000 RSI: 00007f627c134995 RDI:
>> 0000000000000007
>> [    5.531622] RBP: 00007f627c134995 R08: 0000000000000000 R09:
>> 00007ffe053eef80
>> [    5.531687] R10: 0000000000000007 R11: 0000000000000246 R12:
>> 0000000000000000
>> [    5.531751] R13: 000055d6c7459ae0 R14: 0000000000020000 R15:
>> 000055d6c745b690
>> [    5.531816] Code: 48 8b 45 b0 8b 48 18 e8 e0 cb 22 e1 49 8b 44 24 28
>> 4a 8d 8c 33 58 01 00 00 48 8d 75 b8 45 31 c9 41 b8 01 00 00 00 ba 14 00
>> 00 00 <48> 8b 38 e8 a9 2a ff ff 85 c0 0f 88 09 01 00 00 49 8b 84 24 58
> Disassembling that I get:
>
>    4005a0:       48 8b 45 b0             mov    -0x50(%rbp),%rax
>    4005a4:       8b 48 18                mov    0x18(%rax),%ecx
>    4005a7:       e8 e0 cb 22 e1          callq  ffffffffe162d18c <_end+0xffffffffe102c14c>
>    4005ac:       49 8b 44 24 28          mov    0x28(%r12),%rax
>    4005b1:       4a 8d 8c 33 58 01 00    lea    0x158(%rbx,%r14,1),%rcx
>    4005b8:       00
>    4005b9:       48 8d 75 b8             lea    -0x48(%rbp),%rsi
>    4005bd:       45 31 c9                xor    %r9d,%r9d
>    4005c0:       41 b8 01 00 00 00       mov    $0x1,%r8d
>    4005c6:       ba 14 00 00 00          mov    $0x14,%edx
>    4005cb:       48 8b 38                mov    (%rax),%rdi
>    4005ce:       e8 a9 2a ff ff          callq  3f307c <_init-0xd32c>
>    4005d3:       85 c0                   test   %eax,%eax
>    4005d5:       0f 88 09 01 00 00       js     4006e4 <__GNU_EH_FRAME_HDR+0x100>
>    4005db:       49                      rex.WB
>    4005dc:       8b                      .byte 0x8b
>    4005dd:       84 24 58                test   %ah,(%rax,%rbx,2)
>
> Comparing that with the disassembly from my build, that
> first call looks like the snprintf() and the second one would
> then be snd_jack_new(). So it seems to blow in the
> ctx->card_ctx->card dereference. But I don't really see how
> a NULL pointer could sneak in there.

There was an standard PORT_B off-by-one error which resulted in memory 
corruption. the patch below seems to fix the problem, I can get the same 
functionality as before with concurrent playback on HDMI and DP, and the 
jack values are correctly set.

diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c 
b/drivers/gpu/drm/i915/intel_lpe_audio.c
index fa728ed..3bf6528 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_notify(struct drm_i915_private 
*dev_priv,
                 return;

         pdata = dev_get_platdata(&dev_priv->lpe_audio.platdev->dev);
-       ppdata = &pdata->port[port];
+       ppdata = &pdata->port[port - PORT_B];

         spin_lock_irqsave(&pdata->lpe_audio_slock, irqflags);

@@ -359,7 +359,7 @@ void intel_lpe_audio_notify(struct drm_i915_private 
*dev_priv,
         }

         if (pdata->notify_audio_lpe)
- pdata->notify_audio_lpe(dev_priv->lpe_audio.platdev, port);
+ pdata->notify_audio_lpe(dev_priv->lpe_audio.platdev, port - PORT_B);

         spin_unlock_irqrestore(&pdata->lpe_audio_slock, irqflags);
  }
diff --git a/sound/x86/intel_hdmi_audio.c b/sound/x86/intel_hdmi_audio.c
index 909391d..0153439 100644
--- a/sound/x86/intel_hdmi_audio.c
+++ b/sound/x86/intel_hdmi_audio.c
@@ -1579,7 +1579,7 @@ static irqreturn_t 
display_pipe_interrupt_handler(int irq, void *dev_id)
  static void notify_audio_lpe(struct platform_device *pdev, int port)
  {
         struct snd_intelhad_card *card_ctx = platform_get_drvdata(pdev);
-       struct snd_intelhad *ctx = &card_ctx->pcm_ctx[port];
+       struct snd_intelhad *ctx = &card_ctx->pcm_ctx[port]; /* warning: 
offset with -PORT_B */

         schedule_work(&ctx->hdmi_audio_wq);
  }



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

Re: [alsa-devel] [PATCH v2 00/11] drm/i915: LPE audio runtime PM and multipipe (v2)

[Ville Syrjälä]

On Mon, May 01, 2017 at 08:29:10PM -0500, Pierre-Louis Bossart wrote:
> 
> 
> On 04/28/2017 02:37 PM, Ville Syrjälä wrote:
> > On Fri, Apr 28, 2017 at 12:10:31PM -0500, Pierre-Louis Bossart wrote:
> >>
> >> On 04/28/2017 03:41 AM, Takashi Iwai wrote:
> >>> On Thu, 27 Apr 2017 18:02:19 +0200,
> >>> ville.syrjala@linux.intel.com wrote:
> >>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>>>
> >>>> Okay, here's the second attempt at getting multiple pipes playing back
> >>>> audio on the VLV/CHV HDMI LPE audio device. The main change from v1 is
> >>>> that now the PCM devices are associated with ports instead of pipes,
> >>>> so the audio from one device always gets output on the same display.
> >>>>
> >>>> I've also tacked on the alsa-lib conf update. No clue whether it's
> >>>> really correct or not (the config language isn't a close friend
> >>>> of mine).
> >>>>
> >>>> BTW I did notice that with LPE audio all the controls say iface=PCM,
> >>>> whereas on HDA a bunch of them say iface=MIXER. No idea if that's
> >>>> OK or not, just something I spotted when I was comparing the results
> >>>> with HDA.
> >>> We generally accept both iface types for IEC958 stuff, since
> >>> historically many drivers have already mixed them up.  So it's no
> >>> problem :)
> >>>
> >>>
> >>>> Entire series available here:
> >>>> git://github.com/vsyrjala/linux.git lpe_audio_multipipe_2
> >>>>
> >>>> Cc: Takashi Iwai <tiwai@suse.de>
> >>>> Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> >>> All look good, and feel free to take my reviewed-by tag
> >>>     Reviewed-by: Takashi Iwai <tiwai@suse.de>
> >>>
> >>> As said previously, my only slight concern is the compatibility.
> >>> But, in the current situation with PulseAudio, only few people would
> >>> use this driver, so it shouldn't be so big impact, I suppose.
> >>>
> >>> BTW, which port is used in general on BYT/CHT?
> >>>
> >>> Oh, also, I suppose you want to carry these over i915 tree?
> >>> I don't mind either way, I can take them through sound tree if
> >>> preferred, too.
> >> I see frequent oops on startup with this lpe_audio_multipipe_2 branch
> >> with my CHT device not booting or no HDMI audio device created.
> >> Not sure if these issues are due to the new patches or to the rest of
> >> the drm code?
> >>
> >> [    5.529023] BUG: unable to handle kernel NULL pointer dereference
> >> at           (null)
> >> [    5.529143] IP: hdmi_lpe_audio_probe+0x40f/0x650 [snd_hdmi_lpe_audio]
> >> [    5.529202] PGD 0
> >>
> >> [    5.529242] Oops: 0000 [#1] SMP
> >> [    5.529274] Modules linked in: snd_soc_sst_atom_hifi2_platform
> >> snd_soc_sst_match snd_soc_core snd_compress lpc_ich snd_seq
> >> snd_seq_device shpchp snd_hdmi_lpe_audio(+) snd_pcm snd_timer dw_dmac
> >> snd soundcore i2c_designware_platform(+) i2c_designware_core
> >> spi_pxa2xx_platform acpi_pad mac_hid nfsd auth_rpcgss nfs_acl lockd
> >> grace sunrpc ip_tables x_tables hid_generic mmc_block i2c_hid usbhid hid
> >> autofs4
> >> [    5.529605] CPU: 2 PID: 512 Comm: systemd-udevd Not tainted
> >> 4.11.0-rc8-test+ #11
> >> [    5.529671] Hardware name: ZOTAC XXXXXX/Cherry Trail FFD, BIOS 5.11
> >> 09/28/2016
> >> [    5.529736] task: ffff88007485b780 task.stack: ffffc90000bfc000
> >> [    5.529793] RIP: 0010:hdmi_lpe_audio_probe+0x40f/0x650
> >> [snd_hdmi_lpe_audio]
> >> [    5.529855] RSP: 0018:ffffc90000bffaf0 EFLAGS: 00010246
> >> [    5.529904] RAX: 0000000000000000 RBX: ffff880079209898 RCX:
> >> ffff88007920f078
> >> [    5.529967] RDX: 0000000000000014 RSI: ffffc90000bffb28 RDI:
> >> 0000000000000002
> >> [    5.530031] RBP: ffffc90000bffb70 R08: 0000000000000001 R09:
> >> 0000000000000000
> >> [    5.530094] R10: ffff88007441bf00 R11: ffffc90000bffb36 R12:
> >> ffff88007920ef20
> >> [    5.530159] R13: ffff88007920ef48 R14: 0000000000005688 R15:
> >> 0000000000000047
> >> [    5.530225] FS:  00007f627c988640(0000) GS:ffff88007b300000(0000)
> >> knlGS:0000000000000000
> >> [    5.530299] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> >> [    5.530352] CR2: 0000000000000000 CR3: 0000000078cb8000 CR4:
> >> 00000000001006e0
> >> [    5.530416] Call Trace:
> >> [    5.530452]  platform_drv_probe+0x3b/0xa0
> >> [    5.530494]  driver_probe_device+0x2bb/0x460
> >> [    5.530538]  __driver_attach+0xdf/0xf0
> >> [    5.530576]  ? driver_probe_device+0x460/0x460
> >> [    5.530620]  bus_for_each_dev+0x60/0xa0
> >> [    5.530658]  driver_attach+0x1e/0x20
> >> [    5.530693]  bus_add_driver+0x170/0x270
> >> [    5.530731]  driver_register+0x60/0xe0
> >> [    5.530769]  ? 0xffffffffa01cb000
> >> [    5.530803]  __platform_driver_register+0x36/0x40
> >> [    5.530851]  hdmi_lpe_audio_driver_init+0x17/0x1000 [snd_hdmi_lpe_audio]
> >> [    5.530915]  do_one_initcall+0x43/0x180
> >> [    5.530956]  ? __vunmap+0x81/0xd0
> >> [    5.530991]  ? kfree+0x14c/0x160
> >> [    5.531024]  ? kmem_cache_alloc_trace+0x38/0x150
> >> [    5.531070]  do_init_module+0x5f/0x1f8
> >> [    5.531108]  load_module+0x271e/0x2bd0
> >> [    5.531147]  ? kernel_read_file+0x1a3/0x1c0
> >> [    5.531188]  SYSC_finit_module+0xbc/0xf0
> >> [    5.531226]  ? SYSC_finit_module+0xbc/0xf0
> >> [    5.531267]  SyS_finit_module+0xe/0x10
> >> [    5.531305]  do_syscall_64+0x6e/0x180
> >> [    5.531345]  entry_SYSCALL64_slow_path+0x25/0x25
> >> [    5.531389] RIP: 0033:0x7f627b5fbbf9
> >> [    5.531424] RSP: 002b:00007ffe053eee68 EFLAGS: 00000246 ORIG_RAX:
> >> 0000000000000139
> >> [    5.531493] RAX: ffffffffffffffda RBX: 000055d6c745b690 RCX:
> >> 00007f627b5fbbf9
> >> [    5.531558] RDX: 0000000000000000 RSI: 00007f627c134995 RDI:
> >> 0000000000000007
> >> [    5.531622] RBP: 00007f627c134995 R08: 0000000000000000 R09:
> >> 00007ffe053eef80
> >> [    5.531687] R10: 0000000000000007 R11: 0000000000000246 R12:
> >> 0000000000000000
> >> [    5.531751] R13: 000055d6c7459ae0 R14: 0000000000020000 R15:
> >> 000055d6c745b690
> >> [    5.531816] Code: 48 8b 45 b0 8b 48 18 e8 e0 cb 22 e1 49 8b 44 24 28
> >> 4a 8d 8c 33 58 01 00 00 48 8d 75 b8 45 31 c9 41 b8 01 00 00 00 ba 14 00
> >> 00 00 <48> 8b 38 e8 a9 2a ff ff 85 c0 0f 88 09 01 00 00 49 8b 84 24 58
> > Disassembling that I get:
> >
> >    4005a0:       48 8b 45 b0             mov    -0x50(%rbp),%rax
> >    4005a4:       8b 48 18                mov    0x18(%rax),%ecx
> >    4005a7:       e8 e0 cb 22 e1          callq  ffffffffe162d18c <_end+0xffffffffe102c14c>
> >    4005ac:       49 8b 44 24 28          mov    0x28(%r12),%rax
> >    4005b1:       4a 8d 8c 33 58 01 00    lea    0x158(%rbx,%r14,1),%rcx
> >    4005b8:       00
> >    4005b9:       48 8d 75 b8             lea    -0x48(%rbp),%rsi
> >    4005bd:       45 31 c9                xor    %r9d,%r9d
> >    4005c0:       41 b8 01 00 00 00       mov    $0x1,%r8d
> >    4005c6:       ba 14 00 00 00          mov    $0x14,%edx
> >    4005cb:       48 8b 38                mov    (%rax),%rdi
> >    4005ce:       e8 a9 2a ff ff          callq  3f307c <_init-0xd32c>
> >    4005d3:       85 c0                   test   %eax,%eax
> >    4005d5:       0f 88 09 01 00 00       js     4006e4 <__GNU_EH_FRAME_HDR+0x100>
> >    4005db:       49                      rex.WB
> >    4005dc:       8b                      .byte 0x8b
> >    4005dd:       84 24 58                test   %ah,(%rax,%rbx,2)
> >
> > Comparing that with the disassembly from my build, that
> > first call looks like the snprintf() and the second one would
> > then be snd_jack_new(). So it seems to blow in the
> > ctx->card_ctx->card dereference. But I don't really see how
> > a NULL pointer could sneak in there.
> 
> There was an standard PORT_B off-by-one error which resulted in memory 
> corruption.

Doh.

> the patch below seems to fix the problem, I can get the same 
> functionality as before with concurrent playback on HDMI and DP, and the 
> jack values are correctly set.

Thanks. I do wonder a bit which side should apply this offset.
But I guess it doesn't really matter in the end as long as both
sides know what the deal is. So unless there are good reasons
for anything else I'll just squash this into my patch(es).

> 
> diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c 
> b/drivers/gpu/drm/i915/intel_lpe_audio.c
> index fa728ed..3bf6528 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_notify(struct drm_i915_private 
> *dev_priv,
>                  return;
> 
>          pdata = dev_get_platdata(&dev_priv->lpe_audio.platdev->dev);
> -       ppdata = &pdata->port[port];
> +       ppdata = &pdata->port[port - PORT_B];
> 
>          spin_lock_irqsave(&pdata->lpe_audio_slock, irqflags);
> 
> @@ -359,7 +359,7 @@ void intel_lpe_audio_notify(struct drm_i915_private 
> *dev_priv,
>          }
> 
>          if (pdata->notify_audio_lpe)
> - pdata->notify_audio_lpe(dev_priv->lpe_audio.platdev, port);
> + pdata->notify_audio_lpe(dev_priv->lpe_audio.platdev, port - PORT_B);
> 
>          spin_unlock_irqrestore(&pdata->lpe_audio_slock, irqflags);
>   }
> diff --git a/sound/x86/intel_hdmi_audio.c b/sound/x86/intel_hdmi_audio.c
> index 909391d..0153439 100644
> --- a/sound/x86/intel_hdmi_audio.c
> +++ b/sound/x86/intel_hdmi_audio.c
> @@ -1579,7 +1579,7 @@ static irqreturn_t 
> display_pipe_interrupt_handler(int irq, void *dev_id)
>   static void notify_audio_lpe(struct platform_device *pdev, int port)
>   {
>          struct snd_intelhad_card *card_ctx = platform_get_drvdata(pdev);
> -       struct snd_intelhad *ctx = &card_ctx->pcm_ctx[port];
> +       struct snd_intelhad *ctx = &card_ctx->pcm_ctx[port]; /* warning: 
> offset with -PORT_B */
> 
>          schedule_work(&ctx->hdmi_audio_wq);
>   }
> 
> 

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

Re: [alsa-devel] [PATCH v2 00/11] drm/i915: LPE audio runtime PM and multipipe (v2)

[Pierre-Louis Bossart]

On 5/2/17 1:27 PM, Ville Syrjälä wrote:
> On Mon, May 01, 2017 at 08:29:10PM -0500, Pierre-Louis Bossart wrote:
>>
>>
>> On 04/28/2017 02:37 PM, Ville Syrjälä wrote:
>>> On Fri, Apr 28, 2017 at 12:10:31PM -0500, Pierre-Louis Bossart wrote:
>>>>
>>>> On 04/28/2017 03:41 AM, Takashi Iwai wrote:
>>>>> On Thu, 27 Apr 2017 18:02:19 +0200,
>>>>> ville.syrjala@linux.intel.com wrote:
>>>>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>>>>
>>>>>> Okay, here's the second attempt at getting multiple pipes playing back
>>>>>> audio on the VLV/CHV HDMI LPE audio device. The main change from v1 is
>>>>>> that now the PCM devices are associated with ports instead of pipes,
>>>>>> so the audio from one device always gets output on the same display.
>>>>>>
>>>>>> I've also tacked on the alsa-lib conf update. No clue whether it's
>>>>>> really correct or not (the config language isn't a close friend
>>>>>> of mine).
>>>>>>
>>>>>> BTW I did notice that with LPE audio all the controls say iface=PCM,
>>>>>> whereas on HDA a bunch of them say iface=MIXER. No idea if that's
>>>>>> OK or not, just something I spotted when I was comparing the results
>>>>>> with HDA.
>>>>> We generally accept both iface types for IEC958 stuff, since
>>>>> historically many drivers have already mixed them up.  So it's no
>>>>> problem :)
>>>>>
>>>>>
>>>>>> Entire series available here:
>>>>>> git://github.com/vsyrjala/linux.git lpe_audio_multipipe_2
>>>>>>
>>>>>> Cc: Takashi Iwai <tiwai@suse.de>
>>>>>> Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
>>>>> All look good, and feel free to take my reviewed-by tag
>>>>>     Reviewed-by: Takashi Iwai <tiwai@suse.de>
>>>>>
>>>>> As said previously, my only slight concern is the compatibility.
>>>>> But, in the current situation with PulseAudio, only few people would
>>>>> use this driver, so it shouldn't be so big impact, I suppose.
>>>>>
>>>>> BTW, which port is used in general on BYT/CHT?
>>>>>
>>>>> Oh, also, I suppose you want to carry these over i915 tree?
>>>>> I don't mind either way, I can take them through sound tree if
>>>>> preferred, too.
>>>> I see frequent oops on startup with this lpe_audio_multipipe_2 branch
>>>> with my CHT device not booting or no HDMI audio device created.
>>>> Not sure if these issues are due to the new patches or to the rest of
>>>> the drm code?
>>>>
>>>> [    5.529023] BUG: unable to handle kernel NULL pointer dereference
>>>> at           (null)
>>>> [    5.529143] IP: hdmi_lpe_audio_probe+0x40f/0x650 [snd_hdmi_lpe_audio]
>>>> [    5.529202] PGD 0
>>>>
>>>> [    5.529242] Oops: 0000 [#1] SMP
>>>> [    5.529274] Modules linked in: snd_soc_sst_atom_hifi2_platform
>>>> snd_soc_sst_match snd_soc_core snd_compress lpc_ich snd_seq
>>>> snd_seq_device shpchp snd_hdmi_lpe_audio(+) snd_pcm snd_timer dw_dmac
>>>> snd soundcore i2c_designware_platform(+) i2c_designware_core
>>>> spi_pxa2xx_platform acpi_pad mac_hid nfsd auth_rpcgss nfs_acl lockd
>>>> grace sunrpc ip_tables x_tables hid_generic mmc_block i2c_hid usbhid hid
>>>> autofs4
>>>> [    5.529605] CPU: 2 PID: 512 Comm: systemd-udevd Not tainted
>>>> 4.11.0-rc8-test+ #11
>>>> [    5.529671] Hardware name: ZOTAC XXXXXX/Cherry Trail FFD, BIOS 5.11
>>>> 09/28/2016
>>>> [    5.529736] task: ffff88007485b780 task.stack: ffffc90000bfc000
>>>> [    5.529793] RIP: 0010:hdmi_lpe_audio_probe+0x40f/0x650
>>>> [snd_hdmi_lpe_audio]
>>>> [    5.529855] RSP: 0018:ffffc90000bffaf0 EFLAGS: 00010246
>>>> [    5.529904] RAX: 0000000000000000 RBX: ffff880079209898 RCX:
>>>> ffff88007920f078
>>>> [    5.529967] RDX: 0000000000000014 RSI: ffffc90000bffb28 RDI:
>>>> 0000000000000002
>>>> [    5.530031] RBP: ffffc90000bffb70 R08: 0000000000000001 R09:
>>>> 0000000000000000
>>>> [    5.530094] R10: ffff88007441bf00 R11: ffffc90000bffb36 R12:
>>>> ffff88007920ef20
>>>> [    5.530159] R13: ffff88007920ef48 R14: 0000000000005688 R15:
>>>> 0000000000000047
>>>> [    5.530225] FS:  00007f627c988640(0000) GS:ffff88007b300000(0000)
>>>> knlGS:0000000000000000
>>>> [    5.530299] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>>>> [    5.530352] CR2: 0000000000000000 CR3: 0000000078cb8000 CR4:
>>>> 00000000001006e0
>>>> [    5.530416] Call Trace:
>>>> [    5.530452]  platform_drv_probe+0x3b/0xa0
>>>> [    5.530494]  driver_probe_device+0x2bb/0x460
>>>> [    5.530538]  __driver_attach+0xdf/0xf0
>>>> [    5.530576]  ? driver_probe_device+0x460/0x460
>>>> [    5.530620]  bus_for_each_dev+0x60/0xa0
>>>> [    5.530658]  driver_attach+0x1e/0x20
>>>> [    5.530693]  bus_add_driver+0x170/0x270
>>>> [    5.530731]  driver_register+0x60/0xe0
>>>> [    5.530769]  ? 0xffffffffa01cb000
>>>> [    5.530803]  __platform_driver_register+0x36/0x40
>>>> [    5.530851]  hdmi_lpe_audio_driver_init+0x17/0x1000 [snd_hdmi_lpe_audio]
>>>> [    5.530915]  do_one_initcall+0x43/0x180
>>>> [    5.530956]  ? __vunmap+0x81/0xd0
>>>> [    5.530991]  ? kfree+0x14c/0x160
>>>> [    5.531024]  ? kmem_cache_alloc_trace+0x38/0x150
>>>> [    5.531070]  do_init_module+0x5f/0x1f8
>>>> [    5.531108]  load_module+0x271e/0x2bd0
>>>> [    5.531147]  ? kernel_read_file+0x1a3/0x1c0
>>>> [    5.531188]  SYSC_finit_module+0xbc/0xf0
>>>> [    5.531226]  ? SYSC_finit_module+0xbc/0xf0
>>>> [    5.531267]  SyS_finit_module+0xe/0x10
>>>> [    5.531305]  do_syscall_64+0x6e/0x180
>>>> [    5.531345]  entry_SYSCALL64_slow_path+0x25/0x25
>>>> [    5.531389] RIP: 0033:0x7f627b5fbbf9
>>>> [    5.531424] RSP: 002b:00007ffe053eee68 EFLAGS: 00000246 ORIG_RAX:
>>>> 0000000000000139
>>>> [    5.531493] RAX: ffffffffffffffda RBX: 000055d6c745b690 RCX:
>>>> 00007f627b5fbbf9
>>>> [    5.531558] RDX: 0000000000000000 RSI: 00007f627c134995 RDI:
>>>> 0000000000000007
>>>> [    5.531622] RBP: 00007f627c134995 R08: 0000000000000000 R09:
>>>> 00007ffe053eef80
>>>> [    5.531687] R10: 0000000000000007 R11: 0000000000000246 R12:
>>>> 0000000000000000
>>>> [    5.531751] R13: 000055d6c7459ae0 R14: 0000000000020000 R15:
>>>> 000055d6c745b690
>>>> [    5.531816] Code: 48 8b 45 b0 8b 48 18 e8 e0 cb 22 e1 49 8b 44 24 28
>>>> 4a 8d 8c 33 58 01 00 00 48 8d 75 b8 45 31 c9 41 b8 01 00 00 00 ba 14 00
>>>> 00 00 <48> 8b 38 e8 a9 2a ff ff 85 c0 0f 88 09 01 00 00 49 8b 84 24 58
>>> Disassembling that I get:
>>>
>>>    4005a0:       48 8b 45 b0             mov    -0x50(%rbp),%rax
>>>    4005a4:       8b 48 18                mov    0x18(%rax),%ecx
>>>    4005a7:       e8 e0 cb 22 e1          callq  ffffffffe162d18c <_end+0xffffffffe102c14c>
>>>    4005ac:       49 8b 44 24 28          mov    0x28(%r12),%rax
>>>    4005b1:       4a 8d 8c 33 58 01 00    lea    0x158(%rbx,%r14,1),%rcx
>>>    4005b8:       00
>>>    4005b9:       48 8d 75 b8             lea    -0x48(%rbp),%rsi
>>>    4005bd:       45 31 c9                xor    %r9d,%r9d
>>>    4005c0:       41 b8 01 00 00 00       mov    $0x1,%r8d
>>>    4005c6:       ba 14 00 00 00          mov    $0x14,%edx
>>>    4005cb:       48 8b 38                mov    (%rax),%rdi
>>>    4005ce:       e8 a9 2a ff ff          callq  3f307c <_init-0xd32c>
>>>    4005d3:       85 c0                   test   %eax,%eax
>>>    4005d5:       0f 88 09 01 00 00       js     4006e4 <__GNU_EH_FRAME_HDR+0x100>
>>>    4005db:       49                      rex.WB
>>>    4005dc:       8b                      .byte 0x8b
>>>    4005dd:       84 24 58                test   %ah,(%rax,%rbx,2)
>>>
>>> Comparing that with the disassembly from my build, that
>>> first call looks like the snprintf() and the second one would
>>> then be snd_jack_new(). So it seems to blow in the
>>> ctx->card_ctx->card dereference. But I don't really see how
>>> a NULL pointer could sneak in there.
>>
>> There was an standard PORT_B off-by-one error which resulted in memory
>> corruption.
>
> Doh.
>
>> the patch below seems to fix the problem, I can get the same
>> functionality as before with concurrent playback on HDMI and DP, and the
>> jack values are correctly set.
>
> Thanks. I do wonder a bit which side should apply this offset.
> But I guess it doesn't really matter in the end as long as both
> sides know what the deal is. So unless there are good reasons
> for anything else I'll just squash this into my patch(es).

Fine with me.
One open I still have is that it's not self-explanatory for the user to 
figure out which output the devices correspond to, e.g. on my CHT device 
HDMI is device 2 and DP is device 0.
I may be mistaken but in the previous pipe-based solution, audio 
playback would progress at a normal rate even if nothing was connected. 
With the port-based, it looks like playback doesn't progress if there is 
nothing connected - speaker-test is stuck. Not sure if the open or 
hw_params should fail in that case? One could argue that the user should 
know better and check the jack status, but in reality I don't think any 
userspace layer will ever do this.

>
>>
>> diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c
>> b/drivers/gpu/drm/i915/intel_lpe_audio.c
>> index fa728ed..3bf6528 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_notify(struct drm_i915_private
>> *dev_priv,
>>                  return;
>>
>>          pdata = dev_get_platdata(&dev_priv->lpe_audio.platdev->dev);
>> -       ppdata = &pdata->port[port];
>> +       ppdata = &pdata->port[port - PORT_B];
>>
>>          spin_lock_irqsave(&pdata->lpe_audio_slock, irqflags);
>>
>> @@ -359,7 +359,7 @@ void intel_lpe_audio_notify(struct drm_i915_private
>> *dev_priv,
>>          }
>>
>>          if (pdata->notify_audio_lpe)
>> - pdata->notify_audio_lpe(dev_priv->lpe_audio.platdev, port);
>> + pdata->notify_audio_lpe(dev_priv->lpe_audio.platdev, port - PORT_B);
>>
>>          spin_unlock_irqrestore(&pdata->lpe_audio_slock, irqflags);
>>   }
>> diff --git a/sound/x86/intel_hdmi_audio.c b/sound/x86/intel_hdmi_audio.c
>> index 909391d..0153439 100644
>> --- a/sound/x86/intel_hdmi_audio.c
>> +++ b/sound/x86/intel_hdmi_audio.c
>> @@ -1579,7 +1579,7 @@ static irqreturn_t
>> display_pipe_interrupt_handler(int irq, void *dev_id)
>>   static void notify_audio_lpe(struct platform_device *pdev, int port)
>>   {
>>          struct snd_intelhad_card *card_ctx = platform_get_drvdata(pdev);
>> -       struct snd_intelhad *ctx = &card_ctx->pcm_ctx[port];
>> +       struct snd_intelhad *ctx = &card_ctx->pcm_ctx[port]; /* warning:
>> offset with -PORT_B */
>>
>>          schedule_work(&ctx->hdmi_audio_wq);
>>   }
>>
>>
>

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

Re: [alsa-devel] [PATCH v2 00/11] drm/i915: LPE audio runtime PM and multipipe (v2)

[Takashi Iwai]

On Tue, 02 May 2017 22:15:20 +0200,
Pierre-Louis Bossart wrote:
> 
> On 5/2/17 1:27 PM, Ville Syrjälä wrote:
> > On Mon, May 01, 2017 at 08:29:10PM -0500, Pierre-Louis Bossart wrote:
> >>
> >>
> >> On 04/28/2017 02:37 PM, Ville Syrjälä wrote:
> >>> On Fri, Apr 28, 2017 at 12:10:31PM -0500, Pierre-Louis Bossart wrote:
> >>>>
> >>>> On 04/28/2017 03:41 AM, Takashi Iwai wrote:
> >>>>> On Thu, 27 Apr 2017 18:02:19 +0200,
> >>>>> ville.syrjala@linux.intel.com wrote:
> >>>>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>>>>>
> >>>>>> Okay, here's the second attempt at getting multiple pipes playing back
> >>>>>> audio on the VLV/CHV HDMI LPE audio device. The main change from v1 is
> >>>>>> that now the PCM devices are associated with ports instead of pipes,
> >>>>>> so the audio from one device always gets output on the same display.
> >>>>>>
> >>>>>> I've also tacked on the alsa-lib conf update. No clue whether it's
> >>>>>> really correct or not (the config language isn't a close friend
> >>>>>> of mine).
> >>>>>>
> >>>>>> BTW I did notice that with LPE audio all the controls say iface=PCM,
> >>>>>> whereas on HDA a bunch of them say iface=MIXER. No idea if that's
> >>>>>> OK or not, just something I spotted when I was comparing the results
> >>>>>> with HDA.
> >>>>> We generally accept both iface types for IEC958 stuff, since
> >>>>> historically many drivers have already mixed them up.  So it's no
> >>>>> problem :)
> >>>>>
> >>>>>
> >>>>>> Entire series available here:
> >>>>>> git://github.com/vsyrjala/linux.git lpe_audio_multipipe_2
> >>>>>>
> >>>>>> Cc: Takashi Iwai <tiwai@suse.de>
> >>>>>> Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> >>>>> All look good, and feel free to take my reviewed-by tag
> >>>>>     Reviewed-by: Takashi Iwai <tiwai@suse.de>
> >>>>>
> >>>>> As said previously, my only slight concern is the compatibility.
> >>>>> But, in the current situation with PulseAudio, only few people would
> >>>>> use this driver, so it shouldn't be so big impact, I suppose.
> >>>>>
> >>>>> BTW, which port is used in general on BYT/CHT?
> >>>>>
> >>>>> Oh, also, I suppose you want to carry these over i915 tree?
> >>>>> I don't mind either way, I can take them through sound tree if
> >>>>> preferred, too.
> >>>> I see frequent oops on startup with this lpe_audio_multipipe_2 branch
> >>>> with my CHT device not booting or no HDMI audio device created.
> >>>> Not sure if these issues are due to the new patches or to the rest of
> >>>> the drm code?
> >>>>
> >>>> [    5.529023] BUG: unable to handle kernel NULL pointer dereference
> >>>> at           (null)
> >>>> [    5.529143] IP: hdmi_lpe_audio_probe+0x40f/0x650 [snd_hdmi_lpe_audio]
> >>>> [    5.529202] PGD 0
> >>>>
> >>>> [    5.529242] Oops: 0000 [#1] SMP
> >>>> [    5.529274] Modules linked in: snd_soc_sst_atom_hifi2_platform
> >>>> snd_soc_sst_match snd_soc_core snd_compress lpc_ich snd_seq
> >>>> snd_seq_device shpchp snd_hdmi_lpe_audio(+) snd_pcm snd_timer dw_dmac
> >>>> snd soundcore i2c_designware_platform(+) i2c_designware_core
> >>>> spi_pxa2xx_platform acpi_pad mac_hid nfsd auth_rpcgss nfs_acl lockd
> >>>> grace sunrpc ip_tables x_tables hid_generic mmc_block i2c_hid usbhid hid
> >>>> autofs4
> >>>> [    5.529605] CPU: 2 PID: 512 Comm: systemd-udevd Not tainted
> >>>> 4.11.0-rc8-test+ #11
> >>>> [    5.529671] Hardware name: ZOTAC XXXXXX/Cherry Trail FFD, BIOS 5.11
> >>>> 09/28/2016
> >>>> [    5.529736] task: ffff88007485b780 task.stack: ffffc90000bfc000
> >>>> [    5.529793] RIP: 0010:hdmi_lpe_audio_probe+0x40f/0x650
> >>>> [snd_hdmi_lpe_audio]
> >>>> [    5.529855] RSP: 0018:ffffc90000bffaf0 EFLAGS: 00010246
> >>>> [    5.529904] RAX: 0000000000000000 RBX: ffff880079209898 RCX:
> >>>> ffff88007920f078
> >>>> [    5.529967] RDX: 0000000000000014 RSI: ffffc90000bffb28 RDI:
> >>>> 0000000000000002
> >>>> [    5.530031] RBP: ffffc90000bffb70 R08: 0000000000000001 R09:
> >>>> 0000000000000000
> >>>> [    5.530094] R10: ffff88007441bf00 R11: ffffc90000bffb36 R12:
> >>>> ffff88007920ef20
> >>>> [    5.530159] R13: ffff88007920ef48 R14: 0000000000005688 R15:
> >>>> 0000000000000047
> >>>> [    5.530225] FS:  00007f627c988640(0000) GS:ffff88007b300000(0000)
> >>>> knlGS:0000000000000000
> >>>> [    5.530299] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> >>>> [    5.530352] CR2: 0000000000000000 CR3: 0000000078cb8000 CR4:
> >>>> 00000000001006e0
> >>>> [    5.530416] Call Trace:
> >>>> [    5.530452]  platform_drv_probe+0x3b/0xa0
> >>>> [    5.530494]  driver_probe_device+0x2bb/0x460
> >>>> [    5.530538]  __driver_attach+0xdf/0xf0
> >>>> [    5.530576]  ? driver_probe_device+0x460/0x460
> >>>> [    5.530620]  bus_for_each_dev+0x60/0xa0
> >>>> [    5.530658]  driver_attach+0x1e/0x20
> >>>> [    5.530693]  bus_add_driver+0x170/0x270
> >>>> [    5.530731]  driver_register+0x60/0xe0
> >>>> [    5.530769]  ? 0xffffffffa01cb000
> >>>> [    5.530803]  __platform_driver_register+0x36/0x40
> >>>> [    5.530851]  hdmi_lpe_audio_driver_init+0x17/0x1000 [snd_hdmi_lpe_audio]
> >>>> [    5.530915]  do_one_initcall+0x43/0x180
> >>>> [    5.530956]  ? __vunmap+0x81/0xd0
> >>>> [    5.530991]  ? kfree+0x14c/0x160
> >>>> [    5.531024]  ? kmem_cache_alloc_trace+0x38/0x150
> >>>> [    5.531070]  do_init_module+0x5f/0x1f8
> >>>> [    5.531108]  load_module+0x271e/0x2bd0
> >>>> [    5.531147]  ? kernel_read_file+0x1a3/0x1c0
> >>>> [    5.531188]  SYSC_finit_module+0xbc/0xf0
> >>>> [    5.531226]  ? SYSC_finit_module+0xbc/0xf0
> >>>> [    5.531267]  SyS_finit_module+0xe/0x10
> >>>> [    5.531305]  do_syscall_64+0x6e/0x180
> >>>> [    5.531345]  entry_SYSCALL64_slow_path+0x25/0x25
> >>>> [    5.531389] RIP: 0033:0x7f627b5fbbf9
> >>>> [    5.531424] RSP: 002b:00007ffe053eee68 EFLAGS: 00000246 ORIG_RAX:
> >>>> 0000000000000139
> >>>> [    5.531493] RAX: ffffffffffffffda RBX: 000055d6c745b690 RCX:
> >>>> 00007f627b5fbbf9
> >>>> [    5.531558] RDX: 0000000000000000 RSI: 00007f627c134995 RDI:
> >>>> 0000000000000007
> >>>> [    5.531622] RBP: 00007f627c134995 R08: 0000000000000000 R09:
> >>>> 00007ffe053eef80
> >>>> [    5.531687] R10: 0000000000000007 R11: 0000000000000246 R12:
> >>>> 0000000000000000
> >>>> [    5.531751] R13: 000055d6c7459ae0 R14: 0000000000020000 R15:
> >>>> 000055d6c745b690
> >>>> [    5.531816] Code: 48 8b 45 b0 8b 48 18 e8 e0 cb 22 e1 49 8b 44 24 28
> >>>> 4a 8d 8c 33 58 01 00 00 48 8d 75 b8 45 31 c9 41 b8 01 00 00 00 ba 14 00
> >>>> 00 00 <48> 8b 38 e8 a9 2a ff ff 85 c0 0f 88 09 01 00 00 49 8b 84 24 58
> >>> Disassembling that I get:
> >>>
> >>>    4005a0:       48 8b 45 b0             mov    -0x50(%rbp),%rax
> >>>    4005a4:       8b 48 18                mov    0x18(%rax),%ecx
> >>>    4005a7:       e8 e0 cb 22 e1          callq  ffffffffe162d18c <_end+0xffffffffe102c14c>
> >>>    4005ac:       49 8b 44 24 28          mov    0x28(%r12),%rax
> >>>    4005b1:       4a 8d 8c 33 58 01 00    lea    0x158(%rbx,%r14,1),%rcx
> >>>    4005b8:       00
> >>>    4005b9:       48 8d 75 b8             lea    -0x48(%rbp),%rsi
> >>>    4005bd:       45 31 c9                xor    %r9d,%r9d
> >>>    4005c0:       41 b8 01 00 00 00       mov    $0x1,%r8d
> >>>    4005c6:       ba 14 00 00 00          mov    $0x14,%edx
> >>>    4005cb:       48 8b 38                mov    (%rax),%rdi
> >>>    4005ce:       e8 a9 2a ff ff          callq  3f307c <_init-0xd32c>
> >>>    4005d3:       85 c0                   test   %eax,%eax
> >>>    4005d5:       0f 88 09 01 00 00       js     4006e4 <__GNU_EH_FRAME_HDR+0x100>
> >>>    4005db:       49                      rex.WB
> >>>    4005dc:       8b                      .byte 0x8b
> >>>    4005dd:       84 24 58                test   %ah,(%rax,%rbx,2)
> >>>
> >>> Comparing that with the disassembly from my build, that
> >>> first call looks like the snprintf() and the second one would
> >>> then be snd_jack_new(). So it seems to blow in the
> >>> ctx->card_ctx->card dereference. But I don't really see how
> >>> a NULL pointer could sneak in there.
> >>
> >> There was an standard PORT_B off-by-one error which resulted in memory
> >> corruption.
> >
> > Doh.
> >
> >> the patch below seems to fix the problem, I can get the same
> >> functionality as before with concurrent playback on HDMI and DP, and the
> >> jack values are correctly set.
> >
> > Thanks. I do wonder a bit which side should apply this offset.
> > But I guess it doesn't really matter in the end as long as both
> > sides know what the deal is. So unless there are good reasons
> > for anything else I'll just squash this into my patch(es).
> 
> Fine with me.
> One open I still have is that it's not self-explanatory for the user
> to figure out which output the devices correspond to, e.g. on my CHT
> device HDMI is device 2 and DP is device 0.
> I may be mistaken but in the previous pipe-based solution, audio
> playback would progress at a normal rate even if nothing was
> connected. With the port-based, it looks like playback doesn't
> progress if there is nothing connected - speaker-test is stuck. Not
> sure if the open or hw_params should fail in that case? One could
> argue that the user should know better and check the jack status, but
> in reality I don't think any userspace layer will ever do this.

It's the problem we've had from the beginning.  If the open fails, PA
can't detect the PCM device at probe time, and it's gone forever until
PA gets restarted.  It's no hotplug device, thus the PCM streams are
determined only once.

So, the open and co are accepted even without the connection but the
actual playback fails -- it's implemented in that way as a
workaround for PA.


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

Re: [PATCH v2 00/11] drm/i915: LPE audio runtime PM and multipipe (v2)

[Ville Syrjälä]

On Fri, Apr 28, 2017 at 10:41:37AM +0200, Takashi Iwai wrote:
> On Thu, 27 Apr 2017 18:02:19 +0200,
> ville.syrjala@linux.intel.com wrote:
> > 
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Okay, here's the second attempt at getting multiple pipes playing back
> > audio on the VLV/CHV HDMI LPE audio device. The main change from v1 is
> > that now the PCM devices are associated with ports instead of pipes,
> > so the audio from one device always gets output on the same display.
> > 
> > I've also tacked on the alsa-lib conf update. No clue whether it's
> > really correct or not (the config language isn't a close friend
> > of mine).
> > 
> > BTW I did notice that with LPE audio all the controls say iface=PCM,
> > whereas on HDA a bunch of them say iface=MIXER. No idea if that's
> > OK or not, just something I spotted when I was comparing the results
> > with HDA.
> 
> We generally accept both iface types for IEC958 stuff, since
> historically many drivers have already mixed them up.  So it's no
> problem :)
> 
> 
> > Entire series available here:
> > git://github.com/vsyrjala/linux.git lpe_audio_multipipe_2
> > 
> > Cc: Takashi Iwai <tiwai@suse.de>
> > Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> 
> All look good, and feel free to take my reviewed-by tag
>   Reviewed-by: Takashi Iwai <tiwai@suse.de>

Thanks.

> 
> As said previously, my only slight concern is the compatibility.
> But, in the current situation with PulseAudio, only few people would
> use this driver, so it shouldn't be so big impact, I suppose.

What will break? Or you mean the alsa-lib vs. kernel difference
until they sync up? I don't use pulse myself so I don't know really
what it wants.

> 
> BTW, which port is used in general on BYT/CHT?

There's no clear rule I think. But I suppose most manufacturers
follow some reference design, so there could be some layouts
that are more common than other. Having HDMI on port D on CHV
is a fairly common design I've seen. And I think all the
pre-production RVP boards had eDP on port C, so that could also
be how production boards tend to be wired up.

> Oh, also, I suppose you want to carry these over i915 tree?
> I don't mind either way, I can take them through sound tree if
> preferred, too.

Cool. I just pushed the lot to drm-intel-next-queued. I'm
thinking going via dinq might avoid some conflicts later on
if we end up churning the code some more. And we do like to
churn ;)

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

Re: [PATCH v2 00/11] drm/i915: LPE audio runtime PM and multipipe (v2)

[Takashi Iwai]

On Wed, 03 May 2017 15:39:06 +0200,
Ville Syrjälä wrote:
> 
> On Fri, Apr 28, 2017 at 10:41:37AM +0200, Takashi Iwai wrote:
> > On Thu, 27 Apr 2017 18:02:19 +0200,
> > ville.syrjala@linux.intel.com wrote:
> > > 
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > Okay, here's the second attempt at getting multiple pipes playing back
> > > audio on the VLV/CHV HDMI LPE audio device. The main change from v1 is
> > > that now the PCM devices are associated with ports instead of pipes,
> > > so the audio from one device always gets output on the same display.
> > > 
> > > I've also tacked on the alsa-lib conf update. No clue whether it's
> > > really correct or not (the config language isn't a close friend
> > > of mine).
> > > 
> > > BTW I did notice that with LPE audio all the controls say iface=PCM,
> > > whereas on HDA a bunch of them say iface=MIXER. No idea if that's
> > > OK or not, just something I spotted when I was comparing the results
> > > with HDA.
> > 
> > We generally accept both iface types for IEC958 stuff, since
> > historically many drivers have already mixed them up.  So it's no
> > problem :)
> > 
> > 
> > > Entire series available here:
> > > git://github.com/vsyrjala/linux.git lpe_audio_multipipe_2
> > > 
> > > Cc: Takashi Iwai <tiwai@suse.de>
> > > Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> > 
> > All look good, and feel free to take my reviewed-by tag
> >   Reviewed-by: Takashi Iwai <tiwai@suse.de>
> 
> Thanks.
> 
> > 
> > As said previously, my only slight concern is the compatibility.
> > But, in the current situation with PulseAudio, only few people would
> > use this driver, so it shouldn't be so big impact, I suppose.
> 
> What will break? Or you mean the alsa-lib vs. kernel difference
> until they sync up? I don't use pulse myself so I don't know really
> what it wants.

Yes, the alsa-lib stuff is one thing.  Another thing is that it'll
change the behavior, for example, when you used to invoke a command
without PA.  It might not work any longer as is because now you'll
have multiple PCM devices and the attached device might be not the
first one.

PA would be possible to choose the rightly connected PCM stream, so
it's fine for PA.  But the "current situation" I mentioned above is
about PA getting killed at startup when LPE-audio and other drivers
are running.  So, for most people, LPE-audio will be blacklisted until
PA gets fixed, or it's worked around by changing PA config.  In
anyway, the driver should be still minor at this moment, so we may
give some excuse for changing.


> > BTW, which port is used in general on BYT/CHT?
> 
> There's no clear rule I think. But I suppose most manufacturers
> follow some reference design, so there could be some layouts
> that are more common than other. Having HDMI on port D on CHV
> is a fairly common design I've seen. And I think all the
> pre-production RVP boards had eDP on port C, so that could also
> be how production boards tend to be wired up.
> 
> > Oh, also, I suppose you want to carry these over i915 tree?
> > I don't mind either way, I can take them through sound tree if
> > preferred, too.
> 
> Cool. I just pushed the lot to drm-intel-next-queued. I'm
> thinking going via dinq might avoid some conflicts later on
> if we end up churning the code some more. And we do like to
> churn ;)

Have fun :)


thanks,

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