linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/5] Rockchip dw-mipi-dsi driver
@ 2017-01-20 10:10 Chris Zhong
  2017-01-20 10:10 ` [PATCH v3 1/5] dt-bindings: add rk3399 support for dw-mipi-rockchip Chris Zhong
                   ` (4 more replies)
  0 siblings, 5 replies; 13+ messages in thread
From: Chris Zhong @ 2017-01-20 10:10 UTC (permalink / raw)
  To: dianders, tfiga, heiko, yzq, mark.rutland, devicetree, robh+dt,
	galak, pawel.moll, seanpaul
  Cc: linux-rockchip, Chris Zhong, Mark Yao, linux-kernel, dri-devel,
	David Airlie, linux-arm-kernel

Hi all

This patch serial is for RK3399 MIPI DSI. The MIPI DSI controller of
RK3399 is almost the same as RK3288, except a little bit of difference
in phy clock controlling and port id selection register. These patches
add RK3399 support and the power domain support.

And these patches base on John Keeping's series[0], it fixes many bugs,
they have been tested on rk3288 evb board.

[0]:
[01/26] https://patchwork.kernel.org/patch/9340213
[02/26] https://patchwork.kernel.org/patch/9340145
[03/26] https://patchwork.kernel.org/patch/9340235
[04/26] https://patchwork.kernel.org/patch/9340123
[05/26] https://patchwork.kernel.org/patch/9340161
[06/26] https://patchwork.kernel.org/patch/9340203
[07/26] https://patchwork.kernel.org/patch/9340229
[08/26] https://patchwork.kernel.org/patch/9340131
[09/26] https://patchwork.kernel.org/patch/9340191
[10/26] https://patchwork.kernel.org/patch/9340175
[11/26] https://patchwork.kernel.org/patch/9340237
[12/26] https://patchwork.kernel.org/patch/9340207
[13/26] https://patchwork.kernel.org/patch/9340233
[14/26] https://patchwork.kernel.org/patch/9340205
[15/26] https://patchwork.kernel.org/patch/9340189
[16/26] https://patchwork.kernel.org/patch/9340143
[17/26] https://patchwork.kernel.org/patch/9340117
[18/26] https://patchwork.kernel.org/patch/9340193
[19/26] https://patchwork.kernel.org/patch/9340151
[20/26] https://patchwork.kernel.org/patch/9340183
[23/26] https://patchwork.kernel.org/patch/9340173
[24/26] https://patchwork.kernel.org/patch/9340251
[25/26] https://patchwork.kernel.org/patch/9340127
[26/26] https://patchwork.kernel.org/patch/9340139


Changes in v3:
- base on John Keeping's patch series

Chris Zhong (5):
  dt-bindings: add rk3399 support for dw-mipi-rockchip
  drm/rockchip/dsi: dw-mipi: support RK3399 mipi dsi
  drm/rockchip/dsi: remove mode_valid function
  dt-bindings: add power domain node for dw-mipi-rockchip
  drm/rockchip/dsi: add dw-mipi power domain support

 .../display/rockchip/dw_mipi_dsi_rockchip.txt      |   7 +-
 drivers/gpu/drm/rockchip/dw-mipi-dsi.c             | 156 ++++++++++++---------
 2 files changed, 96 insertions(+), 67 deletions(-)

-- 
2.6.3

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v3 1/5] dt-bindings: add rk3399 support for dw-mipi-rockchip
  2017-01-20 10:10 [PATCH v3 0/5] Rockchip dw-mipi-dsi driver Chris Zhong
@ 2017-01-20 10:10 ` Chris Zhong
  2017-01-20 10:10 ` [PATCH v3 2/5] drm/rockchip/dsi: dw-mipi: support RK3399 mipi dsi Chris Zhong
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 13+ messages in thread
From: Chris Zhong @ 2017-01-20 10:10 UTC (permalink / raw)
  To: dianders, tfiga, heiko, yzq, mark.rutland, devicetree, robh+dt,
	galak, pawel.moll, seanpaul
  Cc: linux-rockchip, Chris Zhong, Mark Yao, David Airlie, dri-devel,
	linux-arm-kernel, linux-kernel

The dw-mipi-dsi of rk3399 is almost the same as rk3288, the rk3399 has
additional phy config clock.

Signed-off-by: Chris Zhong <zyw@rock-chips.com>
Acked-by: Rob Herring <robh@kernel.org>
---

Changes in v3: None

 .../devicetree/bindings/display/rockchip/dw_mipi_dsi_rockchip.txt     | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/display/rockchip/dw_mipi_dsi_rockchip.txt b/Documentation/devicetree/bindings/display/rockchip/dw_mipi_dsi_rockchip.txt
index 1753f0c..0f82568 100644
--- a/Documentation/devicetree/bindings/display/rockchip/dw_mipi_dsi_rockchip.txt
+++ b/Documentation/devicetree/bindings/display/rockchip/dw_mipi_dsi_rockchip.txt
@@ -5,10 +5,12 @@ Required properties:
 - #address-cells: Should be <1>.
 - #size-cells: Should be <0>.
 - compatible: "rockchip,rk3288-mipi-dsi", "snps,dw-mipi-dsi".
+	      "rockchip,rk3399-mipi-dsi", "snps,dw-mipi-dsi".
 - reg: Represent the physical address range of the controller.
 - interrupts: Represent the controller's interrupt to the CPU(s).
 - clocks, clock-names: Phandles to the controller's pll reference
-  clock(ref) and APB clock(pclk), as described in [1].
+  clock(ref) and APB clock(pclk). For RK3399, a phy config clock
+  (phy_cfg) is additional required. As described in [1].
 - rockchip,grf: this soc should set GRF regs to mux vopl/vopb.
 - ports: contain a port node with endpoint definitions as defined in [2].
   For vopb,set the reg = <0> and set the reg = <1> for vopl.
-- 
2.6.3

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v3 2/5] drm/rockchip/dsi: dw-mipi: support RK3399 mipi dsi
  2017-01-20 10:10 [PATCH v3 0/5] Rockchip dw-mipi-dsi driver Chris Zhong
  2017-01-20 10:10 ` [PATCH v3 1/5] dt-bindings: add rk3399 support for dw-mipi-rockchip Chris Zhong
@ 2017-01-20 10:10 ` Chris Zhong
  2017-01-23 17:30   ` Sean Paul
  2017-01-20 10:10 ` [PATCH v3 3/5] drm/rockchip/dsi: remove mode_valid function Chris Zhong
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 13+ messages in thread
From: Chris Zhong @ 2017-01-20 10:10 UTC (permalink / raw)
  To: dianders, tfiga, heiko, yzq, mark.rutland, devicetree, robh+dt,
	galak, pawel.moll, seanpaul
  Cc: linux-rockchip, Chris Zhong, Mark Yao, David Airlie, dri-devel,
	linux-arm-kernel, linux-kernel

The vopb/vopl switch register of RK3399 mipi is different from RK3288,
the default setting for mipi dsi mode is different too, so add a
of_device_id structure to distinguish them, and make sure set the
correct mode before mipi phy init.

Signed-off-by: Chris Zhong <zyw@rock-chips.com>
Signed-off-by: Mark Yao <mark.yao@rock-chips.com>
---

Changes in v3:
- base on John Keeping's patch series

 drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 101 ++++++++++++++++++++++++---------
 1 file changed, 74 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
index 45af890..a93ce97 100644
--- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
+++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
@@ -29,9 +29,17 @@
 
 #define DRIVER_NAME    "dw-mipi-dsi"
 
-#define GRF_SOC_CON6                    0x025c
-#define DSI0_SEL_VOP_LIT                (1 << 6)
-#define DSI1_SEL_VOP_LIT                (1 << 9)
+#define RK3288_GRF_SOC_CON6		0x025c
+#define RK3288_DSI0_SEL_VOP_LIT		BIT(6)
+#define RK3288_DSI1_SEL_VOP_LIT		BIT(9)
+
+#define RK3399_GRF_SOC_CON19		0x6250
+#define RK3399_DSI0_SEL_VOP_LIT		BIT(0)
+#define RK3399_DSI1_SEL_VOP_LIT		BIT(4)
+
+/* disable turnrequest, turndisable, forcetxstopmode, forcerxmode */
+#define RK3399_GRF_SOC_CON22		0x6258
+#define RK3399_GRF_DSI_MODE		0xffff0000
 
 #define DSI_VERSION			0x00
 #define DSI_PWR_UP			0x04
@@ -149,7 +157,6 @@
 #define LPRX_TO_CNT(p)			((p) & 0xffff)
 
 #define DSI_BTA_TO_CNT			0x8c
-
 #define DSI_LPCLK_CTRL			0x94
 #define AUTO_CLKLANE_CTRL		BIT(1)
 #define PHY_TXREQUESTCLKHS		BIT(0)
@@ -215,11 +222,11 @@
 
 #define HSFREQRANGE_SEL(val)	(((val) & 0x3f) << 1)
 
-#define INPUT_DIVIDER(val)	((val - 1) & 0x7f)
+#define INPUT_DIVIDER(val)	(((val) - 1) & 0x7f)
 #define LOW_PROGRAM_EN		0
 #define HIGH_PROGRAM_EN		BIT(7)
-#define LOOP_DIV_LOW_SEL(val)	((val - 1) & 0x1f)
-#define LOOP_DIV_HIGH_SEL(val)	(((val - 1) >> 5) & 0x1f)
+#define LOOP_DIV_LOW_SEL(val)	(((val) - 1) & 0x1f)
+#define LOOP_DIV_HIGH_SEL(val)	((((val) - 1) >> 5) & 0x1f)
 #define PLL_LOOP_DIV_EN		BIT(5)
 #define PLL_INPUT_DIV_EN	BIT(4)
 
@@ -265,6 +272,11 @@ enum {
 };
 
 struct dw_mipi_dsi_plat_data {
+	u32 dsi0_en_bit;
+	u32 dsi1_en_bit;
+	u32 grf_switch_reg;
+	u32 grf_dsi0_mode;
+	u32 grf_dsi0_mode_reg;
 	unsigned int max_data_lanes;
 	enum drm_mode_status (*mode_valid)(struct drm_connector *connector,
 					   struct drm_display_mode *mode);
@@ -281,6 +293,7 @@ struct dw_mipi_dsi {
 
 	struct clk *pllref_clk;
 	struct clk *pclk;
+	struct clk *phy_cfg_clk;
 
 	unsigned int lane_mbps; /* per lane */
 	u32 channel;
@@ -356,6 +369,7 @@ static inline struct dw_mipi_dsi *encoder_to_dsi(struct drm_encoder *encoder)
 {
 	return container_of(encoder, struct dw_mipi_dsi, encoder);
 }
+
 static inline void dsi_write(struct dw_mipi_dsi *dsi, u32 reg, u32 val)
 {
 	writel(val, dsi->base + reg);
@@ -367,7 +381,7 @@ static inline u32 dsi_read(struct dw_mipi_dsi *dsi, u32 reg)
 }
 
 static void dw_mipi_dsi_phy_write(struct dw_mipi_dsi *dsi, u8 test_code,
-				 u8 test_data)
+				  u8 test_data)
 {
 	/*
 	 * With the falling edge on TESTCLK, the TESTDIN[7:0] signal content
@@ -426,6 +440,14 @@ static int dw_mipi_dsi_phy_init(struct dw_mipi_dsi *dsi)
 	dsi_write(dsi, DSI_PHY_TST_CTRL0, PHY_TESTCLR);
 	dsi_write(dsi, DSI_PHY_TST_CTRL0, PHY_UNTESTCLR);
 
+	if (!IS_ERR(dsi->phy_cfg_clk)) {
+		ret = clk_prepare_enable(dsi->phy_cfg_clk);
+		if (ret) {
+			dev_err(dsi->dev, "Failed to enable phy_cfg_clk\n");
+			return ret;
+		}
+	}
+
 	dw_mipi_dsi_phy_write(dsi, 0x10, BYPASS_VCO_RANGE |
 					 VCO_RANGE_CON_SEL(vco) |
 					 VCO_IN_CAP_CON_LOW |
@@ -474,22 +496,23 @@ static int dw_mipi_dsi_phy_init(struct dw_mipi_dsi *dsi)
 	dsi_write(dsi, DSI_PHY_RSTZ, PHY_ENFORCEPLL | PHY_ENABLECLK |
 				     PHY_UNRSTZ | PHY_UNSHUTDOWNZ);
 
-
 	ret = readl_poll_timeout(dsi->base + DSI_PHY_STATUS,
 				 val, val & LOCK, 1000, PHY_STATUS_TIMEOUT_US);
 	if (ret < 0) {
 		dev_err(dsi->dev, "failed to wait for phy lock state\n");
-		return ret;
+		goto phy_init_end;
 	}
 
 	ret = readl_poll_timeout(dsi->base + DSI_PHY_STATUS,
 				 val, val & STOP_STATE_CLK_LANE, 1000,
 				 PHY_STATUS_TIMEOUT_US);
-	if (ret < 0) {
+	if (ret < 0)
 		dev_err(dsi->dev,
 			"failed to wait for phy clk lane stop state\n");
-		return ret;
-	}
+
+phy_init_end:
+	if (!IS_ERR(dsi->phy_cfg_clk))
+		clk_disable_unprepare(dsi->phy_cfg_clk);
 
 	return ret;
 }
@@ -548,7 +571,7 @@ static int dw_mipi_dsi_host_attach(struct mipi_dsi_host *host,
 
 	if (device->lanes > dsi->pdata->max_data_lanes) {
 		dev_err(dsi->dev, "the number of data lanes(%u) is too many\n",
-				device->lanes);
+			device->lanes);
 		return -EINVAL;
 	}
 
@@ -936,8 +959,8 @@ static void dw_mipi_dsi_clear_err(struct dw_mipi_dsi *dsi)
 }
 
 static void dw_mipi_dsi_encoder_mode_set(struct drm_encoder *encoder,
-					struct drm_display_mode *mode,
-					struct drm_display_mode *adjusted_mode)
+					 struct drm_display_mode *mode,
+					 struct drm_display_mode *adjusted_mode)
 {
 	struct dw_mipi_dsi *dsi = encoder_to_dsi(encoder);
 
@@ -965,6 +988,7 @@ static void dw_mipi_dsi_encoder_disable(struct drm_encoder *encoder)
 static void dw_mipi_dsi_encoder_enable(struct drm_encoder *encoder)
 {
 	struct dw_mipi_dsi *dsi = encoder_to_dsi(encoder);
+	const struct dw_mipi_dsi_plat_data *pdata = dsi->pdata;
 	int mux = drm_of_encoder_active_endpoint_id(dsi->dev->of_node, encoder);
 	u32 val;
 
@@ -985,6 +1009,10 @@ static void dw_mipi_dsi_encoder_enable(struct drm_encoder *encoder)
 	dw_mipi_dsi_dphy_interface_config(dsi);
 	dw_mipi_dsi_clear_err(dsi);
 
+	if (pdata->grf_dsi0_mode_reg)
+		regmap_write(dsi->grf_regmap, pdata->grf_dsi0_mode_reg,
+			     pdata->grf_dsi0_mode);
+
 	dw_mipi_dsi_phy_init(dsi);
 	dw_mipi_dsi_wait_for_two_frames(dsi);
 
@@ -998,11 +1026,11 @@ static void dw_mipi_dsi_encoder_enable(struct drm_encoder *encoder)
 	clk_disable_unprepare(dsi->pclk);
 
 	if (mux)
-		val = DSI0_SEL_VOP_LIT | (DSI0_SEL_VOP_LIT << 16);
+		val = pdata->dsi0_en_bit | (pdata->dsi0_en_bit << 16);
 	else
-		val = DSI0_SEL_VOP_LIT << 16;
+		val = pdata->dsi0_en_bit << 16;
 
-	regmap_write(dsi->grf_regmap, GRF_SOC_CON6, val);
+	regmap_write(dsi->grf_regmap, pdata->grf_switch_reg, val);
 	dev_dbg(dsi->dev, "vop %s output to dsi0\n", (mux) ? "LIT" : "BIG");
 }
 
@@ -1034,7 +1062,7 @@ dw_mipi_dsi_encoder_atomic_check(struct drm_encoder *encoder,
 	return 0;
 }
 
-static struct drm_encoder_helper_funcs
+static const struct drm_encoder_helper_funcs
 dw_mipi_dsi_encoder_helper_funcs = {
 	.enable = dw_mipi_dsi_encoder_enable,
 	.mode_set = dw_mipi_dsi_encoder_mode_set,
@@ -1042,7 +1070,7 @@ dw_mipi_dsi_encoder_helper_funcs = {
 	.atomic_check = dw_mipi_dsi_encoder_atomic_check,
 };
 
-static struct drm_encoder_funcs dw_mipi_dsi_encoder_funcs = {
+static const struct drm_encoder_funcs dw_mipi_dsi_encoder_funcs = {
 	.destroy = drm_encoder_cleanup,
 };
 
@@ -1078,7 +1106,7 @@ static void dw_mipi_dsi_drm_connector_destroy(struct drm_connector *connector)
 	drm_connector_cleanup(connector);
 }
 
-static struct drm_connector_funcs dw_mipi_dsi_atomic_connector_funcs = {
+static const struct drm_connector_funcs dw_mipi_dsi_atomic_connector_funcs = {
 	.dpms = drm_atomic_helper_connector_dpms,
 	.fill_modes = drm_helper_probe_single_connector_modes,
 	.destroy = dw_mipi_dsi_drm_connector_destroy,
@@ -1088,7 +1116,7 @@ static struct drm_connector_funcs dw_mipi_dsi_atomic_connector_funcs = {
 };
 
 static int dw_mipi_dsi_register(struct drm_device *drm,
-				      struct dw_mipi_dsi *dsi)
+				struct dw_mipi_dsi *dsi)
 {
 	struct drm_encoder *encoder = &dsi->encoder;
 	struct drm_connector *connector = &dsi->connector;
@@ -1109,14 +1137,14 @@ static int dw_mipi_dsi_register(struct drm_device *drm,
 	drm_encoder_helper_add(&dsi->encoder,
 			       &dw_mipi_dsi_encoder_helper_funcs);
 	ret = drm_encoder_init(drm, &dsi->encoder, &dw_mipi_dsi_encoder_funcs,
-			 DRM_MODE_ENCODER_DSI, NULL);
+			       DRM_MODE_ENCODER_DSI, NULL);
 	if (ret) {
 		dev_err(dev, "Failed to initialize encoder with drm\n");
 		return ret;
 	}
 
 	drm_connector_helper_add(connector,
-			&dw_mipi_dsi_connector_helper_funcs);
+				 &dw_mipi_dsi_connector_helper_funcs);
 
 	drm_connector_init(drm, &dsi->connector,
 			   &dw_mipi_dsi_atomic_connector_funcs,
@@ -1162,21 +1190,36 @@ static enum drm_mode_status rk3288_mipi_dsi_mode_valid(
 }
 
 static struct dw_mipi_dsi_plat_data rk3288_mipi_dsi_drv_data = {
+	.dsi0_en_bit = RK3288_DSI0_SEL_VOP_LIT,
+	.dsi1_en_bit = RK3288_DSI1_SEL_VOP_LIT,
+	.grf_switch_reg = RK3288_GRF_SOC_CON6,
 	.max_data_lanes = 4,
 	.mode_valid = rk3288_mipi_dsi_mode_valid,
 };
 
+static struct dw_mipi_dsi_plat_data rk3399_mipi_dsi_drv_data = {
+	.dsi0_en_bit = RK3399_DSI0_SEL_VOP_LIT,
+	.dsi1_en_bit = RK3399_DSI1_SEL_VOP_LIT,
+	.grf_switch_reg = RK3399_GRF_SOC_CON19,
+	.grf_dsi0_mode = RK3399_GRF_DSI_MODE,
+	.grf_dsi0_mode_reg = RK3399_GRF_SOC_CON22,
+	.max_data_lanes = 4,
+};
+
 static const struct of_device_id dw_mipi_dsi_dt_ids[] = {
 	{
 	 .compatible = "rockchip,rk3288-mipi-dsi",
 	 .data = &rk3288_mipi_dsi_drv_data,
+	}, {
+	 .compatible = "rockchip,rk3399-mipi-dsi",
+	 .data = &rk3399_mipi_dsi_drv_data,
 	},
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, dw_mipi_dsi_dt_ids);
 
 static int dw_mipi_dsi_bind(struct device *dev, struct device *master,
-			     void *data)
+			    void *data)
 {
 	const struct of_device_id *of_id =
 			of_match_device(dw_mipi_dsi_dt_ids, dev);
@@ -1249,6 +1292,10 @@ static int dw_mipi_dsi_bind(struct device *dev, struct device *master,
 		clk_disable_unprepare(dsi->pclk);
 	}
 
+	dsi->phy_cfg_clk = devm_clk_get(dev, "phy_cfg");
+	if (IS_ERR(dsi->phy_cfg_clk))
+		dev_dbg(dev, "have not phy_cfg_clk\n");
+
 	ret = clk_prepare_enable(dsi->pllref_clk);
 	if (ret) {
 		dev_err(dev, "%s: Failed to enable pllref_clk\n", __func__);
@@ -1280,7 +1327,7 @@ static int dw_mipi_dsi_bind(struct device *dev, struct device *master,
 }
 
 static void dw_mipi_dsi_unbind(struct device *dev, struct device *master,
-	void *data)
+			       void *data)
 {
 	struct dw_mipi_dsi *dsi = dev_get_drvdata(dev);
 
-- 
2.6.3

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v3 3/5] drm/rockchip/dsi: remove mode_valid function
  2017-01-20 10:10 [PATCH v3 0/5] Rockchip dw-mipi-dsi driver Chris Zhong
  2017-01-20 10:10 ` [PATCH v3 1/5] dt-bindings: add rk3399 support for dw-mipi-rockchip Chris Zhong
  2017-01-20 10:10 ` [PATCH v3 2/5] drm/rockchip/dsi: dw-mipi: support RK3399 mipi dsi Chris Zhong
@ 2017-01-20 10:10 ` Chris Zhong
  2017-01-23 17:48   ` Sean Paul
  2017-01-20 10:10 ` [PATCH v3 4/5] dt-bindings: add power domain node for dw-mipi-rockchip Chris Zhong
  2017-01-20 10:10 ` [PATCH v3 5/5] drm/rockchip/dsi: add dw-mipi power domain support Chris Zhong
  4 siblings, 1 reply; 13+ messages in thread
From: Chris Zhong @ 2017-01-20 10:10 UTC (permalink / raw)
  To: dianders, tfiga, heiko, yzq, mark.rutland, devicetree, robh+dt,
	galak, pawel.moll, seanpaul
  Cc: linux-rockchip, Chris Zhong, Mark Yao, David Airlie, dri-devel,
	linux-arm-kernel, linux-kernel

The MIPI DSI do not need check the validity of resolution, the max
resolution should depend VOP. Hence, remove rk3288_mipi_dsi_mode_valid
here.

Signed-off-by: Chris Zhong <zyw@rock-chips.com>
---

Changes in v3: None

 drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 39 ----------------------------------
 1 file changed, 39 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
index a93ce97..6f0e252 100644
--- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
+++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
@@ -278,8 +278,6 @@ struct dw_mipi_dsi_plat_data {
 	u32 grf_dsi0_mode;
 	u32 grf_dsi0_mode_reg;
 	unsigned int max_data_lanes;
-	enum drm_mode_status (*mode_valid)(struct drm_connector *connector,
-					   struct drm_display_mode *mode);
 };
 
 struct dw_mipi_dsi {
@@ -1081,23 +1079,8 @@ static int dw_mipi_dsi_connector_get_modes(struct drm_connector *connector)
 	return drm_panel_get_modes(dsi->panel);
 }
 
-static enum drm_mode_status dw_mipi_dsi_mode_valid(
-					struct drm_connector *connector,
-					struct drm_display_mode *mode)
-{
-	struct dw_mipi_dsi *dsi = con_to_dsi(connector);
-
-	enum drm_mode_status mode_status = MODE_OK;
-
-	if (dsi->pdata->mode_valid)
-		mode_status = dsi->pdata->mode_valid(connector, mode);
-
-	return mode_status;
-}
-
 static struct drm_connector_helper_funcs dw_mipi_dsi_connector_helper_funcs = {
 	.get_modes = dw_mipi_dsi_connector_get_modes,
-	.mode_valid = dw_mipi_dsi_mode_valid,
 };
 
 static void dw_mipi_dsi_drm_connector_destroy(struct drm_connector *connector)
@@ -1168,33 +1151,11 @@ static int rockchip_mipi_parse_dt(struct dw_mipi_dsi *dsi)
 	return 0;
 }
 
-static enum drm_mode_status rk3288_mipi_dsi_mode_valid(
-					struct drm_connector *connector,
-					struct drm_display_mode *mode)
-{
-	/*
-	 * The VID_PKT_SIZE field in the DSI_VID_PKT_CFG
-	 * register is 11-bit.
-	 */
-	if (mode->hdisplay > 0x7ff)
-		return MODE_BAD_HVALUE;
-
-	/*
-	 * The V_ACTIVE_LINES field in the DSI_VTIMING_CFG
-	 * register is 11-bit.
-	 */
-	if (mode->vdisplay > 0x7ff)
-		return MODE_BAD_VVALUE;
-
-	return MODE_OK;
-}
-
 static struct dw_mipi_dsi_plat_data rk3288_mipi_dsi_drv_data = {
 	.dsi0_en_bit = RK3288_DSI0_SEL_VOP_LIT,
 	.dsi1_en_bit = RK3288_DSI1_SEL_VOP_LIT,
 	.grf_switch_reg = RK3288_GRF_SOC_CON6,
 	.max_data_lanes = 4,
-	.mode_valid = rk3288_mipi_dsi_mode_valid,
 };
 
 static struct dw_mipi_dsi_plat_data rk3399_mipi_dsi_drv_data = {
-- 
2.6.3

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v3 4/5] dt-bindings: add power domain node for dw-mipi-rockchip
  2017-01-20 10:10 [PATCH v3 0/5] Rockchip dw-mipi-dsi driver Chris Zhong
                   ` (2 preceding siblings ...)
  2017-01-20 10:10 ` [PATCH v3 3/5] drm/rockchip/dsi: remove mode_valid function Chris Zhong
@ 2017-01-20 10:10 ` Chris Zhong
  2017-01-20 10:10 ` [PATCH v3 5/5] drm/rockchip/dsi: add dw-mipi power domain support Chris Zhong
  4 siblings, 0 replies; 13+ messages in thread
From: Chris Zhong @ 2017-01-20 10:10 UTC (permalink / raw)
  To: dianders, tfiga, heiko, yzq, mark.rutland, devicetree, robh+dt,
	galak, pawel.moll, seanpaul
  Cc: linux-rockchip, Chris Zhong, Mark Yao, David Airlie, dri-devel,
	linux-arm-kernel, linux-kernel

Signed-off-by: Chris Zhong <zyw@rock-chips.com>
Acked-by: Rob Herring <robh@kernel.org>
---

Changes in v3: None

 .../devicetree/bindings/display/rockchip/dw_mipi_dsi_rockchip.txt      | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/rockchip/dw_mipi_dsi_rockchip.txt b/Documentation/devicetree/bindings/display/rockchip/dw_mipi_dsi_rockchip.txt
index 0f82568..188f6f7 100644
--- a/Documentation/devicetree/bindings/display/rockchip/dw_mipi_dsi_rockchip.txt
+++ b/Documentation/devicetree/bindings/display/rockchip/dw_mipi_dsi_rockchip.txt
@@ -15,6 +15,9 @@ Required properties:
 - ports: contain a port node with endpoint definitions as defined in [2].
   For vopb,set the reg = <0> and set the reg = <1> for vopl.
 
+Optional properties:
+- power-domains: a phandle to mipi dsi power domain node.
+
 [1] Documentation/devicetree/bindings/clock/clock-bindings.txt
 [2] Documentation/devicetree/bindings/media/video-interfaces.txt
 
-- 
2.6.3

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v3 5/5] drm/rockchip/dsi: add dw-mipi power domain support
  2017-01-20 10:10 [PATCH v3 0/5] Rockchip dw-mipi-dsi driver Chris Zhong
                   ` (3 preceding siblings ...)
  2017-01-20 10:10 ` [PATCH v3 4/5] dt-bindings: add power domain node for dw-mipi-rockchip Chris Zhong
@ 2017-01-20 10:10 ` Chris Zhong
  2017-01-23 18:03   ` Sean Paul
  4 siblings, 1 reply; 13+ messages in thread
From: Chris Zhong @ 2017-01-20 10:10 UTC (permalink / raw)
  To: dianders, tfiga, heiko, yzq, mark.rutland, devicetree, robh+dt,
	galak, pawel.moll, seanpaul
  Cc: linux-rockchip, Chris Zhong, Mark Yao, David Airlie, dri-devel,
	linux-arm-kernel, linux-kernel

Reference the power domain incase dw-mipi power down when
in use.

Signed-off-by: Chris Zhong <zyw@rock-chips.com>
---

Changes in v3: None

 drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
index 6f0e252..1462101e 100644
--- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
+++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
@@ -12,6 +12,7 @@
 #include <linux/math64.h>
 #include <linux/module.h>
 #include <linux/of_device.h>
+#include <linux/pm_runtime.h>
 #include <linux/regmap.h>
 #include <linux/reset.h>
 #include <linux/mfd/syscon.h>
@@ -293,6 +294,7 @@ struct dw_mipi_dsi {
 	struct clk *pclk;
 	struct clk *phy_cfg_clk;
 
+	int dpms_mode;
 	unsigned int lane_mbps; /* per lane */
 	u32 channel;
 	u32 lanes;
@@ -969,6 +971,9 @@ static void dw_mipi_dsi_encoder_disable(struct drm_encoder *encoder)
 {
 	struct dw_mipi_dsi *dsi = encoder_to_dsi(encoder);
 
+	if (dsi->dpms_mode != DRM_MODE_DPMS_ON)
+		return;
+
 	if (clk_prepare_enable(dsi->pclk)) {
 		dev_err(dsi->dev, "%s: Failed to enable pclk\n", __func__);
 		return;
@@ -980,7 +985,9 @@ static void dw_mipi_dsi_encoder_disable(struct drm_encoder *encoder)
 	drm_panel_unprepare(dsi->panel);
 
 	dw_mipi_dsi_disable(dsi);
+	pm_runtime_put(dsi->dev);
 	clk_disable_unprepare(dsi->pclk);
+	dsi->dpms_mode = DRM_MODE_DPMS_OFF;
 }
 
 static void dw_mipi_dsi_encoder_enable(struct drm_encoder *encoder)
@@ -990,11 +997,15 @@ static void dw_mipi_dsi_encoder_enable(struct drm_encoder *encoder)
 	int mux = drm_of_encoder_active_endpoint_id(dsi->dev->of_node, encoder);
 	u32 val;
 
+	if (dsi->dpms_mode == DRM_MODE_DPMS_ON)
+		return;
+
 	if (clk_prepare_enable(dsi->pclk)) {
 		dev_err(dsi->dev, "%s: Failed to enable pclk\n", __func__);
 		return;
 	}
 
+	pm_runtime_get_sync(dsi->dev);
 	dw_mipi_dsi_init(dsi);
 	dw_mipi_dsi_dpi_config(dsi);
 	dw_mipi_dsi_packet_handler_config(dsi);
@@ -1030,6 +1041,7 @@ static void dw_mipi_dsi_encoder_enable(struct drm_encoder *encoder)
 
 	regmap_write(dsi->grf_regmap, pdata->grf_switch_reg, val);
 	dev_dbg(dsi->dev, "vop %s output to dsi0\n", (mux) ? "LIT" : "BIG");
+	dsi->dpms_mode = DRM_MODE_DPMS_ON;
 }
 
 static int
@@ -1198,6 +1210,7 @@ static int dw_mipi_dsi_bind(struct device *dev, struct device *master,
 
 	dsi->dev = dev;
 	dsi->pdata = pdata;
+	dsi->dpms_mode = DRM_MODE_DPMS_OFF;
 
 	ret = rockchip_mipi_parse_dt(dsi);
 	if (ret)
@@ -1271,6 +1284,8 @@ static int dw_mipi_dsi_bind(struct device *dev, struct device *master,
 
 	dev_set_drvdata(dev, dsi);
 
+	pm_runtime_enable(dev);
+
 	dsi->dsi_host.ops = &dw_mipi_dsi_host_ops;
 	dsi->dsi_host.dev = dev;
 	ret = mipi_dsi_host_register(&dsi->dsi_host);
@@ -1293,6 +1308,7 @@ static void dw_mipi_dsi_unbind(struct device *dev, struct device *master,
 	struct dw_mipi_dsi *dsi = dev_get_drvdata(dev);
 
 	mipi_dsi_host_unregister(&dsi->dsi_host);
+	pm_runtime_disable(dev);
 	clk_disable_unprepare(dsi->pllref_clk);
 }
 
-- 
2.6.3

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 2/5] drm/rockchip/dsi: dw-mipi: support RK3399 mipi dsi
  2017-01-20 10:10 ` [PATCH v3 2/5] drm/rockchip/dsi: dw-mipi: support RK3399 mipi dsi Chris Zhong
@ 2017-01-23 17:30   ` Sean Paul
  0 siblings, 0 replies; 13+ messages in thread
From: Sean Paul @ 2017-01-23 17:30 UTC (permalink / raw)
  To: Chris Zhong
  Cc: dianders, tfiga, heiko, yzq, mark.rutland, devicetree, robh+dt,
	galak, pawel.moll, linux-kernel, dri-devel, linux-rockchip,
	linux-arm-kernel

On Fri, Jan 20, 2017 at 06:10:48PM +0800, Chris Zhong wrote:
> The vopb/vopl switch register of RK3399 mipi is different from RK3288,
> the default setting for mipi dsi mode is different too, so add a
> of_device_id structure to distinguish them, and make sure set the
> correct mode before mipi phy init.
> 

Hi Chris,
There are a bunch of unrelated changes in this patch. Can you please split out
the whitespace/const changes into a separate clean-up patch so it's easier to
see the meaningful changes?

More comments below.

> Signed-off-by: Chris Zhong <zyw@rock-chips.com>
> Signed-off-by: Mark Yao <mark.yao@rock-chips.com>
> ---
> 
> Changes in v3:
> - base on John Keeping's patch series
> 
>  drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 101 ++++++++++++++++++++++++---------
>  1 file changed, 74 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> index 45af890..a93ce97 100644
> --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> @@ -29,9 +29,17 @@
>  
>  #define DRIVER_NAME    "dw-mipi-dsi"
>  
> -#define GRF_SOC_CON6                    0x025c
> -#define DSI0_SEL_VOP_LIT                (1 << 6)
> -#define DSI1_SEL_VOP_LIT                (1 << 9)
> +#define RK3288_GRF_SOC_CON6		0x025c
> +#define RK3288_DSI0_SEL_VOP_LIT		BIT(6)
> +#define RK3288_DSI1_SEL_VOP_LIT		BIT(9)
> +
> +#define RK3399_GRF_SOC_CON19		0x6250
> +#define RK3399_DSI0_SEL_VOP_LIT		BIT(0)
> +#define RK3399_DSI1_SEL_VOP_LIT		BIT(4)
> +
> +/* disable turnrequest, turndisable, forcetxstopmode, forcerxmode */
> +#define RK3399_GRF_SOC_CON22		0x6258
> +#define RK3399_GRF_DSI_MODE		0xffff0000
>  
>  #define DSI_VERSION			0x00
>  #define DSI_PWR_UP			0x04
> @@ -149,7 +157,6 @@
>  #define LPRX_TO_CNT(p)			((p) & 0xffff)
>  
>  #define DSI_BTA_TO_CNT			0x8c
> -

Unrelated whitespace change

>  #define DSI_LPCLK_CTRL			0x94
>  #define AUTO_CLKLANE_CTRL		BIT(1)
>  #define PHY_TXREQUESTCLKHS		BIT(0)
> @@ -215,11 +222,11 @@
>  
>  #define HSFREQRANGE_SEL(val)	(((val) & 0x3f) << 1)
>  
> -#define INPUT_DIVIDER(val)	((val - 1) & 0x7f)
> +#define INPUT_DIVIDER(val)	(((val) - 1) & 0x7f)

Unrelated change

>  #define LOW_PROGRAM_EN		0
>  #define HIGH_PROGRAM_EN		BIT(7)
> -#define LOOP_DIV_LOW_SEL(val)	((val - 1) & 0x1f)
> -#define LOOP_DIV_HIGH_SEL(val)	(((val - 1) >> 5) & 0x1f)
> +#define LOOP_DIV_LOW_SEL(val)	(((val) - 1) & 0x1f)
> +#define LOOP_DIV_HIGH_SEL(val)	((((val) - 1) >> 5) & 0x1f)

This change doesn't adversely affect other platforms?

>  #define PLL_LOOP_DIV_EN		BIT(5)
>  #define PLL_INPUT_DIV_EN	BIT(4)
>  
> @@ -265,6 +272,11 @@ enum {
>  };
>  
>  struct dw_mipi_dsi_plat_data {
> +	u32 dsi0_en_bit;
> +	u32 dsi1_en_bit;
> +	u32 grf_switch_reg;
> +	u32 grf_dsi0_mode;
> +	u32 grf_dsi0_mode_reg;
>  	unsigned int max_data_lanes;
>  	enum drm_mode_status (*mode_valid)(struct drm_connector *connector,
>  					   struct drm_display_mode *mode);
> @@ -281,6 +293,7 @@ struct dw_mipi_dsi {
>  
>  	struct clk *pllref_clk;
>  	struct clk *pclk;
> +	struct clk *phy_cfg_clk;
>  
>  	unsigned int lane_mbps; /* per lane */
>  	u32 channel;
> @@ -356,6 +369,7 @@ static inline struct dw_mipi_dsi *encoder_to_dsi(struct drm_encoder *encoder)
>  {
>  	return container_of(encoder, struct dw_mipi_dsi, encoder);
>  }
> +
>  static inline void dsi_write(struct dw_mipi_dsi *dsi, u32 reg, u32 val)
>  {
>  	writel(val, dsi->base + reg);
> @@ -367,7 +381,7 @@ static inline u32 dsi_read(struct dw_mipi_dsi *dsi, u32 reg)
>  }
>  
>  static void dw_mipi_dsi_phy_write(struct dw_mipi_dsi *dsi, u8 test_code,
> -				 u8 test_data)
> +				  u8 test_data)

Unrelated whitespace change

>  {
>  	/*
>  	 * With the falling edge on TESTCLK, the TESTDIN[7:0] signal content
> @@ -426,6 +440,14 @@ static int dw_mipi_dsi_phy_init(struct dw_mipi_dsi *dsi)
>  	dsi_write(dsi, DSI_PHY_TST_CTRL0, PHY_TESTCLR);
>  	dsi_write(dsi, DSI_PHY_TST_CTRL0, PHY_UNTESTCLR);
>  
> +	if (!IS_ERR(dsi->phy_cfg_clk)) {
> +		ret = clk_prepare_enable(dsi->phy_cfg_clk);
> +		if (ret) {
> +			dev_err(dsi->dev, "Failed to enable phy_cfg_clk\n");
> +			return ret;
> +		}
> +	}
> +
>  	dw_mipi_dsi_phy_write(dsi, 0x10, BYPASS_VCO_RANGE |
>  					 VCO_RANGE_CON_SEL(vco) |
>  					 VCO_IN_CAP_CON_LOW |
> @@ -474,22 +496,23 @@ static int dw_mipi_dsi_phy_init(struct dw_mipi_dsi *dsi)
>  	dsi_write(dsi, DSI_PHY_RSTZ, PHY_ENFORCEPLL | PHY_ENABLECLK |
>  				     PHY_UNRSTZ | PHY_UNSHUTDOWNZ);
>  
> -
>  	ret = readl_poll_timeout(dsi->base + DSI_PHY_STATUS,
>  				 val, val & LOCK, 1000, PHY_STATUS_TIMEOUT_US);
>  	if (ret < 0) {
>  		dev_err(dsi->dev, "failed to wait for phy lock state\n");
> -		return ret;
> +		goto phy_init_end;
>  	}
>  
>  	ret = readl_poll_timeout(dsi->base + DSI_PHY_STATUS,
>  				 val, val & STOP_STATE_CLK_LANE, 1000,
>  				 PHY_STATUS_TIMEOUT_US);
> -	if (ret < 0) {
> +	if (ret < 0)
>  		dev_err(dsi->dev,
>  			"failed to wait for phy clk lane stop state\n");
> -		return ret;
> -	}
> +
> +phy_init_end:
> +	if (!IS_ERR(dsi->phy_cfg_clk))
> +		clk_disable_unprepare(dsi->phy_cfg_clk);
>  
>  	return ret;
>  }
> @@ -548,7 +571,7 @@ static int dw_mipi_dsi_host_attach(struct mipi_dsi_host *host,
>  
>  	if (device->lanes > dsi->pdata->max_data_lanes) {
>  		dev_err(dsi->dev, "the number of data lanes(%u) is too many\n",
> -				device->lanes);
> +			device->lanes);

Unrelated whitespace change

>  		return -EINVAL;
>  	}
>  
> @@ -936,8 +959,8 @@ static void dw_mipi_dsi_clear_err(struct dw_mipi_dsi *dsi)
>  }
>  
>  static void dw_mipi_dsi_encoder_mode_set(struct drm_encoder *encoder,
> -					struct drm_display_mode *mode,
> -					struct drm_display_mode *adjusted_mode)
> +					 struct drm_display_mode *mode,
> +					 struct drm_display_mode *adjusted_mode)

Unrelated whitespace change

>  {
>  	struct dw_mipi_dsi *dsi = encoder_to_dsi(encoder);
>  
> @@ -965,6 +988,7 @@ static void dw_mipi_dsi_encoder_disable(struct drm_encoder *encoder)
>  static void dw_mipi_dsi_encoder_enable(struct drm_encoder *encoder)
>  {
>  	struct dw_mipi_dsi *dsi = encoder_to_dsi(encoder);
> +	const struct dw_mipi_dsi_plat_data *pdata = dsi->pdata;
>  	int mux = drm_of_encoder_active_endpoint_id(dsi->dev->of_node, encoder);
>  	u32 val;
>  
> @@ -985,6 +1009,10 @@ static void dw_mipi_dsi_encoder_enable(struct drm_encoder *encoder)
>  	dw_mipi_dsi_dphy_interface_config(dsi);
>  	dw_mipi_dsi_clear_err(dsi);
>  
> +	if (pdata->grf_dsi0_mode_reg)
> +		regmap_write(dsi->grf_regmap, pdata->grf_dsi0_mode_reg,
> +			     pdata->grf_dsi0_mode);
> +
>  	dw_mipi_dsi_phy_init(dsi);
>  	dw_mipi_dsi_wait_for_two_frames(dsi);
>  
> @@ -998,11 +1026,11 @@ static void dw_mipi_dsi_encoder_enable(struct drm_encoder *encoder)
>  	clk_disable_unprepare(dsi->pclk);
>  
>  	if (mux)
> -		val = DSI0_SEL_VOP_LIT | (DSI0_SEL_VOP_LIT << 16);
> +		val = pdata->dsi0_en_bit | (pdata->dsi0_en_bit << 16);
>  	else
> -		val = DSI0_SEL_VOP_LIT << 16;
> +		val = pdata->dsi0_en_bit << 16;
>  
> -	regmap_write(dsi->grf_regmap, GRF_SOC_CON6, val);
> +	regmap_write(dsi->grf_regmap, pdata->grf_switch_reg, val);
>  	dev_dbg(dsi->dev, "vop %s output to dsi0\n", (mux) ? "LIT" : "BIG");
>  }
>  
> @@ -1034,7 +1062,7 @@ dw_mipi_dsi_encoder_atomic_check(struct drm_encoder *encoder,
>  	return 0;
>  }
>  
> -static struct drm_encoder_helper_funcs
> +static const struct drm_encoder_helper_funcs
>  dw_mipi_dsi_encoder_helper_funcs = {
>  	.enable = dw_mipi_dsi_encoder_enable,
>  	.mode_set = dw_mipi_dsi_encoder_mode_set,
> @@ -1042,7 +1070,7 @@ dw_mipi_dsi_encoder_helper_funcs = {
>  	.atomic_check = dw_mipi_dsi_encoder_atomic_check,
>  };
>  
> -static struct drm_encoder_funcs dw_mipi_dsi_encoder_funcs = {
> +static const struct drm_encoder_funcs dw_mipi_dsi_encoder_funcs = {
>  	.destroy = drm_encoder_cleanup,
>  };
>  
> @@ -1078,7 +1106,7 @@ static void dw_mipi_dsi_drm_connector_destroy(struct drm_connector *connector)
>  	drm_connector_cleanup(connector);
>  }
>  
> -static struct drm_connector_funcs dw_mipi_dsi_atomic_connector_funcs = {
> +static const struct drm_connector_funcs dw_mipi_dsi_atomic_connector_funcs = {

These 3 const changes are unrelated to the patch

>  	.dpms = drm_atomic_helper_connector_dpms,
>  	.fill_modes = drm_helper_probe_single_connector_modes,
>  	.destroy = dw_mipi_dsi_drm_connector_destroy,
> @@ -1088,7 +1116,7 @@ static struct drm_connector_funcs dw_mipi_dsi_atomic_connector_funcs = {
>  };
>  
>  static int dw_mipi_dsi_register(struct drm_device *drm,
> -				      struct dw_mipi_dsi *dsi)
> +				struct dw_mipi_dsi *dsi)

Unrelated whitespace change

>  {
>  	struct drm_encoder *encoder = &dsi->encoder;
>  	struct drm_connector *connector = &dsi->connector;
> @@ -1109,14 +1137,14 @@ static int dw_mipi_dsi_register(struct drm_device *drm,
>  	drm_encoder_helper_add(&dsi->encoder,
>  			       &dw_mipi_dsi_encoder_helper_funcs);
>  	ret = drm_encoder_init(drm, &dsi->encoder, &dw_mipi_dsi_encoder_funcs,
> -			 DRM_MODE_ENCODER_DSI, NULL);
> +			       DRM_MODE_ENCODER_DSI, NULL);

Unrelated whitespace change

>  	if (ret) {
>  		dev_err(dev, "Failed to initialize encoder with drm\n");
>  		return ret;
>  	}
>  
>  	drm_connector_helper_add(connector,
> -			&dw_mipi_dsi_connector_helper_funcs);
> +				 &dw_mipi_dsi_connector_helper_funcs);

Unrelated whitespace change

>  
>  	drm_connector_init(drm, &dsi->connector,
>  			   &dw_mipi_dsi_atomic_connector_funcs,
> @@ -1162,21 +1190,36 @@ static enum drm_mode_status rk3288_mipi_dsi_mode_valid(
>  }
>  
>  static struct dw_mipi_dsi_plat_data rk3288_mipi_dsi_drv_data = {
> +	.dsi0_en_bit = RK3288_DSI0_SEL_VOP_LIT,
> +	.dsi1_en_bit = RK3288_DSI1_SEL_VOP_LIT,
> +	.grf_switch_reg = RK3288_GRF_SOC_CON6,
>  	.max_data_lanes = 4,
>  	.mode_valid = rk3288_mipi_dsi_mode_valid,
>  };
>  
> +static struct dw_mipi_dsi_plat_data rk3399_mipi_dsi_drv_data = {
> +	.dsi0_en_bit = RK3399_DSI0_SEL_VOP_LIT,
> +	.dsi1_en_bit = RK3399_DSI1_SEL_VOP_LIT,
> +	.grf_switch_reg = RK3399_GRF_SOC_CON19,
> +	.grf_dsi0_mode = RK3399_GRF_DSI_MODE,
> +	.grf_dsi0_mode_reg = RK3399_GRF_SOC_CON22,
> +	.max_data_lanes = 4,
> +};
> +
>  static const struct of_device_id dw_mipi_dsi_dt_ids[] = {
>  	{
>  	 .compatible = "rockchip,rk3288-mipi-dsi",
>  	 .data = &rk3288_mipi_dsi_drv_data,
> +	}, {
> +	 .compatible = "rockchip,rk3399-mipi-dsi",
> +	 .data = &rk3399_mipi_dsi_drv_data,
>  	},
>  	{ /* sentinel */ }
>  };
>  MODULE_DEVICE_TABLE(of, dw_mipi_dsi_dt_ids);
>  
>  static int dw_mipi_dsi_bind(struct device *dev, struct device *master,
> -			     void *data)
> +			    void *data)

Unrelated whitespace change

>  {
>  	const struct of_device_id *of_id =
>  			of_match_device(dw_mipi_dsi_dt_ids, dev);
> @@ -1249,6 +1292,10 @@ static int dw_mipi_dsi_bind(struct device *dev, struct device *master,
>  		clk_disable_unprepare(dsi->pclk);
>  	}
>  
> +	dsi->phy_cfg_clk = devm_clk_get(dev, "phy_cfg");
> +	if (IS_ERR(dsi->phy_cfg_clk))
> +		dev_dbg(dev, "have not phy_cfg_clk\n");
> +
>  	ret = clk_prepare_enable(dsi->pllref_clk);
>  	if (ret) {
>  		dev_err(dev, "%s: Failed to enable pllref_clk\n", __func__);
> @@ -1280,7 +1327,7 @@ static int dw_mipi_dsi_bind(struct device *dev, struct device *master,
>  }
>  
>  static void dw_mipi_dsi_unbind(struct device *dev, struct device *master,
> -	void *data)
> +			       void *data)

Unrelated whitespace change

>  {
>  	struct dw_mipi_dsi *dsi = dev_get_drvdata(dev);
>  
> -- 
> 2.6.3
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Sean Paul, Software Engineer, Google / Chromium OS

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 3/5] drm/rockchip/dsi: remove mode_valid function
  2017-01-20 10:10 ` [PATCH v3 3/5] drm/rockchip/dsi: remove mode_valid function Chris Zhong
@ 2017-01-23 17:48   ` Sean Paul
  2017-01-24  2:27     ` Chris Zhong
  0 siblings, 1 reply; 13+ messages in thread
From: Sean Paul @ 2017-01-23 17:48 UTC (permalink / raw)
  To: Chris Zhong
  Cc: dianders, tfiga, heiko, yzq, mark.rutland, devicetree, robh+dt,
	galak, pawel.moll, linux-kernel, dri-devel, linux-rockchip,
	linux-arm-kernel

On Fri, Jan 20, 2017 at 06:10:49PM +0800, Chris Zhong wrote:
> The MIPI DSI do not need check the validity of resolution, the max
> resolution should depend VOP. Hence, remove rk3288_mipi_dsi_mode_valid
> here.

Does vop actually enforce this, though? I see that mode_config.max_width is
4096, but there is no bounds checking in mode_fixup().

The connector is currently rejecting everything greater than 2047. So I think
you're going to regress behavior here.

Sean


> 
> Signed-off-by: Chris Zhong <zyw@rock-chips.com>
> ---
> 
> Changes in v3: None
> 
>  drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 39 ----------------------------------
>  1 file changed, 39 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> index a93ce97..6f0e252 100644
> --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> @@ -278,8 +278,6 @@ struct dw_mipi_dsi_plat_data {
>  	u32 grf_dsi0_mode;
>  	u32 grf_dsi0_mode_reg;
>  	unsigned int max_data_lanes;
> -	enum drm_mode_status (*mode_valid)(struct drm_connector *connector,
> -					   struct drm_display_mode *mode);
>  };
>  
>  struct dw_mipi_dsi {
> @@ -1081,23 +1079,8 @@ static int dw_mipi_dsi_connector_get_modes(struct drm_connector *connector)
>  	return drm_panel_get_modes(dsi->panel);
>  }
>  
> -static enum drm_mode_status dw_mipi_dsi_mode_valid(
> -					struct drm_connector *connector,
> -					struct drm_display_mode *mode)
> -{
> -	struct dw_mipi_dsi *dsi = con_to_dsi(connector);
> -
> -	enum drm_mode_status mode_status = MODE_OK;
> -
> -	if (dsi->pdata->mode_valid)
> -		mode_status = dsi->pdata->mode_valid(connector, mode);
> -
> -	return mode_status;
> -}
> -
>  static struct drm_connector_helper_funcs dw_mipi_dsi_connector_helper_funcs = {
>  	.get_modes = dw_mipi_dsi_connector_get_modes,
> -	.mode_valid = dw_mipi_dsi_mode_valid,
>  };
>  
>  static void dw_mipi_dsi_drm_connector_destroy(struct drm_connector *connector)
> @@ -1168,33 +1151,11 @@ static int rockchip_mipi_parse_dt(struct dw_mipi_dsi *dsi)
>  	return 0;
>  }
>  
> -static enum drm_mode_status rk3288_mipi_dsi_mode_valid(
> -					struct drm_connector *connector,
> -					struct drm_display_mode *mode)
> -{
> -	/*
> -	 * The VID_PKT_SIZE field in the DSI_VID_PKT_CFG
> -	 * register is 11-bit.
> -	 */
> -	if (mode->hdisplay > 0x7ff)
> -		return MODE_BAD_HVALUE;
> -
> -	/*
> -	 * The V_ACTIVE_LINES field in the DSI_VTIMING_CFG
> -	 * register is 11-bit.
> -	 */
> -	if (mode->vdisplay > 0x7ff)
> -		return MODE_BAD_VVALUE;
> -
> -	return MODE_OK;
> -}
> -
>  static struct dw_mipi_dsi_plat_data rk3288_mipi_dsi_drv_data = {
>  	.dsi0_en_bit = RK3288_DSI0_SEL_VOP_LIT,
>  	.dsi1_en_bit = RK3288_DSI1_SEL_VOP_LIT,
>  	.grf_switch_reg = RK3288_GRF_SOC_CON6,
>  	.max_data_lanes = 4,
> -	.mode_valid = rk3288_mipi_dsi_mode_valid,
>  };
>  
>  static struct dw_mipi_dsi_plat_data rk3399_mipi_dsi_drv_data = {
> -- 
> 2.6.3
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Sean Paul, Software Engineer, Google / Chromium OS

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 5/5] drm/rockchip/dsi: add dw-mipi power domain support
  2017-01-20 10:10 ` [PATCH v3 5/5] drm/rockchip/dsi: add dw-mipi power domain support Chris Zhong
@ 2017-01-23 18:03   ` Sean Paul
  0 siblings, 0 replies; 13+ messages in thread
From: Sean Paul @ 2017-01-23 18:03 UTC (permalink / raw)
  To: Chris Zhong
  Cc: dianders, tfiga, heiko, yzq, mark.rutland, devicetree, robh+dt,
	galak, pawel.moll, linux-kernel, dri-devel, linux-rockchip,
	linux-arm-kernel

On Fri, Jan 20, 2017 at 06:10:51PM +0800, Chris Zhong wrote:
> Reference the power domain incase dw-mipi power down when
> in use.
> 

Reviewed-by: Sean Paul <seanpaul@chromium.org>

> Signed-off-by: Chris Zhong <zyw@rock-chips.com>
> ---
> 
> Changes in v3: None
> 
>  drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> index 6f0e252..1462101e 100644
> --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> @@ -12,6 +12,7 @@
>  #include <linux/math64.h>
>  #include <linux/module.h>
>  #include <linux/of_device.h>
> +#include <linux/pm_runtime.h>
>  #include <linux/regmap.h>
>  #include <linux/reset.h>
>  #include <linux/mfd/syscon.h>
> @@ -293,6 +294,7 @@ struct dw_mipi_dsi {
>  	struct clk *pclk;
>  	struct clk *phy_cfg_clk;
>  
> +	int dpms_mode;
>  	unsigned int lane_mbps; /* per lane */
>  	u32 channel;
>  	u32 lanes;
> @@ -969,6 +971,9 @@ static void dw_mipi_dsi_encoder_disable(struct drm_encoder *encoder)
>  {
>  	struct dw_mipi_dsi *dsi = encoder_to_dsi(encoder);
>  
> +	if (dsi->dpms_mode != DRM_MODE_DPMS_ON)
> +		return;
> +
>  	if (clk_prepare_enable(dsi->pclk)) {
>  		dev_err(dsi->dev, "%s: Failed to enable pclk\n", __func__);
>  		return;
> @@ -980,7 +985,9 @@ static void dw_mipi_dsi_encoder_disable(struct drm_encoder *encoder)
>  	drm_panel_unprepare(dsi->panel);
>  
>  	dw_mipi_dsi_disable(dsi);
> +	pm_runtime_put(dsi->dev);
>  	clk_disable_unprepare(dsi->pclk);
> +	dsi->dpms_mode = DRM_MODE_DPMS_OFF;
>  }
>  
>  static void dw_mipi_dsi_encoder_enable(struct drm_encoder *encoder)
> @@ -990,11 +997,15 @@ static void dw_mipi_dsi_encoder_enable(struct drm_encoder *encoder)
>  	int mux = drm_of_encoder_active_endpoint_id(dsi->dev->of_node, encoder);
>  	u32 val;
>  
> +	if (dsi->dpms_mode == DRM_MODE_DPMS_ON)
> +		return;
> +
>  	if (clk_prepare_enable(dsi->pclk)) {
>  		dev_err(dsi->dev, "%s: Failed to enable pclk\n", __func__);
>  		return;
>  	}
>  
> +	pm_runtime_get_sync(dsi->dev);
>  	dw_mipi_dsi_init(dsi);
>  	dw_mipi_dsi_dpi_config(dsi);
>  	dw_mipi_dsi_packet_handler_config(dsi);
> @@ -1030,6 +1041,7 @@ static void dw_mipi_dsi_encoder_enable(struct drm_encoder *encoder)
>  
>  	regmap_write(dsi->grf_regmap, pdata->grf_switch_reg, val);
>  	dev_dbg(dsi->dev, "vop %s output to dsi0\n", (mux) ? "LIT" : "BIG");
> +	dsi->dpms_mode = DRM_MODE_DPMS_ON;
>  }
>  
>  static int
> @@ -1198,6 +1210,7 @@ static int dw_mipi_dsi_bind(struct device *dev, struct device *master,
>  
>  	dsi->dev = dev;
>  	dsi->pdata = pdata;
> +	dsi->dpms_mode = DRM_MODE_DPMS_OFF;
>  
>  	ret = rockchip_mipi_parse_dt(dsi);
>  	if (ret)
> @@ -1271,6 +1284,8 @@ static int dw_mipi_dsi_bind(struct device *dev, struct device *master,
>  
>  	dev_set_drvdata(dev, dsi);
>  
> +	pm_runtime_enable(dev);
> +
>  	dsi->dsi_host.ops = &dw_mipi_dsi_host_ops;
>  	dsi->dsi_host.dev = dev;
>  	ret = mipi_dsi_host_register(&dsi->dsi_host);
> @@ -1293,6 +1308,7 @@ static void dw_mipi_dsi_unbind(struct device *dev, struct device *master,
>  	struct dw_mipi_dsi *dsi = dev_get_drvdata(dev);
>  
>  	mipi_dsi_host_unregister(&dsi->dsi_host);
> +	pm_runtime_disable(dev);
>  	clk_disable_unprepare(dsi->pllref_clk);
>  }
>  
> -- 
> 2.6.3
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Sean Paul, Software Engineer, Google / Chromium OS

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 3/5] drm/rockchip/dsi: remove mode_valid function
  2017-01-23 17:48   ` Sean Paul
@ 2017-01-24  2:27     ` Chris Zhong
  2017-02-01 18:12       ` Sean Paul
  0 siblings, 1 reply; 13+ messages in thread
From: Chris Zhong @ 2017-01-24  2:27 UTC (permalink / raw)
  To: Sean Paul
  Cc: dianders, tfiga, heiko, yzq, mark.rutland, devicetree, robh+dt,
	galak, pawel.moll, linux-kernel, dri-devel, linux-rockchip,
	linux-arm-kernel

Hi Sean

On 01/24/2017 01:48 AM, Sean Paul wrote:
> On Fri, Jan 20, 2017 at 06:10:49PM +0800, Chris Zhong wrote:
>> The MIPI DSI do not need check the validity of resolution, the max
>> resolution should depend VOP. Hence, remove rk3288_mipi_dsi_mode_valid
>> here.
> Does vop actually enforce this, though? I see that mode_config.max_width is
> 4096, but there is no bounds checking in mode_fixup().
>
> The connector is currently rejecting everything greater than 2047. So I think
> you're going to regress behavior here.
>
> Sean
The mipi controller has not this width limit, it depend the VOP, such as 
RK3399, VOP_LIT only support 2560,
but VOP_BIG support 4K. So this driver should check the width here.

>
>
>> Signed-off-by: Chris Zhong <zyw@rock-chips.com>
>> ---
>>
>> Changes in v3: None
>>
>>   drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 39 ----------------------------------
>>   1 file changed, 39 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
>> index a93ce97..6f0e252 100644
>> --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
>> +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
>> @@ -278,8 +278,6 @@ struct dw_mipi_dsi_plat_data {
>>   	u32 grf_dsi0_mode;
>>   	u32 grf_dsi0_mode_reg;
>>   	unsigned int max_data_lanes;
>> -	enum drm_mode_status (*mode_valid)(struct drm_connector *connector,
>> -					   struct drm_display_mode *mode);
>>   };
>>   
>>   struct dw_mipi_dsi {
>> @@ -1081,23 +1079,8 @@ static int dw_mipi_dsi_connector_get_modes(struct drm_connector *connector)
>>   	return drm_panel_get_modes(dsi->panel);
>>   }
>>   
>> -static enum drm_mode_status dw_mipi_dsi_mode_valid(
>> -					struct drm_connector *connector,
>> -					struct drm_display_mode *mode)
>> -{
>> -	struct dw_mipi_dsi *dsi = con_to_dsi(connector);
>> -
>> -	enum drm_mode_status mode_status = MODE_OK;
>> -
>> -	if (dsi->pdata->mode_valid)
>> -		mode_status = dsi->pdata->mode_valid(connector, mode);
>> -
>> -	return mode_status;
>> -}
>> -
>>   static struct drm_connector_helper_funcs dw_mipi_dsi_connector_helper_funcs = {
>>   	.get_modes = dw_mipi_dsi_connector_get_modes,
>> -	.mode_valid = dw_mipi_dsi_mode_valid,
>>   };
>>   
>>   static void dw_mipi_dsi_drm_connector_destroy(struct drm_connector *connector)
>> @@ -1168,33 +1151,11 @@ static int rockchip_mipi_parse_dt(struct dw_mipi_dsi *dsi)
>>   	return 0;
>>   }
>>   
>> -static enum drm_mode_status rk3288_mipi_dsi_mode_valid(
>> -					struct drm_connector *connector,
>> -					struct drm_display_mode *mode)
>> -{
>> -	/*
>> -	 * The VID_PKT_SIZE field in the DSI_VID_PKT_CFG
>> -	 * register is 11-bit.
>> -	 */
>> -	if (mode->hdisplay > 0x7ff)
>> -		return MODE_BAD_HVALUE;
>> -
>> -	/*
>> -	 * The V_ACTIVE_LINES field in the DSI_VTIMING_CFG
>> -	 * register is 11-bit.
>> -	 */
>> -	if (mode->vdisplay > 0x7ff)
>> -		return MODE_BAD_VVALUE;
>> -
>> -	return MODE_OK;
>> -}
>> -
>>   static struct dw_mipi_dsi_plat_data rk3288_mipi_dsi_drv_data = {
>>   	.dsi0_en_bit = RK3288_DSI0_SEL_VOP_LIT,
>>   	.dsi1_en_bit = RK3288_DSI1_SEL_VOP_LIT,
>>   	.grf_switch_reg = RK3288_GRF_SOC_CON6,
>>   	.max_data_lanes = 4,
>> -	.mode_valid = rk3288_mipi_dsi_mode_valid,
>>   };
>>   
>>   static struct dw_mipi_dsi_plat_data rk3399_mipi_dsi_drv_data = {
>> -- 
>> 2.6.3
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 3/5] drm/rockchip/dsi: remove mode_valid function
  2017-01-24  2:27     ` Chris Zhong
@ 2017-02-01 18:12       ` Sean Paul
  2017-02-05  3:42         ` Chris Zhong
  0 siblings, 1 reply; 13+ messages in thread
From: Sean Paul @ 2017-02-01 18:12 UTC (permalink / raw)
  To: Chris Zhong
  Cc: mark.rutland, devicetree, pawel.moll, yzq, linux-kernel,
	dianders, dri-devel, tfiga, linux-rockchip, robh+dt, galak,
	linux-arm-kernel

On Tue, Jan 24, 2017 at 10:27:27AM +0800, Chris Zhong wrote:
> Hi Sean
> 
> On 01/24/2017 01:48 AM, Sean Paul wrote:
> >On Fri, Jan 20, 2017 at 06:10:49PM +0800, Chris Zhong wrote:
> >>The MIPI DSI do not need check the validity of resolution, the max
> >>resolution should depend VOP. Hence, remove rk3288_mipi_dsi_mode_valid
> >>here.
> >Does vop actually enforce this, though? I see that mode_config.max_width is
> >4096, but there is no bounds checking in mode_fixup().
> >
> >The connector is currently rejecting everything greater than 2047. So I think
> >you're going to regress behavior here.
> >
> >Sean
> The mipi controller has not this width limit, it depend the VOP,
> such as RK3399, VOP_LIT only support 2560,
> but VOP_BIG support 4K. So this driver should check the width here.

I don't see anything in the vop driver that rejects large modes for little vop.
So, while I agree the check shouldn't be here, you should move it to where it
should be instead of removing it entirely.

Sean

> 
> >
> >
> >>Signed-off-by: Chris Zhong <zyw@rock-chips.com>
> >>---
> >>
> >>Changes in v3: None
> >>
> >>  drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 39 ----------------------------------
> >>  1 file changed, 39 deletions(-)
> >>
> >>diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> >>index a93ce97..6f0e252 100644
> >>--- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> >>+++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> >>@@ -278,8 +278,6 @@ struct dw_mipi_dsi_plat_data {
> >>  	u32 grf_dsi0_mode;
> >>  	u32 grf_dsi0_mode_reg;
> >>  	unsigned int max_data_lanes;
> >>-	enum drm_mode_status (*mode_valid)(struct drm_connector *connector,
> >>-					   struct drm_display_mode *mode);
> >>  };
> >>  struct dw_mipi_dsi {
> >>@@ -1081,23 +1079,8 @@ static int dw_mipi_dsi_connector_get_modes(struct drm_connector *connector)
> >>  	return drm_panel_get_modes(dsi->panel);
> >>  }
> >>-static enum drm_mode_status dw_mipi_dsi_mode_valid(
> >>-					struct drm_connector *connector,
> >>-					struct drm_display_mode *mode)
> >>-{
> >>-	struct dw_mipi_dsi *dsi = con_to_dsi(connector);
> >>-
> >>-	enum drm_mode_status mode_status = MODE_OK;
> >>-
> >>-	if (dsi->pdata->mode_valid)
> >>-		mode_status = dsi->pdata->mode_valid(connector, mode);
> >>-
> >>-	return mode_status;
> >>-}
> >>-
> >>  static struct drm_connector_helper_funcs dw_mipi_dsi_connector_helper_funcs = {
> >>  	.get_modes = dw_mipi_dsi_connector_get_modes,
> >>-	.mode_valid = dw_mipi_dsi_mode_valid,
> >>  };
> >>  static void dw_mipi_dsi_drm_connector_destroy(struct drm_connector *connector)
> >>@@ -1168,33 +1151,11 @@ static int rockchip_mipi_parse_dt(struct dw_mipi_dsi *dsi)
> >>  	return 0;
> >>  }
> >>-static enum drm_mode_status rk3288_mipi_dsi_mode_valid(
> >>-					struct drm_connector *connector,
> >>-					struct drm_display_mode *mode)
> >>-{
> >>-	/*
> >>-	 * The VID_PKT_SIZE field in the DSI_VID_PKT_CFG
> >>-	 * register is 11-bit.
> >>-	 */
> >>-	if (mode->hdisplay > 0x7ff)
> >>-		return MODE_BAD_HVALUE;
> >>-
> >>-	/*
> >>-	 * The V_ACTIVE_LINES field in the DSI_VTIMING_CFG
> >>-	 * register is 11-bit.
> >>-	 */
> >>-	if (mode->vdisplay > 0x7ff)
> >>-		return MODE_BAD_VVALUE;
> >>-
> >>-	return MODE_OK;
> >>-}
> >>-
> >>  static struct dw_mipi_dsi_plat_data rk3288_mipi_dsi_drv_data = {
> >>  	.dsi0_en_bit = RK3288_DSI0_SEL_VOP_LIT,
> >>  	.dsi1_en_bit = RK3288_DSI1_SEL_VOP_LIT,
> >>  	.grf_switch_reg = RK3288_GRF_SOC_CON6,
> >>  	.max_data_lanes = 4,
> >>-	.mode_valid = rk3288_mipi_dsi_mode_valid,
> >>  };
> >>  static struct dw_mipi_dsi_plat_data rk3399_mipi_dsi_drv_data = {
> >>-- 
> >>2.6.3
> >>
> >>_______________________________________________
> >>dri-devel mailing list
> >>dri-devel@lists.freedesktop.org
> >>https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Sean Paul, Software Engineer, Google / Chromium OS

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 3/5] drm/rockchip/dsi: remove mode_valid function
  2017-02-01 18:12       ` Sean Paul
@ 2017-02-05  3:42         ` Chris Zhong
  2017-02-05  7:42           ` Mark yao
  0 siblings, 1 reply; 13+ messages in thread
From: Chris Zhong @ 2017-02-05  3:42 UTC (permalink / raw)
  To: Sean Paul
  Cc: mark.rutland, devicetree, pawel.moll, yzq, linux-kernel,
	dianders, dri-devel, tfiga, linux-rockchip, robh+dt, galak,
	linux-arm-kernel



On 02/02/2017 02:12 AM, Sean Paul wrote:
> On Tue, Jan 24, 2017 at 10:27:27AM +0800, Chris Zhong wrote:
>> Hi Sean
>>
>> On 01/24/2017 01:48 AM, Sean Paul wrote:
>>> On Fri, Jan 20, 2017 at 06:10:49PM +0800, Chris Zhong wrote:
>>>> The MIPI DSI do not need check the validity of resolution, the max
>>>> resolution should depend VOP. Hence, remove rk3288_mipi_dsi_mode_valid
>>>> here.
>>> Does vop actually enforce this, though? I see that mode_config.max_width is
>>> 4096, but there is no bounds checking in mode_fixup().
>>>
>>> The connector is currently rejecting everything greater than 2047. So I think
>>> you're going to regress behavior here.
>>>
>>> Sean
>> The mipi controller has not this width limit, it depend the VOP,
>> such as RK3399, VOP_LIT only support 2560,
>> but VOP_BIG support 4K. So this driver should check the width here.
> I don't see anything in the vop driver that rejects large modes for little vop.
> So, while I agree the check shouldn't be here, you should move it to where it
> should be instead of removing it entirely.
>
> Sean

drm_mode_validate_size will check the dev->mode_config.max_width and
dev->mode_config.max_height, these 2 value come from 
rockchip_drm_mode_config_init,
currently, they are both 4096. So you are right, drm driver does not 
distinguish
between vop lit and big.

I think Mark Yao already have a local solution, and he will post it soon.

>
>>>
>>>> Signed-off-by: Chris Zhong <zyw@rock-chips.com>
>>>> ---
>>>>
>>>> Changes in v3: None
>>>>
>>>>   drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 39 ----------------------------------
>>>>   1 file changed, 39 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
>>>> index a93ce97..6f0e252 100644
>>>> --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
>>>> +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
>>>> @@ -278,8 +278,6 @@ struct dw_mipi_dsi_plat_data {
>>>>   	u32 grf_dsi0_mode;
>>>>   	u32 grf_dsi0_mode_reg;
>>>>   	unsigned int max_data_lanes;
>>>> -	enum drm_mode_status (*mode_valid)(struct drm_connector *connector,
>>>> -					   struct drm_display_mode *mode);
>>>>   };
>>>>   struct dw_mipi_dsi {
>>>> @@ -1081,23 +1079,8 @@ static int dw_mipi_dsi_connector_get_modes(struct drm_connector *connector)
>>>>   	return drm_panel_get_modes(dsi->panel);
>>>>   }
>>>> -static enum drm_mode_status dw_mipi_dsi_mode_valid(
>>>> -					struct drm_connector *connector,
>>>> -					struct drm_display_mode *mode)
>>>> -{
>>>> -	struct dw_mipi_dsi *dsi = con_to_dsi(connector);
>>>> -
>>>> -	enum drm_mode_status mode_status = MODE_OK;
>>>> -
>>>> -	if (dsi->pdata->mode_valid)
>>>> -		mode_status = dsi->pdata->mode_valid(connector, mode);
>>>> -
>>>> -	return mode_status;
>>>> -}
>>>> -
>>>>   static struct drm_connector_helper_funcs dw_mipi_dsi_connector_helper_funcs = {
>>>>   	.get_modes = dw_mipi_dsi_connector_get_modes,
>>>> -	.mode_valid = dw_mipi_dsi_mode_valid,
>>>>   };
>>>>   static void dw_mipi_dsi_drm_connector_destroy(struct drm_connector *connector)
>>>> @@ -1168,33 +1151,11 @@ static int rockchip_mipi_parse_dt(struct dw_mipi_dsi *dsi)
>>>>   	return 0;
>>>>   }
>>>> -static enum drm_mode_status rk3288_mipi_dsi_mode_valid(
>>>> -					struct drm_connector *connector,
>>>> -					struct drm_display_mode *mode)
>>>> -{
>>>> -	/*
>>>> -	 * The VID_PKT_SIZE field in the DSI_VID_PKT_CFG
>>>> -	 * register is 11-bit.
>>>> -	 */
>>>> -	if (mode->hdisplay > 0x7ff)
>>>> -		return MODE_BAD_HVALUE;
>>>> -
>>>> -	/*
>>>> -	 * The V_ACTIVE_LINES field in the DSI_VTIMING_CFG
>>>> -	 * register is 11-bit.
>>>> -	 */
>>>> -	if (mode->vdisplay > 0x7ff)
>>>> -		return MODE_BAD_VVALUE;
>>>> -
>>>> -	return MODE_OK;
>>>> -}
>>>> -
>>>>   static struct dw_mipi_dsi_plat_data rk3288_mipi_dsi_drv_data = {
>>>>   	.dsi0_en_bit = RK3288_DSI0_SEL_VOP_LIT,
>>>>   	.dsi1_en_bit = RK3288_DSI1_SEL_VOP_LIT,
>>>>   	.grf_switch_reg = RK3288_GRF_SOC_CON6,
>>>>   	.max_data_lanes = 4,
>>>> -	.mode_valid = rk3288_mipi_dsi_mode_valid,
>>>>   };
>>>>   static struct dw_mipi_dsi_plat_data rk3399_mipi_dsi_drv_data = {
>>>> -- 
>>>> 2.6.3
>>>>
>>>> _______________________________________________
>>>> dri-devel mailing list
>>>> dri-devel@lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 3/5] drm/rockchip/dsi: remove mode_valid function
  2017-02-05  3:42         ` Chris Zhong
@ 2017-02-05  7:42           ` Mark yao
  0 siblings, 0 replies; 13+ messages in thread
From: Mark yao @ 2017-02-05  7:42 UTC (permalink / raw)
  To: Chris Zhong, Sean Paul
  Cc: mark.rutland, devicetree, pawel.moll, yzq, linux-kernel,
	dianders, dri-devel, tfiga, linux-rockchip, robh+dt, galak,
	linux-arm-kernel

On 2017年02月05日 11:42, Chris Zhong wrote:
>
>
> On 02/02/2017 02:12 AM, Sean Paul wrote:
>> On Tue, Jan 24, 2017 at 10:27:27AM +0800, Chris Zhong wrote:
>>> Hi Sean
>>>
>>> On 01/24/2017 01:48 AM, Sean Paul wrote:
>>>> On Fri, Jan 20, 2017 at 06:10:49PM +0800, Chris Zhong wrote:
>>>>> The MIPI DSI do not need check the validity of resolution, the max
>>>>> resolution should depend VOP. Hence, remove 
>>>>> rk3288_mipi_dsi_mode_valid
>>>>> here.
>>>> Does vop actually enforce this, though? I see that 
>>>> mode_config.max_width is
>>>> 4096, but there is no bounds checking in mode_fixup().
>>>>
>>>> The connector is currently rejecting everything greater than 2047. 
>>>> So I think
>>>> you're going to regress behavior here.
>>>>
>>>> Sean
>>> The mipi controller has not this width limit, it depend the VOP,
>>> such as RK3399, VOP_LIT only support 2560,
>>> but VOP_BIG support 4K. So this driver should check the width here.
>> I don't see anything in the vop driver that rejects large modes for 
>> little vop.
>> So, while I agree the check shouldn't be here, you should move it to 
>> where it
>> should be instead of removing it entirely.
>>
>> Sean
>
> drm_mode_validate_size will check the dev->mode_config.max_width and
> dev->mode_config.max_height, these 2 value come from 
> rockchip_drm_mode_config_init,
> currently, they are both 4096. So you are right, drm driver does not 
> distinguish
> between vop lit and big.
>
> I think Mark Yao already have a local solution, and he will post it soon.
Hi

See follow patches, support mode valid with vop callback.

[0] https://patchwork.kernel.org/patch/9555943/
[1] https://patchwork.kernel.org/patch/9555945/

>
>>
>>>>
>>>>> Signed-off-by: Chris Zhong <zyw@rock-chips.com>
>>>>> ---
>>>>>
>>>>> Changes in v3: None
>>>>>
>>>>>   drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 39 
>>>>> ----------------------------------
>>>>>   1 file changed, 39 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c 
>>>>> b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
>>>>> index a93ce97..6f0e252 100644
>>>>> --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
>>>>> +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
>>>>> @@ -278,8 +278,6 @@ struct dw_mipi_dsi_plat_data {
>>>>>       u32 grf_dsi0_mode;
>>>>>       u32 grf_dsi0_mode_reg;
>>>>>       unsigned int max_data_lanes;
>>>>> -    enum drm_mode_status (*mode_valid)(struct drm_connector 
>>>>> *connector,
>>>>> -                       struct drm_display_mode *mode);
>>>>>   };
>>>>>   struct dw_mipi_dsi {
>>>>> @@ -1081,23 +1079,8 @@ static int 
>>>>> dw_mipi_dsi_connector_get_modes(struct drm_connector *connector)
>>>>>       return drm_panel_get_modes(dsi->panel);
>>>>>   }
>>>>> -static enum drm_mode_status dw_mipi_dsi_mode_valid(
>>>>> -                    struct drm_connector *connector,
>>>>> -                    struct drm_display_mode *mode)
>>>>> -{
>>>>> -    struct dw_mipi_dsi *dsi = con_to_dsi(connector);
>>>>> -
>>>>> -    enum drm_mode_status mode_status = MODE_OK;
>>>>> -
>>>>> -    if (dsi->pdata->mode_valid)
>>>>> -        mode_status = dsi->pdata->mode_valid(connector, mode);
>>>>> -
>>>>> -    return mode_status;
>>>>> -}
>>>>> -
>>>>>   static struct drm_connector_helper_funcs 
>>>>> dw_mipi_dsi_connector_helper_funcs = {
>>>>>       .get_modes = dw_mipi_dsi_connector_get_modes,
>>>>> -    .mode_valid = dw_mipi_dsi_mode_valid,
>>>>>   };
>>>>>   static void dw_mipi_dsi_drm_connector_destroy(struct 
>>>>> drm_connector *connector)
>>>>> @@ -1168,33 +1151,11 @@ static int rockchip_mipi_parse_dt(struct 
>>>>> dw_mipi_dsi *dsi)
>>>>>       return 0;
>>>>>   }
>>>>> -static enum drm_mode_status rk3288_mipi_dsi_mode_valid(
>>>>> -                    struct drm_connector *connector,
>>>>> -                    struct drm_display_mode *mode)
>>>>> -{
>>>>> -    /*
>>>>> -     * The VID_PKT_SIZE field in the DSI_VID_PKT_CFG
>>>>> -     * register is 11-bit.
>>>>> -     */
>>>>> -    if (mode->hdisplay > 0x7ff)
>>>>> -        return MODE_BAD_HVALUE;
>>>>> -
>>>>> -    /*
>>>>> -     * The V_ACTIVE_LINES field in the DSI_VTIMING_CFG
>>>>> -     * register is 11-bit.
>>>>> -     */
>>>>> -    if (mode->vdisplay > 0x7ff)
>>>>> -        return MODE_BAD_VVALUE;
>>>>> -
>>>>> -    return MODE_OK;
>>>>> -}
>>>>> -
>>>>>   static struct dw_mipi_dsi_plat_data rk3288_mipi_dsi_drv_data = {
>>>>>       .dsi0_en_bit = RK3288_DSI0_SEL_VOP_LIT,
>>>>>       .dsi1_en_bit = RK3288_DSI1_SEL_VOP_LIT,
>>>>>       .grf_switch_reg = RK3288_GRF_SOC_CON6,
>>>>>       .max_data_lanes = 4,
>>>>> -    .mode_valid = rk3288_mipi_dsi_mode_valid,
>>>>>   };
>>>>>   static struct dw_mipi_dsi_plat_data rk3399_mipi_dsi_drv_data = {
>>>>> -- 
>>>>> 2.6.3
>>>>>
>>>>> _______________________________________________
>>>>> dri-devel mailing list
>>>>> dri-devel@lists.freedesktop.org
>>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>
>>> _______________________________________________
>>> dri-devel mailing list
>>> dri-devel@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
>
>
>


-- 
Mark Yao

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2017-02-05  7:42 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-20 10:10 [PATCH v3 0/5] Rockchip dw-mipi-dsi driver Chris Zhong
2017-01-20 10:10 ` [PATCH v3 1/5] dt-bindings: add rk3399 support for dw-mipi-rockchip Chris Zhong
2017-01-20 10:10 ` [PATCH v3 2/5] drm/rockchip/dsi: dw-mipi: support RK3399 mipi dsi Chris Zhong
2017-01-23 17:30   ` Sean Paul
2017-01-20 10:10 ` [PATCH v3 3/5] drm/rockchip/dsi: remove mode_valid function Chris Zhong
2017-01-23 17:48   ` Sean Paul
2017-01-24  2:27     ` Chris Zhong
2017-02-01 18:12       ` Sean Paul
2017-02-05  3:42         ` Chris Zhong
2017-02-05  7:42           ` Mark yao
2017-01-20 10:10 ` [PATCH v3 4/5] dt-bindings: add power domain node for dw-mipi-rockchip Chris Zhong
2017-01-20 10:10 ` [PATCH v3 5/5] drm/rockchip/dsi: add dw-mipi power domain support Chris Zhong
2017-01-23 18:03   ` Sean Paul

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).