[PATCH 00/12] Add PX30 LVDS support

[PATCH 00/12] Add PX30 LVDS support

[Miquel Raynal]

Hello,

This series aims at supporting LVDS on PX30.

A first couple of patches update the documentation with the new
compatible and the presence of a PHY. Then, the existing Rockchip
driver is cleaned and extended to support PX30 specificities. Finally,
the PX30 DTSI is updated with CRTC routes, the DSI DPHY and the LVDS
IP itself.

Cheers,
Miquèl

Miquel Raynal (12):
  dt-bindings: display: rockchip-lvds: Declare PX30 compatible
  dt-bindings: display: rockchip-lvds: Document PX30 PHY
  drm/rockchip: lvds: Fix indentation of a #define
  drm/rockchip: lvds: Harmonize function names
  drm/rockchip: lvds: Change platform data
  drm/rockchip: lvds: Create an RK3288 specific probe function
  drm/rockchip: lvds: Helpers should return decent values
  drm/rockchip: lvds: Pack functions together
  drm/rockchip: lvds: Add PX30 support
  arm64: dts: rockchip: Add PX30 CRTCs graph LVDS endpoints
  arm64: dts: rockchip: Add PX30 DSI DPHY
  arm64: dts: rockchip: Add PX30 LVDS

 .../display/rockchip/rockchip-lvds.txt        |   4 +
 arch/arm64/boot/dts/rockchip/px30.dtsi        |  58 ++
 drivers/gpu/drm/rockchip/rockchip_lvds.c      | 516 ++++++++++++------
 drivers/gpu/drm/rockchip/rockchip_lvds.h      |  19 +-
 4 files changed, 441 insertions(+), 156 deletions(-)

-- 
2.20.1

[PATCH 01/12] dt-bindings: display: rockchip-lvds: Declare PX30 compatible

[Miquel Raynal]

Document the PX30 LVDS compatible.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 .../devicetree/bindings/display/rockchip/rockchip-lvds.txt       | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/display/rockchip/rockchip-lvds.txt b/Documentation/devicetree/bindings/display/rockchip/rockchip-lvds.txt
index 7849ff039229..aa5663a6fd42 100644
--- a/Documentation/devicetree/bindings/display/rockchip/rockchip-lvds.txt
+++ b/Documentation/devicetree/bindings/display/rockchip/rockchip-lvds.txt
@@ -4,6 +4,7 @@ Rockchip RK3288 LVDS interface
 Required properties:
 - compatible: matching the soc type, one of
 	- "rockchip,rk3288-lvds";
+	- "rockchip,px30-lvds";
 
 - reg: physical base address of the controller and length
 	of memory mapped region.
-- 
2.20.1

[PATCH 02/12] dt-bindings: display: rockchip-lvds: Document PX30 PHY

[Miquel Raynal]

PX30 SoCs use a single PHY shared by two display pipelines: MIPI DSI
and LVDS. In the case of the LVDS IP, document the possibility to fill
a PHY handle.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 .../devicetree/bindings/display/rockchip/rockchip-lvds.txt     | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/rockchip/rockchip-lvds.txt b/Documentation/devicetree/bindings/display/rockchip/rockchip-lvds.txt
index aa5663a6fd42..ec7b4341cfd2 100644
--- a/Documentation/devicetree/bindings/display/rockchip/rockchip-lvds.txt
+++ b/Documentation/devicetree/bindings/display/rockchip/rockchip-lvds.txt
@@ -19,6 +19,9 @@ Required properties:
 - rockchip,grf: phandle to the general register files syscon
 - rockchip,output: "rgb", "lvds" or "duallvds", This describes the output interface
 
+- phys: LVDS/DSI DPHY (px30 only)
+- phy-names: name of the PHY, should be "dphy"
+
 Optional properties:
 - pinctrl-names: must contain a "lcdc" entry.
 - pinctrl-0: pin control group to be used for this controller.
-- 
2.20.1

[PATCH 03/12] drm/rockchip: lvds: Fix indentation of a #define

[Miquel Raynal]

Fix a #define indentation before adding more lines.

Fixes: 34cc0aa25456 ("drm/rockchip: Add support for Rockchip Soc LVDS")
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/gpu/drm/rockchip/rockchip_lvds.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_lvds.h b/drivers/gpu/drm/rockchip/rockchip_lvds.h
index 029bad8e1a14..1387bcbc4bc0 100644
--- a/drivers/gpu/drm/rockchip/rockchip_lvds.h
+++ b/drivers/gpu/drm/rockchip/rockchip_lvds.h
@@ -70,7 +70,7 @@
 #define RK3288_LVDS_CFG_REG21			0x84
 #define RK3288_LVDS_CFG_REG21_TX_ENABLE		0x92
 #define RK3288_LVDS_CFG_REG21_TX_DISABLE	0x00
-#define RK3288_LVDS_CH1_OFFSET                 0x100
+#define RK3288_LVDS_CH1_OFFSET			0x100
 
 /* fbdiv value is split over 2 registers, with bit8 in reg2 */
 #define RK3288_LVDS_PLL_FBDIV_REG2(_fbd) \
-- 
2.20.1

[PATCH 04/12] drm/rockchip: lvds: Harmonize function names

[Miquel Raynal]

Prepare the introduction of PX30 support by clarifying the function
prefixes.

We continue to prefix with 'rockchip_lvds_' generic functions that are
not specific to a single hardware. Functions implying hardware
modifications are now prefixed with 'rk3288_lvds_'.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/gpu/drm/rockchip/rockchip_lvds.c | 138 ++++++++++++-----------
 1 file changed, 73 insertions(+), 65 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_lvds.c b/drivers/gpu/drm/rockchip/rockchip_lvds.c
index 8a4c9af0ba73..34e5d0b1172e 100644
--- a/drivers/gpu/drm/rockchip/rockchip_lvds.c
+++ b/drivers/gpu/drm/rockchip/rockchip_lvds.c
@@ -67,7 +67,8 @@ struct rockchip_lvds {
 	struct dev_pin_info *pins;
 };
 
-static inline void lvds_writel(struct rockchip_lvds *lvds, u32 offset, u32 val)
+static inline void rk3288_writel(struct rockchip_lvds *lvds, u32 offset,
+				 u32 val)
 {
 	writel_relaxed(val, lvds->regs + offset);
 	if (lvds->output == DISPLAY_OUTPUT_LVDS)
@@ -75,7 +76,7 @@ static inline void lvds_writel(struct rockchip_lvds *lvds, u32 offset, u32 val)
 	writel_relaxed(val, lvds->regs + offset + lvds->soc_data->ch1_offset);
 }
 
-static inline int lvds_name_to_format(const char *s)
+static inline int rockchip_lvds_name_to_format(const char *s)
 {
 	if (strncmp(s, "jeida-18", 8) == 0)
 		return LVDS_JEIDA_18;
@@ -87,7 +88,7 @@ static inline int lvds_name_to_format(const char *s)
 	return -EINVAL;
 }
 
-static inline int lvds_name_to_output(const char *s)
+static inline int rockchip_lvds_name_to_output(const char *s)
 {
 	if (strncmp(s, "rgb", 3) == 0)
 		return DISPLAY_OUTPUT_RGB;
@@ -99,7 +100,7 @@ static inline int lvds_name_to_output(const char *s)
 	return -EINVAL;
 }
 
-static int rockchip_lvds_poweron(struct rockchip_lvds *lvds)
+static int rk3288_lvds_poweron(struct rockchip_lvds *lvds)
 {
 	int ret;
 	u32 val;
@@ -121,63 +122,70 @@ static int rockchip_lvds_poweron(struct rockchip_lvds *lvds)
 	if (lvds->output == DISPLAY_OUTPUT_RGB) {
 		val |= RK3288_LVDS_CH0_REG0_TTL_EN |
 			RK3288_LVDS_CH0_REG0_LANECK_EN;
-		lvds_writel(lvds, RK3288_LVDS_CH0_REG0, val);
-		lvds_writel(lvds, RK3288_LVDS_CH0_REG2,
-			    RK3288_LVDS_PLL_FBDIV_REG2(0x46));
-		lvds_writel(lvds, RK3288_LVDS_CH0_REG4,
-			    RK3288_LVDS_CH0_REG4_LANECK_TTL_MODE |
-			    RK3288_LVDS_CH0_REG4_LANE4_TTL_MODE |
-			    RK3288_LVDS_CH0_REG4_LANE3_TTL_MODE |
-			    RK3288_LVDS_CH0_REG4_LANE2_TTL_MODE |
-			    RK3288_LVDS_CH0_REG4_LANE1_TTL_MODE |
-			    RK3288_LVDS_CH0_REG4_LANE0_TTL_MODE);
-		lvds_writel(lvds, RK3288_LVDS_CH0_REG5,
-			    RK3288_LVDS_CH0_REG5_LANECK_TTL_DATA |
-			    RK3288_LVDS_CH0_REG5_LANE4_TTL_DATA |
-			    RK3288_LVDS_CH0_REG5_LANE3_TTL_DATA |
-			    RK3288_LVDS_CH0_REG5_LANE2_TTL_DATA |
-			    RK3288_LVDS_CH0_REG5_LANE1_TTL_DATA |
-			    RK3288_LVDS_CH0_REG5_LANE0_TTL_DATA);
+		rk3288_writel(lvds, RK3288_LVDS_CH0_REG0, val);
+		rk3288_writel(lvds, RK3288_LVDS_CH0_REG2,
+			      RK3288_LVDS_PLL_FBDIV_REG2(0x46));
+		rk3288_writel(lvds, RK3288_LVDS_CH0_REG4,
+			      RK3288_LVDS_CH0_REG4_LANECK_TTL_MODE |
+			      RK3288_LVDS_CH0_REG4_LANE4_TTL_MODE |
+			      RK3288_LVDS_CH0_REG4_LANE3_TTL_MODE |
+			      RK3288_LVDS_CH0_REG4_LANE2_TTL_MODE |
+			      RK3288_LVDS_CH0_REG4_LANE1_TTL_MODE |
+			      RK3288_LVDS_CH0_REG4_LANE0_TTL_MODE);
+		rk3288_writel(lvds, RK3288_LVDS_CH0_REG5,
+			      RK3288_LVDS_CH0_REG5_LANECK_TTL_DATA |
+			      RK3288_LVDS_CH0_REG5_LANE4_TTL_DATA |
+			      RK3288_LVDS_CH0_REG5_LANE3_TTL_DATA |
+			      RK3288_LVDS_CH0_REG5_LANE2_TTL_DATA |
+			      RK3288_LVDS_CH0_REG5_LANE1_TTL_DATA |
+			      RK3288_LVDS_CH0_REG5_LANE0_TTL_DATA);
 	} else {
 		val |= RK3288_LVDS_CH0_REG0_LVDS_EN |
 			    RK3288_LVDS_CH0_REG0_LANECK_EN;
-		lvds_writel(lvds, RK3288_LVDS_CH0_REG0, val);
-		lvds_writel(lvds, RK3288_LVDS_CH0_REG1,
-			    RK3288_LVDS_CH0_REG1_LANECK_BIAS |
-			    RK3288_LVDS_CH0_REG1_LANE4_BIAS |
-			    RK3288_LVDS_CH0_REG1_LANE3_BIAS |
-			    RK3288_LVDS_CH0_REG1_LANE2_BIAS |
-			    RK3288_LVDS_CH0_REG1_LANE1_BIAS |
-			    RK3288_LVDS_CH0_REG1_LANE0_BIAS);
-		lvds_writel(lvds, RK3288_LVDS_CH0_REG2,
-			    RK3288_LVDS_CH0_REG2_RESERVE_ON |
-			    RK3288_LVDS_CH0_REG2_LANECK_LVDS_MODE |
-			    RK3288_LVDS_CH0_REG2_LANE4_LVDS_MODE |
-			    RK3288_LVDS_CH0_REG2_LANE3_LVDS_MODE |
-			    RK3288_LVDS_CH0_REG2_LANE2_LVDS_MODE |
-			    RK3288_LVDS_CH0_REG2_LANE1_LVDS_MODE |
-			    RK3288_LVDS_CH0_REG2_LANE0_LVDS_MODE |
-			    RK3288_LVDS_PLL_FBDIV_REG2(0x46));
-		lvds_writel(lvds, RK3288_LVDS_CH0_REG4, 0x00);
-		lvds_writel(lvds, RK3288_LVDS_CH0_REG5, 0x00);
+		rk3288_writel(lvds, RK3288_LVDS_CH0_REG0, val);
+		rk3288_writel(lvds, RK3288_LVDS_CH0_REG1,
+			      RK3288_LVDS_CH0_REG1_LANECK_BIAS |
+			      RK3288_LVDS_CH0_REG1_LANE4_BIAS |
+			      RK3288_LVDS_CH0_REG1_LANE3_BIAS |
+			      RK3288_LVDS_CH0_REG1_LANE2_BIAS |
+			      RK3288_LVDS_CH0_REG1_LANE1_BIAS |
+			      RK3288_LVDS_CH0_REG1_LANE0_BIAS);
+		rk3288_writel(lvds, RK3288_LVDS_CH0_REG2,
+			      RK3288_LVDS_CH0_REG2_RESERVE_ON |
+			      RK3288_LVDS_CH0_REG2_LANECK_LVDS_MODE |
+			      RK3288_LVDS_CH0_REG2_LANE4_LVDS_MODE |
+			      RK3288_LVDS_CH0_REG2_LANE3_LVDS_MODE |
+			      RK3288_LVDS_CH0_REG2_LANE2_LVDS_MODE |
+			      RK3288_LVDS_CH0_REG2_LANE1_LVDS_MODE |
+			      RK3288_LVDS_CH0_REG2_LANE0_LVDS_MODE |
+			      RK3288_LVDS_PLL_FBDIV_REG2(0x46));
+		rk3288_writel(lvds, RK3288_LVDS_CH0_REG4, 0x00);
+		rk3288_writel(lvds, RK3288_LVDS_CH0_REG5, 0x00);
 	}
-	lvds_writel(lvds, RK3288_LVDS_CH0_REG3, RK3288_LVDS_PLL_FBDIV_REG3(0x46));
-	lvds_writel(lvds, RK3288_LVDS_CH0_REGD, RK3288_LVDS_PLL_PREDIV_REGD(0x0a));
-	lvds_writel(lvds, RK3288_LVDS_CH0_REG20, RK3288_LVDS_CH0_REG20_LSB);
+	rk3288_writel(lvds, RK3288_LVDS_CH0_REG3,
+		      RK3288_LVDS_PLL_FBDIV_REG3(0x46));
+	rk3288_writel(lvds, RK3288_LVDS_CH0_REGD,
+		      RK3288_LVDS_PLL_PREDIV_REGD(0x0a));
+	rk3288_writel(lvds, RK3288_LVDS_CH0_REG20,
+		      RK3288_LVDS_CH0_REG20_LSB);
 
-	lvds_writel(lvds, RK3288_LVDS_CFG_REGC, RK3288_LVDS_CFG_REGC_PLL_ENABLE);
-	lvds_writel(lvds, RK3288_LVDS_CFG_REG21, RK3288_LVDS_CFG_REG21_TX_ENABLE);
+	rk3288_writel(lvds, RK3288_LVDS_CFG_REGC,
+		      RK3288_LVDS_CFG_REGC_PLL_ENABLE);
+	rk3288_writel(lvds, RK3288_LVDS_CFG_REG21,
+		      RK3288_LVDS_CFG_REG21_TX_ENABLE);
 
 	return 0;
 }
 
-static void rockchip_lvds_poweroff(struct rockchip_lvds *lvds)
+static void rk3288_lvds_poweroff(struct rockchip_lvds *lvds)
 {
 	int ret;
 	u32 val;
 
-	lvds_writel(lvds, RK3288_LVDS_CFG_REG21, RK3288_LVDS_CFG_REG21_TX_ENABLE);
-	lvds_writel(lvds, RK3288_LVDS_CFG_REGC, RK3288_LVDS_CFG_REGC_PLL_ENABLE);
+	rk3288_writel(lvds, RK3288_LVDS_CFG_REG21,
+		      RK3288_LVDS_CFG_REG21_TX_ENABLE);
+	rk3288_writel(lvds, RK3288_LVDS_CFG_REGC,
+		      RK3288_LVDS_CFG_REGC_PLL_ENABLE);
 	val = LVDS_DUAL | LVDS_TTL_EN | LVDS_CH0_EN | LVDS_CH1_EN | LVDS_PWRDN;
 	val |= val << 16;
 	ret = regmap_write(lvds->grf, lvds->soc_data->grf_soc_con7, val);
@@ -209,8 +217,8 @@ struct drm_connector_helper_funcs rockchip_lvds_connector_helper_funcs = {
 	.get_modes = rockchip_lvds_connector_get_modes,
 };
 
-static void rockchip_lvds_grf_config(struct drm_encoder *encoder,
-				     struct drm_display_mode *mode)
+static void rk3288_lvds_grf_config(struct drm_encoder *encoder,
+				   struct drm_display_mode *mode)
 {
 	struct rockchip_lvds *lvds = encoder_to_lvds(encoder);
 	u8 pin_hsync = (mode->flags & DRM_MODE_FLAG_PHSYNC) ? 1 : 0;
@@ -241,8 +249,8 @@ static void rockchip_lvds_grf_config(struct drm_encoder *encoder,
 	}
 }
 
-static int rockchip_lvds_set_vop_source(struct rockchip_lvds *lvds,
-					struct drm_encoder *encoder)
+static int rk3288_lvds_set_vop_source(struct rockchip_lvds *lvds,
+				      struct drm_encoder *encoder)
 {
 	u32 val;
 	int ret;
@@ -278,36 +286,36 @@ rockchip_lvds_encoder_atomic_check(struct drm_encoder *encoder,
 	return 0;
 }
 
-static void rockchip_lvds_encoder_enable(struct drm_encoder *encoder)
+static void rk3288_lvds_encoder_enable(struct drm_encoder *encoder)
 {
 	struct rockchip_lvds *lvds = encoder_to_lvds(encoder);
 	struct drm_display_mode *mode = &encoder->crtc->state->adjusted_mode;
 	int ret;
 
 	drm_panel_prepare(lvds->panel);
-	ret = rockchip_lvds_poweron(lvds);
+	ret = rk3288_lvds_poweron(lvds);
 	if (ret < 0) {
 		DRM_DEV_ERROR(lvds->dev, "failed to power on lvds: %d\n", ret);
 		drm_panel_unprepare(lvds->panel);
 	}
-	rockchip_lvds_grf_config(encoder, mode);
-	rockchip_lvds_set_vop_source(lvds, encoder);
+	rk3288_lvds_grf_config(encoder, mode);
+	rk3288_lvds_set_vop_source(lvds, encoder);
 	drm_panel_enable(lvds->panel);
 }
 
-static void rockchip_lvds_encoder_disable(struct drm_encoder *encoder)
+static void rk3288_lvds_encoder_disable(struct drm_encoder *encoder)
 {
 	struct rockchip_lvds *lvds = encoder_to_lvds(encoder);
 
 	drm_panel_disable(lvds->panel);
-	rockchip_lvds_poweroff(lvds);
+	rk3288_lvds_poweroff(lvds);
 	drm_panel_unprepare(lvds->panel);
 }
 
 static const
-struct drm_encoder_helper_funcs rockchip_lvds_encoder_helper_funcs = {
-	.enable = rockchip_lvds_encoder_enable,
-	.disable = rockchip_lvds_encoder_disable,
+struct drm_encoder_helper_funcs rk3288_lvds_encoder_helper_funcs = {
+	.enable = rk3288_lvds_encoder_enable,
+	.disable = rk3288_lvds_encoder_disable,
 	.atomic_check = rockchip_lvds_encoder_atomic_check,
 };
 
@@ -378,7 +386,7 @@ static int rockchip_lvds_bind(struct device *dev, struct device *master,
 		/* default set it as output rgb */
 		lvds->output = DISPLAY_OUTPUT_RGB;
 	else
-		lvds->output = lvds_name_to_output(name);
+		lvds->output = rockchip_lvds_name_to_output(name);
 
 	if (lvds->output < 0) {
 		DRM_DEV_ERROR(dev, "invalid output type [%s]\n", name);
@@ -390,7 +398,7 @@ static int rockchip_lvds_bind(struct device *dev, struct device *master,
 		/* default set it as format vesa 18 */
 		lvds->format = LVDS_VESA_18;
 	else
-		lvds->format = lvds_name_to_format(name);
+		lvds->format = rockchip_lvds_name_to_format(name);
 
 	if (lvds->format < 0) {
 		DRM_DEV_ERROR(dev, "invalid data-mapping format [%s]\n", name);
@@ -410,7 +418,7 @@ static int rockchip_lvds_bind(struct device *dev, struct device *master,
 		goto err_put_remote;
 	}
 
-	drm_encoder_helper_add(encoder, &rockchip_lvds_encoder_helper_funcs);
+	drm_encoder_helper_add(encoder, &rk3288_lvds_encoder_helper_funcs);
 
 	if (lvds->panel) {
 		connector = &lvds->connector;
@@ -472,7 +480,7 @@ static void rockchip_lvds_unbind(struct device *dev, struct device *master,
 {
 	struct rockchip_lvds *lvds = dev_get_drvdata(dev);
 
-	rockchip_lvds_encoder_disable(&lvds->encoder);
+	rk3288_lvds_encoder_disable(&lvds->encoder);
 	if (lvds->panel)
 		drm_panel_detach(lvds->panel);
 	pm_runtime_disable(dev);
-- 
2.20.1

[PATCH 05/12] drm/rockchip: lvds: Change platform data

[Miquel Raynal]

Prepare the introduction of PX30 support by using
drm_encoder_helper_funcs as platform data instead of multiple register
names which are specific to rk3288 and not generic to all Rockchip
IPs. This way adding support for a new flavor of a similar IP will be
a matter of adding the relevant helper funcs.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/gpu/drm/rockchip/rockchip_lvds.c | 32 ++++++++----------------
 drivers/gpu/drm/rockchip/rockchip_lvds.h |  3 +++
 2 files changed, 14 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_lvds.c b/drivers/gpu/drm/rockchip/rockchip_lvds.c
index 34e5d0b1172e..262fec61eb78 100644
--- a/drivers/gpu/drm/rockchip/rockchip_lvds.c
+++ b/drivers/gpu/drm/rockchip/rockchip_lvds.c
@@ -39,16 +39,10 @@
 
 /**
  * rockchip_lvds_soc_data - rockchip lvds Soc private data
- * @ch1_offset: lvds channel 1 registe offset
- * grf_soc_con6: general registe offset for LVDS contrl
- * grf_soc_con7: general registe offset for LVDS contrl
- * has_vop_sel: to indicate whether need to choose from different VOP.
+ * @helper_funcs: LVDS connector helper functions
  */
 struct rockchip_lvds_soc_data {
-	u32 ch1_offset;
-	int grf_soc_con6;
-	int grf_soc_con7;
-	bool has_vop_sel;
+	const struct drm_encoder_helper_funcs *helper_funcs;
 };
 
 struct rockchip_lvds {
@@ -73,7 +67,7 @@ static inline void rk3288_writel(struct rockchip_lvds *lvds, u32 offset,
 	writel_relaxed(val, lvds->regs + offset);
 	if (lvds->output == DISPLAY_OUTPUT_LVDS)
 		return;
-	writel_relaxed(val, lvds->regs + offset + lvds->soc_data->ch1_offset);
+	writel_relaxed(val, lvds->regs + offset + RK3288_LVDS_CH1_OFFSET);
 }
 
 static inline int rockchip_lvds_name_to_format(const char *s)
@@ -188,7 +182,7 @@ static void rk3288_lvds_poweroff(struct rockchip_lvds *lvds)
 		      RK3288_LVDS_CFG_REGC_PLL_ENABLE);
 	val = LVDS_DUAL | LVDS_TTL_EN | LVDS_CH0_EN | LVDS_CH1_EN | LVDS_PWRDN;
 	val |= val << 16;
-	ret = regmap_write(lvds->grf, lvds->soc_data->grf_soc_con7, val);
+	ret = regmap_write(lvds->grf, RK3288_LVDS_GRF_SOC_CON7, val);
 	if (ret != 0)
 		DRM_DEV_ERROR(lvds->dev, "Could not write to GRF: %d\n", ret);
 
@@ -242,7 +236,7 @@ static void rk3288_lvds_grf_config(struct drm_encoder *encoder,
 
 	val |= (pin_dclk << 8) | (pin_hsync << 9);
 	val |= (0xffff << 16);
-	ret = regmap_write(lvds->grf, lvds->soc_data->grf_soc_con7, val);
+	ret = regmap_write(lvds->grf, RK3288_LVDS_GRF_SOC_CON7, val);
 	if (ret != 0) {
 		DRM_DEV_ERROR(lvds->dev, "Could not write to GRF: %d\n", ret);
 		return;
@@ -255,9 +249,6 @@ static int rk3288_lvds_set_vop_source(struct rockchip_lvds *lvds,
 	u32 val;
 	int ret;
 
-	if (!lvds->soc_data->has_vop_sel)
-		return 0;
-
 	ret = drm_of_encoder_active_endpoint_id(lvds->dev->of_node, encoder);
 	if (ret < 0)
 		return ret;
@@ -266,7 +257,7 @@ static int rk3288_lvds_set_vop_source(struct rockchip_lvds *lvds,
 	if (ret)
 		val |= RK3288_LVDS_SOC_CON6_SEL_VOP_LIT;
 
-	ret = regmap_write(lvds->grf, lvds->soc_data->grf_soc_con6, val);
+	ret = regmap_write(lvds->grf, RK3288_LVDS_GRF_SOC_CON6, val);
 	if (ret < 0)
 		return ret;
 
@@ -324,10 +315,7 @@ static const struct drm_encoder_funcs rockchip_lvds_encoder_funcs = {
 };
 
 static const struct rockchip_lvds_soc_data rk3288_lvds_data = {
-	.ch1_offset = 0x100,
-	.grf_soc_con6 = 0x025c,
-	.grf_soc_con7 = 0x0260,
-	.has_vop_sel = true,
+	.helper_funcs = &rk3288_lvds_encoder_helper_funcs,
 };
 
 static const struct of_device_id rockchip_lvds_dt_ids[] = {
@@ -418,7 +406,7 @@ static int rockchip_lvds_bind(struct device *dev, struct device *master,
 		goto err_put_remote;
 	}
 
-	drm_encoder_helper_add(encoder, &rk3288_lvds_encoder_helper_funcs);
+	drm_encoder_helper_add(encoder, lvds->soc_data->helper_funcs);
 
 	if (lvds->panel) {
 		connector = &lvds->connector;
@@ -479,8 +467,10 @@ static void rockchip_lvds_unbind(struct device *dev, struct device *master,
 				void *data)
 {
 	struct rockchip_lvds *lvds = dev_get_drvdata(dev);
+	const struct drm_encoder_helper_funcs *encoder_funcs;
 
-	rk3288_lvds_encoder_disable(&lvds->encoder);
+	encoder_funcs = lvds->soc_data->helper_funcs;
+	encoder_funcs->disable(&lvds->encoder);
 	if (lvds->panel)
 		drm_panel_detach(lvds->panel);
 	pm_runtime_disable(dev);
diff --git a/drivers/gpu/drm/rockchip/rockchip_lvds.h b/drivers/gpu/drm/rockchip/rockchip_lvds.h
index 1387bcbc4bc0..e41e9ab3c306 100644
--- a/drivers/gpu/drm/rockchip/rockchip_lvds.h
+++ b/drivers/gpu/drm/rockchip/rockchip_lvds.h
@@ -72,6 +72,9 @@
 #define RK3288_LVDS_CFG_REG21_TX_DISABLE	0x00
 #define RK3288_LVDS_CH1_OFFSET			0x100
 
+#define RK3288_LVDS_GRF_SOC_CON6		0x025C
+#define RK3288_LVDS_GRF_SOC_CON7		0x0260
+
 /* fbdiv value is split over 2 registers, with bit8 in reg2 */
 #define RK3288_LVDS_PLL_FBDIV_REG2(_fbd) \
 		(_fbd & BIT(8) ? RK3288_LVDS_CH0_REG2_PLL_FBDIV8 : 0)
-- 
2.20.1

[PATCH 06/12] drm/rockchip: lvds: Create an RK3288 specific probe function

[Miquel Raynal]

The probe function is highly tighten to the RK3288 specificities, move
all specific bits into an "rk3288_probe" function, also part of the
platform data.

The goal is to ease the addition of new flavors of Rockchip LVDS IPs.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/gpu/drm/rockchip/rockchip_lvds.c | 94 ++++++++++++++----------
 1 file changed, 57 insertions(+), 37 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_lvds.c b/drivers/gpu/drm/rockchip/rockchip_lvds.c
index 262fec61eb78..0d2902b0d990 100644
--- a/drivers/gpu/drm/rockchip/rockchip_lvds.c
+++ b/drivers/gpu/drm/rockchip/rockchip_lvds.c
@@ -31,6 +31,8 @@
 #define DISPLAY_OUTPUT_LVDS		1
 #define DISPLAY_OUTPUT_DUAL_LVDS	2
 
+struct rockchip_lvds;
+
 #define connector_to_lvds(c) \
 		container_of(c, struct rockchip_lvds, connector)
 
@@ -39,9 +41,11 @@
 
 /**
  * rockchip_lvds_soc_data - rockchip lvds Soc private data
+ * @probe: LVDS platform probe function
  * @helper_funcs: LVDS connector helper functions
  */
 struct rockchip_lvds_soc_data {
+	int (*probe)(struct platform_device *pdev, struct rockchip_lvds *lvds);
 	const struct drm_encoder_helper_funcs *helper_funcs;
 };
 
@@ -303,6 +307,52 @@ static void rk3288_lvds_encoder_disable(struct drm_encoder *encoder)
 	drm_panel_unprepare(lvds->panel);
 }
 
+static int rk3288_lvds_probe(struct platform_device *pdev,
+			     struct rockchip_lvds *lvds)
+{
+	struct resource *res;
+	int ret;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	lvds->regs = devm_ioremap_resource(lvds->dev, res);
+	if (IS_ERR(lvds->regs))
+		return PTR_ERR(lvds->regs);
+
+	lvds->pclk = devm_clk_get(lvds->dev, "pclk_lvds");
+	if (IS_ERR(lvds->pclk)) {
+		DRM_DEV_ERROR(lvds->dev, "could not get pclk_lvds\n");
+		return PTR_ERR(lvds->pclk);
+	}
+
+	lvds->pins = devm_kzalloc(lvds->dev, sizeof(*lvds->pins),
+				  GFP_KERNEL);
+	if (!lvds->pins)
+		return -ENOMEM;
+
+	lvds->pins->p = devm_pinctrl_get(lvds->dev);
+	if (IS_ERR(lvds->pins->p)) {
+		DRM_DEV_ERROR(lvds->dev, "no pinctrl handle\n");
+		devm_kfree(lvds->dev, lvds->pins);
+		lvds->pins = NULL;
+	} else {
+		lvds->pins->default_state =
+			pinctrl_lookup_state(lvds->pins->p, "lcdc");
+		if (IS_ERR(lvds->pins->default_state)) {
+			DRM_DEV_ERROR(lvds->dev, "no default pinctrl state\n");
+			devm_kfree(lvds->dev, lvds->pins);
+			lvds->pins = NULL;
+		}
+	}
+
+	ret = clk_prepare(lvds->pclk);
+	if (ret < 0) {
+		DRM_DEV_ERROR(lvds->dev, "failed to prepare pclk_lvds\n");
+		return ret;
+	}
+
+	return 0;
+}
+
 static const
 struct drm_encoder_helper_funcs rk3288_lvds_encoder_helper_funcs = {
 	.enable = rk3288_lvds_encoder_enable,
@@ -315,6 +365,7 @@ static const struct drm_encoder_funcs rockchip_lvds_encoder_funcs = {
 };
 
 static const struct rockchip_lvds_soc_data rk3288_lvds_data = {
+	.probe = rk3288_lvds_probe,
 	.helper_funcs = &rk3288_lvds_encoder_helper_funcs,
 };
 
@@ -488,7 +539,6 @@ static int rockchip_lvds_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct rockchip_lvds *lvds;
 	const struct of_device_id *match;
-	struct resource *res;
 	int ret;
 
 	if (!dev->of_node)
@@ -504,37 +554,6 @@ static int rockchip_lvds_probe(struct platform_device *pdev)
 		return -ENODEV;
 	lvds->soc_data = match->data;
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	lvds->regs = devm_ioremap_resource(&pdev->dev, res);
-	if (IS_ERR(lvds->regs))
-		return PTR_ERR(lvds->regs);
-
-	lvds->pclk = devm_clk_get(&pdev->dev, "pclk_lvds");
-	if (IS_ERR(lvds->pclk)) {
-		DRM_DEV_ERROR(dev, "could not get pclk_lvds\n");
-		return PTR_ERR(lvds->pclk);
-	}
-
-	lvds->pins = devm_kzalloc(lvds->dev, sizeof(*lvds->pins),
-				  GFP_KERNEL);
-	if (!lvds->pins)
-		return -ENOMEM;
-
-	lvds->pins->p = devm_pinctrl_get(lvds->dev);
-	if (IS_ERR(lvds->pins->p)) {
-		DRM_DEV_ERROR(dev, "no pinctrl handle\n");
-		devm_kfree(lvds->dev, lvds->pins);
-		lvds->pins = NULL;
-	} else {
-		lvds->pins->default_state =
-			pinctrl_lookup_state(lvds->pins->p, "lcdc");
-		if (IS_ERR(lvds->pins->default_state)) {
-			DRM_DEV_ERROR(dev, "no default pinctrl state\n");
-			devm_kfree(lvds->dev, lvds->pins);
-			lvds->pins = NULL;
-		}
-	}
-
 	lvds->grf = syscon_regmap_lookup_by_phandle(dev->of_node,
 						    "rockchip,grf");
 	if (IS_ERR(lvds->grf)) {
@@ -542,13 +561,14 @@ static int rockchip_lvds_probe(struct platform_device *pdev)
 		return PTR_ERR(lvds->grf);
 	}
 
+	ret = lvds->soc_data->probe(pdev, lvds);
+	if (ret) {
+		DRM_DEV_ERROR(dev, "Platform initialization failed\n");
+		return ret;
+	}
+
 	dev_set_drvdata(dev, lvds);
 
-	ret = clk_prepare(lvds->pclk);
-	if (ret < 0) {
-		DRM_DEV_ERROR(dev, "failed to prepare pclk_lvds\n");
-		return ret;
-	}
 	ret = component_add(&pdev->dev, &rockchip_lvds_component_ops);
 	if (ret < 0) {
 		DRM_DEV_ERROR(dev, "failed to add component\n");
-- 
2.20.1

[PATCH 07/12] drm/rockchip: lvds: Helpers should return decent values

[Miquel Raynal]

Return errors instead of returning void from internal helpers. When
these helpers are called, check the returned value and print an error
message in this case.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/gpu/drm/rockchip/rockchip_lvds.c | 31 ++++++++++++++++++------
 1 file changed, 23 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_lvds.c b/drivers/gpu/drm/rockchip/rockchip_lvds.c
index 0d2902b0d990..6365a2c883c3 100644
--- a/drivers/gpu/drm/rockchip/rockchip_lvds.c
+++ b/drivers/gpu/drm/rockchip/rockchip_lvds.c
@@ -215,8 +215,8 @@ struct drm_connector_helper_funcs rockchip_lvds_connector_helper_funcs = {
 	.get_modes = rockchip_lvds_connector_get_modes,
 };
 
-static void rk3288_lvds_grf_config(struct drm_encoder *encoder,
-				   struct drm_display_mode *mode)
+static int rk3288_lvds_grf_config(struct drm_encoder *encoder,
+				  struct drm_display_mode *mode)
 {
 	struct rockchip_lvds *lvds = encoder_to_lvds(encoder);
 	u8 pin_hsync = (mode->flags & DRM_MODE_FLAG_PHSYNC) ? 1 : 0;
@@ -241,10 +241,10 @@ static void rk3288_lvds_grf_config(struct drm_encoder *encoder,
 	val |= (pin_dclk << 8) | (pin_hsync << 9);
 	val |= (0xffff << 16);
 	ret = regmap_write(lvds->grf, RK3288_LVDS_GRF_SOC_CON7, val);
-	if (ret != 0) {
+	if (ret)
 		DRM_DEV_ERROR(lvds->dev, "Could not write to GRF: %d\n", ret);
-		return;
-	}
+
+	return ret;
 }
 
 static int rk3288_lvds_set_vop_source(struct rockchip_lvds *lvds,
@@ -288,13 +288,28 @@ static void rk3288_lvds_encoder_enable(struct drm_encoder *encoder)
 	int ret;
 
 	drm_panel_prepare(lvds->panel);
+
 	ret = rk3288_lvds_poweron(lvds);
 	if (ret < 0) {
-		DRM_DEV_ERROR(lvds->dev, "failed to power on lvds: %d\n", ret);
+		DRM_DEV_ERROR(lvds->dev, "failed to power on LVDS: %d\n", ret);
 		drm_panel_unprepare(lvds->panel);
+		return;
 	}
-	rk3288_lvds_grf_config(encoder, mode);
-	rk3288_lvds_set_vop_source(lvds, encoder);
+
+	ret = rk3288_lvds_grf_config(encoder, mode);
+	if (ret) {
+		DRM_DEV_ERROR(lvds->dev, "failed to configure LVDS: %d\n", ret);
+		drm_panel_unprepare(lvds->panel);
+		return;
+	}
+
+	ret = rk3288_lvds_set_vop_source(lvds, encoder);
+	if (ret) {
+		DRM_DEV_ERROR(lvds->dev, "failed to set VOP source: %d\n", ret);
+		drm_panel_unprepare(lvds->panel);
+		return;
+	}
+
 	drm_panel_enable(lvds->panel);
 }
 
-- 
2.20.1

[PATCH 08/12] drm/rockchip: lvds: Pack functions together

[Miquel Raynal]

Reorganize a bit the functions order to clarify the driver. This
change only moves functions around, there is no functional change.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/gpu/drm/rockchip/rockchip_lvds.c | 90 ++++++++++++------------
 1 file changed, 45 insertions(+), 45 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_lvds.c b/drivers/gpu/drm/rockchip/rockchip_lvds.c
index 6365a2c883c3..a0c203dcd66f 100644
--- a/drivers/gpu/drm/rockchip/rockchip_lvds.c
+++ b/drivers/gpu/drm/rockchip/rockchip_lvds.c
@@ -98,6 +98,40 @@ static inline int rockchip_lvds_name_to_output(const char *s)
 	return -EINVAL;
 }
 
+static const struct drm_connector_funcs rockchip_lvds_connector_funcs = {
+	.fill_modes = drm_helper_probe_single_connector_modes,
+	.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 int rockchip_lvds_connector_get_modes(struct drm_connector *connector)
+{
+	struct rockchip_lvds *lvds = connector_to_lvds(connector);
+	struct drm_panel *panel = lvds->panel;
+
+	return drm_panel_get_modes(panel);
+}
+
+static const
+struct drm_connector_helper_funcs rockchip_lvds_connector_helper_funcs = {
+	.get_modes = rockchip_lvds_connector_get_modes,
+};
+
+static int
+rockchip_lvds_encoder_atomic_check(struct drm_encoder *encoder,
+				   struct drm_crtc_state *crtc_state,
+				   struct drm_connector_state *conn_state)
+{
+	struct rockchip_crtc_state *s = to_rockchip_crtc_state(crtc_state);
+
+	s->output_mode = ROCKCHIP_OUT_MODE_P888;
+	s->output_type = DRM_MODE_CONNECTOR_LVDS;
+
+	return 0;
+}
+
 static int rk3288_lvds_poweron(struct rockchip_lvds *lvds)
 {
 	int ret;
@@ -194,27 +228,6 @@ static void rk3288_lvds_poweroff(struct rockchip_lvds *lvds)
 	clk_disable(lvds->pclk);
 }
 
-static const struct drm_connector_funcs rockchip_lvds_connector_funcs = {
-	.fill_modes = drm_helper_probe_single_connector_modes,
-	.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 int rockchip_lvds_connector_get_modes(struct drm_connector *connector)
-{
-	struct rockchip_lvds *lvds = connector_to_lvds(connector);
-	struct drm_panel *panel = lvds->panel;
-
-	return drm_panel_get_modes(panel);
-}
-
-static const
-struct drm_connector_helper_funcs rockchip_lvds_connector_helper_funcs = {
-	.get_modes = rockchip_lvds_connector_get_modes,
-};
-
 static int rk3288_lvds_grf_config(struct drm_encoder *encoder,
 				  struct drm_display_mode *mode)
 {
@@ -268,19 +281,6 @@ static int rk3288_lvds_set_vop_source(struct rockchip_lvds *lvds,
 	return 0;
 }
 
-static int
-rockchip_lvds_encoder_atomic_check(struct drm_encoder *encoder,
-				   struct drm_crtc_state *crtc_state,
-				   struct drm_connector_state *conn_state)
-{
-	struct rockchip_crtc_state *s = to_rockchip_crtc_state(crtc_state);
-
-	s->output_mode = ROCKCHIP_OUT_MODE_P888;
-	s->output_type = DRM_MODE_CONNECTOR_LVDS;
-
-	return 0;
-}
-
 static void rk3288_lvds_encoder_enable(struct drm_encoder *encoder)
 {
 	struct rockchip_lvds *lvds = encoder_to_lvds(encoder);
@@ -322,6 +322,17 @@ static void rk3288_lvds_encoder_disable(struct drm_encoder *encoder)
 	drm_panel_unprepare(lvds->panel);
 }
 
+static const
+struct drm_encoder_helper_funcs rk3288_lvds_encoder_helper_funcs = {
+	.enable = rk3288_lvds_encoder_enable,
+	.disable = rk3288_lvds_encoder_disable,
+	.atomic_check = rockchip_lvds_encoder_atomic_check,
+};
+
+static const struct drm_encoder_funcs rockchip_lvds_encoder_funcs = {
+	.destroy = drm_encoder_cleanup,
+};
+
 static int rk3288_lvds_probe(struct platform_device *pdev,
 			     struct rockchip_lvds *lvds)
 {
@@ -368,17 +379,6 @@ static int rk3288_lvds_probe(struct platform_device *pdev,
 	return 0;
 }
 
-static const
-struct drm_encoder_helper_funcs rk3288_lvds_encoder_helper_funcs = {
-	.enable = rk3288_lvds_encoder_enable,
-	.disable = rk3288_lvds_encoder_disable,
-	.atomic_check = rockchip_lvds_encoder_atomic_check,
-};
-
-static const struct drm_encoder_funcs rockchip_lvds_encoder_funcs = {
-	.destroy = drm_encoder_cleanup,
-};
-
 static const struct rockchip_lvds_soc_data rk3288_lvds_data = {
 	.probe = rk3288_lvds_probe,
 	.helper_funcs = &rk3288_lvds_encoder_helper_funcs,
-- 
2.20.1

[PATCH 09/12] drm/rockchip: lvds: Add PX30 support

[Miquel Raynal]

Introduce PX30 LVDS support. This means adding the relevant helper
functions, a specific probe and also the initialization of a specific
PHY.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/gpu/drm/rockchip/rockchip_lvds.c | 173 +++++++++++++++++++++++
 drivers/gpu/drm/rockchip/rockchip_lvds.h |  14 ++
 2 files changed, 187 insertions(+)

diff --git a/drivers/gpu/drm/rockchip/rockchip_lvds.c b/drivers/gpu/drm/rockchip/rockchip_lvds.c
index a0c203dcd66f..e550c2f102e0 100644
--- a/drivers/gpu/drm/rockchip/rockchip_lvds.c
+++ b/drivers/gpu/drm/rockchip/rockchip_lvds.c
@@ -10,6 +10,7 @@
 #include <linux/component.h>
 #include <linux/mfd/syscon.h>
 #include <linux/of_graph.h>
+#include <linux/phy/phy.h>
 #include <linux/pinctrl/devinfo.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
@@ -54,6 +55,7 @@ struct rockchip_lvds {
 	void __iomem *regs;
 	struct regmap *grf;
 	struct clk *pclk;
+	struct phy *dphy;
 	const struct rockchip_lvds_soc_data *soc_data;
 	int output; /* rgb lvds or dual lvds output */
 	int format; /* vesa or jeida format */
@@ -322,6 +324,133 @@ static void rk3288_lvds_encoder_disable(struct drm_encoder *encoder)
 	drm_panel_unprepare(lvds->panel);
 }
 
+static int px30_lvds_poweron(struct rockchip_lvds *lvds)
+{
+	int ret;
+
+	ret = pm_runtime_get_sync(lvds->dev);
+	if (ret < 0) {
+		DRM_DEV_ERROR(lvds->dev, "failed to get pm runtime: %d\n", ret);
+		return ret;
+	}
+
+	/* Enable LVDS mode */
+	return regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON1,
+				  PX30_LVDS_MODE_EN(1) | PX30_LVDS_P2S_EN(1),
+				  PX30_LVDS_MODE_EN(1) | PX30_LVDS_P2S_EN(1));
+}
+
+static void px30_lvds_poweroff(struct rockchip_lvds *lvds)
+{
+	regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON1,
+			   PX30_LVDS_MODE_EN(1) | PX30_LVDS_P2S_EN(1),
+			   PX30_LVDS_MODE_EN(0) | PX30_LVDS_P2S_EN(0));
+
+	pm_runtime_put(lvds->dev);
+}
+
+static int px30_lvds_grf_config(struct drm_encoder *encoder,
+				struct drm_display_mode *mode)
+{
+	struct rockchip_lvds *lvds = encoder_to_lvds(encoder);
+	u8 nhsync = !(mode->flags & DRM_MODE_FLAG_PHSYNC);
+	u8 nvsync = !(mode->flags & DRM_MODE_FLAG_PVSYNC);
+	u8 ndclk = !(mode->flags & DRM_MODE_FLAG_PCSYNC);
+	int ret;
+
+	if (lvds->output != DISPLAY_OUTPUT_LVDS) {
+		DRM_DEV_ERROR(lvds->dev, "Unsupported display output %d\n",
+			      lvds->output);
+		return -EINVAL;
+	}
+
+	if (nhsync ^ nvsync) {
+		DRM_DEV_ERROR(lvds->dev, "Unsupported Hsync/Vsync polarity\n");
+		return -EINVAL;
+	}
+
+	/* Set format */
+	ret = regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON1,
+				 PX30_LVDS_FORMAT(lvds->format),
+				 PX30_LVDS_FORMAT(lvds->format));
+	if (ret)
+		return ret;
+
+	/* Control Hsync/Vsync polarity */
+	ret = regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON0,
+				 PX30_LVDS_TIE_CLKS(1),
+				 PX30_LVDS_TIE_CLKS(1));
+	if (ret)
+		return ret;
+
+	/* Set Hsync/Vsync polarity */
+	ret = regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON0,
+				 PX30_LVDS_INVERT_CLKS(1),
+				 PX30_LVDS_INVERT_CLKS(nhsync));
+	if (ret)
+		return ret;
+
+	/* Set dclk polarity */
+	return regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON0,
+				  PX30_LVDS_INVERT_DCLK(1),
+				  PX30_LVDS_INVERT_DCLK(ndclk));
+}
+
+static int px30_lvds_set_vop_source(struct rockchip_lvds *lvds,
+				    struct drm_encoder *encoder)
+{
+	int vop;
+
+	vop = drm_of_encoder_active_endpoint_id(lvds->dev->of_node, encoder);
+	if (vop < 0)
+		return vop;
+
+	return regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON1,
+				  PX30_LVDS_VOP_SEL(1),
+				  PX30_LVDS_VOP_SEL(vop));
+}
+
+static void px30_lvds_encoder_enable(struct drm_encoder *encoder)
+{
+	struct rockchip_lvds *lvds = encoder_to_lvds(encoder);
+	struct drm_display_mode *mode = &encoder->crtc->state->adjusted_mode;
+	int ret;
+
+	drm_panel_prepare(lvds->panel);
+
+	ret = px30_lvds_poweron(lvds);
+	if (ret) {
+		DRM_DEV_ERROR(lvds->dev, "failed to power on LVDS: %d\n", ret);
+		drm_panel_unprepare(lvds->panel);
+		return;
+	}
+
+	ret = px30_lvds_grf_config(encoder, mode);
+	if (ret) {
+		DRM_DEV_ERROR(lvds->dev, "failed to configure LVDS: %d\n", ret);
+		drm_panel_unprepare(lvds->panel);
+		return;
+	}
+
+	ret = px30_lvds_set_vop_source(lvds, encoder);
+	if (ret) {
+		DRM_DEV_ERROR(lvds->dev, "failed to set VOP source: %d\n", ret);
+		drm_panel_unprepare(lvds->panel);
+		return;
+	}
+
+	drm_panel_enable(lvds->panel);
+}
+
+static void px30_lvds_encoder_disable(struct drm_encoder *encoder)
+{
+	struct rockchip_lvds *lvds = encoder_to_lvds(encoder);
+
+	drm_panel_disable(lvds->panel);
+	px30_lvds_poweroff(lvds);
+	drm_panel_unprepare(lvds->panel);
+}
+
 static const
 struct drm_encoder_helper_funcs rk3288_lvds_encoder_helper_funcs = {
 	.enable = rk3288_lvds_encoder_enable,
@@ -329,6 +458,13 @@ struct drm_encoder_helper_funcs rk3288_lvds_encoder_helper_funcs = {
 	.atomic_check = rockchip_lvds_encoder_atomic_check,
 };
 
+static const
+struct drm_encoder_helper_funcs px30_lvds_encoder_helper_funcs = {
+	.enable = px30_lvds_encoder_enable,
+	.disable = px30_lvds_encoder_disable,
+	.atomic_check = rockchip_lvds_encoder_atomic_check,
+};
+
 static const struct drm_encoder_funcs rockchip_lvds_encoder_funcs = {
 	.destroy = drm_encoder_cleanup,
 };
@@ -379,16 +515,53 @@ static int rk3288_lvds_probe(struct platform_device *pdev,
 	return 0;
 }
 
+static int px30_lvds_probe(struct platform_device *pdev,
+			   struct rockchip_lvds *lvds)
+{
+	int ret;
+
+	/* MSB */
+	ret =  regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON1,
+				  PX30_LVDS_MSBSEL(1),
+				  PX30_LVDS_MSBSEL(1));
+	if (ret)
+		return ret;
+
+	/* PHY */
+	lvds->dphy = devm_phy_get(&pdev->dev, "dphy");
+	if (IS_ERR(lvds->dphy))
+		return PTR_ERR(lvds->dphy);
+
+	phy_init(lvds->dphy);
+	if (ret)
+		return ret;
+
+	phy_set_mode(lvds->dphy, PHY_MODE_LVDS);
+	if (ret)
+		return ret;
+
+	return phy_power_on(lvds->dphy);
+}
+
 static const struct rockchip_lvds_soc_data rk3288_lvds_data = {
 	.probe = rk3288_lvds_probe,
 	.helper_funcs = &rk3288_lvds_encoder_helper_funcs,
 };
 
+static const struct rockchip_lvds_soc_data px30_lvds_data = {
+	.probe = px30_lvds_probe,
+	.helper_funcs = &px30_lvds_encoder_helper_funcs,
+};
+
 static const struct of_device_id rockchip_lvds_dt_ids[] = {
 	{
 		.compatible = "rockchip,rk3288-lvds",
 		.data = &rk3288_lvds_data
 	},
+	{
+		.compatible = "rockchip,px30-lvds",
+		.data = &px30_lvds_data
+	},
 	{}
 };
 MODULE_DEVICE_TABLE(of, rockchip_lvds_dt_ids);
diff --git a/drivers/gpu/drm/rockchip/rockchip_lvds.h b/drivers/gpu/drm/rockchip/rockchip_lvds.h
index e41e9ab3c306..7cfb102b4854 100644
--- a/drivers/gpu/drm/rockchip/rockchip_lvds.h
+++ b/drivers/gpu/drm/rockchip/rockchip_lvds.h
@@ -106,4 +106,18 @@
 #define LVDS_VESA_18				2
 #define LVDS_JEIDA_18				3
 
+#define WRITE_EN(v, h, l)  ((GENMASK(h, l) << 16) | (v << l))
+
+#define PX30_LVDS_GRF_PD_VO_CON0		0x434
+#define   PX30_LVDS_TIE_CLKS(val)		WRITE_EN(val,  8,  8)
+#define   PX30_LVDS_INVERT_CLKS(val)		WRITE_EN(val,  9,  9)
+#define   PX30_LVDS_INVERT_DCLK(val)		WRITE_EN(val,  5,  5)
+
+#define PX30_LVDS_GRF_PD_VO_CON1		0x438
+#define   PX30_LVDS_FORMAT(val)			WRITE_EN(val, 14, 13)
+#define   PX30_LVDS_MODE_EN(val)		WRITE_EN(val, 12, 12)
+#define   PX30_LVDS_MSBSEL(val)			WRITE_EN(val, 11, 11)
+#define   PX30_LVDS_P2S_EN(val)			WRITE_EN(val,  6,  6)
+#define   PX30_LVDS_VOP_SEL(val)		WRITE_EN(val,  1,  1)
+
 #endif /* _ROCKCHIP_LVDS_ */
-- 
2.20.1

[PATCH 10/12] arm64: dts: rockchip: Add PX30 CRTCs graph LVDS endpoints

[Miquel Raynal]

Add the display subsystem routes with the two available CRTCs: vopb
and vopl (big and little). For each CRTC, add the LVDS endpoints. MIPI
DSI endpoints will come later.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 arch/arm64/boot/dts/rockchip/px30.dtsi | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/px30.dtsi b/arch/arm64/boot/dts/rockchip/px30.dtsi
index b2af0f02ecbe..1c96ba556daf 100644
--- a/arch/arm64/boot/dts/rockchip/px30.dtsi
+++ b/arch/arm64/boot/dts/rockchip/px30.dtsi
@@ -190,6 +190,16 @@
 		compatible = "rockchip,display-subsystem";
 		ports = <&vopb_out>, <&vopl_out>;
 		status = "disabled";
+
+		route {
+			route_vopb_lvds: route-vopb-lvds {
+				connect = <&vopb_out_lvds>;
+			};
+
+			route_vopl_lvds: route-vopl-lvds {
+				connect = <&vopl_out_lvds>;
+			};
+		};
 	};
 
 	gmac_clkin: external-gmac-clock {
@@ -976,6 +986,11 @@
 		vopb_out: port {
 			#address-cells = <1>;
 			#size-cells = <0>;
+
+			vopb_out_lvds: endpoint@0 {
+				reg = <0>;
+				remote-endpoint = <&lvds_vopb_in>;
+			};
 		};
 	};
 
@@ -1008,6 +1023,11 @@
 		vopl_out: port {
 			#address-cells = <1>;
 			#size-cells = <0>;
+
+			vopl_out_lvds: endpoint@0 {
+				reg = <0>;
+				remote-endpoint = <&lvds_vopl_in>;
+			};
 		};
 	};
 
-- 
2.20.1

[PATCH 11/12] arm64: dts: rockchip: Add PX30 DSI DPHY

[Miquel Raynal]

Add the PHY which outputs MIPI DSI and LVDS.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 arch/arm64/boot/dts/rockchip/px30.dtsi | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/px30.dtsi b/arch/arm64/boot/dts/rockchip/px30.dtsi
index 1c96ba556daf..337306281c07 100644
--- a/arch/arm64/boot/dts/rockchip/px30.dtsi
+++ b/arch/arm64/boot/dts/rockchip/px30.dtsi
@@ -859,6 +859,17 @@
 		};
 	};
 
+	dsi_dphy: phy@ff2e0000 {
+		compatible = "rockchip,px30-dsi-dphy";
+		reg = <0x0 0xff2e0000 0x0 0x10000>;
+		clocks = <&pmucru SCLK_MIPIDSIPHY_REF>, <&cru PCLK_MIPIDSIPHY>;
+		clock-names = "ref", "pclk";
+		resets = <&cru SRST_MIPIDSIPHY_P>;
+		reset-names = "apb";
+		#phy-cells = <0>;
+		status = "disabled";
+	};
+
 	usb20_otg: usb@ff300000 {
 		compatible = "rockchip,px30-usb", "rockchip,rk3066-usb",
 			     "snps,dwc2";
-- 
2.20.1

[PATCH 12/12] arm64: dts: rockchip: Add PX30 LVDS

[Miquel Raynal]

Also create to port endpoints to link with the CRTCs.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 arch/arm64/boot/dts/rockchip/px30.dtsi | 27 ++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/px30.dtsi b/arch/arm64/boot/dts/rockchip/px30.dtsi
index 337306281c07..347432fba865 100644
--- a/arch/arm64/boot/dts/rockchip/px30.dtsi
+++ b/arch/arm64/boot/dts/rockchip/px30.dtsi
@@ -449,6 +449,33 @@
 			compatible = "rockchip,px30-io-voltage-domain";
 			status = "disabled";
 		};
+
+		lvds: lvds {
+			compatible = "rockchip,px30-lvds";
+			rockchip,grf = <&grf>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			phys = <&dsi_dphy>;
+			phy-names = "dphy";
+			rockchip,output = "lvds";
+			status = "disabled";
+
+			port@0 {
+				reg = <0>;
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				lvds_vopb_in: endpoint@0 {
+					reg = <0>;
+					remote-endpoint = <&vopb_out_lvds>;
+				};
+
+				lvds_vopl_in: endpoint@1 {
+					reg = <1>;
+					remote-endpoint = <&vopl_out_lvds>;
+				};
+			};
+		};
 	};
 
 	uart1: serial@ff158000 {
-- 
2.20.1

Re: [PATCH 10/12] arm64: dts: rockchip: Add PX30 CRTCs graph LVDS endpoints

[Heiko Stübner]

Hi Miquel,

Am Freitag, 13. Dezember 2019, 19:10:49 CET schrieb Miquel Raynal:
> Add the display subsystem routes with the two available CRTCs: vopb
> and vopl (big and little). For each CRTC, add the LVDS endpoints. MIPI
> DSI endpoints will come later.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  arch/arm64/boot/dts/rockchip/px30.dtsi | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/px30.dtsi b/arch/arm64/boot/dts/rockchip/px30.dtsi
> index b2af0f02ecbe..1c96ba556daf 100644
> --- a/arch/arm64/boot/dts/rockchip/px30.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/px30.dtsi
> @@ -190,6 +190,16 @@
>  		compatible = "rockchip,display-subsystem";
>  		ports = <&vopb_out>, <&vopl_out>;
>  		status = "disabled";
> +
> +		route {
> +			route_vopb_lvds: route-vopb-lvds {
> +				connect = <&vopb_out_lvds>;
> +			};
> +
> +			route_vopl_lvds: route-vopl-lvds {
> +				connect = <&vopl_out_lvds>;
> +			};
> +		};

where does this route-stuff come from?
The vendor tree? Because so far I've not seen this in mainline-drm
in general nor the Rockchip drm driver itself.


>  	};
>  
>  	gmac_clkin: external-gmac-clock {
> @@ -976,6 +986,11 @@
>  		vopb_out: port {
>  			#address-cells = <1>;
>  			#size-cells = <0>;
> +
> +			vopb_out_lvds: endpoint@0 {
> +				reg = <0>;
> +				remote-endpoint = <&lvds_vopb_in>;
> +			};

This (and the one below) would create dangling phandle references
and compile errors, because the referenced phandles only get introduced
in patch12. So ideally merge this into the last patch.


Heiko

>  		};
>  	};
>  
> @@ -1008,6 +1023,11 @@
>  		vopl_out: port {
>  			#address-cells = <1>;
>  			#size-cells = <0>;
> +
> +			vopl_out_lvds: endpoint@0 {
> +				reg = <0>;
> +				remote-endpoint = <&lvds_vopl_in>;
> +			};
>  		};
>  	};
>  
> 

Re: [PATCH 10/12] arm64: dts: rockchip: Add PX30 CRTCs graph LVDS endpoints

[Miquel Raynal]

Hi Heiko,

Heiko Stübner <heiko@sntech.de> wrote on Fri, 13 Dec 2019 19:28:21
+0100:

> Hi Miquel,
> 
> Am Freitag, 13. Dezember 2019, 19:10:49 CET schrieb Miquel Raynal:
> > Add the display subsystem routes with the two available CRTCs: vopb
> > and vopl (big and little). For each CRTC, add the LVDS endpoints. MIPI
> > DSI endpoints will come later.
> > 
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
> >  arch/arm64/boot/dts/rockchip/px30.dtsi | 20 ++++++++++++++++++++
> >  1 file changed, 20 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/rockchip/px30.dtsi b/arch/arm64/boot/dts/rockchip/px30.dtsi
> > index b2af0f02ecbe..1c96ba556daf 100644
> > --- a/arch/arm64/boot/dts/rockchip/px30.dtsi
> > +++ b/arch/arm64/boot/dts/rockchip/px30.dtsi
> > @@ -190,6 +190,16 @@
> >  		compatible = "rockchip,display-subsystem";
> >  		ports = <&vopb_out>, <&vopl_out>;
> >  		status = "disabled";
> > +
> > +		route {
> > +			route_vopb_lvds: route-vopb-lvds {
> > +				connect = <&vopb_out_lvds>;
> > +			};
> > +
> > +			route_vopl_lvds: route-vopl-lvds {
> > +				connect = <&vopl_out_lvds>;
> > +			};
> > +		};  
> 
> where does this route-stuff come from?
> The vendor tree? Because so far I've not seen this in mainline-drm
> in general nor the Rockchip drm driver itself.

Yes it comes from the vendor tree, I added a few things from the vendor
tree in the "I really want this panel to work" phase and I forgot to
check if it could be removed, I'll probably drop this.
 
> 
> 
> >  	};
> >  
> >  	gmac_clkin: external-gmac-clock {
> > @@ -976,6 +986,11 @@
> >  		vopb_out: port {
> >  			#address-cells = <1>;
> >  			#size-cells = <0>;
> > +
> > +			vopb_out_lvds: endpoint@0 {
> > +				reg = <0>;
> > +				remote-endpoint = <&lvds_vopb_in>;
> > +			};  
> 
> This (and the one below) would create dangling phandle references
> and compile errors, because the referenced phandles only get introduced
> in patch12. So ideally merge this into the last patch.

Actually patch 12 also references these nodes so I should merge them.

> 
> 
> Heiko
> 
> >  		};
> >  	};
> >  
> > @@ -1008,6 +1023,11 @@
> >  		vopl_out: port {
> >  			#address-cells = <1>;
> >  			#size-cells = <0>;
> > +
> > +			vopl_out_lvds: endpoint@0 {
> > +				reg = <0>;
> > +				remote-endpoint = <&lvds_vopl_in>;
> > +			};
> >  		};
> >  	};
> >  
> >   
> 
> 
> 
> 

Thanks for the review!
Miquèl

Re: [PATCH 02/12] dt-bindings: display: rockchip-lvds: Document PX30 PHY

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1101 bytes --]

On Fri, Dec 13, 2019 at 07:10:41PM +0100, Miquel Raynal wrote:
> PX30 SoCs use a single PHY shared by two display pipelines: MIPI DSI
> and LVDS. In the case of the LVDS IP, document the possibility to fill
> a PHY handle.
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  .../devicetree/bindings/display/rockchip/rockchip-lvds.txt     | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/display/rockchip/rockchip-lvds.txt b/Documentation/devicetree/bindings/display/rockchip/rockchip-lvds.txt
> index aa5663a6fd42..ec7b4341cfd2 100644
> --- a/Documentation/devicetree/bindings/display/rockchip/rockchip-lvds.txt
> +++ b/Documentation/devicetree/bindings/display/rockchip/rockchip-lvds.txt
> @@ -19,6 +19,9 @@ Required properties:
>  - rockchip,grf: phandle to the general register files syscon
>  - rockchip,output: "rgb", "lvds" or "duallvds", This describes the output interface
>
> +- phys: LVDS/DSI DPHY (px30 only)
> +- phy-names: name of the PHY, should be "dphy"

Should or must?

Also, phy-names is optional only for px30

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 09/12] drm/rockchip: lvds: Add PX30 support

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1561 bytes --]

Hi,

On Fri, Dec 13, 2019 at 07:10:48PM +0100, Miquel Raynal wrote:
> +static int px30_lvds_grf_config(struct drm_encoder *encoder,
> +				struct drm_display_mode *mode)
> +{
> +	struct rockchip_lvds *lvds = encoder_to_lvds(encoder);
> +	u8 nhsync = !(mode->flags & DRM_MODE_FLAG_PHSYNC);
> +	u8 nvsync = !(mode->flags & DRM_MODE_FLAG_PVSYNC);
> +	u8 ndclk = !(mode->flags & DRM_MODE_FLAG_PCSYNC);
> +	int ret;
> +
> +	if (lvds->output != DISPLAY_OUTPUT_LVDS) {
> +		DRM_DEV_ERROR(lvds->dev, "Unsupported display output %d\n",
> +			      lvds->output);
> +		return -EINVAL;
> +	}
> +
> +	if (nhsync ^ nvsync) {
> +		DRM_DEV_ERROR(lvds->dev, "Unsupported Hsync/Vsync polarity\n");
> +		return -EINVAL;
> +	}
> +
> +	/* Set format */
> +	ret = regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON1,
> +				 PX30_LVDS_FORMAT(lvds->format),
> +				 PX30_LVDS_FORMAT(lvds->format));
> +	if (ret)
> +		return ret;
> +
> +	/* Control Hsync/Vsync polarity */
> +	ret = regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON0,
> +				 PX30_LVDS_TIE_CLKS(1),
> +				 PX30_LVDS_TIE_CLKS(1));
> +	if (ret)
> +		return ret;
> +
> +	/* Set Hsync/Vsync polarity */
> +	ret = regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON0,
> +				 PX30_LVDS_INVERT_CLKS(1),
> +				 PX30_LVDS_INVERT_CLKS(nhsync));
> +	if (ret)
> +		return ret;

I don't know the hardware but it seems pretty weird to me. hsync and
vsync in LVDS are not clocks (or even signals), they're a bit in the
payload. Is there any explanation in the datasheet (or even a
datasheet in the first place)?

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 09/12] drm/rockchip: lvds: Add PX30 support

[Miquel Raynal]

Hi Maxime,

Maxime Ripard <maxime@cerno.tech> wrote on Mon, 16 Dec 2019 11:58:27
+0100:

> Hi,
> 
> On Fri, Dec 13, 2019 at 07:10:48PM +0100, Miquel Raynal wrote:
> > +static int px30_lvds_grf_config(struct drm_encoder *encoder,
> > +				struct drm_display_mode *mode)
> > +{
> > +	struct rockchip_lvds *lvds = encoder_to_lvds(encoder);
> > +	u8 nhsync = !(mode->flags & DRM_MODE_FLAG_PHSYNC);
> > +	u8 nvsync = !(mode->flags & DRM_MODE_FLAG_PVSYNC);
> > +	u8 ndclk = !(mode->flags & DRM_MODE_FLAG_PCSYNC);
> > +	int ret;
> > +
> > +	if (lvds->output != DISPLAY_OUTPUT_LVDS) {
> > +		DRM_DEV_ERROR(lvds->dev, "Unsupported display output %d\n",
> > +			      lvds->output);
> > +		return -EINVAL;
> > +	}
> > +
> > +	if (nhsync ^ nvsync) {
> > +		DRM_DEV_ERROR(lvds->dev, "Unsupported Hsync/Vsync polarity\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	/* Set format */
> > +	ret = regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON1,
> > +				 PX30_LVDS_FORMAT(lvds->format),
> > +				 PX30_LVDS_FORMAT(lvds->format));
> > +	if (ret)
> > +		return ret;
> > +
> > +	/* Control Hsync/Vsync polarity */
> > +	ret = regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON0,
> > +				 PX30_LVDS_TIE_CLKS(1),
> > +				 PX30_LVDS_TIE_CLKS(1));
> > +	if (ret)
> > +		return ret;
> > +
> > +	/* Set Hsync/Vsync polarity */
> > +	ret = regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON0,
> > +				 PX30_LVDS_INVERT_CLKS(1),
> > +				 PX30_LVDS_INVERT_CLKS(nhsync));
> > +	if (ret)
> > +		return ret;  
> 
> I don't know the hardware but it seems pretty weird to me. hsync and
> vsync in LVDS are not clocks (or even signals), they're a bit in the
> payload. Is there any explanation in the datasheet (or even a
> datasheet in the first place)?

There is no explanation about this in the PX30 TRM part 1 (public). But
you are right the naming is weird. Could the "tie clocks" thing above
mean something to you/people knowing the LVDS world?

Cheers,
Miquèl

Re: [PATCH 09/12] drm/rockchip: lvds: Add PX30 support

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 2135 bytes --]

On Mon, Dec 16, 2019 at 12:03:12PM +0100, Miquel Raynal wrote:
> Maxime Ripard <maxime@cerno.tech> wrote on Mon, 16 Dec 2019 11:58:27 +0100:
> > Hi,
> >
> > On Fri, Dec 13, 2019 at 07:10:48PM +0100, Miquel Raynal wrote:
> > > +static int px30_lvds_grf_config(struct drm_encoder *encoder,
> > > +				struct drm_display_mode *mode)
> > > +{
> > > +	struct rockchip_lvds *lvds = encoder_to_lvds(encoder);
> > > +	u8 nhsync = !(mode->flags & DRM_MODE_FLAG_PHSYNC);
> > > +	u8 nvsync = !(mode->flags & DRM_MODE_FLAG_PVSYNC);
> > > +	u8 ndclk = !(mode->flags & DRM_MODE_FLAG_PCSYNC);
> > > +	int ret;
> > > +
> > > +	if (lvds->output != DISPLAY_OUTPUT_LVDS) {
> > > +		DRM_DEV_ERROR(lvds->dev, "Unsupported display output %d\n",
> > > +			      lvds->output);
> > > +		return -EINVAL;
> > > +	}
> > > +
> > > +	if (nhsync ^ nvsync) {
> > > +		DRM_DEV_ERROR(lvds->dev, "Unsupported Hsync/Vsync polarity\n");
> > > +		return -EINVAL;
> > > +	}
> > > +
> > > +	/* Set format */
> > > +	ret = regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON1,
> > > +				 PX30_LVDS_FORMAT(lvds->format),
> > > +				 PX30_LVDS_FORMAT(lvds->format));
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	/* Control Hsync/Vsync polarity */
> > > +	ret = regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON0,
> > > +				 PX30_LVDS_TIE_CLKS(1),
> > > +				 PX30_LVDS_TIE_CLKS(1));
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	/* Set Hsync/Vsync polarity */
> > > +	ret = regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON0,
> > > +				 PX30_LVDS_INVERT_CLKS(1),
> > > +				 PX30_LVDS_INVERT_CLKS(nhsync));
> > > +	if (ret)
> > > +		return ret;
> >
> > I don't know the hardware but it seems pretty weird to me. hsync and
> > vsync in LVDS are not clocks (or even signals), they're a bit in the
> > payload. Is there any explanation in the datasheet (or even a
> > datasheet in the first place)?
>
> There is no explanation about this in the PX30 TRM part 1 (public). But
> you are right the naming is weird. Could the "tie clocks" thing above
> mean something to you/people knowing the LVDS world?

I have no idea what that could mean :)

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 09/12] drm/rockchip: lvds: Add PX30 support【请注意，邮件由linux-rockchip-bounces+andy.yan=rock-chips.com@lists.infradead.org代发】

[Andy Yan]

Hi Miquel:

Thanks for your work here.

A discussion about the grf write macro bellow.

On 12/14/19 2:10 AM, Miquel Raynal wrote:
> Introduce PX30 LVDS support. This means adding the relevant helper
> functions, a specific probe and also the initialization of a specific
> PHY.
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>   drivers/gpu/drm/rockchip/rockchip_lvds.c | 173 +++++++++++++++++++++++
>   drivers/gpu/drm/rockchip/rockchip_lvds.h |  14 ++
>   2 files changed, 187 insertions(+)
>
> diff --git a/drivers/gpu/drm/rockchip/rockchip_lvds.c b/drivers/gpu/drm/rockchip/rockchip_lvds.c
> index a0c203dcd66f..e550c2f102e0 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_lvds.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_lvds.c
> @@ -10,6 +10,7 @@
>   #include <linux/component.h>
>   #include <linux/mfd/syscon.h>
>   #include <linux/of_graph.h>
> +#include <linux/phy/phy.h>
>   #include <linux/pinctrl/devinfo.h>
>   #include <linux/platform_device.h>
>   #include <linux/pm_runtime.h>
> @@ -54,6 +55,7 @@ struct rockchip_lvds {
>   	void __iomem *regs;
>   	struct regmap *grf;
>   	struct clk *pclk;
> +	struct phy *dphy;
>   	const struct rockchip_lvds_soc_data *soc_data;
>   	int output; /* rgb lvds or dual lvds output */
>   	int format; /* vesa or jeida format */
> @@ -322,6 +324,133 @@ static void rk3288_lvds_encoder_disable(struct drm_encoder *encoder)
>   	drm_panel_unprepare(lvds->panel);
>   }
>   
> +static int px30_lvds_poweron(struct rockchip_lvds *lvds)
> +{
> +	int ret;
> +
> +	ret = pm_runtime_get_sync(lvds->dev);
> +	if (ret < 0) {
> +		DRM_DEV_ERROR(lvds->dev, "failed to get pm runtime: %d\n", ret);
> +		return ret;
> +	}
> +
> +	/* Enable LVDS mode */
> +	return regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON1,
> +				  PX30_LVDS_MODE_EN(1) | PX30_LVDS_P2S_EN(1),
> +				  PX30_LVDS_MODE_EN(1) | PX30_LVDS_P2S_EN(1));
> +}
> +
> +static void px30_lvds_poweroff(struct rockchip_lvds *lvds)
> +{
> +	regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON1,
> +			   PX30_LVDS_MODE_EN(1) | PX30_LVDS_P2S_EN(1),
> +			   PX30_LVDS_MODE_EN(0) | PX30_LVDS_P2S_EN(0));
> +
> +	pm_runtime_put(lvds->dev);
> +}
> +
> +static int px30_lvds_grf_config(struct drm_encoder *encoder,
> +				struct drm_display_mode *mode)
> +{
> +	struct rockchip_lvds *lvds = encoder_to_lvds(encoder);
> +	u8 nhsync = !(mode->flags & DRM_MODE_FLAG_PHSYNC);
> +	u8 nvsync = !(mode->flags & DRM_MODE_FLAG_PVSYNC);
> +	u8 ndclk = !(mode->flags & DRM_MODE_FLAG_PCSYNC);
> +	int ret;
> +
> +	if (lvds->output != DISPLAY_OUTPUT_LVDS) {
> +		DRM_DEV_ERROR(lvds->dev, "Unsupported display output %d\n",
> +			      lvds->output);
> +		return -EINVAL;
> +	}
> +
> +	if (nhsync ^ nvsync) {
> +		DRM_DEV_ERROR(lvds->dev, "Unsupported Hsync/Vsync polarity\n");
> +		return -EINVAL;
> +	}
> +
> +	/* Set format */
> +	ret = regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON1,
> +				 PX30_LVDS_FORMAT(lvds->format),
> +				 PX30_LVDS_FORMAT(lvds->format));
> +	if (ret)
> +		return ret;
> +
> +	/* Control Hsync/Vsync polarity */
> +	ret = regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON0,
> +				 PX30_LVDS_TIE_CLKS(1),
> +				 PX30_LVDS_TIE_CLKS(1));
> +	if (ret)
> +		return ret;
> +
> +	/* Set Hsync/Vsync polarity */
> +	ret = regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON0,
> +				 PX30_LVDS_INVERT_CLKS(1),
> +				 PX30_LVDS_INVERT_CLKS(nhsync));
> +	if (ret)
> +		return ret;
> +
> +	/* Set dclk polarity */
> +	return regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON0,
> +				  PX30_LVDS_INVERT_DCLK(1),
> +				  PX30_LVDS_INVERT_DCLK(ndclk));
> +}
> +
> +static int px30_lvds_set_vop_source(struct rockchip_lvds *lvds,
> +				    struct drm_encoder *encoder)
> +{
> +	int vop;
> +
> +	vop = drm_of_encoder_active_endpoint_id(lvds->dev->of_node, encoder);
> +	if (vop < 0)
> +		return vop;
> +
> +	return regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON1,
> +				  PX30_LVDS_VOP_SEL(1),
> +				  PX30_LVDS_VOP_SEL(vop));
> +}
> +
> +static void px30_lvds_encoder_enable(struct drm_encoder *encoder)
> +{
> +	struct rockchip_lvds *lvds = encoder_to_lvds(encoder);
> +	struct drm_display_mode *mode = &encoder->crtc->state->adjusted_mode;
> +	int ret;
> +
> +	drm_panel_prepare(lvds->panel);
> +
> +	ret = px30_lvds_poweron(lvds);
> +	if (ret) {
> +		DRM_DEV_ERROR(lvds->dev, "failed to power on LVDS: %d\n", ret);
> +		drm_panel_unprepare(lvds->panel);
> +		return;
> +	}
> +
> +	ret = px30_lvds_grf_config(encoder, mode);
> +	if (ret) {
> +		DRM_DEV_ERROR(lvds->dev, "failed to configure LVDS: %d\n", ret);
> +		drm_panel_unprepare(lvds->panel);
> +		return;
> +	}
> +
> +	ret = px30_lvds_set_vop_source(lvds, encoder);
> +	if (ret) {
> +		DRM_DEV_ERROR(lvds->dev, "failed to set VOP source: %d\n", ret);
> +		drm_panel_unprepare(lvds->panel);
> +		return;
> +	}
> +
> +	drm_panel_enable(lvds->panel);
> +}
> +
> +static void px30_lvds_encoder_disable(struct drm_encoder *encoder)
> +{
> +	struct rockchip_lvds *lvds = encoder_to_lvds(encoder);
> +
> +	drm_panel_disable(lvds->panel);
> +	px30_lvds_poweroff(lvds);
> +	drm_panel_unprepare(lvds->panel);
> +}
> +
>   static const
>   struct drm_encoder_helper_funcs rk3288_lvds_encoder_helper_funcs = {
>   	.enable = rk3288_lvds_encoder_enable,
> @@ -329,6 +458,13 @@ struct drm_encoder_helper_funcs rk3288_lvds_encoder_helper_funcs = {
>   	.atomic_check = rockchip_lvds_encoder_atomic_check,
>   };
>   
> +static const
> +struct drm_encoder_helper_funcs px30_lvds_encoder_helper_funcs = {
> +	.enable = px30_lvds_encoder_enable,
> +	.disable = px30_lvds_encoder_disable,
> +	.atomic_check = rockchip_lvds_encoder_atomic_check,
> +};
> +
>   static const struct drm_encoder_funcs rockchip_lvds_encoder_funcs = {
>   	.destroy = drm_encoder_cleanup,
>   };
> @@ -379,16 +515,53 @@ static int rk3288_lvds_probe(struct platform_device *pdev,
>   	return 0;
>   }
>   
> +static int px30_lvds_probe(struct platform_device *pdev,
> +			   struct rockchip_lvds *lvds)
> +{
> +	int ret;
> +
> +	/* MSB */
> +	ret =  regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON1,
> +				  PX30_LVDS_MSBSEL(1),
> +				  PX30_LVDS_MSBSEL(1));
> +	if (ret)
> +		return ret;
> +
> +	/* PHY */
> +	lvds->dphy = devm_phy_get(&pdev->dev, "dphy");
> +	if (IS_ERR(lvds->dphy))
> +		return PTR_ERR(lvds->dphy);
> +
> +	phy_init(lvds->dphy);
> +	if (ret)
> +		return ret;
> +
> +	phy_set_mode(lvds->dphy, PHY_MODE_LVDS);
> +	if (ret)
> +		return ret;
> +
> +	return phy_power_on(lvds->dphy);
> +}
> +
>   static const struct rockchip_lvds_soc_data rk3288_lvds_data = {
>   	.probe = rk3288_lvds_probe,
>   	.helper_funcs = &rk3288_lvds_encoder_helper_funcs,
>   };
>   
> +static const struct rockchip_lvds_soc_data px30_lvds_data = {
> +	.probe = px30_lvds_probe,
> +	.helper_funcs = &px30_lvds_encoder_helper_funcs,
> +};
> +
>   static const struct of_device_id rockchip_lvds_dt_ids[] = {
>   	{
>   		.compatible = "rockchip,rk3288-lvds",
>   		.data = &rk3288_lvds_data
>   	},
> +	{
> +		.compatible = "rockchip,px30-lvds",
> +		.data = &px30_lvds_data
> +	},
>   	{}
>   };
>   MODULE_DEVICE_TABLE(of, rockchip_lvds_dt_ids);
> diff --git a/drivers/gpu/drm/rockchip/rockchip_lvds.h b/drivers/gpu/drm/rockchip/rockchip_lvds.h
> index e41e9ab3c306..7cfb102b4854 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_lvds.h
> +++ b/drivers/gpu/drm/rockchip/rockchip_lvds.h
> @@ -106,4 +106,18 @@
>   #define LVDS_VESA_18				2
>   #define LVDS_JEIDA_18				3
>   
> +#define WRITE_EN(v, h, l)  ((GENMASK(h, l) << 16) | (v << l))


How about rename WRITE_EN to HIWORD_UPDATE to keep align with other 
modules that write grf: such as 
dwmac-rk.c/dw-mipi-dsi-rockhip.c/dw-hdmi-rockchip.c

> +
> +#define PX30_LVDS_GRF_PD_VO_CON0		0x434
> +#define   PX30_LVDS_TIE_CLKS(val)		WRITE_EN(val,  8,  8)
> +#define   PX30_LVDS_INVERT_CLKS(val)		WRITE_EN(val,  9,  9)
> +#define   PX30_LVDS_INVERT_DCLK(val)		WRITE_EN(val,  5,  5)
> +
> +#define PX30_LVDS_GRF_PD_VO_CON1		0x438
> +#define   PX30_LVDS_FORMAT(val)			WRITE_EN(val, 14, 13)
> +#define   PX30_LVDS_MODE_EN(val)		WRITE_EN(val, 12, 12)
> +#define   PX30_LVDS_MSBSEL(val)			WRITE_EN(val, 11, 11)
> +#define   PX30_LVDS_P2S_EN(val)			WRITE_EN(val,  6,  6)
> +#define   PX30_LVDS_VOP_SEL(val)		WRITE_EN(val,  1,  1)
> +
>   #endif /* _ROCKCHIP_LVDS_ */

Re: [PATCH 09/12] drm/rockchip: lvds: Add PX30 support【请注意，邮件由linux-rockchip-bounces+andy.yan=rock-chips.com@lists.infradead.org代发】

[Miquel Raynal]

Hi Andy,

Andy Yan <andy.yan@rock-chips.com> wrote on Mon, 16 Dec 2019 20:00:31
+0800:

> Hi Miquel:
> 
> Thanks for your work here.
> 
> A discussion about the grf write macro bellow.
> 
> On 12/14/19 2:10 AM, Miquel Raynal wrote:
> > Introduce PX30 LVDS support. This means adding the relevant helper
> > functions, a specific probe and also the initialization of a specific
> > PHY.
> >
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
> >   drivers/gpu/drm/rockchip/rockchip_lvds.c | 173 +++++++++++++++++++++++
> >   drivers/gpu/drm/rockchip/rockchip_lvds.h |  14 ++
> >   2 files changed, 187 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/rockchip/rockchip_lvds.c b/drivers/gpu/drm/rockchip/rockchip_lvds.c
> > index a0c203dcd66f..e550c2f102e0 100644
> > --- a/drivers/gpu/drm/rockchip/rockchip_lvds.c
> > +++ b/drivers/gpu/drm/rockchip/rockchip_lvds.c
> > @@ -10,6 +10,7 @@
> >   #include <linux/component.h>
> >   #include <linux/mfd/syscon.h>
> >   #include <linux/of_graph.h>
> > +#include <linux/phy/phy.h>
> >   #include <linux/pinctrl/devinfo.h>
> >   #include <linux/platform_device.h>
> >   #include <linux/pm_runtime.h>
> > @@ -54,6 +55,7 @@ struct rockchip_lvds {
> >   	void __iomem *regs;
> >   	struct regmap *grf;
> >   	struct clk *pclk;
> > +	struct phy *dphy;
> >   	const struct rockchip_lvds_soc_data *soc_data;
> >   	int output; /* rgb lvds or dual lvds output */
> >   	int format; /* vesa or jeida format */
> > @@ -322,6 +324,133 @@ static void rk3288_lvds_encoder_disable(struct drm_encoder *encoder)
> >   	drm_panel_unprepare(lvds->panel);
> >   }  
> >   > +static int px30_lvds_poweron(struct rockchip_lvds *lvds)  
> > +{
> > +	int ret;
> > +
> > +	ret = pm_runtime_get_sync(lvds->dev);
> > +	if (ret < 0) {
> > +		DRM_DEV_ERROR(lvds->dev, "failed to get pm runtime: %d\n", ret);
> > +		return ret;
> > +	}
> > +
> > +	/* Enable LVDS mode */
> > +	return regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON1,
> > +				  PX30_LVDS_MODE_EN(1) | PX30_LVDS_P2S_EN(1),
> > +				  PX30_LVDS_MODE_EN(1) | PX30_LVDS_P2S_EN(1));
> > +}
> > +
> > +static void px30_lvds_poweroff(struct rockchip_lvds *lvds)
> > +{
> > +	regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON1,
> > +			   PX30_LVDS_MODE_EN(1) | PX30_LVDS_P2S_EN(1),
> > +			   PX30_LVDS_MODE_EN(0) | PX30_LVDS_P2S_EN(0));
> > +
> > +	pm_runtime_put(lvds->dev);
> > +}
> > +
> > +static int px30_lvds_grf_config(struct drm_encoder *encoder,
> > +				struct drm_display_mode *mode)
> > +{
> > +	struct rockchip_lvds *lvds = encoder_to_lvds(encoder);
> > +	u8 nhsync = !(mode->flags & DRM_MODE_FLAG_PHSYNC);
> > +	u8 nvsync = !(mode->flags & DRM_MODE_FLAG_PVSYNC);
> > +	u8 ndclk = !(mode->flags & DRM_MODE_FLAG_PCSYNC);
> > +	int ret;
> > +
> > +	if (lvds->output != DISPLAY_OUTPUT_LVDS) {
> > +		DRM_DEV_ERROR(lvds->dev, "Unsupported display output %d\n",
> > +			      lvds->output);
> > +		return -EINVAL;
> > +	}
> > +
> > +	if (nhsync ^ nvsync) {
> > +		DRM_DEV_ERROR(lvds->dev, "Unsupported Hsync/Vsync polarity\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	/* Set format */
> > +	ret = regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON1,
> > +				 PX30_LVDS_FORMAT(lvds->format),
> > +				 PX30_LVDS_FORMAT(lvds->format));
> > +	if (ret)
> > +		return ret;
> > +
> > +	/* Control Hsync/Vsync polarity */
> > +	ret = regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON0,
> > +				 PX30_LVDS_TIE_CLKS(1),
> > +				 PX30_LVDS_TIE_CLKS(1));
> > +	if (ret)
> > +		return ret;
> > +
> > +	/* Set Hsync/Vsync polarity */
> > +	ret = regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON0,
> > +				 PX30_LVDS_INVERT_CLKS(1),
> > +				 PX30_LVDS_INVERT_CLKS(nhsync));
> > +	if (ret)
> > +		return ret;
> > +
> > +	/* Set dclk polarity */
> > +	return regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON0,
> > +				  PX30_LVDS_INVERT_DCLK(1),
> > +				  PX30_LVDS_INVERT_DCLK(ndclk));
> > +}
> > +
> > +static int px30_lvds_set_vop_source(struct rockchip_lvds *lvds,
> > +				    struct drm_encoder *encoder)
> > +{
> > +	int vop;
> > +
> > +	vop = drm_of_encoder_active_endpoint_id(lvds->dev->of_node, encoder);
> > +	if (vop < 0)
> > +		return vop;
> > +
> > +	return regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON1,
> > +				  PX30_LVDS_VOP_SEL(1),
> > +				  PX30_LVDS_VOP_SEL(vop));
> > +}
> > +
> > +static void px30_lvds_encoder_enable(struct drm_encoder *encoder)
> > +{
> > +	struct rockchip_lvds *lvds = encoder_to_lvds(encoder);
> > +	struct drm_display_mode *mode = &encoder->crtc->state->adjusted_mode;
> > +	int ret;
> > +
> > +	drm_panel_prepare(lvds->panel);
> > +
> > +	ret = px30_lvds_poweron(lvds);
> > +	if (ret) {
> > +		DRM_DEV_ERROR(lvds->dev, "failed to power on LVDS: %d\n", ret);
> > +		drm_panel_unprepare(lvds->panel);
> > +		return;
> > +	}
> > +
> > +	ret = px30_lvds_grf_config(encoder, mode);
> > +	if (ret) {
> > +		DRM_DEV_ERROR(lvds->dev, "failed to configure LVDS: %d\n", ret);
> > +		drm_panel_unprepare(lvds->panel);
> > +		return;
> > +	}
> > +
> > +	ret = px30_lvds_set_vop_source(lvds, encoder);
> > +	if (ret) {
> > +		DRM_DEV_ERROR(lvds->dev, "failed to set VOP source: %d\n", ret);
> > +		drm_panel_unprepare(lvds->panel);
> > +		return;
> > +	}
> > +
> > +	drm_panel_enable(lvds->panel);
> > +}
> > +
> > +static void px30_lvds_encoder_disable(struct drm_encoder *encoder)
> > +{
> > +	struct rockchip_lvds *lvds = encoder_to_lvds(encoder);
> > +
> > +	drm_panel_disable(lvds->panel);
> > +	px30_lvds_poweroff(lvds);
> > +	drm_panel_unprepare(lvds->panel);
> > +}
> > +
> >   static const
> >   struct drm_encoder_helper_funcs rk3288_lvds_encoder_helper_funcs = {
> >   	.enable = rk3288_lvds_encoder_enable,
> > @@ -329,6 +458,13 @@ struct drm_encoder_helper_funcs rk3288_lvds_encoder_helper_funcs = {
> >   	.atomic_check = rockchip_lvds_encoder_atomic_check,
> >   };  
> >   > +static const  
> > +struct drm_encoder_helper_funcs px30_lvds_encoder_helper_funcs = {
> > +	.enable = px30_lvds_encoder_enable,
> > +	.disable = px30_lvds_encoder_disable,
> > +	.atomic_check = rockchip_lvds_encoder_atomic_check,
> > +};
> > +
> >   static const struct drm_encoder_funcs rockchip_lvds_encoder_funcs = {
> >   	.destroy = drm_encoder_cleanup,
> >   };
> > @@ -379,16 +515,53 @@ static int rk3288_lvds_probe(struct platform_device *pdev,
> >   	return 0;
> >   }  
> >   > +static int px30_lvds_probe(struct platform_device *pdev,  
> > +			   struct rockchip_lvds *lvds)
> > +{
> > +	int ret;
> > +
> > +	/* MSB */
> > +	ret =  regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON1,
> > +				  PX30_LVDS_MSBSEL(1),
> > +				  PX30_LVDS_MSBSEL(1));
> > +	if (ret)
> > +		return ret;
> > +
> > +	/* PHY */
> > +	lvds->dphy = devm_phy_get(&pdev->dev, "dphy");
> > +	if (IS_ERR(lvds->dphy))
> > +		return PTR_ERR(lvds->dphy);
> > +
> > +	phy_init(lvds->dphy);
> > +	if (ret)
> > +		return ret;
> > +
> > +	phy_set_mode(lvds->dphy, PHY_MODE_LVDS);
> > +	if (ret)
> > +		return ret;
> > +
> > +	return phy_power_on(lvds->dphy);
> > +}
> > +
> >   static const struct rockchip_lvds_soc_data rk3288_lvds_data = {
> >   	.probe = rk3288_lvds_probe,
> >   	.helper_funcs = &rk3288_lvds_encoder_helper_funcs,
> >   };  
> >   > +static const struct rockchip_lvds_soc_data px30_lvds_data = {  
> > +	.probe = px30_lvds_probe,
> > +	.helper_funcs = &px30_lvds_encoder_helper_funcs,
> > +};
> > +
> >   static const struct of_device_id rockchip_lvds_dt_ids[] = {
> >   	{
> >   		.compatible = "rockchip,rk3288-lvds",
> >   		.data = &rk3288_lvds_data
> >   	},
> > +	{
> > +		.compatible = "rockchip,px30-lvds",
> > +		.data = &px30_lvds_data
> > +	},
> >   	{}
> >   };
> >   MODULE_DEVICE_TABLE(of, rockchip_lvds_dt_ids);
> > diff --git a/drivers/gpu/drm/rockchip/rockchip_lvds.h b/drivers/gpu/drm/rockchip/rockchip_lvds.h
> > index e41e9ab3c306..7cfb102b4854 100644
> > --- a/drivers/gpu/drm/rockchip/rockchip_lvds.h
> > +++ b/drivers/gpu/drm/rockchip/rockchip_lvds.h
> > @@ -106,4 +106,18 @@
> >   #define LVDS_VESA_18				2
> >   #define LVDS_JEIDA_18				3  
> >   > +#define WRITE_EN(v, h, l)  ((GENMASK(h, l) << 16) | (v << l))  
> 
> 
> How about rename WRITE_EN to HIWORD_UPDATE to keep align with other modules that write grf: such as dwmac-rk.c/dw-mipi-dsi-rockhip.c/dw-hdmi-rockchip.c

Sure. It is also the name of this macro in the BSP but I found it so
undescriptive that I changed it. I don't like very much its new name
neither so I'll go back to the original one.

Thanks,
Miquèl

Re: [PATCH 09/12] drm/rockchip: lvds: Add PX30 support

[sandy.huang]

Hi

     some mistakes with last mail, so resend this mail.

在 2019/12/18 上午11:17, sandy.huang 写道:
> Hi Maxime & Miquel,
>
> 在 2019/12/16 下午7:14, Maxime Ripard 写道:
>> On Mon, Dec 16, 2019 at 12:03:12PM +0100, Miquel Raynal wrote:
>>> Maxime Ripard <maxime@cerno.tech> wrote on Mon, 16 Dec 2019 11:58:27 
>>> +0100:
>>>> Hi,
>>>>
>>>> On Fri, Dec 13, 2019 at 07:10:48PM +0100, Miquel Raynal wrote:
>>>>> +static int px30_lvds_grf_config(struct drm_encoder *encoder,
>>>>> +                struct drm_display_mode *mode)
>>>>> +{
>>>>> +    struct rockchip_lvds *lvds = encoder_to_lvds(encoder);
>>>>> +    u8 nhsync = !(mode->flags & DRM_MODE_FLAG_PHSYNC);
>>>>> +    u8 nvsync = !(mode->flags & DRM_MODE_FLAG_PVSYNC);
>>>>> +    u8 ndclk = !(mode->flags & DRM_MODE_FLAG_PCSYNC);
>>>>> +    int ret;
>>>>> +
>>>>> +    if (lvds->output != DISPLAY_OUTPUT_LVDS) {
>>>>> +        DRM_DEV_ERROR(lvds->dev, "Unsupported display output %d\n",
>>>>> +                  lvds->output);
>>>>> +        return -EINVAL;
>>>>> +    }
>>>>> +
>>>>> +    if (nhsync ^ nvsync) {
>>>>> +        DRM_DEV_ERROR(lvds->dev, "Unsupported Hsync/Vsync 
>>>>> polarity\n");
>>>>> +        return -EINVAL;
>>>>> +    }
>>>>> +
>>>>> +    /* Set format */
>>>>> +    ret = regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON1,
>>>>> +                 PX30_LVDS_FORMAT(lvds->format),
>>>>> +                 PX30_LVDS_FORMAT(lvds->format));
>>>>> +    if (ret)
>>>>> +        return ret;
>>>>> +
>>>>> +    /* Control Hsync/Vsync polarity */
>>>>> +    ret = regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON0,
>>>>> +                 PX30_LVDS_TIE_CLKS(1),
>>>>> +                 PX30_LVDS_TIE_CLKS(1));
>>>>> +    if (ret)
>>>>> +        return ret;
>>>>> +
>>>>> +    /* Set Hsync/Vsync polarity */
>>>>> +    ret = regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON0,
>>>>> +                 PX30_LVDS_INVERT_CLKS(1),
>>>>> +                 PX30_LVDS_INVERT_CLKS(nhsync));
>>>>> +    if (ret)
>>>>> +        return ret;
>>>> I don't know the hardware but it seems pretty weird to me. hsync and
>>>> vsync in LVDS are not clocks (or even signals), they're a bit in the
>>>> payload. Is there any explanation in the datasheet (or even a
>>>> datasheet in the first place)?
>>> There is no explanation about this in the PX30 TRM part 1 (public). But
>>> you are right the naming is weird. Could the "tie clocks" thing above
>>> mean something to you/people knowing the LVDS world?
>> I have no idea what that could mean :)
>
> This two bit(GRF_CON0[9,8]) is not for hsync/vsync polarity config, 
> the polarity is done at vop_crtc_atomic_enable@rockchip_drm_vop.c.
>
> Before px30, all rockchip platform lvds output payload is include 
> hsync,vsync and den clock signal. About years ago, we meet a lvds 
> panel can't work normally at rk3288,  from the panel spec we know the 
> panel need lvds work den mode only, the hsync/vsync need to set 0 at 
> blank time, so we add this two bit(GRF_CON0[9,8]) for this mode.
>
> but now we can't get in touch with the customer and get the panel to 
> test this function, so we can't verify the panel work unnormally is 
> because of  the den mode only and the hsync vsync signal level at 
> blank time.
>
> I recommend not including this part of configuration before we test 
> this funcion at den mode only lvds panel,thanks.
>
>>
>> Maxime
>>
>> _______________________________________________
>> Linux-rockchip mailing list
>> Linux-rockchip@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-rockchip
>

Re: [PATCH 01/12] dt-bindings: display: rockchip-lvds: Declare PX30 compatible

[Rob Herring]

On Fri, 13 Dec 2019 19:10:40 +0100, Miquel Raynal wrote:
> Document the PX30 LVDS compatible.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  .../devicetree/bindings/display/rockchip/rockchip-lvds.txt       | 1 +
>  1 file changed, 1 insertion(+)
> 

Acked-by: Rob Herring <robh@kernel.org>