[PATCH v3 0/3] rockchip-dsi for rk3568

[PATCH v3 0/3] rockchip-dsi for rk3568

[Chris Morgan]

From: Chris Morgan <macromorgan@hotmail.com>

This series adds support for the dsi and dphy controllers on the
Rockchip RK3568.

Tested on an Anbernic RG503, Anbernic RG353P, and Odroid Go Advance.

Changes since V2:
 - Removed dsi controller patches, as those have been merged upstream.
 - Removed notes about rolling back clock drivers. If I set the parent
   clock of the VOP port I'm using to VPLL and set the clock rate of
   PLL_VPLL to 500MHz this series works correctly for my panels without
   rolling anything back (per Heiko this is the correct way).
 - Added additional details about refactoring DPHY driver to add
   2.5GHz for rk356x. All other devices still have a max speed of 1GHz.
 - Notified Heiko that the BIT(5) for both PLL_POST_DIV_ENABLE and
   PLL_POST_DIV_ENABLE_MASK is deliberate, because of how the
   phy_update_bits() works.

Changes since RFCv1:
 - Identified cause of image shift (clock changes).
 - Noted that driver works now.
 - Added devicetree nodes for rk356x.dtsi.

Chris Morgan (3):
  dt-bindings: phy-rockchip-inno-dsidphy: add compatible  for rk3568
  phy/rockchip: inno-dsidphy: Add support for rk3568
  arm64: dts: rockchip: Add DSI and DSI-DPHY nodes to rk356x

 .../bindings/phy/rockchip,px30-dsi-dphy.yaml  |   1 +
 arch/arm64/boot/dts/rockchip/rk356x.dtsi      |  72 +++++++
 .../phy/rockchip/phy-rockchip-inno-dsidphy.c  | 204 ++++++++++++++----
 3 files changed, 231 insertions(+), 46 deletions(-)

-- 
2.25.1

[PATCH V3 1/3] dt-bindings: phy-rockchip-inno-dsidphy: add compatible for rk3568

[Chris Morgan]

From: Chris Morgan <macromorgan@hotmail.com>

Add a compatible string for the rk3568 dsi-dphy.

Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
Acked-by: Rob Herring <robh@kernel.org>
Reviewed-by: Heiko Stuebner <heiko@sntech.de>
---
 .../devicetree/bindings/phy/rockchip,px30-dsi-dphy.yaml          | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/phy/rockchip,px30-dsi-dphy.yaml b/Documentation/devicetree/bindings/phy/rockchip,px30-dsi-dphy.yaml
index 8a3032a3bd73..5c35e5ceec0b 100644
--- a/Documentation/devicetree/bindings/phy/rockchip,px30-dsi-dphy.yaml
+++ b/Documentation/devicetree/bindings/phy/rockchip,px30-dsi-dphy.yaml
@@ -18,6 +18,7 @@ properties:
       - rockchip,px30-dsi-dphy
       - rockchip,rk3128-dsi-dphy
       - rockchip,rk3368-dsi-dphy
+      - rockchip,rk3568-dsi-dphy
 
   reg:
     maxItems: 1
-- 
2.25.1

[PATCH V3 2/3] phy/rockchip: inno-dsidphy: Add support for rk3568

[Chris Morgan]

From: Chris Morgan <macromorgan@hotmail.com>

Add support for the Rockchip RK3568 DSI-DPHY. Registers were taken from
the BSP kernel driver and wherever possible cross referenced with the
TRM.

Refactor the code to allow the different compatible strings to set
either a max 1GHz timing table (all existing hardware) or a max 2.5GHz
timing table (the new RK356x). This works (for me) on both an RK3326
(PX30) and a new RK3566 device.

Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
---
 .../phy/rockchip/phy-rockchip-inno-dsidphy.c  | 204 ++++++++++++++----
 1 file changed, 158 insertions(+), 46 deletions(-)

diff --git a/drivers/phy/rockchip/phy-rockchip-inno-dsidphy.c b/drivers/phy/rockchip/phy-rockchip-inno-dsidphy.c
index 630e01b5c19b..2c5847faff63 100644
--- a/drivers/phy/rockchip/phy-rockchip-inno-dsidphy.c
+++ b/drivers/phy/rockchip/phy-rockchip-inno-dsidphy.c
@@ -84,9 +84,25 @@
 #define DATA_LANE_0_SKEW_PHASE_MASK		GENMASK(2, 0)
 #define DATA_LANE_0_SKEW_PHASE(x)		UPDATE(x, 2, 0)
 /* Analog Register Part: reg08 */
+#define PLL_POST_DIV_ENABLE_MASK		BIT(5)
+#define PLL_POST_DIV_ENABLE			BIT(5)
 #define SAMPLE_CLOCK_DIRECTION_MASK		BIT(4)
 #define SAMPLE_CLOCK_DIRECTION_REVERSE		BIT(4)
 #define SAMPLE_CLOCK_DIRECTION_FORWARD		0
+#define LOWFRE_EN_MASK				BIT(5)
+#define PLL_OUTPUT_FREQUENCY_DIV_BY_1		0
+#define PLL_OUTPUT_FREQUENCY_DIV_BY_2		1
+/* Analog Register Part: reg0b */
+#define CLOCK_LANE_VOD_RANGE_SET_MASK		GENMASK(3, 0)
+#define CLOCK_LANE_VOD_RANGE_SET(x)		UPDATE(x, 3, 0)
+#define VOD_MIN_RANGE				0x1
+#define VOD_MID_RANGE				0x3
+#define VOD_BIG_RANGE				0x7
+#define VOD_MAX_RANGE				0xf
+/* Analog Register Part: reg1E */
+#define PLL_MODE_SEL_MASK			GENMASK(6, 5)
+#define PLL_MODE_SEL_LVDS_MODE			0
+#define PLL_MODE_SEL_MIPI_MODE			BIT(5)
 /* Digital Register Part: reg00 */
 #define REG_DIG_RSTN_MASK			BIT(0)
 #define REG_DIG_RSTN_NORMAL			BIT(0)
@@ -102,20 +118,22 @@
 #define T_LPX_CNT_MASK				GENMASK(5, 0)
 #define T_LPX_CNT(x)				UPDATE(x, 5, 0)
 /* Clock/Data0/Data1/Data2/Data3 Lane Register Part: reg06 */
+#define T_HS_ZERO_CNT_HI_MASK			BIT(7)
+#define T_HS_ZERO_CNT_HI(x)			UPDATE(x, 7, 7)
 #define T_HS_PREPARE_CNT_MASK			GENMASK(6, 0)
 #define T_HS_PREPARE_CNT(x)			UPDATE(x, 6, 0)
 /* Clock/Data0/Data1/Data2/Data3 Lane Register Part: reg07 */
-#define T_HS_ZERO_CNT_MASK			GENMASK(5, 0)
-#define T_HS_ZERO_CNT(x)			UPDATE(x, 5, 0)
+#define T_HS_ZERO_CNT_LO_MASK			GENMASK(5, 0)
+#define T_HS_ZERO_CNT_LO(x)			UPDATE(x, 5, 0)
 /* Clock/Data0/Data1/Data2/Data3 Lane Register Part: reg08 */
 #define T_HS_TRAIL_CNT_MASK			GENMASK(6, 0)
 #define T_HS_TRAIL_CNT(x)			UPDATE(x, 6, 0)
 /* Clock/Data0/Data1/Data2/Data3 Lane Register Part: reg09 */
-#define T_HS_EXIT_CNT_MASK			GENMASK(4, 0)
-#define T_HS_EXIT_CNT(x)			UPDATE(x, 4, 0)
+#define T_HS_EXIT_CNT_LO_MASK			GENMASK(4, 0)
+#define T_HS_EXIT_CNT_LO(x)			UPDATE(x, 4, 0)
 /* Clock/Data0/Data1/Data2/Data3 Lane Register Part: reg0a */
-#define T_CLK_POST_CNT_MASK			GENMASK(3, 0)
-#define T_CLK_POST_CNT(x)			UPDATE(x, 3, 0)
+#define T_CLK_POST_CNT_LO_MASK			GENMASK(3, 0)
+#define T_CLK_POST_CNT_LO(x)			UPDATE(x, 3, 0)
 /* Clock/Data0/Data1/Data2/Data3 Lane Register Part: reg0c */
 #define LPDT_TX_PPI_SYNC_MASK			BIT(2)
 #define LPDT_TX_PPI_SYNC_ENABLE			BIT(2)
@@ -129,9 +147,13 @@
 #define T_CLK_PRE_CNT_MASK			GENMASK(3, 0)
 #define T_CLK_PRE_CNT(x)			UPDATE(x, 3, 0)
 /* Clock/Data0/Data1/Data2/Data3 Lane Register Part: reg10 */
+#define T_CLK_POST_CNT_HI_MASK			GENMASK(7, 6)
+#define T_CLK_POST_CNT_HI(x)			UPDATE(x, 7, 6)
 #define T_TA_GO_CNT_MASK			GENMASK(5, 0)
 #define T_TA_GO_CNT(x)				UPDATE(x, 5, 0)
 /* Clock/Data0/Data1/Data2/Data3 Lane Register Part: reg11 */
+#define T_HS_EXIT_CNT_HI_MASK			BIT(6)
+#define T_HS_EXIT_CNT_HI(x)			UPDATE(x, 6, 6)
 #define T_TA_SURE_CNT_MASK			GENMASK(5, 0)
 #define T_TA_SURE_CNT(x)			UPDATE(x, 5, 0)
 /* Clock/Data0/Data1/Data2/Data3 Lane Register Part: reg12 */
@@ -169,11 +191,23 @@
 #define DSI_PHY_STATUS		0xb0
 #define PHY_LOCK		BIT(0)
 
+enum phy_max_rate {
+	MAX_1GHZ,
+	MAX_2_5GHZ,
+};
+
+struct inno_video_phy_plat_data {
+	const struct inno_mipi_dphy_timing *inno_mipi_dphy_timing_table;
+	const unsigned int num_timings;
+	enum phy_max_rate max_rate;
+};
+
 struct inno_dsidphy {
 	struct device *dev;
 	struct clk *ref_clk;
 	struct clk *pclk_phy;
 	struct clk *pclk_host;
+	const struct inno_video_phy_plat_data *pdata;
 	void __iomem *phy_base;
 	void __iomem *host_base;
 	struct reset_control *rst;
@@ -200,6 +234,53 @@ enum {
 	REGISTER_PART_LVDS,
 };
 
+struct inno_mipi_dphy_timing {
+	unsigned long rate;
+	u8 lpx;
+	u8 hs_prepare;
+	u8 clk_lane_hs_zero;
+	u8 data_lane_hs_zero;
+	u8 hs_trail;
+};
+
+static const
+struct inno_mipi_dphy_timing inno_mipi_dphy_timing_table_max_1ghz[] = {
+	{ 110000000, 0x0, 0x20, 0x16, 0x02, 0x22},
+	{ 150000000, 0x0, 0x06, 0x16, 0x03, 0x45},
+	{ 200000000, 0x0, 0x18, 0x17, 0x04, 0x0b},
+	{ 250000000, 0x0, 0x05, 0x17, 0x05, 0x16},
+	{ 300000000, 0x0, 0x51, 0x18, 0x06, 0x2c},
+	{ 400000000, 0x0, 0x64, 0x19, 0x07, 0x33},
+	{ 500000000, 0x0, 0x20, 0x1b, 0x07, 0x4e},
+	{ 600000000, 0x0, 0x6a, 0x1d, 0x08, 0x3a},
+	{ 700000000, 0x0, 0x3e, 0x1e, 0x08, 0x6a},
+	{ 800000000, 0x0, 0x21, 0x1f, 0x09, 0x29},
+	{1000000000, 0x0, 0x09, 0x20, 0x09, 0x27},
+};
+
+static const
+struct inno_mipi_dphy_timing inno_mipi_dphy_timing_table_max_2_5ghz[] = {
+	{ 110000000, 0x02, 0x7f, 0x16, 0x02, 0x02},
+	{ 150000000, 0x02, 0x7f, 0x16, 0x03, 0x02},
+	{ 200000000, 0x02, 0x7f, 0x17, 0x04, 0x02},
+	{ 250000000, 0x02, 0x7f, 0x17, 0x05, 0x04},
+	{ 300000000, 0x02, 0x7f, 0x18, 0x06, 0x04},
+	{ 400000000, 0x03, 0x7e, 0x19, 0x07, 0x04},
+	{ 500000000, 0x03, 0x7c, 0x1b, 0x07, 0x08},
+	{ 600000000, 0x03, 0x70, 0x1d, 0x08, 0x10},
+	{ 700000000, 0x05, 0x40, 0x1e, 0x08, 0x30},
+	{ 800000000, 0x05, 0x02, 0x1f, 0x09, 0x30},
+	{1000000000, 0x05, 0x08, 0x20, 0x09, 0x30},
+	{1200000000, 0x06, 0x03, 0x32, 0x14, 0x0f},
+	{1400000000, 0x09, 0x03, 0x32, 0x14, 0x0f},
+	{1600000000, 0x0d, 0x42, 0x36, 0x0e, 0x0f},
+	{1800000000, 0x0e, 0x47, 0x7a, 0x0e, 0x0f},
+	{2000000000, 0x11, 0x64, 0x7a, 0x0e, 0x0b},
+	{2200000000, 0x13, 0x64, 0x7e, 0x15, 0x0b},
+	{2400000000, 0x13, 0x33, 0x7f, 0x15, 0x6a},
+	{2500000000, 0x15, 0x54, 0x7f, 0x15, 0x6a},
+};
+
 static inline struct inno_dsidphy *hw_to_inno(struct clk_hw *hw)
 {
 	return container_of(hw, struct inno_dsidphy, pll.hw);
@@ -290,31 +371,15 @@ static unsigned long inno_dsidphy_pll_calc_rate(struct inno_dsidphy *inno,
 static void inno_dsidphy_mipi_mode_enable(struct inno_dsidphy *inno)
 {
 	struct phy_configure_opts_mipi_dphy *cfg = &inno->dphy_cfg;
-	const struct {
-		unsigned long rate;
-		u8 hs_prepare;
-		u8 clk_lane_hs_zero;
-		u8 data_lane_hs_zero;
-		u8 hs_trail;
-	} timings[] = {
-		{ 110000000, 0x20, 0x16, 0x02, 0x22},
-		{ 150000000, 0x06, 0x16, 0x03, 0x45},
-		{ 200000000, 0x18, 0x17, 0x04, 0x0b},
-		{ 250000000, 0x05, 0x17, 0x05, 0x16},
-		{ 300000000, 0x51, 0x18, 0x06, 0x2c},
-		{ 400000000, 0x64, 0x19, 0x07, 0x33},
-		{ 500000000, 0x20, 0x1b, 0x07, 0x4e},
-		{ 600000000, 0x6a, 0x1d, 0x08, 0x3a},
-		{ 700000000, 0x3e, 0x1e, 0x08, 0x6a},
-		{ 800000000, 0x21, 0x1f, 0x09, 0x29},
-		{1000000000, 0x09, 0x20, 0x09, 0x27},
-	};
+	const struct inno_mipi_dphy_timing *timings;
 	u32 t_txbyteclkhs, t_txclkesc;
 	u32 txbyteclkhs, txclkesc, esc_clk_div;
 	u32 hs_exit, clk_post, clk_pre, wakeup, lpx, ta_go, ta_sure, ta_wait;
 	u32 hs_prepare, hs_trail, hs_zero, clk_lane_hs_zero, data_lane_hs_zero;
 	unsigned int i;
 
+	timings = inno->pdata->inno_mipi_dphy_timing_table;
+
 	inno_dsidphy_pll_calc_rate(inno, cfg->hs_clk_rate);
 
 	/* Select MIPI mode */
@@ -327,6 +392,13 @@ static void inno_dsidphy_mipi_mode_enable(struct inno_dsidphy *inno)
 			REG_FBDIV_HI_MASK, REG_FBDIV_HI(inno->pll.fbdiv));
 	phy_update_bits(inno, REGISTER_PART_ANALOG, 0x04,
 			REG_FBDIV_LO_MASK, REG_FBDIV_LO(inno->pll.fbdiv));
+	if (inno->pdata->max_rate == MAX_2_5GHZ) {
+		phy_update_bits(inno, REGISTER_PART_ANALOG, 0x08,
+				PLL_POST_DIV_ENABLE_MASK, PLL_POST_DIV_ENABLE);
+		phy_update_bits(inno, REGISTER_PART_ANALOG, 0x0b,
+				CLOCK_LANE_VOD_RANGE_SET_MASK,
+				CLOCK_LANE_VOD_RANGE_SET(VOD_MAX_RANGE));
+	}
 	/* Enable PLL and LDO */
 	phy_update_bits(inno, REGISTER_PART_ANALOG, 0x01,
 			REG_LDOPD_MASK | REG_PLLPD_MASK,
@@ -367,14 +439,6 @@ static void inno_dsidphy_mipi_mode_enable(struct inno_dsidphy *inno)
 	 */
 	clk_pre = DIV_ROUND_UP(cfg->clk_pre, BITS_PER_BYTE);
 
-	/*
-	 * The value of counter for HS Tlpx Time
-	 * Tlpx = Tpin_txbyteclkhs * (2 + value)
-	 */
-	lpx = DIV_ROUND_UP(cfg->lpx, t_txbyteclkhs);
-	if (lpx >= 2)
-		lpx -= 2;
-
 	/*
 	 * The value of counter for HS Tta-go
 	 * Tta-go for turnaround
@@ -394,13 +458,24 @@ static void inno_dsidphy_mipi_mode_enable(struct inno_dsidphy *inno)
 	 */
 	ta_wait = DIV_ROUND_UP(cfg->ta_get, t_txclkesc);
 
-	for (i = 0; i < ARRAY_SIZE(timings); i++)
+	for (i = 0; i < inno->pdata->num_timings; i++)
 		if (inno->pll.rate <= timings[i].rate)
 			break;
 
-	if (i == ARRAY_SIZE(timings))
+	if (i == inno->pdata->num_timings)
 		--i;
 
+	/*
+	 * The value of counter for HS Tlpx Time
+	 * Tlpx = Tpin_txbyteclkhs * (2 + value)
+	 */
+	if (inno->pdata->max_rate == MAX_1GHZ) {
+		lpx = DIV_ROUND_UP(cfg->lpx, t_txbyteclkhs);
+		if (lpx >= 2)
+			lpx -= 2;
+	} else
+		lpx = timings[i].lpx;
+
 	hs_prepare = timings[i].hs_prepare;
 	hs_trail = timings[i].hs_trail;
 	clk_lane_hs_zero = timings[i].clk_lane_hs_zero;
@@ -417,14 +492,23 @@ static void inno_dsidphy_mipi_mode_enable(struct inno_dsidphy *inno)
 				T_LPX_CNT(lpx));
 		phy_update_bits(inno, i, 0x06, T_HS_PREPARE_CNT_MASK,
 				T_HS_PREPARE_CNT(hs_prepare));
-		phy_update_bits(inno, i, 0x07, T_HS_ZERO_CNT_MASK,
-				T_HS_ZERO_CNT(hs_zero));
+		if (inno->pdata->max_rate == MAX_2_5GHZ)
+			phy_update_bits(inno, i, 0x06, T_HS_ZERO_CNT_HI_MASK,
+					T_HS_ZERO_CNT_HI(hs_zero >> 6));
+		phy_update_bits(inno, i, 0x07, T_HS_ZERO_CNT_LO_MASK,
+				T_HS_ZERO_CNT_LO(hs_zero));
 		phy_update_bits(inno, i, 0x08, T_HS_TRAIL_CNT_MASK,
 				T_HS_TRAIL_CNT(hs_trail));
-		phy_update_bits(inno, i, 0x09, T_HS_EXIT_CNT_MASK,
-				T_HS_EXIT_CNT(hs_exit));
-		phy_update_bits(inno, i, 0x0a, T_CLK_POST_CNT_MASK,
-				T_CLK_POST_CNT(clk_post));
+		if (inno->pdata->max_rate == MAX_2_5GHZ)
+			phy_update_bits(inno, i, 0x11, T_HS_EXIT_CNT_HI_MASK,
+					T_HS_EXIT_CNT_HI(hs_exit >> 5));
+		phy_update_bits(inno, i, 0x09, T_HS_EXIT_CNT_LO_MASK,
+				T_HS_EXIT_CNT_LO(hs_exit));
+		if (inno->pdata->max_rate == MAX_2_5GHZ)
+			phy_update_bits(inno, i, 0x10, T_CLK_POST_CNT_HI_MASK,
+					T_CLK_POST_CNT_HI(clk_post >> 4));
+		phy_update_bits(inno, i, 0x0a, T_CLK_POST_CNT_LO_MASK,
+				T_CLK_POST_CNT_LO(clk_post));
 		phy_update_bits(inno, i, 0x0e, T_CLK_PRE_CNT_MASK,
 				T_CLK_PRE_CNT(clk_pre));
 		phy_update_bits(inno, i, 0x0c, T_WAKEUP_CNT_HI_MASK,
@@ -452,8 +536,9 @@ static void inno_dsidphy_lvds_mode_enable(struct inno_dsidphy *inno)
 
 	/* Sample clock reverse direction */
 	phy_update_bits(inno, REGISTER_PART_ANALOG, 0x08,
-			SAMPLE_CLOCK_DIRECTION_MASK,
-			SAMPLE_CLOCK_DIRECTION_REVERSE);
+			SAMPLE_CLOCK_DIRECTION_MASK | LOWFRE_EN_MASK,
+			SAMPLE_CLOCK_DIRECTION_REVERSE |
+			PLL_OUTPUT_FREQUENCY_DIV_BY_1);
 
 	/* Select LVDS mode */
 	phy_update_bits(inno, REGISTER_PART_LVDS, 0x03,
@@ -473,6 +558,10 @@ static void inno_dsidphy_lvds_mode_enable(struct inno_dsidphy *inno)
 
 	msleep(20);
 
+	/* Select PLL mode */
+	phy_update_bits(inno, REGISTER_PART_ANALOG, 0x1e,
+			PLL_MODE_SEL_MASK, PLL_MODE_SEL_LVDS_MODE);
+
 	/* Reset LVDS digital logic */
 	phy_update_bits(inno, REGISTER_PART_LVDS, 0x00,
 			LVDS_DIGITAL_INTERNAL_RESET_MASK,
@@ -592,6 +681,18 @@ static const struct phy_ops inno_dsidphy_ops = {
 	.owner = THIS_MODULE,
 };
 
+static const struct inno_video_phy_plat_data max_1ghz_video_phy_plat_data = {
+	.inno_mipi_dphy_timing_table = inno_mipi_dphy_timing_table_max_1ghz,
+	.num_timings = ARRAY_SIZE(inno_mipi_dphy_timing_table_max_1ghz),
+	.max_rate = MAX_1GHZ,
+};
+
+static const struct inno_video_phy_plat_data max_2_5ghz_video_phy_plat_data = {
+	.inno_mipi_dphy_timing_table = inno_mipi_dphy_timing_table_max_2_5ghz,
+	.num_timings = ARRAY_SIZE(inno_mipi_dphy_timing_table_max_2_5ghz),
+	.max_rate = MAX_2_5GHZ,
+};
+
 static int inno_dsidphy_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -605,6 +706,7 @@ static int inno_dsidphy_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	inno->dev = dev;
+	inno->pdata = of_device_get_match_data(inno->dev);
 	platform_set_drvdata(pdev, inno);
 
 	inno->phy_base = devm_platform_ioremap_resource(pdev, 0);
@@ -663,9 +765,19 @@ static int inno_dsidphy_remove(struct platform_device *pdev)
 }
 
 static const struct of_device_id inno_dsidphy_of_match[] = {
-	{ .compatible = "rockchip,px30-dsi-dphy", },
-	{ .compatible = "rockchip,rk3128-dsi-dphy", },
-	{ .compatible = "rockchip,rk3368-dsi-dphy", },
+	{
+		.compatible = "rockchip,px30-dsi-dphy",
+		.data = &max_1ghz_video_phy_plat_data,
+	}, {
+		.compatible = "rockchip,rk3128-dsi-dphy",
+		.data = &max_1ghz_video_phy_plat_data,
+	}, {
+		.compatible = "rockchip,rk3368-dsi-dphy",
+		.data = &max_1ghz_video_phy_plat_data,
+	}, {
+		.compatible = "rockchip,rk3568-dsi-dphy",
+		.data = &max_2_5ghz_video_phy_plat_data,
+	},
 	{}
 };
 MODULE_DEVICE_TABLE(of, inno_dsidphy_of_match);
-- 
2.25.1

[PATCH V3 3/3] arm64: dts: rockchip: Add DSI and DSI-DPHY nodes to rk356x

[Chris Morgan]

From: Chris Morgan <macromorgan@hotmail.com>

This adds the DSI controller nodes and DSI-DPHY controller nodes to the
rk356x device tree.

Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
---
 arch/arm64/boot/dts/rockchip/rk356x.dtsi | 72 ++++++++++++++++++++++++
 1 file changed, 72 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk356x.dtsi b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
index 319981c3e9f7..d150568fde82 100644
--- a/arch/arm64/boot/dts/rockchip/rk356x.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
@@ -699,6 +699,54 @@ vop_mmu: iommu@fe043e00 {
 		status = "disabled";
 	};
 
+	dsi0: dsi@fe060000 {
+		compatible = "rockchip,rk3568-mipi-dsi", "snps,dw-mipi-dsi";
+		reg = <0x00 0xfe060000 0x00 0x10000>;
+		interrupts = <GIC_SPI 68 IRQ_TYPE_LEVEL_HIGH>;
+		clock-names = "pclk", "hclk";
+		clocks = <&cru PCLK_DSITX_0>, <&cru HCLK_VO>;
+		phy-names = "dphy";
+		phys = <&mipi_dphy0>;
+		power-domains = <&power RK3568_PD_VO>;
+		reset-names = "apb";
+		resets = <&cru SRST_P_DSITX_0>;
+		rockchip,grf = <&grf>;
+		status = "disabled";
+
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			port@0 {
+				reg = <0>;
+			};
+		};
+	};
+
+	dsi1: dsi@fe070000 {
+		compatible = "rockchip,rk3568-mipi-dsi", "snps,dw-mipi-dsi";
+		reg = <0x0 0xfe070000 0x0 0x10000>;
+		interrupts = <GIC_SPI 69 IRQ_TYPE_LEVEL_HIGH>;
+		clock-names = "pclk", "hclk";
+		clocks = <&cru PCLK_DSITX_1>, <&cru HCLK_VO>;
+		phy-names = "dphy";
+		phys = <&mipi_dphy1>;
+		power-domains = <&power RK3568_PD_VO>;
+		reset-names = "apb";
+		resets = <&cru SRST_P_DSITX_1>;
+		rockchip,grf = <&grf>;
+		status = "disabled";
+
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			port@0 {
+				reg = <0>;
+			};
+		};
+	};
+
 	hdmi: hdmi@fe0a0000 {
 		compatible = "rockchip,rk3568-dw-hdmi";
 		reg = <0x0 0xfe0a0000 0x0 0x20000>;
@@ -1594,6 +1642,30 @@ combphy2: phy@fe840000 {
 		status = "disabled";
 	};
 
+	mipi_dphy0: mipi-dphy@fe850000 {
+		compatible = "rockchip,rk3568-dsi-dphy";
+		reg = <0x0 0xfe850000 0x0 0x10000>;
+		clock-names = "ref", "pclk";
+		clocks = <&pmucru CLK_MIPIDSIPHY0_REF>, <&cru PCLK_MIPIDSIPHY0>;
+		#phy-cells = <0>;
+		power-domains = <&power RK3568_PD_VO>;
+		reset-names = "apb";
+		resets = <&cru SRST_P_MIPIDSIPHY0>;
+		status = "disabled";
+	};
+
+	mipi_dphy1: mipi-dphy@fe860000 {
+		compatible = "rockchip,rk3568-dsi-dphy";
+		reg = <0x0 0xfe860000 0x0 0x10000>;
+		clock-names = "ref", "pclk";
+		clocks = <&pmucru CLK_MIPIDSIPHY1_REF>, <&cru PCLK_MIPIDSIPHY1>;
+		#phy-cells = <0>;
+		power-domains = <&power RK3568_PD_VO>;
+		reset-names = "apb";
+		resets = <&cru SRST_P_MIPIDSIPHY1>;
+		status = "disabled";
+	};
+
 	usb2phy0: usb2phy@fe8a0000 {
 		compatible = "rockchip,rk3568-usb2phy";
 		reg = <0x0 0xfe8a0000 0x0 0x10000>;
-- 
2.25.1

Re: [PATCH V3 3/3] arm64: dts: rockchip: Add DSI and DSI-DPHY nodes to rk356x

[Michael Riesch]

Hi Chris,

Thanks for your efforts, nice work!

A few minor comments below:

On 9/12/22 22:56, Chris Morgan wrote:
> From: Chris Morgan <macromorgan@hotmail.com>
> 
> This adds the DSI controller nodes and DSI-DPHY controller nodes to the
> rk356x device tree.
> 
> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> ---
>  arch/arm64/boot/dts/rockchip/rk356x.dtsi | 72 ++++++++++++++++++++++++
>  1 file changed, 72 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk356x.dtsi b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
> index 319981c3e9f7..d150568fde82 100644
> --- a/arch/arm64/boot/dts/rockchip/rk356x.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
> @@ -699,6 +699,54 @@ vop_mmu: iommu@fe043e00 {
>  		status = "disabled";
>  	};
>  
> +	dsi0: dsi@fe060000 {
> +		compatible = "rockchip,rk3568-mipi-dsi", "snps,dw-mipi-dsi";
> +		reg = <0x00 0xfe060000 0x00 0x10000>;
> +		interrupts = <GIC_SPI 68 IRQ_TYPE_LEVEL_HIGH>;
> +		clock-names = "pclk", "hclk";
> +		clocks = <&cru PCLK_DSITX_0>, <&cru HCLK_VO>;
> +		phy-names = "dphy";
> +		phys = <&mipi_dphy0>;
> +		power-domains = <&power RK3568_PD_VO>;
> +		reset-names = "apb";
> +		resets = <&cru SRST_P_DSITX_0>;
> +		rockchip,grf = <&grf>;
> +		status = "disabled";
> +
> +		ports {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			port@0 {

We'll have to reference that port in the board dts, right? A label would
be helpful, e.g., dsi0_in.

> +				reg = <0>;
> +			};

Would it make sense to add the dsi0_out port at this point?

dsi0_out: port@1 {
	reg = <1>;
};

> +		};
> +	};
> +
> +	dsi1: dsi@fe070000 {
> +		compatible = "rockchip,rk3568-mipi-dsi", "snps,dw-mipi-dsi";
> +		reg = <0x0 0xfe070000 0x0 0x10000>;
> +		interrupts = <GIC_SPI 69 IRQ_TYPE_LEVEL_HIGH>;
> +		clock-names = "pclk", "hclk";
> +		clocks = <&cru PCLK_DSITX_1>, <&cru HCLK_VO>;
> +		phy-names = "dphy";
> +		phys = <&mipi_dphy1>;
> +		power-domains = <&power RK3568_PD_VO>;
> +		reset-names = "apb";
> +		resets = <&cru SRST_P_DSITX_1>;
> +		rockchip,grf = <&grf>;
> +		status = "disabled";
> +
> +		ports {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			port@0 {

Ditto.

> +				reg = <0>;
> +			};
> +		};
> +	};
> +
>  	hdmi: hdmi@fe0a0000 {
>  		compatible = "rockchip,rk3568-dw-hdmi";
>  		reg = <0x0 0xfe0a0000 0x0 0x20000>;
> @@ -1594,6 +1642,30 @@ combphy2: phy@fe840000 {
>  		status = "disabled";
>  	};
>  
> +	mipi_dphy0: mipi-dphy@fe850000 {

May I suggest to call this one "dsi_dphy0" (analogous to "csi_dphy")?

> +		compatible = "rockchip,rk3568-dsi-dphy";
> +		reg = <0x0 0xfe850000 0x0 0x10000>;
> +		clock-names = "ref", "pclk";
> +		clocks = <&pmucru CLK_MIPIDSIPHY0_REF>, <&cru PCLK_MIPIDSIPHY0>;
> +		#phy-cells = <0>;
> +		power-domains = <&power RK3568_PD_VO>;
> +		reset-names = "apb";
> +		resets = <&cru SRST_P_MIPIDSIPHY0>;
> +		status = "disabled";
> +	};
> +
> +	mipi_dphy1: mipi-dphy@fe860000 {

Ditto (well, "dsi_dphy1" obviously).

Best regards,
Michael

> +		compatible = "rockchip,rk3568-dsi-dphy";
> +		reg = <0x0 0xfe860000 0x0 0x10000>;
> +		clock-names = "ref", "pclk";
> +		clocks = <&pmucru CLK_MIPIDSIPHY1_REF>, <&cru PCLK_MIPIDSIPHY1>;
> +		#phy-cells = <0>;
> +		power-domains = <&power RK3568_PD_VO>;
> +		reset-names = "apb";
> +		resets = <&cru SRST_P_MIPIDSIPHY1>;
> +		status = "disabled";
> +	};
> +
>  	usb2phy0: usb2phy@fe8a0000 {
>  		compatible = "rockchip,rk3568-usb2phy";
>  		reg = <0x0 0xfe8a0000 0x0 0x10000>;

Re: [PATCH v3 0/3] rockchip-dsi for rk3568

[Michael Riesch]

Hi Chris,

On 9/12/22 22:56, Chris Morgan wrote:
> From: Chris Morgan <macromorgan@hotmail.com>
> 
> This series adds support for the dsi and dphy controllers on the
> Rockchip RK3568.
> 
> Tested on an Anbernic RG503, Anbernic RG353P, and Odroid Go Advance.
> 
> Changes since V2:
>  - Removed dsi controller patches, as those have been merged upstream.
>  - Removed notes about rolling back clock drivers. If I set the parent
>    clock of the VOP port I'm using to VPLL and set the clock rate of
>    PLL_VPLL to 500MHz this series works correctly for my panels without
>    rolling anything back (per Heiko this is the correct way).

I tried this but it didn't help (neither did reverting ff3187eabb5c
"clk: rockchip: drop CLK_SET_RATE_PARENT from dclk_vop* on rk3568"). On
my display the content is shifted horizontally and the colors are often
wrong.

>  - Added additional details about refactoring DPHY driver to add
>    2.5GHz for rk356x. All other devices still have a max speed of 1GHz.
>  - Notified Heiko that the BIT(5) for both PLL_POST_DIV_ENABLE and
>    PLL_POST_DIV_ENABLE_MASK is deliberate, because of how the
>    phy_update_bits() works.
> 
> Changes since RFCv1:
>  - Identified cause of image shift (clock changes).
>  - Noted that driver works now.
>  - Added devicetree nodes for rk356x.dtsi.
> 
> Chris Morgan (3):
>   dt-bindings: phy-rockchip-inno-dsidphy: add compatible  for rk3568
>   phy/rockchip: inno-dsidphy: Add support for rk3568
>   arm64: dts: rockchip: Add DSI and DSI-DPHY nodes to rk356x

I am testing this on a RK3568 EVB1, which has a display mounted on the
PCB. I'll submit the patches that add support for this setup soon. For
the time being a preliminary

Tested-by: Michael Riesch <michael.riesch@wolfvision.net>

Thanks for your work!
Best regards,
Michael

> 
>  .../bindings/phy/rockchip,px30-dsi-dphy.yaml  |   1 +
>  arch/arm64/boot/dts/rockchip/rk356x.dtsi      |  72 +++++++
>  .../phy/rockchip/phy-rockchip-inno-dsidphy.c  | 204 ++++++++++++++----
>  3 files changed, 231 insertions(+), 46 deletions(-)
> 

Re: [PATCH v3 0/3] rockchip-dsi for rk3568

[Chris Morgan]

On Wed, Sep 14, 2022 at 07:46:41AM +0200, Michael Riesch wrote:
> Hi Chris,
> 
> On 9/12/22 22:56, Chris Morgan wrote:
> > From: Chris Morgan <macromorgan@hotmail.com>
> > 
> > This series adds support for the dsi and dphy controllers on the
> > Rockchip RK3568.
> > 
> > Tested on an Anbernic RG503, Anbernic RG353P, and Odroid Go Advance.
> > 
> > Changes since V2:
> >  - Removed dsi controller patches, as those have been merged upstream.
> >  - Removed notes about rolling back clock drivers. If I set the parent
> >    clock of the VOP port I'm using to VPLL and set the clock rate of
> >    PLL_VPLL to 500MHz this series works correctly for my panels without
> >    rolling anything back (per Heiko this is the correct way).
> 
> I tried this but it didn't help (neither did reverting ff3187eabb5c
> "clk: rockchip: drop CLK_SET_RATE_PARENT from dclk_vop* on rk3568"). On
> my display the content is shifted horizontally and the colors are often
> wrong.

There's still something wrong with the VOP2 driver, and I'm trying to
get to the bottom of it. Are you by chance enabling HDMI? Can you check
the clock for the dclk_vopx (where x is the port) that you are using?
It should be very close or the same as the pixel clock of your panel.

I noticed on mine that the HDMI was interfering with it. For now not
only have I disabled the HDMI but also put it on VP0 while my DSI is
on VP1 (note that if both are active you'll get a null pointer
dereference from the vop2 driver which is another thing I'm chasing
down). I think this is because the hdmi_ref is allowed to set its
parent clock (which is the PLL_HPLL), so it does to 24000000.

Basically here's what I've done to overcome the VOP2 issues and get
DSI working with this patch series.
1) Disabled HDMI (with it on VP0).
2) Enabled DSI and the DSI-DPHY (with it on VP1).
3) Set the parent clock of DCLK_VOP0 to PLL_HPLL.
4) Set the parent clock of DCLK_VOP1 to PLL_VPLL.
5) Set the clock rate for PLL_VPLL to 500000000.

Doing this allows the DCLK_VOP1 to run at the correct speed for my
panel instead of 24000000 like it would otherwise. When this occurs
I get a correct image. If for whatever reason the DCLK_VOPx of the
port I'm trying to run the panel on is at 24000000 is when I get
the shifted image.

The long term fix I'm trying to work on is to figure out how to
successfully get the VOP2 driver to not crash when VP0 and VP1
are both used for the RK3566 (note this actually should work for
you on an RK3568 board though), so that whole bit about disabling
HDMI might not apply to you if it's enabled.

In summary, check the DCLK_VOPx where x is the port you are using.
If it's not at or very close to your pixel clock that's probably
why your image is shifted, at least it was for me.

Thank you.

> 
> >  - Added additional details about refactoring DPHY driver to add
> >    2.5GHz for rk356x. All other devices still have a max speed of 1GHz.
> >  - Notified Heiko that the BIT(5) for both PLL_POST_DIV_ENABLE and
> >    PLL_POST_DIV_ENABLE_MASK is deliberate, because of how the
> >    phy_update_bits() works.
> > 
> > Changes since RFCv1:
> >  - Identified cause of image shift (clock changes).
> >  - Noted that driver works now.
> >  - Added devicetree nodes for rk356x.dtsi.
> > 
> > Chris Morgan (3):
> >   dt-bindings: phy-rockchip-inno-dsidphy: add compatible  for rk3568
> >   phy/rockchip: inno-dsidphy: Add support for rk3568
> >   arm64: dts: rockchip: Add DSI and DSI-DPHY nodes to rk356x
> 
> I am testing this on a RK3568 EVB1, which has a display mounted on the
> PCB. I'll submit the patches that add support for this setup soon. For
> the time being a preliminary
> 
> Tested-by: Michael Riesch <michael.riesch@wolfvision.net>
> 
> Thanks for your work!
> Best regards,
> Michael
> 
> > 
> >  .../bindings/phy/rockchip,px30-dsi-dphy.yaml  |   1 +
> >  arch/arm64/boot/dts/rockchip/rk356x.dtsi      |  72 +++++++
> >  .../phy/rockchip/phy-rockchip-inno-dsidphy.c  | 204 ++++++++++++++----
> >  3 files changed, 231 insertions(+), 46 deletions(-)
> > 

Re: [PATCH v3 0/3] rockchip-dsi for rk3568

[Michael Riesch]

Hi Chris,

On 9/14/22 14:50, Chris Morgan wrote:
> On Wed, Sep 14, 2022 at 07:46:41AM +0200, Michael Riesch wrote:
>> Hi Chris,
>>
>> On 9/12/22 22:56, Chris Morgan wrote:
>>> From: Chris Morgan <macromorgan@hotmail.com>
>>>
>>> This series adds support for the dsi and dphy controllers on the
>>> Rockchip RK3568.
>>>
>>> Tested on an Anbernic RG503, Anbernic RG353P, and Odroid Go Advance.
>>>
>>> Changes since V2:
>>>  - Removed dsi controller patches, as those have been merged upstream.
>>>  - Removed notes about rolling back clock drivers. If I set the parent
>>>    clock of the VOP port I'm using to VPLL and set the clock rate of
>>>    PLL_VPLL to 500MHz this series works correctly for my panels without
>>>    rolling anything back (per Heiko this is the correct way).
>>
>> I tried this but it didn't help (neither did reverting ff3187eabb5c
>> "clk: rockchip: drop CLK_SET_RATE_PARENT from dclk_vop* on rk3568"). On
>> my display the content is shifted horizontally and the colors are often
>> wrong.
> 
> There's still something wrong with the VOP2 driver, and I'm trying to
> get to the bottom of it. Are you by chance enabling HDMI? Can you check
> the clock for the dclk_vopx (where x is the port) that you are using?
> It should be very close or the same as the pixel clock of your panel.

Yes, HDMI is enabled (on VP0) and works fine. MIPI DSI is enabled on
VP1. The clocks dclk_vopx are:

    pll_hpll                          1        1        0   148500000
       0     0  50000         Y
       hpll                           3        3        0   148500000
       0     0  50000         Y
          dclk_vop2                   1        1        0    37125000
       0     0  50000         Y
          dclk_vop0                   2        2        0   148500000
       0     0  50000         Y
          clk_hdmi_ref                1        1        0   148500000
       0     0  50000         Y
          hpll_ph0                    0        0        0    74250000
       0     0  50000         Y

    pll_vpll                          1        1        0   500000000
       0     0  50000         Y
       vpll                           1        1        0   500000000
       0     0  50000         Y
          dclk_vop1                   2        2        0   125000000
       0     0  50000         Y

The pixel clock of my panel is 132 MHz (1080x1920@60). Could this
discrepancy be the cause?

> I noticed on mine that the HDMI was interfering with it. For now not
> only have I disabled the HDMI but also put it on VP0 while my DSI is
> on VP1 (note that if both are active you'll get a null pointer
> dereference from the vop2 driver which is another thing I'm chasing
> down). I think this is because the hdmi_ref is allowed to set its
> parent clock (which is the PLL_HPLL), so it does to 24000000.
> 
> Basically here's what I've done to overcome the VOP2 issues and get
> DSI working with this patch series.
> 1) Disabled HDMI (with it on VP0).
> 2) Enabled DSI and the DSI-DPHY (with it on VP1).
> 3) Set the parent clock of DCLK_VOP0 to PLL_HPLL.
> 4) Set the parent clock of DCLK_VOP1 to PLL_VPLL.
> 5) Set the clock rate for PLL_VPLL to 500000000.

I tried to reproduce this. When I disabled HDMI I realized that the
regulators that produce the 0v9/1v8 image voltages are not turned on.
They are required for the MIPI DSI TX block, though. Could you take this
requirement into account and model it in the device tree?

After setting the voltages to always-on as a hack the result was pretty
much the same: the clock tree is the same as in the case with HDMI and
also matches your description.

> Doing this allows the DCLK_VOP1 to run at the correct speed for my
> panel instead of 24000000 like it would otherwise. When this occurs
> I get a correct image. If for whatever reason the DCLK_VOPx of the
> port I'm trying to run the panel on is at 24000000 is when I get
> the shifted image.
> 
> The long term fix I'm trying to work on is to figure out how to
> successfully get the VOP2 driver to not crash when VP0 and VP1
> are both used for the RK3566 (note this actually should work for
> you on an RK3568 board though), so that whole bit about disabling
> HDMI might not apply to you if it's enabled.
> 
> In summary, check the DCLK_VOPx where x is the port you are using.
> If it's not at or very close to your pixel clock that's probably
> why your image is shifted, at least it was for me.

OK... I am starting to think that I experience a different bug here.
I'll clean up my patches and will try again.

Thanks and regards,
Michael

> 
> Thank you.
> 
>>
>>>  - Added additional details about refactoring DPHY driver to add
>>>    2.5GHz for rk356x. All other devices still have a max speed of 1GHz.
>>>  - Notified Heiko that the BIT(5) for both PLL_POST_DIV_ENABLE and
>>>    PLL_POST_DIV_ENABLE_MASK is deliberate, because of how the
>>>    phy_update_bits() works.
>>>
>>> Changes since RFCv1:
>>>  - Identified cause of image shift (clock changes).
>>>  - Noted that driver works now.
>>>  - Added devicetree nodes for rk356x.dtsi.
>>>
>>> Chris Morgan (3):
>>>   dt-bindings: phy-rockchip-inno-dsidphy: add compatible  for rk3568
>>>   phy/rockchip: inno-dsidphy: Add support for rk3568
>>>   arm64: dts: rockchip: Add DSI and DSI-DPHY nodes to rk356x
>>
>> I am testing this on a RK3568 EVB1, which has a display mounted on the
>> PCB. I'll submit the patches that add support for this setup soon. For
>> the time being a preliminary
>>
>> Tested-by: Michael Riesch <michael.riesch@wolfvision.net>
>>
>> Thanks for your work!
>> Best regards,
>> Michael
>>
>>>
>>>  .../bindings/phy/rockchip,px30-dsi-dphy.yaml  |   1 +
>>>  arch/arm64/boot/dts/rockchip/rk356x.dtsi      |  72 +++++++
>>>  .../phy/rockchip/phy-rockchip-inno-dsidphy.c  | 204 ++++++++++++++----
>>>  3 files changed, 231 insertions(+), 46 deletions(-)
>>>

Re: [PATCH v3 0/3] rockchip-dsi for rk3568

[Chris Morgan]

On Thu, Sep 15, 2022 at 09:16:47AM +0200, Michael Riesch wrote:
> Hi Chris,
> 
> On 9/14/22 14:50, Chris Morgan wrote:
> > On Wed, Sep 14, 2022 at 07:46:41AM +0200, Michael Riesch wrote:
> >> Hi Chris,
> >>
> >> On 9/12/22 22:56, Chris Morgan wrote:
> >>> From: Chris Morgan <macromorgan@hotmail.com>
> >>>
> >>> This series adds support for the dsi and dphy controllers on the
> >>> Rockchip RK3568.
> >>>
> >>> Tested on an Anbernic RG503, Anbernic RG353P, and Odroid Go Advance.
> >>>
> >>> Changes since V2:
> >>>  - Removed dsi controller patches, as those have been merged upstream.
> >>>  - Removed notes about rolling back clock drivers. If I set the parent
> >>>    clock of the VOP port I'm using to VPLL and set the clock rate of
> >>>    PLL_VPLL to 500MHz this series works correctly for my panels without
> >>>    rolling anything back (per Heiko this is the correct way).
> >>
> >> I tried this but it didn't help (neither did reverting ff3187eabb5c
> >> "clk: rockchip: drop CLK_SET_RATE_PARENT from dclk_vop* on rk3568"). On
> >> my display the content is shifted horizontally and the colors are often
> >> wrong.
> > 
> > There's still something wrong with the VOP2 driver, and I'm trying to
> > get to the bottom of it. Are you by chance enabling HDMI? Can you check
> > the clock for the dclk_vopx (where x is the port) that you are using?
> > It should be very close or the same as the pixel clock of your panel.
> 
> Yes, HDMI is enabled (on VP0) and works fine. MIPI DSI is enabled on
> VP1. The clocks dclk_vopx are:
> 
>     pll_hpll                          1        1        0   148500000
>        0     0  50000         Y
>        hpll                           3        3        0   148500000
>        0     0  50000         Y
>           dclk_vop2                   1        1        0    37125000
>        0     0  50000         Y
>           dclk_vop0                   2        2        0   148500000
>        0     0  50000         Y
>           clk_hdmi_ref                1        1        0   148500000
>        0     0  50000         Y
>           hpll_ph0                    0        0        0    74250000
>        0     0  50000         Y
> 
>     pll_vpll                          1        1        0   500000000
>        0     0  50000         Y
>        vpll                           1        1        0   500000000
>        0     0  50000         Y
>           dclk_vop1                   2        2        0   125000000
>        0     0  50000         Y
> 
> The pixel clock of my panel is 132 MHz (1080x1920@60). Could this
> discrepancy be the cause?

It's too low which likely could be the cause, (honestly not sure) but
otherwise everything looks correct. Maybe try setting the PLL_VPLL
rate to 135000000 to force the panel to go faster (135MHz instead of
125MHz)?

I know for one of my examples the panel's pixel clock is 33500000
and I'm running it at 33333333 and it seems to be okay. The other I
am testing with runs either at 25000000 or 50000000 which evenly
divides with the 500000000, which is why I use it.

You can also experiment with different rates, any rate defined in
rk3568_pll_rates[] should work (though the datasheet says for VOP1
don't run the parent clock over 500000000, and then in the BSP kernel
I see the parent clock in my example it running at 503000000 so who
knows). If need be you can also define a new rate and add it there,
but you'll have to consult the datasheet for which rates are supported
and at which dividers (and also the VPLL and NPLLs don't support frac
rate setting).

Thank you.

> 
> > I noticed on mine that the HDMI was interfering with it. For now not
> > only have I disabled the HDMI but also put it on VP0 while my DSI is
> > on VP1 (note that if both are active you'll get a null pointer
> > dereference from the vop2 driver which is another thing I'm chasing
> > down). I think this is because the hdmi_ref is allowed to set its
> > parent clock (which is the PLL_HPLL), so it does to 24000000.
> > 
> > Basically here's what I've done to overcome the VOP2 issues and get
> > DSI working with this patch series.
> > 1) Disabled HDMI (with it on VP0).
> > 2) Enabled DSI and the DSI-DPHY (with it on VP1).
> > 3) Set the parent clock of DCLK_VOP0 to PLL_HPLL.
> > 4) Set the parent clock of DCLK_VOP1 to PLL_VPLL.
> > 5) Set the clock rate for PLL_VPLL to 500000000.
> 
> I tried to reproduce this. When I disabled HDMI I realized that the
> regulators that produce the 0v9/1v8 image voltages are not turned on.
> They are required for the MIPI DSI TX block, though. Could you take this
> requirement into account and model it in the device tree?
> 
> After setting the voltages to always-on as a hack the result was pretty
> much the same: the clock tree is the same as in the case with HDMI and
> also matches your description.
> 
> > Doing this allows the DCLK_VOP1 to run at the correct speed for my
> > panel instead of 24000000 like it would otherwise. When this occurs
> > I get a correct image. If for whatever reason the DCLK_VOPx of the
> > port I'm trying to run the panel on is at 24000000 is when I get
> > the shifted image.
> > 
> > The long term fix I'm trying to work on is to figure out how to
> > successfully get the VOP2 driver to not crash when VP0 and VP1
> > are both used for the RK3566 (note this actually should work for
> > you on an RK3568 board though), so that whole bit about disabling
> > HDMI might not apply to you if it's enabled.
> > 
> > In summary, check the DCLK_VOPx where x is the port you are using.
> > If it's not at or very close to your pixel clock that's probably
> > why your image is shifted, at least it was for me.
> 
> OK... I am starting to think that I experience a different bug here.
> I'll clean up my patches and will try again.
> 
> Thanks and regards,
> Michael
> 
> > 
> > Thank you.
> > 
> >>
> >>>  - Added additional details about refactoring DPHY driver to add
> >>>    2.5GHz for rk356x. All other devices still have a max speed of 1GHz.
> >>>  - Notified Heiko that the BIT(5) for both PLL_POST_DIV_ENABLE and
> >>>    PLL_POST_DIV_ENABLE_MASK is deliberate, because of how the
> >>>    phy_update_bits() works.
> >>>
> >>> Changes since RFCv1:
> >>>  - Identified cause of image shift (clock changes).
> >>>  - Noted that driver works now.
> >>>  - Added devicetree nodes for rk356x.dtsi.
> >>>
> >>> Chris Morgan (3):
> >>>   dt-bindings: phy-rockchip-inno-dsidphy: add compatible  for rk3568
> >>>   phy/rockchip: inno-dsidphy: Add support for rk3568
> >>>   arm64: dts: rockchip: Add DSI and DSI-DPHY nodes to rk356x
> >>
> >> I am testing this on a RK3568 EVB1, which has a display mounted on the
> >> PCB. I'll submit the patches that add support for this setup soon. For
> >> the time being a preliminary
> >>
> >> Tested-by: Michael Riesch <michael.riesch@wolfvision.net>
> >>
> >> Thanks for your work!
> >> Best regards,
> >> Michael
> >>
> >>>
> >>>  .../bindings/phy/rockchip,px30-dsi-dphy.yaml  |   1 +
> >>>  arch/arm64/boot/dts/rockchip/rk356x.dtsi      |  72 +++++++
> >>>  .../phy/rockchip/phy-rockchip-inno-dsidphy.c  | 204 ++++++++++++++----
> >>>  3 files changed, 231 insertions(+), 46 deletions(-)
> >>>