[RFC 0/6] sti: add audio interface to the hdmi driver

[RFC 0/6] sti: add audio interface to the hdmi driver

[Arnaud Pouliquen]

This patch-set is implementation of audio HDMI on sti platform based on 
generic hdmi-codec driver proposed by Jyri Sarha.
	https://patchwork.kernel.org/patch/7215271/ ("ASoC: hdmi-codec: Add hdmi-codec for external HDMI-encoders")

hdmi-codec is compatible with STi implementation.

In addition i have listed below two features that seem missing:

1) IEC958 channel status control support.
If channel status is handled by HDMI codec, it needs also to support 
IEC61937 format for none PCM stream. In this case a control shall be
created to allows user to set the expected mode, and associated parameters.
Another constraint is that this control should be linked to PCM device to 
support as example a SPDIF and a HDMI out with 2 separate controls.

In this patch set I propose an implementation to support it:
	- ALSA: pcm: add IEC958 channel status control helper
		add generic function to control and handle IEC controls
		customer and professional mask controls could be added if
		needed...
	- ASoC: core: add code to complete dai init after pcm creation
		Add a callback in snd_soc_dai_ops to allow to link DAI controls
		to PCM device

2) HDMI Audio info frame speaker mapping.
To be HDMI 1.4b compliant AIF speaker channel allocation (byte 4) needs
to be set for PCM streams, that contain more than 2 channels.
omap2 driver set hard-coded value for stereo, 5.1 and 7.1. But This 
information should be provided by user that decodes the stream.
I did not implement it but what about a generic CEA861 control to support
the feature?

To finish I'm still interested in having your feed back on patch that propose
a generic implementation to compute N and CTS...

Arnaud Pouliquen (6):
  video: hdmi: add helper function for N and CTS
  ALSA: pcm: add IEC958 channel status control helper
  ASoC: core: add code to complete dai init after pcm creation
  drm: sti: Add ASoC generic hdmi codec support.
  ASoc: hdmi-codec: add IEC control.
  ARM: DT: b2120: add audio HDMI dai link in audio card


 arch/arm/boot/dts/stihxxx-b2120.dtsi |  15 ++-
 drivers/gpu/drm/sti/Kconfig          |   1 +
 drivers/gpu/drm/sti/sti_hdmi.c       | 253 ++++++++++++++++++++++++++++++++---
 drivers/gpu/drm/sti/sti_hdmi.h       |  10 ++
 drivers/video/hdmi.c                 | 146 ++++++++++++++++++++
 include/linux/hdmi.h                 |  11 ++
 include/sound/hdmi-codec.h           |   1 +
 include/sound/pcm_iec958.h           |  16 +++
 include/sound/soc-dai.h              |   7 +
 sound/core/pcm_iec958.c              |  90 +++++++++++++
 sound/soc/codecs/hdmi-codec.c        |  59 ++++++--
 sound/soc/soc-core.c                 |  14 ++
 12 files changed, 591 insertions(+), 32 deletions(-)

-- 
1.9.1

[RFC 1/6] video: hdmi: add helper function for N and CTS

[Arnaud Pouliquen]

From: Moise Gergaud <moise.gergaud@st.com>

Add helper function to compute HDMI CTS and N parameters
Implementation is based on HDMI 1.4b specification.

Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@st.com>
---
 drivers/video/hdmi.c | 146 +++++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/hdmi.h |  11 ++++
 2 files changed, 157 insertions(+)

diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
index 1626892..8af33c1 100644
--- a/drivers/video/hdmi.c
+++ b/drivers/video/hdmi.c
@@ -1242,3 +1242,149 @@ int hdmi_infoframe_unpack(union hdmi_infoframe *frame, void *buffer)
 	return ret;
 }
 EXPORT_SYMBOL(hdmi_infoframe_unpack);
+
+/**
+ * audio clock regeneration (acr) parameters
+ * N and CTS computation are based on HDMI specification 1.4b
+ */
+enum audio_rate {
+	HDMI_AUDIO_N_CTS_32KHZ,
+	HDMI_AUDIO_N_CTS_44_1KHZ,
+	HDMI_AUDIO_N_CTS_48KHZ,
+};
+
+struct hdmi_audio_acr {
+	unsigned long pixel_clk;
+	struct hdmi_audio_n_cts n_cts;
+};
+
+static const struct hdmi_audio_acr hdmi_audio_standard_acr[3][12] = {
+	{ /*32 kHz*/
+		{  25175, {  4576,  28125 } }, /* 25,20/1.001  MHz */
+		{  25200, {  4096,  25200 } }, /* 25.20        MHz */
+		{  27000, {  4096,  27000 } }, /* 27.00        MHz */
+		{  27027, {  4096,  27027 } }, /* 27.00*1.001  MHz */
+		{  54000, {  4096,  54000 } }, /* 54.00        MHz */
+		{  54054, {  4096,  54054 } }, /* 54.00*1.001  MHz */
+		{  74176, { 11648, 310938 } }, /* 74.25/1.001  MHz */
+		{  74250, {  4096,  74250 } }, /* 74.25        MHz */
+		{ 148352, { 11648, 421875 } }, /* 148.50/1.001 MHz */
+		{ 148500, {  4096, 148500 } }, /* 148.50       MHz */
+		{ 296703, {  5824, 421875 } }, /* 297/1.001    MHz */
+		{ 297000, {  3072, 222750 } }, /* 297          MHz */
+	},
+	{ /*44.1 kHz, 88.2 kHz  176.4 kHz*/
+		{  25175, {  7007,  31250 } }, /* 25,20/1.001  MHz */
+		{  25200, {  6272,  28000 } }, /* 25.20        MHz */
+		{  27000, {  6272,  30000 } }, /* 27.00        MHz */
+		{  27027, {  6272,  30030 } }, /* 27.00*1.001  MHz */
+		{  54000, {  6272,  60000 } }, /* 54.00        MHz */
+		{  54054, {  6272,  60060 } }, /* 54.00*1.001  MHz */
+		{  74176, { 17836, 234375 } }, /* 74.25/1.001  MHz */
+		{  74250, {  6272,  82500 } }, /* 74.25        MHz */
+		{ 148352, {  8918, 234375 } }, /* 148.50/1.001 MHz */
+		{ 148500, {  6272, 165000 } }, /* 148.50       MHz */
+		{ 296703, {  4459, 234375 } }, /* 297/1.001    MHz */
+		{ 297000, {  4704, 247500 } }, /* 297          MHz */
+	},
+	{ /*48 kHz, 96 kHz  192 kHz*/
+		{  25175, {  6864,  28125 } }, /* 25,20/1.001  MHz */
+		{  25200, {  6144,  25200 } }, /* 25.20        MHz */
+		{  27000, {  6144,  27000 } }, /* 27.00        MHz */
+		{  27027, {  6144,  27027 } }, /* 27.00*1.001  MHz */
+		{  54000, {  6144,  54000 } }, /* 54.00        MHz */
+		{  54054, {  6144,  54054 } }, /* 54.00*1.001  MHz */
+		{  74176, { 11648, 140625 } }, /* 74.25/1.001  MHz */
+		{  74250, {  6144,  74250 } }, /* 74.25        MHz */
+		{ 148352, {  5824, 140625 } }, /* 148.50/1.001 MHz */
+		{ 148500, {  6144, 148500 } }, /* 148.50       MHz */
+		{ 296703, {  5824, 281250 } }, /* 297/1.001    MHz */
+		{ 297000, {  5120, 247500 } }, /* 297          MHz */
+	}
+};
+
+/**
+ * hdmi_compute_n_cts() - compute N and CTS parameters
+ * @audio_fs: audio frame clock frequency in kHz
+ * @pixel_clk: pixel clock frequency in kHz
+ * @n_cts: N and CTS parameter returned to user
+ *
+ * Values computed are based on table described in HDMI specification 1.4b
+ *
+ * Returns 0 on success or a negative error code on failure.
+ */
+int hdmi_audio_compute_n_cts(unsigned int audio_fs, unsigned long pixel_clk,
+			     struct hdmi_audio_n_cts *n_cts)
+{
+	int audio_freq_id, i;
+	int ratio = 1;
+	const struct hdmi_audio_acr  *acr_table;
+	const struct hdmi_audio_n_cts *predef_n_cts = NULL;
+
+	switch (audio_fs) {
+	case 32000:
+		audio_freq_id = HDMI_AUDIO_N_CTS_32KHZ;
+		n_cts->n = 4096;
+		break;
+
+	case 44100:
+		audio_freq_id = HDMI_AUDIO_N_CTS_44_1KHZ;
+		n_cts->n = 6272;
+		break;
+
+	case 48000:
+		audio_freq_id = HDMI_AUDIO_N_CTS_48KHZ;
+		n_cts->n = 6144;
+		break;
+
+	case 88200:
+		audio_freq_id = HDMI_AUDIO_N_CTS_44_1KHZ;
+		ratio = 2;
+		n_cts->n = 6272 * 2;
+		break;
+
+	case 96000:
+		audio_freq_id = HDMI_AUDIO_N_CTS_48KHZ;
+		ratio = 2;
+		n_cts->n = 6144 * 2;
+		break;
+
+	case 176400:
+		audio_freq_id = HDMI_AUDIO_N_CTS_44_1KHZ;
+		ratio = 4;
+		n_cts->n = 6272 * 4;
+		break;
+
+	case 192000:
+		audio_freq_id = HDMI_AUDIO_N_CTS_48KHZ;
+		ratio = 4;
+		n_cts->n = 6144 * 4;
+		break;
+
+	default:
+		return -EINVAL;
+	}
+
+	acr_table = hdmi_audio_standard_acr[audio_freq_id];
+	for (i = 0; i < ARRAY_SIZE(hdmi_audio_standard_acr[0]); i++) {
+		if (pixel_clk == acr_table[i].pixel_clk) {
+			predef_n_cts = &acr_table[i].n_cts;
+			break;
+		}
+	}
+
+	if (!predef_n_cts) {
+		/*
+		 * predefined frequency not found. Compute CTS using formula:
+		 * CTS = (Ftdms_clk * N) / (128* audio_fs)
+		 */
+		n_cts->cts =  pixel_clk * n_cts->n / (128 * audio_fs);
+	} else {
+		n_cts->n = predef_n_cts->n * ratio;
+		n_cts->cts = predef_n_cts->cts;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL(hdmi_audio_compute_n_cts);
+
diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h
index e974420..6508de8 100644
--- a/include/linux/hdmi.h
+++ b/include/linux/hdmi.h
@@ -333,4 +333,15 @@ int hdmi_infoframe_unpack(union hdmi_infoframe *frame, void *buffer);
 void hdmi_infoframe_log(const char *level, struct device *dev,
 			union hdmi_infoframe *frame);
 
+/**
+ * struct hdmi_audio_n_cts - n and cts parameter for ACR packets
+ */
+struct hdmi_audio_n_cts {
+	unsigned int n;
+	unsigned int cts;
+};
+
+int hdmi_audio_compute_n_cts(unsigned int audio_fs, unsigned long pixel_clk,
+			     struct hdmi_audio_n_cts *n_cts);
+
 #endif /* _DRM_HDMI_H */
-- 
1.9.1

[RFC 2/6] ALSA: pcm: add IEC958 channel status control helper

[Arnaud Pouliquen]

Add IEC958 channel status helper that creates control to handle the
IEC60958 status bits.

Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@st.com>
---
 include/sound/pcm_iec958.h | 16 +++++++++
 sound/core/pcm_iec958.c    | 90 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 106 insertions(+)

diff --git a/include/sound/pcm_iec958.h b/include/sound/pcm_iec958.h
index 36f023a..7453ace 100644
--- a/include/sound/pcm_iec958.h
+++ b/include/sound/pcm_iec958.h
@@ -3,9 +3,25 @@
 
 #include <linux/types.h>
 
+/*
+ * IEC 60958 controls parameters
+ * Describes channel status and associated callback
+ */
+struct snd_pcm_iec958_params {
+	/* call when control is updated by user */
+	int (*ctrl_set)(void *pdata, u8 *status, u8 len);
+
+	struct snd_aes_iec958 *iec;
+	void *pdata; /* user private data to retrieve context */
+	struct mutex *mutex; /* use to avoid race condition */
+};
+
 int snd_pcm_create_iec958_consumer(struct snd_pcm_runtime *runtime, u8 *cs,
 	size_t len);
 
 int snd_pcm_create_iec958_consumer_hw_params(struct snd_pcm_hw_params *params,
 					     u8 *cs, size_t len);
+
+int snd_pcm_create_iec958_ctl(struct snd_pcm *pcm,
+			      struct snd_pcm_iec958_params *params, int stream);
 #endif
diff --git a/sound/core/pcm_iec958.c b/sound/core/pcm_iec958.c
index c9f8b66..1d426bc 100644
--- a/sound/core/pcm_iec958.c
+++ b/sound/core/pcm_iec958.c
@@ -7,11 +7,78 @@
  */
 #include <linux/export.h>
 #include <linux/types.h>
+#include <linux/wait.h>
 #include <sound/asoundef.h>
+#include <sound/control.h>
 #include <sound/pcm.h>
 #include <sound/pcm_params.h>
 #include <sound/pcm_iec958.h>
 
+int snd_pcm_iec958_info(struct snd_kcontrol *kcontrol,
+			struct snd_ctl_elem_info *uinfo)
+{
+	uinfo->type = SNDRV_CTL_ELEM_TYPE_IEC958;
+	uinfo->count = 1;
+	return 0;
+}
+
+static int snd_pcm_iec958_get(struct snd_kcontrol *kcontrol,
+			      struct snd_ctl_elem_value *uctl)
+{
+	struct snd_pcm_iec958_params *params = snd_kcontrol_chip(kcontrol);
+
+	if (params->mutex)
+		mutex_unlock(params->mutex);
+	uctl->value.iec958.status[0] = params->iec->status[0];
+	uctl->value.iec958.status[1] = params->iec->status[1];
+	uctl->value.iec958.status[2] = params->iec->status[2];
+	uctl->value.iec958.status[3] = params->iec->status[3];
+	uctl->value.iec958.status[4] = params->iec->status[4];
+	if (params->mutex)
+		mutex_unlock(params->mutex);
+	return 0;
+}
+
+static int snd_pcm_iec958_put(struct snd_kcontrol *kcontrol,
+			      struct snd_ctl_elem_value *uctl)
+{
+	struct snd_pcm_iec958_params *params = snd_kcontrol_chip(kcontrol);
+	int err = 0;
+
+	if (params->mutex)
+		mutex_lock(params->mutex);
+	if (params->ctrl_set)
+		err = params->ctrl_set(params->pdata,
+				       uctl->value.iec958.status, 5);
+	if (!err) {
+		params->iec->status[0] = uctl->value.iec958.status[0];
+		params->iec->status[1] = uctl->value.iec958.status[1];
+		params->iec->status[2] = uctl->value.iec958.status[2];
+		params->iec->status[3] = uctl->value.iec958.status[3];
+		params->iec->status[4] = uctl->value.iec958.status[4];
+	}
+	if (params->mutex)
+		mutex_unlock(params->mutex);
+
+	return 0;
+}
+
+static const struct snd_kcontrol_new iec958_ctls[] = {
+	{
+		.iface = SNDRV_CTL_ELEM_IFACE_PCM,
+		.name = SNDRV_CTL_NAME_IEC958("", PLAYBACK, DEFAULT),
+		.info = snd_pcm_iec958_info,
+		.get = snd_pcm_iec958_get,
+		.put = snd_pcm_iec958_put,
+	},
+	{
+		.iface = SNDRV_CTL_ELEM_IFACE_PCM,
+		.name = SNDRV_CTL_NAME_IEC958("", CAPTURE, DEFAULT),
+		.info = snd_pcm_iec958_info,
+		.get = snd_pcm_iec958_get,
+	},
+};
+
 static int create_iec958_consumer(uint rate, uint sample_width,
 				  u8 *cs, size_t len)
 {
@@ -111,3 +178,26 @@ int snd_pcm_create_iec958_consumer_hw_params(struct snd_pcm_hw_params *params,
 				      cs, len);
 }
 EXPORT_SYMBOL(snd_pcm_create_iec958_consumer_hw_params);
+
+/**
+ * snd_pcm_create_iec958_ctl - create IEC958 channel status default control
+ * pcm: pcm device to associate to the control.
+ * iec958: snd_pcm_iec958_params structure that cntains callbacks
+ *         and channel status buffer
+ * stream: stream type SNDRV_PCM_STREAM_PLAYBACK or SNDRV_PCM_STREAM_CATURE
+ * Returns:  negative error code if something failed.
+ */
+int snd_pcm_create_iec958_ctl(struct snd_pcm *pcm,
+			      struct snd_pcm_iec958_params *params, int stream)
+{
+	struct snd_kcontrol_new knew;
+
+	if (stream > SNDRV_PCM_STREAM_LAST)
+		return -EINVAL;
+
+	knew = iec958_ctls[stream];
+	knew.device = pcm->device;
+	knew.count = pcm->streams[stream].substream_count;
+	return snd_ctl_add(pcm->card, snd_ctl_new1(&knew, params));
+}
+EXPORT_SYMBOL(snd_pcm_create_iec958_ctl);
-- 
1.9.1

[RFC 3/6] ASoC: core: add code to complete dai init after pcm creation

[Arnaud Pouliquen]

Some Controls defined in DAI need to be associated to PCM device (e.g. IEC60958).

This allows to perform post initialization in DAI after PCM device creation.

Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@st.com>
---
 include/sound/soc-dai.h |  7 +++++++
 sound/soc/soc-core.c    | 14 ++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h
index 964b7de..6969c83 100644
--- a/include/sound/soc-dai.h
+++ b/include/sound/soc-dai.h
@@ -205,6 +205,13 @@ struct snd_soc_dai_ops {
 	 */
 	snd_pcm_sframes_t (*delay)(struct snd_pcm_substream *,
 		struct snd_soc_dai *);
+
+	/*
+	 * function called by soc_probe_link_dais to post initialize DAI
+	 * after pcm device creation.
+	 * As example, can be used to link a controls to the pcm device
+	 */
+	int (*pcm_new)(struct snd_soc_pcm_runtime *, struct snd_soc_dai *);
 };
 
 /*
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 790ee2b..50f3f82 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -1587,6 +1587,7 @@ static int soc_probe_link_dais(struct snd_soc_card *card,
 {
 	struct snd_soc_dai_link *dai_link = rtd->dai_link;
 	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
+	const struct snd_soc_dai_ops *ops;
 	int i, ret;
 
 	dev_dbg(card->dev, "ASoC: probe %s dai link %d late %d\n",
@@ -1662,6 +1663,19 @@ static int soc_probe_link_dais(struct snd_soc_card *card,
 		}
 	}
 
+	for (i = 0; i < rtd->num_codecs; i++) {
+		ops = rtd->codec_dais[i]->driver->ops;
+		if (ops->pcm_new)
+			ret = ops->pcm_new(rtd, rtd->codec_dais[i]);
+		if (ret)
+			return ret;
+	}
+	ops = cpu_dai->driver->ops;
+	if (ops->pcm_new)
+			ret = ops->pcm_new(rtd, rtd->codec_dais[i]);
+	if (ret)
+		return ret;
+
 	return 0;
 }
 
-- 
1.9.1

[RFC 4/6] drm: sti: Add ASoC generic hdmi codec support.

[Arnaud Pouliquen]

Add the interface needed by Jyri's generic hdmi-codec driver [1].

[1] https://patchwork.kernel.org/patch/7215271/ ("ASoC: hdmi-codec: Add
        hdmi-codec for external HDMI-encoders")

Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@st.com>
---
 drivers/gpu/drm/sti/Kconfig    |   1 +
 drivers/gpu/drm/sti/sti_hdmi.c | 253 ++++++++++++++++++++++++++++++++++++++---
 drivers/gpu/drm/sti/sti_hdmi.h |  10 ++
 3 files changed, 247 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/sti/Kconfig b/drivers/gpu/drm/sti/Kconfig
index 10c1b19..d739938 100644
--- a/drivers/gpu/drm/sti/Kconfig
+++ b/drivers/gpu/drm/sti/Kconfig
@@ -7,5 +7,6 @@ config DRM_STI
 	select DRM_KMS_CMA_HELPER
 	select DRM_PANEL
 	select FW_LOADER
+	select SND_SOC_HDMI_CODEC if SND_SOC
 	help
 	  Choose this option to enable DRM on STM stiH41x chipset
diff --git a/drivers/gpu/drm/sti/sti_hdmi.c b/drivers/gpu/drm/sti/sti_hdmi.c
index cd50156..79703dd 100644
--- a/drivers/gpu/drm/sti/sti_hdmi.c
+++ b/drivers/gpu/drm/sti/sti_hdmi.c
@@ -17,6 +17,8 @@
 #include <drm/drm_crtc_helper.h>
 #include <drm/drm_edid.h>
 
+#include <sound/hdmi-codec.h>
+
 #include "sti_hdmi.h"
 #include "sti_hdmi_tx3g4c28phy.h"
 #include "sti_hdmi_tx3g0c55phy.h"
@@ -34,6 +36,8 @@
 #define HDMI_DFLT_CHL0_DAT              0x0110
 #define HDMI_DFLT_CHL1_DAT              0x0114
 #define HDMI_DFLT_CHL2_DAT              0x0118
+#define HDMI_AUDIO_CFG                  0x0200
+#define HDMI_SPDIF_FIFO_STATUS          0x0204
 #define HDMI_SW_DI_1_HEAD_WORD          0x0210
 #define HDMI_SW_DI_1_PKT_WORD0          0x0214
 #define HDMI_SW_DI_1_PKT_WORD1          0x0218
@@ -43,6 +47,9 @@
 #define HDMI_SW_DI_1_PKT_WORD5          0x0228
 #define HDMI_SW_DI_1_PKT_WORD6          0x022C
 #define HDMI_SW_DI_CFG                  0x0230
+#define HDMI_SAMPLE_FLAT_MASK           0x0244
+#define HDMI_AUDN                       0x0400
+#define HDMI_AUD_CTS                    0x0404
 #define HDMI_SW_DI_2_HEAD_WORD          0x0600
 #define HDMI_SW_DI_2_PKT_WORD0          0x0604
 #define HDMI_SW_DI_2_PKT_WORD1          0x0608
@@ -91,6 +98,7 @@
 #define HDMI_INT_DLL_LCK                BIT(5)
 #define HDMI_INT_NEW_FRAME              BIT(6)
 #define HDMI_INT_GENCTRL_PKT            BIT(7)
+#define HDMI_INT_AUDIO_FIFO_XRUN        BIT(8)
 #define HDMI_INT_SINK_TERM_PRESENT      BIT(11)
 
 #define HDMI_DEFAULT_INT (HDMI_INT_SINK_TERM_PRESENT \
@@ -99,6 +107,7 @@
 			| HDMI_INT_GLOBAL)
 
 #define HDMI_WORKING_INT (HDMI_INT_SINK_TERM_PRESENT \
+			| HDMI_INT_AUDIO_FIFO_XRUN \
 			| HDMI_INT_GENCTRL_PKT \
 			| HDMI_INT_NEW_FRAME \
 			| HDMI_INT_DLL_LCK \
@@ -109,6 +118,27 @@
 
 #define HDMI_STA_SW_RST                 BIT(1)
 
+#define HDMI_AUD_CFG_8CH		BIT(0)
+#define HDMI_AUD_CFG_SPDIF_DIV_2	BIT(1)
+#define HDMI_AUD_CFG_SPDIF_DIV_3	BIT(2)
+#define HDMI_AUD_CFG_SPDIF_CLK_DIV_4	(BIT(1) | BIT(2))
+#define HDMI_AUD_CFG_CTS_CLK_256FS	BIT(12)
+#define HDMI_AUD_CFG_DTS_INVALID	BIT(16)
+#define HDMI_AUD_CFG_ONE_BIT_INVALID	(BIT(18) | BIT(19) | BIT(20) |  BIT(21))
+#define HDMI_AUD_CFG_CH12_VALID	BIT(28)
+#define HDMI_AUD_CFG_CH34_VALID	BIT(29)
+#define HDMI_AUD_CFG_CH56_VALID	BIT(30)
+#define HDMI_AUD_CFG_CH78_VALID	BIT(31)
+
+/* sample flat mask */
+#define HDMI_SAMPLE_FLAT_NO	 0
+#define HDMI_SAMPLE_FLAT_SP0 BIT(0)
+#define HDMI_SAMPLE_FLAT_SP1 BIT(1)
+#define HDMI_SAMPLE_FLAT_SP2 BIT(2)
+#define HDMI_SAMPLE_FLAT_SP3 BIT(3)
+#define HDMI_SAMPLE_FLAT_ALL (HDMI_SAMPLE_FLAT_SP0 | HDMI_SAMPLE_FLAT_SP1  |\
+			      HDMI_SAMPLE_FLAT_SP2 | HDMI_SAMPLE_FLAT_SP3)
+
 #define HDMI_INFOFRAME_HEADER_TYPE(x)    (((x) & 0xff) <<  0)
 #define HDMI_INFOFRAME_HEADER_VERSION(x) (((x) & 0xff) <<  8)
 #define HDMI_INFOFRAME_HEADER_LEN(x)     (((x) & 0x0f) << 16)
@@ -157,6 +187,10 @@ static irqreturn_t hdmi_irq_thread(int irq, void *arg)
 		wake_up_interruptible(&hdmi->wait_event);
 	}
 
