On Wed, 26 Apr 2017 03:58:57 +0200,
Pierre-Louis Bossart wrote:
>
> On 04/25/2017 03:27 PM, ville.syrjala@xxxxxxxxxxxxxxx wrote:
> > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> >
> > 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@xxxxxxx>
> > Cc: Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > ---
> > 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
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
