* [PATCH 01/11] drm/i915: Fix runtime PM for LPE audio
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
2017-04-28 8:15 ` Takashi Iwai
2017-04-27 16:02 ` [PATCH 02/11] ALSA: x86: Clear the pdata.notify_lpe_audio pointer before teardown ville.syrjala
` (12 subsequent siblings)
13 siblings, 1 reply; 27+ 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>
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] 27+ messages in thread* Re: [PATCH 01/11] drm/i915: Fix runtime PM for LPE audio
2017-04-27 16:02 ` [PATCH 01/11] drm/i915: Fix runtime PM for LPE audio ville.syrjala
@ 2017-04-28 8:15 ` Takashi Iwai
0 siblings, 0 replies; 27+ messages in thread
From: Takashi Iwai @ 2017-04-28 8:15 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx, alsa-devel
On Thu, 27 Apr 2017 18:02:20 +0200,
ville.syrjala@linux.intel.com wrote:
>
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Not calling pm_runtime_enable() means that runtime PM can't be
> enabled at all via sysfs. So we definitely need to call it
> from somewhere.
>
> Calling it from the driver seems like a bad idea because it
> would have to be paired with a pm_runtime_disable() at driver
> unload time, otherwise the core gets upset. Also if there's
> no LPE audio driver loaded then we couldn't runtime suspend
> i915 either.
>
> So it looks like a better plan is to call it from i915 when
> we register the platform device. That seems to match how
> pci generally does things. I cargo culted the
> pm_runtime_forbid() and pm_runtime_set_active() calls from
> pci as well.
>
> The exposed runtime PM API is massive an thorougly misleading, so
> I don't actually know if this is how you're supposed to use the API
> or not. But it seems to work. I can now runtime suspend i915 again
> with or without the LPE audio driver loaded, and reloading the
> LPE audio driver also seems to work.
>
> Note that powertop won't auto-tune runtime PM for platform devices,
> which is a little annoying. So I'm not sure that leaving runtime
> PM in "on" mode by default is the best choice here. But I've left
> it like that for now at least.
The reason I didn't proactively turn on the runtime PM was that it
often caused a few seconds of pause to the A/V receivers before
actually starting playing.
There is a planned feature to keep sending the silent stream even
after stopping the stream, but it's not implemented yet.
> Also remove the comment about there not being much benefit from
> LPE audio runtime PM. Not allowing runtime PM blocks i915 runtime
> PM, which will also block s0ix, and that could have a measurable
> impact on power consumption.
>
> Cc: Takashi Iwai <tiwai@suse.de>
> Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> Fixes: 0b6b524f3915 ("ALSA: x86: Don't enable runtime PM as default")
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Takashi Iwai <tiwai@suse.de>
IMO, this should be tagged with Cc to stable.
thanks,
Takashi
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 02/11] ALSA: x86: Clear the pdata.notify_lpe_audio pointer before teardown
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 01/11] drm/i915: Fix runtime PM for LPE audio ville.syrjala
@ 2017-04-27 16:02 ` ville.syrjala
2017-04-28 8:16 ` Takashi Iwai
2017-04-27 16:02 ` [PATCH 03/11] drm/i915: Stop pretending to mask/unmask LPE audio interrupts ville.syrjala
` (11 subsequent siblings)
13 siblings, 1 reply; 27+ 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>
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] 27+ messages in thread* [PATCH 03/11] drm/i915: Stop pretending to mask/unmask LPE audio interrupts
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 01/11] drm/i915: Fix runtime PM for LPE audio ville.syrjala
2017-04-27 16:02 ` [PATCH 02/11] ALSA: x86: Clear the pdata.notify_lpe_audio pointer before teardown ville.syrjala
@ 2017-04-27 16:02 ` ville.syrjala
2017-04-27 16:02 ` [PATCH v2 04/11] drm/i915: Remove the unused pending_notify from LPE platform data ville.syrjala
` (10 subsequent siblings)
13 siblings, 0 replies; 27+ 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>
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] 27+ messages in thread* [PATCH v2 04/11] drm/i915: Remove the unused pending_notify from LPE platform data
2017-04-27 16:02 [PATCH v2 00/11] drm/i915: LPE audio runtime PM and multipipe (v2) ville.syrjala
` (2 preceding siblings ...)
2017-04-27 16:02 ` [PATCH 03/11] drm/i915: Stop pretending to mask/unmask LPE audio interrupts ville.syrjala
@ 2017-04-27 16:02 ` ville.syrjala
2017-04-27 16:02 ` [PATCH 05/11] drm/i915: Replace tmds_clock_speed and link_rate with just ls_clock ville.syrjala
` (9 subsequent siblings)
13 siblings, 0 replies; 27+ 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>
The pending_notify flag in the LPE audio platform data is pointless,
actually unused. So let's kill it off.
v2: Fix typo in patch subject
Cc: Takashi Iwai <tiwai@suse.de>
Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/intel_lpe_audio.c | 2 --
include/drm/intel_lpe_audio.h | 1 -
sound/x86/intel_hdmi_audio.c | 1 -
3 files changed, 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c
index 292fedf30b00..79b9dca985ff 100644
--- a/drivers/gpu/drm/i915/intel_lpe_audio.c
+++ b/drivers/gpu/drm/i915/intel_lpe_audio.c
@@ -361,8 +361,6 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv,
if (pdata->notify_audio_lpe)
pdata->notify_audio_lpe(dev_priv->lpe_audio.platdev);
- else
- pdata->notify_pending = true;
spin_unlock_irqrestore(&pdata->lpe_audio_slock,
irq_flags);
diff --git a/include/drm/intel_lpe_audio.h b/include/drm/intel_lpe_audio.h
index e9892b4c3af1..c201d39cdfea 100644
--- a/include/drm/intel_lpe_audio.h
+++ b/include/drm/intel_lpe_audio.h
@@ -38,7 +38,6 @@ struct intel_hdmi_lpe_audio_eld {
};
struct intel_hdmi_lpe_audio_pdata {
- bool notify_pending;
int tmds_clock_speed;
bool hdmi_connected;
bool dp_output;
diff --git a/sound/x86/intel_hdmi_audio.c b/sound/x86/intel_hdmi_audio.c
index 5b89662493c9..cbba4a78afb5 100644
--- a/sound/x86/intel_hdmi_audio.c
+++ b/sound/x86/intel_hdmi_audio.c
@@ -1811,7 +1811,6 @@ static int hdmi_lpe_audio_probe(struct platform_device *pdev)
spin_lock_irq(&pdata->lpe_audio_slock);
pdata->notify_audio_lpe = notify_audio_lpe;
- pdata->notify_pending = false;
spin_unlock_irq(&pdata->lpe_audio_slock);
pm_runtime_use_autosuspend(&pdev->dev);
--
2.10.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH 05/11] drm/i915: Replace tmds_clock_speed and link_rate with just ls_clock
2017-04-27 16:02 [PATCH v2 00/11] drm/i915: LPE audio runtime PM and multipipe (v2) ville.syrjala
` (3 preceding siblings ...)
2017-04-27 16:02 ` [PATCH v2 04/11] drm/i915: Remove the unused pending_notify from LPE platform data ville.syrjala
@ 2017-04-27 16:02 ` ville.syrjala
2017-04-27 16:02 ` [PATCH v2 06/11] drm/i915: Remove hdmi_connected from LPE audio pdata ville.syrjala
` (8 subsequent siblings)
13 siblings, 0 replies; 27+ 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>
There's no need to distinguish between the DP link rate and HDMI TMDS
clock for the purposes of the LPE audio. Both are actually the same
thing more or less, which is the link symbol clock. So let's just
call the thing ls_clock and simplify the code.
Cc: Takashi Iwai <tiwai@suse.de>
Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 4 ++--
drivers/gpu/drm/i915/intel_audio.c | 19 ++++---------------
drivers/gpu/drm/i915/intel_lpe_audio.c | 14 ++++++--------
include/drm/intel_lpe_audio.h | 3 +--
sound/x86/intel_hdmi_audio.c | 11 ++++++++---
5 files changed, 21 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index d1f7c48e4ae3..8bf72220ee07 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3721,8 +3721,8 @@ int intel_lpe_audio_init(struct drm_i915_private *dev_priv);
void intel_lpe_audio_teardown(struct drm_i915_private *dev_priv);
void intel_lpe_audio_irq_handler(struct drm_i915_private *dev_priv);
void intel_lpe_audio_notify(struct drm_i915_private *dev_priv,
- void *eld, int port, int pipe, int tmds_clk_speed,
- bool dp_output, int link_rate);
+ void *eld, int port, int pipe, int ls_clock,
+ bool dp_output);
/* intel_i2c.c */
extern int intel_setup_gmbus(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
index 52c207e81f41..79eeef25321f 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -632,20 +632,9 @@ void intel_audio_codec_enable(struct intel_encoder *intel_encoder,
(int) port, (int) pipe);
}
- switch (intel_encoder->type) {
- case INTEL_OUTPUT_HDMI:
- intel_lpe_audio_notify(dev_priv, connector->eld, port, pipe,
- crtc_state->port_clock,
- false, 0);
- break;
- case INTEL_OUTPUT_DP:
- intel_lpe_audio_notify(dev_priv, connector->eld, port, pipe,
- adjusted_mode->crtc_clock,
- true, crtc_state->port_clock);
- break;
- default:
- break;
- }
+ intel_lpe_audio_notify(dev_priv, connector->eld, port, pipe,
+ crtc_state->port_clock,
+ intel_encoder->type == INTEL_OUTPUT_DP);
}
/**
@@ -680,7 +669,7 @@ void intel_audio_codec_disable(struct intel_encoder *intel_encoder)
(int) port, (int) pipe);
}
- intel_lpe_audio_notify(dev_priv, NULL, port, pipe, 0, false, 0);
+ intel_lpe_audio_notify(dev_priv, NULL, port, pipe, 0, false);
}
/**
diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c
index 79b9dca985ff..5a1a37e963f1 100644
--- a/drivers/gpu/drm/i915/intel_lpe_audio.c
+++ b/drivers/gpu/drm/i915/intel_lpe_audio.c
@@ -309,13 +309,14 @@ void intel_lpe_audio_teardown(struct drm_i915_private *dev_priv)
* @eld : ELD data
* @pipe: pipe id
* @port: port id
- * @tmds_clk_speed: tmds clock frequency in Hz
+ * @ls_clock: Link symbol clock in kHz
+ * @dp_output: Driving a DP output?
*
* Notify lpe audio driver of eld change.
*/
void intel_lpe_audio_notify(struct drm_i915_private *dev_priv,
- void *eld, int port, int pipe, int tmds_clk_speed,
- bool dp_output, int link_rate)
+ void *eld, int port, int pipe, int ls_clock,
+ bool dp_output)
{
unsigned long irq_flags;
struct intel_hdmi_lpe_audio_pdata *pdata = NULL;
@@ -337,12 +338,8 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv,
pdata->eld.port_id = port;
pdata->eld.pipe_id = pipe;
pdata->hdmi_connected = true;
-
+ pdata->ls_clock = ls_clock;
pdata->dp_output = dp_output;
- if (tmds_clk_speed)
- pdata->tmds_clock_speed = tmds_clk_speed;
- if (link_rate)
- pdata->link_rate = link_rate;
/* Unmute the amp for both DP and HDMI */
I915_WRITE(VLV_AUD_PORT_EN_DBG(port),
@@ -352,6 +349,7 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv,
memset(pdata->eld.eld_data, 0,
HDMI_MAX_ELD_BYTES);
pdata->hdmi_connected = false;
+ pdata->ls_clock = 0;
pdata->dp_output = false;
/* Mute the amp for both DP and HDMI */
diff --git a/include/drm/intel_lpe_audio.h b/include/drm/intel_lpe_audio.h
index c201d39cdfea..8bf804ce8905 100644
--- a/include/drm/intel_lpe_audio.h
+++ b/include/drm/intel_lpe_audio.h
@@ -38,10 +38,9 @@ struct intel_hdmi_lpe_audio_eld {
};
struct intel_hdmi_lpe_audio_pdata {
- int tmds_clock_speed;
+ int ls_clock;
bool hdmi_connected;
bool dp_output;
- int link_rate;
struct intel_hdmi_lpe_audio_eld eld;
void (*notify_audio_lpe)(struct platform_device *pdev);
spinlock_t lpe_audio_slock;
diff --git a/sound/x86/intel_hdmi_audio.c b/sound/x86/intel_hdmi_audio.c
index cbba4a78afb5..4eaf5de54f61 100644
--- a/sound/x86/intel_hdmi_audio.c
+++ b/sound/x86/intel_hdmi_audio.c
@@ -1568,7 +1568,7 @@ static void had_audio_wq(struct work_struct *work)
struct intel_hdmi_lpe_audio_eld *eld = &pdata->eld;
dev_dbg(ctx->dev, "%s: HAD_NOTIFY_ELD : port = %d, tmds = %d\n",
- __func__, eld->port_id, pdata->tmds_clock_speed);
+ __func__, eld->port_id, pdata->ls_clock);
switch (eld->pipe_id) {
case 0:
@@ -1589,8 +1589,13 @@ static void had_audio_wq(struct work_struct *work)
memcpy(ctx->eld, eld->eld_data, sizeof(ctx->eld));
ctx->dp_output = pdata->dp_output;
- ctx->tmds_clock_speed = pdata->tmds_clock_speed;
- ctx->link_rate = pdata->link_rate;
+ if (ctx->dp_output) {
+ ctx->tmds_clock_speed = 0;
+ ctx->link_rate = pdata->ls_clock;
+ } else {
+ ctx->tmds_clock_speed = pdata->ls_clock;
+ ctx->link_rate = 0;
+ }
had_process_hot_plug(ctx);
--
2.10.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v2 06/11] drm/i915: Remove hdmi_connected from LPE audio pdata
2017-04-27 16:02 [PATCH v2 00/11] drm/i915: LPE audio runtime PM and multipipe (v2) ville.syrjala
` (4 preceding siblings ...)
2017-04-27 16:02 ` [PATCH 05/11] drm/i915: Replace tmds_clock_speed and link_rate with just ls_clock ville.syrjala
@ 2017-04-27 16:02 ` ville.syrjala
2017-04-27 16:02 ` [PATCH 07/11] drm/i915: Reorganize intel_lpe_audio_notify() arguments ville.syrjala
` (7 subsequent siblings)
13 siblings, 0 replies; 27+ 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>
We can determine that the pipe was shut down from pipe<0, so there's
no point in duplicating that information as 'hdmi_connected'.
v2: Use pipe<0 instead of port<0 as we'll want to do per-port
PCM devices later
Initialize pipe to -1 to inidicate inactive initial state
Cc: Takashi Iwai <tiwai@suse.de>
Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/intel_lpe_audio.c | 9 +++++----
include/drm/intel_lpe_audio.h | 3 +--
sound/x86/intel_hdmi_audio.c | 8 ++++----
3 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c
index 5a1a37e963f1..7fd95733eff5 100644
--- a/drivers/gpu/drm/i915/intel_lpe_audio.c
+++ b/drivers/gpu/drm/i915/intel_lpe_audio.c
@@ -111,6 +111,7 @@ lpe_audio_platdev_create(struct drm_i915_private *dev_priv)
pinfo.size_data = sizeof(*pdata);
pinfo.dma_mask = DMA_BIT_MASK(32);
+ pdata->pipe = -1;
spin_lock_init(&pdata->lpe_audio_slock);
platdev = platform_device_register_full(&pinfo);
@@ -332,12 +333,12 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv,
audio_enable = I915_READ(VLV_AUD_PORT_EN_DBG(port));
+ pdata->eld.port_id = port;
+
if (eld != NULL) {
memcpy(pdata->eld.eld_data, eld,
HDMI_MAX_ELD_BYTES);
- pdata->eld.port_id = port;
- pdata->eld.pipe_id = pipe;
- pdata->hdmi_connected = true;
+ pdata->pipe = pipe;
pdata->ls_clock = ls_clock;
pdata->dp_output = dp_output;
@@ -348,7 +349,7 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv,
} else {
memset(pdata->eld.eld_data, 0,
HDMI_MAX_ELD_BYTES);
- pdata->hdmi_connected = false;
+ pdata->pipe = -1;
pdata->ls_clock = 0;
pdata->dp_output = false;
diff --git a/include/drm/intel_lpe_audio.h b/include/drm/intel_lpe_audio.h
index 8bf804ce8905..9a5bdf5ad180 100644
--- a/include/drm/intel_lpe_audio.h
+++ b/include/drm/intel_lpe_audio.h
@@ -33,13 +33,12 @@ struct platform_device;
struct intel_hdmi_lpe_audio_eld {
int port_id;
- int pipe_id;
unsigned char eld_data[HDMI_MAX_ELD_BYTES];
};
struct intel_hdmi_lpe_audio_pdata {
+ int pipe;
int ls_clock;
- bool hdmi_connected;
bool dp_output;
struct intel_hdmi_lpe_audio_eld eld;
void (*notify_audio_lpe)(struct platform_device *pdev);
diff --git a/sound/x86/intel_hdmi_audio.c b/sound/x86/intel_hdmi_audio.c
index 4eaf5de54f61..1a095189db83 100644
--- a/sound/x86/intel_hdmi_audio.c
+++ b/sound/x86/intel_hdmi_audio.c
@@ -1559,7 +1559,7 @@ static void had_audio_wq(struct work_struct *work)
pm_runtime_get_sync(ctx->dev);
mutex_lock(&ctx->mutex);
- if (!pdata->hdmi_connected) {
+ if (pdata->pipe < 0) {
dev_dbg(ctx->dev, "%s: Event: HAD_NOTIFY_HOT_UNPLUG\n",
__func__);
memset(ctx->eld, 0, sizeof(ctx->eld)); /* clear the old ELD */
@@ -1568,9 +1568,9 @@ static void had_audio_wq(struct work_struct *work)
struct intel_hdmi_lpe_audio_eld *eld = &pdata->eld;
dev_dbg(ctx->dev, "%s: HAD_NOTIFY_ELD : port = %d, tmds = %d\n",
- __func__, eld->port_id, pdata->ls_clock);
+ __func__, eld->port_id, pdata->ls_clock);
- switch (eld->pipe_id) {
+ switch (pdata->pipe) {
case 0:
ctx->had_config_offset = AUDIO_HDMI_CONFIG_A;
break;
@@ -1582,7 +1582,7 @@ static void had_audio_wq(struct work_struct *work)
break;
default:
dev_dbg(ctx->dev, "Invalid pipe %d\n",
- eld->pipe_id);
+ pdata->pipe);
break;
}
--
2.10.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 27+ 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
` (5 preceding siblings ...)
2017-04-27 16:02 ` [PATCH v2 06/11] drm/i915: Remove hdmi_connected from LPE audio pdata ville.syrjala
@ 2017-04-27 16:02 ` ville.syrjala
2017-04-27 16:02 ` [PATCH v2 08/11] drm/i915: Clean up the LPE audio platform data ville.syrjala
` (6 subsequent siblings)
13 siblings, 0 replies; 27+ 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] 27+ messages in thread* [PATCH v2 08/11] drm/i915: Clean up the LPE audio platform data
2017-04-27 16:02 [PATCH v2 00/11] drm/i915: LPE audio runtime PM and multipipe (v2) ville.syrjala
` (6 preceding siblings ...)
2017-04-27 16:02 ` [PATCH 07/11] drm/i915: Reorganize intel_lpe_audio_notify() arguments ville.syrjala
@ 2017-04-27 16:02 ` ville.syrjala
2017-04-27 16:02 ` [PATCH 09/11] ALSA: x86: Prepare LPE audio ctls for multiple PCMs ville.syrjala
` (5 subsequent siblings)
13 siblings, 0 replies; 27+ 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>
Split the LPE audio platform data into a port specific
chunk and device specific chunk. Eventually we'll have
a port specific chunk for each port, but for now we'll
stick to just one.
We'll also get rid of the intel_hdmi_lpe_audio_eld structure
which doesn't seem to have any real reason to exist.
v2: Organize per port instead of per pipe
Cc: Takashi Iwai <tiwai@suse.de>
Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/intel_lpe_audio.c | 30 ++++++++++++++----------------
include/drm/intel_lpe_audio.h | 15 ++++++++-------
sound/x86/intel_hdmi_audio.c | 19 +++++++++----------
3 files changed, 31 insertions(+), 33 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c
index 4c770d037f23..bdbc235141b5 100644
--- a/drivers/gpu/drm/i915/intel_lpe_audio.c
+++ b/drivers/gpu/drm/i915/intel_lpe_audio.c
@@ -111,7 +111,7 @@ lpe_audio_platdev_create(struct drm_i915_private *dev_priv)
pinfo.size_data = sizeof(*pdata);
pinfo.dma_mask = DMA_BIT_MASK(32);
- pdata->pipe = -1;
+ pdata->port.pipe = -1;
spin_lock_init(&pdata->lpe_audio_slock);
platdev = platform_device_register_full(&pinfo);
@@ -320,38 +320,36 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv,
const void *eld, int ls_clock, bool dp_output)
{
unsigned long irq_flags;
- struct intel_hdmi_lpe_audio_pdata *pdata = NULL;
+ struct intel_hdmi_lpe_audio_pdata *pdata;
+ struct intel_hdmi_lpe_audio_port_pdata *ppdata;
u32 audio_enable;
if (!HAS_LPE_AUDIO(dev_priv))
return;
- pdata = dev_get_platdata(
- &(dev_priv->lpe_audio.platdev->dev));
+ pdata = dev_get_platdata(&dev_priv->lpe_audio.platdev->dev);
+ ppdata = &pdata->port;
spin_lock_irqsave(&pdata->lpe_audio_slock, irq_flags);
audio_enable = I915_READ(VLV_AUD_PORT_EN_DBG(port));
- pdata->eld.port_id = port;
+ ppdata->port = port;
if (eld != NULL) {
- memcpy(pdata->eld.eld_data, eld,
- HDMI_MAX_ELD_BYTES);
- pdata->pipe = pipe;
- pdata->ls_clock = ls_clock;
- pdata->dp_output = dp_output;
+ memcpy(ppdata->eld, eld, HDMI_MAX_ELD_BYTES);
+ ppdata->pipe = pipe;
+ ppdata->ls_clock = ls_clock;
+ ppdata->dp_output = dp_output;
/* Unmute the amp for both DP and HDMI */
I915_WRITE(VLV_AUD_PORT_EN_DBG(port),
audio_enable & ~VLV_AMP_MUTE);
-
} else {
- memset(pdata->eld.eld_data, 0,
- HDMI_MAX_ELD_BYTES);
- pdata->pipe = -1;
- pdata->ls_clock = 0;
- pdata->dp_output = false;
+ memset(ppdata->eld, 0, HDMI_MAX_ELD_BYTES);
+ ppdata->pipe = -1;
+ ppdata->ls_clock = 0;
+ ppdata->dp_output = false;
/* Mute the amp for both DP and HDMI */
I915_WRITE(VLV_AUD_PORT_EN_DBG(port),
diff --git a/include/drm/intel_lpe_audio.h b/include/drm/intel_lpe_audio.h
index 9a5bdf5ad180..211f1cd61153 100644
--- a/include/drm/intel_lpe_audio.h
+++ b/include/drm/intel_lpe_audio.h
@@ -31,16 +31,17 @@ struct platform_device;
#define HDMI_MAX_ELD_BYTES 128
-struct intel_hdmi_lpe_audio_eld {
- int port_id;
- unsigned char eld_data[HDMI_MAX_ELD_BYTES];
-};
-
-struct intel_hdmi_lpe_audio_pdata {
+struct intel_hdmi_lpe_audio_port_pdata {
+ u8 eld[HDMI_MAX_ELD_BYTES];
+ int port;
int pipe;
int ls_clock;
bool dp_output;
- struct intel_hdmi_lpe_audio_eld eld;
+};
+
+struct intel_hdmi_lpe_audio_pdata {
+ struct intel_hdmi_lpe_audio_port_pdata port;
+
void (*notify_audio_lpe)(struct platform_device *pdev);
spinlock_t lpe_audio_slock;
};
diff --git a/sound/x86/intel_hdmi_audio.c b/sound/x86/intel_hdmi_audio.c
index 1a095189db83..c2b78621852e 100644
--- a/sound/x86/intel_hdmi_audio.c
+++ b/sound/x86/intel_hdmi_audio.c
@@ -1556,21 +1556,20 @@ static void had_audio_wq(struct work_struct *work)
struct snd_intelhad *ctx =
container_of(work, struct snd_intelhad, hdmi_audio_wq);
struct intel_hdmi_lpe_audio_pdata *pdata = ctx->dev->platform_data;
+ struct intel_hdmi_lpe_audio_port_pdata *ppdata = &pdata->port;
pm_runtime_get_sync(ctx->dev);
mutex_lock(&ctx->mutex);
- if (pdata->pipe < 0) {
+ if (ppdata->pipe < 0) {
dev_dbg(ctx->dev, "%s: Event: HAD_NOTIFY_HOT_UNPLUG\n",
__func__);
memset(ctx->eld, 0, sizeof(ctx->eld)); /* clear the old ELD */
had_process_hot_unplug(ctx);
} else {
- struct intel_hdmi_lpe_audio_eld *eld = &pdata->eld;
-
dev_dbg(ctx->dev, "%s: HAD_NOTIFY_ELD : port = %d, tmds = %d\n",
- __func__, eld->port_id, pdata->ls_clock);
+ __func__, ppdata->port, ppdata->ls_clock);
- switch (pdata->pipe) {
+ switch (ppdata->pipe) {
case 0:
ctx->had_config_offset = AUDIO_HDMI_CONFIG_A;
break;
@@ -1582,18 +1581,18 @@ static void had_audio_wq(struct work_struct *work)
break;
default:
dev_dbg(ctx->dev, "Invalid pipe %d\n",
- pdata->pipe);
+ ppdata->pipe);
break;
}
- memcpy(ctx->eld, eld->eld_data, sizeof(ctx->eld));
+ memcpy(ctx->eld, ppdata->eld, sizeof(ctx->eld));
- ctx->dp_output = pdata->dp_output;
+ ctx->dp_output = ppdata->dp_output;
if (ctx->dp_output) {
ctx->tmds_clock_speed = 0;
- ctx->link_rate = pdata->ls_clock;
+ ctx->link_rate = ppdata->ls_clock;
} else {
- ctx->tmds_clock_speed = pdata->ls_clock;
+ ctx->tmds_clock_speed = ppdata->ls_clock;
ctx->link_rate = 0;
}
--
2.10.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH 09/11] ALSA: x86: Prepare LPE audio ctls for multiple PCMs
2017-04-27 16:02 [PATCH v2 00/11] drm/i915: LPE audio runtime PM and multipipe (v2) ville.syrjala
` (7 preceding siblings ...)
2017-04-27 16:02 ` [PATCH v2 08/11] drm/i915: Clean up the LPE audio platform data ville.syrjala
@ 2017-04-27 16:02 ` ville.syrjala
2017-04-27 16:02 ` [PATCH v2 10/11] ALSA: x86: Split snd_intelhad into card and PCM specific structures ville.syrjala
` (4 subsequent siblings)
13 siblings, 0 replies; 27+ 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>
In preparation for register a PCM device for each pipe adjust
link up the ctl elements with the corresponding PCM device.
Cc: Takashi Iwai <tiwai@suse.de>
Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
sound/x86/intel_hdmi_audio.c | 23 +++++++++++++++++++----
1 file changed, 19 insertions(+), 4 deletions(-)
diff --git a/sound/x86/intel_hdmi_audio.c b/sound/x86/intel_hdmi_audio.c
index c2b78621852e..69e10845633a 100644
--- a/sound/x86/intel_hdmi_audio.c
+++ b/sound/x86/intel_hdmi_audio.c
@@ -1609,11 +1609,16 @@ static void had_audio_wq(struct work_struct *work)
/*
* Jack interface
*/
-static int had_create_jack(struct snd_intelhad *ctx)
+static int had_create_jack(struct snd_intelhad *ctx,
+ struct snd_pcm *pcm)
{
+ char hdmi_str[32];
int err;
- err = snd_jack_new(ctx->card, "HDMI/DP", SND_JACK_AVOUT, &ctx->jack,
+ snprintf(hdmi_str, sizeof(hdmi_str),
+ "HDMI/DP,pcm=%d", pcm->device);
+
+ err = snd_jack_new(ctx->card, hdmi_str, SND_JACK_AVOUT, &ctx->jack,
true, false);
if (err < 0)
return err;
@@ -1793,7 +1798,17 @@ static int hdmi_lpe_audio_probe(struct platform_device *pdev)
/* create controls */
for (i = 0; i < ARRAY_SIZE(had_controls); i++) {
- ret = snd_ctl_add(card, snd_ctl_new1(&had_controls[i], ctx));
+ struct snd_kcontrol *kctl;
+
+ kctl = snd_ctl_new1(&had_controls[i], ctx);
+ if (!kctl) {
+ ret = -ENOMEM;
+ goto err;
+ }
+
+ kctl->id.device = pcm->device;
+
+ ret = snd_ctl_add(card, kctl);
if (ret < 0)
goto err;
}
@@ -1805,7 +1820,7 @@ static int hdmi_lpe_audio_probe(struct platform_device *pdev)
if (ret < 0)
goto err;
- ret = had_create_jack(ctx);
+ ret = had_create_jack(ctx, pcm);
if (ret < 0)
goto err;
--
2.10.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v2 10/11] ALSA: x86: Split snd_intelhad into card and PCM specific structures
2017-04-27 16:02 [PATCH v2 00/11] drm/i915: LPE audio runtime PM and multipipe (v2) ville.syrjala
` (8 preceding siblings ...)
2017-04-27 16:02 ` [PATCH 09/11] ALSA: x86: Prepare LPE audio ctls for multiple PCMs ville.syrjala
@ 2017-04-27 16:02 ` ville.syrjala
2017-04-27 16:02 ` [PATCH v2 11/11] ALSA: x86: Register multiple PCM devices for the LPE audio card ville.syrjala
` (3 subsequent siblings)
13 siblings, 0 replies; 27+ 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>
To allow multiple PCM devices to be registered for the LPE audio card,
split the private data into card and PCM specific chunks. For now we'll
stick to just one PCM device as before.
v2: Rework to do a pcm device per port instead of per pipe
Cc: Takashi Iwai <tiwai@suse.de>
Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
sound/x86/intel_hdmi_audio.c | 227 +++++++++++++++++++++++++------------------
sound/x86/intel_hdmi_audio.h | 15 ++-
2 files changed, 142 insertions(+), 100 deletions(-)
diff --git a/sound/x86/intel_hdmi_audio.c b/sound/x86/intel_hdmi_audio.c
index 69e10845633a..12fae26e70bb 100644
--- a/sound/x86/intel_hdmi_audio.c
+++ b/sound/x86/intel_hdmi_audio.c
@@ -42,6 +42,9 @@
#include <drm/intel_lpe_audio.h>
#include "intel_hdmi_audio.h"
+#define for_each_port(card_ctx, port) \
+ for ((port) = 0; (port) < (card_ctx)->num_ports; (port)++)
+
/*standard module options for ALSA. This module supports only one card*/
static int hdmi_card_index = SNDRV_DEFAULT_IDX1;
static char *hdmi_card_id = SNDRV_DEFAULT_STR1;
@@ -192,12 +195,12 @@ static void had_substream_put(struct snd_intelhad *intelhaddata)
/* Register access functions */
static u32 had_read_register_raw(struct snd_intelhad *ctx, u32 reg)
{
- return ioread32(ctx->mmio_start + ctx->had_config_offset + reg);
+ return ioread32(ctx->card_ctx->mmio_start + ctx->had_config_offset + reg);
}
static void had_write_register_raw(struct snd_intelhad *ctx, u32 reg, u32 val)
{
- iowrite32(val, ctx->mmio_start + ctx->had_config_offset + reg);
+ iowrite32(val, ctx->card_ctx->mmio_start + ctx->had_config_offset + reg);
}
static void had_read_register(struct snd_intelhad *ctx, u32 reg, u32 *val)
@@ -1519,22 +1522,27 @@ static const struct snd_kcontrol_new had_controls[] = {
*/
static irqreturn_t display_pipe_interrupt_handler(int irq, void *dev_id)
{
- struct snd_intelhad *ctx = dev_id;
- u32 audio_stat;
+ struct snd_intelhad_card *card_ctx = dev_id;
+ int port;
- /* use raw register access to ack IRQs even while disconnected */
- audio_stat = had_read_register_raw(ctx, AUD_HDMI_STATUS);
+ for_each_port(card_ctx, port) {
+ struct snd_intelhad *ctx = &card_ctx->pcm_ctx[port];
+ u32 audio_stat;
- if (audio_stat & HDMI_AUDIO_UNDERRUN) {
- had_write_register_raw(ctx, AUD_HDMI_STATUS,
- HDMI_AUDIO_UNDERRUN);
- had_process_buffer_underrun(ctx);
- }
+ /* use raw register access to ack IRQs even while disconnected */
+ audio_stat = had_read_register_raw(ctx, AUD_HDMI_STATUS);
+
+ if (audio_stat & HDMI_AUDIO_UNDERRUN) {
+ had_write_register_raw(ctx, AUD_HDMI_STATUS,
+ HDMI_AUDIO_UNDERRUN);
+ had_process_buffer_underrun(ctx);
+ }
- if (audio_stat & HDMI_AUDIO_BUFFER_DONE) {
- had_write_register_raw(ctx, AUD_HDMI_STATUS,
- HDMI_AUDIO_BUFFER_DONE);
- had_process_buffer_done(ctx);
+ if (audio_stat & HDMI_AUDIO_BUFFER_DONE) {
+ had_write_register_raw(ctx, AUD_HDMI_STATUS,
+ HDMI_AUDIO_BUFFER_DONE);
+ had_process_buffer_done(ctx);
+ }
}
return IRQ_HANDLED;
@@ -1545,9 +1553,14 @@ static irqreturn_t display_pipe_interrupt_handler(int irq, void *dev_id)
*/
static void notify_audio_lpe(struct platform_device *pdev)
{
- struct snd_intelhad *ctx = platform_get_drvdata(pdev);
+ struct snd_intelhad_card *card_ctx = platform_get_drvdata(pdev);
+ int port;
+
+ for_each_port(card_ctx, port) {
+ struct snd_intelhad *ctx = &card_ctx->pcm_ctx[port];
- schedule_work(&ctx->hdmi_audio_wq);
+ schedule_work(&ctx->hdmi_audio_wq);
+ }
}
/* the work to handle monitor hot plug/unplug */
@@ -1618,7 +1631,8 @@ static int had_create_jack(struct snd_intelhad *ctx,
snprintf(hdmi_str, sizeof(hdmi_str),
"HDMI/DP,pcm=%d", pcm->device);
- err = snd_jack_new(ctx->card, hdmi_str, SND_JACK_AVOUT, &ctx->jack,
+ err = snd_jack_new(ctx->card_ctx->card, hdmi_str,
+ SND_JACK_AVOUT, &ctx->jack,
true, false);
if (err < 0)
return err;
@@ -1632,13 +1646,18 @@ static int had_create_jack(struct snd_intelhad *ctx,
static int hdmi_lpe_audio_runtime_suspend(struct device *dev)
{
- struct snd_intelhad *ctx = dev_get_drvdata(dev);
- struct snd_pcm_substream *substream;
+ struct snd_intelhad_card *card_ctx = dev_get_drvdata(dev);
+ int port;
- substream = had_substream_get(ctx);
- if (substream) {
- snd_pcm_suspend(substream);
- had_substream_put(ctx);
+ for_each_port(card_ctx, port) {
+ struct snd_intelhad *ctx = &card_ctx->pcm_ctx[port];
+ struct snd_pcm_substream *substream;
+
+ substream = had_substream_get(ctx);
+ if (substream) {
+ snd_pcm_suspend(substream);
+ had_substream_put(ctx);
+ }
}
return 0;
@@ -1646,12 +1665,12 @@ static int hdmi_lpe_audio_runtime_suspend(struct device *dev)
static int __maybe_unused hdmi_lpe_audio_suspend(struct device *dev)
{
- struct snd_intelhad *ctx = dev_get_drvdata(dev);
+ struct snd_intelhad_card *card_ctx = dev_get_drvdata(dev);
int err;
err = hdmi_lpe_audio_runtime_suspend(dev);
if (!err)
- snd_power_change_state(ctx->card, SNDRV_CTL_POWER_D3hot);
+ snd_power_change_state(card_ctx->card, SNDRV_CTL_POWER_D3hot);
return err;
}
@@ -1663,29 +1682,34 @@ static int hdmi_lpe_audio_runtime_resume(struct device *dev)
static int __maybe_unused hdmi_lpe_audio_resume(struct device *dev)
{
- struct snd_intelhad *ctx = dev_get_drvdata(dev);
+ struct snd_intelhad_card *card_ctx = dev_get_drvdata(dev);
hdmi_lpe_audio_runtime_resume(dev);
- snd_power_change_state(ctx->card, SNDRV_CTL_POWER_D0);
+ snd_power_change_state(card_ctx->card, SNDRV_CTL_POWER_D0);
return 0;
}
/* release resources */
static void hdmi_lpe_audio_free(struct snd_card *card)
{
- struct snd_intelhad *ctx = card->private_data;
- struct intel_hdmi_lpe_audio_pdata *pdata = ctx->dev->platform_data;
+ struct snd_intelhad_card *card_ctx = card->private_data;
+ struct intel_hdmi_lpe_audio_pdata *pdata = card_ctx->dev->platform_data;
+ int port;
spin_lock_irq(&pdata->lpe_audio_slock);
pdata->notify_audio_lpe = NULL;
spin_unlock_irq(&pdata->lpe_audio_slock);
- cancel_work_sync(&ctx->hdmi_audio_wq);
+ for_each_port(card_ctx, port) {
+ struct snd_intelhad *ctx = &card_ctx->pcm_ctx[port];
+
+ cancel_work_sync(&ctx->hdmi_audio_wq);
+ }
- if (ctx->mmio_start)
- iounmap(ctx->mmio_start);
- if (ctx->irq >= 0)
- free_irq(ctx->irq, ctx);
+ if (card_ctx->mmio_start)
+ iounmap(card_ctx->mmio_start);
+ if (card_ctx->irq >= 0)
+ free_irq(card_ctx->irq, card_ctx);
}
/*
@@ -1697,12 +1721,12 @@ static void hdmi_lpe_audio_free(struct snd_card *card)
static int hdmi_lpe_audio_probe(struct platform_device *pdev)
{
struct snd_card *card;
- struct snd_intelhad *ctx;
+ struct snd_intelhad_card *card_ctx;
struct snd_pcm *pcm;
struct intel_hdmi_lpe_audio_pdata *pdata;
int irq;
struct resource *res_mmio;
- int i, ret;
+ int port, ret;
pdata = pdev->dev.platform_data;
if (!pdata) {
@@ -1725,39 +1749,30 @@ static int hdmi_lpe_audio_probe(struct platform_device *pdev)
/* create a card instance with ALSA framework */
ret = snd_card_new(&pdev->dev, hdmi_card_index, hdmi_card_id,
- THIS_MODULE, sizeof(*ctx), &card);
+ THIS_MODULE, sizeof(*card_ctx), &card);
if (ret)
return ret;
- ctx = card->private_data;
- spin_lock_init(&ctx->had_spinlock);
- mutex_init(&ctx->mutex);
- ctx->connected = false;
- ctx->dev = &pdev->dev;
- ctx->card = card;
- ctx->aes_bits = SNDRV_PCM_DEFAULT_CON_SPDIF;
+ card_ctx = card->private_data;
+ card_ctx->dev = &pdev->dev;
+ card_ctx->card = card;
strcpy(card->driver, INTEL_HAD);
strcpy(card->shortname, "Intel HDMI/DP LPE Audio");
strcpy(card->longname, "Intel HDMI/DP LPE Audio");
- ctx->irq = -1;
- ctx->tmds_clock_speed = DIS_SAMPLE_RATE_148_5;
- INIT_WORK(&ctx->hdmi_audio_wq, had_audio_wq);
+ card_ctx->irq = -1;
card->private_free = hdmi_lpe_audio_free;
- /* assume pipe A as default */
- ctx->had_config_offset = AUDIO_HDMI_CONFIG_A;
-
- platform_set_drvdata(pdev, ctx);
+ platform_set_drvdata(pdev, card_ctx);
dev_dbg(&pdev->dev, "%s: mmio_start = 0x%x, mmio_end = 0x%x\n",
__func__, (unsigned int)res_mmio->start,
(unsigned int)res_mmio->end);
- ctx->mmio_start = ioremap_nocache(res_mmio->start,
- (size_t)(resource_size(res_mmio)));
- if (!ctx->mmio_start) {
+ card_ctx->mmio_start = ioremap_nocache(res_mmio->start,
+ (size_t)(resource_size(res_mmio)));
+ if (!card_ctx->mmio_start) {
dev_err(&pdev->dev, "Could not get ioremap\n");
ret = -EACCES;
goto err;
@@ -1765,65 +1780,79 @@ static int hdmi_lpe_audio_probe(struct platform_device *pdev)
/* setup interrupt handler */
ret = request_irq(irq, display_pipe_interrupt_handler, 0,
- pdev->name, ctx);
+ pdev->name, card_ctx);
if (ret < 0) {
dev_err(&pdev->dev, "request_irq failed\n");
goto err;
}
- ctx->irq = irq;
-
- ret = snd_pcm_new(card, INTEL_HAD, PCM_INDEX, MAX_PB_STREAMS,
- MAX_CAP_STREAMS, &pcm);
- if (ret)
- goto err;
-
- /* setup private data which can be retrieved when required */
- pcm->private_data = ctx;
- pcm->info_flags = 0;
- strncpy(pcm->name, card->shortname, strlen(card->shortname));
- /* setup the ops for playabck */
- snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_PLAYBACK, &had_pcm_ops);
+ card_ctx->irq = irq;
/* only 32bit addressable */
dma_set_mask(&pdev->dev, DMA_BIT_MASK(32));
dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(32));
- /* allocate dma pages;
- * try to allocate 600k buffer as default which is large enough
- */
- snd_pcm_lib_preallocate_pages_for_all(pcm,
- SNDRV_DMA_TYPE_DEV, NULL,
- HAD_DEFAULT_BUFFER, HAD_MAX_BUFFER);
+ init_channel_allocations();
- /* create controls */
- for (i = 0; i < ARRAY_SIZE(had_controls); i++) {
- struct snd_kcontrol *kctl;
+ card_ctx->num_ports = 1;
- kctl = snd_ctl_new1(&had_controls[i], ctx);
- if (!kctl) {
- ret = -ENOMEM;
+ for_each_port(card_ctx, port) {
+ struct snd_intelhad *ctx = &card_ctx->pcm_ctx[port];
+ int i;
+
+ ctx->card_ctx = card_ctx;
+ ctx->dev = card_ctx->dev;
+
+ INIT_WORK(&ctx->hdmi_audio_wq, had_audio_wq);
+
+ ctx->had_config_offset = AUDIO_HDMI_CONFIG_A;
+
+ ret = snd_pcm_new(card, INTEL_HAD, PCM_INDEX, MAX_PB_STREAMS,
+ MAX_CAP_STREAMS, &pcm);
+ if (ret)
goto err;
+
+ /* setup private data which can be retrieved when required */
+ pcm->private_data = ctx;
+ pcm->info_flags = 0;
+ strncpy(pcm->name, card->shortname, strlen(card->shortname));
+ /* setup the ops for playabck */
+ snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_PLAYBACK, &had_pcm_ops);
+
+ /* allocate dma pages;
+ * try to allocate 600k buffer as default which is large enough
+ */
+ snd_pcm_lib_preallocate_pages_for_all(pcm,
+ SNDRV_DMA_TYPE_DEV, NULL,
+ HAD_DEFAULT_BUFFER, HAD_MAX_BUFFER);
+
+ /* create controls */
+ for (i = 0; i < ARRAY_SIZE(had_controls); i++) {
+ struct snd_kcontrol *kctl;
+
+ kctl = snd_ctl_new1(&had_controls[i], ctx);
+ if (!kctl) {
+ ret = -ENOMEM;
+ goto err;
+ }
+
+ kctl->id.device = pcm->device;
+
+ ret = snd_ctl_add(card, kctl);
+ if (ret < 0)
+ goto err;
}
- kctl->id.device = pcm->device;
+ /* Register channel map controls */
+ ret = had_register_chmap_ctls(ctx, pcm);
+ if (ret < 0)
+ goto err;
- ret = snd_ctl_add(card, kctl);
+ ret = had_create_jack(ctx, pcm);
if (ret < 0)
goto err;
}
- init_channel_allocations();
-
- /* Register channel map controls */
- ret = had_register_chmap_ctls(ctx, pcm);
- if (ret < 0)
- goto err;
-
- ret = had_create_jack(ctx, pcm);
- if (ret < 0)
- goto err;
-
ret = snd_card_register(card);
if (ret)
goto err;
@@ -1837,7 +1866,11 @@ static int hdmi_lpe_audio_probe(struct platform_device *pdev)
pm_runtime_set_active(&pdev->dev);
dev_dbg(&pdev->dev, "%s: handle pending notification\n", __func__);
- schedule_work(&ctx->hdmi_audio_wq);
+ for_each_port(card_ctx, port) {
+ struct snd_intelhad *ctx = &card_ctx->pcm_ctx[port];
+
+ schedule_work(&ctx->hdmi_audio_wq);
+ }
return 0;
@@ -1853,9 +1886,9 @@ static int hdmi_lpe_audio_probe(struct platform_device *pdev)
*/
static int hdmi_lpe_audio_remove(struct platform_device *pdev)
{
- struct snd_intelhad *ctx = platform_get_drvdata(pdev);
+ struct snd_intelhad_card *card_ctx = platform_get_drvdata(pdev);
- snd_card_free(ctx->card);
+ snd_card_free(card_ctx->card);
return 0;
}
diff --git a/sound/x86/intel_hdmi_audio.h b/sound/x86/intel_hdmi_audio.h
index 2d3e389f76b3..2725964ebc46 100644
--- a/sound/x86/intel_hdmi_audio.h
+++ b/sound/x86/intel_hdmi_audio.h
@@ -101,7 +101,7 @@ struct pcm_stream_info {
* @chmap: holds channel map info
*/
struct snd_intelhad {
- struct snd_card *card;
+ struct snd_intelhad_card *card_ctx;
bool connected;
struct pcm_stream_info stream_info;
unsigned char eld[HDMI_MAX_ELD_BYTES];
@@ -123,8 +123,6 @@ struct snd_intelhad {
unsigned int period_bytes; /* PCM period size in bytes */
/* internal stuff */
- int irq;
- void __iomem *mmio_start;
unsigned int had_config_offset;
union aud_cfg aud_config; /* AUD_CONFIG reg value cache */
struct work_struct hdmi_audio_wq;
@@ -133,4 +131,15 @@ struct snd_intelhad {
struct snd_jack *jack;
};
+struct snd_intelhad_card {
+ struct snd_card *card;
+ struct device *dev;
+
+ /* internal stuff */
+ int irq;
+ void __iomem *mmio_start;
+ int num_ports;
+ struct snd_intelhad pcm_ctx[3];
+};
+
#endif /* _INTEL_HDMI_AUDIO_ */
--
2.10.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v2 11/11] ALSA: x86: Register multiple PCM devices for the LPE audio card
2017-04-27 16:02 [PATCH v2 00/11] drm/i915: LPE audio runtime PM and multipipe (v2) ville.syrjala
` (9 preceding siblings ...)
2017-04-27 16:02 ` [PATCH v2 10/11] ALSA: x86: Split snd_intelhad into card and PCM specific structures ville.syrjala
@ 2017-04-27 16:02 ` ville.syrjala
2017-04-28 19:46 ` [alsa-devel] " Pierre-Louis Bossart
2017-04-27 16:02 ` [PATCH alsa-lib] conf: Add multiple hdmi pcm definition for Intel LPE audio ville.syrjala
` (2 subsequent siblings)
13 siblings, 1 reply; 27+ 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>
Now that everything is in place let's register a PCM device for
each port of the display engine. This will make it possible to
actually output audio to multiple displays at the same time. And
it avoids modesets on unrelated displays from clobbering up the
ELD and whatnot for the display currently doing the playback.
v2: Add a PCM per port instead of per pipe
Cc: Takashi Iwai <tiwai@suse.de>
Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/intel_lpe_audio.c | 19 ++---
include/drm/intel_lpe_audio.h | 6 +-
sound/x86/intel_hdmi_audio.c | 126 +++++++++++++++++++--------------
sound/x86/intel_hdmi_audio.h | 7 +-
4 files changed, 92 insertions(+), 66 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c
index bdbc235141b5..fa728ed21d1f 100644
--- a/drivers/gpu/drm/i915/intel_lpe_audio.c
+++ b/drivers/gpu/drm/i915/intel_lpe_audio.c
@@ -111,7 +111,11 @@ lpe_audio_platdev_create(struct drm_i915_private *dev_priv)
pinfo.size_data = sizeof(*pdata);
pinfo.dma_mask = DMA_BIT_MASK(32);
- pdata->port.pipe = -1;
+ pdata->num_pipes = INTEL_INFO(dev_priv)->num_pipes;
+ pdata->num_ports = IS_CHERRYVIEW(dev_priv) ? 3 : 2; /* B,C,D or B,C */
+ pdata->port[0].pipe = -1;
+ pdata->port[1].pipe = -1;
+ pdata->port[2].pipe = -1;
spin_lock_init(&pdata->lpe_audio_slock);
platdev = platform_device_register_full(&pinfo);
@@ -319,7 +323,7 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv,
enum pipe pipe, enum port port,
const void *eld, int ls_clock, bool dp_output)
{
- unsigned long irq_flags;
+ unsigned long irqflags;
struct intel_hdmi_lpe_audio_pdata *pdata;
struct intel_hdmi_lpe_audio_port_pdata *ppdata;
u32 audio_enable;
@@ -328,14 +332,12 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv,
return;
pdata = dev_get_platdata(&dev_priv->lpe_audio.platdev->dev);
- ppdata = &pdata->port;
+ ppdata = &pdata->port[port];
- spin_lock_irqsave(&pdata->lpe_audio_slock, irq_flags);
+ spin_lock_irqsave(&pdata->lpe_audio_slock, irqflags);
audio_enable = I915_READ(VLV_AUD_PORT_EN_DBG(port));
- ppdata->port = port;
-
if (eld != NULL) {
memcpy(ppdata->eld, eld, HDMI_MAX_ELD_BYTES);
ppdata->pipe = pipe;
@@ -357,8 +359,7 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv,
}
if (pdata->notify_audio_lpe)
- pdata->notify_audio_lpe(dev_priv->lpe_audio.platdev);
+ pdata->notify_audio_lpe(dev_priv->lpe_audio.platdev, port);
- spin_unlock_irqrestore(&pdata->lpe_audio_slock,
- irq_flags);
+ spin_unlock_irqrestore(&pdata->lpe_audio_slock, irqflags);
}
diff --git a/include/drm/intel_lpe_audio.h b/include/drm/intel_lpe_audio.h
index 211f1cd61153..a911530c012e 100644
--- a/include/drm/intel_lpe_audio.h
+++ b/include/drm/intel_lpe_audio.h
@@ -40,9 +40,11 @@ struct intel_hdmi_lpe_audio_port_pdata {
};
struct intel_hdmi_lpe_audio_pdata {
- struct intel_hdmi_lpe_audio_port_pdata port;
+ struct intel_hdmi_lpe_audio_port_pdata port[3]; /* ports B,C,D */
+ int num_ports;
+ int num_pipes;
- void (*notify_audio_lpe)(struct platform_device *pdev);
+ void (*notify_audio_lpe)(struct platform_device *pdev, int pipe);
spinlock_t lpe_audio_slock;
};
diff --git a/sound/x86/intel_hdmi_audio.c b/sound/x86/intel_hdmi_audio.c
index 12fae26e70bb..909391d5270c 100644
--- a/sound/x86/intel_hdmi_audio.c
+++ b/sound/x86/intel_hdmi_audio.c
@@ -42,6 +42,8 @@
#include <drm/intel_lpe_audio.h>
#include "intel_hdmi_audio.h"
+#define for_each_pipe(card_ctx, pipe) \
+ for ((pipe) = 0; (pipe) < (card_ctx)->num_pipes; (pipe)++)
#define for_each_port(card_ctx, port) \
for ((port) = 0; (port) < (card_ctx)->num_ports; (port)++)
@@ -192,15 +194,30 @@ static void had_substream_put(struct snd_intelhad *intelhaddata)
spin_unlock_irqrestore(&intelhaddata->had_spinlock, flags);
}
+static u32 had_config_offset(int pipe)
+{
+ switch (pipe) {
+ default:
+ case 0:
+ return AUDIO_HDMI_CONFIG_A;
+ case 1:
+ return AUDIO_HDMI_CONFIG_B;
+ case 2:
+ return AUDIO_HDMI_CONFIG_C;
+ }
+}
+
/* Register access functions */
-static u32 had_read_register_raw(struct snd_intelhad *ctx, u32 reg)
+static u32 had_read_register_raw(struct snd_intelhad_card *card_ctx,
+ int pipe, u32 reg)
{
- return ioread32(ctx->card_ctx->mmio_start + ctx->had_config_offset + reg);
+ return ioread32(card_ctx->mmio_start + had_config_offset(pipe) + reg);
}
-static void had_write_register_raw(struct snd_intelhad *ctx, u32 reg, u32 val)
+static void had_write_register_raw(struct snd_intelhad_card *card_ctx,
+ int pipe, u32 reg, u32 val)
{
- iowrite32(val, ctx->card_ctx->mmio_start + ctx->had_config_offset + reg);
+ iowrite32(val, card_ctx->mmio_start + had_config_offset(pipe) + reg);
}
static void had_read_register(struct snd_intelhad *ctx, u32 reg, u32 *val)
@@ -208,13 +225,13 @@ static void had_read_register(struct snd_intelhad *ctx, u32 reg, u32 *val)
if (!ctx->connected)
*val = 0;
else
- *val = had_read_register_raw(ctx, reg);
+ *val = had_read_register_raw(ctx->card_ctx, ctx->pipe, reg);
}
static void had_write_register(struct snd_intelhad *ctx, u32 reg, u32 val)
{
if (ctx->connected)
- had_write_register_raw(ctx, reg, val);
+ had_write_register_raw(ctx->card_ctx, ctx->pipe, reg, val);
}
/*
@@ -1361,6 +1378,9 @@ static void had_process_hot_plug(struct snd_intelhad *intelhaddata)
return;
}
+ /* Disable Audio */
+ had_enable_audio(intelhaddata, false);
+
intelhaddata->connected = true;
dev_dbg(intelhaddata->dev,
"%s @ %d:DEBUG PLUG/UNPLUG : HAD_DRV_CONNECTED\n",
@@ -1523,26 +1543,31 @@ static const struct snd_kcontrol_new had_controls[] = {
static irqreturn_t display_pipe_interrupt_handler(int irq, void *dev_id)
{
struct snd_intelhad_card *card_ctx = dev_id;
- int port;
+ u32 audio_stat[3] = {};
+ int pipe, port;
+
+ for_each_pipe(card_ctx, pipe) {
+ /* use raw register access to ack IRQs even while disconnected */
+ audio_stat[pipe] = had_read_register_raw(card_ctx, pipe,
+ AUD_HDMI_STATUS) &
+ (HDMI_AUDIO_UNDERRUN | HDMI_AUDIO_BUFFER_DONE);
+
+ if (audio_stat[pipe])
+ had_write_register_raw(card_ctx, pipe,
+ AUD_HDMI_STATUS, audio_stat[pipe]);
+ }
for_each_port(card_ctx, port) {
struct snd_intelhad *ctx = &card_ctx->pcm_ctx[port];
- u32 audio_stat;
+ int pipe = ctx->pipe;
- /* use raw register access to ack IRQs even while disconnected */
- audio_stat = had_read_register_raw(ctx, AUD_HDMI_STATUS);
-
- if (audio_stat & HDMI_AUDIO_UNDERRUN) {
- had_write_register_raw(ctx, AUD_HDMI_STATUS,
- HDMI_AUDIO_UNDERRUN);
- had_process_buffer_underrun(ctx);
- }
+ if (pipe < 0)
+ continue;
- if (audio_stat & HDMI_AUDIO_BUFFER_DONE) {
- had_write_register_raw(ctx, AUD_HDMI_STATUS,
- HDMI_AUDIO_BUFFER_DONE);
+ if (audio_stat[pipe] & HDMI_AUDIO_BUFFER_DONE)
had_process_buffer_done(ctx);
- }
+ if (audio_stat[pipe] & HDMI_AUDIO_UNDERRUN)
+ had_process_buffer_underrun(ctx);
}
return IRQ_HANDLED;
@@ -1551,16 +1576,12 @@ static irqreturn_t display_pipe_interrupt_handler(int irq, void *dev_id)
/*
* monitor plug/unplug notification from i915; just kick off the work
*/
-static void notify_audio_lpe(struct platform_device *pdev)
+static void notify_audio_lpe(struct platform_device *pdev, int port)
{
struct snd_intelhad_card *card_ctx = platform_get_drvdata(pdev);
- int port;
+ struct snd_intelhad *ctx = &card_ctx->pcm_ctx[port];
- for_each_port(card_ctx, port) {
- struct snd_intelhad *ctx = &card_ctx->pcm_ctx[port];
-
- schedule_work(&ctx->hdmi_audio_wq);
- }
+ schedule_work(&ctx->hdmi_audio_wq);
}
/* the work to handle monitor hot plug/unplug */
@@ -1569,34 +1590,27 @@ static void had_audio_wq(struct work_struct *work)
struct snd_intelhad *ctx =
container_of(work, struct snd_intelhad, hdmi_audio_wq);
struct intel_hdmi_lpe_audio_pdata *pdata = ctx->dev->platform_data;
- struct intel_hdmi_lpe_audio_port_pdata *ppdata = &pdata->port;
+ struct intel_hdmi_lpe_audio_port_pdata *ppdata = &pdata->port[ctx->port];
pm_runtime_get_sync(ctx->dev);
mutex_lock(&ctx->mutex);
if (ppdata->pipe < 0) {
- dev_dbg(ctx->dev, "%s: Event: HAD_NOTIFY_HOT_UNPLUG\n",
- __func__);
+ dev_dbg(ctx->dev, "%s: Event: HAD_NOTIFY_HOT_UNPLUG : port = %d\n",
+ __func__, ctx->port);
+
memset(ctx->eld, 0, sizeof(ctx->eld)); /* clear the old ELD */
+
+ ctx->dp_output = false;
+ ctx->tmds_clock_speed = 0;
+ ctx->link_rate = 0;
+
+ /* Shut down the stream */
had_process_hot_unplug(ctx);
+
+ ctx->pipe = -1;
} else {
dev_dbg(ctx->dev, "%s: HAD_NOTIFY_ELD : port = %d, tmds = %d\n",
- __func__, ppdata->port, ppdata->ls_clock);
-
- switch (ppdata->pipe) {
- case 0:
- ctx->had_config_offset = AUDIO_HDMI_CONFIG_A;
- break;
- case 1:
- ctx->had_config_offset = AUDIO_HDMI_CONFIG_B;
- break;
- case 2:
- ctx->had_config_offset = AUDIO_HDMI_CONFIG_C;
- break;
- default:
- dev_dbg(ctx->dev, "Invalid pipe %d\n",
- ppdata->pipe);
- break;
- }
+ __func__, ctx->port, ppdata->ls_clock);
memcpy(ctx->eld, ppdata->eld, sizeof(ctx->eld));
@@ -1609,11 +1623,18 @@ static void had_audio_wq(struct work_struct *work)
ctx->link_rate = 0;
}
+ /*
+ * Shut down the stream before we change
+ * the pipe assignment for this pcm device
+ */
had_process_hot_plug(ctx);
- /* Process mode change if stream is active */
+ ctx->pipe = ppdata->pipe;
+
+ /* Restart the stream if necessary */
had_process_mode_change(ctx);
}
+
mutex_unlock(&ctx->mutex);
pm_runtime_mark_last_busy(ctx->dev);
pm_runtime_put_autosuspend(ctx->dev);
@@ -1794,7 +1815,8 @@ static int hdmi_lpe_audio_probe(struct platform_device *pdev)
init_channel_allocations();
- card_ctx->num_ports = 1;
+ card_ctx->num_pipes = pdata->num_pipes;
+ card_ctx->num_ports = pdata->num_ports;
for_each_port(card_ctx, port) {
struct snd_intelhad *ctx = &card_ctx->pcm_ctx[port];
@@ -1802,12 +1824,12 @@ static int hdmi_lpe_audio_probe(struct platform_device *pdev)
ctx->card_ctx = card_ctx;
ctx->dev = card_ctx->dev;
+ ctx->port = port;
+ ctx->pipe = -1;
INIT_WORK(&ctx->hdmi_audio_wq, had_audio_wq);
- ctx->had_config_offset = AUDIO_HDMI_CONFIG_A;
-
- ret = snd_pcm_new(card, INTEL_HAD, PCM_INDEX, MAX_PB_STREAMS,
+ ret = snd_pcm_new(card, INTEL_HAD, port, MAX_PB_STREAMS,
MAX_CAP_STREAMS, &pcm);
if (ret)
goto err;
diff --git a/sound/x86/intel_hdmi_audio.h b/sound/x86/intel_hdmi_audio.h
index 2725964ebc46..0d91bb5dbab7 100644
--- a/sound/x86/intel_hdmi_audio.h
+++ b/sound/x86/intel_hdmi_audio.h
@@ -32,7 +32,6 @@
#include "intel_hdmi_lpe_audio.h"
-#define PCM_INDEX 0
#define MAX_PB_STREAMS 1
#define MAX_CAP_STREAMS 0
#define BYTES_PER_WORD 0x4
@@ -112,6 +111,8 @@ struct snd_intelhad {
struct snd_pcm_chmap *chmap;
int tmds_clock_speed;
int link_rate;
+ int port; /* fixed */
+ int pipe; /* can change dynamically */
/* ring buffer (BD) position index */
unsigned int bd_head;
@@ -123,7 +124,6 @@ struct snd_intelhad {
unsigned int period_bytes; /* PCM period size in bytes */
/* internal stuff */
- unsigned int had_config_offset;
union aud_cfg aud_config; /* AUD_CONFIG reg value cache */
struct work_struct hdmi_audio_wq;
struct mutex mutex; /* for protecting chmap and eld */
@@ -138,8 +138,9 @@ struct snd_intelhad_card {
/* internal stuff */
int irq;
void __iomem *mmio_start;
+ int num_pipes;
int num_ports;
- struct snd_intelhad pcm_ctx[3];
+ struct snd_intelhad pcm_ctx[3]; /* one for each port */
};
#endif /* _INTEL_HDMI_AUDIO_ */
--
2.10.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [alsa-devel] [PATCH v2 11/11] ALSA: x86: Register multiple PCM devices for the LPE audio card
2017-04-27 16:02 ` [PATCH v2 11/11] ALSA: x86: Register multiple PCM devices for the LPE audio card ville.syrjala
@ 2017-04-28 19:46 ` Pierre-Louis Bossart
0 siblings, 0 replies; 27+ messages in thread
From: Pierre-Louis Bossart @ 2017-04-28 19:46 UTC (permalink / raw)
To: ville.syrjala, intel-gfx; +Cc: Takashi Iwai, alsa-devel
A quick bisect tells me this last patch looks problematic. I don't have
time to look further into it today, hope this helps progress in finding
the issue. This is on a CHT device with HDMI plugged in and DP left out
unconnected for now.
$ git bisect good
9af5b5732365b8ea29000d1ad14800bb091a0724 is the first bad commit
commit 9af5b5732365b8ea29000d1ad14800bb091a0724
Author: Ville Syrjälä <ville.syrjala@linux.intel.com>
Date: Tue Apr 25 20:42:40 2017 +0300
ALSA: x86: Register multiple PCM devices for the LPE audio card
Now that everything is in place let's register a PCM device for
each port of the display engine. This will make it possible to
actually output audio to multiple displays at the same time. And
it avoids modesets on unrelated displays from clobbering up the
ELD and whatnot for the display currently doing the playback.
v2: Add a PCM per port instead of per pipe
Cc: Takashi Iwai <tiwai@suse.de>
Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
:040000 040000 17f94eecd597b97ccc003e2d27c03eadceb279f5
271d4c3130f9cc1334e79bf60b7fdc7337192655 M drivers
:040000 040000 6806ba942f3c0844dcf6ffdfdd751c2007e5680f
8b9e1d1f82a12febe705a771654fadc08c02c90f M include
:040000 040000 e1f46a21e1beaf9535b3e807f4cfeea5ad7dbe47
afd86621214380c86769c30d6405d9335143cf6b M sound
$ git bisect log
git bisect start
# bad: [9af5b5732365b8ea29000d1ad14800bb091a0724] ALSA: x86: Register
multiple PCM devices for the LPE audio card
git bisect bad 9af5b5732365b8ea29000d1ad14800bb091a0724
# good: [8b5a41bbd270c3a8db6d48bc1d6d6bafb59e6753] drm-tip:
2017y-04m-27d-13h-10m-59s UTC integration manifest
git bisect good 8b5a41bbd270c3a8db6d48bc1d6d6bafb59e6753
# good: [63e34e5d2e8aaf85dfe48085e5dbbb7215da80ba] drm/i915: Replace
tmds_clock_speed and link_rate with just ls_clock
git bisect good 63e34e5d2e8aaf85dfe48085e5dbbb7215da80ba
# good: [dc0ae8e9c2f395b24cba7a404f2ff49e7272bf4b] drm/i915: Clean up
the LPE audio platform data
git bisect good dc0ae8e9c2f395b24cba7a404f2ff49e7272bf4b
# good: [7d80cfe6f7eafe6cddf36cd6e227d54a45c6f175] ALSA: x86: Split
snd_intelhad into card and PCM specific structures
git bisect good 7d80cfe6f7eafe6cddf36cd6e227d54a45c6f175
# first bad commit: [9af5b5732365b8ea29000d1ad14800bb091a0724] ALSA:
x86: Register multiple PCM devices for the LPE audio card
On 04/27/2017 11:02 AM, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Now that everything is in place let's register a PCM device for
> each port of the display engine. This will make it possible to
> actually output audio to multiple displays at the same time. And
> it avoids modesets on unrelated displays from clobbering up the
> ELD and whatnot for the display currently doing the playback.
>
> v2: Add a PCM per port instead of per pipe
>
> Cc: Takashi Iwai <tiwai@suse.de>
> Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_lpe_audio.c | 19 ++---
> include/drm/intel_lpe_audio.h | 6 +-
> sound/x86/intel_hdmi_audio.c | 126 +++++++++++++++++++--------------
> sound/x86/intel_hdmi_audio.h | 7 +-
> 4 files changed, 92 insertions(+), 66 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c
> index bdbc235141b5..fa728ed21d1f 100644
> --- a/drivers/gpu/drm/i915/intel_lpe_audio.c
> +++ b/drivers/gpu/drm/i915/intel_lpe_audio.c
> @@ -111,7 +111,11 @@ lpe_audio_platdev_create(struct drm_i915_private *dev_priv)
> pinfo.size_data = sizeof(*pdata);
> pinfo.dma_mask = DMA_BIT_MASK(32);
>
> - pdata->port.pipe = -1;
> + pdata->num_pipes = INTEL_INFO(dev_priv)->num_pipes;
> + pdata->num_ports = IS_CHERRYVIEW(dev_priv) ? 3 : 2; /* B,C,D or B,C */
> + pdata->port[0].pipe = -1;
> + pdata->port[1].pipe = -1;
> + pdata->port[2].pipe = -1;
> spin_lock_init(&pdata->lpe_audio_slock);
>
> platdev = platform_device_register_full(&pinfo);
> @@ -319,7 +323,7 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv,
> enum pipe pipe, enum port port,
> const void *eld, int ls_clock, bool dp_output)
> {
> - unsigned long irq_flags;
> + unsigned long irqflags;
> struct intel_hdmi_lpe_audio_pdata *pdata;
> struct intel_hdmi_lpe_audio_port_pdata *ppdata;
> u32 audio_enable;
> @@ -328,14 +332,12 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv,
> return;
>
> pdata = dev_get_platdata(&dev_priv->lpe_audio.platdev->dev);
> - ppdata = &pdata->port;
> + ppdata = &pdata->port[port];
>
> - spin_lock_irqsave(&pdata->lpe_audio_slock, irq_flags);
> + spin_lock_irqsave(&pdata->lpe_audio_slock, irqflags);
>
> audio_enable = I915_READ(VLV_AUD_PORT_EN_DBG(port));
>
> - ppdata->port = port;
> -
> if (eld != NULL) {
> memcpy(ppdata->eld, eld, HDMI_MAX_ELD_BYTES);
> ppdata->pipe = pipe;
> @@ -357,8 +359,7 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv,
> }
>
> if (pdata->notify_audio_lpe)
> - pdata->notify_audio_lpe(dev_priv->lpe_audio.platdev);
> + pdata->notify_audio_lpe(dev_priv->lpe_audio.platdev, port);
>
> - spin_unlock_irqrestore(&pdata->lpe_audio_slock,
> - irq_flags);
> + spin_unlock_irqrestore(&pdata->lpe_audio_slock, irqflags);
> }
> diff --git a/include/drm/intel_lpe_audio.h b/include/drm/intel_lpe_audio.h
> index 211f1cd61153..a911530c012e 100644
> --- a/include/drm/intel_lpe_audio.h
> +++ b/include/drm/intel_lpe_audio.h
> @@ -40,9 +40,11 @@ struct intel_hdmi_lpe_audio_port_pdata {
> };
>
> struct intel_hdmi_lpe_audio_pdata {
> - struct intel_hdmi_lpe_audio_port_pdata port;
> + struct intel_hdmi_lpe_audio_port_pdata port[3]; /* ports B,C,D */
> + int num_ports;
> + int num_pipes;
>
> - void (*notify_audio_lpe)(struct platform_device *pdev);
> + void (*notify_audio_lpe)(struct platform_device *pdev, int pipe);
> spinlock_t lpe_audio_slock;
> };
>
> diff --git a/sound/x86/intel_hdmi_audio.c b/sound/x86/intel_hdmi_audio.c
> index 12fae26e70bb..909391d5270c 100644
> --- a/sound/x86/intel_hdmi_audio.c
> +++ b/sound/x86/intel_hdmi_audio.c
> @@ -42,6 +42,8 @@
> #include <drm/intel_lpe_audio.h>
> #include "intel_hdmi_audio.h"
>
> +#define for_each_pipe(card_ctx, pipe) \
> + for ((pipe) = 0; (pipe) < (card_ctx)->num_pipes; (pipe)++)
> #define for_each_port(card_ctx, port) \
> for ((port) = 0; (port) < (card_ctx)->num_ports; (port)++)
>
> @@ -192,15 +194,30 @@ static void had_substream_put(struct snd_intelhad *intelhaddata)
> spin_unlock_irqrestore(&intelhaddata->had_spinlock, flags);
> }
>
> +static u32 had_config_offset(int pipe)
> +{
> + switch (pipe) {
> + default:
> + case 0:
> + return AUDIO_HDMI_CONFIG_A;
> + case 1:
> + return AUDIO_HDMI_CONFIG_B;
> + case 2:
> + return AUDIO_HDMI_CONFIG_C;
> + }
> +}
> +
> /* Register access functions */
> -static u32 had_read_register_raw(struct snd_intelhad *ctx, u32 reg)
> +static u32 had_read_register_raw(struct snd_intelhad_card *card_ctx,
> + int pipe, u32 reg)
> {
> - return ioread32(ctx->card_ctx->mmio_start + ctx->had_config_offset + reg);
> + return ioread32(card_ctx->mmio_start + had_config_offset(pipe) + reg);
> }
>
> -static void had_write_register_raw(struct snd_intelhad *ctx, u32 reg, u32 val)
> +static void had_write_register_raw(struct snd_intelhad_card *card_ctx,
> + int pipe, u32 reg, u32 val)
> {
> - iowrite32(val, ctx->card_ctx->mmio_start + ctx->had_config_offset + reg);
> + iowrite32(val, card_ctx->mmio_start + had_config_offset(pipe) + reg);
> }
>
> static void had_read_register(struct snd_intelhad *ctx, u32 reg, u32 *val)
> @@ -208,13 +225,13 @@ static void had_read_register(struct snd_intelhad *ctx, u32 reg, u32 *val)
> if (!ctx->connected)
> *val = 0;
> else
> - *val = had_read_register_raw(ctx, reg);
> + *val = had_read_register_raw(ctx->card_ctx, ctx->pipe, reg);
> }
>
> static void had_write_register(struct snd_intelhad *ctx, u32 reg, u32 val)
> {
> if (ctx->connected)
> - had_write_register_raw(ctx, reg, val);
> + had_write_register_raw(ctx->card_ctx, ctx->pipe, reg, val);
> }
>
> /*
> @@ -1361,6 +1378,9 @@ static void had_process_hot_plug(struct snd_intelhad *intelhaddata)
> return;
> }
>
> + /* Disable Audio */
> + had_enable_audio(intelhaddata, false);
> +
> intelhaddata->connected = true;
> dev_dbg(intelhaddata->dev,
> "%s @ %d:DEBUG PLUG/UNPLUG : HAD_DRV_CONNECTED\n",
> @@ -1523,26 +1543,31 @@ static const struct snd_kcontrol_new had_controls[] = {
> static irqreturn_t display_pipe_interrupt_handler(int irq, void *dev_id)
> {
> struct snd_intelhad_card *card_ctx = dev_id;
> - int port;
> + u32 audio_stat[3] = {};
> + int pipe, port;
> +
> + for_each_pipe(card_ctx, pipe) {
> + /* use raw register access to ack IRQs even while disconnected */
> + audio_stat[pipe] = had_read_register_raw(card_ctx, pipe,
> + AUD_HDMI_STATUS) &
> + (HDMI_AUDIO_UNDERRUN | HDMI_AUDIO_BUFFER_DONE);
> +
> + if (audio_stat[pipe])
> + had_write_register_raw(card_ctx, pipe,
> + AUD_HDMI_STATUS, audio_stat[pipe]);
> + }
>
> for_each_port(card_ctx, port) {
> struct snd_intelhad *ctx = &card_ctx->pcm_ctx[port];
> - u32 audio_stat;
> + int pipe = ctx->pipe;
>
> - /* use raw register access to ack IRQs even while disconnected */
> - audio_stat = had_read_register_raw(ctx, AUD_HDMI_STATUS);
> -
> - if (audio_stat & HDMI_AUDIO_UNDERRUN) {
> - had_write_register_raw(ctx, AUD_HDMI_STATUS,
> - HDMI_AUDIO_UNDERRUN);
> - had_process_buffer_underrun(ctx);
> - }
> + if (pipe < 0)
> + continue;
>
> - if (audio_stat & HDMI_AUDIO_BUFFER_DONE) {
> - had_write_register_raw(ctx, AUD_HDMI_STATUS,
> - HDMI_AUDIO_BUFFER_DONE);
> + if (audio_stat[pipe] & HDMI_AUDIO_BUFFER_DONE)
> had_process_buffer_done(ctx);
> - }
> + if (audio_stat[pipe] & HDMI_AUDIO_UNDERRUN)
> + had_process_buffer_underrun(ctx);
> }
>
> return IRQ_HANDLED;
> @@ -1551,16 +1576,12 @@ static irqreturn_t display_pipe_interrupt_handler(int irq, void *dev_id)
> /*
> * monitor plug/unplug notification from i915; just kick off the work
> */
> -static void notify_audio_lpe(struct platform_device *pdev)
> +static void notify_audio_lpe(struct platform_device *pdev, int port)
> {
> struct snd_intelhad_card *card_ctx = platform_get_drvdata(pdev);
> - int port;
> + struct snd_intelhad *ctx = &card_ctx->pcm_ctx[port];
>
> - for_each_port(card_ctx, port) {
> - struct snd_intelhad *ctx = &card_ctx->pcm_ctx[port];
> -
> - schedule_work(&ctx->hdmi_audio_wq);
> - }
> + schedule_work(&ctx->hdmi_audio_wq);
> }
>
> /* the work to handle monitor hot plug/unplug */
> @@ -1569,34 +1590,27 @@ static void had_audio_wq(struct work_struct *work)
> struct snd_intelhad *ctx =
> container_of(work, struct snd_intelhad, hdmi_audio_wq);
> struct intel_hdmi_lpe_audio_pdata *pdata = ctx->dev->platform_data;
> - struct intel_hdmi_lpe_audio_port_pdata *ppdata = &pdata->port;
> + struct intel_hdmi_lpe_audio_port_pdata *ppdata = &pdata->port[ctx->port];
>
> pm_runtime_get_sync(ctx->dev);
> mutex_lock(&ctx->mutex);
> if (ppdata->pipe < 0) {
> - dev_dbg(ctx->dev, "%s: Event: HAD_NOTIFY_HOT_UNPLUG\n",
> - __func__);
> + dev_dbg(ctx->dev, "%s: Event: HAD_NOTIFY_HOT_UNPLUG : port = %d\n",
> + __func__, ctx->port);
> +
> memset(ctx->eld, 0, sizeof(ctx->eld)); /* clear the old ELD */
> +
> + ctx->dp_output = false;
> + ctx->tmds_clock_speed = 0;
> + ctx->link_rate = 0;
> +
> + /* Shut down the stream */
> had_process_hot_unplug(ctx);
> +
> + ctx->pipe = -1;
> } else {
> dev_dbg(ctx->dev, "%s: HAD_NOTIFY_ELD : port = %d, tmds = %d\n",
> - __func__, ppdata->port, ppdata->ls_clock);
> -
> - switch (ppdata->pipe) {
> - case 0:
> - ctx->had_config_offset = AUDIO_HDMI_CONFIG_A;
> - break;
> - case 1:
> - ctx->had_config_offset = AUDIO_HDMI_CONFIG_B;
> - break;
> - case 2:
> - ctx->had_config_offset = AUDIO_HDMI_CONFIG_C;
> - break;
> - default:
> - dev_dbg(ctx->dev, "Invalid pipe %d\n",
> - ppdata->pipe);
> - break;
> - }
> + __func__, ctx->port, ppdata->ls_clock);
>
> memcpy(ctx->eld, ppdata->eld, sizeof(ctx->eld));
>
> @@ -1609,11 +1623,18 @@ static void had_audio_wq(struct work_struct *work)
> ctx->link_rate = 0;
> }
>
> + /*
> + * Shut down the stream before we change
> + * the pipe assignment for this pcm device
> + */
> had_process_hot_plug(ctx);
>
> - /* Process mode change if stream is active */
> + ctx->pipe = ppdata->pipe;
> +
> + /* Restart the stream if necessary */
> had_process_mode_change(ctx);
> }
> +
> mutex_unlock(&ctx->mutex);
> pm_runtime_mark_last_busy(ctx->dev);
> pm_runtime_put_autosuspend(ctx->dev);
> @@ -1794,7 +1815,8 @@ static int hdmi_lpe_audio_probe(struct platform_device *pdev)
>
> init_channel_allocations();
>
> - card_ctx->num_ports = 1;
> + card_ctx->num_pipes = pdata->num_pipes;
> + card_ctx->num_ports = pdata->num_ports;
>
> for_each_port(card_ctx, port) {
> struct snd_intelhad *ctx = &card_ctx->pcm_ctx[port];
> @@ -1802,12 +1824,12 @@ static int hdmi_lpe_audio_probe(struct platform_device *pdev)
>
> ctx->card_ctx = card_ctx;
> ctx->dev = card_ctx->dev;
> + ctx->port = port;
> + ctx->pipe = -1;
>
> INIT_WORK(&ctx->hdmi_audio_wq, had_audio_wq);
>
> - ctx->had_config_offset = AUDIO_HDMI_CONFIG_A;
> -
> - ret = snd_pcm_new(card, INTEL_HAD, PCM_INDEX, MAX_PB_STREAMS,
> + ret = snd_pcm_new(card, INTEL_HAD, port, MAX_PB_STREAMS,
> MAX_CAP_STREAMS, &pcm);
> if (ret)
> goto err;
> diff --git a/sound/x86/intel_hdmi_audio.h b/sound/x86/intel_hdmi_audio.h
> index 2725964ebc46..0d91bb5dbab7 100644
> --- a/sound/x86/intel_hdmi_audio.h
> +++ b/sound/x86/intel_hdmi_audio.h
> @@ -32,7 +32,6 @@
>
> #include "intel_hdmi_lpe_audio.h"
>
> -#define PCM_INDEX 0
> #define MAX_PB_STREAMS 1
> #define MAX_CAP_STREAMS 0
> #define BYTES_PER_WORD 0x4
> @@ -112,6 +111,8 @@ struct snd_intelhad {
> struct snd_pcm_chmap *chmap;
> int tmds_clock_speed;
> int link_rate;
> + int port; /* fixed */
> + int pipe; /* can change dynamically */
>
> /* ring buffer (BD) position index */
> unsigned int bd_head;
> @@ -123,7 +124,6 @@ struct snd_intelhad {
> unsigned int period_bytes; /* PCM period size in bytes */
>
> /* internal stuff */
> - unsigned int had_config_offset;
> union aud_cfg aud_config; /* AUD_CONFIG reg value cache */
> struct work_struct hdmi_audio_wq;
> struct mutex mutex; /* for protecting chmap and eld */
> @@ -138,8 +138,9 @@ struct snd_intelhad_card {
> /* internal stuff */
> int irq;
> void __iomem *mmio_start;
> + int num_pipes;
> int num_ports;
> - struct snd_intelhad pcm_ctx[3];
> + struct snd_intelhad pcm_ctx[3]; /* one for each port */
> };
>
> #endif /* _INTEL_HDMI_AUDIO_ */
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH alsa-lib] conf: Add multiple hdmi pcm definition for Intel LPE audio
2017-04-27 16:02 [PATCH v2 00/11] drm/i915: LPE audio runtime PM and multipipe (v2) ville.syrjala
` (10 preceding siblings ...)
2017-04-27 16:02 ` [PATCH v2 11/11] ALSA: x86: Register multiple PCM devices for the LPE audio card ville.syrjala
@ 2017-04-27 16:02 ` ville.syrjala
2017-04-27 16:21 ` ✓ Fi.CI.BAT: success for " Patchwork
2017-04-28 8:41 ` [PATCH v2 00/11] drm/i915: LPE audio runtime PM and multipipe (v2) Takashi Iwai
13 siblings, 0 replies; 27+ 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>
Now that the kernel driver exposes several pcm devices, update
the hdmi pcm definitions to match.
Cc: Takashi Iwai <tiwai@suse.de>
Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
src/conf/cards/HdmiLpeAudio.conf | 74 ++++++++++++++++++++++++++++++++++++++--
1 file changed, 72 insertions(+), 2 deletions(-)
diff --git a/src/conf/cards/HdmiLpeAudio.conf b/src/conf/cards/HdmiLpeAudio.conf
index 61bdfeae2917..ad174b8ac450 100644
--- a/src/conf/cards/HdmiLpeAudio.conf
+++ b/src/conf/cards/HdmiLpeAudio.conf
@@ -51,11 +51,14 @@ HdmiLpeAudio.pcm.default {
<confdir:pcm/hdmi.conf>
-HdmiLpeAudio.pcm.hdmi.0 {
- @args [ CARD AES0 AES1 AES2 AES3 ]
+HdmiLpeAudio.pcm.hdmi.common {
+ @args [ CARD DEVICE AES0 AES1 AES2 AES3 ]
@args.CARD {
type string
}
+ @args.DEVICE {
+ type integer
+ }
@args.AES0 {
type integer
}
@@ -72,6 +75,7 @@ HdmiLpeAudio.pcm.hdmi.0 {
slave.pcm {
type hw
card $CARD
+ device $DEVICE
}
hooks.0 {
type ctl_elems
@@ -86,3 +90,69 @@ HdmiLpeAudio.pcm.hdmi.0 {
]
}
}
+
+HdmiLpeAudio.pcm.hdmi.0 {
+ @args [ CARD AES0 AES1 AES2 AES3 ]
+ @args.CARD { type string }
+ @args.AES0 { type integer }
+ @args.AES1 { type integer }
+ @args.AES2 { type integer }
+ @args.AES3 { type integer }
+ @func refer
+ name {
+ @func concat
+ strings [
+ "cards.HdmiLpeAudio.pcm.hdmi.common:"
+ "CARD=" $CARD ","
+ "DEVICE=0,"
+ "AES0=" $AES0 ","
+ "AES1=" $AES1 ","
+ "AES2=" $AES2 ","
+ "AES3=" $AES3
+ ]
+ }
+}
+
+HdmiLpeAudio.pcm.hdmi.1 {
+ @args [ CARD AES0 AES1 AES2 AES3 ]
+ @args.CARD { type string }
+ @args.AES0 { type integer }
+ @args.AES1 { type integer }
+ @args.AES2 { type integer }
+ @args.AES3 { type integer }
+ @func refer
+ name {
+ @func concat
+ strings [
+ "cards.HdmiLpeAudio.pcm.hdmi.common:"
+ "CARD=" $CARD ","
+ "DEVICE=1,"
+ "AES0=" $AES0 ","
+ "AES1=" $AES1 ","
+ "AES2=" $AES2 ","
+ "AES3=" $AES3
+ ]
+ }
+}
+
+HdmiLpeAudio.pcm.hdmi.2 {
+ @args [ CARD AES0 AES1 AES2 AES3 ]
+ @args.CARD { type string }
+ @args.AES0 { type integer }
+ @args.AES1 { type integer }
+ @args.AES2 { type integer }
+ @args.AES3 { type integer }
+ @func refer
+ name {
+ @func concat
+ strings [
+ "cards.HdmiLpeAudio.pcm.hdmi.common:"
+ "CARD=" $CARD ","
+ "DEVICE=2,"
+ "AES0=" $AES0 ","
+ "AES1=" $AES1 ","
+ "AES2=" $AES2 ","
+ "AES3=" $AES3
+ ]
+ }
+}
--
2.10.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 27+ messages in thread* ✓ Fi.CI.BAT: success for conf: Add multiple hdmi pcm definition for Intel LPE audio
2017-04-27 16:02 [PATCH v2 00/11] drm/i915: LPE audio runtime PM and multipipe (v2) ville.syrjala
` (11 preceding siblings ...)
2017-04-27 16:02 ` [PATCH alsa-lib] conf: Add multiple hdmi pcm definition for Intel LPE audio ville.syrjala
@ 2017-04-27 16:21 ` Patchwork
2017-04-28 8:41 ` [PATCH v2 00/11] drm/i915: LPE audio runtime PM and multipipe (v2) Takashi Iwai
13 siblings, 0 replies; 27+ messages in thread
From: Patchwork @ 2017-04-27 16:21 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx
== Series Details ==
Series: conf: Add multiple hdmi pcm definition for Intel LPE audio
URL : https://patchwork.freedesktop.org/series/23639/
State : success
== Summary ==
Series 23639v1 conf: Add multiple hdmi pcm definition for Intel LPE audio
https://patchwork.freedesktop.org/api/1.0/series/23639/revisions/1/mbox/
Test gem_exec_suspend:
Subgroup basic-s4-devices:
dmesg-warn -> PASS (fi-kbl-7560u) fdo#100125
fdo#100125 https://bugs.freedesktop.org/show_bug.cgi?id=100125
fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time:435s
fi-bdw-gvtdvm total:278 pass:256 dwarn:8 dfail:0 fail:0 skip:14 time:423s
fi-bsw-n3050 total:278 pass:242 dwarn:0 dfail:0 fail:0 skip:36 time:577s
fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time:512s
fi-bxt-t5700 total:278 pass:258 dwarn:0 dfail:0 fail:0 skip:20 time:544s
fi-byt-j1900 total:278 pass:254 dwarn:0 dfail:0 fail:0 skip:24 time:487s
fi-byt-n2820 total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time:483s
fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:406s
fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:410s
fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time:415s
fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:495s
fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:480s
fi-kbl-7500u total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:459s
fi-kbl-7560u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:573s
fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:455s
fi-skl-6700hq total:278 pass:261 dwarn:0 dfail:0 fail:0 skip:17 time:565s
fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time:459s
fi-skl-6770hq total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:489s
fi-skl-gvtdvm total:278 pass:265 dwarn:0 dfail:0 fail:0 skip:13 time:429s
fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time:532s
fi-snb-2600 total:278 pass:249 dwarn:0 dfail:0 fail:0 skip:29 time:396s
8b5a41bbd270c3a8db6d48bc1d6d6bafb59e6753 drm-tip: 2017y-04m-27d-13h-10m-59s UTC integration manifest
bc96bd1 ALSA: x86: Register multiple PCM devices for the LPE audio card
e09ae3e ALSA: x86: Split snd_intelhad into card and PCM specific structures
a9f1076 ALSA: x86: Prepare LPE audio ctls for multiple PCMs
ca71392 drm/i915: Clean up the LPE audio platform data
13211f9 drm/i915: Reorganize intel_lpe_audio_notify() arguments
023f2ba drm/i915: Remove hdmi_connected from LPE audio pdata
26eda33 drm/i915: Replace tmds_clock_speed and link_rate with just ls_clock
223ac7f drm/i915: Remove the unused pending_notify from LPE platform data
1df8df6 drm/i915: Stop pretending to mask/unmask LPE audio interrupts
ff5766c ALSA: x86: Clear the pdata.notify_lpe_audio pointer before teardown
ddc7b54 drm/i915: Fix runtime PM for LPE audio
== Logs ==
For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4570/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v2 00/11] drm/i915: LPE audio runtime PM and multipipe (v2)
2017-04-27 16:02 [PATCH v2 00/11] drm/i915: LPE audio runtime PM and multipipe (v2) ville.syrjala
` (12 preceding siblings ...)
2017-04-27 16:21 ` ✓ Fi.CI.BAT: success for " Patchwork
@ 2017-04-28 8:41 ` Takashi Iwai
2017-04-28 17:10 ` [alsa-devel] " Pierre-Louis Bossart
2017-05-03 13:39 ` Ville Syrjälä
13 siblings, 2 replies; 27+ messages in thread
From: Takashi Iwai @ 2017-04-28 8:41 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx, alsa-devel
On Thu, 27 Apr 2017 18:02:19 +0200,
ville.syrjala@linux.intel.com wrote:
>
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Okay, here's the second attempt at getting multiple pipes playing back
> audio on the VLV/CHV HDMI LPE audio device. The main change from v1 is
> that now the PCM devices are associated with ports instead of pipes,
> so the audio from one device always gets output on the same display.
>
> I've also tacked on the alsa-lib conf update. No clue whether it's
> really correct or not (the config language isn't a close friend
> of mine).
>
> BTW I did notice that with LPE audio all the controls say iface=PCM,
> whereas on HDA a bunch of them say iface=MIXER. No idea if that's
> OK or not, just something I spotted when I was comparing the results
> with HDA.
We generally accept both iface types for IEC958 stuff, since
historically many drivers have already mixed them up. So it's no
problem :)
> Entire series available here:
> git://github.com/vsyrjala/linux.git lpe_audio_multipipe_2
>
> Cc: Takashi Iwai <tiwai@suse.de>
> Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
All look good, and feel free to take my reviewed-by tag
Reviewed-by: Takashi Iwai <tiwai@suse.de>
As said previously, my only slight concern is the compatibility.
But, in the current situation with PulseAudio, only few people would
use this driver, so it shouldn't be so big impact, I suppose.
BTW, which port is used in general on BYT/CHT?
Oh, also, I suppose you want to carry these over i915 tree?
I don't mind either way, I can take them through sound tree if
preferred, too.
thanks,
Takashi
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [alsa-devel] [PATCH v2 00/11] drm/i915: LPE audio runtime PM and multipipe (v2)
2017-04-28 8:41 ` [PATCH v2 00/11] drm/i915: LPE audio runtime PM and multipipe (v2) Takashi Iwai
@ 2017-04-28 17:10 ` Pierre-Louis Bossart
2017-04-28 19:37 ` Ville Syrjälä
2017-05-03 13:39 ` Ville Syrjälä
1 sibling, 1 reply; 27+ messages in thread
From: Pierre-Louis Bossart @ 2017-04-28 17:10 UTC (permalink / raw)
To: Takashi Iwai, ville.syrjala; +Cc: intel-gfx, alsa-devel
On 04/28/2017 03:41 AM, Takashi Iwai wrote:
> On Thu, 27 Apr 2017 18:02:19 +0200,
> ville.syrjala@linux.intel.com wrote:
>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>
>> Okay, here's the second attempt at getting multiple pipes playing back
>> audio on the VLV/CHV HDMI LPE audio device. The main change from v1 is
>> that now the PCM devices are associated with ports instead of pipes,
>> so the audio from one device always gets output on the same display.
>>
>> I've also tacked on the alsa-lib conf update. No clue whether it's
>> really correct or not (the config language isn't a close friend
>> of mine).
>>
>> BTW I did notice that with LPE audio all the controls say iface=PCM,
>> whereas on HDA a bunch of them say iface=MIXER. No idea if that's
>> OK or not, just something I spotted when I was comparing the results
>> with HDA.
> We generally accept both iface types for IEC958 stuff, since
> historically many drivers have already mixed them up. So it's no
> problem :)
>
>
>> Entire series available here:
>> git://github.com/vsyrjala/linux.git lpe_audio_multipipe_2
>>
>> Cc: Takashi Iwai <tiwai@suse.de>
>> Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> All look good, and feel free to take my reviewed-by tag
> Reviewed-by: Takashi Iwai <tiwai@suse.de>
>
> As said previously, my only slight concern is the compatibility.
> But, in the current situation with PulseAudio, only few people would
> use this driver, so it shouldn't be so big impact, I suppose.
>
> BTW, which port is used in general on BYT/CHT?
>
> Oh, also, I suppose you want to carry these over i915 tree?
> I don't mind either way, I can take them through sound tree if
> preferred, too.
I see frequent oops on startup with this lpe_audio_multipipe_2 branch
with my CHT device not booting or no HDMI audio device created.
Not sure if these issues are due to the new patches or to the rest of
the drm code?
[ 5.529023] BUG: unable to handle kernel NULL pointer dereference
at (null)
[ 5.529143] IP: hdmi_lpe_audio_probe+0x40f/0x650 [snd_hdmi_lpe_audio]
[ 5.529202] PGD 0
[ 5.529242] Oops: 0000 [#1] SMP
[ 5.529274] Modules linked in: snd_soc_sst_atom_hifi2_platform
snd_soc_sst_match snd_soc_core snd_compress lpc_ich snd_seq
snd_seq_device shpchp snd_hdmi_lpe_audio(+) snd_pcm snd_timer dw_dmac
snd soundcore i2c_designware_platform(+) i2c_designware_core
spi_pxa2xx_platform acpi_pad mac_hid nfsd auth_rpcgss nfs_acl lockd
grace sunrpc ip_tables x_tables hid_generic mmc_block i2c_hid usbhid hid
autofs4
[ 5.529605] CPU: 2 PID: 512 Comm: systemd-udevd Not tainted
4.11.0-rc8-test+ #11
[ 5.529671] Hardware name: ZOTAC XXXXXX/Cherry Trail FFD, BIOS 5.11
09/28/2016
[ 5.529736] task: ffff88007485b780 task.stack: ffffc90000bfc000
[ 5.529793] RIP: 0010:hdmi_lpe_audio_probe+0x40f/0x650
[snd_hdmi_lpe_audio]
[ 5.529855] RSP: 0018:ffffc90000bffaf0 EFLAGS: 00010246
[ 5.529904] RAX: 0000000000000000 RBX: ffff880079209898 RCX:
ffff88007920f078
[ 5.529967] RDX: 0000000000000014 RSI: ffffc90000bffb28 RDI:
0000000000000002
[ 5.530031] RBP: ffffc90000bffb70 R08: 0000000000000001 R09:
0000000000000000
[ 5.530094] R10: ffff88007441bf00 R11: ffffc90000bffb36 R12:
ffff88007920ef20
[ 5.530159] R13: ffff88007920ef48 R14: 0000000000005688 R15:
0000000000000047
[ 5.530225] FS: 00007f627c988640(0000) GS:ffff88007b300000(0000)
knlGS:0000000000000000
[ 5.530299] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 5.530352] CR2: 0000000000000000 CR3: 0000000078cb8000 CR4:
00000000001006e0
[ 5.530416] Call Trace:
[ 5.530452] platform_drv_probe+0x3b/0xa0
[ 5.530494] driver_probe_device+0x2bb/0x460
[ 5.530538] __driver_attach+0xdf/0xf0
[ 5.530576] ? driver_probe_device+0x460/0x460
[ 5.530620] bus_for_each_dev+0x60/0xa0
[ 5.530658] driver_attach+0x1e/0x20
[ 5.530693] bus_add_driver+0x170/0x270
[ 5.530731] driver_register+0x60/0xe0
[ 5.530769] ? 0xffffffffa01cb000
[ 5.530803] __platform_driver_register+0x36/0x40
[ 5.530851] hdmi_lpe_audio_driver_init+0x17/0x1000 [snd_hdmi_lpe_audio]
[ 5.530915] do_one_initcall+0x43/0x180
[ 5.530956] ? __vunmap+0x81/0xd0
[ 5.530991] ? kfree+0x14c/0x160
[ 5.531024] ? kmem_cache_alloc_trace+0x38/0x150
[ 5.531070] do_init_module+0x5f/0x1f8
[ 5.531108] load_module+0x271e/0x2bd0
[ 5.531147] ? kernel_read_file+0x1a3/0x1c0
[ 5.531188] SYSC_finit_module+0xbc/0xf0
[ 5.531226] ? SYSC_finit_module+0xbc/0xf0
[ 5.531267] SyS_finit_module+0xe/0x10
[ 5.531305] do_syscall_64+0x6e/0x180
[ 5.531345] entry_SYSCALL64_slow_path+0x25/0x25
[ 5.531389] RIP: 0033:0x7f627b5fbbf9
[ 5.531424] RSP: 002b:00007ffe053eee68 EFLAGS: 00000246 ORIG_RAX:
0000000000000139
[ 5.531493] RAX: ffffffffffffffda RBX: 000055d6c745b690 RCX:
00007f627b5fbbf9
[ 5.531558] RDX: 0000000000000000 RSI: 00007f627c134995 RDI:
0000000000000007
[ 5.531622] RBP: 00007f627c134995 R08: 0000000000000000 R09:
00007ffe053eef80
[ 5.531687] R10: 0000000000000007 R11: 0000000000000246 R12:
0000000000000000
[ 5.531751] R13: 000055d6c7459ae0 R14: 0000000000020000 R15:
000055d6c745b690
[ 5.531816] Code: 48 8b 45 b0 8b 48 18 e8 e0 cb 22 e1 49 8b 44 24 28
4a 8d 8c 33 58 01 00 00 48 8d 75 b8 45 31 c9 41 b8 01 00 00 00 ba 14 00
00 00 <48> 8b 38 e8 a9 2a ff ff 85 c0 0f 88 09 01 00 00 49 8b 84 24 58
[ 5.532026] RIP: hdmi_lpe_audio_probe+0x40f/0x650
[snd_hdmi_lpe_audio] RSP: ffffc90000bffaf0
[ 5.532101] CR2: 0000000000000000
[ 5.532168] ---[ end trace e832e97f0e744700 ]---
[ 5.534830] i2c i2c-8: i2c read failed
[ 5.554395] i2c i2c-8: i2c read failed
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [alsa-devel] [PATCH v2 00/11] drm/i915: LPE audio runtime PM and multipipe (v2)
2017-04-28 17:10 ` [alsa-devel] " Pierre-Louis Bossart
@ 2017-04-28 19:37 ` Ville Syrjälä
2017-05-02 1:29 ` Pierre-Louis Bossart
0 siblings, 1 reply; 27+ messages in thread
From: Ville Syrjälä @ 2017-04-28 19:37 UTC (permalink / raw)
To: Pierre-Louis Bossart; +Cc: Takashi Iwai, intel-gfx, alsa-devel
On Fri, Apr 28, 2017 at 12:10:31PM -0500, Pierre-Louis Bossart wrote:
>
>
> On 04/28/2017 03:41 AM, Takashi Iwai wrote:
> > On Thu, 27 Apr 2017 18:02:19 +0200,
> > ville.syrjala@linux.intel.com wrote:
> >> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>
> >> Okay, here's the second attempt at getting multiple pipes playing back
> >> audio on the VLV/CHV HDMI LPE audio device. The main change from v1 is
> >> that now the PCM devices are associated with ports instead of pipes,
> >> so the audio from one device always gets output on the same display.
> >>
> >> I've also tacked on the alsa-lib conf update. No clue whether it's
> >> really correct or not (the config language isn't a close friend
> >> of mine).
> >>
> >> BTW I did notice that with LPE audio all the controls say iface=PCM,
> >> whereas on HDA a bunch of them say iface=MIXER. No idea if that's
> >> OK or not, just something I spotted when I was comparing the results
> >> with HDA.
> > We generally accept both iface types for IEC958 stuff, since
> > historically many drivers have already mixed them up. So it's no
> > problem :)
> >
> >
> >> Entire series available here:
> >> git://github.com/vsyrjala/linux.git lpe_audio_multipipe_2
> >>
> >> Cc: Takashi Iwai <tiwai@suse.de>
> >> Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> > All look good, and feel free to take my reviewed-by tag
> > Reviewed-by: Takashi Iwai <tiwai@suse.de>
> >
> > As said previously, my only slight concern is the compatibility.
> > But, in the current situation with PulseAudio, only few people would
> > use this driver, so it shouldn't be so big impact, I suppose.
> >
> > BTW, which port is used in general on BYT/CHT?
> >
> > Oh, also, I suppose you want to carry these over i915 tree?
> > I don't mind either way, I can take them through sound tree if
> > preferred, too.
> I see frequent oops on startup with this lpe_audio_multipipe_2 branch
> with my CHT device not booting or no HDMI audio device created.
> Not sure if these issues are due to the new patches or to the rest of
> the drm code?
>
> [ 5.529023] BUG: unable to handle kernel NULL pointer dereference
> at (null)
> [ 5.529143] IP: hdmi_lpe_audio_probe+0x40f/0x650 [snd_hdmi_lpe_audio]
> [ 5.529202] PGD 0
>
> [ 5.529242] Oops: 0000 [#1] SMP
> [ 5.529274] Modules linked in: snd_soc_sst_atom_hifi2_platform
> snd_soc_sst_match snd_soc_core snd_compress lpc_ich snd_seq
> snd_seq_device shpchp snd_hdmi_lpe_audio(+) snd_pcm snd_timer dw_dmac
> snd soundcore i2c_designware_platform(+) i2c_designware_core
> spi_pxa2xx_platform acpi_pad mac_hid nfsd auth_rpcgss nfs_acl lockd
> grace sunrpc ip_tables x_tables hid_generic mmc_block i2c_hid usbhid hid
> autofs4
> [ 5.529605] CPU: 2 PID: 512 Comm: systemd-udevd Not tainted
> 4.11.0-rc8-test+ #11
> [ 5.529671] Hardware name: ZOTAC XXXXXX/Cherry Trail FFD, BIOS 5.11
> 09/28/2016
> [ 5.529736] task: ffff88007485b780 task.stack: ffffc90000bfc000
> [ 5.529793] RIP: 0010:hdmi_lpe_audio_probe+0x40f/0x650
> [snd_hdmi_lpe_audio]
> [ 5.529855] RSP: 0018:ffffc90000bffaf0 EFLAGS: 00010246
> [ 5.529904] RAX: 0000000000000000 RBX: ffff880079209898 RCX:
> ffff88007920f078
> [ 5.529967] RDX: 0000000000000014 RSI: ffffc90000bffb28 RDI:
> 0000000000000002
> [ 5.530031] RBP: ffffc90000bffb70 R08: 0000000000000001 R09:
> 0000000000000000
> [ 5.530094] R10: ffff88007441bf00 R11: ffffc90000bffb36 R12:
> ffff88007920ef20
> [ 5.530159] R13: ffff88007920ef48 R14: 0000000000005688 R15:
> 0000000000000047
> [ 5.530225] FS: 00007f627c988640(0000) GS:ffff88007b300000(0000)
> knlGS:0000000000000000
> [ 5.530299] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 5.530352] CR2: 0000000000000000 CR3: 0000000078cb8000 CR4:
> 00000000001006e0
> [ 5.530416] Call Trace:
> [ 5.530452] platform_drv_probe+0x3b/0xa0
> [ 5.530494] driver_probe_device+0x2bb/0x460
> [ 5.530538] __driver_attach+0xdf/0xf0
> [ 5.530576] ? driver_probe_device+0x460/0x460
> [ 5.530620] bus_for_each_dev+0x60/0xa0
> [ 5.530658] driver_attach+0x1e/0x20
> [ 5.530693] bus_add_driver+0x170/0x270
> [ 5.530731] driver_register+0x60/0xe0
> [ 5.530769] ? 0xffffffffa01cb000
> [ 5.530803] __platform_driver_register+0x36/0x40
> [ 5.530851] hdmi_lpe_audio_driver_init+0x17/0x1000 [snd_hdmi_lpe_audio]
> [ 5.530915] do_one_initcall+0x43/0x180
> [ 5.530956] ? __vunmap+0x81/0xd0
> [ 5.530991] ? kfree+0x14c/0x160
> [ 5.531024] ? kmem_cache_alloc_trace+0x38/0x150
> [ 5.531070] do_init_module+0x5f/0x1f8
> [ 5.531108] load_module+0x271e/0x2bd0
> [ 5.531147] ? kernel_read_file+0x1a3/0x1c0
> [ 5.531188] SYSC_finit_module+0xbc/0xf0
> [ 5.531226] ? SYSC_finit_module+0xbc/0xf0
> [ 5.531267] SyS_finit_module+0xe/0x10
> [ 5.531305] do_syscall_64+0x6e/0x180
> [ 5.531345] entry_SYSCALL64_slow_path+0x25/0x25
> [ 5.531389] RIP: 0033:0x7f627b5fbbf9
> [ 5.531424] RSP: 002b:00007ffe053eee68 EFLAGS: 00000246 ORIG_RAX:
> 0000000000000139
> [ 5.531493] RAX: ffffffffffffffda RBX: 000055d6c745b690 RCX:
> 00007f627b5fbbf9
> [ 5.531558] RDX: 0000000000000000 RSI: 00007f627c134995 RDI:
> 0000000000000007
> [ 5.531622] RBP: 00007f627c134995 R08: 0000000000000000 R09:
> 00007ffe053eef80
> [ 5.531687] R10: 0000000000000007 R11: 0000000000000246 R12:
> 0000000000000000
> [ 5.531751] R13: 000055d6c7459ae0 R14: 0000000000020000 R15:
> 000055d6c745b690
> [ 5.531816] Code: 48 8b 45 b0 8b 48 18 e8 e0 cb 22 e1 49 8b 44 24 28
> 4a 8d 8c 33 58 01 00 00 48 8d 75 b8 45 31 c9 41 b8 01 00 00 00 ba 14 00
> 00 00 <48> 8b 38 e8 a9 2a ff ff 85 c0 0f 88 09 01 00 00 49 8b 84 24 58
Disassembling that I get:
4005a0: 48 8b 45 b0 mov -0x50(%rbp),%rax
4005a4: 8b 48 18 mov 0x18(%rax),%ecx
4005a7: e8 e0 cb 22 e1 callq ffffffffe162d18c <_end+0xffffffffe102c14c>
4005ac: 49 8b 44 24 28 mov 0x28(%r12),%rax
4005b1: 4a 8d 8c 33 58 01 00 lea 0x158(%rbx,%r14,1),%rcx
4005b8: 00
4005b9: 48 8d 75 b8 lea -0x48(%rbp),%rsi
4005bd: 45 31 c9 xor %r9d,%r9d
4005c0: 41 b8 01 00 00 00 mov $0x1,%r8d
4005c6: ba 14 00 00 00 mov $0x14,%edx
4005cb: 48 8b 38 mov (%rax),%rdi
4005ce: e8 a9 2a ff ff callq 3f307c <_init-0xd32c>
4005d3: 85 c0 test %eax,%eax
4005d5: 0f 88 09 01 00 00 js 4006e4 <__GNU_EH_FRAME_HDR+0x100>
4005db: 49 rex.WB
4005dc: 8b .byte 0x8b
4005dd: 84 24 58 test %ah,(%rax,%rbx,2)
Comparing that with the disassembly from my build, that
first call looks like the snprintf() and the second one would
then be snd_jack_new(). So it seems to blow in the
ctx->card_ctx->card dereference. But I don't really see how
a NULL pointer could sneak in there.
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [alsa-devel] [PATCH v2 00/11] drm/i915: LPE audio runtime PM and multipipe (v2)
2017-04-28 19:37 ` Ville Syrjälä
@ 2017-05-02 1:29 ` Pierre-Louis Bossart
2017-05-02 18:27 ` Ville Syrjälä
0 siblings, 1 reply; 27+ messages in thread
From: Pierre-Louis Bossart @ 2017-05-02 1:29 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: Takashi Iwai, intel-gfx, alsa-devel
On 04/28/2017 02:37 PM, Ville Syrjälä wrote:
> On Fri, Apr 28, 2017 at 12:10:31PM -0500, Pierre-Louis Bossart wrote:
>>
>> On 04/28/2017 03:41 AM, Takashi Iwai wrote:
>>> On Thu, 27 Apr 2017 18:02:19 +0200,
>>> ville.syrjala@linux.intel.com wrote:
>>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>>
>>>> Okay, here's the second attempt at getting multiple pipes playing back
>>>> audio on the VLV/CHV HDMI LPE audio device. The main change from v1 is
>>>> that now the PCM devices are associated with ports instead of pipes,
>>>> so the audio from one device always gets output on the same display.
>>>>
>>>> I've also tacked on the alsa-lib conf update. No clue whether it's
>>>> really correct or not (the config language isn't a close friend
>>>> of mine).
>>>>
>>>> BTW I did notice that with LPE audio all the controls say iface=PCM,
>>>> whereas on HDA a bunch of them say iface=MIXER. No idea if that's
>>>> OK or not, just something I spotted when I was comparing the results
>>>> with HDA.
>>> We generally accept both iface types for IEC958 stuff, since
>>> historically many drivers have already mixed them up. So it's no
>>> problem :)
>>>
>>>
>>>> Entire series available here:
>>>> git://github.com/vsyrjala/linux.git lpe_audio_multipipe_2
>>>>
>>>> Cc: Takashi Iwai <tiwai@suse.de>
>>>> Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
>>> All look good, and feel free to take my reviewed-by tag
>>> Reviewed-by: Takashi Iwai <tiwai@suse.de>
>>>
>>> As said previously, my only slight concern is the compatibility.
>>> But, in the current situation with PulseAudio, only few people would
>>> use this driver, so it shouldn't be so big impact, I suppose.
>>>
>>> BTW, which port is used in general on BYT/CHT?
>>>
>>> Oh, also, I suppose you want to carry these over i915 tree?
>>> I don't mind either way, I can take them through sound tree if
>>> preferred, too.
>> I see frequent oops on startup with this lpe_audio_multipipe_2 branch
>> with my CHT device not booting or no HDMI audio device created.
>> Not sure if these issues are due to the new patches or to the rest of
>> the drm code?
>>
>> [ 5.529023] BUG: unable to handle kernel NULL pointer dereference
>> at (null)
>> [ 5.529143] IP: hdmi_lpe_audio_probe+0x40f/0x650 [snd_hdmi_lpe_audio]
>> [ 5.529202] PGD 0
>>
>> [ 5.529242] Oops: 0000 [#1] SMP
>> [ 5.529274] Modules linked in: snd_soc_sst_atom_hifi2_platform
>> snd_soc_sst_match snd_soc_core snd_compress lpc_ich snd_seq
>> snd_seq_device shpchp snd_hdmi_lpe_audio(+) snd_pcm snd_timer dw_dmac
>> snd soundcore i2c_designware_platform(+) i2c_designware_core
>> spi_pxa2xx_platform acpi_pad mac_hid nfsd auth_rpcgss nfs_acl lockd
>> grace sunrpc ip_tables x_tables hid_generic mmc_block i2c_hid usbhid hid
>> autofs4
>> [ 5.529605] CPU: 2 PID: 512 Comm: systemd-udevd Not tainted
>> 4.11.0-rc8-test+ #11
>> [ 5.529671] Hardware name: ZOTAC XXXXXX/Cherry Trail FFD, BIOS 5.11
>> 09/28/2016
>> [ 5.529736] task: ffff88007485b780 task.stack: ffffc90000bfc000
>> [ 5.529793] RIP: 0010:hdmi_lpe_audio_probe+0x40f/0x650
>> [snd_hdmi_lpe_audio]
>> [ 5.529855] RSP: 0018:ffffc90000bffaf0 EFLAGS: 00010246
>> [ 5.529904] RAX: 0000000000000000 RBX: ffff880079209898 RCX:
>> ffff88007920f078
>> [ 5.529967] RDX: 0000000000000014 RSI: ffffc90000bffb28 RDI:
>> 0000000000000002
>> [ 5.530031] RBP: ffffc90000bffb70 R08: 0000000000000001 R09:
>> 0000000000000000
>> [ 5.530094] R10: ffff88007441bf00 R11: ffffc90000bffb36 R12:
>> ffff88007920ef20
>> [ 5.530159] R13: ffff88007920ef48 R14: 0000000000005688 R15:
>> 0000000000000047
>> [ 5.530225] FS: 00007f627c988640(0000) GS:ffff88007b300000(0000)
>> knlGS:0000000000000000
>> [ 5.530299] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> [ 5.530352] CR2: 0000000000000000 CR3: 0000000078cb8000 CR4:
>> 00000000001006e0
>> [ 5.530416] Call Trace:
>> [ 5.530452] platform_drv_probe+0x3b/0xa0
>> [ 5.530494] driver_probe_device+0x2bb/0x460
>> [ 5.530538] __driver_attach+0xdf/0xf0
>> [ 5.530576] ? driver_probe_device+0x460/0x460
>> [ 5.530620] bus_for_each_dev+0x60/0xa0
>> [ 5.530658] driver_attach+0x1e/0x20
>> [ 5.530693] bus_add_driver+0x170/0x270
>> [ 5.530731] driver_register+0x60/0xe0
>> [ 5.530769] ? 0xffffffffa01cb000
>> [ 5.530803] __platform_driver_register+0x36/0x40
>> [ 5.530851] hdmi_lpe_audio_driver_init+0x17/0x1000 [snd_hdmi_lpe_audio]
>> [ 5.530915] do_one_initcall+0x43/0x180
>> [ 5.530956] ? __vunmap+0x81/0xd0
>> [ 5.530991] ? kfree+0x14c/0x160
>> [ 5.531024] ? kmem_cache_alloc_trace+0x38/0x150
>> [ 5.531070] do_init_module+0x5f/0x1f8
>> [ 5.531108] load_module+0x271e/0x2bd0
>> [ 5.531147] ? kernel_read_file+0x1a3/0x1c0
>> [ 5.531188] SYSC_finit_module+0xbc/0xf0
>> [ 5.531226] ? SYSC_finit_module+0xbc/0xf0
>> [ 5.531267] SyS_finit_module+0xe/0x10
>> [ 5.531305] do_syscall_64+0x6e/0x180
>> [ 5.531345] entry_SYSCALL64_slow_path+0x25/0x25
>> [ 5.531389] RIP: 0033:0x7f627b5fbbf9
>> [ 5.531424] RSP: 002b:00007ffe053eee68 EFLAGS: 00000246 ORIG_RAX:
>> 0000000000000139
>> [ 5.531493] RAX: ffffffffffffffda RBX: 000055d6c745b690 RCX:
>> 00007f627b5fbbf9
>> [ 5.531558] RDX: 0000000000000000 RSI: 00007f627c134995 RDI:
>> 0000000000000007
>> [ 5.531622] RBP: 00007f627c134995 R08: 0000000000000000 R09:
>> 00007ffe053eef80
>> [ 5.531687] R10: 0000000000000007 R11: 0000000000000246 R12:
>> 0000000000000000
>> [ 5.531751] R13: 000055d6c7459ae0 R14: 0000000000020000 R15:
>> 000055d6c745b690
>> [ 5.531816] Code: 48 8b 45 b0 8b 48 18 e8 e0 cb 22 e1 49 8b 44 24 28
>> 4a 8d 8c 33 58 01 00 00 48 8d 75 b8 45 31 c9 41 b8 01 00 00 00 ba 14 00
>> 00 00 <48> 8b 38 e8 a9 2a ff ff 85 c0 0f 88 09 01 00 00 49 8b 84 24 58
> Disassembling that I get:
>
> 4005a0: 48 8b 45 b0 mov -0x50(%rbp),%rax
> 4005a4: 8b 48 18 mov 0x18(%rax),%ecx
> 4005a7: e8 e0 cb 22 e1 callq ffffffffe162d18c <_end+0xffffffffe102c14c>
> 4005ac: 49 8b 44 24 28 mov 0x28(%r12),%rax
> 4005b1: 4a 8d 8c 33 58 01 00 lea 0x158(%rbx,%r14,1),%rcx
> 4005b8: 00
> 4005b9: 48 8d 75 b8 lea -0x48(%rbp),%rsi
> 4005bd: 45 31 c9 xor %r9d,%r9d
> 4005c0: 41 b8 01 00 00 00 mov $0x1,%r8d
> 4005c6: ba 14 00 00 00 mov $0x14,%edx
> 4005cb: 48 8b 38 mov (%rax),%rdi
> 4005ce: e8 a9 2a ff ff callq 3f307c <_init-0xd32c>
> 4005d3: 85 c0 test %eax,%eax
> 4005d5: 0f 88 09 01 00 00 js 4006e4 <__GNU_EH_FRAME_HDR+0x100>
> 4005db: 49 rex.WB
> 4005dc: 8b .byte 0x8b
> 4005dd: 84 24 58 test %ah,(%rax,%rbx,2)
>
> Comparing that with the disassembly from my build, that
> first call looks like the snprintf() and the second one would
> then be snd_jack_new(). So it seems to blow in the
> ctx->card_ctx->card dereference. But I don't really see how
> a NULL pointer could sneak in there.
There was an standard PORT_B off-by-one error which resulted in memory
corruption. the patch below seems to fix the problem, I can get the same
functionality as before with concurrent playback on HDMI and DP, and the
jack values are correctly set.
diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c
b/drivers/gpu/drm/i915/intel_lpe_audio.c
index fa728ed..3bf6528 100644
--- a/drivers/gpu/drm/i915/intel_lpe_audio.c
+++ b/drivers/gpu/drm/i915/intel_lpe_audio.c
@@ -332,7 +332,7 @@ void intel_lpe_audio_notify(struct drm_i915_private
*dev_priv,
return;
pdata = dev_get_platdata(&dev_priv->lpe_audio.platdev->dev);
- ppdata = &pdata->port[port];
+ ppdata = &pdata->port[port - PORT_B];
spin_lock_irqsave(&pdata->lpe_audio_slock, irqflags);
@@ -359,7 +359,7 @@ void intel_lpe_audio_notify(struct drm_i915_private
*dev_priv,
}
if (pdata->notify_audio_lpe)
- pdata->notify_audio_lpe(dev_priv->lpe_audio.platdev, port);
+ pdata->notify_audio_lpe(dev_priv->lpe_audio.platdev, port - PORT_B);
spin_unlock_irqrestore(&pdata->lpe_audio_slock, irqflags);
}
diff --git a/sound/x86/intel_hdmi_audio.c b/sound/x86/intel_hdmi_audio.c
index 909391d..0153439 100644
--- a/sound/x86/intel_hdmi_audio.c
+++ b/sound/x86/intel_hdmi_audio.c
@@ -1579,7 +1579,7 @@ static irqreturn_t
display_pipe_interrupt_handler(int irq, void *dev_id)
static void notify_audio_lpe(struct platform_device *pdev, int port)
{
struct snd_intelhad_card *card_ctx = platform_get_drvdata(pdev);
- struct snd_intelhad *ctx = &card_ctx->pcm_ctx[port];
+ struct snd_intelhad *ctx = &card_ctx->pcm_ctx[port]; /* warning:
offset with -PORT_B */
schedule_work(&ctx->hdmi_audio_wq);
}
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [alsa-devel] [PATCH v2 00/11] drm/i915: LPE audio runtime PM and multipipe (v2)
2017-05-02 1:29 ` Pierre-Louis Bossart
@ 2017-05-02 18:27 ` Ville Syrjälä
2017-05-02 20:15 ` Pierre-Louis Bossart
0 siblings, 1 reply; 27+ messages in thread
From: Ville Syrjälä @ 2017-05-02 18:27 UTC (permalink / raw)
To: Pierre-Louis Bossart; +Cc: Takashi Iwai, intel-gfx, alsa-devel
On Mon, May 01, 2017 at 08:29:10PM -0500, Pierre-Louis Bossart wrote:
>
>
> On 04/28/2017 02:37 PM, Ville Syrjälä wrote:
> > On Fri, Apr 28, 2017 at 12:10:31PM -0500, Pierre-Louis Bossart wrote:
> >>
> >> On 04/28/2017 03:41 AM, Takashi Iwai wrote:
> >>> On Thu, 27 Apr 2017 18:02:19 +0200,
> >>> ville.syrjala@linux.intel.com wrote:
> >>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>>>
> >>>> Okay, here's the second attempt at getting multiple pipes playing back
> >>>> audio on the VLV/CHV HDMI LPE audio device. The main change from v1 is
> >>>> that now the PCM devices are associated with ports instead of pipes,
> >>>> so the audio from one device always gets output on the same display.
> >>>>
> >>>> I've also tacked on the alsa-lib conf update. No clue whether it's
> >>>> really correct or not (the config language isn't a close friend
> >>>> of mine).
> >>>>
> >>>> BTW I did notice that with LPE audio all the controls say iface=PCM,
> >>>> whereas on HDA a bunch of them say iface=MIXER. No idea if that's
> >>>> OK or not, just something I spotted when I was comparing the results
> >>>> with HDA.
> >>> We generally accept both iface types for IEC958 stuff, since
> >>> historically many drivers have already mixed them up. So it's no
> >>> problem :)
> >>>
> >>>
> >>>> Entire series available here:
> >>>> git://github.com/vsyrjala/linux.git lpe_audio_multipipe_2
> >>>>
> >>>> Cc: Takashi Iwai <tiwai@suse.de>
> >>>> Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> >>> All look good, and feel free to take my reviewed-by tag
> >>> Reviewed-by: Takashi Iwai <tiwai@suse.de>
> >>>
> >>> As said previously, my only slight concern is the compatibility.
> >>> But, in the current situation with PulseAudio, only few people would
> >>> use this driver, so it shouldn't be so big impact, I suppose.
> >>>
> >>> BTW, which port is used in general on BYT/CHT?
> >>>
> >>> Oh, also, I suppose you want to carry these over i915 tree?
> >>> I don't mind either way, I can take them through sound tree if
> >>> preferred, too.
> >> I see frequent oops on startup with this lpe_audio_multipipe_2 branch
> >> with my CHT device not booting or no HDMI audio device created.
> >> Not sure if these issues are due to the new patches or to the rest of
> >> the drm code?
> >>
> >> [ 5.529023] BUG: unable to handle kernel NULL pointer dereference
> >> at (null)
> >> [ 5.529143] IP: hdmi_lpe_audio_probe+0x40f/0x650 [snd_hdmi_lpe_audio]
> >> [ 5.529202] PGD 0
> >>
> >> [ 5.529242] Oops: 0000 [#1] SMP
> >> [ 5.529274] Modules linked in: snd_soc_sst_atom_hifi2_platform
> >> snd_soc_sst_match snd_soc_core snd_compress lpc_ich snd_seq
> >> snd_seq_device shpchp snd_hdmi_lpe_audio(+) snd_pcm snd_timer dw_dmac
> >> snd soundcore i2c_designware_platform(+) i2c_designware_core
> >> spi_pxa2xx_platform acpi_pad mac_hid nfsd auth_rpcgss nfs_acl lockd
> >> grace sunrpc ip_tables x_tables hid_generic mmc_block i2c_hid usbhid hid
> >> autofs4
> >> [ 5.529605] CPU: 2 PID: 512 Comm: systemd-udevd Not tainted
> >> 4.11.0-rc8-test+ #11
> >> [ 5.529671] Hardware name: ZOTAC XXXXXX/Cherry Trail FFD, BIOS 5.11
> >> 09/28/2016
> >> [ 5.529736] task: ffff88007485b780 task.stack: ffffc90000bfc000
> >> [ 5.529793] RIP: 0010:hdmi_lpe_audio_probe+0x40f/0x650
> >> [snd_hdmi_lpe_audio]
> >> [ 5.529855] RSP: 0018:ffffc90000bffaf0 EFLAGS: 00010246
> >> [ 5.529904] RAX: 0000000000000000 RBX: ffff880079209898 RCX:
> >> ffff88007920f078
> >> [ 5.529967] RDX: 0000000000000014 RSI: ffffc90000bffb28 RDI:
> >> 0000000000000002
> >> [ 5.530031] RBP: ffffc90000bffb70 R08: 0000000000000001 R09:
> >> 0000000000000000
> >> [ 5.530094] R10: ffff88007441bf00 R11: ffffc90000bffb36 R12:
> >> ffff88007920ef20
> >> [ 5.530159] R13: ffff88007920ef48 R14: 0000000000005688 R15:
> >> 0000000000000047
> >> [ 5.530225] FS: 00007f627c988640(0000) GS:ffff88007b300000(0000)
> >> knlGS:0000000000000000
> >> [ 5.530299] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> >> [ 5.530352] CR2: 0000000000000000 CR3: 0000000078cb8000 CR4:
> >> 00000000001006e0
> >> [ 5.530416] Call Trace:
> >> [ 5.530452] platform_drv_probe+0x3b/0xa0
> >> [ 5.530494] driver_probe_device+0x2bb/0x460
> >> [ 5.530538] __driver_attach+0xdf/0xf0
> >> [ 5.530576] ? driver_probe_device+0x460/0x460
> >> [ 5.530620] bus_for_each_dev+0x60/0xa0
> >> [ 5.530658] driver_attach+0x1e/0x20
> >> [ 5.530693] bus_add_driver+0x170/0x270
> >> [ 5.530731] driver_register+0x60/0xe0
> >> [ 5.530769] ? 0xffffffffa01cb000
> >> [ 5.530803] __platform_driver_register+0x36/0x40
> >> [ 5.530851] hdmi_lpe_audio_driver_init+0x17/0x1000 [snd_hdmi_lpe_audio]
> >> [ 5.530915] do_one_initcall+0x43/0x180
> >> [ 5.530956] ? __vunmap+0x81/0xd0
> >> [ 5.530991] ? kfree+0x14c/0x160
> >> [ 5.531024] ? kmem_cache_alloc_trace+0x38/0x150
> >> [ 5.531070] do_init_module+0x5f/0x1f8
> >> [ 5.531108] load_module+0x271e/0x2bd0
> >> [ 5.531147] ? kernel_read_file+0x1a3/0x1c0
> >> [ 5.531188] SYSC_finit_module+0xbc/0xf0
> >> [ 5.531226] ? SYSC_finit_module+0xbc/0xf0
> >> [ 5.531267] SyS_finit_module+0xe/0x10
> >> [ 5.531305] do_syscall_64+0x6e/0x180
> >> [ 5.531345] entry_SYSCALL64_slow_path+0x25/0x25
> >> [ 5.531389] RIP: 0033:0x7f627b5fbbf9
> >> [ 5.531424] RSP: 002b:00007ffe053eee68 EFLAGS: 00000246 ORIG_RAX:
> >> 0000000000000139
> >> [ 5.531493] RAX: ffffffffffffffda RBX: 000055d6c745b690 RCX:
> >> 00007f627b5fbbf9
> >> [ 5.531558] RDX: 0000000000000000 RSI: 00007f627c134995 RDI:
> >> 0000000000000007
> >> [ 5.531622] RBP: 00007f627c134995 R08: 0000000000000000 R09:
> >> 00007ffe053eef80
> >> [ 5.531687] R10: 0000000000000007 R11: 0000000000000246 R12:
> >> 0000000000000000
> >> [ 5.531751] R13: 000055d6c7459ae0 R14: 0000000000020000 R15:
> >> 000055d6c745b690
> >> [ 5.531816] Code: 48 8b 45 b0 8b 48 18 e8 e0 cb 22 e1 49 8b 44 24 28
> >> 4a 8d 8c 33 58 01 00 00 48 8d 75 b8 45 31 c9 41 b8 01 00 00 00 ba 14 00
> >> 00 00 <48> 8b 38 e8 a9 2a ff ff 85 c0 0f 88 09 01 00 00 49 8b 84 24 58
> > Disassembling that I get:
> >
> > 4005a0: 48 8b 45 b0 mov -0x50(%rbp),%rax
> > 4005a4: 8b 48 18 mov 0x18(%rax),%ecx
> > 4005a7: e8 e0 cb 22 e1 callq ffffffffe162d18c <_end+0xffffffffe102c14c>
> > 4005ac: 49 8b 44 24 28 mov 0x28(%r12),%rax
> > 4005b1: 4a 8d 8c 33 58 01 00 lea 0x158(%rbx,%r14,1),%rcx
> > 4005b8: 00
> > 4005b9: 48 8d 75 b8 lea -0x48(%rbp),%rsi
> > 4005bd: 45 31 c9 xor %r9d,%r9d
> > 4005c0: 41 b8 01 00 00 00 mov $0x1,%r8d
> > 4005c6: ba 14 00 00 00 mov $0x14,%edx
> > 4005cb: 48 8b 38 mov (%rax),%rdi
> > 4005ce: e8 a9 2a ff ff callq 3f307c <_init-0xd32c>
> > 4005d3: 85 c0 test %eax,%eax
> > 4005d5: 0f 88 09 01 00 00 js 4006e4 <__GNU_EH_FRAME_HDR+0x100>
> > 4005db: 49 rex.WB
> > 4005dc: 8b .byte 0x8b
> > 4005dd: 84 24 58 test %ah,(%rax,%rbx,2)
> >
> > Comparing that with the disassembly from my build, that
> > first call looks like the snprintf() and the second one would
> > then be snd_jack_new(). So it seems to blow in the
> > ctx->card_ctx->card dereference. But I don't really see how
> > a NULL pointer could sneak in there.
>
> There was an standard PORT_B off-by-one error which resulted in memory
> corruption.
Doh.
> the patch below seems to fix the problem, I can get the same
> functionality as before with concurrent playback on HDMI and DP, and the
> jack values are correctly set.
Thanks. I do wonder a bit which side should apply this offset.
But I guess it doesn't really matter in the end as long as both
sides know what the deal is. So unless there are good reasons
for anything else I'll just squash this into my patch(es).
>
> diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c
> b/drivers/gpu/drm/i915/intel_lpe_audio.c
> index fa728ed..3bf6528 100644
> --- a/drivers/gpu/drm/i915/intel_lpe_audio.c
> +++ b/drivers/gpu/drm/i915/intel_lpe_audio.c
> @@ -332,7 +332,7 @@ void intel_lpe_audio_notify(struct drm_i915_private
> *dev_priv,
> return;
>
> pdata = dev_get_platdata(&dev_priv->lpe_audio.platdev->dev);
> - ppdata = &pdata->port[port];
> + ppdata = &pdata->port[port - PORT_B];
>
> spin_lock_irqsave(&pdata->lpe_audio_slock, irqflags);
>
> @@ -359,7 +359,7 @@ void intel_lpe_audio_notify(struct drm_i915_private
> *dev_priv,
> }
>
> if (pdata->notify_audio_lpe)
> - pdata->notify_audio_lpe(dev_priv->lpe_audio.platdev, port);
> + pdata->notify_audio_lpe(dev_priv->lpe_audio.platdev, port - PORT_B);
>
> spin_unlock_irqrestore(&pdata->lpe_audio_slock, irqflags);
> }
> diff --git a/sound/x86/intel_hdmi_audio.c b/sound/x86/intel_hdmi_audio.c
> index 909391d..0153439 100644
> --- a/sound/x86/intel_hdmi_audio.c
> +++ b/sound/x86/intel_hdmi_audio.c
> @@ -1579,7 +1579,7 @@ static irqreturn_t
> display_pipe_interrupt_handler(int irq, void *dev_id)
> static void notify_audio_lpe(struct platform_device *pdev, int port)
> {
> struct snd_intelhad_card *card_ctx = platform_get_drvdata(pdev);
> - struct snd_intelhad *ctx = &card_ctx->pcm_ctx[port];
> + struct snd_intelhad *ctx = &card_ctx->pcm_ctx[port]; /* warning:
> offset with -PORT_B */
>
> schedule_work(&ctx->hdmi_audio_wq);
> }
>
>
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [alsa-devel] [PATCH v2 00/11] drm/i915: LPE audio runtime PM and multipipe (v2)
2017-05-02 18:27 ` Ville Syrjälä
@ 2017-05-02 20:15 ` Pierre-Louis Bossart
2017-05-02 20:44 ` Takashi Iwai
0 siblings, 1 reply; 27+ messages in thread
From: Pierre-Louis Bossart @ 2017-05-02 20:15 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: Takashi Iwai, intel-gfx, alsa-devel
On 5/2/17 1:27 PM, Ville Syrjälä wrote:
> On Mon, May 01, 2017 at 08:29:10PM -0500, Pierre-Louis Bossart wrote:
>>
>>
>> On 04/28/2017 02:37 PM, Ville Syrjälä wrote:
>>> On Fri, Apr 28, 2017 at 12:10:31PM -0500, Pierre-Louis Bossart wrote:
>>>>
>>>> On 04/28/2017 03:41 AM, Takashi Iwai wrote:
>>>>> On Thu, 27 Apr 2017 18:02:19 +0200,
>>>>> ville.syrjala@linux.intel.com wrote:
>>>>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>>>>
>>>>>> Okay, here's the second attempt at getting multiple pipes playing back
>>>>>> audio on the VLV/CHV HDMI LPE audio device. The main change from v1 is
>>>>>> that now the PCM devices are associated with ports instead of pipes,
>>>>>> so the audio from one device always gets output on the same display.
>>>>>>
>>>>>> I've also tacked on the alsa-lib conf update. No clue whether it's
>>>>>> really correct or not (the config language isn't a close friend
>>>>>> of mine).
>>>>>>
>>>>>> BTW I did notice that with LPE audio all the controls say iface=PCM,
>>>>>> whereas on HDA a bunch of them say iface=MIXER. No idea if that's
>>>>>> OK or not, just something I spotted when I was comparing the results
>>>>>> with HDA.
>>>>> We generally accept both iface types for IEC958 stuff, since
>>>>> historically many drivers have already mixed them up. So it's no
>>>>> problem :)
>>>>>
>>>>>
>>>>>> Entire series available here:
>>>>>> git://github.com/vsyrjala/linux.git lpe_audio_multipipe_2
>>>>>>
>>>>>> Cc: Takashi Iwai <tiwai@suse.de>
>>>>>> Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
>>>>> All look good, and feel free to take my reviewed-by tag
>>>>> Reviewed-by: Takashi Iwai <tiwai@suse.de>
>>>>>
>>>>> As said previously, my only slight concern is the compatibility.
>>>>> But, in the current situation with PulseAudio, only few people would
>>>>> use this driver, so it shouldn't be so big impact, I suppose.
>>>>>
>>>>> BTW, which port is used in general on BYT/CHT?
>>>>>
>>>>> Oh, also, I suppose you want to carry these over i915 tree?
>>>>> I don't mind either way, I can take them through sound tree if
>>>>> preferred, too.
>>>> I see frequent oops on startup with this lpe_audio_multipipe_2 branch
>>>> with my CHT device not booting or no HDMI audio device created.
>>>> Not sure if these issues are due to the new patches or to the rest of
>>>> the drm code?
>>>>
>>>> [ 5.529023] BUG: unable to handle kernel NULL pointer dereference
>>>> at (null)
>>>> [ 5.529143] IP: hdmi_lpe_audio_probe+0x40f/0x650 [snd_hdmi_lpe_audio]
>>>> [ 5.529202] PGD 0
>>>>
>>>> [ 5.529242] Oops: 0000 [#1] SMP
>>>> [ 5.529274] Modules linked in: snd_soc_sst_atom_hifi2_platform
>>>> snd_soc_sst_match snd_soc_core snd_compress lpc_ich snd_seq
>>>> snd_seq_device shpchp snd_hdmi_lpe_audio(+) snd_pcm snd_timer dw_dmac
>>>> snd soundcore i2c_designware_platform(+) i2c_designware_core
>>>> spi_pxa2xx_platform acpi_pad mac_hid nfsd auth_rpcgss nfs_acl lockd
>>>> grace sunrpc ip_tables x_tables hid_generic mmc_block i2c_hid usbhid hid
>>>> autofs4
>>>> [ 5.529605] CPU: 2 PID: 512 Comm: systemd-udevd Not tainted
>>>> 4.11.0-rc8-test+ #11
>>>> [ 5.529671] Hardware name: ZOTAC XXXXXX/Cherry Trail FFD, BIOS 5.11
>>>> 09/28/2016
>>>> [ 5.529736] task: ffff88007485b780 task.stack: ffffc90000bfc000
>>>> [ 5.529793] RIP: 0010:hdmi_lpe_audio_probe+0x40f/0x650
>>>> [snd_hdmi_lpe_audio]
>>>> [ 5.529855] RSP: 0018:ffffc90000bffaf0 EFLAGS: 00010246
>>>> [ 5.529904] RAX: 0000000000000000 RBX: ffff880079209898 RCX:
>>>> ffff88007920f078
>>>> [ 5.529967] RDX: 0000000000000014 RSI: ffffc90000bffb28 RDI:
>>>> 0000000000000002
>>>> [ 5.530031] RBP: ffffc90000bffb70 R08: 0000000000000001 R09:
>>>> 0000000000000000
>>>> [ 5.530094] R10: ffff88007441bf00 R11: ffffc90000bffb36 R12:
>>>> ffff88007920ef20
>>>> [ 5.530159] R13: ffff88007920ef48 R14: 0000000000005688 R15:
>>>> 0000000000000047
>>>> [ 5.530225] FS: 00007f627c988640(0000) GS:ffff88007b300000(0000)
>>>> knlGS:0000000000000000
>>>> [ 5.530299] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>>>> [ 5.530352] CR2: 0000000000000000 CR3: 0000000078cb8000 CR4:
>>>> 00000000001006e0
>>>> [ 5.530416] Call Trace:
>>>> [ 5.530452] platform_drv_probe+0x3b/0xa0
>>>> [ 5.530494] driver_probe_device+0x2bb/0x460
>>>> [ 5.530538] __driver_attach+0xdf/0xf0
>>>> [ 5.530576] ? driver_probe_device+0x460/0x460
>>>> [ 5.530620] bus_for_each_dev+0x60/0xa0
>>>> [ 5.530658] driver_attach+0x1e/0x20
>>>> [ 5.530693] bus_add_driver+0x170/0x270
>>>> [ 5.530731] driver_register+0x60/0xe0
>>>> [ 5.530769] ? 0xffffffffa01cb000
>>>> [ 5.530803] __platform_driver_register+0x36/0x40
>>>> [ 5.530851] hdmi_lpe_audio_driver_init+0x17/0x1000 [snd_hdmi_lpe_audio]
>>>> [ 5.530915] do_one_initcall+0x43/0x180
>>>> [ 5.530956] ? __vunmap+0x81/0xd0
>>>> [ 5.530991] ? kfree+0x14c/0x160
>>>> [ 5.531024] ? kmem_cache_alloc_trace+0x38/0x150
>>>> [ 5.531070] do_init_module+0x5f/0x1f8
>>>> [ 5.531108] load_module+0x271e/0x2bd0
>>>> [ 5.531147] ? kernel_read_file+0x1a3/0x1c0
>>>> [ 5.531188] SYSC_finit_module+0xbc/0xf0
>>>> [ 5.531226] ? SYSC_finit_module+0xbc/0xf0
>>>> [ 5.531267] SyS_finit_module+0xe/0x10
>>>> [ 5.531305] do_syscall_64+0x6e/0x180
>>>> [ 5.531345] entry_SYSCALL64_slow_path+0x25/0x25
>>>> [ 5.531389] RIP: 0033:0x7f627b5fbbf9
>>>> [ 5.531424] RSP: 002b:00007ffe053eee68 EFLAGS: 00000246 ORIG_RAX:
>>>> 0000000000000139
>>>> [ 5.531493] RAX: ffffffffffffffda RBX: 000055d6c745b690 RCX:
>>>> 00007f627b5fbbf9
>>>> [ 5.531558] RDX: 0000000000000000 RSI: 00007f627c134995 RDI:
>>>> 0000000000000007
>>>> [ 5.531622] RBP: 00007f627c134995 R08: 0000000000000000 R09:
>>>> 00007ffe053eef80
>>>> [ 5.531687] R10: 0000000000000007 R11: 0000000000000246 R12:
>>>> 0000000000000000
>>>> [ 5.531751] R13: 000055d6c7459ae0 R14: 0000000000020000 R15:
>>>> 000055d6c745b690
>>>> [ 5.531816] Code: 48 8b 45 b0 8b 48 18 e8 e0 cb 22 e1 49 8b 44 24 28
>>>> 4a 8d 8c 33 58 01 00 00 48 8d 75 b8 45 31 c9 41 b8 01 00 00 00 ba 14 00
>>>> 00 00 <48> 8b 38 e8 a9 2a ff ff 85 c0 0f 88 09 01 00 00 49 8b 84 24 58
>>> Disassembling that I get:
>>>
>>> 4005a0: 48 8b 45 b0 mov -0x50(%rbp),%rax
>>> 4005a4: 8b 48 18 mov 0x18(%rax),%ecx
>>> 4005a7: e8 e0 cb 22 e1 callq ffffffffe162d18c <_end+0xffffffffe102c14c>
>>> 4005ac: 49 8b 44 24 28 mov 0x28(%r12),%rax
>>> 4005b1: 4a 8d 8c 33 58 01 00 lea 0x158(%rbx,%r14,1),%rcx
>>> 4005b8: 00
>>> 4005b9: 48 8d 75 b8 lea -0x48(%rbp),%rsi
>>> 4005bd: 45 31 c9 xor %r9d,%r9d
>>> 4005c0: 41 b8 01 00 00 00 mov $0x1,%r8d
>>> 4005c6: ba 14 00 00 00 mov $0x14,%edx
>>> 4005cb: 48 8b 38 mov (%rax),%rdi
>>> 4005ce: e8 a9 2a ff ff callq 3f307c <_init-0xd32c>
>>> 4005d3: 85 c0 test %eax,%eax
>>> 4005d5: 0f 88 09 01 00 00 js 4006e4 <__GNU_EH_FRAME_HDR+0x100>
>>> 4005db: 49 rex.WB
>>> 4005dc: 8b .byte 0x8b
>>> 4005dd: 84 24 58 test %ah,(%rax,%rbx,2)
>>>
>>> Comparing that with the disassembly from my build, that
>>> first call looks like the snprintf() and the second one would
>>> then be snd_jack_new(). So it seems to blow in the
>>> ctx->card_ctx->card dereference. But I don't really see how
>>> a NULL pointer could sneak in there.
>>
>> There was an standard PORT_B off-by-one error which resulted in memory
>> corruption.
>
> Doh.
>
>> the patch below seems to fix the problem, I can get the same
>> functionality as before with concurrent playback on HDMI and DP, and the
>> jack values are correctly set.
>
> Thanks. I do wonder a bit which side should apply this offset.
> But I guess it doesn't really matter in the end as long as both
> sides know what the deal is. So unless there are good reasons
> for anything else I'll just squash this into my patch(es).
Fine with me.
One open I still have is that it's not self-explanatory for the user to
figure out which output the devices correspond to, e.g. on my CHT device
HDMI is device 2 and DP is device 0.
I may be mistaken but in the previous pipe-based solution, audio
playback would progress at a normal rate even if nothing was connected.
With the port-based, it looks like playback doesn't progress if there is
nothing connected - speaker-test is stuck. Not sure if the open or
hw_params should fail in that case? One could argue that the user should
know better and check the jack status, but in reality I don't think any
userspace layer will ever do this.
>
>>
>> diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c
>> b/drivers/gpu/drm/i915/intel_lpe_audio.c
>> index fa728ed..3bf6528 100644
>> --- a/drivers/gpu/drm/i915/intel_lpe_audio.c
>> +++ b/drivers/gpu/drm/i915/intel_lpe_audio.c
>> @@ -332,7 +332,7 @@ void intel_lpe_audio_notify(struct drm_i915_private
>> *dev_priv,
>> return;
>>
>> pdata = dev_get_platdata(&dev_priv->lpe_audio.platdev->dev);
>> - ppdata = &pdata->port[port];
>> + ppdata = &pdata->port[port - PORT_B];
>>
>> spin_lock_irqsave(&pdata->lpe_audio_slock, irqflags);
>>
>> @@ -359,7 +359,7 @@ void intel_lpe_audio_notify(struct drm_i915_private
>> *dev_priv,
>> }
>>
>> if (pdata->notify_audio_lpe)
>> - pdata->notify_audio_lpe(dev_priv->lpe_audio.platdev, port);
>> + pdata->notify_audio_lpe(dev_priv->lpe_audio.platdev, port - PORT_B);
>>
>> spin_unlock_irqrestore(&pdata->lpe_audio_slock, irqflags);
>> }
>> diff --git a/sound/x86/intel_hdmi_audio.c b/sound/x86/intel_hdmi_audio.c
>> index 909391d..0153439 100644
>> --- a/sound/x86/intel_hdmi_audio.c
>> +++ b/sound/x86/intel_hdmi_audio.c
>> @@ -1579,7 +1579,7 @@ static irqreturn_t
>> display_pipe_interrupt_handler(int irq, void *dev_id)
>> static void notify_audio_lpe(struct platform_device *pdev, int port)
>> {
>> struct snd_intelhad_card *card_ctx = platform_get_drvdata(pdev);
>> - struct snd_intelhad *ctx = &card_ctx->pcm_ctx[port];
>> + struct snd_intelhad *ctx = &card_ctx->pcm_ctx[port]; /* warning:
>> offset with -PORT_B */
>>
>> schedule_work(&ctx->hdmi_audio_wq);
>> }
>>
>>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [alsa-devel] [PATCH v2 00/11] drm/i915: LPE audio runtime PM and multipipe (v2)
2017-05-02 20:15 ` Pierre-Louis Bossart
@ 2017-05-02 20:44 ` Takashi Iwai
0 siblings, 0 replies; 27+ messages in thread
From: Takashi Iwai @ 2017-05-02 20:44 UTC (permalink / raw)
To: Pierre-Louis Bossart; +Cc: intel-gfx, alsa-devel
On Tue, 02 May 2017 22:15:20 +0200,
Pierre-Louis Bossart wrote:
>
> On 5/2/17 1:27 PM, Ville Syrjälä wrote:
> > On Mon, May 01, 2017 at 08:29:10PM -0500, Pierre-Louis Bossart wrote:
> >>
> >>
> >> On 04/28/2017 02:37 PM, Ville Syrjälä wrote:
> >>> On Fri, Apr 28, 2017 at 12:10:31PM -0500, Pierre-Louis Bossart wrote:
> >>>>
> >>>> On 04/28/2017 03:41 AM, Takashi Iwai wrote:
> >>>>> On Thu, 27 Apr 2017 18:02:19 +0200,
> >>>>> ville.syrjala@linux.intel.com wrote:
> >>>>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>>>>>
> >>>>>> Okay, here's the second attempt at getting multiple pipes playing back
> >>>>>> audio on the VLV/CHV HDMI LPE audio device. The main change from v1 is
> >>>>>> that now the PCM devices are associated with ports instead of pipes,
> >>>>>> so the audio from one device always gets output on the same display.
> >>>>>>
> >>>>>> I've also tacked on the alsa-lib conf update. No clue whether it's
> >>>>>> really correct or not (the config language isn't a close friend
> >>>>>> of mine).
> >>>>>>
> >>>>>> BTW I did notice that with LPE audio all the controls say iface=PCM,
> >>>>>> whereas on HDA a bunch of them say iface=MIXER. No idea if that's
> >>>>>> OK or not, just something I spotted when I was comparing the results
> >>>>>> with HDA.
> >>>>> We generally accept both iface types for IEC958 stuff, since
> >>>>> historically many drivers have already mixed them up. So it's no
> >>>>> problem :)
> >>>>>
> >>>>>
> >>>>>> Entire series available here:
> >>>>>> git://github.com/vsyrjala/linux.git lpe_audio_multipipe_2
> >>>>>>
> >>>>>> Cc: Takashi Iwai <tiwai@suse.de>
> >>>>>> Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> >>>>> All look good, and feel free to take my reviewed-by tag
> >>>>> Reviewed-by: Takashi Iwai <tiwai@suse.de>
> >>>>>
> >>>>> As said previously, my only slight concern is the compatibility.
> >>>>> But, in the current situation with PulseAudio, only few people would
> >>>>> use this driver, so it shouldn't be so big impact, I suppose.
> >>>>>
> >>>>> BTW, which port is used in general on BYT/CHT?
> >>>>>
> >>>>> Oh, also, I suppose you want to carry these over i915 tree?
> >>>>> I don't mind either way, I can take them through sound tree if
> >>>>> preferred, too.
> >>>> I see frequent oops on startup with this lpe_audio_multipipe_2 branch
> >>>> with my CHT device not booting or no HDMI audio device created.
> >>>> Not sure if these issues are due to the new patches or to the rest of
> >>>> the drm code?
> >>>>
> >>>> [ 5.529023] BUG: unable to handle kernel NULL pointer dereference
> >>>> at (null)
> >>>> [ 5.529143] IP: hdmi_lpe_audio_probe+0x40f/0x650 [snd_hdmi_lpe_audio]
> >>>> [ 5.529202] PGD 0
> >>>>
> >>>> [ 5.529242] Oops: 0000 [#1] SMP
> >>>> [ 5.529274] Modules linked in: snd_soc_sst_atom_hifi2_platform
> >>>> snd_soc_sst_match snd_soc_core snd_compress lpc_ich snd_seq
> >>>> snd_seq_device shpchp snd_hdmi_lpe_audio(+) snd_pcm snd_timer dw_dmac
> >>>> snd soundcore i2c_designware_platform(+) i2c_designware_core
> >>>> spi_pxa2xx_platform acpi_pad mac_hid nfsd auth_rpcgss nfs_acl lockd
> >>>> grace sunrpc ip_tables x_tables hid_generic mmc_block i2c_hid usbhid hid
> >>>> autofs4
> >>>> [ 5.529605] CPU: 2 PID: 512 Comm: systemd-udevd Not tainted
> >>>> 4.11.0-rc8-test+ #11
> >>>> [ 5.529671] Hardware name: ZOTAC XXXXXX/Cherry Trail FFD, BIOS 5.11
> >>>> 09/28/2016
> >>>> [ 5.529736] task: ffff88007485b780 task.stack: ffffc90000bfc000
> >>>> [ 5.529793] RIP: 0010:hdmi_lpe_audio_probe+0x40f/0x650
> >>>> [snd_hdmi_lpe_audio]
> >>>> [ 5.529855] RSP: 0018:ffffc90000bffaf0 EFLAGS: 00010246
> >>>> [ 5.529904] RAX: 0000000000000000 RBX: ffff880079209898 RCX:
> >>>> ffff88007920f078
> >>>> [ 5.529967] RDX: 0000000000000014 RSI: ffffc90000bffb28 RDI:
> >>>> 0000000000000002
> >>>> [ 5.530031] RBP: ffffc90000bffb70 R08: 0000000000000001 R09:
> >>>> 0000000000000000
> >>>> [ 5.530094] R10: ffff88007441bf00 R11: ffffc90000bffb36 R12:
> >>>> ffff88007920ef20
> >>>> [ 5.530159] R13: ffff88007920ef48 R14: 0000000000005688 R15:
> >>>> 0000000000000047
> >>>> [ 5.530225] FS: 00007f627c988640(0000) GS:ffff88007b300000(0000)
> >>>> knlGS:0000000000000000
> >>>> [ 5.530299] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> >>>> [ 5.530352] CR2: 0000000000000000 CR3: 0000000078cb8000 CR4:
> >>>> 00000000001006e0
> >>>> [ 5.530416] Call Trace:
> >>>> [ 5.530452] platform_drv_probe+0x3b/0xa0
> >>>> [ 5.530494] driver_probe_device+0x2bb/0x460
> >>>> [ 5.530538] __driver_attach+0xdf/0xf0
> >>>> [ 5.530576] ? driver_probe_device+0x460/0x460
> >>>> [ 5.530620] bus_for_each_dev+0x60/0xa0
> >>>> [ 5.530658] driver_attach+0x1e/0x20
> >>>> [ 5.530693] bus_add_driver+0x170/0x270
> >>>> [ 5.530731] driver_register+0x60/0xe0
> >>>> [ 5.530769] ? 0xffffffffa01cb000
> >>>> [ 5.530803] __platform_driver_register+0x36/0x40
> >>>> [ 5.530851] hdmi_lpe_audio_driver_init+0x17/0x1000 [snd_hdmi_lpe_audio]
> >>>> [ 5.530915] do_one_initcall+0x43/0x180
> >>>> [ 5.530956] ? __vunmap+0x81/0xd0
> >>>> [ 5.530991] ? kfree+0x14c/0x160
> >>>> [ 5.531024] ? kmem_cache_alloc_trace+0x38/0x150
> >>>> [ 5.531070] do_init_module+0x5f/0x1f8
> >>>> [ 5.531108] load_module+0x271e/0x2bd0
> >>>> [ 5.531147] ? kernel_read_file+0x1a3/0x1c0
> >>>> [ 5.531188] SYSC_finit_module+0xbc/0xf0
> >>>> [ 5.531226] ? SYSC_finit_module+0xbc/0xf0
> >>>> [ 5.531267] SyS_finit_module+0xe/0x10
> >>>> [ 5.531305] do_syscall_64+0x6e/0x180
> >>>> [ 5.531345] entry_SYSCALL64_slow_path+0x25/0x25
> >>>> [ 5.531389] RIP: 0033:0x7f627b5fbbf9
> >>>> [ 5.531424] RSP: 002b:00007ffe053eee68 EFLAGS: 00000246 ORIG_RAX:
> >>>> 0000000000000139
> >>>> [ 5.531493] RAX: ffffffffffffffda RBX: 000055d6c745b690 RCX:
> >>>> 00007f627b5fbbf9
> >>>> [ 5.531558] RDX: 0000000000000000 RSI: 00007f627c134995 RDI:
> >>>> 0000000000000007
> >>>> [ 5.531622] RBP: 00007f627c134995 R08: 0000000000000000 R09:
> >>>> 00007ffe053eef80
> >>>> [ 5.531687] R10: 0000000000000007 R11: 0000000000000246 R12:
> >>>> 0000000000000000
> >>>> [ 5.531751] R13: 000055d6c7459ae0 R14: 0000000000020000 R15:
> >>>> 000055d6c745b690
> >>>> [ 5.531816] Code: 48 8b 45 b0 8b 48 18 e8 e0 cb 22 e1 49 8b 44 24 28
> >>>> 4a 8d 8c 33 58 01 00 00 48 8d 75 b8 45 31 c9 41 b8 01 00 00 00 ba 14 00
> >>>> 00 00 <48> 8b 38 e8 a9 2a ff ff 85 c0 0f 88 09 01 00 00 49 8b 84 24 58
> >>> Disassembling that I get:
> >>>
> >>> 4005a0: 48 8b 45 b0 mov -0x50(%rbp),%rax
> >>> 4005a4: 8b 48 18 mov 0x18(%rax),%ecx
> >>> 4005a7: e8 e0 cb 22 e1 callq ffffffffe162d18c <_end+0xffffffffe102c14c>
> >>> 4005ac: 49 8b 44 24 28 mov 0x28(%r12),%rax
> >>> 4005b1: 4a 8d 8c 33 58 01 00 lea 0x158(%rbx,%r14,1),%rcx
> >>> 4005b8: 00
> >>> 4005b9: 48 8d 75 b8 lea -0x48(%rbp),%rsi
> >>> 4005bd: 45 31 c9 xor %r9d,%r9d
> >>> 4005c0: 41 b8 01 00 00 00 mov $0x1,%r8d
> >>> 4005c6: ba 14 00 00 00 mov $0x14,%edx
> >>> 4005cb: 48 8b 38 mov (%rax),%rdi
> >>> 4005ce: e8 a9 2a ff ff callq 3f307c <_init-0xd32c>
> >>> 4005d3: 85 c0 test %eax,%eax
> >>> 4005d5: 0f 88 09 01 00 00 js 4006e4 <__GNU_EH_FRAME_HDR+0x100>
> >>> 4005db: 49 rex.WB
> >>> 4005dc: 8b .byte 0x8b
> >>> 4005dd: 84 24 58 test %ah,(%rax,%rbx,2)
> >>>
> >>> Comparing that with the disassembly from my build, that
> >>> first call looks like the snprintf() and the second one would
> >>> then be snd_jack_new(). So it seems to blow in the
> >>> ctx->card_ctx->card dereference. But I don't really see how
> >>> a NULL pointer could sneak in there.
> >>
> >> There was an standard PORT_B off-by-one error which resulted in memory
> >> corruption.
> >
> > Doh.
> >
> >> the patch below seems to fix the problem, I can get the same
> >> functionality as before with concurrent playback on HDMI and DP, and the
> >> jack values are correctly set.
> >
> > Thanks. I do wonder a bit which side should apply this offset.
> > But I guess it doesn't really matter in the end as long as both
> > sides know what the deal is. So unless there are good reasons
> > for anything else I'll just squash this into my patch(es).
>
> Fine with me.
> One open I still have is that it's not self-explanatory for the user
> to figure out which output the devices correspond to, e.g. on my CHT
> device HDMI is device 2 and DP is device 0.
> I may be mistaken but in the previous pipe-based solution, audio
> playback would progress at a normal rate even if nothing was
> connected. With the port-based, it looks like playback doesn't
> progress if there is nothing connected - speaker-test is stuck. Not
> sure if the open or hw_params should fail in that case? One could
> argue that the user should know better and check the jack status, but
> in reality I don't think any userspace layer will ever do this.
It's the problem we've had from the beginning. If the open fails, PA
can't detect the PCM device at probe time, and it's gone forever until
PA gets restarted. It's no hotplug device, thus the PCM streams are
determined only once.
So, the open and co are accepted even without the connection but the
actual playback fails -- it's implemented in that way as a
workaround for PA.
Takashi
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v2 00/11] drm/i915: LPE audio runtime PM and multipipe (v2)
2017-04-28 8:41 ` [PATCH v2 00/11] drm/i915: LPE audio runtime PM and multipipe (v2) Takashi Iwai
2017-04-28 17:10 ` [alsa-devel] " Pierre-Louis Bossart
@ 2017-05-03 13:39 ` Ville Syrjälä
2017-05-03 13:48 ` Takashi Iwai
1 sibling, 1 reply; 27+ messages in thread
From: Ville Syrjälä @ 2017-05-03 13:39 UTC (permalink / raw)
To: Takashi Iwai; +Cc: intel-gfx, alsa-devel
On Fri, Apr 28, 2017 at 10:41:37AM +0200, Takashi Iwai wrote:
> On Thu, 27 Apr 2017 18:02:19 +0200,
> ville.syrjala@linux.intel.com wrote:
> >
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Okay, here's the second attempt at getting multiple pipes playing back
> > audio on the VLV/CHV HDMI LPE audio device. The main change from v1 is
> > that now the PCM devices are associated with ports instead of pipes,
> > so the audio from one device always gets output on the same display.
> >
> > I've also tacked on the alsa-lib conf update. No clue whether it's
> > really correct or not (the config language isn't a close friend
> > of mine).
> >
> > BTW I did notice that with LPE audio all the controls say iface=PCM,
> > whereas on HDA a bunch of them say iface=MIXER. No idea if that's
> > OK or not, just something I spotted when I was comparing the results
> > with HDA.
>
> We generally accept both iface types for IEC958 stuff, since
> historically many drivers have already mixed them up. So it's no
> problem :)
>
>
> > Entire series available here:
> > git://github.com/vsyrjala/linux.git lpe_audio_multipipe_2
> >
> > Cc: Takashi Iwai <tiwai@suse.de>
> > Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
>
> All look good, and feel free to take my reviewed-by tag
> Reviewed-by: Takashi Iwai <tiwai@suse.de>
Thanks.
>
> As said previously, my only slight concern is the compatibility.
> But, in the current situation with PulseAudio, only few people would
> use this driver, so it shouldn't be so big impact, I suppose.
What will break? Or you mean the alsa-lib vs. kernel difference
until they sync up? I don't use pulse myself so I don't know really
what it wants.
>
> BTW, which port is used in general on BYT/CHT?
There's no clear rule I think. But I suppose most manufacturers
follow some reference design, so there could be some layouts
that are more common than other. Having HDMI on port D on CHV
is a fairly common design I've seen. And I think all the
pre-production RVP boards had eDP on port C, so that could also
be how production boards tend to be wired up.
> Oh, also, I suppose you want to carry these over i915 tree?
> I don't mind either way, I can take them through sound tree if
> preferred, too.
Cool. I just pushed the lot to drm-intel-next-queued. I'm
thinking going via dinq might avoid some conflicts later on
if we end up churning the code some more. And we do like to
churn ;)
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v2 00/11] drm/i915: LPE audio runtime PM and multipipe (v2)
2017-05-03 13:39 ` Ville Syrjälä
@ 2017-05-03 13:48 ` Takashi Iwai
0 siblings, 0 replies; 27+ messages in thread
From: Takashi Iwai @ 2017-05-03 13:48 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx, alsa-devel
On Wed, 03 May 2017 15:39:06 +0200,
Ville Syrjälä wrote:
>
> On Fri, Apr 28, 2017 at 10:41:37AM +0200, Takashi Iwai wrote:
> > On Thu, 27 Apr 2017 18:02:19 +0200,
> > ville.syrjala@linux.intel.com wrote:
> > >
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >
> > > Okay, here's the second attempt at getting multiple pipes playing back
> > > audio on the VLV/CHV HDMI LPE audio device. The main change from v1 is
> > > that now the PCM devices are associated with ports instead of pipes,
> > > so the audio from one device always gets output on the same display.
> > >
> > > I've also tacked on the alsa-lib conf update. No clue whether it's
> > > really correct or not (the config language isn't a close friend
> > > of mine).
> > >
> > > BTW I did notice that with LPE audio all the controls say iface=PCM,
> > > whereas on HDA a bunch of them say iface=MIXER. No idea if that's
> > > OK or not, just something I spotted when I was comparing the results
> > > with HDA.
> >
> > We generally accept both iface types for IEC958 stuff, since
> > historically many drivers have already mixed them up. So it's no
> > problem :)
> >
> >
> > > Entire series available here:
> > > git://github.com/vsyrjala/linux.git lpe_audio_multipipe_2
> > >
> > > Cc: Takashi Iwai <tiwai@suse.de>
> > > Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> >
> > All look good, and feel free to take my reviewed-by tag
> > Reviewed-by: Takashi Iwai <tiwai@suse.de>
>
> Thanks.
>
> >
> > As said previously, my only slight concern is the compatibility.
> > But, in the current situation with PulseAudio, only few people would
> > use this driver, so it shouldn't be so big impact, I suppose.
>
> What will break? Or you mean the alsa-lib vs. kernel difference
> until they sync up? I don't use pulse myself so I don't know really
> what it wants.
Yes, the alsa-lib stuff is one thing. Another thing is that it'll
change the behavior, for example, when you used to invoke a command
without PA. It might not work any longer as is because now you'll
have multiple PCM devices and the attached device might be not the
first one.
PA would be possible to choose the rightly connected PCM stream, so
it's fine for PA. But the "current situation" I mentioned above is
about PA getting killed at startup when LPE-audio and other drivers
are running. So, for most people, LPE-audio will be blacklisted until
PA gets fixed, or it's worked around by changing PA config. In
anyway, the driver should be still minor at this moment, so we may
give some excuse for changing.
> > BTW, which port is used in general on BYT/CHT?
>
> There's no clear rule I think. But I suppose most manufacturers
> follow some reference design, so there could be some layouts
> that are more common than other. Having HDMI on port D on CHV
> is a fairly common design I've seen. And I think all the
> pre-production RVP boards had eDP on port C, so that could also
> be how production boards tend to be wired up.
>
> > Oh, also, I suppose you want to carry these over i915 tree?
> > I don't mind either way, I can take them through sound tree if
> > preferred, too.
>
> Cool. I just pushed the lot to drm-intel-next-queued. I'm
> thinking going via dinq might avoid some conflicts later on
> if we end up churning the code some more. And we do like to
> churn ;)
Have fun :)
thanks,
Takashi
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread