dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH RFC V2 0/5] another generic audio hdmi codec proposal
@ 2015-10-01 16:50 Arnaud Pouliquen
  2015-10-01 16:50 ` [PATCH RFC V2 1/5] video: hdmi: add helper function for N and CTS Arnaud Pouliquen
                   ` (5 more replies)
  0 siblings, 6 replies; 17+ messages in thread
From: Arnaud Pouliquen @ 2015-10-01 16:50 UTC (permalink / raw)
  To: alsa-devel, dri-devel
  Cc: moinejf, linux, bcousson, David Airlie, tomi.valkeinen,
	arnaud.pouliquen, lgirdwood, Jyri Sarha, peter.ujfalusi,
	Takashi Iwai, tony, broonie, Benjamin Gaignard


Version 2:
This version integrates missing features upgraded to be aligned when possible 
with patch set:
[PATCH RFC v4 0/8] Implement generic ASoC HDMI codec and use it in tda998x

It does not support the abort callback, the mute and syclk ops. 
This could be added in V3 or separate patch if justified.  

  [RFC 1/5]video: hdmi: add help function for N and CTS
	- Replace real frequency value by enum in switch case
	Notice that this patch is independant from the rest of the implentation
  [RFC 2/5]drm: add helper functions to add audio capabilities for bridge
	- extend audio bridge ops
  [RFC 3/5]ASoC: codec:  hdmi drm codec driver
	aligned when possible with hdmi-codec.c implementation
	. ELD management for alsa contraints
	. Support of SPDIF and I2S mode
	. hwparam ond set_fmt functions implemented
  [RFC 4/5]drm: sti: connect audio driver
	=> Implementation for STI platform
  [RFC 5/5]DT: sti: add audio HDMI dai link in audio card
	=> Devicetree example

Version 1:

This patch set is a tentative to implement a generic code for the HDMI audio.
Main concept are aligned with solution proposeded for TI platform. 
 -  ASoC codec driver registered by DRM driver
 -  ASOC driver is a generic driver.
 -  compatible with simple card
 
 Difference is that i propose a DRM generic interface based on bridge structure.
 Advantage is that all data exchanges are done through the DRM API.
 
 I think also that some helper functions could been used for N and CTS parameters calculation,
 as suggested by Russell King in a previous mail.
 
 I full aware that some features (like ELD and info frame) are partially or not implemented in my patches. 
 This patch set is more a skeleton than a full implementation...
 I just post it to suggest a possible DRM API.

Arnaud Pouliquen (5):
  video: hdmi: add helper function for N and CTS
  drm: add helper function to add audio capabilities for bridge
  ASoC: codec:  hdmi drm codec driver
  drm: sti: connect audio driver
  DT: sti: add audio HDMI dai link in audio card

 arch/arm/boot/dts/stih410.dtsi       |   6 +-
 arch/arm/boot/dts/stihxxx-b2120.dtsi |  21 ++
 drivers/gpu/drm/drm_bridge.c         | 137 +++++++++++++-
 drivers/gpu/drm/sti/sti_hdmi.c       | 184 ++++++++++++++++--
 drivers/gpu/drm/sti/sti_hdmi.h       |   3 +
 drivers/video/hdmi.c                 | 147 ++++++++++++++
 include/drm/drm_crtc.h               |  62 ++++++
 include/drm/drm_modes.h              |  12 ++
 include/linux/hdmi.h                 |  12 ++
 include/sound/hdmi_drm.h             |  15 ++
 sound/soc/codecs/Kconfig             |   8 +-
 sound/soc/codecs/Makefile            |   2 +
 sound/soc/codecs/hdmi_drm.c          | 358 +++++++++++++++++++++++++++++++++++
 13 files changed, 951 insertions(+), 16 deletions(-)
 create mode 100644 include/sound/hdmi_drm.h
 create mode 100644 sound/soc/codecs/hdmi_drm.c

-- 
1.9.1

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH RFC V2 1/5] video: hdmi: add helper function for N and CTS
  2015-10-01 16:50 [PATCH RFC V2 0/5] another generic audio hdmi codec proposal Arnaud Pouliquen
@ 2015-10-01 16:50 ` Arnaud Pouliquen
  2015-10-01 16:50 ` [PATCH RFC V2 2/5] drm: add helper functions to add audio capabilities for bridge Arnaud Pouliquen
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 17+ messages in thread
From: Arnaud Pouliquen @ 2015-10-01 16:50 UTC (permalink / raw)
  To: alsa-devel, dri-devel
  Cc: moinejf, linux-fbdev, linux, bcousson, David Airlie,
	tomi.valkeinen, arnaud.pouliquen, lgirdwood, Jyri Sarha,
	peter.ujfalusi, Takashi Iwai, tony, broonie, Benjamin Gaignard,
	Jean-Christophe Plagniol-Villard

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>
Cc: linux-fbdev@vger.kernel.org
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/video/hdmi.c | 147 +++++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/hdmi.h |  12 +++++
 2 files changed, 159 insertions(+)

diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
index 1626892..93e2fdf 100644
--- a/drivers/video/hdmi.c
+++ b/drivers/video/hdmi.c
@@ -1242,3 +1242,150 @@ 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 cloack 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(enum hdmi_audio_sample_frequency 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 HDMI_AUDIO_SAMPLE_FREQUENCY_32000:
+		audio_freq_id = HDMI_AUDIO_N_CTS_32KHZ;
+		n_cts->n = 4096;
+		break;
+
+	case HDMI_AUDIO_SAMPLE_FREQUENCY_44100:
+		audio_freq_id = HDMI_AUDIO_N_CTS_44_1KHZ;
+		n_cts->n = 6272;
+		break;
+
+	case HDMI_AUDIO_SAMPLE_FREQUENCY_48000:
+		audio_freq_id = HDMI_AUDIO_N_CTS_48KHZ;
+		n_cts->n = 6144;
+		break;
+
+	case HDMI_AUDIO_SAMPLE_FREQUENCY_88200:
+		audio_freq_id = HDMI_AUDIO_N_CTS_44_1KHZ;
+		ratio = 2;
+		n_cts->n = 6272 * 2;
+		break;
+
+	case HDMI_AUDIO_SAMPLE_FREQUENCY_96000:
+		audio_freq_id = HDMI_AUDIO_N_CTS_48KHZ;
+		ratio = 2;
+		n_cts->n = 6144 * 2;
+		break;
+
+	case HDMI_AUDIO_SAMPLE_FREQUENCY_176400:
+		audio_freq_id = HDMI_AUDIO_N_CTS_44_1KHZ;
+		ratio = 2;
+		n_cts->n = 6272 * 4;
+		break;
+
+	case HDMI_AUDIO_SAMPLE_FREQUENCY_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..95c2b12 100644
--- a/include/linux/hdmi.h
+++ b/include/linux/hdmi.h
@@ -333,4 +333,16 @@ 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(enum hdmi_audio_sample_frequency audio_fs,
+			     unsigned long pixel_clk,
+			     struct hdmi_audio_n_cts *n_cts);
+
 #endif /* _DRM_HDMI_H */
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH RFC V2 2/5] drm: add helper functions to add audio capabilities for bridge
  2015-10-01 16:50 [PATCH RFC V2 0/5] another generic audio hdmi codec proposal Arnaud Pouliquen
  2015-10-01 16:50 ` [PATCH RFC V2 1/5] video: hdmi: add helper function for N and CTS Arnaud Pouliquen
@ 2015-10-01 16:50 ` Arnaud Pouliquen
  2015-10-01 16:50 ` [PATCH RFC V2 3/5] ASoC: codec: hdmi drm codec driver Arnaud Pouliquen
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 17+ messages in thread
From: Arnaud Pouliquen @ 2015-10-01 16:50 UTC (permalink / raw)
  To: alsa-devel, dri-devel
  Cc: moinejf, linux, bcousson, David Airlie, tomi.valkeinen,
	arnaud.pouliquen, lgirdwood, Jyri Sarha, peter.ujfalusi,
	Takashi Iwai, tony, broonie, Benjamin Gaignard

Extend bridge capabilities for audio to enable to connect an audio driver to a
DRM driver with audio capabilities

Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@st.com>
---
 drivers/gpu/drm/drm_bridge.c | 137 ++++++++++++++++++++++++++++++++++++++++++-
 include/drm/drm_crtc.h       |  62 ++++++++++++++++++++
 include/drm/drm_modes.h      |  12 ++++
 3 files changed, 210 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index 6b8f721..d1a437e 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -194,7 +194,7 @@ EXPORT_SYMBOL(drm_bridge_mode_fixup);
  * chain, starting from the last bridge to the first. These are called before
  * calling the encoder's prepare op.
  *
- * Note: the bridge passed should be the one closest to the encoder
+ * Note: the bridge passed should be the othingsne closest to the encoder
  */
 void drm_bridge_disable(struct drm_bridge *bridge)
 {
@@ -328,6 +328,141 @@ struct drm_bridge *of_drm_find_bridge(struct device_node *np)
 EXPORT_SYMBOL(of_drm_find_bridge);
 #endif
 
+/**
+ * DOC: audio bridge callbacks
+ *
+ * The drm_audio_bridge_funcs ops are populated by the bridge driver that has
+ * audio capabilities (e.g. HDMI)
+ * The alsa driver (or asoc codec) uses the defined helpers.
+ * These helpers call a specific drm_audio_bridge_funcs ops defined by
+ * bridges with audio capabilities during encoder configuration.
+ *
+ * pre_enable: this contains actions needed to be done by the bridge before
+ * audio is enabled by its source.
+ *
+ * enable: this contains actions needed to be done by the audio bridge once its
+ * source is enabled. In other words, enable is called once the source is
+ * ready to start stream rendering.
+ *
+ * disable: this contains actions needed to be done by audio bridge when
+ * disable the audio part, assuming that its source is still enabled.
+ *
+ * post_disable: this contains actions needed to be done by the bridge once
+ * its source is disabled.
+ *
+ * mode_set: this sets up the mode by the audio bridge. It assumes that its
+ * audio source is aligned on this mode.
+ *
+ * mode_get: this get the supported modes based on ELD table. this can be use
+ * by audio source to fix audio constraints according to mode.
+ */
+
+/**
+ * drm_audio_bridge_pre_enable - calls 'pre_enable' drm_audio_bridge_funcs op
+ * for audio bridge in the encoder chain.
+ * @bridge: bridge control structure
+ */
+int drm_audio_bridge_pre_enable(struct drm_bridge *bridge,
+				 struct drm_audio_bridge_cfg *cfg)
+{
+	if (!bridge)
+		return -EINVAL;
+
+	if (bridge->audio_funcs->pre_enable)
+		return bridge->audio_funcs->pre_enable(bridge, cfg);
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_audio_bridge_pre_enable);
+
+/**
+ * drm_audio_bridge_disable - calls 'disable' drm_audio_bridge_funcs op
+ * for audio bridge in the encoder chain.
+ * @bridge: bridge control structure
+ */
+int drm_audio_bridge_disable(struct drm_bridge *bridge)
+{
+	if (!bridge)
+		return -EINVAL;
+
+	if (bridge->audio_funcs->disable)
+		return bridge->audio_funcs->disable(bridge);
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_audio_bridge_disable);
+
+/**
+ * drm_audio_bridge_enable - calls 'enable' drm_audio_bridge_funcs audio
+ * bridge in the encoder chain.
+ * @bridge: bridge control structure
+ */
+int drm_audio_bridge_enable(struct drm_bridge *bridge)
+{
+	if (!bridge)
+		return -EINVAL;
+
+	if (bridge->audio_funcs->enable)
+		return bridge->audio_funcs->enable(bridge);
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_audio_bridge_enable);
+
+/**
+ * drm_audio_bridge_post_disable - calls 'disable' drm_audio_bridge_funcs op
+ * for audio bridge in the encoder chain.
+ * @bridge: bridge control structure
+ */
+int drm_audio_bridge_post_disable(struct drm_bridge *bridge)
+{
+	if (!bridge)
+		return -EINVAL;
+
+	if (bridge->audio_funcs->post_disable)
+		return bridge->audio_funcs->post_disable(bridge);
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_audio_bridge_post_disable);
+
+/**
+ * drm_audio_bridge_mode_set - calls 'mode_set' drm_audio_bridge_funcs op
+ * for audio bridge in the encoder chain.
+ * @bridge: bridge control structure
+ * @mode: desired audio mode to be set for the audio bridge
+ */
+int drm_audio_bridge_mode_set(struct drm_bridge *bridge,
+			       struct hdmi_audio_mode *mode)
+{
+	if (!bridge)
+		return -EINVAL;
+
+	if (bridge->audio_funcs->mode_set)
+		return bridge->audio_funcs->mode_set(bridge, mode);
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_audio_bridge_mode_set);
+
+/**
+ * drm_audio_bridge_mode_get - calls 'enable'drm_audio_bridge_funcs op
+ * for audio bridge in the encoder chain.
+ * @bridge: bridge control structure
+ * Note: The returned pointer needs to be freed using kfree().
+ */
+uint8_t *drm_audio_bridge_mode_get(struct drm_bridge *bridge)
+{
+	if (!bridge)
+		return NULL;
+
+	if (bridge->audio_funcs->mode_get)
+		return bridge->audio_funcs->mode_get(bridge);
+
+	return NULL;
+}
+EXPORT_SYMBOL(drm_audio_bridge_mode_get);
+
 MODULE_AUTHOR("Ajay Kumar <ajaykumar.rs@samsung.com>");
 MODULE_DESCRIPTION("DRM bridge infrastructure");
 MODULE_LICENSE("GPL and additional rights");
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 3b4d8a4..f13626a 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -583,6 +583,7 @@ struct drm_encoder_funcs {
  * @possible_clones: bitmask of potential sibling encoders for cloning
  * @crtc: currently bound CRTC
  * @bridge: bridge associated to the encoder
+ * @abridge: optional audio bridge associated to the encoder (HDMI)
  * @funcs: control functions
  * @helper_private: mid-layer private data
  *
@@ -601,6 +602,7 @@ struct drm_encoder {
 
 	struct drm_crtc *crtc;
 	struct drm_bridge *bridge;
+	struct drm_bridge *abridge;
 	const struct drm_encoder_funcs *funcs;
 	const void *helper_private;
 };
@@ -905,6 +907,56 @@ struct drm_bridge_funcs {
 };
 
 /**
+ * struct drm_audio_bridge_cfg - audio interface configuration
+ * @fmt: bus format
+ * @sample_rate: sampling frequency
+ * @sample_width: sample size
+ * @channels: number of channels
+ * @frame_clk_master: frame synchro master
+ * @frame_clk_inv: frame clock inverted
+ * @bit_clk_master: bit clock master
+ * @bit_clk_inv: bit clock inverted
+ */
+struct drm_audio_bridge_cfg {
+	enum {
+		HDMI_I2S,
+		HDMI_RIGHT_J,
+		HDMI_LEFT_J,
+		HDMI_DSP_A,
+		HDMI_DSP_B,
+		HDMI_AC97,
+		HDMI_SPDIF,
+	} fmt;
+	int sample_rate;
+	int sample_width;
+	int channels;
+	int frame_clk_master:1;
+	int frame_clk_inv:1;
+	int bit_clk_master:1;
+	int bit_clk_inv:1;
+};
+
+/**
+ * struct drm_audio_bridge_funcs - audio drm_bridge control functions
+ * @disable: Called to disable the audio bridge
+ * @post_disable: Called for post disable actions
+ * @pre_enable: Called to configure the audio bridge
+ * @enable: Called to enable the audio bridge
+ * @mode_set: Set the audio bridge mode
+ * @mode_get: Get ELD buffer for audio mode supported.
+ */
+struct drm_audio_bridge_funcs {
+	int (*disable)(struct drm_bridge *bridge);
+	int (*post_disable)(struct drm_bridge *bridge);
+	int (*pre_enable)(struct drm_bridge *bridge,
+			   struct drm_audio_bridge_cfg *cfg);
+	int (*enable)(struct drm_bridge *bridge);
+	int  (*mode_set)(struct drm_bridge *bridge,
+			 struct hdmi_audio_mode *mode);
+	uint8_t *(*mode_get)(struct drm_bridge *bridge);
+};
+
+/**
  * struct drm_bridge - central DRM bridge control structure
  * @dev: DRM device this bridge belongs to
  * @encoder: encoder to which this bridge is connected
@@ -925,6 +977,7 @@ struct drm_bridge {
 	struct list_head list;
 
 	const struct drm_bridge_funcs *funcs;
+	const struct drm_audio_bridge_funcs *audio_funcs;
 	void *driver_private;
 };
 
@@ -1266,6 +1319,15 @@ void drm_bridge_mode_set(struct drm_bridge *bridge,
 void drm_bridge_pre_enable(struct drm_bridge *bridge);
 void drm_bridge_enable(struct drm_bridge *bridge);
 
+int drm_audio_bridge_pre_enable(struct drm_bridge *bridge,
+				struct drm_audio_bridge_cfg *cfg);
+int drm_audio_bridge_enable(struct drm_bridge *bridge);
+int drm_audio_bridge_disable(struct drm_bridge *bridge);
+int drm_audio_bridge_post_disable(struct drm_bridge *bridge);
+int drm_audio_bridge_mode_set(struct drm_bridge *bridge,
+			       struct hdmi_audio_mode *mode);
+uint8_t *drm_audio_bridge_mode_get(struct drm_bridge *bridge);
+
 extern int drm_encoder_init(struct drm_device *dev,
 			    struct drm_encoder *encoder,
 			    const struct drm_encoder_funcs *funcs,
diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h
index 08a8cac..e923e32 100644
--- a/include/drm/drm_modes.h
+++ b/include/drm/drm_modes.h
@@ -164,6 +164,18 @@ struct drm_cmdline_mode {
 	enum drm_connector_force force;
 };
 
+/*
+ * struct hdmi_audio_mode - hdmi audio structure for audio configuration
+ * @infoframe: audio info frame
+ * @iec_status: iec60958 channel status bytes
+ *
+ * This is used by audio driver to configure the HDMI audio part
+ */
+struct hdmi_audio_mode {
+	struct hdmi_audio_infoframe infoframe;
+	unsigned char iec_status[24];
+};
+
 /**
  * drm_mode_is_stereo - check for stereo mode flags
  * @mode: drm_display_mode to check
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH RFC V2 3/5] ASoC: codec: hdmi drm codec driver
  2015-10-01 16:50 [PATCH RFC V2 0/5] another generic audio hdmi codec proposal Arnaud Pouliquen
  2015-10-01 16:50 ` [PATCH RFC V2 1/5] video: hdmi: add helper function for N and CTS Arnaud Pouliquen
  2015-10-01 16:50 ` [PATCH RFC V2 2/5] drm: add helper functions to add audio capabilities for bridge Arnaud Pouliquen
@ 2015-10-01 16:50 ` Arnaud Pouliquen
  2015-10-01 16:50 ` [PATCH RFC V2 4/5] drm: sti: connect audio driver Arnaud Pouliquen
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 17+ messages in thread
From: Arnaud Pouliquen @ 2015-10-01 16:50 UTC (permalink / raw)
  To: alsa-devel, dri-devel
  Cc: moinejf, linux, bcousson, David Airlie, tomi.valkeinen,
	arnaud.pouliquen, lgirdwood, Jyri Sarha, peter.ujfalusi,
	Takashi Iwai, tony, broonie, Benjamin Gaignard

Add a generic codec to interface audio with DRM drivers

Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@st.com>
---
 include/sound/hdmi_drm.h    |  15 ++
 sound/soc/codecs/Kconfig    |   8 +-
 sound/soc/codecs/Makefile   |   2 +
 sound/soc/codecs/hdmi_drm.c | 358 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 380 insertions(+), 3 deletions(-)
 create mode 100644 include/sound/hdmi_drm.h
 create mode 100644 sound/soc/codecs/hdmi_drm.c

diff --git a/include/sound/hdmi_drm.h b/include/sound/hdmi_drm.h
new file mode 100644
index 0000000..411acee
--- /dev/null
+++ b/include/sound/hdmi_drm.h
@@ -0,0 +1,15 @@
+/*
+ * Interface for HDMI DRM  codec
+ *
+ * Author: Arnaud Pouliquen <arnaud.pouliquen@st.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#ifndef __HDMI_DRM__H__
+#define __HDMI_DRM__H__
+
+#define HDMI_DRM_CODEC_DRV_NAME "hdmi-drm-audio-codec"
+
+#endif
diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
index 395ef5d..fe5b0c8 100644
--- a/sound/soc/codecs/Kconfig
+++ b/sound/soc/codecs/Kconfig
@@ -80,6 +80,7 @@ config SND_SOC_ALL_CODECS
 	select SND_SOC_MC13783 if MFD_MC13XXX
 	select SND_SOC_ML26124 if I2C
 	select SND_SOC_HDMI_CODEC
+	select SND_SOC_HDMI_DRM_CODEC
 	select SND_SOC_PCM1681 if I2C
 	select SND_SOC_PCM1792A if SPI_MASTER
 	select SND_SOC_PCM3008
@@ -445,9 +446,10 @@ config SND_SOC_DMIC
 config SND_SOC_HDMI_CODEC
        tristate "HDMI stub CODEC"
 
-       tristate
-       select SND_PCM_ELD
-       select SND_PCM_IEC958
+config SND_SOC_HDMI_DRM_CODEC
+	tristate "HDMI DRM CODEC"
+	select SND_PCM_ELD
+	select SND_PCM_IEC958
 
 config SND_SOC_ES8328
 	tristate "Everest Semi ES8328 CODEC"
diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile
index f683b33..41b2dcb 100644
--- a/sound/soc/codecs/Makefile
+++ b/sound/soc/codecs/Makefile
@@ -73,6 +73,7 @@ snd-soc-max9850-objs := max9850.o
 snd-soc-mc13783-objs := mc13783.o
 snd-soc-ml26124-objs := ml26124.o
 snd-soc-hdmi-codec-objs := hdmi-codec.o
+snd-soc-hdmi-drm-codec-objs := hdmi_drm.o
 snd-soc-pcm1681-objs := pcm1681.o
 snd-soc-pcm1792a-codec-objs := pcm1792a.o
 snd-soc-pcm3008-objs := pcm3008.o
@@ -265,6 +266,7 @@ obj-$(CONFIG_SND_SOC_MAX9850)	+= snd-soc-max9850.o
 obj-$(CONFIG_SND_SOC_MC13783)	+= snd-soc-mc13783.o
 obj-$(CONFIG_SND_SOC_ML26124)	+= snd-soc-ml26124.o
 obj-$(CONFIG_SND_SOC_HDMI_CODEC) += snd-soc-hdmi-codec.o
+obj-$(CONFIG_SND_SOC_HDMI_DRM_CODEC) += snd-soc-hdmi-drm-codec.o
 obj-$(CONFIG_SND_SOC_PCM1681)	+= snd-soc-pcm1681.o
 obj-$(CONFIG_SND_SOC_PCM1792A)	+= snd-soc-pcm1792a-codec.o
 obj-$(CONFIG_SND_SOC_PCM3008)	+= snd-soc-pcm3008.o
diff --git a/sound/soc/codecs/hdmi_drm.c b/sound/soc/codecs/hdmi_drm.c
new file mode 100644
index 0000000..1176d3b
--- /dev/null
+++ b/sound/soc/codecs/hdmi_drm.c
@@ -0,0 +1,358 @@
+/*
+ * ALSA SoC codec driver for DRM HDMI device.
+ * Copyright (C) STMicroelectronics SA 2015
+ * Authors: Arnaud Pouliquen <arnaud.pouliquen@st.com>
+ *          for STMicroelectronics.
+ * Inspirate from hdmi-codec (Jyri Sarha <jsarha at ti.com>)
+
+ * License terms:  GNU General Public License (GPL), version 2
+ */
+
+#include <linux/module.h>
+#include <sound/soc.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+
+#include <sound/asoundef.h>
+#include <sound/hdmi_drm.h>
+#include <sound/pcm_params.h>
+#include <sound/pcm_drm_eld.h>
+#include <sound/pcm_iec958.h>
+
+#include <drm/drm_crtc_helper.h>
+
+struct hdmi_drm_dai_data {
+	struct drm_bridge *bridge;
+	struct hdmi_audio_mode mode;
+	struct drm_audio_bridge_cfg cfg;
+	enum {
+		HDMI_DRM_I2S,
+		HDMI_DRM_SPDIF
+	} hai;
+};
+
+static const struct snd_soc_dapm_widget hdmi_drm_widgets[] = {
+	SND_SOC_DAPM_OUTPUT("TX"),
+};
+
+static const struct snd_soc_dapm_route hdmi_drm_routes[] = {
+	{ "TX", NULL, "Playback" },
+};
+
+int hdmi_drm_dai_startup(struct snd_pcm_substream *substream,
+			 struct snd_soc_dai *dai)
+{
+	struct hdmi_drm_dai_data *priv = snd_soc_dai_get_drvdata(dai);
+	uint8_t *eld;
+
+	dev_dbg(dai->dev, "%s: enter for bridge %p\n", __func__, priv->bridge);
+
+	eld = drm_audio_bridge_mode_get(priv->bridge);
+	if (!eld)
+		return 0;
+
+	return snd_pcm_hw_constraint_eld(substream->runtime, eld);
+}
+
+static void hdmi_drm_dai_shutdown(struct snd_pcm_substream *substream,
+			   struct snd_soc_dai *dai)
+{
+	struct hdmi_drm_dai_data *priv = snd_soc_dai_get_drvdata(dai);
+
+	drm_audio_bridge_post_disable(priv->bridge);
+}
+
+static int hdmi_drm_dai_hw_params(struct snd_pcm_substream *substream,
+				  struct snd_pcm_hw_params *params,
+				  struct snd_soc_dai *dai)
+{
+	struct hdmi_drm_dai_data *priv = snd_soc_dai_get_drvdata(dai);
+	struct hdmi_audio_infoframe *iframe = &priv->mode.infoframe;
+	unsigned char *iec = priv->mode.iec_status;
+	struct drm_audio_bridge_cfg *cfg = &priv->cfg;
+	int ret;
+
+	dev_dbg(dai->dev, "%s: enter for bridge %p\n", __func__, priv->bridge);
+
+	/* create iec959 if hadmi audio interface is no SPDIF
+	 * otherwise IEC status should managed by CPU-dai
+	 */
+	if (priv->hai != HDMI_DRM_SPDIF) {
+		ret = snd_pcm_create_iec958_consumer_hw_params(params, iec,
+							       sizeof(iec));
+		if (ret < 0) {
+			dev_err(dai->dev, "create iec status failed (%d)\n",
+				ret);
+			return ret;
+		}
+	}
+
+	hdmi_audio_infoframe_init(iframe);
+
+	iframe->channels = params_channels(params);
+	cfg->channels = iframe->channels;
+
+	switch (params_width(params)) {
+	case 16:
+		iframe->sample_size = HDMI_AUDIO_SAMPLE_SIZE_16;
+		break;
+	case 18:
+		iframe->sample_size = HDMI_AUDIO_SAMPLE_SIZE_20;
+		break;
+	case 20:
+		iframe->sample_size = HDMI_AUDIO_SAMPLE_SIZE_20;
+		break;
+	case 24:
+	case 32:
+		iframe->sample_size = HDMI_AUDIO_SAMPLE_SIZE_24;
+		break;
+	default:
+		dev_err(dai->dev, "sample width not supported! %d\n",
+			params_width(params));
+		return -EINVAL;
+	}
+	cfg->sample_width = params_width(params);
+
+	switch (params_rate(params)) {
+	case 32000:
+		iframe->sample_frequency = HDMI_AUDIO_SAMPLE_FREQUENCY_32000;
+		break;
+	case 44100:
+		iframe->sample_frequency = HDMI_AUDIO_SAMPLE_FREQUENCY_44100;
+		break;
+	case 48000:
+		iframe->sample_frequency = HDMI_AUDIO_SAMPLE_FREQUENCY_48000;
+		break;
+	case 88200:
+		iframe->sample_frequency = HDMI_AUDIO_SAMPLE_FREQUENCY_88200;
+		break;
+	case 96000:
+		iframe->sample_frequency = HDMI_AUDIO_SAMPLE_FREQUENCY_96000;
+		break;
+	case 176400:
+		iframe->sample_frequency = HDMI_AUDIO_SAMPLE_FREQUENCY_176400;
+		break;
+	case 192000:
+		iframe->sample_frequency = HDMI_AUDIO_SAMPLE_FREQUENCY_192000;
+		break;
+	default:
+		dev_err(dai->dev, "rate not supported : %d!\n",
+			params_rate(params));
+		return -EINVAL;
+	}
+	cfg->sample_rate = params_rate(params);
+
+	ret = drm_audio_bridge_mode_set(priv->bridge, &priv->mode);
+	if (ret < 0)
+		return ret;
+
+	return drm_audio_bridge_pre_enable(priv->bridge, &priv->cfg);
+}
+
+static int hdmi_drm_dai_set_fmt(struct snd_soc_dai *dai,
+				unsigned int fmt)
+{
+	struct hdmi_drm_dai_data *priv = snd_soc_dai_get_drvdata(dai);
+	struct drm_audio_bridge_cfg *cfg = &priv->cfg;
+	int ret = 0;
+
+	dev_dbg(dai->dev, "%s: enter for bridge %p\n", __func__, priv->bridge);
+
+	if (priv->hai == HDMI_DRM_SPDIF) {
+		cfg->fmt = HDMI_SPDIF;
+	} else {
+		switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) {
+		case SND_SOC_DAIFMT_CBM_CFM:
+			cfg->bit_clk_master = 1;
+			cfg->frame_clk_master = 1;
+			break;
+		case SND_SOC_DAIFMT_CBS_CFM:
+			cfg->frame_clk_master = 1;
+			break;
+		case SND_SOC_DAIFMT_CBM_CFS:
+			cfg->bit_clk_master = 1;
+			break;
+		case SND_SOC_DAIFMT_CBS_CFS:
+			break;
+		default:
+			return -EINVAL;
+		}
+
+		switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
+		case SND_SOC_DAIFMT_NB_NF:
+			break;
+		case SND_SOC_DAIFMT_NB_IF:
+			cfg->frame_clk_inv = 1;
+			break;
+		case SND_SOC_DAIFMT_IB_NF:
+			cfg->bit_clk_inv = 1;
+			break;
+		case SND_SOC_DAIFMT_IB_IF:
+			cfg->frame_clk_inv = 1;
+			cfg->bit_clk_inv = 1;
+			break;
+		}
+
+		switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
+		case SND_SOC_DAIFMT_I2S:
+			cfg->fmt = HDMI_I2S;
+			break;
+		case SND_SOC_DAIFMT_DSP_A:
+			cfg->fmt = HDMI_DSP_A;
+			break;
+		case SND_SOC_DAIFMT_DSP_B:
+			cfg->fmt = HDMI_DSP_B;
+			break;
+		case SND_SOC_DAIFMT_RIGHT_J:
+			cfg->fmt = HDMI_RIGHT_J;
+			break;
+		case SND_SOC_DAIFMT_LEFT_J:
+			cfg->fmt = HDMI_LEFT_J;
+			break;
+		case SND_SOC_DAIFMT_AC97:
+			cfg->fmt = HDMI_AC97;
+			break;
+		default:
+			dev_err(dai->dev, "Invalid DAI interface format\n");
+			return -EINVAL;
+		}
+	}
+
+	return ret;
+}
+
+int hdmi_drm_dai_trigger(struct snd_pcm_substream *substream, int cmd,
+			 struct snd_soc_dai *dai)
+{
+	struct hdmi_drm_dai_data *priv = snd_soc_dai_get_drvdata(dai);
+
+	dev_dbg(dai->dev, "%s: enter for bridge %p\n", __func__, priv->bridge);
+
+	switch (cmd) {
+	case SNDRV_PCM_TRIGGER_START:
+	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
+	case SNDRV_PCM_TRIGGER_RESUME:
+		return drm_audio_bridge_enable(priv->bridge);
+
+	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
+	case SNDRV_PCM_TRIGGER_STOP:
+	case SNDRV_PCM_TRIGGER_SUSPEND:
+		return drm_audio_bridge_disable(priv->bridge);
+	}
+
+	return 0;
+}
+
+static int st_hdmi_dai_probe(struct snd_soc_dai *dai)
+{
+	struct hdmi_drm_dai_data *priv = snd_soc_dai_get_drvdata(dai);
+
+	priv->bridge = of_drm_find_bridge(dai->dev->parent->of_node);
+
+	if (!priv->bridge) {
+		dev_err(dai->dev, "HDMI audio bridge not found for node %s\n",
+			dai->dev->parent->of_node->name);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static const struct snd_soc_dai_ops hdmi_drm_codec_ops = {
+		.startup	= hdmi_drm_dai_startup,
+		.shutdown	= hdmi_drm_dai_shutdown,
+		.hw_params	= hdmi_drm_dai_hw_params,
+		.trigger	= hdmi_drm_dai_trigger,
+		.set_fmt	= hdmi_drm_dai_set_fmt,
+};
+
+static struct snd_soc_dai_driver hdmi_drm_codec_dai = {
+	.name = "hdmi-hifi",
+	.playback = {
+		.stream_name = "Playback",
+		.channels_min = 2,
+		.channels_max = 8,
+		.rates = SNDRV_PCM_RATE_32000 |
+			SNDRV_PCM_RATE_44100 | SNDRV_PCM_RATE_48000 |
+			SNDRV_PCM_RATE_88200 | SNDRV_PCM_RATE_96000 |
+			SNDRV_PCM_RATE_176400 | SNDRV_PCM_RATE_192000,
+		.formats = SNDRV_PCM_FMTBIT_S16_LE |
+			   SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S32_LE,
+	},
+	.probe = st_hdmi_dai_probe,
+	.ops = &hdmi_drm_codec_ops,
+};
+
+static struct snd_soc_codec_driver hdmi_drm_codec = {
+	.dapm_widgets = hdmi_drm_widgets,
+	.num_dapm_widgets = ARRAY_SIZE(hdmi_drm_widgets),
+	.dapm_routes = hdmi_drm_routes,
+	.num_dapm_routes = ARRAY_SIZE(hdmi_drm_routes),
+};
+
+static int hdmi_drm_codec_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct hdmi_drm_dai_data *priv;
+	struct device_node *np = dev->parent->of_node;
+	struct device_node *np_child;
+	const char *format;
+	int ret;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	np_child = of_get_child_by_name(np, "sound-dai");
+	if (!np_child) {
+		dev_err(dev, "DT not compatible\n");
+		return -ENXIO;
+	}
+
+	ret = of_property_read_string(np_child, "format", &format);
+	if (ret < 0) {
+		dev_err(dev, "missing format\n");
+		goto error;
+	}
+	if (strcmp(format, "i2s") == 0) {
+		priv->hai = HDMI_DRM_I2S;
+	} else if (strcmp(format, "spdif") == 0) {
+		priv->hai = HDMI_DRM_SPDIF;
+	} else {
+		dev_err(dev, "not expected format %s\n", format);
+		goto error;
+	}
+
+	dev_set_drvdata(dev, priv);
+
+	dev_info(dev, "bound audio drm codec\n");
+
+	return snd_soc_register_codec(dev, &hdmi_drm_codec,
+				      &hdmi_drm_codec_dai, 1);
+
+error:
+	dev_err(dev, "DT bus format missing\n");
+
+	return -EINVAL;
+}
+
+static int hdmi_drm_codec_remove(struct platform_device *pdev)
+{
+	snd_soc_unregister_codec(&pdev->dev);
+
+	return 0;
+}
+
+static struct platform_driver hdmi_codec_driver = {
+	.driver = {
+		.name = HDMI_DRM_CODEC_DRV_NAME,
+	},
+	.probe = hdmi_drm_codec_probe,
+	.remove = hdmi_drm_codec_remove,
+};
+
+module_platform_driver(hdmi_codec_driver);
+MODULE_AUTHOR("Arnaud Pouliquen <Arnaud.pouliquen@st.com>");
+MODULE_DESCRIPTION("ASoC HDMI codec driver");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:" HDMI_DRM_CODEC_DRV_NAME);
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH RFC V2 4/5] drm: sti: connect audio driver
  2015-10-01 16:50 [PATCH RFC V2 0/5] another generic audio hdmi codec proposal Arnaud Pouliquen
                   ` (2 preceding siblings ...)
  2015-10-01 16:50 ` [PATCH RFC V2 3/5] ASoC: codec: hdmi drm codec driver Arnaud Pouliquen
@ 2015-10-01 16:50 ` Arnaud Pouliquen
  2015-10-01 16:50 ` [PATCH RFC V2 5/5] DT: sti: add audio HDMI dai link in audio card Arnaud Pouliquen
  2015-10-05 13:27 ` [PATCH RFC V2 0/5] another generic audio hdmi codec proposal Jyri Sarha
  5 siblings, 0 replies; 17+ messages in thread
From: Arnaud Pouliquen @ 2015-10-01 16:50 UTC (permalink / raw)
  To: alsa-devel, dri-devel
  Cc: moinejf, linux, bcousson, David Airlie, tomi.valkeinen,
	arnaud.pouliquen, lgirdwood, Jyri Sarha, peter.ujfalusi,
	Takashi Iwai, tony, broonie, Benjamin Gaignard

Registeur Asoc codec and implement audio bridge ops.

Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@st.com>
---
 drivers/gpu/drm/sti/sti_hdmi.c | 184 ++++++++++++++++++++++++++++++++++++++---
 drivers/gpu/drm/sti/sti_hdmi.h |   3 +
 2 files changed, 176 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/sti/sti_hdmi.c b/drivers/gpu/drm/sti/sti_hdmi.c
index 06595e9..cf0e307 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_drm.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
@@ -109,6 +116,29 @@
 
 #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)
@@ -380,19 +410,15 @@ 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_infoframe *infoframe;
 	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;
-	}
+	DRM_DEBUG_DRIVER("enter %s\n", __func__);
 
-	infofame.channels = 2;
+	infoframe = &hdmi->audio.infoframe;
 
-	ret = hdmi_audio_infoframe_pack(&infofame, buffer, sizeof(buffer));
+	ret = hdmi_audio_infoframe_pack(infoframe, buffer, sizeof(buffer));
 	if (ret < 0) {
 		DRM_ERROR("failed to pack audio infoframe: %d\n", ret);
 		return ret;
@@ -404,6 +430,60 @@ static int hdmi_audio_infoframe_config(struct sti_hdmi *hdmi)
 }
 
 /**
+ * set audio frame rate
+ *
+ * @hdmi: pointer on the hdmi internal structure
+ *
+ */
+static int hdmi_audio_set_infoframe(struct sti_hdmi *hdmi,
+				    struct hdmi_audio_infoframe *info)
+{
+	struct hdmi_audio_n_cts n_cts;
+	int ret, audio_cfg;
+
+	DRM_DEBUG_DRIVER("enter %s\n", __func__);
+
+	hdmi->audio.infoframe = *info;
+
+	if (!hdmi->enabled)
+		return 0;
+
+	/* 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);
+
+	/* update N parameter */
+	ret = hdmi_audio_compute_n_cts(info->sample_frequency,
+				       hdmi->mode.clock, &n_cts);
+
+	DRM_DEBUG_DRIVER("sample_frequency= %d, pix clock = %d\n",
+			 info->sample_frequency, 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);
+
+	return hdmi_audio_infoframe_config(hdmi);
+}
+
+/**
  * Software reset of the hdmi subsystem
  *
  * @hdmi: pointer on the hdmi internal structure
@@ -462,7 +542,6 @@ static void sti_hdmi_disable(struct drm_bridge *bridge)
 	/* Disable HDMI */
 	val &= ~HDMI_CFG_DEVICE_EN;
 	hdmi_write(hdmi, val, HDMI_CFG);
