linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 1/2] dt-binding: Add DSI/LVDS TC358775 bridge bindings
@ 2020-07-01  7:03 Vinay Simha BN
  2020-07-01  7:03 ` [PATCH v5 2/2] display/drm/bridge: TC358775 DSI/LVDS driver Vinay Simha BN
  2020-07-01  9:19 ` [PATCH v5 1/2] dt-binding: Add DSI/LVDS TC358775 bridge bindings Sam Ravnborg
  0 siblings, 2 replies; 6+ messages in thread
From: Vinay Simha BN @ 2020-07-01  7:03 UTC (permalink / raw)
  To: narmstrong, Ken-Sue.Tan, Chris.Cheng, Bhushan.Patel
  Cc: Vinay Simha BN, David Airlie, Daniel Vetter, Rob Herring,
	open list:DRM DRIVERS,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list

This driver is tested with two panels with Apq8016-IFC6309 board
https://www.inforcecomputing.com/products/single-board-computers-sbc/qualcomm-snapdragon-410-inforce-6309-micro-sbc

1. 1366x768@60 auo,b101xtn01 data-mapping = "jeida-24"
2. 800x480@60 innolux,at070tn92 data-mapping = "vesa-24"

Signed-off-by: Vinay Simha BN <simhavcs@gmail.com>

---
v1:
 Initial version wast .txt file

v2:
 From txt to yaml file format

v3:
* Andrzej Hajda review comments incorporated
  dual port lvds implemented

* Laurent Pinchart review comments incorporated
  dsi lanes property removed and it is dynamically
  picked from the dsi ports
  VESA/JEIDA format picked from panel-lvds dts
---
 .../display/bridge/toshiba,tc358775.yaml      | 204 ++++++++++++++++++
 1 file changed, 204 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/bridge/toshiba,tc358775.yaml

diff --git a/Documentation/devicetree/bindings/display/bridge/toshiba,tc358775.yaml b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358775.yaml
new file mode 100644
index 000000000000..ec53d62d408b
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358775.yaml
@@ -0,0 +1,204 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/bridge/toshiba,tc358775.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Toshiba TC358775 DSI to LVDS bridge bindings
+
+maintainers:
+ - Vinay Simha BN <simhavcs@gmail.com>
+
+description: |
+ This binding supports DSI to LVDS bridge TC358775
+
+properties:
+  compatible:
+    const: toshiba,tc358775
+
+  reg:
+    maxItems: 1
+    description: i2c address of the bridge, 0x0f
+
+  vdd-supply:
+    maxItems: 1
+    description:  1.2V LVDS Power Supply
+
+  vddio-supply:
+    maxItems: 1
+    description: 1.8V IO Power Supply
+
+  stby-gpios:
+    maxItems: 1
+    description: Standby pin, Low active
+
+  reset-gpios:
+    maxItems: 1
+    description: Hardware reset, Low active
+
+  ports:
+    type: object
+    description:
+      A node containing input and output port nodes with endpoint definitions
+      as documented in
+      Documentation/devicetree/bindings/media/video-interfaces.txt
+    properties:
+      "#address-cells":
+        const: 1
+
+      "#size-cells":
+        const: 0
+
+      port@0:
+        type: object
+        description: |
+          DSI Input. The remote endpoint phandle should be a
+          reference to a valid mipi_dsi_host device node.
+
+      port@1:
+        type: object
+        description: |
+          Video port for LVDS output (panel or connector).
+
+      port@2:
+        type: object
+        description: |
+          Video port for Dual link LVDS output (panel or connector).
+
+    required:
+      - port@0
+      - port@1
+
+required:
+ - compatible
+ - reg
+ - vdd-supply
+ - vddio-supply
+ - stby-gpios
+ - reset-gpios
+ - ports
+
+examples:
+ - |
+    #include <dt-bindings/gpio/gpio.h>
+
+    i2c@78b8000 {
+        /* On High speed expansion */
+        label = "HS-I2C2";
+        reg = <0x078b8000 0x500>;
+        clock-frequency = <400000>; /* fastmode operation */
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        tc_bridge: bridge@f {
+            compatible = "toshiba,tc358775";
+            reg = <0x0f>;
+
+            vdd-supply = <&pm8916_l2>;
+            vddio-supply = <&pm8916_l6>;
+
+            stby-gpios = <&msmgpio 99 GPIO_ACTIVE_LOW>;
+            reset-gpios = <&msmgpio 72 GPIO_ACTIVE_LOW>;
+
+            ports {
+                #address-cells = <1>;
+                #size-cells = <0>;
+
+                port@0 {
+                    reg = <0>;
+                    d2l_in_test: endpoint {
+                        remote-endpoint = <&dsi0_out>;
+                    };
+                };
+
+                port@1 {
+                    reg = <1>;
+                    lvds_out: endpoint {
+                        remote-endpoint = <&panel_in>;
+                    };
+                };
+            };
+        };
+    };
+
+    dsi@1a98000 {
+        reg = <0x1a98000 0x25c>;
+        reg-names = "dsi_ctrl";
+
+        ports {
+            #address-cells = <1>;
+            #size-cells = <0>;
+            port@1 {
+                reg = <1>;
+                dsi0_out: endpoint {
+                    remote-endpoint = <&d2l_in_test>;
+                        data-lanes = <0 1 2 3>;
+                };
+             };
+         };
+     };
+
+ - |
+    i2c@78b8000 {
+        /* On High speed expansion */
+        label = "HS-I2C2";
+        reg = <0x078b8000 0x500>;
+        clock-frequency = <400000>; /* fastmode operation */
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        tc_bridge_dual: bridge@f {
+            compatible = "toshiba,tc358775";
+            reg = <0x0f>;
+
+            vdd-supply = <&pm8916_l2>;
+            vddio-supply = <&pm8916_l6>;
+
+            stby-gpios = <&msmgpio 99 GPIO_ACTIVE_LOW>;
+            reset-gpios = <&msmgpio 72 GPIO_ACTIVE_LOW>;
+
+            ports {
+                #address-cells = <1>;
+                #size-cells = <0>;
+
+                port@0 {
+                    reg = <0>;
+                    d2l_in_dual: endpoint {
+                        remote-endpoint = <&dsi0_out_dual>;
+                    };
+                };
+
+                port@1 {
+                    reg = <1>;
+                    lvds0_out: endpoint {
+                        remote-endpoint = <&panel_in0>;
+                    };
+                };
+
+                port@2 {
+                    reg = <2>;
+                    lvds1_out: endpoint {
+                        remote-endpoint = <&panel_in1>;
+                    };
+                };
+            };
+        };
+    };
+
+    dsi@1a98000 {
+        reg = <0x1a98000 0x25c>;
+        reg-names = "dsi_ctrl";
+
+        ports {
+            #address-cells = <1>;
+            #size-cells = <0>;
+            port@1 {
+                reg = <1>;
+                dsi0_out_dual: endpoint {
+                    remote-endpoint = <&d2l_in_dual>;
+                        data-lanes = <0 1 2 3>;
+                };
+             };
+         };
+     };
+...
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v5 2/2] display/drm/bridge: TC358775 DSI/LVDS driver
  2020-07-01  7:03 [PATCH v5 1/2] dt-binding: Add DSI/LVDS TC358775 bridge bindings Vinay Simha BN
