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 */