Hi,

I will start to review patch sets for -fixes first so I queued your four patches and you can check exynos-drm-next-todo branch for it.

Thanks, Inki Dae

2018년 05월 30일 21:15에 Maciej Purski 이(가) 쓴 글:

When adding support for peripheral out bridges, the "bridge" name becomes imprecise as it refers to a different device than the "out_bridge".

Signed-off-by: Maciej Purski m.purski@samsung.com

drivers/gpu/drm/exynos/exynos_drm_dsi.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c index eae44fd..9599e6b 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c @@ -279,7 +279,7 @@ struct exynos_dsi { struct list_head transfer_list;

const struct exynos_dsi_driver_data *driver_data;

• struct device_node *bridge_node;

• struct device_node *mic_bridge_node;

};

#define host_to_dsi(host) container_of(host, struct exynos_dsi, dsi_host) @@ -1631,7 +1631,7 @@ static int exynos_dsi_parse_dt(struct exynos_dsi *dsi) if (ret < 0) return ret;

• dsi->bridge_node = of_graph_get_remote_node(node, DSI_PORT_IN, 0);

• dsi->mic_bridge_node = of_graph_get_remote_node(node, DSI_PORT_IN, 0);

  return 0;

} @@ -1642,7 +1642,7 @@ static int exynos_dsi_bind(struct device *dev, struct device *master, struct drm_encoder *encoder = dev_get_drvdata(dev); struct exynos_dsi *dsi = encoder_to_dsi(encoder); struct drm_device *drm_dev = data;

• struct drm_bridge *bridge;

• struct drm_bridge *mic_bridge; int ret;

  drm_encoder_init(drm_dev, encoder, &exynos_dsi_encoder_funcs,

@@ -1661,10 +1661,10 @@ static int exynos_dsi_bind(struct device *dev, struct device *master, return ret; }

• if (dsi->bridge_node) {

• bridge = of_drm_find_bridge(dsi->bridge_node);
  

• if (bridge)
  

• 	drm_bridge_attach(encoder, bridge, NULL);
  

• if (dsi->mic_bridge_node) {

• mic_bridge = of_drm_find_bridge(dsi->mic_bridge_node);
  

• if (mic_bridge)
  

• 	drm_bridge_attach(encoder, mic_bridge, NULL);
  

  }

  return mipi_dsi_host_register(&dsi->dsi_host);

@@ -1783,7 +1783,7 @@ static int exynos_dsi_remove(struct platform_device *pdev) { struct exynos_dsi *dsi = platform_get_drvdata(pdev);

• of_node_put(dsi->bridge_node);

• of_node_put(dsi->mic_bridge_node);

  pm_runtime_disable(&pdev->dev);

Hi Maciej,

On Wednesday, 30 May 2018 15:15:58 EEST Maciej Purski wrote:

From: Andrzej Hajda a.hajda@samsung.com

The patch adds bindings to Toshiba DSI/LVDS bridge TC358764. Bindings describe power supplies, reset gpio and video interfaces.

Signed-off-by: Andrzej Hajda a.hajda@samsung.com Signed-off-by: Maciej Purski m.purski@samsung.com

---

.../bindings/display/bridge/toshiba,tc358764.txt | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/bridge/toshiba,tc358764.txt

diff --git a/Documentation/devicetree/bindings/display/bridge/toshiba,tc358764.txt b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358764.txt new file mode 100644 index 0000000..6eda14f --- /dev/null +++ b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358764.txt @@ -0,0 +1,37 @@ +TC358764 MIPI-DSI to LVDS panel bridge

+Required properties:

• • compatible: "toshiba,tc358764"
• • reg: the virtual channel number of a DSI peripheral
• • vddc-supply: core voltage supply, 1.2V
• • vddio-supply: I/O voltage supply, 1.8V or 3.3V
• • vddlvds-supply: LVDS1/2 voltage supply, 3.3V
• • reset-gpios: a GPIO spec for the reset pin

+The device node can contain zero to two 'port' child nodes, each with one +child 'endpoint' node, according to the bindings defined in [1]. +The following are properties specific to those nodes.

+port:

• • reg: (required) can be 0 for DSI port or 1 for LVDS port;

+[1]: Documentation/devicetree/bindings/media/video-interfaces.txt

Could you please take the comments I made on v1 into account when you'll post v3 ?

+Example:

• bridge@0 {

• reg = <0>;
  

• compatible = "toshiba,tc358764";
  

• vddc-supply = <&vcc_1v2_reg>;
  

• vddio-supply = <&vcc_1v8_reg>;
  

• vddlvds-supply = <&vcc_3v3_reg>;
  

• reset-gpios = <&gpd1 6 GPIO_ACTIVE_LOW>;
  

• #address-cells = <1>;
  

• #size-cells = <0>;
  

• port@1 {
  

• 	reg = <1>;
  

• 	lvds_ep: endpoint {
  

• 		remote-endpoint = <&panel_ep>;
  

• 	};
  

• };
  

• };

On Wed, May 30, 2018 at 02:15:58PM +0200, Maciej Purski wrote:

From: Andrzej Hajda a.hajda@samsung.com

The patch adds bindings to Toshiba DSI/LVDS bridge TC358764. Bindings describe power supplies, reset gpio and video interfaces.

Signed-off-by: Andrzej Hajda a.hajda@samsung.com Signed-off-by: Maciej Purski m.purski@samsung.com

---

.../bindings/display/bridge/toshiba,tc358764.txt | 37 ++++++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/bridge/toshiba,tc358764.txt

diff --git a/Documentation/devicetree/bindings/display/bridge/toshiba,tc358764.txt b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358764.txt new file mode 100644 index 0000000..6eda14f --- /dev/null +++ b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358764.txt @@ -0,0 +1,37 @@ +TC358764 MIPI-DSI to LVDS panel bridge

+Required properties:

• • compatible: "toshiba,tc358764"
• • reg: the virtual channel number of a DSI peripheral
• • vddc-supply: core voltage supply, 1.2V
• • vddio-supply: I/O voltage supply, 1.8V or 3.3V
• • vddlvds-supply: LVDS1/2 voltage supply, 3.3V
• • reset-gpios: a GPIO spec for the reset pin

+The device node can contain zero to two 'port' child nodes, each with one

How would 0 ports be valid?

+child 'endpoint' node, according to the bindings defined in [1]. +The following are properties specific to those nodes.

+port:

• • reg: (required) can be 0 for DSI port or 1 for LVDS port;

+[1]: Documentation/devicetree/bindings/media/video-interfaces.txt

+Example:

• bridge@0 {

• reg = <0>;
  

• compatible = "toshiba,tc358764";
  

• vddc-supply = <&vcc_1v2_reg>;
  

• vddio-supply = <&vcc_1v8_reg>;
  

• vddlvds-supply = <&vcc_3v3_reg>;
  

• reset-gpios = <&gpd1 6 GPIO_ACTIVE_LOW>;
  

• #address-cells = <1>;
  

• #size-cells = <0>;
  

• port@1 {
  

• 	reg = <1>;
  

• 	lvds_ep: endpoint {
  

• 		remote-endpoint = <&panel_ep>;
  

• 	};
  

• };
  

• };

-- 2.7.4

On 05/31/2018 08:51 AM, Archit Taneja wrote:

Hi,

On Wednesday 30 May 2018 05:45 PM, Maciej Purski wrote:

...

From: Andrzej Hajda a.hajda@samsung.com

Add a drm_bridge driver for the Toshiba TC358764 DSI to LVDS bridge.

Signed-off-by: Andrzej Hajda a.hajda@samsung.com Signed-off-by: Maciej Purski m.purski@samsung.com

---

drivers/gpu/drm/bridge/Kconfig    |   9 +   drivers/gpu/drm/bridge/Makefile   |   1 +   drivers/gpu/drm/bridge/tc358764.c | 547 ++++++++++++++++++++++++++++++++++++++   3 files changed, 557 insertions(+)   create mode 100644 drivers/gpu/drm/bridge/tc358764.c

diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig index fa2c799..9bd3eb8 100644 --- a/drivers/gpu/drm/bridge/Kconfig +++ b/drivers/gpu/drm/bridge/Kconfig @@ -110,6 +110,15 @@ config DRM_THINE_THC63LVD1024       ---help---         Thine THC63LVD1024 LVDS/parallel converter driver. +config DRM_TOSHIBA_TC358764 +    tristate "TC358764 DSI/LVDS bridge" +    depends on DRM && DRM_PANEL +    depends on OF +    select DRM_MIPI_DSI +    select VIDEOMODE_HELPERS

I don't see videomode usage in the driver, can we drop this if it isn't used?

It seems that those are some remains of old versions. It is not required now.

...

+    help +      Toshiba TC358764 DSI/LVDS bridge driver

config DRM_TOSHIBA_TC358767       tristate "Toshiba TC358767 eDP bridge"       depends on OF diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile index 35f88d4..bf7c0ce 100644 --- a/drivers/gpu/drm/bridge/Makefile +++ b/drivers/gpu/drm/bridge/Makefile @@ -10,6 +10,7 @@ obj-$(CONFIG_DRM_SIL_SII8620) += sil-sii8620.o   obj-$(CONFIG_DRM_SII902X) += sii902x.o   obj-$(CONFIG_DRM_SII9234) += sii9234.o   obj-$(CONFIG_DRM_THINE_THC63LVD1024) += thc63lvd1024.o +obj-$(CONFIG_DRM_TOSHIBA_TC358764) += tc358764.o   obj-$(CONFIG_DRM_TOSHIBA_TC358767) += tc358767.o   obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix/   obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511/ diff --git a/drivers/gpu/drm/bridge/tc358764.c b/drivers/gpu/drm/bridge/tc358764.c new file mode 100644 index 0000000..3109eba --- /dev/null +++ b/drivers/gpu/drm/bridge/tc358764.c @@ -0,0 +1,547 @@

We'd need a SPDX license here?

...

+/*

• • Copyright (C) 2018 Samsung Electronics Co., Ltd
• • Authors:
• *    Andrzej Hajda a.hajda@samsung.com
• *    Maciej Purski m.purski@samsung.com
• • This program is free software; you can redistribute it and/or modify
• • it under the terms of the GNU General Public License version 2 as
• • published by the Free Software Foundation.
• • This program is distributed in the hope that it will be useful,
• • but WITHOUT ANY WARRANTY; without even the implied warranty of
• • MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
• • GNU General Public License for more details.
• • You should have received a copy of the GNU General Public License
• • along with this program.
• */

+#include <drm/drm_atomic_helper.h>

+#include <drm/drmP.h> +#include <drm/drm_mipi_dsi.h> +#include <drm/drm_panel.h>

+#include <drm/drm_crtc.h> +#include <drm/drm_crtc_helper.h>

+#include <linux/gpio/consumer.h> +#include <linux/of_graph.h> +#include <linux/regulator/consumer.h>

+#include <video/mipi_display.h> +#include <video/of_videomode.h> +#include <video/videomode.h>

+#define FLD_MASK(start, end)    (((1 << ((start) - (end) + 1)) - 1) << (end)) +#define FLD_VAL(val, start, end) (((val) << (end)) & FLD_MASK(start, end))

+/* PPI layer registers */ +#define PPI_STARTPPI        0x0104 /* START control bit */ +#define PPI_LPTXTIMECNT        0x0114 /* LPTX timing signal */ +#define PPI_LANEENABLE        0x0134 /* Enables each lane */ +#define PPI_TX_RX_TA        0x013C /* BTA timing parameters */ +#define PPI_D0S_CLRSIPOCOUNT    0x0164 /* Assertion timer for Lane 0 */ +#define PPI_D1S_CLRSIPOCOUNT    0x0168 /* Assertion timer for Lane 1 */ +#define PPI_D2S_CLRSIPOCOUNT    0x016C /* Assertion timer for Lane 2 */ +#define PPI_D3S_CLRSIPOCOUNT    0x0170 /* Assertion timer for Lane 3 */ +#define PPI_START_FUNCTION    1

+/* DSI layer registers */ +#define DSI_STARTDSI        0x0204 /* START control bit of DSI-TX */ +#define DSI_LANEENABLE        0x0210 /* Enables each lane */ +#define DSI_RX_START        1

+/* Video path registers */ +#define VP_CTRL            0x0450 /* Video Path Control */ +#define VP_CTRL_MSF(v)        FLD_VAL(v, 0, 0) /* Magic square in RGB666 */ +#define VP_CTRL_VTGEN(v)    FLD_VAL(v, 4, 4) /* Use chip clock for timing */ +#define VP_CTRL_EVTMODE(v)    FLD_VAL(v, 5, 5) /* Event mode */ +#define VP_CTRL_RGB888(v)    FLD_VAL(v, 8, 8) /* RGB888 mode */ +#define VP_CTRL_VSDELAY(v)    FLD_VAL(v, 31, 20) /* VSYNC delay */ +#define VP_CTRL_HSPOL        BIT(17) /* Polarity of HSYNC signal */ +#define VP_CTRL_DEPOL        BIT(18) /* Polarity of DE signal */ +#define VP_CTRL_VSPOL        BIT(19) /* Polarity of VSYNC signal */ +#define VP_HTIM1        0x0454 /* Horizontal Timing Control 1 */ +#define VP_HTIM1_HBP(v)        FLD_VAL(v, 24, 16) +#define VP_HTIM1_HSYNC(v)    FLD_VAL(v, 8, 0) +#define VP_HTIM2        0x0458 /* Horizontal Timing Control 2 */ +#define VP_HTIM2_HFP(v)        FLD_VAL(v, 24, 16) +#define VP_HTIM2_HACT(v)    FLD_VAL(v, 10, 0) +#define VP_VTIM1        0x045C /* Vertical Timing Control 1 */ +#define VP_VTIM1_VBP(v)        FLD_VAL(v, 23, 16) +#define VP_VTIM1_VSYNC(v)    FLD_VAL(v, 7, 0) +#define VP_VTIM2        0x0460 /* Vertical Timing Control 2 */ +#define VP_VTIM2_VFP(v)        FLD_VAL(v, 23, 16) +#define VP_VTIM2_VACT(v)    FLD_VAL(v, 10, 0) +#define VP_VFUEN        0x0464 /* Video Frame Timing Update Enable */

+/* LVDS registers */ +#define LV_MX0003        0x0480 /* Mux input bit 0 to 3 */ +#define LV_MX0407        0x0484 /* Mux input bit 4 to 7 */ +#define LV_MX0811        0x0488 /* Mux input bit 8 to 11 */ +#define LV_MX1215        0x048C /* Mux input bit 12 to 15 */ +#define LV_MX1619        0x0490 /* Mux input bit 16 to 19 */ +#define LV_MX2023        0x0494 /* Mux input bit 20 to 23 */ +#define LV_MX2427        0x0498 /* Mux input 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 LV_CFG            0x049C /* LVDS Configuration */ +#define LV_PHY0            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 */

+/* System registers */ +#define SYS_RST            0x0504 /* System Reset */ +#define SYS_ID            0x0580 /* System ID */

+#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 */

+#define LPX_PERIOD        2 +#define TTA_SURE        3 +#define TTA_GET            0x20000

+/* Lane enable PPI and DSI register bits */ +#define LANEENABLE_CLEN        BIT(0) +#define LANEENABLE_L0EN        BIT(1) +#define LANEENABLE_L1EN        BIT(2) +#define LANEENABLE_L2EN        BIT(3) +#define LANEENABLE_L3EN        BIT(4)

+/* LVCFG fields */ +#define LV_CFG_LVEN        BIT(0) +#define LV_CFG_LVDLINK        BIT(1) +#define LV_CFG_CLKPOL1        BIT(2) +#define LV_CFG_CLKPOL2        BIT(3)

+static const char * const tc358764_supplies[] = { +    "vddc", "vddio", "vddmipi", "vddlvds133", "vddlvds112" +};

+struct tc358764 { +    struct device *dev; +    struct drm_bridge bridge; +    struct drm_connector connector; +    struct regulator_bulk_data supplies[ARRAY_SIZE(tc358764_supplies)]; +    struct gpio_desc *gpio_reset;

+    struct drm_panel *panel; +};

+static int tc358764_read(struct tc358764 *ctx, u16 addr, u32 *val) +{ +    struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev); +    const struct mipi_dsi_host_ops *ops = dsi->host->ops; +    struct mipi_dsi_msg msg = { +        .type = MIPI_DSI_GENERIC_READ_REQUEST_2_PARAM, +        .channel = dsi->channel, +        .flags = MIPI_DSI_MSG_USE_LPM, +        .tx_buf = &addr, +        .tx_len = 2, +        .rx_buf = val, +        .rx_len = 4 +    }; +    ssize_t ret;

+    if (!ops || !ops->transfer) +        return -EINVAL;

+    cpu_to_le16s(&addr);

+    ret = ops->transfer(dsi->host, &msg); +    if (ret >= 0) +        le32_to_cpus(val); > + +    dev_dbg(ctx->dev, "read: %d, addr: %d\n", addr, *val);

+    return ret; +}

+static int tc358764_write(struct tc358764 *ctx, u16 addr, u32 val) +{ +    struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev); +    const struct mipi_dsi_host_ops *ops = dsi->host->ops; +    u8 data[6]; +    int ret; +    struct mipi_dsi_msg msg = { +        .type = MIPI_DSI_GENERIC_LONG_WRITE, +        .channel = dsi->channel, +        .flags = MIPI_DSI_MSG_USE_LPM | MIPI_DSI_MSG_REQ_ACK, +        .tx_buf = data, +        .tx_len = 6 +    };

+    if (!ops || !ops->transfer) +        return -EINVAL;

+    data[0] = addr; +    data[1] = addr >> 8; +    data[2] = val; +    data[3] = val >> 8; +    data[4] = val >> 16; +    data[5] = val >> 24;

+    ret = ops->transfer(dsi->host, &msg);

+    return ret; +}

+static inline struct tc358764 *bridge_to_tc358764(struct drm_bridge *bridge) +{ +    return container_of(bridge, struct tc358764, bridge); +}

+static inline +struct tc358764 *connector_to_tc358764(struct drm_connector *connector) +{ +    return container_of(connector, struct tc358764, connector); +}

+static int tc358764_init(struct tc358764 *ctx) +{ +    u32 v = 0;

+    tc358764_read(ctx, SYS_ID, &v); +    dev_info(ctx->dev, "ID: %#x\n", v);

+    /* configure PPI counters */ +    tc358764_write(ctx, PPI_TX_RX_TA, TTA_GET | TTA_SURE); +    tc358764_write(ctx, PPI_LPTXTIMECNT, LPX_PERIOD); +    tc358764_write(ctx, PPI_D0S_CLRSIPOCOUNT, 5); +    tc358764_write(ctx, PPI_D1S_CLRSIPOCOUNT, 5); +    tc358764_write(ctx, PPI_D2S_CLRSIPOCOUNT, 5); +    tc358764_write(ctx, PPI_D3S_CLRSIPOCOUNT, 5);

+    /* enable four data lanes and clock lane */ +    tc358764_write(ctx, PPI_LANEENABLE, LANEENABLE_L3EN | LANEENABLE_L2EN | +               LANEENABLE_L1EN | LANEENABLE_L0EN | LANEENABLE_CLEN); +    tc358764_write(ctx, DSI_LANEENABLE, LANEENABLE_L3EN | LANEENABLE_L2EN | +               LANEENABLE_L1EN | LANEENABLE_L0EN | LANEENABLE_CLEN);

+    /* start */ +    tc358764_write(ctx, PPI_STARTPPI, PPI_START_FUNCTION); +    tc358764_write(ctx, DSI_STARTDSI, DSI_RX_START);

+    /* configure video path */ +    tc358764_write(ctx, VP_CTRL, VP_CTRL_VSDELAY(15) | VP_CTRL_RGB888(1) | +               VP_CTRL_EVTMODE(1) | VP_CTRL_HSPOL | VP_CTRL_VSPOL);

Could we specify somewhere that

Could you clarify, what do you mean here?

...

+    /* reset PHY */ +    tc358764_write(ctx, LV_PHY0, LV_PHY0_RST(1) | +               LV_PHY0_PRBS_ON(4) | LV_PHY0_IS(2) | LV_PHY0_ND(6)); +    tc358764_write(ctx, LV_PHY0, LV_PHY0_PRBS_ON(4) | LV_PHY0_IS(2) | +               LV_PHY0_ND(6));

+    /* reset bridge */ +    tc358764_write(ctx, SYS_RST, SYS_RST_LCD);

+    /* set bit order */ +    tc358764_write(ctx, LV_MX0003, LV_MX(LVI_R0, LVI_R1, LVI_R2, LVI_R3)); +    tc358764_write(ctx, LV_MX0407, LV_MX(LVI_R4, LVI_R7, LVI_R5, LVI_G0)); +    tc358764_write(ctx, LV_MX0811, LV_MX(LVI_G1, LVI_G2, LVI_G6, LVI_G7)); +    tc358764_write(ctx, LV_MX1215, LV_MX(LVI_G3, LVI_G4, LVI_G5, LVI_B0)); +    tc358764_write(ctx, LV_MX1619, LV_MX(LVI_B6, LVI_B7, LVI_B1, LVI_B2)); +    tc358764_write(ctx, LV_MX2023, LV_MX(LVI_B3, LVI_B4, LVI_B5, LVI_L0)); +    tc358764_write(ctx, LV_MX2427, LV_MX(LVI_HS, LVI_VS, LVI_DE, LVI_R6)); +    tc358764_write(ctx, LV_CFG, LV_CFG_CLKPOL2 | LV_CFG_CLKPOL1 | +               LV_CFG_LVEN);

+    return 0; +}

+static void tc358764_reset(struct tc358764 *ctx) +{ +    msleep(20); +    gpiod_set_value(ctx->gpio_reset, 0); +    msleep(20); +    gpiod_set_value(ctx->gpio_reset, 1); +    msleep(40); +}

+static void tc358764_poweroff(struct tc358764 *ctx) +{ +    int ret;

+    tc358764_reset(ctx);

+    drm_panel_disable(ctx->panel); +    msleep(40);

+    ret = regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies); +    if (ret < 0) +        dev_err(ctx->dev, "error disabling regulators (%d)\n", ret); +}

+static int tc358764_get_modes(struct drm_connector *connector) +{ +    struct tc358764 *ctx = connector_to_tc358764(connector);

+    if (ctx->panel && ctx->panel->funcs && ctx->panel->funcs->get_modes) +        return ctx->panel->funcs->get_modes(ctx->panel);

+    return 0; +}

+static const +struct drm_connector_helper_funcs tc358764_connector_helper_funcs = { +    .get_modes = tc358764_get_modes, +};

+static const struct drm_connector_funcs tc358764_connector_funcs = { +    .fill_modes = drm_helper_probe_single_connector_modes, +    .destroy = drm_connector_cleanup, +    .reset = drm_atomic_helper_connector_reset, +    .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, +    .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, +};

+static void tc358764_disable(struct drm_bridge *bridge) +{ +    struct tc358764 *ctx = bridge_to_tc358764(bridge);

+    tc358764_poweroff(ctx); +}

+static void tc358764_pre_enable(struct drm_bridge *bridge) +{ +    struct tc358764 *ctx = bridge_to_tc358764(bridge); +    int ret = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies), +                    ctx->supplies); +    if (ret < 0) +        dev_err(ctx->dev, "error enabling regulators (%d)\n", ret);