@ 2020-07-01  7:03 ` Vinay Simha BN
  2020-07-01  9:35   ` Sam Ravnborg
  2020-07-01  9:19 ` [PATCH v5 1/2] dt-binding: Add DSI/LVDS TC358775 bridge bindings Sam Ravnborg
  1 sibling, 1 reply; 6+ messages in thread
From: Vinay Simha BN @ 2020-07-01  7:03 UTC (permalink / raw)
  To: narmstrong, Ken-Sue.Tan, Chris.Cheng, Bhushan.Patel
  Cc: Vinay Simha BN, Andrzej Hajda, Laurent Pinchart, Jonas Karlman,
	Jernej Skrabec, David Airlie, Daniel Vetter, open list,
	open list:DRM DRIVERS

Signed-off-by: Vinay Simha BN <simhavcs@gmail.com>

---
v1:
 Initial version

v2:
* Andrzej Hajda review comments incorporated
  SPDX identifier
  development debug removed
  alphabetic order headers
  u32 instead of unit32_t
  magic numbers to macros for CLRSI and mux registers
  ignored return value

* Laurent Pinchart review comments incorporated
  mdelay to usleep_range
  bus_formats added

v3:
* Andrzej Hajda review comments incorporated
  drm_connector_status removed
  u32 rev removed and local variabl is used
  regulator enable disable with proper orders and delays
  as per the spec
  devm_drm_panel_bridge_add method used instead of panel
  description modified
  dual port implemented

v4:
* Sam Ravnborg review comments incorporated
  panel->connector_type removed

* Reported-by: kernel test robot <lkp@intel.com>
  parse_dt to static function
  removed the if (endpoint), since data-lanes has to be
  present for dsi dts ports

v5:
  ~vsdelay dynamic value set based on the
  calculation of dsi speed, output speed, blanking
---
 drivers/gpu/drm/bridge/Kconfig    |  10 +
 drivers/gpu/drm/bridge/Makefile   |   1 +
 drivers/gpu/drm/bridge/tc358775.c | 735 ++++++++++++++++++++++++++++++
 3 files changed, 746 insertions(+)
 create mode 100644 drivers/gpu/drm/bridge/tc358775.c

diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index 43271c21d3fc..084e9853944a 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -181,6 +181,16 @@ config DRM_TOSHIBA_TC358768
 	help
 	  Toshiba TC358768AXBG/TC358778XBG DSI bridge chip driver.
 
+config DRM_TOSHIBA_TC358775
+        tristate "Toshiba TC358775 LVDS bridge"
+        depends on OF
+        select DRM_KMS_HELPER
+        select REGMAP_I2C
+        select DRM_PANEL
+	select DRM_MIPI_DSI
+        ---help---
+          Toshiba TC358775 LVDS bridge chip driver.
+
 config DRM_TI_TFP410
 	tristate "TI TFP410 DVI/HDMI bridge"
 	depends on OF
diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
index d63d4b7e4347..23c770b3bfe4 100644
--- a/drivers/gpu/drm/bridge/Makefile
+++ b/drivers/gpu/drm/bridge/Makefile
@@ -15,6 +15,7 @@ obj-$(CONFIG_DRM_THINE_THC63LVD1024) += thc63lvd1024.o
 obj-$(CONFIG_DRM_TOSHIBA_TC358764) += tc358764.o
 obj-$(CONFIG_DRM_TOSHIBA_TC358767) += tc358767.o
 obj-$(CONFIG_DRM_TOSHIBA_TC358768) += tc358768.o
+obj-$(CONFIG_DRM_TOSHIBA_TC358775) += tc358775.o
 obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511/
 obj-$(CONFIG_DRM_TI_SN65DSI86) += ti-sn65dsi86.o
 obj-$(CONFIG_DRM_TI_TFP410) += ti-tfp410.o
diff --git a/drivers/gpu/drm/bridge/tc358775.c b/drivers/gpu/drm/bridge/tc358775.c
new file mode 100644
index 000000000000..e580c544190c
--- /dev/null
+++ b/drivers/gpu/drm/bridge/tc358775.c
@@ -0,0 +1,735 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * tc358775 DSI to LVDS bridge driver
+ *
+ * Copyright (C) 2020 SMART Wireless Computing
+ * Author: Vinay Simha BN <simhavcs@gmail.com>
+ *
+ */
+/* #define DEBUG */
+#include <linux/clk.h>
+#include <linux/device.h>
+#include <linux/gpio/consumer.h>
+#include <linux/i2c.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/regulator/consumer.h>
+#include <linux/slab.h>
+
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_bridge.h>
+#include <drm/drm_crtc_helper.h>
+#include <drm/drm_dp_helper.h>
+#include <drm/drm_mipi_dsi.h>
+#include <drm/drm_of.h>
+#include <drm/drm_panel.h>
+#include <drm/drm_probe_helper.h>
+
+#define FLD_MASK(start, end)    (((1 << ((start) - (end) + 1)) - 1) << (end))
+#define FLD_VAL(val, start, end) (((val) << (end)) & FLD_MASK(start, end))
+
+/* Registers */
+
+/* DSI D-PHY Layer Registers */
+#define D0W_DPHYCONTTX  0x0004  /* Data Lane 0 DPHY Tx Control */
+#define CLW_DPHYCONTRX  0x0020  /* Clock Lane DPHY Rx Control */
+#define D0W_DPHYCONTRX  0x0024  /* Data Lane 0 DPHY Rx Control */
+#define D1W_DPHYCONTRX  0x0028  /* Data Lane 1 DPHY Rx Control */
+#define D2W_DPHYCONTRX  0x002C  /* Data Lane 2 DPHY Rx Control */
+#define D3W_DPHYCONTRX  0x0030  /* Data Lane 3 DPHY Rx Control */
+#define COM_DPHYCONTRX  0x0038  /* DPHY Rx Common Control */
+#define CLW_CNTRL       0x0040  /* Clock Lane Control */
+#define D0W_CNTRL       0x0044  /* Data Lane 0 Control */
+#define D1W_CNTRL       0x0048  /* Data Lane 1 Control */
+#define D2W_CNTRL       0x004C  /* Data Lane 2 Control */
+#define D3W_CNTRL       0x0050  /* Data Lane 3 Control */
+#define DFTMODE_CNTRL   0x0054  /* DFT Mode Control */
+
+/* DSI PPI Layer Registers */
+#define PPI_STARTPPI    0x0104  /* START control bit of PPI-TX function. */
+#define PPI_START_FUNCTION      1
+
+#define PPI_BUSYPPI     0x0108
+#define PPI_LINEINITCNT 0x0110  /* Line Initialization Wait Counter  */
+#define PPI_LPTXTIMECNT 0x0114
+#define PPI_LANEENABLE  0x0134  /* Enables each lane at the PPI layer. */
+#define PPI_TX_RX_TA    0x013C  /* DSI Bus Turn Around timing parameters */
+
+/* Analog timer function enable */
+#define PPI_CLS_ATMR    0x0140  /* Delay for Clock Lane in LPRX  */
+#define PPI_D0S_ATMR    0x0144  /* Delay for Data Lane 0 in LPRX */
+#define PPI_D1S_ATMR    0x0148  /* Delay for Data Lane 1 in LPRX */
+#define PPI_D2S_ATMR    0x014C  /* Delay for Data Lane 2 in LPRX */
+#define PPI_D3S_ATMR    0x0150  /* Delay for Data Lane 3 in LPRX */
+
+#define PPI_D0S_CLRSIPOCOUNT    0x0164  /* For lane 0 */
+#define PPI_D1S_CLRSIPOCOUNT    0x0168  /* For lane 1 */
+#define PPI_D2S_CLRSIPOCOUNT    0x016C  /* For lane 2 */
+#define PPI_D3S_CLRSIPOCOUNT    0x0170  /* For lane 3 */
+
+#define CLS_PRE         0x0180  /* Digital Counter inside of PHY IO */
+#define D0S_PRE         0x0184  /* Digital Counter inside of PHY IO */
+#define D1S_PRE         0x0188  /* Digital Counter inside of PHY IO */
+#define D2S_PRE         0x018C  /* Digital Counter inside of PHY IO */
+#define D3S_PRE         0x0190  /* Digital Counter inside of PHY IO */
+#define CLS_PREP        0x01A0  /* Digital Counter inside of PHY IO */
+#define D0S_PREP        0x01A4  /* Digital Counter inside of PHY IO */
+#define D1S_PREP        0x01A8  /* Digital Counter inside of PHY IO */
+#define D2S_PREP        0x01AC  /* Digital Counter inside of PHY IO */
+#define D3S_PREP        0x01B0  /* Digital Counter inside of PHY IO */
+#define CLS_ZERO        0x01C0  /* Digital Counter inside of PHY IO */
+#define D0S_ZERO        0x01C4  /* Digital Counter inside of PHY IO */
+#define D1S_ZERO        0x01C8  /* Digital Counter inside of PHY IO */
+#define D2S_ZERO        0x01CC  /* Digital Counter inside of PHY IO */
+#define D3S_ZERO        0x01D0  /* Digital Counter inside of PHY IO */
+
+#define PPI_CLRFLG      0x01E0  /* PRE Counters has reached set values */
+#define PPI_CLRSIPO     0x01E4  /* Clear SIPO values, Slave mode use only. */
+#define HSTIMEOUT       0x01F0  /* HS Rx Time Out Counter */
+#define HSTIMEOUTENABLE 0x01F4  /* Enable HS Rx Time Out Counter */
+#define DSI_STARTDSI    0x0204  /* START control bit of DSI-TX function */
+#define DSI_RX_START	1
+
+#define DSI_BUSYDSI     0x0208
+#define DSI_LANEENABLE  0x0210  /* Enables each lane at the Protocol layer. */
+#define DSI_LANESTATUS0 0x0214  /* Displays lane is in HS RX mode. */
+#define DSI_LANESTATUS1 0x0218  /* Displays lane is in ULPS or STOP state */
+
+#define DSI_INTSTATUS   0x0220  /* Interrupt Status */
+#define DSI_INTMASK     0x0224  /* Interrupt Mask */
+#define DSI_INTCLR      0x0228  /* Interrupt Clear */
+#define DSI_LPTXTO      0x0230  /* Low Power Tx Time Out Counter */
+
+#define DSIERRCNT       0x0300  /* DSI Error Count */
+#define APLCTRL         0x0400  /* Application Layer Control */
+#define RDPKTLN         0x0404  /* Command Read Packet Length */
+
+#define VPCTRL          0x0450  /* Video Path Control */
+#define HTIM1           0x0454  /* Horizontal Timing Control 1 */
+#define HTIM2           0x0458  /* Horizontal Timing Control 2 */
+#define VTIM1           0x045C  /* Vertical Timing Control 1 */
+#define VTIM2           0x0460  /* Vertical Timing Control 2 */
+#define VFUEN           0x0464  /* Video Frame Timing Update Enable */
+#define VFUEN_EN	BIT(0)  /* Upload Enable */
+
+/* Mux Input Select for LVDS LINK Input */
+#define LV_MX0003        0x0480  /* Bit 0 to 3 */
+#define LV_MX0407        0x0484  /* Bit 4 to 7 */
+#define LV_MX0811        0x0488  /* Bit 8 to 11 */
+#define LV_MX1215        0x048C  /* Bit 12 to 15 */
+#define LV_MX1619        0x0490  /* Bit 16 to 19 */
+#define LV_MX2023        0x0494  /* Bit 20 to 23 */
+#define LV_MX2427        0x0498  /* Bit 24 to 27 */
+#define LV_MX(b0, b1, b2, b3)	(FLD_VAL(b0, 4, 0) | FLD_VAL(b1, 12, 8) | \
+				FLD_VAL(b2, 20, 16) | FLD_VAL(b3, 28, 24))
+
+/* Input bit numbers used in mux registers */
+enum {
+	LVI_R0,
+	LVI_R1,
+	LVI_R2,
+	LVI_R3,
+	LVI_R4,
+	LVI_R5,
+	LVI_R6,
+	LVI_R7,
+	LVI_G0,
+	LVI_G1,
+	LVI_G2,
+	LVI_G3,
+	LVI_G4,
+	LVI_G5,
+	LVI_G6,
+	LVI_G7,
+	LVI_B0,
+	LVI_B1,
+	LVI_B2,
+	LVI_B3,
+	LVI_B4,
+	LVI_B5,
+	LVI_B6,
+	LVI_B7,
+	LVI_HS,
+	LVI_VS,
+	LVI_DE,
+	LVI_L0
+};
+
+#define LVCFG           0x049C  /* LVDS Configuration  */
+#define LVPHY0          0x04A0  /* LVDS PHY 0 */
+#define LV_PHY0_RST(v)          FLD_VAL(v, 22, 22) /* PHY reset */
+#define LV_PHY0_IS(v)           FLD_VAL(v, 15, 14)
+#define LV_PHY0_ND(v)           FLD_VAL(v, 4, 0) /* Frequency range select */
+#define LV_PHY0_PRBS_ON(v)      FLD_VAL(v, 20, 16) /* Clock/Data Flag pins */
+
+#define LVPHY1          0x04A4  /* LVDS PHY 1 */
+#define SYSSTAT         0x0500  /* System Status  */
+#define SYSRST          0x0504  /* System Reset  */
+
+#define SYS_RST_I2CS	BIT(0) /* Reset I2C-Slave controller */
+#define SYS_RST_I2CM	BIT(1) /* Reset I2C-Master controller */
+#define SYS_RST_LCD	BIT(2) /* Reset LCD controller */
+#define SYS_RST_BM	BIT(3) /* Reset Bus Management controller */
+#define SYS_RST_DSIRX	BIT(4) /* Reset DSI-RX and App controller */
+#define SYS_RST_REG	BIT(5) /* Reset Register module */
+
+/* GPIO Registers */
+#define GPIOC           0x0520  /* GPIO Control  */
+#define GPIOO           0x0524  /* GPIO Output  */
+#define GPIOI           0x0528  /* GPIO Input  */
+
+/* I2C Registers */
+#define I2CTIMCTRL      0x0540  /* I2C IF Timing and Enable Control */
+#define I2CMADDR        0x0544  /* I2C Master Addressing */
+#define WDATAQ          0x0548  /* Write Data Queue */
+#define RDATAQ          0x054C  /* Read Data Queue */
+
+/* Chip ID and Revision ID Register */
+#define IDREG           0x0580
+
+#define LPX_PERIOD		4
+#define TTA_GET			0x40000
+#define TTA_SURE		6
+
+#define TC358775XBG_ID  0x00007500
+
+/* Debug Registers */
+#define DEBUG00         0x05A0  /* Debug */
+#define DEBUG01         0x05A4  /* LVDS Data */
+
+#define DSI_CLEN_BIT		BIT(0)
+#define DIVIDE_BY_3		3 /* PCLK=DCLK/3 */
+#define DIVIDE_BY_6		6 /* PCLK=DCLK/6 */
+#define LVCFG_LVEN_BIT		BIT(0)
+
+#define L0EN BIT(1)
+
+#define TC358775_VPCTRL_VSDELAY__MASK	0x3FF00000
+#define TC358775_VPCTRL_VSDELAY__SHIFT	20
+static inline u32 TC358775_VPCTRL_VSDELAY(uint32_t val)
+{
+	return ((val) << TC358775_VPCTRL_VSDELAY__SHIFT) &
+			TC358775_VPCTRL_VSDELAY__MASK;
+}
+
+#define TC358775_VPCTRL_OPXLFMT__MASK	0x00000100
+#define TC358775_VPCTRL_OPXLFMT__SHIFT	8
+static inline u32 TC358775_VPCTRL_OPXLFMT(uint32_t val)
+{
+	return ((val) << TC358775_VPCTRL_OPXLFMT__SHIFT) &
+			TC358775_VPCTRL_OPXLFMT__MASK;
+}
+
+#define TC358775_VPCTRL_MSF__MASK	0x00000001
+#define TC358775_VPCTRL_MSF__SHIFT	0
+static inline u32 TC358775_VPCTRL_MSF(uint32_t val)
+{
+	return ((val) << TC358775_VPCTRL_MSF__SHIFT) &
+			TC358775_VPCTRL_MSF__MASK;
+}
+
+#define TC358775_LVCFG_PCLKDIV__MASK	0x000000f0
+#define TC358775_LVCFG_PCLKDIV__SHIFT	4
+static inline u32 TC358775_LVCFG_PCLKDIV(uint32_t val)
+{
+	return ((val) << TC358775_LVCFG_PCLKDIV__SHIFT) &
+			TC358775_LVCFG_PCLKDIV__MASK;
+}
+
+#define TC358775_LVCFG_LVDLINK__MASK                         0x00000002
+#define TC358775_LVCFG_LVDLINK__SHIFT                        0
+static inline u32 TC358775_LVCFG_LVDLINK(uint32_t val)
+{
+	return ((val) << TC358775_LVCFG_LVDLINK__SHIFT) &
+			TC358775_LVCFG_LVDLINK__MASK;
+}
+
+enum tc358775_ports {
+	TC358775_DSI_IN,
+	TC358775_LVDS_OUT0,
+	TC358775_LVDS_OUT1,
+};
+
+struct tc_data {
+	struct i2c_client	*i2c;
+	struct device		*dev;
+
+	struct drm_bridge	bridge;
+	struct drm_bridge	*panel_bridge;
+
+	struct device_node *host_node;
+	struct mipi_dsi_device *dsi;
+	u8 num_dsi_lanes;
+
+	struct regulator	*vdd;
+	struct regulator	*vddio;
+	struct gpio_desc	*reset_gpio;
+	struct gpio_desc	*stby_gpio;
+	bool                    dual_link; /* single-link or dual-link */
+};
+
+static inline struct tc_data *bridge_to_tc(struct drm_bridge *b)
+{
+	return container_of(b, struct tc_data, bridge);
+}
+
+static void tc_bridge_pre_enable(struct drm_bridge *bridge)
+{
+	struct tc_data *tc = bridge_to_tc(bridge);
+	struct device *dev = &tc->dsi->dev;
+	int ret;
+
+	ret = regulator_enable(tc->vddio);
+	if (ret < 0)
+		dev_err(dev, "regulator vddio enable failed, %d\n", ret);
+	usleep_range(10000, 11000);
+
+	ret = regulator_enable(tc->vdd);
+	if (ret < 0)
+		dev_err(dev, "regulator vdd enable failed, %d\n", ret);
+	usleep_range(10000, 11000);
+
+	gpiod_set_value(tc->stby_gpio, 0);
+	usleep_range(10000, 11000);
+
+	gpiod_set_value(tc->reset_gpio, 0);
+	usleep_range(10, 20);
+}
+
+static void tc_bridge_post_disable(struct drm_bridge *bridge)
+{
+	struct tc_data *tc = bridge_to_tc(bridge);
+	struct device *dev = &tc->dsi->dev;
+	int ret;
+
+	ret = regulator_disable(tc->vddio);
+	if (ret < 0)
+		dev_err(dev, "regulator vddio disable failed, %d\n", ret);
+	usleep_range(10000, 11000);
+
+	ret = regulator_disable(tc->vdd);
+	if (ret < 0)
+		dev_err(dev, "regulator vdd disable failed, %d\n", ret);
+	usleep_range(10000, 11000);
+
+	gpiod_set_value(tc->stby_gpio, 1);
+	usleep_range(10000, 11000);
+
+	gpiod_set_value(tc->reset_gpio, 1);
+	usleep_range(10, 20);
+}
+
+static u32 d2l_read(struct i2c_client *i2c, u16 reg)
+{
+	int ret;
+	u8 val[4];
+	u8 addr[] = {(reg >> 8) & 0xff, reg & 0xff};
+	u32 value;
+
+	ret = i2c_master_send(i2c, addr, sizeof(reg));
+	if (ret < 0)
+		goto fail;
+
+	ret = i2c_master_recv(i2c, val, sizeof(value));
+	if (ret < 0)
+		goto fail;
+
+	value = val[3] << 24 | val[2] << 16 | val[1] << 8 | val[0];
+
+	pr_debug("d2l: I2C : reg:%04x value:%08x\n", reg, value);
+
+	return value;
+
+fail:
+	dev_err(&i2c->dev, "Error %d reading from subaddress 0x%x\n",
+		ret, reg);
+	return 0;
+}
+
+static void d2l_write(struct i2c_client *i2c, u16 reg, u32 val)
+{
+	u8 buf[6];
+	int ret;
+
+	buf[0] = reg >> 8;
+	buf[1] = reg & 0xFF;
+	buf[2] = (val >> 0) & 0xFF;
+	buf[3] = (val >> 8) & 0xFF;
+	buf[4] = (val >> 16) & 0xFF;
+	buf[5] = (val >> 24) & 0xFF;
+
+	ret = i2c_master_send(i2c, buf, ARRAY_SIZE(buf));
+	if (ret < 0)
+		dev_err(&i2c->dev, "Error %d writing to subaddress 0x%x\n",
+			ret, reg);
+}
+
+/* helper function to access bus_formats */
+static struct drm_connector *get_connector(struct drm_encoder *encoder)
+{
+	struct drm_device *dev = encoder->dev;
+	struct drm_connector *connector;
+
+	list_for_each_entry(connector, &dev->mode_config.connector_list, head)
+		if (connector->encoder == encoder)
+			return connector;
+
+	return NULL;
+}
+
+static void tc_bridge_enable(struct drm_bridge *bridge)
+{
+	struct tc_data *tc = bridge_to_tc(bridge);
+	u32 hbpr, hpw, htime1, hfpr, hsize, htime2;
+	u32 vbpr, vpw, vtime1, vfpr, vsize, vtime2;
+	u32 val = 0, val_lvcfg = 0;
+	u16 dsiclk, clkdiv, byteclk, t1, t2, t3, vsdelay;
+	u8 link, bpc;
+	u16 bus_formats;
+	struct drm_display_mode *mode;
+	struct drm_connector *connector = get_connector(bridge->encoder);
+
+	mode = &bridge->encoder->crtc->state->adjusted_mode;
+
+	hbpr = mode->htotal - mode->hsync_end;
+	hpw  = mode->hsync_end - mode->hsync_start;
+	vbpr = mode->vtotal - mode->vsync_end;
+	vpw  = mode->vsync_end - mode->vsync_start;
+
+	htime1 = (hbpr << 16) + hpw;
+	vtime1 = (vbpr << 16) + vpw;
+
+	hfpr = mode->hsync_start - mode->hdisplay;
+	hsize = mode->hdisplay;
+	vfpr = mode->vsync_start - mode->vdisplay;
+	vsize = mode->vdisplay;
+
+	htime2 = (hfpr << 16) + hsize;
+	vtime2 = (vfpr << 16) + vsize;
+
+	val = d2l_read(tc->i2c, IDREG);
+
+	dev_info(tc->dev, "DSI2LVDS Chip ID.%02x Revision ID. %02x **\n",
+		 (val >> 8) & 0xFF, val & 0xFF);
+
+	d2l_write(tc->i2c, SYSRST, SYS_RST_REG | SYS_RST_DSIRX | SYS_RST_BM |
+		  SYS_RST_LCD | SYS_RST_I2CM | SYS_RST_I2CS);
+	usleep_range(30000, 40000);
+
+	d2l_write(tc->i2c, PPI_TX_RX_TA, TTA_GET | TTA_SURE);
+	d2l_write(tc->i2c, PPI_LPTXTIMECNT, LPX_PERIOD);
+	d2l_write(tc->i2c, PPI_D0S_CLRSIPOCOUNT, 3);
+	d2l_write(tc->i2c, PPI_D1S_CLRSIPOCOUNT, 3);
+	d2l_write(tc->i2c, PPI_D2S_CLRSIPOCOUNT, 3);
+	d2l_write(tc->i2c, PPI_D3S_CLRSIPOCOUNT, 3);
+
+	val = ((L0EN << tc->num_dsi_lanes) - L0EN) | DSI_CLEN_BIT;
+	d2l_write(tc->i2c, PPI_LANEENABLE, val);
+	d2l_write(tc->i2c, DSI_LANEENABLE, val);
+
+	d2l_write(tc->i2c, PPI_STARTPPI, PPI_START_FUNCTION);
+	d2l_write(tc->i2c, DSI_STARTDSI, DSI_RX_START);
+
+	val_lvcfg = LVCFG_LVEN_BIT;
+	if (tc->dual_link) {
+		link = 2;
+		val_lvcfg |= TC358775_LVCFG_LVDLINK(1);
+		val_lvcfg |= TC358775_LVCFG_PCLKDIV(DIVIDE_BY_6);
+	} else {
+		link = 1;
+		val_lvcfg |= TC358775_LVCFG_PCLKDIV(DIVIDE_BY_3);
+	}
+
+	bus_formats = connector->display_info.bus_formats[0];
+	dev_dbg(tc->dev, "bus_formats : %04x\n", bus_formats);
+
+	if (bus_formats == MEDIA_BUS_FMT_RGB888_1X7X4_SPWG ||
+	    bus_formats == MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA) {
+		/* RGB888 */
+		bpc = 8;
+		val = TC358775_VPCTRL_OPXLFMT(1);
+	} else {
+		/* RGB666 */
+		bpc = 6;
+		val |= TC358775_VPCTRL_MSF(1);
+	}
+
+	dsiclk = mode->crtc_clock * 3 * bpc / tc->num_dsi_lanes / 1000;
+	clkdiv = dsiclk / DIVIDE_BY_3 * link;
+	byteclk = dsiclk / 4;
+	t1 = hsize * (bpc * 3 / 8) / tc->num_dsi_lanes;
+	t2 = ((100000 / clkdiv)) * (hsize + hbpr + hpw + hfpr) / 1000;
+	t3 = ((t2 * byteclk) / 100) - (hsize * (bpc * 3 / 8) /
+		tc->num_dsi_lanes);
+
+	vsdelay = (clkdiv * (t1 + t3) / byteclk) - hbpr - hpw - hsize;
+
+	val |= TC358775_VPCTRL_VSDELAY(vsdelay);
+	d2l_write(tc->i2c, VPCTRL, val);
+
+	d2l_write(tc->i2c, HTIM1, htime1);
+	d2l_write(tc->i2c, VTIM1, vtime1);
+	d2l_write(tc->i2c, HTIM2, htime2);
+	d2l_write(tc->i2c, VTIM2, vtime2);
+
+	d2l_write(tc->i2c, VFUEN, VFUEN_EN);
+	d2l_write(tc->i2c, SYSRST, SYS_RST_LCD);
+	d2l_write(tc->i2c, LVPHY0, LV_PHY0_PRBS_ON(4) | LV_PHY0_ND(6));
+
+	/* default jeida-24 */
+	if (bus_formats == MEDIA_BUS_FMT_RGB888_1X7X4_SPWG) {
+		/* vesa-24 */
+		d2l_write(tc->i2c, LV_MX0003, LV_MX(LVI_R0, LVI_R1, LVI_R2, LVI_R3));
+		d2l_write(tc->i2c, LV_MX0407, LV_MX(LVI_R4, LVI_R7, LVI_R5, LVI_G0));
+		d2l_write(tc->i2c, LV_MX0811, LV_MX(LVI_G1, LVI_G2, LVI_G6, LVI_G7));
+		d2l_write(tc->i2c, LV_MX1215, LV_MX(LVI_G3, LVI_G4, LVI_G5, LVI_B0));
+		d2l_write(tc->i2c, LV_MX1619, LV_MX(LVI_B6, LVI_B7, LVI_B1, LVI_B2));
+		d2l_write(tc->i2c, LV_MX2023, LV_MX(LVI_B3, LVI_B4, LVI_B5, LVI_L0));
+		d2l_write(tc->i2c, LV_MX2427, LV_MX(LVI_HS, LVI_VS, LVI_DE, LVI_R6));
+	}
+
+	if (bus_formats == MEDIA_BUS_FMT_RGB666_1X7X3_SPWG) {
+		/* jeida-18 */
+		d2l_write(tc->i2c, LV_MX0003, LV_MX(LVI_R0, LVI_R1, LVI_R2, LVI_R3));
+		d2l_write(tc->i2c, LV_MX0407, LV_MX(LVI_R4, LVI_L0, LVI_R5, LVI_G0));
+		d2l_write(tc->i2c, LV_MX0811, LV_MX(LVI_G1, LVI_G2, LVI_L0, LVI_L0));
+		d2l_write(tc->i2c, LV_MX1215, LV_MX(LVI_G3, LVI_G4, LVI_G5, LVI_B0));
+		d2l_write(tc->i2c, LV_MX1619, LV_MX(LVI_L0, LVI_L0, LVI_B1, LVI_B2));
+		d2l_write(tc->i2c, LV_MX2023, LV_MX(LVI_B3, LVI_B4, LVI_B5, LVI_L0));
+		d2l_write(tc->i2c, LV_MX2427, LV_MX(LVI_HS, LVI_VS, LVI_DE, LVI_L0));
+	}
+
+	d2l_write(tc->i2c, VFUEN, VFUEN_EN);
+	d2l_write(tc->i2c, LVCFG, val_lvcfg);
+}
+
+static int tc_mode_valid(struct drm_bridge *bridge,
+			 const struct drm_display_mode *mode)
+{
+	struct tc_data *tc = bridge_to_tc(bridge);
+
+	/* Maximum pixel clock speed 135MHz-single-link/270MHz-dual-link */
+	if ((mode->clock > 135000 && tc->dual_link) ||
+	    (mode->clock > 270000 && tc->dual_link))
+		return MODE_CLOCK_HIGH;
+
+	return MODE_OK;
+}
+
+static int tc358775_parse_dt(struct device_node *np, struct tc_data *tc)
+{
+	struct device_node *endpoint;
+	struct device_node *parent;
+	struct device_node *remote;
+	struct property *prop;
+	int len;
+
+	endpoint = of_graph_get_endpoint_by_regs(tc->dev->of_node,
+						 TC358775_DSI_IN, -1);
+	if (endpoint) {
+		/* dsi0_out node */
+		parent = of_graph_get_remote_port_parent(endpoint);
+		of_node_put(endpoint);
+		if (parent) {
+			/* dsi0 port 1 */
+			endpoint = of_graph_get_endpoint_by_regs(parent, 1, -1);
+			of_node_put(parent);
+			prop = of_find_property(endpoint, "data-lanes",
+						&len);
+			of_node_put(endpoint);
+			if (!prop) {
+				dev_err(tc->dev,
+					"failed to find data lane\n");
+				return -EPROBE_DEFER;
+			}
+		}
+	}
+
+	tc->num_dsi_lanes = len / sizeof(u32);
+
+	if (tc->num_dsi_lanes < 1 || tc->num_dsi_lanes > 4)
+		return -EINVAL;
+
+	tc->host_node = of_graph_get_remote_node(np, 0, 0);
+	if (!tc->host_node)
+		return -ENODEV;
+
+	of_node_put(tc->host_node);
+
+	endpoint = of_graph_get_endpoint_by_regs(tc->dev->of_node,
+						 TC358775_LVDS_OUT1, -1);
+	if (endpoint) {
+		remote = of_graph_get_remote_port_parent(endpoint);
+		of_node_put(endpoint);
+
+		if (remote) {
+			if (of_device_is_available(remote))
+				tc->dual_link = true;
+			of_node_put(remote);
+		}
+	}
+
+	dev_dbg(tc->dev, "no.of dsi lanes: %d\n", tc->num_dsi_lanes);
+	dev_dbg(tc->dev, "operating in %s-link mode\n",
+		tc->dual_link ? "dual" : "single");
+
+	return 0;
+}
+
+static int tc_bridge_attach(struct drm_bridge *bridge,
+			    enum drm_bridge_attach_flags flags)
+{
+	struct tc_data *tc = bridge_to_tc(bridge);
+	struct device *dev = &tc->i2c->dev;
+	struct mipi_dsi_host *host;
+	struct mipi_dsi_device *dsi;
+	int ret;
+
+	const struct mipi_dsi_device_info info = { .type = "tc358775",
+							.channel = 0,
+							.node = NULL,
+						};
+
+	host = of_find_mipi_dsi_host_by_node(tc->host_node);
+	if (!host) {
+		dev_err(dev, "failed to find dsi host\n");
+		return -EPROBE_DEFER;
+	}
+
+	dsi = mipi_dsi_device_register_full(host, &info);
+	if (IS_ERR(dsi)) {
+		dev_err(dev, "failed to create dsi device\n");
+		ret = PTR_ERR(dsi);
+		goto err_dsi_device;
+	}
+
+	tc->dsi = dsi;
+
+	dsi->lanes = tc->num_dsi_lanes;
+	dsi->format = MIPI_DSI_FMT_RGB888;
+	dsi->mode_flags = MIPI_DSI_MODE_VIDEO;
+
+	ret = mipi_dsi_attach(dsi);
+	if (ret < 0) {
+		dev_err(dev, "failed to attach dsi to host\n");
+		goto err_dsi_attach;
+	}
+
+	/* Attach the panel-bridge to the dsi bridge */
+	return drm_bridge_attach(bridge->encoder, tc->panel_bridge,
+				 &tc->bridge, flags);
+err_dsi_attach:
+	mipi_dsi_device_unregister(dsi);
+err_dsi_device:
+	return ret;
+}
+
+static const struct drm_bridge_funcs tc_bridge_funcs = {
+	.attach = tc_bridge_attach,
+	.pre_enable = tc_bridge_pre_enable,
+	.enable = tc_bridge_enable,
+	.mode_valid = tc_mode_valid,
+	.post_disable = tc_bridge_post_disable,
+};
+
+static int tc_probe(struct i2c_client *client, const struct i2c_device_id *id)
+{
+	struct device *dev = &client->dev;
+	struct drm_panel *panel;
+	struct tc_data *tc;
+	int ret;
+
+	tc = devm_kzalloc(dev, sizeof(*tc), GFP_KERNEL);
+	if (!tc)
+		return -ENOMEM;
+
+	tc->dev = dev;
+	tc->i2c = client;
+
+	ret = drm_of_find_panel_or_bridge(dev->of_node, TC358775_LVDS_OUT0,
+					  0, &panel, NULL);
+	if (ret < 0)
+		return ret;
+	if (!panel)
+		return -ENODEV;
+
+	tc->panel_bridge = devm_drm_panel_bridge_add(dev, panel);
+	if (IS_ERR(tc->panel_bridge))
+		return PTR_ERR(tc->panel_bridge);
+
+	ret = tc358775_parse_dt(dev->of_node, tc);
+	if (ret)
+		return ret;
+
+	tc->vddio = devm_regulator_get(dev, "vddio-supply");
+	if (IS_ERR(tc->vddio)) {
+		ret = PTR_ERR(tc->vddio);
+		dev_err(dev, "vddio-supply not found\n");
+		return ret;
+	}
+
+	tc->vdd = devm_regulator_get(dev, "vdd-supply");
+	if (IS_ERR(tc->vdd)) {
+		ret = PTR_ERR(tc->vddio);
+		dev_err(dev, "vdd-supply not found\n");
+		return ret;
+	}
+
+	tc->stby_gpio = devm_gpiod_get(dev, "stby", GPIOD_OUT_HIGH);
+	if (IS_ERR(tc->stby_gpio)) {
+		ret = PTR_ERR(tc->stby_gpio);
+		dev_err(dev, "cannot get stby-gpio %d\n", ret);
+		return ret;
+	}
+
+	tc->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(tc->reset_gpio)) {
+		ret = PTR_ERR(tc->reset_gpio);
+		dev_err(dev, "cannot get reset-gpios %d\n", ret);
+		return ret;
+	}
+
+	tc->bridge.funcs = &tc_bridge_funcs;
+	tc->bridge.of_node = dev->of_node;
+	drm_bridge_add(&tc->bridge);
+
+	i2c_set_clientdata(client, tc);
+
+	return 0;
+}
+
+static int tc_remove(struct i2c_client *client)
+{
+	struct tc_data *tc = i2c_get_clientdata(client);
+
+	drm_bridge_remove(&tc->bridge);
+
+	return 0;
+}
+
+static const struct i2c_device_id tc358775_i2c_ids[] = {
+	{ "tc358775", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, tc358775_i2c_ids);
+
+static const struct of_device_id tc358775_of_ids[] = {
+	{ .compatible = "toshiba,tc358775", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, tc358775_of_ids);
+
+static struct i2c_driver tc358775_driver = {
+	.driver = {
+		.name = "tc358775",
+		.of_match_table = tc358775_of_ids,
+	},
+	.id_table = tc358775_i2c_ids,
+	.probe = tc_probe,
+	.remove	= tc_remove,
+};
+module_i2c_driver(tc358775_driver);
+
+MODULE_AUTHOR("Vinay Simha BN <simhavcs@gmail.com>");
+MODULE_DESCRIPTION("TC358775 DSI/LVDS bridge driver");
+MODULE_LICENSE("GPL v2");
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v5 1/2] dt-binding: Add DSI/LVDS TC358775 bridge bindings
  2020-07-01  7:03 [PATCH v5 1/2] dt-binding: Add DSI/LVDS TC358775 bridge bindings Vinay Simha BN
  2020-07-01  7:03 ` [PATCH v5 2/2] display/drm/bridge: TC358775 DSI/LVDS driver Vinay Simha BN
@ 2020-07-01  9:19 ` Sam Ravnborg
  2020-07-01  9:44   ` Vinay Simha B N
  1 sibling, 1 reply; 6+ messages in thread
From: Sam Ravnborg @ 2020-07-01  9:19 UTC (permalink / raw)
  To: Vinay Simha BN
  Cc: narmstrong, Ken-Sue.Tan, Chris.Cheng, Bhushan.Patel,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	David Airlie, open list, open list:DRM DRIVERS, Rob Herring

Hi Vinay.

On Wed, Jul 01, 2020 at 12:33:36PM +0530, Vinay Simha BN wrote:
> This driver is tested with two panels with Apq8016-IFC6309 board
> https://www.inforcecomputing.com/products/single-board-computers-sbc/qualcomm-snapdragon-410-inforce-6309-micro-sbc
> 
> 1. 1366x768@60 auo,b101xtn01 data-mapping = "jeida-24"
> 2. 800x480@60 innolux,at070tn92 data-mapping = "vesa-24"
> 
> Signed-off-by: Vinay Simha BN <simhavcs@gmail.com>
> 
> ---
> v1:
>  Initial version wast .txt file
> 
> v2:
>  From txt to yaml file format
> 
> v3:
> * Andrzej Hajda review comments incorporated
>   dual port lvds implemented
> 
> * Laurent Pinchart review comments incorporated
>   dsi lanes property removed and it is dynamically
>   picked from the dsi ports
>   VESA/JEIDA format picked from panel-lvds dts
> ---
>  .../display/bridge/toshiba,tc358775.yaml      | 204 ++++++++++++++++++
>  1 file changed, 204 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/bridge/toshiba,tc358775.yaml
> 
> diff --git a/Documentation/devicetree/bindings/display/bridge/toshiba,tc358775.yaml b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358775.yaml
> new file mode 100644
> index 000000000000..ec53d62d408b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358775.yaml
> @@ -0,0 +1,204 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/bridge/toshiba,tc358775.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Toshiba TC358775 DSI to LVDS bridge bindings
> +
> +maintainers:
> + - Vinay Simha BN <simhavcs@gmail.com>
> +
> +description: |
> + This binding supports DSI to LVDS bridge TC358775
> +
> +properties:
> +  compatible:
> +    const: toshiba,tc358775
> +
> +  reg:
> +    maxItems: 1
> +    description: i2c address of the bridge, 0x0f
> +
> +  vdd-supply:
> +    maxItems: 1
> +    description:  1.2V LVDS Power Supply
> +
> +  vddio-supply:
> +    maxItems: 1
> +    description: 1.8V IO Power Supply
> +
> +  stby-gpios:
> +    maxItems: 1
> +    description: Standby pin, Low active
> +
> +  reset-gpios:
> +    maxItems: 1
> +    description: Hardware reset, Low active
> +
> +  ports:
> +    type: object
> +    description:
> +      A node containing input and output port nodes with endpoint definitions
> +      as documented in
> +      Documentation/devicetree/bindings/media/video-interfaces.txt
> +    properties:
> +      "#address-cells":
> +        const: 1
> +
> +      "#size-cells":
> +        const: 0
> +
> +      port@0:
> +        type: object
> +        description: |
> +          DSI Input. The remote endpoint phandle should be a
> +          reference to a valid mipi_dsi_host device node.
> +
> +      port@1:
> +        type: object
> +        description: |
> +          Video port for LVDS output (panel or connector).
> +
> +      port@2:
> +        type: object
> +        description: |
> +          Video port for Dual link LVDS output (panel or connector).
> +
> +    required:
> +      - port@0
> +      - port@1
> +
> +required:
> + - compatible
> + - reg
> + - vdd-supply
> + - vddio-supply
> + - stby-gpios
> + - reset-gpios
> + - ports
> +
> +examples:
> + - |
> +    #include <dt-bindings/gpio/gpio.h>
> +
> +    i2c@78b8000 {
> +        /* On High speed expansion */
> +        label = "HS-I2C2";
> +        reg = <0x078b8000 0x500>;
> +        clock-frequency = <400000>; /* fastmode operation */
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        tc_bridge: bridge@f {
> +            compatible = "toshiba,tc358775";
> +            reg = <0x0f>;
> +
> +            vdd-supply = <&pm8916_l2>;
> +            vddio-supply = <&pm8916_l6>;
> +
> +            stby-gpios = <&msmgpio 99 GPIO_ACTIVE_LOW>;
> +            reset-gpios = <&msmgpio 72 GPIO_ACTIVE_LOW>;
> +
> +            ports {
> +                #address-cells = <1>;
> +                #size-cells = <0>;
> +
> +                port@0 {
> +                    reg = <0>;
> +                    d2l_in_test: endpoint {
> +                        remote-endpoint = <&dsi0_out>;
> +                    };
> +                };
> +
> +                port@1 {
> +                    reg = <1>;
> +                    lvds_out: endpoint {
> +                        remote-endpoint = <&panel_in>;
> +                    };
> +                };
> +            };
> +        };
> +    };
> +
> +    dsi@1a98000 {
> +        reg = <0x1a98000 0x25c>;
> +        reg-names = "dsi_ctrl";
> +
> +        ports {
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +            port@1 {
> +                reg = <1>;
> +                dsi0_out: endpoint {
> +                    remote-endpoint = <&d2l_in_test>;
> +                        data-lanes = <0 1 2 3>;
> +                };
> +             };
> +         };
> +     };
> +
Fix indent. One of the '}' is indented one space too much.


> + - |
> +    i2c@78b8000 {
> +        /* On High speed expansion */
> +        label = "HS-I2C2";
> +        reg = <0x078b8000 0x500>;
> +        clock-frequency = <400000>; /* fastmode operation */
> +        #address-cells = <1>;
> +        #size-cells = <0>;
What is the purpose with the second example?
Does it explain the binding better in some way?

The examples shall help understanding the binding, not help
understanding the HW. The latter is the role of the binding.


> +
> +        tc_bridge_dual: bridge@f {
> +            compatible = "toshiba,tc358775";
> +            reg = <0x0f>;
> +
> +            vdd-supply = <&pm8916_l2>;
> +            vddio-supply = <&pm8916_l6>;
> +
> +            stby-gpios = <&msmgpio 99 GPIO_ACTIVE_LOW>;
> +            reset-gpios = <&msmgpio 72 GPIO_ACTIVE_LOW>;
> +
> +            ports {
> +                #address-cells = <1>;
> +                #size-cells = <0>;
> +
> +                port@0 {
> +                    reg = <0>;
> +                    d2l_in_dual: endpoint {
> +                        remote-endpoint = <&dsi0_out_dual>;
> +                    };
> +                };
> +
> +                port@1 {
> +                    reg = <1>;
> +                    lvds0_out: endpoint {
> +                        remote-endpoint = <&panel_in0>;
> +                    };
> +                };
> +
> +                port@2 {
> +                    reg = <2>;
> +                    lvds1_out: endpoint {
> +                        remote-endpoint = <&panel_in1>;
> +                    };
> +                };
> +            };
> +        };
> +    };
> +
> +    dsi@1a98000 {
> +        reg = <0x1a98000 0x25c>;
> +        reg-names = "dsi_ctrl";
> +
> +        ports {
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +            port@1 {
> +                reg = <1>;
> +                dsi0_out_dual: endpoint {
> +                    remote-endpoint = <&d2l_in_dual>;
> +                        data-lanes = <0 1 2 3>;
> +                };
> +             };
> +         };
> +     };
If this example is kept then fix indent of '}'.

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v5 2/2] display/drm/bridge: TC358775 DSI/LVDS driver
  2020-07-01  7:03 ` [PATCH v5 2/2] display/drm/bridge: TC358775 DSI/LVDS driver Vinay Simha BN
@ 2020-07-01  9:35   ` Sam Ravnborg
  0 siblings, 0 replies; 6+ messages in thread
From: Sam Ravnborg @ 2020-07-01  9:35 UTC (permalink / raw)
  To: Vinay Simha BN
  Cc: narmstrong, Ken-Sue.Tan, Chris.Cheng, Bhushan.Patel,
	Jernej Skrabec, Jonas Karlman, David Airlie, open list,
	open list:DRM DRIVERS, Andrzej Hajda, Laurent Pinchart

Hi Vinay.

On Wed, Jul 01, 2020 at 12:33:37PM +0530, Vinay Simha BN wrote:

Please provide some kind of changlog.
for example info where this bridge is used.


> Signed-off-by: Vinay Simha BN <simhavcs@gmail.com>

Some comments in the following.

	Sam

> 
> ---
> v1:
>  Initial version
> 
> v2:
> * Andrzej Hajda review comments incorporated
>   SPDX identifier
>   development debug removed
>   alphabetic order headers
>   u32 instead of unit32_t
>   magic numbers to macros for CLRSI and mux registers
>   ignored return value
> 
> * Laurent Pinchart review comments incorporated
>   mdelay to usleep_range
>   bus_formats added
> 
> v3:
> * Andrzej Hajda review comments incorporated
>   drm_connector_status removed
>   u32 rev removed and local variabl is used
>   regulator enable disable with proper orders and delays
>   as per the spec
>   devm_drm_panel_bridge_add method used instead of panel
>   description modified
>   dual port implemented
> 
> v4:
> * Sam Ravnborg review comments incorporated
>   panel->connector_type removed
> 
> * Reported-by: kernel test robot <lkp@intel.com>
>   parse_dt to static function
>   removed the if (endpoint), since data-lanes has to be
>   present for dsi dts ports
> 
> v5:
>   ~vsdelay dynamic value set based on the
>   calculation of dsi speed, output speed, blanking
> ---
>  drivers/gpu/drm/bridge/Kconfig    |  10 +
>  drivers/gpu/drm/bridge/Makefile   |   1 +
>  drivers/gpu/drm/bridge/tc358775.c | 735 ++++++++++++++++++++++++++++++
>  3 files changed, 746 insertions(+)
>  create mode 100644 drivers/gpu/drm/bridge/tc358775.c
> 
> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
> index 43271c21d3fc..084e9853944a 100644
> --- a/drivers/gpu/drm/bridge/Kconfig
> +++ b/drivers/gpu/drm/bridge/Kconfig
> @@ -181,6 +181,16 @@ config DRM_TOSHIBA_TC358768
>  	help
>  	  Toshiba TC358768AXBG/TC358778XBG DSI bridge chip driver.
>  
> +config DRM_TOSHIBA_TC358775
> +        tristate "Toshiba TC358775 LVDS bridge"
> +        depends on OF
> +        select DRM_KMS_HELPER
> +        select REGMAP_I2C
> +        select DRM_PANEL
> +	select DRM_MIPI_DSI
> +        ---help---
--help---
is no longer supported. Use plain "help".

> +          Toshiba TC358775 LVDS bridge chip driver.
This is a very short description. Can you elaborate more here?

> +
>  config DRM_TI_TFP410
>  	tristate "TI TFP410 DVI/HDMI bridge"
>  	depends on OF
> diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
> index d63d4b7e4347..23c770b3bfe4 100644
> --- a/drivers/gpu/drm/bridge/Makefile
> +++ b/drivers/gpu/drm/bridge/Makefile
> @@ -15,6 +15,7 @@ obj-$(CONFIG_DRM_THINE_THC63LVD1024) += thc63lvd1024.o
>  obj-$(CONFIG_DRM_TOSHIBA_TC358764) += tc358764.o
>  obj-$(CONFIG_DRM_TOSHIBA_TC358767) += tc358767.o
>  obj-$(CONFIG_DRM_TOSHIBA_TC358768) += tc358768.o
> +obj-$(CONFIG_DRM_TOSHIBA_TC358775) += tc358775.o
>  obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511/
>  obj-$(CONFIG_DRM_TI_SN65DSI86) += ti-sn65dsi86.o
>  obj-$(CONFIG_DRM_TI_TFP410) += ti-tfp410.o
> diff --git a/drivers/gpu/drm/bridge/tc358775.c b/drivers/gpu/drm/bridge/tc358775.c
> new file mode 100644
> index 000000000000..e580c544190c
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/tc358775.c
> @@ -0,0 +1,735 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * tc358775 DSI to LVDS bridge driver
> + *
> + * Copyright (C) 2020 SMART Wireless Computing
> + * Author: Vinay Simha BN <simhavcs@gmail.com>
> + *
> + */
> +/* #define DEBUG */
> +#include <linux/clk.h>
> +#include <linux/device.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/i2c.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/slab.h>
> +
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_bridge.h>
> +#include <drm/drm_crtc_helper.h>
> +#include <drm/drm_dp_helper.h>
> +#include <drm/drm_mipi_dsi.h>
> +#include <drm/drm_of.h>
> +#include <drm/drm_panel.h>
> +#include <drm/drm_probe_helper.h>
> +
> +#define FLD_MASK(start, end)    (((1 << ((start) - (end) + 1)) - 1) << (end))
> +#define FLD_VAL(val, start, end) (((val) << (end)) & FLD_MASK(start, end))
> +
> +/* Registers */
> +
> +/* DSI D-PHY Layer Registers */
> +#define D0W_DPHYCONTTX  0x0004  /* Data Lane 0 DPHY Tx Control */
> +#define CLW_DPHYCONTRX  0x0020  /* Clock Lane DPHY Rx Control */
> +#define D0W_DPHYCONTRX  0x0024  /* Data Lane 0 DPHY Rx Control */
> +#define D1W_DPHYCONTRX  0x0028  /* Data Lane 1 DPHY Rx Control */
> +#define D2W_DPHYCONTRX  0x002C  /* Data Lane 2 DPHY Rx Control */
> +#define D3W_DPHYCONTRX  0x0030  /* Data Lane 3 DPHY Rx Control */
> +#define COM_DPHYCONTRX  0x0038  /* DPHY Rx Common Control */
> +#define CLW_CNTRL       0x0040  /* Clock Lane Control */
> +#define D0W_CNTRL       0x0044  /* Data Lane 0 Control */
> +#define D1W_CNTRL       0x0048  /* Data Lane 1 Control */
> +#define D2W_CNTRL       0x004C  /* Data Lane 2 Control */
> +#define D3W_CNTRL       0x0050  /* Data Lane 3 Control */
> +#define DFTMODE_CNTRL   0x0054  /* DFT Mode Control */
> +
> +/* DSI PPI Layer Registers */
> +#define PPI_STARTPPI    0x0104  /* START control bit of PPI-TX function. */
> +#define PPI_START_FUNCTION      1
> +
> +#define PPI_BUSYPPI     0x0108
> +#define PPI_LINEINITCNT 0x0110  /* Line Initialization Wait Counter  */
> +#define PPI_LPTXTIMECNT 0x0114
> +#define PPI_LANEENABLE  0x0134  /* Enables each lane at the PPI layer. */
> +#define PPI_TX_RX_TA    0x013C  /* DSI Bus Turn Around timing parameters */
> +
> +/* Analog timer function enable */
> +#define PPI_CLS_ATMR    0x0140  /* Delay for Clock Lane in LPRX  */
> +#define PPI_D0S_ATMR    0x0144  /* Delay for Data Lane 0 in LPRX */
> +#define PPI_D1S_ATMR    0x0148  /* Delay for Data Lane 1 in LPRX */
> +#define PPI_D2S_ATMR    0x014C  /* Delay for Data Lane 2 in LPRX */
> +#define PPI_D3S_ATMR    0x0150  /* Delay for Data Lane 3 in LPRX */
> +
> +#define PPI_D0S_CLRSIPOCOUNT    0x0164  /* For lane 0 */
> +#define PPI_D1S_CLRSIPOCOUNT    0x0168  /* For lane 1 */
> +#define PPI_D2S_CLRSIPOCOUNT    0x016C  /* For lane 2 */
> +#define PPI_D3S_CLRSIPOCOUNT    0x0170  /* For lane 3 */
> +
> +#define CLS_PRE         0x0180  /* Digital Counter inside of PHY IO */
> +#define D0S_PRE         0x0184  /* Digital Counter inside of PHY IO */
> +#define D1S_PRE         0x0188  /* Digital Counter inside of PHY IO */
> +#define D2S_PRE         0x018C  /* Digital Counter inside of PHY IO */
> +#define D3S_PRE         0x0190  /* Digital Counter inside of PHY IO */
> +#define CLS_PREP        0x01A0  /* Digital Counter inside of PHY IO */
> +#define D0S_PREP        0x01A4  /* Digital Counter inside of PHY IO */
> +#define D1S_PREP        0x01A8  /* Digital Counter inside of PHY IO */
> +#define D2S_PREP        0x01AC  /* Digital Counter inside of PHY IO */
> +#define D3S_PREP        0x01B0  /* Digital Counter inside of PHY IO */
> +#define CLS_ZERO        0x01C0  /* Digital Counter inside of PHY IO */
> +#define D0S_ZERO        0x01C4  /* Digital Counter inside of PHY IO */
> +#define D1S_ZERO        0x01C8  /* Digital Counter inside of PHY IO */
> +#define D2S_ZERO        0x01CC  /* Digital Counter inside of PHY IO */
> +#define D3S_ZERO        0x01D0  /* Digital Counter inside of PHY IO */
> +
> +#define PPI_CLRFLG      0x01E0  /* PRE Counters has reached set values */
> +#define PPI_CLRSIPO     0x01E4  /* Clear SIPO values, Slave mode use only. */
> +#define HSTIMEOUT       0x01F0  /* HS Rx Time Out Counter */
> +#define HSTIMEOUTENABLE 0x01F4  /* Enable HS Rx Time Out Counter */
> +#define DSI_STARTDSI    0x0204  /* START control bit of DSI-TX function */
> +#define DSI_RX_START	1
> +
> +#define DSI_BUSYDSI     0x0208
> +#define DSI_LANEENABLE  0x0210  /* Enables each lane at the Protocol layer. */
> +#define DSI_LANESTATUS0 0x0214  /* Displays lane is in HS RX mode. */
> +#define DSI_LANESTATUS1 0x0218  /* Displays lane is in ULPS or STOP state */
> +
> +#define DSI_INTSTATUS   0x0220  /* Interrupt Status */
> +#define DSI_INTMASK     0x0224  /* Interrupt Mask */
> +#define DSI_INTCLR      0x0228  /* Interrupt Clear */
> +#define DSI_LPTXTO      0x0230  /* Low Power Tx Time Out Counter */
> +
> +#define DSIERRCNT       0x0300  /* DSI Error Count */
> +#define APLCTRL         0x0400  /* Application Layer Control */
> +#define RDPKTLN         0x0404  /* Command Read Packet Length */
> +
> +#define VPCTRL          0x0450  /* Video Path Control */
> +#define HTIM1           0x0454  /* Horizontal Timing Control 1 */
> +#define HTIM2           0x0458  /* Horizontal Timing Control 2 */
> +#define VTIM1           0x045C  /* Vertical Timing Control 1 */
> +#define VTIM2           0x0460  /* Vertical Timing Control 2 */
> +#define VFUEN           0x0464  /* Video Frame Timing Update Enable */
> +#define VFUEN_EN	BIT(0)  /* Upload Enable */
> +
> +/* Mux Input Select for LVDS LINK Input */
> +#define LV_MX0003        0x0480  /* Bit 0 to 3 */
> +#define LV_MX0407        0x0484  /* Bit 4 to 7 */
> +#define LV_MX0811        0x0488  /* Bit 8 to 11 */
> +#define LV_MX1215        0x048C  /* Bit 12 to 15 */
> +#define LV_MX1619        0x0490  /* Bit 16 to 19 */
> +#define LV_MX2023        0x0494  /* Bit 20 to 23 */
> +#define LV_MX2427        0x0498  /* Bit 24 to 27 */
> +#define LV_MX(b0, b1, b2, b3)	(FLD_VAL(b0, 4, 0) | FLD_VAL(b1, 12, 8) | \
> +				FLD_VAL(b2, 20, 16) | FLD_VAL(b3, 28, 24))
> +
> +/* Input bit numbers used in mux registers */
> +enum {
> +	LVI_R0,
> +	LVI_R1,
> +	LVI_R2,
> +	LVI_R3,
> +	LVI_R4,
> +	LVI_R5,
> +	LVI_R6,
> +	LVI_R7,
> +	LVI_G0,
> +	LVI_G1,
> +	LVI_G2,
> +	LVI_G3,
> +	LVI_G4,
> +	LVI_G5,
> +	LVI_G6,
> +	LVI_G7,
> +	LVI_B0,
> +	LVI_B1,
> +	LVI_B2,
> +	LVI_B3,
> +	LVI_B4,
> +	LVI_B5,
> +	LVI_B6,
> +	LVI_B7,
> +	LVI_HS,
> +	LVI_VS,
> +	LVI_DE,
> +	LVI_L0
> +};
> +
> +#define LVCFG           0x049C  /* LVDS Configuration  */
> +#define LVPHY0          0x04A0  /* LVDS PHY 0 */
> +#define LV_PHY0_RST(v)          FLD_VAL(v, 22, 22) /* PHY reset */
> +#define LV_PHY0_IS(v)           FLD_VAL(v, 15, 14)
> +#define LV_PHY0_ND(v)           FLD_VAL(v, 4, 0) /* Frequency range select */
> +#define LV_PHY0_PRBS_ON(v)      FLD_VAL(v, 20, 16) /* Clock/Data Flag pins */
> +
> +#define LVPHY1          0x04A4  /* LVDS PHY 1 */
> +#define SYSSTAT         0x0500  /* System Status  */
> +#define SYSRST          0x0504  /* System Reset  */
> +
> +#define SYS_RST_I2CS	BIT(0) /* Reset I2C-Slave controller */
> +#define SYS_RST_I2CM	BIT(1) /* Reset I2C-Master controller */
> +#define SYS_RST_LCD	BIT(2) /* Reset LCD controller */
> +#define SYS_RST_BM	BIT(3) /* Reset Bus Management controller */
> +#define SYS_RST_DSIRX	BIT(4) /* Reset DSI-RX and App controller */
> +#define SYS_RST_REG	BIT(5) /* Reset Register module */
> +
> +/* GPIO Registers */
> +#define GPIOC           0x0520  /* GPIO Control  */
> +#define GPIOO           0x0524  /* GPIO Output  */
> +#define GPIOI           0x0528  /* GPIO Input  */
> +
> +/* I2C Registers */
> +#define I2CTIMCTRL      0x0540  /* I2C IF Timing and Enable Control */
> +#define I2CMADDR        0x0544  /* I2C Master Addressing */
> +#define WDATAQ          0x0548  /* Write Data Queue */
> +#define RDATAQ          0x054C  /* Read Data Queue */
> +
> +/* Chip ID and Revision ID Register */
> +#define IDREG           0x0580
> +
> +#define LPX_PERIOD		4
> +#define TTA_GET			0x40000
> +#define TTA_SURE		6
> +
> +#define TC358775XBG_ID  0x00007500
> +
> +/* Debug Registers */
> +#define DEBUG00         0x05A0  /* Debug */
> +#define DEBUG01         0x05A4  /* LVDS Data */
> +
> +#define DSI_CLEN_BIT		BIT(0)
> +#define DIVIDE_BY_3		3 /* PCLK=DCLK/3 */
> +#define DIVIDE_BY_6		6 /* PCLK=DCLK/6 */
> +#define LVCFG_LVEN_BIT		BIT(0)
> +
> +#define L0EN BIT(1)
> +
> +#define TC358775_VPCTRL_VSDELAY__MASK	0x3FF00000
> +#define TC358775_VPCTRL_VSDELAY__SHIFT	20
> +static inline u32 TC358775_VPCTRL_VSDELAY(uint32_t val)
> +{
> +	return ((val) << TC358775_VPCTRL_VSDELAY__SHIFT) &
> +			TC358775_VPCTRL_VSDELAY__MASK;
> +}
> +
> +#define TC358775_VPCTRL_OPXLFMT__MASK	0x00000100
> +#define TC358775_VPCTRL_OPXLFMT__SHIFT	8
> +static inline u32 TC358775_VPCTRL_OPXLFMT(uint32_t val)
> +{
> +	return ((val) << TC358775_VPCTRL_OPXLFMT__SHIFT) &
> +			TC358775_VPCTRL_OPXLFMT__MASK;
> +}
> +
> +#define TC358775_VPCTRL_MSF__MASK	0x00000001
> +#define TC358775_VPCTRL_MSF__SHIFT	0
> +static inline u32 TC358775_VPCTRL_MSF(uint32_t val)
> +{
> +	return ((val) << TC358775_VPCTRL_MSF__SHIFT) &
> +			TC358775_VPCTRL_MSF__MASK;
> +}
> +
> +#define TC358775_LVCFG_PCLKDIV__MASK	0x000000f0
> +#define TC358775_LVCFG_PCLKDIV__SHIFT	4
> +static inline u32 TC358775_LVCFG_PCLKDIV(uint32_t val)
> +{
> +	return ((val) << TC358775_LVCFG_PCLKDIV__SHIFT) &
> +			TC358775_LVCFG_PCLKDIV__MASK;
> +}
> +
> +#define TC358775_LVCFG_LVDLINK__MASK                         0x00000002
> +#define TC358775_LVCFG_LVDLINK__SHIFT                        0
> +static inline u32 TC358775_LVCFG_LVDLINK(uint32_t val)
> +{
> +	return ((val) << TC358775_LVCFG_LVDLINK__SHIFT) &
> +			TC358775_LVCFG_LVDLINK__MASK;
> +}
> +
> +enum tc358775_ports {
> +	TC358775_DSI_IN,
> +	TC358775_LVDS_OUT0,
> +	TC358775_LVDS_OUT1,
> +};
> +
> +struct tc_data {
> +	struct i2c_client	*i2c;
> +	struct device		*dev;
> +
> +	struct drm_bridge	bridge;
> +	struct drm_bridge	*panel_bridge;
> +
> +	struct device_node *host_node;
> +	struct mipi_dsi_device *dsi;
> +	u8 num_dsi_lanes;
> +
> +	struct regulator	*vdd;
> +	struct regulator	*vddio;
> +	struct gpio_desc	*reset_gpio;
> +	struct gpio_desc	*stby_gpio;
> +	bool                    dual_link; /* single-link or dual-link */
> +};
> +
> +static inline struct tc_data *bridge_to_tc(struct drm_bridge *b)
> +{
> +	return container_of(b, struct tc_data, bridge);
> +}
> +
> +static void tc_bridge_pre_enable(struct drm_bridge *bridge)
> +{
> +	struct tc_data *tc = bridge_to_tc(bridge);
> +	struct device *dev = &tc->dsi->dev;
> +	int ret;
> +
> +	ret = regulator_enable(tc->vddio);
> +	if (ret < 0)
> +		dev_err(dev, "regulator vddio enable failed, %d\n", ret);
> +	usleep_range(10000, 11000);
> +
> +	ret = regulator_enable(tc->vdd);
> +	if (ret < 0)
> +		dev_err(dev, "regulator vdd enable failed, %d\n", ret);
> +	usleep_range(10000, 11000);
> +
> +	gpiod_set_value(tc->stby_gpio, 0);
> +	usleep_range(10000, 11000);
> +
> +	gpiod_set_value(tc->reset_gpio, 0);
> +	usleep_range(10, 20);
> +}
> +
> +static void tc_bridge_post_disable(struct drm_bridge *bridge)
> +{
> +	struct tc_data *tc = bridge_to_tc(bridge);
> +	struct device *dev = &tc->dsi->dev;
> +	int ret;
> +
> +	ret = regulator_disable(tc->vddio);
> +	if (ret < 0)
> +		dev_err(dev, "regulator vddio disable failed, %d\n", ret);
> +	usleep_range(10000, 11000);
> +
> +	ret = regulator_disable(tc->vdd);
> +	if (ret < 0)
> +		dev_err(dev, "regulator vdd disable failed, %d\n", ret);
> +	usleep_range(10000, 11000);
> +
> +	gpiod_set_value(tc->stby_gpio, 1);
> +	usleep_range(10000, 11000);
> +
> +	gpiod_set_value(tc->reset_gpio, 1);
> +	usleep_range(10, 20);
> +}
> +
> +static u32 d2l_read(struct i2c_client *i2c, u16 reg)
> +{
> +	int ret;
> +	u8 val[4];
> +	u8 addr[] = {(reg >> 8) & 0xff, reg & 0xff};
> +	u32 value;
> +
> +	ret = i2c_master_send(i2c, addr, sizeof(reg));
> +	if (ret < 0)
> +		goto fail;
> +
> +	ret = i2c_master_recv(i2c, val, sizeof(value));
> +	if (ret < 0)
> +		goto fail;
> +
> +	value = val[3] << 24 | val[2] << 16 | val[1] << 8 | val[0];
> +
> +	pr_debug("d2l: I2C : reg:%04x value:%08x\n", reg, value);
> +
> +	return value;
> +
> +fail:
> +	dev_err(&i2c->dev, "Error %d reading from subaddress 0x%x\n",
> +		ret, reg);
> +	return 0;
> +}
> +
> +static void d2l_write(struct i2c_client *i2c, u16 reg, u32 val)
> +{
> +	u8 buf[6];
> +	int ret;
> +
> +	buf[0] = reg >> 8;
> +	buf[1] = reg & 0xFF;
> +	buf[2] = (val >> 0) & 0xFF;
> +	buf[3] = (val >> 8) & 0xFF;
> +	buf[4] = (val >> 16) & 0xFF;
> +	buf[5] = (val >> 24) & 0xFF;
> +
> +	ret = i2c_master_send(i2c, buf, ARRAY_SIZE(buf));
> +	if (ret < 0)
> +		dev_err(&i2c->dev, "Error %d writing to subaddress 0x%x\n",
> +			ret, reg);
> +}
> +
> +/* helper function to access bus_formats */
> +static struct drm_connector *get_connector(struct drm_encoder *encoder)
> +{
> +	struct drm_device *dev = encoder->dev;
> +	struct drm_connector *connector;
> +
> +	list_for_each_entry(connector, &dev->mode_config.connector_list, head)
> +		if (connector->encoder == encoder)
> +			return connector;
> +
> +	return NULL;
> +}
> +
> +static void tc_bridge_enable(struct drm_bridge *bridge)
> +{
> +	struct tc_data *tc = bridge_to_tc(bridge);
> +	u32 hbpr, hpw, htime1, hfpr, hsize, htime2;
> +	u32 vbpr, vpw, vtime1, vfpr, vsize, vtime2;
> +	u32 val = 0, val_lvcfg = 0;
> +	u16 dsiclk, clkdiv, byteclk, t1, t2, t3, vsdelay;
> +	u8 link, bpc;
> +	u16 bus_formats;
> +	struct drm_display_mode *mode;
> +	struct drm_connector *connector = get_connector(bridge->encoder);
> +
> +	mode = &bridge->encoder->crtc->state->adjusted_mode;
> +
> +	hbpr = mode->htotal - mode->hsync_end;
> +	hpw  = mode->hsync_end - mode->hsync_start;
> +	vbpr = mode->vtotal - mode->vsync_end;
> +	vpw  = mode->vsync_end - mode->vsync_start;
> +
> +	htime1 = (hbpr << 16) + hpw;
> +	vtime1 = (vbpr << 16) + vpw;
> +
> +	hfpr = mode->hsync_start - mode->hdisplay;
> +	hsize = mode->hdisplay;
> +	vfpr = mode->vsync_start - mode->vdisplay;
> +	vsize = mode->vdisplay;
> +
> +	htime2 = (hfpr << 16) + hsize;
> +	vtime2 = (vfpr << 16) + vsize;

If the local variables here used the same names as used in struct
display_timings then this would be ore readable.
vfpr is less readable compared to vfront_porch - especially because the
latter is well known names.

> +
> +	val = d2l_read(tc->i2c, IDREG);
> +
> +	dev_info(tc->dev, "DSI2LVDS Chip ID.%02x Revision ID. %02x **\n",
> +		 (val >> 8) & 0xFF, val & 0xFF);
> +
> +	d2l_write(tc->i2c, SYSRST, SYS_RST_REG | SYS_RST_DSIRX | SYS_RST_BM |
> +		  SYS_RST_LCD | SYS_RST_I2CM | SYS_RST_I2CS);
> +	usleep_range(30000, 40000);
> +
> +	d2l_write(tc->i2c, PPI_TX_RX_TA, TTA_GET | TTA_SURE);
> +	d2l_write(tc->i2c, PPI_LPTXTIMECNT, LPX_PERIOD);
> +	d2l_write(tc->i2c, PPI_D0S_CLRSIPOCOUNT, 3);
> +	d2l_write(tc->i2c, PPI_D1S_CLRSIPOCOUNT, 3);
> +	d2l_write(tc->i2c, PPI_D2S_CLRSIPOCOUNT, 3);
> +	d2l_write(tc->i2c, PPI_D3S_CLRSIPOCOUNT, 3);
> +
> +	val = ((L0EN << tc->num_dsi_lanes) - L0EN) | DSI_CLEN_BIT;
> +	d2l_write(tc->i2c, PPI_LANEENABLE, val);
> +	d2l_write(tc->i2c, DSI_LANEENABLE, val);
> +
> +	d2l_write(tc->i2c, PPI_STARTPPI, PPI_START_FUNCTION);
> +	d2l_write(tc->i2c, DSI_STARTDSI, DSI_RX_START);
> +
> +	val_lvcfg = LVCFG_LVEN_BIT;
> +	if (tc->dual_link) {
> +		link = 2;
> +		val_lvcfg |= TC358775_LVCFG_LVDLINK(1);
> +		val_lvcfg |= TC358775_LVCFG_PCLKDIV(DIVIDE_BY_6);
> +	} else {
> +		link = 1;
> +		val_lvcfg |= TC358775_LVCFG_PCLKDIV(DIVIDE_BY_3);
> +	}
> +
> +	bus_formats = connector->display_info.bus_formats[0];
> +	dev_dbg(tc->dev, "bus_formats : %04x\n", bus_formats);
> +
> +	if (bus_formats == MEDIA_BUS_FMT_RGB888_1X7X4_SPWG ||
> +	    bus_formats == MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA) {
> +		/* RGB888 */
> +		bpc = 8;
> +		val = TC358775_VPCTRL_OPXLFMT(1);
> +	} else {
> +		/* RGB666 */
> +		bpc = 6;
> +		val |= TC358775_VPCTRL_MSF(1);
> +	}
This code makes some assumptions about bus_format.
Maybe check for supported bus_format and error out if an unspupported
bus_format is found?
> +
> +	dsiclk = mode->crtc_clock * 3 * bpc / tc->num_dsi_lanes / 1000;
> +	clkdiv = dsiclk / DIVIDE_BY_3 * link;
> +	byteclk = dsiclk / 4;
> +	t1 = hsize * (bpc * 3 / 8) / tc->num_dsi_lanes;
> +	t2 = ((100000 / clkdiv)) * (hsize + hbpr + hpw + hfpr) / 1000;
> +	t3 = ((t2 * byteclk) / 100) - (hsize * (bpc * 3 / 8) /
> +		tc->num_dsi_lanes);
> +
> +	vsdelay = (clkdiv * (t1 + t3) / byteclk) - hbpr - hpw - hsize;
> +
> +	val |= TC358775_VPCTRL_VSDELAY(vsdelay);
> +	d2l_write(tc->i2c, VPCTRL, val);
> +
> +	d2l_write(tc->i2c, HTIM1, htime1);
> +	d2l_write(tc->i2c, VTIM1, vtime1);
> +	d2l_write(tc->i2c, HTIM2, htime2);
> +	d2l_write(tc->i2c, VTIM2, vtime2);
> +
> +	d2l_write(tc->i2c, VFUEN, VFUEN_EN);
> +	d2l_write(tc->i2c, SYSRST, SYS_RST_LCD);
> +	d2l_write(tc->i2c, LVPHY0, LV_PHY0_PRBS_ON(4) | LV_PHY0_ND(6));
> +
> +	/* default jeida-24 */
> +	if (bus_formats == MEDIA_BUS_FMT_RGB888_1X7X4_SPWG) {
> +		/* vesa-24 */
> +		d2l_write(tc->i2c, LV_MX0003, LV_MX(LVI_R0, LVI_R1, LVI_R2, LVI_R3));
> +		d2l_write(tc->i2c, LV_MX0407, LV_MX(LVI_R4, LVI_R7, LVI_R5, LVI_G0));
> +		d2l_write(tc->i2c, LV_MX0811, LV_MX(LVI_G1, LVI_G2, LVI_G6, LVI_G7));
> +		d2l_write(tc->i2c, LV_MX1215, LV_MX(LVI_G3, LVI_G4, LVI_G5, LVI_B0));
> +		d2l_write(tc->i2c, LV_MX1619, LV_MX(LVI_B6, LVI_B7, LVI_B1, LVI_B2));
> +		d2l_write(tc->i2c, LV_MX2023, LV_MX(LVI_B3, LVI_B4, LVI_B5, LVI_L0));
> +		d2l_write(tc->i2c, LV_MX2427, LV_MX(LVI_HS, LVI_VS, LVI_DE, LVI_R6));
> +	}
> +
> +	if (bus_formats == MEDIA_BUS_FMT_RGB666_1X7X3_SPWG) {
> +		/* jeida-18 */
> +		d2l_write(tc->i2c, LV_MX0003, LV_MX(LVI_R0, LVI_R1, LVI_R2, LVI_R3));
> +		d2l_write(tc->i2c, LV_MX0407, LV_MX(LVI_R4, LVI_L0, LVI_R5, LVI_G0));
> +		d2l_write(tc->i2c, LV_MX0811, LV_MX(LVI_G1, LVI_G2, LVI_L0, LVI_L0));
> +		d2l_write(tc->i2c, LV_MX1215, LV_MX(LVI_G3, LVI_G4, LVI_G5, LVI_B0));
> +		d2l_write(tc->i2c, LV_MX1619, LV_MX(LVI_L0, LVI_L0, LVI_B1, LVI_B2));
> +		d2l_write(tc->i2c, LV_MX2023, LV_MX(LVI_B3, LVI_B4, LVI_B5, LVI_L0));
> +		d2l_write(tc->i2c, LV_MX2427, LV_MX(LVI_HS, LVI_VS, LVI_DE, LVI_L0));
> +	}
MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA???

> +
> +	d2l_write(tc->i2c, VFUEN, VFUEN_EN);
> +	d2l_write(tc->i2c, LVCFG, val_lvcfg);
> +}
> +
> +static int tc_mode_valid(struct drm_bridge *bridge,
> +			 const struct drm_display_mode *mode)
> +{
> +	struct tc_data *tc = bridge_to_tc(bridge);
> +
> +	/* Maximum pixel clock speed 135MHz-single-link/270MHz-dual-link */
> +	if ((mode->clock > 135000 && tc->dual_link) ||
> +	    (mode->clock > 270000 && tc->dual_link))
> +		return MODE_CLOCK_HIGH;

I think this should read:
> +	if ((mode->clock > 135000 && !tc->dual_link) ||
Or maybe the comment is wrong and 135 Mhz is maximum with dual-link?

> +
> +	return MODE_OK;
> +}
> +
> +static int tc358775_parse_dt(struct device_node *np, struct tc_data *tc)
> +{
> +	struct device_node *endpoint;
> +	struct device_node *parent;
> +	struct device_node *remote;
> +	struct property *prop;
> +	int len;
> +
> +	endpoint = of_graph_get_endpoint_by_regs(tc->dev->of_node,
> +						 TC358775_DSI_IN, -1);
> +	if (endpoint) {
> +		/* dsi0_out node */
		TC358775_DSI_IN is port@0 whic is input.
	
> +		parent = of_graph_get_remote_port_parent(endpoint);
> +		of_node_put(endpoint);
> +		if (parent) {
> +			/* dsi0 port 1 */
> +			endpoint = of_graph_get_endpoint_by_regs(parent, 1, -1);
> +			of_node_put(parent);
> +			prop = of_find_property(endpoint, "data-lanes",
> +						&len);
> +			of_node_put(endpoint);
> +			if (!prop) {
> +				dev_err(tc->dev,
> +					"failed to find data lane\n");
> +				return -EPROBE_DEFER;
> +			}
> +		}
> +	}

Error handling looks wrong.
if endpoint is not set then len is never assigned.

> +	tc->num_dsi_lanes = len / sizeof(u32);
> +
> +	if (tc->num_dsi_lanes < 1 || tc->num_dsi_lanes > 4)
> +		return -EINVAL;
> +
> +	tc->host_node = of_graph_get_remote_node(np, 0, 0);
> +	if (!tc->host_node)
> +		return -ENODEV;
> +
> +	of_node_put(tc->host_node);
> +
> +	endpoint = of_graph_get_endpoint_by_regs(tc->dev->of_node,
> +						 TC358775_LVDS_OUT1, -1);
> +	if (endpoint) {
> +		remote = of_graph_get_remote_port_parent(endpoint);
> +		of_node_put(endpoint);
> +
> +		if (remote) {
> +			if (of_device_is_available(remote))
> +				tc->dual_link = true;
> +			of_node_put(remote);
> +		}
> +	}
> +
> +	dev_dbg(tc->dev, "no.of dsi lanes: %d\n", tc->num_dsi_lanes);
> +	dev_dbg(tc->dev, "operating in %s-link mode\n",
> +		tc->dual_link ? "dual" : "single");
> +
> +	return 0;
> +}
> +
> +static int tc_bridge_attach(struct drm_bridge *bridge,
> +			    enum drm_bridge_attach_flags flags)
> +{
> +	struct tc_data *tc = bridge_to_tc(bridge);
> +	struct device *dev = &tc->i2c->dev;
> +	struct mipi_dsi_host *host;
> +	struct mipi_dsi_device *dsi;
> +	int ret;
> +
> +	const struct mipi_dsi_device_info info = { .type = "tc358775",
> +							.channel = 0,
> +							.node = NULL,
> +						};
> +
> +	host = of_find_mipi_dsi_host_by_node(tc->host_node);
> +	if (!host) {
> +		dev_err(dev, "failed to find dsi host\n");
> +		return -EPROBE_DEFER;
> +	}
> +
> +	dsi = mipi_dsi_device_register_full(host, &info);
> +	if (IS_ERR(dsi)) {
> +		dev_err(dev, "failed to create dsi device\n");
> +		ret = PTR_ERR(dsi);
> +		goto err_dsi_device;
> +	}
> +
> +	tc->dsi = dsi;
> +
> +	dsi->lanes = tc->num_dsi_lanes;
> +	dsi->format = MIPI_DSI_FMT_RGB888;
> +	dsi->mode_flags = MIPI_DSI_MODE_VIDEO;
> +
> +	ret = mipi_dsi_attach(dsi);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to attach dsi to host\n");
> +		goto err_dsi_attach;
> +	}
> +
> +	/* Attach the panel-bridge to the dsi bridge */
> +	return drm_bridge_attach(bridge->encoder, tc->panel_bridge,
> +				 &tc->bridge, flags);
> +err_dsi_attach:
> +	mipi_dsi_device_unregister(dsi);
> +err_dsi_device:
> +	return ret;
> +}
> +
> +static const struct drm_bridge_funcs tc_bridge_funcs = {
> +	.attach = tc_bridge_attach,
> +	.pre_enable = tc_bridge_pre_enable,
> +	.enable = tc_bridge_enable,
> +	.mode_valid = tc_mode_valid,
> +	.post_disable = tc_bridge_post_disable,
> +};
> +
> +static int tc_probe(struct i2c_client *client, const struct i2c_device_id *id)
> +{
> +	struct device *dev = &client->dev;
> +	struct drm_panel *panel;
> +	struct tc_data *tc;
> +	int ret;
> +
> +	tc = devm_kzalloc(dev, sizeof(*tc), GFP_KERNEL);
> +	if (!tc)
> +		return -ENOMEM;
> +
> +	tc->dev = dev;
> +	tc->i2c = client;
> +
> +	ret = drm_of_find_panel_or_bridge(dev->of_node, TC358775_LVDS_OUT0,
> +					  0, &panel, NULL);
> +	if (ret < 0)
> +		return ret;
> +	if (!panel)
> +		return -ENODEV;
> +
> +	tc->panel_bridge = devm_drm_panel_bridge_add(dev, panel);
> +	if (IS_ERR(tc->panel_bridge))
> +		return PTR_ERR(tc->panel_bridge);
> +
> +	ret = tc358775_parse_dt(dev->of_node, tc);
> +	if (ret)
> +		return ret;
> +
> +	tc->vddio = devm_regulator_get(dev, "vddio-supply");
> +	if (IS_ERR(tc->vddio)) {
> +		ret = PTR_ERR(tc->vddio);
> +		dev_err(dev, "vddio-supply not found\n");
> +		return ret;
> +	}
> +
> +	tc->vdd = devm_regulator_get(dev, "vdd-supply");
> +	if (IS_ERR(tc->vdd)) {
> +		ret = PTR_ERR(tc->vddio);
> +		dev_err(dev, "vdd-supply not found\n");
> +		return ret;
> +	}
> +
> +	tc->stby_gpio = devm_gpiod_get(dev, "stby", GPIOD_OUT_HIGH);
> +	if (IS_ERR(tc->stby_gpio)) {
> +		ret = PTR_ERR(tc->stby_gpio);
> +		dev_err(dev, "cannot get stby-gpio %d\n", ret);
> +		return ret;
> +	}
> +
> +	tc->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
> +	if (IS_ERR(tc->reset_gpio)) {
> +		ret = PTR_ERR(tc->reset_gpio);
> +		dev_err(dev, "cannot get reset-gpios %d\n", ret);
> +		return ret;
> +	}
> +
> +	tc->bridge.funcs = &tc_bridge_funcs;
> +	tc->bridge.of_node = dev->of_node;
> +	drm_bridge_add(&tc->bridge);
> +
> +	i2c_set_clientdata(client, tc);
> +
> +	return 0;
> +}
> +
> +static int tc_remove(struct i2c_client *client)
> +{
> +	struct tc_data *tc = i2c_get_clientdata(client);
> +
> +	drm_bridge_remove(&tc->bridge);
> +
> +	return 0;
> +}
> +
> +static const struct i2c_device_id tc358775_i2c_ids[] = {
> +	{ "tc358775", 0 },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, tc358775_i2c_ids);
> +
> +static const struct of_device_id tc358775_of_ids[] = {
> +	{ .compatible = "toshiba,tc358775", },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, tc358775_of_ids);
> +
> +static struct i2c_driver tc358775_driver = {
> +	.driver = {
> +		.name = "tc358775",
> +		.of_match_table = tc358775_of_ids,
> +	},
> +	.id_table = tc358775_i2c_ids,
> +	.probe = tc_probe,
> +	.remove	= tc_remove,
> +};
> +module_i2c_driver(tc358775_driver);
> +
> +MODULE_AUTHOR("Vinay Simha BN <simhavcs@gmail.com>");
> +MODULE_DESCRIPTION("TC358775 DSI/LVDS bridge driver");
> +MODULE_LICENSE("GPL v2");
> -- 
> 2.17.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v5 1/2] dt-binding: Add DSI/LVDS TC358775 bridge bindings
  2020-07-01  9:19 ` [PATCH v5 1/2] dt-binding: Add DSI/LVDS TC358775 bridge bindings Sam Ravnborg
@ 2020-07-01  9:44   ` Vinay Simha B N
  2020-07-01 11:20     ` Sam Ravnborg
  0 siblings, 1 reply; 6+ messages in thread
From: Vinay Simha B N @ 2020-07-01  9:44 UTC (permalink / raw)
  To: Sam Ravnborg
  Cc: Neil Armstrong, Ken-Sue.Tan, Chris.Cheng, Bhushan.Patel,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	David Airlie, open list, open list:DRM DRIVERS, Rob Herring

sam,

I rechecked, but did not find any indents are too spaced.
This is taken from a working dts tree.

I had given two examples for a purpose. One for single lvds
configuration single link mode,
another is for dual lvds configurations dual-link mode and port mappings.

On Wed, Jul 1, 2020 at 2:49 PM Sam Ravnborg <sam@ravnborg.org> wrote:
>
> Hi Vinay.
>
> On Wed, Jul 01, 2020 at 12:33:36PM +0530, Vinay Simha BN wrote:
> > This driver is tested with two panels with Apq8016-IFC6309 board
> > https://www.inforcecomputing.com/products/single-board-computers-sbc/qualcomm-snapdragon-410-inforce-6309-micro-sbc
> >
> > 1. 1366x768@60 auo,b101xtn01 data-mapping = "jeida-24"
> > 2. 800x480@60 innolux,at070tn92 data-mapping = "vesa-24"
> >
> > Signed-off-by: Vinay Simha BN <simhavcs@gmail.com>
> >
> > ---
> > v1:
> >  Initial version wast .txt file
> >
> > v2:
> >  From txt to yaml file format
> >
> > v3:
> > * Andrzej Hajda review comments incorporated
> >   dual port lvds implemented
> >
> > * Laurent Pinchart review comments incorporated
> >   dsi lanes property removed and it is dynamically
> >   picked from the dsi ports
> >   VESA/JEIDA format picked from panel-lvds dts
> > ---
> >  .../display/bridge/toshiba,tc358775.yaml      | 204 ++++++++++++++++++
> >  1 file changed, 204 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/display/bridge/toshiba,tc358775.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/display/bridge/toshiba,tc358775.yaml b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358775.yaml
> > new file mode 100644
> > index 000000000000..ec53d62d408b
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358775.yaml
> > @@ -0,0 +1,204 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/display/bridge/toshiba,tc358775.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Toshiba TC358775 DSI to LVDS bridge bindings
> > +
> > +maintainers:
> > + - Vinay Simha BN <simhavcs@gmail.com>
> > +
> > +description: |
> > + This binding supports DSI to LVDS bridge TC358775
> > +
> > +properties:
> > +  compatible:
> > +    const: toshiba,tc358775
> > +
> > +  reg:
> > +    maxItems: 1
> > +    description: i2c address of the bridge, 0x0f
> > +
> > +  vdd-supply:
> > +    maxItems: 1
> > +    description:  1.2V LVDS Power Supply
> > +
> > +  vddio-supply:
> > +    maxItems: 1
> > +    description: 1.8V IO Power Supply
> > +
> > +  stby-gpios:
> > +    maxItems: 1
> > +    description: Standby pin, Low active
> > +
> > +  reset-gpios:
> > +    maxItems: 1
> > +    description: Hardware reset, Low active
> > +
> > +  ports:
> > +    type: object
> > +    description:
> > +      A node containing input and output port nodes with endpoint definitions
> > +      as documented in
> > +      Documentation/devicetree/bindings/media/video-interfaces.txt
> > +    properties:
> > +      "#address-cells":
> > +        const: 1
> > +
> > +      "#size-cells":
> > +        const: 0
> > +
> > +      port@0:
> > +        type: object
> > +        description: |
> > +          DSI Input. The remote endpoint phandle should be a
> > +          reference to a valid mipi_dsi_host device node.
> > +
> > +      port@1:
> > +        type: object
> > +        description: |
> > +          Video port for LVDS output (panel or connector).
> > +
> > +      port@2:
> > +        type: object
> > +        description: |
> > +          Video port for Dual link LVDS output (panel or connector).
> > +
> > +    required:
> > +      - port@0
> > +      - port@1
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - vdd-supply
> > + - vddio-supply
> > + - stby-gpios
> > + - reset-gpios
> > + - ports
> > +
> > +examples:
> > + - |
> > +    #include <dt-bindings/gpio/gpio.h>
> > +
> > +    i2c@78b8000 {
> > +        /* On High speed expansion */
> > +        label = "HS-I2C2";
> > +        reg = <0x078b8000 0x500>;
> > +        clock-frequency = <400000>; /* fastmode operation */
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +
> > +        tc_bridge: bridge@f {
> > +            compatible = "toshiba,tc358775";
> > +            reg = <0x0f>;
> > +
> > +            vdd-supply = <&pm8916_l2>;
> > +            vddio-supply = <&pm8916_l6>;
> > +
> > +            stby-gpios = <&msmgpio 99 GPIO_ACTIVE_LOW>;
> > +            reset-gpios = <&msmgpio 72 GPIO_ACTIVE_LOW>;
> > +
> > +            ports {
> > +                #address-cells = <1>;
> > +                #size-cells = <0>;
> > +
> > +                port@0 {
> > +                    reg = <0>;
> > +                    d2l_in_test: endpoint {
> > +                        remote-endpoint = <&dsi0_out>;
> > +                    };
> > +                };
> > +
> > +                port@1 {
> > +                    reg = <1>;
> > +                    lvds_out: endpoint {
> > +                        remote-endpoint = <&panel_in>;
> > +                    };
> > +                };
> > +            };
> > +        };
> > +    };
> > +
> > +    dsi@1a98000 {
> > +        reg = <0x1a98000 0x25c>;
> > +        reg-names = "dsi_ctrl";
> > +
> > +        ports {
> > +            #address-cells = <1>;
> > +            #size-cells = <0>;
> > +            port@1 {
> > +                reg = <1>;
> > +                dsi0_out: endpoint {
> > +                    remote-endpoint = <&d2l_in_test>;
> > +                        data-lanes = <0 1 2 3>;
> > +                };
> > +             };
> > +         };
> > +     };
> > +
> Fix indent. One of the '}' is indented one space too much.
>
>
> > + - |
> > +    i2c@78b8000 {
> > +        /* On High speed expansion */
> > +        label = "HS-I2C2";
> > +        reg = <0x078b8000 0x500>;
> > +        clock-frequency = <400000>; /* fastmode operation */
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> What is the purpose with the second example?
> Does it explain the binding better in some way?
>
> The examples shall help understanding the binding, not help
> understanding the HW. The latter is the role of the binding.
>
>
> > +
> > +        tc_bridge_dual: bridge@f {
> > +            compatible = "toshiba,tc358775";
> > +            reg = <0x0f>;
> > +
> > +            vdd-supply = <&pm8916_l2>;
> > +            vddio-supply = <&pm8916_l6>;
> > +
> > +            stby-gpios = <&msmgpio 99 GPIO_ACTIVE_LOW>;
> > +            reset-gpios = <&msmgpio 72 GPIO_ACTIVE_LOW>;
> > +
> > +            ports {
> > +                #address-cells = <1>;
> > +                #size-cells = <0>;
> > +
> > +                port@0 {
> > +                    reg = <0>;
> > +                    d2l_in_dual: endpoint {
> > +                        remote-endpoint = <&dsi0_out_dual>;
> > +                    };
> > +                };
> > +
> > +                port@1 {
> > +                    reg = <1>;
> > +                    lvds0_out: endpoint {
> > +                        remote-endpoint = <&panel_in0>;
> > +                    };
> > +                };
> > +
> > +                port@2 {
> > +                    reg = <2>;
> > +                    lvds1_out: endpoint {
> > +                        remote-endpoint = <&panel_in1>;
> > +                    };
> > +                };
> > +            };
> > +        };
> > +    };
> > +
> > +    dsi@1a98000 {
> > +        reg = <0x1a98000 0x25c>;
> > +        reg-names = "dsi_ctrl";
> > +
> > +        ports {
> > +            #address-cells = <1>;
> > +            #size-cells = <0>;
> > +            port@1 {
> > +                reg = <1>;
> > +                dsi0_out_dual: endpoint {
> > +                    remote-endpoint = <&d2l_in_dual>;
> > +                        data-lanes = <0 1 2 3>;
> > +                };
> > +             };
> > +         };
> > +     };
> If this example is kept then fix indent of '}'.
>
>         Sam
> > +...
> > --
> > 2.17.1
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel



-- 
regards,
vinaysimha

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v5 1/2] dt-binding: Add DSI/LVDS TC358775 bridge bindings
  2020-07-01  9:44   ` Vinay Simha B N
@ 2020-07-01 11:20     ` Sam Ravnborg
  0 siblings, 0 replies; 6+ messages in thread
From: Sam Ravnborg @ 2020-07-01 11:20 UTC (permalink / raw)
  To: Vinay Simha B N
  Cc: Neil Armstrong, Ken-Sue.Tan, Chris.Cheng, Bhushan.Patel,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	David Airlie, open list, open list:DRM DRIVERS, Rob Herring

Hi Vinay.

On Wed, Jul 01, 2020 at 03:14:09PM +0530, Vinay Simha B N wrote:
> sam,
> 
> I rechecked, but did not find any indents are too spaced.
> This is taken from a working dts tree.
> 
            port@1 {
            |   reg = <1>;
            |   dsi0_out: endpoint {
            |       remote-endpoint = <&d2l_in_test>;
            |           data-lanes = <0 1 2 3>;
            |   };
             };
         };
     };
See above, something is wrong with the indent in the examples.


> I had given two examples for a purpose. One for single lvds
> configuration single link mode,
> another is for dual lvds configurations dual-link mode and port mappings.

Please provide a proper description of the single link and dual link
modes in the descriptions of the bindings.
The examples shall support the bindings, they shall not include
information that is not covered in the bindings.

	Sam

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-07-01 11:20 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-01  7:03 [PATCH v5 1/2] dt-binding: Add DSI/LVDS TC358775 bridge bindings Vinay Simha BN
2020-07-01  7:03 ` [PATCH v5 2/2] display/drm/bridge: TC358775 DSI/LVDS driver Vinay Simha BN
2020-07-01  9:35   ` Sam Ravnborg
2020-07-01  9:19 ` [PATCH v5 1/2] dt-binding: Add DSI/LVDS TC358775 bridge bindings Sam Ravnborg
2020-07-01  9:44   ` Vinay Simha B N
2020-07-01 11:20     ` Sam Ravnborg

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).