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

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

[Arnaud Pouliquen]

V2:
     - patch: ALSA: pcm: add IEC958 channel status control helper
          - Return 1 instead of 0 in snd_pcm_iec958_put
          - Add .access field in control structure
          - I have kept condition on mutex for flexibility 
            (but could be cleaned to force user to use a mutex)
     - patch: video: hdmi: add helper function for N and CTS
         - Duplicate function to have a separte treatment for coherent and
           non-coherent clcocks
         - Add ratio fiel for alternate CTS value
         - Clock frequency in Hz for TMDS and audio clocks
         - Add information concerning clocks and CTS calculation. 

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

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       | 248 ++++++++++++++++++++++++++++++++---
 drivers/gpu/drm/sti/sti_hdmi.h       |  10 ++
 drivers/video/hdmi.c                 | 222 +++++++++++++++++++++++++++++++
 include/linux/hdmi.h                 |  22 ++++
 include/sound/hdmi-codec.h           |   1 +
 include/sound/pcm_iec958.h           |  16 +++
 include/sound/soc-dai.h              |   7 +
 sound/core/pcm_iec958.c              |  99 ++++++++++++++
 sound/soc/codecs/hdmi-codec.c        |  59 +++++++--
 sound/soc/soc-core.c                 |  14 ++
 12 files changed, 682 insertions(+), 32 deletions(-)

-- 
1.9.1

[RFC v2 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 | 222 +++++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/hdmi.h |  22 +++++
 2 files changed, 244 insertions(+)

diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
index 1626892..be8b8ed 100644
--- a/drivers/video/hdmi.c
+++ b/drivers/video/hdmi.c
@@ -1242,3 +1242,225 @@ 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 int tmds_clk;
+	struct hdmi_audio_n_cts n_cts;
+};
+
+static const struct hdmi_audio_acr hdmi_audio_standard_acr[3][12] = {
+	{ /*32 kHz*/
+		{  25174825, {  4576,  28125, 0 } }, /* 25,20/1.001  MHz */
+		{  25200000, {  4096,  25200, 0 } }, /* 25.20        MHz */
+		{  27000000, {  4096,  27000, 0 } }, /* 27.00        MHz */
+		{  27027000, {  4096,  27027, 0 } }, /* 27.00*1.001  MHz */
+		{  54000000, {  4096,  54000, 0 } }, /* 54.00        MHz */
+		{  54054000, {  4096,  54054, 0 } }, /* 54.00*1.001  MHz */
+		{  74175824, { 11648, 210937, 50 } }, /* 74.25/1.001  MHz */
+		{  74250000, {  4096,  74250, 0 } }, /* 74.25        MHz */
+		{ 148351648, { 11648, 421875, 0 } }, /* 148.50/1.001 MHz */
+		{ 148500000, {  4096, 148500, 0 } }, /* 148.50       MHz */
+		{ 296703296, {  5824, 421875, 0 } }, /* 297/1.001    MHz */
+		{ 297000000, {  3072, 222750, 0 } }, /* 297          MHz */
+	},
+	{ /*44.1 kHz, 88.2 kHz  176.4 kHz*/
+		{  25174825, {  7007,  31250, 0 } }, /* 25,20/1.001  MHz */
+		{  25200000, {  6272,  28000, 0 } }, /* 25.20        MHz */
+		{  27000000, {  6272,  30000, 0 } }, /* 27.00        MHz */
+		{  27027000, {  6272,  30030, 0 } }, /* 27.00*1.001  MHz */
+		{  54000000, {  6272,  60000, 0 } }, /* 54.00        MHz */
+		{  54054000, {  6272,  60060, 0 } }, /* 54.00*1.001  MHz */
+		{  74175824, { 17836, 234375, 0 } }, /* 74.25/1.001  MHz */
+		{  74250000, {  6272,  82500, 0 } }, /* 74.25        MHz */
+		{ 148351648, {  8918, 234375, 0 } }, /* 148.50/1.001 MHz */
+		{ 148500000, {  6272, 165000, 0 } }, /* 148.50       MHz */
+		{ 296703296, {  4459, 234375, 0 } }, /* 297/1.001    MHz */
+		{ 297000000, {  4704, 247500, 0 } }, /* 297          MHz */
+	},
+	{ /*48 kHz, 96 kHz  192 kHz*/
+		{  25174825, {  6864,  28125, 0 } }, /* 25,20/1.001  MHz */
+		{  25200000, {  6144,  25200, 0 } }, /* 25.20        MHz */
+		{  27000000, {  6144,  27000, 0 } }, /* 27.00        MHz */
+		{  27027000, {  6144,  27027, 0 } }, /* 27.00*1.001  MHz */
+		{  54000000, {  6144,  54000, 0 } }, /* 54.00        MHz */
+		{  54054000, {  6144,  54054, 0 } }, /* 54.00*1.001  MHz */
+		{  74175824, { 11648, 140625, 0 } }, /* 74.25/1.001  MHz */
+		{  74250000, {  6144,  74250, 0 } }, /* 74.25        MHz */
+		{ 148351648, {  5824, 140625, 0 } }, /* 148.50/1.001 MHz */
+		{ 148500000, {  6144, 148500, 0 } }, /* 148.50       MHz */
+		{ 296703296, {  5824, 281250, 0 } }, /* 297/1.001    MHz */
+		{ 297000000, {  5120, 247500, 0 } }, /* 297          MHz */
+	}
+};
+
+/**
+ * hdmi_audio_get_coherent_n_cts() - compute N and CTS parameters for coherent
+ * clocks. Coherent clock means that audio and TMDS clocks have the same
+ * source (no drifts between clocks).
+ *
+ * @audio_fs: audio frame clock frequency in Hz
+ * @tmds_clk: HDMI TMDS clock frequency in Hz
+ * @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_get_coherent_n_cts(unsigned int audio_fs,
+				  unsigned int tmds_clk,
+				  struct hdmi_audio_n_cts *n_cts)
+{
+	int audio_freq_id, i;
+	int rate_coeff = 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;
+		rate_coeff = 2;
+		n_cts->n = 6272 * 2;
+		break;
+
+	case 96000:
+		audio_freq_id = HDMI_AUDIO_N_CTS_48KHZ;
+		rate_coeff = 2;
+		n_cts->n = 6144 * 2;
+		break;
+
+	case 176400:
+		audio_freq_id = HDMI_AUDIO_N_CTS_44_1KHZ;
+		rate_coeff = 4;
+		n_cts->n = 6272 * 4;
+		break;
+
+	case 192000:
+		audio_freq_id = HDMI_AUDIO_N_CTS_48KHZ;
+		rate_coeff = 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 (tmds_clk == acr_table[i].tmds_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)
+		 */
+		uint64_t val, min;
+
+		val = (uint64_t)tmds_clk * n_cts->n;
+
+		n_cts->cts = div64_u64(val, 128UL * audio_fs);
+		min = (uint64_t)n_cts->cts * 128UL * audio_fs;
+		if (min < (val)) {
+			/*
+			 * non-accurate value for CTS
+			 * compute ratio, needed by user to alternate in ACR
+			 * between CTS and CTS + 1 value.
+			 */
+			n_cts->cts_1_ratio = ((int32_t)(val - min)) * 100 /
+					     (128 * audio_fs);
+			n_cts->cts_1_ratio = 100 - n_cts->cts_1_ratio;
+		} else {
+			n_cts->cts_1_ratio = 0;
+		}
+	} else {
+		n_cts->n = predef_n_cts->n * rate_coeff;
+		n_cts->cts = predef_n_cts->cts;
+		n_cts->cts_1_ratio = predef_n_cts->cts_1_ratio;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL(hdmi_audio_get_coherent_n_cts);
+
+/**
+ * hdmi_audio_get_non_coherent_n() - get N parameter for non-coherent
+ * clocks. None-coherent clocks means that audio and TMDS clocks have not the
+ * same source (drifts between clocks). In this case assumption is that CTS is
+ * automatically calculated by hardware.
+ *
+ * @audio_fs: audio frame clock frequency in Hz
+ *
+ * Values computed are based on table described in HDMI specification 1.4b
+ *
+ * Returns n value.
+ */
+int hdmi_audio_get_non_coherent_n(unsigned int audio_fs)
+{
+	unsigned int n;
+
+	switch (audio_fs) {
+	case 32000:
+		n = 4096;
+		break;
+
+	case 44100:
+		n = 6272;
+		break;
+
+	case 48000:
+		n = 6144;
+		break;
+
+	case 88200:
+		n = 6272 * 2;
+		break;
+
+	case 96000:
+		n = 6144 * 2;
+		break;
+
+	case 176400:
+		n = 6272 * 4;
+		break;
+
+	case 192000:
+		n = 6144 * 4;
+		break;
+
+	default:
+		/* Not pre-defined, recommended value: 128 * fs / 1000 */
+		n = (audio_fs * 128) / 1000;
+	}
+
+	return n;
+}
+EXPORT_SYMBOL(hdmi_audio_get_non_coherent_n);
+
diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h
index e974420..bf5c136 100644
--- a/include/linux/hdmi.h
+++ b/include/linux/hdmi.h
@@ -333,4 +333,26 @@ 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
+ * @n: N parameter
+ * @cts: CTS parameter
+ * @cts_1_ratio: ratio from 0 to 99 to alternate "CTS" and "CTS + 1" values
+ *  ratio = 0: CTS parameter is accurate, no need to alternate with "CTS + 1"
+ *             value
+ *  ratio = x: Need to alternate in ACR "CTS + 1" value x% of the time to
+ *             generate accurate audio clock
+ */
+struct hdmi_audio_n_cts {
+	unsigned int n;
+	unsigned int cts;
+	unsigned int cts_1_ratio;
+};
+
+int hdmi_audio_get_coherent_n_cts(unsigned int audio_fs,
+				  unsigned int tmds_clk,
+				  struct hdmi_audio_n_cts *n_cts);
+
+int hdmi_audio_get_non_coherent_n(unsigned int audio_fs);
+
 #endif /* _DRM_HDMI_H */
-- 
1.9.1

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

[Philipp Zabel]

Hi Arnaud,

Am Freitag, den 22.01.2016, 18:48 +0100 schrieb 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>

Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
Tested-by: Philipp Zabel <p.zabel@pengutronix.de>

> ---
>  drivers/video/hdmi.c | 222 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/hdmi.h |  22 +++++
>  2 files changed, 244 insertions(+)
> 
> diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
> index 1626892..be8b8ed 100644
> --- a/drivers/video/hdmi.c
> +++ b/drivers/video/hdmi.c
> @@ -1242,3 +1242,225 @@ 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 int tmds_clk;
> +	struct hdmi_audio_n_cts n_cts;
> +};
> +
> +static const struct hdmi_audio_acr hdmi_audio_standard_acr[3][12] = {
> +	{ /*32 kHz*/
> +		{  25174825, {  4576,  28125, 0 } }, /* 25,20/1.001  MHz */
> +		{  25200000, {  4096,  25200, 0 } }, /* 25.20        MHz */
> +		{  27000000, {  4096,  27000, 0 } }, /* 27.00        MHz */
> +		{  27027000, {  4096,  27027, 0 } }, /* 27.00*1.001  MHz */
> +		{  54000000, {  4096,  54000, 0 } }, /* 54.00        MHz */
> +		{  54054000, {  4096,  54054, 0 } }, /* 54.00*1.001  MHz */
> +		{  74175824, { 11648, 210937, 50 } }, /* 74.25/1.001  MHz */
> +		{  74250000, {  4096,  74250, 0 } }, /* 74.25        MHz */
> +		{ 148351648, { 11648, 421875, 0 } }, /* 148.50/1.001 MHz */
> +		{ 148500000, {  4096, 148500, 0 } }, /* 148.50       MHz */
> +		{ 296703296, {  5824, 421875, 0 } }, /* 297/1.001    MHz */

297/1.001 is about 296703296.7. Should this be rounded to 296703297 ?

regards
Philipp

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

[Arnaud Pouliquen]

On 02/18/2016 03:20 PM, Philipp Zabel wrote:
> Hi Arnaud,
> 
> Am Freitag, den 22.01.2016, 18:48 +0100 schrieb 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>
> 
> Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
> Tested-by: Philipp Zabel <p.zabel@pengutronix.de>
> 
>> ---
>>  drivers/video/hdmi.c | 222 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>  include/linux/hdmi.h |  22 +++++
>>  2 files changed, 244 insertions(+)
>>
>> diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
>> index 1626892..be8b8ed 100644
>> --- a/drivers/video/hdmi.c
>> +++ b/drivers/video/hdmi.c
>> @@ -1242,3 +1242,225 @@ 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 int tmds_clk;
>> +	struct hdmi_audio_n_cts n_cts;
>> +};
>> +
>> +static const struct hdmi_audio_acr hdmi_audio_standard_acr[3][12] = {
>> +	{ /*32 kHz*/
>> +		{  25174825, {  4576,  28125, 0 } }, /* 25,20/1.001  MHz */
>> +		{  25200000, {  4096,  25200, 0 } }, /* 25.20        MHz */
>> +		{  27000000, {  4096,  27000, 0 } }, /* 27.00        MHz */
>> +		{  27027000, {  4096,  27027, 0 } }, /* 27.00*1.001  MHz */
>> +		{  54000000, {  4096,  54000, 0 } }, /* 54.00        MHz */
>> +		{  54054000, {  4096,  54054, 0 } }, /* 54.00*1.001  MHz */
>> +		{  74175824, { 11648, 210937, 50 } }, /* 74.25/1.001  MHz */
>> +		{  74250000, {  4096,  74250, 0 } }, /* 74.25        MHz */
>> +		{ 148351648, { 11648, 421875, 0 } }, /* 148.50/1.001 MHz */
>> +		{ 148500000, {  4096, 148500, 0 } }, /* 148.50       MHz */
>> +		{ 296703296, {  5824, 421875, 0 } }, /* 297/1.001    MHz */
> 
> 297/1.001 is about 296703296.7. Should this be rounded to 296703297 ?
For this value, the rounded and truncate values are different. I will
add both value in table.

Thanks and Regards
Arnaud

[RFC v2 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    | 99 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 115 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..8dd0415 100644
--- a/sound/core/pcm_iec958.c
+++ b/sound/core/pcm_iec958.c
@@ -7,11 +7,87 @@
  */
 #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 < 0) {
+		if (params->mutex)
+			mutex_unlock(params->mutex);
+		return 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 1;
+}
+
+static const struct snd_kcontrol_new iec958_ctls[] = {
+	{
+		.access = (SNDRV_CTL_ELEM_ACCESS_READWRITE |
+			   SNDRV_CTL_ELEM_ACCESS_VOLATILE),
+		.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,
+	},
+	{
+		.access = (SNDRV_CTL_ELEM_ACCESS_READ |
+			   SNDRV_CTL_ELEM_ACCESS_VOLATILE),
+		.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 +187,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

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

[Jyri Sarha]

On 01/22/16 19:48, 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    | 99 ++++++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 115 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..8dd0415 100644
> --- a/sound/core/pcm_iec958.c
> +++ b/sound/core/pcm_iec958.c
> @@ -7,11 +7,87 @@
>    */
>   #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);

This here should be mutex_lock().

Cheers,
Jyri

> +	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 < 0) {
> +		if (params->mutex)
> +			mutex_unlock(params->mutex);
> +		return 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 1;
> +}
> +
> +static const struct snd_kcontrol_new iec958_ctls[] = {
> +	{
> +		.access = (SNDRV_CTL_ELEM_ACCESS_READWRITE |
> +			   SNDRV_CTL_ELEM_ACCESS_VOLATILE),
> +		.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,
> +	},
> +	{
> +		.access = (SNDRV_CTL_ELEM_ACCESS_READ |
> +			   SNDRV_CTL_ELEM_ACCESS_VOLATILE),
> +		.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 +187,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);
>

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

[Arnaud Pouliquen]

On 02/16/2016 09:17 PM, Jyri Sarha wrote:
> On 01/22/16 19:48, 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    | 99 ++++++++++++++++++++++++++++++++++++++++++++++
>>   2 files changed, 115 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..8dd0415 100644
>> --- a/sound/core/pcm_iec958.c
>> +++ b/sound/core/pcm_iec958.c
>> @@ -7,11 +7,87 @@
>>    */
>>   #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);
> 
> This here should be mutex_lock().
oops... Seems that i need to enable some debug configs in my environment...
My apologize for this copy/past and thanks for the test!
Anyway i need to perform further tests before deliver as patches instead
of rfc.
But i prefer to wait a decision on the HDMI codec before finalizing it.

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

[Russell King - ARM Linux]

On Fri, Jan 22, 2016 at 06:48:28PM +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 thought I had already commented about the mutex - maybe not.  Please
don't make these conditional like this: if you need the mutex to
eliminate a race condition, then you need it to eliminate the race
condition and it can't be "optional".  Elimination of race conditions
is never optional!

Please get rid of all these conditions and make it mandatory that a
mutex is supplied.

Thanks.

-- 
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 v2 2/6] ALSA: pcm: add IEC958 channel status control helper

[Arnaud Pouliquen]

On 02/17/2016 01:31 AM, Russell King - ARM Linux wrote:
> On Fri, Jan 22, 2016 at 06:48:28PM +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 thought I had already commented about the mutex - maybe not.  Please
> don't make these conditional like this: if you need the mutex to
> eliminate a race condition, then you need it to eliminate the race
> condition and it can't be "optional".  Elimination of race conditions
> is never optional!
> 
> Please get rid of all these conditions and make it mandatory that a
> mutex is supplied.
> 
> Thanks.

Ok, i will suppress condition. I also need to use a spinlock instead of
the mutex, to be able to support race condition with atomic callbacks
(trigger, pointer, and ack)

[RFC v2 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..cf1d312 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

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

[Jyri Sarha]

On 01/22/16 19:48, Arnaud Pouliquen wrote:
> 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.
>

Sorry for my late reaction to this series (I was on vacation)...

I do not follow why you need this? The rest of the codecs create their 
mixer elements in static struct snd_soc_codec_driver's probe-callback. I 
can not see why it could not be used with hdmi-codec (then again I have 
not tried it).

I'll try to test the patches that affects my Beaglebone-black hdmia 
audio series anyway, I just need to rebase my branch in order to apply 
this patch. For some reason "ASoc: hdmi-codec: add IEC control." did not 
apply out of the box either...

Best regards,
Jyri

> 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..cf1d312 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;
>   }
>
>

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

[Arnaud Pouliquen]

On 02/05/2016 10:58 AM, Jyri Sarha wrote:
> On 01/22/16 19:48, Arnaud Pouliquen wrote:
>> 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.
>>
> 
> Sorry for my late reaction to this series (I was on vacation)...
Same issue for me last week ;-)

> 
> I do not follow why you need this? The rest of the codecs create their 
> mixer elements in static struct snd_soc_codec_driver's probe-callback. I 
> can not see why it could not be used with hdmi-codec (then again I have 
> not tried it).
Some controls like 'IEC958 Playback Default' can need to be indexed on
the PCM device. As example on my board i have a SPDIF and a HDMI out.
Both handle IEC60958. Today, there is not way (or as said, i miss
something) to get PCM device information at DAI level to be able to
index on PCM device.
Aim of this patch is to allow to perform post DAI initialization on PCM
device creation, with PCM runtime information.

> 
> I'll try to test the patches that affects my Beaglebone-black hdmia 
> audio series anyway, I just need to rebase my branch in order to apply 
> this patch. For some reason "ASoc: hdmi-codec: add IEC control." did not 
> apply out of the box either...
> 
> Best regards,
> Jyri
> 
>> 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..cf1d312 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;
>>   }
>>
>>
> 

[RFC v2 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 | 248 ++++++++++++++++++++++++++++++++++++++---
 drivers/gpu/drm/sti/sti_hdmi.h |  10 ++
 3 files changed, 242 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..9c69f78 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,167 @@ 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, n;
+	struct hdmi_audio_infoframe *info = &params->cea;
+
+	DRM_DEBUG_DRIVER("\n");
+
+	if (!hdmi->enabled)
+		return 0;
+
+	/* update N parameter */
+	n = hdmi_audio_get_non_coherent_n(params->sample_rate);
+
+	DRM_DEBUG_DRIVER("audio rate = %d Hz, TMDS clock = %d Hz, n = %d\n",
+			 params->sample_rate, hdmi->mode.clock * 1000, n);
+	hdmi_write(hdmi, 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,
+		.iec_ctl = 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 +932,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 +956,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 +1106,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 v2 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

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

[Jyri Sarha]

On 01/22/16 19:48, Arnaud Pouliquen wrote:
> Create 'IEC958 Playback Default' controls to support IEC61937 formats.
> the use of the alsa control is optional, using 'iec_ctl' flag.
>

I applied the patches "ALSA: pcm: add IEC958 channel status control 
helper", "ASoC: core: add code to complete dai init after pcm creation", 
and "ASoc: hdmi-codec: add IEC control" to my BBB HDMI audio branch. I 
needed to do some trivial conflict solving, but after that everything 
compiled fine. However, when I tried to read the iec mixer with:

# amixer -c0 cget name='IEC958 Playback Default',device=0
amixer: Cannot find the given element from control hw:0

The same command worked just fine on my intel based laptop:
# amixer -c0 cget iface=MIXER,name='IEC958 Playback Default',device=0
numid=31,iface=MIXER,name='IEC958 Playback Default'
   ; type=IEC958,access=rw------,values=1
   : values=[AES0=0x04 AES1=0x00 AES2=0x00 AES3=0x00]

How did you test the mixer yourself?

Best regards,
Jyri

> 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);
>
>

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

[Arnaud Pouliquen]

On 02/06/2016 08:29 PM, Jyri Sarha wrote:
> On 01/22/16 19:48, Arnaud Pouliquen wrote:
>> Create 'IEC958 Playback Default' controls to support IEC61937 formats.
>> the use of the alsa control is optional, using 'iec_ctl' flag.
>>
> 
> I applied the patches "ALSA: pcm: add IEC958 channel status control 
> helper", "ASoC: core: add code to complete dai init after pcm creation", 
> and "ASoc: hdmi-codec: add IEC control" to my BBB HDMI audio branch. I 
> needed to do some trivial conflict solving, but after that everything 
> compiled fine. However, when I tried to read the iec mixer with:
> 
> # amixer -c0 cget name='IEC958 Playback Default',device=0
> amixer: Cannot find the given element from control hw:0
> 
> The same command worked just fine on my intel based laptop:
> # amixer -c0 cget iface=MIXER,name='IEC958 Playback Default',device=0
> numid=31,iface=MIXER,name='IEC958 Playback Default'
>    ; type=IEC958,access=rw------,values=1
>    : values=[AES0=0x04 AES1=0x00 AES2=0x00 AES3=0x00]
Do you enable "iec_ctl" field in hdmi_codec_pdata structure?
i add this field because control can be declared and used by CPU DAI or
codec, depending on hardware.

> 
> How did you test the mixer yourself?
To test on my platform i hacked my code because control is handled by
CPU_DAI...
But just by disabling control creation in CPU DAI for HDMI and set
"iec_ctl" filed to 1, i can see and use the control.

**** List of PLAYBACK Hardware Devices ****
card 0: Default [sti audio card], device 0: Uni Player #0
(HDMI)-i2s-hifi i2s-hifi-0 []
card 0: Default [sti audio card], device 1: Uni Player #1
(DAC)-sas-dai-dac sas-dai-dac-1 []
card 0: Default [sti audio card], device 2: Uni Player #1
(PIO)-sas-dai-spdif-out sas-dai-spdif-out-2 []

# amixer -c0 controls | grep IEC958
numid=5,iface=PCM,name='IEC958 Playback Default'
numid=3,iface=PCM,name='IEC958 Playback Default',device=3


BR,
Arnaud
> 
> Best regards,
> Jyri
> 
>> 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);
>>
>>
> 

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

[Jyri Sarha]

On 02/15/16 12:51, Arnaud Pouliquen wrote:
>
>
> On 02/06/2016 08:29 PM, Jyri Sarha wrote:
>> On 01/22/16 19:48, Arnaud Pouliquen wrote:
>>> Create 'IEC958 Playback Default' controls to support IEC61937 formats.
>>> the use of the alsa control is optional, using 'iec_ctl' flag.
>>>
>>
>> I applied the patches "ALSA: pcm: add IEC958 channel status control
>> helper", "ASoC: core: add code to complete dai init after pcm creation",
>> and "ASoc: hdmi-codec: add IEC control" to my BBB HDMI audio branch. I
>> needed to do some trivial conflict solving, but after that everything
>> compiled fine. However, when I tried to read the iec mixer with:
>>
>> # amixer -c0 cget name='IEC958 Playback Default',device=0
>> amixer: Cannot find the given element from control hw:0
>>
>> The same command worked just fine on my intel based laptop:
>> # amixer -c0 cget iface=MIXER,name='IEC958 Playback Default',device=0
>> numid=31,iface=MIXER,name='IEC958 Playback Default'
>>     ; type=IEC958,access=rw------,values=1
>>     : values=[AES0=0x04 AES1=0x00 AES2=0x00 AES3=0x00]
> Do you enable "iec_ctl" field in hdmi_codec_pdata structure?
> i add this field because control can be declared and used by CPU DAI or
> codec, depending on hardware.
>
>>
>> How did you test the mixer yourself?
> To test on my platform i hacked my code because control is handled by
> CPU_DAI...
> But just by disabling control creation in CPU DAI for HDMI and set
> "iec_ctl" filed to 1, i can see and use the control.
>

Oh, my mistake. Simply overlooked the iec_ctl. However, when testing the 
mixer element again, I found another problem. When getting the IEC958 
value, I got following dump:

  [  690.127298]
[  690.128957] =====================================
[  690.133956] [ BUG: bad unlock balance detected! ]
[  690.138965] 4.4.0-rc6-01061-g2b96fb3-dirty #12 Not tainted
[  690.144785] -------------------------------------
[  690.149785] amixer/1409 is trying to release lock 
(&hcp->current_stream_lock) at:
[  690.157992] [<bf08a09c>] snd_pcm_iec958_get+0x1c/0x70 [snd_pcm]
[  690.164277] but there are no more locks to release!
[  690.169454]
[  690.169454] other info that might help us debug this:
[  690.176388] 2 locks held by amixer/1409:
[  690.180554]  #0:  (&card->power_lock){+.+...}, at: [<bf050f08>] 
snd_ctl_ioctl+0x514/0xcfc [snd]
[  690.190010]  #1:  (&card->controls_rwsem){++++..}, at: [<bf050f28>] 
snd_ctl_ioctl+0x534/0xcfc [snd]
[  690.199760]
[  690.199760] stack backtrace:
[  690.204409] CPU: 0 PID: 1409 Comm: amixer Not tainted 
4.4.0-rc6-01061-g2b96fb3-dirty #12
[  690.212990] Hardware name: Generic AM33XX (Flattened Device Tree)
[  690.219516] [<c0017b84>] (unwind_backtrace) from [<c0013ee8>] 
(show_stack+0x10/0x14)
[  690.227754] [<c0013ee8>] (show_stack) from [<c03480a0>] 
(dump_stack+0x84/0x9c)
[  690.235443] [<c03480a0>] (dump_stack) from [<c008ce8c>] 
(print_unlock_imbalance_bug+0xac/0xdc)
[  690.244598] [<c008ce8c>] (print_unlock_imbalance_bug) from 
[<c0091aa8>] (lock_release+0x268/0x3c0)
[  690.254125] [<c0091aa8>] (lock_release) from [<c0650c7c>] 
(__mutex_unlock_slowpath+0xb4/0x1a4)
[  690.263401] [<c0650c7c>] (__mutex_unlock_slowpath) from [<bf08a09c>] 
(snd_pcm_iec958_get+0x1c/0x70 [snd_pcm])
[  690.274104] [<bf08a09c>] (snd_pcm_iec958_get [snd_pcm]) from 
[<bf050fc4>] (snd_ctl_ioctl+0x5d0/0xcfc [snd])
[  690.284521] [<bf050fc4>] (snd_ctl_ioctl [snd]) from [<c01840bc>] 
(do_vfs_ioctl+0x4c0/0x7e4)
[  690.293405] [<c01840bc>] (do_vfs_ioctl) from [<c018444c>] 
(SyS_ioctl+0x6c/0x7c)
[  690.301191] [<c018444c>] (SyS_ioctl) from [<c000f6e0>] 
(ret_fast_syscall+0x0/0x1c)

It seems you have a bug at sound/core/pcm_iec958.c:49 (I'll comment that 
separately).

BR,
Jyri

[RFC v2 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