[alsa-devel] [PATCH 00/11] drm/i915: LPE audio runtime PM and multipipe
From: Ville Syrjälä ville.syrjala@linux.intel.com
I was wondering why my VLV no longer runtime suspended, and after some thinking I decided it had to be the LPE audio preventing it. Turns out I was right, so here's my attempt at fixing it.
And while looking at the code I couldn't help but notice that it couldn't actually handle multiple pipes playing back audio at the same time. And even having multiple displays active even if only one was playing audio was probably a recipe for failure. So I tried to fix that by registering a separate PCM device for each pipe.
Note that the patch subjects may not reflect the subsystem very well since most of these straddle the border between drm and alsa. I think I just slapped on drm/i915 to most where there was no clear winner.
Entire series available here: git://github.com/vsyrjala/linux.git lpe_audio_multipipe
Cc: Takashi Iwai tiwai@suse.de Cc: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com
Ville Syrjälä (11): drm/i915: Fix runtime PM for LPE audio ALSA: x86: Clear the pdata.notify_lpe_audio pointer before teardown drm/i915: Stop pretending to mask/unmask LPE audio interrupts drm/i915: Remove the unsued pending_notify from LPE platform data drm/i915: Replace tmds_clock_speed and link_rate with just ls_clock drm/i915: Remove hdmi_connected from LPE audio pdata drm/i915: Reorganize intel_lpe_audio_notify() arguments drm/i915: Clean up the LPE audio platform data ALSA: x86: Prepare LPE audio ctls for multiple PCMs ALSA: x86: Split snd_intelhad into card and PCM specific structures ALSA: x86: Register multiple PCM devices for the LPE audio card
drivers/gpu/drm/i915/i915_drv.h | 4 +- drivers/gpu/drm/i915/i915_irq.c | 15 +- drivers/gpu/drm/i915/intel_audio.c | 19 +-- drivers/gpu/drm/i915/intel_lpe_audio.c | 95 ++++------- include/drm/intel_lpe_audio.h | 20 ++- sound/x86/intel_hdmi_audio.c | 288 +++++++++++++++++++-------------- sound/x86/intel_hdmi_audio.h | 17 +- 7 files changed, 230 insertions(+), 228 deletions(-)
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);
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);
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);
On 4/25/17 3:27 PM, ville.syrjala@linux.intel.com wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
vlv_display_irq_postinstall() enables the LPE audio interrupts regardless of whether the LPE audio irq chip has masked/unmasked them. Also the irqchip masking/unmasking doesn't consider the state of the display power well or the device, and hence just leads to dmesg spew when it tries to access the hardware while it's powered down.
If the current way works, then we don't need to do anything in the mask/unmask hooks. If it doesn't work, well, then we'd need to properly track whether the irqchip has masked/unmasked the interrupts when we enable display interrupts. And the mask/unmask hooks would need to check whether display interrupts are even enabled before frobbing with he registers.
So let's just assume the current way works and neuter the mask/unmask hooks. Also clean up vlv_display_irq_postinstall() a bit and stop it from trying to unmask/enable the LPE C interrupt on VLV since it doesn't exist.
No objections, I assumed that we did want to update VLV_IMR and VLV_IIR in the mask/unmask, that was the initial recommendation IIRC
There was also a comment where we removed all tests in vlv_display_irq_postinstall:
if (IS_LPE_AUDIO_ENABLED(dev_priv))if (IS_LPE_AUDIO_IRQ_VALID(dev_priv))I think both of these checks can be removed. We won't unmask the interrupts unless lpe is enabled, so the IIR bits will never be set in that case.
Cc: Takashi Iwai tiwai@suse.de Cc: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
drivers/gpu/drm/i915/i915_irq.c | 15 ++++++-------- drivers/gpu/drm/i915/intel_lpe_audio.c | 36 ---------------------------------- 2 files changed, 6 insertions(+), 45 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index fd97fe00cd0d..190f6aa5d15e 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -2953,7 +2953,6 @@ static void vlv_display_irq_postinstall(struct drm_i915_private *dev_priv) u32 pipestat_mask; u32 enable_mask; enum pipe pipe;
u32 val;
pipestat_mask = PLANE_FLIP_DONE_INT_STATUS_VLV | PIPE_CRC_DONE_INTERRUPT_STATUS;
@@ -2964,18 +2963,16 @@ static void vlv_display_irq_postinstall(struct drm_i915_private *dev_priv)
enable_mask = I915_DISPLAY_PORT_INTERRUPT | I915_DISPLAY_PIPE_A_EVENT_INTERRUPT |
I915_DISPLAY_PIPE_B_EVENT_INTERRUPT;
I915_DISPLAY_PIPE_B_EVENT_INTERRUPT |I915_LPE_PIPE_A_INTERRUPT |I915_LPE_PIPE_B_INTERRUPT;- if (IS_CHERRYVIEW(dev_priv))
enable_mask |= I915_DISPLAY_PIPE_C_EVENT_INTERRUPT;
enable_mask |= I915_DISPLAY_PIPE_C_EVENT_INTERRUPT |I915_LPE_PIPE_C_INTERRUPT;WARN_ON(dev_priv->irq_mask != ~0);
val = (I915_LPE_PIPE_A_INTERRUPT |
I915_LPE_PIPE_B_INTERRUPT |I915_LPE_PIPE_C_INTERRUPT);enable_mask |= val;
dev_priv->irq_mask = ~enable_mask;
GEN5_IRQ_INIT(VLV_, dev_priv->irq_mask, enable_mask);
diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c index 668f00480d97..292fedf30b00 100644 --- a/drivers/gpu/drm/i915/intel_lpe_audio.c +++ b/drivers/gpu/drm/i915/intel_lpe_audio.c @@ -149,44 +149,10 @@ static void lpe_audio_platdev_destroy(struct drm_i915_private *dev_priv)
static void lpe_audio_irq_unmask(struct irq_data *d) {
- struct drm_i915_private *dev_priv = d->chip_data;
- unsigned long irqflags;
- u32 val = (I915_LPE_PIPE_A_INTERRUPT |
I915_LPE_PIPE_B_INTERRUPT);- if (IS_CHERRYVIEW(dev_priv))
val |= I915_LPE_PIPE_C_INTERRUPT;- spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
- dev_priv->irq_mask &= ~val;
- I915_WRITE(VLV_IIR, val);
- I915_WRITE(VLV_IIR, val);
- I915_WRITE(VLV_IMR, dev_priv->irq_mask);
- POSTING_READ(VLV_IMR);
- spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
}
static void lpe_audio_irq_mask(struct irq_data *d) {
- struct drm_i915_private *dev_priv = d->chip_data;
- unsigned long irqflags;
- u32 val = (I915_LPE_PIPE_A_INTERRUPT |
I915_LPE_PIPE_B_INTERRUPT);- if (IS_CHERRYVIEW(dev_priv))
val |= I915_LPE_PIPE_C_INTERRUPT;- spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
- dev_priv->irq_mask |= val;
- I915_WRITE(VLV_IMR, dev_priv->irq_mask);
- I915_WRITE(VLV_IIR, val);
- I915_WRITE(VLV_IIR, val);
- POSTING_READ(VLV_IIR);
- spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
}
static struct irq_chip lpe_audio_irqchip = { @@ -330,8 +296,6 @@ void intel_lpe_audio_teardown(struct drm_i915_private *dev_priv)
desc = irq_to_desc(dev_priv->lpe_audio.irq);
lpe_audio_irq_mask(&desc->irq_data);
lpe_audio_platdev_destroy(dev_priv);
irq_free_desc(dev_priv->lpe_audio.irq);
On Tue, Apr 25, 2017 at 07:27:32PM -0500, Pierre-Louis Bossart wrote:
On 4/25/17 3:27 PM, ville.syrjala@linux.intel.com wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
vlv_display_irq_postinstall() enables the LPE audio interrupts regardless of whether the LPE audio irq chip has masked/unmasked them. Also the irqchip masking/unmasking doesn't consider the state of the display power well or the device, and hence just leads to dmesg spew when it tries to access the hardware while it's powered down.
If the current way works, then we don't need to do anything in the mask/unmask hooks. If it doesn't work, well, then we'd need to properly track whether the irqchip has masked/unmasked the interrupts when we enable display interrupts. And the mask/unmask hooks would need to check whether display interrupts are even enabled before frobbing with he registers.
So let's just assume the current way works and neuter the mask/unmask hooks. Also clean up vlv_display_irq_postinstall() a bit and stop it from trying to unmask/enable the LPE C interrupt on VLV since it doesn't exist.
No objections, I assumed that we did want to update VLV_IMR and VLV_IIR in the mask/unmask, that was the initial recommendation IIRC
There was also a comment where we removed all tests in vlv_display_irq_postinstall:
if (IS_LPE_AUDIO_ENABLED(dev_priv))if (IS_LPE_AUDIO_IRQ_VALID(dev_priv))I think both of these checks can be removed. We won't unmask the interrupts unless lpe is enabled, so the IIR bits will never be set in that case.
Hmm. Right, so I guess the idea originally was to just enable the LPE interrupts in IER, but leave them masked in IMR until the irqchip unmasks them. But the code wasn't actually doing that because it was appending the bits to 'enable_mask' before it assigned the value to irq_mask. Hence the interrupts were always enabled and unmasked.
Cc: Takashi Iwai tiwai@suse.de Cc: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
drivers/gpu/drm/i915/i915_irq.c | 15 ++++++-------- drivers/gpu/drm/i915/intel_lpe_audio.c | 36 ---------------------------------- 2 files changed, 6 insertions(+), 45 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index fd97fe00cd0d..190f6aa5d15e 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -2953,7 +2953,6 @@ static void vlv_display_irq_postinstall(struct drm_i915_private *dev_priv) u32 pipestat_mask; u32 enable_mask; enum pipe pipe;
u32 val;
pipestat_mask = PLANE_FLIP_DONE_INT_STATUS_VLV | PIPE_CRC_DONE_INTERRUPT_STATUS;
@@ -2964,18 +2963,16 @@ static void vlv_display_irq_postinstall(struct drm_i915_private *dev_priv)
enable_mask = I915_DISPLAY_PORT_INTERRUPT | I915_DISPLAY_PIPE_A_EVENT_INTERRUPT |
I915_DISPLAY_PIPE_B_EVENT_INTERRUPT;
I915_DISPLAY_PIPE_B_EVENT_INTERRUPT |I915_LPE_PIPE_A_INTERRUPT |I915_LPE_PIPE_B_INTERRUPT;- if (IS_CHERRYVIEW(dev_priv))
enable_mask |= I915_DISPLAY_PIPE_C_EVENT_INTERRUPT;
enable_mask |= I915_DISPLAY_PIPE_C_EVENT_INTERRUPT |I915_LPE_PIPE_C_INTERRUPT;WARN_ON(dev_priv->irq_mask != ~0);
val = (I915_LPE_PIPE_A_INTERRUPT |
I915_LPE_PIPE_B_INTERRUPT |I915_LPE_PIPE_C_INTERRUPT);enable_mask |= val;
dev_priv->irq_mask = ~enable_mask;
GEN5_IRQ_INIT(VLV_, dev_priv->irq_mask, enable_mask);
diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c index 668f00480d97..292fedf30b00 100644 --- a/drivers/gpu/drm/i915/intel_lpe_audio.c +++ b/drivers/gpu/drm/i915/intel_lpe_audio.c @@ -149,44 +149,10 @@ static void lpe_audio_platdev_destroy(struct drm_i915_private *dev_priv)
static void lpe_audio_irq_unmask(struct irq_data *d) {
- struct drm_i915_private *dev_priv = d->chip_data;
- unsigned long irqflags;
- u32 val = (I915_LPE_PIPE_A_INTERRUPT |
I915_LPE_PIPE_B_INTERRUPT);- if (IS_CHERRYVIEW(dev_priv))
val |= I915_LPE_PIPE_C_INTERRUPT;- spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
- dev_priv->irq_mask &= ~val;
- I915_WRITE(VLV_IIR, val);
- I915_WRITE(VLV_IIR, val);
- I915_WRITE(VLV_IMR, dev_priv->irq_mask);
- POSTING_READ(VLV_IMR);
- spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
}
static void lpe_audio_irq_mask(struct irq_data *d) {
- struct drm_i915_private *dev_priv = d->chip_data;
- unsigned long irqflags;
- u32 val = (I915_LPE_PIPE_A_INTERRUPT |
I915_LPE_PIPE_B_INTERRUPT);- if (IS_CHERRYVIEW(dev_priv))
val |= I915_LPE_PIPE_C_INTERRUPT;- spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
- dev_priv->irq_mask |= val;
- I915_WRITE(VLV_IMR, dev_priv->irq_mask);
- I915_WRITE(VLV_IIR, val);
- I915_WRITE(VLV_IIR, val);
- POSTING_READ(VLV_IIR);
- spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
}
static struct irq_chip lpe_audio_irqchip = { @@ -330,8 +296,6 @@ void intel_lpe_audio_teardown(struct drm_i915_private *dev_priv)
desc = irq_to_desc(dev_priv->lpe_audio.irq);
lpe_audio_irq_mask(&desc->irq_data);
lpe_audio_platdev_destroy(dev_priv);
irq_free_desc(dev_priv->lpe_audio.irq);
From: Ville Syrjälä ville.syrjala@linux.intel.com
The pending_notify flag in the LPE audio platform data is pointless, actually unused. So let's kill it off.
Cc: Takashi Iwai tiwai@suse.de Cc: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/i915/intel_lpe_audio.c | 2 -- include/drm/intel_lpe_audio.h | 1 - sound/x86/intel_hdmi_audio.c | 1 - 3 files changed, 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c index 292fedf30b00..79b9dca985ff 100644 --- a/drivers/gpu/drm/i915/intel_lpe_audio.c +++ b/drivers/gpu/drm/i915/intel_lpe_audio.c @@ -361,8 +361,6 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv,
if (pdata->notify_audio_lpe) pdata->notify_audio_lpe(dev_priv->lpe_audio.platdev); - else - pdata->notify_pending = true;
spin_unlock_irqrestore(&pdata->lpe_audio_slock, irq_flags); diff --git a/include/drm/intel_lpe_audio.h b/include/drm/intel_lpe_audio.h index e9892b4c3af1..c201d39cdfea 100644 --- a/include/drm/intel_lpe_audio.h +++ b/include/drm/intel_lpe_audio.h @@ -38,7 +38,6 @@ struct intel_hdmi_lpe_audio_eld { };
struct intel_hdmi_lpe_audio_pdata { - bool notify_pending; int tmds_clock_speed; bool hdmi_connected; bool dp_output; diff --git a/sound/x86/intel_hdmi_audio.c b/sound/x86/intel_hdmi_audio.c index 5b89662493c9..cbba4a78afb5 100644 --- a/sound/x86/intel_hdmi_audio.c +++ b/sound/x86/intel_hdmi_audio.c @@ -1811,7 +1811,6 @@ static int hdmi_lpe_audio_probe(struct platform_device *pdev)
spin_lock_irq(&pdata->lpe_audio_slock); pdata->notify_audio_lpe = notify_audio_lpe; - pdata->notify_pending = false; spin_unlock_irq(&pdata->lpe_audio_slock);
pm_runtime_use_autosuspend(&pdev->dev);
From: Ville Syrjälä ville.syrjala@linux.intel.com
There's no need to distinguish between the DP link rate and HDMI TMDS clock for the purposes of the LPE audio. Both are actually the same thing more or less, which is the link symbol clock. So let's just call the thing ls_clock and simplify the code.
Cc: Takashi Iwai tiwai@suse.de Cc: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/i915/i915_drv.h | 4 ++-- drivers/gpu/drm/i915/intel_audio.c | 19 ++++--------------- drivers/gpu/drm/i915/intel_lpe_audio.c | 14 ++++++-------- include/drm/intel_lpe_audio.h | 3 +-- sound/x86/intel_hdmi_audio.c | 11 ++++++++--- 5 files changed, 21 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 357b6c6c2f04..876eee56a958 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -3721,8 +3721,8 @@ int intel_lpe_audio_init(struct drm_i915_private *dev_priv); void intel_lpe_audio_teardown(struct drm_i915_private *dev_priv); void intel_lpe_audio_irq_handler(struct drm_i915_private *dev_priv); void intel_lpe_audio_notify(struct drm_i915_private *dev_priv, - void *eld, int port, int pipe, int tmds_clk_speed, - bool dp_output, int link_rate); + void *eld, int port, int pipe, int ls_clock, + bool dp_output);
/* intel_i2c.c */ extern int intel_setup_gmbus(struct drm_i915_private *dev_priv); diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c index 52c207e81f41..79eeef25321f 100644 --- a/drivers/gpu/drm/i915/intel_audio.c +++ b/drivers/gpu/drm/i915/intel_audio.c @@ -632,20 +632,9 @@ void intel_audio_codec_enable(struct intel_encoder *intel_encoder, (int) port, (int) pipe); }
- switch (intel_encoder->type) { - case INTEL_OUTPUT_HDMI: - intel_lpe_audio_notify(dev_priv, connector->eld, port, pipe, - crtc_state->port_clock, - false, 0); - break; - case INTEL_OUTPUT_DP: - intel_lpe_audio_notify(dev_priv, connector->eld, port, pipe, - adjusted_mode->crtc_clock, - true, crtc_state->port_clock); - break; - default: - break; - } + intel_lpe_audio_notify(dev_priv, connector->eld, port, pipe, + crtc_state->port_clock, + intel_encoder->type == INTEL_OUTPUT_DP); }
/** @@ -680,7 +669,7 @@ void intel_audio_codec_disable(struct intel_encoder *intel_encoder) (int) port, (int) pipe); }
- intel_lpe_audio_notify(dev_priv, NULL, port, pipe, 0, false, 0); + intel_lpe_audio_notify(dev_priv, NULL, port, pipe, 0, false); }
/** diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c index 79b9dca985ff..5a1a37e963f1 100644 --- a/drivers/gpu/drm/i915/intel_lpe_audio.c +++ b/drivers/gpu/drm/i915/intel_lpe_audio.c @@ -309,13 +309,14 @@ void intel_lpe_audio_teardown(struct drm_i915_private *dev_priv) * @eld : ELD data * @pipe: pipe id * @port: port id - * @tmds_clk_speed: tmds clock frequency in Hz + * @ls_clock: Link symbol clock in kHz + * @dp_output: Driving a DP output? * * Notify lpe audio driver of eld change. */ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv, - void *eld, int port, int pipe, int tmds_clk_speed, - bool dp_output, int link_rate) + void *eld, int port, int pipe, int ls_clock, + bool dp_output) { unsigned long irq_flags; struct intel_hdmi_lpe_audio_pdata *pdata = NULL; @@ -337,12 +338,8 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv, pdata->eld.port_id = port; pdata->eld.pipe_id = pipe; pdata->hdmi_connected = true; - + pdata->ls_clock = ls_clock; pdata->dp_output = dp_output; - if (tmds_clk_speed) - pdata->tmds_clock_speed = tmds_clk_speed; - if (link_rate) - pdata->link_rate = link_rate;
/* Unmute the amp for both DP and HDMI */ I915_WRITE(VLV_AUD_PORT_EN_DBG(port), @@ -352,6 +349,7 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv, memset(pdata->eld.eld_data, 0, HDMI_MAX_ELD_BYTES); pdata->hdmi_connected = false; + pdata->ls_clock = 0; pdata->dp_output = false;
/* Mute the amp for both DP and HDMI */ diff --git a/include/drm/intel_lpe_audio.h b/include/drm/intel_lpe_audio.h index c201d39cdfea..8bf804ce8905 100644 --- a/include/drm/intel_lpe_audio.h +++ b/include/drm/intel_lpe_audio.h @@ -38,10 +38,9 @@ struct intel_hdmi_lpe_audio_eld { };
struct intel_hdmi_lpe_audio_pdata { - int tmds_clock_speed; + int ls_clock; bool hdmi_connected; bool dp_output; - int link_rate; struct intel_hdmi_lpe_audio_eld eld; void (*notify_audio_lpe)(struct platform_device *pdev); spinlock_t lpe_audio_slock; diff --git a/sound/x86/intel_hdmi_audio.c b/sound/x86/intel_hdmi_audio.c index cbba4a78afb5..4eaf5de54f61 100644 --- a/sound/x86/intel_hdmi_audio.c +++ b/sound/x86/intel_hdmi_audio.c @@ -1568,7 +1568,7 @@ static void had_audio_wq(struct work_struct *work) struct intel_hdmi_lpe_audio_eld *eld = &pdata->eld;
dev_dbg(ctx->dev, "%s: HAD_NOTIFY_ELD : port = %d, tmds = %d\n", - __func__, eld->port_id, pdata->tmds_clock_speed); + __func__, eld->port_id, pdata->ls_clock);
switch (eld->pipe_id) { case 0: @@ -1589,8 +1589,13 @@ static void had_audio_wq(struct work_struct *work) memcpy(ctx->eld, eld->eld_data, sizeof(ctx->eld));
ctx->dp_output = pdata->dp_output; - ctx->tmds_clock_speed = pdata->tmds_clock_speed; - ctx->link_rate = pdata->link_rate; + if (ctx->dp_output) { + ctx->tmds_clock_speed = 0; + ctx->link_rate = pdata->ls_clock; + } else { + ctx->tmds_clock_speed = pdata->ls_clock; + ctx->link_rate = 0; + }
had_process_hot_plug(ctx);
On 4/25/17 3:27 PM, ville.syrjala@linux.intel.com wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
There's no need to distinguish between the DP link rate and HDMI TMDS clock for the purposes of the LPE audio. Both are actually the same thing more or less, which is the link symbol clock. So let's just call the thing ls_clock and simplify the code.
there are still occurences of 'tmds' in sound/x86 and there are are couple of debug messages that don't make sense any longer.
Cc: Takashi Iwai tiwai@suse.de Cc: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
drivers/gpu/drm/i915/i915_drv.h | 4 ++-- drivers/gpu/drm/i915/intel_audio.c | 19 ++++--------------- drivers/gpu/drm/i915/intel_lpe_audio.c | 14 ++++++-------- include/drm/intel_lpe_audio.h | 3 +-- sound/x86/intel_hdmi_audio.c | 11 ++++++++--- 5 files changed, 21 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 357b6c6c2f04..876eee56a958 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -3721,8 +3721,8 @@ int intel_lpe_audio_init(struct drm_i915_private *dev_priv); void intel_lpe_audio_teardown(struct drm_i915_private *dev_priv); void intel_lpe_audio_irq_handler(struct drm_i915_private *dev_priv); void intel_lpe_audio_notify(struct drm_i915_private *dev_priv,
void *eld, int port, int pipe, int tmds_clk_speed,bool dp_output, int link_rate);
void *eld, int port, int pipe, int ls_clock,bool dp_output);/* intel_i2c.c */ extern int intel_setup_gmbus(struct drm_i915_private *dev_priv); diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c index 52c207e81f41..79eeef25321f 100644 --- a/drivers/gpu/drm/i915/intel_audio.c +++ b/drivers/gpu/drm/i915/intel_audio.c @@ -632,20 +632,9 @@ void intel_audio_codec_enable(struct intel_encoder *intel_encoder, (int) port, (int) pipe); }
- switch (intel_encoder->type) {
- case INTEL_OUTPUT_HDMI:
intel_lpe_audio_notify(dev_priv, connector->eld, port, pipe,crtc_state->port_clock,false, 0);break;- case INTEL_OUTPUT_DP:
intel_lpe_audio_notify(dev_priv, connector->eld, port, pipe,adjusted_mode->crtc_clock,true, crtc_state->port_clock);break;- default:
break;- }
- intel_lpe_audio_notify(dev_priv, connector->eld, port, pipe,
crtc_state->port_clock,intel_encoder->type == INTEL_OUTPUT_DP);}
/** @@ -680,7 +669,7 @@ void intel_audio_codec_disable(struct intel_encoder *intel_encoder) (int) port, (int) pipe); }
- intel_lpe_audio_notify(dev_priv, NULL, port, pipe, 0, false, 0);
- intel_lpe_audio_notify(dev_priv, NULL, port, pipe, 0, false);
}
/** diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c index 79b9dca985ff..5a1a37e963f1 100644 --- a/drivers/gpu/drm/i915/intel_lpe_audio.c +++ b/drivers/gpu/drm/i915/intel_lpe_audio.c @@ -309,13 +309,14 @@ void intel_lpe_audio_teardown(struct drm_i915_private *dev_priv)
- @eld : ELD data
- @pipe: pipe id
- @port: port id
- @tmds_clk_speed: tmds clock frequency in Hz
- @ls_clock: Link symbol clock in kHz
*/
- @dp_output: Driving a DP output?
- Notify lpe audio driver of eld change.
void intel_lpe_audio_notify(struct drm_i915_private *dev_priv,
void *eld, int port, int pipe, int tmds_clk_speed,bool dp_output, int link_rate)
void *eld, int port, int pipe, int ls_clock,bool dp_output){ unsigned long irq_flags; struct intel_hdmi_lpe_audio_pdata *pdata = NULL; @@ -337,12 +338,8 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv, pdata->eld.port_id = port; pdata->eld.pipe_id = pipe; pdata->hdmi_connected = true;
pdata->ls_clock = ls_clock;
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);
On Tue, Apr 25, 2017 at 08:09:34PM -0500, Pierre-Louis Bossart wrote:
On 4/25/17 3:27 PM, ville.syrjala@linux.intel.com wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
There's no need to distinguish between the DP link rate and HDMI TMDS clock for the purposes of the LPE audio. Both are actually the same thing more or less, which is the link symbol clock. So let's just call the thing ls_clock and simplify the code.
there are still occurences of 'tmds' in sound/x86 and there are are couple of debug messages that don't make sense any longer.
I was being a bit lazy and didn't remove the tmds vs. link_rate distinction from the audio driver side. Not quite sure if we want to do it not.
Cc: Takashi Iwai tiwai@suse.de Cc: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
drivers/gpu/drm/i915/i915_drv.h | 4 ++-- drivers/gpu/drm/i915/intel_audio.c | 19 ++++--------------- drivers/gpu/drm/i915/intel_lpe_audio.c | 14 ++++++-------- include/drm/intel_lpe_audio.h | 3 +-- sound/x86/intel_hdmi_audio.c | 11 ++++++++--- 5 files changed, 21 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 357b6c6c2f04..876eee56a958 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -3721,8 +3721,8 @@ int intel_lpe_audio_init(struct drm_i915_private *dev_priv); void intel_lpe_audio_teardown(struct drm_i915_private *dev_priv); void intel_lpe_audio_irq_handler(struct drm_i915_private *dev_priv); void intel_lpe_audio_notify(struct drm_i915_private *dev_priv,
void *eld, int port, int pipe, int tmds_clk_speed,bool dp_output, int link_rate);
void *eld, int port, int pipe, int ls_clock,bool dp_output);/* intel_i2c.c */ extern int intel_setup_gmbus(struct drm_i915_private *dev_priv); diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c index 52c207e81f41..79eeef25321f 100644 --- a/drivers/gpu/drm/i915/intel_audio.c +++ b/drivers/gpu/drm/i915/intel_audio.c @@ -632,20 +632,9 @@ void intel_audio_codec_enable(struct intel_encoder *intel_encoder, (int) port, (int) pipe); }
- switch (intel_encoder->type) {
- case INTEL_OUTPUT_HDMI:
intel_lpe_audio_notify(dev_priv, connector->eld, port, pipe,crtc_state->port_clock,false, 0);break;- case INTEL_OUTPUT_DP:
intel_lpe_audio_notify(dev_priv, connector->eld, port, pipe,adjusted_mode->crtc_clock,true, crtc_state->port_clock);break;- default:
break;- }
- intel_lpe_audio_notify(dev_priv, connector->eld, port, pipe,
crtc_state->port_clock,intel_encoder->type == INTEL_OUTPUT_DP);}
/** @@ -680,7 +669,7 @@ void intel_audio_codec_disable(struct intel_encoder *intel_encoder) (int) port, (int) pipe); }
- intel_lpe_audio_notify(dev_priv, NULL, port, pipe, 0, false, 0);
- intel_lpe_audio_notify(dev_priv, NULL, port, pipe, 0, false);
}
/** diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c index 79b9dca985ff..5a1a37e963f1 100644 --- a/drivers/gpu/drm/i915/intel_lpe_audio.c +++ b/drivers/gpu/drm/i915/intel_lpe_audio.c @@ -309,13 +309,14 @@ void intel_lpe_audio_teardown(struct drm_i915_private *dev_priv)
- @eld : ELD data
- @pipe: pipe id
- @port: port id
- @tmds_clk_speed: tmds clock frequency in Hz
- @ls_clock: Link symbol clock in kHz
*/
- @dp_output: Driving a DP output?
- Notify lpe audio driver of eld change.
void intel_lpe_audio_notify(struct drm_i915_private *dev_priv,
void *eld, int port, int pipe, int tmds_clk_speed,bool dp_output, int link_rate)
void *eld, int port, int pipe, int ls_clock,bool dp_output){ unsigned long irq_flags; struct intel_hdmi_lpe_audio_pdata *pdata = NULL; @@ -337,12 +338,8 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv, pdata->eld.port_id = port; pdata->eld.pipe_id = pipe; pdata->hdmi_connected = true;
pdata->ls_clock = ls_clock;
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);
From: Ville Syrjälä ville.syrjala@linux.intel.com
We can determine that the pipe was shut down from port<0, so there's no point in duplicating that information as 'hdmi_connected'.
Cc: Takashi Iwai tiwai@suse.de Cc: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/i915/intel_lpe_audio.c | 5 ++--- include/drm/intel_lpe_audio.h | 3 +-- sound/x86/intel_hdmi_audio.c | 4 ++-- 3 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c index 5a1a37e963f1..1696359bf6e5 100644 --- a/drivers/gpu/drm/i915/intel_lpe_audio.c +++ b/drivers/gpu/drm/i915/intel_lpe_audio.c @@ -335,9 +335,8 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv, if (eld != NULL) { memcpy(pdata->eld.eld_data, eld, HDMI_MAX_ELD_BYTES); - pdata->eld.port_id = port; pdata->eld.pipe_id = pipe; - pdata->hdmi_connected = true; + pdata->port = port; pdata->ls_clock = ls_clock; pdata->dp_output = dp_output;
@@ -348,7 +347,7 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv, } else { memset(pdata->eld.eld_data, 0, HDMI_MAX_ELD_BYTES); - pdata->hdmi_connected = false; + pdata->port = -1; pdata->ls_clock = 0; pdata->dp_output = false;
diff --git a/include/drm/intel_lpe_audio.h b/include/drm/intel_lpe_audio.h index 8bf804ce8905..826d531c3ecc 100644 --- a/include/drm/intel_lpe_audio.h +++ b/include/drm/intel_lpe_audio.h @@ -32,14 +32,13 @@ struct platform_device; #define HDMI_MAX_ELD_BYTES 128
struct intel_hdmi_lpe_audio_eld { - int port_id; int pipe_id; unsigned char eld_data[HDMI_MAX_ELD_BYTES]; };
struct intel_hdmi_lpe_audio_pdata { + int port; int ls_clock; - bool hdmi_connected; bool dp_output; struct intel_hdmi_lpe_audio_eld eld; void (*notify_audio_lpe)(struct platform_device *pdev); diff --git a/sound/x86/intel_hdmi_audio.c b/sound/x86/intel_hdmi_audio.c index 4eaf5de54f61..71f14a2a7fe4 100644 --- a/sound/x86/intel_hdmi_audio.c +++ b/sound/x86/intel_hdmi_audio.c @@ -1559,7 +1559,7 @@ static void had_audio_wq(struct work_struct *work)
pm_runtime_get_sync(ctx->dev); mutex_lock(&ctx->mutex); - if (!pdata->hdmi_connected) { + if (pdata->port < 0) { dev_dbg(ctx->dev, "%s: Event: HAD_NOTIFY_HOT_UNPLUG\n", __func__); memset(ctx->eld, 0, sizeof(ctx->eld)); /* clear the old ELD */ @@ -1568,7 +1568,7 @@ static void had_audio_wq(struct work_struct *work) struct intel_hdmi_lpe_audio_eld *eld = &pdata->eld;
dev_dbg(ctx->dev, "%s: HAD_NOTIFY_ELD : port = %d, tmds = %d\n", - __func__, eld->port_id, pdata->ls_clock); + __func__, pdata->port, pdata->ls_clock);
switch (eld->pipe_id) { case 0:
From: Ville Syrjälä ville.syrjala@linux.intel.com
Shuffle the arguments to intel_lpe_audio_notify() around a bit. Pipe and port being the most important things, so let's put the first, and thre rest can come in as is. Also constify the eld argument.
Cc: Takashi Iwai tiwai@suse.de Cc: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/i915/i915_drv.h | 4 ++-- drivers/gpu/drm/i915/intel_audio.c | 4 ++-- drivers/gpu/drm/i915/intel_lpe_audio.c | 8 ++++---- 3 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 876eee56a958..e6230f68ee80 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -3721,8 +3721,8 @@ int intel_lpe_audio_init(struct drm_i915_private *dev_priv); void intel_lpe_audio_teardown(struct drm_i915_private *dev_priv); void intel_lpe_audio_irq_handler(struct drm_i915_private *dev_priv); void intel_lpe_audio_notify(struct drm_i915_private *dev_priv, - void *eld, int port, int pipe, int ls_clock, - bool dp_output); + enum pipe pipe, enum port port, + const void *eld, int ls_clock, bool dp_output);
/* intel_i2c.c */ extern int intel_setup_gmbus(struct drm_i915_private *dev_priv); diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c index 79eeef25321f..d805b6e6fe71 100644 --- a/drivers/gpu/drm/i915/intel_audio.c +++ b/drivers/gpu/drm/i915/intel_audio.c @@ -632,7 +632,7 @@ void intel_audio_codec_enable(struct intel_encoder *intel_encoder, (int) port, (int) pipe); }
- intel_lpe_audio_notify(dev_priv, connector->eld, port, pipe, + intel_lpe_audio_notify(dev_priv, pipe, port, connector->eld, crtc_state->port_clock, intel_encoder->type == INTEL_OUTPUT_DP); } @@ -669,7 +669,7 @@ void intel_audio_codec_disable(struct intel_encoder *intel_encoder) (int) port, (int) pipe); }
- intel_lpe_audio_notify(dev_priv, NULL, port, pipe, 0, false); + intel_lpe_audio_notify(dev_priv, pipe, port, NULL, 0, false); }
/** diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c index 1696359bf6e5..d6aecf1d382b 100644 --- a/drivers/gpu/drm/i915/intel_lpe_audio.c +++ b/drivers/gpu/drm/i915/intel_lpe_audio.c @@ -306,17 +306,17 @@ void intel_lpe_audio_teardown(struct drm_i915_private *dev_priv) * intel_lpe_audio_notify() - notify lpe audio event * audio driver and i915 * @dev_priv: the i915 drm device private data + * @pipe: pipe + * @port: port * @eld : ELD data - * @pipe: pipe id - * @port: port id * @ls_clock: Link symbol clock in kHz * @dp_output: Driving a DP output? * * Notify lpe audio driver of eld change. */ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv, - void *eld, int port, int pipe, int ls_clock, - bool dp_output) + enum pipe pipe, enum port port, + const void *eld, int ls_clock, bool dp_output) { unsigned long irq_flags; struct intel_hdmi_lpe_audio_pdata *pdata = NULL;
From: Ville Syrjälä ville.syrjala@linux.intel.com
Split the LPE audio platform data into a pipe specific chunk and device specific chunk. Eventually we'll have a pipe specific chunk for each pipe, but for now we'll stick to just one.
We'll also get rid of the intel_hdmi_lpe_audio_eld structure which doesn't seem to have any real reason to exist.
Cc: Takashi Iwai tiwai@suse.de Cc: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/i915/intel_lpe_audio.c | 29 ++++++++++++++--------------- include/drm/intel_lpe_audio.h | 15 ++++++++------- sound/x86/intel_hdmi_audio.c | 19 +++++++++---------- 3 files changed, 31 insertions(+), 32 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c index d6aecf1d382b..a593fdf73171 100644 --- a/drivers/gpu/drm/i915/intel_lpe_audio.c +++ b/drivers/gpu/drm/i915/intel_lpe_audio.c @@ -319,37 +319,36 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv, const void *eld, int ls_clock, bool dp_output) { unsigned long irq_flags; - struct intel_hdmi_lpe_audio_pdata *pdata = NULL; + struct intel_hdmi_lpe_audio_pdata *pdata; + struct intel_hdmi_lpe_audio_pipe_pdata *ppdata; u32 audio_enable;
if (!HAS_LPE_AUDIO(dev_priv)) return;
- pdata = dev_get_platdata( - &(dev_priv->lpe_audio.platdev->dev)); + pdata = dev_get_platdata(&dev_priv->lpe_audio.platdev->dev); + ppdata = &pdata->pipe;
spin_lock_irqsave(&pdata->lpe_audio_slock, irq_flags);
audio_enable = I915_READ(VLV_AUD_PORT_EN_DBG(port));
+ pdata->pipe_id = pipe; + if (eld != NULL) { - memcpy(pdata->eld.eld_data, eld, - HDMI_MAX_ELD_BYTES); - pdata->eld.pipe_id = pipe; - pdata->port = port; - pdata->ls_clock = ls_clock; - pdata->dp_output = dp_output; + memcpy(ppdata->eld, eld, HDMI_MAX_ELD_BYTES); + ppdata->port = port; + ppdata->ls_clock = ls_clock; + ppdata->dp_output = dp_output;
/* Unmute the amp for both DP and HDMI */ I915_WRITE(VLV_AUD_PORT_EN_DBG(port), audio_enable & ~VLV_AMP_MUTE); - } else { - memset(pdata->eld.eld_data, 0, - HDMI_MAX_ELD_BYTES); - pdata->port = -1; - pdata->ls_clock = 0; - pdata->dp_output = false; + memset(ppdata->eld, 0, HDMI_MAX_ELD_BYTES); + ppdata->port = -1; + ppdata->ls_clock = 0; + ppdata->dp_output = false;
/* Mute the amp for both DP and HDMI */ I915_WRITE(VLV_AUD_PORT_EN_DBG(port), diff --git a/include/drm/intel_lpe_audio.h b/include/drm/intel_lpe_audio.h index 826d531c3ecc..26e569ad8683 100644 --- a/include/drm/intel_lpe_audio.h +++ b/include/drm/intel_lpe_audio.h @@ -31,16 +31,17 @@ struct platform_device;
#define HDMI_MAX_ELD_BYTES 128
-struct intel_hdmi_lpe_audio_eld { - int pipe_id; - unsigned char eld_data[HDMI_MAX_ELD_BYTES]; -}; - -struct intel_hdmi_lpe_audio_pdata { +struct intel_hdmi_lpe_audio_pipe_pdata { + u8 eld[HDMI_MAX_ELD_BYTES]; int port; int ls_clock; bool dp_output; - struct intel_hdmi_lpe_audio_eld eld; +}; + +struct intel_hdmi_lpe_audio_pdata { + struct intel_hdmi_lpe_audio_pipe_pdata pipe; + int pipe_id; + void (*notify_audio_lpe)(struct platform_device *pdev); spinlock_t lpe_audio_slock; }; diff --git a/sound/x86/intel_hdmi_audio.c b/sound/x86/intel_hdmi_audio.c index 71f14a2a7fe4..bfb712444098 100644 --- a/sound/x86/intel_hdmi_audio.c +++ b/sound/x86/intel_hdmi_audio.c @@ -1556,21 +1556,20 @@ static void had_audio_wq(struct work_struct *work) struct snd_intelhad *ctx = container_of(work, struct snd_intelhad, hdmi_audio_wq); struct intel_hdmi_lpe_audio_pdata *pdata = ctx->dev->platform_data; + struct intel_hdmi_lpe_audio_pipe_pdata *ppdata = &pdata->pipe;
pm_runtime_get_sync(ctx->dev); mutex_lock(&ctx->mutex); - if (pdata->port < 0) { + if (ppdata->port < 0) { dev_dbg(ctx->dev, "%s: Event: HAD_NOTIFY_HOT_UNPLUG\n", __func__); memset(ctx->eld, 0, sizeof(ctx->eld)); /* clear the old ELD */ had_process_hot_unplug(ctx); } else { - struct intel_hdmi_lpe_audio_eld *eld = &pdata->eld; - dev_dbg(ctx->dev, "%s: HAD_NOTIFY_ELD : port = %d, tmds = %d\n", - __func__, pdata->port, pdata->ls_clock); + __func__, ppdata->port, ppdata->ls_clock);
- switch (eld->pipe_id) { + switch (pdata->pipe_id) { case 0: ctx->had_config_offset = AUDIO_HDMI_CONFIG_A; break; @@ -1582,18 +1581,18 @@ static void had_audio_wq(struct work_struct *work) break; default: dev_dbg(ctx->dev, "Invalid pipe %d\n", - eld->pipe_id); + pdata->pipe_id); break; }
- memcpy(ctx->eld, eld->eld_data, sizeof(ctx->eld)); + memcpy(ctx->eld, ppdata->eld, sizeof(ctx->eld));
- ctx->dp_output = pdata->dp_output; + ctx->dp_output = ppdata->dp_output; if (ctx->dp_output) { ctx->tmds_clock_speed = 0; - ctx->link_rate = pdata->ls_clock; + ctx->link_rate = ppdata->ls_clock; } else { - ctx->tmds_clock_speed = pdata->ls_clock; + ctx->tmds_clock_speed = ppdata->ls_clock; ctx->link_rate = 0; }
From: Ville Syrjälä ville.syrjala@linux.intel.com
In preparation for register a PCM device for each pipe adjust link up the ctl elements with the corresponding PCM device.
Cc: Takashi Iwai tiwai@suse.de Cc: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- sound/x86/intel_hdmi_audio.c | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-)
diff --git a/sound/x86/intel_hdmi_audio.c b/sound/x86/intel_hdmi_audio.c index bfb712444098..a3d15482f07e 100644 --- a/sound/x86/intel_hdmi_audio.c +++ b/sound/x86/intel_hdmi_audio.c @@ -1609,11 +1609,16 @@ static void had_audio_wq(struct work_struct *work) /* * Jack interface */ -static int had_create_jack(struct snd_intelhad *ctx) +static int had_create_jack(struct snd_intelhad *ctx, + struct snd_pcm *pcm) { + char hdmi_str[32]; int err;
- err = snd_jack_new(ctx->card, "HDMI/DP", SND_JACK_AVOUT, &ctx->jack, + snprintf(hdmi_str, sizeof(hdmi_str), + "HDMI/DP,pcm=%d", pcm->device); + + err = snd_jack_new(ctx->card, hdmi_str, SND_JACK_AVOUT, &ctx->jack, true, false); if (err < 0) return err; @@ -1793,7 +1798,17 @@ static int hdmi_lpe_audio_probe(struct platform_device *pdev)
/* create controls */ for (i = 0; i < ARRAY_SIZE(had_controls); i++) { - ret = snd_ctl_add(card, snd_ctl_new1(&had_controls[i], ctx)); + struct snd_kcontrol *kctl; + + kctl = snd_ctl_new1(&had_controls[i], ctx); + if (!kctl) { + ret = -ENOMEM; + goto err; + } + + kctl->id.device = pcm->device; + + ret = snd_ctl_add(card, kctl); if (ret < 0) goto err; } @@ -1805,7 +1820,7 @@ static int hdmi_lpe_audio_probe(struct platform_device *pdev) if (ret < 0) goto err;
- ret = had_create_jack(ctx); + ret = had_create_jack(ctx, pcm); if (ret < 0) goto err;
From: Ville Syrjälä ville.syrjala@linux.intel.com
To allow multiple PCM devices to be registered for the LPE audio card, split the private data into card and PCM specific chunks. For now we'll stick to just one PCM device as before.
Cc: Takashi Iwai tiwai@suse.de Cc: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- sound/x86/intel_hdmi_audio.c | 228 +++++++++++++++++++++++++------------------ sound/x86/intel_hdmi_audio.h | 16 ++- 2 files changed, 144 insertions(+), 100 deletions(-)
diff --git a/sound/x86/intel_hdmi_audio.c b/sound/x86/intel_hdmi_audio.c index a3d15482f07e..5e2149fe5218 100644 --- a/sound/x86/intel_hdmi_audio.c +++ b/sound/x86/intel_hdmi_audio.c @@ -42,6 +42,9 @@ #include <drm/intel_lpe_audio.h> #include "intel_hdmi_audio.h"
+#define for_each_pipe(card_ctx, pipe) \ + for ((pipe) = 0; (pipe) < (card_ctx)->num_pipes; (pipe)++) + /*standard module options for ALSA. This module supports only one card*/ static int hdmi_card_index = SNDRV_DEFAULT_IDX1; static char *hdmi_card_id = SNDRV_DEFAULT_STR1; @@ -192,12 +195,12 @@ static void had_substream_put(struct snd_intelhad *intelhaddata) /* Register access functions */ static u32 had_read_register_raw(struct snd_intelhad *ctx, u32 reg) { - return ioread32(ctx->mmio_start + ctx->had_config_offset + reg); + return ioread32(ctx->card_ctx->mmio_start + ctx->had_config_offset + reg); }
static void had_write_register_raw(struct snd_intelhad *ctx, u32 reg, u32 val) { - iowrite32(val, ctx->mmio_start + ctx->had_config_offset + reg); + iowrite32(val, ctx->card_ctx->mmio_start + ctx->had_config_offset + reg); }
static void had_read_register(struct snd_intelhad *ctx, u32 reg, u32 *val) @@ -1519,22 +1522,27 @@ static const struct snd_kcontrol_new had_controls[] = { */ static irqreturn_t display_pipe_interrupt_handler(int irq, void *dev_id) { - struct snd_intelhad *ctx = dev_id; - u32 audio_stat; + struct snd_intelhad_card *card_ctx = dev_id; + int pipe;
- /* use raw register access to ack IRQs even while disconnected */ - audio_stat = had_read_register_raw(ctx, AUD_HDMI_STATUS); + for_each_pipe(card_ctx, pipe) { + struct snd_intelhad *ctx = &card_ctx->pcm_ctx[pipe]; + u32 audio_stat;
- if (audio_stat & HDMI_AUDIO_UNDERRUN) { - had_write_register_raw(ctx, AUD_HDMI_STATUS, - HDMI_AUDIO_UNDERRUN); - had_process_buffer_underrun(ctx); - } + /* use raw register access to ack IRQs even while disconnected */ + audio_stat = had_read_register_raw(ctx, AUD_HDMI_STATUS); + + if (audio_stat & HDMI_AUDIO_UNDERRUN) { + had_write_register_raw(ctx, AUD_HDMI_STATUS, + HDMI_AUDIO_UNDERRUN); + had_process_buffer_underrun(ctx); + }
- if (audio_stat & HDMI_AUDIO_BUFFER_DONE) { - had_write_register_raw(ctx, AUD_HDMI_STATUS, - HDMI_AUDIO_BUFFER_DONE); - had_process_buffer_done(ctx); + if (audio_stat & HDMI_AUDIO_BUFFER_DONE) { + had_write_register_raw(ctx, AUD_HDMI_STATUS, + HDMI_AUDIO_BUFFER_DONE); + had_process_buffer_done(ctx); + } }
return IRQ_HANDLED; @@ -1545,9 +1553,14 @@ static irqreturn_t display_pipe_interrupt_handler(int irq, void *dev_id) */ static void notify_audio_lpe(struct platform_device *pdev) { - struct snd_intelhad *ctx = platform_get_drvdata(pdev); + struct snd_intelhad_card *card_ctx = platform_get_drvdata(pdev); + int pipe; + + for_each_pipe(card_ctx, pipe) { + struct snd_intelhad *ctx = &card_ctx->pcm_ctx[pipe];
- schedule_work(&ctx->hdmi_audio_wq); + schedule_work(&ctx->hdmi_audio_wq); + } }
/* the work to handle monitor hot plug/unplug */ @@ -1618,7 +1631,8 @@ static int had_create_jack(struct snd_intelhad *ctx, snprintf(hdmi_str, sizeof(hdmi_str), "HDMI/DP,pcm=%d", pcm->device);
- err = snd_jack_new(ctx->card, hdmi_str, SND_JACK_AVOUT, &ctx->jack, + err = snd_jack_new(ctx->card_ctx->card, hdmi_str, + SND_JACK_AVOUT, &ctx->jack, true, false); if (err < 0) return err; @@ -1632,13 +1646,18 @@ static int had_create_jack(struct snd_intelhad *ctx,
static int hdmi_lpe_audio_runtime_suspend(struct device *dev) { - struct snd_intelhad *ctx = dev_get_drvdata(dev); - struct snd_pcm_substream *substream; + struct snd_intelhad_card *card_ctx = dev_get_drvdata(dev); + int pipe;
- substream = had_substream_get(ctx); - if (substream) { - snd_pcm_suspend(substream); - had_substream_put(ctx); + for_each_pipe(card_ctx, pipe) { + struct snd_intelhad *ctx = &card_ctx->pcm_ctx[pipe]; + struct snd_pcm_substream *substream; + + substream = had_substream_get(ctx); + if (substream) { + snd_pcm_suspend(substream); + had_substream_put(ctx); + } }
return 0; @@ -1646,12 +1665,12 @@ static int hdmi_lpe_audio_runtime_suspend(struct device *dev)
static int __maybe_unused hdmi_lpe_audio_suspend(struct device *dev) { - struct snd_intelhad *ctx = dev_get_drvdata(dev); + struct snd_intelhad_card *card_ctx = dev_get_drvdata(dev); int err;
err = hdmi_lpe_audio_runtime_suspend(dev); if (!err) - snd_power_change_state(ctx->card, SNDRV_CTL_POWER_D3hot); + snd_power_change_state(card_ctx->card, SNDRV_CTL_POWER_D3hot); return err; }
@@ -1663,29 +1682,34 @@ static int hdmi_lpe_audio_runtime_resume(struct device *dev)
static int __maybe_unused hdmi_lpe_audio_resume(struct device *dev) { - struct snd_intelhad *ctx = dev_get_drvdata(dev); + struct snd_intelhad_card *card_ctx = dev_get_drvdata(dev);
hdmi_lpe_audio_runtime_resume(dev); - snd_power_change_state(ctx->card, SNDRV_CTL_POWER_D0); + snd_power_change_state(card_ctx->card, SNDRV_CTL_POWER_D0); return 0; }
/* release resources */ static void hdmi_lpe_audio_free(struct snd_card *card) { - struct snd_intelhad *ctx = card->private_data; - struct intel_hdmi_lpe_audio_pdata *pdata = ctx->dev->platform_data; + struct snd_intelhad_card *card_ctx = card->private_data; + struct intel_hdmi_lpe_audio_pdata *pdata = card_ctx->dev->platform_data; + int pipe;
spin_lock_irq(&pdata->lpe_audio_slock); pdata->notify_audio_lpe = NULL; spin_unlock_irq(&pdata->lpe_audio_slock);
- cancel_work_sync(&ctx->hdmi_audio_wq); + for_each_pipe(card_ctx, pipe) { + struct snd_intelhad *ctx = &card_ctx->pcm_ctx[pipe]; + + cancel_work_sync(&ctx->hdmi_audio_wq); + }
- if (ctx->mmio_start) - iounmap(ctx->mmio_start); - if (ctx->irq >= 0) - free_irq(ctx->irq, ctx); + if (card_ctx->mmio_start) + iounmap(card_ctx->mmio_start); + if (card_ctx->irq >= 0) + free_irq(card_ctx->irq, card_ctx); }
/* @@ -1697,12 +1721,12 @@ static void hdmi_lpe_audio_free(struct snd_card *card) static int hdmi_lpe_audio_probe(struct platform_device *pdev) { struct snd_card *card; - struct snd_intelhad *ctx; + struct snd_intelhad_card *card_ctx; struct snd_pcm *pcm; struct intel_hdmi_lpe_audio_pdata *pdata; int irq; struct resource *res_mmio; - int i, ret; + int pipe, ret;
pdata = pdev->dev.platform_data; if (!pdata) { @@ -1725,39 +1749,30 @@ static int hdmi_lpe_audio_probe(struct platform_device *pdev)
/* create a card instance with ALSA framework */ ret = snd_card_new(&pdev->dev, hdmi_card_index, hdmi_card_id, - THIS_MODULE, sizeof(*ctx), &card); + THIS_MODULE, sizeof(*card_ctx), &card); if (ret) return ret;
- ctx = card->private_data; - spin_lock_init(&ctx->had_spinlock); - mutex_init(&ctx->mutex); - ctx->connected = false; - ctx->dev = &pdev->dev; - ctx->card = card; - ctx->aes_bits = SNDRV_PCM_DEFAULT_CON_SPDIF; + card_ctx = card->private_data; + card_ctx->dev = &pdev->dev; + card_ctx->card = card; strcpy(card->driver, INTEL_HAD); strcpy(card->shortname, "Intel HDMI/DP LPE Audio"); strcpy(card->longname, "Intel HDMI/DP LPE Audio");
- ctx->irq = -1; - ctx->tmds_clock_speed = DIS_SAMPLE_RATE_148_5; - INIT_WORK(&ctx->hdmi_audio_wq, had_audio_wq); + card_ctx->irq = -1;
card->private_free = hdmi_lpe_audio_free;
- /* assume pipe A as default */ - ctx->had_config_offset = AUDIO_HDMI_CONFIG_A; - - platform_set_drvdata(pdev, ctx); + platform_set_drvdata(pdev, card_ctx);
dev_dbg(&pdev->dev, "%s: mmio_start = 0x%x, mmio_end = 0x%x\n", __func__, (unsigned int)res_mmio->start, (unsigned int)res_mmio->end);
- ctx->mmio_start = ioremap_nocache(res_mmio->start, - (size_t)(resource_size(res_mmio))); - if (!ctx->mmio_start) { + card_ctx->mmio_start = ioremap_nocache(res_mmio->start, + (size_t)(resource_size(res_mmio))); + if (!card_ctx->mmio_start) { dev_err(&pdev->dev, "Could not get ioremap\n"); ret = -EACCES; goto err; @@ -1765,65 +1780,80 @@ static int hdmi_lpe_audio_probe(struct platform_device *pdev)
/* setup interrupt handler */ ret = request_irq(irq, display_pipe_interrupt_handler, 0, - pdev->name, ctx); + pdev->name, card_ctx); if (ret < 0) { dev_err(&pdev->dev, "request_irq failed\n"); goto err; }
- ctx->irq = irq; - - ret = snd_pcm_new(card, INTEL_HAD, PCM_INDEX, MAX_PB_STREAMS, - MAX_CAP_STREAMS, &pcm); - if (ret) - goto err; - - /* setup private data which can be retrieved when required */ - pcm->private_data = ctx; - pcm->info_flags = 0; - strncpy(pcm->name, card->shortname, strlen(card->shortname)); - /* setup the ops for playabck */ - snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_PLAYBACK, &had_pcm_ops); + card_ctx->irq = irq;
/* only 32bit addressable */ dma_set_mask(&pdev->dev, DMA_BIT_MASK(32)); dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(32));
- /* allocate dma pages; - * try to allocate 600k buffer as default which is large enough - */ - snd_pcm_lib_preallocate_pages_for_all(pcm, - SNDRV_DMA_TYPE_DEV, NULL, - HAD_DEFAULT_BUFFER, HAD_MAX_BUFFER); + init_channel_allocations();
- /* create controls */ - for (i = 0; i < ARRAY_SIZE(had_controls); i++) { - struct snd_kcontrol *kctl; + card_ctx->num_pipes = 1;
- kctl = snd_ctl_new1(&had_controls[i], ctx); - if (!kctl) { - ret = -ENOMEM; + for_each_pipe(card_ctx, pipe) { + struct snd_intelhad *ctx = &card_ctx->pcm_ctx[pipe]; + int i; + + ctx->card_ctx = card_ctx; + ctx->dev = card_ctx->dev; + ctx->pipe = pipe; + + INIT_WORK(&ctx->hdmi_audio_wq, had_audio_wq); + + ctx->had_config_offset = AUDIO_HDMI_CONFIG_A; + + ret = snd_pcm_new(card, INTEL_HAD, PCM_INDEX, MAX_PB_STREAMS, + MAX_CAP_STREAMS, &pcm); + if (ret) goto err; + + /* setup private data which can be retrieved when required */ + pcm->private_data = ctx; + pcm->info_flags = 0; + strncpy(pcm->name, card->shortname, strlen(card->shortname)); + /* setup the ops for playabck */ + snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_PLAYBACK, &had_pcm_ops); + + /* allocate dma pages; + * try to allocate 600k buffer as default which is large enough + */ + snd_pcm_lib_preallocate_pages_for_all(pcm, + SNDRV_DMA_TYPE_DEV, NULL, + HAD_DEFAULT_BUFFER, HAD_MAX_BUFFER); + + /* create controls */ + for (i = 0; i < ARRAY_SIZE(had_controls); i++) { + struct snd_kcontrol *kctl; + + kctl = snd_ctl_new1(&had_controls[i], ctx); + if (!kctl) { + ret = -ENOMEM; + goto err; + } + + kctl->id.device = pcm->device; + + ret = snd_ctl_add(card, kctl); + if (ret < 0) + goto err; }
- kctl->id.device = pcm->device; + /* Register channel map controls */ + ret = had_register_chmap_ctls(ctx, pcm); + if (ret < 0) + goto err;
- ret = snd_ctl_add(card, kctl); + ret = had_create_jack(ctx, pcm); if (ret < 0) goto err; }
- init_channel_allocations(); - - /* Register channel map controls */ - ret = had_register_chmap_ctls(ctx, pcm); - if (ret < 0) - goto err; - - ret = had_create_jack(ctx, pcm); - if (ret < 0) - goto err; - ret = snd_card_register(card); if (ret) goto err; @@ -1837,7 +1867,11 @@ static int hdmi_lpe_audio_probe(struct platform_device *pdev) pm_runtime_set_active(&pdev->dev);
dev_dbg(&pdev->dev, "%s: handle pending notification\n", __func__); - schedule_work(&ctx->hdmi_audio_wq); + for_each_pipe(card_ctx, pipe) { + struct snd_intelhad *ctx = &card_ctx->pcm_ctx[pipe]; + + schedule_work(&ctx->hdmi_audio_wq); + }
return 0;
@@ -1853,9 +1887,9 @@ static int hdmi_lpe_audio_probe(struct platform_device *pdev) */ static int hdmi_lpe_audio_remove(struct platform_device *pdev) { - struct snd_intelhad *ctx = platform_get_drvdata(pdev); + struct snd_intelhad_card *card_ctx = platform_get_drvdata(pdev);
- snd_card_free(ctx->card); + snd_card_free(card_ctx->card); return 0; }
diff --git a/sound/x86/intel_hdmi_audio.h b/sound/x86/intel_hdmi_audio.h index 2d3e389f76b3..a209096b03df 100644 --- a/sound/x86/intel_hdmi_audio.h +++ b/sound/x86/intel_hdmi_audio.h @@ -101,7 +101,7 @@ struct pcm_stream_info { * @chmap: holds channel map info */ struct snd_intelhad { - struct snd_card *card; + struct snd_intelhad_card *card_ctx; bool connected; struct pcm_stream_info stream_info; unsigned char eld[HDMI_MAX_ELD_BYTES]; @@ -112,6 +112,7 @@ struct snd_intelhad { struct snd_pcm_chmap *chmap; int tmds_clock_speed; int link_rate; + int pipe;
/* ring buffer (BD) position index */ unsigned int bd_head; @@ -123,8 +124,6 @@ struct snd_intelhad { unsigned int period_bytes; /* PCM period size in bytes */
/* internal stuff */ - int irq; - void __iomem *mmio_start; unsigned int had_config_offset; union aud_cfg aud_config; /* AUD_CONFIG reg value cache */ struct work_struct hdmi_audio_wq; @@ -133,4 +132,15 @@ struct snd_intelhad { struct snd_jack *jack; };
+struct snd_intelhad_card { + struct snd_card *card; + struct device *dev; + + /* internal stuff */ + int irq; + void __iomem *mmio_start; + int num_pipes; + struct snd_intelhad pcm_ctx[3]; +}; + #endif /* _INTEL_HDMI_AUDIO_ */
From: Ville Syrjälä ville.syrjala@linux.intel.com
Now that everything is in place let's register a PCM device for each pipe of the display engine. This will make it possible to actually output audio to multiple displays at the same time. And it avoids modesets on unrelated displays from clobbering up the ELD and whatnot for the display currently doing the playback.
The alternative would be to have a PCM device per port, but per-pipe is easier since the hardware actually works that way.
Cc: Takashi Iwai tiwai@suse.de Cc: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/i915/intel_lpe_audio.c | 14 ++++----- include/drm/intel_lpe_audio.h | 6 ++-- sound/x86/intel_hdmi_audio.c | 53 +++++++++++++++------------------- sound/x86/intel_hdmi_audio.h | 3 +- 4 files changed, 34 insertions(+), 42 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c index a593fdf73171..270aa3e3f0e2 100644 --- a/drivers/gpu/drm/i915/intel_lpe_audio.c +++ b/drivers/gpu/drm/i915/intel_lpe_audio.c @@ -111,6 +111,7 @@ lpe_audio_platdev_create(struct drm_i915_private *dev_priv) pinfo.size_data = sizeof(*pdata); pinfo.dma_mask = DMA_BIT_MASK(32);
+ pdata->num_pipes = INTEL_INFO(dev_priv)->num_pipes; spin_lock_init(&pdata->lpe_audio_slock);
platdev = platform_device_register_full(&pinfo); @@ -318,7 +319,7 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv, enum pipe pipe, enum port port, const void *eld, int ls_clock, bool dp_output) { - unsigned long irq_flags; + unsigned long irqflags; struct intel_hdmi_lpe_audio_pdata *pdata; struct intel_hdmi_lpe_audio_pipe_pdata *ppdata; u32 audio_enable; @@ -327,14 +328,12 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv, return;
pdata = dev_get_platdata(&dev_priv->lpe_audio.platdev->dev); - ppdata = &pdata->pipe; + ppdata = &pdata->pipe[pipe];
- spin_lock_irqsave(&pdata->lpe_audio_slock, irq_flags); + spin_lock_irqsave(&pdata->lpe_audio_slock, irqflags);
audio_enable = I915_READ(VLV_AUD_PORT_EN_DBG(port));
- pdata->pipe_id = pipe; - if (eld != NULL) { memcpy(ppdata->eld, eld, HDMI_MAX_ELD_BYTES); ppdata->port = port; @@ -356,8 +355,7 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv, }
if (pdata->notify_audio_lpe) - pdata->notify_audio_lpe(dev_priv->lpe_audio.platdev); + pdata->notify_audio_lpe(dev_priv->lpe_audio.platdev, pipe);
- spin_unlock_irqrestore(&pdata->lpe_audio_slock, - irq_flags); + spin_unlock_irqrestore(&pdata->lpe_audio_slock, irqflags); } diff --git a/include/drm/intel_lpe_audio.h b/include/drm/intel_lpe_audio.h index 26e569ad8683..b391b2822140 100644 --- a/include/drm/intel_lpe_audio.h +++ b/include/drm/intel_lpe_audio.h @@ -39,10 +39,10 @@ struct intel_hdmi_lpe_audio_pipe_pdata { };
struct intel_hdmi_lpe_audio_pdata { - struct intel_hdmi_lpe_audio_pipe_pdata pipe; - int pipe_id; + struct intel_hdmi_lpe_audio_pipe_pdata pipe[3]; + int num_pipes;
- void (*notify_audio_lpe)(struct platform_device *pdev); + void (*notify_audio_lpe)(struct platform_device *pdev, int pipe); spinlock_t lpe_audio_slock; };
diff --git a/sound/x86/intel_hdmi_audio.c b/sound/x86/intel_hdmi_audio.c index 5e2149fe5218..e5863a6d3aa9 100644 --- a/sound/x86/intel_hdmi_audio.c +++ b/sound/x86/intel_hdmi_audio.c @@ -195,12 +195,12 @@ static void had_substream_put(struct snd_intelhad *intelhaddata) /* Register access functions */ static u32 had_read_register_raw(struct snd_intelhad *ctx, u32 reg) { - return ioread32(ctx->card_ctx->mmio_start + ctx->had_config_offset + reg); + return ioread32(ctx->mmio_start + reg); }
static void had_write_register_raw(struct snd_intelhad *ctx, u32 reg, u32 val) { - iowrite32(val, ctx->card_ctx->mmio_start + ctx->had_config_offset + reg); + iowrite32(val, ctx->mmio_start + reg); }
static void had_read_register(struct snd_intelhad *ctx, u32 reg, u32 *val) @@ -1551,16 +1551,12 @@ static irqreturn_t display_pipe_interrupt_handler(int irq, void *dev_id) /* * monitor plug/unplug notification from i915; just kick off the work */ -static void notify_audio_lpe(struct platform_device *pdev) +static void notify_audio_lpe(struct platform_device *pdev, int pipe) { struct snd_intelhad_card *card_ctx = platform_get_drvdata(pdev); - int pipe; - - for_each_pipe(card_ctx, pipe) { - struct snd_intelhad *ctx = &card_ctx->pcm_ctx[pipe]; + struct snd_intelhad *ctx = &card_ctx->pcm_ctx[pipe];
- schedule_work(&ctx->hdmi_audio_wq); - } + schedule_work(&ctx->hdmi_audio_wq); }
/* the work to handle monitor hot plug/unplug */ @@ -1569,7 +1565,7 @@ static void had_audio_wq(struct work_struct *work) struct snd_intelhad *ctx = container_of(work, struct snd_intelhad, hdmi_audio_wq); struct intel_hdmi_lpe_audio_pdata *pdata = ctx->dev->platform_data; - struct intel_hdmi_lpe_audio_pipe_pdata *ppdata = &pdata->pipe; + struct intel_hdmi_lpe_audio_pipe_pdata *ppdata = &pdata->pipe[ctx->pipe];
pm_runtime_get_sync(ctx->dev); mutex_lock(&ctx->mutex); @@ -1582,22 +1578,6 @@ static void had_audio_wq(struct work_struct *work) dev_dbg(ctx->dev, "%s: HAD_NOTIFY_ELD : port = %d, tmds = %d\n", __func__, ppdata->port, ppdata->ls_clock);
- switch (pdata->pipe_id) { - case 0: - ctx->had_config_offset = AUDIO_HDMI_CONFIG_A; - break; - case 1: - ctx->had_config_offset = AUDIO_HDMI_CONFIG_B; - break; - case 2: - ctx->had_config_offset = AUDIO_HDMI_CONFIG_C; - break; - default: - dev_dbg(ctx->dev, "Invalid pipe %d\n", - pdata->pipe_id); - break; - } - memcpy(ctx->eld, ppdata->eld, sizeof(ctx->eld));
ctx->dp_output = ppdata->dp_output; @@ -1794,7 +1774,7 @@ static int hdmi_lpe_audio_probe(struct platform_device *pdev)
init_channel_allocations();
- card_ctx->num_pipes = 1; + card_ctx->num_pipes = pdata->num_pipes;
for_each_pipe(card_ctx, pipe) { struct snd_intelhad *ctx = &card_ctx->pcm_ctx[pipe]; @@ -1806,9 +1786,24 @@ static int hdmi_lpe_audio_probe(struct platform_device *pdev)
INIT_WORK(&ctx->hdmi_audio_wq, had_audio_wq);
- ctx->had_config_offset = AUDIO_HDMI_CONFIG_A; + switch (pipe) { + case 0: + ctx->mmio_start = + card_ctx->mmio_start + AUDIO_HDMI_CONFIG_A; + break; + case 1: + ctx->mmio_start = + card_ctx->mmio_start + AUDIO_HDMI_CONFIG_B; + break; + case 2: + ctx->mmio_start = + card_ctx->mmio_start + AUDIO_HDMI_CONFIG_C; + break; + default: + break; + }
- ret = snd_pcm_new(card, INTEL_HAD, PCM_INDEX, MAX_PB_STREAMS, + ret = snd_pcm_new(card, INTEL_HAD, pipe, MAX_PB_STREAMS, MAX_CAP_STREAMS, &pcm); if (ret) goto err; diff --git a/sound/x86/intel_hdmi_audio.h b/sound/x86/intel_hdmi_audio.h index a209096b03df..ab0de5d525f4 100644 --- a/sound/x86/intel_hdmi_audio.h +++ b/sound/x86/intel_hdmi_audio.h @@ -32,7 +32,6 @@
#include "intel_hdmi_lpe_audio.h"
-#define PCM_INDEX 0 #define MAX_PB_STREAMS 1 #define MAX_CAP_STREAMS 0 #define BYTES_PER_WORD 0x4 @@ -124,7 +123,7 @@ struct snd_intelhad { unsigned int period_bytes; /* PCM period size in bytes */
/* internal stuff */ - unsigned int had_config_offset; + void __iomem *mmio_start; union aud_cfg aud_config; /* AUD_CONFIG reg value cache */ struct work_struct hdmi_audio_wq; struct mutex mutex; /* for protecting chmap and eld */
On 04/25/2017 03:27 PM, ville.syrjala@linux.intel.com wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
Now that everything is in place let's register a PCM device for each pipe of the display engine. This will make it possible to actually output audio to multiple displays at the same time. And it avoids modesets on unrelated displays from clobbering up the ELD and whatnot for the display currently doing the playback.
The alternative would be to have a PCM device per port, but per-pipe is easier since the hardware actually works that way.
Very nice. I just tested on a CHT Zotac box which has two connectors (1 HDMI and 1 DP), and I get sound concurrently on both, with hdmi being listed as device 2 and DP as device 0. I thought there were hardware restrictions but you proved me wrong. Kudos.
The only point that I find weird is that the jacks are reported as 'on' on the 3 pipes, is there a way to tie them to an actual cable being used?
[plb@ZOTAC ~]$ amixer -Dhw:0 controls | grep Jack numid=5,iface=CARD,name='HDMI/DP,pcm=0 Jack' numid=10,iface=CARD,name='HDMI/DP,pcm=1 Jack' numid=15,iface=CARD,name='HDMI/DP,pcm=2 Jack' [plb@ZOTAC ~]$ amixer -Dhw:0 cget numid=5 numid=5,iface=CARD,name='HDMI/DP,pcm=0 Jack' ; type=BOOLEAN,access=r-------,values=1 : values=on [plb@ZOTAC ~]$ amixer -Dhw:0 cset numid=5 off amixer: Control hw:0 element write error: Operation not permitted
[plb@ZOTAC ~]$ amixer -Dhw:0 cget numid=10 numid=10,iface=CARD,name='HDMI/DP,pcm=1 Jack' ; type=BOOLEAN,access=r-------,values=1 : values=on [plb@ZOTAC ~]$ amixer -Dhw:0 cget numid=15 numid=15,iface=CARD,name='HDMI/DP,pcm=2 Jack' ; type=BOOLEAN,access=r-------,values=1 : values=on
The ELD controls do show a null set of values for device 1, maybe the jack value should be set in accordance with the ELD validity? Also I am wondering if the display number could be used for the PCM device number, or as a hint in the device description to help the user know which PCM device to use.
Anyway thanks for this patchset, nicely done.
Cc: Takashi Iwai tiwai@suse.de Cc: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
drivers/gpu/drm/i915/intel_lpe_audio.c | 14 ++++----- include/drm/intel_lpe_audio.h | 6 ++-- sound/x86/intel_hdmi_audio.c | 53 +++++++++++++++------------------- sound/x86/intel_hdmi_audio.h | 3 +- 4 files changed, 34 insertions(+), 42 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c index a593fdf73171..270aa3e3f0e2 100644 --- a/drivers/gpu/drm/i915/intel_lpe_audio.c +++ b/drivers/gpu/drm/i915/intel_lpe_audio.c @@ -111,6 +111,7 @@ lpe_audio_platdev_create(struct drm_i915_private *dev_priv) pinfo.size_data = sizeof(*pdata); pinfo.dma_mask = DMA_BIT_MASK(32);
pdata->num_pipes = INTEL_INFO(dev_priv)->num_pipes; spin_lock_init(&pdata->lpe_audio_slock);
platdev = platform_device_register_full(&pinfo);
@@ -318,7 +319,7 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv, enum pipe pipe, enum port port, const void *eld, int ls_clock, bool dp_output) {
- unsigned long irq_flags;
- unsigned long irqflags; struct intel_hdmi_lpe_audio_pdata *pdata; struct intel_hdmi_lpe_audio_pipe_pdata *ppdata; u32 audio_enable;
@@ -327,14 +328,12 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv, return;
pdata = dev_get_platdata(&dev_priv->lpe_audio.platdev->dev);
- ppdata = &pdata->pipe;
- ppdata = &pdata->pipe[pipe];
- spin_lock_irqsave(&pdata->lpe_audio_slock, irq_flags);
spin_lock_irqsave(&pdata->lpe_audio_slock, irqflags);
audio_enable = I915_READ(VLV_AUD_PORT_EN_DBG(port));
- pdata->pipe_id = pipe;
- if (eld != NULL) { memcpy(ppdata->eld, eld, HDMI_MAX_ELD_BYTES); ppdata->port = port;
@@ -356,8 +355,7 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv, }
if (pdata->notify_audio_lpe)
pdata->notify_audio_lpe(dev_priv->lpe_audio.platdev);
pdata->notify_audio_lpe(dev_priv->lpe_audio.platdev, pipe);
- spin_unlock_irqrestore(&pdata->lpe_audio_slock,
irq_flags);
- spin_unlock_irqrestore(&pdata->lpe_audio_slock, irqflags); }
diff --git a/include/drm/intel_lpe_audio.h b/include/drm/intel_lpe_audio.h index 26e569ad8683..b391b2822140 100644 --- a/include/drm/intel_lpe_audio.h +++ b/include/drm/intel_lpe_audio.h @@ -39,10 +39,10 @@ struct intel_hdmi_lpe_audio_pipe_pdata { };
struct intel_hdmi_lpe_audio_pdata {
- struct intel_hdmi_lpe_audio_pipe_pdata pipe;
- int pipe_id;
- struct intel_hdmi_lpe_audio_pipe_pdata pipe[3];
- int num_pipes;
- void (*notify_audio_lpe)(struct platform_device *pdev);
- void (*notify_audio_lpe)(struct platform_device *pdev, int pipe); spinlock_t lpe_audio_slock; };
diff --git a/sound/x86/intel_hdmi_audio.c b/sound/x86/intel_hdmi_audio.c index 5e2149fe5218..e5863a6d3aa9 100644 --- a/sound/x86/intel_hdmi_audio.c +++ b/sound/x86/intel_hdmi_audio.c @@ -195,12 +195,12 @@ static void had_substream_put(struct snd_intelhad *intelhaddata) /* Register access functions */ static u32 had_read_register_raw(struct snd_intelhad *ctx, u32 reg) {
- return ioread32(ctx->card_ctx->mmio_start + ctx->had_config_offset + reg);
return ioread32(ctx->mmio_start + reg); }
static void had_write_register_raw(struct snd_intelhad *ctx, u32 reg, u32 val) {
- iowrite32(val, ctx->card_ctx->mmio_start + ctx->had_config_offset + reg);
iowrite32(val, ctx->mmio_start + reg); }
static void had_read_register(struct snd_intelhad *ctx, u32 reg, u32 *val)
@@ -1551,16 +1551,12 @@ static irqreturn_t display_pipe_interrupt_handler(int irq, void *dev_id) /*
- monitor plug/unplug notification from i915; just kick off the work
*/ -static void notify_audio_lpe(struct platform_device *pdev) +static void notify_audio_lpe(struct platform_device *pdev, int pipe) { struct snd_intelhad_card *card_ctx = platform_get_drvdata(pdev);
- int pipe;
- for_each_pipe(card_ctx, pipe) {
struct snd_intelhad *ctx = &card_ctx->pcm_ctx[pipe];
- struct snd_intelhad *ctx = &card_ctx->pcm_ctx[pipe];
schedule_work(&ctx->hdmi_audio_wq);- }
schedule_work(&ctx->hdmi_audio_wq); }
/* the work to handle monitor hot plug/unplug */
@@ -1569,7 +1565,7 @@ static void had_audio_wq(struct work_struct *work) struct snd_intelhad *ctx = container_of(work, struct snd_intelhad, hdmi_audio_wq); struct intel_hdmi_lpe_audio_pdata *pdata = ctx->dev->platform_data;
- struct intel_hdmi_lpe_audio_pipe_pdata *ppdata = &pdata->pipe;
struct intel_hdmi_lpe_audio_pipe_pdata *ppdata = &pdata->pipe[ctx->pipe];
pm_runtime_get_sync(ctx->dev); mutex_lock(&ctx->mutex);
@@ -1582,22 +1578,6 @@ static void had_audio_wq(struct work_struct *work) dev_dbg(ctx->dev, "%s: HAD_NOTIFY_ELD : port = %d, tmds = %d\n", __func__, ppdata->port, ppdata->ls_clock);
switch (pdata->pipe_id) {case 0:ctx->had_config_offset = AUDIO_HDMI_CONFIG_A;break;case 1:ctx->had_config_offset = AUDIO_HDMI_CONFIG_B;break;case 2:ctx->had_config_offset = AUDIO_HDMI_CONFIG_C;break;default:dev_dbg(ctx->dev, "Invalid pipe %d\n",pdata->pipe_id);break;}memcpy(ctx->eld, ppdata->eld, sizeof(ctx->eld));
ctx->dp_output = ppdata->dp_output;
@@ -1794,7 +1774,7 @@ static int hdmi_lpe_audio_probe(struct platform_device *pdev)
init_channel_allocations();
- card_ctx->num_pipes = 1;
card_ctx->num_pipes = pdata->num_pipes;
for_each_pipe(card_ctx, pipe) { struct snd_intelhad *ctx = &card_ctx->pcm_ctx[pipe];
@@ -1806,9 +1786,24 @@ static int hdmi_lpe_audio_probe(struct platform_device *pdev)
INIT_WORK(&ctx->hdmi_audio_wq, had_audio_wq);
ctx->had_config_offset = AUDIO_HDMI_CONFIG_A;
switch (pipe) {case 0:ctx->mmio_start =card_ctx->mmio_start + AUDIO_HDMI_CONFIG_A;break;case 1:ctx->mmio_start =card_ctx->mmio_start + AUDIO_HDMI_CONFIG_B;break;case 2:ctx->mmio_start =card_ctx->mmio_start + AUDIO_HDMI_CONFIG_C;break;default:break;}
ret = snd_pcm_new(card, INTEL_HAD, PCM_INDEX, MAX_PB_STREAMS,
ret = snd_pcm_new(card, INTEL_HAD, pipe, MAX_PB_STREAMS, MAX_CAP_STREAMS, &pcm);diff --git a/sound/x86/intel_hdmi_audio.h b/sound/x86/intel_hdmi_audio.h index a209096b03df..ab0de5d525f4 100644 --- a/sound/x86/intel_hdmi_audio.h +++ b/sound/x86/intel_hdmi_audio.h @@ -32,7 +32,6 @@
#include "intel_hdmi_lpe_audio.h"
-#define PCM_INDEX 0 #define MAX_PB_STREAMS 1 #define MAX_CAP_STREAMS 0 #define BYTES_PER_WORD 0x4 @@ -124,7 +123,7 @@ struct snd_intelhad { unsigned int period_bytes; /* PCM period size in bytes */
/* internal stuff */
- unsigned int had_config_offset;
- void __iomem *mmio_start; union aud_cfg aud_config; /* AUD_CONFIG reg value cache */ struct work_struct hdmi_audio_wq; struct mutex mutex; /* for protecting chmap and eld */
On Wed, 26 Apr 2017 03:58:57 +0200, Pierre-Louis Bossart wrote:
On 04/25/2017 03:27 PM, ville.syrjala@linux.intel.com wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
Now that everything is in place let's register a PCM device for each pipe of the display engine. This will make it possible to actually output audio to multiple displays at the same time. And it avoids modesets on unrelated displays from clobbering up the ELD and whatnot for the display currently doing the playback.
The alternative would be to have a PCM device per port, but per-pipe is easier since the hardware actually works that way.
Very nice. I just tested on a CHT Zotac box which has two connectors (1 HDMI and 1 DP), and I get sound concurrently on both, with hdmi being listed as device 2 and DP as device 0. I thought there were hardware restrictions but you proved me wrong. Kudos.
The only point that I find weird is that the jacks are reported as 'on' on the 3 pipes, is there a way to tie them to an actual cable being used?
The pdata check was changed to check port=-1 as the monitor off in the patch 6. Maybe the initialization is missing?
Takashi
[plb@ZOTAC ~]$ amixer -Dhw:0 controls | grep Jack numid=5,iface=CARD,name='HDMI/DP,pcm=0 Jack' numid=10,iface=CARD,name='HDMI/DP,pcm=1 Jack' numid=15,iface=CARD,name='HDMI/DP,pcm=2 Jack' [plb@ZOTAC ~]$ amixer -Dhw:0 cget numid=5 numid=5,iface=CARD,name='HDMI/DP,pcm=0 Jack' ; type=BOOLEAN,access=r-------,values=1 : values=on [plb@ZOTAC ~]$ amixer -Dhw:0 cset numid=5 off amixer: Control hw:0 element write error: Operation not permitted
[plb@ZOTAC ~]$ amixer -Dhw:0 cget numid=10 numid=10,iface=CARD,name='HDMI/DP,pcm=1 Jack' ; type=BOOLEAN,access=r-------,values=1 : values=on [plb@ZOTAC ~]$ amixer -Dhw:0 cget numid=15 numid=15,iface=CARD,name='HDMI/DP,pcm=2 Jack' ; type=BOOLEAN,access=r-------,values=1 : values=on
The ELD controls do show a null set of values for device 1, maybe the jack value should be set in accordance with the ELD validity? Also I am wondering if the display number could be used for the PCM device number, or as a hint in the device description to help the user know which PCM device to use.
Anyway thanks for this patchset, nicely done.
Cc: Takashi Iwai tiwai@suse.de Cc: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
drivers/gpu/drm/i915/intel_lpe_audio.c | 14 ++++----- include/drm/intel_lpe_audio.h | 6 ++-- sound/x86/intel_hdmi_audio.c | 53 +++++++++++++++------------------- sound/x86/intel_hdmi_audio.h | 3 +- 4 files changed, 34 insertions(+), 42 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c index a593fdf73171..270aa3e3f0e2 100644 --- a/drivers/gpu/drm/i915/intel_lpe_audio.c +++ b/drivers/gpu/drm/i915/intel_lpe_audio.c @@ -111,6 +111,7 @@ lpe_audio_platdev_create(struct drm_i915_private *dev_priv) pinfo.size_data = sizeof(*pdata); pinfo.dma_mask = DMA_BIT_MASK(32);
- pdata->num_pipes = INTEL_INFO(dev_priv)->num_pipes;
spin_lock_init(&pdata->lpe_audio_slock); platdev = platform_device_register_full(&pinfo); @@ -318,7 +319,7 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv, enum pipe pipe, enum port port, const void *eld, int ls_clock, bool dp_output) {
- unsigned long irq_flags;
- unsigned long irqflags; struct intel_hdmi_lpe_audio_pdata *pdata; struct intel_hdmi_lpe_audio_pipe_pdata *ppdata; u32 audio_enable;
@@ -327,14 +328,12 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv, return; pdata = dev_get_platdata(&dev_priv->lpe_audio.platdev->dev);
- ppdata = &pdata->pipe;
- ppdata = &pdata->pipe[pipe];
- spin_lock_irqsave(&pdata->lpe_audio_slock, irq_flags);
- spin_lock_irqsave(&pdata->lpe_audio_slock, irqflags); audio_enable = I915_READ(VLV_AUD_PORT_EN_DBG(port));
- pdata->pipe_id = pipe;
- if (eld != NULL) { memcpy(ppdata->eld, eld, HDMI_MAX_ELD_BYTES); ppdata->port = port;
@@ -356,8 +355,7 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv, } if (pdata->notify_audio_lpe)
pdata->notify_audio_lpe(dev_priv->lpe_audio.platdev);
pdata->notify_audio_lpe(dev_priv->lpe_audio.platdev, pipe);
- spin_unlock_irqrestore(&pdata->lpe_audio_slock,
irq_flags);
- spin_unlock_irqrestore(&pdata->lpe_audio_slock, irqflags); }
diff --git a/include/drm/intel_lpe_audio.h b/include/drm/intel_lpe_audio.h index 26e569ad8683..b391b2822140 100644 --- a/include/drm/intel_lpe_audio.h +++ b/include/drm/intel_lpe_audio.h @@ -39,10 +39,10 @@ struct intel_hdmi_lpe_audio_pipe_pdata { }; struct intel_hdmi_lpe_audio_pdata {
- struct intel_hdmi_lpe_audio_pipe_pdata pipe;
- int pipe_id;
- struct intel_hdmi_lpe_audio_pipe_pdata pipe[3];
- int num_pipes;
- void (*notify_audio_lpe)(struct platform_device *pdev);
- void (*notify_audio_lpe)(struct platform_device *pdev, int pipe); spinlock_t lpe_audio_slock; }; diff --git a/sound/x86/intel_hdmi_audio.c
b/sound/x86/intel_hdmi_audio.c index 5e2149fe5218..e5863a6d3aa9 100644 --- a/sound/x86/intel_hdmi_audio.c +++ b/sound/x86/intel_hdmi_audio.c @@ -195,12 +195,12 @@ static void had_substream_put(struct snd_intelhad *intelhaddata) /* Register access functions */ static u32 had_read_register_raw(struct snd_intelhad *ctx, u32 reg) {
- return ioread32(ctx->card_ctx->mmio_start + ctx->had_config_offset + reg);
- return ioread32(ctx->mmio_start + reg); } static void had_write_register_raw(struct snd_intelhad *ctx, u32
reg, u32 val) {
- iowrite32(val, ctx->card_ctx->mmio_start + ctx->had_config_offset + reg);
- iowrite32(val, ctx->mmio_start + reg); } static void had_read_register(struct snd_intelhad *ctx, u32 reg,
u32 *val) @@ -1551,16 +1551,12 @@ static irqreturn_t display_pipe_interrupt_handler(int irq, void *dev_id) /*
- monitor plug/unplug notification from i915; just kick off the work
*/ -static void notify_audio_lpe(struct platform_device *pdev) +static void notify_audio_lpe(struct platform_device *pdev, int pipe) { struct snd_intelhad_card *card_ctx = platform_get_drvdata(pdev);
- int pipe;
- for_each_pipe(card_ctx, pipe) {
struct snd_intelhad *ctx = &card_ctx->pcm_ctx[pipe];
- struct snd_intelhad *ctx = &card_ctx->pcm_ctx[pipe];
schedule_work(&ctx->hdmi_audio_wq);
- }
- schedule_work(&ctx->hdmi_audio_wq); } /* the work to handle monitor hot plug/unplug */
@@ -1569,7 +1565,7 @@ static void had_audio_wq(struct work_struct *work) struct snd_intelhad *ctx = container_of(work, struct snd_intelhad, hdmi_audio_wq); struct intel_hdmi_lpe_audio_pdata *pdata = ctx->dev->platform_data;
- struct intel_hdmi_lpe_audio_pipe_pdata *ppdata = &pdata->pipe;
- struct intel_hdmi_lpe_audio_pipe_pdata *ppdata = &pdata->pipe[ctx->pipe]; pm_runtime_get_sync(ctx->dev); mutex_lock(&ctx->mutex);
@@ -1582,22 +1578,6 @@ static void had_audio_wq(struct work_struct *work) dev_dbg(ctx->dev, "%s: HAD_NOTIFY_ELD : port = %d, tmds = %d\n", __func__, ppdata->port, ppdata->ls_clock);
switch (pdata->pipe_id) {case 0:ctx->had_config_offset = AUDIO_HDMI_CONFIG_A;break;case 1:ctx->had_config_offset = AUDIO_HDMI_CONFIG_B;break;case 2:ctx->had_config_offset = AUDIO_HDMI_CONFIG_C;break;default:dev_dbg(ctx->dev, "Invalid pipe %d\n",pdata->pipe_id);break;}- memcpy(ctx->eld, ppdata->eld, sizeof(ctx->eld)); ctx->dp_output = ppdata->dp_output;
@@ -1794,7 +1774,7 @@ static int hdmi_lpe_audio_probe(struct platform_device *pdev) init_channel_allocations();
- card_ctx->num_pipes = 1;
- card_ctx->num_pipes = pdata->num_pipes; for_each_pipe(card_ctx, pipe) { struct snd_intelhad *ctx = &card_ctx->pcm_ctx[pipe];
@@ -1806,9 +1786,24 @@ static int hdmi_lpe_audio_probe(struct platform_device *pdev) INIT_WORK(&ctx->hdmi_audio_wq, had_audio_wq);
ctx->had_config_offset = AUDIO_HDMI_CONFIG_A;
switch (pipe) {case 0:ctx->mmio_start =card_ctx->mmio_start + AUDIO_HDMI_CONFIG_A;break;case 1:ctx->mmio_start =card_ctx->mmio_start + AUDIO_HDMI_CONFIG_B;break;case 2:ctx->mmio_start =card_ctx->mmio_start + AUDIO_HDMI_CONFIG_C;break;default:break;}
ret = snd_pcm_new(card, INTEL_HAD, PCM_INDEX,MAX_PB_STREAMS,
ret = snd_pcm_new(card, INTEL_HAD, pipe, MAX_PB_STREAMS, MAX_CAP_STREAMS, &pcm);diff --git a/sound/x86/intel_hdmi_audio.h b/sound/x86/intel_hdmi_audio.h index a209096b03df..ab0de5d525f4 100644 --- a/sound/x86/intel_hdmi_audio.h +++ b/sound/x86/intel_hdmi_audio.h @@ -32,7 +32,6 @@ #include "intel_hdmi_lpe_audio.h" -#define PCM_INDEX 0 #define MAX_PB_STREAMS 1 #define MAX_CAP_STREAMS 0 #define BYTES_PER_WORD 0x4 @@ -124,7 +123,7 @@ struct snd_intelhad { unsigned int period_bytes; /* PCM period size in bytes */ /* internal stuff */
- unsigned int had_config_offset;
- void __iomem *mmio_start; union aud_cfg aud_config; /* AUD_CONFIG reg value cache */ struct work_struct hdmi_audio_wq; struct mutex mutex; /* for protecting chmap and eld */
On Wed, 26 Apr 2017 09:04:46 +0200, Takashi Iwai wrote:
On Wed, 26 Apr 2017 03:58:57 +0200, Pierre-Louis Bossart wrote:
On 04/25/2017 03:27 PM, ville.syrjala@linux.intel.com wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
Now that everything is in place let's register a PCM device for each pipe of the display engine. This will make it possible to actually output audio to multiple displays at the same time. And it avoids modesets on unrelated displays from clobbering up the ELD and whatnot for the display currently doing the playback.
The alternative would be to have a PCM device per port, but per-pipe is easier since the hardware actually works that way.
Very nice. I just tested on a CHT Zotac box which has two connectors (1 HDMI and 1 DP), and I get sound concurrently on both, with hdmi being listed as device 2 and DP as device 0. I thought there were hardware restrictions but you proved me wrong. Kudos.
The only point that I find weird is that the jacks are reported as 'on' on the 3 pipes, is there a way to tie them to an actual cable being used?
The pdata check was changed to check port=-1 as the monitor off in the patch 6. Maybe the initialization is missing?
I guess the problem is that the hotplug wq is called at the initialization to retrieve the pdata for all pipes. It's called with uninitialized port=0, so all flags are on at init.
And it implies the potential problem: the pdata contains the information only for a single pipe. Maybe it should keep the status/ELD for all three pipes.
Takashi
On Wed, Apr 26, 2017 at 09:19:21AM +0200, Takashi Iwai wrote:
On Wed, 26 Apr 2017 09:04:46 +0200, Takashi Iwai wrote:
On Wed, 26 Apr 2017 03:58:57 +0200, Pierre-Louis Bossart wrote:
On 04/25/2017 03:27 PM, ville.syrjala@linux.intel.com wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
Now that everything is in place let's register a PCM device for each pipe of the display engine. This will make it possible to actually output audio to multiple displays at the same time. And it avoids modesets on unrelated displays from clobbering up the ELD and whatnot for the display currently doing the playback.
The alternative would be to have a PCM device per port, but per-pipe is easier since the hardware actually works that way.
Very nice. I just tested on a CHT Zotac box which has two connectors (1 HDMI and 1 DP), and I get sound concurrently on both, with hdmi being listed as device 2 and DP as device 0. I thought there were hardware restrictions but you proved me wrong. Kudos.
The only point that I find weird is that the jacks are reported as 'on' on the 3 pipes, is there a way to tie them to an actual cable being used?
The pdata check was changed to check port=-1 as the monitor off in the patch 6. Maybe the initialization is missing?
I guess the problem is that the hotplug wq is called at the initialization to retrieve the pdata for all pipes. It's called with uninitialized port=0, so all flags are on at init.
Indeed. That will need to be fixed.
And it implies the potential problem: the pdata contains the information only for a single pipe. Maybe it should keep the status/ELD for all three pipes.
I already did that.
That being said, the entire notify vs. wq is pretty racy. So I was thinking that maybe we could just eliminate the wq and do whatever is needed directly from the notify hook. And then we could eliminate the (ab)use of pdata to transfer the ELD and whatnot between the drivers. I guess the main complication is the driver load case. I didn't really think that one through so far.
On Wed, 26 Apr 2017 15:49:06 +0200, Ville Syrjälä wrote:
On Wed, Apr 26, 2017 at 09:19:21AM +0200, Takashi Iwai wrote:
On Wed, 26 Apr 2017 09:04:46 +0200, Takashi Iwai wrote:
On Wed, 26 Apr 2017 03:58:57 +0200, Pierre-Louis Bossart wrote:
On 04/25/2017 03:27 PM, ville.syrjala@linux.intel.com wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
Now that everything is in place let's register a PCM device for each pipe of the display engine. This will make it possible to actually output audio to multiple displays at the same time. And it avoids modesets on unrelated displays from clobbering up the ELD and whatnot for the display currently doing the playback.
The alternative would be to have a PCM device per port, but per-pipe is easier since the hardware actually works that way.
Very nice. I just tested on a CHT Zotac box which has two connectors (1 HDMI and 1 DP), and I get sound concurrently on both, with hdmi being listed as device 2 and DP as device 0. I thought there were hardware restrictions but you proved me wrong. Kudos.
The only point that I find weird is that the jacks are reported as 'on' on the 3 pipes, is there a way to tie them to an actual cable being used?
The pdata check was changed to check port=-1 as the monitor off in the patch 6. Maybe the initialization is missing?
I guess the problem is that the hotplug wq is called at the initialization to retrieve the pdata for all pipes. It's called with uninitialized port=0, so all flags are on at init.
Indeed. That will need to be fixed.
And it implies the potential problem: the pdata contains the information only for a single pipe. Maybe it should keep the status/ELD for all three pipes.
I already did that.
Oh then it' fine, I just didn't find it.
That being said, the entire notify vs. wq is pretty racy. So I was thinking that maybe we could just eliminate the wq and do whatever is needed directly from the notify hook. And then we could eliminate the (ab)use of pdata to transfer the ELD and whatnot between the drivers. I guess the main complication is the driver load case. I didn't really think that one through so far.
One way would be to implement the get_eld() to call at the probe time. HD-audio calls such one (provided via component) at probe and resume time to sync with the actual h/w state. For LPE audio, it's even easier, as we may call i915 exported function directly.
thanks,
Takashi
On Tue, 25 Apr 2017 22:27:19 +0200, ville.syrjala@linux.intel.com wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
I was wondering why my VLV no longer runtime suspended, and after some thinking I decided it had to be the LPE audio preventing it. Turns out I was right, so here's my attempt at fixing it.
And while looking at the code I couldn't help but notice that it couldn't actually handle multiple pipes playing back audio at the same time. And even having multiple displays active even if only one was playing audio was probably a recipe for failure. So I tried to fix that by registering a separate PCM device for each pipe.
Note that the patch subjects may not reflect the subsystem very well since most of these straddle the border between drm and alsa. I think I just slapped on drm/i915 to most where there was no clear winner.
A nice patchset, thanks for working on it!
One slight concern (other than the jack issue Pierre reported) is the incompatible behavior from the current version. With the pipe-based multiple streams, user would need to choose another one even if the device has a single HDMI output, which is pretty common on BYT/CHV tablets.
Maybe it's no big problem as the users are still limited at the moment. Or, we may need to handle a bit differently, e.g. assigning the PCM stream dynamically per hotplug.
In anyway, with the support of multi streams, alsa-lib config needs to be updated.
Takashi
On Wed, Apr 26, 2017 at 09:29:18AM +0200, Takashi Iwai wrote:
On Tue, 25 Apr 2017 22:27:19 +0200, ville.syrjala@linux.intel.com wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
I was wondering why my VLV no longer runtime suspended, and after some thinking I decided it had to be the LPE audio preventing it. Turns out I was right, so here's my attempt at fixing it.
And while looking at the code I couldn't help but notice that it couldn't actually handle multiple pipes playing back audio at the same time. And even having multiple displays active even if only one was playing audio was probably a recipe for failure. So I tried to fix that by registering a separate PCM device for each pipe.
Note that the patch subjects may not reflect the subsystem very well since most of these straddle the border between drm and alsa. I think I just slapped on drm/i915 to most where there was no clear winner.
A nice patchset, thanks for working on it!
One slight concern (other than the jack issue Pierre reported) is the incompatible behavior from the current version. With the pipe-based multiple streams, user would need to choose another one even if the device has a single HDMI output, which is pretty common on BYT/CHV tablets.
Maybe it's no big problem as the users are still limited at the moment. Or, we may need to handle a bit differently, e.g. assigning the PCM stream dynamically per hotplug.
Yeah, I tied the PCM device to the pipe to match the hardware. But we could certainly register the PCM device per port, and then do a pipe<->port mapping somewhere to make it all work out. One slight complication is that not all ports are necessarily present so we might have eg. just port B and port D, but no port C. Not sure if it would be a bad thing to register a PCM device even for the missing ports anyway?
I don't recall which way HDA works. Device per port I guess? Well, for DP SST/HDMI. But for DP MST I presume it's device per stream (ie. pipe) even with HDA.
In anyway, with the support of multi streams, alsa-lib config needs to be updated.
Hmm. I suppose I'll have to take a gander at what's there.
On Wed, 26 Apr 2017 15:38:53 +0200, Ville Syrjälä wrote:
On Wed, Apr 26, 2017 at 09:29:18AM +0200, Takashi Iwai wrote:
On Tue, 25 Apr 2017 22:27:19 +0200, ville.syrjala@linux.intel.com wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
I was wondering why my VLV no longer runtime suspended, and after some thinking I decided it had to be the LPE audio preventing it. Turns out I was right, so here's my attempt at fixing it.
And while looking at the code I couldn't help but notice that it couldn't actually handle multiple pipes playing back audio at the same time. And even having multiple displays active even if only one was playing audio was probably a recipe for failure. So I tried to fix that by registering a separate PCM device for each pipe.
Note that the patch subjects may not reflect the subsystem very well since most of these straddle the border between drm and alsa. I think I just slapped on drm/i915 to most where there was no clear winner.
A nice patchset, thanks for working on it!
One slight concern (other than the jack issue Pierre reported) is the incompatible behavior from the current version. With the pipe-based multiple streams, user would need to choose another one even if the device has a single HDMI output, which is pretty common on BYT/CHV tablets.
Maybe it's no big problem as the users are still limited at the moment. Or, we may need to handle a bit differently, e.g. assigning the PCM stream dynamically per hotplug.
Yeah, I tied the PCM device to the pipe to match the hardware. But we could certainly register the PCM device per port, and then do a pipe<->port mapping somewhere to make it all work out. One slight complication is that not all ports are necessarily present so we might have eg. just port B and port D, but no port C. Not sure if it would be a bad thing to register a PCM device even for the missing ports anyway?
I don't recall which way HDA works. Device per port I guess? Well, for DP SST/HDMI. But for DP MST I presume it's device per stream (ie. pipe) even with HDA.
HD-audio creates the PCM device per HD-audio widget representation, and they correspond to ports, as it seems. For MST, the situation is more complex, and we assign the PCM stream dynamically. It's for keeping the behavior (more or less) consistent with non-MST.
In anyway, with the support of multi streams, alsa-lib config needs to be updated.
Hmm. I suppose I'll have to take a gander at what's there.
The HDMI PCM definition itself is easy, just just adding more (hdmi.1, hdmi.2, etc). The difficult part would be the front and the default PCM definition, and I have no good idea about it, so far.
Takashi
On Wed, Apr 26, 2017 at 03:47:44PM +0200, Takashi Iwai wrote:
On Wed, 26 Apr 2017 15:38:53 +0200, Ville Syrjälä wrote:
On Wed, Apr 26, 2017 at 09:29:18AM +0200, Takashi Iwai wrote:
On Tue, 25 Apr 2017 22:27:19 +0200, ville.syrjala@linux.intel.com wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
I was wondering why my VLV no longer runtime suspended, and after some thinking I decided it had to be the LPE audio preventing it. Turns out I was right, so here's my attempt at fixing it.
And while looking at the code I couldn't help but notice that it couldn't actually handle multiple pipes playing back audio at the same time. And even having multiple displays active even if only one was playing audio was probably a recipe for failure. So I tried to fix that by registering a separate PCM device for each pipe.
Note that the patch subjects may not reflect the subsystem very well since most of these straddle the border between drm and alsa. I think I just slapped on drm/i915 to most where there was no clear winner.
A nice patchset, thanks for working on it!
One slight concern (other than the jack issue Pierre reported) is the incompatible behavior from the current version. With the pipe-based multiple streams, user would need to choose another one even if the device has a single HDMI output, which is pretty common on BYT/CHV tablets.
Maybe it's no big problem as the users are still limited at the moment. Or, we may need to handle a bit differently, e.g. assigning the PCM stream dynamically per hotplug.
Yeah, I tied the PCM device to the pipe to match the hardware. But we could certainly register the PCM device per port, and then do a pipe<->port mapping somewhere to make it all work out. One slight complication is that not all ports are necessarily present so we might have eg. just port B and port D, but no port C. Not sure if it would be a bad thing to register a PCM device even for the missing ports anyway?
I don't recall which way HDA works. Device per port I guess? Well, for DP SST/HDMI. But for DP MST I presume it's device per stream (ie. pipe) even with HDA.
HD-audio creates the PCM device per HD-audio widget representation, and they correspond to ports, as it seems.
With the slight exception of old stuff like ctg/elk where there is just one audio device IIRC, and that one gets fed into whichever port enables audio, and if both enable it it goes to /dev/null.
For MST, the situation is more complex, and we assign the PCM stream dynamically. It's for keeping the behavior (more or less) consistent with non-MST.
In anyway, with the support of multi streams, alsa-lib config needs to be updated.
Hmm. I suppose I'll have to take a gander at what's there.
The HDMI PCM definition itself is easy, just just adding more (hdmi.1, hdmi.2, etc). The difficult part would be the front and the default PCM definition, and I have no good idea about it, so far.
Takashi
On Wed, Apr 26, 2017 at 03:47:44PM +0200, Takashi Iwai wrote:
On Wed, 26 Apr 2017 15:38:53 +0200, Ville Syrjälä wrote:
On Wed, Apr 26, 2017 at 09:29:18AM +0200, Takashi Iwai wrote:
On Tue, 25 Apr 2017 22:27:19 +0200, ville.syrjala@linux.intel.com wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
I was wondering why my VLV no longer runtime suspended, and after some thinking I decided it had to be the LPE audio preventing it. Turns out I was right, so here's my attempt at fixing it.
And while looking at the code I couldn't help but notice that it couldn't actually handle multiple pipes playing back audio at the same time. And even having multiple displays active even if only one was playing audio was probably a recipe for failure. So I tried to fix that by registering a separate PCM device for each pipe.
Note that the patch subjects may not reflect the subsystem very well since most of these straddle the border between drm and alsa. I think I just slapped on drm/i915 to most where there was no clear winner.
A nice patchset, thanks for working on it!
One slight concern (other than the jack issue Pierre reported) is the incompatible behavior from the current version. With the pipe-based multiple streams, user would need to choose another one even if the device has a single HDMI output, which is pretty common on BYT/CHV tablets.
Maybe it's no big problem as the users are still limited at the moment. Or, we may need to handle a bit differently, e.g. assigning the PCM stream dynamically per hotplug.
Yeah, I tied the PCM device to the pipe to match the hardware. But we could certainly register the PCM device per port, and then do a pipe<->port mapping somewhere to make it all work out. One slight complication is that not all ports are necessarily present so we might have eg. just port B and port D, but no port C. Not sure if it would be a bad thing to register a PCM device even for the missing ports anyway?
I don't recall which way HDA works. Device per port I guess? Well, for DP SST/HDMI. But for DP MST I presume it's device per stream (ie. pipe) even with HDA.
HD-audio creates the PCM device per HD-audio widget representation, and they correspond to ports, as it seems. For MST, the situation is more complex, and we assign the PCM stream dynamically. It's for keeping the behavior (more or less) consistent with non-MST.
OK, I hacked together something that seems to work. I'll still need to squash it with the earlier patch, but in case you want to see what the relative changes look like I pushed it to git://github.com/vsyrjala/linux.git lpe_audio_multipipe
participants (4)
-
Pierre-Louis Bossart -
Takashi Iwai -
Ville Syrjälä -
ville.syrjala＠linux.intel.com