| 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: <CAPj87rNX0vdQquZB7HQDG1rvCCyk=+2wa=isLqgL3_Sx6Y1J=Q@mail.gmail.com>
Date: Fri, 29 Aug 2025 17:35:59 +0200
From: Daniel Stone <daniel@...ishbar.org>
To: Cristian Ciocaltea <cristian.ciocaltea@...labora.com>
Cc: Sandy Huang <hjc@...k-chips.com>, Heiko Stübner <heiko@...ech.de>,
Andy Yan <andy.yan@...k-chips.com>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, Maxime Ripard <mripard@...nel.org>,
Thomas Zimmermann <tzimmermann@...e.de>, David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
Andrzej Hajda <andrzej.hajda@...el.com>, Neil Armstrong <neil.armstrong@...aro.org>,
Robert Foss <rfoss@...nel.org>, Laurent Pinchart <Laurent.pinchart@...asonboard.com>,
Jonas Karlman <jonas@...boo.se>, Jernej Skrabec <jernej.skrabec@...il.com>, kernel@...labora.com,
dri-devel@...ts.freedesktop.org, linux-arm-kernel@...ts.infradead.org,
linux-rockchip@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/5] drm/rockchip: vop2: Check bpc before switching
DCLK source
Hi Cristian,
On Mon, 25 Aug 2025 at 12:08, Cristian Ciocaltea
<cristian.ciocaltea@...labora.com> wrote:
> When making use of the HDMI PHY PLL as a VOP2 DCLK source, it's output
> rate does normally match the mode clock. But this is only applicable
> for default color depth of 8 bpc. For higher depths, the output clock
> is further divided by the hardware according to the formula:
>
> output rate = PHY PLL rate * 8 / bpc
Observing that this results in phy_pll_rate * 8 / 8 == phy_pll_rate
for 8bpc, the formula does actually hold true everywhere.
> @@ -1737,36 +1737,48 @@ static void vop2_crtc_atomic_enable(struct drm_crtc *crtc,
> * Switch to HDMI PHY PLL as DCLK source for display modes up
> * to 4K@...z, if available, otherwise keep using the system CRU.
> */
> - if ((vop2->pll_hdmiphy0 || vop2->pll_hdmiphy1) && clock <= VOP2_MAX_DCLK_RATE) {
> - drm_for_each_encoder_mask(encoder, crtc->dev, crtc_state->encoder_mask) {
> - struct rockchip_encoder *rkencoder = to_rockchip_encoder(encoder);
> + if (vop2->pll_hdmiphy0 || vop2->pll_hdmiphy1) {
> + unsigned long max_dclk;
>
> - if (rkencoder->crtc_endpoint_id == ROCKCHIP_VOP2_EP_HDMI0) {
> - if (!vop2->pll_hdmiphy0)
> - break;
> + if (vcstate->output_bpc > 8)
> + max_dclk = DIV_ROUND_CLOSEST_ULL(VOP2_MAX_DCLK_RATE * 8,
> + vcstate->output_bpc);
> + else
> + max_dclk = VOP2_MAX_DCLK_RATE;
... so this could just be do the mul+div unconditionally.
> + if (clock <= max_dclk) {
> + drm_for_each_encoder_mask(encoder, crtc->dev, crtc_state->encoder_mask) {
> + struct rockchip_encoder *rkencoder = to_rockchip_encoder(encoder);
>
> - ret = clk_set_parent(vp->dclk, vop2->pll_hdmiphy0);
> - if (ret < 0)
> - drm_warn(vop2->drm,
> - "Could not switch to HDMI0 PHY PLL: %d\n", ret);
> - break;
> - }
> + if (rkencoder->crtc_endpoint_id == ROCKCHIP_VOP2_EP_HDMI0) {
> + if (!vop2->pll_hdmiphy0)
> + break;
> +
> + if (!vp->dclk_src)
> + vp->dclk_src = clk_get_parent(vp->dclk);
>
> - if (rkencoder->crtc_endpoint_id == ROCKCHIP_VOP2_EP_HDMI1) {
> - if (!vop2->pll_hdmiphy1)
> + ret = clk_set_parent(vp->dclk, vop2->pll_hdmiphy0);
> + if (ret < 0)
> + drm_warn(vop2->drm,
> + "Could not switch to HDMI0 PHY PLL: %d\n",
> + ret);
> break;
> + }
>
> - if (!vp->dclk_src)
> - vp->dclk_src = clk_get_parent(vp->dclk);
> + if (rkencoder->crtc_endpoint_id == ROCKCHIP_VOP2_EP_HDMI1) {
> + if (!vop2->pll_hdmiphy1)
> + break;
>
> - ret = clk_set_parent(vp->dclk, vop2->pll_hdmiphy1);
> - if (ret < 0)
> - drm_warn(vop2->drm,
> - "Could not switch to HDMI1 PHY PLL: %d\n", ret);
> - break;
> + if (!vp->dclk_src)
> + vp->dclk_src = clk_get_parent(vp->dclk);
> +
> + ret = clk_set_parent(vp->dclk, vop2->pll_hdmiphy1);
> + if (ret < 0)
> + drm_warn(vop2->drm,
> + "Could not switch to HDMI1 PHY PLL: %d\n",
> + ret);
> + break;
> + }
To be honest, this whole thing is a bit weird, and seems like it could
also be struct clk *new_dclk_parent = (rkencoder->crtc_endpoint_id ==
ROCKCHIP_VOP2_EP_HDMI0) ? vop2->pll_hdmiphy0 : vop2->pll_hdmiphy1? But
it's not your code, I know, and the rest of the clock handling is
pretty messy, so I think this is fine as is.
Reviewed-by: Daniel Stone <daniels@...labora.com>
Cheers,
Daniel
Powered by blists - more mailing lists