[PATCH v11 0/5] Add suppport for sn65dsi86 bridge chip and Innolux 2k edp panel

[PATCH v11 0/5] Add suppport for sn65dsi86 bridge chip and Innolux 2k edp panel

[Sandeep Panda]

Changes in current patchset:
 - Read DPPLL_SRC register to determine if continuous dsi clock needed or not.
 - Reorder the patchsets.

Sandeep Panda (5):
  dt-bindings: media: extend interface documentation for DSI and DP
  drm/bridge: add support for sn65dsi86 bridge driver
  dt-bindings: drm/bridge: Document sn65dsi86 bridge bindings
  drm/panel: add Innolux TV123WAM panel driver support
  dt-bindings: drm/panel: Document Innolux TV123WAM panel bindings

 .../bindings/display/bridge/ti,sn65dsi86.txt       |  87 +++
 .../bindings/display/panel/innolux,tv123wam.txt    |  20 +
 .../devicetree/bindings/media/video-interfaces.txt |  10 +-
 drivers/gpu/drm/bridge/Kconfig                     |   9 +
 drivers/gpu/drm/bridge/Makefile                    |   1 +
 drivers/gpu/drm/bridge/ti-sn65dsi86.c              | 696 +++++++++++++++++++++
 drivers/gpu/drm/panel/panel-simple.c               |  27 +
 7 files changed, 846 insertions(+), 4 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt
 create mode 100644 Documentation/devicetree/bindings/display/panel/innolux,tv123wam.txt
 create mode 100644 drivers/gpu/drm/bridge/ti-sn65dsi86.c

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[PATCH v11 1/5] dt-bindings: media: extend interface documentation for DSI and DP

[Sandeep Panda]

Properties like data-lanes, clock-noncontinuous and lane-polarities
are applicable to DSI and DisplayPort interface also. So update the
documentation to mention the same.

Signed-off-by: Sandeep Panda <spanda@codeaurora.org>
---
 Documentation/devicetree/bindings/media/video-interfaces.txt | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt b/Documentation/devicetree/bindings/media/video-interfaces.txt
index 852041a..f53d8e4 100644
--- a/Documentation/devicetree/bindings/media/video-interfaces.txt
+++ b/Documentation/devicetree/bindings/media/video-interfaces.txt
@@ -99,15 +99,16 @@ Optional endpoint properties
   determines the logical lane number, while the value of an entry indicates
   physical lane, e.g. for 2-lane MIPI CSI-2 bus we could have
   "data-lanes = <1 2>;", assuming the clock lane is on hardware lane 0.
-  This property is valid for serial busses only (e.g. MIPI CSI-2).
+  This property is valid for serial busses only (e.g. MIPI CSI-2, MIPI DSI,
+  DisplayPort).
 - clock-lanes: an array of physical clock lane indexes. Position of an entry
   determines the logical lane number, while the value of an entry indicates
   physical lane, e.g. for a MIPI CSI-2 bus we could have "clock-lanes = <0>;",
   which places the clock lane on hardware lane 0. This property is valid for
   serial busses only (e.g. MIPI CSI-2). Note that for the MIPI CSI-2 bus this
   array contains only one entry.
-- clock-noncontinuous: a boolean property to allow MIPI CSI-2 non-continuous
-  clock mode.
+- clock-noncontinuous: a boolean property to allow non-continuous clock mode.
+  This property is valid for serial busses (e.g. MIPI CSI-2, MIPI DSI and DisplayPort).
 - link-frequencies: Allowed data bus frequencies. For MIPI CSI-2, for
   instance, this is the actual frequency of the bus, not bits per clock per
   lane value. An array of 64-bit unsigned integers.
@@ -116,7 +117,8 @@ Optional endpoint properties
   Valid values are 0 (normal) and 1 (inverted). The length of the array
   should be the combined length of data-lanes and clock-lanes properties.
   If the lane-polarities property is omitted, the value must be interpreted
-  as 0 (normal). This property is valid for serial busses only.
+  as 0 (normal). This property is valid for serial busses only (e.g. MIPI CSI-2,
+  MIPI DSI, DisplayPort).
 - strobe: Whether the clock signal is used as clock (0) or strobe (1). Used
   with CCP2, for instance.
 
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH v11 1/5] dt-bindings: media: extend interface documentation for DSI and DP

[Rob Herring]

On Fri, Jun 15, 2018 at 12:13:39PM +0530, Sandeep Panda wrote:
> Properties like data-lanes, clock-noncontinuous and lane-polarities
> are applicable to DSI and DisplayPort interface also. So update the
> documentation to mention the same.
> 
> Signed-off-by: Sandeep Panda <spanda@codeaurora.org>
> ---
>  Documentation/devicetree/bindings/media/video-interfaces.txt | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)

Reviewed-by: Rob Herring <robh@kernel.org>
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[PATCH v11 2/5] drm/bridge: add support for sn65dsi86 bridge driver

[Sandeep Panda]

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).

Changes in v4:
 - Update License/Copyright (Sean Paul).
 - Add Kconfig and Makefile changes (Sean Paul).
 - Drop i2c gpio handling from this bridge driver, since i2c sda/scl gpios
   will be handled by i2c master.
 - Update required supplies names.
 - Remove unnecessary goto statements (Sean Paul).
 - Add mutex lock to power_ctrl API to avoid race conditions (Sean
   Paul).
 - Add support to parse reference clk frequency from dt(optional).
 - Update the bridge chip enable/disable sequence.

Changes in v5:
 - Fixed Kbuild test service reported warnings.

Changes in v6:
 - Use PM runtime based ref-counting instead of local ref_count mechanism
   (Stephen Boyd).
 - Clean up some debug logs and indentations (Sean Paul).
 - Simplify dp rate calculation (Sean Paul).
 - Add support to configure refclk based on input REFCLK pin or DACP/N
   pin (Stephen Boyd).

Changes in v7:
 - Use static supply entries instead of dynamic allocation (Andrzej
   Hajda).
 - Defer bridge driver probe if panel is not probed (Andrzej Hajda).
 - Update of_graph APIs for correct node reference management. (Andrzej
   Hajda).
 - Remove local display_mode object (Andrzej Hajda).
 - Remove version id check function from driver.

Changes in v8:
 - Move dsi register/attach function to bridge driver probe (Andrzej
   Hajda).
 - Introduce a new helper function to write 16bit words into consecutive
   registers (Andrzej Hajda).
 - Remove unnecessary macros (Andrzej Hajda).

Changes in v9:
 - Remove dsi register/attach from bridge probe, since dsi dev register
   completion also waits for any panel or bridge to get added. This creates
   deadlock situation when bridge driver calls dsi dev register and
   attach before bridge add, in its probe function.
 - Fix issues faced during testing of bridge driver on actual HW.
 - Remove unnecessary initializations (Stephen Boyd).
 - Use local refclk lut size instead of global macro (Sean Paul).

Changes in v10:
 - Use refclk to determine if continuous dsi clock is needed or not.

Changes in v11:
 - Read DPPLL_SRC register to determine continuous clock instead of
   using refclk handle (Stephen Boyd).

Signed-off-by: Sandeep Panda <spanda@codeaurora.org>
---
 drivers/gpu/drm/bridge/Kconfig        |   9 +
 drivers/gpu/drm/bridge/Makefile       |   1 +
 drivers/gpu/drm/bridge/ti-sn65dsi86.c | 696 ++++++++++++++++++++++++++++++++++
 3 files changed, 706 insertions(+)
 create mode 100644 drivers/gpu/drm/bridge/ti-sn65dsi86.c

diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index 3b99d5a..8153150 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -108,6 +108,15 @@ config DRM_TI_TFP410
 	---help---
 	  Texas Instruments TFP410 DVI/HDMI Transmitter driver
 
+config DRM_TI_SN65DSI86
+	tristate "TI SN65DSI86 DSI to eDP bridge"
+	depends on OF
+	select DRM_KMS_HELPER
+	select REGMAP_I2C
+	select DRM_PANEL
+	---help---
+	  Texas Instruments SN65DSI86 DSI to eDP Bridge driver
+
 source "drivers/gpu/drm/bridge/analogix/Kconfig"
 
 source "drivers/gpu/drm/bridge/adv7511/Kconfig"
diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
index 373eb28..a7ca000 100644
--- a/drivers/gpu/drm/bridge/Makefile
+++ b/drivers/gpu/drm/bridge/Makefile
@@ -11,5 +11,6 @@ obj-$(CONFIG_DRM_SII9234) += sii9234.o
 obj-$(CONFIG_DRM_TOSHIBA_TC358767) += tc358767.o
 obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix/
 obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511/
+obj-$(CONFIG_DRM_TI_SN65DSI86) += ti-sn65dsi86.o
 obj-$(CONFIG_DRM_TI_TFP410) += ti-tfp410.o
 obj-y += synopsys/
diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
new file mode 100644
index 0000000..5473456
--- /dev/null
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -0,0 +1,696 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018, The Linux Foundation. All rights reserved.
+ */
+
+#include <drm/drmP.h>
+#include <drm/drm_atomic.h>
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_crtc_helper.h>
+#include <drm/drm_mipi_dsi.h>
+#include <drm/drm_of.h>
+#include <drm/drm_panel.h>
+#include <linux/clk.h>
+#include <linux/gpio/consumer.h>
+#include <linux/i2c.h>
+#include <linux/of_graph.h>
+#include <linux/pm_runtime.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+
+/* Link Training specific registers */
+#define SN_DEVICE_REV_REG			0x08
+#define SN_HPD_DISABLE_REG			0x5C
+#define SN_DPPLL_SRC_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_REG			0x76
+#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_VERTICAL_DISPLAY_SIZE_LOW_REG	0x24
+#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_REG		0x3A
+#define SN_DATA_FORMAT_REG			0x5B
+
+#define MIN_DSI_CLK_FREQ_MHZ	40
+
+/* fudge factor required to account for 8b/10b encoding */
+#define DP_CLK_FUDGE_NUM	10
+#define DP_CLK_FUDGE_DEN	8
+
+#define DPPLL_CLK_SRC_REFCLK	0
+#define DPPLL_CLK_SRC_DSICLK	1
+
+#define SN_REFCLK_FREQ_OFFSET	1
+#define SN_DSIA_LANE_OFFSET	3
+#define SN_DP_LANE_OFFSET	4
+#define SN_DP_DATA_RATE_OFFSET	5
+#define SN_SYNC_POLARITY_OFFSET	7
+
+#define SN_ENABLE_VID_STREAM_BIT	BIT(3)
+#define SN_REFCLK_FREQ_BITS		(BIT(3) | BIT(2) | BIT(1))
+#define SN_DSIA_NUM_LANES_BITS		(BIT(4) | BIT(3))
+#define SN_DP_NUM_LANES_BITS		(BIT(5) | BIT(4))
+#define SN_DP_DATA_RATE_BITS		(BIT(7) | BIT(6) | BIT(5))
+#define SN_HPD_DISABLE_BIT		BIT(0)
+
+#define SN_REGULATOR_SUPPLY_NUM		4
+
+struct ti_sn_bridge {
+	struct device			*dev;
+	struct regmap			*regmap;
+	struct drm_bridge		bridge;
+	struct drm_connector		connector;
+	struct device_node		*host_node;
+	struct mipi_dsi_device		*dsi;
+	struct clk			*refclk;
+	struct drm_panel		*panel;
+	struct gpio_desc		*enable_gpio;
+	struct regulator_bulk_data	supplies[SN_REGULATOR_SUPPLY_NUM];
+	struct i2c_adapter		*ddc;
+};
+
+static const struct regmap_range ti_sn_bridge_volatile_ranges[] = {
+	{ .range_min = 0, .range_max = 0xFF },
+};
+
+static const struct regmap_access_table ti_sn_bridge_volatile_table = {
+	.yes_ranges = ti_sn_bridge_volatile_ranges,
+	.n_yes_ranges = ARRAY_SIZE(ti_sn_bridge_volatile_ranges),
+};
+
+static const struct regmap_config ti_sn_bridge_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.volatile_table = &ti_sn_bridge_volatile_table,
+	.cache_type = REGCACHE_NONE,
+};
+
+static void ti_sn_bridge_write_u16(struct ti_sn_bridge *pdata,
+				   unsigned int reg, u16 val)
+{
+	regmap_write(pdata->regmap, reg, val & 0xFF);
+	regmap_write(pdata->regmap, reg + 1, val >> 8);
+}
+
+static int __maybe_unused ti_sn_bridge_resume(struct device *dev)
+{
+	struct ti_sn_bridge *pdata = dev_get_drvdata(dev);
+	int ret;
+
+	ret = regulator_bulk_enable(SN_REGULATOR_SUPPLY_NUM, pdata->supplies);
+	if (ret) {
+		DRM_ERROR("failed to enable supplies %d\n", ret);
+		return ret;
+	}
+
+	gpiod_set_value(pdata->enable_gpio, 1);
+
+	return ret;
+}
+
+static int __maybe_unused ti_sn_bridge_suspend(struct device *dev)
+{
+	struct ti_sn_bridge *pdata = dev_get_drvdata(dev);
+	int ret;
+
+	gpiod_set_value(pdata->enable_gpio, 0);
+
+	ret = regulator_bulk_disable(SN_REGULATOR_SUPPLY_NUM, pdata->supplies);
+	if (ret)
+		DRM_ERROR("failed to disable supplies %d\n", ret);
+
+	return ret;
+}
+
+static const struct dev_pm_ops ti_sn_bridge_pm_ops = {
+	SET_RUNTIME_PM_OPS(ti_sn_bridge_suspend, ti_sn_bridge_resume, NULL)
+};
+
+/* Connector funcs */
+static struct ti_sn_bridge *
+connector_to_ti_sn_bridge(struct drm_connector *connector)
+{
+	return container_of(connector, struct ti_sn_bridge, connector);
+}
+
+static int ti_sn_bridge_connector_get_modes(struct drm_connector *connector)
+{
+	struct ti_sn_bridge *pdata = connector_to_ti_sn_bridge(connector);
+	struct edid *edid;
+	u32 num_modes;
+
+	if (pdata->panel) {
+		DRM_DEBUG_KMS("get mode from connected drm_panel\n");
+		return drm_panel_get_modes(pdata->panel);
+	}
+
+	if (!pdata->ddc)
+		return 0;
+
+	pm_runtime_get_sync(pdata->dev);
+	edid = drm_get_edid(connector, pdata->ddc);
+	pm_runtime_put_sync(pdata->dev);
+	if (!edid)
+		return 0;
+
+	drm_mode_connector_update_edid_property(connector, edid);
+	num_modes = drm_add_edid_modes(connector, edid);
+	kfree(edid);
+
+	return num_modes;
+}
+
+static enum drm_mode_status
+ti_sn_bridge_connector_mode_valid(struct drm_connector *connector,
+			     struct drm_display_mode *mode)
+{
+	/* maximum supported resolution is 4K at 60 fps */
+	if (mode->clock > 594000)
+		return MODE_CLOCK_HIGH;
+
+	return MODE_OK;
+}
+
+static struct drm_connector_helper_funcs ti_sn_bridge_connector_helper_funcs = {
+	.get_modes = ti_sn_bridge_connector_get_modes,
+	.mode_valid = ti_sn_bridge_connector_mode_valid,
+};
+
+static enum drm_connector_status
+ti_sn_bridge_connector_detect(struct drm_connector *connector, bool force)
+{
+	struct ti_sn_bridge *pdata = connector_to_ti_sn_bridge(connector);
+
+	/**
+	 * TODO: Currently if drm_panel is present, then always
+	 * return the status as connected. Need to add support to detect
+	 * device state for no panel(hot pluggable) scenarios.
+	 */
+	if (pdata->panel)
+		return connector_status_connected;
+	else
+		return connector_status_unknown;
+}
+
+static const struct drm_connector_funcs ti_sn_bridge_connector_funcs = {
+	.fill_modes = drm_helper_probe_single_connector_modes,
+	.detect = ti_sn_bridge_connector_detect,
+	.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 struct ti_sn_bridge *bridge_to_ti_sn_bridge(struct drm_bridge *bridge)
+{
+	return container_of(bridge, struct ti_sn_bridge, bridge);
+}
+
+static int ti_sn_bridge_parse_regulators(struct ti_sn_bridge *pdata)
+{
+	unsigned int i;
+	const char * const ti_sn_bridge_supply_names[] = {
+		"vcca", "vcc", "vccio", "vpll",
+	};
+
+	for (i = 0; i < SN_REGULATOR_SUPPLY_NUM; i++)
+		pdata->supplies[i].supply = ti_sn_bridge_supply_names[i];
+
+	return devm_regulator_bulk_get(pdata->dev, SN_REGULATOR_SUPPLY_NUM,
+				       pdata->supplies);
+}
+
+static int ti_sn_bridge_attach(struct drm_bridge *bridge)
+{
+	int ret, val;
+	struct ti_sn_bridge *pdata = bridge_to_ti_sn_bridge(bridge);
+	struct mipi_dsi_host *host;
+	struct mipi_dsi_device *dsi;
+	const struct mipi_dsi_device_info info = { .type = "ti_sn_bridge",
+						   .channel = 0,
+						   .node = NULL,
+						 };
+
+	ret = drm_connector_init(bridge->dev, &pdata->connector,
+				 &ti_sn_bridge_connector_funcs,
+				 DRM_MODE_CONNECTOR_eDP);
+	if (ret) {
+		DRM_ERROR("Failed to initialize connector with drm\n");
+		return ret;
+	}
+
+	drm_connector_helper_add(&pdata->connector,
+				 &ti_sn_bridge_connector_helper_funcs);
+	drm_mode_connector_attach_encoder(&pdata->connector, bridge->encoder);
+
+	host = of_find_mipi_dsi_host_by_node(pdata->host_node);
+	if (!host) {
+		DRM_ERROR("failed to find dsi host\n");
+		ret = -ENODEV;
+		goto err_dsi_host;
+	}
+
+	dsi = mipi_dsi_device_register_full(host, &info);
+	if (IS_ERR(dsi)) {
+		DRM_ERROR("failed to create dsi device\n");
+		ret = PTR_ERR(dsi);
+		goto err_dsi_host;
+	}
+
+	/* TODO: setting to 4 lanes always for now */
+	dsi->lanes = 4;
+	dsi->format = MIPI_DSI_FMT_RGB888;
+	dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_SYNC_PULSE |
+			  MIPI_DSI_MODE_EOT_PACKET | MIPI_DSI_MODE_VIDEO_HSE;
+
+	/* check if continuous dsi clock is required or not */
+	pm_runtime_get_sync(pdata->dev);
+	regmap_read(pdata->regmap, SN_DPPLL_SRC_REG, &val);
+	pm_runtime_put_sync(pdata->dev);
+	if (!(val & DPPLL_CLK_SRC_DSICLK))
+		dsi->mode_flags |= MIPI_DSI_CLOCK_NON_CONTINUOUS;
+
+	ret = mipi_dsi_attach(dsi);
+	if (ret < 0) {
+		DRM_ERROR("failed to attach dsi to host\n");
+		goto err_dsi_attach;
+	}
+	pdata->dsi = dsi;
+
+	/* attach panel to bridge */
+	if (pdata->panel)
+		drm_panel_attach(pdata->panel, &pdata->connector);
+
+	return 0;
+
+err_dsi_attach:
+	mipi_dsi_device_unregister(dsi);
+err_dsi_host:
+	drm_connector_cleanup(&pdata->connector);
+	return ret;
+}
+
+static void ti_sn_bridge_disable(struct drm_bridge *bridge)
+{
+	struct ti_sn_bridge *pdata = bridge_to_ti_sn_bridge(bridge);
+
+	if (pdata->panel) {
+		drm_panel_disable(pdata->panel);
+		drm_panel_unprepare(pdata->panel);
+	}
+
+	/* disable video stream */
+	regmap_update_bits(pdata->regmap, SN_ENH_FRAME_REG,
+			   SN_ENABLE_VID_STREAM_BIT, 0);
+	/* semi auto link training mode OFF */
+	regmap_write(pdata->regmap, SN_ML_TX_MODE_REG, 0);
+	/* disable DP PLL */
+	regmap_write(pdata->regmap, SN_PLL_ENABLE_REG, 0);
+}
+
+static u32 ti_sn_bridge_get_dsi_freq(struct ti_sn_bridge *pdata)
+{
+	u32 bit_rate_khz, clk_freq_khz;
+	struct drm_display_mode *mode =
+		&pdata->bridge.encoder->crtc->state->adjusted_mode;
+
+	bit_rate_khz = mode->clock *
+			mipi_dsi_pixel_format_to_bpp(pdata->dsi->format);
+	clk_freq_khz = bit_rate_khz / (pdata->dsi->lanes * 2);
+
+	return clk_freq_khz;
+}
+
+/* clk frequencies supported by bridge in Hz in case derived from REFCLK pin */
+static const u32 ti_sn_bridge_refclk_lut[] = {
+	12000000,
+	19200000,
+	26000000,
+	27000000,
+	38400000,
+};
+
+/* clk frequencies supported by bridge in Hz in case derived from DACP/N pin */
+static const u32 ti_sn_bridge_dsiclk_lut[] = {
+	468000000,
+	384000000,
+	416000000,
+	486000000,
+	460800000,
+};
+
+static void ti_sn_bridge_set_refclk_freq(struct ti_sn_bridge *pdata)
+{
+	int i;
+	u32 refclk_rate;
+	const u32 *refclk_lut;
+	size_t refclk_lut_size;
+
+	if (pdata->refclk) {
+		refclk_rate = clk_get_rate(pdata->refclk);
+		refclk_lut = ti_sn_bridge_refclk_lut;
+		refclk_lut_size = ARRAY_SIZE(ti_sn_bridge_refclk_lut);
+		clk_prepare_enable(pdata->refclk);
+	} else {
+		refclk_rate = ti_sn_bridge_get_dsi_freq(pdata) * 1000;
+		refclk_lut = ti_sn_bridge_dsiclk_lut;
+		refclk_lut_size = ARRAY_SIZE(ti_sn_bridge_dsiclk_lut);
+	}
+
+	/* for i equals to refclk_lut_size means default frequency */
+	for (i = 0; i < refclk_lut_size; i++)
+		if (refclk_lut[i] == refclk_rate)
+			break;
+
+	regmap_update_bits(pdata->regmap, SN_DPPLL_SRC_REG,
+			   SN_REFCLK_FREQ_BITS, i << SN_REFCLK_FREQ_OFFSET);
+}
+
+/**
+ * LUT index corresponds to register value and
+ * LUT values corresponds to dp data rate supported
+ * by the bridge in Mbps unit.
+ */
+static const unsigned int ti_sn_bridge_dp_rate_lut[] = {
+	0, 1620, 2160, 2430, 2700, 3240, 4320, 5400
+};
+
+static void ti_sn_bridge_set_dsi_dp_rate(struct ti_sn_bridge *pdata)
+{
+	unsigned int bit_rate_mhz, clk_freq_mhz, dp_rate_mhz;
+	unsigned int val, i;
+	struct drm_display_mode *mode =
+		&pdata->bridge.encoder->crtc->state->adjusted_mode;
+
+	/* set DSIA clk frequency */
+	bit_rate_mhz = (mode->clock / 1000) *
+			mipi_dsi_pixel_format_to_bpp(pdata->dsi->format);
+	clk_freq_mhz = bit_rate_mhz / (pdata->dsi->lanes * 2);
+
+	/* for each increment in val, frequency increases by 5MHz */
+	val = (MIN_DSI_CLK_FREQ_MHZ / 5) +
+		(((clk_freq_mhz - MIN_DSI_CLK_FREQ_MHZ) / 5) & 0xFF);
+	regmap_write(pdata->regmap, SN_DSIA_CLK_FREQ_REG, val);
+
+	/* set DP data rate */
+	dp_rate_mhz = ((bit_rate_mhz / pdata->dsi->lanes) * DP_CLK_FUDGE_NUM) /
+							DP_CLK_FUDGE_DEN;
+	for (i = 0; i < ARRAY_SIZE(ti_sn_bridge_dp_rate_lut) - 1; i++)
+		if (ti_sn_bridge_dp_rate_lut[i] > dp_rate_mhz)
+			break;
+
+	regmap_update_bits(pdata->regmap, SN_DATARATE_CONFIG_REG,
+			   SN_DP_DATA_RATE_BITS, i << SN_DP_DATA_RATE_OFFSET);
+}
+
+static void ti_sn_bridge_set_video_timings(struct ti_sn_bridge *pdata)
+{
+	struct drm_display_mode *mode =
+		&pdata->bridge.encoder->crtc->state->adjusted_mode;
+	u8 hsync_polarity = 0, vsync_polarity = 0;
+
+	if (mode->flags & DRM_MODE_FLAG_PHSYNC)
+		hsync_polarity = 0x1;
+	if (mode->flags & DRM_MODE_FLAG_PVSYNC)
+		vsync_polarity = 0x1;
+
+	ti_sn_bridge_write_u16(pdata, SN_CHA_ACTIVE_LINE_LENGTH_LOW_REG,
+			       mode->hdisplay);
+	ti_sn_bridge_write_u16(pdata, SN_CHA_VERTICAL_DISPLAY_SIZE_LOW_REG,
+			       mode->vdisplay);
+	regmap_write(pdata->regmap, SN_CHA_HSYNC_PULSE_WIDTH_LOW_REG,
+		     (mode->hsync_end - mode->hsync_start) & 0xFF);
+	regmap_write(pdata->regmap, SN_CHA_HSYNC_PULSE_WIDTH_HIGH_REG,
+		     (((mode->hsync_end - mode->hsync_start) >> 8) & 0x7F)
+		     | (hsync_polarity << SN_SYNC_POLARITY_OFFSET));
+	regmap_write(pdata->regmap, SN_CHA_VSYNC_PULSE_WIDTH_LOW_REG,
+		     (mode->vsync_end - mode->vsync_start) & 0xFF);
+	regmap_write(pdata->regmap, SN_CHA_VSYNC_PULSE_WIDTH_HIGH_REG,
+		    (((mode->vsync_end - mode->vsync_start) >> 8) & 0x7F)
+		    | (vsync_polarity << SN_SYNC_POLARITY_OFFSET));
+
+	regmap_write(pdata->regmap, SN_CHA_HORIZONTAL_BACK_PORCH_REG,
+		     (mode->htotal - mode->hsync_end) & 0xFF);
+	regmap_write(pdata->regmap, SN_CHA_VERTICAL_BACK_PORCH_REG,
+		     (mode->vtotal - mode->vsync_end) & 0xFF);
+
+	regmap_write(pdata->regmap, SN_CHA_HORIZONTAL_FRONT_PORCH_REG,
+		     (mode->hsync_start - mode->hdisplay) & 0xFF);
+	regmap_write(pdata->regmap, SN_CHA_VERTICAL_FRONT_PORCH_REG,
+		     (mode->vsync_start - mode->vdisplay) & 0xFF);
+
+	usleep_range(10000, 10500); /* 10ms delay recommended by spec */
+}
+
+static void ti_sn_bridge_enable(struct drm_bridge *bridge)
+{
+	struct ti_sn_bridge *pdata = bridge_to_ti_sn_bridge(bridge);
+	unsigned int val;
+
+	if (pdata->panel) {
+		drm_panel_prepare(pdata->panel);
+		/* in case drm_panel is connected then HPD is not supported */
+		regmap_update_bits(pdata->regmap, SN_HPD_DISABLE_REG,
+				   SN_HPD_DISABLE_BIT, SN_HPD_DISABLE_BIT);
+	}
+
+	/* DSI_A lane config */
+	val = (4 - pdata->dsi->lanes) << SN_DSIA_LANE_OFFSET;
+	regmap_update_bits(pdata->regmap, SN_DSI_LANES_REG,
+			   SN_DSIA_NUM_LANES_BITS, val);
+
+	/* DP lane config */
+	val = (pdata->dsi->lanes - 1) << SN_DP_LANE_OFFSET;
+	regmap_update_bits(pdata->regmap, SN_SSC_CONFIG_REG,
+			   SN_DP_NUM_LANES_BITS, val);
+
+	/* set dsi/dp clk frequency value */
+	ti_sn_bridge_set_dsi_dp_rate(pdata);
+
+	/* enable DP PLL */
+	regmap_write(pdata->regmap, SN_PLL_ENABLE_REG, 1);
+	usleep_range(10000, 10500); /* 10ms delay recommended by spec */
+
+	/**
+	 * The SN65DSI86 only supports ASSR Display Authentication method and
+	 * this method is enabled by default. An eDP panel must support this
+	 * authentication method. We need to enable this method in the eDP panel
+	 * at DisplayPort address 0x0010A prior to link training.
+	 */
+	regmap_write(pdata->regmap, SN_AUX_WDATA0_REG, 0x01);
+	regmap_write(pdata->regmap, SN_AUX_ADDR_19_16_REG, 0x00);
+	regmap_write(pdata->regmap, SN_AUX_ADDR_15_8_REG, 0x01);
+	regmap_write(pdata->regmap, SN_AUX_ADDR_7_0_REG, 0x0A);
+	regmap_write(pdata->regmap, SN_AUX_LENGTH_REG, 0x01);
+	regmap_write(pdata->regmap, SN_AUX_CMD_REG, 0x81);
+	usleep_range(10000, 10500); /* 10ms delay recommended by spec */
+
+	/* Semi auto link training mode */
+	regmap_write(pdata->regmap, SN_ML_TX_MODE_REG, 0x0A);
+	msleep(20); /* 20ms delay recommended by spec */
+
+	/* config video parameters */
+	ti_sn_bridge_set_video_timings(pdata);
+
+	/* enable video stream */
+	regmap_update_bits(pdata->regmap, SN_ENH_FRAME_REG,
+			   SN_ENABLE_VID_STREAM_BIT, SN_ENABLE_VID_STREAM_BIT);
+
+	if (pdata->panel)
+		drm_panel_enable(pdata->panel);
+}
+
+static void ti_sn_bridge_pre_enable(struct drm_bridge *bridge)
+{
+	struct ti_sn_bridge *pdata = bridge_to_ti_sn_bridge(bridge);
+
+	pm_runtime_get_sync(pdata->dev);
+
+	/* configure bridge ref_clk */
+	ti_sn_bridge_set_refclk_freq(pdata);
+}
+
+static void ti_sn_bridge_post_disable(struct drm_bridge *bridge)
+{
+	struct ti_sn_bridge *pdata = bridge_to_ti_sn_bridge(bridge);
+
+	if (pdata->refclk)
+		clk_disable_unprepare(pdata->refclk);
+
+	pm_runtime_put_sync(pdata->dev);
+}
+
+static const struct drm_bridge_funcs ti_sn_bridge_funcs = {
+	.attach = ti_sn_bridge_attach,
+	.pre_enable = ti_sn_bridge_pre_enable,
+	.enable = ti_sn_bridge_enable,
+	.disable = ti_sn_bridge_disable,
+	.post_disable = ti_sn_bridge_post_disable,
+};
+
+static int ti_sn_bridge_parse_dsi_host(struct ti_sn_bridge *pdata)
+{
+	struct device_node *np = pdata->dev->of_node;
+
+	pdata->host_node = of_graph_get_remote_node(np, 0, 0);
+
+	if (!pdata->host_node) {
+		DRM_ERROR("remote dsi host node not found\n");
+		return -ENODEV;
+	}
+
+	return 0;
+}
+
+static int ti_sn_bridge_probe(struct i2c_client *client,
+	 const struct i2c_device_id *id)
+{
+	struct ti_sn_bridge *pdata;
+	struct device_node *ddc_node;
+	int ret;
+
+	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
+		DRM_ERROR("device doesn't support I2C\n");
+		return -ENODEV;
+	}
+
+	pdata = devm_kzalloc(&client->dev, sizeof(struct ti_sn_bridge),
+			     GFP_KERNEL);
+	if (!pdata)
+		return -ENOMEM;
+
+	pdata->regmap = devm_regmap_init_i2c(client,
+					     &ti_sn_bridge_regmap_config);
+	if (IS_ERR(pdata->regmap)) {
+		DRM_ERROR("regmap i2c init failed\n");
+		return PTR_ERR(pdata->regmap);
+	}
+
+	pdata->dev = &client->dev;
+	dev_set_drvdata(&client->dev, pdata);
+
+	pdata->enable_gpio = devm_gpiod_get(pdata->dev, "enable",
+					    GPIOD_OUT_LOW);
+	if (IS_ERR(pdata->enable_gpio)) {
+		DRM_ERROR("failed to get enable gpio from DT\n");
+		ret = PTR_ERR(pdata->enable_gpio);
+		return ret;
+	}
+
+	ret = ti_sn_bridge_parse_regulators(pdata);
+	if (ret) {
+		DRM_ERROR("failed to parse regulators\n");
+		return ret;
+	}
+
+	ret = drm_of_find_panel_or_bridge(pdata->dev->of_node, 1, 0,
+					  &pdata->panel, NULL);
+	if (ret) {
+		DRM_ERROR("could not find any drm panel node\n");
+		return ret;
+	}
+
+	pdata->refclk = devm_clk_get(pdata->dev, "refclk");
+	if (IS_ERR(pdata->refclk)) {
+		ret = PTR_ERR(pdata->refclk);
+		if (ret == -EPROBE_DEFER)
+			return ret;
+		DRM_DEBUG_KMS("refclk not found\n");
+		pdata->refclk = NULL;
+	}
+
+	ddc_node = of_parse_phandle(pdata->dev->of_node, "ddc-i2c-bus", 0);
+	if (ddc_node) {
+		pdata->ddc = of_find_i2c_adapter_by_node(ddc_node);
+		of_node_put(ddc_node);
+		if (!pdata->ddc) {
+			DRM_DEBUG_KMS("failed to read ddc node\n");
+			return -EPROBE_DEFER;
+		}
+	} else {
+		DRM_DEBUG_KMS("no ddc property found\n");
+	}
+
+	ret = ti_sn_bridge_parse_dsi_host(pdata);
+	if (ret)
+		return ret;
+
+	pm_runtime_enable(pdata->dev);
+
+	i2c_set_clientdata(client, pdata);
+
+	pdata->bridge.funcs = &ti_sn_bridge_funcs;
+	pdata->bridge.of_node = client->dev.of_node;
+
+	drm_bridge_add(&pdata->bridge);
+
+	return 0;
+}
+
+static int ti_sn_bridge_remove(struct i2c_client *client)
+{
+	struct ti_sn_bridge *pdata = i2c_get_clientdata(client);
+
+	if (!pdata)
+		return -EINVAL;
+
+	if (pdata->host_node)
+		of_node_put(pdata->host_node);
+
+	pm_runtime_disable(pdata->dev);
+
+	if (pdata->dsi) {
+		mipi_dsi_detach(pdata->dsi);
+		mipi_dsi_device_unregister(pdata->dsi);
+	}
+
+	drm_bridge_remove(&pdata->bridge);
+	i2c_put_adapter(pdata->ddc);
+
+	return 0;
+}
+
+static struct i2c_device_id ti_sn_bridge_id[] = {
+	{ "ti,sn65dsi86", 0},
+	{},
+};
+MODULE_DEVICE_TABLE(i2c, ti_sn_bridge_id);
+
+static const struct of_device_id ti_sn_bridge_match_table[] = {
+	{.compatible = "ti,sn65dsi86"},
+	{},
+};
+MODULE_DEVICE_TABLE(of, ti_sn_bridge_match_table);
+
+static struct i2c_driver ti_sn_bridge_driver = {
+	.driver = {
+		.name = "ti_sn65dsi86",
+		.of_match_table = ti_sn_bridge_match_table,
+		.pm = &ti_sn_bridge_pm_ops,
+	},
+	.probe = ti_sn_bridge_probe,
+	.remove = ti_sn_bridge_remove,
+	.id_table = ti_sn_bridge_id,
+};
+
+module_i2c_driver(ti_sn_bridge_driver);
+MODULE_DESCRIPTION("sn65dsi86 DSI to eDP bridge driver");
+MODULE_LICENSE("GPL v2");
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH v11 2/5] drm/bridge: add support for sn65dsi86 bridge driver

[Andrzej Hajda]

Hi Sandeep,

Thanks for the patch, I hope it will be merged soon.

On 15.06.2018 08:43, 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).
>
> Changes in v4:
>  - Update License/Copyright (Sean Paul).
>  - Add Kconfig and Makefile changes (Sean Paul).
>  - Drop i2c gpio handling from this bridge driver, since i2c sda/scl gpios
>    will be handled by i2c master.
>  - Update required supplies names.
>  - Remove unnecessary goto statements (Sean Paul).
>  - Add mutex lock to power_ctrl API to avoid race conditions (Sean
>    Paul).
>  - Add support to parse reference clk frequency from dt(optional).
>  - Update the bridge chip enable/disable sequence.
>
> Changes in v5:
>  - Fixed Kbuild test service reported warnings.
>
> Changes in v6:
>  - Use PM runtime based ref-counting instead of local ref_count mechanism
>    (Stephen Boyd).
>  - Clean up some debug logs and indentations (Sean Paul).
>  - Simplify dp rate calculation (Sean Paul).
>  - Add support to configure refclk based on input REFCLK pin or DACP/N
>    pin (Stephen Boyd).
>
> Changes in v7:
>  - Use static supply entries instead of dynamic allocation (Andrzej
>    Hajda).
>  - Defer bridge driver probe if panel is not probed (Andrzej Hajda).
>  - Update of_graph APIs for correct node reference management. (Andrzej
>    Hajda).
>  - Remove local display_mode object (Andrzej Hajda).
>  - Remove version id check function from driver.
>
> Changes in v8:
>  - Move dsi register/attach function to bridge driver probe (Andrzej
>    Hajda).
>  - Introduce a new helper function to write 16bit words into consecutive
>    registers (Andrzej Hajda).
>  - Remove unnecessary macros (Andrzej Hajda).
>
> Changes in v9:
>  - Remove dsi register/attach from bridge probe, since dsi dev register
>    completion also waits for any panel or bridge to get added. This creates
>    deadlock situation when bridge driver calls dsi dev register and
>    attach before bridge add, in its probe function.
>  - Fix issues faced during testing of bridge driver on actual HW.
>  - Remove unnecessary initializations (Stephen Boyd).
>  - Use local refclk lut size instead of global macro (Sean Paul).
>
> Changes in v10:
>  - Use refclk to determine if continuous dsi clock is needed or not.
>
> Changes in v11:
>  - Read DPPLL_SRC register to determine continuous clock instead of
>    using refclk handle (Stephen Boyd).
>
> Signed-off-by: Sandeep Panda <spanda@codeaurora.org>
> ---
>  drivers/gpu/drm/bridge/Kconfig        |   9 +
>  drivers/gpu/drm/bridge/Makefile       |   1 +
>  drivers/gpu/drm/bridge/ti-sn65dsi86.c | 696 ++++++++++++++++++++++++++++++++++
>  3 files changed, 706 insertions(+)
>  create mode 100644 drivers/gpu/drm/bridge/ti-sn65dsi86.c
>
> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
> index 3b99d5a..8153150 100644
> --- a/drivers/gpu/drm/bridge/Kconfig
> +++ b/drivers/gpu/drm/bridge/Kconfig
> @@ -108,6 +108,15 @@ config DRM_TI_TFP410
>  	---help---
>  	  Texas Instruments TFP410 DVI/HDMI Transmitter driver
>  
> +config DRM_TI_SN65DSI86
> +	tristate "TI SN65DSI86 DSI to eDP bridge"
> +	depends on OF
> +	select DRM_KMS_HELPER
> +	select REGMAP_I2C
> +	select DRM_PANEL
> +	---help---
> +	  Texas Instruments SN65DSI86 DSI to eDP Bridge driver
> +
>  source "drivers/gpu/drm/bridge/analogix/Kconfig"
>  
>  source "drivers/gpu/drm/bridge/adv7511/Kconfig"
> diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
> index 373eb28..a7ca000 100644
> --- a/drivers/gpu/drm/bridge/Makefile
> +++ b/drivers/gpu/drm/bridge/Makefile
> @@ -11,5 +11,6 @@ obj-$(CONFIG_DRM_SII9234) += sii9234.o
>  obj-$(CONFIG_DRM_TOSHIBA_TC358767) += tc358767.o
>  obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix/
>  obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511/
> +obj-$(CONFIG_DRM_TI_SN65DSI86) += ti-sn65dsi86.o
>  obj-$(CONFIG_DRM_TI_TFP410) += ti-tfp410.o
>  obj-y += synopsys/
> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> new file mode 100644
> index 0000000..5473456
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> @@ -0,0 +1,696 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2018, The Linux Foundation. All rights reserved.
> + */
> +
> +#include <drm/drmP.h>
> +#include <drm/drm_atomic.h>
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_crtc_helper.h>
> +#include <drm/drm_mipi_dsi.h>
> +#include <drm/drm_of.h>
> +#include <drm/drm_panel.h>
> +#include <linux/clk.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/i2c.h>
> +#include <linux/of_graph.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> +
> +/* Link Training specific registers */
> +#define SN_DEVICE_REV_REG			0x08
> +#define SN_HPD_DISABLE_REG			0x5C
> +#define SN_DPPLL_SRC_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_REG			0x76
> +#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_VERTICAL_DISPLAY_SIZE_LOW_REG	0x24
> +#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_REG		0x3A
> +#define SN_DATA_FORMAT_REG			0x5B
> +
> +#define MIN_DSI_CLK_FREQ_MHZ	40
> +
> +/* fudge factor required to account for 8b/10b encoding */
> +#define DP_CLK_FUDGE_NUM	10
> +#define DP_CLK_FUDGE_DEN	8
> +
> +#define DPPLL_CLK_SRC_REFCLK	0
> +#define DPPLL_CLK_SRC_DSICLK	1
> +
> +#define SN_REFCLK_FREQ_OFFSET	1
> +#define SN_DSIA_LANE_OFFSET	3
> +#define SN_DP_LANE_OFFSET	4
> +#define SN_DP_DATA_RATE_OFFSET	5
> +#define SN_SYNC_POLARITY_OFFSET	7
> +
> +#define SN_ENABLE_VID_STREAM_BIT	BIT(3)
> +#define SN_REFCLK_FREQ_BITS		(BIT(3) | BIT(2) | BIT(1))
> +#define SN_DSIA_NUM_LANES_BITS		(BIT(4) | BIT(3))
> +#define SN_DP_NUM_LANES_BITS		(BIT(5) | BIT(4))
> +#define SN_DP_DATA_RATE_BITS		(BIT(7) | BIT(6) | BIT(5))

For contiguous bitmasks you could use GENMASK(h, l), but it is up to you.

> +#define SN_HPD_DISABLE_BIT		BIT(0)
> +
> +#define SN_REGULATOR_SUPPLY_NUM		4
> +
> +struct ti_sn_bridge {
> +	struct device			*dev;
> +	struct regmap			*regmap;
> +	struct drm_bridge		bridge;
> +	struct drm_connector		connector;
> +	struct device_node		*host_node;
> +	struct mipi_dsi_device		*dsi;
> +	struct clk			*refclk;
> +	struct drm_panel		*panel;
> +	struct gpio_desc		*enable_gpio;
> +	struct regulator_bulk_data	supplies[SN_REGULATOR_SUPPLY_NUM];
> +	struct i2c_adapter		*ddc;
> +};
> +
> +static const struct regmap_range ti_sn_bridge_volatile_ranges[] = {
> +	{ .range_min = 0, .range_max = 0xFF },
> +};
> +
> +static const struct regmap_access_table ti_sn_bridge_volatile_table = {
> +	.yes_ranges = ti_sn_bridge_volatile_ranges,
> +	.n_yes_ranges = ARRAY_SIZE(ti_sn_bridge_volatile_ranges),
> +};
> +
> +static const struct regmap_config ti_sn_bridge_regmap_config = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +	.volatile_table = &ti_sn_bridge_volatile_table,
> +	.cache_type = REGCACHE_NONE,
> +};
> +
> +static void ti_sn_bridge_write_u16(struct ti_sn_bridge *pdata,
> +				   unsigned int reg, u16 val)
> +{
> +	regmap_write(pdata->regmap, reg, val & 0xFF);
> +	regmap_write(pdata->regmap, reg + 1, val >> 8);
> +}
> +
> +static int __maybe_unused ti_sn_bridge_resume(struct device *dev)
> +{
> +	struct ti_sn_bridge *pdata = dev_get_drvdata(dev);
> +	int ret;
> +
> +	ret = regulator_bulk_enable(SN_REGULATOR_SUPPLY_NUM, pdata->supplies);
> +	if (ret) {
> +		DRM_ERROR("failed to enable supplies %d\n", ret);
> +		return ret;
> +	}
> +
> +	gpiod_set_value(pdata->enable_gpio, 1);
> +
> +	return ret;
> +}
> +
> +static int __maybe_unused ti_sn_bridge_suspend(struct device *dev)
> +{
> +	struct ti_sn_bridge *pdata = dev_get_drvdata(dev);
> +	int ret;
> +
> +	gpiod_set_value(pdata->enable_gpio, 0);
> +
> +	ret = regulator_bulk_disable(SN_REGULATOR_SUPPLY_NUM, pdata->supplies);
> +	if (ret)
> +		DRM_ERROR("failed to disable supplies %d\n", ret);
> +
> +	return ret;
> +}
> +
> +static const struct dev_pm_ops ti_sn_bridge_pm_ops = {
> +	SET_RUNTIME_PM_OPS(ti_sn_bridge_suspend, ti_sn_bridge_resume, NULL)
> +};
> +
> +/* Connector funcs */
> +static struct ti_sn_bridge *
> +connector_to_ti_sn_bridge(struct drm_connector *connector)
> +{
> +	return container_of(connector, struct ti_sn_bridge, connector);
> +}
> +
> +static int ti_sn_bridge_connector_get_modes(struct drm_connector *connector)
> +{
> +	struct ti_sn_bridge *pdata = connector_to_ti_sn_bridge(connector);
> +	struct edid *edid;
> +	u32 num_modes;
> +
> +	if (pdata->panel) {
> +		DRM_DEBUG_KMS("get mode from connected drm_panel\n");
> +		return drm_panel_get_modes(pdata->panel);
> +	}
> +
> +	if (!pdata->ddc)
> +		return 0;
I think it should not happen, ie probe should fail if both panel and ddc
are absent.
> +
> +	pm_runtime_get_sync(pdata->dev);
> +	edid = drm_get_edid(connector, pdata->ddc);
> +	pm_runtime_put_sync(pdata->dev);

Minor nit, you can use here pm_runtime_put, I guess you do not care when
it will be turned off.

> +	if (!edid)
> +		return 0;
> +
> +	drm_mode_connector_update_edid_property(connector, edid);
> +	num_modes = drm_add_edid_modes(connector, edid);
> +	kfree(edid);
> +
> +	return num_modes;
> +}
> +
> +static enum drm_mode_status
> +ti_sn_bridge_connector_mode_valid(struct drm_connector *connector,
> +			     struct drm_display_mode *mode)
> +{
> +	/* maximum supported resolution is 4K at 60 fps */
> +	if (mode->clock > 594000)
> +		return MODE_CLOCK_HIGH;
> +
> +	return MODE_OK;
> +}
> +
> +static struct drm_connector_helper_funcs ti_sn_bridge_connector_helper_funcs = {
> +	.get_modes = ti_sn_bridge_connector_get_modes,
> +	.mode_valid = ti_sn_bridge_connector_mode_valid,
> +};
> +
> +static enum drm_connector_status
> +ti_sn_bridge_connector_detect(struct drm_connector *connector, bool force)
> +{
> +	struct ti_sn_bridge *pdata = connector_to_ti_sn_bridge(connector);
> +
> +	/**
> +	 * TODO: Currently if drm_panel is present, then always
> +	 * return the status as connected. Need to add support to detect
> +	 * device state for no panel(hot pluggable) scenarios.
> +	 */
> +	if (pdata->panel)
> +		return connector_status_connected;
> +	else
> +		return connector_status_unknown;
> +}
> +
> +static const struct drm_connector_funcs ti_sn_bridge_connector_funcs = {
> +	.fill_modes = drm_helper_probe_single_connector_modes,
> +	.detect = ti_sn_bridge_connector_detect,
> +	.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 struct ti_sn_bridge *bridge_to_ti_sn_bridge(struct drm_bridge *bridge)
> +{
> +	return container_of(bridge, struct ti_sn_bridge, bridge);
> +}
> +
> +static int ti_sn_bridge_parse_regulators(struct ti_sn_bridge *pdata)
> +{
> +	unsigned int i;
> +	const char * const ti_sn_bridge_supply_names[] = {
> +		"vcca", "vcc", "vccio", "vpll",
> +	};
> +
> +	for (i = 0; i < SN_REGULATOR_SUPPLY_NUM; i++)
> +		pdata->supplies[i].supply = ti_sn_bridge_supply_names[i];
> +
> +	return devm_regulator_bulk_get(pdata->dev, SN_REGULATOR_SUPPLY_NUM,
> +				       pdata->supplies);
> +}
> +
> +static int ti_sn_bridge_attach(struct drm_bridge *bridge)
> +{
> +	int ret, val;
> +	struct ti_sn_bridge *pdata = bridge_to_ti_sn_bridge(bridge);
> +	struct mipi_dsi_host *host;
> +	struct mipi_dsi_device *dsi;
> +	const struct mipi_dsi_device_info info = { .type = "ti_sn_bridge",
> +						   .channel = 0,
> +						   .node = NULL,
> +						 };
> +
> +	ret = drm_connector_init(bridge->dev, &pdata->connector,
> +				 &ti_sn_bridge_connector_funcs,
> +				 DRM_MODE_CONNECTOR_eDP);
> +	if (ret) {
> +		DRM_ERROR("Failed to initialize connector with drm\n");
> +		return ret;
> +	}
> +
> +	drm_connector_helper_add(&pdata->connector,
> +				 &ti_sn_bridge_connector_helper_funcs);
> +	drm_mode_connector_attach_encoder(&pdata->connector, bridge->encoder);
> +
> +	host = of_find_mipi_dsi_host_by_node(pdata->host_node);
> +	if (!host) {
> +		DRM_ERROR("failed to find dsi host\n");
> +		ret = -ENODEV;
> +		goto err_dsi_host;
> +	}

In case of late host probing it will fail, instead of probe deferring.
If I remember I have asked you to move this code to probe, as deferring
is only possible there.

> +
> +	dsi = mipi_dsi_device_register_full(host, &info);
> +	if (IS_ERR(dsi)) {
> +		DRM_ERROR("failed to create dsi device\n");
> +		ret = PTR_ERR(dsi);
> +		goto err_dsi_host;
> +	}
> +
> +	/* TODO: setting to 4 lanes always for now */
> +	dsi->lanes = 4;
> +	dsi->format = MIPI_DSI_FMT_RGB888;
> +	dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_SYNC_PULSE |
> +			  MIPI_DSI_MODE_EOT_PACKET | MIPI_DSI_MODE_VIDEO_HSE;
> +
> +	/* check if continuous dsi clock is required or not */
> +	pm_runtime_get_sync(pdata->dev);
> +	regmap_read(pdata->regmap, SN_DPPLL_SRC_REG, &val);
> +	pm_runtime_put_sync(pdata->dev);
> +	if (!(val & DPPLL_CLK_SRC_DSICLK))
> +		dsi->mode_flags |= MIPI_DSI_CLOCK_NON_CONTINUOUS;
> +
> +	ret = mipi_dsi_attach(dsi);
> +	if (ret < 0) {
> +		DRM_ERROR("failed to attach dsi to host\n");
> +		goto err_dsi_attach;
> +	}
> +	pdata->dsi = dsi;
> +
> +	/* attach panel to bridge */
> +	if (pdata->panel)
> +		drm_panel_attach(pdata->panel, &pdata->connector);
> +
> +	return 0;
> +
> +err_dsi_attach:
> +	mipi_dsi_device_unregister(dsi);
> +err_dsi_host:
> +	drm_connector_cleanup(&pdata->connector);
> +	return ret;
> +}
> +
> +static void ti_sn_bridge_disable(struct drm_bridge *bridge)
> +{
> +	struct ti_sn_bridge *pdata = bridge_to_ti_sn_bridge(bridge);
> +
> +	if (pdata->panel) {
> +		drm_panel_disable(pdata->panel);
> +		drm_panel_unprepare(pdata->panel);
> +	}
> +
> +	/* disable video stream */
> +	regmap_update_bits(pdata->regmap, SN_ENH_FRAME_REG,
> +			   SN_ENABLE_VID_STREAM_BIT, 0);
> +	/* semi auto link training mode OFF */
> +	regmap_write(pdata->regmap, SN_ML_TX_MODE_REG, 0);
> +	/* disable DP PLL */
> +	regmap_write(pdata->regmap, SN_PLL_ENABLE_REG, 0);

Maybe in your particular case it does not matter but generally
drm_panel_disable should be called 1st, but drm_panel_unprepare should
be called somewhere after disabling video stream.

> +}
> +
> +static u32 ti_sn_bridge_get_dsi_freq(struct ti_sn_bridge *pdata)
> +{
> +	u32 bit_rate_khz, clk_freq_khz;
> +	struct drm_display_mode *mode =
> +		&pdata->bridge.encoder->crtc->state->adjusted_mode;
> +
> +	bit_rate_khz = mode->clock *
> +			mipi_dsi_pixel_format_to_bpp(pdata->dsi->format);
> +	clk_freq_khz = bit_rate_khz / (pdata->dsi->lanes * 2);
> +
> +	return clk_freq_khz;
> +}
> +
> +/* clk frequencies supported by bridge in Hz in case derived from REFCLK pin */
> +static const u32 ti_sn_bridge_refclk_lut[] = {
> +	12000000,
> +	19200000,
> +	26000000,
> +	27000000,
> +	38400000,
> +};
> +
> +/* clk frequencies supported by bridge in Hz in case derived from DACP/N pin */
> +static const u32 ti_sn_bridge_dsiclk_lut[] = {
> +	468000000,
> +	384000000,
> +	416000000,
> +	486000000,
> +	460800000,
> +};
> +
> +static void ti_sn_bridge_set_refclk_freq(struct ti_sn_bridge *pdata)
> +{
> +	int i;
> +	u32 refclk_rate;
> +	const u32 *refclk_lut;
> +	size_t refclk_lut_size;
> +
> +	if (pdata->refclk) {
> +		refclk_rate = clk_get_rate(pdata->refclk);
> +		refclk_lut = ti_sn_bridge_refclk_lut;
> +		refclk_lut_size = ARRAY_SIZE(ti_sn_bridge_refclk_lut);
> +		clk_prepare_enable(pdata->refclk);
> +	} else {
> +		refclk_rate = ti_sn_bridge_get_dsi_freq(pdata) * 1000;
> +		refclk_lut = ti_sn_bridge_dsiclk_lut;
> +		refclk_lut_size = ARRAY_SIZE(ti_sn_bridge_dsiclk_lut);
> +	}
> +
> +	/* for i equals to refclk_lut_size means default frequency */
> +	for (i = 0; i < refclk_lut_size; i++)
> +		if (refclk_lut[i] == refclk_rate)
> +			break;
> +
> +	regmap_update_bits(pdata->regmap, SN_DPPLL_SRC_REG,
> +			   SN_REFCLK_FREQ_BITS, i << SN_REFCLK_FREQ_OFFSET);
> +}
> +
> +/**
> + * LUT index corresponds to register value and
> + * LUT values corresponds to dp data rate supported
> + * by the bridge in Mbps unit.
> + */
> +static const unsigned int ti_sn_bridge_dp_rate_lut[] = {
> +	0, 1620, 2160, 2430, 2700, 3240, 4320, 5400
> +};
> +
> +static void ti_sn_bridge_set_dsi_dp_rate(struct ti_sn_bridge *pdata)
> +{
> +	unsigned int bit_rate_mhz, clk_freq_mhz, dp_rate_mhz;
> +	unsigned int val, i;
> +	struct drm_display_mode *mode =
> +		&pdata->bridge.encoder->crtc->state->adjusted_mode;
> +
> +	/* set DSIA clk frequency */
> +	bit_rate_mhz = (mode->clock / 1000) *
> +			mipi_dsi_pixel_format_to_bpp(pdata->dsi->format);
> +	clk_freq_mhz = bit_rate_mhz / (pdata->dsi->lanes * 2);
> +
> +	/* for each increment in val, frequency increases by 5MHz */
> +	val = (MIN_DSI_CLK_FREQ_MHZ / 5) +
> +		(((clk_freq_mhz - MIN_DSI_CLK_FREQ_MHZ) / 5) & 0xFF);
> +	regmap_write(pdata->regmap, SN_DSIA_CLK_FREQ_REG, val);
> +
> +	/* set DP data rate */
> +	dp_rate_mhz = ((bit_rate_mhz / pdata->dsi->lanes) * DP_CLK_FUDGE_NUM) /
> +							DP_CLK_FUDGE_DEN;
> +	for (i = 0; i < ARRAY_SIZE(ti_sn_bridge_dp_rate_lut) - 1; i++)
> +		if (ti_sn_bridge_dp_rate_lut[i] > dp_rate_mhz)
> +			break;
> +
> +	regmap_update_bits(pdata->regmap, SN_DATARATE_CONFIG_REG,
> +			   SN_DP_DATA_RATE_BITS, i << SN_DP_DATA_RATE_OFFSET);
> +}
> +
> +static void ti_sn_bridge_set_video_timings(struct ti_sn_bridge *pdata)
> +{
> +	struct drm_display_mode *mode =
> +		&pdata->bridge.encoder->crtc->state->adjusted_mode;
> +	u8 hsync_polarity = 0, vsync_polarity = 0;
> +
> +	if (mode->flags & DRM_MODE_FLAG_PHSYNC)
> +		hsync_polarity = 0x1;

You can assign:

hsync_polarity = BIT(SN_SYNC_POLARITY_OFFSET);

and then use it directly in regmap_write.

> +	if (mode->flags & DRM_MODE_FLAG_PVSYNC)
> +		vsync_polarity = 0x1;

The same here.

> +
> +	ti_sn_bridge_write_u16(pdata, SN_CHA_ACTIVE_LINE_LENGTH_LOW_REG,
> +			       mode->hdisplay);
> +	ti_sn_bridge_write_u16(pdata, SN_CHA_VERTICAL_DISPLAY_SIZE_LOW_REG,
> +			       mode->vdisplay);
> +	regmap_write(pdata->regmap, SN_CHA_HSYNC_PULSE_WIDTH_LOW_REG,
> +		     (mode->hsync_end - mode->hsync_start) & 0xFF);
> +	regmap_write(pdata->regmap, SN_CHA_HSYNC_PULSE_WIDTH_HIGH_REG,
> +		     (((mode->hsync_end - mode->hsync_start) >> 8) & 0x7F)
> +		     | (hsync_polarity << SN_SYNC_POLARITY_OFFSET));
> +	regmap_write(pdata->regmap, SN_CHA_VSYNC_PULSE_WIDTH_LOW_REG,
> +		     (mode->vsync_end - mode->vsync_start) & 0xFF);
> +	regmap_write(pdata->regmap, SN_CHA_VSYNC_PULSE_WIDTH_HIGH_REG,
> +		    (((mode->vsync_end - mode->vsync_start) >> 8) & 0x7F)
> +		    | (vsync_polarity << SN_SYNC_POLARITY_OFFSET));
> +
> +	regmap_write(pdata->regmap, SN_CHA_HORIZONTAL_BACK_PORCH_REG,
> +		     (mode->htotal - mode->hsync_end) & 0xFF);
> +	regmap_write(pdata->regmap, SN_CHA_VERTICAL_BACK_PORCH_REG,
> +		     (mode->vtotal - mode->vsync_end) & 0xFF);
> +
> +	regmap_write(pdata->regmap, SN_CHA_HORIZONTAL_FRONT_PORCH_REG,
> +		     (mode->hsync_start - mode->hdisplay) & 0xFF);
> +	regmap_write(pdata->regmap, SN_CHA_VERTICAL_FRONT_PORCH_REG,
> +		     (mode->vsync_start - mode->vdisplay) & 0xFF);
> +
> +	usleep_range(10000, 10500); /* 10ms delay recommended by spec */
> +}
> +
> +static void ti_sn_bridge_enable(struct drm_bridge *bridge)
> +{
> +	struct ti_sn_bridge *pdata = bridge_to_ti_sn_bridge(bridge);
> +	unsigned int val;
> +
> +	if (pdata->panel) {
> +		drm_panel_prepare(pdata->panel);
> +		/* in case drm_panel is connected then HPD is not supported */
> +		regmap_update_bits(pdata->regmap, SN_HPD_DISABLE_REG,
> +				   SN_HPD_DISABLE_BIT, SN_HPD_DISABLE_BIT);

I think HPD should be disabled much earlier, I guess somewhere in
initialization/probe.

> +	}
> +
> +	/* DSI_A lane config */
> +	val = (4 - pdata->dsi->lanes) << SN_DSIA_LANE_OFFSET;
> +	regmap_update_bits(pdata->regmap, SN_DSI_LANES_REG,
> +			   SN_DSIA_NUM_LANES_BITS, val);
> +
> +	/* DP lane config */
> +	val = (pdata->dsi->lanes - 1) << SN_DP_LANE_OFFSET;
> +	regmap_update_bits(pdata->regmap, SN_SSC_CONFIG_REG,
> +			   SN_DP_NUM_LANES_BITS, val);
> +
> +	/* set dsi/dp clk frequency value */
> +	ti_sn_bridge_set_dsi_dp_rate(pdata);
> +
> +	/* enable DP PLL */
> +	regmap_write(pdata->regmap, SN_PLL_ENABLE_REG, 1);
> +	usleep_range(10000, 10500); /* 10ms delay recommended by spec */
> +
> +	/**
> +	 * The SN65DSI86 only supports ASSR Display Authentication method and
> +	 * this method is enabled by default. An eDP panel must support this
> +	 * authentication method. We need to enable this method in the eDP panel
> +	 * at DisplayPort address 0x0010A prior to link training.
> +	 */
> +	regmap_write(pdata->regmap, SN_AUX_WDATA0_REG, 0x01);
> +	regmap_write(pdata->regmap, SN_AUX_ADDR_19_16_REG, 0x00);
> +	regmap_write(pdata->regmap, SN_AUX_ADDR_15_8_REG, 0x01);
> +	regmap_write(pdata->regmap, SN_AUX_ADDR_7_0_REG, 0x0A);
> +	regmap_write(pdata->regmap, SN_AUX_LENGTH_REG, 0x01);
> +	regmap_write(pdata->regmap, SN_AUX_CMD_REG, 0x81);
> +	usleep_range(10000, 10500); /* 10ms delay recommended by spec */
> +
> +	/* Semi auto link training mode */
> +	regmap_write(pdata->regmap, SN_ML_TX_MODE_REG, 0x0A);
> +	msleep(20); /* 20ms delay recommended by spec */
> +
> +	/* config video parameters */
> +	ti_sn_bridge_set_video_timings(pdata);
> +
> +	/* enable video stream */
> +	regmap_update_bits(pdata->regmap, SN_ENH_FRAME_REG,
> +			   SN_ENABLE_VID_STREAM_BIT, SN_ENABLE_VID_STREAM_BIT);
> +
> +	if (pdata->panel)
> +		drm_panel_enable(pdata->panel);
> +}
> +
> +static void ti_sn_bridge_pre_enable(struct drm_bridge *bridge)
> +{
> +	struct ti_sn_bridge *pdata = bridge_to_ti_sn_bridge(bridge);
> +
> +	pm_runtime_get_sync(pdata->dev);
> +
> +	/* configure bridge ref_clk */
> +	ti_sn_bridge_set_refclk_freq(pdata);
> +}
> +
> +static void ti_sn_bridge_post_disable(struct drm_bridge *bridge)
> +{
> +	struct ti_sn_bridge *pdata = bridge_to_ti_sn_bridge(bridge);
> +
> +	if (pdata->refclk)
> +		clk_disable_unprepare(pdata->refclk);
> +
> +	pm_runtime_put_sync(pdata->dev);
> +}
> +
> +static const struct drm_bridge_funcs ti_sn_bridge_funcs = {
> +	.attach = ti_sn_bridge_attach,
> +	.pre_enable = ti_sn_bridge_pre_enable,
> +	.enable = ti_sn_bridge_enable,
> +	.disable = ti_sn_bridge_disable,
> +	.post_disable = ti_sn_bridge_post_disable,
> +};
> +
> +static int ti_sn_bridge_parse_dsi_host(struct ti_sn_bridge *pdata)
> +{
> +	struct device_node *np = pdata->dev->of_node;
> +
> +	pdata->host_node = of_graph_get_remote_node(np, 0, 0);
> +
> +	if (!pdata->host_node) {
> +		DRM_ERROR("remote dsi host node not found\n");
> +		return -ENODEV;
> +	}
> +
> +	return 0;
> +}
> +
> +static int ti_sn_bridge_probe(struct i2c_client *client,
> +	 const struct i2c_device_id *id)
> +{
> +	struct ti_sn_bridge *pdata;
> +	struct device_node *ddc_node;
> +	int ret;
> +
> +	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
> +		DRM_ERROR("device doesn't support I2C\n");
> +		return -ENODEV;
> +	}
> +
> +	pdata = devm_kzalloc(&client->dev, sizeof(struct ti_sn_bridge),
> +			     GFP_KERNEL);
> +	if (!pdata)
> +		return -ENOMEM;
> +
> +	pdata->regmap = devm_regmap_init_i2c(client,
> +					     &ti_sn_bridge_regmap_config);
> +	if (IS_ERR(pdata->regmap)) {
> +		DRM_ERROR("regmap i2c init failed\n");
> +		return PTR_ERR(pdata->regmap);
> +	}
> +
> +	pdata->dev = &client->dev;
> +	dev_set_drvdata(&client->dev, pdata);
> +
> +	pdata->enable_gpio = devm_gpiod_get(pdata->dev, "enable",
> +					    GPIOD_OUT_LOW);
> +	if (IS_ERR(pdata->enable_gpio)) {
> +		DRM_ERROR("failed to get enable gpio from DT\n");
> +		ret = PTR_ERR(pdata->enable_gpio);
> +		return ret;
> +	}
> +
> +	ret = ti_sn_bridge_parse_regulators(pdata);
> +	if (ret) {
> +		DRM_ERROR("failed to parse regulators\n");
> +		return ret;
> +	}
> +
> +	ret = drm_of_find_panel_or_bridge(pdata->dev->of_node, 1, 0,
> +					  &pdata->panel, NULL);
> +	if (ret) {
> +		DRM_ERROR("could not find any drm panel node\n");
> +		return ret;
> +	}
> +
> +	pdata->refclk = devm_clk_get(pdata->dev, "refclk");
> +	if (IS_ERR(pdata->refclk)) {
> +		ret = PTR_ERR(pdata->refclk);
> +		if (ret == -EPROBE_DEFER)
> +			return ret;
> +		DRM_DEBUG_KMS("refclk not found\n");
> +		pdata->refclk = NULL;
> +	}
> +
> +	ddc_node = of_parse_phandle(pdata->dev->of_node, "ddc-i2c-bus", 0);
> +	if (ddc_node) {
> +		pdata->ddc = of_find_i2c_adapter_by_node(ddc_node);
> +		of_node_put(ddc_node);
> +		if (!pdata->ddc) {
> +			DRM_DEBUG_KMS("failed to read ddc node\n");
> +			return -EPROBE_DEFER;
> +		}
> +	} else {
> +		DRM_DEBUG_KMS("no ddc property found\n");
> +	}

I guess there should be exclusively panel or ddc, please fail if
both/none are present.


Regards
Andrzej

> +
> +	ret = ti_sn_bridge_parse_dsi_host(pdata);
> +	if (ret)
> +		return ret;
> +
> +	pm_runtime_enable(pdata->dev);
> +
> +	i2c_set_clientdata(client, pdata);
> +
> +	pdata->bridge.funcs = &ti_sn_bridge_funcs;
> +	pdata->bridge.of_node = client->dev.of_node;
> +
> +	drm_bridge_add(&pdata->bridge);
> +
> +	return 0;
> +}
> +
> +static int ti_sn_bridge_remove(struct i2c_client *client)
> +{
> +	struct ti_sn_bridge *pdata = i2c_get_clientdata(client);
> +
> +	if (!pdata)
> +		return -EINVAL;
> +
> +	if (pdata->host_node)
> +		of_node_put(pdata->host_node);
> +
> +	pm_runtime_disable(pdata->dev);
> +
> +	if (pdata->dsi) {
> +		mipi_dsi_detach(pdata->dsi);
> +		mipi_dsi_device_unregister(pdata->dsi);
> +	}
> +
> +	drm_bridge_remove(&pdata->bridge);
> +	i2c_put_adapter(pdata->ddc);
> +
> +	return 0;
> +}
> +
> +static struct i2c_device_id ti_sn_bridge_id[] = {
> +	{ "ti,sn65dsi86", 0},
> +	{},
> +};
> +MODULE_DEVICE_TABLE(i2c, ti_sn_bridge_id);
> +
> +static const struct of_device_id ti_sn_bridge_match_table[] = {
> +	{.compatible = "ti,sn65dsi86"},
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, ti_sn_bridge_match_table);
> +
> +static struct i2c_driver ti_sn_bridge_driver = {
> +	.driver = {
> +		.name = "ti_sn65dsi86",
> +		.of_match_table = ti_sn_bridge_match_table,
> +		.pm = &ti_sn_bridge_pm_ops,
> +	},
> +	.probe = ti_sn_bridge_probe,
> +	.remove = ti_sn_bridge_remove,
> +	.id_table = ti_sn_bridge_id,
> +};
> +
> +module_i2c_driver(ti_sn_bridge_driver);
> +MODULE_DESCRIPTION("sn65dsi86 DSI to eDP bridge driver");
> +MODULE_LICENSE("GPL v2");


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

Re: [PATCH v11 2/5] drm/bridge: add support for sn65dsi86 bridge driver

[spanda-sgV2jX0FEOL9JmXXK+q4OQ]

On 2018-06-15 16:59, Andrzej Hajda wrote:
> Hi Sandeep,
> 
> Thanks for the patch, I hope it will be merged soon.
> 
> On 15.06.2018 08:43, 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).
>> 
>> Changes in v4:
>>  - Update License/Copyright (Sean Paul).
>>  - Add Kconfig and Makefile changes (Sean Paul).
>>  - Drop i2c gpio handling from this bridge driver, since i2c sda/scl 
>> gpios
>>    will be handled by i2c master.
>>  - Update required supplies names.
>>  - Remove unnecessary goto statements (Sean Paul).
>>  - Add mutex lock to power_ctrl API to avoid race conditions (Sean
>>    Paul).
>>  - Add support to parse reference clk frequency from dt(optional).
>>  - Update the bridge chip enable/disable sequence.
>> 
>> Changes in v5:
>>  - Fixed Kbuild test service reported warnings.
>> 
>> Changes in v6:
>>  - Use PM runtime based ref-counting instead of local ref_count 
>> mechanism
>>    (Stephen Boyd).
>>  - Clean up some debug logs and indentations (Sean Paul).
>>  - Simplify dp rate calculation (Sean Paul).
>>  - Add support to configure refclk based on input REFCLK pin or DACP/N
>>    pin (Stephen Boyd).
>> 
>> Changes in v7:
>>  - Use static supply entries instead of dynamic allocation (Andrzej
>>    Hajda).
>>  - Defer bridge driver probe if panel is not probed (Andrzej Hajda).
>>  - Update of_graph APIs for correct node reference management. 
>> (Andrzej
>>    Hajda).
>>  - Remove local display_mode object (Andrzej Hajda).
>>  - Remove version id check function from driver.
>> 
>> Changes in v8:
>>  - Move dsi register/attach function to bridge driver probe (Andrzej
>>    Hajda).
>>  - Introduce a new helper function to write 16bit words into 
>> consecutive
>>    registers (Andrzej Hajda).
>>  - Remove unnecessary macros (Andrzej Hajda).
>> 
>> Changes in v9:
>>  - Remove dsi register/attach from bridge probe, since dsi dev 
>> register
>>    completion also waits for any panel or bridge to get added. This 
>> creates
>>    deadlock situation when bridge driver calls dsi dev register and
>>    attach before bridge add, in its probe function.
>>  - Fix issues faced during testing of bridge driver on actual HW.
>>  - Remove unnecessary initializations (Stephen Boyd).
>>  - Use local refclk lut size instead of global macro (Sean Paul).
>> 
>> Changes in v10:
>>  - Use refclk to determine if continuous dsi clock is needed or not.
>> 
>> Changes in v11:
>>  - Read DPPLL_SRC register to determine continuous clock instead of
>>    using refclk handle (Stephen Boyd).
>> 
>> Signed-off-by: Sandeep Panda <spanda@codeaurora.org>
>> ---
>>  drivers/gpu/drm/bridge/Kconfig        |   9 +
>>  drivers/gpu/drm/bridge/Makefile       |   1 +
>>  drivers/gpu/drm/bridge/ti-sn65dsi86.c | 696 
>> ++++++++++++++++++++++++++++++++++
>>  3 files changed, 706 insertions(+)
>>  create mode 100644 drivers/gpu/drm/bridge/ti-sn65dsi86.c
>> 
>> diff --git a/drivers/gpu/drm/bridge/Kconfig 
>> b/drivers/gpu/drm/bridge/Kconfig
>> index 3b99d5a..8153150 100644
>> --- a/drivers/gpu/drm/bridge/Kconfig
>> +++ b/drivers/gpu/drm/bridge/Kconfig
>> @@ -108,6 +108,15 @@ config DRM_TI_TFP410
>>  	---help---
>>  	  Texas Instruments TFP410 DVI/HDMI Transmitter driver
>> 
>> +config DRM_TI_SN65DSI86
>> +	tristate "TI SN65DSI86 DSI to eDP bridge"
>> +	depends on OF
>> +	select DRM_KMS_HELPER
>> +	select REGMAP_I2C
>> +	select DRM_PANEL
>> +	---help---
>> +	  Texas Instruments SN65DSI86 DSI to eDP Bridge driver
>> +
>>  source "drivers/gpu/drm/bridge/analogix/Kconfig"
>> 
>>  source "drivers/gpu/drm/bridge/adv7511/Kconfig"
>> diff --git a/drivers/gpu/drm/bridge/Makefile 
>> b/drivers/gpu/drm/bridge/Makefile
>> index 373eb28..a7ca000 100644
>> --- a/drivers/gpu/drm/bridge/Makefile
>> +++ b/drivers/gpu/drm/bridge/Makefile
>> @@ -11,5 +11,6 @@ obj-$(CONFIG_DRM_SII9234) += sii9234.o
>>  obj-$(CONFIG_DRM_TOSHIBA_TC358767) += tc358767.o
>>  obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix/
>>  obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511/
>> +obj-$(CONFIG_DRM_TI_SN65DSI86) += ti-sn65dsi86.o
>>  obj-$(CONFIG_DRM_TI_TFP410) += ti-tfp410.o
>>  obj-y += synopsys/
>> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c 
>> b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
>> new file mode 100644
>> index 0000000..5473456
>> --- /dev/null
>> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
>> @@ -0,0 +1,696 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (c) 2018, The Linux Foundation. All rights reserved.
>> + */
>> +
>> +#include <drm/drmP.h>
>> +#include <drm/drm_atomic.h>
>> +#include <drm/drm_atomic_helper.h>
>> +#include <drm/drm_crtc_helper.h>
>> +#include <drm/drm_mipi_dsi.h>
>> +#include <drm/drm_of.h>
>> +#include <drm/drm_panel.h>
>> +#include <linux/clk.h>
>> +#include <linux/gpio/consumer.h>
>> +#include <linux/i2c.h>
>> +#include <linux/of_graph.h>
>> +#include <linux/pm_runtime.h>
>> +#include <linux/regmap.h>
>> +#include <linux/regulator/consumer.h>
>> +
>> +/* Link Training specific registers */
>> +#define SN_DEVICE_REV_REG			0x08
>> +#define SN_HPD_DISABLE_REG			0x5C
>> +#define SN_DPPLL_SRC_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_REG			0x76
>> +#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_VERTICAL_DISPLAY_SIZE_LOW_REG	0x24
>> +#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_REG		0x3A
>> +#define SN_DATA_FORMAT_REG			0x5B
>> +
>> +#define MIN_DSI_CLK_FREQ_MHZ	40
>> +
>> +/* fudge factor required to account for 8b/10b encoding */
>> +#define DP_CLK_FUDGE_NUM	10
>> +#define DP_CLK_FUDGE_DEN	8
>> +
>> +#define DPPLL_CLK_SRC_REFCLK	0
>> +#define DPPLL_CLK_SRC_DSICLK	1
>> +
>> +#define SN_REFCLK_FREQ_OFFSET	1
>> +#define SN_DSIA_LANE_OFFSET	3
>> +#define SN_DP_LANE_OFFSET	4
>> +#define SN_DP_DATA_RATE_OFFSET	5
>> +#define SN_SYNC_POLARITY_OFFSET	7
>> +
>> +#define SN_ENABLE_VID_STREAM_BIT	BIT(3)
>> +#define SN_REFCLK_FREQ_BITS		(BIT(3) | BIT(2) | BIT(1))
>> +#define SN_DSIA_NUM_LANES_BITS		(BIT(4) | BIT(3))
>> +#define SN_DP_NUM_LANES_BITS		(BIT(5) | BIT(4))
>> +#define SN_DP_DATA_RATE_BITS		(BIT(7) | BIT(6) | BIT(5))
> 
> For contiguous bitmasks you could use GENMASK(h, l), but it is up to 
> you.
Ok will update in next patch.
> 
>> +#define SN_HPD_DISABLE_BIT		BIT(0)
>> +
>> +#define SN_REGULATOR_SUPPLY_NUM		4
>> +
>> +struct ti_sn_bridge {
>> +	struct device			*dev;
>> +	struct regmap			*regmap;
>> +	struct drm_bridge		bridge;
>> +	struct drm_connector		connector;
>> +	struct device_node		*host_node;
>> +	struct mipi_dsi_device		*dsi;
>> +	struct clk			*refclk;
>> +	struct drm_panel		*panel;
>> +	struct gpio_desc		*enable_gpio;
>> +	struct regulator_bulk_data	supplies[SN_REGULATOR_SUPPLY_NUM];
>> +	struct i2c_adapter		*ddc;
>> +};
>> +
>> +static const struct regmap_range ti_sn_bridge_volatile_ranges[] = {
>> +	{ .range_min = 0, .range_max = 0xFF },
>> +};
>> +
>> +static const struct regmap_access_table ti_sn_bridge_volatile_table = 
>> {
>> +	.yes_ranges = ti_sn_bridge_volatile_ranges,
>> +	.n_yes_ranges = ARRAY_SIZE(ti_sn_bridge_volatile_ranges),
>> +};
>> +
>> +static const struct regmap_config ti_sn_bridge_regmap_config = {
>> +	.reg_bits = 8,
>> +	.val_bits = 8,
>> +	.volatile_table = &ti_sn_bridge_volatile_table,
>> +	.cache_type = REGCACHE_NONE,
>> +};
>> +
>> +static void ti_sn_bridge_write_u16(struct ti_sn_bridge *pdata,
>> +				   unsigned int reg, u16 val)
>> +{
>> +	regmap_write(pdata->regmap, reg, val & 0xFF);
>> +	regmap_write(pdata->regmap, reg + 1, val >> 8);
>> +}
>> +
>> +static int __maybe_unused ti_sn_bridge_resume(struct device *dev)
>> +{
>> +	struct ti_sn_bridge *pdata = dev_get_drvdata(dev);
>> +	int ret;
>> +
>> +	ret = regulator_bulk_enable(SN_REGULATOR_SUPPLY_NUM, 
>> pdata->supplies);
>> +	if (ret) {
>> +		DRM_ERROR("failed to enable supplies %d\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	gpiod_set_value(pdata->enable_gpio, 1);
>> +
>> +	return ret;
>> +}
>> +
>> +static int __maybe_unused ti_sn_bridge_suspend(struct device *dev)
>> +{
>> +	struct ti_sn_bridge *pdata = dev_get_drvdata(dev);
>> +	int ret;
>> +
>> +	gpiod_set_value(pdata->enable_gpio, 0);
>> +
>> +	ret = regulator_bulk_disable(SN_REGULATOR_SUPPLY_NUM, 
>> pdata->supplies);
>> +	if (ret)
>> +		DRM_ERROR("failed to disable supplies %d\n", ret);
>> +
>> +	return ret;
>> +}
>> +
>> +static const struct dev_pm_ops ti_sn_bridge_pm_ops = {
>> +	SET_RUNTIME_PM_OPS(ti_sn_bridge_suspend, ti_sn_bridge_resume, NULL)
>> +};
>> +
>> +/* Connector funcs */
>> +static struct ti_sn_bridge *
>> +connector_to_ti_sn_bridge(struct drm_connector *connector)
>> +{
>> +	return container_of(connector, struct ti_sn_bridge, connector);
>> +}
>> +
>> +static int ti_sn_bridge_connector_get_modes(struct drm_connector 
>> *connector)
>> +{
>> +	struct ti_sn_bridge *pdata = connector_to_ti_sn_bridge(connector);
>> +	struct edid *edid;
>> +	u32 num_modes;
>> +
>> +	if (pdata->panel) {
>> +		DRM_DEBUG_KMS("get mode from connected drm_panel\n");
>> +		return drm_panel_get_modes(pdata->panel);
>> +	}
>> +
>> +	if (!pdata->ddc)
>> +		return 0;
> I think it should not happen, ie probe should fail if both panel and 
> ddc
> are absent.
Ok i will remove this check in next patchset.
>> +
>> +	pm_runtime_get_sync(pdata->dev);
>> +	edid = drm_get_edid(connector, pdata->ddc);
>> +	pm_runtime_put_sync(pdata->dev);
> 
> Minor nit, you can use here pm_runtime_put, I guess you do not care 
> when
> it will be turned off.

Ok.
> 
>> +	if (!edid)
>> +		return 0;
>> +
>> +	drm_mode_connector_update_edid_property(connector, edid);
>> +	num_modes = drm_add_edid_modes(connector, edid);
>> +	kfree(edid);
>> +
>> +	return num_modes;
>> +}
>> +
>> +static enum drm_mode_status
>> +ti_sn_bridge_connector_mode_valid(struct drm_connector *connector,
>> +			     struct drm_display_mode *mode)
>> +{
>> +	/* maximum supported resolution is 4K at 60 fps */
>> +	if (mode->clock > 594000)
>> +		return MODE_CLOCK_HIGH;
>> +
>> +	return MODE_OK;
>> +}
>> +
>> +static struct drm_connector_helper_funcs 
>> ti_sn_bridge_connector_helper_funcs = {
>> +	.get_modes = ti_sn_bridge_connector_get_modes,
>> +	.mode_valid = ti_sn_bridge_connector_mode_valid,
>> +};
>> +
>> +static enum drm_connector_status
>> +ti_sn_bridge_connector_detect(struct drm_connector *connector, bool 
>> force)
>> +{
>> +	struct ti_sn_bridge *pdata = connector_to_ti_sn_bridge(connector);
>> +
>> +	/**
>> +	 * TODO: Currently if drm_panel is present, then always
>> +	 * return the status as connected. Need to add support to detect
>> +	 * device state for no panel(hot pluggable) scenarios.
>> +	 */
>> +	if (pdata->panel)
>> +		return connector_status_connected;
>> +	else
>> +		return connector_status_unknown;
>> +}
>> +
>> +static const struct drm_connector_funcs ti_sn_bridge_connector_funcs 
>> = {
>> +	.fill_modes = drm_helper_probe_single_connector_modes,
>> +	.detect = ti_sn_bridge_connector_detect,
>> +	.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 struct ti_sn_bridge *bridge_to_ti_sn_bridge(struct drm_bridge 
>> *bridge)
>> +{
>> +	return container_of(bridge, struct ti_sn_bridge, bridge);
>> +}
>> +
>> +static int ti_sn_bridge_parse_regulators(struct ti_sn_bridge *pdata)
>> +{
>> +	unsigned int i;
>> +	const char * const ti_sn_bridge_supply_names[] = {
>> +		"vcca", "vcc", "vccio", "vpll",
>> +	};
>> +
>> +	for (i = 0; i < SN_REGULATOR_SUPPLY_NUM; i++)
>> +		pdata->supplies[i].supply = ti_sn_bridge_supply_names[i];
>> +
>> +	return devm_regulator_bulk_get(pdata->dev, SN_REGULATOR_SUPPLY_NUM,
>> +				       pdata->supplies);
>> +}
>> +
>> +static int ti_sn_bridge_attach(struct drm_bridge *bridge)
>> +{
>> +	int ret, val;
>> +	struct ti_sn_bridge *pdata = bridge_to_ti_sn_bridge(bridge);
>> +	struct mipi_dsi_host *host;
>> +	struct mipi_dsi_device *dsi;
>> +	const struct mipi_dsi_device_info info = { .type = "ti_sn_bridge",
>> +						   .channel = 0,
>> +						   .node = NULL,
>> +						 };
>> +
>> +	ret = drm_connector_init(bridge->dev, &pdata->connector,
>> +				 &ti_sn_bridge_connector_funcs,
>> +				 DRM_MODE_CONNECTOR_eDP);
>> +	if (ret) {
>> +		DRM_ERROR("Failed to initialize connector with drm\n");
>> +		return ret;
>> +	}
>> +
>> +	drm_connector_helper_add(&pdata->connector,
>> +				 &ti_sn_bridge_connector_helper_funcs);
>> +	drm_mode_connector_attach_encoder(&pdata->connector, 
>> bridge->encoder);
>> +
>> +	host = of_find_mipi_dsi_host_by_node(pdata->host_node);
>> +	if (!host) {
>> +		DRM_ERROR("failed to find dsi host\n");
>> +		ret = -ENODEV;
>> +		goto err_dsi_host;
>> +	}
> 
> In case of late host probing it will fail, instead of probe deferring.
> If I remember I have asked you to move this code to probe, as deferring
> is only possible there.
> 

I moved the this code to probe, and returned probe defer if host not 
found in patchset v9.
This caused a dead lock when i was testing on actual HW. The reason 
being,
DSI host probe completion was waiting for any drm_panel/drm_bridge to 
get
added to the global panel/bridge list, and here in bridge driver we are 
waiting for
DSI host probe completion to happen so that host node gets added to the 
global host list.
Also i found other up-streamed bridge drivers are also doing the same in 
bridge_attach.

>> +
>> +	dsi = mipi_dsi_device_register_full(host, &info);
>> +	if (IS_ERR(dsi)) {
>> +		DRM_ERROR("failed to create dsi device\n");
>> +		ret = PTR_ERR(dsi);
>> +		goto err_dsi_host;
>> +	}
>> +
>> +	/* TODO: setting to 4 lanes always for now */
>> +	dsi->lanes = 4;
>> +	dsi->format = MIPI_DSI_FMT_RGB888;
>> +	dsi->mode_flags = MIPI_DSI_MODE_VIDEO | 
>> MIPI_DSI_MODE_VIDEO_SYNC_PULSE |
>> +			  MIPI_DSI_MODE_EOT_PACKET | MIPI_DSI_MODE_VIDEO_HSE;
>> +
>> +	/* check if continuous dsi clock is required or not */
>> +	pm_runtime_get_sync(pdata->dev);
>> +	regmap_read(pdata->regmap, SN_DPPLL_SRC_REG, &val);
>> +	pm_runtime_put_sync(pdata->dev);
>> +	if (!(val & DPPLL_CLK_SRC_DSICLK))
>> +		dsi->mode_flags |= MIPI_DSI_CLOCK_NON_CONTINUOUS;
>> +
>> +	ret = mipi_dsi_attach(dsi);
>> +	if (ret < 0) {
>> +		DRM_ERROR("failed to attach dsi to host\n");
>> +		goto err_dsi_attach;
>> +	}
>> +	pdata->dsi = dsi;
>> +
>> +	/* attach panel to bridge */
>> +	if (pdata->panel)
>> +		drm_panel_attach(pdata->panel, &pdata->connector);
>> +
>> +	return 0;
>> +
>> +err_dsi_attach:
>> +	mipi_dsi_device_unregister(dsi);
>> +err_dsi_host:
>> +	drm_connector_cleanup(&pdata->connector);
>> +	return ret;
>> +}
>> +
>> +static void ti_sn_bridge_disable(struct drm_bridge *bridge)
>> +{
>> +	struct ti_sn_bridge *pdata = bridge_to_ti_sn_bridge(bridge);
>> +
>> +	if (pdata->panel) {
>> +		drm_panel_disable(pdata->panel);
>> +		drm_panel_unprepare(pdata->panel);
>> +	}
>> +
>> +	/* disable video stream */
>> +	regmap_update_bits(pdata->regmap, SN_ENH_FRAME_REG,
>> +			   SN_ENABLE_VID_STREAM_BIT, 0);
>> +	/* semi auto link training mode OFF */
>> +	regmap_write(pdata->regmap, SN_ML_TX_MODE_REG, 0);
>> +	/* disable DP PLL */
>> +	regmap_write(pdata->regmap, SN_PLL_ENABLE_REG, 0);
> 
> Maybe in your particular case it does not matter but generally
> drm_panel_disable should be called 1st, but drm_panel_unprepare should
> be called somewhere after disabling video stream.

Ok i wil move panel unprepare to end in next patchset.
> 
>> +}
>> +
>> +static u32 ti_sn_bridge_get_dsi_freq(struct ti_sn_bridge *pdata)
>> +{
>> +	u32 bit_rate_khz, clk_freq_khz;
>> +	struct drm_display_mode *mode =
>> +		&pdata->bridge.encoder->crtc->state->adjusted_mode;
>> +
>> +	bit_rate_khz = mode->clock *
>> +			mipi_dsi_pixel_format_to_bpp(pdata->dsi->format);
>> +	clk_freq_khz = bit_rate_khz / (pdata->dsi->lanes * 2);
>> +
>> +	return clk_freq_khz;
>> +}
>> +
>> +/* clk frequencies supported by bridge in Hz in case derived from 
>> REFCLK pin */
>> +static const u32 ti_sn_bridge_refclk_lut[] = {
>> +	12000000,
>> +	19200000,
>> +	26000000,
>> +	27000000,
>> +	38400000,
>> +};
>> +
>> +/* clk frequencies supported by bridge in Hz in case derived from 
>> DACP/N pin */
>> +static const u32 ti_sn_bridge_dsiclk_lut[] = {
>> +	468000000,
>> +	384000000,
>> +	416000000,
>> +	486000000,
>> +	460800000,
>> +};
>> +
>> +static void ti_sn_bridge_set_refclk_freq(struct ti_sn_bridge *pdata)
>> +{
>> +	int i;
>> +	u32 refclk_rate;
>> +	const u32 *refclk_lut;
>> +	size_t refclk_lut_size;
>> +
>> +	if (pdata->refclk) {
>> +		refclk_rate = clk_get_rate(pdata->refclk);
>> +		refclk_lut = ti_sn_bridge_refclk_lut;
>> +		refclk_lut_size = ARRAY_SIZE(ti_sn_bridge_refclk_lut);
>> +		clk_prepare_enable(pdata->refclk);
>> +	} else {
>> +		refclk_rate = ti_sn_bridge_get_dsi_freq(pdata) * 1000;
>> +		refclk_lut = ti_sn_bridge_dsiclk_lut;
>> +		refclk_lut_size = ARRAY_SIZE(ti_sn_bridge_dsiclk_lut);
>> +	}
>> +
>> +	/* for i equals to refclk_lut_size means default frequency */
>> +	for (i = 0; i < refclk_lut_size; i++)
>> +		if (refclk_lut[i] == refclk_rate)
>> +			break;
>> +
>> +	regmap_update_bits(pdata->regmap, SN_DPPLL_SRC_REG,
>> +			   SN_REFCLK_FREQ_BITS, i << SN_REFCLK_FREQ_OFFSET);
>> +}
>> +
>> +/**
>> + * LUT index corresponds to register value and
>> + * LUT values corresponds to dp data rate supported
>> + * by the bridge in Mbps unit.
>> + */
>> +static const unsigned int ti_sn_bridge_dp_rate_lut[] = {
>> +	0, 1620, 2160, 2430, 2700, 3240, 4320, 5400
>> +};
>> +
>> +static void ti_sn_bridge_set_dsi_dp_rate(struct ti_sn_bridge *pdata)
>> +{
>> +	unsigned int bit_rate_mhz, clk_freq_mhz, dp_rate_mhz;
>> +	unsigned int val, i;
>> +	struct drm_display_mode *mode =
>> +		&pdata->bridge.encoder->crtc->state->adjusted_mode;
>> +
>> +	/* set DSIA clk frequency */
>> +	bit_rate_mhz = (mode->clock / 1000) *
>> +			mipi_dsi_pixel_format_to_bpp(pdata->dsi->format);
>> +	clk_freq_mhz = bit_rate_mhz / (pdata->dsi->lanes * 2);
>> +
>> +	/* for each increment in val, frequency increases by 5MHz */
>> +	val = (MIN_DSI_CLK_FREQ_MHZ / 5) +
>> +		(((clk_freq_mhz - MIN_DSI_CLK_FREQ_MHZ) / 5) & 0xFF);
>> +	regmap_write(pdata->regmap, SN_DSIA_CLK_FREQ_REG, val);
>> +
>> +	/* set DP data rate */
>> +	dp_rate_mhz = ((bit_rate_mhz / pdata->dsi->lanes) * 
>> DP_CLK_FUDGE_NUM) /
>> +							DP_CLK_FUDGE_DEN;
>> +	for (i = 0; i < ARRAY_SIZE(ti_sn_bridge_dp_rate_lut) - 1; i++)
>> +		if (ti_sn_bridge_dp_rate_lut[i] > dp_rate_mhz)
>> +			break;
>> +
>> +	regmap_update_bits(pdata->regmap, SN_DATARATE_CONFIG_REG,
>> +			   SN_DP_DATA_RATE_BITS, i << SN_DP_DATA_RATE_OFFSET);
>> +}
>> +
>> +static void ti_sn_bridge_set_video_timings(struct ti_sn_bridge 
>> *pdata)
>> +{
>> +	struct drm_display_mode *mode =
>> +		&pdata->bridge.encoder->crtc->state->adjusted_mode;
>> +	u8 hsync_polarity = 0, vsync_polarity = 0;
>> +
>> +	if (mode->flags & DRM_MODE_FLAG_PHSYNC)
>> +		hsync_polarity = 0x1;
> 
> You can assign:
> 
> hsync_polarity = BIT(SN_SYNC_POLARITY_OFFSET);
> 
> and then use it directly in regmap_write.
> 
>> +	if (mode->flags & DRM_MODE_FLAG_PVSYNC)
>> +		vsync_polarity = 0x1;
> 
> The same here.
> 
Ok.
>> +
>> +	ti_sn_bridge_write_u16(pdata, SN_CHA_ACTIVE_LINE_LENGTH_LOW_REG,
>> +			       mode->hdisplay);
>> +	ti_sn_bridge_write_u16(pdata, SN_CHA_VERTICAL_DISPLAY_SIZE_LOW_REG,
>> +			       mode->vdisplay);
>> +	regmap_write(pdata->regmap, SN_CHA_HSYNC_PULSE_WIDTH_LOW_REG,
>> +		     (mode->hsync_end - mode->hsync_start) & 0xFF);
>> +	regmap_write(pdata->regmap, SN_CHA_HSYNC_PULSE_WIDTH_HIGH_REG,
>> +		     (((mode->hsync_end - mode->hsync_start) >> 8) & 0x7F)
>> +		     | (hsync_polarity << SN_SYNC_POLARITY_OFFSET));
>> +	regmap_write(pdata->regmap, SN_CHA_VSYNC_PULSE_WIDTH_LOW_REG,
>> +		     (mode->vsync_end - mode->vsync_start) & 0xFF);
>> +	regmap_write(pdata->regmap, SN_CHA_VSYNC_PULSE_WIDTH_HIGH_REG,
>> +		    (((mode->vsync_end - mode->vsync_start) >> 8) & 0x7F)
>> +		    | (vsync_polarity << SN_SYNC_POLARITY_OFFSET));
>> +
>> +	regmap_write(pdata->regmap, SN_CHA_HORIZONTAL_BACK_PORCH_REG,
>> +		     (mode->htotal - mode->hsync_end) & 0xFF);
>> +	regmap_write(pdata->regmap, SN_CHA_VERTICAL_BACK_PORCH_REG,
>> +		     (mode->vtotal - mode->vsync_end) & 0xFF);
>> +
>> +	regmap_write(pdata->regmap, SN_CHA_HORIZONTAL_FRONT_PORCH_REG,
>> +		     (mode->hsync_start - mode->hdisplay) & 0xFF);
>> +	regmap_write(pdata->regmap, SN_CHA_VERTICAL_FRONT_PORCH_REG,
>> +		     (mode->vsync_start - mode->vdisplay) & 0xFF);
>> +
>> +	usleep_range(10000, 10500); /* 10ms delay recommended by spec */
>> +}
>> +
>> +static void ti_sn_bridge_enable(struct drm_bridge *bridge)
>> +{
>> +	struct ti_sn_bridge *pdata = bridge_to_ti_sn_bridge(bridge);
>> +	unsigned int val;
>> +
>> +	if (pdata->panel) {
>> +		drm_panel_prepare(pdata->panel);
>> +		/* in case drm_panel is connected then HPD is not supported */
>> +		regmap_update_bits(pdata->regmap, SN_HPD_DISABLE_REG,
>> +				   SN_HPD_DISABLE_BIT, SN_HPD_DISABLE_BIT);
> 
> I think HPD should be disabled much earlier, I guess somewhere in
> initialization/probe.
> 
As per the driver code, we are enabling the bridge power supplies and 
gpio
only in pre-enable function, i can move this HDP disable to pre-enable.

>> +	}
>> +
>> +	/* DSI_A lane config */
>> +	val = (4 - pdata->dsi->lanes) << SN_DSIA_LANE_OFFSET;
>> +	regmap_update_bits(pdata->regmap, SN_DSI_LANES_REG,
>> +			   SN_DSIA_NUM_LANES_BITS, val);
>> +
>> +	/* DP lane config */
>> +	val = (pdata->dsi->lanes - 1) << SN_DP_LANE_OFFSET;
>> +	regmap_update_bits(pdata->regmap, SN_SSC_CONFIG_REG,
>> +			   SN_DP_NUM_LANES_BITS, val);
>> +
>> +	/* set dsi/dp clk frequency value */
>> +	ti_sn_bridge_set_dsi_dp_rate(pdata);
>> +
>> +	/* enable DP PLL */
>> +	regmap_write(pdata->regmap, SN_PLL_ENABLE_REG, 1);
>> +	usleep_range(10000, 10500); /* 10ms delay recommended by spec */
>> +
>> +	/**
>> +	 * The SN65DSI86 only supports ASSR Display Authentication method 
>> and
>> +	 * this method is enabled by default. An eDP panel must support this
>> +	 * authentication method. We need to enable this method in the eDP 
>> panel
>> +	 * at DisplayPort address 0x0010A prior to link training.
>> +	 */
>> +	regmap_write(pdata->regmap, SN_AUX_WDATA0_REG, 0x01);
>> +	regmap_write(pdata->regmap, SN_AUX_ADDR_19_16_REG, 0x00);
>> +	regmap_write(pdata->regmap, SN_AUX_ADDR_15_8_REG, 0x01);
>> +	regmap_write(pdata->regmap, SN_AUX_ADDR_7_0_REG, 0x0A);
>> +	regmap_write(pdata->regmap, SN_AUX_LENGTH_REG, 0x01);
>> +	regmap_write(pdata->regmap, SN_AUX_CMD_REG, 0x81);
>> +	usleep_range(10000, 10500); /* 10ms delay recommended by spec */
>> +
>> +	/* Semi auto link training mode */
>> +	regmap_write(pdata->regmap, SN_ML_TX_MODE_REG, 0x0A);
>> +	msleep(20); /* 20ms delay recommended by spec */
>> +
>> +	/* config video parameters */
>> +	ti_sn_bridge_set_video_timings(pdata);
>> +
>> +	/* enable video stream */
>> +	regmap_update_bits(pdata->regmap, SN_ENH_FRAME_REG,
>> +			   SN_ENABLE_VID_STREAM_BIT, SN_ENABLE_VID_STREAM_BIT);
>> +
>> +	if (pdata->panel)
>> +		drm_panel_enable(pdata->panel);
>> +}
>> +
>> +static void ti_sn_bridge_pre_enable(struct drm_bridge *bridge)
>> +{
>> +	struct ti_sn_bridge *pdata = bridge_to_ti_sn_bridge(bridge);
>> +
>> +	pm_runtime_get_sync(pdata->dev);
>> +
>> +	/* configure bridge ref_clk */
>> +	ti_sn_bridge_set_refclk_freq(pdata);
>> +}
>> +
>> +static void ti_sn_bridge_post_disable(struct drm_bridge *bridge)
>> +{
>> +	struct ti_sn_bridge *pdata = bridge_to_ti_sn_bridge(bridge);
>> +
>> +	if (pdata->refclk)
>> +		clk_disable_unprepare(pdata->refclk);
>> +
>> +	pm_runtime_put_sync(pdata->dev);
>> +}
>> +
>> +static const struct drm_bridge_funcs ti_sn_bridge_funcs = {
>> +	.attach = ti_sn_bridge_attach,
>> +	.pre_enable = ti_sn_bridge_pre_enable,
>> +	.enable = ti_sn_bridge_enable,
>> +	.disable = ti_sn_bridge_disable,
>> +	.post_disable = ti_sn_bridge_post_disable,
>> +};
>> +
>> +static int ti_sn_bridge_parse_dsi_host(struct ti_sn_bridge *pdata)
>> +{
>> +	struct device_node *np = pdata->dev->of_node;
>> +
>> +	pdata->host_node = of_graph_get_remote_node(np, 0, 0);
>> +
>> +	if (!pdata->host_node) {
>> +		DRM_ERROR("remote dsi host node not found\n");
>> +		return -ENODEV;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static int ti_sn_bridge_probe(struct i2c_client *client,
>> +	 const struct i2c_device_id *id)
>> +{
>> +	struct ti_sn_bridge *pdata;
>> +	struct device_node *ddc_node;
>> +	int ret;
>> +
>> +	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
>> +		DRM_ERROR("device doesn't support I2C\n");
>> +		return -ENODEV;
>> +	}
>> +
>> +	pdata = devm_kzalloc(&client->dev, sizeof(struct ti_sn_bridge),
>> +			     GFP_KERNEL);
>> +	if (!pdata)
>> +		return -ENOMEM;
>> +
>> +	pdata->regmap = devm_regmap_init_i2c(client,
>> +					     &ti_sn_bridge_regmap_config);
>> +	if (IS_ERR(pdata->regmap)) {
>> +		DRM_ERROR("regmap i2c init failed\n");
>> +		return PTR_ERR(pdata->regmap);
>> +	}
>> +
>> +	pdata->dev = &client->dev;
>> +	dev_set_drvdata(&client->dev, pdata);
>> +
>> +	pdata->enable_gpio = devm_gpiod_get(pdata->dev, "enable",
>> +					    GPIOD_OUT_LOW);
>> +	if (IS_ERR(pdata->enable_gpio)) {
>> +		DRM_ERROR("failed to get enable gpio from DT\n");
>> +		ret = PTR_ERR(pdata->enable_gpio);
>> +		return ret;
>> +	}
>> +
>> +	ret = ti_sn_bridge_parse_regulators(pdata);
>> +	if (ret) {
>> +		DRM_ERROR("failed to parse regulators\n");
>> +		return ret;
>> +	}
>> +
>> +	ret = drm_of_find_panel_or_bridge(pdata->dev->of_node, 1, 0,
>> +					  &pdata->panel, NULL);
>> +	if (ret) {
>> +		DRM_ERROR("could not find any drm panel node\n");
>> +		return ret;
>> +	}
>> +
>> +	pdata->refclk = devm_clk_get(pdata->dev, "refclk");
>> +	if (IS_ERR(pdata->refclk)) {
>> +		ret = PTR_ERR(pdata->refclk);
>> +		if (ret == -EPROBE_DEFER)
>> +			return ret;
>> +		DRM_DEBUG_KMS("refclk not found\n");
>> +		pdata->refclk = NULL;
>> +	}
>> +
>> +	ddc_node = of_parse_phandle(pdata->dev->of_node, "ddc-i2c-bus", 0);
>> +	if (ddc_node) {
>> +		pdata->ddc = of_find_i2c_adapter_by_node(ddc_node);
>> +		of_node_put(ddc_node);
>> +		if (!pdata->ddc) {
>> +			DRM_DEBUG_KMS("failed to read ddc node\n");
>> +			return -EPROBE_DEFER;
>> +		}
>> +	} else {
>> +		DRM_DEBUG_KMS("no ddc property found\n");
>> +	}
> 
> I guess there should be exclusively panel or ddc, please fail if
> both/none are present.
> 
Just curious, can't an edp panel will have EDID flashed?
> 
> Regards
> Andrzej
> 
>> +
>> +	ret = ti_sn_bridge_parse_dsi_host(pdata);
>> +	if (ret)
>> +		return ret;
>> +
>> +	pm_runtime_enable(pdata->dev);
>> +
>> +	i2c_set_clientdata(client, pdata);
>> +
>> +	pdata->bridge.funcs = &ti_sn_bridge_funcs;
>> +	pdata->bridge.of_node = client->dev.of_node;
>> +
>> +	drm_bridge_add(&pdata->bridge);
>> +
>> +	return 0;
>> +}
>> +
>> +static int ti_sn_bridge_remove(struct i2c_client *client)
>> +{
>> +	struct ti_sn_bridge *pdata = i2c_get_clientdata(client);
>> +
>> +	if (!pdata)
>> +		return -EINVAL;
>> +
>> +	if (pdata->host_node)
>> +		of_node_put(pdata->host_node);
>> +
>> +	pm_runtime_disable(pdata->dev);
>> +
>> +	if (pdata->dsi) {
>> +		mipi_dsi_detach(pdata->dsi);
>> +		mipi_dsi_device_unregister(pdata->dsi);
>> +	}
>> +
>> +	drm_bridge_remove(&pdata->bridge);
>> +	i2c_put_adapter(pdata->ddc);
>> +
>> +	return 0;
>> +}
>> +
>> +static struct i2c_device_id ti_sn_bridge_id[] = {
>> +	{ "ti,sn65dsi86", 0},
>> +	{},
>> +};
>> +MODULE_DEVICE_TABLE(i2c, ti_sn_bridge_id);
>> +
>> +static const struct of_device_id ti_sn_bridge_match_table[] = {
>> +	{.compatible = "ti,sn65dsi86"},
>> +	{},
>> +};
>> +MODULE_DEVICE_TABLE(of, ti_sn_bridge_match_table);
>> +
>> +static struct i2c_driver ti_sn_bridge_driver = {
>> +	.driver = {
>> +		.name = "ti_sn65dsi86",
>> +		.of_match_table = ti_sn_bridge_match_table,
>> +		.pm = &ti_sn_bridge_pm_ops,
>> +	},
>> +	.probe = ti_sn_bridge_probe,
>> +	.remove = ti_sn_bridge_remove,
>> +	.id_table = ti_sn_bridge_id,
>> +};
>> +
>> +module_i2c_driver(ti_sn_bridge_driver);
>> +MODULE_DESCRIPTION("sn65dsi86 DSI to eDP bridge driver");
>> +MODULE_LICENSE("GPL v2");
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH v11 2/5] drm/bridge: add support for sn65dsi86 bridge driver

[Andrzej Hajda]

On 18.06.2018 07:51, spanda@codeaurora.org wrote:
> On 2018-06-15 16:59, Andrzej Hajda wrote:
>> Hi Sandeep,
>>
>> Thanks for the patch, I hope it will be merged soon.
>>
>> On 15.06.2018 08:43, 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).
>>>
>>> Changes in v4:
>>>  - Update License/Copyright (Sean Paul).
>>>  - Add Kconfig and Makefile changes (Sean Paul).
>>>  - Drop i2c gpio handling from this bridge driver, since i2c sda/scl 
>>> gpios
>>>    will be handled by i2c master.
>>>  - Update required supplies names.
>>>  - Remove unnecessary goto statements (Sean Paul).
>>>  - Add mutex lock to power_ctrl API to avoid race conditions (Sean
>>>    Paul).
>>>  - Add support to parse reference clk frequency from dt(optional).
>>>  - Update the bridge chip enable/disable sequence.
>>>
>>> Changes in v5:
>>>  - Fixed Kbuild test service reported warnings.
>>>
>>> Changes in v6:
>>>  - Use PM runtime based ref-counting instead of local ref_count 
>>> mechanism
>>>    (Stephen Boyd).
>>>  - Clean up some debug logs and indentations (Sean Paul).
>>>  - Simplify dp rate calculation (Sean Paul).
>>>  - Add support to configure refclk based on input REFCLK pin or DACP/N
>>>    pin (Stephen Boyd).
>>>
>>> Changes in v7:
>>>  - Use static supply entries instead of dynamic allocation (Andrzej
>>>    Hajda).
>>>  - Defer bridge driver probe if panel is not probed (Andrzej Hajda).
>>>  - Update of_graph APIs for correct node reference management. 
>>> (Andrzej
>>>    Hajda).
>>>  - Remove local display_mode object (Andrzej Hajda).
>>>  - Remove version id check function from driver.
>>>
>>> Changes in v8:
>>>  - Move dsi register/attach function to bridge driver probe (Andrzej
>>>    Hajda).
>>>  - Introduce a new helper function to write 16bit words into 
>>> consecutive
>>>    registers (Andrzej Hajda).
>>>  - Remove unnecessary macros (Andrzej Hajda).
>>>
>>> Changes in v9:
>>>  - Remove dsi register/attach from bridge probe, since dsi dev 
>>> register
>>>    completion also waits for any panel or bridge to get added. This 
>>> creates
>>>    deadlock situation when bridge driver calls dsi dev register and
>>>    attach before bridge add, in its probe function.
>>>  - Fix issues faced during testing of bridge driver on actual HW.
>>>  - Remove unnecessary initializations (Stephen Boyd).
>>>  - Use local refclk lut size instead of global macro (Sean Paul).
>>>
>>> Changes in v10:
>>>  - Use refclk to determine if continuous dsi clock is needed or not.
>>>
>>> Changes in v11:
>>>  - Read DPPLL_SRC register to determine continuous clock instead of
>>>    using refclk handle (Stephen Boyd).
>>>
>>> Signed-off-by: Sandeep Panda <spanda@codeaurora.org>
>>> ---
>>>  drivers/gpu/drm/bridge/Kconfig        |   9 +
>>>  drivers/gpu/drm/bridge/Makefile       |   1 +
>>>  drivers/gpu/drm/bridge/ti-sn65dsi86.c | 696 
>>> ++++++++++++++++++++++++++++++++++
>>>  3 files changed, 706 insertions(+)
>>>  create mode 100644 drivers/gpu/drm/bridge/ti-sn65dsi86.c
>>>
>>> diff --git a/drivers/gpu/drm/bridge/Kconfig 
>>> b/drivers/gpu/drm/bridge/Kconfig
>>> index 3b99d5a..8153150 100644
>>> --- a/drivers/gpu/drm/bridge/Kconfig
>>> +++ b/drivers/gpu/drm/bridge/Kconfig
>>> @@ -108,6 +108,15 @@ config DRM_TI_TFP410
>>>  	---help---
>>>  	  Texas Instruments TFP410 DVI/HDMI Transmitter driver
>>>
>>> +config DRM_TI_SN65DSI86
>>> +	tristate "TI SN65DSI86 DSI to eDP bridge"
>>> +	depends on OF
>>> +	select DRM_KMS_HELPER
>>> +	select REGMAP_I2C
>>> +	select DRM_PANEL
>>> +	---help---
>>> +	  Texas Instruments SN65DSI86 DSI to eDP Bridge driver
>>> +
>>>  source "drivers/gpu/drm/bridge/analogix/Kconfig"
>>>
>>>  source "drivers/gpu/drm/bridge/adv7511/Kconfig"
>>> diff --git a/drivers/gpu/drm/bridge/Makefile 
>>> b/drivers/gpu/drm/bridge/Makefile
>>> index 373eb28..a7ca000 100644
>>> --- a/drivers/gpu/drm/bridge/Makefile
>>> +++ b/drivers/gpu/drm/bridge/Makefile
>>> @@ -11,5 +11,6 @@ obj-$(CONFIG_DRM_SII9234) += sii9234.o
>>>  obj-$(CONFIG_DRM_TOSHIBA_TC358767) += tc358767.o
>>>  obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix/
>>>  obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511/
>>> +obj-$(CONFIG_DRM_TI_SN65DSI86) += ti-sn65dsi86.o
>>>  obj-$(CONFIG_DRM_TI_TFP410) += ti-tfp410.o
>>>  obj-y += synopsys/
>>> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c 
>>> b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
>>> new file mode 100644
>>> index 0000000..5473456
>>> --- /dev/null
>>> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
>>> @@ -0,0 +1,696 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + * Copyright (c) 2018, The Linux Foundation. All rights reserved.
>>> + */
>>> +
>>> +#include <drm/drmP.h>
>>> +#include <drm/drm_atomic.h>
>>> +#include <drm/drm_atomic_helper.h>
>>> +#include <drm/drm_crtc_helper.h>
>>> +#include <drm/drm_mipi_dsi.h>
>>> +#include <drm/drm_of.h>
>>> +#include <drm/drm_panel.h>
>>> +#include <linux/clk.h>
>>> +#include <linux/gpio/consumer.h>
>>> +#include <linux/i2c.h>
>>> +#include <linux/of_graph.h>
>>> +#include <linux/pm_runtime.h>
>>> +#include <linux/regmap.h>
>>> +#include <linux/regulator/consumer.h>
>>> +
>>> +/* Link Training specific registers */
>>> +#define SN_DEVICE_REV_REG			0x08
>>> +#define SN_HPD_DISABLE_REG			0x5C
>>> +#define SN_DPPLL_SRC_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_REG			0x76
>>> +#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_VERTICAL_DISPLAY_SIZE_LOW_REG	0x24
>>> +#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_REG		0x3A
>>> +#define SN_DATA_FORMAT_REG			0x5B
>>> +
>>> +#define MIN_DSI_CLK_FREQ_MHZ	40
>>> +
>>> +/* fudge factor required to account for 8b/10b encoding */
>>> +#define DP_CLK_FUDGE_NUM	10
>>> +#define DP_CLK_FUDGE_DEN	8
>>> +
>>> +#define DPPLL_CLK_SRC_REFCLK	0
>>> +#define DPPLL_CLK_SRC_DSICLK	1
>>> +
>>> +#define SN_REFCLK_FREQ_OFFSET	1
>>> +#define SN_DSIA_LANE_OFFSET	3
>>> +#define SN_DP_LANE_OFFSET	4
>>> +#define SN_DP_DATA_RATE_OFFSET	5
>>> +#define SN_SYNC_POLARITY_OFFSET	7
>>> +
>>> +#define SN_ENABLE_VID_STREAM_BIT	BIT(3)
>>> +#define SN_REFCLK_FREQ_BITS		(BIT(3) | BIT(2) | BIT(1))
>>> +#define SN_DSIA_NUM_LANES_BITS		(BIT(4) | BIT(3))
>>> +#define SN_DP_NUM_LANES_BITS		(BIT(5) | BIT(4))
>>> +#define SN_DP_DATA_RATE_BITS		(BIT(7) | BIT(6) | BIT(5))
>> For contiguous bitmasks you could use GENMASK(h, l), but it is up to 
>> you.
> Ok will update in next patch.
>>> +#define SN_HPD_DISABLE_BIT		BIT(0)
>>> +
>>> +#define SN_REGULATOR_SUPPLY_NUM		4
>>> +
>>> +struct ti_sn_bridge {
>>> +	struct device			*dev;
>>> +	struct regmap			*regmap;
>>> +	struct drm_bridge		bridge;
>>> +	struct drm_connector		connector;
>>> +	struct device_node		*host_node;
>>> +	struct mipi_dsi_device		*dsi;
>>> +	struct clk			*refclk;
>>> +	struct drm_panel		*panel;
>>> +	struct gpio_desc		*enable_gpio;
>>> +	struct regulator_bulk_data	supplies[SN_REGULATOR_SUPPLY_NUM];
>>> +	struct i2c_adapter		*ddc;
>>> +};
>>> +
>>> +static const struct regmap_range ti_sn_bridge_volatile_ranges[] = {
>>> +	{ .range_min = 0, .range_max = 0xFF },
>>> +};
>>> +
>>> +static const struct regmap_access_table ti_sn_bridge_volatile_table = 
>>> {
>>> +	.yes_ranges = ti_sn_bridge_volatile_ranges,
>>> +	.n_yes_ranges = ARRAY_SIZE(ti_sn_bridge_volatile_ranges),
>>> +};
>>> +
>>> +static const struct regmap_config ti_sn_bridge_regmap_config = {
>>> +	.reg_bits = 8,
>>> +	.val_bits = 8,
>>> +	.volatile_table = &ti_sn_bridge_volatile_table,
>>> +	.cache_type = REGCACHE_NONE,
>>> +};
>>> +
>>> +static void ti_sn_bridge_write_u16(struct ti_sn_bridge *pdata,
>>> +				   unsigned int reg, u16 val)
>>> +{
>>> +	regmap_write(pdata->regmap, reg, val & 0xFF);
>>> +	regmap_write(pdata->regmap, reg + 1, val >> 8);
>>> +}
>>> +
>>> +static int __maybe_unused ti_sn_bridge_resume(struct device *dev)
>>> +{
>>> +	struct ti_sn_bridge *pdata = dev_get_drvdata(dev);
>>> +	int ret;
>>> +
>>> +	ret = regulator_bulk_enable(SN_REGULATOR_SUPPLY_NUM, 
>>> pdata->supplies);
>>> +	if (ret) {
>>> +		DRM_ERROR("failed to enable supplies %d\n", ret);
>>> +		return ret;
>>> +	}
>>> +
>>> +	gpiod_set_value(pdata->enable_gpio, 1);
>>> +
>>> +	return ret;
>>> +}
>>> +
>>> +static int __maybe_unused ti_sn_bridge_suspend(struct device *dev)
>>> +{
>>> +	struct ti_sn_bridge *pdata = dev_get_drvdata(dev);
>>> +	int ret;
>>> +
>>> +	gpiod_set_value(pdata->enable_gpio, 0);
>>> +
>>> +	ret = regulator_bulk_disable(SN_REGULATOR_SUPPLY_NUM, 
>>> pdata->supplies);
>>> +	if (ret)
>>> +		DRM_ERROR("failed to disable supplies %d\n", ret);
>>> +
>>> +	return ret;
>>> +}
>>> +
>>> +static const struct dev_pm_ops ti_sn_bridge_pm_ops = {
>>> +	SET_RUNTIME_PM_OPS(ti_sn_bridge_suspend, ti_sn_bridge_resume, NULL)
>>> +};
>>> +
>>> +/* Connector funcs */
>>> +static struct ti_sn_bridge *
>>> +connector_to_ti_sn_bridge(struct drm_connector *connector)
>>> +{
>>> +	return container_of(connector, struct ti_sn_bridge, connector);
>>> +}
>>> +
>>> +static int ti_sn_bridge_connector_get_modes(struct drm_connector 
>>> *connector)
>>> +{
>>> +	struct ti_sn_bridge *pdata = connector_to_ti_sn_bridge(connector);
>>> +	struct edid *edid;
>>> +	u32 num_modes;
>>> +
>>> +	if (pdata->panel) {
>>> +		DRM_DEBUG_KMS("get mode from connected drm_panel\n");
>>> +		return drm_panel_get_modes(pdata->panel);
>>> +	}
>>> +
>>> +	if (!pdata->ddc)
>>> +		return 0;
>> I think it should not happen, ie probe should fail if both panel and 
>> ddc
>> are absent.
> Ok i will remove this check in next patchset.
>>> +
>>> +	pm_runtime_get_sync(pdata->dev);
>>> +	edid = drm_get_edid(connector, pdata->ddc);
>>> +	pm_runtime_put_sync(pdata->dev);
>> Minor nit, you can use here pm_runtime_put, I guess you do not care 
>> when
>> it will be turned off.
> Ok.
>>> +	if (!edid)
>>> +		return 0;
>>> +
>>> +	drm_mode_connector_update_edid_property(connector, edid);
>>> +	num_modes = drm_add_edid_modes(connector, edid);
>>> +	kfree(edid);
>>> +
>>> +	return num_modes;
>>> +}
>>> +
>>> +static enum drm_mode_status
>>> +ti_sn_bridge_connector_mode_valid(struct drm_connector *connector,
>>> +			     struct drm_display_mode *mode)
>>> +{
>>> +	/* maximum supported resolution is 4K at 60 fps */
>>> +	if (mode->clock > 594000)
>>> +		return MODE_CLOCK_HIGH;
>>> +
>>> +	return MODE_OK;
>>> +}
>>> +
>>> +static struct drm_connector_helper_funcs 
>>> ti_sn_bridge_connector_helper_funcs = {
>>> +	.get_modes = ti_sn_bridge_connector_get_modes,
>>> +	.mode_valid = ti_sn_bridge_connector_mode_valid,
>>> +};
>>> +
>>> +static enum drm_connector_status
>>> +ti_sn_bridge_connector_detect(struct drm_connector *connector, bool 
>>> force)
>>> +{
>>> +	struct ti_sn_bridge *pdata = connector_to_ti_sn_bridge(connector);
>>> +
>>> +	/**
>>> +	 * TODO: Currently if drm_panel is present, then always
>>> +	 * return the status as connected. Need to add support to detect
>>> +	 * device state for no panel(hot pluggable) scenarios.
>>> +	 */
>>> +	if (pdata->panel)
>>> +		return connector_status_connected;
>>> +	else
>>> +		return connector_status_unknown;
>>> +}
>>> +
>>> +static const struct drm_connector_funcs ti_sn_bridge_connector_funcs 
>>> = {
>>> +	.fill_modes = drm_helper_probe_single_connector_modes,
>>> +	.detect = ti_sn_bridge_connector_detect,
>>> +	.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 struct ti_sn_bridge *bridge_to_ti_sn_bridge(struct drm_bridge 
>>> *bridge)
>>> +{
>>> +	return container_of(bridge, struct ti_sn_bridge, bridge);
>>> +}
>>> +
>>> +static int ti_sn_bridge_parse_regulators(struct ti_sn_bridge *pdata)
>>> +{
>>> +	unsigned int i;
>>> +	const char * const ti_sn_bridge_supply_names[] = {
>>> +		"vcca", "vcc", "vccio", "vpll",
>>> +	};
>>> +
>>> +	for (i = 0; i < SN_REGULATOR_SUPPLY_NUM; i++)
>>> +		pdata->supplies[i].supply = ti_sn_bridge_supply_names[i];
>>> +
>>> +	return devm_regulator_bulk_get(pdata->dev, SN_REGULATOR_SUPPLY_NUM,
>>> +				       pdata->supplies);
>>> +}
>>> +
>>> +static int ti_sn_bridge_attach(struct drm_bridge *bridge)
>>> +{
>>> +	int ret, val;
>>> +	struct ti_sn_bridge *pdata = bridge_to_ti_sn_bridge(bridge);
>>> +	struct mipi_dsi_host *host;
>>> +	struct mipi_dsi_device *dsi;
>>> +	const struct mipi_dsi_device_info info = { .type = "ti_sn_bridge",
>>> +						   .channel = 0,
>>> +						   .node = NULL,
>>> +						 };
>>> +
>>> +	ret = drm_connector_init(bridge->dev, &pdata->connector,
>>> +				 &ti_sn_bridge_connector_funcs,
>>> +				 DRM_MODE_CONNECTOR_eDP);
>>> +	if (ret) {
>>> +		DRM_ERROR("Failed to initialize connector with drm\n");
>>> +		return ret;
>>> +	}
>>> +
>>> +	drm_connector_helper_add(&pdata->connector,
>>> +				 &ti_sn_bridge_connector_helper_funcs);
>>> +	drm_mode_connector_attach_encoder(&pdata->connector, 
>>> bridge->encoder);
>>> +
>>> +	host = of_find_mipi_dsi_host_by_node(pdata->host_node);
>>> +	if (!host) {
>>> +		DRM_ERROR("failed to find dsi host\n");
>>> +		ret = -ENODEV;
>>> +		goto err_dsi_host;
>>> +	}
>> In case of late host probing it will fail, instead of probe deferring.
>> If I remember I have asked you to move this code to probe, as deferring
>> is only possible there.
>>
> I moved the this code to probe, and returned probe defer if host not 
> found in patchset v9.
> This caused a dead lock when i was testing on actual HW. The reason 
> being,
> DSI host probe completion was waiting for any drm_panel/drm_bridge to 
> get
> added to the global panel/bridge list, and here in bridge driver we are 
> waiting for
> DSI host probe completion to happen so that host node gets added to the 
> global host list.
> Also i found other up-streamed bridge drivers are also doing the same in 
> bridge_attach.

So the dsi host driver behaves incorrectly. DSI host should look for
devices (panel or subsequent bridge) on its DSI bus in attach callback,
or later. Otherwise we risk different deadlocks, as the one above. On
the other side panel should advertise drm_panel only if it is fully
ready, ie it should not call drm_panel_add if it has not gathered all
required resources.
Grep+manual check on sources shows most of the drivers follows these
rules (cdns, synopsys, exynos, rockchip, sun6i, tegra) but there are
exceptions: kirin, mtk, msm, vc4(?).
If you can you can try to fix DSI host, otherwise the problem will
strike back in the future. If it is hard to do please leave big comment
in the code why it is done this way.

>
>>> +
>>> +	dsi = mipi_dsi_device_register_full(host, &info);
>>> +	if (IS_ERR(dsi)) {
>>> +		DRM_ERROR("failed to create dsi device\n");
>>> +		ret = PTR_ERR(dsi);
>>> +		goto err_dsi_host;
>>> +	}
>>> +
>>> +	/* TODO: setting to 4 lanes always for now */
>>> +	dsi->lanes = 4;
>>> +	dsi->format = MIPI_DSI_FMT_RGB888;
>>> +	dsi->mode_flags = MIPI_DSI_MODE_VIDEO | 
>>> MIPI_DSI_MODE_VIDEO_SYNC_PULSE |
>>> +			  MIPI_DSI_MODE_EOT_PACKET | MIPI_DSI_MODE_VIDEO_HSE;
>>> +
>>> +	/* check if continuous dsi clock is required or not */
>>> +	pm_runtime_get_sync(pdata->dev);
>>> +	regmap_read(pdata->regmap, SN_DPPLL_SRC_REG, &val);
>>> +	pm_runtime_put_sync(pdata->dev);
>>> +	if (!(val & DPPLL_CLK_SRC_DSICLK))
>>> +		dsi->mode_flags |= MIPI_DSI_CLOCK_NON_CONTINUOUS;
>>> +
>>> +	ret = mipi_dsi_attach(dsi);
>>> +	if (ret < 0) {
>>> +		DRM_ERROR("failed to attach dsi to host\n");
>>> +		goto err_dsi_attach;
>>> +	}
>>> +	pdata->dsi = dsi;
>>> +
>>> +	/* attach panel to bridge */
>>> +	if (pdata->panel)
>>> +		drm_panel_attach(pdata->panel, &pdata->connector);
>>> +
>>> +	return 0;
>>> +
>>> +err_dsi_attach:
>>> +	mipi_dsi_device_unregister(dsi);
>>> +err_dsi_host:
>>> +	drm_connector_cleanup(&pdata->connector);
>>> +	return ret;
>>> +}
>>> +
>>> +static void ti_sn_bridge_disable(struct drm_bridge *bridge)
>>> +{
>>> +	struct ti_sn_bridge *pdata = bridge_to_ti_sn_bridge(bridge);
>>> +
>>> +	if (pdata->panel) {
>>> +		drm_panel_disable(pdata->panel);
>>> +		drm_panel_unprepare(pdata->panel);
>>> +	}
>>> +
>>> +	/* disable video stream */
>>> +	regmap_update_bits(pdata->regmap, SN_ENH_FRAME_REG,
>>> +			   SN_ENABLE_VID_STREAM_BIT, 0);
>>> +	/* semi auto link training mode OFF */
>>> +	regmap_write(pdata->regmap, SN_ML_TX_MODE_REG, 0);
>>> +	/* disable DP PLL */
>>> +	regmap_write(pdata->regmap, SN_PLL_ENABLE_REG, 0);
>> Maybe in your particular case it does not matter but generally
>> drm_panel_disable should be called 1st, but drm_panel_unprepare should
>> be called somewhere after disabling video stream.
> Ok i wil move panel unprepare to end in next patchset.
>>> +}
>>> +
>>> +static u32 ti_sn_bridge_get_dsi_freq(struct ti_sn_bridge *pdata)
>>> +{
>>> +	u32 bit_rate_khz, clk_freq_khz;
>>> +	struct drm_display_mode *mode =
>>> +		&pdata->bridge.encoder->crtc->state->adjusted_mode;
>>> +
>>> +	bit_rate_khz = mode->clock *
>>> +			mipi_dsi_pixel_format_to_bpp(pdata->dsi->format);
>>> +	clk_freq_khz = bit_rate_khz / (pdata->dsi->lanes * 2);
>>> +
>>> +	return clk_freq_khz;
>>> +}
>>> +
>>> +/* clk frequencies supported by bridge in Hz in case derived from 
>>> REFCLK pin */
>>> +static const u32 ti_sn_bridge_refclk_lut[] = {
>>> +	12000000,
>>> +	19200000,
>>> +	26000000,
>>> +	27000000,
>>> +	38400000,
>>> +};
>>> +
>>> +/* clk frequencies supported by bridge in Hz in case derived from 
>>> DACP/N pin */
>>> +static const u32 ti_sn_bridge_dsiclk_lut[] = {
>>> +	468000000,
>>> +	384000000,
>>> +	416000000,
>>> +	486000000,
>>> +	460800000,
>>> +};
>>> +
>>> +static void ti_sn_bridge_set_refclk_freq(struct ti_sn_bridge *pdata)
>>> +{
>>> +	int i;
>>> +	u32 refclk_rate;
>>> +	const u32 *refclk_lut;
>>> +	size_t refclk_lut_size;
>>> +
>>> +	if (pdata->refclk) {
>>> +		refclk_rate = clk_get_rate(pdata->refclk);
>>> +		refclk_lut = ti_sn_bridge_refclk_lut;
>>> +		refclk_lut_size = ARRAY_SIZE(ti_sn_bridge_refclk_lut);
>>> +		clk_prepare_enable(pdata->refclk);
>>> +	} else {
>>> +		refclk_rate = ti_sn_bridge_get_dsi_freq(pdata) * 1000;
>>> +		refclk_lut = ti_sn_bridge_dsiclk_lut;
>>> +		refclk_lut_size = ARRAY_SIZE(ti_sn_bridge_dsiclk_lut);
>>> +	}
>>> +
>>> +	/* for i equals to refclk_lut_size means default frequency */
>>> +	for (i = 0; i < refclk_lut_size; i++)
>>> +		if (refclk_lut[i] == refclk_rate)
>>> +			break;
>>> +
>>> +	regmap_update_bits(pdata->regmap, SN_DPPLL_SRC_REG,
>>> +			   SN_REFCLK_FREQ_BITS, i << SN_REFCLK_FREQ_OFFSET);
>>> +}
>>> +
>>> +/**
>>> + * LUT index corresponds to register value and
>>> + * LUT values corresponds to dp data rate supported
>>> + * by the bridge in Mbps unit.
>>> + */
>>> +static const unsigned int ti_sn_bridge_dp_rate_lut[] = {
>>> +	0, 1620, 2160, 2430, 2700, 3240, 4320, 5400
>>> +};
>>> +
>>> +static void ti_sn_bridge_set_dsi_dp_rate(struct ti_sn_bridge *pdata)
>>> +{
>>> +	unsigned int bit_rate_mhz, clk_freq_mhz, dp_rate_mhz;
>>> +	unsigned int val, i;
>>> +	struct drm_display_mode *mode =
>>> +		&pdata->bridge.encoder->crtc->state->adjusted_mode;
>>> +
>>> +	/* set DSIA clk frequency */
>>> +	bit_rate_mhz = (mode->clock / 1000) *
>>> +			mipi_dsi_pixel_format_to_bpp(pdata->dsi->format);
>>> +	clk_freq_mhz = bit_rate_mhz / (pdata->dsi->lanes * 2);
>>> +
>>> +	/* for each increment in val, frequency increases by 5MHz */
>>> +	val = (MIN_DSI_CLK_FREQ_MHZ / 5) +
>>> +		(((clk_freq_mhz - MIN_DSI_CLK_FREQ_MHZ) / 5) & 0xFF);
>>> +	regmap_write(pdata->regmap, SN_DSIA_CLK_FREQ_REG, val);
>>> +
>>> +	/* set DP data rate */
>>> +	dp_rate_mhz = ((bit_rate_mhz / pdata->dsi->lanes) * 
>>> DP_CLK_FUDGE_NUM) /
>>> +							DP_CLK_FUDGE_DEN;
>>> +	for (i = 0; i < ARRAY_SIZE(ti_sn_bridge_dp_rate_lut) - 1; i++)
>>> +		if (ti_sn_bridge_dp_rate_lut[i] > dp_rate_mhz)
>>> +			break;
>>> +
>>> +	regmap_update_bits(pdata->regmap, SN_DATARATE_CONFIG_REG,
>>> +			   SN_DP_DATA_RATE_BITS, i << SN_DP_DATA_RATE_OFFSET);
>>> +}
>>> +
>>> +static void ti_sn_bridge_set_video_timings(struct ti_sn_bridge 
>>> *pdata)
>>> +{
>>> +	struct drm_display_mode *mode =
>>> +		&pdata->bridge.encoder->crtc->state->adjusted_mode;
>>> +	u8 hsync_polarity = 0, vsync_polarity = 0;
>>> +
>>> +	if (mode->flags & DRM_MODE_FLAG_PHSYNC)
>>> +		hsync_polarity = 0x1;
>> You can assign:
>>
>> hsync_polarity = BIT(SN_SYNC_POLARITY_OFFSET);
>>
>> and then use it directly in regmap_write.
>>
>>> +	if (mode->flags & DRM_MODE_FLAG_PVSYNC)
>>> +		vsync_polarity = 0x1;
>> The same here.
>>
> Ok.
>>> +
>>> +	ti_sn_bridge_write_u16(pdata, SN_CHA_ACTIVE_LINE_LENGTH_LOW_REG,
>>> +			       mode->hdisplay);
>>> +	ti_sn_bridge_write_u16(pdata, SN_CHA_VERTICAL_DISPLAY_SIZE_LOW_REG,
>>> +			       mode->vdisplay);
>>> +	regmap_write(pdata->regmap, SN_CHA_HSYNC_PULSE_WIDTH_LOW_REG,
>>> +		     (mode->hsync_end - mode->hsync_start) & 0xFF);
>>> +	regmap_write(pdata->regmap, SN_CHA_HSYNC_PULSE_WIDTH_HIGH_REG,
>>> +		     (((mode->hsync_end - mode->hsync_start) >> 8) & 0x7F)
>>> +		     | (hsync_polarity << SN_SYNC_POLARITY_OFFSET));
>>> +	regmap_write(pdata->regmap, SN_CHA_VSYNC_PULSE_WIDTH_LOW_REG,
>>> +		     (mode->vsync_end - mode->vsync_start) & 0xFF);
>>> +	regmap_write(pdata->regmap, SN_CHA_VSYNC_PULSE_WIDTH_HIGH_REG,
>>> +		    (((mode->vsync_end - mode->vsync_start) >> 8) & 0x7F)
>>> +		    | (vsync_polarity << SN_SYNC_POLARITY_OFFSET));
>>> +
>>> +	regmap_write(pdata->regmap, SN_CHA_HORIZONTAL_BACK_PORCH_REG,
>>> +		     (mode->htotal - mode->hsync_end) & 0xFF);
>>> +	regmap_write(pdata->regmap, SN_CHA_VERTICAL_BACK_PORCH_REG,
>>> +		     (mode->vtotal - mode->vsync_end) & 0xFF);
>>> +
>>> +	regmap_write(pdata->regmap, SN_CHA_HORIZONTAL_FRONT_PORCH_REG,
>>> +		     (mode->hsync_start - mode->hdisplay) & 0xFF);
>>> +	regmap_write(pdata->regmap, SN_CHA_VERTICAL_FRONT_PORCH_REG,
>>> +		     (mode->vsync_start - mode->vdisplay) & 0xFF);
>>> +
>>> +	usleep_range(10000, 10500); /* 10ms delay recommended by spec */
>>> +}
>>> +
>>> +static void ti_sn_bridge_enable(struct drm_bridge *bridge)
>>> +{
>>> +	struct ti_sn_bridge *pdata = bridge_to_ti_sn_bridge(bridge);
>>> +	unsigned int val;
>>> +
>>> +	if (pdata->panel) {
>>> +		drm_panel_prepare(pdata->panel);
>>> +		/* in case drm_panel is connected then HPD is not supported */
>>> +		regmap_update_bits(pdata->regmap, SN_HPD_DISABLE_REG,
>>> +				   SN_HPD_DISABLE_BIT, SN_HPD_DISABLE_BIT);
>> I think HPD should be disabled much earlier, I guess somewhere in
>> initialization/probe.
>>
> As per the driver code, we are enabling the bridge power supplies and 
> gpio
> only in pre-enable function, i can move this HDP disable to pre-enable.

Yes, it looks better.


>
>>> +	}
>>> +
>>> +	/* DSI_A lane config */
>>> +	val = (4 - pdata->dsi->lanes) << SN_DSIA_LANE_OFFSET;
>>> +	regmap_update_bits(pdata->regmap, SN_DSI_LANES_REG,
>>> +			   SN_DSIA_NUM_LANES_BITS, val);
>>> +
>>> +	/* DP lane config */
>>> +	val = (pdata->dsi->lanes - 1) << SN_DP_LANE_OFFSET;
>>> +	regmap_update_bits(pdata->regmap, SN_SSC_CONFIG_REG,
>>> +			   SN_DP_NUM_LANES_BITS, val);
>>> +
>>> +	/* set dsi/dp clk frequency value */
>>> +	ti_sn_bridge_set_dsi_dp_rate(pdata);
>>> +
>>> +	/* enable DP PLL */
>>> +	regmap_write(pdata->regmap, SN_PLL_ENABLE_REG, 1);
>>> +	usleep_range(10000, 10500); /* 10ms delay recommended by spec */
>>> +
>>> +	/**
>>> +	 * The SN65DSI86 only supports ASSR Display Authentication method 
>>> and
>>> +	 * this method is enabled by default. An eDP panel must support this
>>> +	 * authentication method. We need to enable this method in the eDP 
>>> panel
>>> +	 * at DisplayPort address 0x0010A prior to link training.
>>> +	 */
>>> +	regmap_write(pdata->regmap, SN_AUX_WDATA0_REG, 0x01);
>>> +	regmap_write(pdata->regmap, SN_AUX_ADDR_19_16_REG, 0x00);
>>> +	regmap_write(pdata->regmap, SN_AUX_ADDR_15_8_REG, 0x01);
>>> +	regmap_write(pdata->regmap, SN_AUX_ADDR_7_0_REG, 0x0A);
>>> +	regmap_write(pdata->regmap, SN_AUX_LENGTH_REG, 0x01);
>>> +	regmap_write(pdata->regmap, SN_AUX_CMD_REG, 0x81);
>>> +	usleep_range(10000, 10500); /* 10ms delay recommended by spec */
>>> +
>>> +	/* Semi auto link training mode */
>>> +	regmap_write(pdata->regmap, SN_ML_TX_MODE_REG, 0x0A);
>>> +	msleep(20); /* 20ms delay recommended by spec */
>>> +
>>> +	/* config video parameters */
>>> +	ti_sn_bridge_set_video_timings(pdata);
>>> +
>>> +	/* enable video stream */
>>> +	regmap_update_bits(pdata->regmap, SN_ENH_FRAME_REG,
>>> +			   SN_ENABLE_VID_STREAM_BIT, SN_ENABLE_VID_STREAM_BIT);
>>> +
>>> +	if (pdata->panel)
>>> +		drm_panel_enable(pdata->panel);
>>> +}
>>> +
>>> +static void ti_sn_bridge_pre_enable(struct drm_bridge *bridge)
>>> +{
>>> +	struct ti_sn_bridge *pdata = bridge_to_ti_sn_bridge(bridge);
>>> +
>>> +	pm_runtime_get_sync(pdata->dev);
>>> +
>>> +	/* configure bridge ref_clk */
>>> +	ti_sn_bridge_set_refclk_freq(pdata);
>>> +}
>>> +
>>> +static void ti_sn_bridge_post_disable(struct drm_bridge *bridge)
>>> +{
>>> +	struct ti_sn_bridge *pdata = bridge_to_ti_sn_bridge(bridge);
>>> +
>>> +	if (pdata->refclk)
>>> +		clk_disable_unprepare(pdata->refclk);
>>> +
>>> +	pm_runtime_put_sync(pdata->dev);
>>> +}
>>> +
>>> +static const struct drm_bridge_funcs ti_sn_bridge_funcs = {
>>> +	.attach = ti_sn_bridge_attach,
>>> +	.pre_enable = ti_sn_bridge_pre_enable,
>>> +	.enable = ti_sn_bridge_enable,
>>> +	.disable = ti_sn_bridge_disable,
>>> +	.post_disable = ti_sn_bridge_post_disable,
>>> +};
>>> +
>>> +static int ti_sn_bridge_parse_dsi_host(struct ti_sn_bridge *pdata)
>>> +{
>>> +	struct device_node *np = pdata->dev->of_node;
>>> +
>>> +	pdata->host_node = of_graph_get_remote_node(np, 0, 0);
>>> +
>>> +	if (!pdata->host_node) {
>>> +		DRM_ERROR("remote dsi host node not found\n");
>>> +		return -ENODEV;
>>> +	}
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static int ti_sn_bridge_probe(struct i2c_client *client,
>>> +	 const struct i2c_device_id *id)
>>> +{
>>> +	struct ti_sn_bridge *pdata;
>>> +	struct device_node *ddc_node;
>>> +	int ret;
>>> +
>>> +	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
>>> +		DRM_ERROR("device doesn't support I2C\n");
>>> +		return -ENODEV;
>>> +	}
>>> +
>>> +	pdata = devm_kzalloc(&client->dev, sizeof(struct ti_sn_bridge),
>>> +			     GFP_KERNEL);
>>> +	if (!pdata)
>>> +		return -ENOMEM;
>>> +
>>> +	pdata->regmap = devm_regmap_init_i2c(client,
>>> +					     &ti_sn_bridge_regmap_config);
>>> +	if (IS_ERR(pdata->regmap)) {
>>> +		DRM_ERROR("regmap i2c init failed\n");
>>> +		return PTR_ERR(pdata->regmap);
>>> +	}
>>> +
>>> +	pdata->dev = &client->dev;
>>> +	dev_set_drvdata(&client->dev, pdata);
>>> +
>>> +	pdata->enable_gpio = devm_gpiod_get(pdata->dev, "enable",
>>> +					    GPIOD_OUT_LOW);
>>> +	if (IS_ERR(pdata->enable_gpio)) {
>>> +		DRM_ERROR("failed to get enable gpio from DT\n");
>>> +		ret = PTR_ERR(pdata->enable_gpio);
>>> +		return ret;
>>> +	}
>>> +
>>> +	ret = ti_sn_bridge_parse_regulators(pdata);
>>> +	if (ret) {
>>> +		DRM_ERROR("failed to parse regulators\n");
>>> +		return ret;
>>> +	}
>>> +
>>> +	ret = drm_of_find_panel_or_bridge(pdata->dev->of_node, 1, 0,
>>> +					  &pdata->panel, NULL);
>>> +	if (ret) {
>>> +		DRM_ERROR("could not find any drm panel node\n");
>>> +		return ret;
>>> +	}
>>> +
>>> +	pdata->refclk = devm_clk_get(pdata->dev, "refclk");
>>> +	if (IS_ERR(pdata->refclk)) {
>>> +		ret = PTR_ERR(pdata->refclk);
>>> +		if (ret == -EPROBE_DEFER)
>>> +			return ret;
>>> +		DRM_DEBUG_KMS("refclk not found\n");
>>> +		pdata->refclk = NULL;
>>> +	}
>>> +
>>> +	ddc_node = of_parse_phandle(pdata->dev->of_node, "ddc-i2c-bus", 0);
>>> +	if (ddc_node) {
>>> +		pdata->ddc = of_find_i2c_adapter_by_node(ddc_node);
>>> +		of_node_put(ddc_node);
>>> +		if (!pdata->ddc) {
>>> +			DRM_DEBUG_KMS("failed to read ddc node\n");
>>> +			return -EPROBE_DEFER;
>>> +		}
>>> +	} else {
>>> +		DRM_DEBUG_KMS("no ddc property found\n");
>>> +	}
>> I guess there should be exclusively panel or ddc, please fail if
>> both/none are present.
>>
> Just curious, can't an edp panel will have EDID flashed?


It can, if you have such case you can decide how to handle different
sources of info about modes, but for making EDID working you should 1st
implement HPD :)

Regards
Andrzej

>> Regards
>> Andrzej
>>
>>> +
>>> +	ret = ti_sn_bridge_parse_dsi_host(pdata);
>>> +	if (ret)
>>> +		return ret;
>>> +
>>> +	pm_runtime_enable(pdata->dev);
>>> +
>>> +	i2c_set_clientdata(client, pdata);
>>> +
>>> +	pdata->bridge.funcs = &ti_sn_bridge_funcs;
>>> +	pdata->bridge.of_node = client->dev.of_node;
>>> +
>>> +	drm_bridge_add(&pdata->bridge);
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static int ti_sn_bridge_remove(struct i2c_client *client)
>>> +{
>>> +	struct ti_sn_bridge *pdata = i2c_get_clientdata(client);
>>> +
>>> +	if (!pdata)
>>> +		return -EINVAL;
>>> +
>>> +	if (pdata->host_node)
>>> +		of_node_put(pdata->host_node);
>>> +
>>> +	pm_runtime_disable(pdata->dev);
>>> +
>>> +	if (pdata->dsi) {
>>> +		mipi_dsi_detach(pdata->dsi);
>>> +		mipi_dsi_device_unregister(pdata->dsi);
>>> +	}
>>> +
>>> +	drm_bridge_remove(&pdata->bridge);
>>> +	i2c_put_adapter(pdata->ddc);
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static struct i2c_device_id ti_sn_bridge_id[] = {
>>> +	{ "ti,sn65dsi86", 0},
>>> +	{},
>>> +};
>>> +MODULE_DEVICE_TABLE(i2c, ti_sn_bridge_id);
>>> +
>>> +static const struct of_device_id ti_sn_bridge_match_table[] = {
>>> +	{.compatible = "ti,sn65dsi86"},
>>> +	{},
>>> +};
>>> +MODULE_DEVICE_TABLE(of, ti_sn_bridge_match_table);
>>> +
>>> +static struct i2c_driver ti_sn_bridge_driver = {
>>> +	.driver = {
>>> +		.name = "ti_sn65dsi86",
>>> +		.of_match_table = ti_sn_bridge_match_table,
>>> +		.pm = &ti_sn_bridge_pm_ops,
>>> +	},
>>> +	.probe = ti_sn_bridge_probe,
>>> +	.remove = ti_sn_bridge_remove,
>>> +	.id_table = ti_sn_bridge_id,
>>> +};
>>> +
>>> +module_i2c_driver(ti_sn_bridge_driver);
>>> +MODULE_DESCRIPTION("sn65dsi86 DSI to eDP bridge driver");
>>> +MODULE_LICENSE("GPL v2");
>
>

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

[PATCH v11 3/5] dt-bindings: drm/bridge: Document sn65dsi86 bridge bindings

[Sandeep Panda]

Document the bindings used for the sn65dsi86 DSI to eDP bridge.

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

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

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

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

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

Changes in v6:
 - Add property to mention the lane mapping scheme and polarity inversion
   (Stephen Boyd).

Changes in v7:
 - Detail description of lane mapping scheme dt property (Andrzej
   Hajda/ Rob Herring).
 - Removed HDP gpio binding, since the bridge uses IRQ signal to
   determine HPD, and IRQ property is already documented in binding.

Changes in v8:
 - Removed unnecessary explanation of lane mapping and polarity dt
   property, since these are already explained in media/video-interface
   dt binidng (Rob Herring).

Changes in v9:
 - Avoid putting re-definition of lane mapping and polarity dt binding
   (Rob Herring).

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

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

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH v11 3/5] dt-bindings: drm/bridge: Document sn65dsi86 bridge bindings

[Andrzej Hajda]

On 15.06.2018 08:43, Sandeep Panda wrote:
> Document the bindings used for the sn65dsi86 DSI to eDP bridge.
>
> Changes in v1:
>  - Rephrase the dt-binding descriptions to be more inline with existing
>    bindings (Andrzej Hajda).
>  - Add missing dt-binding that are parsed by corresponding driver
>    (Andrzej Hajda).
>
> Changes in v2:
>  - Remove edp panel specific dt-binding entries. Only keep bridge
>    specific entries (Sean Paul).
>  - Remove custom-modes dt entry since its usage is removed from driver also (Sean Paul).
>  - Remove is-pluggable dt entry since this will not be needed anymore (Sean Paul).
>
> Changes in v3:
>  - Remove irq-gpio dt entry and instead populate is an interrupt
>    property (Rob Herring).
>
> Changes in v4:
>  - Add link to bridge chip datasheet (Stephen Boyd)
>  - Add vpll and vcc regulator supply bindings (Stephen Boyd)
>  - Add ref clk optional dt binding (Stephen Boyd)
>  - Add gpio-controller optional dt binding (Stephen Boyd)
>
> Changes in v5:
>  - Use clock property to specify the input refclk (Stephen Boyd).
>  - Update gpio cell and pwm cell numbers (Stephen Boyd).
>
> Changes in v6:
>  - Add property to mention the lane mapping scheme and polarity inversion
>    (Stephen Boyd).
>
> Changes in v7:
>  - Detail description of lane mapping scheme dt property (Andrzej
>    Hajda/ Rob Herring).
>  - Removed HDP gpio binding, since the bridge uses IRQ signal to
>    determine HPD, and IRQ property is already documented in binding.
>
> Changes in v8:
>  - Removed unnecessary explanation of lane mapping and polarity dt
>    property, since these are already explained in media/video-interface
>    dt binidng (Rob Herring).
>
> Changes in v9:
>  - Avoid putting re-definition of lane mapping and polarity dt binding
>    (Rob Herring).
>
> Signed-off-by: Sandeep Panda <spanda@codeaurora.org>
> ---
>  .../bindings/display/bridge/ti,sn65dsi86.txt       | 87 ++++++++++++++++++++++
>  1 file changed, 87 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt
>
> diff --git a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt
> new file mode 100644
> index 0000000..507efbb
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt
> @@ -0,0 +1,87 @@
> +SN65DSI86 DSI to eDP bridge chip
> +--------------------------------
> +
> +This is the binding for Texas Instruments SN65DSI86 bridge.
> +http://www.ti.com/general/docs/lit/getliterature.tsp?genericPartNumber=sn65dsi86&fileType=pdf
> +
> +Required properties:
> +- compatible: Must be "ti,sn65dsi86"
> +- reg: i2c address of the chip, 0x2d as per datasheet
> +- enable-gpios: OF device-tree gpio specification for bridge_en pin (active high)
> +
> +- vccio-supply: A 1.8V supply that powers up the digital IOs.
> +- vpll-supply: A 1.8V supply that powers up the displayport PLL.
> +- vcca-supply: A 1.2V supply that powers up the analog circuits.
> +- vcc-supply: A 1.2V supply that powers up the digital core.

Since there are only two voltage levels (1.8 and 1.2) and power on/off
sequence does not require specific order I guess you could merge
supplies with the same level, but this is just an idea, up to you.
For example you can drop (or make optional) vpll-supply and vcca-supply.
> +
> +Optional properties:
> +- interrupts: Specifier for the SN65DSI86 interrupt line.

or interrupts-extended

> +
> +- ddc-i2c-bus: phandle of the I2C controller used for DDC EDID probing
> +
> +- gpio-controller: Marks the device has a GPIO controller.
> +- #gpio-cells    : Should be two. The first cell is the pin number and
> +                   the second cell is used to specify flags.
> +                   See ../../gpio/gpio.txt for more information.
> +- #pwm-cells : Should be one. See ../../pwm/pwm.txt for description of
> +               the cell formats.
> +
> +- clock-names: should be "refclk"
> +- clocks: Specification for input reference clock. The reference
> +	  clock rate must be 12 MHz, 19.2 MHz, 26 MHz, 27 MHz or 38.4 MHz.
> +
> +- data-lanes: See ../../media/video-interface.txt
> +- lane-polarities: See ../../media/video-interface.txt

Either you should describe which port these properties are related to,
either put them into endpoint node. According to
./../media/video-interface.txt only the latter is correct.

> +
> +Required nodes:
> +This device has two video ports. Their connections are modelled using the
> +OF graph bindings specified in Documentation/devicetree/bindings/graph.txt.
> +
> +- Video port 0 for DSI input
> +- Video port 1 for eDP output
> +
> +Example
> +-------
> +
> +edp-bridge@2d {
> +	compatible = "ti,sn65dsi86";
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +	reg = <0x2d>;
> +
> +	enable-gpios = <&msmgpio 33 GPIO_ACTIVE_HIGH>;
> +	interrupt-parent = <&gpio3>;
> +	interrupts = <4 IRQ_TYPE_EDGE_FALLING>;

In case of non-default interrupt parent interrupts-extended is more
compact/convenient - you can drop interrupt-parent property:
    interrupts-extended =  <&gpio3 4 IRQ_TYPE_EDGE_FALLING>;

> +
> +	vccio-supply = <&pm8916_l17>;
> +	vcca-supply = <&pm8916_l6>;
> +	vpll-supply = <&pm8916_l17>;
> +	vcc-supply = <&pm8916_l6>;
> +
> +	clock-names = "refclk";
> +	clocks = <&input_refclk>;
> +
> +	data-lanes = <2 1 3 0>;
> +	lane-polarities = <0 1 0 1>;

And this should be fixed as well.

> +
> +	ports {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		port@0 {
> +			reg = <0>;
> +
> +			edp_bridge_in: endpoint {
> +				remote-endpoint = <&dsi_out>;
> +			};
> +		};
> +
> +		port@1 {
> +			reg = <1>;
> +
> +			edp_bridge_out: endpoint {
> +				remote-endpoint = <&edp_panel_in>;
> +			};
> +		};
> +	};
> +}


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

Re: [PATCH v11 3/5] dt-bindings: drm/bridge: Document sn65dsi86 bridge bindings

[spanda-sgV2jX0FEOL9JmXXK+q4OQ]

On 2018-06-15 12:53, Andrzej Hajda wrote:
> On 15.06.2018 08:43, Sandeep Panda wrote:
>> Document the bindings used for the sn65dsi86 DSI to eDP bridge.
>> 
>> Changes in v1:
>>  - Rephrase the dt-binding descriptions to be more inline with 
>> existing
>>    bindings (Andrzej Hajda).
>>  - Add missing dt-binding that are parsed by corresponding driver
>>    (Andrzej Hajda).
>> 
>> Changes in v2:
>>  - Remove edp panel specific dt-binding entries. Only keep bridge
>>    specific entries (Sean Paul).
>>  - Remove custom-modes dt entry since its usage is removed from driver 
>> also (Sean Paul).
>>  - Remove is-pluggable dt entry since this will not be needed anymore 
>> (Sean Paul).
>> 
>> Changes in v3:
>>  - Remove irq-gpio dt entry and instead populate is an interrupt
>>    property (Rob Herring).
>> 
>> Changes in v4:
>>  - Add link to bridge chip datasheet (Stephen Boyd)
>>  - Add vpll and vcc regulator supply bindings (Stephen Boyd)
>>  - Add ref clk optional dt binding (Stephen Boyd)
>>  - Add gpio-controller optional dt binding (Stephen Boyd)
>> 
>> Changes in v5:
>>  - Use clock property to specify the input refclk (Stephen Boyd).
>>  - Update gpio cell and pwm cell numbers (Stephen Boyd).
>> 
>> Changes in v6:
>>  - Add property to mention the lane mapping scheme and polarity 
>> inversion
>>    (Stephen Boyd).
>> 
>> Changes in v7:
>>  - Detail description of lane mapping scheme dt property (Andrzej
>>    Hajda/ Rob Herring).
>>  - Removed HDP gpio binding, since the bridge uses IRQ signal to
>>    determine HPD, and IRQ property is already documented in binding.
>> 
>> Changes in v8:
>>  - Removed unnecessary explanation of lane mapping and polarity dt
>>    property, since these are already explained in 
>> media/video-interface
>>    dt binidng (Rob Herring).
>> 
>> Changes in v9:
>>  - Avoid putting re-definition of lane mapping and polarity dt binding
>>    (Rob Herring).
>> 
>> Signed-off-by: Sandeep Panda <spanda@codeaurora.org>
>> ---
>>  .../bindings/display/bridge/ti,sn65dsi86.txt       | 87 
>> ++++++++++++++++++++++
>>  1 file changed, 87 insertions(+)
>>  create mode 100644 
>> Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt
>> 
>> diff --git 
>> a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt 
>> b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt
>> new file mode 100644
>> index 0000000..507efbb
>> --- /dev/null
>> +++ 
>> b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt
>> @@ -0,0 +1,87 @@
>> +SN65DSI86 DSI to eDP bridge chip
>> +--------------------------------
>> +
>> +This is the binding for Texas Instruments SN65DSI86 bridge.
>> +http://www.ti.com/general/docs/lit/getliterature.tsp?genericPartNumber=sn65dsi86&fileType=pdf
>> +
>> +Required properties:
>> +- compatible: Must be "ti,sn65dsi86"
>> +- reg: i2c address of the chip, 0x2d as per datasheet
>> +- enable-gpios: OF device-tree gpio specification for bridge_en pin 
>> (active high)
>> +
>> +- vccio-supply: A 1.8V supply that powers up the digital IOs.
>> +- vpll-supply: A 1.8V supply that powers up the displayport PLL.
>> +- vcca-supply: A 1.2V supply that powers up the analog circuits.
>> +- vcc-supply: A 1.2V supply that powers up the digital core.
> 
> Since there are only two voltage levels (1.8 and 1.2) and power on/off
> sequence does not require specific order I guess you could merge
> supplies with the same level, but this is just an idea, up to you.
> For example you can drop (or make optional) vpll-supply and 
> vcca-supply.
>> +

Since the bridge datasheet mentions about 4 different power supplies and 
there
was also comment from other reviewers to keep all of them so i have put 
it like this.
Also this will help in scenarios where board design has 4 different 
sources for these 4
power supplies.

>> +Optional properties:
>> +- interrupts: Specifier for the SN65DSI86 interrupt line.
> 
> or interrupts-extended
> 
>> +
>> +- ddc-i2c-bus: phandle of the I2C controller used for DDC EDID 
>> probing
>> +
>> +- gpio-controller: Marks the device has a GPIO controller.
>> +- #gpio-cells    : Should be two. The first cell is the pin number 
>> and
>> +                   the second cell is used to specify flags.
>> +                   See ../../gpio/gpio.txt for more information.
>> +- #pwm-cells : Should be one. See ../../pwm/pwm.txt for description 
>> of
>> +               the cell formats.
>> +
>> +- clock-names: should be "refclk"
>> +- clocks: Specification for input reference clock. The reference
>> +	  clock rate must be 12 MHz, 19.2 MHz, 26 MHz, 27 MHz or 38.4 MHz.
>> +
>> +- data-lanes: See ../../media/video-interface.txt
>> +- lane-polarities: See ../../media/video-interface.txt
> 
> Either you should describe which port these properties are related to,
> either put them into endpoint node. According to
> ./../media/video-interface.txt only the latter is correct.
> 

Data lane and lane-polarity is applicable to both DSI and eDP interface. 
I have
updated the same in ../../media/video-interface.txt. Do you want to 
explicitly
mention here that this property is for eDP data lanes mapping?

>> +
>> +Required nodes:
>> +This device has two video ports. Their connections are modelled using 
>> the
>> +OF graph bindings specified in 
>> Documentation/devicetree/bindings/graph.txt.
>> +
>> +- Video port 0 for DSI input
>> +- Video port 1 for eDP output
>> +
>> +Example
>> +-------
>> +
>> +edp-bridge@2d {
>> +	compatible = "ti,sn65dsi86";
>> +	#address-cells = <1>;
>> +	#size-cells = <0>;
>> +	reg = <0x2d>;
>> +
>> +	enable-gpios = <&msmgpio 33 GPIO_ACTIVE_HIGH>;
>> +	interrupt-parent = <&gpio3>;
>> +	interrupts = <4 IRQ_TYPE_EDGE_FALLING>;
> 
> In case of non-default interrupt parent interrupts-extended is more
> compact/convenient - you can drop interrupt-parent property:
>     interrupts-extended =  <&gpio3 4 IRQ_TYPE_EDGE_FALLING>;

Ok. will update in next patch.
> 
>> +
>> +	vccio-supply = <&pm8916_l17>;
>> +	vcca-supply = <&pm8916_l6>;
>> +	vpll-supply = <&pm8916_l17>;
>> +	vcc-supply = <&pm8916_l6>;
>> +
>> +	clock-names = "refclk";
>> +	clocks = <&input_refclk>;
>> +
>> +	data-lanes = <2 1 3 0>;
>> +	lane-polarities = <0 1 0 1>;
> 
> And this should be fixed as well.
Ok i will move these to edp out endpoint node.
> 
>> +
>> +	ports {
>> +		#address-cells = <1>;
>> +		#size-cells = <0>;
>> +
>> +		port@0 {
>> +			reg = <0>;
>> +
>> + 			edp_bridge_in: endpoint {
>> +				remote-endpoint = <&dsi_out>;
>> +			};
>> +		};
>> +
>> +		port@1 {
>> +			reg = <1>;
>> +
>> +			edp_bridge_out: endpoint {
>> +				remote-endpoint = <&edp_panel_in>;
>> +			};
>> +		};
>> +	};
>> +}
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH v11 3/5] dt-bindings: drm/bridge: Document sn65dsi86 bridge bindings

[Rob Herring]

On Fri, Jun 15, 2018 at 12:13:41PM +0530, Sandeep Panda wrote:
> Document the bindings used for the sn65dsi86 DSI to eDP bridge.
> 
> Changes in v1:
>  - Rephrase the dt-binding descriptions to be more inline with existing
>    bindings (Andrzej Hajda).
>  - Add missing dt-binding that are parsed by corresponding driver
>    (Andrzej Hajda).
> 
> Changes in v2:
>  - Remove edp panel specific dt-binding entries. Only keep bridge
>    specific entries (Sean Paul).
>  - Remove custom-modes dt entry since its usage is removed from driver also (Sean Paul).
>  - Remove is-pluggable dt entry since this will not be needed anymore (Sean Paul).
> 
> Changes in v3:
>  - Remove irq-gpio dt entry and instead populate is an interrupt
>    property (Rob Herring).
> 
> Changes in v4:
>  - Add link to bridge chip datasheet (Stephen Boyd)
>  - Add vpll and vcc regulator supply bindings (Stephen Boyd)
>  - Add ref clk optional dt binding (Stephen Boyd)
>  - Add gpio-controller optional dt binding (Stephen Boyd)
> 
> Changes in v5:
>  - Use clock property to specify the input refclk (Stephen Boyd).
>  - Update gpio cell and pwm cell numbers (Stephen Boyd).
> 
> Changes in v6:
>  - Add property to mention the lane mapping scheme and polarity inversion
>    (Stephen Boyd).
> 
> Changes in v7:
>  - Detail description of lane mapping scheme dt property (Andrzej
>    Hajda/ Rob Herring).
>  - Removed HDP gpio binding, since the bridge uses IRQ signal to
>    determine HPD, and IRQ property is already documented in binding.
> 
> Changes in v8:
>  - Removed unnecessary explanation of lane mapping and polarity dt
>    property, since these are already explained in media/video-interface
>    dt binidng (Rob Herring).
> 
> Changes in v9:
>  - Avoid putting re-definition of lane mapping and polarity dt binding
>    (Rob Herring).
> 
> Signed-off-by: Sandeep Panda <spanda@codeaurora.org>
> ---
>  .../bindings/display/bridge/ti,sn65dsi86.txt       | 87 ++++++++++++++++++++++
>  1 file changed, 87 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt

Reviewed-by: Rob Herring <robh@kernel.org>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v11 4/5] drm/panel: add Innolux TV123WAM panel driver support

[Sandeep Panda]

Add support for Innolux TV123WAM, which is a 12.3" eDP
display panel with 2160x1440 resolution.

Changes in v1:
 - Add the compatibility string, display_mode and panel_desc
   structures in alphabetical order (Sean Paul).

Signed-off-by: Sandeep Panda <spanda@codeaurora.org>
Reviewed-by: Sean Paul <seanpaul@chromium.org>
---
 drivers/gpu/drm/panel/panel-simple.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
index 234af81..8c72270 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -1190,6 +1190,30 @@ static void panel_simple_shutdown(struct device *dev)
 	},
 };
 
+static const struct drm_display_mode innolux_tv123wam_mode = {
+	.clock = 206016,
+	.hdisplay = 2160,
+	.hsync_start = 2160 + 48,
+	.hsync_end = 2160 + 48 + 32,
+	.htotal = 2160 + 48 + 32 + 80,
+	.vdisplay = 1440,
+	.vsync_start = 1440 + 3,
+	.vsync_end = 1440 + 3 + 10,
+	.vtotal = 1440 + 3 + 10 + 27,
+	.vrefresh = 60,
+	.flags = DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC,
+};
+
+static const struct panel_desc innolux_tv123wam = {
+	.modes = &innolux_tv123wam_mode,
+	.num_modes = 1,
+	.bpc = 8,
+	.size = {
+		.width = 259,
+		.height = 173,
+	},
+};
+
 static const struct drm_display_mode innolux_zj070na_01p_mode = {
 	.clock = 51501,
 	.hdisplay = 1024,
@@ -2037,6 +2061,9 @@ static void panel_simple_shutdown(struct device *dev)
 		.compatible = "innolux,n156bge-l21",
 		.data = &innolux_n156bge_l21,
 	}, {
+		.compatible = "innolux,tv123wam",
+		.data = &innolux_tv123wam,
+	}, {
 		.compatible = "innolux,zj070na-01p",
 		.data = &innolux_zj070na_01p,
 	}, {
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[PATCH v11 5/5] dt-bindings: drm/panel: Document Innolux TV123WAM panel bindings

[Sandeep Panda]

Innolux TV123WAM is a 12.3" eDP display panel with
2160x1440 resolution, which can be supported by simple
panel driver.

Changes in v1:
 - Make use of simple panel driver instead of creating
   a new driver for this panel (Sean Paul).
 - Combine dt-binding and driver changes into one patch
   as done by other existing panel support changes.

Changes in v2:
 - Separate driver change from dt-binding documentation (Rob Herring).
 - Add the properties from simple-panel binding that are applicable to
   this panel (Rob Herring).

Signed-off-by: Sandeep Panda <spanda@codeaurora.org>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 .../bindings/display/panel/innolux,tv123wam.txt      | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/panel/innolux,tv123wam.txt

diff --git a/Documentation/devicetree/bindings/display/panel/innolux,tv123wam.txt b/Documentation/devicetree/bindings/display/panel/innolux,tv123wam.txt
new file mode 100644
index 0000000..a9b3526
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/innolux,tv123wam.txt
@@ -0,0 +1,20 @@
+Innolux TV123WAM 12.3 inch eDP 2K display panel
+
+This binding is compatible with the simple-panel binding, which is specified
+in simple-panel.txt in this directory.
+
+Required properties:
+- compatible: should be "innolux,tv123wam"
+- power-supply: regulator to provide the supply voltage
+
+Optional properties:
+- enable-gpios: GPIO pin to enable or disable the panel
+- backlight: phandle of the backlight device attached to the panel
+
+Example:
+	panel_edp: panel-edp {
+		compatible = "innolux,tv123wam";
+		enable-gpios = <&msmgpio 31 GPIO_ACTIVE_LOW>;
+		power-supply = <&pm8916_l2>;
+		backlight = <&backlight>;
+	};
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno