[Intel-gfx] [PATCH 4/7] ALSA: x86: hdmi: Add audio support for BYT and CHT
Anand, Jerome
jerome.anand at intel.com
Thu Dec 15 11:08:21 UTC 2016
> -----Original Message-----
> From: Takashi Iwai [mailto:tiwai at suse.de]
> Sent: Wednesday, December 14, 2016 6:26 PM
> To: Anand, Jerome <jerome.anand at intel.com>
> Cc: intel-gfx at lists.freedesktop.org; alsa-devel at alsa-project.org;
> ville.syrjala at linux.intel.com; broonie at kernel.org; pierre-
> louis.bossart at linux.intel.com; Ughreja, Rakesh A
> <rakesh.a.ughreja at intel.com>
> Subject: Re: [PATCH 4/7] ALSA: x86: hdmi: Add audio support for BYT and CHT
>
> On Mon, 12 Dec 2016 19:10:40 +0100,
> Jerome Anand wrote:
> >
> > Hdmi audio driver based on the child platform device created by gfx
> > driver is implemented.
> > This audio driver is derived from legacy intel hdmi audio driver.
> >
> > The interfaces for interaction between gfx and audio are updated and
> > the driver implementation updated to derive interrupts in its own
> > address space based on irq chip framework
> >
> > Signed-off-by: Pierre-Louis Bossart
> > <pierre-louis.bossart at linux.intel.com>
> > Signed-off-by: Jerome Anand <jerome.anand at intel.com>
> > ---
> > sound/x86/Makefile | 2 +
> > sound/x86/intel_hdmi_audio.c | 1907
> ++++++++++++++++++++++++++++++++++++++
> > sound/x86/intel_hdmi_audio.h | 201 ++++
> > sound/x86/intel_hdmi_audio_if.c | 551 +++++++++++
> > sound/x86/intel_hdmi_lpe_audio.c | 16 +-
> > 5 files changed, 2671 insertions(+), 6 deletions(-) create mode
> > 100644 sound/x86/intel_hdmi_audio.c create mode 100644
> > sound/x86/intel_hdmi_audio.h create mode 100644
> > sound/x86/intel_hdmi_audio_if.c
> >
> > diff --git a/sound/x86/Makefile b/sound/x86/Makefile index
> > 78b2ae1..bc074d0 100644
> > --- a/sound/x86/Makefile
> > +++ b/sound/x86/Makefile
> > @@ -3,6 +3,8 @@ DRIVER_NAME := hdmi_lpe_audio ccflags-y +=
> > -Idrivers/gpu/drm/i915
> >
> > $(DRIVER_NAME)-objs += \
> > + intel_hdmi_audio.o \
> > + intel_hdmi_audio_if.o \
> > intel_hdmi_lpe_audio.o
> >
> > obj-$(CONFIG_HDMI_LPE_AUDIO) += $(DRIVER_NAME).o diff --git
> > a/sound/x86/intel_hdmi_audio.c b/sound/x86/intel_hdmi_audio.c new
> file
> > mode 100644 index 0000000..461b7d7
> > --- /dev/null
> > +++ b/sound/x86/intel_hdmi_audio.c
> > @@ -0,0 +1,1907 @@
> > +/*
> > + * intel_hdmi_audio.c - Intel HDMI audio driver
> > + *
> > + * Copyright (C) 2016 Intel Corp
> > + * Authors: Sailaja Bandarupalli <sailaja.bandarupalli at intel.com>
> > + * Ramesh Babu K V <ramesh.babu at intel.com>
> > + * Vaibhav Agarwal <vaibhav.agarwal at intel.com>
> > + * Jerome Anand <jerome.anand at intel.com>
> > + *
> >
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ~~~~~~~~~~~~
> > +~~~~~
> > + *
> > + * This program is free software; you can redistribute it and/or
> > +modify
> > + * it under the terms of the GNU General Public License as published
> > +by
> > + * the Free Software Foundation; version 2 of the License.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > +but
> > + * WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> GNU
> > + * General Public License for more details.
> > + *
> > + *
> >
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ~~~~~~~~~~~~
> > +~~~~~
> > + * ALSA driver for Intel HDMI audio
> > + */
> > +
> > +#define pr_fmt(fmt) "had: " fmt
> > +
> > +#include <linux/platform_device.h>
> > +#include <linux/io.h>
> > +#include <linux/slab.h>
> > +#include <linux/module.h>
> > +#include <linux/acpi.h>
> > +#include <asm/cacheflush.h>
> > +#include <sound/pcm.h>
> > +#include <sound/core.h>
> > +#include <sound/pcm_params.h>
> > +#include <sound/initval.h>
> > +#include <sound/control.h>
> > +#include <sound/initval.h>
> > +#include "intel_hdmi_audio.h"
> > +
> > +static DEFINE_MUTEX(had_mutex);
> > +
> > +/*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; static struct snd_intelhad
> > +*had_data;
> > +
> > +module_param(hdmi_card_index, int, 0444);
>
> Use module_param_named(index, hdmi_card_index, int, 0444); Ditto for id.
>
OK
> > +MODULE_PARM_DESC(hdmi_card_index,
> > + "Index value for INTEL Intel HDMI Audio controller.");
> > +module_param(hdmi_card_id, charp, 0444);
OK
> > +MODULE_PARM_DESC(hdmi_card_id,
> > + "ID string for INTEL Intel HDMI Audio controller.");
> > +
> > +/*
> > + * ELD SA bits in the CEA Speaker Allocation data block */ static int
> > +eld_speaker_allocation_bits[] = {
> > + [0] = FL | FR,
> > + [1] = LFE,
> > + [2] = FC,
> > + [3] = RL | RR,
> > + [4] = RC,
> > + [5] = FLC | FRC,
> > + [6] = RLC | RRC,
> > + /* the following are not defined in ELD yet */
> > + [7] = 0,
> > +};
> > +
> > +/*
> > + * This is an ordered list!
> > + *
> > + * The preceding ones have better chances to be selected by
> > + * hdmi_channel_allocation().
> > + */
> > +static struct cea_channel_speaker_allocation channel_allocations[] = {
> > +/* channel: 7 6 5 4 3 2 1 0 */
> > +{ .ca_index = 0x00, .speakers = { 0, 0, 0, 0, 0, 0, FR, FL } },
> > + /* 2.1 */
> > +{ .ca_index = 0x01, .speakers = { 0, 0, 0, 0, 0, LFE, FR, FL } },
> > + /* Dolby Surround */
> > +{ .ca_index = 0x02, .speakers = { 0, 0, 0, 0, FC, 0, FR, FL } },
> > + /* surround40 */
> > +{ .ca_index = 0x08, .speakers = { 0, 0, RR, RL, 0, 0, FR, FL } },
> > + /* surround41 */
> > +{ .ca_index = 0x09, .speakers = { 0, 0, RR, RL, 0, LFE, FR, FL } },
> > + /* surround50 */
> > +{ .ca_index = 0x0a, .speakers = { 0, 0, RR, RL, FC, 0, FR, FL } },
> > + /* surround51 */
> > +{ .ca_index = 0x0b, .speakers = { 0, 0, RR, RL, FC, LFE, FR, FL } },
> > + /* 6.1 */
> > +{ .ca_index = 0x0f, .speakers = { 0, RC, RR, RL, FC, LFE, FR, FL } },
> > + /* surround71 */
> > +{ .ca_index = 0x13, .speakers = { RRC, RLC, RR, RL, FC, LFE,
> > +FR, FL } },
> > +
> > +{ .ca_index = 0x03, .speakers = { 0, 0, 0, 0, FC, LFE, FR, FL } },
> > +{ .ca_index = 0x04, .speakers = { 0, 0, 0, RC, 0, 0, FR, FL } },
> > +{ .ca_index = 0x05, .speakers = { 0, 0, 0, RC, 0, LFE, FR, FL } },
> > +{ .ca_index = 0x06, .speakers = { 0, 0, 0, RC, FC, 0, FR, FL } },
> > +{ .ca_index = 0x07, .speakers = { 0, 0, 0, RC, FC, LFE, FR, FL } },
> > +{ .ca_index = 0x0c, .speakers = { 0, RC, RR, RL, 0, 0, FR, FL } },
> > +{ .ca_index = 0x0d, .speakers = { 0, RC, RR, RL, 0, LFE, FR, FL } },
> > +{ .ca_index = 0x0e, .speakers = { 0, RC, RR, RL, FC, 0, FR, FL } },
> > +{ .ca_index = 0x10, .speakers = { RRC, RLC, RR, RL, 0, 0, FR, FL } },
> > +{ .ca_index = 0x11, .speakers = { RRC, RLC, RR, RL, 0, LFE, FR, FL } },
> > +{ .ca_index = 0x12, .speakers = { RRC, RLC, RR, RL, FC, 0, FR, FL } },
> > +{ .ca_index = 0x14, .speakers = { FRC, FLC, 0, 0, 0, 0, FR, FL } },
> > +{ .ca_index = 0x15, .speakers = { FRC, FLC, 0, 0, 0, LFE, FR, FL } },
> > +{ .ca_index = 0x16, .speakers = { FRC, FLC, 0, 0, FC, 0, FR, FL } },
> > +{ .ca_index = 0x17, .speakers = { FRC, FLC, 0, 0, FC, LFE, FR, FL } },
> > +{ .ca_index = 0x18, .speakers = { FRC, FLC, 0, RC, 0, 0, FR, FL } },
> > +{ .ca_index = 0x19, .speakers = { FRC, FLC, 0, RC, 0, LFE, FR, FL } },
> > +{ .ca_index = 0x1a, .speakers = { FRC, FLC, 0, RC, FC, 0, FR, FL } },
> > +{ .ca_index = 0x1b, .speakers = { FRC, FLC, 0, RC, FC, LFE, FR, FL } },
> > +{ .ca_index = 0x1c, .speakers = { FRC, FLC, RR, RL, 0, 0, FR, FL } },
> > +{ .ca_index = 0x1d, .speakers = { FRC, FLC, RR, RL, 0, LFE, FR, FL } },
> > +{ .ca_index = 0x1e, .speakers = { FRC, FLC, RR, RL, FC, 0, FR, FL } },
> > +{ .ca_index = 0x1f, .speakers = { FRC, FLC, RR, RL, FC, LFE,
> > +FR, FL } }, };
> > +
> > +static struct channel_map_table map_tables[] = {
> > + { SNDRV_CHMAP_FL, 0x00, FL },
> > + { SNDRV_CHMAP_FR, 0x01, FR },
> > + { SNDRV_CHMAP_RL, 0x04, RL },
> > + { SNDRV_CHMAP_RR, 0x05, RR },
> > + { SNDRV_CHMAP_LFE, 0x02, LFE },
> > + { SNDRV_CHMAP_FC, 0x03, FC },
> > + { SNDRV_CHMAP_RLC, 0x06, RLC },
> > + { SNDRV_CHMAP_RRC, 0x07, RRC },
> > + {} /* terminator */
> > +};
>
> We really have the same stuff in multiple places...
> A serious cleanup will be needed later, but it's OK for now to make the code
> self-contained.
>
>
OK - I'll send a cleanup patch after these are accepted.
> > +/* hardware capability structure */
> > +static const struct snd_pcm_hardware snd_intel_hadstream = {
> > + .info = (SNDRV_PCM_INFO_INTERLEAVED |
> > + SNDRV_PCM_INFO_DOUBLE |
> > + SNDRV_PCM_INFO_MMAP|
> > + SNDRV_PCM_INFO_MMAP_VALID |
> > + SNDRV_PCM_INFO_BATCH),
> > + .formats = (SNDRV_PCM_FMTBIT_S24 |
> > + SNDRV_PCM_FMTBIT_U24),
>
> Are only these formats available? I see the code dealing with 16bit samples...
We support only 24 bit samples. Changes related to 16 can be removed.
> Also, does it support unsigned format?
>
Yes its supported
> (snip)
> > +static int snd_intelhad_pcm_trigger(struct snd_pcm_substream
> *substream,
> > + int cmd)
> > +{
> > + int caps, retval = 0;
> > + unsigned long flag_irq;
> > + struct snd_intelhad *intelhaddata;
> > + struct had_stream_pvt *stream;
> > + struct had_pvt_data *had_stream;
> > +
> > + pr_debug("snd_intelhad_pcm_trigger called\n");
> > +
> > + intelhaddata = snd_pcm_substream_chip(substream);
> > + stream = substream->runtime->private_data;
> > + had_stream = intelhaddata->private_data;
> > +
> > + switch (cmd) {
> > + case SNDRV_PCM_TRIGGER_START:
> > + pr_debug("Trigger Start\n");
> > +
> > + /* Disable local INTRs till register prgmng is done */
> > + if (had_get_hwstate(intelhaddata)) {
> > + pr_err("_START: HDMI cable plugged-out\n");
> > + retval = -ENODEV;
> > + break;
> > + }
> > + stream->stream_status = STREAM_RUNNING;
> > +
> > + spin_lock_irqsave(&intelhaddata->had_spinlock, flag_irq);
>
> The trigger callback is always atomic and already spin-irqlocked.
>
OK
> > +static int snd_intelhad_pcm_prepare(struct snd_pcm_substream
> > +*substream) {
> > + int retval;
> > + u32 disp_samp_freq, n_param;
> > + struct snd_intelhad *intelhaddata;
> > + struct snd_pcm_runtime *runtime;
> > + struct had_pvt_data *had_stream;
> > +
> > + pr_debug("snd_intelhad_pcm_prepare called\n");
> > +
> > + intelhaddata = snd_pcm_substream_chip(substream);
> > + runtime = substream->runtime;
> > + had_stream = intelhaddata->private_data;
> > +
> > + if (had_get_hwstate(intelhaddata)) {
> > + pr_err("%s: HDMI cable plugged-out\n", __func__);
> > + snd_pcm_stop(substream,
> SNDRV_PCM_STATE_DISCONNECTED);
>
> I wonder whether this works well with this driver.
> SNDRV_PCM_STATE_DISCONNECTED is for the hot-unplug. It's fine to use it,
> but then PulseAudio assumes that the whole card got unplugged, and
> removes the device entry, thus there is no way back even if you replug,
> unless you really reload the driver.
>
OK - snd_pcm_stop() must not be done. Else, the device needs to be rebooted for PB
>
> thanks,
>
> Takashi
More information about the Intel-gfx
mailing list