Re: [PATCH v6 15/21] drm: renesas: rz-du: Add RZ/G3E support
| Newsgroups | org.freedesktop.lists.dri-devel,org.kernel.vger.linux-clk,org.kernel.vger.linux-devicetree |
|---|---|
| Message-ID | <[email protected]> |
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Strict equality check for `rstate->outputs` fails for dual-link or cloned configurations, causing incorrect clock duty cycle. -- --- Patch [15]: [PATCH v6 15/21] drm: renesas: rz-du: Add RZ/G3E support --- commit 93ac2aa593af29d345db6eb285c2df970c9009bd Author: Tommaso Merciai <[email protected]> drm: renesas: rz-du: Add RZ/G3E support This commit adds support for the RZ/G3E SoC to the RZ/G2L DU driver. It introduces the RZG2L_DU_FEATURE_SMUX2_DSI_CLK feature flag to set the correct clock duty cycle based on the selected output (LVDS vs DSI/RGB). > diff --git a/drivers/gpu/drm/renesas/rz-du/rzg2l_du_crtc.c b/drivers/gpu/drm/renesas/rz-du/rzg2l_du_crtc.c > --- a/drivers/gpu/drm/renesas/rz-du/rzg2l_du_crtc.c > +++ b/drivers/gpu/drm/renesas/rz-du/rzg2l_du_crtc.c [ ... ] > @@ -64,11 +64,32 @@ > static void rzg2l_du_crtc_set_display_timing(struct rzg2l_du_crtc *rcrtc) > { [ ... ] > + if (rzg2l_du_has(rcdu, RZG2L_DU_FEATURE_SMUX2_DSI_CLK)) { > + struct clk *clk_parent; > + > + clk_parent = clk_get_parent(rcrtc->rzg2l_clocks.dclk); > + > + /* > + * Request appropriate duty cycle to let clock driver select > + * the correct parent: > + * - CDIV7_DSIx_CLK (LVDS path) has DUTY H/L=4/3, 4/7 duty cycle. > + * - CSDIV_2to16_PLLDSIx (DSI/RGB path) has symmetric 50% duty cycle. > + */ > + if (rstate->outputs == BIT(RZG2L_DU_OUTPUT_LVDS0) || > + rstate->outputs == BIT(RZG2L_DU_OUTPUT_LVDS1)) [Severity: High] Will this strict equality check fail for dual-link LVDS configurations? If a dual-channel LVDS panel is configured, the DRM encoder mask may contain two encoders to drive both links. In that case, rzg2l_du_crtc_atomic_check() would accumulate both bits, meaning rstate->outputs would have both LVDS0 and LVDS1 bits set. Because this relies on strict equality rather than a bitwise mask check, does this condition evaluate to false and incorrectly fall back to the 50% duty cycle intended for DSI/RGB paths? > + clk_set_duty_cycle(clk_parent, 4, 7); > + else > + clk_set_duty_cycle(clk_parent, 1, 2); > + } > + > clk_set_rate(rcrtc->rzg2l_clocks.dclk, mode_clock); -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=15