Re: [alsa-devel] [PATCH 11/11] ALSA: x86: Register multiple PCM devices for the LPE audio card

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux