Re: [PATCH v4 1/3] drm/bridge/synopsys: Add MIPI DSI2 host controller bridge

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On Tue, Dec 10, 2024 at 12:10:19AM +0100, Heiko Stuebner wrote:
> From: Heiko Stuebner <heiko.stuebner@xxxxxxxxx>
> 
> Add a Synopsys Designware MIPI DSI host DRM bridge driver for their
> DSI2 host controller, based on the Rockchip version from the driver
> rockchip/dw-mipi-dsi2.c in their vendor-kernel with phy & bridge APIs.
> 
> While the driver is heavily modelled after the previous IP, the register
> set of this DSI2 controller is completely different and there are also
> additional properties like the variable-width phy interface.
> 
> Tested-by: Daniel Semkowicz <dse@xxxxxxxxxxxxx>
> Tested-by: Dmitry Yashin <dmt.yashin@xxxxxxxxx>
> Reviewed-by: Neil Armstrong <neil.armstrong@xxxxxxxxxx>
> Signed-off-by: Heiko Stuebner <heiko.stuebner@xxxxxxxxx>
[..]
> +static void dw_mipi_dsi2_set_data_stream_mode(struct dw_mipi_dsi2 *dsi2)
> +{
> +	u32 mode;
> +	int ret;
> +
> +	regmap_write(dsi2->regmap, DSI2_MODE_CTRL, DATA_STREAM_MODE);
> +	ret = regmap_read_poll_timeout(dsi2->regmap, DSI2_MODE_STATUS,
> +				       mode, mode & DATA_STREAM_MODE,
> +				       1000, MODE_STATUS_TIMEOUT_US);
> +	if (ret < 0)
> +		dev_err(dsi2->dev, "failed to enter data stream mode\n");
> +}
> +
> +static void dw_mipi_dsi2_set_cmd_mode(struct dw_mipi_dsi2 *dsi2)
> +{
> +	u32 mode;
> +	int ret;
> +
> +	regmap_write(dsi2->regmap, DSI2_MODE_CTRL, COMMAND_MODE);
> +	ret = regmap_read_poll_timeout(dsi2->regmap, DSI2_MODE_STATUS,
> +				       mode, mode & COMMAND_MODE,
> +				       1000, MODE_STATUS_TIMEOUT_US);
> +	if (ret < 0)
> +		dev_err(dsi2->dev, "failed to enter data stream mode\n");

We've failed to enter command mode, not stream mode. Wrong error message caused
by copy and paste?

> +}
> +
> +static void dw_mipi_dsi2_host_softrst(struct dw_mipi_dsi2 *dsi2)
> +{

Looks like it is necessary to also do apb reset here, like vendor kernel does.
Without apb reset, panel sometimes does not properly come up after DPMS switch.

Tested with

modetest -M rockchip -w 83:DPMS:3

I'm not sure whether it is Debian specific, but setting connector's DPMS property
to OFF causes immediate panel reset. Something is restoring its value to ON
immediately.

> +	regmap_write(dsi2->regmap, DSI2_SOFT_RESET, 0x0);
> +	usleep_range(50, 100);
> +	regmap_write(dsi2->regmap, DSI2_SOFT_RESET,
> +		   SYS_RSTN | PHY_RSTN | IPI_RSTN);
> +}

...

> +static void dw_mipi_dsi2_phy_ratio_cfg(struct dw_mipi_dsi2 *dsi2)
> +{
> +	struct drm_display_mode *mode = &dsi2->mode;
> +	u64 sys_clk = clk_get_rate(dsi2->sys_clk);
> +	u64 pixel_clk, ipi_clk, phy_hsclk;
> +	u64 tmp;
> +
> +	/*
> +	 * in DPHY mode, the phy_hstx_clk is exactly 1/16 the Lane high-speed
> +	 * data rate; In CPHY mode, the phy_hstx_clk is exactly 1/7 the trio
> +	 * high speed symbol rate.
> +	 */
> +	phy_hsclk = DIV_ROUND_CLOSEST_ULL(dsi2->lane_mbps * USEC_PER_SEC, 16);
> +
> +	/* IPI_RATIO_MAN_CFG = PHY_HSTX_CLK / IPI_CLK */
> +	pixel_clk = mode->crtc_clock * MSEC_PER_SEC;
> +	ipi_clk = pixel_clk / 4;
> +
> +	tmp = DIV_ROUND_CLOSEST_ULL(phy_hsclk << 16, ipi_clk);
> +	regmap_write(dsi2->regmap, DSI2_PHY_IPI_RATIO_MAN_CFG,
> +		   PHY_IPI_RATIO(tmp));
> +
> +	/*
> +	 * SYS_RATIO_MAN_CFG = MIPI_DCPHY_HSCLK_Freq / MIPI_DCPHY_HSCLK_Freq
> +	 */

According to TRM

SYS_RATIO_MAN_CFG = MIPI_DCPHY_HSCLK_Freq / SYS_CLK_Freq

So, this comment is simply wrong. Looks like a typo. Also, earlier comment does
not mention _Freq part in frequency names. Maybe it would be better to make it
consistent?

> +	tmp = DIV_ROUND_CLOSEST_ULL(phy_hsclk << 16, sys_clk);
> +	regmap_write(dsi2->regmap, DSI2_PHY_SYS_RATIO_MAN_CFG,
> +		   PHY_SYS_RATIO(tmp));
> +}
> +
> +static void dw_mipi_dsi2_lp2hs_or_hs2lp_cfg(struct dw_mipi_dsi2 *dsi2)
> +{
> +	const struct dw_mipi_dsi2_phy_ops *phy_ops = dsi2->plat_data->phy_ops;
> +	struct dw_mipi_dsi2_phy_timing timing;
> +	int ret;
> +
> +	ret = phy_ops->get_timing(dsi2->plat_data->priv_data,
> +				  dsi2->lane_mbps, &timing);
> +	if (ret)
> +		dev_err(dsi2->dev, "Retrieving phy timings failed\n");
> +
> +	regmap_write(dsi2->regmap, DSI2_PHY_LP2HS_MAN_CFG, PHY_LP2HS_TIME(timing.data_lp2hs));
> +	regmap_write(dsi2->regmap, DSI2_PHY_HS2LP_MAN_CFG, PHY_HS2LP_TIME(timing.data_hs2lp));

I also had to set DSI2_PHY_MAX_RD_T_MAN_CFG to 10000 as it is done in
dw-mipi-dsi (v1) driver to make my panel work properly in LPM mode.

> +}

...

> +static const struct regmap_config dw_mipi_dsi2_regmap_config = {
> +	.name = "dsi2-host",
> +	.reg_bits = 32,
> +	.val_bits = 32,
> +	.reg_stride = 4,
> +	.fast_io = true,

Maybe it would be good to also set max_register here to make proper regmap
available through debugfs? Or max_register_is_0 otherwise? Without this, only
first register is available for dumping.

Not quite relevant here, as this bridge is not currently used alone, but
relevant for Rockchip specific glue driver.

> +};

...


I've managed to successfully enable my DSI panel [0]. It has Lincoln Technology
Solutions ZV070WUV-L50 board [1], which, in turn, is based on Focal Tech FT7250
display controller.

This panel works flawlessly with rk3568 (but at most at ~55 Hz refresh rate,
because rk3568 does not support modes higher than 1920x1080@60, so, I had to
lower refresh rate to make it work). But has some troubles with rk3588. It
works fine with vendor kernel with LPM disabled and "auto-calculation-mode"
device tree property set. As far as I can understand, auto calculation mode is
not going to be supported by mainline, and disabling LPM is not an option to
me.

So, to make it work with this series I had to increase PHY_SYS_RATIO twice.

Maybe Andy could shade the light on what automatic mode calculation algorithm
does? Why in automatic mode PHY_SYS_RATIO value is twice larger than value
computed by formula in DSI controller application note?

Panel configuration:

static const struct drm_display_mode default_mode_lincoln = {
	.hdisplay    = 1200,
	.hsync_start = 1200 + 20,
	.hsync_end   = 1200 + 20 + 8,
	.htotal      = 1200 + 20 + 8 + 22,
	.vdisplay    = 1920,
	.vsync_start = 1920 + 178,
	.vsync_end   = 1920 + 178 + 8,
	.vtotal      = 1920 + 178 + 8 + 32,
	.clock       = 148500,
	.width_mm    = 94,
	.height_mm   = 151,
};

dsi->lanes = 4;
dsi->format = MIPI_DSI_FMT_RGB888;
dsi->mode_flags = MIPI_DSI_MODE_LPM | MIPI_DSI_MODE_VIDEO |
	MIPI_DSI_MODE_NO_EOT_PACKET | MIPI_DSI_MODE_VIDEO_BURST;

Regmap diff between manual mode and automatic mode:

diff --git a/root/regs_manual b/root/regs_auto
index 117cb5b..5dce301 100644
--- a/root/regs_manual
+++ b/root/regs_auto
@@ -7,9 +7,9 @@
 018: 00000000
 01c: 00000003
 020: 00000303
-024: 00000001
+024: 00000000
 028: 00000000
-02c: 0161082c
+02c: 04b70728
 030: 00000000
 034: 00000000
 038: 00000000
@@ -65,20 +65,20 @@
 100: 00000130
 104: 00000900
 108: 001f0000
-10c: 000cb0b5
-110: 00000000
-114: 000ae64f
-118: 00000000
+10c: 00000000
+110: 0013be38
+114: 00000000
+118: 00114671
 11c: 00000000
-120: 00000000
+120: 000003d3
 124: 00000000
-128: 00000000
+128: 00952167
 12c: 00000000
-130: 00000000
-134: 0001aaab
-138: 00000000
-13c: 00002d21
-140: 00000000
+130: 00658920
+134: 00000000
+138: 0001aaad
+13c: 00000000
+140: 00005a41
 144: 00000000
 148: 00000000
 14c: 00000000
@@ -129,7 +129,7 @@
 200: 00000002
 204: 00000000
 208: ffff0000
-20c: 00100072
+20c: 00000072
 210: 00000000
 214: 00000000
 218: 00000000
@@ -191,20 +191,20 @@
 2f8: 00000000
 2fc: 00000000
 300: 00000050
-304: 00035555
-308: 00000000
-30c: 00092aab
-310: 00000000
-314: 01f40000
-318: 00000000
-31c: 0208d555
-320: 00000000
-324: 00000008
-328: 00000000
-32c: 00000020
-330: 00000000
-334: 00000780
-338: 00000000
-33c: 000000b2
-340: 00000000
+304: 00000000
+308: 0003555a
+30c: 00000000
+310: 00092ab7
+314: 00000000
+318: 01f402bc
+31c: 00000000
+320: 0208d82e
+324: 00000000
+328: 00000008
+32c: 00000000
+330: 00000020
+334: 00000000
+338: 00000780
+33c: 00000000
+340: 000000b2
 344: 000004b0

The only irrelevant difference here is LPM mode enable bit.

Calculated hsclk is 61875000, sys clk is 351000000, MIPI lane speed is
990 mbps. VOP pixel clock is 37125000 if it is relevant in any way.

Thanks, Pavel

[0] https://www.hello-lighting.com/product/7-0-ips-1200x1920/
[1] http://www.microtech-lcd.cn/upfile/202109/2021092649634025.jpg




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux