drm/bridge: sii902x: HDMI-audio support and some fixes

PATCH Superseded Jyri Sarha 3 participants 17 messages Mar 13, 2019 – Mar 14, 2019

Subsystems (5)

Device tree bindings DRM drivers DRM misc ASoC +1 more subsystems

lore

Share & tools

lore

Jyri Sarha

author

2019-03-13 16:01 UTC #0 lore raw

v3 cover drm/bridge: sii902x: HDMI-audio support and some fixes

Changes since v2:
- split dt-binding and related header to a separate patch
- add vendor prefix to i2s-fifo-routing dt-property
- add license identifier and copyright notice to sii902x-audio.h
- sii902x_get_modes() reorder local variables
- put clk_prepare_enable() before clk_get_rate(), according to documentation clk_get_rate() is only valid once the clock is enabled

The three already reviewed patches are in front:
- drm/bridge: sii902x: add input_bus_flags
- drm/bridge: sii902x: Set output mode to HDMI or DVI according to EDID
- drm/bridge: sii902x: pixel clock unit is 10kHz instead of 1kHz

The previous review round can be found here:
https://patchwork.kernel.org/cover/10832351/

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       |  36 +-
 drivers/gpu/drm/bridge/sii902x.c              | 472 +++++++++++++++++-
 include/dt-bindings/sound/sii902x-audio.h     |  17 +
 3 files changed, 517 insertions(+), 8 deletions(-)
 create mode 100644 include/dt-bindings/sound/sii902x-audio.h

Signature · 6 lines

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

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Jyri Sarha

author

2019-03-13 16:01 UTC #1 lore raw

PATCH v3 1/6 drm/bridge: sii902x: add input_bus_flags

From: Tomi Valkeinen <[email protected]>

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 <[email protected]>

Signed-off-by Jyri Sarha <[email protected]>

Reviewed-by Andrzej Hajda <[email protected]>

---
 drivers/gpu/drm/bridge/sii902x.c | 7 +++++++
 1 file changed, 7 insertions(+)

drivers/gpu/drm/bridge/sii902x.c +7 -0

index bfa902013aa4..1afa000141d5 100644
@@ -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);

Signature · 6 lines

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

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Laurent Pinchart

reviewer Reviewed-by on v3 1/6

2019-03-13 16:23 UTC #2 lore raw

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

Hello Jyri,

Thank you for the patch.

Quoted 10 lines · Jyri Sarha

From: Tomi Valkeinen <[email protected]>
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 <[email protected]>
Signed-off-by Jyri Sarha <[email protected]>
Reviewed-by Andrzej Hajda <[email protected]>

Reviewed-by Laurent Pinchart <[email protected]>

Quoted 29 lines

---
 drivers/gpu/drm/bridge/sii902x.c | 7 +++++++
 1 file changed, 7 insertions(+)

drivers/gpu/drm/bridge/sii902x.c +7 -0

index bfa902013aa4..1afa000141d5 100644
@@ -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);

Signature · 7 lines

Regards,

Laurent Pinchart
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Jyri Sarha

author

2019-03-13 16:01 UTC #3 lore raw

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

In reply to Jyri Sarha

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

Signed-off-by Jyri Sarha <[email protected]>

Reviewed-by Andrzej Hajda <[email protected]>

---
 drivers/gpu/drm/bridge/sii902x.c | 9 +++++++++
 1 file changed, 9 insertions(+)

drivers/gpu/drm/bridge/sii902x.c +9 -0

index 1afa000141d5..0e21fa419d27 100644
@@ -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;
 }

Signature · 6 lines

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

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Laurent Pinchart

reviewer Reviewed-by on v3 2/6

2019-03-13 16:23 UTC #4 lore raw

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

Hello Jyri,

Thank you for the patch.

Quoted 4 lines · Jyri Sarha

Set output mode to HDMI or DVI according to EDID HDMI signature.
Signed-off-by Jyri Sarha <[email protected]>
Reviewed-by Andrzej Hajda <[email protected]>

Reviewed-by Laurent Pinchart <[email protected]>

Quoted 36 lines

---
 drivers/gpu/drm/bridge/sii902x.c | 9 +++++++++
 1 file changed, 9 insertions(+)

drivers/gpu/drm/bridge/sii902x.c +9 -0

index 1afa000141d5..0e21fa419d27 100644
@@ -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;
 }

Signature · 7 lines

Regards,

Laurent Pinchart
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Jyri Sarha

author

2019-03-13 16:01 UTC #5 lore raw

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

In reply to 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 <[email protected]>

Reviewed-by Andrzej Hajda <[email protected]>

---
 drivers/gpu/drm/bridge/sii902x.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

drivers/gpu/drm/bridge/sii902x.c +3 -2

index 0e21fa419d27..1e917777ed72 100644
@@ -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;

Signature · 6 lines

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

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Laurent Pinchart

reviewer Reviewed-by on v3 3/6

2019-03-13 16:24 UTC #6 lore raw

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

Hello Jyri,

Thank you for the patch.

Quoted 15 lines · 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 <[email protected]>
Reviewed-by Andrzej Hajda <[email protected]>
---
 drivers/gpu/drm/bridge/sii902x.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
drivers/gpu/drm/bridge/sii902x.c +0 -0
index 0e21fa419d27..1e917777ed72 100644

drivers/gpu/drm/bridge/sii902x.c static void sii902x_bridge_mode_set(struct drm_bridge *bridge, · line 254

Quoted 9 lines

drivers/gpu/drm/bridge/sii902x.c +2 -2

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

Hex constants usually use lowercase in the kernel. Apart from that,

Reviewed-by Laurent Pinchart <[email protected]>

Quoted 4 lines

drivers/gpu/drm/bridge/sii902x.c +1 -0

+	buf[1] = pixel_clock_10kHz >> 8;
 	buf[2] = adj->vrefresh;
 	buf[3] = 0x00;
 	buf[4] = adj->hdisplay;

Signature · 7 lines

Regards,

Laurent Pinchart
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Jyri Sarha

author

2019-03-13 16:01 UTC #7 lore raw

PATCH v3 4/6 dt-bindings: display: sii902x: Remove trailing white space

In reply to Jyri Sarha

Remove trailing white space from sii902x display bridge binding.

Signed-off-by Jyri Sarha <[email protected]>

---
 Documentation/devicetree/bindings/display/bridge/sii902x.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Documentation/devicetree/bindings/display/bridge/sii902x.txt +1 -1

index 72d2dc6c3e6b..c4c1855ca654 100644
@@ -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.

Signature · 6 lines

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

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Laurent Pinchart

reviewer Reviewed-by on v3 4/6

2019-03-13 16:24 UTC #8 lore raw

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

Hello Jyri,

Thank you for the patch.

Quoted 3 lines · Jyri Sarha

Remove trailing white space from sii902x display bridge binding.
Signed-off-by Jyri Sarha <[email protected]>

Reviewed-by Laurent Pinchart <[email protected]>

Quoted 17 lines

---
 Documentation/devicetree/bindings/display/bridge/sii902x.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Documentation/devicetree/bindings/display/bridge/sii902x.txt +1 -1

index 72d2dc6c3e6b..c4c1855ca654 100644
@@ -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.

Signature · 7 lines

Regards,

Laurent Pinchart
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Jyri Sarha

author

2019-03-13 16:01 UTC #9 lore raw

PATCH v3 5/6 dt-bindings: display: sii902x: Add HDMI audio bindings

In reply to 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 <[email protected]>

---
 .../bindings/display/bridge/sii902x.txt       | 34 +++++++++++++++++++
 include/dt-bindings/sound/sii902x-audio.h     | 17 ++++++++++
 2 files changed, 51 insertions(+)
 create mode 100644 include/dt-bindings/sound/sii902x-audio.h

Documentation/devicetree/bindings/display/bridge/sii902x.txt +34 -0

index c4c1855ca654..977756841193 100644
@@ -8,6 +8,29 @@ Optional properties:
 	- interrupts: describe the interrupt line used to inform the host
 	  about hotplug events.
 	- reset-gpios: OF device-tree gpio specification for RST_N pin.
+	- sil,i2s-fifo-routing: Array of exactly 4 integers indicating i2s
+	  pins for audio fifo routing. First integer defines routing to
+	  fifo 0 and second to fifo 1, etc. Integers can be filled with
+	  definitions from: include/dt-bindings/sound/sii902x-audio.h
+	  The available definitions are:
+	  - ENABLE_BIT:	enable this audio fifo
+	  - CONNECT_SD#: route audio input from SD0, SD1, SD2, or SD3 i2s
+			 data input pin
+	  - LEFT_RIGHT_SWAP_BIT: swap i2s input channels for this fifo
+	  I2S HDMI audio is configured only if this property is found.
+	- clocks: phandle mclk
+	- clock-names: "mclk"
+	    Describes SII902x MCLK input. MCLK is used to produce
+	    HDMI audio CTS values. This property is required if
+	    "i2s-fifo-routing"-property is present. This property follows
+	    Documentation/devicetree/bindings/clock/clock-bindings.txt
+	    consumer binding.
+	- #sound-dai-cells = <0>: ASoC codec dai available for simple-card
+	    If audio properties are present sii902x provides an ASoC
+	    codec component driver that can be used by other ASoC
+	    components like simple-card. See binding document for
+	    details:
+	    Documentation/devicetree/bindings/sound/simple-card.txt
 
 Optional subnodes:
 	- video input: this subnode can contain a video input port node

@@ -21,6 +44,17 @@ Example:
 		compatible = "sil,sii9022";
 		reg = <0x39>;
 		reset-gpios = <&pioA 1 0>;
+
+		#sound-dai-cells = <0>;
+		sil,i2s-fifo-routing = <
+			(ENABLE_BIT|CONNECT_SD0)
+			0
+			0
+			0
+		>;
+		clocks = <&mclk>;
+		clock-names = "mclk";
+
 		ports {
 			#address-cells = <1>;
 			#size-cells = <0>;

include/dt-bindings/sound/sii902x-audio.h new file +17 -0

new file mode 100644
index 000000000000..0a849904754b
@@ -0,0 +1,17 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com/
+ * Author: Jyri Sarha <[email protected]>
+ */
+
+#ifndef __DT_SII9022_AUDIO_H
+#define __DT_SII9022_AUDIO_H
+
+#define ENABLE_BIT			0x80
+#define CONNECT_SD0			0x00
+#define CONNECT_SD1			0x10
+#define CONNECT_SD2			0x20
+#define CONNECT_SD3			0x30
+#define LEFT_RIGHT_SWAP_BIT		0x04
+
+#endif /* __DT_SII9022_AUDIO_H */

Signature · 6 lines

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

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Laurent Pinchart

reviewer

2019-03-13 16:29 UTC #10 lore raw

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

Hello Jyri,

Thank you for the patch.

Quoted 17 lines · 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 <[email protected]>
---
 .../bindings/display/bridge/sii902x.txt       | 34 +++++++++++++++++++
 include/dt-bindings/sound/sii902x-audio.h     | 17 ++++++++++
 2 files changed, 51 insertions(+)
 create mode 100644 include/dt-bindings/sound/sii902x-audio.h
Documentation/devicetree/bindings/display/bridge/sii902x.txt +0 -0
index c4c1855ca654..977756841193 100644
@@ -8,6 +8,29 @@ Optional properties:
 	- interrupts: describe the interrupt line used to inform the host
 	  about hotplug events.

Documentation/devicetree/bindings/display/bridge/sii902x.txt Optional properties: · line 19

Quoted 10 lines

Documentation/devicetree/bindings/display/bridge/sii902x.txt +9 -0

 	- reset-gpios: OF device-tree gpio specification for RST_N pin.
+	- sil,i2s-fifo-routing: Array of exactly 4 integers indicating i2s
+	  pins for audio fifo routing. First integer defines routing to
+	  fifo 0 and second to fifo 1, etc. Integers can be filled with
+	  definitions from: include/dt-bindings/sound/sii902x-audio.h
+	  The available definitions are:
+	  - ENABLE_BIT:	enable this audio fifo
+	  - CONNECT_SD#: route audio input from SD0, SD1, SD2, or SD3 i2s
+			 data input pin
+	  - LEFT_RIGHT_SWAP_BIT: swap i2s input channels for this fifo

Are all combinations valid ? For instance, could we have D1 routed to the third FIFO, and all other FIFOs disabled ?

Documentation/devicetree/bindings/display/bridge/sii902x.txt Optional properties: · line 21

Quoted 2 lines

Documentation/devicetree/bindings/display/bridge/sii902x.txt +2 -0
+	  I2S HDMI audio is configured only if this property is found.
+	- clocks: phandle mclk

Maybe "clocks: phandle and clock specifier for each clock listed in the clock-names property" ?

Documentation/devicetree/bindings/display/bridge/sii902x.txt Optional properties: · line 25

Quoted 4 lines

Documentation/devicetree/bindings/display/bridge/sii902x.txt +4 -0

+	- clock-names: "mclk"
+	    Describes SII902x MCLK input. MCLK is used to produce
+	    HDMI audio CTS values. This property is required if
+	    "i2s-fifo-routing"-property is present. This property follows

The property is named sil,i2s-fifo-routing.

Quoted 40 lines

Documentation/devicetree/bindings/display/bridge/sii902x.txt +19 -0

+	    Documentation/devicetree/bindings/clock/clock-bindings.txt
+	    consumer binding.
+	- #sound-dai-cells = <0>: ASoC codec dai available for simple-card
+	    If audio properties are present sii902x provides an ASoC
+	    codec component driver that can be used by other ASoC
+	    components like simple-card. See binding document for
+	    details:
+	    Documentation/devicetree/bindings/sound/simple-card.txt
 
 Optional subnodes:
 	- video input: this subnode can contain a video input port node

@@ -21,6 +44,17 @@ Example:
 		compatible = "sil,sii9022";
 		reg = <0x39>;
 		reset-gpios = <&pioA 1 0>;
+
+		#sound-dai-cells = <0>;
+		sil,i2s-fifo-routing = <
+			(ENABLE_BIT|CONNECT_SD0)
+			0
+			0
+			0
+		>;
+		clocks = <&mclk>;
+		clock-names = "mclk";
+
 		ports {
 			#address-cells = <1>;
 			#size-cells = <0>;

include/dt-bindings/sound/sii902x-audio.h new file +5 -0

new file mode 100644
index 000000000000..0a849904754b
@@ -0,0 +1,17 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com/
+ * Author: Jyri Sarha <[email protected]>
+ */

include/dt-bindings/sound/sii902x-audio.h line 15

Quoted 10 lines

include/dt-bindings/sound/sii902x-audio.h new file +10 -0

+
+#ifndef __DT_SII9022_AUDIO_H
+#define __DT_SII9022_AUDIO_H
+
+#define ENABLE_BIT			0x80
+#define CONNECT_SD0			0x00
+#define CONNECT_SD1			0x10
+#define CONNECT_SD2			0x20
+#define CONNECT_SD3			0x30
+#define LEFT_RIGHT_SWAP_BIT		0x04

This is fairly generic, should you prefix the macros with SII9022 ?

Quoted 2 lines

include/dt-bindings/sound/sii902x-audio.h new file +2 -0
+
+#endif /* __DT_SII9022_AUDIO_H */

Signature · 7 lines

Regards,

Laurent Pinchart
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Laurent Pinchart

reviewer

2019-03-13 16:47 UTC #11 lore raw

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

Hello again,

Quoted 23 lines · Laurent Pinchart

Quoted 22 lines · 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 <[email protected]>

---
 .../bindings/display/bridge/sii902x.txt       | 34 +++++++++++++++++++
 include/dt-bindings/sound/sii902x-audio.h     | 17 ++++++++++
 2 files changed, 51 insertions(+)
 create mode 100644 include/dt-bindings/sound/sii902x-audio.h

Documentation/devicetree/bindings/display/bridge/sii902x.txt +4 -0

index c4c1855ca654..977756841193 100644
@@ -8,6 +8,29 @@ Optional properties:
 	- interrupts: describe the interrupt line used to inform the host
 	  about hotplug events.
 	- reset-gpios: OF device-tree gpio specification for RST_N pin.
+	- sil,i2s-fifo-routing: Array of exactly 4 integers indicating i2s
+	  pins for audio fifo routing. First integer defines routing to
+	  fifo 0 and second to fifo 1, etc. Integers can be filled with
+	  definitions from: include/dt-bindings/sound/sii902x-audio.h

Quoted 8 lines

Documentation/devicetree/bindings/display/bridge/sii902x.txt Optional properties: · line 19
Quoted 5 lines
Documentation/devicetree/bindings/display/bridge/sii902x.txt +5 -0
+	  The available definitions are:
+	  - ENABLE_BIT:	enable this audio fifo
+	  - CONNECT_SD#: route audio input from SD0, SD1, SD2, or SD3 i2s
+			 data input pin
+	  - LEFT_RIGHT_SWAP_BIT: swap i2s input channels for this fifo
Are all combinations valid ? For instance, could we have D1 routed to the third FIFO, and all other FIFOs disabled ?

I found the answer to this question in the datasheet:

"Note that no gaps are allowed when mapping channels to FIFOs – SD pins must be mapped to FIFO#0 and FIFO#1 before mapping a channel to FIFO#2, and so on."

I think we can thus simplify the bindings, and use an approach similar to the one taken by the data-lanes property for CSI-2. Furthermore, I think this should be standardized, not left device-specific.

How about an sd-lanes property (better names are welcome) that would store an array of N integers, where each sd-lanes[i] tells which SD pin the i-th FIFO is connected to ?

Quoted 33 lines

Documentation/devicetree/bindings/display/bridge/sii902x.txt Optional properties: · line 21
Quoted 2 lines
Documentation/devicetree/bindings/display/bridge/sii902x.txt +2 -0
+	  I2S HDMI audio is configured only if this property is found.
+	- clocks: phandle mclk
Maybe "clocks: phandle and clock specifier for each clock listed in the clock-names property" ?
Documentation/devicetree/bindings/display/bridge/sii902x.txt Optional properties: · line 25
Quoted 4 lines
Documentation/devicetree/bindings/display/bridge/sii902x.txt +4 -0
+	- clock-names: "mclk"
+	    Describes SII902x MCLK input. MCLK is used to produce
+	    HDMI audio CTS values. This property is required if
+	    "i2s-fifo-routing"-property is present. This property follows
The property is named sil,i2s-fifo-routing.
Quoted 21 lines
Documentation/devicetree/bindings/display/bridge/sii902x.txt +14 -0
+	    Documentation/devicetree/bindings/clock/clock-bindings.txt
+	    consumer binding.
+	- #sound-dai-cells = <0>: ASoC codec dai available for simple-card
+	    If audio properties are present sii902x provides an ASoC
+	    codec component driver that can be used by other ASoC
+	    components like simple-card. See binding document for
+	    details:
+	    Documentation/devicetree/bindings/sound/simple-card.txt
 
 Optional subnodes:
 	- video input: this subnode can contain a video input port node
@@ -21,6 +44,17 @@ Example:
 		compatible = "sil,sii9022";
 		reg = <0x39>;
 		reset-gpios = <&pioA 1 0>;
+
+		#sound-dai-cells = <0>;
+		sil,i2s-fifo-routing = <
+			(ENABLE_BIT|CONNECT_SD0)
+			0
+			0

Quoted 8 lines

Documentation/devicetree/bindings/display/bridge/sii902x.txt Example: · line 60
Quoted 8 lines
Documentation/devicetree/bindings/display/bridge/sii902x.txt +5 -0
+			0
+		>;
+		clocks = <&mclk>;
+		clock-names = "mclk";
+
 		ports {
 			#address-cells = <1>;
 			#size-cells = <0>;

I also forgot to mention that the audio connection should be modeled using OF graph too.

Quoted 26 lines

Quoted 11 lines
include/dt-bindings/sound/sii902x-audio.h new file +5 -0
new file mode 100644
index 000000000000..0a849904754b
@@ -0,0 +1,17 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com/
+ * Author: Jyri Sarha <[email protected]>
+ */
include/dt-bindings/sound/sii902x-audio.h line 15
Quoted 10 lines
include/dt-bindings/sound/sii902x-audio.h new file +10 -0
+
+#ifndef __DT_SII9022_AUDIO_H
+#define __DT_SII9022_AUDIO_H
+
+#define ENABLE_BIT			0x80
+#define CONNECT_SD0			0x00
+#define CONNECT_SD1			0x10
+#define CONNECT_SD2			0x20
+#define CONNECT_SD3			0x30
+#define LEFT_RIGHT_SWAP_BIT		0x04
This is fairly generic, should you prefix the macros with SII9022 ?
Quoted 2 lines
include/dt-bindings/sound/sii902x-audio.h new file +2 -0
+
+#endif /* __DT_SII9022_AUDIO_H */

Signature · 7 lines

Regards,

Laurent Pinchart
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Jyri Sarha

author

2019-03-13 17:52 UTC #12 lore raw

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

Quoted 41 lines · Laurent Pinchart

Hello again,
Quoted 23 lines · Laurent Pinchart
Quoted 22 lines · 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 <[email protected]>
---
 .../bindings/display/bridge/sii902x.txt       | 34 +++++++++++++++++++
 include/dt-bindings/sound/sii902x-audio.h     | 17 ++++++++++
 2 files changed, 51 insertions(+)
 create mode 100644 include/dt-bindings/sound/sii902x-audio.h
Documentation/devicetree/bindings/display/bridge/sii902x.txt +4 -0
index c4c1855ca654..977756841193 100644
@@ -8,6 +8,29 @@ Optional properties:
 	- interrupts: describe the interrupt line used to inform the host
 	  about hotplug events.
 	- reset-gpios: OF device-tree gpio specification for RST_N pin.
+	- sil,i2s-fifo-routing: Array of exactly 4 integers indicating i2s
+	  pins for audio fifo routing. First integer defines routing to
+	  fifo 0 and second to fifo 1, etc. Integers can be filled with
+	  definitions from: include/dt-bindings/sound/sii902x-audio.h
Quoted 8 lines
Documentation/devicetree/bindings/display/bridge/sii902x.txt Optional properties: · line 19
Quoted 5 lines
Documentation/devicetree/bindings/display/bridge/sii902x.txt +5 -0
+	  The available definitions are:
+	  - ENABLE_BIT:	enable this audio fifo
+	  - CONNECT_SD#: route audio input from SD0, SD1, SD2, or SD3 i2s
+			 data input pin
+	  - LEFT_RIGHT_SWAP_BIT: swap i2s input channels for this fifo
Are all combinations valid ? For instance, could we have D1 routed to the third FIFO, and all other FIFOs disabled ?
I found the answer to this question in the datasheet:
"Note that no gaps are allowed when mapping channels to FIFOs – SD pins must be mapped to FIFO#0 and FIFO#1 before mapping a channel to FIFO#2, and so on."

Quoted 7 lines

I think we can thus simplify the bindings, and use an approach similar to the one taken by the data-lanes property for CSI-2. Furthermore, I think this should be standardized, not left device-specific.
How about an sd-lanes property (better names are welcome) that would store an array of N integers, where each sd-lanes[i] tells which SD pin the i-th FIFO is connected to ?

I agree otherwise, but I would still rather use i2s than sd, because it is more explicit. SD overlaps with so many other acronyms. So how would i2s-lanes sound?

Quoted 6 lines

Quoted 6 lines
Documentation/devicetree/bindings/display/bridge/sii902x.txt Optional properties: · line 21
Quoted 3 lines
Documentation/devicetree/bindings/display/bridge/sii902x.txt +2 -0
+	  I2S HDMI audio is configured only if this property is found.
+	- clocks: phandle mclk
Maybe "clocks: phandle and clock specifier for each clock listed in the clock-names property" ?

Quoted 7 lines

Quoted 7 lines
Documentation/devicetree/bindings/display/bridge/sii902x.txt Optional properties: · line 25
Quoted 4 lines
Documentation/devicetree/bindings/display/bridge/sii902x.txt +4 -0
+	- clock-names: "mclk"
+	    Describes SII902x MCLK input. MCLK is used to produce
+	    HDMI audio CTS values. This property is required if
+	    "i2s-fifo-routing"-property is present. This property follows
The property is named sil,i2s-fifo-routing.

Quoted 24 lines

Documentation/devicetree/bindings/display/bridge/sii902x.txt +17 -0

+	    Documentation/devicetree/bindings/clock/clock-bindings.txt
+	    consumer binding.
+	- #sound-dai-cells = <0>: ASoC codec dai available for simple-card
+	    If audio properties are present sii902x provides an ASoC
+	    codec component driver that can be used by other ASoC
+	    components like simple-card. See binding document for
+	    details:
+	    Documentation/devicetree/bindings/sound/simple-card.txt
 
 Optional subnodes:
 	- video input: this subnode can contain a video input port node

@@ -21,6 +44,17 @@ Example:
 		compatible = "sil,sii9022";
 		reg = <0x39>;
 		reset-gpios = <&pioA 1 0>;
+
+		#sound-dai-cells = <0>;
+		sil,i2s-fifo-routing = <
+			(ENABLE_BIT|CONNECT_SD0)
+			0
+			0
+			0
+		>;
+		clocks = <&mclk>;

Quoted 9 lines

Quoted 5 lines
Documentation/devicetree/bindings/display/bridge/sii902x.txt Example: · line 60
Quoted 5 lines
Documentation/devicetree/bindings/display/bridge/sii902x.txt +2 -0
+		clock-names = "mclk";
+
 		ports {
 			#address-cells = <1>;
 			#size-cells = <0>;
I also forgot to mention that the audio connection should be modeled using OF graph too.

With the audio configuration sii902x becomes an ASoC codec component. There are current more than one way to describe the connections between ASoC components (see [1] and [2] for details). The individual ASoC DAI drivers (digital audio interface) do not need to know about the audio system topology, that is taken care of by the machine driver (or card driver if you will), for instance simple-card or audio-graph-card.

However, since sii902x device is agnostic about the way the connections are described, I don't think I should go into too many details about how the possible bindings. However, I should add a reference at least to audio-graph-card too.

[1] Documentation/devicetree/bindings/sound/simple-card.txt
[2] Documentation/devicetree/bindings/sound/audio-graph-card.txt

Quoted 27 lines

Quoted 26 lines
Quoted 11 lines
include/dt-bindings/sound/sii902x-audio.h new file +5 -0
new file mode 100644
index 000000000000..0a849904754b
@@ -0,0 +1,17 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com/
+ * Author: Jyri Sarha <[email protected]>
+ */
include/dt-bindings/sound/sii902x-audio.h line 15
Quoted 10 lines
include/dt-bindings/sound/sii902x-audio.h new file +10 -0
+
+#ifndef __DT_SII9022_AUDIO_H
+#define __DT_SII9022_AUDIO_H
+
+#define ENABLE_BIT			0x80
+#define CONNECT_SD0			0x00
+#define CONNECT_SD1			0x10
+#define CONNECT_SD2			0x20
+#define CONNECT_SD3			0x30
+#define LEFT_RIGHT_SWAP_BIT		0x04
This is fairly generic, should you prefix the macros with SII9022 ?
Quoted 2 lines
include/dt-bindings/sound/sii902x-audio.h new file +2 -0
+
+#endif /* __DT_SII9022_AUDIO_H */

Signature · 6 lines

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

Laurent Pinchart

reviewer

2019-03-13 18:12 UTC #13 lore raw

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

Hi Jyri,

Quoted 44 lines · Jyri Sarha

Quoted 43 lines · Laurent Pinchart
Quoted 23 lines · Laurent Pinchart
Quoted 22 lines · 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 <[email protected]>
---
 .../bindings/display/bridge/sii902x.txt       | 34 +++++++++++++++++++
 include/dt-bindings/sound/sii902x-audio.h     | 17 ++++++++++
 2 files changed, 51 insertions(+)
 create mode 100644 include/dt-bindings/sound/sii902x-audio.h
Documentation/devicetree/bindings/display/bridge/sii902x.txt +4 -0
index c4c1855ca654..977756841193 100644
@@ -8,6 +8,29 @@ Optional properties:
 	- interrupts: describe the interrupt line used to inform the host
 	  about hotplug events.
 	- reset-gpios: OF device-tree gpio specification for RST_N pin.
+	- sil,i2s-fifo-routing: Array of exactly 4 integers indicating i2s
+	  pins for audio fifo routing. First integer defines routing to
+	  fifo 0 and second to fifo 1, etc. Integers can be filled with
+	  definitions from: include/dt-bindings/sound/sii902x-audio.h
Quoted 8 lines
Documentation/devicetree/bindings/display/bridge/sii902x.txt Optional properties: · line 19
Quoted 5 lines
Documentation/devicetree/bindings/display/bridge/sii902x.txt +5 -0
+	  The available definitions are:
+	  - ENABLE_BIT:	enable this audio fifo
+	  - CONNECT_SD#: route audio input from SD0, SD1, SD2, or SD3 i2s
+			 data input pin
+	  - LEFT_RIGHT_SWAP_BIT: swap i2s input channels for this fifo
Are all combinations valid ? For instance, could we have D1 routed to the third FIFO, and all other FIFOs disabled ?
I found the answer to this question in the datasheet:
"Note that no gaps are allowed when mapping channels to FIFOs – SD pins must be mapped to FIFO#0 and FIFO#1 before mapping a channel to FIFO#2, and so on."
I think we can thus simplify the bindings, and use an approach similar to the one taken by the data-lanes property for CSI-2. Furthermore, I think this should be standardized, not left device-specific.

Quoted 7 lines

Quoted 3 lines
How about an sd-lanes property (better names are welcome) that would store an array of N integers, where each sd-lanes[i] tells which SD pin the i-th FIFO is connected to ?
I agree otherwise, but I would still rather use i2s than sd, because it is more explicit. SD overlaps with so many other acronyms. So how would i2s-lanes sound?

Sounds good to me. It's a better name, so it's welcome :-) I don't know what terminology is usually used in the audio world for this, so I was pretty sure my initial name proposal was bad.

Is there a risk of needing to describe the clock lane separately in the future (for this or another I2S-related chip) ? If so, maybe i2s-data-lanes, or just data-lanes, would be a better pick.

Quoted 57 lines

Quoted 5 lines
Quoted 5 lines
Documentation/devicetree/bindings/display/bridge/sii902x.txt Optional properties: · line 21
Quoted 2 lines
Documentation/devicetree/bindings/display/bridge/sii902x.txt +2 -0
+	  I2S HDMI audio is configured only if this property is found.
+	- clocks: phandle mclk
Maybe "clocks: phandle and clock specifier for each clock listed in the clock-names property" ?
Ok
Quoted 6 lines
Quoted 6 lines
Documentation/devicetree/bindings/display/bridge/sii902x.txt Optional properties: · line 25
Quoted 4 lines
Documentation/devicetree/bindings/display/bridge/sii902x.txt +4 -0
+	- clock-names: "mclk"
+	    Describes SII902x MCLK input. MCLK is used to produce
+	    HDMI audio CTS values. This property is required if
+	    "i2s-fifo-routing"-property is present. This property follows
The property is named sil,i2s-fifo-routing.
Ok
Quoted 24 lines
Quoted 24 lines
Quoted 24 lines
Documentation/devicetree/bindings/display/bridge/sii902x.txt +17 -0
+	    Documentation/devicetree/bindings/clock/clock-bindings.txt
+	    consumer binding.
+	- #sound-dai-cells = <0>: ASoC codec dai available for simple-card
+	    If audio properties are present sii902x provides an ASoC
+	    codec component driver that can be used by other ASoC
+	    components like simple-card. See binding document for
+	    details:
+	    Documentation/devicetree/bindings/sound/simple-card.txt
 
 Optional subnodes:
 	- video input: this subnode can contain a video input port node
@@ -21,6 +44,17 @@ Example:
 		compatible = "sil,sii9022";
 		reg = <0x39>;
 		reset-gpios = <&pioA 1 0>;
+
+		#sound-dai-cells = <0>;
+		sil,i2s-fifo-routing = <
+			(ENABLE_BIT|CONNECT_SD0)
+			0
+			0
+			0
+		>;
+		clocks = <&mclk>;
Quoted 8 lines
Quoted 5 lines
Documentation/devicetree/bindings/display/bridge/sii902x.txt Example: · line 60
Quoted 5 lines
Documentation/devicetree/bindings/display/bridge/sii902x.txt +2 -0
+		clock-names = "mclk";
+
 		ports {
 			#address-cells = <1>;
 			#size-cells = <0>;
I also forgot to mention that the audio connection should be modeled using OF graph too.
With the audio configuration sii902x becomes an ASoC codec component. There are current more than one way to describe the connections between ASoC components (see [1] and [2] for details). The individual ASoC DAI drivers (digital audio interface) do not need to know about the audio system topology, that is taken care of by the machine driver (or card driver if you will), for instance simple-card or audio-graph-card.

Quoted 7 lines

However, since sii902x device is agnostic about the way the connections are described, I don't think I should go into too many details about how the possible bindings. However, I should add a reference at least to audio-graph-card too.
[1] Documentation/devicetree/bindings/sound/simple-card.txt
[2] Documentation/devicetree/bindings/sound/audio-graph-card.txt

References to those bindings would be helpful, yes.

Quoted 26 lines

Quoted 26 lines
Quoted 26 lines
Quoted 11 lines
include/dt-bindings/sound/sii902x-audio.h new file +5 -0
new file mode 100644
index 000000000000..0a849904754b
@@ -0,0 +1,17 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com/
+ * Author: Jyri Sarha <[email protected]>
+ */
include/dt-bindings/sound/sii902x-audio.h line 15
Quoted 10 lines
include/dt-bindings/sound/sii902x-audio.h new file +10 -0
+
+#ifndef __DT_SII9022_AUDIO_H
+#define __DT_SII9022_AUDIO_H
+
+#define ENABLE_BIT			0x80
+#define CONNECT_SD0			0x00
+#define CONNECT_SD1			0x10
+#define CONNECT_SD2			0x20
+#define CONNECT_SD3			0x30
+#define LEFT_RIGHT_SWAP_BIT		0x04
This is fairly generic, should you prefix the macros with SII9022 ?
Quoted 2 lines
include/dt-bindings/sound/sii902x-audio.h new file +2 -0
+
+#endif /* __DT_SII9022_AUDIO_H */

Signature · 7 lines

Regards,

Laurent Pinchart
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Jyri Sarha

author

2019-03-13 19:28 UTC #14 lore raw

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

Quoted 51 lines · Laurent Pinchart

Hi Jyri,
Quoted 42 lines · Jyri Sarha
Quoted 41 lines · Laurent Pinchart
Quoted 22 lines · Laurent Pinchart
Quoted 21 lines · 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 <[email protected]>
---
 .../bindings/display/bridge/sii902x.txt       | 34 +++++++++++++++++++
 include/dt-bindings/sound/sii902x-audio.h     | 17 ++++++++++
 2 files changed, 51 insertions(+)
 create mode 100644 include/dt-bindings/sound/sii902x-audio.h
Documentation/devicetree/bindings/display/bridge/sii902x.txt +3 -0
index c4c1855ca654..977756841193 100644
@@ -8,6 +8,29 @@ Optional properties:
 	- interrupts: describe the interrupt line used to inform the host
 	  about hotplug events.
 	- reset-gpios: OF device-tree gpio specification for RST_N pin.
+	- sil,i2s-fifo-routing: Array of exactly 4 integers indicating i2s
+	  pins for audio fifo routing. First integer defines routing to
+	  fifo 0 and second to fifo 1, etc. Integers can be filled with
Quoted 8 lines
Documentation/devicetree/bindings/display/bridge/sii902x.txt Optional properties: · line 19
Quoted 6 lines
Documentation/devicetree/bindings/display/bridge/sii902x.txt +6 -0
+	  definitions from: include/dt-bindings/sound/sii902x-audio.h
+	  The available definitions are:
+	  - ENABLE_BIT:	enable this audio fifo
+	  - CONNECT_SD#: route audio input from SD0, SD1, SD2, or SD3 i2s
+			 data input pin
+	  - LEFT_RIGHT_SWAP_BIT: swap i2s input channels for this fifo
Are all combinations valid ? For instance, could we have D1 routed to the third FIFO, and all other FIFOs disabled ?
I found the answer to this question in the datasheet:
"Note that no gaps are allowed when mapping channels to FIFOs – SD pins must be mapped to FIFO#0 and FIFO#1 before mapping a channel to FIFO#2, and so on."
I think we can thus simplify the bindings, and use an approach similar to the one taken by the data-lanes property for CSI-2. Furthermore, I think this should be standardized, not left device-specific.
Quoted 6 lines
Quoted 3 lines
How about an sd-lanes property (better names are welcome) that would store an array of N integers, where each sd-lanes[i] tells which SD pin the i-th FIFO is connected to ?
I agree otherwise, but I would still rather use i2s than sd, because it is more explicit. SD overlaps with so many other acronyms. So how would i2s-lanes sound?

Quoted 8 lines

Sounds good to me. It's a better name, so it's welcome :-) I don't know what terminology is usually used in the audio world for this, so I was pretty sure my initial name proposal was bad.
Is there a risk of needing to describe the clock lane separately in the future (for this or another I2S-related chip) ? If so, maybe i2s-data-lanes, or just data-lanes, would be a better pick.

Usually (or always AFAIK) there is only one bit clock and one frame clock lane, and 1 - n data lanes. But still being more explicit does not hurt, let's make it i2s-data-lanes.

Thanks for the prompt review. I'll try to make the changes for tomorrow.

Best regards,
Jyri

Signature · 6 lines

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

Olivier MOYSAN

2019-03-14 10:11 UTC #15 lore raw

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

In reply to Jyri Sarha

Hello Jyri,

Quoted 114 lines · Jyri Sarha

Quoted 41 lines · Laurent Pinchart
Hello again,
Quoted 23 lines · Laurent Pinchart
Quoted 22 lines · 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 <[email protected]>
---
  .../bindings/display/bridge/sii902x.txt       | 34 +++++++++++++++++++
  include/dt-bindings/sound/sii902x-audio.h     | 17 ++++++++++
  2 files changed, 51 insertions(+)
create mode 100644 include/dt-bindings/sound/sii902x-audio.h
Documentation/devicetree/bindings/display/bridge/sii902x.txt +4 -0
index c4c1855ca654..977756841193 100644
@@ -8,6 +8,29 @@ Optional properties:
  	- interrupts: describe the interrupt line used to inform the host
  	  about hotplug events.
  	- reset-gpios: OF device-tree gpio specification for RST_N pin.
+	- sil,i2s-fifo-routing: Array of exactly 4 integers indicating i2s
+	  pins for audio fifo routing. First integer defines routing to
+	  fifo 0 and second to fifo 1, etc. Integers can be filled with
+	  definitions from: include/dt-bindings/sound/sii902x-audio.h
Quoted 8 lines
Documentation/devicetree/bindings/display/bridge/sii902x.txt Optional properties: · line 19
Quoted 5 lines
Documentation/devicetree/bindings/display/bridge/sii902x.txt +5 -0
+	  The available definitions are:
+	  - ENABLE_BIT:	enable this audio fifo
+	  - CONNECT_SD#: route audio input from SD0, SD1, SD2, or SD3 i2s
+			 data input pin
+	  - LEFT_RIGHT_SWAP_BIT: swap i2s input channels for this fifo
Are all combinations valid ? For instance, could we have D1 routed to the third FIFO, and all other FIFOs disabled ?
I found the answer to this question in the datasheet:
"Note that no gaps are allowed when mapping channels to FIFOs – SD pins must be mapped to FIFO#0 and FIFO#1 before mapping a channel to FIFO#2, and so on."
Quoted 7 lines
I think we can thus simplify the bindings, and use an approach similar to the one taken by the data-lanes property for CSI-2. Furthermore, I think this should be standardized, not left device-specific.
How about an sd-lanes property (better names are welcome) that would store an array of N integers, where each sd-lanes[i] tells which SD pin the i-th FIFO is connected to ?
I agree otherwise, but I would still rather use i2s than sd, because it is more explicit. SD overlaps with so many other acronyms. So how would i2s-lanes sound?
Quoted 6 lines
Quoted 6 lines
Documentation/devicetree/bindings/display/bridge/sii902x.txt Optional properties: · line 21
Quoted 3 lines
Documentation/devicetree/bindings/display/bridge/sii902x.txt +2 -0
+	  I2S HDMI audio is configured only if this property is found.
+	- clocks: phandle mclk
Maybe "clocks: phandle and clock specifier for each clock listed in the clock-names property" ?
Ok
Quoted 7 lines
Quoted 7 lines
Documentation/devicetree/bindings/display/bridge/sii902x.txt Optional properties: · line 25
Quoted 4 lines
Documentation/devicetree/bindings/display/bridge/sii902x.txt +4 -0
+	- clock-names: "mclk"
+	    Describes SII902x MCLK input. MCLK is used to produce
+	    HDMI audio CTS values. This property is required if
+	    "i2s-fifo-routing"-property is present. This property follows
The property is named sil,i2s-fifo-routing.
Ok
Quoted 24 lines
Quoted 24 lines
Quoted 24 lines
Documentation/devicetree/bindings/display/bridge/sii902x.txt +17 -0
+	    Documentation/devicetree/bindings/clock/clock-bindings.txt
+	    consumer binding.
+	- #sound-dai-cells = <0>: ASoC codec dai available for simple-card
+	    If audio properties are present sii902x provides an ASoC
+	    codec component driver that can be used by other ASoC
+	    components like simple-card. See binding document for
+	    details:
+	    Documentation/devicetree/bindings/sound/simple-card.txt
 
  Optional subnodes:
  	- video input: this subnode can contain a video input port node
@@ -21,6 +44,17 @@ Example:
  		compatible = "sil,sii9022";
  		reg = <0x39>;
  		reset-gpios = <&pioA 1 0>;
+
+		#sound-dai-cells = <0>;
+		sil,i2s-fifo-routing = <
+			(ENABLE_BIT|CONNECT_SD0)
+			0
+			0
+			0
+		>;
+		clocks = <&mclk>;
Quoted 9 lines
Quoted 5 lines
Documentation/devicetree/bindings/display/bridge/sii902x.txt Example: · line 60
Quoted 5 lines
Documentation/devicetree/bindings/display/bridge/sii902x.txt +2 -0
+		clock-names = "mclk";
+
  		ports {
  			#address-cells = <1>;
  			#size-cells = <0>;
I also forgot to mention that the audio connection should be modeled using OF graph too.
With the audio configuration sii902x becomes an ASoC codec component. There are current more than one way to describe the connections between ASoC components (see [1] and [2] for details). The individual ASoC DAI drivers (digital audio interface) do not need to know about the audio system topology, that is taken care of by the machine driver (or card driver if you will), for instance simple-card or audio-graph-card.

Quoted 9 lines

However, since sii902x device is agnostic about the way the connections are described, I don't think I should go into too many details about how the possible bindings. However, I should add a reference at least to audio-graph-card too.
[1] Documentation/devicetree/bindings/sound/simple-card.txt
[2] Documentation/devicetree/bindings/sound/audio-graph-card.txt

My understanding is that of graph is now the preferred way to describe audio topology. So a think that it needs to be more detailed here.

Moreover, the support of audio-graph-card requires a driver update.

audio graph card parses endpoints to retrieve codec DAI id. For hdmi codec, get_dai_id() callback has to be implemented, as stated in snd_soc_get_dai_id(), to retrieve the right DAI id:
* For example HDMI case, HDMI has video/sound port,
* but ALSA SoC needs sound port number only.
* Thus counting HDMI DT port/endpoint doesn't work.
* Then, it should have .of_xlate_dai_id

Regards
Olivier

Quoted 24 lines

Quoted 24 lines
Quoted 24 lines
Quoted 11 lines
include/dt-bindings/sound/sii902x-audio.h new file +5 -0
new file mode 100644
index 000000000000..0a849904754b
@@ -0,0 +1,17 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com/
+ * Author: Jyri Sarha <[email protected]>
+ */
include/dt-bindings/sound/sii902x-audio.h line 15
Quoted 10 lines
include/dt-bindings/sound/sii902x-audio.h new file +10 -0
+
+#ifndef __DT_SII9022_AUDIO_H
+#define __DT_SII9022_AUDIO_H
+
+#define ENABLE_BIT			0x80
+#define CONNECT_SD0			0x00
+#define CONNECT_SD1			0x10
+#define CONNECT_SD2			0x20
+#define CONNECT_SD3			0x30
+#define LEFT_RIGHT_SWAP_BIT		0x04
This is fairly generic, should you prefix the macros with SII9022 ?

Quoted 5 lines

Quoted 3 lines
Quoted 2 lines
include/dt-bindings/sound/sii902x-audio.h line 17
Quoted 2 lines
include/dt-bindings/sound/sii902x-audio.h new file +2 -0
+
+#endif /* __DT_SII9022_AUDIO_H */

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Jyri Sarha

author

2019-03-13 16:01 UTC #16 lore raw

PATCH v3 6/6 drm/bridge: sii902x: Implement HDMI audio support

In reply to 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 <[email protected]>

---
 drivers/gpu/drm/bridge/sii902x.c | 455 ++++++++++++++++++++++++++++++-
 1 file changed, 448 insertions(+), 7 deletions(-)

drivers/gpu/drm/bridge/sii902x.c +448 -7

index 1e917777ed72..3f49bf24e2a3 100644
@@ -27,12 +27,16 @@
 #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>
+#include <dt-bindings/sound/sii902x-audio.h>
+
 #define SII902X_TPI_VIDEO_DATA			0x0
 
 #define SII902X_TPI_PIXEL_REPETITION		0x8

@@ -74,6 +78,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 +156,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 +175,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_routing[4];
+	} audio;
 };
 
 static int sii902x_read_unlocked(struct i2c_client *i2c, u8 reg, u8 *val)

@@ -161,8 +256,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 +279,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 +298,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 +330,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 +378,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 +443,323 @@ 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; i < ARRAY_SIZE(sii902x->audio.i2s_fifo_routing); i++)
+		regmap_write(sii902x->regmap,
+			     SII902X_TPI_I2S_ENABLE_MAPPING_REG,
+			     sii902x->audio.i2s_fifo_routing[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 i2s_fifo_defaults[] = {
+		SII902X_TPI_I2S_CONFIG_FIFO0,
+		SII902X_TPI_I2S_CONFIG_FIFO1,
+		SII902X_TPI_I2S_CONFIG_FIFO2,
+		SII902X_TPI_I2S_CONFIG_FIFO3,
+	};
+	struct hdmi_codec_pdata codec_data = {
+		.ops = &sii902x_audio_codec_ops,
+		.i2s = 1, /* Only i2s support for now. */
+		.spdif = 0,
+		.max_i2s_channels = 0,
+	};
+	int ret, i;
+
+	ret = of_property_read_u32_array(dev->of_node, "sil,i2s-fifo-routing",
+					 sii902x->audio.i2s_fifo_routing,
+					 ARRAY_SIZE(sii902x->audio.i2s_fifo_routing));
+
+	if (ret != 0) {
+		if (ret == -EINVAL)
+			dev_dbg(dev, "%s: No \"i2s-fifo-routing\", no audio\n",
+			__func__);
+		else
+			dev_err(dev,
+				"%s: Error gettin \"i2s-fifo-routing\": %d\n",
+				__func__, ret);
+		return 0;
+	}
+
+	sii902x->audio.mclk = devm_clk_get(dev, "mclk");
+	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;
+	}
+
+	for (i = 0; i < ARRAY_SIZE(sii902x->audio.i2s_fifo_routing); i++) {
+		if (sii902x->audio.i2s_fifo_routing[i] & ENABLE_BIT)
+			codec_data.max_i2s_channels += 2;
+		sii902x->audio.i2s_fifo_routing[i] |= i2s_fifo_defaults[i];
+	}
+
+	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 +772,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 +945,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 +987,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,

Signature · 6 lines

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

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel