ASoC: tda998x: add a codec to the HDMI transmitter

The audio constraints of the HDMI interface are defined by the EDID which is sent by the connected device.

The HDMI transmitters may have one or many audio sources.

This patch adds two functions to the HDMI CODEC:
- it updates the audio constraints from the EDID,
- it gives the audio source type to the HDMI transmitter on start of audio streaming.

Signed-off-by Jean-Francois Moine <[email protected]>

---
 include/sound/hdmi.h    |  20 ++++++
 sound/soc/codecs/hdmi.c | 174 ++++++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 188 insertions(+), 6 deletions(-)
 create mode 100644 include/sound/hdmi.h

include/sound/hdmi.h new file +20 -0

new file mode 100644
index 0000000..49062c7
@@ -0,0 +1,20 @@
+#ifndef SND_HDMI_H
+#define SND_HDMI_H
+
+#include <sound/soc.h>
+
+/* platform_data */
+struct hdmi_data {
+	int (*get_audio)(struct device *dev,
+			 int *max_channels,
+			 int *rate_mask,
+			 int *fmt);
+	void (*audio_switch)(struct device *dev,
+			     int port_index,
+			     unsigned sample_rate,
+			     int sample_format);
+	int ndais;
+	struct snd_soc_dai_driver *dais;
+	struct snd_soc_codec_driver *driver;
+};
+#endif

sound/soc/codecs/hdmi.c +34 -0

index 1087fd5..6ea2772 100644
@@ -22,9 +22,146 @@
 #include <sound/soc.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
+#include <sound/pcm_params.h>
+#include <sound/hdmi.h>
 
 #define DRV_NAME "hdmi-audio-codec"
 
+struct hdmi_priv {
+	struct hdmi_data hdmi_data;
+	struct snd_pcm_hw_constraint_list rate_constraints;
+};
+
+static int hdmi_dev_match(struct device *dev, void *data)
+{
+	return !strcmp(dev_name(dev), (char *) data);
+}
+
+/* get the codec device */
+static struct device *hdmi_get_cdev(struct device *dev)
+{
+	struct device *cdev;
+
+	cdev = device_find_child(dev,
+				 DRV_NAME,
+				 hdmi_dev_match);
+	if (!cdev)
+		dev_err(dev, "Cannot get codec device");
+	else
+		put_device(cdev);
+	return cdev;
+}
+
+static int hdmi_startup(struct snd_pcm_substream *substream,
+			struct snd_soc_dai *dai)
+{
+	struct snd_pcm_runtime *runtime = substream->runtime;
+	struct device *cdev;
+	struct hdmi_priv *priv;
+	struct snd_pcm_hw_constraint_list *rate_constraints;

sound/soc/codecs/hdmi.c +124 -6

+
+	/* get the EDID values and the rate constraints buffer */
+	ret = priv->hdmi_data.get_audio(dai->dev,
+					&max_channels, &rate_mask, &fmt);
+	if (ret < 0)
+		return ret;				/* no screen */
+
+	/* convert the EDID values to audio constraints */
+	rate_constraints = &priv->rate_constraints;
+	rate_constraints->list = hdmi_rates;
+	rate_constraints->count = ARRAY_SIZE(hdmi_rates);
+	rate_constraints->mask = rate_mask;
+	snd_pcm_hw_constraint_list(runtime, 0,
+				   SNDRV_PCM_HW_PARAM_RATE,
+				   rate_constraints);
+
+	formats = 0;
+	if (fmt & 1)
+		formats |= SNDRV_PCM_FMTBIT_S16_LE;
+	if (fmt & 2)
+		formats |= SNDRV_PCM_FMTBIT_S20_3LE;
+	if (fmt & 4)
+		formats |= SNDRV_PCM_FMTBIT_S24_LE;
+	snd_pcm_hw_constraint_mask64(runtime,
+				SNDRV_PCM_HW_PARAM_FORMAT,
+				formats);
+
+	snd_pcm_hw_constraint_minmax(runtime,
+				SNDRV_PCM_HW_PARAM_CHANNELS,
+				1, max_channels);
+	return 0;
+}
+
+static int hdmi_hw_params(struct snd_pcm_substream *substream,
+			  struct snd_pcm_hw_params *params,
+			  struct snd_soc_dai *dai)
+{
+	struct device *cdev;
+	struct hdmi_priv *priv;
+
+	cdev = hdmi_get_cdev(dai->dev);
+	if (!cdev)
+		return -ENODEV;
+	priv = dev_get_drvdata(cdev);
+
+	priv->hdmi_data.audio_switch(dai->dev, dai->id,
+				     params_rate(params),
+				     params_format(params));
+	return 0;
+}
+
+static void hdmi_shutdown(struct snd_pcm_substream *substream,
+			  struct snd_soc_dai *dai)
+{
+	struct device *cdev;
+	struct hdmi_priv *priv;
+
+	cdev = hdmi_get_cdev(dai->dev);
+	if (!cdev)
+		return;
+	priv = dev_get_drvdata(cdev);
+
+	priv->hdmi_data.audio_switch(dai->dev, -1, 0, 0);	/* stop */
+}
+
+static const struct snd_soc_dai_ops hdmi_ops = {
+	.startup = hdmi_startup,
+	.hw_params = hdmi_hw_params,
+	.shutdown = hdmi_shutdown,
+};
+
+static int hdmi_codec_probe(struct snd_soc_codec *codec)
+{
+	struct hdmi_priv *priv;
+	struct device *dev = codec->dev;	/* encoder device */
+	struct device *cdev;			/* codec device */
+
+	cdev = hdmi_get_cdev(dev);
+	if (!cdev)
+		return -ENODEV;
+
+	/* allocate some memory to store
+	 * the encoder callback functions and the rate constraints */
+	priv = devm_kzalloc(cdev, sizeof *priv, GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+	dev_set_drvdata(cdev, priv);
+
+	memcpy(&priv->hdmi_data, cdev->platform_data,
+				sizeof priv->hdmi_data);
+	return 0;
+}
+
 static const struct snd_soc_dapm_widget hdmi_widgets[] = {
 	SND_SOC_DAPM_INPUT("RX"),
 	SND_SOC_DAPM_OUTPUT("TX"),

@@ -77,13 +214,38 @@ static struct snd_soc_codec_driver hdmi_codec = {
 	.num_dapm_routes = ARRAY_SIZE(hdmi_routes),
 };
 
-static int hdmi_codec_probe(struct platform_device *pdev)
+static int hdmi_codec_dev_probe(struct platform_device *pdev)
 {
-	return snd_soc_register_codec(&pdev->dev, &hdmi_codec,
-			&hdmi_codec_dai, 1);
+	struct hdmi_data *pdata = pdev->dev.platform_data;
+	struct snd_soc_dai_driver *dais;
+	struct snd_soc_codec_driver *driver;
+	int i, ret;
+
+	if (!pdata)
+		return snd_soc_register_codec(&pdev->dev, &hdmi_codec,
+						&hdmi_codec_dai, 1);
+
+	/* creation from a video encoder as a child device */
+	dais = devm_kmemdup(&pdev->dev,
+			    pdata->dais,
+			    sizeof *pdata->dais * pdata->ndais,
+			    GFP_KERNEL);
+	for (i = 0; i < pdata->ndais; i++)
+		dais[i].ops = &hdmi_ops;
+
+	driver = devm_kmemdup(&pdev->dev,
+			    pdata->driver,
+			    sizeof *pdata->driver,
+			    GFP_KERNEL);
+	driver->probe = hdmi_codec_probe;
+
+	/* register the codec on the video encoder */
+	ret = snd_soc_register_codec(pdev->dev.parent, driver,
+					dais, pdata->ndais);
+	return ret;
 }
 
-static int hdmi_codec_remove(struct platform_device *pdev)
+static int hdmi_codec_dev_remove(struct platform_device *pdev)
 {
 	snd_soc_unregister_codec(&pdev->dev);
 	return 0;

@@ -96,8 +258,8 @@ static struct platform_driver hdmi_codec_driver = {
 		.of_match_table = of_match_ptr(hdmi_audio_codec_ids),
 	},
 
-	.probe		= hdmi_codec_probe,
-	.remove		= hdmi_codec_remove,
+	.probe		= hdmi_codec_dev_probe,
+	.remove		= hdmi_codec_dev_remove,
 };
 
 module_platform_driver(hdmi_codec_driver);

which is sent by the connected device.

The HDMI transmitters may have one or many audio sources.

This patch adds two functions to the HDMI CODEC:
- it updates the audio constraints from the EDID,
- it gives the audio source type to the HDMI transmitter on start of audio streaming.

Signed-off-by Jean-Francois Moine <[email protected]>

---
  include/sound/hdmi.h    |  20 ++++++
  sound/soc/codecs/hdmi.c | 174 ++++++++++++++++++++++++++++++++++++++++++++++--
  2 files changed, 188 insertions(+), 6 deletions(-)

create mode 100644 include/sound/hdmi.h

include/sound/hdmi.h new file +1 -0

new file mode 100644
index 0000000..49062c7
@@ -0,0 +1,20 @@
+#ifndef SND_HDMI_H

include/sound/hdmi.h new file +1 -0

+#endif

sound/soc/codecs/hdmi.c +57 -0

index 1087fd5..6ea2772 100644
@@ -22,9 +22,146 @@
  #include <sound/soc.h>
  #include <linux/of.h>
  #include <linux/of_device.h>
+#include <sound/pcm_params.h>
+#include <sound/hdmi.h>
 
  #define DRV_NAME "hdmi-audio-codec"
 
+struct hdmi_priv {
+	struct hdmi_data hdmi_data;
+	struct snd_pcm_hw_constraint_list rate_constraints;
+};
+
+static int hdmi_dev_match(struct device *dev, void *data)
+{
+	return !strcmp(dev_name(dev), (char *) data);
+}
+
+/* get the codec device */
+static struct device *hdmi_get_cdev(struct device *dev)
+{
+	struct device *cdev;
+
+	cdev = device_find_child(dev,
+				 DRV_NAME,
+				 hdmi_dev_match);
+	if (!cdev)
+		dev_err(dev, "Cannot get codec device");
+	else
+		put_device(cdev);
+	return cdev;
+}
+
+static int hdmi_startup(struct snd_pcm_substream *substream,
+			struct snd_soc_dai *dai)
+{
+	struct snd_pcm_runtime *runtime = substream->runtime;
+	struct device *cdev;
+	struct hdmi_priv *priv;
+	struct snd_pcm_hw_constraint_list *rate_constraints;
+	int ret, max_channels, rate_mask, fmt;
+	u64 formats;
+	static const u32 hdmi_rates[] = {
+		32000, 44100, 48000, 88200, 96000, 176400, 192000
+	};
+
+	cdev = hdmi_get_cdev(dai->dev);
+	if (!cdev)
+		return -ENODEV;
+	priv = dev_get_drvdata(cdev);
+
+	/* get the EDID values and the rate constraints buffer */
+	ret = priv->hdmi_data.get_audio(dai->dev,
+					&max_channels, &rate_mask, &fmt);
+	if (ret < 0)
+		return ret;				/* no screen */
+
+	/* convert the EDID values to audio constraints */
+	rate_constraints = &priv->rate_constraints;
+	rate_constraints->list = hdmi_rates;
+	rate_constraints->count = ARRAY_SIZE(hdmi_rates);
+	rate_constraints->mask = rate_mask;
+	snd_pcm_hw_constraint_list(runtime, 0,

sound/soc/codecs/hdmi.c +87 -3

+	snd_pcm_hw_constraint_mask64(runtime,
+				SNDRV_PCM_HW_PARAM_FORMAT,
+				formats);
+
+	snd_pcm_hw_constraint_minmax(runtime,
+				SNDRV_PCM_HW_PARAM_CHANNELS,
+				1, max_channels);
+	return 0;
+}
+
+static int hdmi_hw_params(struct snd_pcm_substream *substream,
+			  struct snd_pcm_hw_params *params,
+			  struct snd_soc_dai *dai)
+{
+	struct device *cdev;
+	struct hdmi_priv *priv;
+
+	cdev = hdmi_get_cdev(dai->dev);
+	if (!cdev)
+		return -ENODEV;
+	priv = dev_get_drvdata(cdev);
+
+	priv->hdmi_data.audio_switch(dai->dev, dai->id,
+				     params_rate(params),
+				     params_format(params));
+	return 0;
+}
+
+static void hdmi_shutdown(struct snd_pcm_substream *substream,
+			  struct snd_soc_dai *dai)
+{
+	struct device *cdev;
+	struct hdmi_priv *priv;
+
+	cdev = hdmi_get_cdev(dai->dev);
+	if (!cdev)
+		return;
+	priv = dev_get_drvdata(cdev);
+
+	priv->hdmi_data.audio_switch(dai->dev, -1, 0, 0);	/* stop */
+}
+
+static const struct snd_soc_dai_ops hdmi_ops = {
+	.startup = hdmi_startup,
+	.hw_params = hdmi_hw_params,
+	.shutdown = hdmi_shutdown,
+};
+
+static int hdmi_codec_probe(struct snd_soc_codec *codec)
+{
+	struct hdmi_priv *priv;
+	struct device *dev = codec->dev;	/* encoder device */
+	struct device *cdev;			/* codec device */
+
+	cdev = hdmi_get_cdev(dev);
+	if (!cdev)
+		return -ENODEV;
+
+	/* allocate some memory to store
+	 * the encoder callback functions and the rate constraints */
+	priv = devm_kzalloc(cdev, sizeof *priv, GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+	dev_set_drvdata(cdev, priv);
+
+	memcpy(&priv->hdmi_data, cdev->platform_data,
+				sizeof priv->hdmi_data);
+	return 0;
+}
+
  static const struct snd_soc_dapm_widget hdmi_widgets[] = {
  	SND_SOC_DAPM_INPUT("RX"),
  	SND_SOC_DAPM_OUTPUT("TX"),

@@ -77,13 +214,38 @@ static struct snd_soc_codec_driver hdmi_codec = {
  	.num_dapm_routes = ARRAY_SIZE(hdmi_routes),
  };
 
-static int hdmi_codec_probe(struct platform_device *pdev)
+static int hdmi_codec_dev_probe(struct platform_device *pdev)
  {
-	return snd_soc_register_codec(&pdev->dev, &hdmi_codec,
-			&hdmi_codec_dai, 1);
+	struct hdmi_data *pdata = pdev->dev.platform_data;
+	struct snd_soc_dai_driver *dais;
+	struct snd_soc_codec_driver *driver;
+	int i, ret;
+
+	if (!pdata)
+		return snd_soc_register_codec(&pdev->dev, &hdmi_codec,
+						&hdmi_codec_dai, 1);
+
+	/* creation from a video encoder as a child device */
+	dais = devm_kmemdup(&pdev->dev,
+			    pdata->dais,
+			    sizeof *pdata->dais * pdata->ndais,
+			    GFP_KERNEL);
+	for (i = 0; i < pdata->ndais; i++)
+		dais[i].ops = &hdmi_ops;

sound/soc/codecs/hdmi.c +2 -1

+	return ret;
  }
 
-static int hdmi_codec_remove(struct platform_device *pdev)
+static int hdmi_codec_dev_remove(struct platform_device *pdev)
  {
  	snd_soc_unregister_codec(&pdev->dev);
  	return 0;

On 09/24/2014 10:49 AM, Jean-Francois Moine wrote:> The audio constraints of the HDMI interface are defined by the EDID

Quoted 9 lines

which is sent by the connected device.

The HDMI transmitters may have one or many audio sources.

This patch adds two functions to the HDMI CODEC:
- it updates the audio constraints from the EDID,
- it gives the audio source type to the HDMI transmitter on start of audio streaming.

Quoted 4 lines

Signed-off-by Jean-Francois Moine <[email protected]>

---
  include/sound/hdmi.h    |  20 ++++++
  sound/soc/codecs/hdmi.c | 174 

++++++++++++++++++++++++++++++++++++++++++++++--

Quoted 10 lines

  2 files changed, 188 insertions(+), 6 deletions(-)

create mode 100644 include/sound/hdmi.h

include/sound/hdmi.h new file +1 -0

new file mode 100644
index 0000000..49062c7
@@ -0,0 +1,20 @@
+#ifndef SND_HDMI_H

include/sound/hdmi.h line 11

Quoted 10 lines

include/sound/hdmi.h new file +10 -0

+#define SND_HDMI_H
+
+#include <sound/soc.h>
+
+/* platform_data */
+struct hdmi_data {
+	int (*get_audio)(struct device *dev,
+			 int *max_channels,
+			 int *rate_mask,
+			 int *fmt);

This looks good for the api to HDMI ASoC codec.

include/sound/hdmi.h line 15

Quoted 4 lines

include/sound/hdmi.h new file +4 -0

+	void (*audio_switch)(struct device *dev,
+			     int port_index,
+			     unsigned sample_rate,
+			     int sample_format);

I see nothing about selecting the i2s format. I do not know too much about tda998x registers but sii9022 can select left and right justified mode, bit-clock and frame-clock polarity etc. Should we need a callback that corresponds to snd_soc_dai_ops .set_fmt ?

SiI9022 also uses an external clock as some kind of reference for audio and that needs to be divided to right ballpark in relation sample-rate (I have no idea why this is, I am just talking about experience). Should we need callback that corresponds to snd_soc_dai_ops .set_sysclk?

Maybe the above two could also be left for later.

include/sound/hdmi.h line 19

Quoted 4 lines

include/sound/hdmi.h new file +4 -0

+	int ndais;
+	struct snd_soc_dai_driver *dais;
+	struct snd_soc_codec_driver *driver;
+};

The need to include sound/soc.h is something I was trying to avoid in my early design for a similar generic HDMI ASoC codec. I came up with this requirement from Mark's comments about my OMAP HDMI audio patches. So the bellow suggestion are really up to Mark.

If I understand right, the need for ndais, dais, and driver is coming solely from spdif/i2s switching change. Since at least I am not aware of any other DAIs found in HDMI encoders, could we just give the info whether i2s and/or spdif is supporter with a simple bit-field or enum? The snd_soc_dai_driver and snd_soc_codec_driver definitions could then be all put to ASoC side of source tree.

I have also been wondering why we do not have SND_SOC_DAIFMT_SPDIF definition in sound/soc-dai.h. With that there would be no need to define two different dais for spdif and i2s. But this is more a suggestion for ASoC core future development. Because I do not currently have any HW to experiment with this the suggestion should probably be left only as a side note for now.

Quoted 69 lines

include/sound/hdmi.h new file +1 -0

+#endif

sound/soc/codecs/hdmi.c +57 -0

index 1087fd5..6ea2772 100644
@@ -22,9 +22,146 @@
  #include <sound/soc.h>
  #include <linux/of.h>
  #include <linux/of_device.h>
+#include <sound/pcm_params.h>
+#include <sound/hdmi.h>
 
  #define DRV_NAME "hdmi-audio-codec"
 
+struct hdmi_priv {
+	struct hdmi_data hdmi_data;
+	struct snd_pcm_hw_constraint_list rate_constraints;
+};
+
+static int hdmi_dev_match(struct device *dev, void *data)
+{
+	return !strcmp(dev_name(dev), (char *) data);
+}
+
+/* get the codec device */
+static struct device *hdmi_get_cdev(struct device *dev)
+{
+	struct device *cdev;
+
+	cdev = device_find_child(dev,
+				 DRV_NAME,
+				 hdmi_dev_match);
+	if (!cdev)
+		dev_err(dev, "Cannot get codec device");
+	else
+		put_device(cdev);
+	return cdev;
+}
+
+static int hdmi_startup(struct snd_pcm_substream *substream,
+			struct snd_soc_dai *dai)
+{
+	struct snd_pcm_runtime *runtime = substream->runtime;
+	struct device *cdev;
+	struct hdmi_priv *priv;
+	struct snd_pcm_hw_constraint_list *rate_constraints;
+	int ret, max_channels, rate_mask, fmt;
+	u64 formats;
+	static const u32 hdmi_rates[] = {
+		32000, 44100, 48000, 88200, 96000, 176400, 192000
+	};
+
+	cdev = hdmi_get_cdev(dai->dev);
+	if (!cdev)
+		return -ENODEV;
+	priv = dev_get_drvdata(cdev);
+
+	/* get the EDID values and the rate constraints buffer */
+	ret = priv->hdmi_data.get_audio(dai->dev,
+					&max_channels, &rate_mask, &fmt);
+	if (ret < 0)
+		return ret;				/* no screen */
+
+	/* convert the EDID values to audio constraints */
+	rate_constraints = &priv->rate_constraints;
+	rate_constraints->list = hdmi_rates;
+	rate_constraints->count = ARRAY_SIZE(hdmi_rates);
+	rate_constraints->mask = rate_mask;
+	snd_pcm_hw_constraint_list(runtime, 0,

sound/soc/codecs/hdmi.c line 94

Quoted 10 lines

sound/soc/codecs/hdmi.c +10 -0

+				   SNDRV_PCM_HW_PARAM_RATE,
+				   rate_constraints);
+
+	formats = 0;
+	if (fmt & 1)
+		formats |= SNDRV_PCM_FMTBIT_S16_LE;
+	if (fmt & 2)
+		formats |= SNDRV_PCM_FMTBIT_S20_3LE;
+	if (fmt & 4)
+		formats |= SNDRV_PCM_FMTBIT_S24_LE;

I think we should add here:

		formats |= SNDRV_PCM_FMTBIT_S24_3LE;
and probably also
		formats |= SNDRV_PCM_FMTBIT_S32_LE;

The S24_3LE is equal to S24_LE at i2s level and the 32bit format would help to get better than 16bit audio working with if there is trouble supporting 24bit formats (like I have with wip sii9022 code). Both tda998x and sii9022 are able to play 32bit i2s audio just fine. The 8 least significant bits are naturally ignored, but we can set .sig_bits = 24 in snd_soc_pcm_stream.

Quoted 105 lines

sound/soc/codecs/hdmi.c +94 -3

+	snd_pcm_hw_constraint_mask64(runtime,
+				SNDRV_PCM_HW_PARAM_FORMAT,
+				formats);
+
+	snd_pcm_hw_constraint_minmax(runtime,
+				SNDRV_PCM_HW_PARAM_CHANNELS,
+				1, max_channels);
+	return 0;
+}
+
+static int hdmi_hw_params(struct snd_pcm_substream *substream,
+			  struct snd_pcm_hw_params *params,
+			  struct snd_soc_dai *dai)
+{
+	struct device *cdev;
+	struct hdmi_priv *priv;
+
+	cdev = hdmi_get_cdev(dai->dev);
+	if (!cdev)
+		return -ENODEV;
+	priv = dev_get_drvdata(cdev);
+
+	priv->hdmi_data.audio_switch(dai->dev, dai->id,
+				     params_rate(params),
+				     params_format(params));
+	return 0;
+}
+
+static void hdmi_shutdown(struct snd_pcm_substream *substream,
+			  struct snd_soc_dai *dai)
+{
+	struct device *cdev;
+	struct hdmi_priv *priv;
+
+	cdev = hdmi_get_cdev(dai->dev);
+	if (!cdev)
+		return;
+	priv = dev_get_drvdata(cdev);
+
+	priv->hdmi_data.audio_switch(dai->dev, -1, 0, 0);	/* stop */
+}
+
+static const struct snd_soc_dai_ops hdmi_ops = {
+	.startup = hdmi_startup,
+	.hw_params = hdmi_hw_params,
+	.shutdown = hdmi_shutdown,
+};
+
+static int hdmi_codec_probe(struct snd_soc_codec *codec)
+{
+	struct hdmi_priv *priv;
+	struct device *dev = codec->dev;	/* encoder device */
+	struct device *cdev;			/* codec device */
+
+	cdev = hdmi_get_cdev(dev);
+	if (!cdev)
+		return -ENODEV;
+
+	/* allocate some memory to store
+	 * the encoder callback functions and the rate constraints */
+	priv = devm_kzalloc(cdev, sizeof *priv, GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+	dev_set_drvdata(cdev, priv);
+
+	memcpy(&priv->hdmi_data, cdev->platform_data,
+				sizeof priv->hdmi_data);
+	return 0;
+}
+
  static const struct snd_soc_dapm_widget hdmi_widgets[] = {
  	SND_SOC_DAPM_INPUT("RX"),
  	SND_SOC_DAPM_OUTPUT("TX"),

@@ -77,13 +214,38 @@ static struct snd_soc_codec_driver hdmi_codec = {
  	.num_dapm_routes = ARRAY_SIZE(hdmi_routes),
  };
 
-static int hdmi_codec_probe(struct platform_device *pdev)
+static int hdmi_codec_dev_probe(struct platform_device *pdev)
  {
-	return snd_soc_register_codec(&pdev->dev, &hdmi_codec,
-			&hdmi_codec_dai, 1);
+	struct hdmi_data *pdata = pdev->dev.platform_data;
+	struct snd_soc_dai_driver *dais;
+	struct snd_soc_codec_driver *driver;
+	int i, ret;
+
+	if (!pdata)
+		return snd_soc_register_codec(&pdev->dev, &hdmi_codec,
+						&hdmi_codec_dai, 1);
+
+	/* creation from a video encoder as a child device */
+	dais = devm_kmemdup(&pdev->dev,
+			    pdata->dais,
+			    sizeof *pdata->dais * pdata->ndais,
+			    GFP_KERNEL);
+	for (i = 0; i < pdata->ndais; i++)
+		dais[i].ops = &hdmi_ops;
+
+	driver = devm_kmemdup(&pdev->dev,
+			    pdata->driver,
+			    sizeof *pdata->driver,
+			    GFP_KERNEL);
+	driver->probe = hdmi_codec_probe;
+

sound/soc/codecs/hdmi.c static struct snd_soc_codec_driver hdmi_codec = { · line 244

Quoted 3 lines

sound/soc/codecs/hdmi.c +3 -0

+	/* register the codec on the video encoder */
+	ret = snd_soc_register_codec(pdev->dev.parent, driver,
+					dais, pdata->ndais);

Registering the codec under the tda998x driver makes me wonder why would we need the a separate platform device in the first palce. At least it makes it possible to unify the old dummy HDMI codec with the new code, but I am not even sure if that makes eny sense.

Maybe we should have another dummy codec to be used in the situations where you really just need a dummy endpoint to satisfy ASoC. AFAIK the only setups that use the current hdmi codec are BBB HDMI audio and OMAP4+ HDMI audio, which both are currently broken in the upstream.

Quoted 10 lines

sound/soc/codecs/hdmi.c +2 -1

+	return ret;
  }
 
-static int hdmi_codec_remove(struct platform_device *pdev)
+static int hdmi_codec_dev_remove(struct platform_device *pdev)
  {
  	snd_soc_unregister_codec(&pdev->dev);
  	return 0;

sound/soc/codecs/hdmi.c static struct platform_driver hdmi_codec_driver = { · line 265

Quoted 10 lines

sound/soc/codecs/hdmi.c +2 -2

@@ -96,8 +258,8 @@ static struct platform_driver hdmi_codec_driver = {
  		.of_match_table = of_match_ptr(hdmi_audio_codec_ids),
  	},
 
-	.probe		= hdmi_codec_probe,
-	.remove		= hdmi_codec_remove,
+	.probe		= hdmi_codec_dev_probe,
+	.remove		= hdmi_codec_dev_remove,
  };
 
  module_platform_driver(hdmi_codec_driver);

What comes to compatibility to the old use of hdmi codec everything appears to be fine after this patch.

This patch interfaces the HDMI transmitter with the audio system.

drivers/gpu/drm/i2c/tda998x_drv.c tda998x_configure_audio(struct tda998x_priv *priv, · line 891

Quoted 7 lines

drivers/gpu/drm/i2c/tda998x_drv.c +7 -0

+	request_module("snd-soc-hdmi-codec");
+	pdev = platform_device_register_resndata(&priv->hdmi->dev,
+						 "hdmi-audio-codec",
+						  PLATFORM_DEVID_NONE,
+						  NULL, 0,
+						  &tda998x_hdmi_data,
+						  sizeof tda998x_hdmi_data);  

Why is this request_module() needed? If there is a good reason for it we should have some documentation.

Quoted 10 lines · Mark Brown

drivers/gpu/drm/i2c/tda998x_drv.c tda998x_configure_audio(struct tda998x_priv *priv, · line 891

Quoted 7 lines

drivers/gpu/drm/i2c/tda998x_drv.c +7 -0

+	request_module("snd-soc-hdmi-codec");
+	pdev = platform_device_register_resndata(&priv->hdmi->dev,
+						 "hdmi-audio-codec",
+						  PLATFORM_DEVID_NONE,
+						  NULL, 0,
+						  &tda998x_hdmi_data,
+						  sizeof tda998x_hdmi_data);  

Why is this request_module() needed? If there is a good reason for it we should have some documentation.

The reason is simple: as the HDMI CODEC is not declared in the DT, the associated module must be loaded in memory.

Quoted 10 lines · On Tue, 30 Sep 2014 20:25:40 +0100 Mark Brown <[email protected]> wrote:

drivers/gpu/drm/i2c/tda998x_drv.c tda998x_configure_audio(struct tda998x_priv *priv, · line 891

Quoted 7 lines

drivers/gpu/drm/i2c/tda998x_drv.c +7 -0

+	request_module("snd-soc-hdmi-codec");
+	pdev = platform_device_register_resndata(&priv->hdmi->dev,
+						 "hdmi-audio-codec",
+						  PLATFORM_DEVID_NONE,
+						  NULL, 0,
+						  &tda998x_hdmi_data,
+						  sizeof tda998x_hdmi_data);  

Why is this request_module() needed? If there is a good reason for it we should have some documentation.

The reason is simple: as the HDMI CODEC is not declared in the DT, the associated module must be loaded in memory.

This patch interfaces the HDMI transmitter with the audio system.

Signed-off-by Jean-Francois Moine <[email protected]>

---
  .../devicetree/bindings/drm/i2c/tda998x.txt        |  18 ++
  drivers/gpu/drm/i2c/Kconfig                        |   1 +
  drivers/gpu/drm/i2c/tda998x_drv.c                  | 299 +++++++++++++++++++--
  3 files changed, 300 insertions(+), 18 deletions(-)

Documentation/devicetree/bindings/drm/i2c/tda998x.txt +18 -0

index e9e4bce..e50e7cd 100644
@@ -17,6 +17,20 @@ Optional properties:
    - video-ports: 24 bits value which defines how the video controller
  	output is wired to the TDA998x input - default: <0x230145>
 
+  - audio-ports: must contain one or two values selecting the source
+	in the audio port.
+	The source type is given by the corresponding entry in
+	the audio-port-names property.
+
+  - audio-port-names: must contain entries matching the entries in
+	the audio-ports property.
+	Each value may be "i2s" or "spdif", giving the type of
+	the audio source.
+
+  - #sound-dai-cells: must be set to <1> for use with the simple-card.
+	The TDA998x audio CODEC always defines two DAIs.
+	The DAI 0 is the S/PDIF input and the DAI 1 is the I2S input.
+
  Example:
 
  	tda998x: hdmi-encoder {

@@ -26,4 +40,8 @@ Example:
  		interrupts = <27 2>;		/* falling edge */
  		pinctrl-0 = <&pmx_camera>;
  		pinctrl-names = "default";
+
+		audio-ports = <0x04>, <0x03>;
+		audio-port-names = "spdif", "i2s";
+		#sound-dai-cells = <1>;
  	};

drivers/gpu/drm/i2c/Kconfig +1 -0

index 4d341db..42ca744 100644
@@ -22,6 +22,7 @@ config DRM_I2C_SIL164
  config DRM_I2C_NXP_TDA998X
  	tristate "NXP Semiconductors TDA998X HDMI encoder"
  	default m if DRM_TILCDC
+	select SND_SOC_HDMI_CODEC
  	help
  	  Support for NXP Semiconductors TDA998X HDMI encoders.
 

drivers/gpu/drm/i2c/tda998x_drv.c +23 -3

index d476279..66c41c0 100644
@@ -20,12 +20,14 @@
  #include <linux/module.h>
  #include <linux/irq.h>
  #include <sound/asoundef.h>
+#include <linux/platform_device.h>
 
  #include <drm/drmP.h>
  #include <drm/drm_crtc_helper.h>
  #include <drm/drm_encoder_slave.h>
  #include <drm/drm_edid.h>
  #include <drm/i2c/tda998x.h>
+#include <sound/hdmi.h>
 
  #define DBG(fmt, ...) DRM_DEBUG(fmt"\n", ##__VA_ARGS__)
 

@@ -44,6 +46,22 @@ struct tda998x_priv {
  	wait_queue_head_t wq_edid;
  	volatile int wq_edid_wait;
  	struct drm_encoder *encoder;
+
+	/* audio variables */
+	struct platform_device *pdev_codec;
+	u8 audio_ports[2];
+
+	u8 max_channels;		/* EDID parameters */
+	u8 rate_mask;
+	u8 fmt;
+
+	int audio_sample_format;
+};
+
+struct tda998x_priv2 {
+	struct tda998x_priv base;
+	struct drm_encoder encoder;
+	struct drm_connector connector;
  };
 
  #define to_tda998x_priv(x)  ((struct tda998x_priv *)to_encoder_slave(x)->slave_priv)

@@ -624,6 +642,8 @@ tda998x_write_avi(struct tda998x_priv *priv, struct drm_display_mode *mode)
  			 sizeof(buf));
  }
 
+/* audio functions */
+
  static void tda998x_audio_mute(struct tda998x_priv *priv, bool on)
  {
  	if (on) {

@@ -639,12 +659,11 @@ static void
  tda998x_configure_audio(struct tda998x_priv *priv,
  		struct drm_display_mode *mode, struct tda998x_encoder_params *p)
  {
-	uint8_t buf[6], clksel_aip, clksel_fs, cts_n, adiv;
+	uint8_t buf[6], clksel_aip, clksel_fs, cts_n, adiv, aclk;
  	uint32_t n;
 
  	/* Enable audio ports */
  	reg_write(priv, REG_ENA_AP, p->audio_cfg);
-	reg_write(priv, REG_ENA_ACLK, p->audio_clk_cfg);
 
  	/* Set audio input source */
  	switch (p->audio_format) {

@@ -653,13 +672,29 @@ tda998x_configure_audio(struct tda998x_priv *priv,
  		clksel_aip = AIP_CLKSEL_AIP_SPDIF;
  		clksel_fs = AIP_CLKSEL_FS_FS64SPDIF;
  		cts_n = CTS_N_M(3) | CTS_N_K(3);
+		aclk = 0;				/* no clock */
  		break;
 
  	case AFMT_I2S:
  		reg_write(priv, REG_MUX_AP, MUX_AP_SELECT_I2S);
  		clksel_aip = AIP_CLKSEL_AIP_I2S;
  		clksel_fs = AIP_CLKSEL_FS_ACLK;
-		cts_n = CTS_N_M(3) | CTS_N_K(3);
+

drivers/gpu/drm/i2c/tda998x_drv.c +144 -0

+		case SNDRV_PCM_FORMAT_S32_LE:
+			cts_n = CTS_N_M(3) | CTS_N_K(3);
+			break;
+		}
+		aclk = 1;				/* clock enable */
  		break;
 
  	default:

@@ -671,6 +706,7 @@ tda998x_configure_audio(struct tda998x_priv *priv,
  	reg_clear(priv, REG_AIP_CNTRL_0, AIP_CNTRL_0_LAYOUT |
  					AIP_CNTRL_0_ACR_MAN);	/* auto CTS */
  	reg_write(priv, REG_CTS_N, cts_n);
+	reg_write(priv, REG_ENA_ACLK, aclk);
 
  	/*
  	 * Audio input somehow depends on HDMI line rate which is

@@ -727,6 +763,144 @@ tda998x_configure_audio(struct tda998x_priv *priv,
  	tda998x_write_aif(priv, p);
  }
 
+/* audio codec interface */
+
+/* return the audio parameters extracted from the last EDID */
+static int tda998x_get_audio(struct device *dev,
+			int *max_channels,
+			int *rate_mask,
+			int *fmt)
+{
+	struct tda998x_priv2 *priv2 = dev_get_drvdata(dev);
+	struct tda998x_priv *priv = &priv2->base;
+
+	if (!priv->encoder->crtc)
+		return -ENODEV;
+
+	*max_channels = priv->max_channels;
+	*rate_mask = priv->rate_mask;
+	*fmt = priv->fmt;
+	return 0;
+}
+
+/* switch the audio port and initialize the audio parameters for streaming */
+static void tda998x_audio_switch(struct device *dev,
+				 int port_index,
+				 unsigned sample_rate,
+				 int sample_format)
+{
+	struct tda998x_priv2 *priv2 = dev_get_drvdata(dev);
+	struct tda998x_priv *priv = &priv2->base;
+	struct tda998x_encoder_params *p = &priv->params;
+
+	if (!priv->encoder->crtc)
+		return;
+
+	/*
+	 * if port_index is negative (streaming stop),
+	 * disable the audio port
+	 */
+	if (port_index < 0) {
+		reg_write(priv, REG_ENA_AP, 0);
+		return;
+	}
+
+	/* if same audio parameters, just enable the audio port */
+	if (p->audio_cfg == priv->audio_ports[port_index] &&
+	    p->audio_sample_rate == sample_rate &&
+	    priv->audio_sample_format == sample_format) {
+		reg_write(priv, REG_ENA_AP, p->audio_cfg);
+		return;
+	}
+
+	p->audio_format = port_index;
+	p->audio_cfg = priv->audio_ports[port_index];
+	p->audio_sample_rate = sample_rate;
+	priv->audio_sample_format = sample_format;
+	tda998x_configure_audio(priv, &priv->encoder->crtc->hwmode, p);
+}
+
+#define TDA998X_FORMATS	(SNDRV_PCM_FMTBIT_S16_LE | \
+			SNDRV_PCM_FMTBIT_S20_3LE | \
+			SNDRV_PCM_FMTBIT_S24_LE | \
+			SNDRV_PCM_FMTBIT_S32_LE)
+
+static struct snd_soc_dai_driver tda998x_dais[] = {
+	{
+		.name = "spdif-hifi",
+		.id = AFMT_SPDIF,
+		.playback = {
+			.stream_name	= "HDMI SPDIF Playback",
+			.channels_min	= 1,
+			.channels_max	= 2,
+			.rates		= SNDRV_PCM_RATE_CONTINUOUS,
+			.rate_min	= 22050,
+			.rate_max	= 192000,
+			.formats	= TDA998X_FORMATS,
+		},
+	},
+	{
+		.name = "i2s-hifi",
+		.id = AFMT_I2S,
+		.playback = {
+			.stream_name	= "HDMI I2S Playback",
+			.channels_min	= 1,
+			.channels_max	= 8,
+			.rates		= SNDRV_PCM_RATE_CONTINUOUS,
+			.rate_min	= 5512,
+			.rate_max	= 192000,
+			.formats	= TDA998X_FORMATS,
+		},
+	},
+};
+
+static const struct snd_soc_dapm_widget tda998x_widgets[] = {
+	SND_SOC_DAPM_OUTPUT("hdmi-out"),
+};
+static const struct snd_soc_dapm_route tda998x_routes[] = {
+	{ "hdmi-out", NULL, "HDMI I2S Playback" },
+	{ "hdmi-out", NULL, "HDMI SPDIF Playback" },
+};
+
+static struct snd_soc_codec_driver tda998x_codec_driver = {
+	.dapm_widgets = tda998x_widgets,
+	.num_dapm_widgets = ARRAY_SIZE(tda998x_widgets),
+	.dapm_routes = tda998x_routes,
+	.num_dapm_routes = ARRAY_SIZE(tda998x_routes),
+};
+
+static struct hdmi_data tda998x_hdmi_data = {
+	.get_audio = tda998x_get_audio,
+	.audio_switch = tda998x_audio_switch,
+	.ndais = ARRAY_SIZE(tda998x_dais),
+	.dais = tda998x_dais,
+	.driver = &tda998x_codec_driver,
+};
+
+static void tda998x_create_audio_codec(struct tda998x_priv *priv)
+{
+	struct platform_device *pdev;
+	struct module *module;
+
+	request_module("snd-soc-hdmi-codec");
+	pdev = platform_device_register_resndata(&priv->hdmi->dev,
+						 "hdmi-audio-codec",
+						  PLATFORM_DEVID_NONE,
+						  NULL, 0,
+						  &tda998x_hdmi_data,
+						  sizeof tda998x_hdmi_data);
+	if (IS_ERR(pdev)) {
+		dev_err(&priv->hdmi->dev, "cannot create codec: %ld\n",
+			PTR_ERR(pdev));
+		return;
+	}
+
+	priv->pdev_codec = pdev;
+	module = pdev->dev.driver->owner;
+	if (module)
+		try_module_get(module);
+}
+
  /* DRM encoder functions */
 
  static void tda998x_encoder_set_config(struct tda998x_priv *priv,

drivers/gpu/drm/i2c/tda998x_drv.c +102 -9

  }
 
  static void tda998x_encoder_dpms(struct tda998x_priv *priv, int mode)

@@ -1128,6 +1304,47 @@ fail:
  	return NULL;
  }
 
+static void tda998x_set_audio(struct tda998x_priv *priv,
+			      struct drm_connector *connector)
+{
+	u8 *eld = connector->eld;
+	u8 *sad;
+	int sad_count;
+	unsigned eld_ver, mnl, rate_mask;
+	unsigned max_channels, fmt;
+
+	/* adjust the hw params from the ELD (EDID) */
+	eld_ver = eld[0] >> 3;
+	if (eld_ver != 2 && eld_ver != 31)
+		return;
+
+	mnl = eld[4] & 0x1f;
+	if (mnl > 16)
+		return;
+
+	sad_count = eld[5] >> 4;
+	sad = eld + 20 + mnl;
+
+	/* Start from the basic audio settings */
+	max_channels = 2;
+	rate_mask = 0;
+	fmt = 0;
+	while (sad_count--) {
+		switch (sad[0] & 0x78) {
+		case 0x08: /* PCM */
+			max_channels = max(max_channels, (sad[0] & 7) + 1u);
+			rate_mask |= sad[1];
+			fmt |= sad[2] & 0x07;
+			break;
+		}
+		sad += 3;
+	}
+
+	priv->max_channels = max_channels;
+	priv->rate_mask = rate_mask;
+	priv->fmt = fmt;
+}
+
  static int
  tda998x_encoder_get_modes(struct tda998x_priv *priv,
  			  struct drm_connector *connector)

@@ -1139,6 +1356,12 @@ tda998x_encoder_get_modes(struct tda998x_priv *priv,
  		drm_mode_connector_update_edid_property(connector, edid);
  		n = drm_add_edid_modes(connector, edid);
  		priv->is_hdmi_sink = drm_detect_hdmi_monitor(edid);
+
+		/* set the audio parameters from the EDID */
+		if (priv->is_hdmi_sink) {
+			drm_edid_to_eld(connector, edid);
+			tda998x_set_audio(priv, connector);
+		}
  		kfree(edid);
  	}
 

@@ -1167,12 +1390,19 @@ tda998x_encoder_set_property(struct drm_encoder *encoder,
 
  static void tda998x_destroy(struct tda998x_priv *priv)
  {
+	struct module *module;
+
  	/* disable all IRQs and free the IRQ handler */
  	cec_write(priv, REG_CEC_RXSHPDINTENA, 0);
  	reg_clear(priv, REG_INT_FLAGS_2, INT_FLAGS_2_EDID_BLK_RD);
  	if (priv->hdmi->irq)
  		free_irq(priv->hdmi->irq, priv);
 
+	if (priv->pdev_codec) {
+		module = priv->pdev_codec->dev.driver->owner;
+		module_put(module);
+		platform_device_del(priv->pdev_codec);
+	}
  	i2c_unregister_device(priv->cec);
  }
 

@@ -1254,12 +1484,16 @@ static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv)
  {
  	struct device_node *np = client->dev.of_node;
  	u32 video;
-	int rev_lo, rev_hi, ret;
+	int i, j, rev_lo, rev_hi, ret;
+	const char *p;
 
  	priv->vip_cntrl_0 = VIP_CNTRL_0_SWAP_A(2) | VIP_CNTRL_0_SWAP_B(3);
  	priv->vip_cntrl_1 = VIP_CNTRL_1_SWAP_C(0) | VIP_CNTRL_1_SWAP_D(1);
  	priv->vip_cntrl_2 = VIP_CNTRL_2_SWAP_E(4) | VIP_CNTRL_2_SWAP_F(5);
 
+	priv->params.audio_frame[1] = 1;		/* channels - 1 */
+	priv->params.audio_sample_rate = 48000;		/* 48kHz */
+
  	priv->current_page = 0xff;
  	priv->hdmi = client;
  	priv->cec = i2c_new_dummy(client->adapter, 0x34);

@@ -1351,17 +1585,48 @@ static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv)
  	/* enable EDID read irq: */
  	reg_set(priv, REG_INT_FLAGS_2, INT_FLAGS_2_EDID_BLK_RD);
 
-	if (!np)
-		return 0;		/* non-DT */
+	/* get the device tree parameters */
+	if (np) {
 
-	/* get the optional video properties */
-	ret = of_property_read_u32(np, "video-ports", &video);
-	if (ret == 0) {
-		priv->vip_cntrl_0 = video >> 16;
-		priv->vip_cntrl_1 = video >> 8;
-		priv->vip_cntrl_2 = video;
+		/* optional video properties */
+		ret = of_property_read_u32(np, "video-ports", &video);
+		if (ret == 0) {
+			priv->vip_cntrl_0 = video >> 16;
+			priv->vip_cntrl_1 = video >> 8;
+			priv->vip_cntrl_2 = video;
+		}
+
+		/* audio properties */
+		for (i = 0; i < 2; i++) {
+			u32 port;
+
+			ret = of_property_read_u32_index(np, "audio-ports", i, &port);
+			if (ret)
+				break;
+			ret = of_property_read_string_index(np, "audio-port-names",
+							i, &p);
+			if (ret) {
+				dev_err(&client->dev,
+					"missing audio-port-names[%d]\n", i);
+				break;
+			}
+			if (strcmp(p, "spdif") == 0) {
+				j = AFMT_SPDIF;
+			} else if (strcmp(p, "i2s") == 0) {
+				j = AFMT_I2S;
+			} else {
+				dev_err(&client->dev,
+					"bad audio-port-names '%s'\n", p);
+				break;
+			}
+			priv->audio_ports[j] = port;
+		}
  	}
 
+	/* create the audio CODEC */
+	if (priv->audio_ports[AFMT_SPDIF] || priv->audio_ports[AFMT_I2S])
+		tda998x_create_audio_codec(priv);
+
  	return 0;
 
  fail:

@@ -1395,15 +1660,13 @@ static int tda998x_encoder_init(struct i2c_client *client,
  	encoder_slave->slave_priv = priv;
  	encoder_slave->slave_funcs = &tda998x_encoder_slave_funcs;
 
+	/* set the drvdata pointer to priv2 for CODEC calls */
+	dev_set_drvdata(&client->dev,
+			container_of(priv, struct tda998x_priv2, base));
+
  	return 0;
  }

Quoted 6 lines

  	case AFMT_I2S:
  		reg_write(priv, REG_MUX_AP, MUX_AP_SELECT_I2S);
  		clksel_aip = AIP_CLKSEL_AIP_I2S;
  		clksel_fs = AIP_CLKSEL_FS_ACLK;

drivers/gpu/drm/i2c/tda998x_drv.c +1 -1

-		cts_n = CTS_N_M(3) | CTS_N_K(3);
+

drivers/gpu/drm/i2c/tda998x_drv.c tda998x_configure_audio(struct tda998x_priv *priv, · line 692

Quoted 10 lines

drivers/gpu/drm/i2c/tda998x_drv.c +10 -0

+		/* with I2S input, the CTS_N predivider depends on
+		 * the sample width */
+		switch (priv->audio_sample_format) {
+		case SNDRV_PCM_FORMAT_S16_LE:
+			cts_n = CTS_N_M(3) | CTS_N_K(1);
+			break;
+		case SNDRV_PCM_FORMAT_S24_LE:
+			cts_n = CTS_N_M(3) | CTS_N_K(2);
+			break;
+		default:  

Setting the default here does not really help, because priv->audio_sample_format is initialized to SNDRV_PCM_FORMAT_S24_LE in tda998x_encoder_set_config(). But I am Ok with the default being changed for 24 bit samples on i2s interface.

drivers/gpu/drm/i2c/tda998x_drv.c tda998x_configure_audio(struct tda998x_priv *priv, · line 696

Quoted 4 lines

drivers/gpu/drm/i2c/tda998x_drv.c +4 -0

+		case SNDRV_PCM_FORMAT_S32_LE:
+			cts_n = CTS_N_M(3) | CTS_N_K(3);
+			break;
+		}

This patch interfaces the HDMI transmitter with the audio system.

drivers/gpu/drm/i2c/tda998x_drv.c +12 -0

+static void tda998x_create_audio_codec(struct tda998x_priv *priv)
+{
+	struct platform_device *pdev;
+	struct module *module;
+
+	request_module("snd-soc-hdmi-codec");
+	pdev = platform_device_register_resndata(&priv->hdmi->dev,
+						 "hdmi-audio-codec",
+						  PLATFORM_DEVID_NONE,
+						  NULL, 0,
+						  &tda998x_hdmi_data,
+						  sizeof tda998x_hdmi_data);

drivers/gpu/drm/i2c/tda998x_drv.c +1 -0

@@ -1167,12 +1390,19 @@ tda998x_encoder_set_property(struct drm_encoder *encoder,
 
 static void tda998x_destroy(struct tda998x_priv *priv)
 {
+	struct module *module;

drivers/gpu/drm/i2c/tda998x_drv.c +3 -0

@@ -1395,15 +1660,13 @@ static int tda998x_encoder_init(struct i2c_client *client,
 	encoder_slave->slave_priv = priv;
 	encoder_slave->slave_funcs = &tda998x_encoder_slave_funcs;
 
+	/* set the drvdata pointer to priv2 for CODEC calls */
+	dev_set_drvdata(&client->dev,
+			container_of(priv, struct tda998x_priv2, base));

I would prefer this structure to stay here, as code above this point should have no business knowing how these are packaged together. I would suggest either:

- moving the audio codec code below this point, or
- storing struct tda998x_priv in the device private pointer, and converting it to struct tda998x_priv2 via container_of() where necessary below this point.

The NXP TDA998x HDMI transmitter may transmit audio to the HDMI link from 2 different sources, I2S and S/PDIF.

This patch set adds an interface between the HDMI transmitter and the HDMI CODEC.