Messages in this thread | | | Subject | Re: [PATCH v2 2/2] drm/rockchip: dw_hdmi: add rk3568 support | From | Andy Yan <> | Date | Wed, 14 Jul 2021 09:26:28 +0800 |
| |
Hi Alex:
On 7/13/21 7:40 PM, Alex Bee wrote: > Hi Benjamin, > > Am 07.07.21 um 14:03 schrieb Benjamin Gaignard: >> Add a new dw_hdmi_plat_data struct and new compatible for rk3568. >> This version of the HDMI hardware block need two clocks to provide >> phy reference clock: hclk_vio and hclk. >> >> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com> >> --- >> version 2: >> - Add the clocks needed for the phy. > > If got Alega's comment correct, it wasn't about the hclks. > It looks like for this variant, there is another reference clock > required (for the phy) like vpll is already (looks like downstream > uses HPLL ( = "HDMI-PLL" ?) for that - which also has to switch the > frequency according the the drm mode rate - the two clocks you added > here are get just enabled (and disabled) here.
Yes, it's HPLL, and the frequency of HPLL and drm mode rate(vop dclk) should keep 1:1.
> > Alega, Andy: Is it really required to enable hclk_vio and hclk(_vop) > in the hdmi driver? Are they required to be enabled for the other > output variants (i.e. mipi, dsi, rgb ....) as well and shouldn't > better be enabled in the (not-yet existing) vop2 driver?
hclk_vop should be enabled, other wise you can't access hdmi registers. This is only required for HDMI(mipi dis, edp, rgb don't need it)
> > Overall: I'm not sure of the benefit of adding this hdmi variant for a > SoC where the display driver isn't implemented upstream yet. The > "VOP2" IP seems widely new and should probably be ported first. (even > if the HDMI part seems a low hanging fruit according to the vendor > sources)
Yes, VOP2 IP is widely totaly new and complicated, I have a plan to do the upstream. But I am in a rush now, so please give me a lite time😁.
> > Best, > Alex > >> >> drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 68 +++++++++++++++++++++ >> 1 file changed, 68 insertions(+) >> >> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c >> b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c >> index 830bdd5e9b7ce..dc0e255e45745 100644 >> --- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c >> +++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c >> @@ -50,6 +50,10 @@ >> #define RK3399_GRF_SOC_CON20 0x6250 >> #define RK3399_HDMI_LCDC_SEL BIT(6) >> +#define RK3568_GRF_VO_CON1 0x0364 >> +#define RK3568_HDMI_SDAIN_MSK BIT(15) >> +#define RK3568_HDMI_SCLIN_MSK BIT(14) >> + >> #define HIWORD_UPDATE(val, mask) (val | (mask) << 16) >> /** >> @@ -71,6 +75,8 @@ struct rockchip_hdmi { >> const struct rockchip_hdmi_chip_data *chip_data; >> struct clk *vpll_clk; >> struct clk *grf_clk; >> + struct clk *hclk_vio; >> + struct clk *hclk_vop; >> struct dw_hdmi *hdmi; >> struct phy *phy; >> }; >> @@ -216,6 +222,26 @@ static int rockchip_hdmi_parse_dt(struct >> rockchip_hdmi *hdmi) >> return PTR_ERR(hdmi->grf_clk); >> } >> + hdmi->hclk_vio = devm_clk_get(hdmi->dev, "hclk_vio"); >> + if (PTR_ERR(hdmi->hclk_vio) == -ENOENT) { >> + hdmi->hclk_vio = NULL; >> + } else if (PTR_ERR(hdmi->hclk_vio) == -EPROBE_DEFER) { >> + return -EPROBE_DEFER; >> + } else if (IS_ERR(hdmi->hclk_vio)) { >> + dev_err(hdmi->dev, "failed to get hclk_vio clock\n"); >> + return PTR_ERR(hdmi->hclk_vio); >> + } >> + >> + hdmi->hclk_vop = devm_clk_get(hdmi->dev, "hclk"); >> + if (PTR_ERR(hdmi->hclk_vop) == -ENOENT) { >> + hdmi->hclk_vop = NULL; >> + } else if (PTR_ERR(hdmi->hclk_vop) == -EPROBE_DEFER) { >> + return -EPROBE_DEFER; >> + } else if (IS_ERR(hdmi->hclk_vop)) { >> + dev_err(hdmi->dev, "failed to get hclk_vop clock\n"); >> + return PTR_ERR(hdmi->hclk_vop); >> + } >> + >> return 0; >> } >> @@ -467,6 +493,19 @@ static const struct dw_hdmi_plat_data >> rk3399_hdmi_drv_data = { >> .use_drm_infoframe = true, >> }; >> +static struct rockchip_hdmi_chip_data rk3568_chip_data = { >> + .lcdsel_grf_reg = -1, >> +}; >> + >> +static const struct dw_hdmi_plat_data rk3568_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, >> + .phy_data = &rk3568_chip_data, >> + .use_drm_infoframe = true, >> +}; >> + >> static const struct of_device_id dw_hdmi_rockchip_dt_ids[] = { >> { .compatible = "rockchip,rk3228-dw-hdmi", >> .data = &rk3228_hdmi_drv_data >> @@ -480,6 +519,9 @@ static const struct of_device_id >> dw_hdmi_rockchip_dt_ids[] = { >> { .compatible = "rockchip,rk3399-dw-hdmi", >> .data = &rk3399_hdmi_drv_data >> }, >> + { .compatible = "rockchip,rk3568-dw-hdmi", >> + .data = &rk3568_hdmi_drv_data >> + }, >> {}, >> }; >> MODULE_DEVICE_TABLE(of, dw_hdmi_rockchip_dt_ids); >> @@ -536,6 +578,28 @@ static int dw_hdmi_rockchip_bind(struct device >> *dev, struct device *master, >> return ret; >> } >> + ret = clk_prepare_enable(hdmi->hclk_vio); >> + if (ret) { >> + dev_err(hdmi->dev, "Failed to enable HDMI hclk_vio: %d\n", >> + ret); >> + return ret; >> + } >> + >> + ret = clk_prepare_enable(hdmi->hclk_vop); >> + if (ret) { >> + dev_err(hdmi->dev, "Failed to enable HDMI hclk_vop: %d\n", >> + ret); >> + return ret; >> + } >> + >> + if (hdmi->chip_data == &rk3568_chip_data) { >> + regmap_write(hdmi->regmap, RK3568_GRF_VO_CON1, >> + HIWORD_UPDATE(RK3568_HDMI_SDAIN_MSK | >> + RK3568_HDMI_SCLIN_MSK, >> + RK3568_HDMI_SDAIN_MSK | >> + RK3568_HDMI_SCLIN_MSK)); >> + } >> + >> hdmi->phy = devm_phy_optional_get(dev, "hdmi"); >> if (IS_ERR(hdmi->phy)) { >> ret = PTR_ERR(hdmi->phy); >> @@ -559,6 +623,8 @@ static int dw_hdmi_rockchip_bind(struct device >> *dev, struct device *master, >> ret = PTR_ERR(hdmi->hdmi); >> drm_encoder_cleanup(encoder); >> clk_disable_unprepare(hdmi->vpll_clk); >> + clk_disable_unprepare(hdmi->hclk_vio); >> + clk_disable_unprepare(hdmi->hclk_vop); >> } >> return ret; >> @@ -571,6 +637,8 @@ static void dw_hdmi_rockchip_unbind(struct device >> *dev, struct device *master, >> dw_hdmi_unbind(hdmi->hdmi); >> clk_disable_unprepare(hdmi->vpll_clk); >> + clk_disable_unprepare(hdmi->hclk_vio); >> + clk_disable_unprepare(hdmi->hclk_vop); >> } >> static const struct component_ops dw_hdmi_rockchip_ops = { >> > > > >
| |