-
 	hdmi_write(hdmi, 0xffffffff, HDMI_INT_CLR);
 
 	/* Stop the phy */
@@ -567,6 +646,53 @@ static const struct drm_bridge_funcs sti_hdmi_bridge_funcs = {
 	.mode_set = sti_hdmi_set_mode,
 };
 
+static int sti_hdmi_audio_disable(struct drm_bridge *bridge)
+{
+	struct sti_hdmi *hdmi = bridge->driver_private;
+
+	DRM_DEBUG_DRIVER("enter %s\n", __func__);
+	/* mute */
+	hdmi_write(hdmi, HDMI_SAMPLE_FLAT_ALL, HDMI_SAMPLE_FLAT_MASK);
+
+	return 0;
+}
+
+static int sti_hdmi_audio_set_mode(struct drm_bridge *bridge,
+			    struct hdmi_audio_mode *mode)
+{
+	struct sti_hdmi *hdmi = bridge->driver_private;
+
+	DRM_DEBUG_DRIVER("enter %s\n", __func__);
+	hdmi_audio_set_infoframe(hdmi, &mode->infoframe);
+
+	return 0;
+}
+
+static int sti_hdmi_audio_bridge_enable(struct drm_bridge *bridge)
+{
+	struct sti_hdmi *hdmi = bridge->driver_private;
+
+	DRM_DEBUG_DRIVER("enter %s\n", __func__);
+	/* unmute */
+	hdmi_write(hdmi, HDMI_SAMPLE_FLAT_NO, HDMI_SAMPLE_FLAT_MASK);
+
+	return 0;
+}
+
+static uint8_t *sti_hdmi_audio_get_mode(struct drm_bridge *bridge)
+{
+	struct sti_hdmi *hdmi = bridge->driver_private;
+
+	return hdmi->drm_connector->eld;
+}
+
+static const struct drm_audio_bridge_funcs sti_hdmi_audio_bridge_funcs = {
+	.enable = sti_hdmi_audio_bridge_enable,
+	.disable = sti_hdmi_audio_disable,
+	.mode_set = sti_hdmi_audio_set_mode,
+	.mode_get = sti_hdmi_audio_get_mode,
+};
+
 static int sti_hdmi_connector_get_modes(struct drm_connector *connector)
 {
 	struct sti_hdmi_connector *hdmi_connector
@@ -583,6 +709,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;
@@ -657,10 +784,13 @@ static void sti_hdmi_connector_destroy(struct drm_connector *connector)
 {
 	struct sti_hdmi_connector *hdmi_connector
 		= to_sti_hdmi_connector(connector);
+	struct sti_hdmi *hdmi = hdmi_connector->hdmi;
 
+	drm_bridge_remove(connector->encoder->bridge);
 	drm_connector_unregister(connector);
 	drm_connector_cleanup(connector);
 	kfree(hdmi_connector);
+	hdmi->drm_connector = NULL;
 }
 
 static struct drm_connector_funcs sti_hdmi_connector_funcs = {
@@ -698,6 +828,9 @@ static int sti_hdmi_bind(struct device *dev, struct device *master, void *data)
 	/* Set the drm device handle */
 	hdmi->drm_dev = drm_dev;
 
+	/* initialise audio infoframe */
+	hdmi_audio_infoframe_init(&hdmi->audio.infoframe);
+
 	encoder = sti_hdmi_find_encoder(drm_dev);
 	if (!encoder)
 		goto err_adapt;
@@ -705,8 +838,6 @@ static int sti_hdmi_bind(struct device *dev, struct device *master, void *data)
 	connector = devm_kzalloc(dev, sizeof(*connector), GFP_KERNEL);
 	if (!connector)
 		goto err_adapt;
-
-
 	connector->hdmi = hdmi;
 
 	bridge = devm_kzalloc(dev, sizeof(*bridge), GFP_KERNEL);
@@ -715,8 +846,14 @@ static int sti_hdmi_bind(struct device *dev, struct device *master, void *data)
 
 	bridge->driver_private = hdmi;
 	bridge->funcs = &sti_hdmi_bridge_funcs;
+	bridge->audio_funcs = &sti_hdmi_audio_bridge_funcs;
 	drm_bridge_attach(drm_dev, bridge);
 
+	bridge->of_node = dev->of_node;
+	err = drm_bridge_add(bridge);
+	if (err)
+		goto err_adapt;
+
 	encoder->bridge = bridge;
 	connector->encoder = encoder;
 
@@ -733,6 +870,8 @@ 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");
@@ -748,6 +887,8 @@ err_sysfs:
 	drm_connector_unregister(drm_connector);
 err_connector:
 	drm_connector_cleanup(drm_connector);
+	drm_bridge_remove(bridge);
+	hdmi->drm_connector = NULL;
 err_adapt:
 	put_device(&hdmi->ddc_adapt->dev);
 	return -EINVAL;
@@ -777,6 +918,25 @@ static const struct of_device_id hdmi_of_match[] = {
 };
 MODULE_DEVICE_TABLE(of, hdmi_of_match);
 
+static void sti_hdmi_register_audio_driver(struct device *dev)
+{
+	struct device_node *np = dev->of_node;
+	struct device_node *np_child;
+	struct platform_device *pdev;
+
+	np_child = of_get_child_by_name(np, "sound-dai");
+	if (!np_child)
+		return;
+
+	pdev = platform_device_register_data(dev,
+					     HDMI_DRM_CODEC_DRV_NAME,
+					     PLATFORM_DEVID_AUTO, NULL, 0);
+	if (IS_ERR(pdev))
+		return;
+
+	DRM_INFO("%s driver bound to HDMI\n", HDMI_DRM_CODEC_DRV_NAME);
+}
+
 static int sti_hdmi_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -877,6 +1037,8 @@ static int sti_hdmi_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, hdmi);
 
+	sti_hdmi_register_audio_driver(dev);
+
 	return component_add(&pdev->dev, &sti_hdmi_ops);
 }
 
diff --git a/drivers/gpu/drm/sti/sti_hdmi.h b/drivers/gpu/drm/sti/sti_hdmi.h
index 3d22390..54bd824 100644
--- a/drivers/gpu/drm/sti/sti_hdmi.h
+++ b/drivers/gpu/drm/sti/sti_hdmi.h
@@ -36,6 +36,7 @@ struct hdmi_phy_ops {
  * @clk_tmds: hdmi tmds clock
  * @clk_phy: hdmi phy clock
  * @clk_audio: hdmi audio clock
+ * @audio: hdmi audio state
  * @irq: hdmi interrupt number
  * @irq_status: interrupt status register
  * @phy_ops: phy start/stop operations
@@ -55,6 +56,7 @@ struct sti_hdmi {
 	struct clk *clk_tmds;
 	struct clk *clk_phy;
 	struct clk *clk_audio;
+	struct hdmi_audio_mode audio;
 	int irq;
 	u32 irq_status;
 	struct hdmi_phy_ops *phy_ops;
@@ -64,6 +66,7 @@ struct sti_hdmi {
 	bool event_received;
 	struct reset_control *reset;
 	struct i2c_adapter *ddc_adapt;
+	struct drm_connector *drm_connector;
 };
 
 u32 hdmi_read(struct sti_hdmi *hdmi, int offset);
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH RFC V2 5/5] DT: sti: add audio HDMI dai link in audio card
  2015-10-01 16:50 [PATCH RFC V2 0/5] another generic audio hdmi codec proposal Arnaud Pouliquen
                   ` (3 preceding siblings ...)
  2015-10-01 16:50 ` [PATCH RFC V2 4/5] drm: sti: connect audio driver Arnaud Pouliquen
@ 2015-10-01 16:50 ` Arnaud Pouliquen
  2015-10-05 13:27 ` [PATCH RFC V2 0/5] another generic audio hdmi codec proposal Jyri Sarha
  5 siblings, 0 replies; 17+ messages in thread
From: Arnaud Pouliquen @ 2015-10-01 16:50 UTC (permalink / raw)
  To: alsa-devel, dri-devel
  Cc: moinejf, linux, bcousson, David Airlie, tomi.valkeinen,
	arnaud.pouliquen, lgirdwood, Jyri Sarha, peter.ujfalusi,
	Takashi Iwai, tony, broonie, Benjamin Gaignard

Add the HDMI dai link to support audio for HDMi output

Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@st.com>
---
 arch/arm/boot/dts/stih410.dtsi       |  6 +++++-
 arch/arm/boot/dts/stihxxx-b2120.dtsi | 21 +++++++++++++++++++++
 2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/stih410.dtsi b/arch/arm/boot/dts/stih410.dtsi
index 7ee7dc0..dfc61a9 100644
--- a/arch/arm/boot/dts/stih410.dtsi
+++ b/arch/arm/boot/dts/stih410.dtsi
@@ -186,8 +186,9 @@
 							 <&clk_s_d2_quadfs 0>;
 			};
 