+	/* Audio FIFO underrun IRQ */
+	if (hdmi->irq_status & HDMI_INT_AUDIO_FIFO_XRUN)
+		DRM_ERROR("audio FIFO underrun occurs");
+
 	return IRQ_HANDLED;
 }
 
@@ -380,26 +414,29 @@ static int hdmi_avi_infoframe_config(struct sti_hdmi *hdmi)
  */
 static int hdmi_audio_infoframe_config(struct sti_hdmi *hdmi)
 {
-	struct hdmi_audio_infoframe infofame;
+	struct hdmi_audio_params *audio = &hdmi->audio;
 	u8 buffer[HDMI_INFOFRAME_SIZE(AUDIO)];
-	int ret;
-
-	ret = hdmi_audio_infoframe_init(&infofame);
-	if (ret < 0) {
-		DRM_ERROR("failed to setup audio infoframe: %d\n", ret);
-		return ret;
-	}
-
-	infofame.channels = 2;
-
-	ret = hdmi_audio_infoframe_pack(&infofame, buffer, sizeof(buffer));
-	if (ret < 0) {
-		DRM_ERROR("failed to pack audio infoframe: %d\n", ret);
-		return ret;
+	int ret, val;
+
+	DRM_DEBUG_DRIVER("enter %s, AIF %s\n", __func__,
+			 audio->enabled ? "enable" : "disable");
+	if (audio->enabled) {
+		/* set audio parameters stored*/
+		ret = hdmi_audio_infoframe_pack(&audio->cea, buffer,
+						sizeof(buffer));
+		if (ret < 0) {
+			DRM_ERROR("failed to pack audio infoframe: %d\n", ret);
+			return ret;
+		}
+		hdmi_infoframe_write_infopack(hdmi, buffer);
+	} else {
+		/*disable audio info frame transmission */
+		val = hdmi_read(hdmi, HDMI_SW_DI_CFG);
+		val &= ~HDMI_IFRAME_CFG_DI_N(HDMI_IFRAME_MASK,
+					     HDMI_IFRAME_SLOT_AUDIO);
+		hdmi_write(hdmi, val, HDMI_SW_DI_CFG);
 	}
 
-	hdmi_infoframe_write_infopack(hdmi, buffer);
-
 	return 0;
 }
 
@@ -583,6 +620,7 @@ static int sti_hdmi_connector_get_modes(struct drm_connector *connector)
 
 	count = drm_add_edid_modes(connector, edid);
 	drm_mode_connector_update_edid_property(connector, edid);
+	drm_edid_to_eld(connector, edid);
 
 	kfree(edid);
 	return count;
@@ -686,6 +724,172 @@ static struct drm_encoder *sti_hdmi_find_encoder(struct drm_device *dev)
 	return NULL;
 }
 
+/**
+ * set audio  info frame
+ *
+ * @hdmi: pointer on the hdmi internal structure
+ *
+ */
+static int hdmi_audio_configure(struct sti_hdmi *hdmi,
+				struct hdmi_audio_params *params)
+{
+	int audio_cfg, ret;
+	struct hdmi_audio_n_cts n_cts;
+	struct hdmi_audio_infoframe *info = &params->cea;
+
+	DRM_DEBUG_DRIVER("\n");
+
+	if (!hdmi->enabled)
+		return 0;
+
+	/* update N parameter */
+	ret = hdmi_audio_compute_n_cts(params->sample_rate,
+				       hdmi->mode.clock, &n_cts);
+	if (ret < 0)
+		return ret;
+
+	DRM_DEBUG_DRIVER("sample_frequency= %d, pix clock = %d\n",
+			 params->sample_rate, hdmi->mode.clock);
+	DRM_DEBUG_DRIVER("n= %d, cts = %d\n", n_cts.n, n_cts.cts);
+
+	hdmi_write(hdmi, n_cts.n, HDMI_AUDN);
+
+	/* update HDMI registers according to configuration */
+	audio_cfg = HDMI_AUD_CFG_SPDIF_DIV_2 | HDMI_AUD_CFG_DTS_INVALID |
+		    HDMI_AUD_CFG_ONE_BIT_INVALID;
+
+	switch (info->channels) {
+	case 8:
+		audio_cfg |= HDMI_AUD_CFG_CH78_VALID;
+	case 6:
+		audio_cfg |= HDMI_AUD_CFG_CH56_VALID;
+	case 4:
+		audio_cfg |= HDMI_AUD_CFG_CH34_VALID | HDMI_AUD_CFG_8CH;
+	case 2:
+		audio_cfg |= HDMI_AUD_CFG_CH12_VALID;
+		break;
+	default:
+		DRM_ERROR("ERROR: Unsupported number of channels (%d)!\n",
+			  info->channels);
+		return -EINVAL;
+	}
+
+	hdmi_write(hdmi, audio_cfg, HDMI_AUDIO_CFG);
+
+	hdmi->audio = *params;
+
+	return hdmi_audio_infoframe_config(hdmi);
+}
+
+static void hdmi_audio_shutdown(struct device *dev)
+{
+	struct sti_hdmi *hdmi = dev_get_drvdata(dev);
+	int audio_cfg;
+
+	DRM_DEBUG_DRIVER("\n");
+
+	/* disable audio */
+	audio_cfg = HDMI_AUD_CFG_SPDIF_DIV_2 | HDMI_AUD_CFG_DTS_INVALID |
+		    HDMI_AUD_CFG_ONE_BIT_INVALID;
+	hdmi_write(hdmi, audio_cfg, HDMI_AUDIO_CFG);
+
+	hdmi->audio.enabled = 0;
+	hdmi_audio_infoframe_config(hdmi);
+}
+
+static int hdmi_audio_hw_params(struct device *dev,
+				struct hdmi_codec_daifmt *daifmt,
+				struct hdmi_codec_params *params)
+{
+	struct sti_hdmi *hdmi = dev_get_drvdata(dev);
+	int ret;
+	struct hdmi_audio_params audio = {
+		.sample_width = params->sample_width,
+		.sample_rate = params->sample_rate,
+		.cea = params->cea,
+	};
+
+	DRM_DEBUG_DRIVER("\n");
+
+	if (!hdmi->enabled)
+		return 0;
+
+	if ((daifmt->fmt != HDMI_I2S) || daifmt->bit_clk_inv ||
+	    daifmt->frame_clk_inv || daifmt->bit_clk_master ||
+	    daifmt->frame_clk_master) {
+		dev_err(dev, "%s: Bad flags %d %d %d %d\n", __func__,
+			daifmt->bit_clk_inv, daifmt->frame_clk_inv,
+			daifmt->bit_clk_master,
+			daifmt->frame_clk_master);
+		return -EINVAL;
+	}
+
+	audio.enabled = 1;
+
+	ret = hdmi_audio_configure(hdmi, &audio);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
+static int hdmi_audio_digital_mute(struct device *dev, bool enable)
+{
+	struct sti_hdmi *hdmi = dev_get_drvdata(dev);
+
+	DRM_DEBUG_DRIVER("%s \n", enable ? "enable" : "disable");
+
+	if (enable)
+		hdmi_write(hdmi, HDMI_SAMPLE_FLAT_ALL, HDMI_SAMPLE_FLAT_MASK);
+	else
+		hdmi_write(hdmi, HDMI_SAMPLE_FLAT_NO, HDMI_SAMPLE_FLAT_MASK);
+
+	return 0;
+}
+
+static int hdmi_audio_get_eld(struct device *dev, uint8_t *buf, size_t len)
+{
+	struct sti_hdmi *hdmi = dev_get_drvdata(dev);
+	struct drm_connector *connector = hdmi->drm_connector;
+
+	DRM_DEBUG_DRIVER("\n");
+	memcpy(buf, connector->eld, min(sizeof(connector->eld), len));
+
+	return 0;
+}
+
+static const struct hdmi_codec_ops audio_codec_ops = {
+	.hw_params = hdmi_audio_hw_params,
+	.audio_shutdown = hdmi_audio_shutdown,
+	.digital_mute = hdmi_audio_digital_mute,
+	.get_eld = hdmi_audio_get_eld,
+};
+
+static int sti_hdmi_register_audio_driver(struct device *dev,
+					  struct sti_hdmi *hdmi)
+{
+	struct hdmi_codec_pdata codec_data = {
+		.ops = &audio_codec_ops,
+		.max_i2s_channels = 8,
+		.i2s = 1,
+	};
+
+	DRM_DEBUG_DRIVER("\n");
+
+	hdmi->audio.enabled = 0;
+
+	hdmi->audio_pdev = platform_device_register_data(
+		dev, HDMI_CODEC_DRV_NAME, PLATFORM_DEVID_AUTO,
+		&codec_data, sizeof(codec_data));
+
+	if (IS_ERR(hdmi->audio_pdev))
+		return PTR_ERR(hdmi->audio_pdev);
+
+	DRM_INFO("%s driver bound %s\n", HDMI_CODEC_DRV_NAME, dev_name(dev));
+
+	return 0;
+}
+
 static int sti_hdmi_bind(struct device *dev, struct device *master, void *data)
 {
 	struct sti_hdmi *hdmi = dev_get_drvdata(dev);
@@ -733,12 +937,23 @@ static int sti_hdmi_bind(struct device *dev, struct device *master, void *data)
 	if (err)
 		goto err_connector;
 
+	hdmi->drm_connector = drm_connector;
+
 	err = drm_mode_connector_attach_encoder(drm_connector, encoder);
 	if (err) {
 		DRM_ERROR("Failed to attach a connector to a encoder\n");
 		goto err_sysfs;
 	}
 
+	err = sti_hdmi_register_audio_driver(dev, hdmi);
+	if (err) {
+		DRM_ERROR("Failed to attach an audio codec\n");
+		goto err_sysfs;
+	}
+
+	/* initialise audio infoframe */
+	hdmi_audio_infoframe_init(&hdmi->audio.cea);
+
 	/* Enable default interrupts */
 	hdmi_write(hdmi, HDMI_DEFAULT_INT, HDMI_INT_EN);
 
@@ -746,6 +961,7 @@ static int sti_hdmi_bind(struct device *dev, struct device *master, void *data)
 
 err_sysfs:
 	drm_connector_unregister(drm_connector);
+	hdmi->drm_connector = NULL;
 err_connector:
 	drm_connector_cleanup(drm_connector);
 
@@ -895,6 +1111,9 @@ static int sti_hdmi_remove(struct platform_device *pdev)
 	struct sti_hdmi *hdmi = dev_get_drvdata(&pdev->dev);
 
 	i2c_put_adapter(hdmi->ddc_adapt);
+
+	if (hdmi->audio_pdev)
+		platform_device_unregister(hdmi->audio_pdev);
 	component_del(&pdev->dev, &sti_hdmi_ops);
 
 	return 0;
diff --git a/drivers/gpu/drm/sti/sti_hdmi.h b/drivers/gpu/drm/sti/sti_hdmi.h
index 3d22390..afc4927 100644
--- a/drivers/gpu/drm/sti/sti_hdmi.h
+++ b/drivers/gpu/drm/sti/sti_hdmi.h
@@ -24,6 +24,13 @@ struct hdmi_phy_ops {
 	void (*stop)(struct sti_hdmi *hdmi);
 };
 
+struct hdmi_audio_params {
+	bool enabled;
+	unsigned sample_width;
+	unsigned sample_rate;
+	struct hdmi_audio_infoframe cea;
+};
+
 /**
  * STI hdmi structure
  *
@@ -64,6 +71,9 @@ struct sti_hdmi {
 	bool event_received;
 	struct reset_control *reset;
 	struct i2c_adapter *ddc_adapt;
+	struct platform_device *audio_pdev;
+	struct hdmi_audio_params audio;
+	struct drm_connector *drm_connector;
 };
 
 u32 hdmi_read(struct sti_hdmi *hdmi, int offset);
-- 
1.9.1

[RFC 5/6] ASoc: hdmi-codec: add IEC control.

[Arnaud Pouliquen]

Create 'IEC958 Playback Default' controls to support IEC61937 formats.
the use of the alsa control is optional, using 'iec_ctl' flag.

Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@st.com>
---
 include/sound/hdmi-codec.h    |  1 +
 sound/soc/codecs/hdmi-codec.c | 59 +++++++++++++++++++++++++++++++++----------
 2 files changed, 46 insertions(+), 14 deletions(-)

diff --git a/include/sound/hdmi-codec.h b/include/sound/hdmi-codec.h
index ed780b2..58e8eae 100644
--- a/include/sound/hdmi-codec.h
+++ b/include/sound/hdmi-codec.h
@@ -96,6 +96,7 @@ struct hdmi_codec_pdata {
 	const struct hdmi_codec_ops *ops;
 	uint i2s:1;
 	uint spdif:1;
+	uint iec_ctl:1;
 	int max_i2s_channels;
 };
 
diff --git a/sound/soc/codecs/hdmi-codec.c b/sound/soc/codecs/hdmi-codec.c
index 94349b3..1b6cb5a 100644
--- a/sound/soc/codecs/hdmi-codec.c
+++ b/sound/soc/codecs/hdmi-codec.c
@@ -32,6 +32,7 @@ struct hdmi_codec_priv {
 	struct snd_pcm_substream *current_stream;
 	struct snd_pcm_hw_constraint_list ratec;
 	uint8_t eld[MAX_ELD_BYTES];
+	struct snd_aes_iec958 iec;
 };
 
 static const struct snd_soc_dapm_widget hdmi_widgets[] = {
@@ -140,27 +141,30 @@ static int hdmi_codec_hw_params(struct snd_pcm_substream *substream,
 				struct snd_soc_dai *dai)
 {
 	struct hdmi_codec_priv *hcp = snd_soc_dai_get_drvdata(dai);
-	struct hdmi_codec_params hp = {
-		.iec = {
-			.status = { 0 },
-			.subcode = { 0 },
-			.pad = 0,
-			.dig_subframe = { 0 },
-		}
-	};
+	struct hdmi_codec_params hp;
 	int ret;
 
 	dev_dbg(dai->dev, "%s() width %d rate %d channels %d\n", __func__,
 		params_width(params), params_rate(params),
 		params_channels(params));
 
-	ret = snd_pcm_create_iec958_consumer_hw_params(params, hp.iec.status,
-						       sizeof(hp.iec.status));
-	if (ret < 0) {
-		dev_err(dai->dev, "Creating IEC958 channel status failed %d\n",
-			ret);
-		return ret;
+	mutex_lock(&hcp->current_stream_lock);
+	hp.iec = hcp->iec;
+
+	if (!hcp->hcd.iec_ctl) {
+		/*
+		* only PCM format supported
+		*channel status set according to runtime parameters
+		*/
+		ret = snd_pcm_create_iec958_consumer_hw_params(params,
+					hp.iec.status, sizeof(hp.iec.status));
+		if (ret < 0) {
+			dev_err(dai->dev, "Creating IEC958 status failed %d\n",
+				ret);
+			return ret;
+		}
 	}
+	mutex_unlock(&hcp->current_stream_lock);
 
 	ret = hdmi_codec_new_stream(substream, dai);
 	if (ret)
@@ -266,12 +270,37 @@ static int hdmi_codec_digital_mute(struct snd_soc_dai *dai, int mute)
 	return 0;
 }
 
+static int hdmi_codec_pcm_new(struct snd_soc_pcm_runtime *rtd,
+			      struct snd_soc_dai *dai)
+{
+	struct hdmi_codec_priv *hcp = snd_soc_dai_get_drvdata(dai);
+	struct snd_pcm_iec958_params *iec958_params;
+
+	dev_dbg(dai->dev, "%s()\n", __func__);
+
+	if (!hcp->hcd.iec_ctl)
+		return 0;
+
+	iec958_params = devm_kzalloc(dai->dev, sizeof(*iec958_params),
+				     GFP_KERNEL);
+	if (!iec958_params)
+		return -ENOMEM;
+
+	iec958_params->iec = &hcp->iec;
+	iec958_params->pdata = hcp;
+	iec958_params->mutex = &hcp->current_stream_lock;
+
+	return snd_pcm_create_iec958_ctl(rtd->pcm, iec958_params,
+					 SNDRV_PCM_STREAM_PLAYBACK);
+}
+
 static const struct snd_soc_dai_ops hdmi_dai_ops = {
 	.startup        = hdmi_codec_startup,
 	.shutdown       = hdmi_codec_shutdown,
 	.hw_params      = hdmi_codec_hw_params,
 	.set_fmt        = hdmi_codec_set_fmt,
 	.digital_mute   = hdmi_codec_digital_mute,
+	.pcm_new        = hdmi_codec_pcm_new,
 };
 
 
@@ -352,6 +381,8 @@ static int hdmi_codec_probe(struct platform_device *pdev)
 	if (!hcp)
 		return -ENOMEM;
 
+	memset(&hcp->iec, 0, sizeof(hcp->iec));
+
 	hcp->hcd = *hcd;
 	mutex_init(&hcp->current_stream_lock);
 
-- 
1.9.1

[RFC 6/6] ARM: DT: b2120: add audio HDMI dai link in audio card

[Arnaud Pouliquen]

Add the HDMI dai link to support audio for HDMI output

Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@st.com>
---
 arch/arm/boot/dts/stihxxx-b2120.dtsi | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/stihxxx-b2120.dtsi b/arch/arm/boot/dts/stihxxx-b2120.dtsi
index 1ec71fe..a773c00 100644
--- a/arch/arm/boot/dts/stihxxx-b2120.dtsi
+++ b/arch/arm/boot/dts/stihxxx-b2120.dtsi
@@ -126,6 +126,19 @@
 		status = "okay";
 
 		simple-audio-card,dai-link@0 {
+			/* HDMI */
+			format = "i2s";
+			mclk-fs = <128>;
+			cpu {
+				sound-dai = <&sti_uni_player0>;
+			};
+
+			codec {
+				sound-dai = <&sti_hdmi>;
+			};
+		};
+
+		simple-audio-card,dai-link@1 {
 			/* DAC */
 			format = "i2s";
 			dai-tdm-slot-width = <32>;
@@ -139,7 +152,7 @@
 			};
 		};
 
-		simple-audio-card,dai-link@1 {
+		simple-audio-card,dai-link@2 {
 			/* SPDIF */
 			format = "left_j";
 			mclk-fs = <128>;
-- 
1.9.1

Re: [RFC 1/6] video: hdmi: add helper function for N and CTS

[Russell King - ARM Linux]

On Tue, Jan 19, 2016 at 02:40:31PM +0100, Arnaud Pouliquen wrote:
> From: Moise Gergaud <moise.gergaud@st.com>
> 
> Add helper function to compute HDMI CTS and N parameters
> Implementation is based on HDMI 1.4b specification.

You should note that these CTS/N parameters are for where the audio and
video clocks are coherent: iow, they are derived from the same source.
If they can drift (because they're generated from different sources)
then a hardware implementation which automatically calculates CTS is
needed.

However, some of the values given in the HDMI specs for CTS are actually
two values, because a single value does not specify the audio clock even
with the N values given.  For these, the values returned will not give
an accurate clock - this should also be noted.

Next, does "pixel clock" make sense when you have pixel repetition?
Would calling this "tdms_clock" be more accurate to the specification
and less mis-leading to users?

> Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@st.com>
> ---
>  drivers/video/hdmi.c | 146 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/hdmi.h |  11 ++++
>  2 files changed, 157 insertions(+)
> 
> diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
> index 1626892..8af33c1 100644
> --- a/drivers/video/hdmi.c
> +++ b/drivers/video/hdmi.c
> @@ -1242,3 +1242,149 @@ int hdmi_infoframe_unpack(union hdmi_infoframe *frame, void *buffer)
>  	return ret;
>  }
>  EXPORT_SYMBOL(hdmi_infoframe_unpack);
> +
> +/**
> + * audio clock regeneration (acr) parameters
> + * N and CTS computation are based on HDMI specification 1.4b
> + */
> +enum audio_rate {
> +	HDMI_AUDIO_N_CTS_32KHZ,
> +	HDMI_AUDIO_N_CTS_44_1KHZ,
> +	HDMI_AUDIO_N_CTS_48KHZ,
> +};
> +
> +struct hdmi_audio_acr {
> +	unsigned long pixel_clk;
> +	struct hdmi_audio_n_cts n_cts;
> +};
> +
> +static const struct hdmi_audio_acr hdmi_audio_standard_acr[3][12] = {
> +	{ /*32 kHz*/
> +		{  25175, {  4576,  28125 } }, /* 25,20/1.001  MHz */
> +		{  25200, {  4096,  25200 } }, /* 25.20        MHz */
> +		{  27000, {  4096,  27000 } }, /* 27.00        MHz */
> +		{  27027, {  4096,  27027 } }, /* 27.00*1.001  MHz */
> +		{  54000, {  4096,  54000 } }, /* 54.00        MHz */
> +		{  54054, {  4096,  54054 } }, /* 54.00*1.001  MHz */
> +		{  74176, { 11648, 310938 } }, /* 74.25/1.001  MHz */
> +		{  74250, {  4096,  74250 } }, /* 74.25        MHz */
> +		{ 148352, { 11648, 421875 } }, /* 148.50/1.001 MHz */
> +		{ 148500, {  4096, 148500 } }, /* 148.50       MHz */
> +		{ 296703, {  5824, 421875 } }, /* 297/1.001    MHz */
> +		{ 297000, {  3072, 222750 } }, /* 297          MHz */
> +	},
> +	{ /*44.1 kHz, 88.2 kHz  176.4 kHz*/
> +		{  25175, {  7007,  31250 } }, /* 25,20/1.001  MHz */
> +		{  25200, {  6272,  28000 } }, /* 25.20        MHz */
> +		{  27000, {  6272,  30000 } }, /* 27.00        MHz */
> +		{  27027, {  6272,  30030 } }, /* 27.00*1.001  MHz */
> +		{  54000, {  6272,  60000 } }, /* 54.00        MHz */
> +		{  54054, {  6272,  60060 } }, /* 54.00*1.001  MHz */
> +		{  74176, { 17836, 234375 } }, /* 74.25/1.001  MHz */
> +		{  74250, {  6272,  82500 } }, /* 74.25        MHz */
> +		{ 148352, {  8918, 234375 } }, /* 148.50/1.001 MHz */
> +		{ 148500, {  6272, 165000 } }, /* 148.50       MHz */
> +		{ 296703, {  4459, 234375 } }, /* 297/1.001    MHz */
> +		{ 297000, {  4704, 247500 } }, /* 297          MHz */
> +	},
> +	{ /*48 kHz, 96 kHz  192 kHz*/
> +		{  25175, {  6864,  28125 } }, /* 25,20/1.001  MHz */
> +		{  25200, {  6144,  25200 } }, /* 25.20        MHz */
> +		{  27000, {  6144,  27000 } }, /* 27.00        MHz */
> +		{  27027, {  6144,  27027 } }, /* 27.00*1.001  MHz */
> +		{  54000, {  6144,  54000 } }, /* 54.00        MHz */
> +		{  54054, {  6144,  54054 } }, /* 54.00*1.001  MHz */
> +		{  74176, { 11648, 140625 } }, /* 74.25/1.001  MHz */
> +		{  74250, {  6144,  74250 } }, /* 74.25        MHz */
> +		{ 148352, {  5824, 140625 } }, /* 148.50/1.001 MHz */
> +		{ 148500, {  6144, 148500 } }, /* 148.50       MHz */
> +		{ 296703, {  5824, 281250 } }, /* 297/1.001    MHz */
> +		{ 297000, {  5120, 247500 } }, /* 297          MHz */
> +	}
> +};
> +
> +/**
> + * hdmi_compute_n_cts() - compute N and CTS parameters
> + * @audio_fs: audio frame clock frequency in kHz

This is wrong.  You're expecting values such as 32000 here.  32000kHz
would be 32MHz, which is way too high for an audio frame clock.  I
think you mean Hz.

> + * @pixel_clk: pixel clock frequency in kHz

This is correct though.

> + * @n_cts: N and CTS parameter returned to user
> + *
> + * Values computed are based on table described in HDMI specification 1.4b
> + *
> + * Returns 0 on success or a negative error code on failure.
> + */
> +int hdmi_audio_compute_n_cts(unsigned int audio_fs, unsigned long pixel_clk,
> +			     struct hdmi_audio_n_cts *n_cts)
> +{
> +	int audio_freq_id, i;
> +	int ratio = 1;
> +	const struct hdmi_audio_acr  *acr_table;
> +	const struct hdmi_audio_n_cts *predef_n_cts = NULL;
> +
> +	switch (audio_fs) {
> +	case 32000:
> +		audio_freq_id = HDMI_AUDIO_N_CTS_32KHZ;
> +		n_cts->n = 4096;
> +		break;
> +
> +	case 44100:
> +		audio_freq_id = HDMI_AUDIO_N_CTS_44_1KHZ;
> +		n_cts->n = 6272;
> +		break;
> +
> +	case 48000:
> +		audio_freq_id = HDMI_AUDIO_N_CTS_48KHZ;
> +		n_cts->n = 6144;
> +		break;
> +
> +	case 88200:
> +		audio_freq_id = HDMI_AUDIO_N_CTS_44_1KHZ;
> +		ratio = 2;
> +		n_cts->n = 6272 * 2;
> +		break;
> +
> +	case 96000:
> +		audio_freq_id = HDMI_AUDIO_N_CTS_48KHZ;
> +		ratio = 2;
> +		n_cts->n = 6144 * 2;
> +		break;
> +
> +	case 176400:
> +		audio_freq_id = HDMI_AUDIO_N_CTS_44_1KHZ;
> +		ratio = 4;
> +		n_cts->n = 6272 * 4;
> +		break;
> +
> +	case 192000:
> +		audio_freq_id = HDMI_AUDIO_N_CTS_48KHZ;
> +		ratio = 4;
> +		n_cts->n = 6144 * 4;
> +		break;
> +
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	acr_table = hdmi_audio_standard_acr[audio_freq_id];
> +	for (i = 0; i < ARRAY_SIZE(hdmi_audio_standard_acr[0]); i++) {
> +		if (pixel_clk == acr_table[i].pixel_clk) {
> +			predef_n_cts = &acr_table[i].n_cts;
> +			break;
> +		}
> +	}
> +
> +	if (!predef_n_cts) {
> +		/*
> +		 * predefined frequency not found. Compute CTS using formula:
> +		 * CTS = (Ftdms_clk * N) / (128* audio_fs)
> +		 */
> +		n_cts->cts =  pixel_clk * n_cts->n / (128 * audio_fs);

This looks like it could overflow.  192kHz audio clock, 297MHz tdms
clock -> (297000 * 6144 * 4) = 7299072000.  32 bit unsigned math
overflows beyond 4294967295.

-- 
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: [RFC 2/6] ALSA: pcm: add IEC958 channel status control helper

[Russell King - ARM Linux]

On Tue, Jan 19, 2016 at 02:40:32PM +0100, Arnaud Pouliquen wrote:
> Add IEC958 channel status helper that creates control to handle the
> IEC60958 status bits.
> 
> Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@st.com>
> ---
>  include/sound/pcm_iec958.h | 16 +++++++++
>  sound/core/pcm_iec958.c    | 90 ++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 106 insertions(+)
> 
> diff --git a/include/sound/pcm_iec958.h b/include/sound/pcm_iec958.h
> index 36f023a..7453ace 100644
> --- a/include/sound/pcm_iec958.h
> +++ b/include/sound/pcm_iec958.h
> @@ -3,9 +3,25 @@
>  
>  #include <linux/types.h>
>  
> +/*
> + * IEC 60958 controls parameters
> + * Describes channel status and associated callback
> + */
> +struct snd_pcm_iec958_params {
> +	/* call when control is updated by user */
> +	int (*ctrl_set)(void *pdata, u8 *status, u8 len);
> +
> +	struct snd_aes_iec958 *iec;
> +	void *pdata; /* user private data to retrieve context */
> +	struct mutex *mutex; /* use to avoid race condition */
> +};
> +
>  int snd_pcm_create_iec958_consumer(struct snd_pcm_runtime *runtime, u8 *cs,
>  	size_t len);
>  
>  int snd_pcm_create_iec958_consumer_hw_params(struct snd_pcm_hw_params *params,
>  					     u8 *cs, size_t len);
> +
> +int snd_pcm_create_iec958_ctl(struct snd_pcm *pcm,
> +			      struct snd_pcm_iec958_params *params, int stream);
>  #endif
> diff --git a/sound/core/pcm_iec958.c b/sound/core/pcm_iec958.c
> index c9f8b66..1d426bc 100644
> --- a/sound/core/pcm_iec958.c
> +++ b/sound/core/pcm_iec958.c
> @@ -7,11 +7,78 @@
>   */
>  #include <linux/export.h>
>  #include <linux/types.h>
> +#include <linux/wait.h>
>  #include <sound/asoundef.h>
> +#include <sound/control.h>
>  #include <sound/pcm.h>
>  #include <sound/pcm_params.h>
>  #include <sound/pcm_iec958.h>
>  
> +int snd_pcm_iec958_info(struct snd_kcontrol *kcontrol,
> +			struct snd_ctl_elem_info *uinfo)
> +{
> +	uinfo->type = SNDRV_CTL_ELEM_TYPE_IEC958;
> +	uinfo->count = 1;
> +	return 0;
> +}
> +
> +static int snd_pcm_iec958_get(struct snd_kcontrol *kcontrol,
> +			      struct snd_ctl_elem_value *uctl)
> +{
> +	struct snd_pcm_iec958_params *params = snd_kcontrol_chip(kcontrol);
> +
> +	if (params->mutex)
> +		mutex_unlock(params->mutex);
> +	uctl->value.iec958.status[0] = params->iec->status[0];
> +	uctl->value.iec958.status[1] = params->iec->status[1];
> +	uctl->value.iec958.status[2] = params->iec->status[2];
> +	uctl->value.iec958.status[3] = params->iec->status[3];
> +	uctl->value.iec958.status[4] = params->iec->status[4];
> +	if (params->mutex)
> +		mutex_unlock(params->mutex);

I'm not sure it makes sense for the mutex to be optional.

> +	return 0;
> +}
> +
> +static int snd_pcm_iec958_put(struct snd_kcontrol *kcontrol,
> +			      struct snd_ctl_elem_value *uctl)
> +{
> +	struct snd_pcm_iec958_params *params = snd_kcontrol_chip(kcontrol);
> +	int err = 0;
> +
> +	if (params->mutex)
> +		mutex_lock(params->mutex);
> +	if (params->ctrl_set)
> +		err = params->ctrl_set(params->pdata,
> +				       uctl->value.iec958.status, 5);
> +	if (!err) {
> +		params->iec->status[0] = uctl->value.iec958.status[0];
> +		params->iec->status[1] = uctl->value.iec958.status[1];
> +		params->iec->status[2] = uctl->value.iec958.status[2];
> +		params->iec->status[3] = uctl->value.iec958.status[3];
> +		params->iec->status[4] = uctl->value.iec958.status[4];
> +	}
> +	if (params->mutex)
> +		mutex_unlock(params->mutex);
> +
> +	return 0;

I think you're supposed to report whether anything changed, rather
than just zero on success.

> +}
> +
> +static const struct snd_kcontrol_new iec958_ctls[] = {
> +	{
> +		.iface = SNDRV_CTL_ELEM_IFACE_PCM,
> +		.name = SNDRV_CTL_NAME_IEC958("", PLAYBACK, DEFAULT),

Shouldn't this have a .access member?  What if the audio driver
modifies the IEC958 status for PCM playback - shouldn't it have
the ability to have SNDRV_CTL_ELEM_ACCESS_VOLATILE added?

-- 
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: [RFC 4/6] drm: sti: Add ASoC generic hdmi codec support.

[Russell King - ARM Linux]

On Tue, Jan 19, 2016 at 02:40:34PM +0100, Arnaud Pouliquen wrote:
> +	/* update N parameter */
> +	ret = hdmi_audio_compute_n_cts(params->sample_rate,
> +				       hdmi->mode.clock, &n_cts);
> +	if (ret < 0)
> +		return ret;
> +
> +	DRM_DEBUG_DRIVER("sample_frequency= %d, pix clock = %d\n",
> +			 params->sample_rate, hdmi->mode.clock);
> +	DRM_DEBUG_DRIVER("n= %d, cts = %d\n", n_cts.n, n_cts.cts);
> +
> +	hdmi_write(hdmi, n_cts.n, HDMI_AUDN);

This looks like you only make use of the N parameter, which suggests
that your hardware measures the CTS parameter itself - which means
you have the non-coherent clock case.  You'll want to refer to the
HDMI specs and use different N values from the coherent case.

-- 
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: [RFC 1/6] video: hdmi: add helper function for N and CTS

[Mark Brown]

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

On Tue, Jan 19, 2016 at 04:54:30PM +0000, Russell King - ARM Linux wrote:
> On Tue, Jan 19, 2016 at 02:40:31PM +0100, Arnaud Pouliquen wrote:
> > From: Moise Gergaud <moise.gergaud@st.com>

> > Add helper function to compute HDMI CTS and N parameters
> > Implementation is based on HDMI 1.4b specification.

> You should note that these CTS/N parameters are for where the audio and
> video clocks are coherent: iow, they are derived from the same source.
> If they can drift (because they're generated from different sources)
> then a hardware implementation which automatically calculates CTS is
> needed.

> However, some of the values given in the HDMI specs for CTS are actually
> two values, because a single value does not specify the audio clock even
> with the N values given.  For these, the values returned will not give
> an accurate clock - this should also be noted.

> Next, does "pixel clock" make sense when you have pixel repetition?
> Would calling this "tdms_clock" be more accurate to the specification
> and less mis-leading to users?

It'd also be good to capture some of the above information from Russell
in the code so other people don't miss it when using the helpers.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

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

Re: [RFC 1/6] video: hdmi: add helper function for N and CTS

[Arnaud Pouliquen]

On 01/19/2016 05:54 PM, Russell King - ARM Linux wrote:
> On Tue, Jan 19, 2016 at 02:40:31PM +0100, Arnaud Pouliquen wrote:
>> From: Moise Gergaud <moise.gergaud@st.com>
>>
>> Add helper function to compute HDMI CTS and N parameters
>> Implementation is based on HDMI 1.4b specification.
> 
> You should note that these CTS/N parameters are for where the audio and
> video clocks are coherent: iow, they are derived from the same source.
> If they can drift (because they're generated from different sources)
> then a hardware implementation which automatically calculates CTS is
> needed.
Yes this is the case on STI platform, CTS is computed by hardware. In
this case hdmi driver only takes into account the N value.
> 
> However, some of the values given in the HDMI specs for CTS are actually
> two values, because a single value does not specify the audio clock even
> with the N values given.  For these, the values returned will not give
> an accurate clock - this should also be noted.
In HDMI specs i only see a double value for 32 kHz with TMDS clock at
74.324  MHz. I can add a  alt_CTS params in hdmi_audio_n_cts that will
be not null if alternate a value is needed. Is this something reasonable
for you?
As also suggest Mark, i need to add all the restriction info in code.

> 
> Next, does "pixel clock" make sense when you have pixel repetition?
> Would calling this "tdms_clock" be more accurate to the specification
> and less mis-leading to users?
> 
>> Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@st.com>
>> ---
>>  drivers/video/hdmi.c | 146 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>  include/linux/hdmi.h |  11 ++++
>>  2 files changed, 157 insertions(+)
>>
>> diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
>> index 1626892..8af33c1 100644
>> --- a/drivers/video/hdmi.c
>> +++ b/drivers/video/hdmi.c
>> @@ -1242,3 +1242,149 @@ int hdmi_infoframe_unpack(union hdmi_infoframe *frame, void *buffer)
>>  	return ret;
>>  }
>>  EXPORT_SYMBOL(hdmi_infoframe_unpack);
>> +
>> +/**
>> + * audio clock regeneration (acr) parameters
>> + * N and CTS computation are based on HDMI specification 1.4b
>> + */
>> +enum audio_rate {
>> +	HDMI_AUDIO_N_CTS_32KHZ,
>> +	HDMI_AUDIO_N_CTS_44_1KHZ,
>> +	HDMI_AUDIO_N_CTS_48KHZ,
>> +};
>> +
>> +struct hdmi_audio_acr {
>> +	unsigned long pixel_clk;
>> +	struct hdmi_audio_n_cts n_cts;
>> +};
>> +
>> +static const struct hdmi_audio_acr hdmi_audio_standard_acr[3][12] = {
>> +	{ /*32 kHz*/
>> +		{  25175, {  4576,  28125 } }, /* 25,20/1.001  MHz */
>> +		{  25200, {  4096,  25200 } }, /* 25.20        MHz */
>> +		{  27000, {  4096,  27000 } }, /* 27.00        MHz */
>> +		{  27027, {  4096,  27027 } }, /* 27.00*1.001  MHz */
>> +		{  54000, {  4096,  54000 } }, /* 54.00        MHz */
>> +		{  54054, {  4096,  54054 } }, /* 54.00*1.001  MHz */
>> +		{  74176, { 11648, 310938 } }, /* 74.25/1.001  MHz */
>> +		{  74250, {  4096,  74250 } }, /* 74.25        MHz */
>> +		{ 148352, { 11648, 421875 } }, /* 148.50/1.001 MHz */
>> +		{ 148500, {  4096, 148500 } }, /* 148.50       MHz */
>> +		{ 296703, {  5824, 421875 } }, /* 297/1.001    MHz */
>> +		{ 297000, {  3072, 222750 } }, /* 297          MHz */
>> +	},
>> +	{ /*44.1 kHz, 88.2 kHz  176.4 kHz*/
>> +		{  25175, {  7007,  31250 } }, /* 25,20/1.001  MHz */
>> +		{  25200, {  6272,  28000 } }, /* 25.20        MHz */
>> +		{  27000, {  6272,  30000 } }, /* 27.00        MHz */
>> +		{  27027, {  6272,  30030 } }, /* 27.00*1.001  MHz */
>> +		{  54000, {  6272,  60000 } }, /* 54.00        MHz */
>> +		{  54054, {  6272,  60060 } }, /* 54.00*1.001  MHz */
>> +		{  74176, { 17836, 234375 } }, /* 74.25/1.001  MHz */
>> +		{  74250, {  6272,  82500 } }, /* 74.25        MHz */
>> +		{ 148352, {  8918, 234375 } }, /* 148.50/1.001 MHz */
>> +		{ 148500, {  6272, 165000 } }, /* 148.50       MHz */
>> +		{ 296703, {  4459, 234375 } }, /* 297/1.001    MHz */
>> +		{ 297000, {  4704, 247500 } }, /* 297          MHz */
>> +	},
>> +	{ /*48 kHz, 96 kHz  192 kHz*/
>> +		{  25175, {  6864,  28125 } }, /* 25,20/1.001  MHz */
>> +		{  25200, {  6144,  25200 } }, /* 25.20        MHz */
>> +		{  27000, {  6144,  27000 } }, /* 27.00        MHz */
>> +		{  27027, {  6144,  27027 } }, /* 27.00*1.001  MHz */
>> +		{  54000, {  6144,  54000 } }, /* 54.00        MHz */
>> +		{  54054, {  6144,  54054 } }, /* 54.00*1.001  MHz */
>> +		{  74176, { 11648, 140625 } }, /* 74.25/1.001  MHz */
>> +		{  74250, {  6144,  74250 } }, /* 74.25        MHz */
>> +		{ 148352, {  5824, 140625 } }, /* 148.50/1.001 MHz */
>> +		{ 148500, {  6144, 148500 } }, /* 148.50       MHz */
>> +		{ 296703, {  5824, 281250 } }, /* 297/1.001    MHz */
>> +		{ 297000, {  5120, 247500 } }, /* 297          MHz */
>> +	}
>> +};
>> +
>> +/**
>> + * hdmi_compute_n_cts() - compute N and CTS parameters
>> + * @audio_fs: audio frame clock frequency in kHz
> 
> This is wrong.  You're expecting values such as 32000 here.  32000kHz
> would be 32MHz, which is way too high for an audio frame clock.  I
> think you mean Hz.
> 
Oops, yes Hz not kHz
>> + * @pixel_clk: pixel clock frequency in kHz
> 
> This is correct though.
> 
>> + * @n_cts: N and CTS parameter returned to user
>> + *
>> + * Values computed are based on table described in HDMI specification 1.4b
>> + *
>> + * Returns 0 on success or a negative error code on failure.
>> + */
>> +int hdmi_audio_compute_n_cts(unsigned int audio_fs, unsigned long pixel_clk,
>> +			     struct hdmi_audio_n_cts *n_cts)
>> +{
>> +	int audio_freq_id, i;
>> +	int ratio = 1;
>> +	const struct hdmi_audio_acr  *acr_table;
>> +	const struct hdmi_audio_n_cts *predef_n_cts = NULL;
>> +
>> +	switch (audio_fs) {
>> +	case 32000:
>> +		audio_freq_id = HDMI_AUDIO_N_CTS_32KHZ;
>> +		n_cts->n = 4096;
>> +		break;
>> +
>> +	case 44100:
>> +		audio_freq_id = HDMI_AUDIO_N_CTS_44_1KHZ;
>> +		n_cts->n = 6272;
>> +		break;
>> +
>> +	case 48000:
>> +		audio_freq_id = HDMI_AUDIO_N_CTS_48KHZ;
>> +		n_cts->n = 6144;
>> +		break;
>> +
>> +	case 88200:
>> +		audio_freq_id = HDMI_AUDIO_N_CTS_44_1KHZ;
>> +		ratio = 2;
>> +		n_cts->n = 6272 * 2;
>> +		break;
>> +
>> +	case 96000:
>> +		audio_freq_id = HDMI_AUDIO_N_CTS_48KHZ;
>> +		ratio = 2;
>> +		n_cts->n = 6144 * 2;
>> +		break;
>> +
>> +	case 176400:
>> +		audio_freq_id = HDMI_AUDIO_N_CTS_44_1KHZ;
>> +		ratio = 4;
>> +		n_cts->n = 6272 * 4;
>> +		break;
>> +
>> +	case 192000:
>> +		audio_freq_id = HDMI_AUDIO_N_CTS_48KHZ;
>> +		ratio = 4;
>> +		n_cts->n = 6144 * 4;
>> +		break;
>> +
>> +	default:
>> +		return -EINVAL;
>> +	}
>> +
>> +	acr_table = hdmi_audio_standard_acr[audio_freq_id];
>> +	for (i = 0; i < ARRAY_SIZE(hdmi_audio_standard_acr[0]); i++) {
>> +		if (pixel_clk == acr_table[i].pixel_clk) {
>> +			predef_n_cts = &acr_table[i].n_cts;
>> +			break;
>> +		}
>> +	}
>> +
>> +	if (!predef_n_cts) {
>> +		/*
>> +		 * predefined frequency not found. Compute CTS using formula:
>> +		 * CTS = (Ftdms_clk * N) / (128* audio_fs)
>> +		 */
>> +		n_cts->cts =  pixel_clk * n_cts->n / (128 * audio_fs);
> 
> This looks like it could overflow.  192kHz audio clock, 297MHz tdms
> clock -> (297000 * 6144 * 4) = 7299072000.  32 bit unsigned math
> overflows beyond 4294967295.
> 
Yes, need to use intermediate 64 bits variable for calculation

Re: [RFC 1/6] video: hdmi: add helper function for N and CTS

[Russell King - ARM Linux]

On Wed, Jan 20, 2016 at 02:26:08PM +0100, Arnaud Pouliquen wrote:
> 
> 
> On 01/19/2016 05:54 PM, Russell King - ARM Linux wrote:
> > However, some of the values given in the HDMI specs for CTS are actually
> > two values, because a single value does not specify the audio clock even
> > with the N values given.  For these, the values returned will not give
> > an accurate clock - this should also be noted.
>
> In HDMI specs i only see a double value for 32 kHz with TMDS clock at
> 74.324  MHz. I can add a  alt_CTS params in hdmi_audio_n_cts that will
> be not null if alternate a value is needed. Is this something reasonable
> for you?

They aren't "alternatives".  They give two values because, in order to
generate that audio sample rate, you need to switch between the two
values to achieve an average audio sample rate that matches the requested
rate.  I suspect that unless you have special hardware which does that
for you automatically, these values are less than ideal.

For example, for 32kHz:

74.25 / 1.001              11648               210937-210938*

which is:

 74250000 / (1.001 * 210937 * 128) * 11648 = 32000.076
 74250000 / (1.001 * 210938 * 128) * 11648 = 31999.924

So, alternating between each for 50% of the time gives 32kHz.

For my other points:

  Section 7.2.3:
  It is recommended that Sources with non-coherent clocks use the
  values listed for a TMDS clock of "Other".

As you have non-coherent clocks, and your CTS value is measured by
the hardware, using the "Other" N values seems to be recommended by
the standard.

-- 
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: [RFC 2/6] ALSA: pcm: add IEC958 channel status control helper

[Arnaud Pouliquen]

On 01/19/2016 06:00 PM, Russell King - ARM Linux wrote:
> On Tue, Jan 19, 2016 at 02:40:32PM +0100, Arnaud Pouliquen wrote:

>> +static int snd_pcm_iec958_get(struct snd_kcontrol *kcontrol,
>> +			      struct snd_ctl_elem_value *uctl)
>> +{
>> +	struct snd_pcm_iec958_params *params = snd_kcontrol_chip(kcontrol);
>> +
>> +	if (params->mutex)
>> +		mutex_unlock(params->mutex);
>> +	uctl->value.iec958.status[0] = params->iec->status[0];
>> +	uctl->value.iec958.status[1] = params->iec->status[1];
>> +	uctl->value.iec958.status[2] = params->iec->status[2];
>> +	uctl->value.iec958.status[3] = params->iec->status[3];
>> +	uctl->value.iec958.status[4] = params->iec->status[4];
>> +	if (params->mutex)
>> +		mutex_unlock(params->mutex);
> 
> I'm not sure it makes sense for the mutex to be optional.
I have no use case in mind that justifies optional mutex.
Just did it for flexibility...
I can suppress condition to force drivers to use it.

> 
>> +	return 0;
>> +}
>> +
>> +static int snd_pcm_iec958_put(struct snd_kcontrol *kcontrol,
>> +			      struct snd_ctl_elem_value *uctl)
>> +{
>> +	struct snd_pcm_iec958_params *params = snd_kcontrol_chip(kcontrol);
>> +	int err = 0;
>> +
>> +	if (params->mutex)
>> +		mutex_lock(params->mutex);
>> +	if (params->ctrl_set)
>> +		err = params->ctrl_set(params->pdata,
>> +				       uctl->value.iec958.status, 5);
>> +	if (!err) {
>> +		params->iec->status[0] = uctl->value.iec958.status[0];
>> +		params->iec->status[1] = uctl->value.iec958.status[1];
>> +		params->iec->status[2] = uctl->value.iec958.status[2];
>> +		params->iec->status[3] = uctl->value.iec958.status[3];
>> +		params->iec->status[4] = uctl->value.iec958.status[4];
>> +	}
>> +	if (params->mutex)
>> +		mutex_unlock(params->mutex);
>> +
>> +	return 0;
> 
> I think you're supposed to report whether anything changed, rather
> than just zero on success.
right, need to return 1 or err
> 
>> +}
>> +
>> +static const struct snd_kcontrol_new iec958_ctls[] = {
>> +	{
>> +		.iface = SNDRV_CTL_ELEM_IFACE_PCM,
>> +		.name = SNDRV_CTL_NAME_IEC958("", PLAYBACK, DEFAULT),
> 
> Shouldn't this have a .access member?  What if the audio driver
> modifies the IEC958 status for PCM playback - shouldn't it have
> the ability to have SNDRV_CTL_ELEM_ACCESS_VOLATILE added?
> 
i will add READ + VOLATILE access for capture, RW +VOLATILE access for
playback.
Should i also add some other controls by default, like controls for
consumer and professional mask for playback?