On Wed, 26 Apr 2017 03:58:57 +0200, Pierre-Louis Bossart wrote: > > On 04/25/2017 03:27 PM, [email protected] wrote: > > From: Ville Syrjälä <[email protected]> > > > > 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 <[email protected]> > > Cc: Pierre-Louis Bossart <[email protected]> > > Signed-off-by: Ville Syrjälä <[email protected]> > > --- > > drivers/gpu/drm/i915/intel_lpe_audio.c | 14 ++++----- > > include/drm/intel_lpe_audio.h | 6 ++-- > > sound/x86/intel_hdmi_audio.c | 53 > > +++++++++++++++------------------- > > sound/x86/intel_hdmi_audio.h | 3 +- > > 4 files changed, 34 insertions(+), 42 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c > > b/drivers/gpu/drm/i915/intel_lpe_audio.c > > index a593fdf73171..270aa3e3f0e2 100644 > > --- a/drivers/gpu/drm/i915/intel_lpe_audio.c > > +++ b/drivers/gpu/drm/i915/intel_lpe_audio.c > > @@ -111,6 +111,7 @@ lpe_audio_platdev_create(struct drm_i915_private > > *dev_priv) > > pinfo.size_data = sizeof(*pdata); > > pinfo.dma_mask = DMA_BIT_MASK(32); > > + pdata->num_pipes = INTEL_INFO(dev_priv)->num_pipes; > > spin_lock_init(&pdata->lpe_audio_slock); > > platdev = platform_device_register_full(&pinfo); > > @@ -318,7 +319,7 @@ void intel_lpe_audio_notify(struct drm_i915_private > > *dev_priv, > > enum pipe pipe, enum port port, > > const void *eld, int ls_clock, bool dp_output) > > { > > - unsigned long irq_flags; > > + unsigned long irqflags; > > struct intel_hdmi_lpe_audio_pdata *pdata; > > struct intel_hdmi_lpe_audio_pipe_pdata *ppdata; > > u32 audio_enable; > > @@ -327,14 +328,12 @@ void intel_lpe_audio_notify(struct drm_i915_private > > *dev_priv, > > return; > > pdata = dev_get_platdata(&dev_priv->lpe_audio.platdev->dev); > > - ppdata = &pdata->pipe; > > + ppdata = &pdata->pipe[pipe]; > > - spin_lock_irqsave(&pdata->lpe_audio_slock, irq_flags); > > + spin_lock_irqsave(&pdata->lpe_audio_slock, irqflags); > > audio_enable = I915_READ(VLV_AUD_PORT_EN_DBG(port)); > > - pdata->pipe_id = pipe; > > - > > if (eld != NULL) { > > memcpy(ppdata->eld, eld, HDMI_MAX_ELD_BYTES); > > ppdata->port = port; > > @@ -356,8 +355,7 @@ void intel_lpe_audio_notify(struct drm_i915_private > > *dev_priv, > > } > > if (pdata->notify_audio_lpe) > > - pdata->notify_audio_lpe(dev_priv->lpe_audio.platdev); > > + pdata->notify_audio_lpe(dev_priv->lpe_audio.platdev, pipe); > > - spin_unlock_irqrestore(&pdata->lpe_audio_slock, > > - irq_flags); > > + spin_unlock_irqrestore(&pdata->lpe_audio_slock, irqflags); > > } > > diff --git a/include/drm/intel_lpe_audio.h b/include/drm/intel_lpe_audio.h > > index 26e569ad8683..b391b2822140 100644 > > --- a/include/drm/intel_lpe_audio.h > > +++ b/include/drm/intel_lpe_audio.h > > @@ -39,10 +39,10 @@ struct intel_hdmi_lpe_audio_pipe_pdata { > > }; > > struct intel_hdmi_lpe_audio_pdata { > > - struct intel_hdmi_lpe_audio_pipe_pdata pipe; > > - int pipe_id; > > + struct intel_hdmi_lpe_audio_pipe_pdata pipe[3]; > > + int num_pipes; > > - void (*notify_audio_lpe)(struct platform_device *pdev); > > + void (*notify_audio_lpe)(struct platform_device *pdev, int pipe); > > spinlock_t lpe_audio_slock; > > }; > > diff --git a/sound/x86/intel_hdmi_audio.c > > b/sound/x86/intel_hdmi_audio.c > > index 5e2149fe5218..e5863a6d3aa9 100644 > > --- a/sound/x86/intel_hdmi_audio.c > > +++ b/sound/x86/intel_hdmi_audio.c > > @@ -195,12 +195,12 @@ static void had_substream_put(struct snd_intelhad > > *intelhaddata) > > /* Register access functions */ > > static u32 had_read_register_raw(struct snd_intelhad *ctx, u32 reg) > > { > > - return ioread32(ctx->card_ctx->mmio_start + ctx->had_config_offset + > > reg); > > + return ioread32(ctx->mmio_start + reg); > > } > > static void had_write_register_raw(struct snd_intelhad *ctx, u32 > > reg, u32 val) > > { > > - iowrite32(val, ctx->card_ctx->mmio_start + ctx->had_config_offset + > > reg); > > + iowrite32(val, ctx->mmio_start + reg); > > } > > static void had_read_register(struct snd_intelhad *ctx, u32 reg, > > u32 *val) > > @@ -1551,16 +1551,12 @@ static irqreturn_t > > display_pipe_interrupt_handler(int irq, void *dev_id) > > /* > > * monitor plug/unplug notification from i915; just kick off the work > > */ > > -static void notify_audio_lpe(struct platform_device *pdev) > > +static void notify_audio_lpe(struct platform_device *pdev, int pipe) > > { > > struct snd_intelhad_card *card_ctx = platform_get_drvdata(pdev); > > - int pipe; > > - > > - for_each_pipe(card_ctx, pipe) { > > - struct snd_intelhad *ctx = &card_ctx->pcm_ctx[pipe]; > > + struct snd_intelhad *ctx = &card_ctx->pcm_ctx[pipe]; > > - schedule_work(&ctx->hdmi_audio_wq); > > - } > > + schedule_work(&ctx->hdmi_audio_wq); > > } > > /* the work to handle monitor hot plug/unplug */ > > @@ -1569,7 +1565,7 @@ static void had_audio_wq(struct work_struct *work) > > struct snd_intelhad *ctx = > > container_of(work, struct snd_intelhad, hdmi_audio_wq); > > struct intel_hdmi_lpe_audio_pdata *pdata = ctx->dev->platform_data; > > - struct intel_hdmi_lpe_audio_pipe_pdata *ppdata = &pdata->pipe; > > + struct intel_hdmi_lpe_audio_pipe_pdata *ppdata = > > &pdata->pipe[ctx->pipe]; > > pm_runtime_get_sync(ctx->dev); > > mutex_lock(&ctx->mutex); > > @@ -1582,22 +1578,6 @@ static void had_audio_wq(struct work_struct *work) > > dev_dbg(ctx->dev, "%s: HAD_NOTIFY_ELD : port = %d, tmds = %d\n", > > __func__, ppdata->port, ppdata->ls_clock); > > - switch (pdata->pipe_id) { > > - case 0: > > - ctx->had_config_offset = AUDIO_HDMI_CONFIG_A; > > - break; > > - case 1: > > - ctx->had_config_offset = AUDIO_HDMI_CONFIG_B; > > - break; > > - case 2: > > - ctx->had_config_offset = AUDIO_HDMI_CONFIG_C; > > - break; > > - default: > > - dev_dbg(ctx->dev, "Invalid pipe %d\n", > > - pdata->pipe_id); > > - break; > > - } > > - > > memcpy(ctx->eld, ppdata->eld, sizeof(ctx->eld)); > > ctx->dp_output = ppdata->dp_output; > > @@ -1794,7 +1774,7 @@ static int hdmi_lpe_audio_probe(struct > > platform_device *pdev) > > init_channel_allocations(); > > - card_ctx->num_pipes = 1; > > + card_ctx->num_pipes = pdata->num_pipes; > > for_each_pipe(card_ctx, pipe) { > > struct snd_intelhad *ctx = &card_ctx->pcm_ctx[pipe]; > > @@ -1806,9 +1786,24 @@ static int hdmi_lpe_audio_probe(struct > > platform_device *pdev) > > INIT_WORK(&ctx->hdmi_audio_wq, had_audio_wq); > > - ctx->had_config_offset = AUDIO_HDMI_CONFIG_A; > > + switch (pipe) { > > + case 0: > > + ctx->mmio_start = > > + card_ctx->mmio_start + AUDIO_HDMI_CONFIG_A; > > + break; > > + case 1: > > + ctx->mmio_start = > > + card_ctx->mmio_start + AUDIO_HDMI_CONFIG_B; > > + break; > > + case 2: > > + ctx->mmio_start = > > + card_ctx->mmio_start + AUDIO_HDMI_CONFIG_C; > > + break; > > + default: > > + break; > > + } > > - ret = snd_pcm_new(card, INTEL_HAD, PCM_INDEX, > > MAX_PB_STREAMS, > > + ret = snd_pcm_new(card, INTEL_HAD, pipe, MAX_PB_STREAMS, > > MAX_CAP_STREAMS, &pcm); > > if (ret) > > goto err; > > diff --git a/sound/x86/intel_hdmi_audio.h b/sound/x86/intel_hdmi_audio.h > > index a209096b03df..ab0de5d525f4 100644 > > --- a/sound/x86/intel_hdmi_audio.h > > +++ b/sound/x86/intel_hdmi_audio.h > > @@ -32,7 +32,6 @@ > > #include "intel_hdmi_lpe_audio.h" > > -#define PCM_INDEX 0 > > #define MAX_PB_STREAMS 1 > > #define MAX_CAP_STREAMS 0 > > #define BYTES_PER_WORD 0x4 > > @@ -124,7 +123,7 @@ struct snd_intelhad { > > unsigned int period_bytes; /* PCM period size in bytes */ > > /* internal stuff */ > > - unsigned int had_config_offset; > > + void __iomem *mmio_start; > > union aud_cfg aud_config; /* AUD_CONFIG reg value cache */ > > struct work_struct hdmi_audio_wq; > > struct mutex mutex; /* for protecting chmap and eld */ > _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
