[PATCH V2 1/2] dt-bindings: drm/bridge: ti-sn65dsi83: Add TI SN65DSI83 and SN65DSI84 bindings

[PATCH V2 1/2] dt-bindings: drm/bridge: ti-sn65dsi83: Add TI SN65DSI83 and SN65DSI84 bindings

[Marek Vasut]

Add DT binding document for TI SN65DSI83 and SN65DSI84 DSI to LVDS bridge.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Douglas Anderson <dianders@chromium.org>
Cc: Jagan Teki <jagan@amarulasolutions.com>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Sam Ravnborg <sam@ravnborg.org>
Cc: Stephen Boyd <swboyd@chromium.org>
Cc: devicetree@vger.kernel.org
To: dri-devel@lists.freedesktop.org
---
V2: Add compatible string for SN65DSI84, since this is now tested on it
---
 .../bindings/display/bridge/ti,sn65dsi83.yaml | 134 ++++++++++++++++++
 1 file changed, 134 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml

diff --git a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
new file mode 100644
index 000000000000..42d11b46a1eb
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
@@ -0,0 +1,134 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/bridge/ti,sn65dsi83.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: SN65DSI83 and SN65DSI84 DSI to LVDS bridge chip
+
+maintainers:
+  - Marek Vasut <marex@denx.de>
+
+description: |
+  Texas Instruments SN65DSI83 1x Single-link MIPI DSI
+  to 1x Single-link LVDS
+  https://www.ti.com/lit/gpn/sn65dsi83
+  Texas Instruments SN65DSI84 1x Single-link MIPI DSI
+  to 1x Dual-link or 2x Single-link LVDS
+  https://www.ti.com/lit/gpn/sn65dsi84
+
+properties:
+  compatible:
+    oneOf:
+      - const: ti,sn65dsi83
+      - const: ti,sn65dsi84
+
+  reg:
+    const: 0x2d
+
+  enable-gpios:
+    maxItems: 1
+    description: GPIO specifier for bridge_en pin (active high).
+
+  ports:
+    type: object
+    additionalProperties: false
+
+    properties:
+      "#address-cells":
+        const: 1
+
+      "#size-cells":
+        const: 0
+
+      port@0:
+        type: object
+        additionalProperties: false
+
+        description:
+          Video port for MIPI DSI input
+
+        properties:
+          reg:
+            const: 0
+
+          endpoint:
+            type: object
+            additionalProperties: false
+            properties:
+              remote-endpoint: true
+              data-lanes:
+                description: array of physical DSI data lane indexes.
+
+        required:
+          - reg
+
+      port@1:
+        type: object
+        additionalProperties: false
+
+        description:
+          Video port for LVDS output (panel or bridge).
+
+        properties:
+          reg:
+            const: 1
+
+          endpoint:
+            type: object
+            additionalProperties: false
+            properties:
+              remote-endpoint: true
+
+        required:
+          - reg
+
+    required:
+      - "#address-cells"
+      - "#size-cells"
+      - port@0
+      - port@1
+
+required:
+  - compatible
+  - reg
+  - enable-gpios
+  - ports
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      bridge@2d {
+        compatible = "ti,sn65dsi83";
+        reg = <0x2d>;
+
+        enable-gpios = <&gpio2 1 GPIO_ACTIVE_HIGH>;
+
+        ports {
+          #address-cells = <1>;
+          #size-cells = <0>;
+
+          port@0 {
+            reg = <0>;
+            endpoint {
+              remote-endpoint = <&dsi0_out>;
+              data-lanes = <1 2 3 4>;
+            };
+          };
+
+          port@1 {
+            reg = <1>;
+            endpoint {
+              remote-endpoint = <&panel_in_lvds>;
+            };
+          };
+        };
+      };
+    };
-- 
2.30.2

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

[PATCH V2 2/2] drm/bridge: ti-sn65dsi83: Add TI SN65DSI83 and SN65DSI84 driver

[Marek Vasut]

Add driver for TI SN65DSI83 Single-link DSI to Single-link LVDS bridge
and TI SN65DSI84 Single-link DSI to Dual-link or 2x Single-link LVDS
bridge. TI SN65DSI85 is unsupported due to lack of hardware to test on,
but easy to add.

The driver operates the chip via I2C bus. Currently the LVDS clock are
always derived from DSI clock lane, which is the usual mode of operation.
Support for clock from external oscillator is not implemented, but it is
easy to add if ever needed. Only RGB888 pixel format is implemented, the
LVDS666 is not supported, but could be added if needed.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Douglas Anderson <dianders@chromium.org>
Cc: Jagan Teki <jagan@amarulasolutions.com>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Philippe Schenker <philippe.schenker@toradex.com>
Cc: Sam Ravnborg <sam@ravnborg.org>
Cc: Stephen Boyd <swboyd@chromium.org>
Cc: Valentin Raevsky <valentin@compulab.co.il>
To: dri-devel@lists.freedesktop.org
---
V2: - Use dev_err_probe()
    - Set REG_RC_RESET as volatile
    - Wait for PLL stabilization by polling REG_RC_LVDS_PLL
    - Use ctx->mode = *adj instead of *mode in sn65dsi83_mode_set
    - Add tested DSI84 support in dual-link mode
    - Correctly set VCOM
    - Fill in missing DSI CHB and LVDS CHB bits from DSI84 and DSI85
      datasheets, with that all the reserved bits make far more sense
      as the DSI83 and DSI84 seems to be reduced version of DSI85
---
 drivers/gpu/drm/bridge/Kconfig        |  10 +
 drivers/gpu/drm/bridge/Makefile       |   1 +
 drivers/gpu/drm/bridge/ti-sn65dsi83.c | 617 ++++++++++++++++++++++++++
 3 files changed, 628 insertions(+)
 create mode 100644 drivers/gpu/drm/bridge/ti-sn65dsi83.c

diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index cc401786962a..4cd2712f90fd 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -234,6 +234,16 @@ config DRM_TI_TFP410
 	help
 	  Texas Instruments TFP410 DVI/HDMI Transmitter driver
 
+config DRM_TI_SN65DSI83
+	tristate "TI SN65DSI83 and SN65DSI84 DSI to LVDS bridge"
+	depends on OF
+	select DRM_KMS_HELPER
+	select REGMAP_I2C
+	select DRM_PANEL
+	select DRM_MIPI_DSI
+	help
+	  Texas Instruments SN65DSI83 and SN65DSI84 DSI to LVDS Bridge driver
+
 config DRM_TI_SN65DSI86
 	tristate "TI SN65DSI86 DSI to eDP bridge"
 	depends on OF
diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
index 21ea8fe09992..e11415db0a31 100644
--- a/drivers/gpu/drm/bridge/Makefile
+++ b/drivers/gpu/drm/bridge/Makefile
@@ -21,6 +21,7 @@ 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_SN65DSI83) += ti-sn65dsi83.o
 obj-$(CONFIG_DRM_TI_SN65DSI86) += ti-sn65dsi86.o
 obj-$(CONFIG_DRM_TI_TFP410) += ti-tfp410.o
 obj-$(CONFIG_DRM_TI_TPD12S015) += ti-tpd12s015.o
diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
new file mode 100644
index 000000000000..2471ee4c77b6
--- /dev/null
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
@@ -0,0 +1,617 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * TI SN65DSI83,84,85 driver
+ *
+ * Currently supported:
+ * - SN65DSI83
+ *   = 1x Single-link DSI ~ 1x Single-link LVDS
+ *   - Supported
+ *   - Single-link LVDS mode tested
+ * - SN65DSI84
+ *   = 1x Single-link DSI ~ 2x Single-link or 1x Dual-link LVDS
+ *   - Supported
+ *   - Dual-link LVDS mode tested
+ *   - 2x Single-link LVDS mode unsupported
+ *     (should be easy to add by someone who has the HW)
+ * - SN65DSI85
+ *   = 2x Single-link or 1x Dual-link DSI ~ 2x Single-link or 1x Dual-link LVDS
+ *   - Unsupported
+ *     (should be easy to add by someone who has the HW)
+ *
+ * Copyright (C) 2021 Marek Vasut <marex@denx.de>
+ *
+ * Based on previous work of:
+ * Valentin Raevsky <valentin@compulab.co.il>
+ * Philippe Schenker <philippe.schenker@toradex.com>
+ */
+
+#include <linux/bits.h>
+#include <linux/clk.h>
+#include <linux/gpio/consumer.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/of_graph.h>
+#include <linux/regmap.h>
+
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_bridge.h>
+#include <drm/drm_mipi_dsi.h>
+#include <drm/drm_of.h>
+#include <drm/drm_panel.h>
+#include <drm/drm_print.h>
+#include <drm/drm_probe_helper.h>
+
+/* ID registers */
+#define REG_ID(n)				(0x00 + (n))
+/* Reset and clock registers */
+#define REG_RC_RESET				0x09
+#define  REG_RC_RESET_SOFT_RESET		BIT(0)
+#define REG_RC_LVDS_PLL				0x0a
+#define  REG_RC_LVDS_PLL_PLL_EN_STAT		BIT(7)
+#define  REG_RC_LVDS_PLL_LVDS_CLK_RANGE(n)	(((n) & 0x7) << 1)
+#define  REG_RC_LVDS_PLL_HS_CLK_SRC_DPHY	BIT(0)
+#define REG_RC_DSI_CLK				0x0b
+#define  REG_RC_DSI_CLK_DSI_CLK_DIVIDER(n)	(((n) & 0x1f) << 3)
+#define  REG_RC_DSI_CLK_REFCLK_MULTIPLIER(n)	((n) & 0x3)
+#define REG_RC_PLL_EN				0x0d
+#define  REG_RC_PLL_EN_PLL_EN			BIT(0)
+/* DSI registers */
+#define REG_DSI_LANE				0x10
+#define  REG_DSI_LANE_LVDS_LINK_CFG_DUAL	BIT(5) /* dual or 2x single */
+#define  REG_DSI_LANE_CHA_DSI_LANES(n)		(((n) & 0x3) << 3)
+#define  REG_DSI_LANE_CHB_DSI_LANES(n)		(((n) & 0x3) << 1)
+#define  REG_DSI_LANE_SOT_ERR_TOL_DIS		BIT(0)
+#define REG_DSI_EQ				0x11
+#define  REG_DSI_EQ_CHA_DSI_DATA_EQ(n)		(((n) & 0x3) << 6)
+#define  REG_DSI_EQ_CHA_DSI_CLK_EQ(n)		(((n) & 0x3) << 2)
+#define REG_DSI_CLK				0x12
+#define  REG_DSI_CLK_CHA_DSI_CLK_RANGE(n)	((n) & 0xff)
+/* LVDS registers */
+#define REG_LVDS_FMT				0x18
+#define  REG_LVDS_FMT_DE_NEG_POLARITY		BIT(7)
+#define  REG_LVDS_FMT_HS_NEG_POLARITY		BIT(6)
+#define  REG_LVDS_FMT_VS_NEG_POLARITY		BIT(5)
+#define  REG_LVDS_FMT_LVDS_LINK_CFG		BIT(4)	/* 0:AB 1:A-only */
+#define  REG_LVDS_FMT_CHA_24BPP_MODE		BIT(3)
+#define  REG_LVDS_FMT_CHB_24BPP_MODE		BIT(2)
+#define  REG_LVDS_FMT_CHA_24BPP_FORMAT1		BIT(1)
+#define  REG_LVDS_FMT_CHB_24BPP_FORMAT1		BIT(0)
+#define REG_LVDS_VCOM				0x19
+#define  REG_LVDS_VCOM_CHA_LVDS_VOCM		BIT(6)
+#define  REG_LVDS_VCOM_CHB_LVDS_VOCM		BIT(4)
+#define  REG_LVDS_VCOM_CHA_LVDS_VOD_SWING(n)	(((n) & 0x3) << 2)
+#define  REG_LVDS_VCOM_CHB_LVDS_VOD_SWING(n)	((n) & 0x3)
+#define REG_LVDS_LANE				0x1a
+#define  REG_LVDS_LANE_EVEN_ODD_SWAP		BIT(6)
+#define  REG_LVDS_LANE_CHA_REVERSE_LVDS		BIT(5)
+#define  REG_LVDS_LANE_CHB_REVERSE_LVDS		BIT(4)
+#define  REG_LVDS_LANE_CHA_LVDS_TERM		BIT(1)
+#define  REG_LVDS_LANE_CHB_LVDS_TERM		BIT(0)
+#define REG_LVDS_CM				0x1b
+#define  REG_LVDS_CM_CHA_LVDS_CM_ADJUST(n)	(((n) & 0x3) << 4)
+#define  REG_LVDS_CM_CHB_LVDS_CM_ADJUST(n)	((n) & 0x3)
+/* Video registers */
+#define REG_VID_CHA_ACTIVE_LINE_LENGTH_LOW	0x20
+#define REG_VID_CHA_ACTIVE_LINE_LENGTH_HIGH	0x21
+#define REG_VID_CHA_VERTICAL_DISPLAY_SIZE_LOW	0x24
+#define REG_VID_CHA_VERTICAL_DISPLAY_SIZE_HIGH	0x25
+#define REG_VID_CHA_SYNC_DELAY_LOW		0x28
+#define REG_VID_CHA_SYNC_DELAY_HIGH		0x29
+#define REG_VID_CHA_HSYNC_PULSE_WIDTH_LOW	0x2c
+#define REG_VID_CHA_HSYNC_PULSE_WIDTH_HIGH	0x2d
+#define REG_VID_CHA_VSYNC_PULSE_WIDTH_LOW	0x30
+#define REG_VID_CHA_VSYNC_PULSE_WIDTH_HIGH	0x31
+#define REG_VID_CHA_HORIZONTAL_BACK_PORCH	0x34
+#define REG_VID_CHA_VERTICAL_BACK_PORCH		0x36
+#define REG_VID_CHA_HORIZONTAL_FRONT_PORCH	0x38
+#define REG_VID_CHA_VERTICAL_FRONT_PORCH	0x3a
+#define REG_VID_CHA_TEST_PATTERN		0x3c
+/* IRQ registers */
+#define REG_IRQ_GLOBAL				0xe0
+#define  REG_IRQ_GLOBAL_IRQ_EN			BIT(0)
+#define REG_IRQ_EN				0xe1
+#define  REG_IRQ_EN_CHA_SYNCH_ERR_EN		BIT(7)
+#define  REG_IRQ_EN_CHA_CRC_ERR_EN		BIT(6)
+#define  REG_IRQ_EN_CHA_UNC_ECC_ERR_EN		BIT(5)
+#define  REG_IRQ_EN_CHA_COR_ECC_ERR_EN		BIT(4)
+#define  REG_IRQ_EN_CHA_LLP_ERR_EN		BIT(3)
+#define  REG_IRQ_EN_CHA_SOT_BIT_ERR_EN		BIT(2)
+#define  REG_IRQ_EN_CHA_PLL_UNLOCK_EN		BIT(0)
+#define REG_IRQ_STAT				0xe5
+#define  REG_IRQ_STAT_CHA_SYNCH_ERR		BIT(7)
+#define  REG_IRQ_STAT_CHA_CRC_ERR		BIT(6)
+#define  REG_IRQ_STAT_CHA_UNC_ECC_ERR		BIT(5)
+#define  REG_IRQ_STAT_CHA_COR_ECC_ERR		BIT(4)
+#define  REG_IRQ_STAT_CHA_LLP_ERR		BIT(3)
+#define  REG_IRQ_STAT_CHA_SOT_BIT_ERR		BIT(2)
+#define  REG_IRQ_STAT_CHA_PLL_UNLOCK		BIT(0)
+
+enum sn65dsi83_model {
+	MODEL_SN65DSI83,
+	MODEL_SN65DSI84,
+};
+
+struct sn65dsi83 {
+	struct drm_bridge		bridge;
+	struct drm_display_mode		mode;
+	struct device			*dev;
+	struct regmap			*regmap;
+	struct device_node		*host_node;
+	struct mipi_dsi_device		*dsi;
+	struct drm_bridge		*panel_bridge;
+	struct gpio_desc		*enable_gpio;
+	int				dsi_lanes;
+	bool				lvds_dual_link;
+};
+
+static const struct regmap_range sn65dsi83_readable_ranges[] = {
+	regmap_reg_range(REG_ID(0), REG_ID(8)),
+	regmap_reg_range(REG_RC_LVDS_PLL, REG_RC_DSI_CLK),
+	regmap_reg_range(REG_RC_PLL_EN, REG_RC_PLL_EN),
+	regmap_reg_range(REG_DSI_LANE, REG_DSI_CLK),
+	regmap_reg_range(REG_LVDS_FMT, REG_LVDS_CM),
+	regmap_reg_range(REG_VID_CHA_ACTIVE_LINE_LENGTH_LOW,
+			 REG_VID_CHA_ACTIVE_LINE_LENGTH_HIGH),
+	regmap_reg_range(REG_VID_CHA_VERTICAL_DISPLAY_SIZE_LOW,
+			 REG_VID_CHA_VERTICAL_DISPLAY_SIZE_HIGH),
+	regmap_reg_range(REG_VID_CHA_SYNC_DELAY_LOW,
+			 REG_VID_CHA_SYNC_DELAY_HIGH),
+	regmap_reg_range(REG_VID_CHA_HSYNC_PULSE_WIDTH_LOW,
+			 REG_VID_CHA_HSYNC_PULSE_WIDTH_HIGH),
+	regmap_reg_range(REG_VID_CHA_VSYNC_PULSE_WIDTH_LOW,
+			 REG_VID_CHA_VSYNC_PULSE_WIDTH_HIGH),
+	regmap_reg_range(REG_VID_CHA_HORIZONTAL_BACK_PORCH,
+			 REG_VID_CHA_HORIZONTAL_BACK_PORCH),
+	regmap_reg_range(REG_VID_CHA_VERTICAL_BACK_PORCH,
+			 REG_VID_CHA_VERTICAL_BACK_PORCH),
+	regmap_reg_range(REG_VID_CHA_HORIZONTAL_FRONT_PORCH,
+			 REG_VID_CHA_HORIZONTAL_FRONT_PORCH),
+	regmap_reg_range(REG_VID_CHA_VERTICAL_FRONT_PORCH,
+			 REG_VID_CHA_VERTICAL_FRONT_PORCH),
+	regmap_reg_range(REG_VID_CHA_TEST_PATTERN, REG_VID_CHA_TEST_PATTERN),
+	regmap_reg_range(REG_IRQ_GLOBAL, REG_IRQ_EN),
+	regmap_reg_range(REG_IRQ_STAT, REG_IRQ_STAT),
+};
+
+static const struct regmap_access_table sn65dsi83_readable_table = {
+	.yes_ranges = sn65dsi83_readable_ranges,
+	.n_yes_ranges = ARRAY_SIZE(sn65dsi83_readable_ranges),
+};
+
+static const struct regmap_range sn65dsi83_writeable_ranges[] = {
+	regmap_reg_range(REG_RC_RESET, REG_RC_DSI_CLK),
+	regmap_reg_range(REG_RC_PLL_EN, REG_RC_PLL_EN),
+	regmap_reg_range(REG_DSI_LANE, REG_DSI_CLK),
+	regmap_reg_range(REG_LVDS_FMT, REG_LVDS_CM),
+	regmap_reg_range(REG_VID_CHA_ACTIVE_LINE_LENGTH_LOW,
+			 REG_VID_CHA_ACTIVE_LINE_LENGTH_HIGH),
+	regmap_reg_range(REG_VID_CHA_VERTICAL_DISPLAY_SIZE_LOW,
+			 REG_VID_CHA_VERTICAL_DISPLAY_SIZE_HIGH),
+	regmap_reg_range(REG_VID_CHA_SYNC_DELAY_LOW,
+			 REG_VID_CHA_SYNC_DELAY_HIGH),
+	regmap_reg_range(REG_VID_CHA_HSYNC_PULSE_WIDTH_LOW,
+			 REG_VID_CHA_HSYNC_PULSE_WIDTH_HIGH),
+	regmap_reg_range(REG_VID_CHA_VSYNC_PULSE_WIDTH_LOW,
+			 REG_VID_CHA_VSYNC_PULSE_WIDTH_HIGH),
+	regmap_reg_range(REG_VID_CHA_HORIZONTAL_BACK_PORCH,
+			 REG_VID_CHA_HORIZONTAL_BACK_PORCH),
+	regmap_reg_range(REG_VID_CHA_VERTICAL_BACK_PORCH,
+			 REG_VID_CHA_VERTICAL_BACK_PORCH),
+	regmap_reg_range(REG_VID_CHA_HORIZONTAL_FRONT_PORCH,
+			 REG_VID_CHA_HORIZONTAL_FRONT_PORCH),
+	regmap_reg_range(REG_VID_CHA_VERTICAL_FRONT_PORCH,
+			 REG_VID_CHA_VERTICAL_FRONT_PORCH),
+	regmap_reg_range(REG_VID_CHA_TEST_PATTERN, REG_VID_CHA_TEST_PATTERN),
+	regmap_reg_range(REG_IRQ_GLOBAL, REG_IRQ_EN),
+	regmap_reg_range(REG_IRQ_STAT, REG_IRQ_STAT),
+};
+
+static const struct regmap_access_table sn65dsi83_writeable_table = {
+	.yes_ranges = sn65dsi83_writeable_ranges,
+	.n_yes_ranges = ARRAY_SIZE(sn65dsi83_writeable_ranges),
+};
+
+static const struct regmap_range sn65dsi83_volatile_ranges[] = {
+	regmap_reg_range(REG_RC_RESET, REG_RC_RESET),
+	regmap_reg_range(REG_RC_LVDS_PLL, REG_RC_LVDS_PLL),
+	regmap_reg_range(REG_IRQ_STAT, REG_IRQ_STAT),
+};
+
+static const struct regmap_access_table sn65dsi83_volatile_table = {
+	.yes_ranges = sn65dsi83_volatile_ranges,
+	.n_yes_ranges = ARRAY_SIZE(sn65dsi83_volatile_ranges),
+};
+
+static const struct regmap_config sn65dsi83_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.rd_table = &sn65dsi83_readable_table,
+	.wr_table = &sn65dsi83_writeable_table,
+	.volatile_table = &sn65dsi83_volatile_table,
+	.cache_type = REGCACHE_RBTREE,
+	.max_register = REG_IRQ_STAT,
+};
+
+static struct sn65dsi83 *bridge_to_sn65dsi83(struct drm_bridge *bridge)
+{
+	return container_of(bridge, struct sn65dsi83, bridge);
+}
+
+static int sn65dsi83_attach(struct drm_bridge *bridge,
+			    enum drm_bridge_attach_flags flags)
+{
+	struct sn65dsi83 *ctx = bridge_to_sn65dsi83(bridge);
+	struct device *dev = ctx->dev;
+	struct mipi_dsi_device *dsi;
+	struct mipi_dsi_host *host;
+	int ret = 0;
+
+	const struct mipi_dsi_device_info info = {
+		.type = "sn65dsi83",
+		.channel = 0,
+		.node = NULL,
+	};
+
+	host = of_find_mipi_dsi_host_by_node(ctx->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)) {
+		return dev_err_probe(dev, PTR_ERR(dsi),
+				     "failed to create dsi device\n");
+	}
+
+	ctx->dsi = dsi;
+
+	dsi->lanes = ctx->dsi_lanes;
+	dsi->format = MIPI_DSI_FMT_RGB888;
+	dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST;
+
+	ret = mipi_dsi_attach(dsi);
+	if (ret < 0) {
+		dev_err(dev, "failed to attach dsi to host\n");
+		goto err_dsi_attach;
+	}
+
+	return drm_bridge_attach(bridge->encoder, ctx->panel_bridge,
+				 &ctx->bridge, flags);
+
+err_dsi_attach:
+	mipi_dsi_device_unregister(dsi);
+	return ret;
+}
+
+static void sn65dsi83_pre_enable(struct drm_bridge *bridge)
+{
+	struct sn65dsi83 *ctx = bridge_to_sn65dsi83(bridge);
+
+	/*
+	 * Reset the chip, pull EN line low for t_reset=10ms,
+	 * then high for t_en=1ms.
+	 */
+	regcache_mark_dirty(ctx->regmap);
+	gpiod_set_value(ctx->enable_gpio, 0);
+	usleep_range(10000, 11000);
+	gpiod_set_value(ctx->enable_gpio, 1);
+	usleep_range(1000, 1100);
+}
+
+static u8 sn65dsi83_get_lvds_range(struct sn65dsi83 *ctx)
+{
+	/*
+	 * The encoding of the LVDS_CLK_RANGE is as follows:
+	 * 000 - 25 MHz <= LVDS_CLK < 37.5 MHz
+	 * 001 - 37.5 MHz <= LVDS_CLK < 62.5 MHz
+	 * 010 - 62.5 MHz <= LVDS_CLK < 87.5 MHz
+	 * 011 - 87.5 MHz <= LVDS_CLK < 112.5 MHz
+	 * 100 - 112.5 MHz <= LVDS_CLK < 137.5 MHz
+	 * 101 - 137.5 MHz <= LVDS_CLK <= 154 MHz
+	 * which is a range of 12.5MHz..162.5MHz in 50MHz steps, except that
+	 * the ends of the ranges are clamped to the supported range. Since
+	 * sn65dsi83_mode_valid() already filters the valid modes and limits
+	 * the clock to 25..154 MHz, the range calculation can be simplified
+	 * as follows:
+	 */
+	int mode_clock = ctx->mode.clock;
+
+	if (ctx->lvds_dual_link)
+		mode_clock /= 2;
+
+	return (mode_clock - 12500) / 25000;
+}
+
+static u8 sn65dsi83_get_dsi_range(struct sn65dsi83 *ctx)
+{
+	/*
+	 * The encoding of the CHA_DSI_CLK_RANGE is as follows:
+	 * 0x00 through 0x07 - Reserved
+	 * 0x08 - 40 <= DSI_CLK < 45 MHz
+	 * 0x09 - 45 <= DSI_CLK < 50 MHz
+	 * ...
+	 * 0x63 - 495 <= DSI_CLK < 500 MHz
+	 * 0x64 - 500 MHz
+	 * 0x65 through 0xFF - Reserved
+	 * which is DSI clock in 5 MHz steps, clamped to 40..500 MHz.
+	 * The DSI clock are calculated as:
+	 *  DSI_CLK = mode clock * bpp / dsi_data_lanes / 2
+	 * the 2 is there because the bus is DDR.
+	 */
+	return DIV_ROUND_UP(clamp((unsigned int)ctx->mode.clock *
+			    mipi_dsi_pixel_format_to_bpp(ctx->dsi->format) /
+			    ctx->dsi_lanes / 2, 40000U, 500000U), 5000U);
+}
+
+static u8 sn65dsi83_get_dsi_div(struct sn65dsi83 *ctx)
+{
+	/* The divider is (DSI_CLK / LVDS_CLK) - 1, which really is: */
+	unsigned int dsi_div = mipi_dsi_pixel_format_to_bpp(ctx->dsi->format);
+
+	dsi_div /= ctx->dsi_lanes;
+
+	if (!ctx->lvds_dual_link)
+		dsi_div /= 2;
+
+	return dsi_div - 1;
+}
+
+static void sn65dsi83_enable(struct drm_bridge *bridge)
+{
+	struct sn65dsi83 *ctx = bridge_to_sn65dsi83(bridge);
+	unsigned int pval;
+	u16 val;
+	int ret;
+
+	/* Clear reset, disable PLL */
+	regmap_write(ctx->regmap, REG_RC_RESET, 0x00);
+	regmap_write(ctx->regmap, REG_RC_PLL_EN, 0x00);
+
+	/* Reference clock derived from DSI link clock. */
+	regmap_write(ctx->regmap, REG_RC_LVDS_PLL,
+		REG_RC_LVDS_PLL_LVDS_CLK_RANGE(sn65dsi83_get_lvds_range(ctx)) |
+		REG_RC_LVDS_PLL_HS_CLK_SRC_DPHY);
+	regmap_write(ctx->regmap, REG_DSI_CLK,
+		REG_DSI_CLK_CHA_DSI_CLK_RANGE(sn65dsi83_get_dsi_range(ctx)));
+	regmap_write(ctx->regmap, REG_RC_DSI_CLK,
+		REG_RC_DSI_CLK_DSI_CLK_DIVIDER(sn65dsi83_get_dsi_div(ctx)));
+
+	/* Set number of DSI lanes and LVDS link config. */
+	regmap_write(ctx->regmap, REG_DSI_LANE,
+		REG_DSI_LANE_LVDS_LINK_CFG_DUAL |
+		REG_DSI_LANE_CHA_DSI_LANES(~(ctx->dsi_lanes - 1)) |
+		/* CHB is DSI85-only, set to default on DSI83/DSI84 */
+		REG_DSI_LANE_CHB_DSI_LANES(3));
+	/* No equalization. */
+	regmap_write(ctx->regmap, REG_DSI_EQ, 0x00);
+
+	/* RGB888 is the only format supported so far. */
+	val = (ctx->mode.flags & DRM_MODE_FLAG_NHSYNC ?
+	       REG_LVDS_FMT_HS_NEG_POLARITY : 0) |
+	      (ctx->mode.flags & DRM_MODE_FLAG_NVSYNC ?
+	       REG_LVDS_FMT_VS_NEG_POLARITY : 0) |
+	      REG_LVDS_FMT_CHA_24BPP_MODE;
+	if (ctx->lvds_dual_link)
+		val |= REG_LVDS_FMT_CHB_24BPP_MODE;
+	else
+		val |= REG_LVDS_FMT_LVDS_LINK_CFG;
+
+	regmap_write(ctx->regmap, REG_LVDS_FMT, val);
+	regmap_write(ctx->regmap, REG_LVDS_VCOM, 0x05);
+	regmap_write(ctx->regmap, REG_LVDS_LANE,
+		REG_LVDS_LANE_CHA_LVDS_TERM |
+		REG_LVDS_LANE_CHB_LVDS_TERM);
+	regmap_write(ctx->regmap, REG_LVDS_CM, 0x00);
+
+	regmap_bulk_write(ctx->regmap, REG_VID_CHA_ACTIVE_LINE_LENGTH_LOW,
+			  &ctx->mode.hdisplay, 2);
+	regmap_bulk_write(ctx->regmap, REG_VID_CHA_VERTICAL_DISPLAY_SIZE_LOW,
+			  &ctx->mode.vdisplay, 2);
+	val = 32 + 1;	/* 32 + 1 pixel clock to ensure proper operation */
+	regmap_bulk_write(ctx->regmap, REG_VID_CHA_SYNC_DELAY_LOW, &val, 2);
+	val = ctx->mode.hsync_end - ctx->mode.hsync_start;
+	regmap_bulk_write(ctx->regmap, REG_VID_CHA_HSYNC_PULSE_WIDTH_LOW,
+			  &val, 2);
+	val = ctx->mode.vsync_end - ctx->mode.vsync_start;
+	regmap_bulk_write(ctx->regmap, REG_VID_CHA_VSYNC_PULSE_WIDTH_LOW,
+			  &val, 2);
+	regmap_write(ctx->regmap, REG_VID_CHA_HORIZONTAL_BACK_PORCH,
+		     ctx->mode.htotal - ctx->mode.hsync_end);
+	regmap_write(ctx->regmap, REG_VID_CHA_VERTICAL_BACK_PORCH,
+		     ctx->mode.vtotal - ctx->mode.vsync_end);
+	regmap_write(ctx->regmap, REG_VID_CHA_HORIZONTAL_FRONT_PORCH,
+		     ctx->mode.hsync_start - ctx->mode.hdisplay);
+	regmap_write(ctx->regmap, REG_VID_CHA_VERTICAL_FRONT_PORCH,
+		     ctx->mode.vsync_start - ctx->mode.vdisplay);
+	regmap_write(ctx->regmap, REG_VID_CHA_TEST_PATTERN, 0x00);
+
+	/* Enable PLL */
+	regmap_write(ctx->regmap, REG_RC_PLL_EN, REG_RC_PLL_EN_PLL_EN);
+	usleep_range(3000, 4000);
+	ret = regmap_read_poll_timeout(ctx->regmap, REG_RC_LVDS_PLL, pval,
+					pval & REG_RC_LVDS_PLL_PLL_EN_STAT,
+					1000, 100000);
+	if (ret) {
+		dev_err(ctx->dev, "failed to lock PLL, ret=%i\n", ret);
+		/* On failure, disable PLL again and exit. */
+		regmap_write(ctx->regmap, REG_RC_PLL_EN, 0x00);
+		return;
+	}
+
+	/* Trigger reset after CSR register update. */
+	regmap_write(ctx->regmap, REG_RC_RESET, REG_RC_RESET_SOFT_RESET);
+
+	/* Clear all errors that got asserted during initialization. */
+	regmap_read(ctx->regmap, REG_IRQ_STAT, &pval);
+	regmap_write(ctx->regmap, REG_IRQ_STAT, pval);
+}
+
+static void sn65dsi83_disable(struct drm_bridge *bridge)
+{
+	struct sn65dsi83 *ctx = bridge_to_sn65dsi83(bridge);
+
+	/* Clear reset, disable PLL */
+	regmap_write(ctx->regmap, REG_RC_RESET, 0x00);
+	regmap_write(ctx->regmap, REG_RC_PLL_EN, 0x00);
+}
+
+static void sn65dsi83_post_disable(struct drm_bridge *bridge)
+{
+	struct sn65dsi83 *ctx = bridge_to_sn65dsi83(bridge);
+
+	/* Put the chip in reset, pull EN line low. */
+	gpiod_set_value(ctx->enable_gpio, 0);
+}
+
+static enum drm_mode_status
+sn65dsi83_mode_valid(struct drm_bridge *bridge,
+		     const struct drm_display_info *info,
+		     const struct drm_display_mode *mode)
+{
+	/* LVDS output clock range 25..154 MHz */
+	if (mode->clock < 25000)
+		return MODE_CLOCK_LOW;
+	if (mode->clock > 154000)
+		return MODE_CLOCK_HIGH;
+
+	return MODE_OK;
+}
+
+static void sn65dsi83_mode_set(struct drm_bridge *bridge,
+			       const struct drm_display_mode *mode,
+			       const struct drm_display_mode *adj)
+{
+	struct sn65dsi83 *ctx = bridge_to_sn65dsi83(bridge);
+
+	ctx->mode = *adj;
+}
+
+static const struct drm_bridge_funcs sn65dsi83_funcs = {
+	.attach		= sn65dsi83_attach,
+	.pre_enable	= sn65dsi83_pre_enable,
+	.enable		= sn65dsi83_enable,
+	.disable	= sn65dsi83_disable,
+	.post_disable	= sn65dsi83_post_disable,
+	.mode_valid	= sn65dsi83_mode_valid,
+	.mode_set	= sn65dsi83_mode_set,
+};
+
+static int sn65dsi83_parse_dt(struct sn65dsi83 *ctx)
+{
+	struct drm_bridge *panel_bridge;
+	struct device *dev = ctx->dev;
+	struct device_node *endpoint;
+	struct drm_panel *panel;
+	int ret;
+
+	endpoint = of_graph_get_endpoint_by_regs(dev->of_node, 0, 0);
+	ctx->dsi_lanes = of_property_count_u32_elems(endpoint, "data-lanes");
+	ctx->host_node = of_graph_get_remote_port_parent(endpoint);
+	of_node_put(endpoint);
+
+	if (ctx->dsi_lanes < 0 || ctx->dsi_lanes > 4)
+		return -EINVAL;
+	if (!ctx->host_node)
+		return -ENODEV;
+
+	ret = drm_of_find_panel_or_bridge(dev->of_node, 1, 0, &panel, &panel_bridge);
+	if (ret < 0)
+		return ret;
+	if (panel) {
+		panel_bridge = devm_drm_panel_bridge_add(dev, panel);
+		if (IS_ERR(panel_bridge))
+			return PTR_ERR(panel_bridge);
+	}
+
+	ctx->panel_bridge = panel_bridge;
+
+	return 0;
+}
+
+static int sn65dsi83_probe(struct i2c_client *client,
+			   const struct i2c_device_id *id)
+{
+	struct device *dev = &client->dev;
+	enum sn65dsi83_model model;
+	struct sn65dsi83 *ctx;
+	int ret;
+
+	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
+	if (!ctx)
+		return -ENOMEM;
+
+	ctx->dev = dev;
+
+	if (dev->of_node)
+		model = (enum sn65dsi83_model)of_device_get_match_data(dev);
+	else
+		model = id->driver_data;
+
+	/* Default to dual-link LVDS on all but DSI83. */
+	if (model != MODEL_SN65DSI83)
+		ctx->lvds_dual_link = true;
+
+	ctx->enable_gpio = devm_gpiod_get(ctx->dev, "enable", GPIOD_OUT_LOW);
+	if (IS_ERR(ctx->enable_gpio))
+		return PTR_ERR(ctx->enable_gpio);
+
+	ret = sn65dsi83_parse_dt(ctx);
+	if (ret)
+		return ret;
+
+	ctx->regmap = devm_regmap_init_i2c(client, &sn65dsi83_regmap_config);
+	if (IS_ERR(ctx->regmap))
+		return PTR_ERR(ctx->regmap);
+
+	dev_set_drvdata(dev, ctx);
+	i2c_set_clientdata(client, ctx);
+
+	ctx->bridge.funcs = &sn65dsi83_funcs;
+	ctx->bridge.of_node = dev->of_node;
+	drm_bridge_add(&ctx->bridge);
+
+	return 0;
+}
+
+static int sn65dsi83_remove(struct i2c_client *client)
+{
+	struct sn65dsi83 *ctx = i2c_get_clientdata(client);
+
+	mipi_dsi_detach(ctx->dsi);
+	mipi_dsi_device_unregister(ctx->dsi);
+	drm_bridge_remove(&ctx->bridge);
+	of_node_put(ctx->host_node);
+
+	return 0;
+}
+
+static struct i2c_device_id sn65dsi83_id[] = {
+	{ "ti,sn65dsi83", MODEL_SN65DSI83 },
+	{ "ti,sn65dsi84", MODEL_SN65DSI84 },
+	{},
+};
+MODULE_DEVICE_TABLE(i2c, sn65dsi83_id);
+
+static const struct of_device_id sn65dsi83_match_table[] = {
+	{ .compatible = "ti,sn65dsi83", .data = (void *)MODEL_SN65DSI83 },
+	{ .compatible = "ti,sn65dsi84", .data = (void *)MODEL_SN65DSI84 },
+	{},
+};
+MODULE_DEVICE_TABLE(of, sn65dsi83_match_table);
+
+static struct i2c_driver sn65dsi83_driver = {
+	.probe = sn65dsi83_probe,
+	.remove = sn65dsi83_remove,
+	.id_table = sn65dsi83_id,
+	.driver = {
+		.name = "sn65dsi83",
+		.of_match_table = sn65dsi83_match_table,
+	},
+};
+module_i2c_driver(sn65dsi83_driver);
+
+MODULE_AUTHOR("Marek Vasut <marex@denx.de>");
+MODULE_DESCRIPTION("TI SN65DSI83 DSI to LVDS bridge driver");
+MODULE_LICENSE("GPL v2");
-- 
2.30.2

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

Re: [PATCH V2 2/2] drm/bridge: ti-sn65dsi83: Add TI SN65DSI83 and SN65DSI84 driver

[Loic Poulain]

On 22/04/2021 00:31, Marek Vasut wrote:
> Add driver for TI SN65DSI83 Single-link DSI to Single-link LVDS bridge
> and TI SN65DSI84 Single-link DSI to Dual-link or 2x Single-link LVDS
> bridge. TI SN65DSI85 is unsupported due to lack of hardware to test on,
> but easy to add.
> 
> The driver operates the chip via I2C bus. Currently the LVDS clock are
> always derived from DSI clock lane, which is the usual mode of operation.
> Support for clock from external oscillator is not implemented, but it is
> easy to add if ever needed. Only RGB888 pixel format is implemented, the
> LVDS666 is not supported, but could be added if needed.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Douglas Anderson <dianders@chromium.org>
> Cc: Jagan Teki <jagan@amarulasolutions.com>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Philippe Schenker <philippe.schenker@toradex.com>
> Cc: Sam Ravnborg <sam@ravnborg.org>
> Cc: Stephen Boyd <swboyd@chromium.org>
> Cc: Valentin Raevsky <valentin@compulab.co.il>
> To: dri-devel@lists.freedesktop.org
> ---
> V2: - Use dev_err_probe()
>      - Set REG_RC_RESET as volatile
>      - Wait for PLL stabilization by polling REG_RC_LVDS_PLL
>      - Use ctx->mode = *adj instead of *mode in sn65dsi83_mode_set
>      - Add tested DSI84 support in dual-link mode
>      - Correctly set VCOM
>      - Fill in missing DSI CHB and LVDS CHB bits from DSI84 and DSI85
>        datasheets, with that all the reserved bits make far more sense
>        as the DSI83 and DSI84 seems to be reduced version of DSI85
> ---
>   drivers/gpu/drm/bridge/Kconfig        |  10 +
>   drivers/gpu/drm/bridge/Makefile       |   1 +
>   drivers/gpu/drm/bridge/ti-sn65dsi83.c | 617 ++++++++++++++++++++++++++
>   3 files changed, 628 insertions(+)
>   create mode 100644 drivers/gpu/drm/bridge/ti-sn65dsi83.c

Tested on MSC-SM2S-IMX8MINI module with a 1024x768 (VESA-24) single LVDS 
channel panel.

Tested-by: Loic Poulain <loic.poulain@linaro.org>

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

Re: [PATCH V2 2/2] drm/bridge: ti-sn65dsi83: Add TI SN65DSI83 and SN65DSI84 driver

[Frieder Schrempf]

On 22.04.21 00:31, Marek Vasut wrote:
> Add driver for TI SN65DSI83 Single-link DSI to Single-link LVDS bridge
> and TI SN65DSI84 Single-link DSI to Dual-link or 2x Single-link LVDS
> bridge. TI SN65DSI85 is unsupported due to lack of hardware to test on,
> but easy to add.
> 
> The driver operates the chip via I2C bus. Currently the LVDS clock are
> always derived from DSI clock lane, which is the usual mode of operation.
> Support for clock from external oscillator is not implemented, but it is
> easy to add if ever needed. Only RGB888 pixel format is implemented, the
> LVDS666 is not supported, but could be added if needed.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Douglas Anderson <dianders@chromium.org>
> Cc: Jagan Teki <jagan@amarulasolutions.com>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Philippe Schenker <philippe.schenker@toradex.com>
> Cc: Sam Ravnborg <sam@ravnborg.org>
> Cc: Stephen Boyd <swboyd@chromium.org>
> Cc: Valentin Raevsky <valentin@compulab.co.il>
> To: dri-devel@lists.freedesktop.org
> Tested-by: Loic Poulain <loic.poulain@linaro.org>
> ---
> V2: - Use dev_err_probe()
>      - Set REG_RC_RESET as volatile
>      - Wait for PLL stabilization by polling REG_RC_LVDS_PLL
>      - Use ctx->mode = *adj instead of *mode in sn65dsi83_mode_set
>      - Add tested DSI84 support in dual-link mode
>      - Correctly set VCOM
>      - Fill in missing DSI CHB and LVDS CHB bits from DSI84 and DSI85
>        datasheets, with that all the reserved bits make far more sense
>        as the DSI83 and DSI84 seems to be reduced version of DSI85
> ---
>   drivers/gpu/drm/bridge/Kconfig        |  10 +
>   drivers/gpu/drm/bridge/Makefile       |   1 +
>   drivers/gpu/drm/bridge/ti-sn65dsi83.c | 617 ++++++++++++++++++++++++++
>   3 files changed, 628 insertions(+)
>   create mode 100644 drivers/gpu/drm/bridge/ti-sn65dsi83.c
> 
[...]
> +static int sn65dsi83_probe(struct i2c_client *client,
> +			   const struct i2c_device_id *id)
> +{
> +	struct device *dev = &client->dev;
> +	enum sn65dsi83_model model;
> +	struct sn65dsi83 *ctx;
> +	int ret;
> +
> +	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
> +	if (!ctx)
> +		return -ENOMEM;
> +
> +	ctx->dev = dev;
> +
> +	if (dev->of_node)
> +		model = (enum sn65dsi83_model)of_device_get_match_data(dev);
> +	else
> +		model = id->driver_data;
> +
> +	/* Default to dual-link LVDS on all but DSI83. */
> +	if (model != MODEL_SN65DSI83)
> +		ctx->lvds_dual_link = true;

What if I use the DSI84 with a single link LVDS? I can't see any way to 
configure that right now.

> +
> +	ctx->enable_gpio = devm_gpiod_get(ctx->dev, "enable", GPIOD_OUT_LOW);
> +	if (IS_ERR(ctx->enable_gpio))
> +		return PTR_ERR(ctx->enable_gpio);
> +
> +	ret = sn65dsi83_parse_dt(ctx);
> +	if (ret)
> +		return ret;
> +
> +	ctx->regmap = devm_regmap_init_i2c(client, &sn65dsi83_regmap_config);
> +	if (IS_ERR(ctx->regmap))
> +		return PTR_ERR(ctx->regmap);
> +
> +	dev_set_drvdata(dev, ctx);
> +	i2c_set_clientdata(client, ctx);
> +
> +	ctx->bridge.funcs = &sn65dsi83_funcs;
> +	ctx->bridge.of_node = dev->of_node;
> +	drm_bridge_add(&ctx->bridge);
> +
> +	return 0;
> +}
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH V2 2/2] drm/bridge: ti-sn65dsi83: Add TI SN65DSI83 and SN65DSI84 driver

[Frieder Schrempf]

On 28.04.21 09:51, Frieder Schrempf wrote:
> On 22.04.21 00:31, Marek Vasut wrote:
>> Add driver for TI SN65DSI83 Single-link DSI to Single-link LVDS bridge
>> and TI SN65DSI84 Single-link DSI to Dual-link or 2x Single-link LVDS
>> bridge. TI SN65DSI85 is unsupported due to lack of hardware to test on,
>> but easy to add.
>>
>> The driver operates the chip via I2C bus. Currently the LVDS clock are
>> always derived from DSI clock lane, which is the usual mode of operation.
>> Support for clock from external oscillator is not implemented, but it is
>> easy to add if ever needed. Only RGB888 pixel format is implemented, the
>> LVDS666 is not supported, but could be added if needed.
>>
>> Signed-off-by: Marek Vasut <marex@denx.de>
>> Cc: Douglas Anderson <dianders@chromium.org>
>> Cc: Jagan Teki <jagan@amarulasolutions.com>
>> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>> Cc: Linus Walleij <linus.walleij@linaro.org>
>> Cc: Philippe Schenker <philippe.schenker@toradex.com>
>> Cc: Sam Ravnborg <sam@ravnborg.org>
>> Cc: Stephen Boyd <swboyd@chromium.org>
>> Cc: Valentin Raevsky <valentin@compulab.co.il>
>> To: dri-devel@lists.freedesktop.org
>> Tested-by: Loic Poulain <loic.poulain@linaro.org>
>> ---
>> V2: - Use dev_err_probe()
>>      - Set REG_RC_RESET as volatile
>>      - Wait for PLL stabilization by polling REG_RC_LVDS_PLL
>>      - Use ctx->mode = *adj instead of *mode in sn65dsi83_mode_set
>>      - Add tested DSI84 support in dual-link mode
>>      - Correctly set VCOM
>>      - Fill in missing DSI CHB and LVDS CHB bits from DSI84 and DSI85
>>        datasheets, with that all the reserved bits make far more sense
>>        as the DSI83 and DSI84 seems to be reduced version of DSI85
>> ---
>>   drivers/gpu/drm/bridge/Kconfig        |  10 +
>>   drivers/gpu/drm/bridge/Makefile       |   1 +
>>   drivers/gpu/drm/bridge/ti-sn65dsi83.c | 617 ++++++++++++++++++++++++++
>>   3 files changed, 628 insertions(+)
>>   create mode 100644 drivers/gpu/drm/bridge/ti-sn65dsi83.c
>>
> [...]
>> +static int sn65dsi83_probe(struct i2c_client *client,
>> +               const struct i2c_device_id *id)
>> +{
>> +    struct device *dev = &client->dev;
>> +    enum sn65dsi83_model model;
>> +    struct sn65dsi83 *ctx;
>> +    int ret;
>> +
>> +    ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
>> +    if (!ctx)
>> +        return -ENOMEM;
>> +
>> +    ctx->dev = dev;
>> +
>> +    if (dev->of_node)
>> +        model = (enum sn65dsi83_model)of_device_get_match_data(dev);
>> +    else
>> +        model = id->driver_data;
>> +
>> +    /* Default to dual-link LVDS on all but DSI83. */
>> +    if (model != MODEL_SN65DSI83)
>> +        ctx->lvds_dual_link = true;
> 
> What if I use the DSI84 with a single link LVDS? I can't see any way to 
> configure that right now.

I just saw the note in the header of the driver that says that single 
link mode is unsupported for the DSI84.

I have hardware with a single link display and if I set 
ctx->lvds_dual_link = false it works just fine.

How is this supposed to be selected? Does it need an extra devicetree 
property? And would you mind adding single-link support in the next 
version or do you prefer adding it in a follow-up patch?

> 
>> +
>> +    ctx->enable_gpio = devm_gpiod_get(ctx->dev, "enable", 
>> GPIOD_OUT_LOW);
>> +    if (IS_ERR(ctx->enable_gpio))
>> +        return PTR_ERR(ctx->enable_gpio);
>> +
>> +    ret = sn65dsi83_parse_dt(ctx);
>> +    if (ret)
>> +        return ret;
>> +
>> +    ctx->regmap = devm_regmap_init_i2c(client, 
>> &sn65dsi83_regmap_config);
>> +    if (IS_ERR(ctx->regmap))
>> +        return PTR_ERR(ctx->regmap);
>> +
>> +    dev_set_drvdata(dev, ctx);
>> +    i2c_set_clientdata(client, ctx);
>> +
>> +    ctx->bridge.funcs = &sn65dsi83_funcs;
>> +    ctx->bridge.of_node = dev->of_node;
>> +    drm_bridge_add(&ctx->bridge);
>> +
>> +    return 0;
>> +}
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH V2 2/2] drm/bridge: ti-sn65dsi83: Add TI SN65DSI83 and SN65DSI84 driver

[Loic Poulain]

On Wed, 28 Apr 2021 at 10:13, Frieder Schrempf
<frieder.schrempf@kontron.de> wrote:
>
> On 28.04.21 09:51, Frieder Schrempf wrote:
> > On 22.04.21 00:31, Marek Vasut wrote:
> >> Add driver for TI SN65DSI83 Single-link DSI to Single-link LVDS bridge
> >> and TI SN65DSI84 Single-link DSI to Dual-link or 2x Single-link LVDS
> >> bridge. TI SN65DSI85 is unsupported due to lack of hardware to test on,
> >> but easy to add.
> >>
> >> The driver operates the chip via I2C bus. Currently the LVDS clock are
> >> always derived from DSI clock lane, which is the usual mode of operation.
> >> Support for clock from external oscillator is not implemented, but it is
> >> easy to add if ever needed. Only RGB888 pixel format is implemented, the
> >> LVDS666 is not supported, but could be added if needed.
> >>
> >> Signed-off-by: Marek Vasut <marex@denx.de>
> >> Cc: Douglas Anderson <dianders@chromium.org>
> >> Cc: Jagan Teki <jagan@amarulasolutions.com>
> >> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >> Cc: Linus Walleij <linus.walleij@linaro.org>
> >> Cc: Philippe Schenker <philippe.schenker@toradex.com>
> >> Cc: Sam Ravnborg <sam@ravnborg.org>
> >> Cc: Stephen Boyd <swboyd@chromium.org>
> >> Cc: Valentin Raevsky <valentin@compulab.co.il>
> >> To: dri-devel@lists.freedesktop.org
> >> Tested-by: Loic Poulain <loic.poulain@linaro.org>
> >> ---
> >> V2: - Use dev_err_probe()
> >>      - Set REG_RC_RESET as volatile
> >>      - Wait for PLL stabilization by polling REG_RC_LVDS_PLL
> >>      - Use ctx->mode = *adj instead of *mode in sn65dsi83_mode_set
> >>      - Add tested DSI84 support in dual-link mode
> >>      - Correctly set VCOM
> >>      - Fill in missing DSI CHB and LVDS CHB bits from DSI84 and DSI85
> >>        datasheets, with that all the reserved bits make far more sense
> >>        as the DSI83 and DSI84 seems to be reduced version of DSI85
> >> ---
> >>   drivers/gpu/drm/bridge/Kconfig        |  10 +
> >>   drivers/gpu/drm/bridge/Makefile       |   1 +
> >>   drivers/gpu/drm/bridge/ti-sn65dsi83.c | 617 ++++++++++++++++++++++++++
> >>   3 files changed, 628 insertions(+)
> >>   create mode 100644 drivers/gpu/drm/bridge/ti-sn65dsi83.c
> >>
> > [...]
> >> +static int sn65dsi83_probe(struct i2c_client *client,
> >> +               const struct i2c_device_id *id)
> >> +{
> >> +    struct device *dev = &client->dev;
> >> +    enum sn65dsi83_model model;
> >> +    struct sn65dsi83 *ctx;
> >> +    int ret;
> >> +
> >> +    ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
> >> +    if (!ctx)
> >> +        return -ENOMEM;
> >> +
> >> +    ctx->dev = dev;
> >> +
> >> +    if (dev->of_node)
> >> +        model = (enum sn65dsi83_model)of_device_get_match_data(dev);
> >> +    else
> >> +        model = id->driver_data;
> >> +
> >> +    /* Default to dual-link LVDS on all but DSI83. */
> >> +    if (model != MODEL_SN65DSI83)
> >> +        ctx->lvds_dual_link = true;
> >
> > What if I use the DSI84 with a single link LVDS? I can't see any way to
> > configure that right now.

I assume the simplest way would be to use the "ti,sn65dsi83"
compatible string in your dts, since the way you wired it is
'compatible' with sn65dsi83, right?

>
> I just saw the note in the header of the driver that says that single
> link mode is unsupported for the DSI84.
>
> I have hardware with a single link display and if I set
> ctx->lvds_dual_link = false it works just fine.
>
> How is this supposed to be selected? Does it need an extra devicetree
> property? And would you mind adding single-link support in the next
> version or do you prefer adding it in a follow-up patch?

If this has to be supported I think the proper way would be to support
two output ports in the dts (e.g. lvds0_out, lvds1_out), in the same
way as supported by the 'advantech,idk-2121wr' panel.

Regards,
Loic
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH V2 2/2] drm/bridge: ti-sn65dsi83: Add TI SN65DSI83 and SN65DSI84 driver

[Neil Armstrong]

On 28/04/2021 11:26, Loic Poulain wrote:
> On Wed, 28 Apr 2021 at 10:13, Frieder Schrempf
> <frieder.schrempf@kontron.de> wrote:
>>
>> On 28.04.21 09:51, Frieder Schrempf wrote:
>>> On 22.04.21 00:31, Marek Vasut wrote:
>>>> Add driver for TI SN65DSI83 Single-link DSI to Single-link LVDS bridge
>>>> and TI SN65DSI84 Single-link DSI to Dual-link or 2x Single-link LVDS
>>>> bridge. TI SN65DSI85 is unsupported due to lack of hardware to test on,
>>>> but easy to add.
>>>>
>>>> The driver operates the chip via I2C bus. Currently the LVDS clock are
>>>> always derived from DSI clock lane, which is the usual mode of operation.
>>>> Support for clock from external oscillator is not implemented, but it is
>>>> easy to add if ever needed. Only RGB888 pixel format is implemented, the
>>>> LVDS666 is not supported, but could be added if needed.
>>>>
>>>> Signed-off-by: Marek Vasut <marex@denx.de>
>>>> Cc: Douglas Anderson <dianders@chromium.org>
>>>> Cc: Jagan Teki <jagan@amarulasolutions.com>
>>>> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>>> Cc: Linus Walleij <linus.walleij@linaro.org>
>>>> Cc: Philippe Schenker <philippe.schenker@toradex.com>
>>>> Cc: Sam Ravnborg <sam@ravnborg.org>
>>>> Cc: Stephen Boyd <swboyd@chromium.org>
>>>> Cc: Valentin Raevsky <valentin@compulab.co.il>
>>>> To: dri-devel@lists.freedesktop.org
>>>> Tested-by: Loic Poulain <loic.poulain@linaro.org>
>>>> ---
>>>> V2: - Use dev_err_probe()
>>>>      - Set REG_RC_RESET as volatile
>>>>      - Wait for PLL stabilization by polling REG_RC_LVDS_PLL
>>>>      - Use ctx->mode = *adj instead of *mode in sn65dsi83_mode_set
>>>>      - Add tested DSI84 support in dual-link mode
>>>>      - Correctly set VCOM
>>>>      - Fill in missing DSI CHB and LVDS CHB bits from DSI84 and DSI85
>>>>        datasheets, with that all the reserved bits make far more sense
>>>>        as the DSI83 and DSI84 seems to be reduced version of DSI85
>>>> ---
>>>>   drivers/gpu/drm/bridge/Kconfig        |  10 +
>>>>   drivers/gpu/drm/bridge/Makefile       |   1 +
>>>>   drivers/gpu/drm/bridge/ti-sn65dsi83.c | 617 ++++++++++++++++++++++++++
>>>>   3 files changed, 628 insertions(+)
>>>>   create mode 100644 drivers/gpu/drm/bridge/ti-sn65dsi83.c
>>>>
>>> [...]
>>>> +static int sn65dsi83_probe(struct i2c_client *client,
>>>> +               const struct i2c_device_id *id)
>>>> +{
>>>> +    struct device *dev = &client->dev;
>>>> +    enum sn65dsi83_model model;
>>>> +    struct sn65dsi83 *ctx;
>>>> +    int ret;
>>>> +
>>>> +    ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
>>>> +    if (!ctx)
>>>> +        return -ENOMEM;
>>>> +
>>>> +    ctx->dev = dev;
>>>> +
>>>> +    if (dev->of_node)
>>>> +        model = (enum sn65dsi83_model)of_device_get_match_data(dev);
>>>> +    else
>>>> +        model = id->driver_data;
>>>> +
>>>> +    /* Default to dual-link LVDS on all but DSI83. */
>>>> +    if (model != MODEL_SN65DSI83)
>>>> +        ctx->lvds_dual_link = true;
>>>
>>> What if I use the DSI84 with a single link LVDS? I can't see any way to
>>> configure that right now.
> 
> I assume the simplest way would be to use the "ti,sn65dsi83"
> compatible string in your dts, since the way you wired it is
> 'compatible' with sn65dsi83, right?

No this isn't the right way to to, if sn65dsi84 is supported and the bindings only support single lvds link,
the driver must only support single link on sn65dsi84, or add the dual link lvds in the bindings only for sn65dsi84.

> 
>>
>> I just saw the note in the header of the driver that says that single
>> link mode is unsupported for the DSI84.
>>
>> I have hardware with a single link display and if I set
>> ctx->lvds_dual_link = false it works just fine.
>>
>> How is this supposed to be selected? Does it need an extra devicetree
>> property? And would you mind adding single-link support in the next
>> version or do you prefer adding it in a follow-up patch?
> 
> If this has to be supported I think the proper way would be to support
> two output ports in the dts (e.g. lvds0_out, lvds1_out), in the same
> way as supported by the 'advantech,idk-2121wr' panel.

Yes, this is why I asked to have the dual-link lvds in the bindings.

Neil

> 
> Regards,
> Loic
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

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

Re: [PATCH V2 2/2] drm/bridge: ti-sn65dsi83: Add TI SN65DSI83 and SN65DSI84 driver

[Jagan Teki]

On Wed, Apr 28, 2021 at 2:54 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
>
> On 28/04/2021 11:26, Loic Poulain wrote:
> > On Wed, 28 Apr 2021 at 10:13, Frieder Schrempf
> > <frieder.schrempf@kontron.de> wrote:
> >>
> >> On 28.04.21 09:51, Frieder Schrempf wrote:
> >>> On 22.04.21 00:31, Marek Vasut wrote:
> >>>> Add driver for TI SN65DSI83 Single-link DSI to Single-link LVDS bridge
> >>>> and TI SN65DSI84 Single-link DSI to Dual-link or 2x Single-link LVDS
> >>>> bridge. TI SN65DSI85 is unsupported due to lack of hardware to test on,
> >>>> but easy to add.
> >>>>
> >>>> The driver operates the chip via I2C bus. Currently the LVDS clock are
> >>>> always derived from DSI clock lane, which is the usual mode of operation.
> >>>> Support for clock from external oscillator is not implemented, but it is
> >>>> easy to add if ever needed. Only RGB888 pixel format is implemented, the
> >>>> LVDS666 is not supported, but could be added if needed.
> >>>>
> >>>> Signed-off-by: Marek Vasut <marex@denx.de>
> >>>> Cc: Douglas Anderson <dianders@chromium.org>
> >>>> Cc: Jagan Teki <jagan@amarulasolutions.com>
> >>>> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >>>> Cc: Linus Walleij <linus.walleij@linaro.org>
> >>>> Cc: Philippe Schenker <philippe.schenker@toradex.com>
> >>>> Cc: Sam Ravnborg <sam@ravnborg.org>
> >>>> Cc: Stephen Boyd <swboyd@chromium.org>
> >>>> Cc: Valentin Raevsky <valentin@compulab.co.il>
> >>>> To: dri-devel@lists.freedesktop.org
> >>>> Tested-by: Loic Poulain <loic.poulain@linaro.org>
> >>>> ---
> >>>> V2: - Use dev_err_probe()
> >>>>      - Set REG_RC_RESET as volatile
> >>>>      - Wait for PLL stabilization by polling REG_RC_LVDS_PLL
> >>>>      - Use ctx->mode = *adj instead of *mode in sn65dsi83_mode_set
> >>>>      - Add tested DSI84 support in dual-link mode
> >>>>      - Correctly set VCOM
> >>>>      - Fill in missing DSI CHB and LVDS CHB bits from DSI84 and DSI85
> >>>>        datasheets, with that all the reserved bits make far more sense
> >>>>        as the DSI83 and DSI84 seems to be reduced version of DSI85
> >>>> ---
> >>>>   drivers/gpu/drm/bridge/Kconfig        |  10 +
> >>>>   drivers/gpu/drm/bridge/Makefile       |   1 +
> >>>>   drivers/gpu/drm/bridge/ti-sn65dsi83.c | 617 ++++++++++++++++++++++++++
> >>>>   3 files changed, 628 insertions(+)
> >>>>   create mode 100644 drivers/gpu/drm/bridge/ti-sn65dsi83.c
> >>>>
> >>> [...]
> >>>> +static int sn65dsi83_probe(struct i2c_client *client,
> >>>> +               const struct i2c_device_id *id)
> >>>> +{
> >>>> +    struct device *dev = &client->dev;
> >>>> +    enum sn65dsi83_model model;
> >>>> +    struct sn65dsi83 *ctx;
> >>>> +    int ret;
> >>>> +
> >>>> +    ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
> >>>> +    if (!ctx)
> >>>> +        return -ENOMEM;
> >>>> +
> >>>> +    ctx->dev = dev;
> >>>> +
> >>>> +    if (dev->of_node)
> >>>> +        model = (enum sn65dsi83_model)of_device_get_match_data(dev);
> >>>> +    else
> >>>> +        model = id->driver_data;
> >>>> +
> >>>> +    /* Default to dual-link LVDS on all but DSI83. */
> >>>> +    if (model != MODEL_SN65DSI83)
> >>>> +        ctx->lvds_dual_link = true;
> >>>
> >>> What if I use the DSI84 with a single link LVDS? I can't see any way to
> >>> configure that right now.
> >
> > I assume the simplest way would be to use the "ti,sn65dsi83"
> > compatible string in your dts, since the way you wired it is
> > 'compatible' with sn65dsi83, right?
>
> No this isn't the right way to to, if sn65dsi84 is supported and the bindings only support single lvds link,
> the driver must only support single link on sn65dsi84, or add the dual link lvds in the bindings only for sn65dsi84.
>
> >
> >>
> >> I just saw the note in the header of the driver that says that single
> >> link mode is unsupported for the DSI84.
> >>
> >> I have hardware with a single link display and if I set
> >> ctx->lvds_dual_link = false it works just fine.
> >>
> >> How is this supposed to be selected? Does it need an extra devicetree
> >> property? And would you mind adding single-link support in the next
> >> version or do you prefer adding it in a follow-up patch?
> >
> > If this has to be supported I think the proper way would be to support
> > two output ports in the dts (e.g. lvds0_out, lvds1_out), in the same
> > way as supported by the 'advantech,idk-2121wr' panel.
>
> Yes, this is why I asked to have the dual-link lvds in the bindings.

Agreed with Neil, this is what we discussed on my v3. Each of these 3
chips has its own compatible and supporting dual-link lvds and
dual-link dsi as to be done by 84/85 and 85 respectively.

Maybe I can push my configuration changes in gist if required?

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

Re: [PATCH V2 2/2] drm/bridge: ti-sn65dsi83: Add TI SN65DSI83 and SN65DSI84 driver

[Marek Vasut]

On 4/28/21 11:49 AM, Jagan Teki wrote:
[...]

>>>> I just saw the note in the header of the driver that says that single
>>>> link mode is unsupported for the DSI84.
>>>>
>>>> I have hardware with a single link display and if I set
>>>> ctx->lvds_dual_link = false it works just fine.
>>>>
>>>> How is this supposed to be selected? Does it need an extra devicetree
>>>> property? And would you mind adding single-link support in the next
>>>> version or do you prefer adding it in a follow-up patch?
>>>
>>> If this has to be supported I think the proper way would be to support
>>> two output ports in the dts (e.g. lvds0_out, lvds1_out), in the same
>>> way as supported by the 'advantech,idk-2121wr' panel.
>>
>> Yes, this is why I asked to have the dual-link lvds in the bindings.
> 
> Agreed with Neil, this is what we discussed on my v3. Each of these 3
> chips has its own compatible and supporting dual-link lvds and
> dual-link dsi as to be done by 84/85 and 85 respectively.

I have a counter-proposal to this single/dual link LVDS panel handling, 
maybe this should really be done using DRM_BUS_FLAG added to the panel, 
to indicate whether it is single or dual link. Then the bridge can 
figure that out, without any extra DT props.

> Maybe I can push my configuration changes in gist if required?

Please summarize the v3 discussion, yes.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH V2 2/2] drm/bridge: ti-sn65dsi83: Add TI SN65DSI83 and SN65DSI84 driver

[Marek Vasut]

On 4/28/21 11:24 AM, Neil Armstrong wrote:
[...]

>>>>> +static int sn65dsi83_probe(struct i2c_client *client,
>>>>> +               const struct i2c_device_id *id)
>>>>> +{
>>>>> +    struct device *dev = &client->dev;
>>>>> +    enum sn65dsi83_model model;
>>>>> +    struct sn65dsi83 *ctx;
>>>>> +    int ret;
>>>>> +
>>>>> +    ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
>>>>> +    if (!ctx)
>>>>> +        return -ENOMEM;
>>>>> +
>>>>> +    ctx->dev = dev;
>>>>> +
>>>>> +    if (dev->of_node)
>>>>> +        model = (enum sn65dsi83_model)of_device_get_match_data(dev);
>>>>> +    else
>>>>> +        model = id->driver_data;
>>>>> +
>>>>> +    /* Default to dual-link LVDS on all but DSI83. */
>>>>> +    if (model != MODEL_SN65DSI83)
>>>>> +        ctx->lvds_dual_link = true;
>>>>
>>>> What if I use the DSI84 with a single link LVDS? I can't see any way to
>>>> configure that right now.
>>
>> I assume the simplest way would be to use the "ti,sn65dsi83"
>> compatible string in your dts, since the way you wired it is
>> 'compatible' with sn65dsi83, right?
> 
> No this isn't the right way to to, if sn65dsi84 is supported and the bindings only support single lvds link,
> the driver must only support single link on sn65dsi84, or add the dual link lvds in the bindings only for sn65dsi84.

The driver has a comment about what is supported and tested, as Frieder 
already pointed out:

Currently supported:
- SN65DSI83
   = 1x Single-link DSI ~ 1x Single-link LVDS
   - Supported
   - Single-link LVDS mode tested
- SN65DSI84
   = 1x Single-link DSI ~ 2x Single-link or 1x Dual-link LVDS
   - Supported
   - Dual-link LVDS mode tested
   - 2x Single-link LVDS mode unsupported
     (should be easy to add by someone who has the HW)
- SN65DSI85
   = 2x Single-link or 1x Dual-link DSI ~ 2x Single-link or 1x Dual-link 
LVDS
   - Unsupported
     (should be easy to add by someone who has the HW)

So,
DSI83 is always single-link DSI, single-link LVDS.
DSI84 is always single-link DSI, and currently always dual-link LVDS.

The DSI83 can do one thing on the LVDS end:
- 1x single link lVDS

The DSI84 can do two things on the LVDS end:
- 1x single link lVDS
- 1x dual link LVDS

There is also some sort of mention in the DSI84 datasheet about 2x 
single link LVDS, but I suspect that might be copied from DSI85 
datasheet instead, which would make sense. The other option is that it 
behaves as a mirror (i.e. same pixels are scanned out of LVDS channel A 
and B). Either option can be added by either adding a DT property which 
would enable the mirror mode, or new port linking the LVDS endpoint to 
the same panel twice, and/or two new ports for DSI85, so there is no 
problem to extend the bindings without breaking them. So for now I would 
ignore this mode.

So ultimately, what we have to sort out is the 1x single / 1x dual link 
LVDS mode setting on DSI84. Frieder already pointed out how the driver 
can be tweaked to support the single-link mode on DSI84, so now we need 
to tie it into DT bindings.

Currently, neither the LVDS panels in upstream in panel-simple nor 
lvds.yaml provide any indication that the panel is single-link or 
dual-link. Those dual-link LVDS panels seem to always set 2x pixel clock 
and let the bridge somehow sort it out.

Maybe that isn't always the best approach, maybe we should add a new 
DRM_BUS_FLAG for those panels and handle the flag in the bridge driver ? 
Such a new flag could be added over time to panels where applicable, so 
old setups won't be broken by that either, they will just ignore the new 
flag and work as before.

>>> I just saw the note in the header of the driver that says that single
>>> link mode is unsupported for the DSI84.
>>>
>>> I have hardware with a single link display and if I set
>>> ctx->lvds_dual_link = false it works just fine.
>>>
>>> How is this supposed to be selected? Does it need an extra devicetree
>>> property? And would you mind adding single-link support in the next
>>> version or do you prefer adding it in a follow-up patch?
>>
>> If this has to be supported I think the proper way would be to support
>> two output ports in the dts (e.g. lvds0_out, lvds1_out), in the same
>> way as supported by the 'advantech,idk-2121wr' panel.
> 
> Yes, this is why I asked to have the dual-link lvds in the bindings.

Maybe it shouldn't really be in the bindings, see above.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH V2 2/2] drm/bridge: ti-sn65dsi83: Add TI SN65DSI83 and SN65DSI84 driver

[Frieder Schrempf]

On 28.04.21 16:16, Marek Vasut wrote:
> On 4/28/21 11:24 AM, Neil Armstrong wrote:
> [...]
> 
>>>>>> +static int sn65dsi83_probe(struct i2c_client *client,
>>>>>> +               const struct i2c_device_id *id)
>>>>>> +{
>>>>>> +    struct device *dev = &client->dev;
>>>>>> +    enum sn65dsi83_model model;
>>>>>> +    struct sn65dsi83 *ctx;
>>>>>> +    int ret;
>>>>>> +
>>>>>> +    ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
>>>>>> +    if (!ctx)
>>>>>> +        return -ENOMEM;
>>>>>> +
>>>>>> +    ctx->dev = dev;
>>>>>> +
>>>>>> +    if (dev->of_node)
>>>>>> +        model = (enum sn65dsi83_model)of_device_get_match_data(dev);
>>>>>> +    else
>>>>>> +        model = id->driver_data;
>>>>>> +
>>>>>> +    /* Default to dual-link LVDS on all but DSI83. */
>>>>>> +    if (model != MODEL_SN65DSI83)
>>>>>> +        ctx->lvds_dual_link = true;
>>>>>
>>>>> What if I use the DSI84 with a single link LVDS? I can't see any 
>>>>> way to
>>>>> configure that right now.
>>>
>>> I assume the simplest way would be to use the "ti,sn65dsi83"
>>> compatible string in your dts, since the way you wired it is
>>> 'compatible' with sn65dsi83, right?
>>
>> No this isn't the right way to to, if sn65dsi84 is supported and the 
>> bindings only support single lvds link,
>> the driver must only support single link on sn65dsi84, or add the dual 
>> link lvds in the bindings only for sn65dsi84.
> 
> The driver has a comment about what is supported and tested, as Frieder 
> already pointed out:
> 
> Currently supported:
> - SN65DSI83
>    = 1x Single-link DSI ~ 1x Single-link LVDS
>    - Supported
>    - Single-link LVDS mode tested
> - SN65DSI84
>    = 1x Single-link DSI ~ 2x Single-link or 1x Dual-link LVDS
>    - Supported
>    - Dual-link LVDS mode tested
>    - 2x Single-link LVDS mode unsupported
>      (should be easy to add by someone who has the HW)
> - SN65DSI85
>    = 2x Single-link or 1x Dual-link DSI ~ 2x Single-link or 1x Dual-link 
> LVDS
>    - Unsupported
>      (should be easy to add by someone who has the HW)
> 
> So,
> DSI83 is always single-link DSI, single-link LVDS.
> DSI84 is always single-link DSI, and currently always dual-link LVDS.
> 
> The DSI83 can do one thing on the LVDS end:
> - 1x single link lVDS
> 
> The DSI84 can do two things on the LVDS end:
> - 1x single link lVDS
> - 1x dual link LVDS
> 
> There is also some sort of mention in the DSI84 datasheet about 2x 
> single link LVDS, but I suspect that might be copied from DSI85 
> datasheet instead, which would make sense. The other option is that it 
> behaves as a mirror (i.e. same pixels are scanned out of LVDS channel A 
> and B). Either option can be added by either adding a DT property which 
> would enable the mirror mode, or new port linking the LVDS endpoint to 
> the same panel twice, and/or two new ports for DSI85, so there is no 
> problem to extend the bindings without breaking them. So for now I would 
> ignore this mode.

Agreed.

> 
> So ultimately, what we have to sort out is the 1x single / 1x dual link 
> LVDS mode setting on DSI84. Frieder already pointed out how the driver 
> can be tweaked to support the single-link mode on DSI84, so now we need 
> to tie it into DT bindings.
> 
> Currently, neither the LVDS panels in upstream in panel-simple nor 
> lvds.yaml provide any indication that the panel is single-link or 
> dual-link. Those dual-link LVDS panels seem to always set 2x pixel clock 
> and let the bridge somehow sort it out.
> 
> Maybe that isn't always the best approach, maybe we should add a new 
> DRM_BUS_FLAG for those panels and handle the flag in the bridge driver ? 
> Such a new flag could be added over time to panels where applicable, so 
> old setups won't be broken by that either, they will just ignore the new 
> flag and work as before.

I agree that the panel driver should report whether the LVDS input is 
expected to be single or dual link. So introducing a DRM_BUS_FLAG for 
this seems like a good solution.

This wouldn't reflect the physical ports, but I don't really know how we 
could use two ports connected to the same panel for this case, as all 
the existing dual link panels don't follow this scheme.

I would suggest, that the driver defaults to single link for the DSI84 
and then switches to dual link if the panel requests it using the flag.

> 
>>>> I just saw the note in the header of the driver that says that single
>>>> link mode is unsupported for the DSI84.
>>>>
>>>> I have hardware with a single link display and if I set
>>>> ctx->lvds_dual_link = false it works just fine.
>>>>
>>>> How is this supposed to be selected? Does it need an extra devicetree
>>>> property? And would you mind adding single-link support in the next
>>>> version or do you prefer adding it in a follow-up patch?
>>>
>>> If this has to be supported I think the proper way would be to support
>>> two output ports in the dts (e.g. lvds0_out, lvds1_out), in the same
>>> way as supported by the 'advantech,idk-2121wr' panel.
>>
>> Yes, this is why I asked to have the dual-link lvds in the bindings.
> 
> Maybe it shouldn't really be in the bindings, see above.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH V2 2/2] drm/bridge: ti-sn65dsi83: Add TI SN65DSI83 and SN65DSI84 driver

[Neil Armstrong]

On 29/04/2021 18:27, Frieder Schrempf wrote:
> On 28.04.21 16:16, Marek Vasut wrote:
>> On 4/28/21 11:24 AM, Neil Armstrong wrote:
>> [...]
>>
>>>>>>> +static int sn65dsi83_probe(struct i2c_client *client,
>>>>>>> +               const struct i2c_device_id *id)
>>>>>>> +{
>>>>>>> +    struct device *dev = &client->dev;
>>>>>>> +    enum sn65dsi83_model model;
>>>>>>> +    struct sn65dsi83 *ctx;
>>>>>>> +    int ret;
>>>>>>> +
>>>>>>> +    ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
>>>>>>> +    if (!ctx)
>>>>>>> +        return -ENOMEM;
>>>>>>> +
>>>>>>> +    ctx->dev = dev;
>>>>>>> +
>>>>>>> +    if (dev->of_node)
>>>>>>> +        model = (enum sn65dsi83_model)of_device_get_match_data(dev);
>>>>>>> +    else
>>>>>>> +        model = id->driver_data;
>>>>>>> +
>>>>>>> +    /* Default to dual-link LVDS on all but DSI83. */
>>>>>>> +    if (model != MODEL_SN65DSI83)
>>>>>>> +        ctx->lvds_dual_link = true;
>>>>>>
>>>>>> What if I use the DSI84 with a single link LVDS? I can't see any way to
>>>>>> configure that right now.
>>>>
>>>> I assume the simplest way would be to use the "ti,sn65dsi83"
>>>> compatible string in your dts, since the way you wired it is
>>>> 'compatible' with sn65dsi83, right?
>>>
>>> No this isn't the right way to to, if sn65dsi84 is supported and the bindings only support single lvds link,
>>> the driver must only support single link on sn65dsi84, or add the dual link lvds in the bindings only for sn65dsi84.
>>
>> The driver has a comment about what is supported and tested, as Frieder already pointed out:
>>
>> Currently supported:
>> - SN65DSI83
>>    = 1x Single-link DSI ~ 1x Single-link LVDS
>>    - Supported
>>    - Single-link LVDS mode tested
>> - SN65DSI84
>>    = 1x Single-link DSI ~ 2x Single-link or 1x Dual-link LVDS
>>    - Supported
>>    - Dual-link LVDS mode tested
>>    - 2x Single-link LVDS mode unsupported
>>      (should be easy to add by someone who has the HW)
>> - SN65DSI85
>>    = 2x Single-link or 1x Dual-link DSI ~ 2x Single-link or 1x Dual-link LVDS
>>    - Unsupported
>>      (should be easy to add by someone who has the HW)
>>
>> So,
>> DSI83 is always single-link DSI, single-link LVDS.
>> DSI84 is always single-link DSI, and currently always dual-link LVDS.
>>
>> The DSI83 can do one thing on the LVDS end:
>> - 1x single link lVDS
>>
>> The DSI84 can do two things on the LVDS end:
>> - 1x single link lVDS
>> - 1x dual link LVDS
>>
>> There is also some sort of mention in the DSI84 datasheet about 2x single link LVDS, but I suspect that might be copied from DSI85 datasheet instead, which would make sense. The other option is that it behaves as a mirror (i.e. same pixels are scanned out of LVDS channel A and B). Either option can be added by either adding a DT property which would enable the mirror mode, or new port linking the LVDS endpoint to the same panel twice, and/or two new ports for DSI85, so there is no problem to extend the bindings without breaking them. So for now I would ignore this mode.
> 
> Agreed.
> 
>>
>> So ultimately, what we have to sort out is the 1x single / 1x dual link LVDS mode setting on DSI84. Frieder already pointed out how the driver can be tweaked to support the single-link mode on DSI84, so now we need to tie it into DT bindings.
>>
>> Currently, neither the LVDS panels in upstream in panel-simple nor lvds.yaml provide any indication that the panel is single-link or dual-link. Those dual-link LVDS panels seem to always set 2x pixel clock and let the bridge somehow sort it out.
>>
>> Maybe that isn't always the best approach, maybe we should add a new DRM_BUS_FLAG for those panels and handle the flag in the bridge driver ? Such a new flag could be added over time to panels where applicable, so old setups won't be broken by that either, they will just ignore the new flag and work as before.
> 
> I agree that the panel driver should report whether the LVDS input is expected to be single or dual link. So introducing a DRM_BUS_FLAG for this seems like a good solution.
> 
> This wouldn't reflect the physical ports, but I don't really know how we could use two ports connected to the same panel for this case, as all the existing dual link panels don't follow this scheme.

A dual-link LVDS panel should need 2 ports, because each LVDS link could come from different controllers, here by the same but simply connect the 2 panel ports to the 2 controller's ports.

Neil

> 
> I would suggest, that the driver defaults to single link for the DSI84 and then switches to dual link if the panel requests it using the flag.
> 
>>
>>>>> I just saw the note in the header of the driver that says that single
>>>>> link mode is unsupported for the DSI84.
>>>>>
>>>>> I have hardware with a single link display and if I set
>>>>> ctx->lvds_dual_link = false it works just fine.
>>>>>
>>>>> How is this supposed to be selected? Does it need an extra devicetree
>>>>> property? And would you mind adding single-link support in the next
>>>>> version or do you prefer adding it in a follow-up patch?
>>>>
>>>> If this has to be supported I think the proper way would be to support
>>>> two output ports in the dts (e.g. lvds0_out, lvds1_out), in the same
>>>> way as supported by the 'advantech,idk-2121wr' panel.
>>>
>>> Yes, this is why I asked to have the dual-link lvds in the bindings.
>>
>> Maybe it shouldn't really be in the bindings, see above.

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

Re: [PATCH V2 1/2] dt-bindings: drm/bridge: ti-sn65dsi83: Add TI SN65DSI83 and SN65DSI84 bindings

[Laurent Pinchart]

Hi Marek,

Thank you for the patch.

On Thu, Apr 22, 2021 at 12:31:21AM +0200, Marek Vasut wrote:
> Add DT binding document for TI SN65DSI83 and SN65DSI84 DSI to LVDS bridge.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Douglas Anderson <dianders@chromium.org>
> Cc: Jagan Teki <jagan@amarulasolutions.com>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Sam Ravnborg <sam@ravnborg.org>
> Cc: Stephen Boyd <swboyd@chromium.org>
> Cc: devicetree@vger.kernel.org
> To: dri-devel@lists.freedesktop.org
> ---
> V2: Add compatible string for SN65DSI84, since this is now tested on it
> ---
>  .../bindings/display/bridge/ti,sn65dsi83.yaml | 134 ++++++++++++++++++
>  1 file changed, 134 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
> 
> diff --git a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
> new file mode 100644
> index 000000000000..42d11b46a1eb
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
> @@ -0,0 +1,134 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/bridge/ti,sn65dsi83.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: SN65DSI83 and SN65DSI84 DSI to LVDS bridge chip
> +
> +maintainers:
> +  - Marek Vasut <marex@denx.de>
> +
> +description: |
> +  Texas Instruments SN65DSI83 1x Single-link MIPI DSI
> +  to 1x Single-link LVDS
> +  https://www.ti.com/lit/gpn/sn65dsi83
> +  Texas Instruments SN65DSI84 1x Single-link MIPI DSI
> +  to 1x Dual-link or 2x Single-link LVDS
> +  https://www.ti.com/lit/gpn/sn65dsi84
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - const: ti,sn65dsi83
> +      - const: ti,sn65dsi84
> +
> +  reg:
> +    const: 0x2d
> +
> +  enable-gpios:
> +    maxItems: 1
> +    description: GPIO specifier for bridge_en pin (active high).
> +
> +  ports:
> +    type: object

Could you use the OF graph schema, now that it is available ? There
should be plenty of examples in bindings, both in display and in media.
You will be able to drop quite a bit of boilerplate.

> +    additionalProperties: false
> +
> +    properties:
> +      "#address-cells":
> +        const: 1
> +
> +      "#size-cells":
> +        const: 0
> +
> +      port@0:
> +        type: object
> +        additionalProperties: false
> +
> +        description:
> +          Video port for MIPI DSI input
> +
> +        properties:
> +          reg:
> +            const: 0
> +
> +          endpoint:
> +            type: object
> +            additionalProperties: false
> +            properties:
> +              remote-endpoint: true
> +              data-lanes:
> +                description: array of physical DSI data lane indexes.
> +
> +        required:
> +          - reg
> +
> +      port@1:
> +        type: object
> +        additionalProperties: false
> +
> +        description:
> +          Video port for LVDS output (panel or bridge).
> +
> +        properties:
> +          reg:
> +            const: 1
> +
> +          endpoint:
> +            type: object
> +            additionalProperties: false
> +            properties:
> +              remote-endpoint: true
> +
> +        required:
> +          - reg
> +
> +    required:
> +      - "#address-cells"
> +      - "#size-cells"
> +      - port@0
> +      - port@1
> +
> +required:
> +  - compatible
> +  - reg
> +  - enable-gpios
> +  - ports
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +
> +    i2c {
> +      #address-cells = <1>;
> +      #size-cells = <0>;

While not a hard rule, it's customary to indent DT examples with 4
spaces. I find it to increase readability, but if you feel otherwise,
that's OK.

> +
> +      bridge@2d {
> +        compatible = "ti,sn65dsi83";
> +        reg = <0x2d>;
> +
> +        enable-gpios = <&gpio2 1 GPIO_ACTIVE_HIGH>;
> +
> +        ports {
> +          #address-cells = <1>;
> +          #size-cells = <0>;
> +
> +          port@0 {
> +            reg = <0>;
> +            endpoint {
> +              remote-endpoint = <&dsi0_out>;
> +              data-lanes = <1 2 3 4>;
> +            };
> +          };
> +
> +          port@1 {
> +            reg = <1>;
> +            endpoint {
> +              remote-endpoint = <&panel_in_lvds>;
> +            };
> +          };
> +        };
> +      };
> +    };

-- 
Regards,

Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH V2 1/2] dt-bindings: drm/bridge: ti-sn65dsi83: Add TI SN65DSI83 and SN65DSI84 bindings

[Neil Armstrong]

On 22/04/2021 00:31, Marek Vasut wrote:
> Add DT binding document for TI SN65DSI83 and SN65DSI84 DSI to LVDS bridge.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Douglas Anderson <dianders@chromium.org>
> Cc: Jagan Teki <jagan@amarulasolutions.com>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Sam Ravnborg <sam@ravnborg.org>
> Cc: Stephen Boyd <swboyd@chromium.org>
> Cc: devicetree@vger.kernel.org
> To: dri-devel@lists.freedesktop.org
> ---
> V2: Add compatible string for SN65DSI84, since this is now tested on it
> ---
>  .../bindings/display/bridge/ti,sn65dsi83.yaml | 134 ++++++++++++++++++
>  1 file changed, 134 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
> 
> diff --git a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
> new file mode 100644
> index 000000000000..42d11b46a1eb
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
> @@ -0,0 +1,134 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/bridge/ti,sn65dsi83.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: SN65DSI83 and SN65DSI84 DSI to LVDS bridge chip
> +
> +maintainers:
> +  - Marek Vasut <marex@denx.de>
> +
> +description: |
> +  Texas Instruments SN65DSI83 1x Single-link MIPI DSI
> +  to 1x Single-link LVDS
> +  https://www.ti.com/lit/gpn/sn65dsi83
> +  Texas Instruments SN65DSI84 1x Single-link MIPI DSI
> +  to 1x Dual-link or 2x Single-link LVDS
> +  https://www.ti.com/lit/gpn/sn65dsi84
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - const: ti,sn65dsi83
> +      - const: ti,sn65dsi84
> +
> +  reg:
> +    const: 0x2d
> +
> +  enable-gpios:
> +    maxItems: 1
> +    description: GPIO specifier for bridge_en pin (active high).
> +
> +  ports:
> +    type: object
> +    additionalProperties: false
> +
> +    properties:
> +      "#address-cells":
> +        const: 1
> +
> +      "#size-cells":
> +        const: 0
> +
> +      port@0:
> +        type: object
> +        additionalProperties: false
> +
> +        description:
> +          Video port for MIPI DSI input
> +
> +        properties:
> +          reg:
> +            const: 0
> +
> +          endpoint:
> +            type: object
> +            additionalProperties: false
> +            properties:
> +              remote-endpoint: true
> +              data-lanes:
> +                description: array of physical DSI data lane indexes.
> +
> +        required:
> +          - reg
> +
> +      port@1:
> +        type: object
> +        additionalProperties: false
> +
> +        description:
> +          Video port for LVDS output (panel or bridge).
> +
> +        properties:
> +          reg:
> +            const: 1
> +
> +          endpoint:
> +            type: object
> +            additionalProperties: false
> +            properties:
> +              remote-endpoint: true

Similar to Jagan's serie, would be great to add bindings for the dual-link LVDS even if not supported
by the driver (the driver can fails with a verbose error).

Neil

> +
> +        required:
> +          - reg
> +
> +    required:
> +      - "#address-cells"
> +      - "#size-cells"
> +      - port@0
> +      - port@1
> +
> +required:
> +  - compatible
> +  - reg
> +  - enable-gpios
> +  - ports
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +
> +    i2c {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +
> +      bridge@2d {
> +        compatible = "ti,sn65dsi83";
> +        reg = <0x2d>;
> +
> +        enable-gpios = <&gpio2 1 GPIO_ACTIVE_HIGH>;
> +
> +        ports {
> +          #address-cells = <1>;
> +          #size-cells = <0>;
> +
> +          port@0 {
> +            reg = <0>;
> +            endpoint {
> +              remote-endpoint = <&dsi0_out>;
> +              data-lanes = <1 2 3 4>;
> +            };
> +          };
> +
> +          port@1 {
> +            reg = <1>;
> +            endpoint {
> +              remote-endpoint = <&panel_in_lvds>;
> +            };
> +          };
> +        };
> +      };
> +    };
> 

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

Re: [PATCH V2 1/2] dt-bindings: drm/bridge: ti-sn65dsi83: Add TI SN65DSI83 and SN65DSI84 bindings

[Marek Vasut]

On 4/22/21 10:38 AM, Neil Armstrong wrote:
[...]
>> +      port@1:
>> +        type: object
>> +        additionalProperties: false
>> +
>> +        description:
>> +          Video port for LVDS output (panel or bridge).
>> +
>> +        properties:
>> +          reg:
>> +            const: 1
>> +
>> +          endpoint:
>> +            type: object
>> +            additionalProperties: false
>> +            properties:
>> +              remote-endpoint: true
> 
> Similar to Jagan's serie, would be great to add bindings for the dual-link LVDS even if not supported
> by the driver (the driver can fails with a verbose error).

I don't want to add any sort of bindings which I cannot validate against 
real hardware. I would argue that adding the 2x single / dual link LVDS 
DT property could be added when someone has a need for it and can test 
it on real hardware, and such a binding should be simple develop and 
add. And that is better, because we won't end up with some possibly 
misdesigned untested DT binding which would become part of the ABI and 
would have to be supported forever.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH V2 1/2] dt-bindings: drm/bridge: ti-sn65dsi83: Add TI SN65DSI83 and SN65DSI84 bindings

[Jagan Teki]

On Thu, Apr 22, 2021 at 4:01 AM Marek Vasut <marex@denx.de> wrote:
>
> Add DT binding document for TI SN65DSI83 and SN65DSI84 DSI to LVDS bridge.
>
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Douglas Anderson <dianders@chromium.org>
> Cc: Jagan Teki <jagan@amarulasolutions.com>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Sam Ravnborg <sam@ravnborg.org>
> Cc: Stephen Boyd <swboyd@chromium.org>
> Cc: devicetree@vger.kernel.org
> To: dri-devel@lists.freedesktop.org
> ---
> V2: Add compatible string for SN65DSI84, since this is now tested on it
> ---
>  .../bindings/display/bridge/ti,sn65dsi83.yaml | 134 ++++++++++++++++++
>  1 file changed, 134 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
>
> diff --git a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
> new file mode 100644
> index 000000000000..42d11b46a1eb
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
> @@ -0,0 +1,134 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/bridge/ti,sn65dsi83.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: SN65DSI83 and SN65DSI84 DSI to LVDS bridge chip

Can it possible to wait for my v4 to have dual-link LVDS supported
which is quite discussing points on previous versions?

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

Re: [PATCH V2 1/2] dt-bindings: drm/bridge: ti-sn65dsi83: Add TI SN65DSI83 and SN65DSI84 bindings

[Marek Vasut]

On 4/22/21 10:43 AM, Jagan Teki wrote:

[...]

>> diff --git a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
>> new file mode 100644
>> index 000000000000..42d11b46a1eb
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
>> @@ -0,0 +1,134 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/display/bridge/ti,sn65dsi83.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: SN65DSI83 and SN65DSI84 DSI to LVDS bridge chip
> 
> Can it possible to wait for my v4 to have dual-link LVDS supported
> which is quite discussing points on previous versions?

This driver already has the dual-link LVDS support on DSI84 and it is 
tested on real hardware. So is the single-link LVDS on DSI83, on 
multiple hardware instances.

In addition to that, this driver has proper regmap support and constant 
time calculation of DSI83/84 clock register settings.

Note that I did wait for your v4 for over two weeks already, it just 
didn't materialize, so I determined it might be a good idea to restart 
work on this driver instead.

Maybe you can test this driver and check what is missing for your usecase?
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH V2 1/2] dt-bindings: drm/bridge: ti-sn65dsi83: Add TI SN65DSI83 and SN65DSI84 bindings

[Frieder Schrempf]

On 22.04.21 00:31, Marek Vasut wrote:
> Add DT binding document for TI SN65DSI83 and SN65DSI84 DSI to LVDS bridge.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Douglas Anderson <dianders@chromium.org>
> Cc: Jagan Teki <jagan@amarulasolutions.com>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Sam Ravnborg <sam@ravnborg.org>
> Cc: Stephen Boyd <swboyd@chromium.org>
> Cc: devicetree@vger.kernel.org
> To: dri-devel@lists.freedesktop.org
> ---
> V2: Add compatible string for SN65DSI84, since this is now tested on it
> ---
>   .../bindings/display/bridge/ti,sn65dsi83.yaml | 134 ++++++++++++++++++
>   1 file changed, 134 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
> 
> diff --git a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
> new file mode 100644
> index 000000000000..42d11b46a1eb
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
> @@ -0,0 +1,134 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/bridge/ti,sn65dsi83.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: SN65DSI83 and SN65DSI84 DSI to LVDS bridge chip
> +
> +maintainers:
> +  - Marek Vasut <marex@denx.de>
> +
> +description: |
> +  Texas Instruments SN65DSI83 1x Single-link MIPI DSI
> +  to 1x Single-link LVDS
> +  https://www.ti.com/lit/gpn/sn65dsi83
> +  Texas Instruments SN65DSI84 1x Single-link MIPI DSI
> +  to 1x Dual-link or 2x Single-link LVDS
> +  https://www.ti.com/lit/gpn/sn65dsi84
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - const: ti,sn65dsi83
> +      - const: ti,sn65dsi84
> +
> +  reg:
> +    const: 0x2d

There is a strapping pin to select the last bit of the address, so apart 
from 0x2d also 0x2c is valid here.

> +
> +  enable-gpios:
> +    maxItems: 1
> +    description: GPIO specifier for bridge_en pin (active high).
> +
> +  ports:
> +    type: object
> +    additionalProperties: false
> +
> +    properties:
> +      "#address-cells":
> +        const: 1
> +
> +      "#size-cells":
> +        const: 0
> +
> +      port@0:
> +        type: object
> +        additionalProperties: false
> +
> +        description:
> +          Video port for MIPI DSI input
> +
> +        properties:
> +          reg:
> +            const: 0
> +
> +          endpoint:
> +            type: object
> +            additionalProperties: false
> +            properties:
> +              remote-endpoint: true
> +              data-lanes:
> +                description: array of physical DSI data lane indexes.
> +
> +        required:
> +          - reg
> +
> +      port@1:
> +        type: object
> +        additionalProperties: false
> +
> +        description:
> +          Video port for LVDS output (panel or bridge).
> +
> +        properties:
> +          reg:
> +            const: 1
> +
> +          endpoint:
> +            type: object
> +            additionalProperties: false
> +            properties:
> +              remote-endpoint: true
> +
> +        required:
> +          - reg
> +
> +    required:
> +      - "#address-cells"
> +      - "#size-cells"
> +      - port@0
> +      - port@1
> +
> +required:
> +  - compatible
> +  - reg
> +  - enable-gpios
> +  - ports
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +
> +    i2c {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +
> +      bridge@2d {
> +        compatible = "ti,sn65dsi83";
> +        reg = <0x2d>;
> +
> +        enable-gpios = <&gpio2 1 GPIO_ACTIVE_HIGH>;
> +
> +        ports {
> +          #address-cells = <1>;
> +          #size-cells = <0>;
> +
> +          port@0 {
> +            reg = <0>;
> +            endpoint {
> +              remote-endpoint = <&dsi0_out>;
> +              data-lanes = <1 2 3 4>;
> +            };
> +          };
> +
> +          port@1 {
> +            reg = <1>;
> +            endpoint {
> +              remote-endpoint = <&panel_in_lvds>;
> +            };
> +          };
> +        };
> +      };
> +    };
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH V2 1/2] dt-bindings: drm/bridge: ti-sn65dsi83: Add TI SN65DSI83 and SN65DSI84 bindings

[Marek Vasut]

On 4/28/21 9:56 AM, Frieder Schrempf wrote:
[...]
>> +properties:
>> +  compatible:
>> +    oneOf:
>> +      - const: ti,sn65dsi83
>> +      - const: ti,sn65dsi84
>> +
>> +  reg:
>> +    const: 0x2d
> 
> There is a strapping pin to select the last bit of the address, so apart 
> from 0x2d also 0x2c is valid here.

Fixed, along with the other DT details pointed out by Laurent, thanks.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH V2 2/2] drm/bridge: ti-sn65dsi83: Add TI SN65DSI83 and SN65DSI84 driver

[Felix Radensky]

 Hi Marek,

SN65DSI84 also supports single-link LVDS. We have quite a few customers using SN65DSI84 on Variscite SOMs with low resolution single-channel LVDS panels. I think having a DTS binding to indicate the number of links used by SN65DSI84 is more flexible than forcing dual-link mode on all SN65DSI84 users.
Setting "ti,sn65dsi83" compatible when actually SN65DSI84 is used by the board is a bit misleading.

There are also some bridge properties that are currently not supported by the driver and not reflected in the DTS bindings, e.g. RGB666 vs RGB888, Output Format 1 vs Output Format 2. Do you have any plans to support them ?

Thanks a lot for working on this driver, your efforts are much appreciated.

Felix.


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