[PATCH v4 0/6] drm/bridge: sii902x: HDMI-audio support and some fixes

[PATCH v4 0/6] drm/bridge: sii902x: HDMI-audio support and some fixes

[Jyri Sarha]

Changes since v3:
- Change sil,i2s-fifo-routing of property to i2s-data-lanes.
  Both the binding and the implementation has been updated.
- Refer to audio-graph-card.txt and simple-card.txt binding
- remove include/dt-bindings/sound/sii902x-audio.h
- lower case hex constant
- add Laurent's reviewed-by to applicable patches

The already reviewed patches are in front. Not reviewed still:
- "dt-bindings: display: sii902x: Add HDMI audio bindings"
- "drm/bridge: sii902x: Implement HDMI audio support"

The previous round:
https://patchwork.kernel.org/project/dri-devel/list/?series=91145

Jyri Sarha (5):
  drm/bridge: sii902x: Set output mode to HDMI or DVI according to EDID
  drm/bridge: sii902x: pixel clock unit is 10kHz instead of 1kHz
  dt-bindings: display: sii902x: Remove trailing white space
  dt-bindings: display: sii902x: Add HDMI audio bindings
  drm/bridge: sii902x: Implement HDMI audio support

Tomi Valkeinen (1):
  drm/bridge: sii902x: add input_bus_flags

 .../bindings/display/bridge/sii902x.txt       |  35 +-
 drivers/gpu/drm/bridge/sii902x.c              | 477 +++++++++++++++++-
 2 files changed, 504 insertions(+), 8 deletions(-)

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v4 1/6] drm/bridge: sii902x: add input_bus_flags

[Jyri Sarha]

From: Tomi Valkeinen <tomi.valkeinen@ti.com>

The driver always sets InputBusFmt:EDGE to 0 (falling edge).

Add drm_bridge_timings's input_bus_flags to reflect that the bridge
samples on falling edges.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Signed-off-by: Jyri Sarha <jsarha@ti.com>
Reviewed-by: Andrzej Hajda <a.hajda@samsung.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/gpu/drm/bridge/sii902x.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
index bfa902013aa4..1afa000141d5 100644
--- a/drivers/gpu/drm/bridge/sii902x.c
+++ b/drivers/gpu/drm/bridge/sii902x.c
@@ -459,6 +459,12 @@ static int sii902x_i2c_bypass_deselect(struct i2c_mux_core *mux, u32 chan_id)
 	return 0;
 }
 
+static const struct drm_bridge_timings default_sii902x_timings = {
+	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE
+		 | DRM_BUS_FLAG_SYNC_SAMPLE_NEGEDGE
+		 | DRM_BUS_FLAG_DE_HIGH,
+};
+
 static int sii902x_probe(struct i2c_client *client,
 			 const struct i2c_device_id *id)
 {
@@ -529,6 +535,7 @@ static int sii902x_probe(struct i2c_client *client,
 
 	sii902x->bridge.funcs = &sii902x_bridge_funcs;
 	sii902x->bridge.of_node = dev->of_node;
+	sii902x->bridge.timings = &default_sii902x_timings;
 	drm_bridge_add(&sii902x->bridge);
 
 	i2c_set_clientdata(client, sii902x);
-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v4 2/6] drm/bridge: sii902x: Set output mode to HDMI or DVI according to EDID

[Jyri Sarha]

Set output mode to HDMI or DVI according to EDID HDMI signature.

Signed-off-by: Jyri Sarha <jsarha@ti.com>
Reviewed-by: Andrzej Hajda <a.hajda@samsung.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/gpu/drm/bridge/sii902x.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
index 1afa000141d5..0e21fa419d27 100644
--- a/drivers/gpu/drm/bridge/sii902x.c
+++ b/drivers/gpu/drm/bridge/sii902x.c
@@ -181,11 +181,15 @@ static int sii902x_get_modes(struct drm_connector *connector)
 	struct sii902x *sii902x = connector_to_sii902x(connector);
 	u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24;
 	struct edid *edid;
+	u8 output_mode = SII902X_SYS_CTRL_OUTPUT_DVI;
 	int num = 0, ret;
 
 	edid = drm_get_edid(connector, sii902x->i2cmux->adapter[0]);
 	drm_connector_update_edid_property(connector, edid);
 	if (edid) {
+	        if (drm_detect_hdmi_monitor(edid))
+			output_mode = SII902X_SYS_CTRL_OUTPUT_HDMI;
+
 		num = drm_add_edid_modes(connector, edid);
 		kfree(edid);
 	}
@@ -195,6 +199,11 @@ static int sii902x_get_modes(struct drm_connector *connector)
 	if (ret)
 		return ret;
 
+	ret = regmap_update_bits(sii902x->regmap, SII902X_SYS_CTRL_DATA,
+				 SII902X_SYS_CTRL_OUTPUT_MODE, output_mode);
+	if (ret)
+		return ret;
+
 	return num;
 }
 
-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v4 3/6] drm/bridge: sii902x: pixel clock unit is 10kHz instead of 1kHz

[Jyri Sarha]

The pixel clock unit in the first two registers (0x00 and 0x01) of
sii9022 is 10kHz, not 1kHz as in struct drm_display_mode. Division by
10 fixes the issue.

Signed-off-by: Jyri Sarha <jsarha@ti.com>
Reviewed-by: Andrzej Hajda <a.hajda@samsung.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/gpu/drm/bridge/sii902x.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
index 0e21fa419d27..1d99108440c1 100644
--- a/drivers/gpu/drm/bridge/sii902x.c
+++ b/drivers/gpu/drm/bridge/sii902x.c
@@ -248,10 +248,11 @@ static void sii902x_bridge_mode_set(struct drm_bridge *bridge,
 	struct regmap *regmap = sii902x->regmap;
 	u8 buf[HDMI_INFOFRAME_SIZE(AVI)];
 	struct hdmi_avi_infoframe frame;
+	u16 pixel_clock_10kHz = adj->clock / 10;
 	int ret;
 
-	buf[0] = adj->clock;
-	buf[1] = adj->clock >> 8;
+	buf[0] = pixel_clock_10kHz & 0xff;
+	buf[1] = pixel_clock_10kHz >> 8;
 	buf[2] = adj->vrefresh;
 	buf[3] = 0x00;
 	buf[4] = adj->hdisplay;
-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v4 4/6] dt-bindings: display: sii902x: Remove trailing white space

[Jyri Sarha]

Remove trailing white space from sii902x display bridge binding.

Signed-off-by: Jyri Sarha <jsarha@ti.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 Documentation/devicetree/bindings/display/bridge/sii902x.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/display/bridge/sii902x.txt b/Documentation/devicetree/bindings/display/bridge/sii902x.txt
index 72d2dc6c3e6b..c4c1855ca654 100644
--- a/Documentation/devicetree/bindings/display/bridge/sii902x.txt
+++ b/Documentation/devicetree/bindings/display/bridge/sii902x.txt
@@ -5,7 +5,7 @@ Required properties:
 	- reg: i2c address of the bridge
 
 Optional properties:
-	- interrupts: describe the interrupt line used to inform the host 
+	- interrupts: describe the interrupt line used to inform the host
 	  about hotplug events.
 	- reset-gpios: OF device-tree gpio specification for RST_N pin.
 
-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v4 5/6] dt-bindings: display: sii902x: Add HDMI audio bindings

[Jyri Sarha]

The sii902x chip family supports also HDMI audio. Add binding for
describing the necessary i2s and mclk wiring for it.

Signed-off-by: Jyri Sarha <jsarha@ti.com>
---
 .../bindings/display/bridge/sii902x.txt       | 33 +++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/bridge/sii902x.txt b/Documentation/devicetree/bindings/display/bridge/sii902x.txt
index c4c1855ca654..1a37bbe7c597 100644
--- a/Documentation/devicetree/bindings/display/bridge/sii902x.txt
+++ b/Documentation/devicetree/bindings/display/bridge/sii902x.txt
@@ -9,6 +9,33 @@ Optional properties:
 	  about hotplug events.
 	- reset-gpios: OF device-tree gpio specification for RST_N pin.
 
+	HDMI audio properties:
+	- i2s-data-lanes: Array of up to 4 integers with values of 0-3
+	   Each integer indicates which i2s pin is connected to which
+	   audio fifo. The first integer selects i2s audio pin for the
+	   first audio fifo#0 (HDMI channels 1&2), second for fifo#1
+	   (HDMI channels 3&4), and so on. There is 4 fifos and 4 i2s
+	   pins (SD0 - SD3). Any i2s pin can be connected to any fifo,
+	   but there can be no gaps. E.g. an i2s pin must be mapped to
+	   fifo#0 and fifo#1 before mapping a channel to fifo#2.
+	   I2S HDMI audio is configured only if this property is found.
+	- clocks: phandle and clock specifier for each clock listed in
+          the clock-names property
+	- clock-names: "mclk"
+	    Describes SII902x MCLK input. MCLK is used to produce
+	    HDMI audio CTS values. This property is required if
+	    "i2s-data-lanes"-property is present. This property follows
+	    Documentation/devicetree/bindings/clock/clock-bindings.txt
+	    consumer binding.
+
+	If HDMI audio is configured the sii902x device becomes an ASoC
+	codec component, that can be used in configuring full audio
+	devices with ASoC simple-card or audio-graph-card. See their
+	binding documents on how to describe how the sii902x device is
+	connected to the rest of the audio system:
+	Documentation/devicetree/bindings/sound/simple-card.txt
+	Documentation/devicetree/bindings/sound/audio-graph-card.txt
+
 Optional subnodes:
 	- video input: this subnode can contain a video input port node
 	  to connect the bridge to a display controller output (See this
@@ -21,6 +48,12 @@ Example:
 		compatible = "sil,sii9022";
 		reg = <0x39>;
 		reset-gpios = <&pioA 1 0>;
+
+		#sound-dai-cells = <0>;
+		i2s-data-lanes = < 0 1 2 >;
+		clocks = <&mclk>;
+		clock-names = "mclk";
+
 		ports {
 			#address-cells = <1>;
 			#size-cells = <0>;
-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v4 6/6] drm/bridge: sii902x: Implement HDMI audio support

[Jyri Sarha]

Implement HDMI audio support by using ASoC HDMI codec. The commit
implements the necessary callbacks and configuration for the HDMI
codec and registers a virtual platform device for the codec to attach.

Signed-off-by: Jyri Sarha <jsarha@ti.com>
---
 drivers/gpu/drm/bridge/sii902x.c | 460 ++++++++++++++++++++++++++++++-
 1 file changed, 453 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
index 1d99108440c1..39a0d25009f2 100644
--- a/drivers/gpu/drm/bridge/sii902x.c
+++ b/drivers/gpu/drm/bridge/sii902x.c
@@ -27,12 +27,15 @@
 #include <linux/i2c.h>
 #include <linux/module.h>
 #include <linux/regmap.h>
+#include <linux/clk.h>
 
 #include <drm/drmP.h>
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_crtc_helper.h>
 #include <drm/drm_edid.h>
 
+#include <sound/hdmi-codec.h>
+
 #define SII902X_TPI_VIDEO_DATA			0x0
 
 #define SII902X_TPI_PIXEL_REPETITION		0x8
@@ -74,6 +77,77 @@
 #define SII902X_AVI_POWER_STATE_MSK		GENMASK(1, 0)
 #define SII902X_AVI_POWER_STATE_D(l)		((l) & SII902X_AVI_POWER_STATE_MSK)
 
+/* Audio  */
+#define SII902X_TPI_I2S_ENABLE_MAPPING_REG	0x1f
+#define SII902X_TPI_I2S_CONFIG_FIFO0			(0 << 0)
+#define SII902X_TPI_I2S_CONFIG_FIFO1			(1 << 0)
+#define SII902X_TPI_I2S_CONFIG_FIFO2			(2 << 0)
+#define SII902X_TPI_I2S_CONFIG_FIFO3			(3 << 0)
+#define SII902X_TPI_I2S_LEFT_RIGHT_SWAP			(1 << 2)
+#define SII902X_TPI_I2S_AUTO_DOWNSAMPLE			(1 << 3)
+#define SII902X_TPI_I2S_SELECT_SD0			(0 << 4)
+#define SII902X_TPI_I2S_SELECT_SD1			(1 << 4)
+#define SII902X_TPI_I2S_SELECT_SD2			(2 << 4)
+#define SII902X_TPI_I2S_SELECT_SD3			(3 << 4)
+#define SII902X_TPI_I2S_FIFO_ENABLE			(1 << 7)
+
+#define SII902X_TPI_I2S_INPUT_CONFIG_REG	0x20
+#define SII902X_TPI_I2S_FIRST_BIT_SHIFT_YES		(0 << 0)
+#define SII902X_TPI_I2S_FIRST_BIT_SHIFT_NO		(1 << 0)
+#define SII902X_TPI_I2S_SD_DIRECTION_MSB_FIRST		(0 << 1)
+#define SII902X_TPI_I2S_SD_DIRECTION_LSB_FIRST		(1 << 1)
+#define SII902X_TPI_I2S_SD_JUSTIFY_LEFT			(0 << 2)
+#define SII902X_TPI_I2S_SD_JUSTIFY_RIGHT		(1 << 2)
+#define SII902X_TPI_I2S_WS_POLARITY_LOW			(0 << 3)
+#define SII902X_TPI_I2S_WS_POLARITY_HIGH		(1 << 3)
+#define SII902X_TPI_I2S_MCLK_MULTIPLIER_128		(0 << 4)
+#define SII902X_TPI_I2S_MCLK_MULTIPLIER_256		(1 << 4)
+#define SII902X_TPI_I2S_MCLK_MULTIPLIER_384		(2 << 4)
+#define SII902X_TPI_I2S_MCLK_MULTIPLIER_512		(3 << 4)
+#define SII902X_TPI_I2S_MCLK_MULTIPLIER_768		(4 << 4)
+#define SII902X_TPI_I2S_MCLK_MULTIPLIER_1024		(5 << 4)
+#define SII902X_TPI_I2S_MCLK_MULTIPLIER_1152		(6 << 4)
+#define SII902X_TPI_I2S_MCLK_MULTIPLIER_192		(7 << 4)
+#define SII902X_TPI_I2S_SCK_EDGE_FALLING		(0 << 7)
+#define SII902X_TPI_I2S_SCK_EDGE_RISING			(1 << 7)
+
+#define SII902X_TPI_I2S_STRM_HDR_BASE	0x21
+#define SII902X_TPI_I2S_STRM_HDR_SIZE	5
+
+#define SII902X_TPI_AUDIO_CONFIG_BYTE2_REG	0x26
+#define SII902X_TPI_AUDIO_CODING_STREAM_HEADER		(0 << 0)
+#define SII902X_TPI_AUDIO_CODING_PCM			(1 << 0)
+#define SII902X_TPI_AUDIO_CODING_AC3			(2 << 0)
+#define SII902X_TPI_AUDIO_CODING_MPEG1			(3 << 0)
+#define SII902X_TPI_AUDIO_CODING_MP3			(4 << 0)
+#define SII902X_TPI_AUDIO_CODING_MPEG2			(5 << 0)
+#define SII902X_TPI_AUDIO_CODING_AAC			(6 << 0)
+#define SII902X_TPI_AUDIO_CODING_DTS			(7 << 0)
+#define SII902X_TPI_AUDIO_CODING_ATRAC			(8 << 0)
+#define SII902X_TPI_AUDIO_MUTE_DISABLE			(0 << 4)
+#define SII902X_TPI_AUDIO_MUTE_ENABLE			(1 << 4)
+#define SII902X_TPI_AUDIO_LAYOUT_2_CHANNELS		(0 << 5)
+#define SII902X_TPI_AUDIO_LAYOUT_8_CHANNELS		(1 << 5)
+#define SII902X_TPI_AUDIO_INTERFACE_DISABLE		(0 << 6)
+#define SII902X_TPI_AUDIO_INTERFACE_SPDIF		(1 << 6)
+#define SII902X_TPI_AUDIO_INTERFACE_I2S			(2 << 6)
+
+#define SII902X_TPI_AUDIO_CONFIG_BYTE3_REG	0x27
+#define SII902X_TPI_AUDIO_FREQ_STREAM			(0 << 3)
+#define SII902X_TPI_AUDIO_FREQ_32KHZ			(1 << 3)
+#define SII902X_TPI_AUDIO_FREQ_44KHZ			(2 << 3)
+#define SII902X_TPI_AUDIO_FREQ_48KHZ			(3 << 3)
+#define SII902X_TPI_AUDIO_FREQ_88KHZ			(4 << 3)
+#define SII902X_TPI_AUDIO_FREQ_96KHZ			(5 << 3)
+#define SII902X_TPI_AUDIO_FREQ_176KHZ			(6 << 3)
+#define SII902X_TPI_AUDIO_FREQ_192KHZ			(7 << 3)
+#define SII902X_TPI_AUDIO_SAMPLE_SIZE_STREAM		(0 << 6)
+#define SII902X_TPI_AUDIO_SAMPLE_SIZE_16		(1 << 6)
+#define SII902X_TPI_AUDIO_SAMPLE_SIZE_20		(2 << 6)
+#define SII902X_TPI_AUDIO_SAMPLE_SIZE_24		(3 << 6)
+
+#define SII902X_TPI_AUDIO_CONFIG_BYTE4_REG	0x28
+
 #define SII902X_INT_ENABLE			0x3c
 #define SII902X_INT_STATUS			0x3d
 #define SII902X_HOTPLUG_EVENT			BIT(0)
@@ -81,6 +155,16 @@
 
 #define SII902X_REG_TPI_RQB			0xc7
 
+/* Indirect internal register access */
+#define SII902X_IND_SET_PAGE			0xbc
+#define SII902X_IND_OFFSET			0xbd
+#define SII902X_IND_VALUE			0xbe
+
+#define SII902X_TPI_MISC_INFOFRAME_BASE		0xbf
+#define SII902X_TPI_MISC_INFOFRAME_END		0xde
+#define SII902X_TPI_MISC_INFOFRAME_SIZE	\
+	(SII902X_TPI_MISC_INFOFRAME_END - SII902X_TPI_MISC_INFOFRAME_BASE)
+
 #define SII902X_I2C_BUS_ACQUISITION_TIMEOUT_MS	500
 
 struct sii902x {
@@ -90,6 +174,16 @@ struct sii902x {
 	struct drm_connector connector;
 	struct gpio_desc *reset_gpio;
 	struct i2c_mux_core *i2cmux;
+	/*
+	 * Mutex protects audio and video functions from interfering
+	 * each other, by keeping their i2c command sequences atomic.
+	 */
+	struct mutex mutex;
+	struct sii902x_audio {
+		struct platform_device *pdev;
+		struct clk *mclk;
+		u32 i2s_fifo_sequence[4];
+	} audio;
 };
 
 static int sii902x_read_unlocked(struct i2c_client *i2c, u8 reg, u8 *val)
@@ -161,8 +255,12 @@ sii902x_connector_detect(struct drm_connector *connector, bool force)
 	struct sii902x *sii902x = connector_to_sii902x(connector);
 	unsigned int status;
 
+	mutex_lock(&sii902x->mutex);
+
 	regmap_read(sii902x->regmap, SII902X_INT_STATUS, &status);
 
+	mutex_unlock(&sii902x->mutex);
+
 	return (status & SII902X_PLUGGED_STATUS) ?
 	       connector_status_connected : connector_status_disconnected;
 }
@@ -180,10 +278,12 @@ static int sii902x_get_modes(struct drm_connector *connector)
 {
 	struct sii902x *sii902x = connector_to_sii902x(connector);
 	u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24;
-	struct edid *edid;
 	u8 output_mode = SII902X_SYS_CTRL_OUTPUT_DVI;
+	struct edid *edid;
 	int num = 0, ret;
 
+	mutex_lock(&sii902x->mutex);
+
 	edid = drm_get_edid(connector, sii902x->i2cmux->adapter[0]);
 	drm_connector_update_edid_property(connector, edid);
 	if (edid) {
@@ -197,14 +297,19 @@ static int sii902x_get_modes(struct drm_connector *connector)
 	ret = drm_display_info_set_bus_formats(&connector->display_info,
 					       &bus_format, 1);
 	if (ret)
-		return ret;
+		goto error_out;
 
 	ret = regmap_update_bits(sii902x->regmap, SII902X_SYS_CTRL_DATA,
 				 SII902X_SYS_CTRL_OUTPUT_MODE, output_mode);
 	if (ret)
-		return ret;
+		goto error_out;
 
-	return num;
+	ret = num;
+
+error_out:
+	mutex_unlock(&sii902x->mutex);
+
+	return ret;
 }
 
 static enum drm_mode_status sii902x_mode_valid(struct drm_connector *connector,
@@ -224,20 +329,28 @@ static void sii902x_bridge_disable(struct drm_bridge *bridge)
 {
 	struct sii902x *sii902x = bridge_to_sii902x(bridge);
 
+	mutex_lock(&sii902x->mutex);
+
 	regmap_update_bits(sii902x->regmap, SII902X_SYS_CTRL_DATA,
 			   SII902X_SYS_CTRL_PWR_DWN,
 			   SII902X_SYS_CTRL_PWR_DWN);
+
+	mutex_unlock(&sii902x->mutex);
 }
 
 static void sii902x_bridge_enable(struct drm_bridge *bridge)
 {
 	struct sii902x *sii902x = bridge_to_sii902x(bridge);
 
+	mutex_lock(&sii902x->mutex);
+
 	regmap_update_bits(sii902x->regmap, SII902X_PWR_STATE_CTRL,
 			   SII902X_AVI_POWER_STATE_MSK,
 			   SII902X_AVI_POWER_STATE_D(0));
 	regmap_update_bits(sii902x->regmap, SII902X_SYS_CTRL_DATA,
 			   SII902X_SYS_CTRL_PWR_DWN, 0);
+
+	mutex_unlock(&sii902x->mutex);
 }
 
 static void sii902x_bridge_mode_set(struct drm_bridge *bridge,
@@ -264,26 +377,31 @@ static void sii902x_bridge_mode_set(struct drm_bridge *bridge,
 	buf[9] = SII902X_TPI_AVI_INPUT_RANGE_AUTO |
 		 SII902X_TPI_AVI_INPUT_COLORSPACE_RGB;
 
+	mutex_lock(&sii902x->mutex);
+
 	ret = regmap_bulk_write(regmap, SII902X_TPI_VIDEO_DATA, buf, 10);
 	if (ret)
-		return;
+		goto out;
 
 	ret = drm_hdmi_avi_infoframe_from_display_mode(&frame, adj, false);
 	if (ret < 0) {
 		DRM_ERROR("couldn't fill AVI infoframe\n");
-		return;
+		goto out;
 	}
 
 	ret = hdmi_avi_infoframe_pack(&frame, buf, sizeof(buf));
 	if (ret < 0) {
 		DRM_ERROR("failed to pack AVI infoframe: %d\n", ret);
-		return;
+		goto out;
 	}
 
 	/* Do not send the infoframe header, but keep the CRC field. */
 	regmap_bulk_write(regmap, SII902X_TPI_AVI_INFOFRAME,
 			  buf + HDMI_INFOFRAME_HEADER_SIZE - 1,
 			  HDMI_AVI_INFOFRAME_SIZE + 1);
+
+out:
+	mutex_unlock(&sii902x->mutex);
 }
 
 static int sii902x_bridge_attach(struct drm_bridge *bridge)
@@ -324,6 +442,329 @@ static const struct drm_bridge_funcs sii902x_bridge_funcs = {
 	.enable = sii902x_bridge_enable,
 };
 
+static int sii902x_mute(struct sii902x *sii902x, bool mute)
+{
+	struct device *dev = &sii902x->i2c->dev;
+	unsigned int val = mute ? SII902X_TPI_AUDIO_MUTE_ENABLE :
+		SII902X_TPI_AUDIO_MUTE_DISABLE;
+
+	dev_dbg(dev, "%s: %s\n", __func__, mute ? "Muted" : "Unmuted");
+
+	return regmap_update_bits(sii902x->regmap,
+				  SII902X_TPI_AUDIO_CONFIG_BYTE2_REG,
+				  SII902X_TPI_AUDIO_MUTE_ENABLE, val);
+}
+
+static const unsigned int sii902x_mclk_div_table[] = {
+	128, 256, 384, 512, 768, 1024, 1152, 192 };
+
+static int sii902x_select_mclk_div(u8 *i2s_config_reg, unsigned int rate,
+				   unsigned int mclk)
+{
+	unsigned int div = mclk / rate;
+	int distance = 100000;
+	u8 i, nearest = 0;
+
+	for (i = 0; i < ARRAY_SIZE(sii902x_mclk_div_table); i++) {
+		unsigned int d = abs(div - sii902x_mclk_div_table[i]);
+
+		if (d >= distance)
+			continue;
+
+		nearest = i;
+		distance = d;
+		if (d == 0)
+			break;
+	}
+
+	*i2s_config_reg |= nearest << 4;
+
+	if (distance != 0)
+		return sii902x_mclk_div_table[nearest];
+
+	return 0;
+}
+
+static const struct sii902x_sample_freq {
+	u32 freq;
+	u8 val;
+} sii902x_sample_freq[] = {
+	{ .freq = 32000,	.val = SII902X_TPI_AUDIO_FREQ_32KHZ },
+	{ .freq = 44000,	.val = SII902X_TPI_AUDIO_FREQ_44KHZ },
+	{ .freq = 48000,	.val = SII902X_TPI_AUDIO_FREQ_48KHZ },
+	{ .freq = 88000,	.val = SII902X_TPI_AUDIO_FREQ_88KHZ },
+	{ .freq = 96000,	.val = SII902X_TPI_AUDIO_FREQ_96KHZ },
+	{ .freq = 176000,	.val = SII902X_TPI_AUDIO_FREQ_176KHZ },
+	{ .freq = 192000,	.val = SII902X_TPI_AUDIO_FREQ_192KHZ },
+};
+
+static int sii902x_audio_hw_params(struct device *dev, void *data,
+				   struct hdmi_codec_daifmt *daifmt,
+				   struct hdmi_codec_params *params)
+{
+	struct sii902x *sii902x = dev_get_drvdata(dev);
+	u8 i2s_config_reg = SII902X_TPI_I2S_SD_DIRECTION_MSB_FIRST;
+	u8 config_byte2_reg = (SII902X_TPI_AUDIO_INTERFACE_I2S |
+			       SII902X_TPI_AUDIO_MUTE_ENABLE |
+			       SII902X_TPI_AUDIO_CODING_PCM);
+	u8 config_byte3_reg = 0;
+	u8 infoframe_buf[HDMI_INFOFRAME_SIZE(AUDIO)];
+	unsigned long mclk_rate;
+	int i, ret;
+
+	if (daifmt->bit_clk_master || daifmt->frame_clk_master) {
+		dev_dbg(dev, "%s: I2S master mode not supported\n", __func__);
+		return -EINVAL;
+	}
+
+	switch (daifmt->fmt) {
+	case HDMI_I2S:
+		i2s_config_reg |= SII902X_TPI_I2S_FIRST_BIT_SHIFT_YES |
+			SII902X_TPI_I2S_SD_JUSTIFY_LEFT;
+		break;
+	case HDMI_RIGHT_J:
+		i2s_config_reg |= SII902X_TPI_I2S_SD_JUSTIFY_RIGHT;
+		break;
+	case HDMI_LEFT_J:
+		i2s_config_reg |= SII902X_TPI_I2S_SD_JUSTIFY_LEFT;
+		break;
+	default:
+		dev_dbg(dev, "%s: Unsupported i2s format %u\n", __func__,
+			daifmt->fmt);
+		return -EINVAL;
+	}
+
+	if (daifmt->bit_clk_inv)
+		i2s_config_reg |= SII902X_TPI_I2S_SCK_EDGE_FALLING;
+	else
+		i2s_config_reg |= SII902X_TPI_I2S_SCK_EDGE_RISING;
+
+	if (daifmt->frame_clk_inv)
+		i2s_config_reg |= SII902X_TPI_I2S_WS_POLARITY_LOW;
+	else
+		i2s_config_reg |= SII902X_TPI_I2S_WS_POLARITY_HIGH;
+
+	if (params->channels > 2)
+		config_byte2_reg |= SII902X_TPI_AUDIO_LAYOUT_8_CHANNELS;
+	else
+		config_byte2_reg |= SII902X_TPI_AUDIO_LAYOUT_2_CHANNELS;
+
+	switch (params->sample_width) {
+	case 16:
+		config_byte3_reg |= SII902X_TPI_AUDIO_SAMPLE_SIZE_16;
+		break;
+	case 20:
+		config_byte3_reg |= SII902X_TPI_AUDIO_SAMPLE_SIZE_20;
+		break;
+	case 24:
+	case 32:
+		config_byte3_reg |= SII902X_TPI_AUDIO_SAMPLE_SIZE_24;
+		break;
+	default:
+		dev_err(dev, "%s: Unsupported sample width %u\n", __func__,
+			params->sample_width);
+		return -EINVAL;
+	}
+
+	for (i = 0; i < ARRAY_SIZE(sii902x_sample_freq); i++) {
+		if (params->sample_rate == sii902x_sample_freq[i].freq) {
+			config_byte3_reg |= sii902x_sample_freq[i].val;
+			break;
+		}
+	}
+
+	ret = clk_prepare_enable(sii902x->audio.mclk);
+	if (ret) {
+		dev_err(dev, "Enabling mclk failed: %d\n", ret);
+		return ret;
+	}
+
+	mclk_rate = clk_get_rate(sii902x->audio.mclk);
+
+	ret = sii902x_select_mclk_div(&i2s_config_reg, params->sample_rate,
+				      mclk_rate);
+	if (ret)
+		dev_dbg(dev, "Inaccurate reference clock (%ld/%d != %u)\n",
+			mclk_rate, ret, params->sample_rate);
+
+	mutex_lock(&sii902x->mutex);
+
+	ret = regmap_write(sii902x->regmap,
+			   SII902X_TPI_AUDIO_CONFIG_BYTE2_REG,
+			   config_byte2_reg);
+	if (ret < 0)
+		goto out;
+
+	ret = regmap_write(sii902x->regmap, SII902X_TPI_I2S_INPUT_CONFIG_REG,
+			   i2s_config_reg);
+	if (ret)
+		goto out;
+
+	for (i = 0; sii902x->audio.i2s_fifo_sequence[i] &&
+		     i < ARRAY_SIZE(sii902x->audio.i2s_fifo_sequence); i++)
+		regmap_write(sii902x->regmap,
+			     SII902X_TPI_I2S_ENABLE_MAPPING_REG,
+			     sii902x->audio.i2s_fifo_sequence[i]);
+
+	ret = regmap_write(sii902x->regmap, SII902X_TPI_AUDIO_CONFIG_BYTE3_REG,
+			   config_byte3_reg);
+	if (ret)
+		goto out;
+
+	ret = regmap_bulk_write(sii902x->regmap, SII902X_TPI_I2S_STRM_HDR_BASE,
+				params->iec.status,
+				min((size_t) SII902X_TPI_I2S_STRM_HDR_SIZE,
+				    sizeof(params->iec.status)));
+	if (ret)
+		goto out;
+
+	ret = hdmi_audio_infoframe_pack(&params->cea, infoframe_buf,
+					sizeof(infoframe_buf));
+	if (ret < 0) {
+		dev_err(dev, "%s: Failed to pack audio infoframe: %d\n",
+			__func__, ret);
+		goto out;
+	}
+
+	ret = regmap_bulk_write(sii902x->regmap,
+				SII902X_TPI_MISC_INFOFRAME_BASE,
+				infoframe_buf,
+				min(ret, SII902X_TPI_MISC_INFOFRAME_SIZE));
+	if (ret)
+		goto out;
+
+	/* Decode Level 0 Packets */
+	ret = regmap_write(sii902x->regmap, SII902X_IND_SET_PAGE, 0x02);
+	if (ret)
+		goto out;
+
+	ret = regmap_write(sii902x->regmap, SII902X_IND_OFFSET, 0x24);
+	if (ret)
+		goto out;
+
+	ret = regmap_write(sii902x->regmap, SII902X_IND_VALUE, 0x02);
+	if (ret)
+		goto out;
+
+	dev_dbg(dev, "%s: hdmi audio enabled\n", __func__);
+out:
+	mutex_unlock(&sii902x->mutex);
+
+	if (ret) {
+		clk_disable_unprepare(sii902x->audio.mclk);
+		dev_err(dev, "%s: hdmi audio enable failed: %d\n", __func__,
+			ret);
+	}
+
+	return ret;
+}
+
+static void sii902x_audio_shutdown(struct device *dev, void *data)
+{
+	struct sii902x *sii902x = dev_get_drvdata(dev);
+
+	mutex_lock(&sii902x->mutex);
+
+	regmap_write(sii902x->regmap, SII902X_TPI_AUDIO_CONFIG_BYTE2_REG,
+		     SII902X_TPI_AUDIO_INTERFACE_DISABLE);
+
+	mutex_unlock(&sii902x->mutex);
+
+	clk_disable_unprepare(sii902x->audio.mclk);
+}
+
+int sii902x_audio_digital_mute(struct device *dev, void *data, bool enable)
+{
+	struct sii902x *sii902x = dev_get_drvdata(dev);
+
+	mutex_lock(&sii902x->mutex);
+
+	sii902x_mute(sii902x, enable);
+
+	mutex_unlock(&sii902x->mutex);
+
+	return 0;
+}
+
+static int sii902x_audio_get_eld(struct device *dev, void *data,
+				 uint8_t *buf, size_t len)
+{
+	struct sii902x *sii902x = dev_get_drvdata(dev);
+
+	mutex_lock(&sii902x->mutex);
+
+	memcpy(buf, sii902x->connector.eld,
+	       min(sizeof(sii902x->connector.eld), len));
+
+	mutex_unlock(&sii902x->mutex);
+
+	return 0;
+}
+
+static const struct hdmi_codec_ops sii902x_audio_codec_ops = {
+	.hw_params = sii902x_audio_hw_params,
+	.audio_shutdown = sii902x_audio_shutdown,
+	.digital_mute = sii902x_audio_digital_mute,
+	.get_eld = sii902x_audio_get_eld,
+};
+
+static int sii902x_audio_codec_init(struct sii902x *sii902x,
+				    struct device *dev)
+{
+	static const u8 audio_fifo_id[] = {
+		SII902X_TPI_I2S_CONFIG_FIFO0,
+		SII902X_TPI_I2S_CONFIG_FIFO1,
+		SII902X_TPI_I2S_CONFIG_FIFO2,
+		SII902X_TPI_I2S_CONFIG_FIFO3,
+	};
+	static const u8 i2s_lane_id[] = {
+		SII902X_TPI_I2S_SELECT_SD0,
+		SII902X_TPI_I2S_SELECT_SD1,
+		SII902X_TPI_I2S_SELECT_SD2,
+		SII902X_TPI_I2S_SELECT_SD3,
+	};
+	struct hdmi_codec_pdata codec_data = {
+		.ops = &sii902x_audio_codec_ops,
+		.i2s = 1, /* Only i2s support for now. */
+		.spdif = 0,
+		.max_i2s_channels = 0,
+	};
+	u8 lanes[4];
+	u32 num_lanes, i;
+
+	num_lanes = of_property_read_variable_u8_array(dev->of_node,
+						       "i2s-data-lanes",
+						       lanes, 1,
+						       ARRAY_SIZE(lanes));
+	if (num_lanes < 0) {
+		if (num_lanes == -EINVAL)
+			dev_dbg(dev, "%s: No \"i2s-data-lanes\", no audio\n",
+			__func__);
+		else
+			dev_err(dev,
+				"%s: Error gettin \"i2s-data-lanes\": %d\n",
+				__func__, num_lanes);
+		return 0;
+	}
+	codec_data.max_i2s_channels = 2 * num_lanes;
+
+	for (i = 0; i < num_lanes; i++)
+		sii902x->audio.i2s_fifo_sequence[i] |= audio_fifo_id[i] |
+			i2s_lane_id[lanes[i]] |	SII902X_TPI_I2S_FIFO_ENABLE;
+
+	if (IS_ERR(sii902x->audio.mclk)) {
+		dev_err(dev, "%s: No clock (audio mclk) found: %ld\n",
+			__func__, PTR_ERR(sii902x->audio.mclk));
+		return 0;
+	}
+
+	sii902x->audio.pdev = platform_device_register_data(
+		dev, HDMI_CODEC_DRV_NAME, PLATFORM_DEVID_AUTO,
+		&codec_data, sizeof(codec_data));
+
+	return PTR_ERR_OR_ZERO(sii902x->audio.pdev);
+}
+
 static const struct regmap_range sii902x_volatile_ranges[] = {
 	{ .range_min = 0, .range_max = 0xff },
 };
@@ -336,6 +777,7 @@ static const struct regmap_access_table sii902x_volatile_table = {
 static const struct regmap_config sii902x_regmap_config = {
 	.reg_bits = 8,
 	.val_bits = 8,
+	.max_register = SII902X_TPI_MISC_INFOFRAME_END,
 	.volatile_table = &sii902x_volatile_table,
 	.cache_type = REGCACHE_NONE,
 };
@@ -508,6 +950,8 @@ static int sii902x_probe(struct i2c_client *client,
 		return PTR_ERR(sii902x->reset_gpio);
 	}
 
+	mutex_init(&sii902x->mutex);
+
 	sii902x_reset(sii902x);
 
 	ret = regmap_write(sii902x->regmap, SII902X_REG_TPI_RQB, 0x0);
@@ -548,6 +992,8 @@ static int sii902x_probe(struct i2c_client *client,
 	sii902x->bridge.timings = &default_sii902x_timings;
 	drm_bridge_add(&sii902x->bridge);
 
+	sii902x_audio_codec_init(sii902x, dev);
+
 	i2c_set_clientdata(client, sii902x);
 
 	sii902x->i2cmux = i2c_mux_alloc(client->adapter, dev,
-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v4 6/6] drm/bridge: sii902x: Implement HDMI audio support

[Olivier MOYSAN]

Hello Jyri,

STM32MP157C-DK2 discovery boards use a SiI9022A HDMI transmitter.
see: https://www.st.com/en/evaluation-tools/stm32mp157c-dk2.html

On this board audio samples are sent to HDMI transmitter through the i2s 
link. This i2s link does not provide a master clock. internal PLL is 
used instead.

The support of audio on this board is already available through the 
following patch:
https://github.com/STMicroelectronics/linux/blob/v4.19-stm32mp/drivers/gpu/drm/bridge/sii902x.c

I tried your patch on this STM32MP157C-DK2 board. I could get audio, 
after reworking the patch. However, I had to make the following changes:
- make master clock configuration optional
- add of graph support for audio

Could you please update current patch to take these restrictions into 
account ?

BRs
Olivier


On 3/14/19 12:27 PM, Jyri Sarha wrote:
> Implement HDMI audio support by using ASoC HDMI codec. The commit
> implements the necessary callbacks and configuration for the HDMI
> codec and registers a virtual platform device for the codec to attach.
> 
> Signed-off-by: Jyri Sarha <jsarha@ti.com>
> ---
>   drivers/gpu/drm/bridge/sii902x.c | 460 ++++++++++++++++++++++++++++++-
>   1 file changed, 453 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
> index 1d99108440c1..39a0d25009f2 100644
> --- a/drivers/gpu/drm/bridge/sii902x.c
> +++ b/drivers/gpu/drm/bridge/sii902x.c
> @@ -27,12 +27,15 @@
>   #include <linux/i2c.h>
>   #include <linux/module.h>
>   #include <linux/regmap.h>
> +#include <linux/clk.h>
>   
>   #include <drm/drmP.h>
>   #include <drm/drm_atomic_helper.h>
>   #include <drm/drm_crtc_helper.h>
>   #include <drm/drm_edid.h>
>   
> +#include <sound/hdmi-codec.h>
> +
>   #define SII902X_TPI_VIDEO_DATA			0x0
>   
>   #define SII902X_TPI_PIXEL_REPETITION		0x8
> @@ -74,6 +77,77 @@
>   #define SII902X_AVI_POWER_STATE_MSK		GENMASK(1, 0)
>   #define SII902X_AVI_POWER_STATE_D(l)		((l) & SII902X_AVI_POWER_STATE_MSK)
>   
> +/* Audio  */
> +#define SII902X_TPI_I2S_ENABLE_MAPPING_REG	0x1f
> +#define SII902X_TPI_I2S_CONFIG_FIFO0			(0 << 0)
> +#define SII902X_TPI_I2S_CONFIG_FIFO1			(1 << 0)
> +#define SII902X_TPI_I2S_CONFIG_FIFO2			(2 << 0)
> +#define SII902X_TPI_I2S_CONFIG_FIFO3			(3 << 0)
> +#define SII902X_TPI_I2S_LEFT_RIGHT_SWAP			(1 << 2)
> +#define SII902X_TPI_I2S_AUTO_DOWNSAMPLE			(1 << 3)
> +#define SII902X_TPI_I2S_SELECT_SD0			(0 << 4)
> +#define SII902X_TPI_I2S_SELECT_SD1			(1 << 4)
> +#define SII902X_TPI_I2S_SELECT_SD2			(2 << 4)
> +#define SII902X_TPI_I2S_SELECT_SD3			(3 << 4)
> +#define SII902X_TPI_I2S_FIFO_ENABLE			(1 << 7)
> +
> +#define SII902X_TPI_I2S_INPUT_CONFIG_REG	0x20
> +#define SII902X_TPI_I2S_FIRST_BIT_SHIFT_YES		(0 << 0)
> +#define SII902X_TPI_I2S_FIRST_BIT_SHIFT_NO		(1 << 0)
> +#define SII902X_TPI_I2S_SD_DIRECTION_MSB_FIRST		(0 << 1)
> +#define SII902X_TPI_I2S_SD_DIRECTION_LSB_FIRST		(1 << 1)
> +#define SII902X_TPI_I2S_SD_JUSTIFY_LEFT			(0 << 2)
> +#define SII902X_TPI_I2S_SD_JUSTIFY_RIGHT		(1 << 2)
> +#define SII902X_TPI_I2S_WS_POLARITY_LOW			(0 << 3)
> +#define SII902X_TPI_I2S_WS_POLARITY_HIGH		(1 << 3)
> +#define SII902X_TPI_I2S_MCLK_MULTIPLIER_128		(0 << 4)
> +#define SII902X_TPI_I2S_MCLK_MULTIPLIER_256		(1 << 4)
> +#define SII902X_TPI_I2S_MCLK_MULTIPLIER_384		(2 << 4)
> +#define SII902X_TPI_I2S_MCLK_MULTIPLIER_512		(3 << 4)
> +#define SII902X_TPI_I2S_MCLK_MULTIPLIER_768		(4 << 4)
> +#define SII902X_TPI_I2S_MCLK_MULTIPLIER_1024		(5 << 4)
> +#define SII902X_TPI_I2S_MCLK_MULTIPLIER_1152		(6 << 4)
> +#define SII902X_TPI_I2S_MCLK_MULTIPLIER_192		(7 << 4)
> +#define SII902X_TPI_I2S_SCK_EDGE_FALLING		(0 << 7)
> +#define SII902X_TPI_I2S_SCK_EDGE_RISING			(1 << 7)
> +
> +#define SII902X_TPI_I2S_STRM_HDR_BASE	0x21
> +#define SII902X_TPI_I2S_STRM_HDR_SIZE	5
> +
> +#define SII902X_TPI_AUDIO_CONFIG_BYTE2_REG	0x26
> +#define SII902X_TPI_AUDIO_CODING_STREAM_HEADER		(0 << 0)
> +#define SII902X_TPI_AUDIO_CODING_PCM			(1 << 0)
> +#define SII902X_TPI_AUDIO_CODING_AC3			(2 << 0)
> +#define SII902X_TPI_AUDIO_CODING_MPEG1			(3 << 0)
> +#define SII902X_TPI_AUDIO_CODING_MP3			(4 << 0)
> +#define SII902X_TPI_AUDIO_CODING_MPEG2			(5 << 0)
> +#define SII902X_TPI_AUDIO_CODING_AAC			(6 << 0)
> +#define SII902X_TPI_AUDIO_CODING_DTS			(7 << 0)
> +#define SII902X_TPI_AUDIO_CODING_ATRAC			(8 << 0)
> +#define SII902X_TPI_AUDIO_MUTE_DISABLE			(0 << 4)
> +#define SII902X_TPI_AUDIO_MUTE_ENABLE			(1 << 4)
> +#define SII902X_TPI_AUDIO_LAYOUT_2_CHANNELS		(0 << 5)
> +#define SII902X_TPI_AUDIO_LAYOUT_8_CHANNELS		(1 << 5)
> +#define SII902X_TPI_AUDIO_INTERFACE_DISABLE		(0 << 6)
> +#define SII902X_TPI_AUDIO_INTERFACE_SPDIF		(1 << 6)
> +#define SII902X_TPI_AUDIO_INTERFACE_I2S			(2 << 6)
> +
> +#define SII902X_TPI_AUDIO_CONFIG_BYTE3_REG	0x27
> +#define SII902X_TPI_AUDIO_FREQ_STREAM			(0 << 3)
> +#define SII902X_TPI_AUDIO_FREQ_32KHZ			(1 << 3)
> +#define SII902X_TPI_AUDIO_FREQ_44KHZ			(2 << 3)
> +#define SII902X_TPI_AUDIO_FREQ_48KHZ			(3 << 3)
> +#define SII902X_TPI_AUDIO_FREQ_88KHZ			(4 << 3)
> +#define SII902X_TPI_AUDIO_FREQ_96KHZ			(5 << 3)
> +#define SII902X_TPI_AUDIO_FREQ_176KHZ			(6 << 3)
> +#define SII902X_TPI_AUDIO_FREQ_192KHZ			(7 << 3)
> +#define SII902X_TPI_AUDIO_SAMPLE_SIZE_STREAM		(0 << 6)
> +#define SII902X_TPI_AUDIO_SAMPLE_SIZE_16		(1 << 6)
> +#define SII902X_TPI_AUDIO_SAMPLE_SIZE_20		(2 << 6)
> +#define SII902X_TPI_AUDIO_SAMPLE_SIZE_24		(3 << 6)
> +
> +#define SII902X_TPI_AUDIO_CONFIG_BYTE4_REG	0x28
> +
>   #define SII902X_INT_ENABLE			0x3c
>   #define SII902X_INT_STATUS			0x3d
>   #define SII902X_HOTPLUG_EVENT			BIT(0)
> @@ -81,6 +155,16 @@
>   
>   #define SII902X_REG_TPI_RQB			0xc7
>   
> +/* Indirect internal register access */
> +#define SII902X_IND_SET_PAGE			0xbc
> +#define SII902X_IND_OFFSET			0xbd
> +#define SII902X_IND_VALUE			0xbe
> +
> +#define SII902X_TPI_MISC_INFOFRAME_BASE		0xbf
> +#define SII902X_TPI_MISC_INFOFRAME_END		0xde
> +#define SII902X_TPI_MISC_INFOFRAME_SIZE	\
> +	(SII902X_TPI_MISC_INFOFRAME_END - SII902X_TPI_MISC_INFOFRAME_BASE)
> +
>   #define SII902X_I2C_BUS_ACQUISITION_TIMEOUT_MS	500
>   
>   struct sii902x {
> @@ -90,6 +174,16 @@ struct sii902x {
>   	struct drm_connector connector;
>   	struct gpio_desc *reset_gpio;
>   	struct i2c_mux_core *i2cmux;
> +	/*
> +	 * Mutex protects audio and video functions from interfering
> +	 * each other, by keeping their i2c command sequences atomic.
> +	 */
> +	struct mutex mutex;
> +	struct sii902x_audio {
> +		struct platform_device *pdev;
> +		struct clk *mclk;
> +		u32 i2s_fifo_sequence[4];
> +	} audio;
>   };
>   
>   static int sii902x_read_unlocked(struct i2c_client *i2c, u8 reg, u8 *val)
> @@ -161,8 +255,12 @@ sii902x_connector_detect(struct drm_connector *connector, bool force)
>   	struct sii902x *sii902x = connector_to_sii902x(connector);
>   	unsigned int status;
>   
> +	mutex_lock(&sii902x->mutex);
> +
>   	regmap_read(sii902x->regmap, SII902X_INT_STATUS, &status);
>   
> +	mutex_unlock(&sii902x->mutex);
> +
>   	return (status & SII902X_PLUGGED_STATUS) ?
>   	       connector_status_connected : connector_status_disconnected;
>   }
> @@ -180,10 +278,12 @@ static int sii902x_get_modes(struct drm_connector *connector)
>   {
>   	struct sii902x *sii902x = connector_to_sii902x(connector);
>   	u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24;
> -	struct edid *edid;
>   	u8 output_mode = SII902X_SYS_CTRL_OUTPUT_DVI;
> +	struct edid *edid;
>   	int num = 0, ret;
>   
> +	mutex_lock(&sii902x->mutex);
> +
>   	edid = drm_get_edid(connector, sii902x->i2cmux->adapter[0]);
>   	drm_connector_update_edid_property(connector, edid);
>   	if (edid) {
> @@ -197,14 +297,19 @@ static int sii902x_get_modes(struct drm_connector *connector)
>   	ret = drm_display_info_set_bus_formats(&connector->display_info,
>   					       &bus_format, 1);
>   	if (ret)
> -		return ret;
> +		goto error_out;
>   
>   	ret = regmap_update_bits(sii902x->regmap, SII902X_SYS_CTRL_DATA,
>   				 SII902X_SYS_CTRL_OUTPUT_MODE, output_mode);
>   	if (ret)
> -		return ret;
> +		goto error_out;
>   
> -	return num;
> +	ret = num;
> +
> +error_out:
> +	mutex_unlock(&sii902x->mutex);
> +
> +	return ret;
>   }
>   
>   static enum drm_mode_status sii902x_mode_valid(struct drm_connector *connector,
> @@ -224,20 +329,28 @@ static void sii902x_bridge_disable(struct drm_bridge *bridge)
>   {
>   	struct sii902x *sii902x = bridge_to_sii902x(bridge);
>   
> +	mutex_lock(&sii902x->mutex);
> +
>   	regmap_update_bits(sii902x->regmap, SII902X_SYS_CTRL_DATA,
>   			   SII902X_SYS_CTRL_PWR_DWN,
>   			   SII902X_SYS_CTRL_PWR_DWN);
> +
> +	mutex_unlock(&sii902x->mutex);
>   }
>   
>   static void sii902x_bridge_enable(struct drm_bridge *bridge)
>   {
>   	struct sii902x *sii902x = bridge_to_sii902x(bridge);
>   
> +	mutex_lock(&sii902x->mutex);
> +
>   	regmap_update_bits(sii902x->regmap, SII902X_PWR_STATE_CTRL,
>   			   SII902X_AVI_POWER_STATE_MSK,
>   			   SII902X_AVI_POWER_STATE_D(0));
>   	regmap_update_bits(sii902x->regmap, SII902X_SYS_CTRL_DATA,
>   			   SII902X_SYS_CTRL_PWR_DWN, 0);
> +
> +	mutex_unlock(&sii902x->mutex);
>   }
>   
>   static void sii902x_bridge_mode_set(struct drm_bridge *bridge,
> @@ -264,26 +377,31 @@ static void sii902x_bridge_mode_set(struct drm_bridge *bridge,
>   	buf[9] = SII902X_TPI_AVI_INPUT_RANGE_AUTO |
>   		 SII902X_TPI_AVI_INPUT_COLORSPACE_RGB;
>   
> +	mutex_lock(&sii902x->mutex);
> +
>   	ret = regmap_bulk_write(regmap, SII902X_TPI_VIDEO_DATA, buf, 10);
>   	if (ret)
> -		return;
> +		goto out;
>   
>   	ret = drm_hdmi_avi_infoframe_from_display_mode(&frame, adj, false);
>   	if (ret < 0) {
>   		DRM_ERROR("couldn't fill AVI infoframe\n");
> -		return;
> +		goto out;
>   	}
>   
>   	ret = hdmi_avi_infoframe_pack(&frame, buf, sizeof(buf));
>   	if (ret < 0) {
>   		DRM_ERROR("failed to pack AVI infoframe: %d\n", ret);
> -		return;
> +		goto out;
>   	}
>   
>   	/* Do not send the infoframe header, but keep the CRC field. */
>   	regmap_bulk_write(regmap, SII902X_TPI_AVI_INFOFRAME,
>   			  buf + HDMI_INFOFRAME_HEADER_SIZE - 1,
>   			  HDMI_AVI_INFOFRAME_SIZE + 1);
> +
> +out:
> +	mutex_unlock(&sii902x->mutex);
>   }
>   
>   static int sii902x_bridge_attach(struct drm_bridge *bridge)
> @@ -324,6 +442,329 @@ static const struct drm_bridge_funcs sii902x_bridge_funcs = {
>   	.enable = sii902x_bridge_enable,
>   };
>   
> +static int sii902x_mute(struct sii902x *sii902x, bool mute)
> +{
> +	struct device *dev = &sii902x->i2c->dev;
> +	unsigned int val = mute ? SII902X_TPI_AUDIO_MUTE_ENABLE :
> +		SII902X_TPI_AUDIO_MUTE_DISABLE;
> +
> +	dev_dbg(dev, "%s: %s\n", __func__, mute ? "Muted" : "Unmuted");
> +
> +	return regmap_update_bits(sii902x->regmap,
> +				  SII902X_TPI_AUDIO_CONFIG_BYTE2_REG,
> +				  SII902X_TPI_AUDIO_MUTE_ENABLE, val);
> +}
> +
> +static const unsigned int sii902x_mclk_div_table[] = {
> +	128, 256, 384, 512, 768, 1024, 1152, 192 };
> +
> +static int sii902x_select_mclk_div(u8 *i2s_config_reg, unsigned int rate,
> +				   unsigned int mclk)
> +{
> +	unsigned int div = mclk / rate;
> +	int distance = 100000;
> +	u8 i, nearest = 0;
> +
> +	for (i = 0; i < ARRAY_SIZE(sii902x_mclk_div_table); i++) {
> +		unsigned int d = abs(div - sii902x_mclk_div_table[i]);
> +
> +		if (d >= distance)
> +			continue;
> +
> +		nearest = i;
> +		distance = d;
> +		if (d == 0)
> +			break;
> +	}
> +
> +	*i2s_config_reg |= nearest << 4;
> +
> +	if (distance != 0)
> +		return sii902x_mclk_div_table[nearest];
> +
> +	return 0;
> +}
> +
> +static const struct sii902x_sample_freq {
> +	u32 freq;
> +	u8 val;
> +} sii902x_sample_freq[] = {
> +	{ .freq = 32000,	.val = SII902X_TPI_AUDIO_FREQ_32KHZ },
> +	{ .freq = 44000,	.val = SII902X_TPI_AUDIO_FREQ_44KHZ },
> +	{ .freq = 48000,	.val = SII902X_TPI_AUDIO_FREQ_48KHZ },
> +	{ .freq = 88000,	.val = SII902X_TPI_AUDIO_FREQ_88KHZ },
> +	{ .freq = 96000,	.val = SII902X_TPI_AUDIO_FREQ_96KHZ },
> +	{ .freq = 176000,	.val = SII902X_TPI_AUDIO_FREQ_176KHZ },
> +	{ .freq = 192000,	.val = SII902X_TPI_AUDIO_FREQ_192KHZ },
> +};
> +
> +static int sii902x_audio_hw_params(struct device *dev, void *data,
> +				   struct hdmi_codec_daifmt *daifmt,
> +				   struct hdmi_codec_params *params)
> +{
> +	struct sii902x *sii902x = dev_get_drvdata(dev);
> +	u8 i2s_config_reg = SII902X_TPI_I2S_SD_DIRECTION_MSB_FIRST;
> +	u8 config_byte2_reg = (SII902X_TPI_AUDIO_INTERFACE_I2S |
> +			       SII902X_TPI_AUDIO_MUTE_ENABLE |
> +			       SII902X_TPI_AUDIO_CODING_PCM);
> +	u8 config_byte3_reg = 0;
> +	u8 infoframe_buf[HDMI_INFOFRAME_SIZE(AUDIO)];
> +	unsigned long mclk_rate;
> +	int i, ret;
> +
> +	if (daifmt->bit_clk_master || daifmt->frame_clk_master) {
> +		dev_dbg(dev, "%s: I2S master mode not supported\n", __func__);
> +		return -EINVAL;
> +	}
> +
> +	switch (daifmt->fmt) {
> +	case HDMI_I2S:
> +		i2s_config_reg |= SII902X_TPI_I2S_FIRST_BIT_SHIFT_YES |
> +			SII902X_TPI_I2S_SD_JUSTIFY_LEFT;
> +		break;
> +	case HDMI_RIGHT_J:
> +		i2s_config_reg |= SII902X_TPI_I2S_SD_JUSTIFY_RIGHT;
> +		break;
> +	case HDMI_LEFT_J:
> +		i2s_config_reg |= SII902X_TPI_I2S_SD_JUSTIFY_LEFT;
> +		break;
> +	default:
> +		dev_dbg(dev, "%s: Unsupported i2s format %u\n", __func__,
> +			daifmt->fmt);
> +		return -EINVAL;
> +	}
> +
> +	if (daifmt->bit_clk_inv)
> +		i2s_config_reg |= SII902X_TPI_I2S_SCK_EDGE_FALLING;
> +	else
> +		i2s_config_reg |= SII902X_TPI_I2S_SCK_EDGE_RISING;
> +
> +	if (daifmt->frame_clk_inv)
> +		i2s_config_reg |= SII902X_TPI_I2S_WS_POLARITY_LOW;
> +	else
> +		i2s_config_reg |= SII902X_TPI_I2S_WS_POLARITY_HIGH;
> +
> +	if (params->channels > 2)
> +		config_byte2_reg |= SII902X_TPI_AUDIO_LAYOUT_8_CHANNELS;
> +	else
> +		config_byte2_reg |= SII902X_TPI_AUDIO_LAYOUT_2_CHANNELS;
> +
> +	switch (params->sample_width) {
> +	case 16:
> +		config_byte3_reg |= SII902X_TPI_AUDIO_SAMPLE_SIZE_16;
> +		break;
> +	case 20:
> +		config_byte3_reg |= SII902X_TPI_AUDIO_SAMPLE_SIZE_20;
> +		break;
> +	case 24:
> +	case 32:
> +		config_byte3_reg |= SII902X_TPI_AUDIO_SAMPLE_SIZE_24;
> +		break;
> +	default:
> +		dev_err(dev, "%s: Unsupported sample width %u\n", __func__,
> +			params->sample_width);
> +		return -EINVAL;
> +	}
> +
> +	for (i = 0; i < ARRAY_SIZE(sii902x_sample_freq); i++) {
> +		if (params->sample_rate == sii902x_sample_freq[i].freq) {
> +			config_byte3_reg |= sii902x_sample_freq[i].val;
> +			break;
> +		}
> +	}
> +
> +	ret = clk_prepare_enable(sii902x->audio.mclk);
> +	if (ret) {
> +		dev_err(dev, "Enabling mclk failed: %d\n", ret);
> +		return ret;
> +	}
> +
> +	mclk_rate = clk_get_rate(sii902x->audio.mclk);
> +
> +	ret = sii902x_select_mclk_div(&i2s_config_reg, params->sample_rate,
> +				      mclk_rate);
> +	if (ret)
> +		dev_dbg(dev, "Inaccurate reference clock (%ld/%d != %u)\n",
> +			mclk_rate, ret, params->sample_rate);
> +
> +	mutex_lock(&sii902x->mutex);
> +
> +	ret = regmap_write(sii902x->regmap,
> +			   SII902X_TPI_AUDIO_CONFIG_BYTE2_REG,
> +			   config_byte2_reg);
> +	if (ret < 0)
> +		goto out;
> +
> +	ret = regmap_write(sii902x->regmap, SII902X_TPI_I2S_INPUT_CONFIG_REG,
> +			   i2s_config_reg);
> +	if (ret)
> +		goto out;
> +
> +	for (i = 0; sii902x->audio.i2s_fifo_sequence[i] &&
> +		     i < ARRAY_SIZE(sii902x->audio.i2s_fifo_sequence); i++)
> +		regmap_write(sii902x->regmap,
> +			     SII902X_TPI_I2S_ENABLE_MAPPING_REG,
> +			     sii902x->audio.i2s_fifo_sequence[i]);
> +
> +	ret = regmap_write(sii902x->regmap, SII902X_TPI_AUDIO_CONFIG_BYTE3_REG,
> +			   config_byte3_reg);
> +	if (ret)
> +		goto out;
> +
> +	ret = regmap_bulk_write(sii902x->regmap, SII902X_TPI_I2S_STRM_HDR_BASE,
> +				params->iec.status,
> +				min((size_t) SII902X_TPI_I2S_STRM_HDR_SIZE,
> +				    sizeof(params->iec.status)));
> +	if (ret)
> +		goto out;
> +
> +	ret = hdmi_audio_infoframe_pack(&params->cea, infoframe_buf,
> +					sizeof(infoframe_buf));
> +	if (ret < 0) {
> +		dev_err(dev, "%s: Failed to pack audio infoframe: %d\n",
> +			__func__, ret);
> +		goto out;
> +	}
> +
> +	ret = regmap_bulk_write(sii902x->regmap,
> +				SII902X_TPI_MISC_INFOFRAME_BASE,
> +				infoframe_buf,
> +				min(ret, SII902X_TPI_MISC_INFOFRAME_SIZE));
> +	if (ret)
> +		goto out;
> +
> +	/* Decode Level 0 Packets */
> +	ret = regmap_write(sii902x->regmap, SII902X_IND_SET_PAGE, 0x02);
> +	if (ret)
> +		goto out;
> +
> +	ret = regmap_write(sii902x->regmap, SII902X_IND_OFFSET, 0x24);
> +	if (ret)
> +		goto out;
> +
> +	ret = regmap_write(sii902x->regmap, SII902X_IND_VALUE, 0x02);
> +	if (ret)
> +		goto out;
> +
> +	dev_dbg(dev, "%s: hdmi audio enabled\n", __func__);
> +out:
> +	mutex_unlock(&sii902x->mutex);
> +
> +	if (ret) {
> +		clk_disable_unprepare(sii902x->audio.mclk);
> +		dev_err(dev, "%s: hdmi audio enable failed: %d\n", __func__,
> +			ret);
> +	}
> +
> +	return ret;
> +}
> +
> +static void sii902x_audio_shutdown(struct device *dev, void *data)
> +{
> +	struct sii902x *sii902x = dev_get_drvdata(dev);
> +
> +	mutex_lock(&sii902x->mutex);
> +
> +	regmap_write(sii902x->regmap, SII902X_TPI_AUDIO_CONFIG_BYTE2_REG,
> +		     SII902X_TPI_AUDIO_INTERFACE_DISABLE);
> +
> +	mutex_unlock(&sii902x->mutex);
> +
> +	clk_disable_unprepare(sii902x->audio.mclk);
> +}
> +
> +int sii902x_audio_digital_mute(struct device *dev, void *data, bool enable)
> +{
> +	struct sii902x *sii902x = dev_get_drvdata(dev);
> +
> +	mutex_lock(&sii902x->mutex);
> +
> +	sii902x_mute(sii902x, enable);
> +
> +	mutex_unlock(&sii902x->mutex);
> +
> +	return 0;
> +}
> +
> +static int sii902x_audio_get_eld(struct device *dev, void *data,
> +				 uint8_t *buf, size_t len)
> +{
> +	struct sii902x *sii902x = dev_get_drvdata(dev);
> +
> +	mutex_lock(&sii902x->mutex);
> +
> +	memcpy(buf, sii902x->connector.eld,
> +	       min(sizeof(sii902x->connector.eld), len));
> +
> +	mutex_unlock(&sii902x->mutex);
> +
> +	return 0;
> +}
> +
> +static const struct hdmi_codec_ops sii902x_audio_codec_ops = {
> +	.hw_params = sii902x_audio_hw_params,
> +	.audio_shutdown = sii902x_audio_shutdown,
> +	.digital_mute = sii902x_audio_digital_mute,
> +	.get_eld = sii902x_audio_get_eld,
> +};
> +
> +static int sii902x_audio_codec_init(struct sii902x *sii902x,
> +				    struct device *dev)
> +{
> +	static const u8 audio_fifo_id[] = {
> +		SII902X_TPI_I2S_CONFIG_FIFO0,
> +		SII902X_TPI_I2S_CONFIG_FIFO1,
> +		SII902X_TPI_I2S_CONFIG_FIFO2,
> +		SII902X_TPI_I2S_CONFIG_FIFO3,
> +	};
> +	static const u8 i2s_lane_id[] = {
> +		SII902X_TPI_I2S_SELECT_SD0,
> +		SII902X_TPI_I2S_SELECT_SD1,
> +		SII902X_TPI_I2S_SELECT_SD2,
> +		SII902X_TPI_I2S_SELECT_SD3,
> +	};
> +	struct hdmi_codec_pdata codec_data = {
> +		.ops = &sii902x_audio_codec_ops,
> +		.i2s = 1, /* Only i2s support for now. */
> +		.spdif = 0,
> +		.max_i2s_channels = 0,
> +	};
> +	u8 lanes[4];
> +	u32 num_lanes, i;
> +
> +	num_lanes = of_property_read_variable_u8_array(dev->of_node,
> +						       "i2s-data-lanes",
> +						       lanes, 1,
> +						       ARRAY_SIZE(lanes));
> +	if (num_lanes < 0) {
> +		if (num_lanes == -EINVAL)
> +			dev_dbg(dev, "%s: No \"i2s-data-lanes\", no audio\n",
> +			__func__);
> +		else
> +			dev_err(dev,
> +				"%s: Error gettin \"i2s-data-lanes\": %d\n",
> +				__func__, num_lanes);
> +		return 0;
> +	}
> +	codec_data.max_i2s_channels = 2 * num_lanes;
> +
> +	for (i = 0; i < num_lanes; i++)
> +		sii902x->audio.i2s_fifo_sequence[i] |= audio_fifo_id[i] |
> +			i2s_lane_id[lanes[i]] |	SII902X_TPI_I2S_FIFO_ENABLE;
> +
> +	if (IS_ERR(sii902x->audio.mclk)) {
> +		dev_err(dev, "%s: No clock (audio mclk) found: %ld\n",
> +			__func__, PTR_ERR(sii902x->audio.mclk));
> +		return 0;
> +	}
> +
> +	sii902x->audio.pdev = platform_device_register_data(
> +		dev, HDMI_CODEC_DRV_NAME, PLATFORM_DEVID_AUTO,
> +		&codec_data, sizeof(codec_data));
> +
> +	return PTR_ERR_OR_ZERO(sii902x->audio.pdev);
> +}
> +
>   static const struct regmap_range sii902x_volatile_ranges[] = {
>   	{ .range_min = 0, .range_max = 0xff },
>   };
> @@ -336,6 +777,7 @@ static const struct regmap_access_table sii902x_volatile_table = {
>   static const struct regmap_config sii902x_regmap_config = {
>   	.reg_bits = 8,
>   	.val_bits = 8,
> +	.max_register = SII902X_TPI_MISC_INFOFRAME_END,
>   	.volatile_table = &sii902x_volatile_table,
>   	.cache_type = REGCACHE_NONE,
>   };
> @@ -508,6 +950,8 @@ static int sii902x_probe(struct i2c_client *client,
>   		return PTR_ERR(sii902x->reset_gpio);
>   	}
>   
> +	mutex_init(&sii902x->mutex);
> +
>   	sii902x_reset(sii902x);
>   
>   	ret = regmap_write(sii902x->regmap, SII902X_REG_TPI_RQB, 0x0);
> @@ -548,6 +992,8 @@ static int sii902x_probe(struct i2c_client *client,
>   	sii902x->bridge.timings = &default_sii902x_timings;
>   	drm_bridge_add(&sii902x->bridge);
>   
> +	sii902x_audio_codec_init(sii902x, dev);
> +
>   	i2c_set_clientdata(client, sii902x);
>   
>   	sii902x->i2cmux = i2c_mux_alloc(client->adapter, dev,
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v4 4/6] dt-bindings: display: sii902x: Remove trailing white space

[Rob Herring]

On Thu, 14 Mar 2019 13:27:50 +0200, Jyri Sarha wrote:
> Remove trailing white space from sii902x display bridge binding.
> 
> Signed-off-by: Jyri Sarha <jsarha@ti.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  Documentation/devicetree/bindings/display/bridge/sii902x.txt | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 

Reviewed-by: Rob Herring <robh@kernel.org>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v4 5/6] dt-bindings: display: sii902x: Add HDMI audio bindings

[Laurent Pinchart]

Hi Jyri,

Thank you for the patch.

On Thu, Mar 14, 2019 at 01:27:51PM +0200, Jyri Sarha wrote:
> The sii902x chip family supports also HDMI audio. Add binding for
> describing the necessary i2s and mclk wiring for it.
> 
> Signed-off-by: Jyri Sarha <jsarha@ti.com>
> ---
>  .../bindings/display/bridge/sii902x.txt       | 33 +++++++++++++++++++
>  1 file changed, 33 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/bridge/sii902x.txt b/Documentation/devicetree/bindings/display/bridge/sii902x.txt
> index c4c1855ca654..1a37bbe7c597 100644
> --- a/Documentation/devicetree/bindings/display/bridge/sii902x.txt
> +++ b/Documentation/devicetree/bindings/display/bridge/sii902x.txt
> @@ -9,6 +9,33 @@ Optional properties:
>  	  about hotplug events.
>  	- reset-gpios: OF device-tree gpio specification for RST_N pin.
>  
> +	HDMI audio properties:
> +	- i2s-data-lanes: Array of up to 4 integers with values of 0-3
> +	   Each integer indicates which i2s pin is connected to which
> +	   audio fifo. The first integer selects i2s audio pin for the
> +	   first audio fifo#0 (HDMI channels 1&2), second for fifo#1
> +	   (HDMI channels 3&4), and so on. There is 4 fifos and 4 i2s
> +	   pins (SD0 - SD3). Any i2s pin can be connected to any fifo,
> +	   but there can be no gaps. E.g. an i2s pin must be mapped to
> +	   fifo#0 and fifo#1 before mapping a channel to fifo#2.
> +	   I2S HDMI audio is configured only if this property is found.

This looks good to me, but shouldn't the property be defined somewhere
in Documentation/devicetree/bindings/sound/ ? The sound bindings seem to
be a bit messy though, with little common or high-level documentation,
so I don't want to ask you to fix all that :-) Maybe just adding a
common-i2s.txt file there would be enough ? 

Apart from this the rest looks good to me, but I'd appreciate if it
could be reviewed by audio people.

> +	- clocks: phandle and clock specifier for each clock listed in
> +          the clock-names property
> +	- clock-names: "mclk"
> +	    Describes SII902x MCLK input. MCLK is used to produce
> +	    HDMI audio CTS values. This property is required if
> +	    "i2s-data-lanes"-property is present. This property follows
> +	    Documentation/devicetree/bindings/clock/clock-bindings.txt
> +	    consumer binding.
> +
> +	If HDMI audio is configured the sii902x device becomes an ASoC
> +	codec component, that can be used in configuring full audio
> +	devices with ASoC simple-card or audio-graph-card. See their
> +	binding documents on how to describe how the sii902x device is
> +	connected to the rest of the audio system:
> +	Documentation/devicetree/bindings/sound/simple-card.txt
> +	Documentation/devicetree/bindings/sound/audio-graph-card.txt
> +
>  Optional subnodes:
>  	- video input: this subnode can contain a video input port node
>  	  to connect the bridge to a display controller output (See this
> @@ -21,6 +48,12 @@ Example:
>  		compatible = "sil,sii9022";
>  		reg = <0x39>;
>  		reset-gpios = <&pioA 1 0>;
> +
> +		#sound-dai-cells = <0>;
> +		i2s-data-lanes = < 0 1 2 >;
> +		clocks = <&mclk>;
> +		clock-names = "mclk";
> +
>  		ports {
>  			#address-cells = <1>;
>  			#size-cells = <0>;

-- 
Regards,

Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v4 5/6] dt-bindings: display: sii902x: Add HDMI audio bindings

[Jyri Sarha]

On 17/03/2019 18:16, Laurent Pinchart wrote:
> Hi Jyri,
> 
> Thank you for the patch.
> 
> On Thu, Mar 14, 2019 at 01:27:51PM +0200, Jyri Sarha wrote:
>> The sii902x chip family supports also HDMI audio. Add binding for
>> describing the necessary i2s and mclk wiring for it.
>>
>> Signed-off-by: Jyri Sarha <jsarha@ti.com>
>> ---
>>  .../bindings/display/bridge/sii902x.txt       | 33 +++++++++++++++++++
>>  1 file changed, 33 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/display/bridge/sii902x.txt b/Documentation/devicetree/bindings/display/bridge/sii902x.txt
>> index c4c1855ca654..1a37bbe7c597 100644
>> --- a/Documentation/devicetree/bindings/display/bridge/sii902x.txt
>> +++ b/Documentation/devicetree/bindings/display/bridge/sii902x.txt
>> @@ -9,6 +9,33 @@ Optional properties:
>>  	  about hotplug events.
>>  	- reset-gpios: OF device-tree gpio specification for RST_N pin.
>>  
>> +	HDMI audio properties:
>> +	- i2s-data-lanes: Array of up to 4 integers with values of 0-3
>> +	   Each integer indicates which i2s pin is connected to which
>> +	   audio fifo. The first integer selects i2s audio pin for the
>> +	   first audio fifo#0 (HDMI channels 1&2), second for fifo#1
>> +	   (HDMI channels 3&4), and so on. There is 4 fifos and 4 i2s
>> +	   pins (SD0 - SD3). Any i2s pin can be connected to any fifo,
>> +	   but there can be no gaps. E.g. an i2s pin must be mapped to
>> +	   fifo#0 and fifo#1 before mapping a channel to fifo#2.
>> +	   I2S HDMI audio is configured only if this property is found.
> This looks good to me, but shouldn't the property be defined somewhere
> in Documentation/devicetree/bindings/sound/ ? The sound bindings seem to
> be a bit messy though, with little common or high-level documentation,
> so I don't want to ask you to fix all that :-) Maybe just adding a
> common-i2s.txt file there would be enough ? 
> 
> Apart from this the rest looks good to me, but I'd appreciate if it
> could be reviewed by audio people.
> 
When I think about this property, it hardly is a generic enough to for
all i2s devices or even codecs. That would probably require gaps in the
i2s-data-lane order. I would not want to create common-i2s.txt just for
this property. The is some many other properties that should be added
there first. Maybe this should be made vendor specific property after all.

I think I make a resend with alsa-devel included.

Best regards,
Jyri

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v4 5/6] dt-bindings: display: sii902x: Add HDMI audio bindings

[Rob Herring]

On Thu, Mar 14, 2019 at 01:27:51PM +0200, Jyri Sarha wrote:
> The sii902x chip family supports also HDMI audio. Add binding for
> describing the necessary i2s and mclk wiring for it.
> 
> Signed-off-by: Jyri Sarha <jsarha@ti.com>
> ---
>  .../bindings/display/bridge/sii902x.txt       | 33 +++++++++++++++++++
>  1 file changed, 33 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/bridge/sii902x.txt b/Documentation/devicetree/bindings/display/bridge/sii902x.txt
> index c4c1855ca654..1a37bbe7c597 100644
> --- a/Documentation/devicetree/bindings/display/bridge/sii902x.txt
> +++ b/Documentation/devicetree/bindings/display/bridge/sii902x.txt
> @@ -9,6 +9,33 @@ Optional properties:
>  	  about hotplug events.
>  	- reset-gpios: OF device-tree gpio specification for RST_N pin.
>  
> +	HDMI audio properties:
> +	- i2s-data-lanes: Array of up to 4 integers with values of 0-3
> +	   Each integer indicates which i2s pin is connected to which
> +	   audio fifo. The first integer selects i2s audio pin for the
> +	   first audio fifo#0 (HDMI channels 1&2), second for fifo#1
> +	   (HDMI channels 3&4), and so on. There is 4 fifos and 4 i2s
> +	   pins (SD0 - SD3). Any i2s pin can be connected to any fifo,
> +	   but there can be no gaps. E.g. an i2s pin must be mapped to
> +	   fifo#0 and fifo#1 before mapping a channel to fifo#2.
> +	   I2S HDMI audio is configured only if this property is found.

Seems like a default should be allowed and you should enable audio based 
on #sound-dai-cells which you didn't document.

> +	- clocks: phandle and clock specifier for each clock listed in
> +          the clock-names property
> +	- clock-names: "mclk"
> +	    Describes SII902x MCLK input. MCLK is used to produce
> +	    HDMI audio CTS values. This property is required if
> +	    "i2s-data-lanes"-property is present. This property follows
> +	    Documentation/devicetree/bindings/clock/clock-bindings.txt
> +	    consumer binding.
> +
> +	If HDMI audio is configured the sii902x device becomes an ASoC
> +	codec component, that can be used in configuring full audio
> +	devices with ASoC simple-card or audio-graph-card. See their

With the graph-card, don't you need to define which port is audio?

> +	binding documents on how to describe how the sii902x device is
> +	connected to the rest of the audio system:
> +	Documentation/devicetree/bindings/sound/simple-card.txt
> +	Documentation/devicetree/bindings/sound/audio-graph-card.txt
> +
>  Optional subnodes:
>  	- video input: this subnode can contain a video input port node
>  	  to connect the bridge to a display controller output (See this
> @@ -21,6 +48,12 @@ Example:
>  		compatible = "sil,sii9022";
>  		reg = <0x39>;
>  		reset-gpios = <&pioA 1 0>;
> +
> +		#sound-dai-cells = <0>;
> +		i2s-data-lanes = < 0 1 2 >;
> +		clocks = <&mclk>;
> +		clock-names = "mclk";
> +
>  		ports {
>  			#address-cells = <1>;
>  			#size-cells = <0>;
> -- 
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v4 5/6] dt-bindings: display: sii902x: Add HDMI audio bindings

[Laurent Pinchart]

Hi Jyri,

On Tue, Mar 19, 2019 at 04:38:45PM +0200, Jyri Sarha wrote:
> On 17/03/2019 18:16, Laurent Pinchart wrote:
> > On Thu, Mar 14, 2019 at 01:27:51PM +0200, Jyri Sarha wrote:
> >> The sii902x chip family supports also HDMI audio. Add binding for
> >> describing the necessary i2s and mclk wiring for it.
> >>
> >> Signed-off-by: Jyri Sarha <jsarha@ti.com>
> >> ---
> >>  .../bindings/display/bridge/sii902x.txt       | 33 +++++++++++++++++++
> >>  1 file changed, 33 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/display/bridge/sii902x.txt b/Documentation/devicetree/bindings/display/bridge/sii902x.txt
> >> index c4c1855ca654..1a37bbe7c597 100644
> >> --- a/Documentation/devicetree/bindings/display/bridge/sii902x.txt
> >> +++ b/Documentation/devicetree/bindings/display/bridge/sii902x.txt
> >> @@ -9,6 +9,33 @@ Optional properties:
> >>  	  about hotplug events.
> >>  	- reset-gpios: OF device-tree gpio specification for RST_N pin.
> >>  
> >> +	HDMI audio properties:
> >> +	- i2s-data-lanes: Array of up to 4 integers with values of 0-3
> >> +	   Each integer indicates which i2s pin is connected to which
> >> +	   audio fifo. The first integer selects i2s audio pin for the
> >> +	   first audio fifo#0 (HDMI channels 1&2), second for fifo#1
> >> +	   (HDMI channels 3&4), and so on. There is 4 fifos and 4 i2s
> >> +	   pins (SD0 - SD3). Any i2s pin can be connected to any fifo,
> >> +	   but there can be no gaps. E.g. an i2s pin must be mapped to
> >> +	   fifo#0 and fifo#1 before mapping a channel to fifo#2.
> >> +	   I2S HDMI audio is configured only if this property is found.
> > 
> > This looks good to me, but shouldn't the property be defined somewhere
> > in Documentation/devicetree/bindings/sound/ ? The sound bindings seem to
> > be a bit messy though, with little common or high-level documentation,
> > so I don't want to ask you to fix all that :-) Maybe just adding a
> > common-i2s.txt file there would be enough ? 
> > 
> > Apart from this the rest looks good to me, but I'd appreciate if it
> > could be reviewed by audio people.
> > 
> When I think about this property, it hardly is a generic enough to for
> all i2s devices or even codecs. That would probably require gaps in the
> i2s-data-lane order.

We can start by documenting it without gaps allowed, and then allow gaps
if needed, leaving the support of gaps defined in device bindings.

> I would not want to create common-i2s.txt just for this property. The
> is some many other properties that should be added there first. Maybe
> this should be made vendor specific property after all.

You don't need to populate common-i2s.txt with all I2S properties.
Creating the file for i2s-data-lanes would be a nice first step, you can
then let other audio developers take over :-)

> I think I make a resend with alsa-devel included.

-- 
Regards,

Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel