diff mbox

[v6,2/4] dt-bindings: drm/bridge: Document sn65dsi86 bridge bindings

Message ID 1526363564-13823-3-git-send-email-spanda@codeaurora.org (mailing list archive)
State New, archived
Headers show

Commit Message

Sandeep Panda May 15, 2018, 5:52 a.m. UTC 
Document the bindings used for the sn65dsi86 DSI to eDP bridge.

Changes in v1:
 - Rephrase the dt-binding descriptions to be more inline with existing
   bindings (Andrzej Hajda).
 - Add missing dt-binding that are parsed by corresponding driver
   (Andrzej Hajda).

Changes in v2:
 - Remove edp panel specific dt-binding entries. Only keep bridge
   specific entries (Sean Paul).
 - Remove custom-modes dt entry since its usage is removed from driver also (Sean Paul).
 - Remove is-pluggable dt entry since this will not be needed anymore (Sean Paul).

Changes in v3:
 - Remove irq-gpio dt entry and instead populate is an interrupt
   property (Rob Herring).

Changes in v4:
 - Add link to bridge chip datasheet (Stephen Boyd)
 - Add vpll and vcc regulator supply bindings (Stephen Boyd)
 - Add ref clk optional dt binding (Stephen Boyd)
 - Add gpio-controller optional dt binding (Stephen Boyd)

Changes in v5:
 - Use clock property to specify the input refclk (Stephen Boyd).
 - Update gpio cell and pwm cell numbers (Stephen Boyd).

Signed-off-by: Sandeep Panda <spanda@codeaurora.org>
---
 .../bindings/display/bridge/ti,sn65dsi86.txt       | 81 ++++++++++++++++++++++
 1 file changed, 81 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt

Comments

Stephen Boyd May 16, 2018, 4:27 p.m. UTC | #1 
Quoting Sandeep Panda (2018-05-14 22:52:42)
> diff --git a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt
> new file mode 100644
> index 0000000..b82bb56
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt
> @@ -0,0 +1,81 @@
> +Optional properties:
> +- interrupts: Specifier for the SN65DSI86 interrupt line.
> +- hpd-gpios: OF device-tree gpio specifications for HPD pin.
> +
> +- gpio-controller: Marks the device has a GPIO controller.
> +- #gpio-cells    : Should be two. The first cell is the pin number and
> +                   the second cell is used to specify flags.
> +                   See ../../gpio/gpio.txt for more information.
> +- #pwm-cells : Should be one. See ../../pwm/pwm.txt for description of
> +               the cell formats.
> +
> +- clock-names: should be "refclk"
> +- clocks: OF device-tree clock specification for refclk input. The reference

What is "OF device-tree .* specification" providing? This is all an OF
device-tree specification.

> +  clock rate must be 12 MHz, 19.2 MHz, 26 MHz, 27 MHz or 38.4 MHz.
> +
> +Required nodes:
> +
> +This device has two video ports. Their connections are modelled using the
> +OF graph bindings specified in Documentation/devicetree/bindings/graph.txt.
> +
> +- Video port 0 for DSI input
> +- Video port 1 for eDP output
> +
> +Example
> +-------
> +
> +edp-bridge@2d {
> +       compatible = "ti,sn65dsi86";
> +       #address-cells = <1>;
> +       #size-cells = <0>;
> +       reg = <0x2d>;
> +
> +       enable-gpios = <&msmgpio 33 GPIO_ACTIVE_HIGH>;
> +       interrupt-parent = <&gpio3>;
> +       interrupts = <4 IRQ_TYPE_EDGE_FALLING>;
> +
> +       vccio-supply = <&pm8916_l17>;
> +       vcca-supply = <&pm8916_l6>;
> +       vpll-supply = <&pm8916_l17>;
> +       vcc-supply = <&pm8916_l6>;
> +
> +       clock-names = "refclk";
> +       clocks = <&input_refclk>;
> +
> +       ports {
> +               #address-cells = <1>;
> +               #size-cells = <0>;
> +
> +               port@0 {
> +                       reg = <0>;
> +
> +                       edp_bridge_in: endpoint {
> +                               remote-endpoint = <&dsi_out>;

How do we know the number of lanes that are connected and if there's one
channel (A) or two channels (A and B)? Would there be two endpoints in
that case?

> +                       };
> +               };
> +
> +               port@1 {
> +                       reg = <1>;
> +
> +                       edp_bridge_out: endpoint {
> +                               remote-endpoint = <&edp_panel_in>;

The hardware looks to support some sort of lane renumbering scheme,
where the eDP logical lane 0 can be routed through a different pin than
MLP/N0, same for logical lane 1, etc. I don't have a use case for this
right now, but I hope that it could be added somewhere in the binding as
an optional property to describe this lane remapping feature. It also
has some sort of lane polarity inversion feature. Perhaps there needs to
be a lane-config property that does this remapping and inversion with
two cells.

	lane-config = <0 0>, /* Lane 0 logical is lane 0 phys (!inv) */
	              <1 0>, /* Lane 1 logical is lane 1 phys (!inv) */
		      <2 0>, /* Lane 2 logical is lane 2 phys (!inv) */
		      <3 0>; /* Lane 3 logical is lane 3 phys (!inv) */

Or

	lane-config = <2 1>, /* Lane 2 logical is lane 0 phys (inv) */
	              <1 0>, /* Lane 1 logical is lane 1 phys (!inv) */
		      <3 1>, /* Lane 3 logical is lane 2 phys (inv) */
		      <0 0>; /* Lane 0 logical is lane 3 phys (!inv) */


> +                       };
> +               };
> +       };
Sandeep Panda May 23, 2018, 3:47 p.m. UTC | #2 
On 2018-05-16 21:57, Stephen Boyd wrote:
> Quoting Sandeep Panda (2018-05-14 22:52:42)
>> diff --git 
>> a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt 
>> b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt
>> new file mode 100644
>> index 0000000..b82bb56
>> --- /dev/null
>> +++ 
>> b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt
>> @@ -0,0 +1,81 @@
>> +Optional properties:
>> +- interrupts: Specifier for the SN65DSI86 interrupt line.
>> +- hpd-gpios: OF device-tree gpio specifications for HPD pin.
>> +
>> +- gpio-controller: Marks the device has a GPIO controller.
>> +- #gpio-cells    : Should be two. The first cell is the pin number 
>> and
>> +                   the second cell is used to specify flags.
>> +                   See ../../gpio/gpio.txt for more information.
>> +- #pwm-cells : Should be one. See ../../pwm/pwm.txt for description 
>> of
>> +               the cell formats.
>> +
>> +- clock-names: should be "refclk"
>> +- clocks: OF device-tree clock specification for refclk input. The 
>> reference
> 
> What is "OF device-tree .* specification" providing? This is all an OF
> device-tree specification.
Ok. i will remove OF device tree, only keep specification for refclk 
input.
> 
>> +  clock rate must be 12 MHz, 19.2 MHz, 26 MHz, 27 MHz or 38.4 MHz.
>> +
>> +Required nodes:
>> +
>> +This device has two video ports. Their connections are modelled using 
>> the
>> +OF graph bindings specified in 
>> Documentation/devicetree/bindings/graph.txt.
>> +
>> +- Video port 0 for DSI input
>> +- Video port 1 for eDP output
>> +
>> +Example
>> +-------
>> +
>> +edp-bridge@2d {
>> +       compatible = "ti,sn65dsi86";
>> +       #address-cells = <1>;
>> +       #size-cells = <0>;
>> +       reg = <0x2d>;
>> +
>> +       enable-gpios = <&msmgpio 33 GPIO_ACTIVE_HIGH>;
>> +       interrupt-parent = <&gpio3>;
>> +       interrupts = <4 IRQ_TYPE_EDGE_FALLING>;
>> +
>> +       vccio-supply = <&pm8916_l17>;
>> +       vcca-supply = <&pm8916_l6>;
>> +       vpll-supply = <&pm8916_l17>;
>> +       vcc-supply = <&pm8916_l6>;
>> +
>> +       clock-names = "refclk";
>> +       clocks = <&input_refclk>;
>> +
>> +       ports {
>> +               #address-cells = <1>;
>> +               #size-cells = <0>;
>> +
>> +               port@0 {
>> +                       reg = <0>;
>> +
>> +                       edp_bridge_in: endpoint {
>> +                               remote-endpoint = <&dsi_out>;
> 
> How do we know the number of lanes that are connected and if there's 
> one
> channel (A) or two channels (A and B)? Would there be two endpoints in
> that case?
Currently we have hard coded the number of lanes to 4 in driver. And 
regarding supporting 2 DSI input channels dt-binding, we are planning to 
add those entries once we upload a patchset support dual dsi 
configuration in bridge driver.
> 
>> +                       };
>> +               };
>> +
>> +               port@1 {
>> +                       reg = <1>;
>> +
>> +                       edp_bridge_out: endpoint {
>> +                               remote-endpoint = <&edp_panel_in>;
> 
> The hardware looks to support some sort of lane renumbering scheme,
> where the eDP logical lane 0 can be routed through a different pin than
> MLP/N0, same for logical lane 1, etc. I don't have a use case for this
> right now, but I hope that it could be added somewhere in the binding 
> as
> an optional property to describe this lane remapping feature. It also
> has some sort of lane polarity inversion feature. Perhaps there needs 
> to
> be a lane-config property that does this remapping and inversion with
> two cells.
Ok. I will add this feature to dt binding.
> 
> 	lane-config = <0 0>, /* Lane 0 logical is lane 0 phys (!inv) */
> 	              <1 0>, /* Lane 1 logical is lane 1 phys (!inv) */
> 		      <2 0>, /* Lane 2 logical is lane 2 phys (!inv) */
> 		      <3 0>; /* Lane 3 logical is lane 3 phys (!inv) */
> 
> Or
> 
> 	lane-config = <2 1>, /* Lane 2 logical is lane 0 phys (inv) */
> 	              <1 0>, /* Lane 1 logical is lane 1 phys (!inv) */
> 		      <3 1>, /* Lane 3 logical is lane 2 phys (inv) */
> 		      <0 0>; /* Lane 0 logical is lane 3 phys (!inv) */
> 
> 
>> +                       };
>> +               };
>> +       };
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt
new file mode 100644
index 0000000..b82bb56
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt
@@ -0,0 +1,81 @@ 
+SN65DSI86 DSI to eDP bridge chip
+--------------------------------
+
+This is the binding for Texas Instruments SN65DSI86 bridge.
+http://www.ti.com/general/docs/lit/getliterature.tsp?genericPartNumber=sn65dsi86&fileType=pdf
+
+Required properties:
+- compatible: Must be "ti,sn65dsi86"
+- reg: i2c address of the chip, 0x2d as per datasheet
+- enable-gpios: OF device-tree gpio specification for bridge_en pin
+
+- vccio-supply: A 1.8V supply that powers up the digital IOs.
+- vpll-supply: A 1.8V supply that powers up the displayport PLL.
+- vcca-supply: A 1.2V supply that powers up the analog circuits.
+- vcc-supply: A 1.2V supply that powers up the digital core.
+
+Optional properties:
+- interrupts: Specifier for the SN65DSI86 interrupt line.
+- hpd-gpios: OF device-tree gpio specifications for HPD pin.
+
+- gpio-controller: Marks the device has a GPIO controller.
+- #gpio-cells    : Should be two. The first cell is the pin number and
+                   the second cell is used to specify flags.
+                   See ../../gpio/gpio.txt for more information.
+- #pwm-cells : Should be one. See ../../pwm/pwm.txt for description of
+               the cell formats.
+
+- clock-names: should be "refclk"
+- clocks: OF device-tree clock specification for refclk input. The reference
+  clock rate must be 12 MHz, 19.2 MHz, 26 MHz, 27 MHz or 38.4 MHz.
+
+Required nodes:
+
+This device has two video ports. Their connections are modelled using the
+OF graph bindings specified in Documentation/devicetree/bindings/graph.txt.
+
+- Video port 0 for DSI input
+- Video port 1 for eDP output
+
+Example
+-------
+
+edp-bridge@2d {
+	compatible = "ti,sn65dsi86";
+	#address-cells = <1>;
+	#size-cells = <0>;
+	reg = <0x2d>;
+
+	enable-gpios = <&msmgpio 33 GPIO_ACTIVE_HIGH>;
+	interrupt-parent = <&gpio3>;
+	interrupts = <4 IRQ_TYPE_EDGE_FALLING>;
+
+	vccio-supply = <&pm8916_l17>;
+	vcca-supply = <&pm8916_l6>;
+	vpll-supply = <&pm8916_l17>;
+	vcc-supply = <&pm8916_l6>;
+
+	clock-names = "refclk";
+	clocks = <&input_refclk>;
+
+	ports {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		port@0 {
+			reg = <0>;
+
+			edp_bridge_in: endpoint {
+				remote-endpoint = <&dsi_out>;
+			};
+		};
+
+		port@1 {
+			reg = <1>;
+
+			edp_bridge_out: endpoint {
+				remote-endpoint = <&edp_panel_in>;
+			};
+		};
+	};
+}