[PATCH v2 0/2] ARM: OMAP2+: HDMI: Update platform devices for audio

[PATCH v2 0/2] ARM: OMAP2+: HDMI: Update platform devices for audio

[Ricardo Neri]

Hi Mark, Tomi, Liam, Tony,

This set aims to be the version 2 of my previous submission[1] and aims to
address the potential issues that Mark and Tomi described on such submission.

The creation of the platform device for HDMI audio from the OMAPDSS HDMI driver
that was previously submitted[2] is resubmitted to be complemented with code to
remove the platform device created in /arch/arm/mach-omap2/devices.c in order
to have a complete series that does not break functionality in any patch.

Also, a second patch proposes new names and locations for the ASoC HDMI CPU DAI
and machine drivers. This intends to give to the platform devices more
descriptive names and create them from more relevant locations.

This set of patches requires that

commit 14840b9a83c6a56629db2ba0ec247503e975f143
Author: Ricardo Neri <ricardo.neri@ti.com>
Date:   Tue Nov 6 00:19:17 2012 -0600

    OMAPDSS: HDMI: Create platform device for audio support

is reverted in Tomi's git://gitorious.org/linux-omap-dss2/linux.git master

Changes from v1.
*Put in a single series all the patches related to platform device updates.
*Now HDMI audio works correctly in every patch.
*Remove reference to the TPD12S015 HDMI companion chip as the ASoC drivers
 are not aware of this and other chips could be used in the future.

 BR,

 Ricardo

[1]. http://www.mail-archive.com/linux-omap@vger.kernel.org/msg80678.html
[2]. http://www.mail-archive.com/linux-omap@vger.kernel.org/msg79795.html

Ricardo Neri (2):
  ARM: OMAP2+: HDMI: Relocate audio platform device creation
  ARM: OMAP4+: HDMI: Rearrange platform devices for ASoC drivers

 arch/arm/mach-omap2/board-4430sdp.c    |    6 +++
 arch/arm/mach-omap2/board-omap4panda.c |    6 +++
 arch/arm/mach-omap2/devices.c          |   31 ----------------
 drivers/video/omap2/dss/hdmi.c         |   62 ++++++++++++++++++++++++++++++++
 sound/soc/omap/omap-hdmi-card.c        |    4 +-
 sound/soc/omap/omap-hdmi.c             |    5 +--
 sound/soc/omap/omap-hdmi.h             |    2 -
 7 files changed, 78 insertions(+), 38 deletions(-)

-- 
1.7.5.4

[PATCH v2 1/2] ARM: OMAP2+: HDMI: Relocate audio platform device creation

[Ricardo Neri]

Creating the accessory devices (such as audio) from the HDMI driver,
allows to regard HDMI as a single entity with audio an display
functionality. This intends to follow the design of drivers such
as MFD-type, in which a single entity handles the creation of the accessory
devices. Such devices are then used by domain-specific drivers (audio in
this case). This is in line with the DT implementation of HDMI, in which
we will have a single node to describe this feature of the OMAP SoC. Otherwise,
we would need to have separate nodes for audio and video functionality.

Previously, the platform device for the audio driver was created in
arch/arm/mach-omap2/devices.c. Thus, this is removed.

Also, as the platform device for audio created by the OMAPDSS HDMI now provides
a resource for the DMA port for audio samples, we do not need to specify
any offset in the ASoC HDMI CPU DAI driver.

Signed-off-by: Ricardo Neri <ricardo.neri@ti.com>
---
 arch/arm/mach-omap2/devices.c  |   14 ---------
 drivers/video/omap2/dss/hdmi.c |   62 ++++++++++++++++++++++++++++++++++++++++
 sound/soc/omap/omap-hdmi.c     |    3 +-
 sound/soc/omap/omap-hdmi.h     |    2 -
 4 files changed, 63 insertions(+), 18 deletions(-)

diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c
index c8c2117..417a87d 100644
--- a/arch/arm/mach-omap2/devices.c
+++ b/arch/arm/mach-omap2/devices.c
@@ -362,20 +362,6 @@ static struct platform_device omap_hdmi_audio = {
 
 static void __init omap_init_hdmi_audio(void)
 {
-	struct omap_hwmod *oh;
-	struct platform_device *pdev;
-
-	oh = omap_hwmod_lookup("dss_hdmi");
-	if (!oh) {
-		printk(KERN_ERR "Could not look up dss_hdmi hw_mod\n");
-		return;
-	}
-
-	pdev = omap_device_build("omap-hdmi-audio-dai",
-		-1, oh, NULL, 0, NULL, 0, 0);
-	WARN(IS_ERR(pdev),
-	     "Can't build omap_device for omap-hdmi-audio-dai.\n");
-
 	platform_device_register(&omap_hdmi_audio);
 }
 #else
diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c
index 24a2eef..6d48026 100644
--- a/drivers/video/omap2/dss/hdmi.c
+++ b/drivers/video/omap2/dss/hdmi.c
@@ -60,6 +60,9 @@
 static struct {
 	struct mutex lock;
 	struct platform_device *pdev;
+#if defined(CONFIG_OMAP4_DSS_HDMI_AUDIO)
+	struct platform_device *audio_pdev;
+#endif
 
 	struct hdmi_ip_data ip_data;
 
@@ -822,6 +825,54 @@ static void hdmi_put_clocks(void)
 }
 
 #if defined(CONFIG_OMAP4_DSS_HDMI_AUDIO)
+static int hdmi_probe_audio(struct platform_device *pdev)
+{
+	struct resource *res;
+	struct platform_device *aud_pdev;
+	u32 port_offset, port_size;
+	struct resource aud_res[2] = {
+		DEFINE_RES_MEM(-1, -1),
+		DEFINE_RES_DMA(-1),
+	};
+
+	res = platform_get_resource(hdmi.pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		DSSERR("can't get IORESOURCE_MEM HDMI\n");
+		return -EINVAL;
+	}
+
+	/*
+	 * Pass DMA audio port to audio drivers.
+	 * Audio drivers should not ioremap it.
+	 */
+	hdmi.ip_data.ops->audio_get_dma_port(&port_offset, &port_size);
+
+	aud_res[0].start = res->start + port_offset;
+	aud_res[0].end = aud_res[0].start + port_size - 1;
+
+	res = platform_get_resource(hdmi.pdev, IORESOURCE_DMA, 0);
+	if (!res) {
+		DSSERR("can't get IORESOURCE_DMA HDMI\n");
+		return -EINVAL;
+	}
+
+	/* Pass the audio DMA request resource to audio drivers. */
+	aud_res[1].start = res->start;
+
+	/* create platform device for HDMI audio driver */
+	aud_pdev = platform_device_register_simple("omap-hdmi-audio-dai",
+						   pdev->id, aud_res,
+						   ARRAY_SIZE(aud_res));
+	if (IS_ERR(aud_pdev)) {
+		DSSERR("Can't instantiate hdmi-audio\n");
+		return -ENODEV;
+	}
+
+	hdmi.audio_pdev = aud_pdev;
+
+	return 0;
+}
+
 int hdmi_compute_acr(u32 sample_freq, u32 *n, u32 *cts)
 {
 	u32 deep_color;
@@ -1102,6 +1153,12 @@ static int __init omapdss_hdmihw_probe(struct platform_device *pdev)
 
 	hdmi_probe_pdata(pdev);
 
+#if defined(CONFIG_OMAP4_DSS_HDMI_AUDIO)
+	r = hdmi_probe_audio(pdev);
+	if (r)
+		DSSWARN("could not create platform device for audio");
+#endif
+
 	return 0;
 
 err_panel_init:
@@ -1118,6 +1175,11 @@ static int __exit hdmi_remove_child(struct device *dev, void *data)
 
 static int __exit omapdss_hdmihw_remove(struct platform_device *pdev)
 {
+#if defined(CONFIG_OMAP4_DSS_HDMI_AUDIO)
+	if (hdmi.audio_pdev != NULL)
+		platform_device_unregister(hdmi.audio_pdev);
+#endif
+
 	device_for_each_child(&pdev->dev, NULL, hdmi_remove_child);
 
 	dss_unregister_child_devices(&pdev->dev);
diff --git a/sound/soc/omap/omap-hdmi.c b/sound/soc/omap/omap-hdmi.c
index f59c69f..8034cf7 100644
--- a/sound/soc/omap/omap-hdmi.c
+++ b/sound/soc/omap/omap-hdmi.c
@@ -281,8 +281,7 @@ static __devinit int omap_hdmi_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
-	hdmi_data->dma_params.port_addr =  hdmi_rsrc->start
-		+ OMAP_HDMI_AUDIO_DMA_PORT;
+	hdmi_data->dma_params.port_addr =  hdmi_rsrc->start;
 
 	hdmi_rsrc = platform_get_resource(pdev, IORESOURCE_DMA, 0);
 	if (!hdmi_rsrc) {
diff --git a/sound/soc/omap/omap-hdmi.h b/sound/soc/omap/omap-hdmi.h
index 6ad2bf4..33d7a93 100644
--- a/sound/soc/omap/omap-hdmi.h
+++ b/sound/soc/omap/omap-hdmi.h
@@ -25,8 +25,6 @@
 #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 | \
 				SNDRV_PCM_RATE_88200 | SNDRV_PCM_RATE_96000 | \
-- 
1.7.5.4

Re: [PATCH v2 1/2] ARM: OMAP2+: HDMI: Relocate audio platform device creation

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 325 bytes --]

On Thu, Nov 15, 2012 at 07:36:58PM -0600, Ricardo Neri wrote:
> Creating the accessory devices (such as audio) from the HDMI driver,
> allows to regard HDMI as a single entity with audio an display
> functionality. This intends to follow the design of drivers such

Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v2 1/2] ARM: OMAP2+: HDMI: Relocate audio platform device creation

[Tomi Valkeinen]

[-- Attachment #1: Type: text/plain, Size: 1439 bytes --]

Hi,

On 2012-11-16 03:36, Ricardo Neri wrote:
> Creating the accessory devices (such as audio) from the HDMI driver,
> allows to regard HDMI as a single entity with audio an display
> functionality. This intends to follow the design of drivers such
> as MFD-type, in which a single entity handles the creation of the accessory
> devices. Such devices are then used by domain-specific drivers (audio in
> this case). This is in line with the DT implementation of HDMI, in which
> we will have a single node to describe this feature of the OMAP SoC. Otherwise,
> we would need to have separate nodes for audio and video functionality.
> 
> Previously, the platform device for the audio driver was created in
> arch/arm/mach-omap2/devices.c. Thus, this is removed.
> 
> Also, as the platform device for audio created by the OMAPDSS HDMI now provides
> a resource for the DMA port for audio samples, we do not need to specify
> any offset in the ASoC HDMI CPU DAI driver.

If you notice yourself writing "also, the patch does this" in the patch
description, it's usually a sign that the patch needs to be split =).

That's perhaps not so important when a patch only deals with one
subsystem or one file, but when the patch changes arch, video and audio
drivers at the same time I would like to have the patches as simple as
possible.

Here I suggest you handle the DMA port change in a separate patch.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 897 bytes --]

Re: [PATCH v2 1/2] ARM: OMAP2+: HDMI: Relocate audio platform device creation

[Ricardo Neri]

Hi Tomi,

On 11/16/2012 01:38 AM, Tomi Valkeinen wrote:
> Hi,
>
> On 2012-11-16 03:36, Ricardo Neri wrote:
>> Creating the accessory devices (such as audio) from the HDMI driver,
>> allows to regard HDMI as a single entity with audio an display
>> functionality. This intends to follow the design of drivers such
>> as MFD-type, in which a single entity handles the creation of the accessory
>> devices. Such devices are then used by domain-specific drivers (audio in
>> this case). This is in line with the DT implementation of HDMI, in which
>> we will have a single node to describe this feature of the OMAP SoC. Otherwise,
>> we would need to have separate nodes for audio and video functionality.
>>
>> Previously, the platform device for the audio driver was created in
>> arch/arm/mach-omap2/devices.c. Thus, this is removed.
>>
>> Also, as the platform device for audio created by the OMAPDSS HDMI now provides
>> a resource for the DMA port for audio samples, we do not need to specify
>> any offset in the ASoC HDMI CPU DAI driver.
>
> If you notice yourself writing "also, the patch does this" in the patch
> description, it's usually a sign that the patch needs to be split =).
:)

>
> That's perhaps not so important when a patch only deals with one
> subsystem or one file, but when the patch changes arch, video and audio
> drivers at the same time I would like to have the patches as simple as
> possible.
>
> Here I suggest you handle the DMA port change in a separate patch.

I went ahead and submitted this part in the same patch to make sure that 
HDMI audio works in every patch.

What I can do is that a first patch creates the platform device for HDMI 
and just passes the whole register space to the audio platform device to 
not break ASoC HDMI. Then, a second patch will refine the pdev audio 
resources and remove the offset from the ASoC HDMI driver. Does that 
make sense to you?

BR,

Ricardo
>
>   Tomi
>
>

[PATCH v2 2/2] ARM: OMAP4+: HDMI: Rearrange platform devices for ASoC drivers

[Ricardo Neri]

This relocates and renames the platform devices for ASoC HDMI drivers
to give them a more logical structure.

The previous omap-hdmi-audio device is renamed as omap-hdmi-audio-card
and is relocated to the SDP4430 and Pandaboard board files. This is to
better illustrate the fact that it describes the whole HDMI audio
functionality on such boards, including the companion chip.

The previous omap-hdmi-audio-dai is renamed as omap-hdmi-audio. The -dai
part is removed to not have references to ASoC concepts in the OMAPDSS
HDMI driver. Also, as it will be used by the ASoC HDMI CPU DAI driver,
the name refers only to OMAP HDMI audio functionality, irrespective of the
board.

The names of the ASoC drivers are also updated accordingly.

Signed-off-by: Ricardo Neri <ricardo.neri@ti.com>
---
 arch/arm/mach-omap2/board-4430sdp.c    |    6 ++++++
 arch/arm/mach-omap2/board-omap4panda.c |    6 ++++++
 arch/arm/mach-omap2/devices.c          |   17 -----------------
 drivers/video/omap2/dss/hdmi.c         |    2 +-
 sound/soc/omap/omap-hdmi-card.c        |    4 ++--
 sound/soc/omap/omap-hdmi.c             |    2 +-
 6 files changed, 16 insertions(+), 21 deletions(-)

diff --git a/arch/arm/mach-omap2/board-4430sdp.c b/arch/arm/mach-omap2/board-4430sdp.c
index 3669c12..97bdff3 100644
--- a/arch/arm/mach-omap2/board-4430sdp.c
+++ b/arch/arm/mach-omap2/board-4430sdp.c
@@ -388,6 +388,11 @@ static struct platform_device sdp4430_hdmi_audio_codec = {
 	.id	= -1,
 };
 
+static struct platform_device sdp4430_hdmi_audio_card = {
+	.name	= "omap-hdmi-audio-card",
+	.id	= -1,
+};
+
 static struct omap_abe_twl6040_data sdp4430_abe_audio_data = {
 	.card_name = "SDP4430",
 	.has_hs		= ABE_TWL6040_LEFT | ABE_TWL6040_RIGHT,
@@ -423,6 +428,7 @@ static struct platform_device *sdp4430_devices[] __initdata = {
 	&sdp4430_dmic_codec,
 	&sdp4430_abe_audio,
 	&sdp4430_hdmi_audio_codec,
+	&sdp4430_hdmi_audio_card,
 };
 
 static struct omap_musb_board_data musb_board_data = {
diff --git a/arch/arm/mach-omap2/board-omap4panda.c b/arch/arm/mach-omap2/board-omap4panda.c
index bfcd397..e03eae1 100644
--- a/arch/arm/mach-omap2/board-omap4panda.c
+++ b/arch/arm/mach-omap2/board-omap4panda.c
@@ -131,6 +131,11 @@ static struct platform_device panda_hdmi_audio_codec = {
 	.id	= -1,
 };
 
+static struct platform_device panda_hdmi_audio_card = {
+	.name	= "omap-hdmi-audio-card",
+	.id	= -1,
+};
+
 static struct platform_device btwilink_device = {
 	.name	= "btwilink",
 	.id	= -1,
@@ -141,6 +146,7 @@ static struct platform_device *panda_devices[] __initdata = {
 	&wl1271_device,
 	&panda_abe_audio,
 	&panda_hdmi_audio_codec,
+	&panda_hdmi_audio_card,
 	&btwilink_device,
 };
 
diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c
index 417a87d..bea0b40 100644
--- a/arch/arm/mach-omap2/devices.c
+++ b/arch/arm/mach-omap2/devices.c
@@ -352,22 +352,6 @@ static void __init omap_init_dmic(void)
 static inline void omap_init_dmic(void) {}
 #endif
 
-#if defined(CONFIG_SND_OMAP_SOC_OMAP_HDMI) || \
-		defined(CONFIG_SND_OMAP_SOC_OMAP_HDMI_MODULE)
-
-static struct platform_device omap_hdmi_audio = {
-	.name	= "omap-hdmi-audio",
-	.id	= -1,
-};
-
-static void __init omap_init_hdmi_audio(void)
-{
-	platform_device_register(&omap_hdmi_audio);
-}
-#else
-static inline void omap_init_hdmi_audio(void) {}
-#endif
-
 #if defined(CONFIG_SPI_OMAP24XX) || defined(CONFIG_SPI_OMAP24XX_MODULE)
 
 #include <linux/platform_data/spi-omap2-mcspi.h>
@@ -613,7 +597,6 @@ static int __init omap2_init_devices(void)
 	 */
 	omap_init_audio();
 	omap_init_camera();
-	omap_init_hdmi_audio();
 	omap_init_mbox();
 	/* If dtb is there, the devices will be created dynamically */
 	if (!of_have_populated_dt()) {
diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c
index 6d48026..c5743e1 100644
--- a/drivers/video/omap2/dss/hdmi.c
+++ b/drivers/video/omap2/dss/hdmi.c
@@ -860,7 +860,7 @@ static int hdmi_probe_audio(struct platform_device *pdev)
 	aud_res[1].start = res->start;
 
 	/* create platform device for HDMI audio driver */
-	aud_pdev = platform_device_register_simple("omap-hdmi-audio-dai",
+	aud_pdev = platform_device_register_simple("omap-hdmi-audio",
 						   pdev->id, aud_res,
 						   ARRAY_SIZE(aud_res));
 	if (IS_ERR(aud_pdev)) {
diff --git a/sound/soc/omap/omap-hdmi-card.c b/sound/soc/omap/omap-hdmi-card.c
index eaa2ea0..07b9959 100644
--- a/sound/soc/omap/omap-hdmi-card.c
+++ b/sound/soc/omap/omap-hdmi-card.c
@@ -27,12 +27,12 @@
 #include <asm/mach-types.h>
 #include <video/omapdss.h>
 
-#define DRV_NAME "omap-hdmi-audio"
+#define DRV_NAME "omap-hdmi-audio-card"
 
 static struct snd_soc_dai_link omap_hdmi_dai = {
 	.name = "HDMI",
 	.stream_name = "HDMI",
-	.cpu_dai_name = "omap-hdmi-audio-dai",
+	.cpu_dai_name = "omap-hdmi-audio",
 	.platform_name = "omap-pcm-audio",
 	.codec_name = "hdmi-audio-codec",
 	.codec_dai_name = "omap-hdmi-hifi",
diff --git a/sound/soc/omap/omap-hdmi.c b/sound/soc/omap/omap-hdmi.c
index 8034cf7..33418fc 100644
--- a/sound/soc/omap/omap-hdmi.c
+++ b/sound/soc/omap/omap-hdmi.c
@@ -37,7 +37,7 @@
 #include "omap-pcm.h"
 #include "omap-hdmi.h"
 
-#define DRV_NAME "omap-hdmi-audio-dai"
+#define DRV_NAME "omap-hdmi-audio"
 
 struct hdmi_priv {
 	struct omap_pcm_dma_data dma_params;
-- 
1.7.5.4

Re: [PATCH v2 2/2] ARM: OMAP4+: HDMI: Rearrange platform devices for ASoC drivers

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 235 bytes --]

On Thu, Nov 15, 2012 at 07:36:59PM -0600, Ricardo Neri wrote:
> This relocates and renames the platform devices for ASoC HDMI drivers
> to give them a more logical structure.

Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v2 2/2] ARM: OMAP4+: HDMI: Rearrange platform devices for ASoC drivers

[Tomi Valkeinen]

[-- Attachment #1.1: Type: text/plain, Size: 3466 bytes --]

On 2012-11-16 03:36, Ricardo Neri wrote:
> This relocates and renames the platform devices for ASoC HDMI drivers
> to give them a more logical structure.
> 
> The previous omap-hdmi-audio device is renamed as omap-hdmi-audio-card
> and is relocated to the SDP4430 and Pandaboard board files. This is to
> better illustrate the fact that it describes the whole HDMI audio
> functionality on such boards, including the companion chip.
> 
> The previous omap-hdmi-audio-dai is renamed as omap-hdmi-audio. The -dai
> part is removed to not have references to ASoC concepts in the OMAPDSS
> HDMI driver. Also, as it will be used by the ASoC HDMI CPU DAI driver,
> the name refers only to OMAP HDMI audio functionality, irrespective of the
> board.
> 
> The names of the ASoC drivers are also updated accordingly.

And same thing here as with the previous patch. Do the move and rename
in separate patches for clarity.

> Signed-off-by: Ricardo Neri <ricardo.neri@ti.com>
> ---
>  arch/arm/mach-omap2/board-4430sdp.c    |    6 ++++++
>  arch/arm/mach-omap2/board-omap4panda.c |    6 ++++++
>  arch/arm/mach-omap2/devices.c          |   17 -----------------
>  drivers/video/omap2/dss/hdmi.c         |    2 +-
>  sound/soc/omap/omap-hdmi-card.c        |    4 ++--
>  sound/soc/omap/omap-hdmi.c             |    2 +-
>  6 files changed, 16 insertions(+), 21 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/board-4430sdp.c b/arch/arm/mach-omap2/board-4430sdp.c
> index 3669c12..97bdff3 100644
> --- a/arch/arm/mach-omap2/board-4430sdp.c
> +++ b/arch/arm/mach-omap2/board-4430sdp.c
> @@ -388,6 +388,11 @@ static struct platform_device sdp4430_hdmi_audio_codec = {
>  	.id	= -1,
>  };
>  
> +static struct platform_device sdp4430_hdmi_audio_card = {
> +	.name	= "omap-hdmi-audio-card",
> +	.id	= -1,
> +};
> +
>  static struct omap_abe_twl6040_data sdp4430_abe_audio_data = {
>  	.card_name = "SDP4430",
>  	.has_hs		= ABE_TWL6040_LEFT | ABE_TWL6040_RIGHT,
> @@ -423,6 +428,7 @@ static struct platform_device *sdp4430_devices[] __initdata = {
>  	&sdp4430_dmic_codec,
>  	&sdp4430_abe_audio,
>  	&sdp4430_hdmi_audio_codec,
> +	&sdp4430_hdmi_audio_card,
>  };

I don't know anything at all about the audio drivers, but this doesn't
feel good to me. The HDMI audio is tied to the HDMI video, both of which
are parts of OMAP SoC. So if you have two boards with HDMI video (and
thus audio), the device data related to HDMI video and audio are
identical except for a few HW details like the GPIOs for the TPD chip.

So is there any reason to add hdmi audio devices in each board file? It
sounds to me that a common place to add the device for all boards would
make more sense. This could, perhaps, be arch/arm/mach-omap2/display.c
which handles adding the HDMI device, or some other similar file
(although you just removed it from such a file, the devices.c...).

And actually, why isn't the card driver added in the hdmi video driver,
like the omap-hdmi-audio-dai?

You say the omap-hdmi-audio-card covers also the TPD chip, but why does
HDMI audio even need to cover that chip? It has no relevance to the
audio side, as long as the video driver enables it properly, right?

Perhaps I'm missing something here, as I don't have any knowledge of the
audio side, though. What do the different audio devices represent?

So I'm not saying your approach is wrong, I just don't understand it =).

 Tomi



[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 897 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH v2 2/2] ARM: OMAP4+: HDMI: Rearrange platform devices for ASoC drivers

[Ricardo Neri]

On 11/16/2012 01:52 AM, Tomi Valkeinen wrote:
> On 2012-11-16 03:36, Ricardo Neri wrote:
>> This relocates and renames the platform devices for ASoC HDMI drivers
>> to give them a more logical structure.
>>
>> The previous omap-hdmi-audio device is renamed as omap-hdmi-audio-card
>> and is relocated to the SDP4430 and Pandaboard board files. This is to
>> better illustrate the fact that it describes the whole HDMI audio
>> functionality on such boards, including the companion chip.
>>
>> The previous omap-hdmi-audio-dai is renamed as omap-hdmi-audio. The -dai
>> part is removed to not have references to ASoC concepts in the OMAPDSS
>> HDMI driver. Also, as it will be used by the ASoC HDMI CPU DAI driver,
>> the name refers only to OMAP HDMI audio functionality, irrespective of the
>> board.
>>
>> The names of the ASoC drivers are also updated accordingly.
>
> And same thing here as with the previous patch. Do the move and rename
> in separate patches for clarity.

OK. I'll do.
>
>> Signed-off-by: Ricardo Neri <ricardo.neri@ti.com>
>> ---
>>   arch/arm/mach-omap2/board-4430sdp.c    |    6 ++++++
>>   arch/arm/mach-omap2/board-omap4panda.c |    6 ++++++
>>   arch/arm/mach-omap2/devices.c          |   17 -----------------
>>   drivers/video/omap2/dss/hdmi.c         |    2 +-
>>   sound/soc/omap/omap-hdmi-card.c        |    4 ++--
>>   sound/soc/omap/omap-hdmi.c             |    2 +-
>>   6 files changed, 16 insertions(+), 21 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/board-4430sdp.c b/arch/arm/mach-omap2/board-4430sdp.c
>> index 3669c12..97bdff3 100644
>> --- a/arch/arm/mach-omap2/board-4430sdp.c
>> +++ b/arch/arm/mach-omap2/board-4430sdp.c
>> @@ -388,6 +388,11 @@ static struct platform_device sdp4430_hdmi_audio_codec = {
>>   	.id	= -1,
>>   };
>>
>> +static struct platform_device sdp4430_hdmi_audio_card = {
>> +	.name	= "omap-hdmi-audio-card",
>> +	.id	= -1,
>> +};
>> +
>>   static struct omap_abe_twl6040_data sdp4430_abe_audio_data = {
>>   	.card_name = "SDP4430",
>>   	.has_hs		= ABE_TWL6040_LEFT | ABE_TWL6040_RIGHT,
>> @@ -423,6 +428,7 @@ static struct platform_device *sdp4430_devices[] __initdata = {
>>   	&sdp4430_dmic_codec,
>>   	&sdp4430_abe_audio,
>>   	&sdp4430_hdmi_audio_codec,
>> +	&sdp4430_hdmi_audio_card,
>>   };
>
> I don't know anything at all about the audio drivers, but this doesn't
> feel good to me. The HDMI audio is tied to the HDMI video, both of which
> are parts of OMAP SoC. So if you have two boards with HDMI video (and
> thus audio), the device data related to HDMI video and audio are
> identical except for a few HW details like the GPIOs for the TPD chip.
>
> So is there any reason to add hdmi audio devices in each board file? It
> sounds to me that a common place to add the device for all boards would
> make more sense. This could, perhaps, be arch/arm/mach-omap2/display.c
> which handles adding the HDMI device, or some other similar file
> (although you just removed it from such a file, the devices.c...).

In ASoC, we have three drivers for ASoC HDMI audio. The CPU-DAI driver 
deals with the CPU audio interface. So, I regard the OMAP HDMI IP as the 
CPU DAI. A device is needed to probe the driver, but as HDMI audio and 
video are the same physical component, it made sense to have the HDMI 
video driver to create it. Furthermore, except for the TPD handling the 
HDMI driver deals only with OMAP stuff. Also, we will have a single node 
for HDMI when DT comes. Thus, the device for the ASoC CPU DAI has to be 
created somewhere.

We also have codec. ASoC codecs are chips like TWL6040 that 
render/capture audio. For ASoC HDMI, a TV or a home theater unit could 
be regarded as the codec. Strictly speaking, it is not a device mounted 
on the board such as TWL6040 but does the same work and we have to 
represent it for ASoC to use.

Finally we have the ASoC machine (or board) driver, that glues together 
the DAI and codec.
>
> And actually, why isn't the card driver added in the hdmi video driver,
> like the omap-hdmi-audio-dai?

The card driver represents a board. It made sense to me to relocate it 
into the board files. Furthermore, when HDMI DT is supported in the 
feature, the node for this machine driver will be in the DT; so, we will 
not needed and we would end up relocating it anyways.
>
> You say the omap-hdmi-audio-card covers also the TPD chip, but why does
> HDMI audio even need to cover that chip? It has no relevance to the
> audio side, as long as the video driver enables it properly, right?

Yes, audio does not have anything to do with the TPD chip, but we do 
need an ASoC machine driver. Thus, the only components that are there to 
describe an ASoC machine driver are the OMAP, TPD and the HDMI 
connector. Indeed, to not tie the ASoC machine driver to a specific 
companion chip (as commented by Liam), I just used the -card suffix.
>
> Perhaps I'm missing something here, as I don't have any knowledge of the
> audio side, though. What do the different audio devices represent?

I hope the explanation above provides more clarity to you. I think HDMI 
does not fit seamlessly into the ASoC driver model, as we don't have a 
real codec and no machine driver seems to be needed. This is the best I 
could get. :/ :)


>
> So I'm not saying your approach is wrong, I just don't understand it =).

:)

BR,

Ricardo
>
>   Tomi
>
>

Re: [PATCH v2 2/2] ARM: OMAP4+: HDMI: Rearrange platform devices for ASoC drivers

[Tomi Valkeinen]

[-- Attachment #1: Type: text/plain, Size: 1686 bytes --]

On 2012-11-16 20:05, Ricardo Neri wrote:

> I hope the explanation above provides more clarity to you. I think HDMI
> does not fit seamlessly into the ASoC driver model, as we don't have a
> real codec and no machine driver seems to be needed. This is the best I
> could get. :/ :)

Ok. I can't say I fully grasp everything about the audio architecture,
but this clarified it anyway.

So I'm fine with the three audio related devices. But I still have the
following points:

The name of the codec platform_device is "hdmi-audio-codec". Isn't that
too generic name? Shouldn't it be "omap-hdmi-audio-codec"? Then again,
you said in this case it represents the tv-set. If so, should it be a
generic codec driver, without any reference to omap?

I still don't understand why the codec and machine drivers need to be
created in the board file. That just forces us to replicate the same
code for all OMAP boards that have OMAP HDMI output. Why not create the
devices in some common code, for example arch/arm/mach-omap2/display.c?

With DT this should be similar: OMAP's hdmi devices should be presented
in the omap4.dtsi file, not in each individual board dts. Although the
DT data should represent the hardware, and if the code and machine
devices are not really there in the HW, then... I don't know =).

And something that confuses me: sound/soc/codecs/omap-hdmi.c contains a
codec and dai drivers, but sound/soc/omap/omap-hdmi.c also contains a
dai driver. The latter actually contains two dai drivers, the other a
platform driver and the other a snd_soc_dai_driver. But I guess this is
asoc details, and not relevant to this disuccsion =).

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 897 bytes --]

Re: [PATCH v2 2/2] ARM: OMAP4+: HDMI: Rearrange platform devices for ASoC drivers

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1516 bytes --]

On Mon, Nov 19, 2012 at 02:58:41PM +0200, Tomi Valkeinen wrote:

> I still don't understand why the codec and machine drivers need to be
> created in the board file. That just forces us to replicate the same
> code for all OMAP boards that have OMAP HDMI output. Why not create the
> devices in some common code, for example arch/arm/mach-omap2/display.c?

Yes, this would be more sensible if there's no board specifics involved.

> With DT this should be similar: OMAP's hdmi devices should be presented
> in the omap4.dtsi file, not in each individual board dts. Although the
> DT data should represent the hardware, and if the code and machine
> devices are not really there in the HW, then... I don't know =).

Well, in a case like this where the sound card is essentially autoprobed
based on the detection of the hardware at runtime the sound card
probably shouldn't appear in the device tree at all - you'll probably
want something to say there's a physical HDMI port it's worth looking at
there but everything else should be figured out at runtime.

> And something that confuses me: sound/soc/codecs/omap-hdmi.c contains a
> codec and dai drivers, but sound/soc/omap/omap-hdmi.c also contains a
> dai driver. The latter actually contains two dai drivers, the other a
> platform driver and the other a snd_soc_dai_driver. But I guess this is
> asoc details, and not relevant to this disuccsion =).

There's an interaface on each end of the link, they're wired together to
communicate between the two devices.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v2 2/2] ARM: OMAP4+: HDMI: Rearrange platform devices for ASoC drivers

[Ricardo Neri]

Hi Tomi, Mark,

On 11/19/2012 07:15 PM, Mark Brown wrote:
> On Mon, Nov 19, 2012 at 02:58:41PM +0200, Tomi Valkeinen wrote:
>
>> I still don't understand why the codec and machine drivers need to be
>> created in the board file. That just forces us to replicate the same
>> code for all OMAP boards that have OMAP HDMI output. Why not create the
>> devices in some common code, for example arch/arm/mach-omap2/display.c?
>
> Yes, this would be more sensible if there's no board specifics involved.

I think they are truly board-specific. For instance, there could be some 
board that does not have the OMAP HDMI IP wired to an external 
connector. We don't want the drivers to be probed in that case. If they 
are in common code, the devices will be created even if a board does not 
have HDMI output.
>
>> With DT this should be similar: OMAP's hdmi devices should be presented
>> in the omap4.dtsi file, not in each individual board dts. Although the
>> DT data should represent the hardware, and if the code and machine
>> devices are not really there in the HW, then... I don't know =).
>
> Well, in a case like this where the sound card is essentially autoprobed
> based on the detection of the hardware at runtime the sound card
> probably shouldn't appear in the device tree at all - you'll probably
> want something to say there's a physical HDMI port it's worth looking at
> there but everything else should be figured out at runtime.

Yes, I was planning to rely on the DSS DT entries in the omap4.dtsi 
file. However, no HDMI audio support should be probed if the board does 
not have an HDMI connector. Also, the TPD chip should appear on the 
Pandaboard/SDP4430 dts files. Only if both conditions are met, probe the 
HDMI audio drivers, this conditions will be checked at run time by the 
ASoC HDMI machine driver.

Something like this:

	sound_hdmi {
		compatible = "ti,omap-hdmi-card-audio";
		ti,model = "OMAP4HDMI";

		ti,hdmi_audio = <&hdmi>;
		ti,level_shifter = <&tpd12s015>;
	};

The ASoC machine driver would create the platform device for the HDMI 
codec if the DT has the required nodes.
>
>> And something that confuses me: sound/soc/codecs/omap-hdmi.c contains a
>> codec and dai drivers, but sound/soc/omap/omap-hdmi.c also contains a
>> dai driver. The latter actually contains two dai drivers, the other a
>> platform driver and the other a snd_soc_dai_driver. But I guess this is
>> asoc details, and not relevant to this disuccsion =).
>
> There's an interaface on each end of the link, they're wired together to
> communicate between the two devices.

BR,

Ricardo
>

Re: [PATCH v2 2/2] ARM: OMAP4+: HDMI: Rearrange platform devices for ASoC drivers

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1169 bytes --]

On Wed, Nov 21, 2012 at 06:20:00PM -0600, Ricardo Neri wrote:
> On 11/19/2012 07:15 PM, Mark Brown wrote:

> >Yes, this would be more sensible if there's no board specifics involved.

> I think they are truly board-specific. For instance, there could be
> some board that does not have the OMAP HDMI IP wired to an external
> connector. We don't want the drivers to be probed in that case. If
> they are in common code, the devices will be created even if a board
> does not have HDMI output.

That's just a case of having a flag in the platform data for the device
saying "don't use this port" as opposed to having the entire ASoC device
instantiation infrastructure in there which is rather Linux specific.

> Something like this:

> 	sound_hdmi {
> 		compatible = "ti,omap-hdmi-card-audio";
> 		ti,model = "OMAP4HDMI";

> 		ti,hdmi_audio = <&hdmi>;
> 		ti,level_shifter = <&tpd12s015>;
> 	};

> The ASoC machine driver would create the platform device for the
> HDMI codec if the DT has the required nodes.

Why not just make this a property of the main HDMI controller - the
compatible property here looks like it's describing the Linux specifics
not the hardware?

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH v2 2/2] ARM: OMAP4+: HDMI: Rearrange platform devices for ASoC drivers

[Ricardo Neri]

Hi Mark,

On 11/21/2012 07:03 PM, Mark Brown wrote:
> On Wed, Nov 21, 2012 at 06:20:00PM -0600, Ricardo Neri wrote:
>> On 11/19/2012 07:15 PM, Mark Brown wrote:
>
>>> Yes, this would be more sensible if there's no board specifics involved.
>
>> I think they are truly board-specific. For instance, there could be
>> some board that does not have the OMAP HDMI IP wired to an external
>> connector. We don't want the drivers to be probed in that case. If
>> they are in common code, the devices will be created even if a board
>> does not have HDMI output.
>
> That's just a case of having a flag in the platform data for the device
> saying "don't use this port"
Ok. I guess I can put code so that the devices for ASoC are created only 
if there are HDMI displays in the omapdss_board_info.devices array.

 > as opposed to having the entire ASoC device

> instantiation infrastructure in there which is rather Linux specific.
But the board files are only for Linux, right? The ASoC drivers will 
always be initialized anyways. They will reach probe only if the devices 
are present.

>
>> Something like this:
>
>> 	sound_hdmi {
>> 		compatible = "ti,omap-hdmi-card-audio";
>> 		ti,model = "OMAP4HDMI";
>
>> 		ti,hdmi_audio = <&hdmi>;
>> 		ti,level_shifter = <&tpd12s015>;
>> 	};
>
>> The ASoC machine driver would create the platform device for the
>> HDMI codec if the DT has the required nodes.
>
> Why not just make this a property of the main HDMI controller - the
> compatible property here looks like it's describing the Linux specifics
> not the hardware?

I see. So it seems that there will be nothing to add for DT support for 
HDMI from ASoC. Display code is able to take care of it. BTW, thanks for 
pointing out the issue with the compatible property, I have not noticed it.

BR,

Ricardo
>

Re: [PATCH v2 2/2] ARM: OMAP4+: HDMI: Rearrange platform devices for ASoC drivers

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 378 bytes --]

On Thu, Nov 22, 2012 at 08:03:53PM -0600, Ricardo Neri wrote:
> On 11/21/2012 07:03 PM, Mark Brown wrote:

> >instantiation infrastructure in there which is rather Linux specific.

> But the board files are only for Linux, right? The ASoC drivers will
> always be initialized anyways. They will reach probe only if the
> devices are present.

Could you be more specific please?

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v2 2/2] ARM: OMAP4+: HDMI: Rearrange platform devices for ASoC drivers

[Ricardo Neri]

On 11/22/2012 08:12 PM, Mark Brown wrote:
> On Thu, Nov 22, 2012 at 08:03:53PM -0600, Ricardo Neri wrote:
>> On 11/21/2012 07:03 PM, Mark Brown wrote:
>
>>> instantiation infrastructure in there which is rather Linux specific.
>
>> But the board files are only for Linux, right? The ASoC drivers will
>> always be initialized anyways. They will reach probe only if the
>> devices are present.
>
> Could you be more specific please?
>

Sorry, I meant that the drivers will try to register anyways. The will 
only probe if there is a matching device for them.

Re: [PATCH v2 2/2] ARM: OMAP4+: HDMI: Rearrange platform devices for ASoC drivers

[Tomi Valkeinen]

[-- Attachment #1: Type: text/plain, Size: 3836 bytes --]

On 2012-11-22 02:20, Ricardo Neri wrote:
> Hi Tomi, Mark,
> 
> On 11/19/2012 07:15 PM, Mark Brown wrote:
>> On Mon, Nov 19, 2012 at 02:58:41PM +0200, Tomi Valkeinen wrote:
>>
>>> I still don't understand why the codec and machine drivers need to be
>>> created in the board file. That just forces us to replicate the same
>>> code for all OMAP boards that have OMAP HDMI output. Why not create the
>>> devices in some common code, for example arch/arm/mach-omap2/display.c?
>>
>> Yes, this would be more sensible if there's no board specifics involved.
> 
> I think they are truly board-specific. For instance, there could be some

I don't =).

> board that does not have the OMAP HDMI IP wired to an external
> connector. We don't want the drivers to be probed in that case. If they
> are in common code, the devices will be created even if a board does not
> have HDMI output.

The HDMI devices are still there in the HW even if we don't have a HDMI
connector. I don't see any problem with probing the HDMI audio driver
even in that case.

But of course the user space shouldn't see a usable HDMI display/audio
if there's no connector. For display side this is managed so that the
HDMI IP driver is always loaded, but a HDMI panel driver is only there
if the board file tells that we have a connector.

I guess this could be optimized by having a "disabled" flag for HDMI IP
driver, so that it wouldn't even need to allocate any private data
structures of such. But the savings would be quite minimal.

>>> With DT this should be similar: OMAP's hdmi devices should be presented
>>> in the omap4.dtsi file, not in each individual board dts. Although the
>>> DT data should represent the hardware, and if the code and machine
>>> devices are not really there in the HW, then... I don't know =).
>>
>> Well, in a case like this where the sound card is essentially autoprobed
>> based on the detection of the hardware at runtime the sound card
>> probably shouldn't appear in the device tree at all - you'll probably
>> want something to say there's a physical HDMI port it's worth looking at
>> there but everything else should be figured out at runtime.
> 
> Yes, I was planning to rely on the DSS DT entries in the omap4.dtsi
> file. However, no HDMI audio support should be probed if the board does
> not have an HDMI connector. Also, the TPD chip should appear on the
> Pandaboard/SDP4430 dts files. Only if both conditions are met, probe the
> HDMI audio drivers, this conditions will be checked at run time by the
> ASoC HDMI machine driver.
> 
> Something like this:
> 
>     sound_hdmi {
>         compatible = "ti,omap-hdmi-card-audio";
>         ti,model = "OMAP4HDMI";
> 
>         ti,hdmi_audio = <&hdmi>;
>         ti,level_shifter = <&tpd12s015>;
>     };
> 
> The ASoC machine driver would create the platform device for the HDMI
> codec if the DT has the required nodes.

The TPD chip really shouldn't be here in. It's an external component,
not related to audio in any way. I think the HDMI audio driver should be
only concerned about the HDMI IP. The HDMI IP video driver shouldn't
care about TPD chip either, but for now we need to manage it somewhere,
and that's the easiest place for it.

So... I'm not sure how this should be managed, but I am 99% sure that
there's nothing board specific with HDMI audio, and thus it should be
managed in common .c files in arch code or dss code, or .dts files. If
we add the hdmi display in the .dts files, I think the audio should just
work.

Or is there something in ASoC that forces us to represent it in the
.dts? I don't think there's really anything related to HW to describe
there related to HDMI audio. If we have HDMI video output, we also have
the audio, as simple as that.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]

Re: [PATCH v2 2/2] ARM: OMAP4+: HDMI: Rearrange platform devices for ASoC drivers

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 228 bytes --]

On Thu, Nov 22, 2012 at 02:52:50PM +0200, Tomi Valkeinen wrote:

> Or is there something in ASoC that forces us to represent it in the
> .dts? I don't think there's really anything related to HW to describe

There shouldn't be.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v2 2/2] ARM: OMAP4+: HDMI: Rearrange platform devices for ASoC drivers

[Ricardo Neri]

On 11/22/2012 06:52 AM, Tomi Valkeinen wrote:
> On 2012-11-22 02:20, Ricardo Neri wrote:
>> Hi Tomi, Mark,
>>
>> On 11/19/2012 07:15 PM, Mark Brown wrote:
>>> On Mon, Nov 19, 2012 at 02:58:41PM +0200, Tomi Valkeinen wrote:
>>>
>>>> I still don't understand why the codec and machine drivers need to be
>>>> created in the board file. That just forces us to replicate the same
>>>> code for all OMAP boards that have OMAP HDMI output. Why not create the
>>>> devices in some common code, for example arch/arm/mach-omap2/display.c?
>>>
>>> Yes, this would be more sensible if there's no board specifics involved.
>>
>> I think they are truly board-specific. For instance, there could be some
>
> I don't =).
>
>> board that does not have the OMAP HDMI IP wired to an external
>> connector. We don't want the drivers to be probed in that case. If they
>> are in common code, the devices will be created even if a board does not
>> have HDMI output.
>
> The HDMI devices are still there in the HW even if we don't have a HDMI
> connector. I don't see any problem with probing the HDMI audio driver
> even in that case.
>
> But of course the user space shouldn't see a usable HDMI display/audio
> if there's no connector. For display side this is managed so that the
> HDMI IP driver is always loaded, but a HDMI panel driver is only there
> if the board file tells that we have a connector.
>
> I guess this could be optimized by having a "disabled" flag for HDMI IP
> driver, so that it wouldn't even need to allocate any private data
> structures of such. But the savings would be quite minimal.

