From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: drm: renesas: rz-du: Add RZ/G3E support Date: Sun, 12 Apr 2026 12:45:59 +1000 Message-ID: In-Reply-To: <11313371ef0b31cb5f014e171ba0d9868eb0710d.1775636898.git.tommaso.merciai.xr@bp.renesas.com> References: <11313371ef0b31cb5f014e171ba0d9868eb0710d.1775636898.git.tommaso.merciai.xr@bp.renesas.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Patch Review Adds the DU driver support for RZ/G3E with SMUX2 clock mux selection via du= ty cycle. **Issue (design - duty cycle at wrong level)**: In `rzg2l_du_crtc_set_displ= ay_timing()`: ```c if (rzg2l_du_has(rcdu, RZG2L_DU_FEATURE_SMUX2_DSI_CLK)) { struct clk *clk_parent; clk_parent =3D clk_get_parent(rcrtc->rzg2l_clocks.dclk); ... if (rstate->outputs =3D=3D BIT(RZG2L_DU_OUTPUT_LVDS0) || rstate->outputs =3D=3D BIT(RZG2L_DU_OUTPUT_LVDS1)) clk_set_duty_cycle(clk_parent, 4, 7); else clk_set_duty_cycle(clk_parent, 1, 2); } ``` The code gets the parent of `dclk` and sets the duty cycle on it. This mean= s the DU driver needs to know about the clock tree internals (that the pare= nt is a SMUX clock that uses duty cycle to select paths). This is a layerin= g violation. If the clock tree ever changes, this DU code would break. Howe= ver, this appears to be a conscious design choice discussed in earlier revi= ew rounds. **The `clk_parent` is not released**: `clk_get_parent()` returns a `struct = clk *`. The kernel's `clk_get_parent()` does not increment the refcount so = no `clk_put` is needed =E2=80=94 this is correct. **Observation**: The `rzg2l_du_crtc_atomic_check()` function iterates over = encoders to determine output routes, matching the rcar DU pattern. This is = clean. **Minor**: The `rzg2l_du_r9a09g047_du_info` only defines `channels_mask =3D= BIT(0)` (single channel). Since the SoC has two DU instances (DU0, DU1), e= ach DU node in DTS will probe as a separate device with its own single chan= nel. This is correct. --- Generated by Claude Code Patch Reviewer