drm/bridge: sii902x: HDMI-audio support and some fixes
Subsystems (5)
v2 cover drm/bridge: sii902x: HDMI-audio support and some fixes
Changes since first version:
- Moved reviewed patches to 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
- Added a new fix:
- drm/bridge: sii902x: Select I2C_MUX
- Applied some review suggestions to
- drm/bridge: sii902x: Implement HDMI audio support
- use clock-names property to name mclk
- move comment describing added mutex to struct sii902x and improve it
- cleanup sii902x_mute()
- cleanup sii902x_select_mclk_div()
- fix condition for checking ENABLE_BIT from i2s_fifo_routing in sii902x_audio_codec_init()
Still to do
- Agree on i2s wires to HDMI audio fifo routing in dts.
The current scheme is quite straight forward, but there is maybe there is even more straight forward solutions like:
audio-fifo-enable = <1 1 1 1>; audio-i2s-pin-to-fifo = <0 1 2 3>;
Meaning that all fifos are enabled and SD0 is routed to fifo 0, SD1 to fifo 1, etc. I am not sure if the channel swap functionality should show in dts binding.
Jyri Sarha (4):
drm/bridge: sii902x: Set output mode to HDMI or DVI according to EDID drm/bridge: sii902x: pixel clock unit is 10kHz instead of 1kHz drm/bridge: sii902x: Select I2C_MUX
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/Kconfig | 1 + drivers/gpu/drm/bridge/sii902x.c | 472 +++++++++++++++++- include/dt-bindings/sound/sii902x-audio.h | 11 + 4 files changed, 512 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
PATCH v2 1/5 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.
--- drivers/gpu/drm/bridge/sii902x.c | 7 +++++++ 1 file changed, 7 insertions(+)
drivers/gpu/drm/bridge/sii902x.c +7 -0
@@ -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
Re: [PATCH v2 1/5] drm/bridge: sii902x: add input_bus_flags
Hi 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_flagsto 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]>
Quoted 9 lines
--- drivers/gpu/drm/bridge/sii902x.c | 7 +++++++ 1 file changed, 7 insertions(+)drivers/gpu/drm/bridge/sii902x.c +0
-0
drivers/gpu/drm/bridge/sii902x.c static int sii902x_i2c_bypass_deselect(struct i2c_mux_core *mux, u32 chan_id) · line 467
Quoted 9 lines
drivers/gpu/drm/bridge/sii902x.c +6
-0@@ -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 , +}; +
Should you add a comment where the drive sets the EDGE bit to 0 to state that default_sii902x_timings should then be made dynamic ? I suppose it would be hard to miss that though, so maybe it's overkill. In any case,
Quoted 11 lines
drivers/gpu/drm/bridge/sii902x.c +1
-0static 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
PATCH v2 2/5 drm/bridge: sii902x: Set output mode to HDMI or DVI according to EDID
In reply to Jyri SarhaSet output mode to HDMI or DVI according to EDID HDMI signature.
--- drivers/gpu/drm/bridge/sii902x.c | 9 +++++++++ 1 file changed, 9 insertions(+)
drivers/gpu/drm/bridge/sii902x.c +9 -0
@@ -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
reviewerRe: [PATCH v2 2/5] drm/bridge: sii902x: Set output mode to HDMI or DVI according to EDID
Hi Jyri,
Thank you for the patch.
Quoted 13 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]>--- drivers/gpu/drm/bridge/sii902x.c | 9 +++++++++ 1 file changed, 9 insertions(+)drivers/gpu/drm/bridge/sii902x.c +0
-0
drivers/gpu/drm/bridge/sii902x.c static int sii902x_get_modes(struct drm_connector *connector) · line 184
Quoted 4 lines
drivers/gpu/drm/bridge/sii902x.c +1
-0@@ -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 ;
I'd move this one line up, but that's certainly a matter of taste :-)
Quoted 12 lines
drivers/gpu/drm/bridge/sii902x.c +3
-0int 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); }
drivers/gpu/drm/bridge/sii902x.c static int sii902x_get_modes(struct drm_connector *connector) · line 206
Quoted 8 lines
drivers/gpu/drm/bridge/sii902x.c +5
-0@@ -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; +
Is this the right place to update the register, shouldn't this be done in sii902x_bridge_enable() instead ? I could foresee cases where the chip could be powered down between get_modes() and enable(), losing its internal state.
Quoted 3 lines
drivers/gpu/drm/bridge/sii902x.c +0
-0return num; }
Signature · 7 lines
Regards, Laurent Pinchart _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 2/5] drm/bridge: sii902x: Set output mode to HDMI or DVI according to EDID
Quoted 12 lines · Laurent Pinchart
Hi Jyri,
Thank you for the patch.
Quoted 8 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]>--- drivers/gpu/drm/bridge/sii902x.c | 9 +++++++++ 1 file changed, 9 insertions(+)
Quoted 11 lines
drivers/gpu/drm/bridge/sii902x.c static int sii902x_get_modes(struct drm_connector *connector) · line 184
Quoted 9 lines
drivers/gpu/drm/bridge/sii902x.c +1
-0@@ -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 ;I'd move this one line up, but that's certainly a matter of taste :-)
I usually sort the local variables by length too. I wonder why I did not do it this time... I'll fix it :).
Quoted 15 lines
Quoted 15 lines
drivers/gpu/drm/bridge/sii902x.c +3
-0int 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;
Quoted 11 lines
drivers/gpu/drm/bridge/sii902x.c static int sii902x_get_modes(struct drm_connector *connector) · line 206
Quoted 5 lines
drivers/gpu/drm/bridge/sii902x.c +5
-0+ ret =regmap_update_bits (sii902x->regmap,SII902X_SYS_CTRL_DATA , +SII902X_SYS_CTRL_OUTPUT_MODE , output_mode); +if (ret) +return ret; +Is this the right place to update the register, shouldn't this be done in
sii902x_bridge_enable()instead ? I could foresee cases where the chip could be powered down betweenget_modes()andenable(), losing its internal state.
I have a spec (unfortunately I can not share it) that describes the sequence of handling a hot plug event on sii9022. There it is said that the HDMI mode, if HDMI signature is found, should be set at the same time while releasing the DCC access by setting SII902X_SYS_CTRL_DATA bits #1 and #2 to zero.
However, I did not dare to change sii902x_i2c_bypass_deselect() function, so I set the HDMI mode right after the DCC pass trough mode is disabled.
Having it there is logical too, since the HDMI/DVI-mode should not change without a hot plug event. In practice sii9022 does not appear to be too picky when the bit is set.
Quoted 4 lines
Quoted 3 lines
drivers/gpu/drm/bridge/sii902x.c +0
-0return 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
PATCH v2 3/5 drm/bridge: sii902x: pixel clock unit is 10kHz instead of 1kHz
In reply to Jyri SarhaThe 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.
--- drivers/gpu/drm/bridge/sii902x.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
drivers/gpu/drm/bridge/sii902x.c +3 -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 ; + 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
Re: [PATCH v2 3/5] drm/bridge: sii902x: pixel clock unit is 10kHz instead of 1kHz
Hi Jyri,
Thank you for the patch.
Quoted 15 lines · Jyri Sarha
The pixel clock unit in the first two registers (
0x00and0x01) of sii9022 is 10kHz, not 1kHz as in structdrm_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
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 ;
Nitpicking, we usually use lowercase hex values.
Apart from that,
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
PATCH v2 4/5 drm/bridge: sii902x: Select I2C_MUX
In reply to Jyri Sarha"drm/bridge/sii902x: Fix EDID readback"-commit added a dependency to I2C_MUX, but not indicate it in the Kconfig entry. Fix it by selecting I2C_MUX for DRM_SII902X config option.
--- drivers/gpu/drm/bridge/Kconfig | 1 + 1 file changed, 1 insertion(+)
drivers/gpu/drm/bridge/Kconfig +1 -0
@@ -95,6 +95,7 @@ config DRM_SII902X depends on OF select DRM_KMS_HELPER select REGMAP_I2C + select I2C_MUX ---help--- Silicon Image sii902x bridge chip driver.
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
Re: [PATCH v2 4/5] drm/bridge: sii902x: Select I2C_MUX
Quoted 6 lines · Jyri Sarha
"drm/bridge/sii902x: Fix EDID readback"-commit added a dependency to
I2C_MUX, but not indicate it in theKconfigentry. Fix it by selectingI2C_MUXforDRM_SII902Xconfig option.Fixes 88664675239 ("drm/bridge/sii902x: Fix EDID readback")Signed-off-by Jyri Sarha <[email protected]>
--
Regards
Andrzej
Quoted 9 lines
--- drivers/gpu/drm/bridge/Kconfig | 1 + 1 file changed, 1 insertion(+)drivers/gpu/drm/bridge/Kconfig +0
-0
drivers/gpu/drm/bridge/Kconfig config DRM_SII902X · line 101
Quoted 7 lines
drivers/gpu/drm/bridge/Kconfig +1
-0@@ -95,6 +95,7 @@ config DRM_SII902X depends on OF select DRM_KMS_HELPER select REGMAP_I2C + select I2C_MUX ---help--- Silicon Image sii902x bridge chip driver.
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Laurent Pinchart
reviewerRe: [PATCH v2 4/5] drm/bridge: sii902x: Select I2C_MUX
In reply to Jyri SarhaHi Jyri,
Thank you for the patch.
Quoted 15 lines · Jyri Sarha
"drm/bridge/sii902x: Fix EDID readback"-commit added a dependency to
I2C_MUX, but not indicate it in theKconfigentry. Fix it by selectingI2C_MUXforDRM_SII902Xconfig option.Fixes 88664675239 ("drm/bridge/sii902x: Fix EDID readback")Signed-off-by Jyri Sarha <[email protected]>--- drivers/gpu/drm/bridge/Kconfig | 1 + 1 file changed, 1 insertion(+)drivers/gpu/drm/bridge/Kconfig +0
-0
drivers/gpu/drm/bridge/Kconfig config DRM_SII902X · line 101
Quoted 7 lines
drivers/gpu/drm/bridge/Kconfig +1
-0@@ -95,6 +95,7 @@ config DRM_SII902X depends on OF select DRM_KMS_HELPER select REGMAP_I2C + select I2C_MUX ---help--- Silicon Image sii902x bridge chip driver.
This is already present in v5.0.
commit ea6b13e9fed0fda9532ee04d38ed1bef1edbfdbf
Author: Fabrizio Castro <[email protected]> Date: Mon Nov 19 13:26:18 2018 +0000
drm/bridge/sii902x: Add missing dependency on I2C_MUX
Fabrizio stated in the commit message that "Quite obviously the driver depends on I2C_MUX, but adding a "depends on" introduces a recursive dependency, therefore this patch selects I2C_MUX instead.". Given that I2C_MUX is a tristate user-selectable option, "depend on" should be the right solution. I wonder if we could fix the root cause of the recursive dependency.
Signature · 7 lines
Regards, Laurent Pinchart _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH v2 5/5 drm/bridge: sii902x: Implement HDMI audio support
In reply to Jyri SarhaImplement 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.
--- .../bindings/display/bridge/sii902x.txt | 36 +- drivers/gpu/drm/bridge/sii902x.c | 453 +++++++++++++++++- include/dt-bindings/sound/sii902x-audio.h | 11 + 3 files changed, 493 insertions(+), 7 deletions(-) create mode 100644 include/dt-bindings/sound/sii902x-audio.h
Documentation/devicetree/bindings/display/bridge/sii902x.txt +35 -1
@@ -5,9 +5,32 @@ 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. + - 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>; + i2s-fifo-routing = < + (ENABLE_BIT|CONNECT_SD0) + 0 + 0 + 0 + >; + clocks = <&mclk>; + clock-names = "mclk"; + ports { #address-cells = <1>; #size-cells = <0>;
drivers/gpu/drm/bridge/sii902x.c +447 -6
575 lines not on this page · raw message
include/dt-bindings/sound/sii902x-audio.h new file +11 -0
@@ -0,0 +1,11 @@ +#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
Re: [PATCH v2 5/5] drm/bridge: sii902x: Implement HDMI audio support
Quoted 37 lines · 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]>--- .../bindings/display/bridge/sii902x.txt | 36 +- drivers/gpu/drm/bridge/sii902x.c | 453 +++++++++++++++++- include/dt-bindings/sound/sii902x-audio.h | 11 + 3 files changed, 493 insertions(+), 7 deletions(-) create mode 100644 include/dt-bindings/sound/sii902x-audio.hDocumentation/devicetree/bindings/display/bridge/sii902x.txt +14
-1@@ -5,9 +5,32 @@ 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. + - 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
Documentation/devicetree/bindings/display/bridge/sii902x.txt Required properties: · line 33
Quoted 10 lines
Documentation/devicetree/bindings/display/bridge/sii902x.txt +10
-0+ 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
I am not an audio expert, so please forgive me if I make stupid mistakes :)
As I wrote previously, lack of DT connection between the bridge and the audio producer looks for me as incomplete binding - hardware connection is neither discoverable neither represented in DT. As I understand it works because there is usually only one sound subsystem per board, and there is only one hdmi output. Am I right?
But I guess boards with more than one hdmi output are not uncommon, how do you discover topology in such case?
Regards
Andrzej
Quoted 620 lines
Documentation/devicetree/bindings/display/bridge/sii902x.txt +11
-0Optional 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>; + i2s-fifo-routing = < + (ENABLE_BIT|CONNECT_SD0) + 0 + 0 + 0 + >; + clocks = <&mclk>; + clock-names = "mclk"; + ports { #address-cells = <1>; #size-cells = <0>;drivers/gpu/drm/bridge/sii902x.c +447
-6@@ -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 500struct 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; }@@ -184,6 +283,8 @@ static int sii902x_get_modes(struct drm_connector *connector)u8 output_mode =SII902X_SYS_CTRL_OUTPUT_DVI ;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; + + ret = num; + +error_out: +mutex_unlock (&sii902x->mutex); -return num; +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 ; + } + } + + 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); + + ret =clk_prepare_enable (sii902x->audio.mclk); +if (ret) { +dev_err (dev,"Enabling mclk failed: %d\n" , ret); +return ret; + } + +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 (¶ms->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,"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,include/dt-bindings/sound/sii902x-audio.h new file +1
-0@@ -0,0 +1,11 @@ +#ifndef __DT_SII9022_AUDIO_H
include/dt-bindings/sound/sii902x-audio.h line 11
Quoted 10 lines
include/dt-bindings/sound/sii902x-audio.h new file +10
-0+#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 */
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Rob Herring
maintainerRe: [PATCH v2 5/5] drm/bridge: sii902x: Implement HDMI audio support
In reply to Jyri SarhaQuoted 9 lines · 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]>--- .../bindings/display/bridge/sii902x.txt | 36 +- drivers/gpu/drm/bridge/sii902x.c | 453 +++++++++++++++++- include/dt-bindings/sound/sii902x-audio.h | 11 +
Please split bindings (doc and header) to a separate patch.
Documentation/devicetree/bindings/display/bridge/sii902x.txt Required properties: · line 8
Quoted 13 lines
3 files changed, 493 insertions(+), 7 deletions(-) create mode 100644 include/dt-bindings/sound/sii902x-audio.hDocumentation/devicetree/bindings/display/bridge/sii902x.txt +1
-1@@ -5,9 +5,32 @@ 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
Unrelated change.
Documentation/devicetree/bindings/display/bridge/sii902x.txt Required properties: · line 11
Quoted 3 lines
Documentation/devicetree/bindings/display/bridge/sii902x.txt +1
-0about hotplug events. - reset-gpios: OF device-tree gpio specification for RST_N pin. + - i2s-fifo-routing: Array of exactly 4 integers indicating i2s
Needs a vendor prefix.
Quoted 42 lines
Documentation/devicetree/bindings/display/bridge/sii902x.txt +32
-0+ 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 fo> + 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>; + 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
Quoted 6 lines
include/dt-bindings/sound/sii902x-audio.h new file +0
-0@@ -0,0 +1,11 @@
License?
Quoted 14 lines
include/dt-bindings/sound/sii902x-audio.h new file +11
-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 + +#endif /* __DT_SII9022_AUDIO_H */ Signature · 1 line
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
reviewerHi Jyri,
Quoted 23 lines · Jyri Sarha
Changes since first version:
- Moved reviewed patches to front:
- drm/bridge: sii902x: addinput_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
- Added a new fix:
- drm/bridge: sii902x: SelectI2C_MUX
- Applied some review suggestions to
- drm/bridge: sii902x: Implement HDMI audio support
- use clock-names property to name mclk
- move comment describing added mutex to struct sii902x and improve it
- cleanupsii902x_mute()
- cleanupsii902x_select_mclk_div()
- fix condition for checkingENABLE_BITfromi2s_fifo_routinginsii902x_audio_codec_init()Still to do
- Agree on i2s wires to HDMI audio fifo routing in dts.
The current scheme is quite straight forward, but there is maybe there is even more straight forward solutions like:
Quoted 6 lines
audio-fifo-enable = <1 1 1 1>; audio-i2s-pin-to-fifo = <0 1 2 3>;Meaning that all fifos are enabled and SD0 is routed to fifo 0, SD1 to fifo 1, etc. I am not sure if the channel swap functionality should show in dts binding.
Please forgive my lack of audio knowledge, but it this a system description that should be encoded in DT, or a policy that should be handled purely in software (either fully inside the kernel or with the help of userspace) ?
Quoted 15 lines
Jyri Sarha (4):
drm/bridge: sii902x: Set output mode to HDMI or DVI according to EDID drm/bridge: sii902x: pixel clock unit is 10kHz instead of 1kHz drm/bridge: sii902x: SelectI2C_MUX
drm/bridge: sii902x: Implement HDMI audio supportTomi Valkeinen (1):
drm/bridge: sii902x: addinput_bus_flags.../bindings/display/bridge/sii902x.txt | 36 +- drivers/gpu/drm/bridge/Kconfig | 1 + drivers/gpu/drm/bridge/sii902x.c | 472 +++++++++++++++++- include/dt-bindings/sound/sii902x-audio.h | 11 + 4 files changed, 512 insertions(+), 8 deletions(-) create mode 100644 include/dt-bindings/sound/sii902x-audio.h
Signature · 7 lines
Regards, Laurent Pinchart _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
Quoted 29 lines · Laurent Pinchart
Hi Jyri,
Quoted 26 lines · Jyri Sarha
Changes since first version:
- Moved reviewed patches to front:
- drm/bridge: sii902x: addinput_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
- Added a new fix:
- drm/bridge: sii902x: SelectI2C_MUX
- Applied some review suggestions to
- drm/bridge: sii902x: Implement HDMI audio support
- use clock-names property to name mclk
- move comment describing added mutex to struct sii902x and improve it
- cleanupsii902x_mute()
- cleanupsii902x_select_mclk_div()
- fix condition for checkingENABLE_BITfromi2s_fifo_routinginsii902x_audio_codec_init()Still to do
- Agree on i2s wires to HDMI audio fifo routing in dts.
The current scheme is quite straight forward, but there is maybe there is even more straight forward solutions like:
audio-fifo-enable = <1 1 1 1>; audio-i2s-pin-to-fifo = <0 1 2 3>;
Quoted 8 lines
Quoted 3 lines
Meaning that all fifos are enabled and SD0 is routed to fifo 0, SD1 to fifo 1, etc. I am not sure if the channel swap functionality should show in dts binding.
Please forgive my lack of audio knowledge, but it this a system description that should be encoded in DT, or a policy that should be handled purely in software (either fully inside the kernel or with the help of userspace) ?
This property describes how many i2s wires are connected to sii902x and in what order, so I think it belongs to DTS.
One might of course wonder why anybody would put the i2s wires to any other order than 0 <-> 0, 1 <-> 1, 2 <-> 2, and 3 <-> 3, but then a again I've seen weirder board designs.
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
Laurent Pinchart
reviewerHi Jyri,
Quoted 32 lines · Jyri Sarha
Quoted 31 lines · Laurent Pinchart
Quoted 29 lines · Jyri Sarha
Changes since first version:
- Moved reviewed patches to front:
- drm/bridge: sii902x: addinput_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
- Added a new fix:
- drm/bridge: sii902x: SelectI2C_MUX
- Applied some review suggestions to
- drm/bridge: sii902x: Implement HDMI audio support
- use clock-names property to name mclk
- move comment describing added mutex to struct sii902x and improve it
- cleanupsii902x_mute()
- cleanupsii902x_select_mclk_div()
- fix condition for checkingENABLE_BITfromi2s_fifo_routinginsii902x_audio_codec_init()Still to do
- Agree on i2s wires to HDMI audio fifo routing in dts.
The current scheme is quite straight forward, but there is maybe there is even more straight forward solutions like:
audio-fifo-enable = <1 1 1 1>; audio-i2s-pin-to-fifo = <0 1 2 3>;Meaning that all fifos are enabled and SD0 is routed to fifo 0, SD1 to fifo 1, etc. I am not sure if the channel swap functionality should show in dts binding.
Quoted 7 lines
Quoted 4 lines
Please forgive my lack of audio knowledge, but it this a system description that should be encoded in DT, or a policy that should be handled purely in software (either fully inside the kernel or with the help of userspace) ?
This property describes how many i2s wires are connected to sii902x and in what order, so I think it belongs to DTS.
That would belong to DT, yes. We have solved a similar problem with CSI-2 receivers, where the number of data lanes and their mapping can often be configured. See the definition of the data-lanes property in Documentation/devicetree/bindings/media/video-interfaces.txt for more information. How about using similar bindings ? It would also solve Andrzej's concerns that you don't describe the audio connection in DT.
Quoted 3 lines
One might of course wonder why anybody would put the i2s wires to any other order than 0 <-> 0, 1 <-> 1, 2 <-> 2, and 3 <-> 3, but then a again I've seen weirder board designs.
It can be useful to simplify signal routing on the board.
Signature · 7 lines
Regards, Laurent Pinchart _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
Hi Jyri,
I also implemented HDMI audio support for sii902x to enable audio on a STM32 board. As you submitted your patches first, I will align on it. I had a first look at the current patch and I have some comments below. I will review more in details and make some tests, asap.
I agree with Laurent and Andrzej regarding the missing audio connection in DT. I would expect a subnode in DT describing the connection between the HDMI codec and the CPU DAI.
Typically:
port@1 {
reg = <1>;
codec_endpoint: endpoint {
remote-endpoint = <&cpu_dai_endpoint>;
};
};Then the hdmi codec get_dai_id callback can be implemented to check the endpoint used for audio.
mclk and i2s-fifo-routing properties are defined as optional properties in bindings.
However, these properties are required to initialize to the audio codec in the code.
- master clock:
The i2s link of sii902x can be used without master clock. So the master clock property has to be made actually optional. Probably, error code -ENOENT should be checked on devm_clk_get() call, to ignore mclk if the property is not defined.
- i2s-fifo-routing:
fifo mapping maybe set by default, according to i2s_fifo_defaults array if the property is not set.
Best regards
Olivier
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel