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 ---
.../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]. - 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.
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 ---
drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+)
diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c index f84f9ae..d8f24f2 100644 --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c @@ -291,6 +291,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; @@ -979,6 +980,16 @@ 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. + */ + ret = clk_prepare_enable(dsi->grf_clk); + if (ret) { + dev_err(dsi->dev, "Failed to enable grf_clk\n"); + return; + } + if (pdata->grf_dsi0_mode_reg) regmap_write(dsi->grf_regmap, pdata->grf_dsi0_mode_reg, pdata->grf_dsi0_mode); @@ -1003,6 +1014,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 @@ -1238,6 +1251,17 @@ static int dw_mipi_dsi_bind(struct device *dev, struct device *master, dev_dbg(dev, "have not phy_cfg_clk\n"); }
+ dsi->grf_clk = devm_clk_get(dev, "grf"); + if (IS_ERR(dsi->grf_clk)) { + ret = PTR_ERR(dsi->grf_clk); + if (ret != -ENOENT) { + dev_err(dev, "Unable to get grf_clk: %d\n", ret); + return ret; + } + dsi->grf_clk = NULL; + dev_dbg(dev, "have not grf_clk\n"); + } + ret = clk_prepare_enable(dsi->pllref_clk); if (ret) { dev_err(dev, "%s: Failed to enable pllref_clk\n", __func__);
Hi Chris,
On Wed, Mar 15, 2017 at 04:42:31PM +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
drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+)
diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c index f84f9ae..d8f24f2 100644 --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c @@ -291,6 +291,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;
@@ -979,6 +980,16 @@ 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.*/- ret = clk_prepare_enable(dsi->grf_clk);
- if (ret) {
dev_err(dsi->dev, "Failed to enable grf_clk\n");return;- }
- if (pdata->grf_dsi0_mode_reg) regmap_write(dsi->grf_regmap, pdata->grf_dsi0_mode_reg, pdata->grf_dsi0_mode);
@@ -1003,6 +1014,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 @@ -1238,6 +1251,17 @@ static int dw_mipi_dsi_bind(struct device *dev, struct device *master, dev_dbg(dev, "have not phy_cfg_clk\n"); }
- dsi->grf_clk = devm_clk_get(dev, "grf");
- if (IS_ERR(dsi->grf_clk)) {
ret = PTR_ERR(dsi->grf_clk);if (ret != -ENOENT) {dev_err(dev, "Unable to get grf_clk: %d\n", ret);
If you're going to print an error, you should probably check for -EPROBE_DEFER. This driver isn't quite consistent about this.
return ret;}dsi->grf_clk = NULL;dev_dbg(dev, "have not grf_clk\n");
The wording is a little awkward; maybe "no grf_clk provided\n"?
Also, to be clear, this clock is required for RK3399, but not for others (e.g., RK3288) right? I guess this makes sense then, to just treat it as optional for all cases, even if you document it as "required" for RK3399.
Brian
- }
- ret = clk_prepare_enable(dsi->pllref_clk); if (ret) { dev_err(dev, "%s: Failed to enable pllref_clk\n", __func__);
Brian
On Wed, 15 Mar 2017 09:47:29 -0700, Brian Norris wrote:
On Wed, Mar 15, 2017 at 04:42:31PM +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
drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+)
diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c index f84f9ae..d8f24f2 100644 --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c @@ -291,6 +291,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;
@@ -979,6 +980,16 @@ 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.*/- ret = clk_prepare_enable(dsi->grf_clk);
- if (ret) {
dev_err(dsi->dev, "Failed to enable grf_clk\n");return;- }
- if (pdata->grf_dsi0_mode_reg) regmap_write(dsi->grf_regmap, pdata->grf_dsi0_mode_reg, pdata->grf_dsi0_mode);
@@ -1003,6 +1014,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 @@ -1238,6 +1251,17 @@ static int dw_mipi_dsi_bind(struct device *dev, struct device *master, dev_dbg(dev, "have not phy_cfg_clk\n"); }
- dsi->grf_clk = devm_clk_get(dev, "grf");
- if (IS_ERR(dsi->grf_clk)) {
ret = PTR_ERR(dsi->grf_clk);if (ret != -ENOENT) {dev_err(dev, "Unable to get grf_clk: %d\n", ret);If you're going to print an error, you should probably check for -EPROBE_DEFER. This driver isn't quite consistent about this.
return ret;}dsi->grf_clk = NULL;dev_dbg(dev, "have not grf_clk\n");The wording is a little awkward; maybe "no grf_clk provided\n"?
Also, to be clear, this clock is required for RK3399, but not for others (e.g., RK3288) right? I guess this makes sense then, to just treat it as optional for all cases, even if you document it as "required" for RK3399.
If RK3399 is broken without grf_clk, I wonder if there should be a flag in dw_mipi_dsi_plat_data so that the driver can enforce "required for RK3399".
John
For the RK3399, the grf_switch_reg name should be RK3399_GRF_SOC_CON20, not RK3399_GRF_SOC_CON19.
Signed-off-by: Chris Zhong zyw@rock-chips.com ---
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 d8f24f2..ddd5dba 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)
@@ -1146,7 +1146,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, .max_data_lanes = 4,
Am Mittwoch, 15. März 2017, 16:42:30 CET schrieb Chris Zhong:
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
.../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.t xt b/Documentation/devicetree/bindings/display/rockchip/dw_mipi_dsi_rockchip.t xt index 188f6f7..7e17a60 100644
a/Documentation/devicetree/bindings/display/rockchip/dw_mipi_dsi_rockchip.t xt +++ b/Documentation/devicetree/bindings/display/rockchip/dw_mipi_dsi_rockchip.t xt @@ -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].
your "grf" clock is optional, as it is not present on all socs (like the rk3288) so should probably move to a separate section and not be in the required properties
Heiko
Hi Heiko
On 03/15/2017 05:03 PM, Heiko Stübner wrote:
Am Mittwoch, 15. März 2017, 16:42:30 CET schrieb Chris Zhong:
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
.../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.t xt b/Documentation/devicetree/bindings/display/rockchip/dw_mipi_dsi_rockchip.t xt index 188f6f7..7e17a60 100644
a/Documentation/devicetree/bindings/display/rockchip/dw_mipi_dsi_rockchip.t xt +++ b/Documentation/devicetree/bindings/display/rockchip/dw_mipi_dsi_rockchip.t xt @@ -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].
your "grf" clock is optional, as it is not present on all socs (like the rk3288) so should probably move to a separate section and not be in the required properties
For RK3399, the grf clock is required, according to the advice provided by rob[0], put it into "required properties " is better. [0] https://patchwork.kernel.org/patch/9220187/
Heiko
Am Mittwoch, 15. März 2017, 17:55:23 CET schrieb Chris Zhong:
Hi Heiko
On 03/15/2017 05:03 PM, Heiko Stübner wrote:
Am Mittwoch, 15. März 2017, 16:42:30 CET schrieb Chris Zhong:
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
.../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 .t xt b/Documentation/devicetree/bindings/display/rockchip/dw_mipi_dsi_rockchip .t xt index 188f6f7..7e17a60 100644
a/Documentation/devicetree/bindings/display/rockchip/dw_mipi_dsi_rockchip .t xt +++ b/Documentation/devicetree/bindings/display/rockchip/dw_mipi_dsi_rockchip .t
xt @@ -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].
your "grf" clock is optional, as it is not present on all socs (like the rk3288) so should probably move to a separate section and not be in the required properties
For RK3399, the grf clock is required, according to the advice provided by rob[0], put it into "required properties " is better. [0] https://patchwork.kernel.org/patch/9220187/
ok, but it should probably be made clear, that this is rk3399-specific
dri-devel@lists.freedesktop.org
-
Brian Norris -
Chris Zhong -
Heiko Stübner -
John Keeping