Re: [PATCH v2 1/2] drm/bridge: add Silicon Image SiI9234 driver

[Andrzej Hajda]

Hi Laurent,

On 05.09.2017 17:05, Laurent Pinchart wrote:
> Hi Maciej,
>
> On Tuesday, 5 September 2017 16:01:54 EEST Maciej Purski wrote:
>> Hi Laurent,
>>
>> Thank you for your reply. The problem was already discussed when adding
>> sil8620 driver. It can be solved later. I'm CC-ing Andrzej Hajda, as he
>> used to discuss it with you.
> I'm afraid it can't be solved later. DT bindings are supposed to be a stable 
> ABI, we can't merge a binding that we already know isn't correct.

The problem here is that bridge output is connected to micro-USB
connector(not HDMI !!!).
And currently there are no bindings for micro-USB connector. Creating
good bindings can be tricky
because micro-USB connector can be used to multiple functions:
- USB,
- charging,
- UART,
- MHL,
- audio,
- ...
And it can be controlled by different chips (MUICs), with different
capabilities.

I plan to send RFC in near future of such bindings (as I need them for
sii8620), but I guess it would require more review before it will be
accepted.

Returning to SiI9234, solution would be to add output port bindings to
SiI9234, but do not add them to dts. It will make bindings for Sii9234
complete.

Regards
Andrzej


>
>> https://patchwork.freedesktop.org/patch/114224/
>> https://lists.freedesktop.org/archives/dri-devel/2015-December/096756.html
>>
>> Regards,
>>
>>  Maciej
>>
>> On 31/08/17 15:30, Laurent Pinchart wrote:
>>> Hi Maciej,
>>>
>>> Thank you for the patch.
>>>
>>> On Thursday, 31 August 2017 15:27:13 EEST Maciej Purski wrote:
 SiI9234 transmitter converts eTMDS/HDMI signal to MHL 1.0.
 It is controlled via I2C bus. Its interaction with other
 devices in video pipeline is performed mainly on HW level.
 The only interaction it does on device driver level is
 filtering-out unsupported video modes, it exposes drm_bridge
 interface to perform this operation.

 This patch is based on the code refactored by Tomasz Stanislawski
 , which was initially developed by:
 Adam Hampson 
 Erik Gilling 
 Shankar Bandal 
 Dharam Kumar 

 Signed-off-by: Maciej Purski 
 ---
 Changes in v2:
 - use bulk_requlators instead of single one
 - substitute some of the magic values with macros
 - improve coding style
 - improved error handling in sii9234_probe()
 ---

   .../devicetree/bindings/display/bridge/sii9234.txt |  34 +
   drivers/gpu/drm/bridge/Kconfig |   8 +
   drivers/gpu/drm/bridge/Makefile|   1 +
   drivers/gpu/drm/bridge/sii9234.c   | 993
   ++
   4 files changed, 1036 insertions(+)
   create mode 100644

 Documentation/devicetree/bindings/display/bridge/sii9234.txt create mode
 100644 drivers/gpu/drm/bridge/sii9234.c

 diff --git a/Documentation/devicetree/bindings/display/bridge/sii9234.txt
 b/Documentation/devicetree/bindings/display/bridge/sii9234.txt new file
 mode 100644
 index 000..3ce7413
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/display/bridge/sii9234.txt
 @@ -0,0 +1,34 @@
 +Silicon Image SiI9234 HDMI/MHL bridge bindings
 +
 +Required properties:
 +  - compatible : "sil,sii9234".
 +  - reg : I2C address for TPI interface, use 0x39
 +  - avcc33-supply : MHL/USB Switch Supply Voltage (3.3V)
 +  - iovcc18-supply : I/O Supply Voltage (1.8V)
 +  - avcc12-supply : TMDS Analog Supply Voltage (1.2V)
 +  - cvcc12-supply : Digital Core Supply Voltage (1.2V)
 +  - interrupts, interrupt-parent: interrupt specifier of INT pin
 +  - reset-gpios: gpio specifier of RESET pin (active low)
 +  - video interfaces: Device node can contain video interface port
 +  node for HDMI encoder according to [1].
 +
 +[1]: Documentation/devicetree/bindings/media/video-interfaces.txt
>>> Doesn't this chip have two ports, one input connected to the SoC and one
>>> output connected to an HDMI connector ? If so there should be two ports in
>>> DT too.
>>>
 +Example:
 +  sii9234@39 {
 +  compatible = "sil,sii9234";
 +  reg = <0x39>;
 +  avcc33-supply = <>;
 +  iovcc18-supply = <>;
 +  avcc12-supply = <>;
 +  cvcc12-supply = <>;
 +  reset-gpios = < 4 GPIO_ACTIVE_LOW>;
 +  interrupt-parent = <>;
 +  interrupts = <5 IRQ_TYPE_LEVEL_HIGH>;
 +
 +  port {
 +  mhl_to_hdmi: endpoint {
 +  remote-endpoint = <_to_mhl>;
>>> It would be useful to include the remote DT nodes in the example too.
>>>
 +  };
 +  };
 +  };
>

___
dri-devel mailing list

Re: [PATCH v2 1/2] drm/bridge: add Silicon Image SiI9234 driver

[Laurent Pinchart]

Hi Maciej,

On Tuesday, 5 September 2017 16:01:54 EEST Maciej Purski wrote:
> Hi Laurent,
> 
> Thank you for your reply. The problem was already discussed when adding
> sil8620 driver. It can be solved later. I'm CC-ing Andrzej Hajda, as he
> used to discuss it with you.

I'm afraid it can't be solved later. DT bindings are supposed to be a stable 
ABI, we can't merge a binding that we already know isn't correct.

> https://patchwork.freedesktop.org/patch/114224/
> https://lists.freedesktop.org/archives/dri-devel/2015-December/096756.html
> 
> Regards,
> 
>   Maciej
> 
> On 31/08/17 15:30, Laurent Pinchart wrote:
> > Hi Maciej,
> > 
> > Thank you for the patch.
> > 
> > On Thursday, 31 August 2017 15:27:13 EEST Maciej Purski wrote:
> >> SiI9234 transmitter converts eTMDS/HDMI signal to MHL 1.0.
> >> It is controlled via I2C bus. Its interaction with other
> >> devices in video pipeline is performed mainly on HW level.
> >> The only interaction it does on device driver level is
> >> filtering-out unsupported video modes, it exposes drm_bridge
> >> interface to perform this operation.
> >> 
> >> This patch is based on the code refactored by Tomasz Stanislawski
> >> , which was initially developed by:
> >> Adam Hampson 
> >> Erik Gilling 
> >> Shankar Bandal 
> >> Dharam Kumar 
> >> 
> >> Signed-off-by: Maciej Purski 
> >> ---
> >> Changes in v2:
> >> - use bulk_requlators instead of single one
> >> - substitute some of the magic values with macros
> >> - improve coding style
> >> - improved error handling in sii9234_probe()
> >> ---
> >> 
> >>   .../devicetree/bindings/display/bridge/sii9234.txt |  34 +
> >>   drivers/gpu/drm/bridge/Kconfig |   8 +
> >>   drivers/gpu/drm/bridge/Makefile|   1 +
> >>   drivers/gpu/drm/bridge/sii9234.c   | 993
> >>   ++
> >>   4 files changed, 1036 insertions(+)
> >>   create mode 100644
> >> 
> >> Documentation/devicetree/bindings/display/bridge/sii9234.txt create mode
> >> 100644 drivers/gpu/drm/bridge/sii9234.c
> >> 
> >> diff --git a/Documentation/devicetree/bindings/display/bridge/sii9234.txt
> >> b/Documentation/devicetree/bindings/display/bridge/sii9234.txt new file
> >> mode 100644
> >> index 000..3ce7413
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/display/bridge/sii9234.txt
> >> @@ -0,0 +1,34 @@
> >> +Silicon Image SiI9234 HDMI/MHL bridge bindings
> >> +
> >> +Required properties:
> >> +  - compatible : "sil,sii9234".
> >> +  - reg : I2C address for TPI interface, use 0x39
> >> +  - avcc33-supply : MHL/USB Switch Supply Voltage (3.3V)
> >> +  - iovcc18-supply : I/O Supply Voltage (1.8V)
> >> +  - avcc12-supply : TMDS Analog Supply Voltage (1.2V)
> >> +  - cvcc12-supply : Digital Core Supply Voltage (1.2V)
> >> +  - interrupts, interrupt-parent: interrupt specifier of INT pin
> >> +  - reset-gpios: gpio specifier of RESET pin (active low)
> >> +  - video interfaces: Device node can contain video interface port
> >> +  node for HDMI encoder according to [1].
> >> +
> >> +[1]: Documentation/devicetree/bindings/media/video-interfaces.txt
> > 
> > Doesn't this chip have two ports, one input connected to the SoC and one
> > output connected to an HDMI connector ? If so there should be two ports in
> > DT too.
> > 
> >> +Example:
> >> +  sii9234@39 {
> >> +  compatible = "sil,sii9234";
> >> +  reg = <0x39>;
> >> +  avcc33-supply = <>;
> >> +  iovcc18-supply = <>;
> >> +  avcc12-supply = <>;
> >> +  cvcc12-supply = <>;
> >> +  reset-gpios = < 4 GPIO_ACTIVE_LOW>;
> >> +  interrupt-parent = <>;
> >> +  interrupts = <5 IRQ_TYPE_LEVEL_HIGH>;
> >> +
> >> +  port {
> >> +  mhl_to_hdmi: endpoint {
> >> +  remote-endpoint = <_to_mhl>;
> > 
> > It would be useful to include the remote DT nodes in the example too.
> > 
> >> +  };
> >> +  };
> >> +  };


-- 
Regards,

Laurent Pinchart

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

Re: [PATCH v2 1/2] drm/bridge: add Silicon Image SiI9234 driver

[Maciej Purski]

Hi Laurent,

Thank you for your reply. The problem was already discussed when adding sil8620 
driver.
It can be solved later. I'm CC-ing Andrzej Hajda, as he used to discuss it with 
you.

https://patchwork.freedesktop.org/patch/114224/
https://lists.freedesktop.org/archives/dri-devel/2015-December/096756.html

Regards,

Maciej

On 31/08/17 15:30, Laurent Pinchart wrote:


Hi Maciej,

Thank you for the patch.

On Thursday, 31 August 2017 15:27:13 EEST Maciej Purski wrote:

SiI9234 transmitter converts eTMDS/HDMI signal to MHL 1.0.
It is controlled via I2C bus. Its interaction with other
devices in video pipeline is performed mainly on HW level.
The only interaction it does on device driver level is
filtering-out unsupported video modes, it exposes drm_bridge
interface to perform this operation.

This patch is based on the code refactored by Tomasz Stanislawski
, which was initially developed by:
Adam Hampson 
Erik Gilling 
Shankar Bandal 
Dharam Kumar 

Signed-off-by: Maciej Purski 
---
Changes in v2:
- use bulk_requlators instead of single one
- substitute some of the magic values with macros
- improve coding style
- improved error handling in sii9234_probe()
---
  .../devicetree/bindings/display/bridge/sii9234.txt |  34 +
  drivers/gpu/drm/bridge/Kconfig |   8 +
  drivers/gpu/drm/bridge/Makefile|   1 +
  drivers/gpu/drm/bridge/sii9234.c   | 993 ++
  4 files changed, 1036 insertions(+)
  create mode 100644
Documentation/devicetree/bindings/display/bridge/sii9234.txt create mode
100644 drivers/gpu/drm/bridge/sii9234.c

diff --git a/Documentation/devicetree/bindings/display/bridge/sii9234.txt
b/Documentation/devicetree/bindings/display/bridge/sii9234.txt new file
mode 100644
index 000..3ce7413
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/bridge/sii9234.txt
@@ -0,0 +1,34 @@
+Silicon Image SiI9234 HDMI/MHL bridge bindings
+
+Required properties:
+   - compatible : "sil,sii9234".
+   - reg : I2C address for TPI interface, use 0x39
+   - avcc33-supply : MHL/USB Switch Supply Voltage (3.3V)
+   - iovcc18-supply : I/O Supply Voltage (1.8V)
+   - avcc12-supply : TMDS Analog Supply Voltage (1.2V)
+   - cvcc12-supply : Digital Core Supply Voltage (1.2V)
+   - interrupts, interrupt-parent: interrupt specifier of INT pin
+   - reset-gpios: gpio specifier of RESET pin (active low)
+   - video interfaces: Device node can contain video interface port
+   node for HDMI encoder according to [1].
+
+[1]: Documentation/devicetree/bindings/media/video-interfaces.txt

Doesn't this chip have two ports, one input connected to the SoC and one
output connected to an HDMI connector ? If so there should be two ports in DT
too.