+    tc358764_reset(ctx); +    tc358764_init(ctx); +}

+static void tc358764_enable(struct drm_bridge *bridge) +{ +    struct tc358764 *ctx = bridge_to_tc358764(bridge); +    int ret;

+    drm_panel_prepare(ctx->panel);

+    ret = drm_panel_enable(ctx->panel); +    if (ret < 0) +        pr_err("panel enable failed\n");

+    msleep(40); +}

+static int tc358764_attach(struct drm_bridge *bridge) +{ +    struct tc358764 *ctx = bridge_to_tc358764(bridge); +    struct drm_device *drm = bridge->dev; +    int ret;

+    if (!bridge->encoder) { +        DRM_ERROR("Encoder not found\n"); +        return -ENODEV; +    }

+    ctx->connector.polled = DRM_CONNECTOR_POLL_HPD; +    ret = drm_connector_init(drm, &ctx->connector, +                 &tc358764_connector_funcs, +                 DRM_MODE_CONNECTOR_LVDS); +    if (ret) { +        DRM_ERROR("Failed to initialize connector\n"); +        return ret; +    }

+    drm_connector_helper_add(&ctx->connector, +                 &tc358764_connector_helper_funcs);

+    drm_mode_connector_attach_encoder(&ctx->connector, bridge->encoder);

+    if (ctx->panel) +        drm_panel_attach(ctx->panel, &ctx->connector);

+    drm_atomic_helper_connector_reset(&ctx->connector); +    drm_connector_register(&ctx->connector);

+    return 0; +}

+static const struct drm_bridge_funcs tc358764_bridge_funcs = { +    .disable = tc358764_disable, +    .enable = tc358764_enable, +    .pre_enable = tc358764_pre_enable, +    .attach = tc358764_attach, +};

+static struct device_node *tc358764_of_find_panel_node(struct device *dev) +{ +    struct device_node *np, *ep;

+    ep = of_graph_get_endpoint_by_regs(dev->of_node, 1, 0); +    if (!ep) { +        pr_err("faile to get endpoint\n"); +        return NULL; +    }

+    np = of_graph_get_remote_port_parent(ep);

+    return np; +}

+static int tc358764_parse_dt(struct tc358764 *ctx) +{ +    struct device *dev = ctx->dev; +    struct device_node *np = dev->of_node; +    struct device_node *lvds;

+    ctx->gpio_reset = devm_gpiod_get_from_of_node(dev, np, "reset", 0, +                              GPIOD_OUT_LOW, +                              "tc358764-reset"); +    if (IS_ERR(ctx->gpio_reset)) { +        dev_err(dev, "no reset GPIO pin provided\n"); +        return PTR_ERR(ctx->gpio_reset); +    }

+    lvds = tc358764_of_find_panel_node(ctx->dev); +    if (!lvds) { +        dev_err(dev, "cannot find panel node\n"); +        return -EINVAL; +    }

+    ctx->panel = of_drm_find_panel(lvds); +    if (!ctx->panel) { +        dev_err(dev, "panel not registered\n"); +        return -EPROBE_DEFER; +    }

+    return 0; +}

+static int tc358764_configure_regulators(struct tc358764 *ctx) +{ +    int i, ret;

+    for (i = 0; i < ARRAY_SIZE(ctx->supplies); ++i) +        ctx->supplies[i].supply = tc358764_supplies[i];

+    ret = devm_regulator_bulk_get(ctx->dev, ARRAY_SIZE(ctx->supplies), +                      ctx->supplies); +    if (ret < 0) +        dev_err(ctx->dev, "failed to get regulators: %d\n", ret);

+    return ret; +}

+static int tc358764_probe(struct mipi_dsi_device *dsi) +{ +    struct device *dev = &dsi->dev; +    struct tc358764 *ctx; +    int ret;

+    ctx = devm_kzalloc(dev, sizeof(struct tc358764), GFP_KERNEL); +    if (!ctx) +        return -ENOMEM;

+    mipi_dsi_set_drvdata(dsi, ctx);

+    ctx->dev = dev;

+    dsi->lanes = 4; +    dsi->format = MIPI_DSI_FMT_RGB888; +    dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST +        | MIPI_DSI_MODE_VIDEO_AUTO_VERT;

if you use the mipi_dsi_device_transfer() helper, I guess you'd need to add the LPM flag here.

I'll check, if we can use the helper instead of calling ops->transfer() directly.

Thanks,

Maciej Purski

Looks good to me otherwise. Once the DT port issue is concluded:

Reviewed-by: Archit Taneja architt@codeaurora.org

...

+    ret = tc358764_parse_dt(ctx); +    if (ret < 0) +        return ret;

+    ret = tc358764_configure_regulators(ctx); +    if (ret < 0) +        return ret;

+    ctx->bridge.funcs = &tc358764_bridge_funcs; +    ctx->bridge.of_node = dev->of_node;

+    drm_bridge_add(&ctx->bridge);

+    ret = mipi_dsi_attach(dsi); +    if (ret < 0) { +        drm_bridge_remove(&ctx->bridge); +        dev_err(dev, "failed to attach dsi\n"); +    }

+    return ret; +}

+static int tc358764_remove(struct mipi_dsi_device *dsi) +{ +    struct tc358764 *ctx = mipi_dsi_get_drvdata(dsi);

+    tc358764_poweroff(ctx);

+    mipi_dsi_detach(dsi); +    drm_bridge_remove(&ctx->bridge);

+    return 0; +}

+static const struct of_device_id tc358764_of_match[] = { +    { .compatible = "toshiba,tc358764" }, +    { } +}; +MODULE_DEVICE_TABLE(of, tc358764_of_match);

+static struct mipi_dsi_driver tc358764_driver = { +    .probe = tc358764_probe, +    .remove = tc358764_remove, +    .driver = { +        .name = "tc358764", +        .owner = THIS_MODULE, +        .of_match_table = tc358764_of_match, +    }, +}; +module_mipi_dsi_driver(tc358764_driver);

+MODULE_AUTHOR("Andrzej Hajda a.hajda@samsung.com"); +MODULE_AUTHOR("Maciej Purski m.purski@samsung.com"); +MODULE_DESCRIPTION("MIPI-DSI based Driver for TC358764 DSI/LVDS Bridge"); +MODULE_LICENSE("GPL v2");