Maybe, just create the devices for ASoC only if it sees a HDMI dss 
device in the omapdss_board_info?
>
>>>> With DT this should be similar: OMAP's hdmi devices should be presented
>>>> in the omap4.dtsi file, not in each individual board dts. Although the
>>>> DT data should represent the hardware, and if the code and machine
>>>> devices are not really there in the HW, then... I don't know =).
>>>
>>> Well, in a case like this where the sound card is essentially autoprobed
>>> based on the detection of the hardware at runtime the sound card
>>> probably shouldn't appear in the device tree at all - you'll probably
>>> want something to say there's a physical HDMI port it's worth looking at
>>> there but everything else should be figured out at runtime.
>>
>> Yes, I was planning to rely on the DSS DT entries in the omap4.dtsi
>> file. However, no HDMI audio support should be probed if the board does
>> not have an HDMI connector. Also, the TPD chip should appear on the
>> Pandaboard/SDP4430 dts files. Only if both conditions are met, probe the
>> HDMI audio drivers, this conditions will be checked at run time by the
>> ASoC HDMI machine driver.
>>
>> Something like this:
>>
>>      sound_hdmi {
>>          compatible = "ti,omap-hdmi-card-audio";
>>          ti,model = "OMAP4HDMI";
>>
>>          ti,hdmi_audio = <&hdmi>;
>>          ti,level_shifter = <&tpd12s015>;
>>      };
>>
>> The ASoC machine driver would create the platform device for the HDMI
>> codec if the DT has the required nodes.
>
> The TPD chip really shouldn't be here in. It's an external component,
> not related to audio in any way. I think the HDMI audio driver should be
> only concerned about the HDMI IP.

OK. So display code will handle all the DT details for the audio drivers 
to use.

> The HDMI IP video driver shouldn't
> care about TPD chip either, but for now we need to manage it somewhere,
> and that's the easiest place for it.
BTW, I have a draft for this, but more urgent things have been consuming 
my bandwidth. :/

BR,

Ricardo
>
> So... I'm not sure how this should be managed, but I am 99% sure that
> there's nothing board specific with HDMI audio, and thus it should be
> managed in common .c files in arch code or dss code, or .dts files. If
> we add the hdmi display in the .dts files, I think the audio should just
> work.
>
> Or is there something in ASoC that forces us to represent it in the
> .dts? I don't think there's really anything related to HW to describe
> there related to HDMI audio. If we have HDMI video output, we also have
> the audio, as simple as that.
>
>   Tomi
>
>

Re: [PATCH v2 2/2] ARM: OMAP4+: HDMI: Rearrange platform devices for ASoC drivers

[Ricardo Neri]

Hi Tomi,

Sorry for the delayed response.

On 11/19/2012 06:58 AM, Tomi Valkeinen wrote:
> On 2012-11-16 20:05, Ricardo Neri wrote:
>
>> I hope the explanation above provides more clarity to you. I think HDMI
>> does not fit seamlessly into the ASoC driver model, as we don't have a
>> real codec and no machine driver seems to be needed. This is the best I
>> could get. :/ :)
>
> Ok. I can't say I fully grasp everything about the audio architecture,
> but this clarified it anyway.
>
> So I'm fine with the three audio related devices. But I still have the
> following points:
>
> The name of the codec platform_device is "hdmi-audio-codec". Isn't that
> too generic name? Shouldn't it be "omap-hdmi-audio-codec"? Then again,
> you said in this case it represents the tv-set. If so, should it be a
> generic codec driver, without any reference to omap?

Yes, it could be a generic codec driver. However, I was envisioning that 
this component to further represent the TV-set by exposing the audio 
capabilities of the HDMI sink as represented in the EDID. For instance, 
exposing to ALSA only the sample rates supported by the sink even if the 
HDMI IP in the OMAP supports more than that. For this, I was planing to 
rely on omap_dss_get_device and omap_dss_driver.read_edid. Thus, the 
HDMI codec could not be generic unless there is a more generic support 
from the video drivers for this (framebuffer/drm maybe?).
>
> I still don't understand why the codec and machine drivers need to be
> created in the board file. That just forces us to replicate the same
> code for all OMAP boards that have OMAP HDMI output. Why not create the
> devices in some common code, for example arch/arm/mach-omap2/display.c?
>
> With DT this should be similar: OMAP's hdmi devices should be presented
> in the omap4.dtsi file, not in each individual board dts. Although the
> DT data should represent the hardware, and if the code and machine
> devices are not really there in the HW, then... I don't know =).
>
> And something that confuses me: sound/soc/codecs/omap-hdmi.c contains a
> codec and dai drivers, but sound/soc/omap/omap-hdmi.c also contains a
> dai driver. The latter actually contains two dai drivers, the other a
> platform driver and the other a snd_soc_dai_driver. But I guess this is
> asoc details, and not relevant to this disuccsion =).

As Mark, pointed out, there is an interface on each end of the link.

BR,

Ricardo
>
>   Tomi
>
>