+Example:
+   sii9234@39 {
+   compatible = "sil,sii9234";
+   reg = <0x39>;
+   avcc33-supply = <>;
+   iovcc18-supply = <>;
+   avcc12-supply = <>;
+   cvcc12-supply = <>;
+   reset-gpios = < 4 GPIO_ACTIVE_LOW>;
+   interrupt-parent = <>;
+   interrupts = <5 IRQ_TYPE_LEVEL_HIGH>;
+
+   port {
+   mhl_to_hdmi: endpoint {
+   remote-endpoint = <_to_mhl>;

It would be useful to include the remote DT nodes in the example too.


+   };
+   };
+   };


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

Re: [PATCH v2 1/2] drm/bridge: add Silicon Image SiI9234 driver

[Laurent Pinchart]

Hi Maciej,

Thank you for the patch.

On Thursday, 31 August 2017 15:27:13 EEST Maciej Purski wrote:
> SiI9234 transmitter converts eTMDS/HDMI signal to MHL 1.0.
> It is controlled via I2C bus. Its interaction with other
> devices in video pipeline is performed mainly on HW level.
> The only interaction it does on device driver level is
> filtering-out unsupported video modes, it exposes drm_bridge
> interface to perform this operation.
> 
> This patch is based on the code refactored by Tomasz Stanislawski
> , which was initially developed by:
> Adam Hampson 
> Erik Gilling 
> Shankar Bandal 
> Dharam Kumar 
> 
> Signed-off-by: Maciej Purski 
> ---
> Changes in v2:
> - use bulk_requlators instead of single one
> - substitute some of the magic values with macros
> - improve coding style
> - improved error handling in sii9234_probe()
> ---
>  .../devicetree/bindings/display/bridge/sii9234.txt |  34 +
>  drivers/gpu/drm/bridge/Kconfig |   8 +
>  drivers/gpu/drm/bridge/Makefile|   1 +
>  drivers/gpu/drm/bridge/sii9234.c   | 993 ++
>  4 files changed, 1036 insertions(+)
>  create mode 100644
> Documentation/devicetree/bindings/display/bridge/sii9234.txt create mode
> 100644 drivers/gpu/drm/bridge/sii9234.c
> 
> diff --git a/Documentation/devicetree/bindings/display/bridge/sii9234.txt
> b/Documentation/devicetree/bindings/display/bridge/sii9234.txt new file
> mode 100644
> index 000..3ce7413
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bridge/sii9234.txt
> @@ -0,0 +1,34 @@
> +Silicon Image SiI9234 HDMI/MHL bridge bindings
> +
> +Required properties:
> + - compatible : "sil,sii9234".
> + - reg : I2C address for TPI interface, use 0x39
> + - avcc33-supply : MHL/USB Switch Supply Voltage (3.3V)
> + - iovcc18-supply : I/O Supply Voltage (1.8V)
> + - avcc12-supply : TMDS Analog Supply Voltage (1.2V)
> + - cvcc12-supply : Digital Core Supply Voltage (1.2V)
> + - interrupts, interrupt-parent: interrupt specifier of INT pin
> + - reset-gpios: gpio specifier of RESET pin (active low)
> + - video interfaces: Device node can contain video interface port
> + node for HDMI encoder according to [1].
> +
> +[1]: Documentation/devicetree/bindings/media/video-interfaces.txt

Doesn't this chip have two ports, one input connected to the SoC and one 
output connected to an HDMI connector ? If so there should be two ports in DT 
too.

> +Example:
> + sii9234@39 {
> + compatible = "sil,sii9234";
> + reg = <0x39>;
> + avcc33-supply = <>;
> + iovcc18-supply = <>;
> + avcc12-supply = <>;
> + cvcc12-supply = <>;
> + reset-gpios = < 4 GPIO_ACTIVE_LOW>;
> + interrupt-parent = <>;
> + interrupts = <5 IRQ_TYPE_LEVEL_HIGH>;
> +
> + port {
> + mhl_to_hdmi: endpoint {
> + remote-endpoint = <_to_mhl>;

It would be useful to include the remote DT nodes in the example too.

> + };
> + };
> + };

-- 
Regards,

Laurent Pinchart

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

[PATCH v2 1/2] drm/bridge: add Silicon Image SiI9234 driver

[Maciej Purski]

SiI9234 transmitter converts eTMDS/HDMI signal to MHL 1.0.
It is controlled via I2C bus. Its interaction with other
devices in video pipeline is performed mainly on HW level.
The only interaction it does on device driver level is
filtering-out unsupported video modes, it exposes drm_bridge
interface to perform this operation.

This patch is based on the code refactored by Tomasz Stanislawski
, which was initially developed by:
Adam Hampson 
Erik Gilling 
Shankar Bandal 
Dharam Kumar 

Signed-off-by: Maciej Purski 
---
Changes in v2:
- use bulk_requlators instead of single one
- substitute some of the magic values with macros
- improve coding style
- improved error handling in sii9234_probe()
---
 .../devicetree/bindings/display/bridge/sii9234.txt |  34 +
 drivers/gpu/drm/bridge/Kconfig |   8 +
 drivers/gpu/drm/bridge/Makefile|   1 +
 drivers/gpu/drm/bridge/sii9234.c   | 993 +
 4 files changed, 1036 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/bridge/sii9234.txt
 create mode 100644 drivers/gpu/drm/bridge/sii9234.c

diff --git a/Documentation/devicetree/bindings/display/bridge/sii9234.txt 
b/Documentation/devicetree/bindings/display/bridge/sii9234.txt
new file mode 100644
index 000..3ce7413
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/bridge/sii9234.txt
@@ -0,0 +1,34 @@
+Silicon Image SiI9234 HDMI/MHL bridge bindings
+
+Required properties:
+   - compatible : "sil,sii9234".
+   - reg : I2C address for TPI interface, use 0x39
+   - avcc33-supply : MHL/USB Switch Supply Voltage (3.3V)
+   - iovcc18-supply : I/O Supply Voltage (1.8V)
+   - avcc12-supply : TMDS Analog Supply Voltage (1.2V)
+   - cvcc12-supply : Digital Core Supply Voltage (1.2V)
+   - interrupts, interrupt-parent: interrupt specifier of INT pin
+   - reset-gpios: gpio specifier of RESET pin (active low)
+   - video interfaces: Device node can contain video interface port
+   node for HDMI encoder according to [1].
+
+[1]: Documentation/devicetree/bindings/media/video-interfaces.txt
+
+Example:
+   sii9234@39 {
+   compatible = "sil,sii9234";
+   reg = <0x39>;
+   avcc33-supply = <>;
+   iovcc18-supply = <>;
+   avcc12-supply = <>;
+   cvcc12-supply = <>;
+   reset-gpios = < 4 GPIO_ACTIVE_LOW>;
+   interrupt-parent = <>;
+   interrupts = <5 IRQ_TYPE_LEVEL_HIGH>;
+
+   port {
+   mhl_to_hdmi: endpoint {
+   remote-endpoint = <_to_mhl>;
+   };
+   };
+   };
diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index adf9ae0..9dba16fd 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -84,6 +84,14 @@ config DRM_SII902X
---help---
  Silicon Image sii902x bridge chip driver.
 
+config DRM_SII9234
+   tristate "Silicon Image SII9234 HDMI/MHL bridge"
+   depends on OF
+   ---help---
+ Say Y here if you want support for the MHL interface.
+ It is an I2C driver, that detects connection of MHL bridge
+ and starts encapsulation of HDMI signal.
+
 config DRM_TOSHIBA_TC358767
tristate "Toshiba TC358767 eDP bridge"
depends on OF
diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
index defcf1e..e3d5eb0 100644
--- a/drivers/gpu/drm/bridge/Makefile
+++ b/drivers/gpu/drm/bridge/Makefile
@@ -6,6 +6,7 @@ obj-$(CONFIG_DRM_NXP_PTN3460) += nxp-ptn3460.o
 obj-$(CONFIG_DRM_PARADE_PS8622) += parade-ps8622.o
 obj-$(CONFIG_DRM_SIL_SII8620) += sil-sii8620.o
 obj-$(CONFIG_DRM_SII902X) += sii902x.o
+obj-$(CONFIG_DRM_SII9234) += sii9234.o
 obj-$(CONFIG_DRM_TOSHIBA_TC358767) += tc358767.o
 obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix/
 obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511/
diff --git a/drivers/gpu/drm/bridge/sii9234.c b/drivers/gpu/drm/bridge/sii9234.c
new file mode 100644
index 000..b70d69a
--- /dev/null
+++ b/drivers/gpu/drm/bridge/sii9234.c
@@ -0,0 +1,993 @@
+/*
+ * Copyright (C) 2017 Samsung Electronics
+ *
+ * Authors:
+ *Tomasz Stanislawski 
+ *Maciej Purski 
+ *
+ * Based on sii9234 driver created by:
+ *Adam Hampson 
+ *Erik Gilling 
+ *Shankar Bandal 
+ *Dharam Kumar 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *