Mark,
RK3399 and RK3288 shared the same HDMI IP controller, only some light difference with GRF configure, and an external VPLL clock need to configure.
Thanks, - Yakir
Douglas Anderson (2): drm/rockchip: dw_hdmi: Set cur_ctr to 0 always drm/rockchip: dw_hdmi: Use auto-generated tables
Yakir Yang (4): drm/rockchip: dw_hdmi: adjust cklvl & txlvl for RF/EMI drm/rockchip: dw_hdmi: add RK3399 HDMI support drm/rockchip: dw_hdmi: introduce the VPLL clock setting drm/rockchip: dw_hdmi: introduce the pclk for grf
.../devicetree/bindings/display/bridge/dw_hdmi.txt | 1 + .../bindings/display/rockchip/dw_hdmi-rockchip.txt | 7 +- drivers/gpu/drm/bridge/dw-hdmi.c | 2 +- drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 238 +++++++++++++-------- include/drm/bridge/dw_hdmi.h | 6 + 5 files changed, 167 insertions(+), 87 deletions(-)
From: Douglas Anderson dianders@chromium.org
Jitter was improved by lowering the MPLL bandwidth to account for high frequency noise in the rk3288 PLL. In each case MPLL bandwidth was lowered only enough to get us a comfortable margin. We believe that lowering the bandwidth like this is safe given sufficient testing.
Signed-off-by: Douglas Anderson dianders@chromium.org Signed-off-by: Yakir Yang ykk@rock-chips.com --- drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c index 801110f..a621118 100644 --- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c +++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c @@ -115,20 +115,8 @@ static const struct dw_hdmi_mpll_config rockchip_mpll_cfg[] = { static const struct dw_hdmi_curr_ctrl rockchip_cur_ctr[] = { /* pixelclk bpp8 bpp10 bpp12 */ { - 40000000, { 0x0018, 0x0018, 0x0018 }, - }, { - 65000000, { 0x0028, 0x0028, 0x0028 }, - }, { - 66000000, { 0x0038, 0x0038, 0x0038 }, - }, { - 74250000, { 0x0028, 0x0038, 0x0038 }, - }, { - 83500000, { 0x0028, 0x0038, 0x0038 }, - }, { - 146250000, { 0x0038, 0x0038, 0x0038 }, - }, { - 148500000, { 0x0000, 0x0038, 0x0038 }, - }, { + 600000000, { 0x0000, 0x0000, 0x0000 }, + }, { ~0UL, { 0x0000, 0x0000, 0x0000}, } };
Dut to the high HDMI signal voltage driver, Mickey have meet a serious RF/EMI problem, so we decided to reduce HDMI signal voltage to a proper value.
The default params for phy is cklvl = 20 & txlvl = 13 (RF/EMI failed) ck: lvl = 13, term=100, vlo = 2.71, vhi=3.14, vswing = 0.43 tx: lvl = 20, term=100, vlo = 2.81, vhi=3.16, vswing = 0.35
1. We decided to reduce voltage value to lower, but VSwing still keep high, RF/EMI have been improved but still failed. ck: lvl = 6, term=100, vlo = 2.61, vhi=3.11, vswing = 0.50 tx: lvl = 6, term=100, vlo = 2.61, vhi=3.11, vswing = 0.50
2. We try to keep voltage value and vswing both lower, then RF/EMI test all passed ;) ck: lvl = 11, term= 66, vlo = 2.68, vhi=3.09, vswing = 0.40 tx: lvl = 11, term= 66, vlo = 2.68, vhi=3.09, vswing = 0.40 When we back to run HDMI different test and single-end test, we see different test passed, but signle-end test failed. The oscilloscope show that simgle-end clock's VL value is 1.78v (which remind LowLimit should not lower then 2.6v).
3. That's to say there are some different between PHY document and measure value. And according to experiment 2 results, we need to higher clock voltage and lower data voltage, then we can keep RF/EMI satisfied and single-end & differen test passed. ck: lvl = 9, term=100, vlo = 2.65, vhi=3.12, vswing = 0.47 tx: lvl = 16, term=100, vlo = 2.75, vhi=3.15, vswing = 0.39
Signed-off-by: Yakir Yang ykk@rock-chips.com Reviewed-by: Douglas Anderson dianders@chromium.org --- drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c index a621118..cdb63ac 100644 --- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c +++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c @@ -124,7 +124,7 @@ static const struct dw_hdmi_curr_ctrl rockchip_cur_ctr[] = { static const struct dw_hdmi_phy_config rockchip_phy_config[] = { /*pixelclk symbol term vlev*/ { 74250000, 0x8009, 0x0004, 0x0272}, - { 148500000, 0x802b, 0x0004, 0x028d}, + { 165000000, 0x802b, 0x0004, 0x0209}, { 297000000, 0x8039, 0x0005, 0x028d}, { ~0UL, 0x0000, 0x0000, 0x0000} };
From: Douglas Anderson dianders@chromium.org
The previous tables for mpll_cfg and curr_ctrl were created using the 20-pages of example settings provided by the PHY vendor. Those example settings weren't particularly dense, so there were places where we were guessing what the settings would be for 10-bit and 12-bit (not that we use those anyway). It was also always a lot of extra work every time we wanted to add a new clock rate since we had to cross-reference several tables.
In http://crosreview.com/285855 I've gone through the work to figure out how to generate this table automatically. Let's now use the automatically generated table and then we'll never need to look at it again.
We only support 8-bit mode right now and only support a small number of clock rates and and I've verified that the only 8-bit rate that was affected was 148.5. That mode appears to have been wrong in the old table.
Signed-off-by: Douglas Anderson dianders@chromium.org Signed-off-by: Yakir Yang ykk@rock-chips.com Reviewed-by: Stéphane Marchesin marcheu@chromium.org --- drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 130 +++++++++++++++------------- 1 file changed, 69 insertions(+), 61 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c index cdb63ac..8cb9ed2 100644 --- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c +++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c @@ -34,80 +34,88 @@ struct rockchip_hdmi {
static const struct dw_hdmi_mpll_config rockchip_mpll_cfg[] = { { - 27000000, { - { 0x00b3, 0x0000}, - { 0x2153, 0x0000}, - { 0x40f3, 0x0000} + 30666000, { + { 0x00b3, 0x0000 }, + { 0x2153, 0x0000 }, + { 0x40f3, 0x0000 }, }, - }, { - 36000000, { - { 0x00b3, 0x0000}, - { 0x2153, 0x0000}, - { 0x40f3, 0x0000} + }, { + 36800000, { + { 0x00b3, 0x0000 }, + { 0x2153, 0x0000 }, + { 0x40a2, 0x0001 }, }, - }, { - 40000000, { - { 0x00b3, 0x0000}, - { 0x2153, 0x0000}, - { 0x40f3, 0x0000} + }, { + 46000000, { + { 0x00b3, 0x0000 }, + { 0x2142, 0x0001 }, + { 0x40a2, 0x0001 }, }, - }, { - 54000000, { - { 0x0072, 0x0001}, - { 0x2142, 0x0001}, - { 0x40a2, 0x0001}, + }, { + 61333000, { + { 0x0072, 0x0001 }, + { 0x2142, 0x0001 }, + { 0x40a2, 0x0001 }, }, - }, { - 65000000, { - { 0x0072, 0x0001}, - { 0x2142, 0x0001}, - { 0x40a2, 0x0001}, + }, { + 73600000, { + { 0x0072, 0x0001 }, + { 0x2142, 0x0001 }, + { 0x4061, 0x0002 }, }, - }, { - 66000000, { - { 0x013e, 0x0003}, - { 0x217e, 0x0002}, - { 0x4061, 0x0002} + }, { + 92000000, { + { 0x0072, 0x0001 }, + { 0x2145, 0x0002 }, + { 0x4061, 0x0002 }, + }, + }, { + 122666000, { + { 0x0051, 0x0002 }, + { 0x2145, 0x0002 }, + { 0x4061, 0x0002 }, }, - }, { - 74250000, { - { 0x0072, 0x0001}, - { 0x2145, 0x0002}, - { 0x4061, 0x0002} + }, { + 147200000, { + { 0x0051, 0x0002 }, + { 0x2145, 0x0002 }, + { 0x4064, 0x0003 }, }, - }, { - 83500000, { - { 0x0072, 0x0001}, + }, { + 184000000, { + { 0x0051, 0x0002 }, + { 0x214c, 0x0003 }, + { 0x4064, 0x0003 }, }, - }, { - 108000000, { - { 0x0051, 0x0002}, - { 0x2145, 0x0002}, - { 0x4061, 0x0002} + }, { + 226666000, { + { 0x0040, 0x0003 }, + { 0x214c, 0x0003 }, + { 0x4064, 0x0003 }, }, - }, { - 106500000, { - { 0x0051, 0x0002}, - { 0x2145, 0x0002}, - { 0x4061, 0x0002} + }, { + 272000000, { + { 0x0040, 0x0003 }, + { 0x214c, 0x0003 }, + { 0x5a64, 0x0003 }, }, - }, { - 146250000, { - { 0x0051, 0x0002}, - { 0x2145, 0x0002}, - { 0x4061, 0x0002} + }, { + 340000000, { + { 0x0040, 0x0003 }, + { 0x3b4c, 0x0003 }, + { 0x5a64, 0x0003 }, }, - }, { - 148500000, { - { 0x0051, 0x0003}, - { 0x214c, 0x0003}, - { 0x4064, 0x0003} + }, { + 600000000, { + { 0x1a40, 0x0003 }, + { 0x3b4c, 0x0003 }, + { 0x5a64, 0x0003 }, }, - }, { + }, { ~0UL, { - { 0x00a0, 0x000a }, - { 0x2001, 0x000f }, - { 0x4002, 0x000f }, + { 0x0000, 0x0000 }, + { 0x0000, 0x0000 }, + { 0x0000, 0x0000 }, }, } };
RK3399 and RK3288 shared the same HDMI IP controller, only some light difference with GRF configure.
Signed-off-by: Yakir Yang ykk@rock-chips.com --- .../devicetree/bindings/display/bridge/dw_hdmi.txt | 1 + .../bindings/display/rockchip/dw_hdmi-rockchip.txt | 3 +- drivers/gpu/drm/bridge/dw-hdmi.c | 2 +- drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 45 ++++++++++++++++++---- include/drm/bridge/dw_hdmi.h | 6 +++ 5 files changed, 48 insertions(+), 9 deletions(-)
diff --git a/Documentation/devicetree/bindings/display/bridge/dw_hdmi.txt b/Documentation/devicetree/bindings/display/bridge/dw_hdmi.txt index dc1452f..f50d4d5 100644 --- a/Documentation/devicetree/bindings/display/bridge/dw_hdmi.txt +++ b/Documentation/devicetree/bindings/display/bridge/dw_hdmi.txt @@ -6,6 +6,7 @@ Required properties: * "fsl,imx6q-hdmi" * "fsl,imx6dl-hdmi" * "rockchip,rk3288-dw-hdmi" + * "rockchip,rk3399-dw-hdmi" - reg: Physical base address and length of the controller's registers. - interrupts: The HDMI interrupt number - clocks, clock-names : must have the phandles to the HDMI iahb and isfr clocks, diff --git a/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt b/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt index 668091f..4e573d2 100644 --- a/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt +++ b/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt @@ -2,7 +2,8 @@ Rockchip specific extensions to the Synopsys Designware HDMI ================================
Required properties: -- compatible: "rockchip,rk3288-dw-hdmi"; +- compatible: "rockchip,rk3288-dw-hdmi", + "rockchip,rk3399-dw-hdmi"; - reg: Physical base address and length of the controller's registers. - clocks: phandle to hdmi iahb and isfr clocks. - clock-names: should be "iahb" "isfr" diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c index 70b1f7d..29f855a 100644 --- a/drivers/gpu/drm/bridge/dw-hdmi.c +++ b/drivers/gpu/drm/bridge/dw-hdmi.c @@ -833,7 +833,7 @@ static int hdmi_phy_configure(struct dw_hdmi *hdmi, unsigned char prep, dw_hdmi_phy_gen2_txpwron(hdmi, 1); dw_hdmi_phy_gen2_pddq(hdmi, 0);
- if (hdmi->dev_type == RK3288_HDMI) + if (is_rockchip(hdmi->dev_type)) dw_hdmi_phy_enable_spare(hdmi, 1);
/*Wait for PHY PLL lock */ diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c index 8cb9ed2..329099b 100644 --- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c +++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c @@ -21,13 +21,18 @@ #include "rockchip_drm_drv.h" #include "rockchip_drm_vop.h"
-#define GRF_SOC_CON6 0x025c -#define HDMI_SEL_VOP_LIT (1 << 4) +#define RK3288_GRF_SOC_CON6 0x025C +#define RK3288_HDMI_LCDC_SEL BIT(4) +#define RK3399_GRF_SOC_CON20 0x6250 +#define RK3399_HDMI_LCDC_SEL BIT(6) + +#define HIWORD_UPDATE(val, mask) (val | (mask) << 16)
struct rockchip_hdmi { struct device *dev; struct regmap *regmap; struct drm_encoder encoder; + enum dw_hdmi_devtype dev_type; };
#define to_rockchip_hdmi(x) container_of(x, struct rockchip_hdmi, x) @@ -194,16 +199,30 @@ static void dw_hdmi_rockchip_encoder_mode_set(struct drm_encoder *encoder, static void dw_hdmi_rockchip_encoder_enable(struct drm_encoder *encoder) { struct rockchip_hdmi *hdmi = to_rockchip_hdmi(encoder); + u32 lcdsel_grf_reg, lcdsel_mask; u32 val; int mux;
+ switch (hdmi->dev_type) { + case RK3288_HDMI: + lcdsel_grf_reg = RK3288_GRF_SOC_CON6; + lcdsel_mask = RK3288_HDMI_LCDC_SEL; + break; + case RK3399_HDMI: + lcdsel_grf_reg = RK3399_GRF_SOC_CON20; + lcdsel_mask = RK3399_HDMI_LCDC_SEL; + break; + default: + return; + }; + mux = drm_of_encoder_active_endpoint_id(hdmi->dev->of_node, encoder); if (mux) - val = HDMI_SEL_VOP_LIT | (HDMI_SEL_VOP_LIT << 16); + val = HIWORD_UPDATE(lcdsel_mask, lcdsel_mask); else - val = HDMI_SEL_VOP_LIT << 16; + val = HIWORD_UPDATE(0, lcdsel_mask);
- regmap_write(hdmi->regmap, GRF_SOC_CON6, val); + regmap_write(hdmi->regmap, lcdsel_grf_reg, val); dev_dbg(hdmi->dev, "vop %s output to hdmi\n", (mux) ? "LIT" : "BIG"); } @@ -229,7 +248,7 @@ static const struct drm_encoder_helper_funcs dw_hdmi_rockchip_encoder_helper_fun .atomic_check = dw_hdmi_rockchip_encoder_atomic_check, };
-static const struct dw_hdmi_plat_data rockchip_hdmi_drv_data = { +static const struct dw_hdmi_plat_data rk3288_hdmi_drv_data = { .mode_valid = dw_hdmi_rockchip_mode_valid, .mpll_cfg = rockchip_mpll_cfg, .cur_ctr = rockchip_cur_ctr, @@ -237,9 +256,20 @@ static const struct dw_hdmi_plat_data rockchip_hdmi_drv_data = { .dev_type = RK3288_HDMI, };
+static const struct dw_hdmi_plat_data rk3399_hdmi_drv_data = { + .mode_valid = dw_hdmi_rockchip_mode_valid, + .mpll_cfg = rockchip_mpll_cfg, + .cur_ctr = rockchip_cur_ctr, + .phy_config = rockchip_phy_config, + .dev_type = RK3399_HDMI, +}; + static const struct of_device_id dw_hdmi_rockchip_dt_ids[] = { { .compatible = "rockchip,rk3288-dw-hdmi", - .data = &rockchip_hdmi_drv_data + .data = &rk3288_hdmi_drv_data + }, + { .compatible = "rockchip,rk3399-dw-hdmi", + .data = &rk3399_hdmi_drv_data }, {}, }; @@ -268,6 +298,7 @@ static int dw_hdmi_rockchip_bind(struct device *dev, struct device *master, match = of_match_node(dw_hdmi_rockchip_dt_ids, pdev->dev.of_node); plat_data = match->data; hdmi->dev = &pdev->dev; + hdmi->dev_type = plat_data->dev_type; encoder = &hdmi->encoder;
irq = platform_get_irq(pdev, 0); diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h index bae79f3..26c06d5 100644 --- a/include/drm/bridge/dw_hdmi.h +++ b/include/drm/bridge/dw_hdmi.h @@ -25,6 +25,7 @@ enum dw_hdmi_devtype { IMX6Q_HDMI, IMX6DL_HDMI, RK3288_HDMI, + RK3399_HDMI, };
struct dw_hdmi_mpll_config { @@ -56,6 +57,11 @@ struct dw_hdmi_plat_data { struct drm_display_mode *mode); };
+static inline bool is_rockchip(enum dw_hdmi_devtype dev_type) +{ + return dev_type == RK3288_HDMI || dev_type == RK3399_HDMI; +} + void dw_hdmi_unbind(struct device *dev, struct device *master, void *data); int dw_hdmi_bind(struct device *dev, struct device *master, void *data, struct drm_encoder *encoder,
Am Montag, den 11.07.2016, 19:05 +0800 schrieb Yakir Yang:
RK3399 and RK3288 shared the same HDMI IP controller, only some light difference with GRF configure.
Signed-off-by: Yakir Yang ykk@rock-chips.com
Reviewed-by: Philipp Zabel p.zabel@pengutronix.de
regards Philipp
Philipp,
On 07/11/2016 07:51 PM, Philipp Zabel wrote:
Am Montag, den 11.07.2016, 19:05 +0800 schrieb Yakir Yang:
RK3399 and RK3288 shared the same HDMI IP controller, only some light difference with GRF configure.
Signed-off-by: Yakir Yang ykk@rock-chips.com
Reviewed-by: Philipp Zabel p.zabel@pengutronix.de
Thanks for your fast respond :-D - Yakir
regards Philipp
On Mon, Jul 11, 2016 at 07:05:46PM +0800, Yakir Yang wrote:
RK3399 and RK3288 shared the same HDMI IP controller, only some light difference with GRF configure.
Signed-off-by: Yakir Yang ykk@rock-chips.com
.../devicetree/bindings/display/bridge/dw_hdmi.txt | 1 + .../bindings/display/rockchip/dw_hdmi-rockchip.txt | 3 +-
Acked-by: Rob Herring robh@kernel.org
drivers/gpu/drm/bridge/dw-hdmi.c | 2 +- drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 45 ++++++++++++++++++---- include/drm/bridge/dw_hdmi.h | 6 +++ 5 files changed, 48 insertions(+), 9 deletions(-)
For RK3399 HDMI, there is an external clock need for HDMI PHY, and it should keep the same clock rate with VOP DCLK.
VPLL have supported the clock for HDMI PHY, but there is no clock divider bewteen VPLL and HDMI PHY. So we need to set the VPLL rate manually in HDMI driver.
Signed-off-by: Yakir Yang ykk@rock-chips.com --- .../bindings/display/rockchip/dw_hdmi-rockchip.txt | 3 ++- drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 25 +++++++++++++++++++++- 2 files changed, 26 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt b/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt index 4e573d2..4e23ca4 100644 --- a/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt +++ b/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt @@ -17,7 +17,8 @@ Required properties:
Optional properties - ddc-i2c-bus: phandle of an I2C controller used for DDC EDID probing -- clocks, clock-names: phandle to the HDMI CEC clock, name should be "cec" +- clocks, clock-names: phandle to the HDMI CEC clock, name should be "cec", + phandle to the VPLL clock, name should be "vpll".
Example: hdmi: hdmi@ff980000 { diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c index 329099b..701bb73 100644 --- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c +++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c @@ -7,10 +7,12 @@ * (at your option) any later version. */
+#include <linux/clk.h> +#include <linux/mfd/syscon.h> #include <linux/module.h> #include <linux/platform_device.h> -#include <linux/mfd/syscon.h> #include <linux/regmap.h> + #include <drm/drm_of.h> #include <drm/drmP.h> #include <drm/drm_crtc_helper.h> @@ -33,6 +35,7 @@ struct rockchip_hdmi { struct regmap *regmap; struct drm_encoder encoder; enum dw_hdmi_devtype dev_type; + struct clk *vpll_clk; };
#define to_rockchip_hdmi(x) container_of(x, struct rockchip_hdmi, x) @@ -145,6 +148,7 @@ static const struct dw_hdmi_phy_config rockchip_phy_config[] = { static int rockchip_hdmi_parse_dt(struct rockchip_hdmi *hdmi) { struct device_node *np = hdmi->dev->of_node; + int ret;
hdmi->regmap = syscon_regmap_lookup_by_phandle(np, "rockchip,grf"); if (IS_ERR(hdmi->regmap)) { @@ -152,6 +156,22 @@ static int rockchip_hdmi_parse_dt(struct rockchip_hdmi *hdmi) return PTR_ERR(hdmi->regmap); }
+ hdmi->vpll_clk = devm_clk_get(hdmi->dev, "vpll"); + if (PTR_ERR(hdmi->vpll_clk) == -ENOENT) { + hdmi->vpll_clk = NULL; + } else if (PTR_ERR(hdmi->vpll_clk) == -EPROBE_DEFER) { + return -EPROBE_DEFER; + } else if (IS_ERR(hdmi->vpll_clk)) { + dev_err(hdmi->dev, "failed to get grf clock\n"); + return PTR_ERR(hdmi->vpll_clk); + } + + ret = clk_prepare_enable(hdmi->vpll_clk); + if (ret) { + dev_err(hdmi->dev, "Failed to enable HDMI vpll: %d\n", ret); + return ret; + } + return 0; }
@@ -194,6 +214,9 @@ static void dw_hdmi_rockchip_encoder_mode_set(struct drm_encoder *encoder, struct drm_display_mode *mode, struct drm_display_mode *adj_mode) { + struct rockchip_hdmi *hdmi = to_rockchip_hdmi(encoder); + + clk_set_rate(hdmi->vpll_clk, adj_mode->clock * 1000); }
static void dw_hdmi_rockchip_encoder_enable(struct drm_encoder *encoder)
Hi Yakir,
Am Montag, 11. Juli 2016, 19:05:49 schrieb Yakir Yang:
For RK3399 HDMI, there is an external clock need for HDMI PHY, and it should keep the same clock rate with VOP DCLK.
VPLL have supported the clock for HDMI PHY, but there is no clock divider bewteen VPLL and HDMI PHY. So we need to set the VPLL rate manually in HDMI driver.
I don't think reserving the vpll for the hdmi at all times is the right way to go.
While I do agree that reserving the VPLL (and NPLL on the rk3288) for graphics use looks like the right way, I think the core Rockchip drm driver should be responsible for managing it and also for deciding which output encoder gets to use it.
While true that on the Chromebook device-types the edp is static and hdmi needs the broad range of dynamic frequencies, this is not necessarily the case for all future device types and/or socs.
In the other thread we discussed adding that as rockchip,dclk-pll = <&...>; to the base display-subsystem node, Doug didn't manage to find time to respond yet though - and is on vacation right now.
I still believe that would be the best solution :-) .
That property could list 1 or even 2 plls, depending on the soc or board layout - maybe someone frees up the cpll in some special layout or something. If none are listed, then the drm driver would need to cope with its available clocks.
Implementation-wise you could even still keep your shortcut until we encounter a device with different , let the drm use the pll for hdmi only until someone comes up with a better concept, but still the binding should be correct and versatile from the start.
Heiko
.../bindings/display/rockchip/dw_hdmi-rockchip.txt | 3 ++- drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 25 +++++++++++++++++++++- 2 files changed, 26 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt b/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt index 4e573d2..4e23ca4 100644
a/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt +++ b/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt @@ -17,7 +17,8 @@ Required properties:
Optional properties
- ddc-i2c-bus: phandle of an I2C controller used for DDC EDID probing
-- clocks, clock-names: phandle to the HDMI CEC clock, name should be "cec" +- clocks, clock-names: phandle to the HDMI CEC clock, name should be "cec",
phandle to the VPLL clock, name should be "vpll".Example: hdmi: hdmi@ff980000 { diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c index 329099b..701bb73 100644 --- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c +++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c @@ -7,10 +7,12 @@
- (at your option) any later version.
*/
+#include <linux/clk.h> +#include <linux/mfd/syscon.h> #include <linux/module.h> #include <linux/platform_device.h> -#include <linux/mfd/syscon.h> #include <linux/regmap.h>
#include <drm/drm_of.h> #include <drm/drmP.h> #include <drm/drm_crtc_helper.h> @@ -33,6 +35,7 @@ struct rockchip_hdmi { struct regmap *regmap; struct drm_encoder encoder; enum dw_hdmi_devtype dev_type;
- struct clk *vpll_clk;
};
#define to_rockchip_hdmi(x) container_of(x, struct rockchip_hdmi, x) @@ -145,6 +148,7 @@ static const struct dw_hdmi_phy_config rockchip_phy_config[] = { static int rockchip_hdmi_parse_dt(struct rockchip_hdmi *hdmi) { struct device_node *np = hdmi->dev->of_node;
int ret;
hdmi->regmap = syscon_regmap_lookup_by_phandle(np, "rockchip,grf"); if (IS_ERR(hdmi->regmap)) {
@@ -152,6 +156,22 @@ static int rockchip_hdmi_parse_dt(struct rockchip_hdmi *hdmi) return PTR_ERR(hdmi->regmap); }
- hdmi->vpll_clk = devm_clk_get(hdmi->dev, "vpll");
- if (PTR_ERR(hdmi->vpll_clk) == -ENOENT) {
hdmi->vpll_clk = NULL;- } else if (PTR_ERR(hdmi->vpll_clk) == -EPROBE_DEFER) {
return -EPROBE_DEFER;- } else if (IS_ERR(hdmi->vpll_clk)) {
dev_err(hdmi->dev, "failed to get grf clock\n");return PTR_ERR(hdmi->vpll_clk);- }
- ret = clk_prepare_enable(hdmi->vpll_clk);
- if (ret) {
dev_err(hdmi->dev, "Failed to enable HDMI vpll: %d\n", ret);return ret;- }
- return 0;
}
@@ -194,6 +214,9 @@ static void dw_hdmi_rockchip_encoder_mode_set(struct drm_encoder *encoder, struct drm_display_mode *mode, struct drm_display_mode *adj_mode) {
- struct rockchip_hdmi *hdmi = to_rockchip_hdmi(encoder);
- clk_set_rate(hdmi->vpll_clk, adj_mode->clock * 1000);
}
static void dw_hdmi_rockchip_encoder_enable(struct drm_encoder *encoder)
Heiko,
On 07/12/2016 10:27 PM, Heiko Stübner wrote:
Hi Yakir,
Am Montag, 11. Juli 2016, 19:05:49 schrieb Yakir Yang:
For RK3399 HDMI, there is an external clock need for HDMI PHY, and it should keep the same clock rate with VOP DCLK.
VPLL have supported the clock for HDMI PHY, but there is no clock divider bewteen VPLL and HDMI PHY. So we need to set the VPLL rate manually in HDMI driver.
I don't think reserving the vpll for the hdmi at all times is the right way to go.
While I do agree that reserving the VPLL (and NPLL on the rk3288) for graphics use looks like the right way, I think the core Rockchip drm driver should be responsible for managing it and also for deciding which output encoder gets to use it. While true that on the Chromebook device-types the edp is static and hdmi needs the broad range of dynamic frequencies, this is not necessarily the case for all future device types and/or socs.
In the other thread we discussed adding that as rockchip,dclk-pll = <&...>; to the base display-subsystem node, Doug didn't manage to find time to respond yet though - and is on vacation right now.
Great idea. Let a separate drm-pll driver to maintain all connector's clock requirement.
I still believe that would be the best solution :-) .
That property could list 1 or even 2 plls, depending on the soc or board layout - maybe someone frees up the cpll in some special layout or something. If none are listed, then the drm driver would need to cope with its available clocks.
Implementation-wise you could even still keep your shortcut until we encounter a device with different , let the drm use the pll for hdmi only until someone comes up with a better concept, but still the binding should be correct and versatile from the start.
Someone should start to prepare the drm core pll driver, it's great idea.
BR, - Yakir
Heiko
.../bindings/display/rockchip/dw_hdmi-rockchip.txt | 3 ++- drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 25 +++++++++++++++++++++- 2 files changed, 26 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt b/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt index 4e573d2..4e23ca4 100644
a/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt +++ b/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt @@ -17,7 +17,8 @@ Required properties:
Optional properties
- ddc-i2c-bus: phandle of an I2C controller used for DDC EDID probing
-- clocks, clock-names: phandle to the HDMI CEC clock, name should be "cec" +- clocks, clock-names: phandle to the HDMI CEC clock, name should be "cec",
phandle to the VPLL clock, name should be "vpll".Example: hdmi: hdmi@ff980000 {
diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c index 329099b..701bb73 100644 --- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c +++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c @@ -7,10 +7,12 @@
- (at your option) any later version.
*/
+#include <linux/clk.h> +#include <linux/mfd/syscon.h> #include <linux/module.h> #include <linux/platform_device.h> -#include <linux/mfd/syscon.h> #include <linux/regmap.h>
- #include <drm/drm_of.h> #include <drm/drmP.h> #include <drm/drm_crtc_helper.h>
@@ -33,6 +35,7 @@ struct rockchip_hdmi { struct regmap *regmap; struct drm_encoder encoder; enum dw_hdmi_devtype dev_type;
struct clk *vpll_clk; };
#define to_rockchip_hdmi(x) container_of(x, struct rockchip_hdmi, x)
@@ -145,6 +148,7 @@ static const struct dw_hdmi_phy_config rockchip_phy_config[] = { static int rockchip_hdmi_parse_dt(struct rockchip_hdmi *hdmi) { struct device_node *np = hdmi->dev->of_node;
int ret;
hdmi->regmap = syscon_regmap_lookup_by_phandle(np, "rockchip,grf"); if (IS_ERR(hdmi->regmap)) {
@@ -152,6 +156,22 @@ static int rockchip_hdmi_parse_dt(struct rockchip_hdmi *hdmi) return PTR_ERR(hdmi->regmap); }
- hdmi->vpll_clk = devm_clk_get(hdmi->dev, "vpll");
- if (PTR_ERR(hdmi->vpll_clk) == -ENOENT) {
hdmi->vpll_clk = NULL;- } else if (PTR_ERR(hdmi->vpll_clk) == -EPROBE_DEFER) {
return -EPROBE_DEFER;- } else if (IS_ERR(hdmi->vpll_clk)) {
dev_err(hdmi->dev, "failed to get grf clock\n");return PTR_ERR(hdmi->vpll_clk);- }
- ret = clk_prepare_enable(hdmi->vpll_clk);
- if (ret) {
dev_err(hdmi->dev, "Failed to enable HDMI vpll: %d\n", ret);return ret;- }
- return 0; }
@@ -194,6 +214,9 @@ static void dw_hdmi_rockchip_encoder_mode_set(struct drm_encoder *encoder, struct drm_display_mode *mode, struct drm_display_mode *adj_mode) {
struct rockchip_hdmi *hdmi = to_rockchip_hdmi(encoder);
clk_set_rate(hdmi->vpll_clk, adj_mode->clock * 1000); }
static void dw_hdmi_rockchip_encoder_enable(struct drm_encoder *encoder)
For RK3399's GRF module, if we want to operate the graphic related grf registers, we need to enable the pclk_vio_grf which supply power for VIO GRF IOs, so it's better to introduce an optional grf clock in driver.
Signed-off-by: Yakir Yang ykk@rock-chips.com --- .../bindings/display/rockchip/dw_hdmi-rockchip.txt | 3 ++- drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 20 ++++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt b/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt index 4e23ca4..e22d70f 100644 --- a/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt +++ b/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt @@ -18,7 +18,8 @@ Required properties: Optional properties - ddc-i2c-bus: phandle of an I2C controller used for DDC EDID probing - clocks, clock-names: phandle to the HDMI CEC clock, name should be "cec", - phandle to the VPLL clock, name should be "vpll". + phandle to the VPLL clock, name should be "vpll", + phandle to the GRF clock, name should be "grf".
Example: hdmi: hdmi@ff980000 { diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c index 701bb73..69e6efb 100644 --- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c +++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c @@ -36,6 +36,7 @@ struct rockchip_hdmi { struct drm_encoder encoder; enum dw_hdmi_devtype dev_type; struct clk *vpll_clk; + struct clk *grf_clk; };
#define to_rockchip_hdmi(x) container_of(x, struct rockchip_hdmi, x) @@ -166,6 +167,16 @@ static int rockchip_hdmi_parse_dt(struct rockchip_hdmi *hdmi) return PTR_ERR(hdmi->vpll_clk); }
+ hdmi->grf_clk = devm_clk_get(hdmi->dev, "grf"); + if (PTR_ERR(hdmi->grf_clk) == -ENOENT) { + hdmi->grf_clk = NULL; + } else if (PTR_ERR(hdmi->grf_clk) == -EPROBE_DEFER) { + return -EPROBE_DEFER; + } else if (IS_ERR(hdmi->grf_clk)) { + dev_err(hdmi->dev, "failed to get grf clock\n"); + return PTR_ERR(hdmi->grf_clk); + } + ret = clk_prepare_enable(hdmi->vpll_clk); if (ret) { dev_err(hdmi->dev, "Failed to enable HDMI vpll: %d\n", ret); @@ -225,6 +236,7 @@ static void dw_hdmi_rockchip_encoder_enable(struct drm_encoder *encoder) u32 lcdsel_grf_reg, lcdsel_mask; u32 val; int mux; + int ret;
switch (hdmi->dev_type) { case RK3288_HDMI: @@ -245,9 +257,17 @@ static void dw_hdmi_rockchip_encoder_enable(struct drm_encoder *encoder) else val = HIWORD_UPDATE(0, lcdsel_mask);
+ ret = clk_prepare_enable(hdmi->grf_clk); + if (ret < 0) { + dev_err(hdmi->dev, "failed to enable grfclk %d\n", ret); + return; + } + regmap_write(hdmi->regmap, lcdsel_grf_reg, val); dev_dbg(hdmi->dev, "vop %s output to hdmi\n", (mux) ? "LIT" : "BIG"); + + clk_disable_unprepare(hdmi->grf_clk); }
static int
dri-devel@lists.freedesktop.org
-
Heiko Stübner -
Philipp Zabel -
Rob Herring -
Yakir Yang