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

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

v2: Fix typo in patch subject

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

diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c index 292fedf30b00..79b9dca985ff 100644 --- a/drivers/gpu/drm/i915/intel_lpe_audio.c +++ b/drivers/gpu/drm/i915/intel_lpe_audio.c @@ -361,8 +361,6 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv,

if (pdata->notify_audio_lpe) pdata->notify_audio_lpe(dev_priv->lpe_audio.platdev); - else - pdata->notify_pending = true;

spin_unlock_irqrestore(&pdata->lpe_audio_slock, irq_flags); diff --git a/include/drm/intel_lpe_audio.h b/include/drm/intel_lpe_audio.h index e9892b4c3af1..c201d39cdfea 100644 --- a/include/drm/intel_lpe_audio.h +++ b/include/drm/intel_lpe_audio.h @@ -38,7 +38,6 @@ struct intel_hdmi_lpe_audio_eld { };

struct intel_hdmi_lpe_audio_pdata { - bool notify_pending; int tmds_clock_speed; bool hdmi_connected; bool dp_output; diff --git a/sound/x86/intel_hdmi_audio.c b/sound/x86/intel_hdmi_audio.c index 5b89662493c9..cbba4a78afb5 100644 --- a/sound/x86/intel_hdmi_audio.c +++ b/sound/x86/intel_hdmi_audio.c @@ -1811,7 +1811,6 @@ static int hdmi_lpe_audio_probe(struct platform_device *pdev)

spin_lock_irq(&pdata->lpe_audio_slock); pdata->notify_audio_lpe = notify_audio_lpe; - pdata->notify_pending = false; spin_unlock_irq(&pdata->lpe_audio_slock);

pm_runtime_use_autosuspend(&pdev->dev);

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

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

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

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

diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c index 5a1a37e963f1..7fd95733eff5 100644 --- a/drivers/gpu/drm/i915/intel_lpe_audio.c +++ b/drivers/gpu/drm/i915/intel_lpe_audio.c @@ -111,6 +111,7 @@ lpe_audio_platdev_create(struct drm_i915_private *dev_priv) pinfo.size_data = sizeof(*pdata); pinfo.dma_mask = DMA_BIT_MASK(32);

+ pdata->pipe = -1; spin_lock_init(&pdata->lpe_audio_slock);

platdev = platform_device_register_full(&pinfo); @@ -332,12 +333,12 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv,

audio_enable = I915_READ(VLV_AUD_PORT_EN_DBG(port));

+ pdata->eld.port_id = port; + if (eld != NULL) { memcpy(pdata->eld.eld_data, eld, HDMI_MAX_ELD_BYTES); - pdata->eld.port_id = port; - pdata->eld.pipe_id = pipe; - pdata->hdmi_connected = true; + pdata->pipe = pipe; pdata->ls_clock = ls_clock; pdata->dp_output = dp_output;

@@ -348,7 +349,7 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv, } else { memset(pdata->eld.eld_data, 0, HDMI_MAX_ELD_BYTES); - pdata->hdmi_connected = false; + pdata->pipe = -1; pdata->ls_clock = 0; pdata->dp_output = false;

diff --git a/include/drm/intel_lpe_audio.h b/include/drm/intel_lpe_audio.h index 8bf804ce8905..9a5bdf5ad180 100644 --- a/include/drm/intel_lpe_audio.h +++ b/include/drm/intel_lpe_audio.h @@ -33,13 +33,12 @@ struct platform_device;

struct intel_hdmi_lpe_audio_eld { int port_id; - int pipe_id; unsigned char eld_data[HDMI_MAX_ELD_BYTES]; };

struct intel_hdmi_lpe_audio_pdata { + int pipe; int ls_clock; - bool hdmi_connected; bool dp_output; struct intel_hdmi_lpe_audio_eld eld; void (*notify_audio_lpe)(struct platform_device *pdev); diff --git a/sound/x86/intel_hdmi_audio.c b/sound/x86/intel_hdmi_audio.c index 4eaf5de54f61..1a095189db83 100644 --- a/sound/x86/intel_hdmi_audio.c +++ b/sound/x86/intel_hdmi_audio.c @@ -1559,7 +1559,7 @@ static void had_audio_wq(struct work_struct *work)

pm_runtime_get_sync(ctx->dev); mutex_lock(&ctx->mutex); - if (!pdata->hdmi_connected) { + if (pdata->pipe < 0) { dev_dbg(ctx->dev, "%s: Event: HAD_NOTIFY_HOT_UNPLUG\n", __func__); memset(ctx->eld, 0, sizeof(ctx->eld)); /* clear the old ELD */ @@ -1568,9 +1568,9 @@ static void had_audio_wq(struct work_struct *work) struct intel_hdmi_lpe_audio_eld *eld = &pdata->eld;

dev_dbg(ctx->dev, "%s: HAD_NOTIFY_ELD : port = %d, tmds = %d\n", - __func__, eld->port_id, pdata->ls_clock); + __func__, eld->port_id, pdata->ls_clock);

- switch (eld->pipe_id) { + switch (pdata->pipe) { case 0: ctx->had_config_offset = AUDIO_HDMI_CONFIG_A; break; @@ -1582,7 +1582,7 @@ static void had_audio_wq(struct work_struct *work) break; default: dev_dbg(ctx->dev, "Invalid pipe %d\n", - eld->pipe_id); + pdata->pipe); break; }

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

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

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

v2: Organize per port instead of per pipe

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

diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c index 4c770d037f23..bdbc235141b5 100644 --- a/drivers/gpu/drm/i915/intel_lpe_audio.c +++ b/drivers/gpu/drm/i915/intel_lpe_audio.c @@ -111,7 +111,7 @@ lpe_audio_platdev_create(struct drm_i915_private *dev_priv) pinfo.size_data = sizeof(*pdata); pinfo.dma_mask = DMA_BIT_MASK(32);

- pdata->pipe = -1; + pdata->port.pipe = -1; spin_lock_init(&pdata->lpe_audio_slock);

platdev = platform_device_register_full(&pinfo); @@ -320,38 +320,36 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv, const void *eld, int ls_clock, bool dp_output) { unsigned long irq_flags; - struct intel_hdmi_lpe_audio_pdata *pdata = NULL; + struct intel_hdmi_lpe_audio_pdata *pdata; + struct intel_hdmi_lpe_audio_port_pdata *ppdata; u32 audio_enable;

if (!HAS_LPE_AUDIO(dev_priv)) return;

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

spin_lock_irqsave(&pdata->lpe_audio_slock, irq_flags);

audio_enable = I915_READ(VLV_AUD_PORT_EN_DBG(port));

- pdata->eld.port_id = port; + ppdata->port = port;

if (eld != NULL) { - memcpy(pdata->eld.eld_data, eld, - HDMI_MAX_ELD_BYTES); - pdata->pipe = pipe; - pdata->ls_clock = ls_clock; - pdata->dp_output = dp_output; + memcpy(ppdata->eld, eld, HDMI_MAX_ELD_BYTES); + ppdata->pipe = pipe; + ppdata->ls_clock = ls_clock; + ppdata->dp_output = dp_output;

/* Unmute the amp for both DP and HDMI */ I915_WRITE(VLV_AUD_PORT_EN_DBG(port), audio_enable & ~VLV_AMP_MUTE); - } else { - memset(pdata->eld.eld_data, 0, - HDMI_MAX_ELD_BYTES); - pdata->pipe = -1; - pdata->ls_clock = 0; - pdata->dp_output = false; + memset(ppdata->eld, 0, HDMI_MAX_ELD_BYTES); + ppdata->pipe = -1; + ppdata->ls_clock = 0; + ppdata->dp_output = false;

/* Mute the amp for both DP and HDMI */ I915_WRITE(VLV_AUD_PORT_EN_DBG(port), diff --git a/include/drm/intel_lpe_audio.h b/include/drm/intel_lpe_audio.h index 9a5bdf5ad180..211f1cd61153 100644 --- a/include/drm/intel_lpe_audio.h +++ b/include/drm/intel_lpe_audio.h @@ -31,16 +31,17 @@ struct platform_device;

#define HDMI_MAX_ELD_BYTES 128

-struct intel_hdmi_lpe_audio_eld { - int port_id; - unsigned char eld_data[HDMI_MAX_ELD_BYTES]; -}; - -struct intel_hdmi_lpe_audio_pdata { +struct intel_hdmi_lpe_audio_port_pdata { + u8 eld[HDMI_MAX_ELD_BYTES]; + int port; int pipe; int ls_clock; bool dp_output; - struct intel_hdmi_lpe_audio_eld eld; +}; + +struct intel_hdmi_lpe_audio_pdata { + struct intel_hdmi_lpe_audio_port_pdata port; + void (*notify_audio_lpe)(struct platform_device *pdev); spinlock_t lpe_audio_slock; }; diff --git a/sound/x86/intel_hdmi_audio.c b/sound/x86/intel_hdmi_audio.c index 1a095189db83..c2b78621852e 100644 --- a/sound/x86/intel_hdmi_audio.c +++ b/sound/x86/intel_hdmi_audio.c @@ -1556,21 +1556,20 @@ static void had_audio_wq(struct work_struct *work) struct snd_intelhad *ctx = container_of(work, struct snd_intelhad, hdmi_audio_wq); struct intel_hdmi_lpe_audio_pdata *pdata = ctx->dev->platform_data; + struct intel_hdmi_lpe_audio_port_pdata *ppdata = &pdata->port;

pm_runtime_get_sync(ctx->dev); mutex_lock(&ctx->mutex); - if (pdata->pipe < 0) { + if (ppdata->pipe < 0) { dev_dbg(ctx->dev, "%s: Event: HAD_NOTIFY_HOT_UNPLUG\n", __func__); memset(ctx->eld, 0, sizeof(ctx->eld)); /* clear the old ELD */ had_process_hot_unplug(ctx); } else { - struct intel_hdmi_lpe_audio_eld *eld = &pdata->eld; - dev_dbg(ctx->dev, "%s: HAD_NOTIFY_ELD : port = %d, tmds = %d\n", - __func__, eld->port_id, pdata->ls_clock); + __func__, ppdata->port, ppdata->ls_clock);

- switch (pdata->pipe) { + switch (ppdata->pipe) { case 0: ctx->had_config_offset = AUDIO_HDMI_CONFIG_A; break; @@ -1582,18 +1581,18 @@ static void had_audio_wq(struct work_struct *work) break; default: dev_dbg(ctx->dev, "Invalid pipe %d\n", - pdata->pipe); + ppdata->pipe); break; }

- memcpy(ctx->eld, eld->eld_data, sizeof(ctx->eld)); + memcpy(ctx->eld, ppdata->eld, sizeof(ctx->eld));

- ctx->dp_output = pdata->dp_output; + ctx->dp_output = ppdata->dp_output; if (ctx->dp_output) { ctx->tmds_clock_speed = 0; - ctx->link_rate = pdata->ls_clock; + ctx->link_rate = ppdata->ls_clock; } else { - ctx->tmds_clock_speed = pdata->ls_clock; + ctx->tmds_clock_speed = ppdata->ls_clock; ctx->link_rate = 0; }

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

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

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

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

diff --git a/sound/x86/intel_hdmi_audio.c b/sound/x86/intel_hdmi_audio.c index 69e10845633a..12fae26e70bb 100644 --- a/sound/x86/intel_hdmi_audio.c +++ b/sound/x86/intel_hdmi_audio.c @@ -42,6 +42,9 @@ #include <drm/intel_lpe_audio.h> #include "intel_hdmi_audio.h"

+#define for_each_port(card_ctx, port) \ + for ((port) = 0; (port) < (card_ctx)->num_ports; (port)++) + /*standard module options for ALSA. This module supports only one card*/ static int hdmi_card_index = SNDRV_DEFAULT_IDX1; static char *hdmi_card_id = SNDRV_DEFAULT_STR1; @@ -192,12 +195,12 @@ static void had_substream_put(struct snd_intelhad *intelhaddata) /* Register access functions */ static u32 had_read_register_raw(struct snd_intelhad *ctx, u32 reg) { - return ioread32(ctx->mmio_start + ctx->had_config_offset + reg); + return ioread32(ctx->card_ctx->mmio_start + ctx->had_config_offset + reg); }

static void had_write_register_raw(struct snd_intelhad *ctx, u32 reg, u32 val) { - iowrite32(val, ctx->mmio_start + ctx->had_config_offset + reg); + iowrite32(val, ctx->card_ctx->mmio_start + ctx->had_config_offset + reg); }

static void had_read_register(struct snd_intelhad *ctx, u32 reg, u32 *val) @@ -1519,22 +1522,27 @@ static const struct snd_kcontrol_new had_controls[] = { */ static irqreturn_t display_pipe_interrupt_handler(int irq, void *dev_id) { - struct snd_intelhad *ctx = dev_id; - u32 audio_stat; + struct snd_intelhad_card *card_ctx = dev_id; + int port;

- /* use raw register access to ack IRQs even while disconnected */ - audio_stat = had_read_register_raw(ctx, AUD_HDMI_STATUS); + for_each_port(card_ctx, port) { + struct snd_intelhad *ctx = &card_ctx->pcm_ctx[port]; + u32 audio_stat;

- if (audio_stat & HDMI_AUDIO_UNDERRUN) { - had_write_register_raw(ctx, AUD_HDMI_STATUS, - HDMI_AUDIO_UNDERRUN); - had_process_buffer_underrun(ctx); - } + /* use raw register access to ack IRQs even while disconnected */ + audio_stat = had_read_register_raw(ctx, AUD_HDMI_STATUS); + + if (audio_stat & HDMI_AUDIO_UNDERRUN) { + had_write_register_raw(ctx, AUD_HDMI_STATUS, + HDMI_AUDIO_UNDERRUN); + had_process_buffer_underrun(ctx); + }

- if (audio_stat & HDMI_AUDIO_BUFFER_DONE) { - had_write_register_raw(ctx, AUD_HDMI_STATUS, - HDMI_AUDIO_BUFFER_DONE); - had_process_buffer_done(ctx); + if (audio_stat & HDMI_AUDIO_BUFFER_DONE) { + had_write_register_raw(ctx, AUD_HDMI_STATUS, + HDMI_AUDIO_BUFFER_DONE); + had_process_buffer_done(ctx); + } }

return IRQ_HANDLED; @@ -1545,9 +1553,14 @@ static irqreturn_t display_pipe_interrupt_handler(int irq, void *dev_id) */ static void notify_audio_lpe(struct platform_device *pdev) { - struct snd_intelhad *ctx = platform_get_drvdata(pdev); + struct snd_intelhad_card *card_ctx = platform_get_drvdata(pdev); + int port; + + for_each_port(card_ctx, port) { + struct snd_intelhad *ctx = &card_ctx->pcm_ctx[port];

- schedule_work(&ctx->hdmi_audio_wq); + schedule_work(&ctx->hdmi_audio_wq); + } }

/* the work to handle monitor hot plug/unplug */ @@ -1618,7 +1631,8 @@ static int had_create_jack(struct snd_intelhad *ctx, snprintf(hdmi_str, sizeof(hdmi_str), "HDMI/DP,pcm=%d", pcm->device);

- err = snd_jack_new(ctx->card, hdmi_str, SND_JACK_AVOUT, &ctx->jack, + err = snd_jack_new(ctx->card_ctx->card, hdmi_str, + SND_JACK_AVOUT, &ctx->jack, true, false); if (err < 0) return err; @@ -1632,13 +1646,18 @@ static int had_create_jack(struct snd_intelhad *ctx,

static int hdmi_lpe_audio_runtime_suspend(struct device *dev) { - struct snd_intelhad *ctx = dev_get_drvdata(dev); - struct snd_pcm_substream *substream; + struct snd_intelhad_card *card_ctx = dev_get_drvdata(dev); + int port;

- substream = had_substream_get(ctx); - if (substream) { - snd_pcm_suspend(substream); - had_substream_put(ctx); + for_each_port(card_ctx, port) { + struct snd_intelhad *ctx = &card_ctx->pcm_ctx[port]; + struct snd_pcm_substream *substream; + + substream = had_substream_get(ctx); + if (substream) { + snd_pcm_suspend(substream); + had_substream_put(ctx); + } }

return 0; @@ -1646,12 +1665,12 @@ static int hdmi_lpe_audio_runtime_suspend(struct device *dev)

static int __maybe_unused hdmi_lpe_audio_suspend(struct device *dev) { - struct snd_intelhad *ctx = dev_get_drvdata(dev); + struct snd_intelhad_card *card_ctx = dev_get_drvdata(dev); int err;

err = hdmi_lpe_audio_runtime_suspend(dev); if (!err) - snd_power_change_state(ctx->card, SNDRV_CTL_POWER_D3hot); + snd_power_change_state(card_ctx->card, SNDRV_CTL_POWER_D3hot); return err; }

@@ -1663,29 +1682,34 @@ static int hdmi_lpe_audio_runtime_resume(struct device *dev)

static int __maybe_unused hdmi_lpe_audio_resume(struct device *dev) { - struct snd_intelhad *ctx = dev_get_drvdata(dev); + struct snd_intelhad_card *card_ctx = dev_get_drvdata(dev);

hdmi_lpe_audio_runtime_resume(dev); - snd_power_change_state(ctx->card, SNDRV_CTL_POWER_D0); + snd_power_change_state(card_ctx->card, SNDRV_CTL_POWER_D0); return 0; }

/* release resources */ static void hdmi_lpe_audio_free(struct snd_card *card) { - struct snd_intelhad *ctx = card->private_data; - struct intel_hdmi_lpe_audio_pdata *pdata = ctx->dev->platform_data; + struct snd_intelhad_card *card_ctx = card->private_data; + struct intel_hdmi_lpe_audio_pdata *pdata = card_ctx->dev->platform_data; + int port;

spin_lock_irq(&pdata->lpe_audio_slock); pdata->notify_audio_lpe = NULL; spin_unlock_irq(&pdata->lpe_audio_slock);

- cancel_work_sync(&ctx->hdmi_audio_wq); + for_each_port(card_ctx, port) { + struct snd_intelhad *ctx = &card_ctx->pcm_ctx[port]; + + cancel_work_sync(&ctx->hdmi_audio_wq); + }

- if (ctx->mmio_start) - iounmap(ctx->mmio_start); - if (ctx->irq >= 0) - free_irq(ctx->irq, ctx); + if (card_ctx->mmio_start) + iounmap(card_ctx->mmio_start); + if (card_ctx->irq >= 0) + free_irq(card_ctx->irq, card_ctx); }

/* @@ -1697,12 +1721,12 @@ static void hdmi_lpe_audio_free(struct snd_card *card) static int hdmi_lpe_audio_probe(struct platform_device *pdev) { struct snd_card *card; - struct snd_intelhad *ctx; + struct snd_intelhad_card *card_ctx; struct snd_pcm *pcm; struct intel_hdmi_lpe_audio_pdata *pdata; int irq; struct resource *res_mmio; - int i, ret; + int port, ret;

pdata = pdev->dev.platform_data; if (!pdata) { @@ -1725,39 +1749,30 @@ static int hdmi_lpe_audio_probe(struct platform_device *pdev)

/* create a card instance with ALSA framework */ ret = snd_card_new(&pdev->dev, hdmi_card_index, hdmi_card_id, - THIS_MODULE, sizeof(*ctx), &card); + THIS_MODULE, sizeof(*card_ctx), &card); if (ret) return ret;

- ctx = card->private_data; - spin_lock_init(&ctx->had_spinlock); - mutex_init(&ctx->mutex); - ctx->connected = false; - ctx->dev = &pdev->dev; - ctx->card = card; - ctx->aes_bits = SNDRV_PCM_DEFAULT_CON_SPDIF; + card_ctx = card->private_data; + card_ctx->dev = &pdev->dev; + card_ctx->card = card; strcpy(card->driver, INTEL_HAD); strcpy(card->shortname, "Intel HDMI/DP LPE Audio"); strcpy(card->longname, "Intel HDMI/DP LPE Audio");

- ctx->irq = -1; - ctx->tmds_clock_speed = DIS_SAMPLE_RATE_148_5; - INIT_WORK(&ctx->hdmi_audio_wq, had_audio_wq); + card_ctx->irq = -1;

card->private_free = hdmi_lpe_audio_free;

- /* assume pipe A as default */ - ctx->had_config_offset = AUDIO_HDMI_CONFIG_A; - - platform_set_drvdata(pdev, ctx); + platform_set_drvdata(pdev, card_ctx);

dev_dbg(&pdev->dev, "%s: mmio_start = 0x%x, mmio_end = 0x%x\n", __func__, (unsigned int)res_mmio->start, (unsigned int)res_mmio->end);

- ctx->mmio_start = ioremap_nocache(res_mmio->start, - (size_t)(resource_size(res_mmio))); - if (!ctx->mmio_start) { + card_ctx->mmio_start = ioremap_nocache(res_mmio->start, + (size_t)(resource_size(res_mmio))); + if (!card_ctx->mmio_start) { dev_err(&pdev->dev, "Could not get ioremap\n"); ret = -EACCES; goto err; @@ -1765,65 +1780,79 @@ static int hdmi_lpe_audio_probe(struct platform_device *pdev)

/* setup interrupt handler */ ret = request_irq(irq, display_pipe_interrupt_handler, 0, - pdev->name, ctx); + pdev->name, card_ctx); if (ret < 0) { dev_err(&pdev->dev, "request_irq failed\n"); goto err; }

- ctx->irq = irq; - - ret = snd_pcm_new(card, INTEL_HAD, PCM_INDEX, MAX_PB_STREAMS, - MAX_CAP_STREAMS, &pcm); - if (ret) - goto err; - - /* setup private data which can be retrieved when required */ - pcm->private_data = ctx; - pcm->info_flags = 0; - strncpy(pcm->name, card->shortname, strlen(card->shortname)); - /* setup the ops for playabck */ - snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_PLAYBACK, &had_pcm_ops); + card_ctx->irq = irq;

/* only 32bit addressable */ dma_set_mask(&pdev->dev, DMA_BIT_MASK(32)); dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(32));

- /* allocate dma pages; - * try to allocate 600k buffer as default which is large enough - */ - snd_pcm_lib_preallocate_pages_for_all(pcm, - SNDRV_DMA_TYPE_DEV, NULL, - HAD_DEFAULT_BUFFER, HAD_MAX_BUFFER); + init_channel_allocations();

- /* create controls */ - for (i = 0; i < ARRAY_SIZE(had_controls); i++) { - struct snd_kcontrol *kctl; + card_ctx->num_ports = 1;

- kctl = snd_ctl_new1(&had_controls[i], ctx); - if (!kctl) { - ret = -ENOMEM; + for_each_port(card_ctx, port) { + struct snd_intelhad *ctx = &card_ctx->pcm_ctx[port]; + int i; + + ctx->card_ctx = card_ctx; + ctx->dev = card_ctx->dev; + + INIT_WORK(&ctx->hdmi_audio_wq, had_audio_wq); + + ctx->had_config_offset = AUDIO_HDMI_CONFIG_A; + + ret = snd_pcm_new(card, INTEL_HAD, PCM_INDEX, MAX_PB_STREAMS, + MAX_CAP_STREAMS, &pcm); + if (ret) goto err; + + /* setup private data which can be retrieved when required */ + pcm->private_data = ctx; + pcm->info_flags = 0; + strncpy(pcm->name, card->shortname, strlen(card->shortname)); + /* setup the ops for playabck */ + snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_PLAYBACK, &had_pcm_ops); + + /* allocate dma pages; + * try to allocate 600k buffer as default which is large enough + */ + snd_pcm_lib_preallocate_pages_for_all(pcm, + SNDRV_DMA_TYPE_DEV, NULL, + HAD_DEFAULT_BUFFER, HAD_MAX_BUFFER); + + /* create controls */ + for (i = 0; i < ARRAY_SIZE(had_controls); i++) { + struct snd_kcontrol *kctl; + + kctl = snd_ctl_new1(&had_controls[i], ctx); + if (!kctl) { + ret = -ENOMEM; + goto err; + } + + kctl->id.device = pcm->device; + + ret = snd_ctl_add(card, kctl); + if (ret < 0) + goto err; }

- kctl->id.device = pcm->device; + /* Register channel map controls */ + ret = had_register_chmap_ctls(ctx, pcm); + if (ret < 0) + goto err;

- ret = snd_ctl_add(card, kctl); + ret = had_create_jack(ctx, pcm); if (ret < 0) goto err; }

- init_channel_allocations(); - - /* Register channel map controls */ - ret = had_register_chmap_ctls(ctx, pcm); - if (ret < 0) - goto err; - - ret = had_create_jack(ctx, pcm); - if (ret < 0) - goto err; - ret = snd_card_register(card); if (ret) goto err; @@ -1837,7 +1866,11 @@ static int hdmi_lpe_audio_probe(struct platform_device *pdev) pm_runtime_set_active(&pdev->dev);

dev_dbg(&pdev->dev, "%s: handle pending notification\n", __func__); - schedule_work(&ctx->hdmi_audio_wq); + for_each_port(card_ctx, port) { + struct snd_intelhad *ctx = &card_ctx->pcm_ctx[port]; + + schedule_work(&ctx->hdmi_audio_wq); + }

return 0;

@@ -1853,9 +1886,9 @@ static int hdmi_lpe_audio_probe(struct platform_device *pdev) */ static int hdmi_lpe_audio_remove(struct platform_device *pdev) { - struct snd_intelhad *ctx = platform_get_drvdata(pdev); + struct snd_intelhad_card *card_ctx = platform_get_drvdata(pdev);

- snd_card_free(ctx->card); + snd_card_free(card_ctx->card); return 0; }

diff --git a/sound/x86/intel_hdmi_audio.h b/sound/x86/intel_hdmi_audio.h index 2d3e389f76b3..2725964ebc46 100644 --- a/sound/x86/intel_hdmi_audio.h +++ b/sound/x86/intel_hdmi_audio.h @@ -101,7 +101,7 @@ struct pcm_stream_info { * @chmap: holds channel map info */ struct snd_intelhad { - struct snd_card *card; + struct snd_intelhad_card *card_ctx; bool connected; struct pcm_stream_info stream_info; unsigned char eld[HDMI_MAX_ELD_BYTES]; @@ -123,8 +123,6 @@ struct snd_intelhad { unsigned int period_bytes; /* PCM period size in bytes */

/* internal stuff */ - int irq; - void __iomem *mmio_start; unsigned int had_config_offset; union aud_cfg aud_config; /* AUD_CONFIG reg value cache */ struct work_struct hdmi_audio_wq; @@ -133,4 +131,15 @@ struct snd_intelhad { struct snd_jack *jack; };

+struct snd_intelhad_card { + struct snd_card *card; + struct device *dev; + + /* internal stuff */ + int irq; + void __iomem *mmio_start; + int num_ports; + struct snd_intelhad pcm_ctx[3]; +}; + #endif /* _INTEL_HDMI_AUDIO_ */

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

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

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

diff --git a/src/conf/cards/HdmiLpeAudio.conf b/src/conf/cards/HdmiLpeAudio.conf index 61bdfeae2917..ad174b8ac450 100644 --- a/src/conf/cards/HdmiLpeAudio.conf +++ b/src/conf/cards/HdmiLpeAudio.conf @@ -51,11 +51,14 @@ HdmiLpeAudio.pcm.default {

confdir:pcm/hdmi.conf

-HdmiLpeAudio.pcm.hdmi.0 { - @args [ CARD AES0 AES1 AES2 AES3 ] +HdmiLpeAudio.pcm.hdmi.common { + @args [ CARD DEVICE AES0 AES1 AES2 AES3 ] @args.CARD { type string } + @args.DEVICE { + type integer + } @args.AES0 { type integer } @@ -72,6 +75,7 @@ HdmiLpeAudio.pcm.hdmi.0 { slave.pcm { type hw card $CARD + device $DEVICE } hooks.0 { type ctl_elems @@ -86,3 +90,69 @@ HdmiLpeAudio.pcm.hdmi.0 { ] } } + +HdmiLpeAudio.pcm.hdmi.0 { + @args [ CARD AES0 AES1 AES2 AES3 ] + @args.CARD { type string } + @args.AES0 { type integer } + @args.AES1 { type integer } + @args.AES2 { type integer } + @args.AES3 { type integer } + @func refer + name { + @func concat + strings [ + "cards.HdmiLpeAudio.pcm.hdmi.common:" + "CARD=" $CARD "," + "DEVICE=0," + "AES0=" $AES0 "," + "AES1=" $AES1 "," + "AES2=" $AES2 "," + "AES3=" $AES3 + ] + } +} + +HdmiLpeAudio.pcm.hdmi.1 { + @args [ CARD AES0 AES1 AES2 AES3 ] + @args.CARD { type string } + @args.AES0 { type integer } + @args.AES1 { type integer } + @args.AES2 { type integer } + @args.AES3 { type integer } + @func refer + name { + @func concat + strings [ + "cards.HdmiLpeAudio.pcm.hdmi.common:" + "CARD=" $CARD "," + "DEVICE=1," + "AES0=" $AES0 "," + "AES1=" $AES1 "," + "AES2=" $AES2 "," + "AES3=" $AES3 + ] + } +} + +HdmiLpeAudio.pcm.hdmi.2 { + @args [ CARD AES0 AES1 AES2 AES3 ] + @args.CARD { type string } + @args.AES0 { type integer } + @args.AES1 { type integer } + @args.AES2 { type integer } + @args.AES3 { type integer } + @func refer + name { + @func concat + strings [ + "cards.HdmiLpeAudio.pcm.hdmi.common:" + "CARD=" $CARD "," + "DEVICE=2," + "AES0=" $AES0 "," + "AES1=" $AES1 "," + "AES2=" $AES2 "," + "AES3=" $AES3 + ] + } +}

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

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

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

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

v2: Add a PCM per port instead of per pipe

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

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

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

On 04/27/2017 11:02 AM, ville.syrjala@linux.intel.com wrote:

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

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

v2: Add a PCM per port instead of per pipe

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

---

drivers/gpu/drm/i915/intel_lpe_audio.c | 19 ++--- include/drm/intel_lpe_audio.h | 6 +- sound/x86/intel_hdmi_audio.c | 126 +++++++++++++++++++-------------- sound/x86/intel_hdmi_audio.h | 7 +- 4 files changed, 92 insertions(+), 66 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c index bdbc235141b5..fa728ed21d1f 100644 --- a/drivers/gpu/drm/i915/intel_lpe_audio.c +++ b/drivers/gpu/drm/i915/intel_lpe_audio.c @@ -111,7 +111,11 @@ lpe_audio_platdev_create(struct drm_i915_private *dev_priv) pinfo.size_data = sizeof(*pdata); pinfo.dma_mask = DMA_BIT_MASK(32);

• pdata->port.pipe = -1;

• pdata->num_pipes = INTEL_INFO(dev_priv)->num_pipes;

• pdata->num_ports = IS_CHERRYVIEW(dev_priv) ? 3 : 2; /* B,C,D or B,C */

• pdata->port[0].pipe = -1;

• pdata->port[1].pipe = -1;

• pdata->port[2].pipe = -1; spin_lock_init(&pdata->lpe_audio_slock);

  platdev = platform_device_register_full(&pinfo);

@@ -319,7 +323,7 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv, enum pipe pipe, enum port port, const void *eld, int ls_clock, bool dp_output) {

• unsigned long irq_flags;

• unsigned long irqflags; struct intel_hdmi_lpe_audio_pdata *pdata; struct intel_hdmi_lpe_audio_port_pdata *ppdata; u32 audio_enable;

@@ -328,14 +332,12 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv, return;

pdata = dev_get_platdata(&dev_priv->lpe_audio.platdev->dev);

• ppdata = &pdata->port;

• ppdata = &pdata->port[port];

• spin_lock_irqsave(&pdata->lpe_audio_slock, irq_flags);

• spin_lock_irqsave(&pdata->lpe_audio_slock, irqflags);

  audio_enable = I915_READ(VLV_AUD_PORT_EN_DBG(port));

• ppdata->port = port;
• if (eld != NULL) { memcpy(ppdata->eld, eld, HDMI_MAX_ELD_BYTES); ppdata->pipe = pipe;

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

if (pdata->notify_audio_lpe)

• pdata->notify_audio_lpe(dev_priv->lpe_audio.platdev);
  

• pdata->notify_audio_lpe(dev_priv->lpe_audio.platdev, port);
  
  

• spin_unlock_irqrestore(&pdata->lpe_audio_slock,

• 	irq_flags);
  

• spin_unlock_irqrestore(&pdata->lpe_audio_slock, irqflags); }

diff --git a/include/drm/intel_lpe_audio.h b/include/drm/intel_lpe_audio.h index 211f1cd61153..a911530c012e 100644 --- a/include/drm/intel_lpe_audio.h +++ b/include/drm/intel_lpe_audio.h @@ -40,9 +40,11 @@ struct intel_hdmi_lpe_audio_port_pdata { };

struct intel_hdmi_lpe_audio_pdata {

• struct intel_hdmi_lpe_audio_port_pdata port;

• struct intel_hdmi_lpe_audio_port_pdata port[3]; /* ports B,C,D */
• int num_ports;
• int num_pipes;

• void (*notify_audio_lpe)(struct platform_device *pdev);

• void (*notify_audio_lpe)(struct platform_device *pdev, int pipe); spinlock_t lpe_audio_slock; };

diff --git a/sound/x86/intel_hdmi_audio.c b/sound/x86/intel_hdmi_audio.c index 12fae26e70bb..909391d5270c 100644 --- a/sound/x86/intel_hdmi_audio.c +++ b/sound/x86/intel_hdmi_audio.c @@ -42,6 +42,8 @@ #include <drm/intel_lpe_audio.h> #include "intel_hdmi_audio.h"

+#define for_each_pipe(card_ctx, pipe) \

• for ((pipe) = 0; (pipe) < (card_ctx)->num_pipes; (pipe)++) #define for_each_port(card_ctx, port) \ for ((port) = 0; (port) < (card_ctx)->num_ports; (port)++)

@@ -192,15 +194,30 @@ static void had_substream_put(struct snd_intelhad *intelhaddata) spin_unlock_irqrestore(&intelhaddata->had_spinlock, flags); }

+static u32 had_config_offset(int pipe) +{

• switch (pipe) {
• default:
• case 0:

• return AUDIO_HDMI_CONFIG_A;
  

• case 1:

• return AUDIO_HDMI_CONFIG_B;
  

• case 2:

• return AUDIO_HDMI_CONFIG_C;
  

• }

+}

• /* Register access functions */

-static u32 had_read_register_raw(struct snd_intelhad *ctx, u32 reg) +static u32 had_read_register_raw(struct snd_intelhad_card *card_ctx,

• 		 int pipe, u32 reg)
  

  {

• return ioread32(ctx->card_ctx->mmio_start + ctx->had_config_offset + reg);

• return ioread32(card_ctx->mmio_start + had_config_offset(pipe) + reg); }

-static void had_write_register_raw(struct snd_intelhad *ctx, u32 reg, u32 val) +static void had_write_register_raw(struct snd_intelhad_card *card_ctx,

• 		   int pipe, u32 reg, u32 val)
  

  {

• iowrite32(val, ctx->card_ctx->mmio_start + ctx->had_config_offset + reg);

• iowrite32(val, card_ctx->mmio_start + had_config_offset(pipe) + reg); }

  static void had_read_register(struct snd_intelhad *ctx, u32 reg, u32 *val)

@@ -208,13 +225,13 @@ static void had_read_register(struct snd_intelhad *ctx, u32 reg, u32 *val) if (!ctx->connected) *val = 0; else

• *val = had_read_register_raw(ctx, reg);
  

• *val = had_read_register_raw(ctx->card_ctx, ctx->pipe, reg);
  

  }

  static void had_write_register(struct snd_intelhad *ctx, u32 reg, u32 val) { if (ctx->connected)

• had_write_register_raw(ctx, reg, val);
  

• had_write_register_raw(ctx->card_ctx, ctx->pipe, reg, val);
  

  }

  /*

@@ -1361,6 +1378,9 @@ static void had_process_hot_plug(struct snd_intelhad *intelhaddata) return; }

• /* Disable Audio */
• had_enable_audio(intelhaddata, false);
• intelhaddata->connected = true; dev_dbg(intelhaddata->dev, "%s @ %d:DEBUG PLUG/UNPLUG : HAD_DRV_CONNECTED\n",

@@ -1523,26 +1543,31 @@ static const struct snd_kcontrol_new had_controls[] = { static irqreturn_t display_pipe_interrupt_handler(int irq, void *dev_id) { struct snd_intelhad_card *card_ctx = dev_id;

• int port;

• u32 audio_stat[3] = {};

• int pipe, port;

• for_each_pipe(card_ctx, pipe) {

• /* use raw register access to ack IRQs even while disconnected */
  

• audio_stat[pipe] = had_read_register_raw(card_ctx, pipe,
  

• 					 AUD_HDMI_STATUS) &
  

• 	(HDMI_AUDIO_UNDERRUN | HDMI_AUDIO_BUFFER_DONE);
  

• if (audio_stat[pipe])
  

• 	had_write_register_raw(card_ctx, pipe,
  

• 			       AUD_HDMI_STATUS, audio_stat[pipe]);
  

• }

  for_each_port(card_ctx, port) { struct snd_intelhad *ctx = &card_ctx->pcm_ctx[port];

• u32 audio_stat;
  

• int pipe = ctx->pipe;
  
  

• /* use raw register access to ack IRQs even while disconnected */
  

• audio_stat = had_read_register_raw(ctx, AUD_HDMI_STATUS);
  

• if (audio_stat & HDMI_AUDIO_UNDERRUN) {
  

• 	had_write_register_raw(ctx, AUD_HDMI_STATUS,
  

• 			       HDMI_AUDIO_UNDERRUN);
  

• 	had_process_buffer_underrun(ctx);
  

• }
  

• if (pipe < 0)
  

• 	continue;
  
  

• if (audio_stat & HDMI_AUDIO_BUFFER_DONE) {
  

• 	had_write_register_raw(ctx, AUD_HDMI_STATUS,
  

• 			       HDMI_AUDIO_BUFFER_DONE);
  

• if (audio_stat[pipe] & HDMI_AUDIO_BUFFER_DONE)
  had_process_buffer_done(ctx);
  

• }
  

• if (audio_stat[pipe] & HDMI_AUDIO_UNDERRUN)
  

• 	had_process_buffer_underrun(ctx);
  

  }

  return IRQ_HANDLED;

@@ -1551,16 +1576,12 @@ static irqreturn_t display_pipe_interrupt_handler(int irq, void *dev_id) /*

• monitor plug/unplug notification from i915; just kick off the work

*/ -static void notify_audio_lpe(struct platform_device *pdev) +static void notify_audio_lpe(struct platform_device *pdev, int port) { struct snd_intelhad_card *card_ctx = platform_get_drvdata(pdev);

• int port;

• struct snd_intelhad *ctx = &card_ctx->pcm_ctx[port];

• for_each_port(card_ctx, port) {

• struct snd_intelhad *ctx = &card_ctx->pcm_ctx[port];
  

• schedule_work(&ctx->hdmi_audio_wq);
  

• }

• schedule_work(&ctx->hdmi_audio_wq); }

  /* the work to handle monitor hot plug/unplug */

@@ -1569,34 +1590,27 @@ static void had_audio_wq(struct work_struct *work) struct snd_intelhad *ctx = container_of(work, struct snd_intelhad, hdmi_audio_wq); struct intel_hdmi_lpe_audio_pdata *pdata = ctx->dev->platform_data;

• struct intel_hdmi_lpe_audio_port_pdata *ppdata = &pdata->port;

• struct intel_hdmi_lpe_audio_port_pdata *ppdata = &pdata->port[ctx->port];

  pm_runtime_get_sync(ctx->dev); mutex_lock(&ctx->mutex); if (ppdata->pipe < 0) {

• dev_dbg(ctx->dev, "%s: Event: HAD_NOTIFY_HOT_UNPLUG\n",
  

• 	__func__);
  

• dev_dbg(ctx->dev, "%s: Event: HAD_NOTIFY_HOT_UNPLUG : port = %d\n",
  

• 	__func__, ctx->port);
  

• memset(ctx->eld, 0, sizeof(ctx->eld)); /* clear the old ELD */

• ctx->dp_output = false;
  

• ctx->tmds_clock_speed = 0;
  

• ctx->link_rate = 0;
  

• /* Shut down the stream */
  

  had_process_hot_unplug(ctx);

• ctx->pipe = -1;
  

  } else { dev_dbg(ctx->dev, "%s: HAD_NOTIFY_ELD : port = %d, tmds = %d\n",

• 	__func__, ppdata->port, ppdata->ls_clock);
  

• switch (ppdata->pipe) {
  

• case 0:
  

• 	ctx->had_config_offset = AUDIO_HDMI_CONFIG_A;
  

• 	break;
  

• case 1:
  

• 	ctx->had_config_offset = AUDIO_HDMI_CONFIG_B;
  

• 	break;
  

• case 2:
  

• 	ctx->had_config_offset = AUDIO_HDMI_CONFIG_C;
  

• 	break;
  

• default:
  

• 	dev_dbg(ctx->dev, "Invalid pipe %d\n",
  

• 		ppdata->pipe);
  

• 	break;
  

• }
  

• 	__func__, ctx->port, ppdata->ls_clock);
  
  

  memcpy(ctx->eld, ppdata->eld, sizeof(ctx->eld));

@@ -1609,11 +1623,18 @@ static void had_audio_wq(struct work_struct *work) ctx->link_rate = 0; }

• /*
  

•  * Shut down the stream before we change
  

•  * the pipe assignment for this pcm device
  

•  */
  

  had_process_hot_plug(ctx);

• /* Process mode change if stream is active */
  

• ctx->pipe = ppdata->pipe;
  

• /* Restart the stream if necessary */
  

  had_process_mode_change(ctx); }
• mutex_unlock(&ctx->mutex); pm_runtime_mark_last_busy(ctx->dev); pm_runtime_put_autosuspend(ctx->dev);

@@ -1794,7 +1815,8 @@ static int hdmi_lpe_audio_probe(struct platform_device *pdev)

init_channel_allocations();

• card_ctx->num_ports = 1;

• card_ctx->num_pipes = pdata->num_pipes;

• card_ctx->num_ports = pdata->num_ports;

  for_each_port(card_ctx, port) { struct snd_intelhad *ctx = &card_ctx->pcm_ctx[port];

@@ -1802,12 +1824,12 @@ static int hdmi_lpe_audio_probe(struct platform_device *pdev)

ctx->card_ctx = card_ctx;
ctx->dev = card_ctx->dev;

• ctx->port = port;
  

• ctx->pipe = -1;
  
  

  INIT_WORK(&ctx->hdmi_audio_wq, had_audio_wq);

• ctx->had_config_offset = AUDIO_HDMI_CONFIG_A;
  

• ret = snd_pcm_new(card, INTEL_HAD, PCM_INDEX, MAX_PB_STREAMS,
  

• ret = snd_pcm_new(card, INTEL_HAD, port, MAX_PB_STREAMS,
  	  MAX_CAP_STREAMS, &pcm);
  

  if (ret) goto err;

diff --git a/sound/x86/intel_hdmi_audio.h b/sound/x86/intel_hdmi_audio.h index 2725964ebc46..0d91bb5dbab7 100644 --- a/sound/x86/intel_hdmi_audio.h +++ b/sound/x86/intel_hdmi_audio.h @@ -32,7 +32,6 @@

#include "intel_hdmi_lpe_audio.h"

-#define PCM_INDEX 0 #define MAX_PB_STREAMS 1 #define MAX_CAP_STREAMS 0 #define BYTES_PER_WORD 0x4 @@ -112,6 +111,8 @@ struct snd_intelhad { struct snd_pcm_chmap *chmap; int tmds_clock_speed; int link_rate;

• int port; /* fixed */

• int pipe; /* can change dynamically */

  /* ring buffer (BD) position index */ unsigned int bd_head;

@@ -123,7 +124,6 @@ struct snd_intelhad { unsigned int period_bytes; /* PCM period size in bytes */

/* internal stuff */

• unsigned int had_config_offset; union aud_cfg aud_config; /* AUD_CONFIG reg value cache */ struct work_struct hdmi_audio_wq; struct mutex mutex; /* for protecting chmap and eld */

@@ -138,8 +138,9 @@ struct snd_intelhad_card { /* internal stuff */ int irq; void __iomem *mmio_start;

• int num_pipes; int num_ports;

• struct snd_intelhad pcm_ctx[3];

• struct snd_intelhad pcm_ctx[3]; /* one for each port */ };

  #endif /* _INTEL_HDMI_AUDIO_ */

On Mon, May 01, 2017 at 08:29:10PM -0500, Pierre-Louis Bossart wrote:

On 04/28/2017 02:37 PM, Ville Syrjälä wrote:

...

On Fri, Apr 28, 2017 at 12:10:31PM -0500, Pierre-Louis Bossart wrote:

...

On 04/28/2017 03:41 AM, Takashi Iwai wrote:

...

On Thu, 27 Apr 2017 18:02:19 +0200, ville.syrjala@linux.intel.com wrote:

...

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

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

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

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

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

...

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

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

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

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

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

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

I see frequent oops on startup with this lpe_audio_multipipe_2 branch with my CHT device not booting or no HDMI audio device created. Not sure if these issues are due to the new patches or to the rest of the drm code?

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

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

Disassembling that I get:

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

Comparing that with the disassembly from my build, that first call looks like the snprintf() and the second one would then be snd_jack_new(). So it seems to blow in the ctx->card_ctx->card dereference. But I don't really see how a NULL pointer could sneak in there.

There was an standard PORT_B off-by-one error which resulted in memory corruption.

Doh.

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

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

diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c index fa728ed..3bf6528 100644 --- a/drivers/gpu/drm/i915/intel_lpe_audio.c +++ b/drivers/gpu/drm/i915/intel_lpe_audio.c @@ -332,7 +332,7 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv, return;

     pdata = dev_get_platdata(&dev_priv->lpe_audio.platdev->dev);

•   ppdata = &pdata->port[port];
  

•   ppdata = &pdata->port[port - PORT_B];
  
     spin_lock_irqsave(&pdata->lpe_audio_slock, irqflags);
  
  

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

     if (pdata->notify_audio_lpe)

• pdata->notify_audio_lpe(dev_priv->lpe_audio.platdev, port);

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

     spin_unlock_irqrestore(&pdata->lpe_audio_slock, irqflags);
  

  }

diff --git a/sound/x86/intel_hdmi_audio.c b/sound/x86/intel_hdmi_audio.c index 909391d..0153439 100644 --- a/sound/x86/intel_hdmi_audio.c +++ b/sound/x86/intel_hdmi_audio.c @@ -1579,7 +1579,7 @@ static irqreturn_t display_pipe_interrupt_handler(int irq, void *dev_id) static void notify_audio_lpe(struct platform_device *pdev, int port) { struct snd_intelhad_card *card_ctx = platform_get_drvdata(pdev);

•   struct snd_intelhad *ctx = &card_ctx->pcm_ctx[port];
  

•   struct snd_intelhad *ctx = &card_ctx->pcm_ctx[port]; /* warning: 
  

offset with -PORT_B */

     schedule_work(&ctx->hdmi_audio_wq);

}