[PATCH 0/3] drm/rockchip: lvds: add support for rk356x

[PATCH 0/3] drm/rockchip: lvds: add support for rk356x

[Alibek Omarov]

Hi all,

This series adds support for the LVDS controller on the Rockchip RK3566 and
RK3568. First patch adds the support in rockchip-lvds.c driver, setting all
the needed GRF registers. Second patch adds device tree bindings, and third
patch adds a note in the documentation.

LVDS controller on rk356x does share DSI DPHY with MIPI DSI, and all
groundwork on enabling it is done by Chris Morgan.

Tested on Autogramma Monitor RockChip, custom board based on Radxa Rock 3
Computing Module Plus.

Alibek Omarov (3):
  drm/rockchip: lvds: add rk3568 support
  arm64: dts: rockchip: rk356x: add LVDS bindings
  dt-bindings: display: rockchip-lvds: add compatible string for RK3568

 .../display/rockchip/rockchip-lvds.txt        |   1 +
 arch/arm64/boot/dts/rockchip/rk356x.dtsi      |  25 +++
 drivers/gpu/drm/rockchip/rockchip_lvds.c      | 144 +++++++++++++++++-
 drivers/gpu/drm/rockchip/rockchip_lvds.h      |  10 ++
 4 files changed, 173 insertions(+), 7 deletions(-)

-- 
2.34.1

[PATCH 0/3] drm/rockchip: lvds: add support for rk356x

[Alibek Omarov]

Hi all,

This series adds support for the LVDS controller on the Rockchip RK3566 and
RK3568. First patch adds the support in rockchip-lvds.c driver, setting all
the needed GRF registers. Second patch adds device tree bindings, and third
patch adds a note in the documentation.

LVDS controller on rk356x does share DSI DPHY with MIPI DSI, and all
groundwork on enabling it is done by Chris Morgan.

Tested on Autogramma Monitor RockChip, custom board based on Radxa Rock 3
Computing Module Plus.

Alibek Omarov (3):
  drm/rockchip: lvds: add rk3568 support
  arm64: dts: rockchip: rk356x: add LVDS bindings
  dt-bindings: display: rockchip-lvds: add compatible string for RK3568

 .../display/rockchip/rockchip-lvds.txt        |   1 +
 arch/arm64/boot/dts/rockchip/rk356x.dtsi      |  25 +++
 drivers/gpu/drm/rockchip/rockchip_lvds.c      | 144 +++++++++++++++++-
 drivers/gpu/drm/rockchip/rockchip_lvds.h      |  10 ++
 4 files changed, 173 insertions(+), 7 deletions(-)

-- 
2.34.1


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH 0/3] drm/rockchip: lvds: add support for rk356x

[Alibek Omarov]

Hi all,

This series adds support for the LVDS controller on the Rockchip RK3566 and
RK3568. First patch adds the support in rockchip-lvds.c driver, setting all
the needed GRF registers. Second patch adds device tree bindings, and third
patch adds a note in the documentation.

LVDS controller on rk356x does share DSI DPHY with MIPI DSI, and all
groundwork on enabling it is done by Chris Morgan.

Tested on Autogramma Monitor RockChip, custom board based on Radxa Rock 3
Computing Module Plus.

Alibek Omarov (3):
  drm/rockchip: lvds: add rk3568 support
  arm64: dts: rockchip: rk356x: add LVDS bindings
  dt-bindings: display: rockchip-lvds: add compatible string for RK3568

 .../display/rockchip/rockchip-lvds.txt        |   1 +
 arch/arm64/boot/dts/rockchip/rk356x.dtsi      |  25 +++
 drivers/gpu/drm/rockchip/rockchip_lvds.c      | 144 +++++++++++++++++-
 drivers/gpu/drm/rockchip/rockchip_lvds.h      |  10 ++
 4 files changed, 173 insertions(+), 7 deletions(-)

-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 0/3] drm/rockchip: lvds: add support for rk356x

[Alibek Omarov]

Hi all,

This series adds support for the LVDS controller on the Rockchip RK3566 and
RK3568. First patch adds the support in rockchip-lvds.c driver, setting all
the needed GRF registers. Second patch adds device tree bindings, and third
patch adds a note in the documentation.

LVDS controller on rk356x does share DSI DPHY with MIPI DSI, and all
groundwork on enabling it is done by Chris Morgan.

Tested on Autogramma Monitor RockChip, custom board based on Radxa Rock 3
Computing Module Plus.

Alibek Omarov (3):
  drm/rockchip: lvds: add rk3568 support
  arm64: dts: rockchip: rk356x: add LVDS bindings
  dt-bindings: display: rockchip-lvds: add compatible string for RK3568

 .../display/rockchip/rockchip-lvds.txt        |   1 +
 arch/arm64/boot/dts/rockchip/rk356x.dtsi      |  25 +++
 drivers/gpu/drm/rockchip/rockchip_lvds.c      | 144 +++++++++++++++++-
 drivers/gpu/drm/rockchip/rockchip_lvds.h      |  10 ++
 4 files changed, 173 insertions(+), 7 deletions(-)

-- 
2.34.1

[PATCH 1/3] drm/rockchip: lvds: add rk3568 support

[Alibek Omarov]

One of the ports of RK3568 can be configured as LVDS, re-using the DSI DPHY

Signed-off-by: Alibek Omarov <a1ba.omarov@gmail.com>
---
 drivers/gpu/drm/rockchip/rockchip_lvds.c | 144 +++++++++++++++++++++--
 drivers/gpu/drm/rockchip/rockchip_lvds.h |  10 ++
 2 files changed, 147 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_lvds.c b/drivers/gpu/drm/rockchip/rockchip_lvds.c
index 68f6ebb33460..83c60240af85 100644
--- a/drivers/gpu/drm/rockchip/rockchip_lvds.c
+++ b/drivers/gpu/drm/rockchip/rockchip_lvds.c
@@ -433,6 +433,90 @@ static void px30_lvds_encoder_disable(struct drm_encoder *encoder)
 	drm_panel_unprepare(lvds->panel);
 }
 
