[PATCH 0/6] ASoC: OMAP4: Add support for HDMI audio

[PATCH 0/6] ASoC: OMAP4: Add support for HDMI audio

[Ricardo Neri]

This is to add support for HDMI audio on OMAP4 chips. This work has two
parts: DSS implementation and ASoC implementation. This set of patches
presents the ASoC implementation. The approach is to utilize the DSS
HDMI driver to act as an ASoC codec. This set of patches adds the ASoC
CPU DAI and machine drivers. These drivers will utilize the ASoC HDMI
codec provided in the DSS HDMI driver. DSS part of the implementation 
is currently under review in the linux-omap list.

The DSS HDMI driver controls the availability of HDMI output and audio
can be played only when the HDMI video device is enabled and the video
timing in use is one of the CEA-861-D video timings supported by the
OMAP4 HDMI IP.

This implementation is for Basic Audio as defined in CEA-861-D: 16 bit/sample
linear PCM 2-channel audio with sample rates of 32, 44.1 and 48kHz. As
described in the HDMI specification, support for Basic Audio is mandatory
for HDMI sinks and does not require of EDID parsing.

Additionally, this implementation supports 24-bit samples in 32-bit words.
Further audio capabilities such as more sample rates and multichannel audio
will be added in the future when EDID parsing is available for HDMI driver.

This implementation is based on on:
* OMAP4: DSS2: Add dss_dss_clk opt clock for OMAP4, by Sumit Semwal
 (http://gitorious.org/linux-omap-dss2/linux/commit/0da5637569bb9bf93e6930cd1c3b452eede50ba2)
* Kernel 2.6.39-rc6
 (git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git v2.6.39-rc6)

* v4 of OMAP4: DSS2: HDMI: Add support for audio
 git://gitorious.org/omap-audio/linux-audio.git ricardon/topic/hdmi-audio-v1
 
In order to validate functionality, it is necessary to utilize the DSS
HDMI ASoC codec. The complete implementation, DSS and ASoC parts, is available at:
 git://gitorious.org/omap-audio/linux-audio.git ricardon/topic/hdmi-audio-v1
 
Validation was performed on HDMI TV. Penguins were present on the display
and audio playback was performed with aplay at 32, 44.1 and 48kHz with S16_LE
and S24_LE sample formats. Validation was performed on SDP4430 ES2.1
and Panda ES2.1 and ES2.0.

Ricardo Neri (6):
  OMAP4: HDMI: Add OMAP device for HDMI audio CPU DAI
  OMAP4: Add device for HDMI OMAP4 audio for ASoC machine driver
  ASoC: OMAP: Add header for HDMI CPU DAI
  ASoC: OMAP: Add CPU DAI driver for HDMI
  ASoC: OMAP4: Add HDMI Audio machine driver for OMAP4 boards
  ASoC: OMAP: Update Makefile and Kconfig for HDMI audio

 arch/arm/mach-omap2/devices.c    |   32 +++++++
 sound/soc/omap/Kconfig           |   11 +++
 sound/soc/omap/Makefile          |    4 +
 sound/soc/omap/omap-hdmi.c       |  169 ++++++++++++++++++++++++++++++++++++++
 sound/soc/omap/omap-hdmi.h       |   36 ++++++++
 sound/soc/omap/omap4-hdmi-card.c |  100 ++++++++++++++++++++++
 6 files changed, 352 insertions(+), 0 deletions(-)
 create mode 100644 sound/soc/omap/omap-hdmi.c
 create mode 100644 sound/soc/omap/omap-hdmi.h
 create mode 100644 sound/soc/omap/omap4-hdmi-card.c

[PATCH 1/6] OMAP4: HDMI: Add OMAP device for HDMI audio CPU DAI

[Ricardo Neri]

Add device for HDMI audio. This device uses hwmod data from
dss_hdmi. This device is to be used by the ASoC HDMI CPU DAI to
obtain base memory address and DMA request number.

Signed-off-by: Ricardo Neri <ricardo.neri@ti.com>
---
 arch/arm/mach-omap2/devices.c |   19 +++++++++++++++++++
 1 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c
index 7b85585..a62f691 100644
--- a/arch/arm/mach-omap2/devices.c
+++ b/arch/arm/mach-omap2/devices.c
@@ -313,6 +313,25 @@ OMAP_MCBSP_PLATFORM_DEVICE(5);
 
 static void omap_init_audio(void)
 {
+#if defined(CONFIG_SND_OMAP_SOC_OMAP4_HDMI) || \
+	defined(CONFIG_SND_OMAP_SOC_OMAP4_HDMI_MODULE)
+	struct omap_hwmod *oh_hdmi;
+	struct omap_device *od_hdmi;
+	char *oh_hdmi_name = "dss_hdmi";
+	char *dev_hdmi_name = "hdmi-audio-dai";
+
+	if (cpu_is_omap44xx()) {
+		oh_hdmi = omap_hwmod_lookup(oh_hdmi_name);
+		WARN(!oh_hdmi, "%s: could not find omap_hwmod for %s\n",
+			__func__, oh_hdmi_name);
+
+		od_hdmi = omap_device_build(dev_hdmi_name, -1, oh_hdmi, NULL, 0,
+			NULL, 0, false);
+		WARN(IS_ERR(od_hdmi), "%s: could not build omap_device for %s\n",
+			__func__, dev_hdmi_name);
+	}
+#endif
+
 	platform_device_register(&omap_mcbsp1);
 	platform_device_register(&omap_mcbsp2);
 	if (cpu_is_omap243x() || cpu_is_omap34xx() || cpu_is_omap44xx()) {
-- 
1.7.1

Re: [PATCH 1/6] OMAP4: HDMI: Add OMAP device for HDMI audio CPU DAI

[Mark Brown]

On Thu, May 05, 2011 at 02:21:48AM -0500, Ricardo Neri wrote:

> +#if defined(CONFIG_SND_OMAP_SOC_OMAP4_HDMI) || \
> +	defined(CONFIG_SND_OMAP_SOC_OMAP4_HDMI_MODULE)

Why is this conditional...

>  	platform_device_register(&omap_mcbsp1);
>  	platform_device_register(&omap_mcbsp2);
>  	if (cpu_is_omap243x() || cpu_is_omap34xx() || cpu_is_omap44xx()) {

...but this not?

Re: [PATCH 1/6] OMAP4: HDMI: Add OMAP device for HDMI audio CPU DAI

[Ricardo Neri]

Hi Mark,

Thanks for your comments!

Am Donnerstag, den 05.05.2011, 03:17 -0500 schrieb Mark Brown:
> On Thu, May 05, 2011 at 02:21:48AM -0500, Ricardo Neri wrote:
> 
> > +#if defined(CONFIG_SND_OMAP_SOC_OMAP4_HDMI) || \
> > +	defined(CONFIG_SND_OMAP_SOC_OMAP4_HDMI_MODULE)
> 
> Why is this conditional...
> 
> >  	platform_device_register(&omap_mcbsp1);
> >  	platform_device_register(&omap_mcbsp2);
> >  	if (cpu_is_omap243x() || cpu_is_omap34xx() || cpu_is_omap44xx()) {
> 
> ...but this not?
The device is specific to OMAP4. I added the #if because I think the
device should be built only if HDMI audio option is selected. Also, the
rest of the devices are conditionally built with CONFIG_SND_SOC option.
Do you think I should use cpu_is_omap44xx() instead? In that case, the
hdmi-audio-dai will be built even if HDMI audio option is not selected.

Re: [PATCH 1/6] OMAP4: HDMI: Add OMAP device for HDMI audio CPU DAI

[Mark Brown]

On Thu, May 05, 2011 at 02:24:11PM -0500, Ricardo Neri wrote:

> Do you think I should use cpu_is_omap44xx() instead? In that case, the
> hdmi-audio-dai will be built even if HDMI audio option is not selected.

That doesn't seem like a real problem.

Re: [PATCH 1/6] OMAP4: HDMI: Add OMAP device for HDMI audio CPU DAI

[Ricardo Neri]

Hi Mark,
Am Freitag, den 06.05.2011, 08:16 -0500 schrieb Mark Brown:
> On Thu, May 05, 2011 at 02:24:11PM -0500, Ricardo Neri wrote:
> 
> > Do you think I should use cpu_is_omap44xx() instead? In that case, the
> > hdmi-audio-dai will be built even if HDMI audio option is not selected.
> 
> That doesn't seem like a real problem.
OK, in that case I will remove the #if conditional and all the audio
devices will be created as whole when the SND_SOC config is selected.

[PATCH 2/6] OMAP4: Add device for HDMI OMAP4 audio for ASoC machine driver

[Ricardo Neri]

This device is used by the OMAP4 HDMI audio machine driver to register the
HDMI sound card.

Signed-off-by: Ricardo Neri <ricardo.neri@ti.com>
---
 arch/arm/mach-omap2/devices.c |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c
index a62f691..c407b9f 100644
--- a/arch/arm/mach-omap2/devices.c
+++ b/arch/arm/mach-omap2/devices.c
@@ -299,6 +299,17 @@ static struct platform_device omap_pcm = {
 	.id	= -1,
 };
 
+#if defined(CONFIG_SND_OMAP_SOC_OMAP4_HDMI) || \
+	defined(CONFIG_SND_OMAP_SOC_OMAP4_HDMI_MODULE)
+/*
+ * Device for the ASoC OMAP4 HDMI machine driver
+ */
+static struct platform_device omap4_hdmi_audio = {
+	.name	= "omap4-hdmi-audio",
+	.id	= -1,
+};
+#endif
+
 /*
  * OMAP2420 has 2 McBSP ports
  * OMAP2430 has 5 McBSP ports
@@ -329,6 +340,8 @@ static void omap_init_audio(void)
 			NULL, 0, false);
 		WARN(IS_ERR(od_hdmi), "%s: could not build omap_device for %s\n",
 			__func__, dev_hdmi_name);
+
+		platform_device_register(&omap4_hdmi_audio);
 	}
 #endif
 
-- 
1.7.1

Re: [PATCH 2/6] OMAP4: Add device for HDMI OMAP4 audio for ASoC machine driver

[Mark Brown]

On Thu, May 05, 2011 at 02:21:49AM -0500, Ricardo Neri wrote:

> +#if defined(CONFIG_SND_OMAP_SOC_OMAP4_HDMI) || \
> +	defined(CONFIG_SND_OMAP_SOC_OMAP4_HDMI_MODULE)

Similar comment here...

Re: [PATCH 2/6] OMAP4: Add device for HDMI OMAP4 audio for ASoC machine driver

[Ricardo Neri]

Am Donnerstag, den 05.05.2011, 03:18 -0500 schrieb Mark Brown:
> On Thu, May 05, 2011 at 02:21:49AM -0500, Ricardo Neri wrote:
> 
> > +#if defined(CONFIG_SND_OMAP_SOC_OMAP4_HDMI) || \
> > +	defined(CONFIG_SND_OMAP_SOC_OMAP4_HDMI_MODULE)
> 
> Similar comment here...
I wanted to build the omap4-hdmi-audio device only if the HDMI audio
option is selected in menuconfig.

[PATCH 3/6] ASoC: OMAP: Add header for HDMI CPU DAI

[Ricardo Neri]

This header contains definitions used by the ASoC HDMI CPU DAI.
These are used to define the DMA port as well as the supported
audio sample formats and rates.

Signed-off-by: Ricardo Neri <ricardo.neri@ti.com>
---
 sound/soc/omap/omap-hdmi.h |   36 ++++++++++++++++++++++++++++++++++++
 1 files changed, 36 insertions(+), 0 deletions(-)
 create mode 100644 sound/soc/omap/omap-hdmi.h

diff --git a/sound/soc/omap/omap-hdmi.h b/sound/soc/omap/omap-hdmi.h
new file mode 100644
index 0000000..d71a970
--- /dev/null
+++ b/sound/soc/omap/omap-hdmi.h
@@ -0,0 +1,36 @@
+/*
+ * omap-hdmi.h
+ *
+ * Definitions for OMAP ALSA SoC DAI driver for HDMI audio on OMAP4 processors.
+ * Copyright (C) 2010-2011 Texas Instruments Incorporated - http://www.ti.com/
+ * Authors: Jorge Candelaria <jorge.candelaria@gmail.com>
+ *          Ricardo Neri <ricardo.neri@ti.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
+ * 02110-1301 USA
+ *
+ */
+
+#ifndef __OMAP_HDMI_H__
+#define __OMAP_HDMI_H__
+
+#define OMAP_HDMI_AUDIO_DMA_PORT 0x8c
+
+#define OMAP_HDMI_RATES	(SNDRV_PCM_RATE_32000 | \
+				SNDRV_PCM_RATE_44100 | SNDRV_PCM_RATE_48000)
+
+#define OMAP_HDMI_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | \
+				SNDRV_PCM_FMTBIT_S24_LE)
+
+#endif
-- 
1.7.1

Re: [PATCH 3/6] ASoC: OMAP: Add header for HDMI CPU DAI

[Mark Brown]

On Thu, May 05, 2011 at 02:21:50AM -0500, Ricardo Neri wrote:
> This header contains definitions used by the ASoC HDMI CPU DAI.
> These are used to define the DMA port as well as the supported
> audio sample formats and rates.

Just include this as part of the driver.

Re: [PATCH 3/6] ASoC: OMAP: Add header for HDMI CPU DAI

[Ricardo Neri]

Am Donnerstag, den 05.05.2011, 03:25 -0500 schrieb Mark Brown:
> On Thu, May 05, 2011 at 02:21:50AM -0500, Ricardo Neri wrote:
> > This header contains definitions used by the ASoC HDMI CPU DAI.
> > These are used to define the DMA port as well as the supported
> > audio sample formats and rates.
> 
> Just include this as part of the driver.
I will merge with the driver patch.

[PATCH 4/6] ASoC: OMAP: Add CPU DAI driver for HDMI

[Ricardo Neri]

Addition of the HDMI CPU DAI driver for OMAP4. This driver is in
charge of configuring DMA settings for HDMI. Also, it finds
the HDMI video device and determines if audio playback can proceed.

Signed-off-by: Ricardo Neri <ricardo.neri@ti.com>
---
 sound/soc/omap/omap-hdmi.c |  169 ++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 169 insertions(+), 0 deletions(-)
 create mode 100644 sound/soc/omap/omap-hdmi.c

diff --git a/sound/soc/omap/omap-hdmi.c b/sound/soc/omap/omap-hdmi.c
new file mode 100644
index 0000000..ba80b35
--- /dev/null
+++ b/sound/soc/omap/omap-hdmi.c
@@ -0,0 +1,169 @@
+/*
+ * omap-hdmi.c
+ *
+ * OMAP ALSA SoC DAI driver for HDMI audio on OMAP4 processors.
+ * Copyright (C) 2010-2011 Texas Instruments Incorporated - http://www.ti.com/
+ * Authors: Jorge Candelaria <jorge.candelaria@gmail.com>
+ *          Ricardo Neri <ricardo.neri@ti.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
+ * 02110-1301 USA
+ *
+ */
+
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/device.h>
+#include <sound/core.h>
+#include <sound/pcm.h>
+#include <sound/pcm_params.h>
+#include <sound/initval.h>
+#include <sound/soc.h>
+
+#include <plat/dma.h>
+#include <plat/display.h>
+#include <plat/omap44xx.h>
+#include "omap-pcm.h"
+#include "omap-hdmi.h"
+
+static struct omap_pcm_dma_data omap_hdmi_dai_dma_params = {
+	.name = "HDMI playback",
+	.sync_mode = OMAP_DMA_SYNC_PACKET,
+};
+
+static int omap_hdmi_dai_startup(struct snd_pcm_substream *substream,
+				  struct snd_soc_dai *dai)
+{
+	int err = 0, i;
+	struct omap_overlay *ovl = NULL;
+
+	/*
+	 * Make sure that the period bytes are multiple of the DMA packet size.
+	 * Largest packet size we use is 32 32-bit words = 128 bytes
+	 */
+	err = snd_pcm_hw_constraint_step(substream->runtime, 0,
+				 SNDRV_PCM_HW_PARAM_PERIOD_BYTES, 128);
+	if (err < 0)
+		return err;
+
+	/* find DSS HDMI device */
+	for (i = 0; i < omap_dss_get_num_overlays(); i++) {
+		ovl = omap_dss_get_overlay(i);
+		if (strcmp(ovl->manager->device->name, "hdmi") == 0)
+			break;
+	}
+	if (ovl->manager->device->state != OMAP_DSS_DISPLAY_ACTIVE) {
+		pr_err("HDMI display is not active!\n");
+		return -EIO;
+	}
+	return err;
+}
+
+static int omap_hdmi_dai_hw_params(struct snd_pcm_substream *substream,
+				    struct snd_pcm_hw_params *params,
+				    struct snd_soc_dai *dai)
+{
+	int err = 0;
+
+	switch (params_format(params)) {
+	case SNDRV_PCM_FORMAT_S16_LE:
+		omap_hdmi_dai_dma_params.data_type = OMAP_DMA_DATA_TYPE_S32;
+		omap_hdmi_dai_dma_params.packet_size = 16;
+		break;
+	case SNDRV_PCM_FORMAT_S24_LE:
+		omap_hdmi_dai_dma_params.data_type = OMAP_DMA_DATA_TYPE_S32;
+		omap_hdmi_dai_dma_params.packet_size = 32;
+		break;
+	default:
+		err = -EINVAL;
+	}
+
+	snd_soc_dai_set_dma_data(dai, substream,
+				 &omap_hdmi_dai_dma_params);
+
+	return err;
+}
+
+static struct snd_soc_dai_ops omap_hdmi_dai_ops = {
+	.startup	= omap_hdmi_dai_startup,
+	.hw_params	= omap_hdmi_dai_hw_params,
+};
+
+static struct snd_soc_dai_driver omap_hdmi_dai = {
+	.playback = {
+		.channels_min = 2,
+		.channels_max = 2,
+		.rates = OMAP_HDMI_RATES,
+		.formats = OMAP_HDMI_FORMATS,
+	},
+	.ops = &omap_hdmi_dai_ops,
+};
+
+static __devinit int omap_hdmi_probe(struct platform_device *pdev)
+{
+	int ret;
+	struct resource *hdmi_rsrc;
+
+	hdmi_rsrc = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!hdmi_rsrc) {
+		pr_err("Cannot obtain IORESOURCE_MEM HDMI\n");
+		return -EINVAL;
+	}
+
+	omap_hdmi_dai_dma_params.port_addr =  hdmi_rsrc->start
+		+ OMAP_HDMI_AUDIO_DMA_PORT;
+
+	hdmi_rsrc = platform_get_resource(pdev, IORESOURCE_DMA, 0);
+	if (!hdmi_rsrc) {
+		pr_err("Cannot obtain IORESOURCE_DMA HDMI\n");
+		return -EINVAL;
+	}
+
+	omap_hdmi_dai_dma_params.dma_req =  hdmi_rsrc->start;
+
+	ret = snd_soc_register_dai(&pdev->dev, &omap_hdmi_dai);
+	return ret;
+}
+
+static int __devexit omap_hdmi_remove(struct platform_device *pdev)
+{
+	snd_soc_unregister_dai(&pdev->dev);
+	return 0;
+}
+
+static struct platform_driver hdmi_dai_driver = {
+	.driver = {
+		.name = "hdmi-audio-dai",
+		.owner = THIS_MODULE,
+	},
+	.probe = omap_hdmi_probe,
+	.remove = __devexit_p(omap_hdmi_remove),
+};
+
+static int __init hdmi_dai_init(void)
+{
+	return platform_driver_register(&hdmi_dai_driver);
+}
+module_init(hdmi_dai_init);
+
+static void __exit hdmi_dai_exit(void)
+{
+	platform_driver_unregister(&hdmi_dai_driver);
+}
+module_exit(hdmi_dai_exit);
+
+MODULE_AUTHOR("Jorge Candelaria <jorge.candelaria@gmail.com>");
+MODULE_AUTHOR("Ricardo Neri <ricardo.neri@ti.com>");
+MODULE_DESCRIPTION("OMAP HDMI SoC Interface");
+MODULE_LICENSE("GPL");
-- 
1.7.1

Re: [PATCH 4/6] ASoC: OMAP: Add CPU DAI driver for HDMI

[Mark Brown]

On Thu, May 05, 2011 at 02:21:51AM -0500, Ricardo Neri wrote:

> +static int omap_hdmi_dai_startup(struct snd_pcm_substream *substream,
> +				  struct snd_soc_dai *dai)
> +{

> +	if (ovl->manager->device->state != OMAP_DSS_DISPLAY_ACTIVE) {
> +		pr_err("HDMI display is not active!\n");
> +		return -EIO;
> +	}

dev_er().  Also, why are you checking this, and are you checking it too
early?

> +MODULE_AUTHOR("Jorge Candelaria <jorge.candelaria@gmail.com>");
> +MODULE_AUTHOR("Ricardo Neri <ricardo.neri@ti.com>");
> +MODULE_DESCRIPTION("OMAP HDMI SoC Interface");
> +MODULE_LICENSE("GPL");

MODULE_ALIAS().

Re: [PATCH 4/6] ASoC: OMAP: Add CPU DAI driver for HDMI

[Ricardo Neri]

Hi Mark,

Am Donnerstag, den 05.05.2011, 03:22 -0500 schrieb Mark Brown:
> On Thu, May 05, 2011 at 02:21:51AM -0500, Ricardo Neri wrote:
> 
> > +static int omap_hdmi_dai_startup(struct snd_pcm_substream *substream,
> > +				  struct snd_soc_dai *dai)
> > +{
> 
> > +	if (ovl->manager->device->state != OMAP_DSS_DISPLAY_ACTIVE) {
> > +		pr_err("HDMI display is not active!\n");
> > +		return -EIO;
> > +	}
> 
> dev_er().  Also, why are you checking this, and are you checking it too
> early?

I will replace with dev_err(). I need to check if HDMI display is active
because the HDMI DSS driver is in charge of powering on/off the HDMI IP.
Audio can play only if the HDMI IP is on. I don't have a .trigger
function in my DAI so I thought it was OK to perform the check
at .startup. Do you think I should create a .trigger function and
perform the check from there?
> 
> > +MODULE_AUTHOR("Jorge Candelaria <jorge.candelaria@gmail.com>");
> > +MODULE_AUTHOR("Ricardo Neri <ricardo.neri@ti.com>");
> > +MODULE_DESCRIPTION("OMAP HDMI SoC Interface");
> > +MODULE_LICENSE("GPL");
> 
> MODULE_ALIAS().
I will add.

Re: [PATCH 4/6] ASoC: OMAP: Add CPU DAI driver for HDMI

[Mark Brown]

On Thu, May 05, 2011 at 02:35:05PM -0500, Ricardo Neri wrote:
> Am Donnerstag, den 05.05.2011, 03:22 -0500 schrieb Mark Brown:

> > dev_er().  Also, why are you checking this, and are you checking it too
> > early?

> I will replace with dev_err(). I need to check if HDMI display is active
> because the HDMI DSS driver is in charge of powering on/off the HDMI IP.
> Audio can play only if the HDMI IP is on. I don't have a .trigger
> function in my DAI so I thought it was OK to perform the check
> at .startup. Do you think I should create a .trigger function and
> perform the check from there?

Why do you need to check at all?  The playback won't work when the
output isn't connected but the application layer will find that out when
there's no data transfer anyway.  If you implement the check in startup
I'd imagine there may be some races with hotplug of HDMI cables.

Re: [PATCH 4/6] ASoC: OMAP: Add CPU DAI driver for HDMI

[Ricardo Neri]

Hi Mark,
Am Freitag, den 06.05.2011, 08:21 -0500 schrieb Mark Brown:
> On Thu, May 05, 2011 at 02:35:05PM -0500, Ricardo Neri wrote:
> > Am Donnerstag, den 05.05.2011, 03:22 -0500 schrieb Mark Brown:
> 
> > > dev_er().  Also, why are you checking this, and are you checking it too
> > > early?
> 
> > I will replace with dev_err(). I need to check if HDMI display is active
> > because the HDMI DSS driver is in charge of powering on/off the HDMI IP.
> > Audio can play only if the HDMI IP is on. I don't have a .trigger
> > function in my DAI so I thought it was OK to perform the check
> > at .startup. Do you think I should create a .trigger function and
> > perform the check from there?
> 
> Why do you need to check at all?  The playback won't work when the
> output isn't connected but the application layer will find that out when
> there's no data transfer anyway.  If you implement the check in startup
> I'd imagine there may be some races with hotplug of HDMI cables.

I need to check somewhere if the Display Subsystem (and the HDMI IP) is
on. Otherwise I will have L3 interconnect errors when trying to access
the DSS while it is power-disconnected. I need the HDMI IP on for
getting configurations (e.g, deep color mode) and use the HDMI audio
FIFO.

Can you clarify what kind of races with hotplug are you concerned? The
DSS would detect cable connection events and power on the HDMI IP
accordingly. HDMI audio driver would only check if it is powered-on.
 
Perhaps I could create a .trigger function and perform the check at the
start of audio playback. Furthermore, I could move the check to the HDMI
audio codec, as it is more related to DSS.

Re: [PATCH 4/6] ASoC: OMAP: Add CPU DAI driver for HDMI

[Mark Brown]

On Sat, May 07, 2011 at 07:05:17PM -0500, Ricardo Neri wrote:
> Am Freitag, den 06.05.2011, 08:21 -0500 schrieb Mark Brown:

> > Why do you need to check at all?  The playback won't work when the
> > output isn't connected but the application layer will find that out when
> > there's no data transfer anyway.  If you implement the check in startup
> > I'd imagine there may be some races with hotplug of HDMI cables.

> I need to check somewhere if the Display Subsystem (and the HDMI IP) is
> on. Otherwise I will have L3 interconnect errors when trying to access
> the DSS while it is power-disconnected. I need the HDMI IP on for
> getting configurations (e.g, deep color mode) and use the HDMI audio
> FIFO.

I repeat my question - why do you need to check this?  You'll get errors
but you're generating an error anyway so it's not clear what the urgency
is.

> Can you clarify what kind of races with hotplug are you concerned? The
> DSS would detect cable connection events and power on the HDMI IP
> accordingly. HDMI audio driver would only check if it is powered-on.

Something noticing that the cable is plugged in and trying to begin
audio setup while the link is still being initialised.

Re: [PATCH 4/6] ASoC: OMAP: Add CPU DAI driver for HDMI

[Liam Girdwood]

On 08/05/11 11:34, Mark Brown wrote:
> On Sat, May 07, 2011 at 07:05:17PM -0500, Ricardo Neri wrote:
>> Am Freitag, den 06.05.2011, 08:21 -0500 schrieb Mark Brown:
> 
>>> Why do you need to check at all?  The playback won't work when the
>>> output isn't connected but the application layer will find that out when
>>> there's no data transfer anyway.  If you implement the check in startup
>>> I'd imagine there may be some races with hotplug of HDMI cables.
> 
>> I need to check somewhere if the Display Subsystem (and the HDMI IP) is
>> on. Otherwise I will have L3 interconnect errors when trying to access
>> the DSS while it is power-disconnected. I need the HDMI IP on for
>> getting configurations (e.g, deep color mode) and use the HDMI audio
>> FIFO.
> 
> I repeat my question - why do you need to check this?  You'll get errors
> but you're generating an error anyway so it's not clear what the urgency
> is.
> 

I think the problem here is that the errors are fatal data aborts here when L3 DSS access is disabled.

Liam

Re: [PATCH 4/6] ASoC: OMAP: Add CPU DAI driver for HDMI

[Mark Brown]

On Sun, May 08, 2011 at 12:14:04PM +0100, Liam Girdwood wrote:
> On 08/05/11 11:34, Mark Brown wrote:

> > I repeat my question - why do you need to check this?  You'll get errors
> > but you're generating an error anyway so it's not clear what the urgency
> > is.

> I think the problem here is that the errors are fatal data aborts here
> when L3 DSS access is disabled.

So it'll segfault or something instead of just not transferring data or
whatever?

Re: [PATCH 4/6] ASoC: OMAP: Add CPU DAI driver for HDMI

[Liam Girdwood]

On 08/05/11 16:56, Mark Brown wrote:
> On Sun, May 08, 2011 at 12:14:04PM +0100, Liam Girdwood wrote:
>> On 08/05/11 11:34, Mark Brown wrote:
> 
>>> I repeat my question - why do you need to check this?  You'll get errors
>>> but you're generating an error anyway so it's not clear what the urgency
>>> is.
> 
>> I think the problem here is that the errors are fatal data aborts here
>> when L3 DSS access is disabled.
> 
> So it'll segfault or something instead of just not transferring data or
> whatever?

Yep, it will cause a CPU data abort exception in this case.

Liam

Re: [PATCH 4/6] ASoC: OMAP: Add CPU DAI driver for HDMI

[Mark Brown]

On Sun, May 08, 2011 at 08:44:41PM +0100, Liam Girdwood wrote:
> On 08/05/11 16:56, Mark Brown wrote:

> > So it'll segfault or something instead of just not transferring data or
> > whatever?

> Yep, it will cause a CPU data abort exception in this case.

OK, that makes things clearer.  It'd still be good to postpone the check
for as long as possible, just in case things get racy - currently it
feels too early in the process - but having the check makes sense.

Re: [PATCH 4/6] ASoC: OMAP: Add CPU DAI driver for HDMI

[Neri, Ricardo]

Hi Mark, Liam,

On Sun, May 8, 2011 at 18:42, Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:
> On Sun, May 08, 2011 at 08:44:41PM +0100, Liam Girdwood wrote:
>> On 08/05/11 16:56, Mark Brown wrote:
>
>> > So it'll segfault or something instead of just not transferring data or
>> > whatever?
>
>> Yep, it will cause a CPU data abort exception in this case.
>
> OK, that makes things clearer.  It'd still be good to postpone the check
> for as long as possible, just in case things get racy - currently it
> feels too early in the process - but having the check makes sense.
I think if the check can be done in .hw_params. because this check makes sure the hw is available.

[PATCH 5/6] ASoC: OMAP4: Add HDMI Audio machine driver for OMAP4 boards

[Ricardo Neri]

Add machine driver for HDMI audio on OMAP4 boards. This driver is
in charge of putting together the HDMI audio codec and the CPU DAI
and register the HDMI sound card with ALSA.

Signed-off-by: Ricardo Neri <ricardo.neri@ti.com>
---
 sound/soc/omap/omap4-hdmi-card.c |  100 ++++++++++++++++++++++++++++++++++++++
 1 files changed, 100 insertions(+), 0 deletions(-)
 create mode 100644 sound/soc/omap/omap4-hdmi-card.c

diff --git a/sound/soc/omap/omap4-hdmi-card.c b/sound/soc/omap/omap4-hdmi-card.c
new file mode 100644
index 0000000..b1ecfc4
--- /dev/null
+++ b/sound/soc/omap/omap4-hdmi-card.c
@@ -0,0 +1,100 @@
+/*
+ * omap4-hdmi-card.c
+ *
+ * OMAP ALSA SoC machine driver for TI OMAP4 HDMI
+ * Copyright (C) 2011 Texas Instruments Incorporated - http://www.ti.com/
+ * Author: Ricardo Neri <ricardo.neri@ti.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
+ * 02110-1301 USA
+ *
+ */
+
+#include <sound/pcm.h>
+#include <sound/soc.h>
+#include <asm/mach-types.h>
+
+static struct snd_soc_dai_link omap4_hdmi_dai = {
+		.name = "HDMI",
+		.stream_name = "HDMI",
+		.cpu_dai_name = "hdmi-audio-dai",
+		.platform_name = "omap-pcm-audio",
+		.codec_name = "omapdss_hdmi",
+		.codec_dai_name = "hdmi-audio-codec"
+};
+
+static struct snd_soc_card snd_soc_omap4_hdmi = {
+	.name = "OMAP4HDMI",
+	.dai_link = &omap4_hdmi_dai,
+	.num_links = 1,
+};
+
+static __devinit int omap4_hdmi_probe(struct platform_device *pdev)
+{
+	struct snd_soc_card *card = &snd_soc_omap4_hdmi;
+	int ret;
+
+	if (!cpu_is_omap44xx()) {
+		dev_err(&pdev->dev, "Not running on OMAP4!\n");
+		return -ENODEV;
+	}
+
+	card->dev = &pdev->dev;
+	platform_set_drvdata(pdev, card);
+
+	ret = snd_soc_register_card(card);
+	if (ret) {
+		dev_err(&pdev->dev, "snd_soc_register_card failed (%d)\n", ret);
+		platform_set_drvdata(pdev, NULL);
+		card->dev = NULL;
+		return ret;
+	}
+	return 0;
+}
+
+static int __devexit omap4_hdmi_remove(struct platform_device *pdev)
+{
+	struct snd_soc_card *card = platform_get_drvdata(pdev);
+
+	snd_soc_unregister_card(card);
+	platform_set_drvdata(pdev, NULL);
+	card->dev = NULL;
+	return 0;
+}
+
+static struct platform_driver omap4_hdmi_driver = {
+	.driver = {
+		.name = "omap4-hdmi-audio",
+		.owner = THIS_MODULE,
+	},
+	.probe = omap4_hdmi_probe,
+	.remove = __devexit_p(omap4_hdmi_remove),
+};
+
+
+static int __init omap4_hdmi_init(void)
+{
+	return platform_driver_register(&omap4_hdmi_driver);
+}
+module_init(omap4_hdmi_init);
+
+static void __exit omap4_hdmi_exit(void)
+{
+	platform_driver_unregister(&omap4_hdmi_driver);
+}
+module_exit(omap4_hdmi_exit);
+
+MODULE_AUTHOR("Ricardo Neri <ricardo.neri@ti.com>");
+MODULE_DESCRIPTION("OMAP4 HDMI machine ASoC driver");
+MODULE_LICENSE("GPL");
-- 
1.7.1

Re: [PATCH 5/6] ASoC: OMAP4: Add HDMI Audio machine driver for OMAP4 boards

[Mark Brown]

On Thu, May 05, 2011 at 02:21:52AM -0500, Ricardo Neri wrote:

> +static struct snd_soc_dai_link omap4_hdmi_dai = {
> +		.name = "HDMI",
> +		.stream_name = "HDMI",

Indentation.

> +static __devinit int omap4_hdmi_probe(struct platform_device *pdev)
> +{
> +	struct snd_soc_card *card = &snd_soc_omap4_hdmi;
> +	int ret;
> +
> +	if (!cpu_is_omap44xx()) {
> +		dev_err(&pdev->dev, "Not running on OMAP4!\n");
> +		return -ENODEV;
> +	}

Why check this?  If the system can't support the DAIs they won't
register so it's redundant and it means more work for OMAP5.

> +	card->dev = &pdev->dev;
> +	platform_set_drvdata(pdev, card);

The set_drvdata() isn't needed.

> +	snd_soc_unregister_card(card);
> +	platform_set_drvdata(pdev, NULL);

You never need to reset driver data on unregister, nothing should
reference it except a driver.

> +MODULE_AUTHOR("Ricardo Neri <ricardo.neri@ti.com>");
> +MODULE_DESCRIPTION("OMAP4 HDMI machine ASoC driver");
> +MODULE_LICENSE("GPL");

MODULE_ALIAS().

Re: [PATCH 5/6] ASoC: OMAP4: Add HDMI Audio machine driver for OMAP4 boards

[Ricardo Neri]

Am Donnerstag, den 05.05.2011, 03:25 -0500 schrieb Mark Brown:
> On Thu, May 05, 2011 at 02:21:52AM -0500, Ricardo Neri wrote:
> 
> > +static struct snd_soc_dai_link omap4_hdmi_dai = {
> > +		.name = "HDMI",
> > +		.stream_name = "HDMI",
> 
> Indentation.
I will correct.
> 
> > +static __devinit int omap4_hdmi_probe(struct platform_device *pdev)
> > +{
> > +	struct snd_soc_card *card = &snd_soc_omap4_hdmi;
> > +	int ret;
> > +
> > +	if (!cpu_is_omap44xx()) {
> > +		dev_err(&pdev->dev, "Not running on OMAP4!\n");
> > +		return -ENODEV;
> > +	}
> 
> Why check this?  If the system can't support the DAIs they won't
> register so it's redundant and it means more work for OMAP5.
I will remove this verification.
> 
> > +	card->dev = &pdev->dev;
> > +	platform_set_drvdata(pdev, card);
> 
> The set_drvdata() isn't needed.
I will remove this as well.
> 
> > +	snd_soc_unregister_card(card);
> > +	platform_set_drvdata(pdev, NULL);
> 
> You never need to reset driver data on unregister, nothing should
> reference it except a driver.
I saw that other drivers (harmony and mfld) reset driver data after
unregister. Is that incorrect as well? Anyways, I don't needed for my
driver.

> 
> > +MODULE_AUTHOR("Ricardo Neri <ricardo.neri@ti.com>");
> > +MODULE_DESCRIPTION("OMAP4 HDMI machine ASoC driver");
> > +MODULE_LICENSE("GPL");
> 
> MODULE_ALIAS().
I will add.

Re: [PATCH 5/6] ASoC: OMAP4: Add HDMI Audio machine driver for OMAP4 boards

[Mark Brown]

On Thu, May 05, 2011 at 04:09:19PM -0500, Ricardo Neri wrote:
> Am Donnerstag, den 05.05.2011, 03:25 -0500 schrieb Mark Brown:

> > You never need to reset driver data on unregister, nothing should
> > reference it except a driver.
> I saw that other drivers (harmony and mfld) reset driver data after
> unregister. Is that incorrect as well? Anyways, I don't needed for my
> driver.

It's always a waste of time.

[PATCH 6/6] ASoC: OMAP: Update Makefile and Kconfig for HDMI audio

[Ricardo Neri]

Update Makefile and Kconfig to build HDMI audio support for
OMAP4 SDP and Panda boards.

Signed-off-by: Ricardo Neri <ricardo.neri@ti.com>
---
 sound/soc/omap/Kconfig  |   11 +++++++++++
 sound/soc/omap/Makefile |    4 ++++
 2 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/sound/soc/omap/Kconfig b/sound/soc/omap/Kconfig
index b592298..e8ca5be 100644
--- a/sound/soc/omap/Kconfig
+++ b/sound/soc/omap/Kconfig
@@ -9,6 +9,9 @@ config SND_OMAP_SOC_MCBSP
 config SND_OMAP_SOC_MCPDM
 	tristate
 
+config SND_OMAP_SOC_HDMI
+	tristate
+
 config SND_OMAP_SOC_N810
 	tristate "SoC Audio support for Nokia N810"
 	depends on SND_OMAP_SOC && MACH_NOKIA_N810 && I2C
@@ -108,6 +111,14 @@ config SND_OMAP_SOC_SDP4430
 	  Say Y if you want to add support for SoC audio on Texas Instruments
 	  SDP4430.
 
+config SND_OMAP_SOC_OMAP4_HDMI
+	tristate "SoC Audio support for Texas Instruments OMAP4 HDMI"
+	depends on SND_OMAP_SOC && OMAP4_DSS_HDMI && OMAP2_DSS && ARCH_OMAP4
+	select SND_OMAP_SOC_HDMI
+	help
+	  Say Y if you want to add support for SoC HDMI audio on Texas Instruments
+	  OMAP4 chips
+
 config SND_OMAP_SOC_OMAP3_PANDORA
 	tristate "SoC Audio support for OMAP3 Pandora"
 	depends on TWL4030_CORE && SND_OMAP_SOC && MACH_OMAP3_PANDORA
diff --git a/sound/soc/omap/Makefile b/sound/soc/omap/Makefile
index ba9fc65..8016642 100644
--- a/sound/soc/omap/Makefile
+++ b/sound/soc/omap/Makefile
@@ -2,10 +2,12 @@
 snd-soc-omap-objs := omap-pcm.o
 snd-soc-omap-mcbsp-objs := omap-mcbsp.o
 snd-soc-omap-mcpdm-objs := omap-mcpdm.o mcpdm.o
+snd-soc-omap-hdmi-objs := omap-hdmi.o
 
 obj-$(CONFIG_SND_OMAP_SOC) += snd-soc-omap.o
 obj-$(CONFIG_SND_OMAP_SOC_MCBSP) += snd-soc-omap-mcbsp.o
 obj-$(CONFIG_SND_OMAP_SOC_MCPDM) += snd-soc-omap-mcpdm.o
+obj-$(CONFIG_SND_OMAP_SOC_HDMI) += snd-soc-omap-hdmi.o
 
 # OMAP Machine Support
 snd-soc-n810-objs := n810.o
@@ -22,6 +24,7 @@ snd-soc-omap3pandora-objs := omap3pandora.o
 snd-soc-omap3beagle-objs := omap3beagle.o
 snd-soc-zoom2-objs := zoom2.o
 snd-soc-igep0020-objs := igep0020.o
+snd-soc-omap4-hdmi-objs := omap4-hdmi-card.o
 
 obj-$(CONFIG_SND_OMAP_SOC_N810) += snd-soc-n810.o
 obj-$(CONFIG_SND_OMAP_SOC_RX51) += snd-soc-rx51.o
@@ -37,3 +40,4 @@ obj-$(CONFIG_SND_OMAP_SOC_OMAP3_PANDORA) += snd-soc-omap3pandora.o
 obj-$(CONFIG_SND_OMAP_SOC_OMAP3_BEAGLE) += snd-soc-omap3beagle.o
 obj-$(CONFIG_SND_OMAP_SOC_ZOOM2) += snd-soc-zoom2.o
 obj-$(CONFIG_SND_OMAP_SOC_IGEP0020) += snd-soc-igep0020.o
+obj-$(CONFIG_SND_OMAP_SOC_OMAP4_HDMI) += snd-soc-omap4-hdmi.o
-- 
1.7.1