-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Wednesday, December 14, 2016 6:26 PM To: Anand, Jerome jerome.anand@intel.com Cc: intel-gfx@lists.freedesktop.org; alsa-devel@alsa-project.org; ville.syrjala@linux.intel.com; broonie@kernel.org; pierre- louis.bossart@linux.intel.com; Ughreja, Rakesh A rakesh.a.ughreja@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@linux.intel.com Signed-off-by: Jerome Anand jerome.anand@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@intel.com
Ramesh Babu K V <ramesh.babu@intel.com>
Vaibhav Agarwal <vaibhav.agarwal@intel.com>
Jerome Anand <jerome.anand@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