All of lore.kernel.org
 help / color / mirror / Atom feed
From: Takashi Iwai <tiwai@suse.de>
To: Jerome Anand <jerome.anand@intel.com>
Cc: alsa-devel@alsa-project.org, intel-gfx@lists.freedesktop.org,
	broonie@kernel.org, rakesh.a.ughreja@intel.com
Subject: Re: [PATCH 4/7] ALSA: x86: hdmi: Add audio support for BYT and CHT
Date: Wed, 14 Dec 2016 13:56:13 +0100	[thread overview]
Message-ID: <s5hshpqekk2.wl-tiwai@suse.de> (raw)
In-Reply-To: <20161212181043.12512-5-jerome.anand@intel.com>

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.

> +MODULE_PARM_DESC(hdmi_card_index,
> +		"Index value for INTEL Intel HDMI Audio controller.");
> +module_param(hdmi_card_id, charp, 0444);
> +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.


> +/* 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...  Also, does it support unsigned format?

(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.

> +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.


thanks,

Takashi
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2016-12-14 12:56 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-12-12 18:10 [PATCH 0/7] Add support for Legacy HDMI audio drivers Jerome Anand
2016-12-12  6:53 ` ✓ Fi.CI.BAT: success for Add support for Legacy HDMI audio drivers (rev2) Patchwork
2016-12-12 18:10 ` [PATCH 1/7] drm/i915: setup bridge for HDMI LPE audio driver Jerome Anand
2016-12-14 11:43   ` Takashi Iwai
2016-12-15  6:06     ` Anand, Jerome
2016-12-14 12:52   ` Daniel Vetter
2016-12-14 13:03     ` Takashi Iwai
2016-12-15  6:10     ` Anand, Jerome
2016-12-15 19:04     ` [alsa-devel] " Pierre-Louis Bossart
2016-12-12 18:10 ` [PATCH 2/7] drm/i915: Add support for audio driver notifications Jerome Anand
2016-12-14 11:50   ` Takashi Iwai
2016-12-15 10:18     ` Anand, Jerome
2016-12-15 11:48       ` Ville Syrjälä
2016-12-14 12:55   ` Daniel Vetter
2016-12-14 13:13     ` Takashi Iwai
2016-12-15 19:32       ` Pierre-Louis Bossart
2016-12-15 10:21     ` Anand, Jerome
2016-12-12 18:10 ` [PATCH 3/7] ALSA: add shell for Intel HDMI LPE audio driver Jerome Anand
2016-12-14 12:45   ` Takashi Iwai
2016-12-15 10:55     ` Anand, Jerome
2016-12-15 11:14       ` Takashi Iwai
2016-12-15 11:44         ` Mark Brown
2016-12-15 20:37         ` [alsa-devel] " Pierre-Louis Bossart
2016-12-15 21:01           ` Takashi Iwai
2016-12-12 18:10 ` [PATCH 4/7] ALSA: x86: hdmi: Add audio support for BYT and CHT Jerome Anand
2016-12-14 12:56   ` Takashi Iwai [this message]
2016-12-15 11:08     ` Anand, Jerome
2016-12-12 18:10 ` [PATCH 5/7] ALSA: x86: hdmi: Improve position reporting Jerome Anand
2016-12-14 12:57   ` Takashi Iwai
2016-12-14 14:09     ` Pierre-Louis Bossart
2016-12-14 14:36       ` Takashi Iwai
2016-12-14 23:39         ` [alsa-devel] " Pierre-Louis Bossart
2016-12-12 18:10 ` [PATCH 6/7] ALSA: x86: hdmi: Fixup some monitor Jerome Anand
2016-12-14 12:58   ` Takashi Iwai
2016-12-12 18:10 ` [PATCH 7/7] ALSA: x86: hdmi: continue playback even when display resolution changes Jerome Anand
2016-12-14 13:01   ` Takashi Iwai
2016-12-15 11:14     ` Anand, Jerome
2016-12-15 20:44       ` [alsa-devel] " Pierre-Louis Bossart
2016-12-14 11:07 ` [PATCH 0/7] Add support for Legacy HDMI audio drivers Takashi Iwai

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=s5hshpqekk2.wl-tiwai@suse.de \
    --to=tiwai@suse.de \
    --cc=alsa-devel@alsa-project.org \
    --cc=broonie@kernel.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=jerome.anand@intel.com \
    --cc=rakesh.a.ughreja@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.