[PATCH 0/4] OMAPDSS/ASoC: Repair broken HDMI audio in K3.2-rcX

[PATCH 0/4] OMAPDSS/ASoC: Repair broken HDMI audio in K3.2-rcX

[Ricardo Neri]

It has been detected that HDMI audio is broken in K3.2-rcX due to
the most recent updates in the DSS. This set of patches aims to repair such
break. It also improves the implementation of the ASoC codec to better follow
the DSS design by clearly separating functions that need acess to IP-
specific data.

This set of patches is based on
git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git v3.2-rc5

and is available at
git://gitorious.org/omap-audio/linux-audio.git ricardon/topic/fix-hdmi-audio-3.2

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

Ricardo Neri (4):
  ASoC: OMAP: HDMI: Introduce driver data for audio codec
  ASoC: OMAP: HDMI: Correct signature of ASoC functions
  OMAPDSS: HDMI: Create function to enable HDMI audio
  ASoC: OMAP: HDMI: Move HDMI codec trigger function to generic HDMI
    driver

 drivers/video/omap2/dss/dss_features.c    |    4 ++
 drivers/video/omap2/dss/hdmi.c            |   46 +++++++++++++++++++++++++++-
 drivers/video/omap2/dss/ti_hdmi.h         |   10 +++++-
 drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c |   37 ++++-------------------
 drivers/video/omap2/dss/ti_hdmi_4xxx_ip.h |    3 --
 5 files changed, 64 insertions(+), 36 deletions(-)

[PATCH 1/4] ASoC: OMAP: HDMI: Introduce driver data for audio codec

[Ricardo Neri]

Under the new DSS architecture for HDMI, there is a clear separation
between general DSS code and HDMI IP-specific data. Functions
that require access to the HDMI driver IP-specific data receive an
hdmi_ip_data structure. The ASoC codec require access to such
IP-specific data. Then, instead of accessing it directly, it will be
passed as codec driver data. This also helps to have a clear separation
between DSS and ASoC portions of the code.

Signed-off-by: Ricardo Neri <ricardo.neri@ti.com>
---
 drivers/video/omap2/dss/hdmi.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c
index c56378c..bd56a8a 100644
--- a/drivers/video/omap2/dss/hdmi.c
+++ b/drivers/video/omap2/dss/hdmi.c
@@ -698,7 +698,16 @@ static int hdmi_audio_startup(struct snd_pcm_substream *substream,
 	return 0;
 }
 
