All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/11] drm/i915: LPE audio runtime PM and multipipe
@ 2017-04-25 20:27 ville.syrjala
  2017-04-25 20:27 ` [PATCH 01/11] drm/i915: Fix runtime PM for LPE audio ville.syrjala
                   ` (12 more replies)
  0 siblings, 13 replies; 28+ messages in thread
From: ville.syrjala @ 2017-04-25 20:27 UTC (permalink / raw)
  To: intel-gfx; +Cc: Takashi Iwai, alsa-devel

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

I was wondering why my VLV no longer runtime suspended, and after some
thinking I decided it had to be the LPE audio preventing it. Turns out
I was right, so here's my attempt at fixing it.

And while looking at the code I couldn't help but notice that it
couldn't actually handle multiple pipes playing back audio at the
same time. And even having multiple displays active even if only
one was playing audio was probably a recipe for failure. So I
tried to fix that by registering a separate PCM device for each
pipe.

Note that the patch subjects may not reflect the subsystem
very well since most of these straddle the border between drm
and alsa. I think I just slapped on drm/i915 to most where
there was no clear winner.

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

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 unsued 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 |  95 ++++-------
 include/drm/intel_lpe_audio.h          |  20 ++-
 sound/x86/intel_hdmi_audio.c           | 288 +++++++++++++++++++--------------
 sound/x86/intel_hdmi_audio.h           |  17 +-
 7 files changed, 230 insertions(+), 228 deletions(-)

-- 
2.10.2

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

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

* [PATCH 01/11] drm/i915: Fix runtime PM for LPE audio
  2017-04-25 20:27 [PATCH 00/11] drm/i915: LPE audio runtime PM and multipipe ville.syrjala
@ 2017-04-25 20:27 ` ville.syrjala
  2017-04-25 20:27 ` [PATCH 02/11] ALSA: x86: Clear the pdata.notify_lpe_audio pointer before teardown ville.syrjala
                   ` (11 subsequent siblings)
  12 siblings, 0 replies; 28+ messages in thread
From: ville.syrjala @ 2017-04-25 20:27 UTC (permalink / raw)
  To: intel-gfx; +Cc: Takashi Iwai, alsa-devel

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

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

* [PATCH 02/11] ALSA: x86: Clear the pdata.notify_lpe_audio pointer before teardown
  2017-04-25 20:27 [PATCH 00/11] drm/i915: LPE audio runtime PM and multipipe ville.syrjala
  2017-04-25 20:27 ` [PATCH 01/11] drm/i915: Fix runtime PM for LPE audio ville.syrjala
@ 2017-04-25 20:27 ` ville.syrjala
  2017-04-25 20:27 ` [PATCH 03/11] drm/i915: Stop pretending to mask/unmask LPE audio interrupts ville.syrjala
                   ` (10 subsequent siblings)
  12 siblings, 0 replies; 28+ messages in thread
From: ville.syrjala @ 2017-04-25 20:27 UTC (permalink / raw)
  To: intel-gfx; +Cc: Takashi Iwai, alsa-devel

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

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

* [PATCH 03/11] drm/i915: Stop pretending to mask/unmask LPE audio interrupts
  2017-04-25 20:27 [PATCH 00/11] drm/i915: LPE audio runtime PM and multipipe ville.syrjala
  2017-04-25 20:27 ` [PATCH 01/11] drm/i915: Fix runtime PM for LPE audio ville.syrjala
  2017-04-25 20:27 ` [PATCH 02/11] ALSA: x86: Clear the pdata.notify_lpe_audio pointer before teardown ville.syrjala
@ 2017-04-25 20:27 ` ville.syrjala
  2017-04-26  0:27   ` Pierre-Louis Bossart
  2017-04-25 20:27 ` [PATCH 04/11] drm/i915: Remove the unsued pending_notify from LPE platform data ville.syrjala
                   ` (9 subsequent siblings)
  12 siblings, 1 reply; 28+ messages in thread
From: ville.syrjala @ 2017-04-25 20:27 UTC (permalink / raw)
  To: intel-gfx; +Cc: Takashi Iwai, alsa-devel

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

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

* [PATCH 04/11] drm/i915: Remove the unsued pending_notify from LPE platform data
  2017-04-25 20:27 [PATCH 00/11] drm/i915: LPE audio runtime PM and multipipe ville.syrjala
                   ` (2 preceding siblings ...)
  2017-04-25 20:27 ` [PATCH 03/11] drm/i915: Stop pretending to mask/unmask LPE audio interrupts ville.syrjala
@ 2017-04-25 20:27 ` ville.syrjala
  2017-04-25 20:27 ` [PATCH 05/11] drm/i915: Replace tmds_clock_speed and link_rate with just ls_clock ville.syrjala
                   ` (8 subsequent siblings)
  12 siblings, 0 replies; 28+ messages in thread
From: ville.syrjala @ 2017-04-25 20:27 UTC (permalink / raw)
  To: intel-gfx; +Cc: Takashi Iwai, alsa-devel

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.

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

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

* [PATCH 05/11] drm/i915: Replace tmds_clock_speed and link_rate with just ls_clock
  2017-04-25 20:27 [PATCH 00/11] drm/i915: LPE audio runtime PM and multipipe ville.syrjala
                   ` (3 preceding siblings ...)
  2017-04-25 20:27 ` [PATCH 04/11] drm/i915: Remove the unsued pending_notify from LPE platform data ville.syrjala
@ 2017-04-25 20:27 ` ville.syrjala
  2017-04-26  1:09   ` [alsa-devel] " Pierre-Louis Bossart
  2017-04-25 20:27 ` [PATCH 06/11] drm/i915: Remove hdmi_connected from LPE audio pdata ville.syrjala
                   ` (7 subsequent siblings)
  12 siblings, 1 reply; 28+ messages in thread
From: ville.syrjala @ 2017-04-25 20:27 UTC (permalink / raw)
  To: intel-gfx; +Cc: Takashi Iwai, alsa-devel, Pierre-Louis Bossart

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 357b6c6c2f04..876eee56a958 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

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

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

* [PATCH 06/11] drm/i915: Remove hdmi_connected from LPE audio pdata
  2017-04-25 20:27 [PATCH 00/11] drm/i915: LPE audio runtime PM and multipipe ville.syrjala
                   ` (4 preceding siblings ...)
  2017-04-25 20:27 ` [PATCH 05/11] drm/i915: Replace tmds_clock_speed and link_rate with just ls_clock ville.syrjala
@ 2017-04-25 20:27 ` ville.syrjala
  2017-04-25 20:27 ` [PATCH 07/11] drm/i915: Reorganize intel_lpe_audio_notify() arguments ville.syrjala
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 28+ messages in thread
From: ville.syrjala @ 2017-04-25 20:27 UTC (permalink / raw)
  To: intel-gfx; +Cc: Takashi Iwai, alsa-devel

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

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

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 | 5 ++---
 include/drm/intel_lpe_audio.h          | 3 +--
 sound/x86/intel_hdmi_audio.c           | 4 ++--
 3 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c
index 5a1a37e963f1..1696359bf6e5 100644
--- a/drivers/gpu/drm/i915/intel_lpe_audio.c
+++ b/drivers/gpu/drm/i915/intel_lpe_audio.c
@@ -335,9 +335,8 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv,
 	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->port = port;
 		pdata->ls_clock = ls_clock;
 		pdata->dp_output = dp_output;
 
@@ -348,7 +347,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->port = -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..826d531c3ecc 100644
--- a/include/drm/intel_lpe_audio.h
+++ b/include/drm/intel_lpe_audio.h
@@ -32,14 +32,13 @@ struct platform_device;
 #define HDMI_MAX_ELD_BYTES	128
 
 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 port;
 	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..71f14a2a7fe4 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->port < 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,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->ls_clock);
+			__func__, pdata->port, pdata->ls_clock);
 
 		switch (eld->pipe_id) {
 		case 0:
-- 
2.10.2

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

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

* [PATCH 07/11] drm/i915: Reorganize intel_lpe_audio_notify() arguments
  2017-04-25 20:27 [PATCH 00/11] drm/i915: LPE audio runtime PM and multipipe ville.syrjala
                   ` (5 preceding siblings ...)
  2017-04-25 20:27 ` [PATCH 06/11] drm/i915: Remove hdmi_connected from LPE audio pdata ville.syrjala
@ 2017-04-25 20:27 ` ville.syrjala
  2017-04-25 20:27 ` [PATCH 08/11] drm/i915: Clean up the LPE audio platform data ville.syrjala
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 28+ messages in thread
From: ville.syrjala @ 2017-04-25 20:27 UTC (permalink / raw)
  To: intel-gfx; +Cc: Takashi Iwai, alsa-devel, Pierre-Louis Bossart

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 876eee56a958..e6230f68ee80 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 1696359bf6e5..d6aecf1d382b 100644
--- a/drivers/gpu/drm/i915/intel_lpe_audio.c
+++ b/drivers/gpu/drm/i915/intel_lpe_audio.c
@@ -306,17 +306,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

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

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

* [PATCH 08/11] drm/i915: Clean up the LPE audio platform data
  2017-04-25 20:27 [PATCH 00/11] drm/i915: LPE audio runtime PM and multipipe ville.syrjala
                   ` (6 preceding siblings ...)
  2017-04-25 20:27 ` [PATCH 07/11] drm/i915: Reorganize intel_lpe_audio_notify() arguments ville.syrjala
@ 2017-04-25 20:27 ` ville.syrjala
  2017-04-25 20:27 ` [PATCH 09/11] ALSA: x86: Prepare LPE audio ctls for multiple PCMs ville.syrjala
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 28+ messages in thread
From: ville.syrjala @ 2017-04-25 20:27 UTC (permalink / raw)
  To: intel-gfx; +Cc: Takashi Iwai, alsa-devel

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

Split the LPE audio platform data into a pipe specific
chunk and device specific chunk. Eventually we'll have
a pipe specific chunk for each pipe, 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.

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 | 29 ++++++++++++++---------------
 include/drm/intel_lpe_audio.h          | 15 ++++++++-------
 sound/x86/intel_hdmi_audio.c           | 19 +++++++++----------
 3 files changed, 31 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c
index d6aecf1d382b..a593fdf73171 100644
--- a/drivers/gpu/drm/i915/intel_lpe_audio.c
+++ b/drivers/gpu/drm/i915/intel_lpe_audio.c
@@ -319,37 +319,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_pipe_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->pipe;
 
 	spin_lock_irqsave(&pdata->lpe_audio_slock, irq_flags);
 
 	audio_enable = I915_READ(VLV_AUD_PORT_EN_DBG(port));
 
+	pdata->pipe_id = pipe;
+
 	if (eld != NULL) {
-		memcpy(pdata->eld.eld_data, eld,
-			HDMI_MAX_ELD_BYTES);
-		pdata->eld.pipe_id = pipe;
-		pdata->port = port;
-		pdata->ls_clock = ls_clock;
-		pdata->dp_output = dp_output;
+		memcpy(ppdata->eld, eld, HDMI_MAX_ELD_BYTES);
+		ppdata->port = port;
+		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->port = -1;
-		pdata->ls_clock = 0;
-		pdata->dp_output = false;
+		memset(ppdata->eld, 0, HDMI_MAX_ELD_BYTES);
+		ppdata->port = -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 826d531c3ecc..26e569ad8683 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 pipe_id;
-	unsigned char eld_data[HDMI_MAX_ELD_BYTES];
-};
-
-struct intel_hdmi_lpe_audio_pdata {
+struct intel_hdmi_lpe_audio_pipe_pdata {
+	u8 eld[HDMI_MAX_ELD_BYTES];
 	int port;
 	int ls_clock;
 	bool dp_output;
-	struct intel_hdmi_lpe_audio_eld eld;
+};
+
+struct intel_hdmi_lpe_audio_pdata {
+	struct intel_hdmi_lpe_audio_pipe_pdata pipe;
+	int pipe_id;
+
 	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 71f14a2a7fe4..bfb712444098 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_pipe_pdata *ppdata = &pdata->pipe;
 
 	pm_runtime_get_sync(ctx->dev);
 	mutex_lock(&ctx->mutex);
-	if (pdata->port < 0) {
+	if (ppdata->port < 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__, pdata->port, pdata->ls_clock);
+			__func__, ppdata->port, ppdata->ls_clock);
 
-		switch (eld->pipe_id) {
+		switch (pdata->pipe_id) {
 		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",
-				eld->pipe_id);
+				pdata->pipe_id);
 			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

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

* [PATCH 09/11] ALSA: x86: Prepare LPE audio ctls for multiple PCMs
  2017-04-25 20:27 [PATCH 00/11] drm/i915: LPE audio runtime PM and multipipe ville.syrjala
                   ` (7 preceding siblings ...)
  2017-04-25 20:27 ` [PATCH 08/11] drm/i915: Clean up the LPE audio platform data ville.syrjala
@ 2017-04-25 20:27 ` ville.syrjala
  2017-04-25 20:27 ` [PATCH 10/11] ALSA: x86: Split snd_intelhad into card and PCM specific structures ville.syrjala
                   ` (3 subsequent siblings)
  12 siblings, 0 replies; 28+ messages in thread
From: ville.syrjala @ 2017-04-25 20:27 UTC (permalink / raw)
  To: intel-gfx; +Cc: Takashi Iwai, alsa-devel

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

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

* [PATCH 10/11] ALSA: x86: Split snd_intelhad into card and PCM specific structures
  2017-04-25 20:27 [PATCH 00/11] drm/i915: LPE audio runtime PM and multipipe ville.syrjala
                   ` (8 preceding siblings ...)
  2017-04-25 20:27 ` [PATCH 09/11] ALSA: x86: Prepare LPE audio ctls for multiple PCMs ville.syrjala
@ 2017-04-25 20:27 ` ville.syrjala
  2017-04-25 20:27 ` [PATCH 11/11] ALSA: x86: Register multiple PCM devices for the LPE audio card ville.syrjala
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 28+ messages in thread
From: ville.syrjala @ 2017-04-25 20:27 UTC (permalink / raw)
  To: intel-gfx; +Cc: Takashi Iwai, alsa-devel

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.

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 | 228 +++++++++++++++++++++++++------------------
 sound/x86/intel_hdmi_audio.h |  16 ++-
 2 files changed, 144 insertions(+), 100 deletions(-)

diff --git a/sound/x86/intel_hdmi_audio.c b/sound/x86/intel_hdmi_audio.c
index a3d15482f07e..5e2149fe5218 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_pipe(card_ctx, pipe) \
+	for ((pipe) = 0; (pipe) < (card_ctx)->num_pipes; (pipe)++)
+
 /*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 pipe;
 
-	/* use raw register access to ack IRQs even while disconnected */
-	audio_stat = had_read_register_raw(ctx, AUD_HDMI_STATUS);
+	for_each_pipe(card_ctx, pipe) {
+		struct snd_intelhad *ctx = &card_ctx->pcm_ctx[pipe];
+		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 pipe;
+
+	for_each_pipe(card_ctx, pipe) {
+		struct snd_intelhad *ctx = &card_ctx->pcm_ctx[pipe];
 
-	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 pipe;
 
-	substream = had_substream_get(ctx);
-	if (substream) {
-		snd_pcm_suspend(substream);
-		had_substream_put(ctx);
+	for_each_pipe(card_ctx, pipe) {
+		struct snd_intelhad *ctx = &card_ctx->pcm_ctx[pipe];
+		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 pipe;
 
 	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_pipe(card_ctx, pipe) {
+		struct snd_intelhad *ctx = &card_ctx->pcm_ctx[pipe];
+
+		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 pipe, 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,80 @@ 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_pipes = 1;
 
-		kctl = snd_ctl_new1(&had_controls[i], ctx);
-		if (!kctl) {
-			ret = -ENOMEM;
+	for_each_pipe(card_ctx, pipe) {
+		struct snd_intelhad *ctx = &card_ctx->pcm_ctx[pipe];
+		int i;
+
+		ctx->card_ctx = card_ctx;
+		ctx->dev = card_ctx->dev;
+		ctx->pipe = pipe;
+
+		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 +1867,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_pipe(card_ctx, pipe) {
+		struct snd_intelhad *ctx = &card_ctx->pcm_ctx[pipe];
+
+		schedule_work(&ctx->hdmi_audio_wq);
+	}
 
 	return 0;
 
@@ -1853,9 +1887,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..a209096b03df 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];
@@ -112,6 +112,7 @@ struct snd_intelhad {
 	struct snd_pcm_chmap *chmap;
 	int tmds_clock_speed;
 	int link_rate;
+	int pipe;
 
 	/* ring buffer (BD) position index */
 	unsigned int bd_head;
@@ -123,8 +124,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 +132,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_pipes;
+	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

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

* [PATCH 11/11] ALSA: x86: Register multiple PCM devices for the LPE audio card
  2017-04-25 20:27 [PATCH 00/11] drm/i915: LPE audio runtime PM and multipipe ville.syrjala
                   ` (9 preceding siblings ...)
  2017-04-25 20:27 ` [PATCH 10/11] ALSA: x86: Split snd_intelhad into card and PCM specific structures ville.syrjala
@ 2017-04-25 20:27 ` ville.syrjala
  2017-04-26  1:58   ` Pierre-Louis Bossart
  2017-04-25 20:46 ` ✓ Fi.CI.BAT: success for drm/i915: LPE audio runtime PM and multipipe Patchwork
  2017-04-26  7:29 ` [PATCH 00/11] " Takashi Iwai
  12 siblings, 1 reply; 28+ messages in thread
From: ville.syrjala @ 2017-04-25 20:27 UTC (permalink / raw)
  To: intel-gfx; +Cc: Takashi Iwai, alsa-devel

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

Now that everything is in place let's register a PCM device for
each pipe 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.

The alternative would be to have a PCM device per port, but per-pipe
is easier since the hardware actually works that way.

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 | 14 ++++-----
 include/drm/intel_lpe_audio.h          |  6 ++--
 sound/x86/intel_hdmi_audio.c           | 53 +++++++++++++++-------------------
 sound/x86/intel_hdmi_audio.h           |  3 +-
 4 files changed, 34 insertions(+), 42 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c
index a593fdf73171..270aa3e3f0e2 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->num_pipes = INTEL_INFO(dev_priv)->num_pipes;
 	spin_lock_init(&pdata->lpe_audio_slock);
 
 	platdev = platform_device_register_full(&pinfo);
@@ -318,7 +319,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_pipe_pdata *ppdata;
 	u32 audio_enable;
@@ -327,14 +328,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->pipe;
+	ppdata = &pdata->pipe[pipe];
 
-	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));
 
-	pdata->pipe_id = pipe;
-
 	if (eld != NULL) {
 		memcpy(ppdata->eld, eld, HDMI_MAX_ELD_BYTES);
 		ppdata->port = port;
@@ -356,8 +355,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, pipe);
 
-	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 26e569ad8683..b391b2822140 100644
--- a/include/drm/intel_lpe_audio.h
+++ b/include/drm/intel_lpe_audio.h
@@ -39,10 +39,10 @@ struct intel_hdmi_lpe_audio_pipe_pdata {
 };
 
 struct intel_hdmi_lpe_audio_pdata {
-	struct intel_hdmi_lpe_audio_pipe_pdata pipe;
-	int pipe_id;
+	struct intel_hdmi_lpe_audio_pipe_pdata pipe[3];
+	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 5e2149fe5218..e5863a6d3aa9 100644
--- a/sound/x86/intel_hdmi_audio.c
+++ b/sound/x86/intel_hdmi_audio.c
@@ -195,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->card_ctx->mmio_start + ctx->had_config_offset + reg);
+	return ioread32(ctx->mmio_start + reg);
 }
 
 static void had_write_register_raw(struct snd_intelhad *ctx, u32 reg, u32 val)
 {
-	iowrite32(val, ctx->card_ctx->mmio_start + ctx->had_config_offset + reg);
+	iowrite32(val, ctx->mmio_start + reg);
 }
 
 static void had_read_register(struct snd_intelhad *ctx, u32 reg, u32 *val)
@@ -1551,16 +1551,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 pipe)
 {
 	struct snd_intelhad_card *card_ctx = platform_get_drvdata(pdev);
-	int pipe;
-
-	for_each_pipe(card_ctx, pipe) {
-		struct snd_intelhad *ctx = &card_ctx->pcm_ctx[pipe];
+	struct snd_intelhad *ctx = &card_ctx->pcm_ctx[pipe];
 
-		schedule_work(&ctx->hdmi_audio_wq);
-	}
+	schedule_work(&ctx->hdmi_audio_wq);
 }
 
 /* the work to handle monitor hot plug/unplug */
@@ -1569,7 +1565,7 @@ 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_pipe_pdata *ppdata = &pdata->pipe;
+	struct intel_hdmi_lpe_audio_pipe_pdata *ppdata = &pdata->pipe[ctx->pipe];
 
 	pm_runtime_get_sync(ctx->dev);
 	mutex_lock(&ctx->mutex);
@@ -1582,22 +1578,6 @@ static void had_audio_wq(struct work_struct *work)
 		dev_dbg(ctx->dev, "%s: HAD_NOTIFY_ELD : port = %d, tmds = %d\n",
 			__func__, ppdata->port, ppdata->ls_clock);
 
-		switch (pdata->pipe_id) {
-		case 0:
-			ctx->had_config_offset = AUDIO_HDMI_CONFIG_A;
-			break;
-		case 1:
-			ctx->had_config_offset = AUDIO_HDMI_CONFIG_B;
-			break;
-		case 2:
-			ctx->had_config_offset = AUDIO_HDMI_CONFIG_C;
-			break;
-		default:
-			dev_dbg(ctx->dev, "Invalid pipe %d\n",
-				pdata->pipe_id);
-			break;
-		}
-
 		memcpy(ctx->eld, ppdata->eld, sizeof(ctx->eld));
 
 		ctx->dp_output = ppdata->dp_output;
@@ -1794,7 +1774,7 @@ static int hdmi_lpe_audio_probe(struct platform_device *pdev)
 
 	init_channel_allocations();
 
-	card_ctx->num_pipes = 1;
+	card_ctx->num_pipes = pdata->num_pipes;
 
 	for_each_pipe(card_ctx, pipe) {
 		struct snd_intelhad *ctx = &card_ctx->pcm_ctx[pipe];
@@ -1806,9 +1786,24 @@ static int hdmi_lpe_audio_probe(struct platform_device *pdev)
 
 		INIT_WORK(&ctx->hdmi_audio_wq, had_audio_wq);
 
-		ctx->had_config_offset = AUDIO_HDMI_CONFIG_A;
+		switch (pipe) {
+		case 0:
+			ctx->mmio_start =
+				card_ctx->mmio_start + AUDIO_HDMI_CONFIG_A;
+			break;
+		case 1:
+			ctx->mmio_start =
+				card_ctx->mmio_start + AUDIO_HDMI_CONFIG_B;
+			break;
+		case 2:
+			ctx->mmio_start =
+				card_ctx->mmio_start + AUDIO_HDMI_CONFIG_C;
+			break;
+		default:
+			break;
+		}
 
-		ret = snd_pcm_new(card, INTEL_HAD, PCM_INDEX, MAX_PB_STREAMS,
+		ret = snd_pcm_new(card, INTEL_HAD, pipe, 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 a209096b03df..ab0de5d525f4 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
@@ -124,7 +123,7 @@ struct snd_intelhad {
 	unsigned int period_bytes;	/* PCM period size in bytes */
 
 	/* internal stuff */
-	unsigned int had_config_offset;
+	void __iomem *mmio_start;
 	union aud_cfg aud_config;	/* AUD_CONFIG reg value cache */
 	struct work_struct hdmi_audio_wq;
 	struct mutex mutex; /* for protecting chmap and eld */
-- 
2.10.2

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

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

* ✓ Fi.CI.BAT: success for drm/i915: LPE audio runtime PM and multipipe
  2017-04-25 20:27 [PATCH 00/11] drm/i915: LPE audio runtime PM and multipipe ville.syrjala
                   ` (10 preceding siblings ...)
  2017-04-25 20:27 ` [PATCH 11/11] ALSA: x86: Register multiple PCM devices for the LPE audio card ville.syrjala
@ 2017-04-25 20:46 ` Patchwork
  2017-04-26  7:29 ` [PATCH 00/11] " Takashi Iwai
  12 siblings, 0 replies; 28+ messages in thread
From: Patchwork @ 2017-04-25 20:46 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: LPE audio runtime PM and multipipe
URL   : https://patchwork.freedesktop.org/series/23526/
State : success

== Summary ==

Series 23526v1 drm/i915: LPE audio runtime PM and multipipe
https://patchwork.freedesktop.org/api/1.0/series/23526/revisions/1/mbox/

Test gem_exec_suspend:
        Subgroup basic-s4-devices:
                pass       -> DMESG-WARN (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:428s
fi-bdw-gvtdvm    total:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  time:424s
fi-bsw-n3050     total:278  pass:242  dwarn:0   dfail:0   fail:0   skip:36  time:587s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time:507s
fi-byt-j1900     total:278  pass:254  dwarn:0   dfail:0   fail:0   skip:24  time:493s
fi-byt-n2820     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:479s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:410s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:408s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time:418s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:492s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:482s
fi-kbl-7500u     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:452s
fi-kbl-7560u     total:278  pass:267  dwarn:1   dfail:0   fail:0   skip:10  time:571s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:452s
fi-skl-6700hq    total:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17  time:574s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time:457s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:500s
fi-skl-gvtdvm    total:278  pass:265  dwarn:0   dfail:0   fail:0   skip:13  time:436s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:530s
fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29  time:414s

7ffb3045557cbc7b49695b20416351e4e812179c drm-tip: 2017y-04m-25d-14h-42m-59s UTC integration manifest
1e6aff4 ALSA: x86: Register multiple PCM devices for the LPE audio card
94e8ba1 ALSA: x86: Split snd_intelhad into card and PCM specific structures
5efb22e ALSA: x86: Prepare LPE audio ctls for multiple PCMs
d070759 drm/i915: Clean up the LPE audio platform data
e00a731 drm/i915: Reorganize intel_lpe_audio_notify() arguments
c9354d7 drm/i915: Remove hdmi_connected from LPE audio pdata
b97f535 drm/i915: Replace tmds_clock_speed and link_rate with just ls_clock
eed2aa0 drm/i915: Remove the unsued pending_notify from LPE platform data
d947cd9 drm/i915: Stop pretending to mask/unmask LPE audio interrupts
7a53acb ALSA: x86: Clear the pdata.notify_lpe_audio pointer before teardown
8c0286e drm/i915: Fix runtime PM for LPE audio

== Logs ==

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

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

* Re: [PATCH 03/11] drm/i915: Stop pretending to mask/unmask LPE audio interrupts
  2017-04-25 20:27 ` [PATCH 03/11] drm/i915: Stop pretending to mask/unmask LPE audio interrupts ville.syrjala
@ 2017-04-26  0:27   ` Pierre-Louis Bossart
  2017-04-26 13:27     ` Ville Syrjälä
  0 siblings, 1 reply; 28+ messages in thread
From: Pierre-Louis Bossart @ 2017-04-26  0:27 UTC (permalink / raw)
  To: ville.syrjala, intel-gfx; +Cc: Takashi Iwai, alsa-devel

On 4/25/17 3:27 PM, ville.syrjala@linux.intel.com wrote:
> 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.

No objections, I assumed that we did want to update VLV_IMR and VLV_IIR 
in the mask/unmask, that was the initial recommendation IIRC

There was also a comment where we removed all tests in 
vlv_display_irq_postinstall:

 >> +		if (IS_LPE_AUDIO_ENABLED(dev_priv))
 >> +			if (IS_LPE_AUDIO_IRQ_VALID(dev_priv))
 >
 >I think both of these checks can be removed. We won't unmask the
 >interrupts unless lpe is enabled, so the IIR bits will never be set in
 >that case.




>
> 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);
>

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

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

* Re: [alsa-devel] [PATCH 05/11] drm/i915: Replace tmds_clock_speed and link_rate with just ls_clock
  2017-04-25 20:27 ` [PATCH 05/11] drm/i915: Replace tmds_clock_speed and link_rate with just ls_clock ville.syrjala
@ 2017-04-26  1:09   ` Pierre-Louis Bossart
  2017-04-26 13:28     ` Ville Syrjälä
  0 siblings, 1 reply; 28+ messages in thread
From: Pierre-Louis Bossart @ 2017-04-26  1:09 UTC (permalink / raw)
  To: ville.syrjala, intel-gfx; +Cc: Takashi Iwai, alsa-devel

On 4/25/17 3:27 PM, ville.syrjala@linux.intel.com wrote:
> 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.

there are still occurences of 'tmds' in sound/x86 and there are are 
couple of debug messages that don't make sense any longer.

>
> 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 357b6c6c2f04..876eee56a958 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);
>
>

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

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

* Re: [PATCH 11/11] ALSA: x86: Register multiple PCM devices for the LPE audio card
  2017-04-25 20:27 ` [PATCH 11/11] ALSA: x86: Register multiple PCM devices for the LPE audio card ville.syrjala
@ 2017-04-26  1:58   ` Pierre-Louis Bossart
  2017-04-26  7:04     ` [alsa-devel] " Takashi Iwai
  0 siblings, 1 reply; 28+ messages in thread
From: Pierre-Louis Bossart @ 2017-04-26  1:58 UTC (permalink / raw)
  To: ville.syrjala, intel-gfx; +Cc: Takashi Iwai, alsa-devel



On 04/25/2017 03:27 PM, 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 pipe 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.
>
> The alternative would be to have a PCM device per port, but per-pipe
> is easier since the hardware actually works that way.
Very nice. I just tested on a CHT Zotac box which has two connectors (1 
HDMI and 1 DP), and I get sound concurrently on both, with hdmi being 
listed as device 2 and DP as device 0.
I thought there were hardware restrictions but you proved me wrong. Kudos.

The only point that I find weird is that the jacks are reported as 'on' 
on the 3 pipes, is there a way to tie them to an actual cable being used?

[plb@ZOTAC ~]$ amixer -Dhw:0 controls | grep Jack
numid=5,iface=CARD,name='HDMI/DP,pcm=0 Jack'
numid=10,iface=CARD,name='HDMI/DP,pcm=1 Jack'
numid=15,iface=CARD,name='HDMI/DP,pcm=2 Jack'
[plb@ZOTAC ~]$ amixer -Dhw:0 cget numid=5
numid=5,iface=CARD,name='HDMI/DP,pcm=0 Jack'
   ; type=BOOLEAN,access=r-------,values=1
   : values=on
[plb@ZOTAC ~]$ amixer -Dhw:0 cset numid=5 off
amixer: Control hw:0 element write error: Operation not permitted

[plb@ZOTAC ~]$ amixer -Dhw:0 cget numid=10
numid=10,iface=CARD,name='HDMI/DP,pcm=1 Jack'
   ; type=BOOLEAN,access=r-------,values=1
   : values=on
[plb@ZOTAC ~]$ amixer -Dhw:0 cget numid=15
numid=15,iface=CARD,name='HDMI/DP,pcm=2 Jack'
   ; type=BOOLEAN,access=r-------,values=1
   : values=on

The ELD controls do show a null set of values for device 1, maybe the 
jack value should be set in accordance with the ELD validity?
Also I am wondering if the display number could be used for the PCM 
device number, or as a hint in the device description to help the user 
know which PCM device to use.

Anyway thanks for this patchset, nicely done.

>
> 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 | 14 ++++-----
>   include/drm/intel_lpe_audio.h          |  6 ++--
>   sound/x86/intel_hdmi_audio.c           | 53 +++++++++++++++-------------------
>   sound/x86/intel_hdmi_audio.h           |  3 +-
>   4 files changed, 34 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c
> index a593fdf73171..270aa3e3f0e2 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->num_pipes = INTEL_INFO(dev_priv)->num_pipes;
>   	spin_lock_init(&pdata->lpe_audio_slock);
>   
>   	platdev = platform_device_register_full(&pinfo);
> @@ -318,7 +319,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_pipe_pdata *ppdata;
>   	u32 audio_enable;
> @@ -327,14 +328,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->pipe;
> +	ppdata = &pdata->pipe[pipe];
>   
> -	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));
>   
> -	pdata->pipe_id = pipe;
> -
>   	if (eld != NULL) {
>   		memcpy(ppdata->eld, eld, HDMI_MAX_ELD_BYTES);
>   		ppdata->port = port;
> @@ -356,8 +355,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, pipe);
>   
> -	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 26e569ad8683..b391b2822140 100644
> --- a/include/drm/intel_lpe_audio.h
> +++ b/include/drm/intel_lpe_audio.h
> @@ -39,10 +39,10 @@ struct intel_hdmi_lpe_audio_pipe_pdata {
>   };
>   
>   struct intel_hdmi_lpe_audio_pdata {
> -	struct intel_hdmi_lpe_audio_pipe_pdata pipe;
> -	int pipe_id;
> +	struct intel_hdmi_lpe_audio_pipe_pdata pipe[3];
> +	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 5e2149fe5218..e5863a6d3aa9 100644
> --- a/sound/x86/intel_hdmi_audio.c
> +++ b/sound/x86/intel_hdmi_audio.c
> @@ -195,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->card_ctx->mmio_start + ctx->had_config_offset + reg);
> +	return ioread32(ctx->mmio_start + reg);
>   }
>   
>   static void had_write_register_raw(struct snd_intelhad *ctx, u32 reg, u32 val)
>   {
> -	iowrite32(val, ctx->card_ctx->mmio_start + ctx->had_config_offset + reg);
> +	iowrite32(val, ctx->mmio_start + reg);
>   }
>   
>   static void had_read_register(struct snd_intelhad *ctx, u32 reg, u32 *val)
> @@ -1551,16 +1551,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 pipe)
>   {
>   	struct snd_intelhad_card *card_ctx = platform_get_drvdata(pdev);
> -	int pipe;
> -
> -	for_each_pipe(card_ctx, pipe) {
> -		struct snd_intelhad *ctx = &card_ctx->pcm_ctx[pipe];
> +	struct snd_intelhad *ctx = &card_ctx->pcm_ctx[pipe];
>   
> -		schedule_work(&ctx->hdmi_audio_wq);
> -	}
> +	schedule_work(&ctx->hdmi_audio_wq);
>   }
>   
>   /* the work to handle monitor hot plug/unplug */
> @@ -1569,7 +1565,7 @@ 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_pipe_pdata *ppdata = &pdata->pipe;
> +	struct intel_hdmi_lpe_audio_pipe_pdata *ppdata = &pdata->pipe[ctx->pipe];
>   
>   	pm_runtime_get_sync(ctx->dev);
>   	mutex_lock(&ctx->mutex);
> @@ -1582,22 +1578,6 @@ static void had_audio_wq(struct work_struct *work)
>   		dev_dbg(ctx->dev, "%s: HAD_NOTIFY_ELD : port = %d, tmds = %d\n",
>   			__func__, ppdata->port, ppdata->ls_clock);
>   
> -		switch (pdata->pipe_id) {
> -		case 0:
> -			ctx->had_config_offset = AUDIO_HDMI_CONFIG_A;
> -			break;
> -		case 1:
> -			ctx->had_config_offset = AUDIO_HDMI_CONFIG_B;
> -			break;
> -		case 2:
> -			ctx->had_config_offset = AUDIO_HDMI_CONFIG_C;
> -			break;
> -		default:
> -			dev_dbg(ctx->dev, "Invalid pipe %d\n",
> -				pdata->pipe_id);
> -			break;
> -		}
> -
>   		memcpy(ctx->eld, ppdata->eld, sizeof(ctx->eld));
>   
>   		ctx->dp_output = ppdata->dp_output;
> @@ -1794,7 +1774,7 @@ static int hdmi_lpe_audio_probe(struct platform_device *pdev)
>   
>   	init_channel_allocations();
>   
> -	card_ctx->num_pipes = 1;
> +	card_ctx->num_pipes = pdata->num_pipes;
>   
>   	for_each_pipe(card_ctx, pipe) {
>   		struct snd_intelhad *ctx = &card_ctx->pcm_ctx[pipe];
> @@ -1806,9 +1786,24 @@ static int hdmi_lpe_audio_probe(struct platform_device *pdev)
>   
>   		INIT_WORK(&ctx->hdmi_audio_wq, had_audio_wq);
>   
> -		ctx->had_config_offset = AUDIO_HDMI_CONFIG_A;
> +		switch (pipe) {
> +		case 0:
> +			ctx->mmio_start =
> +				card_ctx->mmio_start + AUDIO_HDMI_CONFIG_A;
> +			break;
> +		case 1:
> +			ctx->mmio_start =
> +				card_ctx->mmio_start + AUDIO_HDMI_CONFIG_B;
> +			break;
> +		case 2:
> +			ctx->mmio_start =
> +				card_ctx->mmio_start + AUDIO_HDMI_CONFIG_C;
> +			break;
> +		default:
> +			break;
> +		}
>   
> -		ret = snd_pcm_new(card, INTEL_HAD, PCM_INDEX, MAX_PB_STREAMS,
> +		ret = snd_pcm_new(card, INTEL_HAD, pipe, 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 a209096b03df..ab0de5d525f4 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
> @@ -124,7 +123,7 @@ struct snd_intelhad {
>   	unsigned int period_bytes;	/* PCM period size in bytes */
>   
>   	/* internal stuff */
> -	unsigned int had_config_offset;
> +	void __iomem *mmio_start;
>   	union aud_cfg aud_config;	/* AUD_CONFIG reg value cache */
>   	struct work_struct hdmi_audio_wq;
>   	struct mutex mutex; /* for protecting chmap and eld */

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

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

* Re: [alsa-devel] [PATCH 11/11] ALSA: x86: Register multiple PCM devices for the LPE audio card
  2017-04-26  1:58   ` Pierre-Louis Bossart
@ 2017-04-26  7:04     ` Takashi Iwai
  2017-04-26  7:19       ` Takashi Iwai
  0 siblings, 1 reply; 28+ messages in thread
From: Takashi Iwai @ 2017-04-26  7:04 UTC (permalink / raw)
  To: Pierre-Louis Bossart; +Cc: intel-gfx, alsa-devel

On Wed, 26 Apr 2017 03:58:57 +0200,
Pierre-Louis Bossart wrote:
> 
> On 04/25/2017 03:27 PM, 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 pipe 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.
> >
> > The alternative would be to have a PCM device per port, but per-pipe
> > is easier since the hardware actually works that way.
> Very nice. I just tested on a CHT Zotac box which has two connectors
> (1 HDMI and 1 DP), and I get sound concurrently on both, with hdmi
> being listed as device 2 and DP as device 0.
> I thought there were hardware restrictions but you proved me wrong. Kudos.
> 
> The only point that I find weird is that the jacks are reported as
> 'on' on the 3 pipes, is there a way to tie them to an actual cable
> being used?

The pdata check was changed to check port=-1 as the monitor off in the
patch 6.  Maybe the initialization is missing?


Takashi

> 
> [plb@ZOTAC ~]$ amixer -Dhw:0 controls | grep Jack
> numid=5,iface=CARD,name='HDMI/DP,pcm=0 Jack'
> numid=10,iface=CARD,name='HDMI/DP,pcm=1 Jack'
> numid=15,iface=CARD,name='HDMI/DP,pcm=2 Jack'
> [plb@ZOTAC ~]$ amixer -Dhw:0 cget numid=5
> numid=5,iface=CARD,name='HDMI/DP,pcm=0 Jack'
>   ; type=BOOLEAN,access=r-------,values=1
>   : values=on
> [plb@ZOTAC ~]$ amixer -Dhw:0 cset numid=5 off
> amixer: Control hw:0 element write error: Operation not permitted
> 
> [plb@ZOTAC ~]$ amixer -Dhw:0 cget numid=10
> numid=10,iface=CARD,name='HDMI/DP,pcm=1 Jack'
>   ; type=BOOLEAN,access=r-------,values=1
>   : values=on
> [plb@ZOTAC ~]$ amixer -Dhw:0 cget numid=15
> numid=15,iface=CARD,name='HDMI/DP,pcm=2 Jack'
>   ; type=BOOLEAN,access=r-------,values=1
>   : values=on
> 
> The ELD controls do show a null set of values for device 1, maybe the
> jack value should be set in accordance with the ELD validity?
> Also I am wondering if the display number could be used for the PCM
> device number, or as a hint in the device description to help the user
> know which PCM device to use.
> 
> Anyway thanks for this patchset, nicely done.
> 
> >
> > 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 | 14 ++++-----
> >   include/drm/intel_lpe_audio.h          |  6 ++--
> >   sound/x86/intel_hdmi_audio.c           | 53 +++++++++++++++-------------------
> >   sound/x86/intel_hdmi_audio.h           |  3 +-
> >   4 files changed, 34 insertions(+), 42 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c
> > index a593fdf73171..270aa3e3f0e2 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->num_pipes = INTEL_INFO(dev_priv)->num_pipes;
> >   	spin_lock_init(&pdata->lpe_audio_slock);
> >     	platdev = platform_device_register_full(&pinfo);
> > @@ -318,7 +319,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_pipe_pdata *ppdata;
> >   	u32 audio_enable;
> > @@ -327,14 +328,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->pipe;
> > +	ppdata = &pdata->pipe[pipe];
> >   -	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));
> >   -	pdata->pipe_id = pipe;
> > -
> >   	if (eld != NULL) {
> >   		memcpy(ppdata->eld, eld, HDMI_MAX_ELD_BYTES);
> >   		ppdata->port = port;
> > @@ -356,8 +355,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, pipe);
> >   -	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 26e569ad8683..b391b2822140 100644
> > --- a/include/drm/intel_lpe_audio.h
> > +++ b/include/drm/intel_lpe_audio.h
> > @@ -39,10 +39,10 @@ struct intel_hdmi_lpe_audio_pipe_pdata {
> >   };
> >     struct intel_hdmi_lpe_audio_pdata {
> > -	struct intel_hdmi_lpe_audio_pipe_pdata pipe;
> > -	int pipe_id;
> > +	struct intel_hdmi_lpe_audio_pipe_pdata pipe[3];
> > +	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 5e2149fe5218..e5863a6d3aa9 100644
> > --- a/sound/x86/intel_hdmi_audio.c
> > +++ b/sound/x86/intel_hdmi_audio.c
> > @@ -195,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->card_ctx->mmio_start + ctx->had_config_offset + reg);
> > +	return ioread32(ctx->mmio_start + reg);
> >   }
> >     static void had_write_register_raw(struct snd_intelhad *ctx, u32
> > reg, u32 val)
> >   {
> > -	iowrite32(val, ctx->card_ctx->mmio_start + ctx->had_config_offset + reg);
> > +	iowrite32(val, ctx->mmio_start + reg);
> >   }
> >     static void had_read_register(struct snd_intelhad *ctx, u32 reg,
> > u32 *val)
> > @@ -1551,16 +1551,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 pipe)
> >   {
> >   	struct snd_intelhad_card *card_ctx = platform_get_drvdata(pdev);
> > -	int pipe;
> > -
> > -	for_each_pipe(card_ctx, pipe) {
> > -		struct snd_intelhad *ctx = &card_ctx->pcm_ctx[pipe];
> > +	struct snd_intelhad *ctx = &card_ctx->pcm_ctx[pipe];
> >   -		schedule_work(&ctx->hdmi_audio_wq);
> > -	}
> > +	schedule_work(&ctx->hdmi_audio_wq);
> >   }
> >     /* the work to handle monitor hot plug/unplug */
> > @@ -1569,7 +1565,7 @@ 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_pipe_pdata *ppdata = &pdata->pipe;
> > +	struct intel_hdmi_lpe_audio_pipe_pdata *ppdata = &pdata->pipe[ctx->pipe];
> >     	pm_runtime_get_sync(ctx->dev);
> >   	mutex_lock(&ctx->mutex);
> > @@ -1582,22 +1578,6 @@ static void had_audio_wq(struct work_struct *work)
> >   		dev_dbg(ctx->dev, "%s: HAD_NOTIFY_ELD : port = %d, tmds = %d\n",
> >   			__func__, ppdata->port, ppdata->ls_clock);
> >   -		switch (pdata->pipe_id) {
> > -		case 0:
> > -			ctx->had_config_offset = AUDIO_HDMI_CONFIG_A;
> > -			break;
> > -		case 1:
> > -			ctx->had_config_offset = AUDIO_HDMI_CONFIG_B;
> > -			break;
> > -		case 2:
> > -			ctx->had_config_offset = AUDIO_HDMI_CONFIG_C;
> > -			break;
> > -		default:
> > -			dev_dbg(ctx->dev, "Invalid pipe %d\n",
> > -				pdata->pipe_id);
> > -			break;
> > -		}
> > -
> >   		memcpy(ctx->eld, ppdata->eld, sizeof(ctx->eld));
> >     		ctx->dp_output = ppdata->dp_output;
> > @@ -1794,7 +1774,7 @@ static int hdmi_lpe_audio_probe(struct platform_device *pdev)
> >     	init_channel_allocations();
> >   -	card_ctx->num_pipes = 1;
> > +	card_ctx->num_pipes = pdata->num_pipes;
> >     	for_each_pipe(card_ctx, pipe) {
> >   		struct snd_intelhad *ctx = &card_ctx->pcm_ctx[pipe];
> > @@ -1806,9 +1786,24 @@ static int hdmi_lpe_audio_probe(struct platform_device *pdev)
> >     		INIT_WORK(&ctx->hdmi_audio_wq, had_audio_wq);
> >   -		ctx->had_config_offset = AUDIO_HDMI_CONFIG_A;
> > +		switch (pipe) {
> > +		case 0:
> > +			ctx->mmio_start =
> > +				card_ctx->mmio_start + AUDIO_HDMI_CONFIG_A;
> > +			break;
> > +		case 1:
> > +			ctx->mmio_start =
> > +				card_ctx->mmio_start + AUDIO_HDMI_CONFIG_B;
> > +			break;
> > +		case 2:
> > +			ctx->mmio_start =
> > +				card_ctx->mmio_start + AUDIO_HDMI_CONFIG_C;
> > +			break;
> > +		default:
> > +			break;
> > +		}
> >   -		ret = snd_pcm_new(card, INTEL_HAD, PCM_INDEX,
> > MAX_PB_STREAMS,
> > +		ret = snd_pcm_new(card, INTEL_HAD, pipe, 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 a209096b03df..ab0de5d525f4 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
> > @@ -124,7 +123,7 @@ struct snd_intelhad {
> >   	unsigned int period_bytes;	/* PCM period size in bytes */
> >     	/* internal stuff */
> > -	unsigned int had_config_offset;
> > +	void __iomem *mmio_start;
> >   	union aud_cfg aud_config;	/* AUD_CONFIG reg value cache */
> >   	struct work_struct hdmi_audio_wq;
> >   	struct mutex mutex; /* for protecting chmap and eld */
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [alsa-devel] [PATCH 11/11] ALSA: x86: Register multiple PCM devices for the LPE audio card
  2017-04-26  7:04     ` [alsa-devel] " Takashi Iwai
@ 2017-04-26  7:19       ` Takashi Iwai
  2017-04-26 13:49         ` Ville Syrjälä
  0 siblings, 1 reply; 28+ messages in thread
From: Takashi Iwai @ 2017-04-26  7:19 UTC (permalink / raw)
  To: Pierre-Louis Bossart; +Cc: intel-gfx, alsa-devel

On Wed, 26 Apr 2017 09:04:46 +0200,
Takashi Iwai wrote:
> 
> On Wed, 26 Apr 2017 03:58:57 +0200,
> Pierre-Louis Bossart wrote:
> > 
> > On 04/25/2017 03:27 PM, 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 pipe 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.
> > >
> > > The alternative would be to have a PCM device per port, but per-pipe
> > > is easier since the hardware actually works that way.
> > Very nice. I just tested on a CHT Zotac box which has two connectors
> > (1 HDMI and 1 DP), and I get sound concurrently on both, with hdmi
> > being listed as device 2 and DP as device 0.
> > I thought there were hardware restrictions but you proved me wrong. Kudos.
> > 
> > The only point that I find weird is that the jacks are reported as
> > 'on' on the 3 pipes, is there a way to tie them to an actual cable
> > being used?
> 
> The pdata check was changed to check port=-1 as the monitor off in the
> patch 6.  Maybe the initialization is missing?

I guess the problem is that the hotplug wq is called at the
initialization to retrieve the pdata for all pipes.  It's called with
uninitialized port=0, so all flags are on at init.

And it implies the potential problem: the pdata contains the
information only for a single pipe.  Maybe it should keep the
status/ELD for all three pipes.


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

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

* Re: [PATCH 00/11] drm/i915: LPE audio runtime PM and multipipe
  2017-04-25 20:27 [PATCH 00/11] drm/i915: LPE audio runtime PM and multipipe ville.syrjala
                   ` (11 preceding siblings ...)
  2017-04-25 20:46 ` ✓ Fi.CI.BAT: success for drm/i915: LPE audio runtime PM and multipipe Patchwork
@ 2017-04-26  7:29 ` Takashi Iwai
  2017-04-26 13:38   ` Ville Syrjälä
  12 siblings, 1 reply; 28+ messages in thread
From: Takashi Iwai @ 2017-04-26  7:29 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx, alsa-devel

On Tue, 25 Apr 2017 22:27:19 +0200,
ville.syrjala@linux.intel.com wrote:
> 
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> I was wondering why my VLV no longer runtime suspended, and after some
> thinking I decided it had to be the LPE audio preventing it. Turns out
> I was right, so here's my attempt at fixing it.
> 
> And while looking at the code I couldn't help but notice that it
> couldn't actually handle multiple pipes playing back audio at the
> same time. And even having multiple displays active even if only
> one was playing audio was probably a recipe for failure. So I
> tried to fix that by registering a separate PCM device for each
> pipe.
> 
> Note that the patch subjects may not reflect the subsystem
> very well since most of these straddle the border between drm
> and alsa. I think I just slapped on drm/i915 to most where
> there was no clear winner.

A nice patchset, thanks for working on it!

One slight concern (other than the jack issue Pierre reported) is the
incompatible behavior from the current version.  With the pipe-based
multiple streams, user would need to choose another one even if the
device has a single HDMI output, which is pretty common on BYT/CHV
tablets.

Maybe it's no big problem as the users are still limited at the
moment.  Or, we may need to handle a bit differently, e.g. assigning
the PCM stream dynamically per hotplug.

In anyway, with the support of multi streams, alsa-lib config needs to
be updated.


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

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

* Re: [PATCH 03/11] drm/i915: Stop pretending to mask/unmask LPE audio interrupts
  2017-04-26  0:27   ` Pierre-Louis Bossart
@ 2017-04-26 13:27     ` Ville Syrjälä
  0 siblings, 0 replies; 28+ messages in thread
From: Ville Syrjälä @ 2017-04-26 13:27 UTC (permalink / raw)
  To: Pierre-Louis Bossart; +Cc: Takashi Iwai, intel-gfx, alsa-devel

On Tue, Apr 25, 2017 at 07:27:32PM -0500, Pierre-Louis Bossart wrote:
> On 4/25/17 3:27 PM, ville.syrjala@linux.intel.com wrote:
> > 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.
> 
> No objections, I assumed that we did want to update VLV_IMR and VLV_IIR 
> in the mask/unmask, that was the initial recommendation IIRC
> 
> There was also a comment where we removed all tests in 
> vlv_display_irq_postinstall:
> 
>  >> +		if (IS_LPE_AUDIO_ENABLED(dev_priv))
>  >> +			if (IS_LPE_AUDIO_IRQ_VALID(dev_priv))
>  >
>  >I think both of these checks can be removed. We won't unmask the
>  >interrupts unless lpe is enabled, so the IIR bits will never be set in
>  >that case.

Hmm. Right, so I guess the idea originally was to just enable the
LPE interrupts in IER, but leave them masked in IMR until the
irqchip unmasks them. But the code wasn't actually doing that
because it was appending the bits to 'enable_mask' before it
assigned the value to irq_mask. Hence the interrupts were always
enabled and unmasked.

> 
> 
> 
> 
> >
> > 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);
> >

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

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

* Re: [alsa-devel] [PATCH 05/11] drm/i915: Replace tmds_clock_speed and link_rate with just ls_clock
  2017-04-26  1:09   ` [alsa-devel] " Pierre-Louis Bossart
@ 2017-04-26 13:28     ` Ville Syrjälä
  0 siblings, 0 replies; 28+ messages in thread
From: Ville Syrjälä @ 2017-04-26 13:28 UTC (permalink / raw)
  To: Pierre-Louis Bossart; +Cc: Takashi Iwai, intel-gfx, alsa-devel

On Tue, Apr 25, 2017 at 08:09:34PM -0500, Pierre-Louis Bossart wrote:
> On 4/25/17 3:27 PM, ville.syrjala@linux.intel.com wrote:
> > 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.
> 
> there are still occurences of 'tmds' in sound/x86 and there are are 
> couple of debug messages that don't make sense any longer.

I was being a bit lazy and didn't remove the tmds vs. link_rate
distinction from the audio driver side. Not quite sure if we
want to do it not.

> 
> >
> > 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 357b6c6c2f04..876eee56a958 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);
> >
> >

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

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

* Re: [PATCH 00/11] drm/i915: LPE audio runtime PM and multipipe
  2017-04-26  7:29 ` [PATCH 00/11] " Takashi Iwai
@ 2017-04-26 13:38   ` Ville Syrjälä
  2017-04-26 13:47     ` Takashi Iwai
  0 siblings, 1 reply; 28+ messages in thread
From: Ville Syrjälä @ 2017-04-26 13:38 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: intel-gfx, alsa-devel

On Wed, Apr 26, 2017 at 09:29:18AM +0200, Takashi Iwai wrote:
> On Tue, 25 Apr 2017 22:27:19 +0200,
> ville.syrjala@linux.intel.com wrote:
> > 
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > I was wondering why my VLV no longer runtime suspended, and after some
> > thinking I decided it had to be the LPE audio preventing it. Turns out
> > I was right, so here's my attempt at fixing it.
> > 
> > And while looking at the code I couldn't help but notice that it
> > couldn't actually handle multiple pipes playing back audio at the
> > same time. And even having multiple displays active even if only
> > one was playing audio was probably a recipe for failure. So I
> > tried to fix that by registering a separate PCM device for each
> > pipe.
> > 
> > Note that the patch subjects may not reflect the subsystem
> > very well since most of these straddle the border between drm
> > and alsa. I think I just slapped on drm/i915 to most where
> > there was no clear winner.
> 
> A nice patchset, thanks for working on it!
> 
> One slight concern (other than the jack issue Pierre reported) is the
> incompatible behavior from the current version.  With the pipe-based
> multiple streams, user would need to choose another one even if the
> device has a single HDMI output, which is pretty common on BYT/CHV
> tablets.
> 
> Maybe it's no big problem as the users are still limited at the
> moment.  Or, we may need to handle a bit differently, e.g. assigning
> the PCM stream dynamically per hotplug.

Yeah, I tied the PCM device to the pipe to match the hardware. But we
could certainly register the PCM device per port, and then do a 
pipe<->port mapping somewhere to make it all work out. One slight
complication is that not all ports are necessarily present so we
might have eg. just port B and port D, but no port C. Not sure if
it would be a bad thing to register a PCM device even for the
missing ports anyway?

I don't recall which way HDA works. Device per port I guess? Well,
for DP SST/HDMI. But for DP MST I presume it's device per stream
(ie. pipe) even with HDA.

> In anyway, with the support of multi streams, alsa-lib config needs to
> be updated.

Hmm. I suppose I'll have to take a gander at what's there.

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

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

* Re: [PATCH 00/11] drm/i915: LPE audio runtime PM and multipipe
  2017-04-26 13:38   ` Ville Syrjälä
@ 2017-04-26 13:47     ` Takashi Iwai
  2017-04-26 13:56       ` Ville Syrjälä
  2017-04-26 19:59       ` Ville Syrjälä
  0 siblings, 2 replies; 28+ messages in thread
From: Takashi Iwai @ 2017-04-26 13:47 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx, alsa-devel

On Wed, 26 Apr 2017 15:38:53 +0200,
Ville Syrjälä wrote:
> 
> On Wed, Apr 26, 2017 at 09:29:18AM +0200, Takashi Iwai wrote:
> > On Tue, 25 Apr 2017 22:27:19 +0200,
> > ville.syrjala@linux.intel.com wrote:
> > > 
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > I was wondering why my VLV no longer runtime suspended, and after some
> > > thinking I decided it had to be the LPE audio preventing it. Turns out
> > > I was right, so here's my attempt at fixing it.
> > > 
> > > And while looking at the code I couldn't help but notice that it
> > > couldn't actually handle multiple pipes playing back audio at the
> > > same time. And even having multiple displays active even if only
> > > one was playing audio was probably a recipe for failure. So I
> > > tried to fix that by registering a separate PCM device for each
> > > pipe.
> > > 
> > > Note that the patch subjects may not reflect the subsystem
> > > very well since most of these straddle the border between drm
> > > and alsa. I think I just slapped on drm/i915 to most where
> > > there was no clear winner.
> > 
> > A nice patchset, thanks for working on it!
> > 
> > One slight concern (other than the jack issue Pierre reported) is the
> > incompatible behavior from the current version.  With the pipe-based
> > multiple streams, user would need to choose another one even if the
> > device has a single HDMI output, which is pretty common on BYT/CHV
> > tablets.
> > 
> > Maybe it's no big problem as the users are still limited at the
> > moment.  Or, we may need to handle a bit differently, e.g. assigning
> > the PCM stream dynamically per hotplug.
> 
> Yeah, I tied the PCM device to the pipe to match the hardware. But we
> could certainly register the PCM device per port, and then do a 
> pipe<->port mapping somewhere to make it all work out. One slight
> complication is that not all ports are necessarily present so we
> might have eg. just port B and port D, but no port C. Not sure if
> it would be a bad thing to register a PCM device even for the
> missing ports anyway?
> 
> I don't recall which way HDA works. Device per port I guess? Well,
> for DP SST/HDMI. But for DP MST I presume it's device per stream
> (ie. pipe) even with HDA.

HD-audio creates the PCM device per HD-audio widget representation,
and they correspond to ports, as it seems.  For MST, the situation is
more complex, and we assign the PCM stream dynamically.  It's for
keeping the behavior (more or less) consistent with non-MST.

> > In anyway, with the support of multi streams, alsa-lib config needs to
> > be updated.
> 
> Hmm. I suppose I'll have to take a gander at what's there.

The HDMI PCM definition itself is easy, just just adding more (hdmi.1,
hdmi.2, etc).  The difficult part would be the front and the default
PCM definition, and I have no good idea about it, so far.


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

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

* Re: [alsa-devel] [PATCH 11/11] ALSA: x86: Register multiple PCM devices for the LPE audio card
  2017-04-26  7:19       ` Takashi Iwai
@ 2017-04-26 13:49         ` Ville Syrjälä
  2017-04-26 14:04           ` Takashi Iwai
  0 siblings, 1 reply; 28+ messages in thread
From: Ville Syrjälä @ 2017-04-26 13:49 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: intel-gfx, alsa-devel

On Wed, Apr 26, 2017 at 09:19:21AM +0200, Takashi Iwai wrote:
> On Wed, 26 Apr 2017 09:04:46 +0200,
> Takashi Iwai wrote:
> > 
> > On Wed, 26 Apr 2017 03:58:57 +0200,
> > Pierre-Louis Bossart wrote:
> > > 
> > > On 04/25/2017 03:27 PM, 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 pipe 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.
> > > >
> > > > The alternative would be to have a PCM device per port, but per-pipe
> > > > is easier since the hardware actually works that way.
> > > Very nice. I just tested on a CHT Zotac box which has two connectors
> > > (1 HDMI and 1 DP), and I get sound concurrently on both, with hdmi
> > > being listed as device 2 and DP as device 0.
> > > I thought there were hardware restrictions but you proved me wrong. Kudos.
> > > 
> > > The only point that I find weird is that the jacks are reported as
> > > 'on' on the 3 pipes, is there a way to tie them to an actual cable
> > > being used?
> > 
> > The pdata check was changed to check port=-1 as the monitor off in the
> > patch 6.  Maybe the initialization is missing?
> 
> I guess the problem is that the hotplug wq is called at the
> initialization to retrieve the pdata for all pipes.  It's called with
> uninitialized port=0, so all flags are on at init.

Indeed. That will need to be fixed.

> 
> And it implies the potential problem: the pdata contains the
> information only for a single pipe.  Maybe it should keep the
> status/ELD for all three pipes.

I already did that.

That being said, the entire notify vs. wq is pretty racy. So I was
thinking that maybe we could just eliminate the wq and do whatever is
needed directly from the notify hook. And then we could eliminate the
(ab)use of pdata to transfer the ELD and whatnot between the drivers.
I guess the main complication is the driver load case. I didn't
really think that one through so far.

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

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

* Re: [PATCH 00/11] drm/i915: LPE audio runtime PM and multipipe
  2017-04-26 13:47     ` Takashi Iwai
@ 2017-04-26 13:56       ` Ville Syrjälä
  2017-04-26 19:59       ` Ville Syrjälä
  1 sibling, 0 replies; 28+ messages in thread
From: Ville Syrjälä @ 2017-04-26 13:56 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: intel-gfx, alsa-devel

On Wed, Apr 26, 2017 at 03:47:44PM +0200, Takashi Iwai wrote:
> On Wed, 26 Apr 2017 15:38:53 +0200,
> Ville Syrjälä wrote:
> > 
> > On Wed, Apr 26, 2017 at 09:29:18AM +0200, Takashi Iwai wrote:
> > > On Tue, 25 Apr 2017 22:27:19 +0200,
> > > ville.syrjala@linux.intel.com wrote:
> > > > 
> > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > 
> > > > I was wondering why my VLV no longer runtime suspended, and after some
> > > > thinking I decided it had to be the LPE audio preventing it. Turns out
> > > > I was right, so here's my attempt at fixing it.
> > > > 
> > > > And while looking at the code I couldn't help but notice that it
> > > > couldn't actually handle multiple pipes playing back audio at the
> > > > same time. And even having multiple displays active even if only
> > > > one was playing audio was probably a recipe for failure. So I
> > > > tried to fix that by registering a separate PCM device for each
> > > > pipe.
> > > > 
> > > > Note that the patch subjects may not reflect the subsystem
> > > > very well since most of these straddle the border between drm
> > > > and alsa. I think I just slapped on drm/i915 to most where
> > > > there was no clear winner.
> > > 
> > > A nice patchset, thanks for working on it!
> > > 
> > > One slight concern (other than the jack issue Pierre reported) is the
> > > incompatible behavior from the current version.  With the pipe-based
> > > multiple streams, user would need to choose another one even if the
> > > device has a single HDMI output, which is pretty common on BYT/CHV
> > > tablets.
> > > 
> > > Maybe it's no big problem as the users are still limited at the
> > > moment.  Or, we may need to handle a bit differently, e.g. assigning
> > > the PCM stream dynamically per hotplug.
> > 
> > Yeah, I tied the PCM device to the pipe to match the hardware. But we
> > could certainly register the PCM device per port, and then do a 
> > pipe<->port mapping somewhere to make it all work out. One slight
> > complication is that not all ports are necessarily present so we
> > might have eg. just port B and port D, but no port C. Not sure if
> > it would be a bad thing to register a PCM device even for the
> > missing ports anyway?
> > 
> > I don't recall which way HDA works. Device per port I guess? Well,
> > for DP SST/HDMI. But for DP MST I presume it's device per stream
> > (ie. pipe) even with HDA.
> 
> HD-audio creates the PCM device per HD-audio widget representation,
> and they correspond to ports, as it seems.

With the slight exception of old stuff like ctg/elk where there is just
one audio device IIRC, and that one gets fed into whichever port enables
audio, and if both enable it it goes to /dev/null.

> For MST, the situation is
> more complex, and we assign the PCM stream dynamically.  It's for
> keeping the behavior (more or less) consistent with non-MST.
> 
> > > In anyway, with the support of multi streams, alsa-lib config needs to
> > > be updated.
> > 
> > Hmm. I suppose I'll have to take a gander at what's there.
> 
> The HDMI PCM definition itself is easy, just just adding more (hdmi.1,
> hdmi.2, etc).  The difficult part would be the front and the default
> PCM definition, and I have no good idea about it, so far.
> 
> 
> Takashi

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

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

* Re: [alsa-devel] [PATCH 11/11] ALSA: x86: Register multiple PCM devices for the LPE audio card
  2017-04-26 13:49         ` Ville Syrjälä
@ 2017-04-26 14:04           ` Takashi Iwai
  0 siblings, 0 replies; 28+ messages in thread
From: Takashi Iwai @ 2017-04-26 14:04 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx, alsa-devel

On Wed, 26 Apr 2017 15:49:06 +0200,
Ville Syrjälä wrote:
> 
> On Wed, Apr 26, 2017 at 09:19:21AM +0200, Takashi Iwai wrote:
> > On Wed, 26 Apr 2017 09:04:46 +0200,
> > Takashi Iwai wrote:
> > > 
> > > On Wed, 26 Apr 2017 03:58:57 +0200,
> > > Pierre-Louis Bossart wrote:
> > > > 
> > > > On 04/25/2017 03:27 PM, 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 pipe 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.
> > > > >
> > > > > The alternative would be to have a PCM device per port, but per-pipe
> > > > > is easier since the hardware actually works that way.
> > > > Very nice. I just tested on a CHT Zotac box which has two connectors
> > > > (1 HDMI and 1 DP), and I get sound concurrently on both, with hdmi
> > > > being listed as device 2 and DP as device 0.
> > > > I thought there were hardware restrictions but you proved me wrong. Kudos.
> > > > 
> > > > The only point that I find weird is that the jacks are reported as
> > > > 'on' on the 3 pipes, is there a way to tie them to an actual cable
> > > > being used?
> > > 
> > > The pdata check was changed to check port=-1 as the monitor off in the
> > > patch 6.  Maybe the initialization is missing?
> > 
> > I guess the problem is that the hotplug wq is called at the
> > initialization to retrieve the pdata for all pipes.  It's called with
> > uninitialized port=0, so all flags are on at init.
> 
> Indeed. That will need to be fixed.
> 
> > 
> > And it implies the potential problem: the pdata contains the
> > information only for a single pipe.  Maybe it should keep the
> > status/ELD for all three pipes.
> 
> I already did that.

Oh then it' fine, I just didn't find it.

> That being said, the entire notify vs. wq is pretty racy. So I was
> thinking that maybe we could just eliminate the wq and do whatever is
> needed directly from the notify hook. And then we could eliminate the
> (ab)use of pdata to transfer the ELD and whatnot between the drivers.
> I guess the main complication is the driver load case. I didn't
> really think that one through so far.

One way would be to implement the get_eld() to call at the probe
time.  HD-audio calls such one (provided via component) at probe and
resume time to sync with the actual h/w state.  For LPE audio, it's
even easier, as we may call i915 exported function directly.


thanks,

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

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

* Re: [PATCH 00/11] drm/i915: LPE audio runtime PM and multipipe
  2017-04-26 13:47     ` Takashi Iwai
  2017-04-26 13:56       ` Ville Syrjälä
@ 2017-04-26 19:59       ` Ville Syrjälä
  1 sibling, 0 replies; 28+ messages in thread
From: Ville Syrjälä @ 2017-04-26 19:59 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: intel-gfx, alsa-devel

On Wed, Apr 26, 2017 at 03:47:44PM +0200, Takashi Iwai wrote:
> On Wed, 26 Apr 2017 15:38:53 +0200,
> Ville Syrjälä wrote:
> > 
> > On Wed, Apr 26, 2017 at 09:29:18AM +0200, Takashi Iwai wrote:
> > > On Tue, 25 Apr 2017 22:27:19 +0200,
> > > ville.syrjala@linux.intel.com wrote:
> > > > 
> > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > 
> > > > I was wondering why my VLV no longer runtime suspended, and after some
> > > > thinking I decided it had to be the LPE audio preventing it. Turns out
> > > > I was right, so here's my attempt at fixing it.
> > > > 
> > > > And while looking at the code I couldn't help but notice that it
> > > > couldn't actually handle multiple pipes playing back audio at the
> > > > same time. And even having multiple displays active even if only
> > > > one was playing audio was probably a recipe for failure. So I
> > > > tried to fix that by registering a separate PCM device for each
> > > > pipe.
> > > > 
> > > > Note that the patch subjects may not reflect the subsystem
> > > > very well since most of these straddle the border between drm
> > > > and alsa. I think I just slapped on drm/i915 to most where
> > > > there was no clear winner.
> > > 
> > > A nice patchset, thanks for working on it!
> > > 
> > > One slight concern (other than the jack issue Pierre reported) is the
> > > incompatible behavior from the current version.  With the pipe-based
> > > multiple streams, user would need to choose another one even if the
> > > device has a single HDMI output, which is pretty common on BYT/CHV
> > > tablets.
> > > 
> > > Maybe it's no big problem as the users are still limited at the
> > > moment.  Or, we may need to handle a bit differently, e.g. assigning
> > > the PCM stream dynamically per hotplug.
> > 
> > Yeah, I tied the PCM device to the pipe to match the hardware. But we
> > could certainly register the PCM device per port, and then do a 
> > pipe<->port mapping somewhere to make it all work out. One slight
> > complication is that not all ports are necessarily present so we
> > might have eg. just port B and port D, but no port C. Not sure if
> > it would be a bad thing to register a PCM device even for the
> > missing ports anyway?
> > 
> > I don't recall which way HDA works. Device per port I guess? Well,
> > for DP SST/HDMI. But for DP MST I presume it's device per stream
> > (ie. pipe) even with HDA.
> 
> HD-audio creates the PCM device per HD-audio widget representation,
> and they correspond to ports, as it seems.  For MST, the situation is
> more complex, and we assign the PCM stream dynamically.  It's for
> keeping the behavior (more or less) consistent with non-MST.

OK, I hacked together something that seems to work. I'll still need
to squash it with the earlier patch, but in case you want to see what
the relative changes look like I pushed it to
git://github.com/vsyrjala/linux.git lpe_audio_multipipe

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

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

* [PATCH 07/11] drm/i915: Reorganize intel_lpe_audio_notify() arguments
  2017-04-27 16:02 [PATCH v2 00/11] drm/i915: LPE audio runtime PM and multipipe (v2) ville.syrjala
@ 2017-04-27 16:02 ` ville.syrjala
  0 siblings, 0 replies; 28+ messages in thread
From: ville.syrjala @ 2017-04-27 16:02 UTC (permalink / raw)
  To: intel-gfx; +Cc: Takashi Iwai, alsa-devel

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

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

end of thread, other threads:[~2017-04-27 16:02 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-25 20:27 [PATCH 00/11] drm/i915: LPE audio runtime PM and multipipe ville.syrjala
2017-04-25 20:27 ` [PATCH 01/11] drm/i915: Fix runtime PM for LPE audio ville.syrjala
2017-04-25 20:27 ` [PATCH 02/11] ALSA: x86: Clear the pdata.notify_lpe_audio pointer before teardown ville.syrjala
2017-04-25 20:27 ` [PATCH 03/11] drm/i915: Stop pretending to mask/unmask LPE audio interrupts ville.syrjala
2017-04-26  0:27   ` Pierre-Louis Bossart
2017-04-26 13:27     ` Ville Syrjälä
2017-04-25 20:27 ` [PATCH 04/11] drm/i915: Remove the unsued pending_notify from LPE platform data ville.syrjala
2017-04-25 20:27 ` [PATCH 05/11] drm/i915: Replace tmds_clock_speed and link_rate with just ls_clock ville.syrjala
2017-04-26  1:09   ` [alsa-devel] " Pierre-Louis Bossart
2017-04-26 13:28     ` Ville Syrjälä
2017-04-25 20:27 ` [PATCH 06/11] drm/i915: Remove hdmi_connected from LPE audio pdata ville.syrjala
2017-04-25 20:27 ` [PATCH 07/11] drm/i915: Reorganize intel_lpe_audio_notify() arguments ville.syrjala
2017-04-25 20:27 ` [PATCH 08/11] drm/i915: Clean up the LPE audio platform data ville.syrjala
2017-04-25 20:27 ` [PATCH 09/11] ALSA: x86: Prepare LPE audio ctls for multiple PCMs ville.syrjala
2017-04-25 20:27 ` [PATCH 10/11] ALSA: x86: Split snd_intelhad into card and PCM specific structures ville.syrjala
2017-04-25 20:27 ` [PATCH 11/11] ALSA: x86: Register multiple PCM devices for the LPE audio card ville.syrjala
2017-04-26  1:58   ` Pierre-Louis Bossart
2017-04-26  7:04     ` [alsa-devel] " Takashi Iwai
2017-04-26  7:19       ` Takashi Iwai
2017-04-26 13:49         ` Ville Syrjälä
2017-04-26 14:04           ` Takashi Iwai
2017-04-25 20:46 ` ✓ Fi.CI.BAT: success for drm/i915: LPE audio runtime PM and multipipe Patchwork
2017-04-26  7:29 ` [PATCH 00/11] " Takashi Iwai
2017-04-26 13:38   ` Ville Syrjälä
2017-04-26 13:47     ` Takashi Iwai
2017-04-26 13:56       ` Ville Syrjälä
2017-04-26 19:59       ` Ville Syrjälä
2017-04-27 16:02 [PATCH v2 00/11] drm/i915: LPE audio runtime PM and multipipe (v2) ville.syrjala
2017-04-27 16:02 ` [PATCH 07/11] drm/i915: Reorganize intel_lpe_audio_notify() arguments ville.syrjala

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.