> -----Original Message----- > From: Takashi Iwai [mailto:tiwai@xxxxxxx] > Sent: Wednesday, December 14, 2016 6:26 PM > To: Anand, Jerome <jerome.anand@xxxxxxxxx> > Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; alsa-devel@xxxxxxxxxxxxxxxx; > ville.syrjala@xxxxxxxxxxxxxxx; broonie@xxxxxxxxxx; pierre- > louis.bossart@xxxxxxxxxxxxxxx; Ughreja, Rakesh A > <rakesh.a.ughreja@xxxxxxxxx> > 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@xxxxxxxxxxxxxxx> > > Signed-off-by: Jerome Anand <jerome.anand@xxxxxxxxx> > > --- > > 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@xxxxxxxxx> > > + * Ramesh Babu K V <ramesh.babu@xxxxxxxxx> > > + * Vaibhav Agarwal <vaibhav.agarwal@xxxxxxxxx> > > + * Jerome Anand <jerome.anand@xxxxxxxxx> > > + * > > > +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > ~~~~~~~~~~~~ > > +~~~~~ > > + * > > + * 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 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