+static int hdmi_audio_codec_probe(struct snd_soc_codec *codec)
+{
+	struct hdmi_ip_data *priv = &hdmi.ip_data;
+
+	snd_soc_codec_set_drvdata(codec, priv);
+	return 0;
+}
+
 static struct snd_soc_codec_driver hdmi_audio_codec_drv = {
+	.probe = hdmi_audio_codec_probe,
 };
 
 static struct snd_soc_dai_ops hdmi_audio_codec_ops = {
-- 
1.7.0.4

[PATCH 2/4] ASoC: OMAP: HDMI: Correct signature of ASoC functions

[Ricardo Neri]

These functions require access to IP-secific data. However, it is not possible
to pass such data as a function argument as such functions have a specific
signature specified by ASoC. Instead, they will have access to the IP-specific
data by calling snd_soc_codec_get_drvdata. The codec driver data is set
at probe time.

Signed-off-by: Ricardo Neri <ricardo.neri@ti.com>
---
 drivers/video/omap2/dss/hdmi.c            |    6 ++++--
 drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c |    7 +++++--
 drivers/video/omap2/dss/ti_hdmi_4xxx_ip.h |    3 +--
 3 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c
index bd56a8a..a0e254e 100644
--- a/drivers/video/omap2/dss/hdmi.c
+++ b/drivers/video/omap2/dss/hdmi.c
@@ -554,11 +554,13 @@ void omapdss_hdmi_display_disable(struct omap_dss_device *dssdev)
 #if defined(CONFIG_SND_OMAP_SOC_OMAP4_HDMI) || \
 	defined(CONFIG_SND_OMAP_SOC_OMAP4_HDMI_MODULE)
 
-static int hdmi_audio_hw_params(struct hdmi_ip_data *ip_data,
-					struct snd_pcm_substream *substream,
+static int hdmi_audio_hw_params(struct snd_pcm_substream *substream,
 				    struct snd_pcm_hw_params *params,
 				    struct snd_soc_dai *dai)
 {
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct snd_soc_codec *codec = rtd->codec;
+	struct hdmi_ip_data *ip_data = snd_soc_codec_get_drvdata(codec);
 	struct hdmi_audio_format audio_format;
 	struct hdmi_audio_dma audio_dma;
 	struct hdmi_core_audio_config core_cfg;
diff --git a/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c b/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c
index e1a6ce5..3f74f55 100644
--- a/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c
+++ b/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c
@@ -1204,11 +1204,14 @@ int hdmi_config_audio_acr(struct hdmi_ip_data *ip_data,
 	return 0;
 }
 
-int hdmi_audio_trigger(struct hdmi_ip_data *ip_data,
-				struct snd_pcm_substream *substream, int cmd,
+int hdmi_audio_trigger(struct snd_pcm_substream *substream, int cmd,
 				struct snd_soc_dai *dai)
 {
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct snd_soc_codec *codec = rtd->codec;
+	struct hdmi_ip_data *ip_data = snd_soc_codec_get_drvdata(codec);
 	int err = 0;
+
 	switch (cmd) {
 	case SNDRV_PCM_TRIGGER_START:
 	case SNDRV_PCM_TRIGGER_RESUME:
diff --git a/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.h b/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.h
index 2040956..eb5e8f7 100644
--- a/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.h
+++ b/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.h
@@ -576,8 +576,7 @@ struct hdmi_core_audio_config {
 
 #if defined(CONFIG_SND_OMAP_SOC_OMAP4_HDMI) || \
 	defined(CONFIG_SND_OMAP_SOC_OMAP4_HDMI_MODULE)
-int hdmi_audio_trigger(struct hdmi_ip_data *ip_data,
-				struct snd_pcm_substream *substream, int cmd,
+int hdmi_audio_trigger(struct snd_pcm_substream *substream, int cmd,
 				struct snd_soc_dai *dai);
 int hdmi_config_audio_acr(struct hdmi_ip_data *ip_data,
 				u32 sample_freq, u32 *n, u32 *cts);
-- 
1.7.0.4

[PATCH 3/4] OMAPDSS: HDMI: Create function to enable HDMI audio

[Ricardo Neri]

In order to separate clearly IP-specific code from general DSS code,
a function for OMAP4 audio enable is created. This function is
included in the HDMI IP ops to align with the current implementation
of the DSS HDMI driver. This function is to be used by the ASoC
HDMI audio codec.

Signed-off-by: Ricardo Neri <ricardo.neri@ti.com>
---
 drivers/video/omap2/dss/dss_features.c    |    4 +++
 drivers/video/omap2/dss/ti_hdmi.h         |   10 ++++++++-
 drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c |   30 +++++++++++++++++-----------
 3 files changed, 31 insertions(+), 13 deletions(-)

diff --git a/drivers/video/omap2/dss/dss_features.c b/drivers/video/omap2/dss/dss_features.c
index b402699..43b6037 100644
--- a/drivers/video/omap2/dss/dss_features.c
+++ b/drivers/video/omap2/dss/dss_features.c
@@ -465,6 +465,10 @@ static const struct ti_hdmi_ip_ops omap4_hdmi_functions = {
 	.dump_core		=	ti_hdmi_4xxx_core_dump,
 	.dump_pll		=	ti_hdmi_4xxx_pll_dump,
 	.dump_phy		=	ti_hdmi_4xxx_phy_dump,
+#if defined(CONFIG_SND_OMAP_SOC_OMAP4_HDMI) || \
+	defined(CONFIG_SND_OMAP_SOC_OMAP4_HDMI_MODULE)
+	.audio_enable		=       ti_hdmi_4xxx_wp_audio_enable,
+#endif
 
 };
 
diff --git a/drivers/video/omap2/dss/ti_hdmi.h b/drivers/video/omap2/dss/ti_hdmi.h
index 2c3443d..7503f7f 100644
--- a/drivers/video/omap2/dss/ti_hdmi.h
+++ b/drivers/video/omap2/dss/ti_hdmi.h
@@ -110,6 +110,11 @@ struct ti_hdmi_ip_ops {
 
 	void (*dump_phy)(struct hdmi_ip_data *ip_data, struct seq_file *s);
 
+#if defined(CONFIG_SND_OMAP_SOC_OMAP4_HDMI) || \
+	defined(CONFIG_SND_OMAP_SOC_OMAP4_HDMI_MODULE)
+	void (*audio_enable)(struct hdmi_ip_data *ip_data, bool start);
+#endif
+
 };
 
 struct hdmi_ip_data {
@@ -134,5 +139,8 @@ void ti_hdmi_4xxx_wp_dump(struct hdmi_ip_data *ip_data, struct seq_file *s);
 void ti_hdmi_4xxx_pll_dump(struct hdmi_ip_data *ip_data, struct seq_file *s);
 void ti_hdmi_4xxx_core_dump(struct hdmi_ip_data *ip_data, struct seq_file *s);
 void ti_hdmi_4xxx_phy_dump(struct hdmi_ip_data *ip_data, struct seq_file *s);
-
+#if defined(CONFIG_SND_OMAP_SOC_OMAP4_HDMI) || \
+	defined(CONFIG_SND_OMAP_SOC_OMAP4_HDMI_MODULE)
+void ti_hdmi_4xxx_wp_audio_enable(struct hdmi_ip_data *ip_data, bool enable);
+#endif
 #endif
diff --git a/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c b/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c
index 3f74f55..220e0ce 100644
--- a/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c
+++ b/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c
@@ -1204,35 +1204,41 @@ int hdmi_config_audio_acr(struct hdmi_ip_data *ip_data,
 	return 0;
 }
 
+void ti_hdmi_4xxx_wp_audio_enable(struct hdmi_ip_data *ip_data, bool enable)
+{
+	REG_FLD_MOD(hdmi_av_base(ip_data),
+				HDMI_CORE_AV_AUD_MODE, enable, 0, 0);
+	REG_FLD_MOD(hdmi_wp_base(ip_data),
+				HDMI_WP_AUDIO_CTRL, enable, 31, 31);
+	REG_FLD_MOD(hdmi_wp_base(ip_data),
+				HDMI_WP_AUDIO_CTRL, enable, 30, 30);
+}
+
 int hdmi_audio_trigger(struct snd_pcm_substream *substream, int cmd,
 				struct snd_soc_dai *dai)
 {
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
 	struct snd_soc_codec *codec = rtd->codec;
+	struct platform_device *pdev = to_platform_device(codec->dev);
 	struct hdmi_ip_data *ip_data = snd_soc_codec_get_drvdata(codec);
 	int err = 0;
 
+	if (!(ip_data->ops) && !(ip_data->ops->audio_enable)) {
+		dev_err(&pdev->dev, "Cannot enable/disable audio\n");
+		return -ENODEV;
+	}
+
 	switch (cmd) {
 	case SNDRV_PCM_TRIGGER_START:
 	case SNDRV_PCM_TRIGGER_RESUME:
 	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
-		REG_FLD_MOD(hdmi_av_base(ip_data),
-					HDMI_CORE_AV_AUD_MODE, 1, 0, 0);
-		REG_FLD_MOD(hdmi_wp_base(ip_data),
-					HDMI_WP_AUDIO_CTRL, 1, 31, 31);
-		REG_FLD_MOD(hdmi_wp_base(ip_data),
-					HDMI_WP_AUDIO_CTRL, 1, 30, 30);
+		ip_data->ops->audio_enable(ip_data, true);
 		break;
 
 	case SNDRV_PCM_TRIGGER_STOP:
 	case SNDRV_PCM_TRIGGER_SUSPEND:
 	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
-		REG_FLD_MOD(hdmi_av_base(ip_data),
-					HDMI_CORE_AV_AUD_MODE, 0, 0, 0);
-		REG_FLD_MOD(hdmi_wp_base(ip_data),
-					HDMI_WP_AUDIO_CTRL, 0, 30, 30);
-		REG_FLD_MOD(hdmi_wp_base(ip_data),
-					HDMI_WP_AUDIO_CTRL, 0, 31, 31);
+		ip_data->ops->audio_enable(ip_data, false);
 		break;
 	default:
 		err = -EINVAL;
-- 
1.7.0.4

[PATCH 4/4] ASoC: OMAP: HDMI: Move HDMI codec trigger function to generic HDMI driver

[Ricardo Neri]

The function hdmi_audio_trigger is a callback used by ASoC to stop/start
HDMI audio. Also, it does not perform IP-specific configuration directly.
Hence, it should be placed in the general portion of the HDMI driver,
along with the other ASoC callbacks.

Signed-off-by: Ricardo Neri <ricardo.neri@ti.com>
---
 drivers/video/omap2/dss/hdmi.c            |   31 ++++++++++++++++++++++++++++
 drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c |   32 -----------------------------
 drivers/video/omap2/dss/ti_hdmi_4xxx_ip.h |    2 -
 3 files changed, 31 insertions(+), 34 deletions(-)

diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c
index a0e254e..5c93041 100644
--- a/drivers/video/omap2/dss/hdmi.c
+++ b/drivers/video/omap2/dss/hdmi.c
@@ -554,6 +554,37 @@ void omapdss_hdmi_display_disable(struct omap_dss_device *dssdev)
 #if defined(CONFIG_SND_OMAP_SOC_OMAP4_HDMI) || \
 	defined(CONFIG_SND_OMAP_SOC_OMAP4_HDMI_MODULE)
 
+int hdmi_audio_trigger(struct snd_pcm_substream *substream, int cmd,
+				struct snd_soc_dai *dai)
+{
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct snd_soc_codec *codec = rtd->codec;
+	struct platform_device *pdev = to_platform_device(codec->dev);
+	struct hdmi_ip_data *ip_data = snd_soc_codec_get_drvdata(codec);
+	int err = 0;
+
+	if (!(ip_data->ops) && !(ip_data->ops->audio_enable)) {
+		dev_err(&pdev->dev, "Cannot enable/disable audio\n");
+		return -ENODEV;
+	}
+
+	switch (cmd) {
+	case SNDRV_PCM_TRIGGER_START:
+	case SNDRV_PCM_TRIGGER_RESUME:
+	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
+		ip_data->ops->audio_enable(ip_data, true);
+		break;
+	case SNDRV_PCM_TRIGGER_STOP:
+	case SNDRV_PCM_TRIGGER_SUSPEND:
+	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
+		ip_data->ops->audio_enable(ip_data, false);
+		break;
+	default:
+		err = -EINVAL;
+	}
+	return err;
+}
+
 static int hdmi_audio_hw_params(struct snd_pcm_substream *substream,
 				    struct snd_pcm_hw_params *params,
 				    struct snd_soc_dai *dai)
diff --git a/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c b/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c
index 220e0ce..9af81f1 100644
--- a/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c
+++ b/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c
@@ -1213,36 +1213,4 @@ void ti_hdmi_4xxx_wp_audio_enable(struct hdmi_ip_data *ip_data, bool enable)
 	REG_FLD_MOD(hdmi_wp_base(ip_data),
 				HDMI_WP_AUDIO_CTRL, enable, 30, 30);
 }
-
-int hdmi_audio_trigger(struct snd_pcm_substream *substream, int cmd,
-				struct snd_soc_dai *dai)
-{
-	struct snd_soc_pcm_runtime *rtd = substream->private_data;
-	struct snd_soc_codec *codec = rtd->codec;
-	struct platform_device *pdev = to_platform_device(codec->dev);
-	struct hdmi_ip_data *ip_data = snd_soc_codec_get_drvdata(codec);
-	int err = 0;
-
-	if (!(ip_data->ops) && !(ip_data->ops->audio_enable)) {
-		dev_err(&pdev->dev, "Cannot enable/disable audio\n");
-		return -ENODEV;
-	}
-
-	switch (cmd) {
-	case SNDRV_PCM_TRIGGER_START:
-	case SNDRV_PCM_TRIGGER_RESUME:
-	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
-		ip_data->ops->audio_enable(ip_data, true);
-		break;
-
-	case SNDRV_PCM_TRIGGER_STOP:
-	case SNDRV_PCM_TRIGGER_SUSPEND:
-	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
-		ip_data->ops->audio_enable(ip_data, false);
-		break;
-	default:
-		err = -EINVAL;
-	}
-	return err;
-}
 #endif
diff --git a/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.h b/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.h
index eb5e8f7..a442998 100644
--- a/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.h
+++ b/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.h
@@ -576,8 +576,6 @@ struct hdmi_core_audio_config {
 
 #if defined(CONFIG_SND_OMAP_SOC_OMAP4_HDMI) || \
 	defined(CONFIG_SND_OMAP_SOC_OMAP4_HDMI_MODULE)
-int hdmi_audio_trigger(struct snd_pcm_substream *substream, int cmd,
-				struct snd_soc_dai *dai);
 int hdmi_config_audio_acr(struct hdmi_ip_data *ip_data,
 				u32 sample_freq, u32 *n, u32 *cts);
 void hdmi_core_audio_infoframe_config(struct hdmi_ip_data *ip_data,
-- 
1.7.0.4

Re: [PATCH 4/4] ASoC: OMAP: HDMI: Move HDMI codec trigger function to generic HDMI driver

[Tomi Valkeinen]

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

Hi Ricardo,

On Fri, 2011-12-16 at 01:03 -0600, Ricardo Neri wrote:
> The function hdmi_audio_trigger is a callback used by ASoC to stop/start
> HDMI audio. Also, it does not perform IP-specific configuration directly.
> Hence, it should be placed in the general portion of the HDMI driver,
> along with the other ASoC callbacks.
> 
> Signed-off-by: Ricardo Neri <ricardo.neri@ti.com>
> ---
>  drivers/video/omap2/dss/hdmi.c            |   31 ++++++++++++++++++++++++++++
>  drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c |   32 -----------------------------
>  drivers/video/omap2/dss/ti_hdmi_4xxx_ip.h |    2 -
>  3 files changed, 31 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c
> index a0e254e..5c93041 100644
> --- a/drivers/video/omap2/dss/hdmi.c
> +++ b/drivers/video/omap2/dss/hdmi.c
> @@ -554,6 +554,37 @@ void omapdss_hdmi_display_disable(struct omap_dss_device *dssdev)
>  #if defined(CONFIG_SND_OMAP_SOC_OMAP4_HDMI) || \
>  	defined(CONFIG_SND_OMAP_SOC_OMAP4_HDMI_MODULE)
>  
> +int hdmi_audio_trigger(struct snd_pcm_substream *substream, int cmd,
> +				struct snd_soc_dai *dai)

This should be static. Otherwise the patch set looks fine to me.

 Tomi


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 0/4] OMAPDSS/ASoC: Repair broken HDMI audio in K3.2-rcX

[K, Mythri P]

Hi Ricardo,

On Fri, Dec 16, 2011 at 12:33 PM, Ricardo Neri <ricardo.neri@ti.com> wrote:
> It has been detected that HDMI audio is broken in K3.2-rcX due to
> the most recent updates in the DSS. This set of patches aims to repair such
> break. It also improves the implementation of the ASoC codec to better follow
> the DSS design by clearly separating functions that need acess to IP-
> specific data.
>

We could not also try to move the ASoC HDMI audio codec driver from
DSS to sound, but this is good for now.
Acked-by: Mythri P K <mythripk@ti.com>

Thanks and regards,
Mythri.

> This set of patches is based on
> git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git v3.2-rc5
>
> and is available at
> git://gitorious.org/omap-audio/linux-audio.git ricardon/topic/fix-hdmi-audio-3.2
>
> 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.2.
>
> Ricardo Neri (4):
>  ASoC: OMAP: HDMI: Introduce driver data for audio codec
>  ASoC: OMAP: HDMI: Correct signature of ASoC functions
>  OMAPDSS: HDMI: Create function to enable HDMI audio
>  ASoC: OMAP: HDMI: Move HDMI codec trigger function to generic HDMI
>    driver
>
>  drivers/video/omap2/dss/dss_features.c    |    4 ++
>  drivers/video/omap2/dss/hdmi.c            |   46 +++++++++++++++++++++++++++-
>  drivers/video/omap2/dss/ti_hdmi.h         |   10 +++++-
>  drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c |   37 ++++-------------------
>  drivers/video/omap2/dss/ti_hdmi_4xxx_ip.h |    3 --
>  5 files changed, 64 insertions(+), 36 deletions(-)
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/4] OMAPDSS/ASoC: Repair broken HDMI audio in K3.2-rcX

[Mark Brown]

On Thu, Dec 29, 2011 at 03:26:48PM +0530, K, Mythri P wrote:

> We could not also try to move the ASoC HDMI audio codec driver from
> DSS to sound, but this is good for now.

You should certainly send the driver for review on the ALSA list...

Re: [PATCH 0/4] OMAPDSS/ASoC: Repair broken HDMI audio in K3.2-rcX

[Ricardo Neri]

Hi Mythri, Mark,
On Thu, 2011-12-29 at 18:48 +0000, Mark Brown wrote:
> On Thu, Dec 29, 2011 at 03:26:48PM +0530, K, Mythri P wrote:
> 
> > We could not also try to move the ASoC HDMI audio codec driver from
> > DSS to sound, but this is good for now.
> 
> You should certainly send the driver for review on the ALSA list...

Yes, the goal is to move the ASoC OMAP4 HDMI codec to sound for K3.3. I
will submit to the ALSA list when I have a first draft.

Thanks,

Ricardo

Re: [PATCH 0/4] OMAPDSS/ASoC: Repair broken HDMI audio in K3.2-rcX

[Mark Brown]

On Mon, Jan 02, 2012 at 11:57:54PM -0600, Ricardo Neri wrote:
> On Thu, 2011-12-29 at 18:48 +0000, Mark Brown wrote:
> > On Thu, Dec 29, 2011 at 03:26:48PM +0530, K, Mythri P wrote:

> > > We could not also try to move the ASoC HDMI audio codec driver from
> > > DSS to sound, but this is good for now.

> > You should certainly send the driver for review on the ALSA list...

> Yes, the goal is to move the ASoC OMAP4 HDMI codec to sound for K3.3. I
> will submit to the ALSA list when I have a first draft.

It shouldn't really have hit mainline without ALSA side review.  For the
HDMI stuff it's not actually clear that keeping the CPU side code out of
the video directory is sensible, there's rather more video complexity
than audio going on and the two are heavily interlinked.  The main thing
is to make sure that the code that's there is sensible.

Re: [PATCH 0/4] OMAPDSS/ASoC: Repair broken HDMI audio in K3.2-rcX

[Ricardo Neri]

Hi Mark,

Sorry, I missed this e-mail...

On Tue, 2012-01-03 at 20:09 +0000, Mark Brown wrote:
> On Mon, Jan 02, 2012 at 11:57:54PM -0600, Ricardo Neri wrote:
> > On Thu, 2011-12-29 at 18:48 +0000, Mark Brown wrote:
> > > On Thu, Dec 29, 2011 at 03:26:48PM +0530, K, Mythri P wrote:
> 
> > > > We could not also try to move the ASoC HDMI audio codec driver from
> > > > DSS to sound, but this is good for now.
> 
> > > You should certainly send the driver for review on the ALSA list...
> 
> > Yes, the goal is to move the ASoC OMAP4 HDMI codec to sound for K3.3. I
> > will submit to the ALSA list when I have a first draft.
> 
> It shouldn't really have hit mainline without ALSA side review.  For the
> HDMI stuff it's not actually clear that keeping the CPU side code out of
> the video directory is sensible, there's rather more video complexity
> than audio going on and the two are heavily interlinked.  The main thing
> is to make sure that the code that's there is sensible.

The approach that I am planning to follow is to move the ASOC HDMI OMAP4
codec to the sound directory and leave all the IP-specific functions
(e.g., to set registers) in the video directory. The audio code should
be generic in the sense that it does not change an IP register directly.
This is to follow a similar approach as that followed by DSS in which
all IP-specific code is separated from DSS code. Also, it makes more
sense to me to have the ASoC codec under sound/soc/codecs. 

In order to implement it, we would obtain the parameters required by
audio (such as the pixel clock, deep color configuration) using DSS
functions. Also, callbacks would be required to notify events such as
HDMI plug/unplug events, changes in video resolutions, etc.

In my opinion, this would make the code clearer and more logical. It
could also help in isolating future changes that are audio or
IP-specific. On the down side, it could also slow-down the intergration
of some changes. For instance, if a new IP-specific function is required
by audio, audio code could only be integrated after the IP-specific
change is integrated under DSS, but as you mention, HDMI video and audio
are tightly interlinked and such dependency will never go away.

What do you think?

Ricardo

Re: [PATCH 0/4] OMAPDSS/ASoC: Repair broken HDMI audio in K3.2-rcX

[Mark Brown]

On Tue, Jan 10, 2012 at 08:44:41PM -0600, Ricardo Neri wrote:

> The approach that I am planning to follow is to move the ASOC HDMI OMAP4
> codec to the sound directory and leave all the IP-specific functions
> (e.g., to set registers) in the video directory. The audio code should
> be generic in the sense that it does not change an IP register directly.
> This is to follow a similar approach as that followed by DSS in which
> all IP-specific code is separated from DSS code. Also, it makes more
> sense to me to have the ASoC codec under sound/soc/codecs. 

Depending on how abstract that interface is it may be better to make it
into something more cross platform than just an OMAP thing - if you're
talking about not having any knowledge of the registers or anything then
it seems like that should be possible.  It would preclude the use of
something OMAP specific like DSS though.

Re: [PATCH 0/4] OMAPDSS/ASoC: Repair broken HDMI audio in K3.2-rcX

[Ricardo Neri]

HI Mark,
On Thu, 2012-01-12 at 02:59 +0000, Mark Brown wrote:
> On Tue, Jan 10, 2012 at 08:44:41PM -0600, Ricardo Neri wrote:
> 
> > The approach that I am planning to follow is to move the ASOC HDMI OMAP4
> > codec to the sound directory and leave all the IP-specific functions
> > (e.g., to set registers) in the video directory. The audio code should
> > be generic in the sense that it does not change an IP register directly.
> > This is to follow a similar approach as that followed by DSS in which
> > all IP-specific code is separated from DSS code. Also, it makes more
> > sense to me to have the ASoC codec under sound/soc/codecs. 
> 
> Depending on how abstract that interface is it may be better to make it
> into something more cross platform than just an OMAP thing - if you're
> talking about not having any knowledge of the registers or anything then
> it seems like that should be possible.  It would preclude the use of
> something OMAP specific like DSS though.

Maybe what I am trying to do is something similar to what is done for
the TWL6040 drivers: the power part goes to drivers/mfd and and the
audio part goes to sound. In my case, the video part goes to DSS and the
audio part goes to sound plus taking advantage of the the register
abstraction and functionality that DSS is trying to provide. As you
mention, this ties me to OMAP atm. I am an audio person :) and I am not
sure all the functionality I require for audio is exported to more
generic layers to allow not using DSS. I could try go take a look.

Ricardo