-				sti-hdmi@8d04000 {
+				sti_hdmi: sti-hdmi@8d04000 {
 					compatible = "st,stih407-hdmi";
+					#sound-dai-cells = <0>;
 					reg = <0x8d04000 0x1000>;
 					reg-names = "hdmi-reg";
 					interrupts = <GIC_SPI 106 IRQ_TYPE_NONE>;
@@ -211,6 +212,9 @@
 					resets = <&softreset STIH407_HDMI_TX_PHY_SOFTRESET>;
 					ddc = <&hdmiddc>;
 
+					sound-dai {
+						format = "spdif";
+					};
 				};
 
 				sti-hda@8d02000 {
diff --git a/arch/arm/boot/dts/stihxxx-b2120.dtsi b/arch/arm/boot/dts/stihxxx-b2120.dtsi
index 3ad9c82..80fed19 100644
--- a/arch/arm/boot/dts/stihxxx-b2120.dtsi
+++ b/arch/arm/boot/dts/stihxxx-b2120.dtsi
@@ -81,6 +81,9 @@
 		audio_controller: sti-asoc-platform {
 			status = "okay";
 		};
+		sti_uni_player0: sti-uni-player@0 {
+			status = "okay";
+		};
 		sti_uni_player2: sti-uni-player@2 {
 			status = "okay";
 		};
@@ -93,6 +96,12 @@
 		sti_uni_reader1: sti-uni-reader@1 {
 			status = "okay";
 		};
+
+		sti-display-subsystem {
+			sti_hdmi: sti-hdmi@8d04000 {
+				status = "okay";
+			};
+		};
 	};
 
 	sound {
@@ -125,6 +134,18 @@
 				sound-dai = <&sti_sasg_codec 0>;
 			};
 		};
+		simple-audio-card,dai-link@2 {
+			/* HDMI */
+			format = "i2s";
+			mclk-fs = <128>;
+			cpu {
+				sound-dai = <&sti_uni_player0>;
+			};
+
+			codec {
+				sound-dai = <&sti_hdmi>;
+			};
+		};
 	};
 
 };
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH RFC V2 0/5] another generic audio hdmi codec proposal
  2015-10-01 16:50 [PATCH RFC V2 0/5] another generic audio hdmi codec proposal Arnaud Pouliquen
                   ` (4 preceding siblings ...)
  2015-10-01 16:50 ` [PATCH RFC V2 5/5] DT: sti: add audio HDMI dai link in audio card Arnaud Pouliquen
@ 2015-10-05 13:27 ` Jyri Sarha
  2015-10-06  9:23   ` Arnaud Pouliquen
  5 siblings, 1 reply; 17+ messages in thread
From: Jyri Sarha @ 2015-10-05 13:27 UTC (permalink / raw)
  To: Arnaud Pouliquen, alsa-devel, dri-devel
  Cc: moinejf, linux, Benjamin Gaignard, David Airlie, broonie,
	lgirdwood, peter.ujfalusi, Takashi Iwai, tony, tomi.valkeinen,
	bcousson

On 10/01/15 19:50, Arnaud Pouliquen wrote:
>
> Version 2:
> This version integrates missing features upgraded to be aligned when possible
> with patch set:
> [PATCH RFC v4 0/8] Implement generic ASoC HDMI codec and use it in tda998x
>

There are still some details I would like to change if we decide to go
the drm audio bridge way. But before all that, I would like to ask,
why should we go forward with your approach? Is there anything that
can be done with your approach, but can not be done with mine?

Don't take me wrong, I do not see anything fundamentally wrong with
your approach. I would just like hear some justification why we should
abandon my approach - that I've been working on for some time - and go
forward with yours.

Here is couple of benefits I can name in my approach:
­ Video side agnostic implementation
    The ASoC side does not need to know anything about video side
    implementation. There is no real exposure ASoC side internals in
    video side either. Even fbdev driver, or some other non DRM video
    driver, could use my implementation.
- HDMI encoder driver implementations that do not use DRM bridge
   abstraction do not need add an extra DRM object just to get the
   audio working.

Short comings I see in the current HDMI audio bridge approach:

In its current from the DRM audio bridge abstraction pretends to be a
generic audio abstraction for DRM devices, but the implementation is
quite specific to external HDMI encoders with spdif and/or i2s
interface. There is a lot of HDMI video devices that provide the
digital audio interface (ASoC DAI) directly and there is no need for
anything but dummy codec implementation (if following ASoC
paradigm). Before going forward I think we should at least consider
how this abstraction would serve those devices.

Also, I am not entirely happy how the drm_audio_bridge_funcs are used
at the moment. The do not map too well to ASoC DAI callbacks and I do
not see too much point in creating a completely new audio-callback
abstraction, that is sligtly incompatible with ALSA, and then
translating alsa callbacks to these new callbacks. I think the
callbacks should map more or less directly ALSA callbacks.

Best regards,
Jyri


> It does not support the abort callback, the mute and syclk ops.
> This could be added in V3 or separate patch if justified.
>
>    [RFC 1/5]video: hdmi: add help function for N and CTS
> 	- Replace real frequency value by enum in switch case
> 	Notice that this patch is independant from the rest of the implentation
>    [RFC 2/5]drm: add helper functions to add audio capabilities for bridge
> 	- extend audio bridge ops
>    [RFC 3/5]ASoC: codec:  hdmi drm codec driver
> 	aligned when possible with hdmi-codec.c implementation
> 	. ELD management for alsa contraints
> 	. Support of SPDIF and I2S mode
> 	. hwparam ond set_fmt functions implemented
>    [RFC 4/5]drm: sti: connect audio driver
> 	=> Implementation for STI platform
>    [RFC 5/5]DT: sti: add audio HDMI dai link in audio card
> 	=> Devicetree example
>
> Version 1:
>
> This patch set is a tentative to implement a generic code for the HDMI audio.
> Main concept are aligned with solution proposeded for TI platform.
>   -  ASoC codec driver registered by DRM driver
>   -  ASOC driver is a generic driver.
>   -  compatible with simple card
>
>   Difference is that i propose a DRM generic interface based on bridge structure.
>   Advantage is that all data exchanges are done through the DRM API.
>
>   I think also that some helper functions could been used for N and CTS parameters calculation,
>   as suggested by Russell King in a previous mail.
>
>   I full aware that some features (like ELD and info frame) are partially or not implemented in my patches.
>   This patch set is more a skeleton than a full implementation...
>   I just post it to suggest a possible DRM API.
>
> Arnaud Pouliquen (5):
>    video: hdmi: add helper function for N and CTS
>    drm: add helper function to add audio capabilities for bridge
>    ASoC: codec:  hdmi drm codec driver
>    drm: sti: connect audio driver
>    DT: sti: add audio HDMI dai link in audio card
>
>   arch/arm/boot/dts/stih410.dtsi       |   6 +-
>   arch/arm/boot/dts/stihxxx-b2120.dtsi |  21 ++
>   drivers/gpu/drm/drm_bridge.c         | 137 +++++++++++++-
>   drivers/gpu/drm/sti/sti_hdmi.c       | 184 ++++++++++++++++--
>   drivers/gpu/drm/sti/sti_hdmi.h       |   3 +
>   drivers/video/hdmi.c                 | 147 ++++++++++++++
>   include/drm/drm_crtc.h               |  62 ++++++
>   include/drm/drm_modes.h              |  12 ++
>   include/linux/hdmi.h                 |  12 ++
>   include/sound/hdmi_drm.h             |  15 ++
>   sound/soc/codecs/Kconfig             |   8 +-
>   sound/soc/codecs/Makefile            |   2 +
>   sound/soc/codecs/hdmi_drm.c          | 358 +++++++++++++++++++++++++++++++++++
>   13 files changed, 951 insertions(+), 16 deletions(-)
>   create mode 100644 include/sound/hdmi_drm.h
>   create mode 100644 sound/soc/codecs/hdmi_drm.c
>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH RFC V2 0/5] another generic audio hdmi codec proposal
  2015-10-05 13:27 ` [PATCH RFC V2 0/5] another generic audio hdmi codec proposal Jyri Sarha
@ 2015-10-06  9:23   ` Arnaud Pouliquen
  2015-10-06 14:33     ` [alsa-devel] " Jean-Francois Moine
  2015-10-06 15:44     ` Jyri Sarha
  0 siblings, 2 replies; 17+ messages in thread
From: Arnaud Pouliquen @ 2015-10-06  9:23 UTC (permalink / raw)
  To: Jyri Sarha, alsa-devel, dri-devel
  Cc: moinejf, linux, Benjamin Gaignard, David Airlie, broonie,
	lgirdwood, peter.ujfalusi, Takashi Iwai, tony, tomi.valkeinen,
	bcousson

Hello Jyri,

Thanks your feedback, my answers in line


On 10/05/2015 03:27 PM, Jyri Sarha wrote:
> On 10/01/15 19:50, Arnaud Pouliquen wrote:
>>
>> Version 2:
>> This version integrates missing features upgraded to be aligned when possible
>> with patch set:
>> [PATCH RFC v4 0/8] Implement generic ASoC HDMI codec and use it in tda998x
>>
>
> There are still some details I would like to change if we decide to go
> the drm audio bridge way. But before all that, I would like to ask,
> why should we go forward with your approach? Is there anything that
> can be done with your approach, but can not be done with mine?
>
> Don't take me wrong, I do not see anything fundamentally wrong with
> your approach. I would just like hear some justification why we should
> abandon my approach - that I've been working on for some time - and go
> forward with yours.

Both implementations are similar in term of feature. And i think both 
have advantages and drawbacks...
The main difference, is that my approach is based on a standard service 
client-provider model.
Means that ops are defined by code in charge of providing the service 
(DRM) and not by the client (ALSA).
I don't want to impose my implementation but just propose an alternative
that makes sense for me.

In a first step, before going deep in discussion on the approach, it 
should be interesting to have maintainers feedback, to be sure that my 
approach could make sense from DRM and ALSA point of view.

@DRM (and ALSA) maintainers:
Please, could you give a first feedback on such implementation based on 
DRM API extension?
Is it something that could be acceptable (or not) from your point of view?

>
> Here is couple of benefits I can name in my approach:
> ­ Video side agnostic implementation
>      The ASoC side does not need to know anything about video side
>      implementation. There is no real exposure ASoC side internals in
>      video side either. Even fbdev driver, or some other non DRM video
>      driver, could use my implementation.
My approach is the reverse: DRM driver does not need to know anything 
about audio side. As ALSA is the client of DRM, seems more logical from 
my point of view ...
Now if a generic solution must be found for all video drivers, sure, 
your solution is more flexible.
But if i well understood fbdev drivers are no more accepted for upstream
(please correct me if I'm wrong).
So i don't know we have to keep fbdev in picture...

> - HDMI encoder driver implementations that do not use DRM bridge
>     abstraction do not need add an extra DRM object just to get the
>     audio working.
>
> Short comings I see in the current HDMI audio bridge approach:
>
> In its current from the DRM audio bridge abstraction pretends to be a
> generic audio abstraction for DRM devices, but the implementation is
> quite specific to external HDMI encoders with spdif and/or i2s
> interface. There is a lot of HDMI video devices that provide the
> digital audio interface (ASoC DAI) directly and there is no need for
> anything but dummy codec implementation (if following ASoC
> paradigm). Before going forward I think we should at least consider
> how this abstraction would serve those devices.
Sorry, but i don't see any difference between both implementations for 
this point.In both implementations, ops are called only if defined.
Could you give me name of the drivers you have in mind?
>
> Also, I am not entirely happy how the drm_audio_bridge_funcs are used
> at the moment. The do not map too well to ASoC DAI callbacks and I do
> not see too much point in creating a completely new audio-callback
> abstraction, that is sligtly incompatible with ALSA, and then
> translating alsa callbacks to these new callbacks. I think the
> callbacks should map more or less directly ALSA callbacks.

As API is defined in DRM, it seems more logical to match it with the one
defined for video. From my windows, i didn't see any blocking point to
connect codec callback with this API.
But anyway, this API is not freezed, it could be improved with your help.

Best Regards,
Arnaud

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [alsa-devel] [PATCH RFC V2 0/5] another generic audio hdmi codec proposal
  2015-10-06  9:23   ` Arnaud Pouliquen
@ 2015-10-06 14:33     ` Jean-Francois Moine
  2015-10-07  7:48       ` Arnaud Pouliquen
  2015-10-06 15:44     ` Jyri Sarha
  1 sibling, 1 reply; 17+ messages in thread
From: Jean-Francois Moine @ 2015-10-06 14:33 UTC (permalink / raw)
  To: Arnaud Pouliquen
  Cc: alsa-devel, linux, Benjamin Gaignard, broonie, lgirdwood,
	dri-devel, peter.ujfalusi, tony, tomi.valkeinen, Jyri Sarha,
	bcousson

On Tue, 6 Oct 2015 11:23:03 +0200
Arnaud Pouliquen <arnaud.pouliquen@st.com> wrote:
	[snip]
> As API is defined in DRM, it seems more logical to match it with the one
> defined for video. From my windows, i didn't see any blocking point to
> connect codec callback with this API.
> But anyway, this API is not freezed, it could be improved with your help.

Arnaud, your implementation seems more heavy than Jyri's, and I don't
feel the DRM bridge.

For HDMI, the exchanges between DRM and ALSA are only:
- DRM -> ALSA
  - device connection with the audio constraints
  - device disconnection
- ALSA -> DRM
  - start audio with the chosen audio parameters
  - stop audio

and, in the system, the HDMI devices are seen as DRM connectors (video
view) and as ASoC CODECs (audio view).

We just need a link between these entities.
I don't think that the bridge offers this.

(going further, it seems natural to me that both entities would be
supported by the same kernel device and that the exchange functions and
values could be defined both ways from a common structure - audio
constraints, playback parameters, connector2codec functions,
codec2connector functions - but this is an other story...)

-- 
Ken ar c'hentañ	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH RFC V2 0/5] another generic audio hdmi codec proposal
  2015-10-06  9:23   ` Arnaud Pouliquen
  2015-10-06 14:33     ` [alsa-devel] " Jean-Francois Moine
@ 2015-10-06 15:44     ` Jyri Sarha
  2015-10-06 16:46       ` Mark Brown
  2015-10-07  8:19       ` Arnaud Pouliquen
  1 sibling, 2 replies; 17+ messages in thread
From: Jyri Sarha @ 2015-10-06 15:44 UTC (permalink / raw)
  To: Arnaud Pouliquen, alsa-devel, dri-devel
  Cc: moinejf, linux, Benjamin Gaignard, David Airlie, broonie,
	lgirdwood, peter.ujfalusi, Takashi Iwai, tony, tomi.valkeinen,
	bcousson

On 10/06/15 12:23, Arnaud Pouliquen wrote:
> Hello Jyri,
>
> Thanks your feedback, my answers in line
>
>
> On 10/05/2015 03:27 PM, Jyri Sarha wrote:
>> On 10/01/15 19:50, Arnaud Pouliquen wrote:
>>>
>>> Version 2:
>>> This version integrates missing features upgraded to be aligned when
>>> possible
>>> with patch set:
>>> [PATCH RFC v4 0/8] Implement generic ASoC HDMI codec and use it in
>>> tda998x
>>>
>>
>> There are still some details I would like to change if we decide to go
>> the drm audio bridge way. But before all that, I would like to ask,
>> why should we go forward with your approach? Is there anything that
>> can be done with your approach, but can not be done with mine?
>>
>> Don't take me wrong, I do not see anything fundamentally wrong with
>> your approach. I would just like hear some justification why we should
>> abandon my approach - that I've been working on for some time - and go
>> forward with yours.
>
> Both implementations are similar in term of feature. And i think both
> have advantages and drawbacks...
> The main difference, is that my approach is based on a standard service
> client-provider model.
> Means that ops are defined by code in charge of providing the service
> (DRM) and not by the client (ALSA).
> I don't want to impose my implementation but just propose an alternative
> that makes sense for me.
>

My model merely sees a driver providing access to a piece standard HDMI 
HW with audio functionality trough ASoC codec DAI API.

> In a first step, before going deep in discussion on the approach, it
> should be interesting to have maintainers feedback, to be sure that my
> approach could make sense from DRM and ALSA point of view.
>

Absolutely. In the end the maintainers need to make the final decision 
any way.

> @DRM (and ALSA) maintainers:
> Please, could you give a first feedback on such implementation based on
> DRM API extension?
> Is it something that could be acceptable (or not) from your point of view?
>
>>
>> Here is couple of benefits I can name in my approach:
>> ­ Video side agnostic implementation
>>      The ASoC side does not need to know anything about video side
>>      implementation. There is no real exposure ASoC side internals in
>>      video side either. Even fbdev driver, or some other non DRM video
>>      driver, could use my implementation.
> My approach is the reverse: DRM driver does not need to know anything
> about audio side. As ALSA is the client of DRM, seems more logical from
> my point of view ...
> Now if a generic solution must be found for all video drivers, sure,
> your solution is more flexible.
> But if i well understood fbdev drivers are no more accepted for upstream
> (please correct me if I'm wrong).
> So i don't know we have to keep fbdev in picture...
>

I am not promoting fbdev support. I am merely asking if we want to force 
all HDMI drivers to implement a drm_bridge if they want to support audio.

>> - HDMI encoder driver implementations that do not use DRM bridge
>>     abstraction do not need add an extra DRM object just to get the
>>     audio working.
>>
>> Short comings I see in the current HDMI audio bridge approach:
>>
>> In its current from the DRM audio bridge abstraction pretends to be a
>> generic audio abstraction for DRM devices, but the implementation is
>> quite specific to external HDMI encoders with spdif and/or i2s
>> interface. There is a lot of HDMI video devices that provide the
>> digital audio interface (ASoC DAI) directly and there is no need for
>> anything but dummy codec implementation (if following ASoC
>> paradigm). Before going forward I think we should at least consider
>> how this abstraction would serve those devices.
> Sorry, but i don't see any difference between both implementations for
> this point.In both implementations, ops are called only if defined.
> Could you give me name of the drivers you have in mind?

I am not talking about Beaglebone-Black or tda998x here. There are 
platforms where video HW provides the digital audio interface for HDMI 
audio directly. For instance OMAP4 and OMAP5 (see 
sound/soc/omap/omap-hdmi-audio.c and drivers/video/fbdev/omap2/dss/) are 
like that.

>>
>> Also, I am not entirely happy how the drm_audio_bridge_funcs are used
>> at the moment. The do not map too well to ASoC DAI callbacks and I do
>> not see too much point in creating a completely new audio-callback
>> abstraction, that is sligtly incompatible with ALSA, and then
>> translating alsa callbacks to these new callbacks. I think the
>> callbacks should map more or less directly ALSA callbacks.
>
> As API is defined in DRM, it seems more logical to match it with the one
> defined for video. From my windows, i didn't see any blocking point to
> connect codec callback with this API.
> But anyway, this API is not freezed, it could be improved with your help.

The most usual things can be made to work with your API too, but the 
translation is not the most efficient one and some features are missing, 
for instance muting. Also, in ALSA side the prepare and trigger 
callbacks are meant to be used only for starting and stopping the audio 
stream. The stream is configured before either one of those are called. 
With your API each pause+resume from a video client will reconfigure the 
audio from the scratch.

Best regards,
Jyri

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH RFC V2 0/5] another generic audio hdmi codec proposal
  2015-10-06 15:44     ` Jyri Sarha
@ 2015-10-06 16:46       ` Mark Brown
  2015-10-06 18:51         ` [alsa-devel] " Russell King - ARM Linux
  2015-10-07  8:19       ` Arnaud Pouliquen
  1 sibling, 1 reply; 17+ messages in thread
From: Mark Brown @ 2015-10-06 16:46 UTC (permalink / raw)
  To: Jyri Sarha
  Cc: moinejf, alsa-devel, linux, Benjamin Gaignard, David Airlie,
	Arnaud Pouliquen, lgirdwood, dri-devel, peter.ujfalusi,
	Takashi Iwai, tony, tomi.valkeinen, bcousson


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

On Tue, Oct 06, 2015 at 06:44:34PM +0300, Jyri Sarha wrote:
> On 10/06/15 12:23, Arnaud Pouliquen wrote:

> >In a first step, before going deep in discussion on the approach, it
> >should be interesting to have maintainers feedback, to be sure that my
> >approach could make sense from DRM and ALSA point of view.

> Absolutely. In the end the maintainers need to make the final decision any
> way.

As I keep saying at least for me a very big part of that is going to be
seeing some degree of consensus among the people working on HDMI.  I
really don't want to get something merged and then having people turn up
shortly afterwards saying that there's some reason to redo everything
and I don't have much familiarity with the video stuff so I'm largely
relying on consensus and review from the DRM side (which seems to have
been absent thus far).

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

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



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [alsa-devel] [PATCH RFC V2 0/5] another generic audio hdmi codec proposal
  2015-10-06 16:46       ` Mark Brown
@ 2015-10-06 18:51         ` Russell King - ARM Linux
  2015-10-07  7:36           ` Jyri Sarha
  0 siblings, 1 reply; 17+ messages in thread
From: Russell King - ARM Linux @ 2015-10-06 18:51 UTC (permalink / raw)
  To: Mark Brown
  Cc: alsa-devel, Benjamin Gaignard, Arnaud Pouliquen, lgirdwood,
	dri-devel, peter.ujfalusi, tony, tomi.valkeinen, Jyri Sarha,
	bcousson

On Tue, Oct 06, 2015 at 05:46:02PM +0100, Mark Brown wrote:
> On Tue, Oct 06, 2015 at 06:44:34PM +0300, Jyri Sarha wrote:
> > On 10/06/15 12:23, Arnaud Pouliquen wrote:
> 
> > >In a first step, before going deep in discussion on the approach, it
> > >should be interesting to have maintainers feedback, to be sure that my
> > >approach could make sense from DRM and ALSA point of view.
> 
> > Absolutely. In the end the maintainers need to make the final decision any
> > way.
> 
> As I keep saying at least for me a very big part of that is going to be
> seeing some degree of consensus among the people working on HDMI.  I
> really don't want to get something merged and then having people turn up
> shortly afterwards saying that there's some reason to redo everything
> and I don't have much familiarity with the video stuff so I'm largely
> relying on consensus and review from the DRM side (which seems to have
> been absent thus far).

As I've found, having also looked at getting CEC up and running both on
TDA998x and on dw-hdmi, having some kind of fixed structure between the
video side and the audio side doesn't work.

We at least need some kind of notification system from the video part
to the other parts, so that we can sanely transmit connect/disconnect/
edid updates to different parts of the system *not* that I'm suggesting
that in the case of the Dove Cubox, seeing a disconnect on HDMI should
kill the audio - it most _certainly_ _never_ should there, it would be
utterly insane, and I'm going to play hard-ball on that case.

CEC is much more critical to getting these events - knowing when the
sink device has been plugged in or unplugged is critical to proper
operation of CEC, as is knowing the CEC physical bus address - and
that comes from the EDID.  Hence why a notification system that drivers
can hook into is critical.

Audio is just another example of needing to have communication between
the video and audio parts.

I'm not saying that I have something that's a perfect solution; I've
been rapidly losing interest in iMX6 recently due to the glacial
progress of everything in that arena, and the endless stream of new
bugs being introduced (I'm pretty sure Freescale have a bug quota
which must be maintained at a certain level in all software.)  As I
now have a couple of new Armada 388 boards, this has taken my focus
almost completely off iMX6 and onto that right now.

However, here's the kind of thing that I prototyped to get CEC working
on iMX6 and Dove.  It's not a patch set, because that requires me to
pull together patches from two different trees and sort out out properly,
so this is to give some ideas.

It's also pending sorting out what CEC subsystem is going to make it
into the kernel - I've only last night managed to find the time to try
out the framework which Hans Verkuil posted at the start of September,
which I still have a few concerns with, especially with the publication
of the device prior to driver setup having been completed.

Anyway, here's the prototype patch.  Obviously won't apply to any
kernel version anyone has, but should give some food for thought.

 drivers/cec/Makefile             |  1 +
 drivers/cec/cec-dw_hdmi.c        | 90 ++++++++++++++++++++++++++++++++++++++++
 drivers/cec/hdmi-not.c           | 61 +++++++++++++++++++++++++++
 drivers/gpu/drm/bridge/dw_hdmi.c |  9 ++++
 include/linux/hdmi-not.h         | 39 +++++++++++++++++
 5 files changed, 200 insertions(+)

diff --git a/drivers/cec/Makefile b/drivers/cec/Makefile
index 9e31a9333294..69c354c60b07 100644
--- a/drivers/cec/Makefile
+++ b/drivers/cec/Makefile
@@ -1,3 +1,4 @@
+obj-y += hdmi-not.o
 obj-$(CONFIG_HDMI_CEC_CORE) += cec-dev.o
 obj-$(CONFIG_HDMI_CEC_EVDEV) += cec-input.o
 
diff --git a/drivers/cec/cec-dw_hdmi.c b/drivers/cec/cec-dw_hdmi.c
index b56844296d3e..a693808a881a 100644
--- a/drivers/cec/cec-dw_hdmi.c
+++ b/drivers/cec/cec-dw_hdmi.c
@@ -1,14 +1,18 @@
 /* http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git/
  * tree/drivers/mxc/hdmi-cec/mxc_hdmi-cec.c?h=imx_3.0.35_4.1.0 */
 #include <linux/cec-dev.h>
+#include <linux/hdmi-not.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
 #include <linux/module.h>
+#include <linux/notifier.h>
 #include <linux/platform_data/dw_hdmi-cec.h>
 #include <linux/platform_device.h>
 #include <linux/sched.h>
 #include <linux/slab.h>
 
+#include <drm/drm_edid.h>
+
 #define DEV_NAME "mxc_hdmi_cec"
 
 enum {
@@ -47,6 +51,8 @@ struct dw_hdmi_cec {
 	void *ops_data;
 	int retries;
 	int irq;
+	struct cec_dev *cecdev;
+	struct notifier_block nb;
 };
 
 static void dw_hdmi_set_address(struct cec_dev *cecdev, unsigned addresses)
@@ -162,6 +168,83 @@ static const struct cec_dev_ops dw_hdmi_cec_dev_ops = {
 	.set_address = dw_hdmi_set_address,
 };
 
+static unsigned int cea_len(const u8 *ext, unsigned int offset)
+{
+	return ext[offset] & 0x1f;
+}
+
+static unsigned int cea_tag(const u8 *ext, unsigned int offset)
+{
+	return ext[offset] >> 5;
+}
+
+static unsigned int parse_hdmi_addr(const struct edid *edid)
+{
+	unsigned int i, end;
+	const u8 *edid_ext = NULL;
+
+	if (!edid || edid->extensions == 0)
+		return (u16)~0;
+
+	for (i = 0; i < edid->extensions; i++) {
+		edid_ext = (u8 *)edid + EDID_LENGTH * (i + 1);
+		if (edid_ext[0] == CEA_EXT)
+			break;
+		edid_ext = NULL;
+	}
+
+	if (!edid_ext)
+		return (u16)~0;
+
+	end = edid_ext[2];
+	if (end < 4 || end > 127)
+		return (u16)~0;
+
+	for (i = 4; i < end && i + cea_len(edid_ext, i) < end; i += 1 + cea_len(edid_ext, i)) {
+pr_info("cea tag %u len %u\n", cea_tag(edid_ext, i), cea_len(edid_ext, i));
+		if (cea_tag(edid_ext, i) != 3 || cea_len(edid_ext, i) < 5)
+			continue;
+
+		if (edid_ext[i + 1] != 0x03 ||
+		    edid_ext[i + 2] != 0x0c ||
+		    edid_ext[i + 3] != 0x00)
+			continue;
+
+		return edid_ext[i + 4] << 8 | edid_ext[i + 5];
+	}
+
+	return (u16)~0;
+}
+
+static int dw_hdmi_cec_notify(struct notifier_block *nb, unsigned long event,
+			      void *data)
+{
+	struct dw_hdmi_cec *cec = container_of(nb, struct dw_hdmi_cec, nb);
+	union hdmi_event *event_block = data;
+	unsigned int phys;
+
+	dev_info(cec->cecdev->class_dev.parent, "event %lu\n", event);
+
+	if (event_block->base.source != cec->cecdev->class_dev.parent)
+		return NOTIFY_OK;
+
+	switch (event) {
+	case HDMI_CONNECTED:
+		break;
+
+	case HDMI_DISCONNECTED:
+		cec_dev_disconnect(cec->cecdev);
+		break;
+
+	case HDMI_NEW_EDID:
+		phys = parse_hdmi_addr(event_block->edid.edid);
+		cec_dev_connect(cec->cecdev, phys);
+		break;
+	}
+
+	return NOTIFY_OK;
+}
+
 static int dw_hdmi_cec_probe(struct platform_device *pdev)
 {
 	struct dw_hdmi_cec_data *data = dev_get_platdata(&pdev->dev);
@@ -186,6 +269,8 @@ static int dw_hdmi_cec_probe(struct platform_device *pdev)
 	cec->irq = data->irq;
 	cec->ops = data->ops;
 	cec->ops_data = data->ops_data;
+	cec->cecdev = cecdev;
+	cec->nb.notifier_call = dw_hdmi_cec_notify;
 
 	cecdev->ops = &dw_hdmi_cec_dev_ops;
 
@@ -197,6 +282,8 @@ static int dw_hdmi_cec_probe(struct platform_device *pdev)
 	writeb_relaxed(~0, cec->base + HDMI_IH_MUTE_CEC_STAT0);
 	writeb_relaxed(0, cec->base + HDMI_CEC_POLARITY);
 
+	hdmi_register_notifier(&cec->nb);
+
 	platform_set_drvdata(pdev, cecdev);
 
 	ret = cec_dev_add(cecdev);
@@ -209,6 +296,9 @@ static int dw_hdmi_cec_probe(struct platform_device *pdev)
 static int dw_hdmi_cec_remove(struct platform_device *pdev)
 {
 	struct cec_dev *cecdev = platform_get_drvdata(pdev);
+	struct dw_hdmi_cec *cec = cecdev_priv(cecdev);
+
+	hdmi_unregister_notifier(&cec->nb);
 
 	cec_dev_remove(cecdev);
 	cec_dev_free(cecdev);
diff --git a/drivers/cec/hdmi-not.c b/drivers/cec/hdmi-not.c
new file mode 100644
index 000000000000..ba3be8afeb8f
--- /dev/null
+++ b/drivers/cec/hdmi-not.c
@@ -0,0 +1,61 @@
+#include <linux/export.h>
+#include <linux/hdmi-not.h>
+#include <linux/notifier.h>
+#include <linux/string.h>
+
+static BLOCKING_NOTIFIER_HEAD(hdmi_notifier);
+
+int hdmi_register_notifier(struct notifier_block *nb)
+{
+	return blocking_notifier_chain_register(&hdmi_notifier, nb);
+}
+EXPORT_SYMBOL_GPL(hdmi_register_notifier);
+
+int hdmi_unregister_notifier(struct notifier_block *nb)
+{
+	return blocking_notifier_chain_unregister(&hdmi_notifier, nb);
+}
+EXPORT_SYMBOL_GPL(hdmi_unregister_notifier);
+
+void hdmi_event_connect(struct device *dev)
+{
+	struct hdmi_event_base base;
+
+	base.source = dev;
+
+	blocking_notifier_call_chain(&hdmi_notifier, HDMI_CONNECTED, &base);
+}
+EXPORT_SYMBOL_GPL(hdmi_event_connect);
+
+void hdmi_event_disconnect(struct device *dev)
+{
+	struct hdmi_event_base base;
+
+	base.source = dev;
+
+	blocking_notifier_call_chain(&hdmi_notifier, HDMI_DISCONNECTED, &base);
+}
+EXPORT_SYMBOL_GPL(hdmi_event_disconnect);
+
+void hdmi_event_new_edid(struct device *dev, const void *edid, size_t size)
+{
+	struct hdmi_event_new_edid new_edid;
+
+	new_edid.base.source = dev;
+	new_edid.edid = edid;
+	new_edid.size = size;
+
+	blocking_notifier_call_chain(&hdmi_notifier, HDMI_NEW_EDID, &new_edid);
+}
+EXPORT_SYMBOL_GPL(hdmi_event_new_edid);
+
+void hdmi_event_new_eld(struct device *dev, const void *eld)
+{
+	struct hdmi_event_new_eld new_eld;
+
+	new_eld.base.source = dev;
+	memcpy(new_eld.eld, eld, sizeof(new_eld.eld));
+
+	blocking_notifier_call_chain(&hdmi_notifier, HDMI_NEW_ELD, &new_eld);
+}
+EXPORT_SYMBOL_GPL(hdmi_event_new_eld);
diff --git a/drivers/gpu/drm/bridge/dw_hdmi.c b/drivers/gpu/drm/bridge/dw_hdmi.c
index 41d08f0cf88c..cb8764eecd70 100644
--- a/drivers/gpu/drm/bridge/dw_hdmi.c
+++ b/drivers/gpu/drm/bridge/dw_hdmi.c
@@ -16,6 +16,7 @@
 #include <linux/err.h>
 #include <linux/clk.h>
 #include <linux/hdmi.h>
+#include <linux/hdmi-not.h>
 #include <linux/mutex.h>
 #include <linux/of_device.h>
 #include <linux/platform_data/dw_hdmi-cec.h>
@@ -1484,9 +1485,11 @@ static int dw_hdmi_connector_get_modes(struct drm_connector *connector)
 		hdmi->sink_is_hdmi = drm_detect_hdmi_monitor(edid);
 		hdmi->sink_has_audio = drm_detect_monitor_audio(edid);
 		drm_mode_connector_update_edid_property(connector, edid);
+		hdmi_event_new_edid(hdmi->dev, edid, 0);
 		ret = drm_add_edid_modes(connector, edid);
 		/* Store the ELD */
 		drm_edid_to_eld(connector, edid);
+		hdmi_event_new_eld(hdmi->dev, connector->eld);
 		kfree(edid);
 	} else {
 		dev_dbg(hdmi->dev, "failed to get edid\n");
@@ -1648,6 +1651,12 @@ static irqreturn_t dw_hdmi_irq(int irq, void *dev_id)
 			dw_hdmi_update_phy_mask(hdmi);
 		}
 		mutex_unlock(&hdmi->mutex);
+
+		if ((phy_stat & (HDMI_PHY_RX_SENSE | HDMI_PHY_HPD)) == 0)
+			hdmi_event_disconnect(hdmi->dev);
+		else if ((phy_stat & (HDMI_PHY_RX_SENSE | HDMI_PHY_HPD)) ==
+			 (HDMI_IH_PHY_STAT0_RX_SENSE | HDMI_PHY_HPD))
+			hdmi_event_connect(hdmi->dev);
 	}
 
 	if (intr_stat & HDMI_IH_PHY_STAT0_HPD) {
diff --git a/include/linux/hdmi-not.h b/include/linux/hdmi-not.h
new file mode 100644
index 000000000000..940ece45bbaf
--- /dev/null
+++ b/include/linux/hdmi-not.h
@@ -0,0 +1,39 @@
+#include <linux/types.h>
+
+enum {
+	HDMI_CONNECTED,
+	HDMI_DISCONNECTED,
+	HDMI_NEW_EDID,
+	HDMI_NEW_ELD,
+};
+
+struct hdmi_event_base {
+	struct device *source;
+};
+
+struct hdmi_event_new_edid {
+	struct hdmi_event_base base;
+	const void *edid;
+	size_t size;
+};
+
+struct hdmi_event_new_eld {
+	struct hdmi_event_base base;
+	unsigned char eld[128];
+};
+
+union hdmi_event {
+	struct hdmi_event_base base;
+	struct hdmi_event_new_edid edid;
+	struct hdmi_event_new_eld eld;
+};
+
+struct notifier_block;
+
+int hdmi_register_notifier(struct notifier_block *nb);
+int hdmi_unregister_notifier(struct notifier_block *nb);
+
+void hdmi_event_connect(struct device *dev);
+void hdmi_event_disconnect(struct device *dev);
+void hdmi_event_new_edid(struct device *dev, const void *edid, size_t size);
+void hdmi_event_new_eld(struct device *dev, const void *eld);


-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH RFC V2 0/5] another generic audio hdmi codec proposal
  2015-10-06 18:51         ` [alsa-devel] " Russell King - ARM Linux
@ 2015-10-07  7:36           ` Jyri Sarha
  0 siblings, 0 replies; 17+ messages in thread
From: Jyri Sarha @ 2015-10-07  7:36 UTC (permalink / raw)
  To: Russell King - ARM Linux, Mark Brown
  Cc: moinejf, alsa-devel, Benjamin Gaignard, David Airlie,
	Arnaud Pouliquen, lgirdwood, dri-devel, peter.ujfalusi,
	Takashi Iwai, tony, tomi.valkeinen, bcousson

On 10/06/15 21:51, Russell King - ARM Linux wrote:
> We at least need some kind of notification system from the video part
> to the other parts, so that we can sanely transmit connect/disconnect/
> edid updates to different parts of the system*not*  that I'm suggesting
> that in the case of the Dove Cubox, seeing a disconnect on HDMI should
> kill the audio - it most_certainly_  _never_  should there, it would be
> utterly insane, and I'm going to play hard-ball on that case.

I am not proposing any automatic mechanism for killing the audio in the 
my hdmi codec patch set. I am merely providing the means for the video 
driver to gracefully abort the audio stream if the driver decides it can 
not support it any more.

For external HDMI encoders that is hardly ever needed, since the CPU DAI 
is usually the bit- and frame-clock master in those cases. The CPU DAI 
can go on banging the bits even if the encoder isn't listening. So we 
can drop the abort_cb() that if it still annoys you after this explanation.

Best regards,
Jyri

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH RFC V2 0/5] another generic audio hdmi codec proposal
  2015-10-06 14:33     ` [alsa-devel] " Jean-Francois Moine
@ 2015-10-07  7:48       ` Arnaud Pouliquen
  2015-10-07  9:05         ` [alsa-devel] " Russell King - ARM Linux
  0 siblings, 1 reply; 17+ messages in thread
From: Arnaud Pouliquen @ 2015-10-07  7:48 UTC (permalink / raw)
  To: Jean-Francois Moine
  Cc: alsa-devel, linux, Benjamin Gaignard, David Airlie, broonie,
	lgirdwood, dri-devel, peter.ujfalusi, Takashi Iwai, tony,
	tomi.valkeinen, Jyri Sarha, bcousson


On 10/06/2015 04:33 PM, Jean-Francois Moine wrote:
> On Tue, 6 Oct 2015 11:23:03 +0200
> Arnaud Pouliquen <arnaud.pouliquen@st.com> wrote:
> 	[snip]
>> As API is defined in DRM, it seems more logical to match it with the one
>> defined for video. From my windows, i didn't see any blocking point to
>> connect codec callback with this API.
>> But anyway, this API is not freezed, it could be improved with your help.
>
> Arnaud, your implementation seems more heavy than Jyri's, and I don't
> feel the DRM bridge.
>
> For HDMI, the exchanges between DRM and ALSA are only:
> - DRM -> ALSA
>    - device connection with the audio constraints
>    - device disconnection
> - ALSA -> DRM
>    - start audio with the chosen audio parameters
>    - stop audio
>
> and, in the system, the HDMI devices are seen as DRM connectors (video
> view) and as ASoC CODECs (audio view).
>
> We just need a link between these entities.
> I don't think that the bridge offers this.
Not so heavy...
In fact another way to differentiate the twice approaches could be to
answer to this question:
Should ALSA bypasses or not the DRM core/API to access to DRM HDMI 
connector?

Concerning bridge usage, I tried to found a link to address DRM driver 
trough DRM API. Bridge seemed a good candidate. Perhaps the DRM encoder 
API could be an other option to study...

Ken ar wech all/ best Regards
Arnaud

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH RFC V2 0/5] another generic audio hdmi codec proposal
  2015-10-06 15:44     ` Jyri Sarha
  2015-10-06 16:46       ` Mark Brown
@ 2015-10-07  8:19       ` Arnaud Pouliquen
  2015-10-07 10:21         ` Lars-Peter Clausen
  1 sibling, 1 reply; 17+ messages in thread
From: Arnaud Pouliquen @ 2015-10-07  8:19 UTC (permalink / raw)
  To: Jyri Sarha, alsa-devel, dri-devel
  Cc: moinejf, linux, Benjamin Gaignard, David Airlie, broonie,
	lgirdwood, peter.ujfalusi, Takashi Iwai, tony, tomi.valkeinen,
	bcousson



>> My approach is the reverse: DRM driver does not need to know anything
>> about audio side. As ALSA is the client of DRM, seems more logical from
>> my point of view ...
>> Now if a generic solution must be found for all video drivers, sure,
>> your solution is more flexible.
>> But if i well understood fbdev drivers are no more accepted for upstream
>> (please correct me if I'm wrong).
>> So i don't know we have to keep fbdev in picture...
>>
>
> I am not promoting fbdev support. I am merely asking if we want to force
> all HDMI drivers to implement a drm_bridge if they want to support audio.
>
Yes this is a good point... My implementation is based on hypothesis 
that HDMI drivers are now upstreamed as DRM drivers.

>>> - HDMI encoder driver implementations that do not use DRM bridge
>>>      abstraction do not need add an extra DRM object just to get the
>>>      audio working.
>>>
>>> Short comings I see in the current HDMI audio bridge approach:
>>>
>>> In its current from the DRM audio bridge abstraction pretends to be a
>>> generic audio abstraction for DRM devices, but the implementation is
>>> quite specific to external HDMI encoders with spdif and/or i2s
>>> interface. There is a lot of HDMI video devices that provide the
>>> digital audio interface (ASoC DAI) directly and there is no need for
>>> anything but dummy codec implementation (if following ASoC
>>> paradigm). Before going forward I think we should at least consider
>>> how this abstraction would serve those devices.
>> Sorry, but i don't see any difference between both implementations for
>> this point.In both implementations, ops are called only if defined.
>> Could you give me name of the drivers you have in mind?
>
> I am not talking about Beaglebone-Black or tda998x here. There are
> platforms where video HW provides the digital audio interface for HDMI
> audio directly. For instance OMAP4 and OMAP5 (see
> sound/soc/omap/omap-hdmi-audio.c and drivers/video/fbdev/omap2/dss/) are
> like that.
Not checked in details but seems similar to your approach except ops 
used by cpu_dai.

>
>>>
>>> Also, I am not entirely happy how the drm_audio_bridge_funcs are used
>>> at the moment. The do not map too well to ASoC DAI callbacks and I do
>>> not see too much point in creating a completely new audio-callback
>>> abstraction, that is sligtly incompatible with ALSA, and then
>>> translating alsa callbacks to these new callbacks. I think the
>>> callbacks should map more or less directly ALSA callbacks.
>>
>> As API is defined in DRM, it seems more logical to match it with the one
>> defined for video. From my windows, i didn't see any blocking point to
>> connect codec callback with this API.
>> But anyway, this API is not freezed, it could be improved with your help.
>
> The most usual things can be made to work with your API too, but the
> translation is not the most efficient one and some features are missing,
> for instance muting.
Could be added but is it necessary if trigger is implemented?( see my 
next comments)

> Also, in ALSA side the prepare and trigger
> callbacks are meant to be used only for starting and stopping the audio
> stream.
Yes but for me, it is required. Otherwise how to manage CPU-DAI master 
configuration?
if stream is stopped, cpu_dai can be stopped as there is no more stream 
to sent on bus. If no information is sent to HDMI, This generates HDMI 
codec or protocol issue...
Use startup/shutdown/digital_mute seems not sufficient.

The stream is configured before either one of those are called.
> With your API each pause+resume from a video client will reconfigure the
> audio from the scratch.
Configuration is sent using pre_enable operation. So no reconfiguration 
should be re-applied.

Best Regards
Arnaud

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [alsa-devel] [PATCH RFC V2 0/5] another generic audio hdmi codec proposal
  2015-10-07  7:48       ` Arnaud Pouliquen
@ 2015-10-07  9:05         ` Russell King - ARM Linux
  0 siblings, 0 replies; 17+ messages in thread
From: Russell King - ARM Linux @ 2015-10-07  9:05 UTC (permalink / raw)
  To: Arnaud Pouliquen
  Cc: alsa-devel, Benjamin Gaignard, broonie, lgirdwood, dri-devel,
	peter.ujfalusi, tony, tomi.valkeinen, Jyri Sarha, bcousson

On Wed, Oct 07, 2015 at 09:48:42AM +0200, Arnaud Pouliquen wrote:
> Should ALSA bypasses or not the DRM core/API to access to DRM HDMI
> connector?

You don't need direct access to the connector - and actually, having
direct access to the connector is potentially racy.  So no.

> Concerning bridge usage, I tried to found a link to address DRM driver
> trough DRM API. Bridge seemed a good candidate. Perhaps the DRM encoder API
> could be an other option to study...

Different parts of DRM don't help you with this: whether it's a connector,
encoder or bridge, you still need some way to get information in and out
from the HDMI video component to both HDMI audio and HDMI CEC components.

Please start seeing that the problem you have in front of you covers more
than _just_ audio.  If you design it just for audio, then we'll probably
have to redesign it when addressing CEC support.

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH RFC V2 0/5] another generic audio hdmi codec proposal
  2015-10-07  8:19       ` Arnaud Pouliquen
@ 2015-10-07 10:21         ` Lars-Peter Clausen
  0 siblings, 0 replies; 17+ messages in thread
From: Lars-Peter Clausen @ 2015-10-07 10:21 UTC (permalink / raw)
  To: Arnaud Pouliquen, Jyri Sarha, alsa-devel, dri-devel
  Cc: moinejf, linux, bcousson, David Airlie, tomi.valkeinen,
	lgirdwood, peter.ujfalusi, Takashi Iwai, tony, broonie,
	Hans Verkuil, Benjamin Gaignard, linux-media


Added Hans, who's working a lot on the HDMI transmitter drivers (including
audio support) as well as the media list to Cc.

On 10/07/2015 10:19 AM, Arnaud Pouliquen wrote:
> 
> 
>>> My approach is the reverse: DRM driver does not need to know anything
>>> about audio side. As ALSA is the client of DRM, seems more logical from
>>> my point of view ...
>>> Now if a generic solution must be found for all video drivers, sure,
>>> your solution is more flexible.
>>> But if i well understood fbdev drivers are no more accepted for upstream
>>> (please correct me if I'm wrong).
>>> So i don't know we have to keep fbdev in picture...
>>>
>>
>> I am not promoting fbdev support. I am merely asking if we want to force
>> all HDMI drivers to implement a drm_bridge if they want to support audio.
>>
> Yes this is a good point... My implementation is based on hypothesis that
> HDMI drivers are now upstreamed as DRM drivers.

The other place where you can find HDMI support is in V4L2, both receive as
well as transmit. And while the hope for fbdev is that it will be phased out
V4L2 will stay around for a while. And we probably want to have a common API
that can take care of both DRM and V4L so we do not need two sets of helper
functions for things like EDID parsing etc.

- Lars

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2015-10-07 10:21 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-01 16:50 [PATCH RFC V2 0/5] another generic audio hdmi codec proposal Arnaud Pouliquen
2015-10-01 16:50 ` [PATCH RFC V2 1/5] video: hdmi: add helper function for N and CTS Arnaud Pouliquen
2015-10-01 16:50 ` [PATCH RFC V2 2/5] drm: add helper functions to add audio capabilities for bridge Arnaud Pouliquen
2015-10-01 16:50 ` [PATCH RFC V2 3/5] ASoC: codec: hdmi drm codec driver Arnaud Pouliquen
2015-10-01 16:50 ` [PATCH RFC V2 4/5] drm: sti: connect audio driver Arnaud Pouliquen
2015-10-01 16:50 ` [PATCH RFC V2 5/5] DT: sti: add audio HDMI dai link in audio card Arnaud Pouliquen
2015-10-05 13:27 ` [PATCH RFC V2 0/5] another generic audio hdmi codec proposal Jyri Sarha
2015-10-06  9:23   ` Arnaud Pouliquen
2015-10-06 14:33     ` [alsa-devel] " Jean-Francois Moine
2015-10-07  7:48       ` Arnaud Pouliquen
2015-10-07  9:05         ` [alsa-devel] " Russell King - ARM Linux
2015-10-06 15:44     ` Jyri Sarha
2015-10-06 16:46       ` Mark Brown
2015-10-06 18:51         ` [alsa-devel] " Russell King - ARM Linux
2015-10-07  7:36           ` Jyri Sarha
2015-10-07  8:19       ` Arnaud Pouliquen
2015-10-07 10:21         ` Lars-Peter Clausen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).