Re: [Freedreno] [[RFC]DPU PATCH 1/4] drm/bridge: add support for sn65dsi86 bridge driver

2018-04-24 Thread spanda

On 2018-04-21 01:06, Sean Paul wrote:

On Thu, Apr 19, 2018 at 11:26:05PM +0530, Sandeep Panda wrote:

Add support for TI's sn65dsi86 dsi2edp bridge chip.
The chip converts DSI transmitted signal to eDP signal,
which is fed to the connected eDP panel.

This chip can be controlled via either i2c interface or
dsi interface. Currently in driver all the control registers
are being accessed through i2c interface only.
Also as of now HPD support has not been added to bridge
chip driver.

Changes in v1:
 - Split the dt-bindings and the driver support into separate patches
   (Andrzej Hajda).
 - Use of gpiod APIs to parse and configure gpios instead of obsolete 
ones

   (Andrzej Hajda).
 - Use macros to define the register offsets (Andrzej Hajda).

Changes in v2:
 - Separate out edp panel specific HW resource handling from bridge
   driver and create a separate edp panel drivers to handle panel
   specific mode information and HW resources (Sean Paul).
 - Replace pr_* APIs to DRM_* APIs to log error or debug information
   (Sean Paul).
 - Remove some of the unnecessary structure/variable from driver (Sean
   Paul).
 - Rename the function and structure prefix "sn65dsi86" to 
"ti_sn_bridge"

   (Sean Paul / Rob Herring).
 - Remove most of the hard-coding and modified the bridge init 
sequence

   based on current mode (Sean Paul).
 - Remove the existing function to retrieve the EDID data and
   implemented this as an i2c_adapter and use drm_get_edid() (Sean 
Paul).

 - Remove the dummy irq handler implementation, will add back the
   proper irq handling later (Sean Paul).
 - Capture the required enable gpios in a single array based on dt 
entry

   instead of having individual descriptor for each gpio (Sean Paul).

Changes in v3:
 - Remove usage of irq_gpio and replace it as "interrupts" property 
(Rob

   Herring).
 - Remove the unnecessary header file inclusions (Sean Paul).
 - Rearrange the header files in alphabetical order (Sean Paul).
 - Use regmap interface to perform i2c transactions.
 - Update Copyright/License field and address other review comments
   (Jordan Crouse).

Signed-off-by: Sandeep Panda 
---
 drivers/gpu/drm/bridge/ti-sn65dsi86.c | 690 
++


What about Kconfig/Makefile?


Thought of adding this in another change once review is over. But will 
add to current change in next patchset.





 1 file changed, 690 insertions(+)
 create mode 100644 drivers/gpu/drm/bridge/ti-sn65dsi86.c

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c 
b/drivers/gpu/drm/bridge/ti-sn65dsi86.c

new file mode 100644
index 000..a2a95f5
--- /dev/null
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -0,0 +1,690 @@
+/*
+ * Copyright (c) 2018, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or 
modify

+ * it under the terms of the GNU General Public License version 2 and
+ * only 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.
+ */


Use SPDX license


Will add back SPDX along with copyright in next patchset.




+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define SN_BRIDGE_REVISION_ID 0x2
+
+/* Link Training specific registers */
+#define SN_DEVICE_REV_REG  0x08
+#define SN_REFCLK_FREQ_REG 0x0A
+#define SN_DSI_LANES_REG   0x10
+#define SN_DSIA_CLK_FREQ_REG   0x12
+#define SN_ENH_FRAME_REG   0x5A
+#define SN_SSC_CONFIG_REG  0x93
+#define SN_DATARATE_CONFIG_REG 0x94
+#define SN_PLL_ENABLE_REG  0x0D
+#define SN_SCRAMBLE_CONFIG_REG 0x95
+#define SN_AUX_WDATA0_REG  0x64
+#define SN_AUX_ADDR_19_16_REG  0x74
+#define SN_AUX_ADDR_15_8_REG   0x75
+#define SN_AUX_ADDR_7_0_REG0x76
+#define SN_AUX_LENGTH_REG  0x77
+#define SN_AUX_CMD_REG 0x78
+#define SN_ML_TX_MODE_REG  0x96
+/* video config specific registers */
+#define SN_CHA_ACTIVE_LINE_LENGTH_LOW_REG  0x20
+#define SN_CHA_ACTIVE_LINE_LENGTH_HIGH_REG 0x21
+#define SN_CHA_VERTICAL_DISPLAY_SIZE_LOW_REG   0x24
+#define SN_CHA_VERTICAL_DISPLAY_SIZE_HIGH_REG  0x25
+#define SN_CHA_HSYNC_PULSE_WIDTH_LOW_REG   0x2C
+#define SN_CHA_HSYNC_PULSE_WIDTH_HIGH_REG  0x2D
+#define SN_CHA_VSYNC_PULSE_WIDTH_LOW_REG   0x30
+#define SN_CHA_VSYNC_PULSE_WIDTH_HIGH_REG  0x31
+#define SN_CHA_HORIZONTAL_BACK_PORCH_REG   0x34
+#define SN_CHA_VERTICAL_BACK_PORCH_REG 0x36
+#define 

Re: [Freedreno] [[RFC]DPU PATCH 1/4] drm/bridge: add support for sn65dsi86 bridge driver

2018-04-20 Thread Sean Paul
On Thu, Apr 19, 2018 at 11:26:05PM +0530, Sandeep Panda wrote:
> Add support for TI's sn65dsi86 dsi2edp bridge chip.
> The chip converts DSI transmitted signal to eDP signal,
> which is fed to the connected eDP panel.
> 
> This chip can be controlled via either i2c interface or
> dsi interface. Currently in driver all the control registers
> are being accessed through i2c interface only.
> Also as of now HPD support has not been added to bridge
> chip driver.
> 
> Changes in v1:
>  - Split the dt-bindings and the driver support into separate patches
>(Andrzej Hajda).
>  - Use of gpiod APIs to parse and configure gpios instead of obsolete ones
>(Andrzej Hajda).
>  - Use macros to define the register offsets (Andrzej Hajda).
> 
> Changes in v2:
>  - Separate out edp panel specific HW resource handling from bridge
>driver and create a separate edp panel drivers to handle panel
>specific mode information and HW resources (Sean Paul).
>  - Replace pr_* APIs to DRM_* APIs to log error or debug information
>(Sean Paul).
>  - Remove some of the unnecessary structure/variable from driver (Sean
>Paul).
>  - Rename the function and structure prefix "sn65dsi86" to "ti_sn_bridge"
>(Sean Paul / Rob Herring).
>  - Remove most of the hard-coding and modified the bridge init sequence
>based on current mode (Sean Paul).
>  - Remove the existing function to retrieve the EDID data and
>implemented this as an i2c_adapter and use drm_get_edid() (Sean Paul).
>  - Remove the dummy irq handler implementation, will add back the
>proper irq handling later (Sean Paul).
>  - Capture the required enable gpios in a single array based on dt entry
>instead of having individual descriptor for each gpio (Sean Paul).
> 
> Changes in v3:
>  - Remove usage of irq_gpio and replace it as "interrupts" property (Rob
>Herring).
>  - Remove the unnecessary header file inclusions (Sean Paul).
>  - Rearrange the header files in alphabetical order (Sean Paul).
>  - Use regmap interface to perform i2c transactions.
>  - Update Copyright/License field and address other review comments
>(Jordan Crouse).
> 
> Signed-off-by: Sandeep Panda 
> ---
>  drivers/gpu/drm/bridge/ti-sn65dsi86.c | 690 
> ++

What about Kconfig/Makefile?

>  1 file changed, 690 insertions(+)
>  create mode 100644 drivers/gpu/drm/bridge/ti-sn65dsi86.c
> 
> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c 
> b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> new file mode 100644
> index 000..a2a95f5
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> @@ -0,0 +1,690 @@
> +/*
> + * Copyright (c) 2018, The Linux Foundation. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only 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.
> + */

Use SPDX license

> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define SN_BRIDGE_REVISION_ID 0x2
> +
> +/* Link Training specific registers */
> +#define SN_DEVICE_REV_REG0x08
> +#define SN_REFCLK_FREQ_REG   0x0A
> +#define SN_DSI_LANES_REG 0x10
> +#define SN_DSIA_CLK_FREQ_REG 0x12
> +#define SN_ENH_FRAME_REG 0x5A
> +#define SN_SSC_CONFIG_REG0x93
> +#define SN_DATARATE_CONFIG_REG   0x94
> +#define SN_PLL_ENABLE_REG0x0D
> +#define SN_SCRAMBLE_CONFIG_REG   0x95
> +#define SN_AUX_WDATA0_REG0x64
> +#define SN_AUX_ADDR_19_16_REG0x74
> +#define SN_AUX_ADDR_15_8_REG 0x75
> +#define SN_AUX_ADDR_7_0_REG  0x76
> +#define SN_AUX_LENGTH_REG0x77
> +#define SN_AUX_CMD_REG   0x78
> +#define SN_ML_TX_MODE_REG0x96
> +/* video config specific registers */
> +#define SN_CHA_ACTIVE_LINE_LENGTH_LOW_REG0x20
> +#define SN_CHA_ACTIVE_LINE_LENGTH_HIGH_REG   0x21
> +#define SN_CHA_VERTICAL_DISPLAY_SIZE_LOW_REG 0x24
> +#define SN_CHA_VERTICAL_DISPLAY_SIZE_HIGH_REG0x25
> +#define SN_CHA_HSYNC_PULSE_WIDTH_LOW_REG 0x2C
> +#define SN_CHA_HSYNC_PULSE_WIDTH_HIGH_REG0x2D
> +#define SN_CHA_VSYNC_PULSE_WIDTH_LOW_REG 0x30
> +#define SN_CHA_VSYNC_PULSE_WIDTH_HIGH_REG0x31
> +#define SN_CHA_HORIZONTAL_BACK_PORCH_REG 0x34
> +#define SN_CHA_VERTICAL_BACK_PORCH_REG   0x36
> +#define SN_CHA_HORIZONTAL_FRONT_PORCH_REG

Re: [Freedreno] [[RFC]DPU PATCH 1/4] drm/bridge: add support for sn65dsi86 bridge driver

2018-04-19 Thread Jordan Crouse
On Wed, Apr 18, 2018 at 05:49:59PM +0530, Sandeep Panda wrote:
> Add support for TI's sn65dsi86 dsi2edp bridge chip.
> The chip converts DSI transmitted signal to eDP signal,
> which is fed to the connected eDP panel.
> 
> This chip can be controlled via either i2c interface or
> dsi interface. Currently in driver all the control registers
> are being accessed through i2c interface only.
> Also as of now HPD support has not been added to bridge
> chip driver.
> 
> Signed-off-by: Sandeep Panda 
> ---
>  drivers/gpu/drm/bridge/ti-sn65dsi86.c | 742 
> ++
>  1 file changed, 742 insertions(+)
>  create mode 100644 drivers/gpu/drm/bridge/ti-sn65dsi86.c
> 
> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c 
> b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> new file mode 100644
> index 000..c798f2f
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> @@ -0,0 +1,742 @@
> +/* SPDX-License-Identifier: GPL-2.0 */

There should be a copyright here.

> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define SN_BRIDGE_REVISION_ID 0x2
> +
> +/* Link Training specific registers */
> +#define SN_DEVICE_REV_REG0x08
> +#define SN_REFCLK_FREQ_REG   0x0A
> +#define SN_DSI_LANES_REG 0x10
> +#define SN_DSIA_CLK_FREQ_REG 0x12
> +#define SN_ENH_FRAME_REG 0x5A
> +#define SN_SSC_CONFIG_REG0x93
> +#define SN_DATARATE_CONFIG_REG   0x94
> +#define SN_PLL_ENABLE_REG0x0D
> +#define SN_SCRAMBLE_CONFIG_REG   0x95
> +#define SN_AUX_WDATA0_REG0x64
> +#define SN_AUX_ADDR_19_16_REG0x74
> +#define SN_AUX_ADDR_15_8_REG 0x75
> +#define SN_AUX_ADDR_7_0_REG  0x76
> +#define SN_AUX_LENGTH_REG0x77
> +#define SN_AUX_CMD_REG   0x78
> +#define SN_ML_TX_MODE_REG0x96
> +#define SN_PAGE_SELECT_REG   0xFF
> +#define SN_AUX_RDATA0_REG0x79
> +/* video config specific registers */
> +#define SN_CHA_ACTIVE_LINE_LENGTH_LOW_REG0x20
> +#define SN_CHA_ACTIVE_LINE_LENGTH_HIGH_REG   0x21
> +#define SN_CHA_VERTICAL_DISPLAY_SIZE_LOW_REG 0x24
> +#define SN_CHA_VERTICAL_DISPLAY_SIZE_HIGH_REG0x25
> +#define SN_CHA_HSYNC_PULSE_WIDTH_LOW_REG 0x2C
> +#define SN_CHA_HSYNC_PULSE_WIDTH_HIGH_REG0x2D
> +#define SN_CHA_VSYNC_PULSE_WIDTH_LOW_REG 0x30
> +#define SN_CHA_VSYNC_PULSE_WIDTH_HIGH_REG0x31
> +#define SN_CHA_HORIZONTAL_BACK_PORCH_REG 0x34
> +#define SN_CHA_VERTICAL_BACK_PORCH_REG   0x36
> +#define SN_CHA_HORIZONTAL_FRONT_PORCH_REG0x38
> +#define SN_CHA_VERTICAL_FRONT_PORCH_REG  0x3A
> +#define SN_DATA_FORMAT_REG   0x5B
> +#define SN_COLOR_BAR_CONFIG_REG  0x3C
> +
> +#define DPPLL_CLK_SRC_REFCLK 0
> +#define DPPLL_CLK_SRC_DSICLK 1
> +#define MIN_DSI_CLK_FREQ_MHZ 40
> +
> +enum ti_sn_bridge_ref_clks {
> + SN_REF_CLK_12_MHZ = 0,
> + SN_REF_CLK_19_2_MHZ,
> + SN_REF_CLK_26_MHZ,
> + SN_REF_CLK_27_MHZ,
> + SN_REF_CLK_38_4_MHZ,
> + SN_REF_CLK_MAX,
> +};
> +
> +struct ti_sn_bridge {
> + struct device *dev;
> + struct drm_bridge bridge;
> + struct drm_connector connector;
> + struct device_node *host_node;
> + struct mipi_dsi_device *dsi;
> + /* handle to the connected eDP panel */
> + struct drm_panel *panel;
> + int irq;
> + struct gpio_desc *irq_gpio;
> + /* list of gpios needed to enable the bridge functionality */
> + struct gpio_descs *enable_gpios;
> + unsigned int num_supplies;
> + struct regulator_bulk_data *supplies;
> + struct i2c_client *i2c_client;
> + struct i2c_adapter *ddc;
> + bool power_on;
> + u32 num_modes;
> + struct drm_display_mode curr_mode;
> +};
> +
> +static int ti_sn_bridge_write(struct ti_sn_bridge *pdata, u8 reg, u8 val)
> +{
> + struct i2c_client *client = pdata->i2c_client;
> + u8 buf[2] = {reg, val};
> + struct i2c_msg msg = {
> + .addr = client->addr,
> + .flags = 0,
> + .len = 2,
> + .buf = buf,
> + };
> +
> + if (i2c_transfer(client->adapter, , 1) < 1) {
> + DRM_ERROR("i2c write failed\n");
> + return -EIO;
> + }
> +
> + return 0;
> +}
> +
> +static int ti_sn_bridge_read(struct ti_sn_bridge *pdata,
> + u8 reg, char *buf, u32 size)
> +{
> + struct i2c_client *client = pdata->i2c_client;
> + struct i2c_msg msg[2] = {
> + {
> + .addr = client->addr,
> + .flags = 0,
> +  

Re: [Freedreno] [[RFC]DPU PATCH 1/4] drm/bridge: add support for sn65dsi86 bridge driver

2018-04-18 Thread spanda

On 2018-04-18 19:41, Sean Paul wrote:

On Wed, Apr 18, 2018 at 05:49:59PM +0530, Sandeep Panda wrote:

Add support for TI's sn65dsi86 dsi2edp bridge chip.
The chip converts DSI transmitted signal to eDP signal,
which is fed to the connected eDP panel.

This chip can be controlled via either i2c interface or
dsi interface. Currently in driver all the control registers
are being accessed through i2c interface only.
Also as of now HPD support has not been added to bridge
chip driver.

Signed-off-by: Sandeep Panda 
---
 drivers/gpu/drm/bridge/ti-sn65dsi86.c | 742 
++

 1 file changed, 742 insertions(+)
 create mode 100644 drivers/gpu/drm/bridge/ti-sn65dsi86.c

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c 
b/drivers/gpu/drm/bridge/ti-sn65dsi86.c

new file mode 100644
index 000..c798f2f
--- /dev/null
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -0,0 +1,742 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 


I have a hard time believing that you need all of these includes. Off 
the top of
my head, you could probably remove types, kernel, init, 
platform_device, delay,
drmP, drm_atomic, drm_crtc_helper. You could probably trim it even 
further with

the help of your compiler. These should also be in alphabetical order.


+
+#define SN_BRIDGE_REVISION_ID 0x2
+
+/* Link Training specific registers */
+#define SN_DEVICE_REV_REG  0x08
+#define SN_REFCLK_FREQ_REG 0x0A
+#define SN_DSI_LANES_REG   0x10
+#define SN_DSIA_CLK_FREQ_REG   0x12
+#define SN_ENH_FRAME_REG   0x5A
+#define SN_SSC_CONFIG_REG  0x93
+#define SN_DATARATE_CONFIG_REG 0x94
+#define SN_PLL_ENABLE_REG  0x0D
+#define SN_SCRAMBLE_CONFIG_REG 0x95
+#define SN_AUX_WDATA0_REG  0x64
+#define SN_AUX_ADDR_19_16_REG  0x74
+#define SN_AUX_ADDR_15_8_REG   0x75
+#define SN_AUX_ADDR_7_0_REG0x76
+#define SN_AUX_LENGTH_REG  0x77
+#define SN_AUX_CMD_REG 0x78
+#define SN_ML_TX_MODE_REG  0x96
+#define SN_PAGE_SELECT_REG 0xFF
+#define SN_AUX_RDATA0_REG  0x79
+/* video config specific registers */
+#define SN_CHA_ACTIVE_LINE_LENGTH_LOW_REG  0x20
+#define SN_CHA_ACTIVE_LINE_LENGTH_HIGH_REG 0x21
+#define SN_CHA_VERTICAL_DISPLAY_SIZE_LOW_REG   0x24
+#define SN_CHA_VERTICAL_DISPLAY_SIZE_HIGH_REG  0x25
+#define SN_CHA_HSYNC_PULSE_WIDTH_LOW_REG   0x2C
+#define SN_CHA_HSYNC_PULSE_WIDTH_HIGH_REG  0x2D
+#define SN_CHA_VSYNC_PULSE_WIDTH_LOW_REG   0x30
+#define SN_CHA_VSYNC_PULSE_WIDTH_HIGH_REG  0x31
+#define SN_CHA_HORIZONTAL_BACK_PORCH_REG   0x34
+#define SN_CHA_VERTICAL_BACK_PORCH_REG 0x36
+#define SN_CHA_HORIZONTAL_FRONT_PORCH_REG  0x38
+#define SN_CHA_VERTICAL_FRONT_PORCH_REG0x3A
+#define SN_DATA_FORMAT_REG 0x5B
+#define SN_COLOR_BAR_CONFIG_REG0x3C
+
+#define DPPLL_CLK_SRC_REFCLK   0
+#define DPPLL_CLK_SRC_DSICLK   1
+#define MIN_DSI_CLK_FREQ_MHZ   40
+
+enum ti_sn_bridge_ref_clks {
+   SN_REF_CLK_12_MHZ = 0,
+   SN_REF_CLK_19_2_MHZ,
+   SN_REF_CLK_26_MHZ,
+   SN_REF_CLK_27_MHZ,
+   SN_REF_CLK_38_4_MHZ,
+   SN_REF_CLK_MAX,
+};
+
+struct ti_sn_bridge {
+   struct device *dev;
+   struct drm_bridge bridge;
+   struct drm_connector connector;
+   struct device_node *host_node;
+   struct mipi_dsi_device *dsi;
+   /* handle to the connected eDP panel */
+   struct drm_panel *panel;
+   int irq;
+   struct gpio_desc *irq_gpio;
+   /* list of gpios needed to enable the bridge functionality */
+   struct gpio_descs *enable_gpios;


Why are the enable gpios a list?


This as per the review comment in previous patchset
"I see IRQ and EN in the datasheet, but not the others. It seems like 
the aux_*
and edp_* gpios are always equal to en. So if you actually need them, 
don't
specify a new struct, just add irq_gpio to the main struct and add an 
array of

en_gpios to handle the rest."

I see in schematic 2 more gpios are needed to enable aux_channel 
communication
through the ddc. So i have put an array of enable gpios. Based on dt it 
will dynamically

parse one or more gpios.




+   unsigned int num_supplies;
+   struct regulator_bulk_data *supplies;
+   struct i2c_client *i2c_client;
+   struct i2c_adapter *ddc;
+   bool power_on;
+   u32 num_modes;
+   struct drm_display_mode curr_mode;
+};
+
+static int ti_sn_bridge_write(struct ti_sn_bridge *pdata, u8 reg, u8 
val)

+{
+   struct i2c_client 

Re: [Freedreno] [[RFC]DPU PATCH 1/4] drm/bridge: add support for sn65dsi86 bridge driver

2018-04-18 Thread spanda

On 2018-04-18 19:41, Sean Paul wrote:

On Wed, Apr 18, 2018 at 05:49:59PM +0530, Sandeep Panda wrote:

Add support for TI's sn65dsi86 dsi2edp bridge chip.
The chip converts DSI transmitted signal to eDP signal,
which is fed to the connected eDP panel.

This chip can be controlled via either i2c interface or
dsi interface. Currently in driver all the control registers
are being accessed through i2c interface only.
Also as of now HPD support has not been added to bridge
chip driver.

Signed-off-by: Sandeep Panda 
---
 drivers/gpu/drm/bridge/ti-sn65dsi86.c | 742 
++

 1 file changed, 742 insertions(+)
 create mode 100644 drivers/gpu/drm/bridge/ti-sn65dsi86.c

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c 
b/drivers/gpu/drm/bridge/ti-sn65dsi86.c

new file mode 100644
index 000..c798f2f
--- /dev/null
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -0,0 +1,742 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 


I have a hard time believing that you need all of these includes. Off 
the top of
my head, you could probably remove types, kernel, init, 
platform_device, delay,
drmP, drm_atomic, drm_crtc_helper. You could probably trim it even 
further with

the help of your compiler. These should also be in alphabetical order.

I will take care of alphabetical order and generic kernel header file 
inclusion. But the header files of

drmP, drm_atomic, drm_crtc_helper are required here.

drmP, drm_atomic, drm_crtc_helper all these are needed because of the 
drm api that we using in this driver
for modes, connectors and bridges. the drm header files i have already 
checked with compiler.




+
+#define SN_BRIDGE_REVISION_ID 0x2
+
+/* Link Training specific registers */
+#define SN_DEVICE_REV_REG  0x08
+#define SN_REFCLK_FREQ_REG 0x0A
+#define SN_DSI_LANES_REG   0x10
+#define SN_DSIA_CLK_FREQ_REG   0x12
+#define SN_ENH_FRAME_REG   0x5A
+#define SN_SSC_CONFIG_REG  0x93
+#define SN_DATARATE_CONFIG_REG 0x94
+#define SN_PLL_ENABLE_REG  0x0D
+#define SN_SCRAMBLE_CONFIG_REG 0x95
+#define SN_AUX_WDATA0_REG  0x64
+#define SN_AUX_ADDR_19_16_REG  0x74
+#define SN_AUX_ADDR_15_8_REG   0x75
+#define SN_AUX_ADDR_7_0_REG0x76
+#define SN_AUX_LENGTH_REG  0x77
+#define SN_AUX_CMD_REG 0x78
+#define SN_ML_TX_MODE_REG  0x96
+#define SN_PAGE_SELECT_REG 0xFF
+#define SN_AUX_RDATA0_REG  0x79
+/* video config specific registers */
+#define SN_CHA_ACTIVE_LINE_LENGTH_LOW_REG  0x20
+#define SN_CHA_ACTIVE_LINE_LENGTH_HIGH_REG 0x21
+#define SN_CHA_VERTICAL_DISPLAY_SIZE_LOW_REG   0x24
+#define SN_CHA_VERTICAL_DISPLAY_SIZE_HIGH_REG  0x25
+#define SN_CHA_HSYNC_PULSE_WIDTH_LOW_REG   0x2C
+#define SN_CHA_HSYNC_PULSE_WIDTH_HIGH_REG  0x2D
+#define SN_CHA_VSYNC_PULSE_WIDTH_LOW_REG   0x30
+#define SN_CHA_VSYNC_PULSE_WIDTH_HIGH_REG  0x31
+#define SN_CHA_HORIZONTAL_BACK_PORCH_REG   0x34
+#define SN_CHA_VERTICAL_BACK_PORCH_REG 0x36
+#define SN_CHA_HORIZONTAL_FRONT_PORCH_REG  0x38
+#define SN_CHA_VERTICAL_FRONT_PORCH_REG0x3A
+#define SN_DATA_FORMAT_REG 0x5B
+#define SN_COLOR_BAR_CONFIG_REG0x3C
+
+#define DPPLL_CLK_SRC_REFCLK   0
+#define DPPLL_CLK_SRC_DSICLK   1
+#define MIN_DSI_CLK_FREQ_MHZ   40
+
+enum ti_sn_bridge_ref_clks {
+   SN_REF_CLK_12_MHZ = 0,
+   SN_REF_CLK_19_2_MHZ,
+   SN_REF_CLK_26_MHZ,
+   SN_REF_CLK_27_MHZ,
+   SN_REF_CLK_38_4_MHZ,
+   SN_REF_CLK_MAX,
+};
+
+struct ti_sn_bridge {
+   struct device *dev;
+   struct drm_bridge bridge;
+   struct drm_connector connector;
+   struct device_node *host_node;
+   struct mipi_dsi_device *dsi;
+   /* handle to the connected eDP panel */
+   struct drm_panel *panel;
+   int irq;
+   struct gpio_desc *irq_gpio;
+   /* list of gpios needed to enable the bridge functionality */
+   struct gpio_descs *enable_gpios;


Why are the enable gpios a list?


+   unsigned int num_supplies;
+   struct regulator_bulk_data *supplies;
+   struct i2c_client *i2c_client;
+   struct i2c_adapter *ddc;
+   bool power_on;
+   u32 num_modes;
+   struct drm_display_mode curr_mode;
+};
+
+static int ti_sn_bridge_write(struct ti_sn_bridge *pdata, u8 reg, u8 
val)

+{
+   struct i2c_client *client = pdata->i2c_client;
+   u8 buf[2] = {reg, val};
+   struct i2c_msg msg = {
+   .addr = client->addr,
+   .flags = 0,
+