| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <ff1c398c-5140-2cf0-a4aa-b9cf7e0a68d3@rock-chips.com>
Date: Wed, 14 Jul 2021 09:26:28 +0800
From: Andy Yan <andy.yan@...k-chips.com>
To: Alex Bee <knaerzche@...il.com>,
Benjamin Gaignard <benjamin.gaignard@...labora.com>,
hjc@...k-chips.com, heiko@...ech.de, airlied@...ux.ie,
daniel@...ll.ch, robh+dt@...nel.org, algea.cao@...k-chips.com
Cc: dri-devel@...ts.freedesktop.org, devicetree@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org,
linux-rockchip@...ts.infradead.org, linux-kernel@...r.kernel.org,
kernel@...labora.com
Subject: Re: [PATCH v2 2/2] drm/rockchip: dw_hdmi: add rk3568 support
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@...labora.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 = {
>>
>
>
>
>
Powered by blists - more mailing lists