Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues: - [High] Sleeping functions are called from CRTC atomic enable and disable callbacks, which execute in atomic context. - [High] 32-bit integer overflow in MIPI D-PHY target clock calculation before 64-bit promotion. - [High] 32-bit integer overflow in HDMI PHY link rate calculation. -- commit 5fecee5ab858bfe428aa0c5173e40c8dd13919cc Author: Cristian Ciocaltea <[email protected]> drm/rockchip: vop2: Fix VOP2_MAX_DCLK_RATE overflow on 32-bit This commit fixes a 32-bit integer overflow when calculating the maximum DCLK for the HDMI PHY PLL by redefining VOP2_MAX_DCLK_RATE as an unsigned long long. This ensures the value is evaluated in 64-bit arithmetic, preventing high-resolution modes from being wrongly rejected. Closes: https://lore.kernel.org/all/[email protected]/ > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c > b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c > index e966c0a0266ea..0799eb0dd29fc 100644 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c > @@ -102,7 +102,7 @@ enum vop2_afbc_format { > VOP2_AFBC_FMT_INVALID = -1, > }; > > -#define VOP2_MAX_DCLK_RATE 600000000UL > +#define VOP2_MAX_DCLK_RATE 600000000ULL > > /* > * bus-format types. [Severity: High] This is a pre-existing issue, but do vop2_crtc_atomic_enable() and vop2_crtc_atomic_disable() call sleeping functions from atomic context? Looking at drivers/gpu/drm/rockchip/rockchip_drm_vop2.c: vop2_crtc_atomic_enable() { ... vop2_lock(vop2); ... } And: vop2_crtc_atomic_disable() { ... vop2_lock(vop2); ... } According to the DRM display hardware programming guidelines, CRTC atomic enable and disable callbacks run in atomic context during non-blocking commits, where sleeping is strictly forbidden. However, vop2_crtc_atomic_enable() calls vop2_lock() (which takes a mutex), clk_prepare_enable(), and vop2_enable(). Similarly, vop2_crtc_atomic_disable() calls vop2_lock(), wait_for_completion_timeout(), clk_disable_unprepare(), and vop2_disable(). Can this cause kernel warnings, system instability, and potential deadlocks? [Severity: High] This isn't a bug introduced by this patch, but does the MIPI D-PHY target clock calculation in dw_mipi_dsi2_get_lane_mbps() overflow on 32-bit architectures? Looking at drivers/gpu/drm/rockchip/dw-mipi-dsi2-rockchip.c: dw_mipi_dsi2_get_lane_mbps() { ... target_phyclk = DIV_ROUND_CLOSEST_ULL(lane_rate_kbps * lanes * 1000, bpp); ... } The multiplication lane_rate_kbps * lanes * 1000 is evaluated in 32-bit arithmetic before DIV_ROUND_CLOSEST_ULL() can promote it to 64-bit. For high-resolution outputs, this could exceed the 32-bit unsigned maximum and wrap around. Does this result in an incorrectly configured PHY clock? [Severity: High] This is a pre-existing issue, but does the HDMI PHY link rate calculation in rk_hdptx_phy_clk_set_rate() also suffer from a 32-bit integer overflow? Looking at drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c: rk_hdptx_phy_clk_set_rate() { ... link_rate = DIV_ROUND_CLOSEST_ULL(rate * hdptx->hdmi_cfg.bpc, 8); ... } The expression rate * hdptx->hdmi_cfg.bpc is evaluated as an unsigned long. On 32-bit architectures, this is a 32-bit operation. When rate is large (e.g., 594,000,000 for 4K@60Hz) and bpc is 8, the product exceeds the 32-bit unsigned limit, causing an overflow before promotion to 64-bit. Could this yield an invalid HDMI PHY link rate and break display output on 32-bit architectures? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=5
