From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752105AbcGMHqM (ORCPT ); Wed, 13 Jul 2016 03:46:12 -0400 Received: from lucky1.263xmail.com ([211.157.147.133]:60176 "EHLO lucky1.263xmail.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750854AbcGMHqC (ORCPT ); Wed, 13 Jul 2016 03:46:02 -0400 X-263anti-spam: KSV:0; X-MAIL-GRAY: 1 X-MAIL-DELIVERY: 0 X-KSVirus-check: 0 X-ABS-CHECKED: 4 X-ADDR-CHECKED: 0 X-RL-SENDER: ykk@rock-chips.com X-FST-TO: linux-arm-kernel@lists.infradead.org X-SENDER-IP: 103.29.142.67 X-LOGIN-NAME: ykk@rock-chips.com X-UNIQUE-TAG: <489f55a49e3b2db573aa8208296f2139> X-ATTACHMENT-NUM: 0 X-DNS-TYPE: 0 Subject: Re: [PATCH v1 5/6] drm/rockchip: dw_hdmi: introduce the VPLL clock setting To: =?UTF-8?Q?Heiko_St=c3=bcbner?= References: <1468235079-29152-1-git-send-email-ykk@rock-chips.com> <1468235149-29625-1-git-send-email-ykk@rock-chips.com> <3636683.EuFECY2EQB@diego> Cc: Mark Yao , Rob Herring , Russell King , Philipp Zabel , Andy Yan , David Airlie , Daniel Vetter , Kumar Gala , Zheng Yang , xhc@rock-chips.com, dri-devel@lists.freedesktop.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-rockchip@lists.infradead.org, linux-arm-kernel@lists.infradead.org From: Yakir Yang Message-ID: <5785F182.5070901@rock-chips.com> Date: Wed, 13 Jul 2016 15:45:06 +0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.8.0 MIME-Version: 1.0 In-Reply-To: <3636683.EuFECY2EQB@diego> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 >> +#include >> #include >> #include >> -#include >> #include >> + >> #include >> #include >> #include >> @@ -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) > > >