Re: [PATCH v8 1/4] drm/bridge: add support for sn65dsi86 bridge driver

spanda@xxxxxxxxxxxxxx · Wed, 13 Jun 2018 16:35:23 +0530

On 2018-06-06 04:29, Sean Paul wrote:
On Tue, Jun 05, 2018 at 11:10:15AM +0530, Sandeep Panda wrote:
Add support for TI's sn65dsi86 dsi2edp bridge chip.
The chip converts DSI transmitted signal to eDP signal,
which is fed to the connected eDP panel.

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

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

Changes in v2:
 - Separate out edp panel specific HW resource handling from bridge
   driver and create a separate edp panel drivers to handle panel
   specific mode information and HW resources (Sean Paul).
 - Replace pr_* APIs to DRM_* APIs to log error or debug information
   (Sean Paul).
 - Remove some of the unnecessary structure/variable from driver (Sean
   Paul).
 - Rename the function and structure prefix "sn65dsi86" to 
"ti_sn_bridge"
   (Sean Paul / Rob Herring).
 - Remove most of the hard-coding and modified the bridge init 
sequence
   based on current mode (Sean Paul).
 - Remove the existing function to retrieve the EDID data and
   implemented this as an i2c_adapter and use drm_get_edid() (Sean 
Paul).
 - Remove the dummy irq handler implementation, will add back the
   proper irq handling later (Sean Paul).
 - Capture the required enable gpios in a single array based on dt 
entry
   instead of having individual descriptor for each gpio (Sean Paul).

Changes in v3:
 - Remove usage of irq_gpio and replace it as "interrupts" property 
(Rob
   Herring).
 - Remove the unnecessary header file inclusions (Sean Paul).
 - Rearrange the header files in alphabetical order (Sean Paul).
 - Use regmap interface to perform i2c transactions.
 - Update Copyright/License field and address other review comments
   (Jordan Crouse).

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

Signed-off-by: Sandeep Panda <spanda@xxxxxxxxxxxxxx>
---
 drivers/gpu/drm/bridge/Kconfig        |   9 +
 drivers/gpu/drm/bridge/Makefile       |   1 +
 drivers/gpu/drm/bridge/ti-sn65dsi86.c | 666 
++++++++++++++++++++++++++++++++++
 3 files changed, 676 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..3711be8 100644
--- a/drivers/gpu/drm/bridge/Makefile
+++ b/drivers/gpu/drm/bridge/Makefile
@@ -12,4 +12,5 @@ obj-$(CONFIG_DRM_TOSHIBA_TC358767) += tc358767.o
 obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix/
 obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511/
 obj-$(CONFIG_DRM_TI_TFP410) += ti-tfp410.o
+obj-$(CONFIG_DRM_TI_SN65DSI86) += ti-sn65dsi86.o

SN65DSI86 should be in front of TFP410 here and above.


Ok.

 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..add6e0f
--- /dev/null
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c

/snip

+
+#define REFCLK_LUT_SIZE	5
+
+/* 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(struct ti_sn_bridge *pdata)
+{
+	int i = 0;
+	u8 refclk_src;
+	u32 refclk_rate;
+	const u32 *refclk_lut;
+
+	if (pdata->refclk) {
+		refclk_src = DPPLL_CLK_SRC_REFCLK;
+		refclk_rate = clk_get_rate(pdata->refclk);
+		refclk_lut = ti_sn_bridge_refclk_lut;
+		clk_prepare_enable(pdata->refclk);
+	} else {
+		refclk_src = DPPLL_CLK_SRC_DSICLK;
+		refclk_rate = ti_sn_bridge_get_dsi_freq(pdata) * 1000;
+		refclk_lut = ti_sn_bridge_dsiclk_lut;
+	}
+
+	/* for i equals to REFCLK_LUT_SIZE means default frequency */
+	for (i = 0; i < REFCLK_LUT_SIZE; i++)

Instead of REFCLK_LUT_SIZE, use a local variable, ie:

        size_t refclk_lut_size;

        ...

        if(pdata->refclk) {
                ...
                refclk_lut = ti_sn_bridge_refclk_lut;
                refclk_lut_size = ARRAY_SIZE(ti_sn_bridge_refclk_lut);
                ...
        } else {
                ...
                refclk_lut = ti_sn_bridge_dsiclk_lut;
                refclk_lut_size = ARRAY_SIZE(ti_sn_bridge_dsiclk_lut);
        }

And remove REFCLK_LUT_SIZE.

Ok.


+		if (refclk_lut[i] == refclk_rate)
+			break;
+
+	regmap_write(pdata->regmap, SN_REFCLK_FREQ_REG,
+		     (refclk_src | (i << SN_DSIA_REFCLK_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 = 0, i = 0;
+	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;
+
+	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);
+	ti_sn_bridge_write_u16(pdata, SN_CHA_HSYNC_PULSE_WIDTH_LOW_REG,
+			       mode->hsync_end - mode->hsync_start);
+	ti_sn_bridge_write_u16(pdata, SN_CHA_VSYNC_PULSE_WIDTH_LOW_REG,
+			       mode->vsync_end - mode->vsync_start);
+
+	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 = 0;
+
+	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, !0);

What is !0?

There was a comment to make it like this, but i think SN_HPD_DISABLE_BIT 
makes it more readable. I will change this as it was previously.

Sean

/snip
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html