+static int rk3568_lvds_poweron(struct rockchip_lvds *lvds)
+{
+	int ret;
+
+	ret = clk_enable(lvds->pclk);
+	if (ret < 0) {
+		DRM_DEV_ERROR(lvds->dev, "failed to enable lvds pclk %d\n", ret);
+		return ret;
+	}
+
+	ret = pm_runtime_get_sync(lvds->dev);
+	if (ret < 0) {
+		DRM_DEV_ERROR(lvds->dev, "failed to get pm runtime: %d\n", ret);
+		clk_disable(lvds->pclk);
+		return ret;
+	}
+
+	/* Enable LVDS mode */
+	return regmap_update_bits(lvds->grf, RK3568_GRF_VO_CON2,
+				  RK3568_LVDS0_MODE_EN(1),
+				  RK3568_LVDS0_MODE_EN(1));
+}
+
+static void rk3568_lvds_poweroff(struct rockchip_lvds *lvds)
+{
+	regmap_update_bits(lvds->grf, RK3568_GRF_VO_CON2,
+			   RK3568_LVDS0_MODE_EN(1) | RK3568_LVDS0_P2S_EN(1),
+			   RK3568_LVDS0_MODE_EN(0) | RK3568_LVDS0_P2S_EN(0));
+
+	pm_runtime_put(lvds->dev);
+	clk_disable(lvds->pclk);
+}
+
+static int rk3568_lvds_grf_config(struct drm_encoder *encoder,
+				struct drm_display_mode *mode)
+{
+	struct rockchip_lvds *lvds = encoder_to_lvds(encoder);
+
+	if (lvds->output != DISPLAY_OUTPUT_LVDS) {
+		DRM_DEV_ERROR(lvds->dev, "Unsupported display output %d\n",
+			      lvds->output);
+		return -EINVAL;
+	}
+
+	/* Set format */
+	return regmap_update_bits(lvds->grf, RK3568_GRF_VO_CON0,
+				  RK3568_LVDS0_SELECT(3),
+				  RK3568_LVDS0_SELECT(lvds->format));
+}
+
+static void rk3568_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 = rk3568_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 = rk3568_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;
+	}
+
+	drm_panel_enable(lvds->panel);
+}
+
+static void rk3568_lvds_encoder_disable(struct drm_encoder *encoder)
+{
+	struct rockchip_lvds *lvds = encoder_to_lvds(encoder);
+
+	drm_panel_disable(lvds->panel);
+	rk3568_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,
@@ -447,6 +531,13 @@ struct drm_encoder_helper_funcs px30_lvds_encoder_helper_funcs = {
 	.atomic_check = rockchip_lvds_encoder_atomic_check,
 };
 
+static const
+struct drm_encoder_helper_funcs rk3568_lvds_encoder_helper_funcs = {
+	.enable = rk3568_lvds_encoder_enable,
+	.disable = rk3568_lvds_encoder_disable,
+	.atomic_check = rockchip_lvds_encoder_atomic_check,
+};
+
 static int rk3288_lvds_probe(struct platform_device *pdev,
 			     struct rockchip_lvds *lvds)
 {
@@ -491,6 +582,26 @@ static int rk3288_lvds_probe(struct platform_device *pdev,
 	return 0;
 }
 
+static int rockchip_lvds_phy_probe(struct platform_device *pdev,
+				   struct rockchip_lvds *lvds)
+{
+	int ret;
+
+	lvds->dphy = devm_phy_get(&pdev->dev, "dphy");
+	if (IS_ERR(lvds->dphy))
+		return PTR_ERR(lvds->dphy);
+
+	ret = phy_init(lvds->dphy);
+	if (ret)
+		return ret;
+
+	ret = phy_set_mode(lvds->dphy, PHY_MODE_LVDS);
+	if (ret)
+		return ret;
+
+	return phy_power_on(lvds->dphy);
+}
+
 static int px30_lvds_probe(struct platform_device *pdev,
 			   struct rockchip_lvds *lvds)
 {
@@ -503,20 +614,28 @@ static int px30_lvds_probe(struct platform_device *pdev,
 	if (ret)
 		return ret;
 
-	/* PHY */
-	lvds->dphy = devm_phy_get(&pdev->dev, "dphy");
-	if (IS_ERR(lvds->dphy))
-		return PTR_ERR(lvds->dphy);
+	return rockchip_lvds_phy_probe(pdev, lvds);
+}
 
-	ret = phy_init(lvds->dphy);
+static int rk3568_lvds_probe(struct platform_device *pdev,
+			     struct rockchip_lvds *lvds)
+{
+	int ret;
+
+	ret = regmap_update_bits(lvds->grf, RK3568_GRF_VO_CON0,
+				  RK3568_LVDS0_MSBSEL(1),
+				  RK3568_LVDS0_MSBSEL(1));
 	if (ret)
 		return ret;
 
-	ret = phy_set_mode(lvds->dphy, PHY_MODE_LVDS);
+	ret = regmap_update_bits(lvds->grf, RK3568_GRF_VO_CON2,
+				 RK3568_LVDS0_P2S_EN(1),
+				 RK3568_LVDS0_P2S_EN(1));
+
 	if (ret)
 		return ret;
 
-	return phy_power_on(lvds->dphy);
+	return rockchip_lvds_phy_probe(pdev, lvds);
 }
 
 static const struct rockchip_lvds_soc_data rk3288_lvds_data = {
@@ -529,6 +648,11 @@ static const struct rockchip_lvds_soc_data px30_lvds_data = {
 	.helper_funcs = &px30_lvds_encoder_helper_funcs,
 };
 
+static const struct rockchip_lvds_soc_data rk3568_lvds_data = {
+	.probe = rk3568_lvds_probe,
+	.helper_funcs = &rk3568_lvds_encoder_helper_funcs,
+};
+
 static const struct of_device_id rockchip_lvds_dt_ids[] = {
 	{
 		.compatible = "rockchip,rk3288-lvds",
@@ -538,6 +662,10 @@ static const struct of_device_id rockchip_lvds_dt_ids[] = {
 		.compatible = "rockchip,px30-lvds",
 		.data = &px30_lvds_data
 	},
+	{
+		.compatible = "rockchip,rk3568-lvds",
+		.data = &rk3568_lvds_data
+	},
 	{}
 };
 MODULE_DEVICE_TABLE(of, rockchip_lvds_dt_ids);
@@ -612,6 +740,8 @@ static int rockchip_lvds_bind(struct device *dev, struct device *master,
 	encoder = &lvds->encoder.encoder;
 	encoder->possible_crtcs = drm_of_find_possible_crtcs(drm_dev,
 							     dev->of_node);
+	rockchip_drm_encoder_set_crtc_endpoint_id(&lvds->encoder,
+						  dev->of_node, 0, 0);
 
 	ret = drm_simple_encoder_init(drm_dev, encoder, DRM_MODE_ENCODER_LVDS);
 	if (ret < 0) {
diff --git a/drivers/gpu/drm/rockchip/rockchip_lvds.h b/drivers/gpu/drm/rockchip/rockchip_lvds.h
index 4ce967d23813..57decb33f779 100644
--- a/drivers/gpu/drm/rockchip/rockchip_lvds.h
+++ b/drivers/gpu/drm/rockchip/rockchip_lvds.h
@@ -120,4 +120,14 @@
 #define   PX30_LVDS_P2S_EN(val)			HIWORD_UPDATE(val,  6,  6)
 #define   PX30_LVDS_VOP_SEL(val)		HIWORD_UPDATE(val,  1,  1)
 
+#define RK3568_GRF_VO_CON0			0x0360
+#define   RK3568_LVDS0_SELECT(val)		HIWORD_UPDATE(val,  5,  4)
+#define   RK3568_LVDS0_MSBSEL(val)		HIWORD_UPDATE(val,  3,  3)
+
+#define RK3568_GRF_VO_CON2			0x0368
+#define   RK3568_LVDS0_DCLK_INV_SEL(val)	HIWORD_UPDATE(val,  9,  9)
+#define   RK3568_LVDS0_DCLK_DIV2_SEL(val)	HIWORD_UPDATE(val,  8,  8)
+#define   RK3568_LVDS0_MODE_EN(val)		HIWORD_UPDATE(val,  1,  1)
+#define   RK3568_LVDS0_P2S_EN(val)		HIWORD_UPDATE(val,  0,  0)
+
 #endif /* _ROCKCHIP_LVDS_ */
-- 
2.34.1

[PATCH 1/3] drm/rockchip: lvds: add rk3568 support

[Alibek Omarov]

One of the ports of RK3568 can be configured as LVDS, re-using the DSI DPHY

Signed-off-by: Alibek Omarov <a1ba.omarov@gmail.com>
---
 drivers/gpu/drm/rockchip/rockchip_lvds.c | 144 +++++++++++++++++++++--
 drivers/gpu/drm/rockchip/rockchip_lvds.h |  10 ++
 2 files changed, 147 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_lvds.c b/drivers/gpu/drm/rockchip/rockchip_lvds.c
index 68f6ebb33460..83c60240af85 100644
--- a/drivers/gpu/drm/rockchip/rockchip_lvds.c
+++ b/drivers/gpu/drm/rockchip/rockchip_lvds.c
@@ -433,6 +433,90 @@ static void px30_lvds_encoder_disable(struct drm_encoder *encoder)
 	drm_panel_unprepare(lvds->panel);
 }
 
+static int rk3568_lvds_poweron(struct rockchip_lvds *lvds)
+{
+	int ret;
+
+	ret = clk_enable(lvds->pclk);
+	if (ret < 0) {
+		DRM_DEV_ERROR(lvds->dev, "failed to enable lvds pclk %d\n", ret);
+		return ret;
+	}
+
+	ret = pm_runtime_get_sync(lvds->dev);
+	if (ret < 0) {
+		DRM_DEV_ERROR(lvds->dev, "failed to get pm runtime: %d\n", ret);
+		clk_disable(lvds->pclk);
+		return ret;
+	}
+
+	/* Enable LVDS mode */
+	return regmap_update_bits(lvds->grf, RK3568_GRF_VO_CON2,
+				  RK3568_LVDS0_MODE_EN(1),
+				  RK3568_LVDS0_MODE_EN(1));
+}
+
+static void rk3568_lvds_poweroff(struct rockchip_lvds *lvds)
+{
+	regmap_update_bits(lvds->grf, RK3568_GRF_VO_CON2,
+			   RK3568_LVDS0_MODE_EN(1) | RK3568_LVDS0_P2S_EN(1),
+			   RK3568_LVDS0_MODE_EN(0) | RK3568_LVDS0_P2S_EN(0));
+
+	pm_runtime_put(lvds->dev);
+	clk_disable(lvds->pclk);
+}
+
+static int rk3568_lvds_grf_config(struct drm_encoder *encoder,
+				struct drm_display_mode *mode)
+{
+	struct rockchip_lvds *lvds = encoder_to_lvds(encoder);
+
+	if (lvds->output != DISPLAY_OUTPUT_LVDS) {
+		DRM_DEV_ERROR(lvds->dev, "Unsupported display output %d\n",
+			      lvds->output);
+		return -EINVAL;
+	}
+
+	/* Set format */
+	return regmap_update_bits(lvds->grf, RK3568_GRF_VO_CON0,
+				  RK3568_LVDS0_SELECT(3),
+				  RK3568_LVDS0_SELECT(lvds->format));
+}
+
+static void rk3568_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 = rk3568_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 = rk3568_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;
+	}
+
+	drm_panel_enable(lvds->panel);
+}
+
+static void rk3568_lvds_encoder_disable(struct drm_encoder *encoder)
+{
+	struct rockchip_lvds *lvds = encoder_to_lvds(encoder);
+
+	drm_panel_disable(lvds->panel);
+	rk3568_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,
@@ -447,6 +531,13 @@ struct drm_encoder_helper_funcs px30_lvds_encoder_helper_funcs = {
 	.atomic_check = rockchip_lvds_encoder_atomic_check,
 };
 
+static const
+struct drm_encoder_helper_funcs rk3568_lvds_encoder_helper_funcs = {
+	.enable = rk3568_lvds_encoder_enable,
+	.disable = rk3568_lvds_encoder_disable,
+	.atomic_check = rockchip_lvds_encoder_atomic_check,
+};
+
 static int rk3288_lvds_probe(struct platform_device *pdev,
 			     struct rockchip_lvds *lvds)
 {
@@ -491,6 +582,26 @@ static int rk3288_lvds_probe(struct platform_device *pdev,
 	return 0;
 }
 
+static int rockchip_lvds_phy_probe(struct platform_device *pdev,
+				   struct rockchip_lvds *lvds)
+{
+	int ret;
+
+	lvds->dphy = devm_phy_get(&pdev->dev, "dphy");
+	if (IS_ERR(lvds->dphy))
+		return PTR_ERR(lvds->dphy);
+
+	ret = phy_init(lvds->dphy);
+	if (ret)
+		return ret;
+
+	ret = phy_set_mode(lvds->dphy, PHY_MODE_LVDS);
+	if (ret)
+		return ret;
+
+	return phy_power_on(lvds->dphy);
+}
+
 static int px30_lvds_probe(struct platform_device *pdev,
 			   struct rockchip_lvds *lvds)
 {
@@ -503,20 +614,28 @@ static int px30_lvds_probe(struct platform_device *pdev,
 	if (ret)
 		return ret;
 
-	/* PHY */
-	lvds->dphy = devm_phy_get(&pdev->dev, "dphy");
-	if (IS_ERR(lvds->dphy))
-		return PTR_ERR(lvds->dphy);
+	return rockchip_lvds_phy_probe(pdev, lvds);
+}
 
-	ret = phy_init(lvds->dphy);
+static int rk3568_lvds_probe(struct platform_device *pdev,
+			     struct rockchip_lvds *lvds)
+{
+	int ret;
+
+	ret = regmap_update_bits(lvds->grf, RK3568_GRF_VO_CON0,
+				  RK3568_LVDS0_MSBSEL(1),
+				  RK3568_LVDS0_MSBSEL(1));
 	if (ret)
 		return ret;
 
-	ret = phy_set_mode(lvds->dphy, PHY_MODE_LVDS);
+	ret = regmap_update_bits(lvds->grf, RK3568_GRF_VO_CON2,
+				 RK3568_LVDS0_P2S_EN(1),
+				 RK3568_LVDS0_P2S_EN(1));
+
 	if (ret)
 		return ret;
 
-	return phy_power_on(lvds->dphy);
+	return rockchip_lvds_phy_probe(pdev, lvds);
 }
 
 static const struct rockchip_lvds_soc_data rk3288_lvds_data = {
@@ -529,6 +648,11 @@ static const struct rockchip_lvds_soc_data px30_lvds_data = {
 	.helper_funcs = &px30_lvds_encoder_helper_funcs,
 };
 
+static const struct rockchip_lvds_soc_data rk3568_lvds_data = {
+	.probe = rk3568_lvds_probe,
+	.helper_funcs = &rk3568_lvds_encoder_helper_funcs,
+};
+
 static const struct of_device_id rockchip_lvds_dt_ids[] = {
 	{
 		.compatible = "rockchip,rk3288-lvds",
@@ -538,6 +662,10 @@ static const struct of_device_id rockchip_lvds_dt_ids[] = {
 		.compatible = "rockchip,px30-lvds",
 		.data = &px30_lvds_data
 	},
+	{
+		.compatible = "rockchip,rk3568-lvds",
+		.data = &rk3568_lvds_data
+	},
 	{}
 };
 MODULE_DEVICE_TABLE(of, rockchip_lvds_dt_ids);
@@ -612,6 +740,8 @@ static int rockchip_lvds_bind(struct device *dev, struct device *master,
 	encoder = &lvds->encoder.encoder;
 	encoder->possible_crtcs = drm_of_find_possible_crtcs(drm_dev,
 							     dev->of_node);
+	rockchip_drm_encoder_set_crtc_endpoint_id(&lvds->encoder,
+						  dev->of_node, 0, 0);
 
 	ret = drm_simple_encoder_init(drm_dev, encoder, DRM_MODE_ENCODER_LVDS);
 	if (ret < 0) {
diff --git a/drivers/gpu/drm/rockchip/rockchip_lvds.h b/drivers/gpu/drm/rockchip/rockchip_lvds.h
index 4ce967d23813..57decb33f779 100644
--- a/drivers/gpu/drm/rockchip/rockchip_lvds.h
+++ b/drivers/gpu/drm/rockchip/rockchip_lvds.h
@@ -120,4 +120,14 @@
 #define   PX30_LVDS_P2S_EN(val)			HIWORD_UPDATE(val,  6,  6)
 #define   PX30_LVDS_VOP_SEL(val)		HIWORD_UPDATE(val,  1,  1)
 
+#define RK3568_GRF_VO_CON0			0x0360
+#define   RK3568_LVDS0_SELECT(val)		HIWORD_UPDATE(val,  5,  4)
+#define   RK3568_LVDS0_MSBSEL(val)		HIWORD_UPDATE(val,  3,  3)
+
+#define RK3568_GRF_VO_CON2			0x0368
+#define   RK3568_LVDS0_DCLK_INV_SEL(val)	HIWORD_UPDATE(val,  9,  9)
+#define   RK3568_LVDS0_DCLK_DIV2_SEL(val)	HIWORD_UPDATE(val,  8,  8)
+#define   RK3568_LVDS0_MODE_EN(val)		HIWORD_UPDATE(val,  1,  1)
+#define   RK3568_LVDS0_P2S_EN(val)		HIWORD_UPDATE(val,  0,  0)
+
 #endif /* _ROCKCHIP_LVDS_ */
-- 
2.34.1


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH 1/3] drm/rockchip: lvds: add rk3568 support

[Alibek Omarov]

One of the ports of RK3568 can be configured as LVDS, re-using the DSI DPHY

Signed-off-by: Alibek Omarov <a1ba.omarov@gmail.com>
---
 drivers/gpu/drm/rockchip/rockchip_lvds.c | 144 +++++++++++++++++++++--
 drivers/gpu/drm/rockchip/rockchip_lvds.h |  10 ++
 2 files changed, 147 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_lvds.c b/drivers/gpu/drm/rockchip/rockchip_lvds.c
index 68f6ebb33460..83c60240af85 100644
--- a/drivers/gpu/drm/rockchip/rockchip_lvds.c
+++ b/drivers/gpu/drm/rockchip/rockchip_lvds.c
@@ -433,6 +433,90 @@ static void px30_lvds_encoder_disable(struct drm_encoder *encoder)
 	drm_panel_unprepare(lvds->panel);
 }
 
+static int rk3568_lvds_poweron(struct rockchip_lvds *lvds)
+{
+	int ret;
+
+	ret = clk_enable(lvds->pclk);
+	if (ret < 0) {
+		DRM_DEV_ERROR(lvds->dev, "failed to enable lvds pclk %d\n", ret);
+		return ret;
+	}
+
+	ret = pm_runtime_get_sync(lvds->dev);
+	if (ret < 0) {
+		DRM_DEV_ERROR(lvds->dev, "failed to get pm runtime: %d\n", ret);
+		clk_disable(lvds->pclk);
+		return ret;
+	}
+
+	/* Enable LVDS mode */
+	return regmap_update_bits(lvds->grf, RK3568_GRF_VO_CON2,
+				  RK3568_LVDS0_MODE_EN(1),
+				  RK3568_LVDS0_MODE_EN(1));
+}
+
+static void rk3568_lvds_poweroff(struct rockchip_lvds *lvds)
+{
+	regmap_update_bits(lvds->grf, RK3568_GRF_VO_CON2,
+			   RK3568_LVDS0_MODE_EN(1) | RK3568_LVDS0_P2S_EN(1),
+			   RK3568_LVDS0_MODE_EN(0) | RK3568_LVDS0_P2S_EN(0));
+
+	pm_runtime_put(lvds->dev);
+	clk_disable(lvds->pclk);
+}
+
+static int rk3568_lvds_grf_config(struct drm_encoder *encoder,
+				struct drm_display_mode *mode)
+{
+	struct rockchip_lvds *lvds = encoder_to_lvds(encoder);
+
+	if (lvds->output != DISPLAY_OUTPUT_LVDS) {
+		DRM_DEV_ERROR(lvds->dev, "Unsupported display output %d\n",
+			      lvds->output);
+		return -EINVAL;
+	}
+
+	/* Set format */
+	return regmap_update_bits(lvds->grf, RK3568_GRF_VO_CON0,
+				  RK3568_LVDS0_SELECT(3),
+				  RK3568_LVDS0_SELECT(lvds->format));
+}
+
+static void rk3568_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 = rk3568_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 = rk3568_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;
+	}
+
+	drm_panel_enable(lvds->panel);
+}
+
+static void rk3568_lvds_encoder_disable(struct drm_encoder *encoder)
+{
+	struct rockchip_lvds *lvds = encoder_to_lvds(encoder);
+
+	drm_panel_disable(lvds->panel);
+	rk3568_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,
@@ -447,6 +531,13 @@ struct drm_encoder_helper_funcs px30_lvds_encoder_helper_funcs = {
 	.atomic_check = rockchip_lvds_encoder_atomic_check,
 };
 
+static const
+struct drm_encoder_helper_funcs rk3568_lvds_encoder_helper_funcs = {
+	.enable = rk3568_lvds_encoder_enable,
+	.disable = rk3568_lvds_encoder_disable,
+	.atomic_check = rockchip_lvds_encoder_atomic_check,
+};
+
 static int rk3288_lvds_probe(struct platform_device *pdev,
 			     struct rockchip_lvds *lvds)
 {
@@ -491,6 +582,26 @@ static int rk3288_lvds_probe(struct platform_device *pdev,
 	return 0;
 }
 
+static int rockchip_lvds_phy_probe(struct platform_device *pdev,
+				   struct rockchip_lvds *lvds)
+{
+	int ret;
+
+	lvds->dphy = devm_phy_get(&pdev->dev, "dphy");
+	if (IS_ERR(lvds->dphy))
+		return PTR_ERR(lvds->dphy);
+
+	ret = phy_init(lvds->dphy);
+	if (ret)
+		return ret;
+
+	ret = phy_set_mode(lvds->dphy, PHY_MODE_LVDS);
+	if (ret)
+		return ret;
+
+	return phy_power_on(lvds->dphy);
+}
+
 static int px30_lvds_probe(struct platform_device *pdev,
 			   struct rockchip_lvds *lvds)
 {
@@ -503,20 +614,28 @@ static int px30_lvds_probe(struct platform_device *pdev,
 	if (ret)
 		return ret;
 
-	/* PHY */
-	lvds->dphy = devm_phy_get(&pdev->dev, "dphy");
-	if (IS_ERR(lvds->dphy))
-		return PTR_ERR(lvds->dphy);
+	return rockchip_lvds_phy_probe(pdev, lvds);
+}
 
-	ret = phy_init(lvds->dphy);
+static int rk3568_lvds_probe(struct platform_device *pdev,
+			     struct rockchip_lvds *lvds)
+{
+	int ret;
+
+	ret = regmap_update_bits(lvds->grf, RK3568_GRF_VO_CON0,
+				  RK3568_LVDS0_MSBSEL(1),
+				  RK3568_LVDS0_MSBSEL(1));
 	if (ret)
 		return ret;
 
-	ret = phy_set_mode(lvds->dphy, PHY_MODE_LVDS);
+	ret = regmap_update_bits(lvds->grf, RK3568_GRF_VO_CON2,
+				 RK3568_LVDS0_P2S_EN(1),
+				 RK3568_LVDS0_P2S_EN(1));
+
 	if (ret)
 		return ret;
 
-	return phy_power_on(lvds->dphy);
+	return rockchip_lvds_phy_probe(pdev, lvds);
 }
 
 static const struct rockchip_lvds_soc_data rk3288_lvds_data = {
@@ -529,6 +648,11 @@ static const struct rockchip_lvds_soc_data px30_lvds_data = {
 	.helper_funcs = &px30_lvds_encoder_helper_funcs,
 };
 
+static const struct rockchip_lvds_soc_data rk3568_lvds_data = {
+	.probe = rk3568_lvds_probe,
+	.helper_funcs = &rk3568_lvds_encoder_helper_funcs,
+};
+
 static const struct of_device_id rockchip_lvds_dt_ids[] = {
 	{
 		.compatible = "rockchip,rk3288-lvds",
@@ -538,6 +662,10 @@ static const struct of_device_id rockchip_lvds_dt_ids[] = {
 		.compatible = "rockchip,px30-lvds",
 		.data = &px30_lvds_data
 	},
+	{
+		.compatible = "rockchip,rk3568-lvds",
+		.data = &rk3568_lvds_data
+	},
 	{}
 };
 MODULE_DEVICE_TABLE(of, rockchip_lvds_dt_ids);
@@ -612,6 +740,8 @@ static int rockchip_lvds_bind(struct device *dev, struct device *master,
 	encoder = &lvds->encoder.encoder;
 	encoder->possible_crtcs = drm_of_find_possible_crtcs(drm_dev,
 							     dev->of_node);
+	rockchip_drm_encoder_set_crtc_endpoint_id(&lvds->encoder,
+						  dev->of_node, 0, 0);
 
 	ret = drm_simple_encoder_init(drm_dev, encoder, DRM_MODE_ENCODER_LVDS);
 	if (ret < 0) {
diff --git a/drivers/gpu/drm/rockchip/rockchip_lvds.h b/drivers/gpu/drm/rockchip/rockchip_lvds.h
index 4ce967d23813..57decb33f779 100644
--- a/drivers/gpu/drm/rockchip/rockchip_lvds.h
+++ b/drivers/gpu/drm/rockchip/rockchip_lvds.h
@@ -120,4 +120,14 @@
 #define   PX30_LVDS_P2S_EN(val)			HIWORD_UPDATE(val,  6,  6)
 #define   PX30_LVDS_VOP_SEL(val)		HIWORD_UPDATE(val,  1,  1)
 
+#define RK3568_GRF_VO_CON0			0x0360
+#define   RK3568_LVDS0_SELECT(val)		HIWORD_UPDATE(val,  5,  4)
+#define   RK3568_LVDS0_MSBSEL(val)		HIWORD_UPDATE(val,  3,  3)
+
+#define RK3568_GRF_VO_CON2			0x0368
+#define   RK3568_LVDS0_DCLK_INV_SEL(val)	HIWORD_UPDATE(val,  9,  9)
+#define   RK3568_LVDS0_DCLK_DIV2_SEL(val)	HIWORD_UPDATE(val,  8,  8)
+#define   RK3568_LVDS0_MODE_EN(val)		HIWORD_UPDATE(val,  1,  1)
+#define   RK3568_LVDS0_P2S_EN(val)		HIWORD_UPDATE(val,  0,  0)
+
 #endif /* _ROCKCHIP_LVDS_ */
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/3] drm/rockchip: lvds: add rk3568 support

[Alibek Omarov]

One of the ports of RK3568 can be configured as LVDS, re-using the DSI DPHY

Signed-off-by: Alibek Omarov <a1ba.omarov@gmail.com>
---
 drivers/gpu/drm/rockchip/rockchip_lvds.c | 144 +++++++++++++++++++++--
 drivers/gpu/drm/rockchip/rockchip_lvds.h |  10 ++
 2 files changed, 147 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_lvds.c b/drivers/gpu/drm/rockchip/rockchip_lvds.c
index 68f6ebb33460..83c60240af85 100644
--- a/drivers/gpu/drm/rockchip/rockchip_lvds.c
+++ b/drivers/gpu/drm/rockchip/rockchip_lvds.c
@@ -433,6 +433,90 @@ static void px30_lvds_encoder_disable(struct drm_encoder *encoder)
 	drm_panel_unprepare(lvds->panel);
 }
 
+static int rk3568_lvds_poweron(struct rockchip_lvds *lvds)
+{
+	int ret;
+
+	ret = clk_enable(lvds->pclk);
+	if (ret < 0) {
+		DRM_DEV_ERROR(lvds->dev, "failed to enable lvds pclk %d\n", ret);
+		return ret;
+	}
+
+	ret = pm_runtime_get_sync(lvds->dev);
+	if (ret < 0) {
+		DRM_DEV_ERROR(lvds->dev, "failed to get pm runtime: %d\n", ret);
+		clk_disable(lvds->pclk);
+		return ret;
+	}
+
+	/* Enable LVDS mode */
+	return regmap_update_bits(lvds->grf, RK3568_GRF_VO_CON2,
+				  RK3568_LVDS0_MODE_EN(1),
+				  RK3568_LVDS0_MODE_EN(1));
+}
+
+static void rk3568_lvds_poweroff(struct rockchip_lvds *lvds)
+{
+	regmap_update_bits(lvds->grf, RK3568_GRF_VO_CON2,
+			   RK3568_LVDS0_MODE_EN(1) | RK3568_LVDS0_P2S_EN(1),
+			   RK3568_LVDS0_MODE_EN(0) | RK3568_LVDS0_P2S_EN(0));
+
+	pm_runtime_put(lvds->dev);
+	clk_disable(lvds->pclk);
+}
+
+static int rk3568_lvds_grf_config(struct drm_encoder *encoder,
+				struct drm_display_mode *mode)
+{
+	struct rockchip_lvds *lvds = encoder_to_lvds(encoder);
+
+	if (lvds->output != DISPLAY_OUTPUT_LVDS) {
+		DRM_DEV_ERROR(lvds->dev, "Unsupported display output %d\n",
+			      lvds->output);
+		return -EINVAL;
+	}
+
+	/* Set format */
+	return regmap_update_bits(lvds->grf, RK3568_GRF_VO_CON0,
+				  RK3568_LVDS0_SELECT(3),
+				  RK3568_LVDS0_SELECT(lvds->format));
+}
+
+static void rk3568_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 = rk3568_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 = rk3568_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;
+	}
+
+	drm_panel_enable(lvds->panel);
+}
+
+static void rk3568_lvds_encoder_disable(struct drm_encoder *encoder)
+{
+	struct rockchip_lvds *lvds = encoder_to_lvds(encoder);
+
+	drm_panel_disable(lvds->panel);
+	rk3568_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,
@@ -447,6 +531,13 @@ struct drm_encoder_helper_funcs px30_lvds_encoder_helper_funcs = {
 	.atomic_check = rockchip_lvds_encoder_atomic_check,
 };
 
+static const
+struct drm_encoder_helper_funcs rk3568_lvds_encoder_helper_funcs = {
+	.enable = rk3568_lvds_encoder_enable,
+	.disable = rk3568_lvds_encoder_disable,
+	.atomic_check = rockchip_lvds_encoder_atomic_check,
+};
+
 static int rk3288_lvds_probe(struct platform_device *pdev,
 			     struct rockchip_lvds *lvds)
 {
@@ -491,6 +582,26 @@ static int rk3288_lvds_probe(struct platform_device *pdev,
 	return 0;
 }
 
+static int rockchip_lvds_phy_probe(struct platform_device *pdev,
+				   struct rockchip_lvds *lvds)
+{
+	int ret;
+
+	lvds->dphy = devm_phy_get(&pdev->dev, "dphy");
+	if (IS_ERR(lvds->dphy))
+		return PTR_ERR(lvds->dphy);
+
+	ret = phy_init(lvds->dphy);
+	if (ret)
+		return ret;
+
+	ret = phy_set_mode(lvds->dphy, PHY_MODE_LVDS);
+	if (ret)
+		return ret;
+
+	return phy_power_on(lvds->dphy);
+}
+
 static int px30_lvds_probe(struct platform_device *pdev,
 			   struct rockchip_lvds *lvds)
 {
@@ -503,20 +614,28 @@ static int px30_lvds_probe(struct platform_device *pdev,
 	if (ret)
 		return ret;
 
-	/* PHY */
-	lvds->dphy = devm_phy_get(&pdev->dev, "dphy");
-	if (IS_ERR(lvds->dphy))
-		return PTR_ERR(lvds->dphy);
+	return rockchip_lvds_phy_probe(pdev, lvds);
+}
 
-	ret = phy_init(lvds->dphy);
+static int rk3568_lvds_probe(struct platform_device *pdev,
+			     struct rockchip_lvds *lvds)
+{
+	int ret;
+
+	ret = regmap_update_bits(lvds->grf, RK3568_GRF_VO_CON0,
+				  RK3568_LVDS0_MSBSEL(1),
+				  RK3568_LVDS0_MSBSEL(1));
 	if (ret)
 		return ret;
 
-	ret = phy_set_mode(lvds->dphy, PHY_MODE_LVDS);
+	ret = regmap_update_bits(lvds->grf, RK3568_GRF_VO_CON2,
+				 RK3568_LVDS0_P2S_EN(1),
+				 RK3568_LVDS0_P2S_EN(1));
+
 	if (ret)
 		return ret;
 
-	return phy_power_on(lvds->dphy);
+	return rockchip_lvds_phy_probe(pdev, lvds);
 }
 
 static const struct rockchip_lvds_soc_data rk3288_lvds_data = {
@@ -529,6 +648,11 @@ static const struct rockchip_lvds_soc_data px30_lvds_data = {
 	.helper_funcs = &px30_lvds_encoder_helper_funcs,
 };
 
+static const struct rockchip_lvds_soc_data rk3568_lvds_data = {
+	.probe = rk3568_lvds_probe,
+	.helper_funcs = &rk3568_lvds_encoder_helper_funcs,
+};
+
 static const struct of_device_id rockchip_lvds_dt_ids[] = {
 	{
 		.compatible = "rockchip,rk3288-lvds",
@@ -538,6 +662,10 @@ static const struct of_device_id rockchip_lvds_dt_ids[] = {
 		.compatible = "rockchip,px30-lvds",
 		.data = &px30_lvds_data
 	},
+	{
+		.compatible = "rockchip,rk3568-lvds",
+		.data = &rk3568_lvds_data
+	},
 	{}
 };
 MODULE_DEVICE_TABLE(of, rockchip_lvds_dt_ids);
@@ -612,6 +740,8 @@ static int rockchip_lvds_bind(struct device *dev, struct device *master,
 	encoder = &lvds->encoder.encoder;
 	encoder->possible_crtcs = drm_of_find_possible_crtcs(drm_dev,
 							     dev->of_node);
+	rockchip_drm_encoder_set_crtc_endpoint_id(&lvds->encoder,
+						  dev->of_node, 0, 0);
 
 	ret = drm_simple_encoder_init(drm_dev, encoder, DRM_MODE_ENCODER_LVDS);
 	if (ret < 0) {
diff --git a/drivers/gpu/drm/rockchip/rockchip_lvds.h b/drivers/gpu/drm/rockchip/rockchip_lvds.h
index 4ce967d23813..57decb33f779 100644
--- a/drivers/gpu/drm/rockchip/rockchip_lvds.h
+++ b/drivers/gpu/drm/rockchip/rockchip_lvds.h
@@ -120,4 +120,14 @@
 #define   PX30_LVDS_P2S_EN(val)			HIWORD_UPDATE(val,  6,  6)
 #define   PX30_LVDS_VOP_SEL(val)		HIWORD_UPDATE(val,  1,  1)
 
+#define RK3568_GRF_VO_CON0			0x0360
+#define   RK3568_LVDS0_SELECT(val)		HIWORD_UPDATE(val,  5,  4)
+#define   RK3568_LVDS0_MSBSEL(val)		HIWORD_UPDATE(val,  3,  3)
+
+#define RK3568_GRF_VO_CON2			0x0368
+#define   RK3568_LVDS0_DCLK_INV_SEL(val)	HIWORD_UPDATE(val,  9,  9)
+#define   RK3568_LVDS0_DCLK_DIV2_SEL(val)	HIWORD_UPDATE(val,  8,  8)
+#define   RK3568_LVDS0_MODE_EN(val)		HIWORD_UPDATE(val,  1,  1)
+#define   RK3568_LVDS0_P2S_EN(val)		HIWORD_UPDATE(val,  0,  0)
+
 #endif /* _ROCKCHIP_LVDS_ */
-- 
2.34.1

[PATCH 2/3] arm64: dts: rockchip: rk356x: add LVDS bindings

[Alibek Omarov]

Exposes ports for VOP2 and for panel

Tested-by: Alexander Sverdlin <alexander.sverdlin@siemens.com>
Link: https://patchwork.kernel.org/project/linux-rockchip/patch/20220923160115.2946615-3-a1ba.omarov@gmail.com/#25097661
Signed-off-by: Alibek Omarov <a1ba.omarov@gmail.com>
---
 arch/arm64/boot/dts/rockchip/rk356x.dtsi | 25 ++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk356x.dtsi b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
index 5706c3e24f0a..34d1b943f312 100644
--- a/arch/arm64/boot/dts/rockchip/rk356x.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
@@ -827,6 +827,31 @@ hdmi_out: port@1 {
 		};
 	};
 
+	lvds: lvds {
+		compatible = "rockchip,rk3568-lvds";
+		clocks = <&cru PCLK_DSITX_0>;
+		clock-names = "pclk_lvds";
+		phys = <&dsi_dphy0>;
+		phy-names = "dphy";
+		power-domains = <&power RK3568_PD_VO>;
+		rockchip,grf = <&grf>;
+		rockchip,output = "lvds";
+		status = "disabled";
+
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			lvds_in: port@0 {
+				reg = <0>;
+			};
+
+			lvds_out: port@1 {
+				reg = <1>;
+			};
+		};
+	};
+
 	qos_gpu: qos@fe128000 {
 		compatible = "rockchip,rk3568-qos", "syscon";
 		reg = <0x0 0xfe128000 0x0 0x20>;
-- 
2.34.1

[PATCH 2/3] arm64: dts: rockchip: rk356x: add LVDS bindings

[Alibek Omarov]

Exposes ports for VOP2 and for panel

Tested-by: Alexander Sverdlin <alexander.sverdlin@siemens.com>
Link: https://patchwork.kernel.org/project/linux-rockchip/patch/20220923160115.2946615-3-a1ba.omarov@gmail.com/#25097661
Signed-off-by: Alibek Omarov <a1ba.omarov@gmail.com>
---
 arch/arm64/boot/dts/rockchip/rk356x.dtsi | 25 ++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk356x.dtsi b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
index 5706c3e24f0a..34d1b943f312 100644
--- a/arch/arm64/boot/dts/rockchip/rk356x.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
@@ -827,6 +827,31 @@ hdmi_out: port@1 {
 		};
 	};
 
+	lvds: lvds {
+		compatible = "rockchip,rk3568-lvds";
+		clocks = <&cru PCLK_DSITX_0>;
+		clock-names = "pclk_lvds";
+		phys = <&dsi_dphy0>;
+		phy-names = "dphy";
+		power-domains = <&power RK3568_PD_VO>;
+		rockchip,grf = <&grf>;
+		rockchip,output = "lvds";
+		status = "disabled";
+
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			lvds_in: port@0 {
+				reg = <0>;
+			};
+
+			lvds_out: port@1 {
+				reg = <1>;
+			};
+		};
+	};
+
 	qos_gpu: qos@fe128000 {
 		compatible = "rockchip,rk3568-qos", "syscon";
 		reg = <0x0 0xfe128000 0x0 0x20>;
-- 
2.34.1


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH 2/3] arm64: dts: rockchip: rk356x: add LVDS bindings

[Alibek Omarov]

Exposes ports for VOP2 and for panel

Tested-by: Alexander Sverdlin <alexander.sverdlin@siemens.com>
Link: https://patchwork.kernel.org/project/linux-rockchip/patch/20220923160115.2946615-3-a1ba.omarov@gmail.com/#25097661
Signed-off-by: Alibek Omarov <a1ba.omarov@gmail.com>
---
 arch/arm64/boot/dts/rockchip/rk356x.dtsi | 25 ++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk356x.dtsi b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
index 5706c3e24f0a..34d1b943f312 100644
--- a/arch/arm64/boot/dts/rockchip/rk356x.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
@@ -827,6 +827,31 @@ hdmi_out: port@1 {
 		};
 	};
 
+	lvds: lvds {
+		compatible = "rockchip,rk3568-lvds";
+		clocks = <&cru PCLK_DSITX_0>;
+		clock-names = "pclk_lvds";
+		phys = <&dsi_dphy0>;
+		phy-names = "dphy";
+		power-domains = <&power RK3568_PD_VO>;
+		rockchip,grf = <&grf>;
+		rockchip,output = "lvds";
+		status = "disabled";
+
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			lvds_in: port@0 {
+				reg = <0>;
+			};
+
+			lvds_out: port@1 {
+				reg = <1>;
+			};
+		};
+	};
+
 	qos_gpu: qos@fe128000 {
 		compatible = "rockchip,rk3568-qos", "syscon";
 		reg = <0x0 0xfe128000 0x0 0x20>;
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/3] arm64: dts: rockchip: rk356x: add LVDS bindings

[Alibek Omarov]

Exposes ports for VOP2 and for panel

Tested-by: Alexander Sverdlin <alexander.sverdlin@siemens.com>
Link: https://patchwork.kernel.org/project/linux-rockchip/patch/20220923160115.2946615-3-a1ba.omarov@gmail.com/#25097661
Signed-off-by: Alibek Omarov <a1ba.omarov@gmail.com>
---
 arch/arm64/boot/dts/rockchip/rk356x.dtsi | 25 ++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk356x.dtsi b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
index 5706c3e24f0a..34d1b943f312 100644
--- a/arch/arm64/boot/dts/rockchip/rk356x.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
@@ -827,6 +827,31 @@ hdmi_out: port@1 {
 		};
 	};
 
+	lvds: lvds {
+		compatible = "rockchip,rk3568-lvds";
+		clocks = <&cru PCLK_DSITX_0>;
+		clock-names = "pclk_lvds";
+		phys = <&dsi_dphy0>;
+		phy-names = "dphy";
+		power-domains = <&power RK3568_PD_VO>;
+		rockchip,grf = <&grf>;
+		rockchip,output = "lvds";
+		status = "disabled";
+
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			lvds_in: port@0 {
+				reg = <0>;
+			};
+
+			lvds_out: port@1 {
+				reg = <1>;
+			};
+		};
+	};
+
 	qos_gpu: qos@fe128000 {
 		compatible = "rockchip,rk3568-qos", "syscon";
 		reg = <0x0 0xfe128000 0x0 0x20>;
-- 
2.34.1

[PATCH 3/3] dt-bindings: display: rockchip-lvds: add compatible string for RK3568

[Alibek Omarov]

RK3568 SoCs have a single LVDS output, that use PHY shared by two display
pipelines: LVDS and MIPI DSI.

Signed-off-by: Alibek Omarov <a1ba.omarov@gmail.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 aaf8c44cf90f..ec8a8d05368a 100644
--- a/Documentation/devicetree/bindings/display/rockchip/rockchip-lvds.txt
+++ b/Documentation/devicetree/bindings/display/rockchip/rockchip-lvds.txt
@@ -5,6 +5,7 @@ Required properties:
 - compatible: matching the soc type, one of
 	- "rockchip,rk3288-lvds";
 	- "rockchip,px30-lvds";
+	- "rockchip,rk3568-lvds";
 
 - reg: physical base address of the controller and length
 	of memory mapped region.
-- 
2.34.1

[PATCH 3/3] dt-bindings: display: rockchip-lvds: add compatible string for RK3568

[Alibek Omarov]

RK3568 SoCs have a single LVDS output, that use PHY shared by two display
pipelines: LVDS and MIPI DSI.

Signed-off-by: Alibek Omarov <a1ba.omarov@gmail.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 aaf8c44cf90f..ec8a8d05368a 100644
--- a/Documentation/devicetree/bindings/display/rockchip/rockchip-lvds.txt
+++ b/Documentation/devicetree/bindings/display/rockchip/rockchip-lvds.txt
@@ -5,6 +5,7 @@ Required properties:
 - compatible: matching the soc type, one of
 	- "rockchip,rk3288-lvds";
 	- "rockchip,px30-lvds";
+	- "rockchip,rk3568-lvds";
 
 - reg: physical base address of the controller and length
 	of memory mapped region.
-- 
2.34.1


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH 3/3] dt-bindings: display: rockchip-lvds: add compatible string for RK3568

[Alibek Omarov]

RK3568 SoCs have a single LVDS output, that use PHY shared by two display
pipelines: LVDS and MIPI DSI.

Signed-off-by: Alibek Omarov <a1ba.omarov@gmail.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 aaf8c44cf90f..ec8a8d05368a 100644
--- a/Documentation/devicetree/bindings/display/rockchip/rockchip-lvds.txt
+++ b/Documentation/devicetree/bindings/display/rockchip/rockchip-lvds.txt
@@ -5,6 +5,7 @@ Required properties:
 - compatible: matching the soc type, one of
 	- "rockchip,rk3288-lvds";
 	- "rockchip,px30-lvds";
+	- "rockchip,rk3568-lvds";
 
 - reg: physical base address of the controller and length
 	of memory mapped region.
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/3] dt-bindings: display: rockchip-lvds: add compatible string for RK3568

[Alibek Omarov]

RK3568 SoCs have a single LVDS output, that use PHY shared by two display
pipelines: LVDS and MIPI DSI.

Signed-off-by: Alibek Omarov <a1ba.omarov@gmail.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 aaf8c44cf90f..ec8a8d05368a 100644
--- a/Documentation/devicetree/bindings/display/rockchip/rockchip-lvds.txt
+++ b/Documentation/devicetree/bindings/display/rockchip/rockchip-lvds.txt
@@ -5,6 +5,7 @@ Required properties:
 - compatible: matching the soc type, one of
 	- "rockchip,rk3288-lvds";
 	- "rockchip,px30-lvds";
+	- "rockchip,rk3568-lvds";
 
 - reg: physical base address of the controller and length
 	of memory mapped region.
-- 
2.34.1

Re: [PATCH 3/3] dt-bindings: display: rockchip-lvds: add compatible string for RK3568

[Krzysztof Kozlowski]

On 19/01/2023 19:48, Alibek Omarov wrote:
> RK3568 SoCs have a single LVDS output, that use PHY shared by two display
> pipelines: LVDS and MIPI DSI.
> 
> Signed-off-by: Alibek Omarov <a1ba.omarov@gmail.com>


Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH 3/3] dt-bindings: display: rockchip-lvds: add compatible string for RK3568

[Krzysztof Kozlowski]

On 19/01/2023 19:48, Alibek Omarov wrote:
> RK3568 SoCs have a single LVDS output, that use PHY shared by two display
> pipelines: LVDS and MIPI DSI.
> 
> Signed-off-by: Alibek Omarov <a1ba.omarov@gmail.com>


Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH 3/3] dt-bindings: display: rockchip-lvds: add compatible string for RK3568

[Krzysztof Kozlowski]

On 19/01/2023 19:48, Alibek Omarov wrote:
> RK3568 SoCs have a single LVDS output, that use PHY shared by two display
> pipelines: LVDS and MIPI DSI.
> 
> Signed-off-by: Alibek Omarov <a1ba.omarov@gmail.com>


Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/3] dt-bindings: display: rockchip-lvds: add compatible string for RK3568

[Krzysztof Kozlowski]

On 19/01/2023 19:48, Alibek Omarov wrote:
> RK3568 SoCs have a single LVDS output, that use PHY shared by two display
> pipelines: LVDS and MIPI DSI.
> 
> Signed-off-by: Alibek Omarov <a1ba.omarov@gmail.com>


Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH 1/3] drm/rockchip: lvds: add rk3568 support

[Sascha Hauer]

On Thu, Jan 19, 2023 at 09:48:03PM +0300, Alibek Omarov wrote:
> One of the ports of RK3568 can be configured as LVDS, re-using the DSI DPHY
> 
> Signed-off-by: Alibek Omarov <a1ba.omarov@gmail.com>
> ---
>  drivers/gpu/drm/rockchip/rockchip_lvds.c | 144 +++++++++++++++++++++--
>  drivers/gpu/drm/rockchip/rockchip_lvds.h |  10 ++
>  2 files changed, 147 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/rockchip_lvds.c b/drivers/gpu/drm/rockchip/rockchip_lvds.c
> index 68f6ebb33460..83c60240af85 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_lvds.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_lvds.c
> @@ -433,6 +433,90 @@ static void px30_lvds_encoder_disable(struct drm_encoder *encoder)
>  	drm_panel_unprepare(lvds->panel);
>  }
>  
> +static int rk3568_lvds_poweron(struct rockchip_lvds *lvds)
> +{
> +	int ret;
> +
> +	ret = clk_enable(lvds->pclk);
> +	if (ret < 0) {
> +		DRM_DEV_ERROR(lvds->dev, "failed to enable lvds pclk %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = pm_runtime_get_sync(lvds->dev);
> +	if (ret < 0) {
> +		DRM_DEV_ERROR(lvds->dev, "failed to get pm runtime: %d\n", ret);
> +		clk_disable(lvds->pclk);
> +		return ret;
> +	}
> +
> +	/* Enable LVDS mode */
> +	return regmap_update_bits(lvds->grf, RK3568_GRF_VO_CON2,
> +				  RK3568_LVDS0_MODE_EN(1),
> +				  RK3568_LVDS0_MODE_EN(1));

Isn't this the same as:

	regmap_write(lvds->grf, RK3568_GRF_VO_CON2, RK3568_LVDS0_MODE_EN(1));

Unless I am missing something I find a plain regmap_write() easier to
read.

> +}
> +
> +static void rk3568_lvds_poweroff(struct rockchip_lvds *lvds)
> +{
> +	regmap_update_bits(lvds->grf, RK3568_GRF_VO_CON2,
> +			   RK3568_LVDS0_MODE_EN(1) | RK3568_LVDS0_P2S_EN(1),
> +			   RK3568_LVDS0_MODE_EN(0) | RK3568_LVDS0_P2S_EN(0));

Same here:

	regmap_write(lvds->grf, RK3568_GRF_VO_CON2,
		     RK3568_LVDS0_MODE_EN(0) | RK3568_LVDS0_P2S_EN(0));

What about the RK3568_LVDS0_P2S_EN bit? This is set in probe() and
cleared here. For symmetry reasons shouldn't it be set in
rk3568_lvds_poweron() instead?

> +
> +	pm_runtime_put(lvds->dev);
> +	clk_disable(lvds->pclk);
> +}
> +
> +static int rk3568_lvds_grf_config(struct drm_encoder *encoder,
> +				struct drm_display_mode *mode)
> +{
> +	struct rockchip_lvds *lvds = encoder_to_lvds(encoder);
> +
> +	if (lvds->output != DISPLAY_OUTPUT_LVDS) {
> +		DRM_DEV_ERROR(lvds->dev, "Unsupported display output %d\n",
> +			      lvds->output);
> +		return -EINVAL;
> +	}
> +
> +	/* Set format */
> +	return regmap_update_bits(lvds->grf, RK3568_GRF_VO_CON0,
> +				  RK3568_LVDS0_SELECT(3),
> +				  RK3568_LVDS0_SELECT(lvds->format));

It seems lvds->format does not match what the register expects. We
have:

#define LVDS_VESA_24                            0
#define LVDS_JEIDA_24                           1
#define LVDS_VESA_18                            2
#define LVDS_JEIDA_18                           3

According to the reference manual the register expects:

lvdsformat_lvds0_select
 2'b00: VESA 24bit
 2'b01: JEIDA 24bit
 2'b10: JEIDA 18bit
 2'b11: VESA 18bit

I only have the RK3568 manual but no PX30 or RK3288 manual, so I can't say if
they changed the register mapping between the SoCs or if it's wrong on
the other SoCs as well.

BTW you correctly set the mask to RK3568_LVDS0_SELECT(3), but for the
PX30 it looks wrong:

	return regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON1,
				  PX30_LVDS_FORMAT(lvds->format),
				  PX30_LVDS_FORMAT(lvds->format));

I really think regmap_write() would be better to use here to avoid such
things.

> +
> +static void rk3568_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;

'mode' is unused.

> +	int ret;
> +
> +	drm_panel_prepare(lvds->panel);
> +
> +	ret = rk3568_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 = rk3568_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;
> +	}
> +
> +	drm_panel_enable(lvds->panel);
> +}
> +
> +static void rk3568_lvds_encoder_disable(struct drm_encoder *encoder)
> +{
> +	struct rockchip_lvds *lvds = encoder_to_lvds(encoder);
> +
> +	drm_panel_disable(lvds->panel);
> +	rk3568_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,
> @@ -447,6 +531,13 @@ struct drm_encoder_helper_funcs px30_lvds_encoder_helper_funcs = {
>  	.atomic_check = rockchip_lvds_encoder_atomic_check,
>  };
>  
> +static const
> +struct drm_encoder_helper_funcs rk3568_lvds_encoder_helper_funcs = {
> +	.enable = rk3568_lvds_encoder_enable,
> +	.disable = rk3568_lvds_encoder_disable,
> +	.atomic_check = rockchip_lvds_encoder_atomic_check,
> +};
> +
>  static int rk3288_lvds_probe(struct platform_device *pdev,
>  			     struct rockchip_lvds *lvds)
>  {
> @@ -491,6 +582,26 @@ static int rk3288_lvds_probe(struct platform_device *pdev,
>  	return 0;
>  }
>  
> +static int rockchip_lvds_phy_probe(struct platform_device *pdev,
> +				   struct rockchip_lvds *lvds)
> +{
> +	int ret;
> +
> +	lvds->dphy = devm_phy_get(&pdev->dev, "dphy");
> +	if (IS_ERR(lvds->dphy))
> +		return PTR_ERR(lvds->dphy);
> +
> +	ret = phy_init(lvds->dphy);
> +	if (ret)
> +		return ret;
> +
> +	ret = phy_set_mode(lvds->dphy, PHY_MODE_LVDS);
> +	if (ret)
> +		return ret;
> +
> +	return phy_power_on(lvds->dphy);
> +}

You factor out the steps done for px30 to a separate function in order
to reuse it on rk3568. You could make a separate patch from this to
make it easier to understand and to verify that there is no functional
change involved for the px30.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 1/3] drm/rockchip: lvds: add rk3568 support

[Sascha Hauer]

On Thu, Jan 19, 2023 at 09:48:03PM +0300, Alibek Omarov wrote:
> One of the ports of RK3568 can be configured as LVDS, re-using the DSI DPHY
> 
> Signed-off-by: Alibek Omarov <a1ba.omarov@gmail.com>
> ---
>  drivers/gpu/drm/rockchip/rockchip_lvds.c | 144 +++++++++++++++++++++--
>  drivers/gpu/drm/rockchip/rockchip_lvds.h |  10 ++
>  2 files changed, 147 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/rockchip_lvds.c b/drivers/gpu/drm/rockchip/rockchip_lvds.c
> index 68f6ebb33460..83c60240af85 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_lvds.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_lvds.c
> @@ -433,6 +433,90 @@ static void px30_lvds_encoder_disable(struct drm_encoder *encoder)
>  	drm_panel_unprepare(lvds->panel);
>  }
>  
> +static int rk3568_lvds_poweron(struct rockchip_lvds *lvds)
> +{
> +	int ret;
> +
> +	ret = clk_enable(lvds->pclk);
> +	if (ret < 0) {
> +		DRM_DEV_ERROR(lvds->dev, "failed to enable lvds pclk %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = pm_runtime_get_sync(lvds->dev);
> +	if (ret < 0) {
> +		DRM_DEV_ERROR(lvds->dev, "failed to get pm runtime: %d\n", ret);
> +		clk_disable(lvds->pclk);
> +		return ret;
> +	}
> +
> +	/* Enable LVDS mode */
> +	return regmap_update_bits(lvds->grf, RK3568_GRF_VO_CON2,
> +				  RK3568_LVDS0_MODE_EN(1),
> +				  RK3568_LVDS0_MODE_EN(1));

Isn't this the same as:

	regmap_write(lvds->grf, RK3568_GRF_VO_CON2, RK3568_LVDS0_MODE_EN(1));

Unless I am missing something I find a plain regmap_write() easier to
read.

> +}
> +
> +static void rk3568_lvds_poweroff(struct rockchip_lvds *lvds)
> +{
> +	regmap_update_bits(lvds->grf, RK3568_GRF_VO_CON2,
> +			   RK3568_LVDS0_MODE_EN(1) | RK3568_LVDS0_P2S_EN(1),
> +			   RK3568_LVDS0_MODE_EN(0) | RK3568_LVDS0_P2S_EN(0));

Same here:

	regmap_write(lvds->grf, RK3568_GRF_VO_CON2,
		     RK3568_LVDS0_MODE_EN(0) | RK3568_LVDS0_P2S_EN(0));

What about the RK3568_LVDS0_P2S_EN bit? This is set in probe() and
cleared here. For symmetry reasons shouldn't it be set in
rk3568_lvds_poweron() instead?

> +
> +	pm_runtime_put(lvds->dev);
> +	clk_disable(lvds->pclk);
> +}
> +
> +static int rk3568_lvds_grf_config(struct drm_encoder *encoder,
> +				struct drm_display_mode *mode)
> +{
> +	struct rockchip_lvds *lvds = encoder_to_lvds(encoder);
> +
> +	if (lvds->output != DISPLAY_OUTPUT_LVDS) {
> +		DRM_DEV_ERROR(lvds->dev, "Unsupported display output %d\n",
> +			      lvds->output);
> +		return -EINVAL;
> +	}
> +
> +	/* Set format */
> +	return regmap_update_bits(lvds->grf, RK3568_GRF_VO_CON0,
> +				  RK3568_LVDS0_SELECT(3),
> +				  RK3568_LVDS0_SELECT(lvds->format));

It seems lvds->format does not match what the register expects. We
have:

#define LVDS_VESA_24                            0
#define LVDS_JEIDA_24                           1
#define LVDS_VESA_18                            2
#define LVDS_JEIDA_18                           3

According to the reference manual the register expects:

lvdsformat_lvds0_select
 2'b00: VESA 24bit
 2'b01: JEIDA 24bit
 2'b10: JEIDA 18bit
 2'b11: VESA 18bit

I only have the RK3568 manual but no PX30 or RK3288 manual, so I can't say if
they changed the register mapping between the SoCs or if it's wrong on
the other SoCs as well.

BTW you correctly set the mask to RK3568_LVDS0_SELECT(3), but for the
PX30 it looks wrong:

	return regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON1,
				  PX30_LVDS_FORMAT(lvds->format),
				  PX30_LVDS_FORMAT(lvds->format));

I really think regmap_write() would be better to use here to avoid such
things.

> +
> +static void rk3568_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;

'mode' is unused.

> +	int ret;
> +
> +	drm_panel_prepare(lvds->panel);
> +
> +	ret = rk3568_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 = rk3568_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;
> +	}
> +
> +	drm_panel_enable(lvds->panel);
> +}
> +
> +static void rk3568_lvds_encoder_disable(struct drm_encoder *encoder)
> +{
> +	struct rockchip_lvds *lvds = encoder_to_lvds(encoder);
> +
> +	drm_panel_disable(lvds->panel);
> +	rk3568_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,
> @@ -447,6 +531,13 @@ struct drm_encoder_helper_funcs px30_lvds_encoder_helper_funcs = {
>  	.atomic_check = rockchip_lvds_encoder_atomic_check,
>  };
>  
> +static const
> +struct drm_encoder_helper_funcs rk3568_lvds_encoder_helper_funcs = {
> +	.enable = rk3568_lvds_encoder_enable,
> +	.disable = rk3568_lvds_encoder_disable,
> +	.atomic_check = rockchip_lvds_encoder_atomic_check,
> +};
> +
>  static int rk3288_lvds_probe(struct platform_device *pdev,
>  			     struct rockchip_lvds *lvds)
>  {
> @@ -491,6 +582,26 @@ static int rk3288_lvds_probe(struct platform_device *pdev,
>  	return 0;
>  }
>  
> +static int rockchip_lvds_phy_probe(struct platform_device *pdev,
> +				   struct rockchip_lvds *lvds)
> +{
> +	int ret;
> +
> +	lvds->dphy = devm_phy_get(&pdev->dev, "dphy");
> +	if (IS_ERR(lvds->dphy))
> +		return PTR_ERR(lvds->dphy);
> +
> +	ret = phy_init(lvds->dphy);
> +	if (ret)
> +		return ret;
> +
> +	ret = phy_set_mode(lvds->dphy, PHY_MODE_LVDS);
> +	if (ret)
> +		return ret;
> +
> +	return phy_power_on(lvds->dphy);
> +}

You factor out the steps done for px30 to a separate function in order
to reuse it on rk3568. You could make a separate patch from this to
make it easier to understand and to verify that there is no functional
change involved for the px30.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 1/3] drm/rockchip: lvds: add rk3568 support

[Sascha Hauer]

On Thu, Jan 19, 2023 at 09:48:03PM +0300, Alibek Omarov wrote:
> One of the ports of RK3568 can be configured as LVDS, re-using the DSI DPHY
> 
> Signed-off-by: Alibek Omarov <a1ba.omarov@gmail.com>
> ---
>  drivers/gpu/drm/rockchip/rockchip_lvds.c | 144 +++++++++++++++++++++--
>  drivers/gpu/drm/rockchip/rockchip_lvds.h |  10 ++
>  2 files changed, 147 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/rockchip_lvds.c b/drivers/gpu/drm/rockchip/rockchip_lvds.c
> index 68f6ebb33460..83c60240af85 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_lvds.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_lvds.c
> @@ -433,6 +433,90 @@ static void px30_lvds_encoder_disable(struct drm_encoder *encoder)
>  	drm_panel_unprepare(lvds->panel);
>  }
>  
> +static int rk3568_lvds_poweron(struct rockchip_lvds *lvds)
> +{
> +	int ret;
> +
> +	ret = clk_enable(lvds->pclk);
> +	if (ret < 0) {
> +		DRM_DEV_ERROR(lvds->dev, "failed to enable lvds pclk %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = pm_runtime_get_sync(lvds->dev);
> +	if (ret < 0) {
> +		DRM_DEV_ERROR(lvds->dev, "failed to get pm runtime: %d\n", ret);
> +		clk_disable(lvds->pclk);
> +		return ret;
> +	}
> +
> +	/* Enable LVDS mode */
> +	return regmap_update_bits(lvds->grf, RK3568_GRF_VO_CON2,
> +				  RK3568_LVDS0_MODE_EN(1),
> +				  RK3568_LVDS0_MODE_EN(1));

Isn't this the same as:

	regmap_write(lvds->grf, RK3568_GRF_VO_CON2, RK3568_LVDS0_MODE_EN(1));

Unless I am missing something I find a plain regmap_write() easier to
read.

> +}
> +
> +static void rk3568_lvds_poweroff(struct rockchip_lvds *lvds)
> +{
> +	regmap_update_bits(lvds->grf, RK3568_GRF_VO_CON2,
> +			   RK3568_LVDS0_MODE_EN(1) | RK3568_LVDS0_P2S_EN(1),
> +			   RK3568_LVDS0_MODE_EN(0) | RK3568_LVDS0_P2S_EN(0));

Same here:

	regmap_write(lvds->grf, RK3568_GRF_VO_CON2,
		     RK3568_LVDS0_MODE_EN(0) | RK3568_LVDS0_P2S_EN(0));

What about the RK3568_LVDS0_P2S_EN bit? This is set in probe() and
cleared here. For symmetry reasons shouldn't it be set in
rk3568_lvds_poweron() instead?

> +
> +	pm_runtime_put(lvds->dev);
> +	clk_disable(lvds->pclk);
> +}
> +
> +static int rk3568_lvds_grf_config(struct drm_encoder *encoder,
> +				struct drm_display_mode *mode)
> +{
> +	struct rockchip_lvds *lvds = encoder_to_lvds(encoder);
> +
> +	if (lvds->output != DISPLAY_OUTPUT_LVDS) {
> +		DRM_DEV_ERROR(lvds->dev, "Unsupported display output %d\n",
> +			      lvds->output);
> +		return -EINVAL;
> +	}
> +
> +	/* Set format */
> +	return regmap_update_bits(lvds->grf, RK3568_GRF_VO_CON0,
> +				  RK3568_LVDS0_SELECT(3),
> +				  RK3568_LVDS0_SELECT(lvds->format));

It seems lvds->format does not match what the register expects. We
have:

#define LVDS_VESA_24                            0
#define LVDS_JEIDA_24                           1
#define LVDS_VESA_18                            2
#define LVDS_JEIDA_18                           3

According to the reference manual the register expects:

lvdsformat_lvds0_select
 2'b00: VESA 24bit
 2'b01: JEIDA 24bit
 2'b10: JEIDA 18bit
 2'b11: VESA 18bit

I only have the RK3568 manual but no PX30 or RK3288 manual, so I can't say if
they changed the register mapping between the SoCs or if it's wrong on
the other SoCs as well.

BTW you correctly set the mask to RK3568_LVDS0_SELECT(3), but for the
PX30 it looks wrong:

	return regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON1,
				  PX30_LVDS_FORMAT(lvds->format),
				  PX30_LVDS_FORMAT(lvds->format));

I really think regmap_write() would be better to use here to avoid such
things.

> +
> +static void rk3568_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;

'mode' is unused.

> +	int ret;
> +
> +	drm_panel_prepare(lvds->panel);
> +
> +	ret = rk3568_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 = rk3568_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;
> +	}
> +
> +	drm_panel_enable(lvds->panel);
> +}
> +
> +static void rk3568_lvds_encoder_disable(struct drm_encoder *encoder)
> +{
> +	struct rockchip_lvds *lvds = encoder_to_lvds(encoder);
> +
> +	drm_panel_disable(lvds->panel);
> +	rk3568_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,
> @@ -447,6 +531,13 @@ struct drm_encoder_helper_funcs px30_lvds_encoder_helper_funcs = {
>  	.atomic_check = rockchip_lvds_encoder_atomic_check,
>  };
>  
> +static const
> +struct drm_encoder_helper_funcs rk3568_lvds_encoder_helper_funcs = {
> +	.enable = rk3568_lvds_encoder_enable,
> +	.disable = rk3568_lvds_encoder_disable,
> +	.atomic_check = rockchip_lvds_encoder_atomic_check,
> +};
> +
>  static int rk3288_lvds_probe(struct platform_device *pdev,
>  			     struct rockchip_lvds *lvds)
>  {
> @@ -491,6 +582,26 @@ static int rk3288_lvds_probe(struct platform_device *pdev,
>  	return 0;
>  }
>  
> +static int rockchip_lvds_phy_probe(struct platform_device *pdev,
> +				   struct rockchip_lvds *lvds)
> +{
> +	int ret;
> +
> +	lvds->dphy = devm_phy_get(&pdev->dev, "dphy");
> +	if (IS_ERR(lvds->dphy))
> +		return PTR_ERR(lvds->dphy);
> +
> +	ret = phy_init(lvds->dphy);
> +	if (ret)
> +		return ret;
> +
> +	ret = phy_set_mode(lvds->dphy, PHY_MODE_LVDS);
> +	if (ret)
> +		return ret;
> +
> +	return phy_power_on(lvds->dphy);
> +}

You factor out the steps done for px30 to a separate function in order
to reuse it on rk3568. You could make a separate patch from this to
make it easier to understand and to verify that there is no functional
change involved for the px30.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH 1/3] drm/rockchip: lvds: add rk3568 support

[Sascha Hauer]

On Thu, Jan 19, 2023 at 09:48:03PM +0300, Alibek Omarov wrote:
> One of the ports of RK3568 can be configured as LVDS, re-using the DSI DPHY
> 
> Signed-off-by: Alibek Omarov <a1ba.omarov@gmail.com>
> ---
>  drivers/gpu/drm/rockchip/rockchip_lvds.c | 144 +++++++++++++++++++++--
>  drivers/gpu/drm/rockchip/rockchip_lvds.h |  10 ++
>  2 files changed, 147 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/rockchip_lvds.c b/drivers/gpu/drm/rockchip/rockchip_lvds.c
> index 68f6ebb33460..83c60240af85 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_lvds.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_lvds.c
> @@ -433,6 +433,90 @@ static void px30_lvds_encoder_disable(struct drm_encoder *encoder)
>  	drm_panel_unprepare(lvds->panel);
>  }
>  
> +static int rk3568_lvds_poweron(struct rockchip_lvds *lvds)
> +{
> +	int ret;
> +
> +	ret = clk_enable(lvds->pclk);
> +	if (ret < 0) {
> +		DRM_DEV_ERROR(lvds->dev, "failed to enable lvds pclk %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = pm_runtime_get_sync(lvds->dev);
> +	if (ret < 0) {
> +		DRM_DEV_ERROR(lvds->dev, "failed to get pm runtime: %d\n", ret);
> +		clk_disable(lvds->pclk);
> +		return ret;
> +	}
> +
> +	/* Enable LVDS mode */
> +	return regmap_update_bits(lvds->grf, RK3568_GRF_VO_CON2,
> +				  RK3568_LVDS0_MODE_EN(1),
> +				  RK3568_LVDS0_MODE_EN(1));

Isn't this the same as:

	regmap_write(lvds->grf, RK3568_GRF_VO_CON2, RK3568_LVDS0_MODE_EN(1));

Unless I am missing something I find a plain regmap_write() easier to
read.

> +}
> +
> +static void rk3568_lvds_poweroff(struct rockchip_lvds *lvds)
> +{
> +	regmap_update_bits(lvds->grf, RK3568_GRF_VO_CON2,
> +			   RK3568_LVDS0_MODE_EN(1) | RK3568_LVDS0_P2S_EN(1),
> +			   RK3568_LVDS0_MODE_EN(0) | RK3568_LVDS0_P2S_EN(0));

Same here:

	regmap_write(lvds->grf, RK3568_GRF_VO_CON2,
		     RK3568_LVDS0_MODE_EN(0) | RK3568_LVDS0_P2S_EN(0));

What about the RK3568_LVDS0_P2S_EN bit? This is set in probe() and
cleared here. For symmetry reasons shouldn't it be set in
rk3568_lvds_poweron() instead?

> +
> +	pm_runtime_put(lvds->dev);
> +	clk_disable(lvds->pclk);
> +}
> +
> +static int rk3568_lvds_grf_config(struct drm_encoder *encoder,
> +				struct drm_display_mode *mode)
> +{
> +	struct rockchip_lvds *lvds = encoder_to_lvds(encoder);
> +
> +	if (lvds->output != DISPLAY_OUTPUT_LVDS) {
> +		DRM_DEV_ERROR(lvds->dev, "Unsupported display output %d\n",
> +			      lvds->output);
> +		return -EINVAL;
> +	}
> +
> +	/* Set format */
> +	return regmap_update_bits(lvds->grf, RK3568_GRF_VO_CON0,
> +				  RK3568_LVDS0_SELECT(3),
> +				  RK3568_LVDS0_SELECT(lvds->format));

It seems lvds->format does not match what the register expects. We
have:

#define LVDS_VESA_24                            0
#define LVDS_JEIDA_24                           1
#define LVDS_VESA_18                            2
#define LVDS_JEIDA_18                           3

According to the reference manual the register expects:

lvdsformat_lvds0_select
 2'b00: VESA 24bit
 2'b01: JEIDA 24bit
 2'b10: JEIDA 18bit
 2'b11: VESA 18bit

I only have the RK3568 manual but no PX30 or RK3288 manual, so I can't say if
they changed the register mapping between the SoCs or if it's wrong on
the other SoCs as well.

BTW you correctly set the mask to RK3568_LVDS0_SELECT(3), but for the
PX30 it looks wrong:

	return regmap_update_bits(lvds->grf, PX30_LVDS_GRF_PD_VO_CON1,
				  PX30_LVDS_FORMAT(lvds->format),
				  PX30_LVDS_FORMAT(lvds->format));

I really think regmap_write() would be better to use here to avoid such
things.

> +
> +static void rk3568_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;

'mode' is unused.

> +	int ret;
> +
> +	drm_panel_prepare(lvds->panel);
> +
> +	ret = rk3568_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 = rk3568_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;
> +	}
> +
> +	drm_panel_enable(lvds->panel);
> +}
> +
> +static void rk3568_lvds_encoder_disable(struct drm_encoder *encoder)
> +{
> +	struct rockchip_lvds *lvds = encoder_to_lvds(encoder);
> +
> +	drm_panel_disable(lvds->panel);
> +	rk3568_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,
> @@ -447,6 +531,13 @@ struct drm_encoder_helper_funcs px30_lvds_encoder_helper_funcs = {
>  	.atomic_check = rockchip_lvds_encoder_atomic_check,
>  };
>  
> +static const
> +struct drm_encoder_helper_funcs rk3568_lvds_encoder_helper_funcs = {
> +	.enable = rk3568_lvds_encoder_enable,
> +	.disable = rk3568_lvds_encoder_disable,
> +	.atomic_check = rockchip_lvds_encoder_atomic_check,
> +};
> +
>  static int rk3288_lvds_probe(struct platform_device *pdev,
>  			     struct rockchip_lvds *lvds)
>  {
> @@ -491,6 +582,26 @@ static int rk3288_lvds_probe(struct platform_device *pdev,
>  	return 0;
>  }
>  
> +static int rockchip_lvds_phy_probe(struct platform_device *pdev,
> +				   struct rockchip_lvds *lvds)
> +{
> +	int ret;
> +
> +	lvds->dphy = devm_phy_get(&pdev->dev, "dphy");
> +	if (IS_ERR(lvds->dphy))
> +		return PTR_ERR(lvds->dphy);
> +
> +	ret = phy_init(lvds->dphy);
> +	if (ret)
> +		return ret;
> +
> +	ret = phy_set_mode(lvds->dphy, PHY_MODE_LVDS);
> +	if (ret)
> +		return ret;
> +
> +	return phy_power_on(lvds->dphy);
> +}

You factor out the steps done for px30 to a separate function in order
to reuse it on rk3568. You could make a separate patch from this to
make it easier to understand and to verify that there is no functional
change involved for the px30.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/3] drm/rockchip: lvds: add rk3568 support

[Sverdlin, Alexander]

Hello Sascha,

On Fri, 2023-01-20 at 10:16 +0100, Sascha Hauer wrote:
> > +       /* Enable LVDS mode */
> > +       return regmap_update_bits(lvds->grf, RK3568_GRF_VO_CON2,
> > +                                 RK3568_LVDS0_MODE_EN(1),
> > +                                 RK3568_LVDS0_MODE_EN(1));
> 
> Isn't this the same as:
> 
>         regmap_write(lvds->grf, RK3568_GRF_VO_CON2,
> RK3568_LVDS0_MODE_EN(1));
> 
> Unless I am missing something I find a plain regmap_write() easier to
> read.

the former is setting a bit in a RMW operation, the latter is a plain
write, isn't it?

-- 
Alexander Sverdlin
Siemens AG
www.siemens.com

Re: [PATCH 1/3] drm/rockchip: lvds: add rk3568 support

[Sverdlin, Alexander]

Hello Sascha,

On Fri, 2023-01-20 at 10:16 +0100, Sascha Hauer wrote:
> > +       /* Enable LVDS mode */
> > +       return regmap_update_bits(lvds->grf, RK3568_GRF_VO_CON2,
> > +                                 RK3568_LVDS0_MODE_EN(1),
> > +                                 RK3568_LVDS0_MODE_EN(1));
> 
> Isn't this the same as:
> 
>         regmap_write(lvds->grf, RK3568_GRF_VO_CON2,
> RK3568_LVDS0_MODE_EN(1));
> 
> Unless I am missing something I find a plain regmap_write() easier to
> read.

the former is setting a bit in a RMW operation, the latter is a plain
write, isn't it?

-- 
Alexander Sverdlin
Siemens AG
www.siemens.com
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH 1/3] drm/rockchip: lvds: add rk3568 support

[Sverdlin, Alexander]

Hello Sascha,

On Fri, 2023-01-20 at 10:16 +0100, Sascha Hauer wrote:
> > +       /* Enable LVDS mode */
> > +       return regmap_update_bits(lvds->grf, RK3568_GRF_VO_CON2,
> > +                                 RK3568_LVDS0_MODE_EN(1),
> > +                                 RK3568_LVDS0_MODE_EN(1));
> 
> Isn't this the same as:
> 
>         regmap_write(lvds->grf, RK3568_GRF_VO_CON2,
> RK3568_LVDS0_MODE_EN(1));
> 
> Unless I am missing something I find a plain regmap_write() easier to
> read.

the former is setting a bit in a RMW operation, the latter is a plain
write, isn't it?

-- 
Alexander Sverdlin
Siemens AG
www.siemens.com
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/3] drm/rockchip: lvds: add rk3568 support

[Sverdlin, Alexander]

Hello Sascha,

On Fri, 2023-01-20 at 10:16 +0100, Sascha Hauer wrote:
> > +       /* Enable LVDS mode */
> > +       return regmap_update_bits(lvds->grf, RK3568_GRF_VO_CON2,
> > +                                 RK3568_LVDS0_MODE_EN(1),
> > +                                 RK3568_LVDS0_MODE_EN(1));
> 
> Isn't this the same as:
> 
>         regmap_write(lvds->grf, RK3568_GRF_VO_CON2,
> RK3568_LVDS0_MODE_EN(1));
> 
> Unless I am missing something I find a plain regmap_write() easier to
> read.

the former is setting a bit in a RMW operation, the latter is a plain
write, isn't it?

-- 
Alexander Sverdlin
Siemens AG
www.siemens.com

Re: [PATCH 1/3] drm/rockchip: lvds: add rk3568 support

[s.hauer]

On Fri, Jan 20, 2023 at 09:31:43AM +0000, Sverdlin, Alexander wrote:
> Hello Sascha,
> 
> On Fri, 2023-01-20 at 10:16 +0100, Sascha Hauer wrote:
> > > +       /* Enable LVDS mode */
> > > +       return regmap_update_bits(lvds->grf, RK3568_GRF_VO_CON2,
> > > +                                 RK3568_LVDS0_MODE_EN(1),
> > > +                                 RK3568_LVDS0_MODE_EN(1));
> > 
> > Isn't this the same as:
> > 
> >         regmap_write(lvds->grf, RK3568_GRF_VO_CON2,
> > RK3568_LVDS0_MODE_EN(1));
> > 
> > Unless I am missing something I find a plain regmap_write() easier to
> > read.
> 
> the former is setting a bit in a RMW operation, the latter is a plain
> write, isn't it?

That's right from the view what the function itself does. Note the
registers that are accessed here are a bit special. They effectively
are 16bit wide, the upper 16bit contain a mask. Only the bits set in the
mask are actually modified in the lower bits. See the register bit
definitions:

#define HIWORD_UPDATE(v, h, l)  ((GENMASK(h, l) << 16) | ((v) << (l)))

#define   RK3568_LVDS0_SELECT(val)             HIWORD_UPDATE(val,  5, 4)

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 1/3] drm/rockchip: lvds: add rk3568 support

[s.hauer]

On Fri, Jan 20, 2023 at 09:31:43AM +0000, Sverdlin, Alexander wrote:
> Hello Sascha,
> 
> On Fri, 2023-01-20 at 10:16 +0100, Sascha Hauer wrote:
> > > +       /* Enable LVDS mode */
> > > +       return regmap_update_bits(lvds->grf, RK3568_GRF_VO_CON2,
> > > +                                 RK3568_LVDS0_MODE_EN(1),
> > > +                                 RK3568_LVDS0_MODE_EN(1));
> > 
> > Isn't this the same as:
> > 
> >         regmap_write(lvds->grf, RK3568_GRF_VO_CON2,
> > RK3568_LVDS0_MODE_EN(1));
> > 
> > Unless I am missing something I find a plain regmap_write() easier to
> > read.
> 
> the former is setting a bit in a RMW operation, the latter is a plain
> write, isn't it?

That's right from the view what the function itself does. Note the
registers that are accessed here are a bit special. They effectively
are 16bit wide, the upper 16bit contain a mask. Only the bits set in the
mask are actually modified in the lower bits. See the register bit
definitions:

#define HIWORD_UPDATE(v, h, l)  ((GENMASK(h, l) << 16) | ((v) << (l)))

#define   RK3568_LVDS0_SELECT(val)             HIWORD_UPDATE(val,  5, 4)

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 1/3] drm/rockchip: lvds: add rk3568 support

[s.hauer]

On Fri, Jan 20, 2023 at 09:31:43AM +0000, Sverdlin, Alexander wrote:
> Hello Sascha,
> 
> On Fri, 2023-01-20 at 10:16 +0100, Sascha Hauer wrote:
> > > +       /* Enable LVDS mode */
> > > +       return regmap_update_bits(lvds->grf, RK3568_GRF_VO_CON2,
> > > +                                 RK3568_LVDS0_MODE_EN(1),
> > > +                                 RK3568_LVDS0_MODE_EN(1));
> > 
> > Isn't this the same as:
> > 
> >         regmap_write(lvds->grf, RK3568_GRF_VO_CON2,
> > RK3568_LVDS0_MODE_EN(1));
> > 
> > Unless I am missing something I find a plain regmap_write() easier to
> > read.
> 
> the former is setting a bit in a RMW operation, the latter is a plain
> write, isn't it?

That's right from the view what the function itself does. Note the
registers that are accessed here are a bit special. They effectively
are 16bit wide, the upper 16bit contain a mask. Only the bits set in the
mask are actually modified in the lower bits. See the register bit
definitions:

#define HIWORD_UPDATE(v, h, l)  ((GENMASK(h, l) << 16) | ((v) << (l)))

#define   RK3568_LVDS0_SELECT(val)             HIWORD_UPDATE(val,  5, 4)

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH 1/3] drm/rockchip: lvds: add rk3568 support

[s.hauer]

On Fri, Jan 20, 2023 at 09:31:43AM +0000, Sverdlin, Alexander wrote:
> Hello Sascha,
> 
> On Fri, 2023-01-20 at 10:16 +0100, Sascha Hauer wrote:
> > > +       /* Enable LVDS mode */
> > > +       return regmap_update_bits(lvds->grf, RK3568_GRF_VO_CON2,
> > > +                                 RK3568_LVDS0_MODE_EN(1),
> > > +                                 RK3568_LVDS0_MODE_EN(1));
> > 
> > Isn't this the same as:
> > 
> >         regmap_write(lvds->grf, RK3568_GRF_VO_CON2,
> > RK3568_LVDS0_MODE_EN(1));
> > 
> > Unless I am missing something I find a plain regmap_write() easier to
> > read.
> 
> the former is setting a bit in a RMW operation, the latter is a plain
> write, isn't it?

That's right from the view what the function itself does. Note the
registers that are accessed here are a bit special. They effectively
are 16bit wide, the upper 16bit contain a mask. Only the bits set in the
mask are actually modified in the lower bits. See the register bit
definitions:

#define HIWORD_UPDATE(v, h, l)  ((GENMASK(h, l) << 16) | ((v) << (l)))

#define   RK3568_LVDS0_SELECT(val)             HIWORD_UPDATE(val,  5, 4)

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel