On Fri, Mar 17, 2017 at 11:54:21AM +0800, Chris Zhong wrote:

For RK3399, the phy_cfg_clk is a required clock, if phy_cfg_clk is disabled, MIPI phy can not work. Let's return a error if there is no phy_cfg_clk in dts property, when the pdata match RK3399.

The dt bindings say this is a required clock, I think you'll need to update them to reflect that this is optional for certain SoCs

Sean

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

Changes in v3:

• add a DW_MIPI_NEEDS_PHY_CFG_CLK for RK3399

Changes in v2: None

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

diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c index f84f9ae..68f48b0 100644 --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c @@ -251,6 +251,8 @@ #define THS_PRE_PROGRAM_EN BIT(7) #define THS_ZERO_PROGRAM_EN BIT(6)

+#define DW_MIPI_NEEDS_PHY_CFG_CLK BIT(0)

enum { BANDGAP_97_07, BANDGAP_98_05, @@ -279,6 +281,7 @@ struct dw_mipi_dsi_plat_data { u32 grf_switch_reg; u32 grf_dsi0_mode; u32 grf_dsi0_mode_reg;

• unsigned int flags; unsigned int max_data_lanes;

};

@@ -1136,6 +1139,7 @@ static struct dw_mipi_dsi_plat_data rk3399_mipi_dsi_drv_data = { .grf_switch_reg = RK3399_GRF_SOC_CON19, .grf_dsi0_mode = RK3399_GRF_DSI_MODE, .grf_dsi0_mode_reg = RK3399_GRF_SOC_CON22,

• .flags = DW_MIPI_NEEDS_PHY_CFG_CLK, .max_data_lanes = 4,

};

@@ -1227,15 +1231,13 @@ 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)) {

• ret = PTR_ERR(dsi->phy_cfg_clk);
  

• if (ret != -ENOENT) {
  

• if (pdata->flags & DW_MIPI_NEEDS_PHY_CFG_CLK) {

• dsi->phy_cfg_clk = devm_clk_get(dev, "phy_cfg");
  

• if (IS_ERR(dsi->phy_cfg_clk)) {
  

• 	ret = PTR_ERR(dsi->phy_cfg_clk);
  dev_err(dev, "Unable to get phy_cfg_clk: %d\n", ret);
  return ret;
  

  }

• dsi->phy_cfg_clk = NULL;
  

• dev_dbg(dev, "have not phy_cfg_clk\n");
  

  }

  ret = clk_prepare_enable(dsi->pllref_clk);

-- 2.6.3

---

dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel

On Tue, Mar 21, 2017 at 04:17:00PM -0400, Sean Paul wrote:

On Fri, Mar 17, 2017 at 11:54:22AM +0800, Chris Zhong wrote:

...

For RK3399, the grf clock should be controlled by dw-mipi-dsi driver, add the description for this clock.

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

Changes in v3: None Changes in v2: None

.../devicetree/bindings/display/rockchip/dw_mipi_dsi_rockchip.txt | 2 +- 1 file changed, 1 insertion(+), 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 188f6f7..7e17a60 100644 --- a/Documentation/devicetree/bindings/display/rockchip/dw_mipi_dsi_rockchip.txt +++ b/Documentation/devicetree/bindings/display/rockchip/dw_mipi_dsi_rockchip.txt @@ -10,7 +10,7 @@ Required properties:

• 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). For RK3399, a phy config clock
• (phy_cfg) is additional required. As described in [1].

• (phy_cfg) and a grf clock(grf) are additional required. As described in [1].

These are only required for rk3399, you should make that clear.

I completely missed "For RK3399" on my first pass, sigh. Sorry for the reading comprehension fail.

Minor nit if you want: s/additional// && s/. As/, as/

Either way,

Reviewed-by: Sean Paul seanpaul@chromium.org

Sean

Sean

...

• 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

---

dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

On Fri, Mar 17, 2017 at 11:54:23AM +0800, Chris Zhong wrote:

For RK3399, the grf clk should be enabled before writing grf registers, otherwise the register value can not be changed.

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

Minor nit below, with that:

Reviewed-by: Sean Paul seanpaul@chromium.org

---

Changes in v3:

• add a DW_MIPI_NEEDS_GRF_CLK for RK3399

Changes in v2:

• check the grf_clk only for RK3399

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

diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c index 68f48b0..5a18281 100644 --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c @@ -252,6 +252,7 @@ #define THS_ZERO_PROGRAM_EN BIT(6)

#define DW_MIPI_NEEDS_PHY_CFG_CLK BIT(0) +#define DW_MIPI_NEEDS_GRF_CLK BIT(1)

enum { BANDGAP_97_07, @@ -294,6 +295,7 @@ struct dw_mipi_dsi { struct regmap *grf_regmap; void __iomem *base;

• struct clk *grf_clk; struct clk *pllref_clk; struct clk *pclk; struct clk *phy_cfg_clk;

@@ -982,6 +984,17 @@ static void dw_mipi_dsi_encoder_enable(struct drm_encoder *encoder) dw_mipi_dsi_dphy_interface_config(dsi); dw_mipi_dsi_clear_err(dsi);

• /*

• * For the RK3399, the clk of grf must be enabled before writing grf
  

• * register. And for RK3288 or other soc, this grf_clk must be NULL,
  

• * the clk_prepare_enable return true directly.
  

• */
  

• ret = clk_prepare_enable(dsi->grf_clk);
• if (ret) {

• dev_err(dsi->dev, "Failed to enable grf_clk\n");
  

nit: Print ret?

• return;
  

• }
• if (pdata->grf_dsi0_mode_reg) regmap_write(dsi->grf_regmap, pdata->grf_dsi0_mode_reg, pdata->grf_dsi0_mode);

@@ -1006,6 +1019,8 @@ 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;

• clk_disable_unprepare(dsi->grf_clk);

}

static int @@ -1139,7 +1154,7 @@ static struct dw_mipi_dsi_plat_data rk3399_mipi_dsi_drv_data = { .grf_switch_reg = RK3399_GRF_SOC_CON19, .grf_dsi0_mode = RK3399_GRF_DSI_MODE, .grf_dsi0_mode_reg = RK3399_GRF_SOC_CON22,

• .flags = DW_MIPI_NEEDS_PHY_CFG_CLK,

• .flags = DW_MIPI_NEEDS_PHY_CFG_CLK | DW_MIPI_NEEDS_GRF_CLK, .max_data_lanes = 4,

};

@@ -1240,6 +1255,15 @@ static int dw_mipi_dsi_bind(struct device *dev, struct device *master, } }

• if (pdata->flags & DW_MIPI_NEEDS_GRF_CLK) {

• dsi->grf_clk = devm_clk_get(dev, "grf");
  

• if (IS_ERR(dsi->grf_clk)) {
  

• 	ret = PTR_ERR(dsi->grf_clk);
  

• 	dev_err(dev, "Unable to get grf_clk: %d\n", ret);
  

• 	return ret;
  

• }
  

• }
• ret = clk_prepare_enable(dsi->pllref_clk); if (ret) { dev_err(dev, "%s: Failed to enable pllref_clk\n", __func__);

-- 2.6.3

---

dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel

On Fri, Mar 17, 2017 at 11:54:24AM +0800, Chris Zhong wrote:

For the RK3399, the grf_switch_reg name should be RK3399_GRF_SOC_CON20, not RK3399_GRF_SOC_CON19.

Matches the TRM for me, and otherwise it's a no-op:

Reviewed-by: Brian Norris briannorris@chromium.org

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

Changes in v3: None Changes in v2: None

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

diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c index 5a18281..19b9208 100644 --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c @@ -34,7 +34,7 @@ #define RK3288_DSI0_SEL_VOP_LIT BIT(6) #define RK3288_DSI1_SEL_VOP_LIT BIT(9)

-#define RK3399_GRF_SOC_CON19 0x6250 +#define RK3399_GRF_SOC_CON20 0x6250 #define RK3399_DSI0_SEL_VOP_LIT BIT(0) #define RK3399_DSI1_SEL_VOP_LIT BIT(4)

@@ -1151,7 +1151,7 @@ static struct dw_mipi_dsi_plat_data rk3288_mipi_dsi_drv_data = { 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_switch_reg = RK3399_GRF_SOC_CON20, .grf_dsi0_mode = RK3399_GRF_DSI_MODE, .grf_dsi0_mode_reg = RK3399_GRF_SOC_CON22, .flags = DW_MIPI_NEEDS_PHY_CFG_CLK | DW_MIPI_NEEDS_GRF_CLK,

-- 2.6.3