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: Move mode_valid logic to per-SoC clock limits Date: Tue, 05 May 2026 11:22:43 +1000 Message-ID: In-Reply-To: <20260429170012.366537-4-prabhakar.mahadev-lad.rj@bp.renesas.com> References: <20260429170012.366537-1-prabhakar.mahadev-lad.rj@bp.renesas.com> <20260429170012.366537-4-prabhakar.mahadev-lad.rj@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 Good refactoring. The old code was: ```c if (renc->output =3D=3D RZG2L_DU_OUTPUT_DPAD0 && mode->clock > 83500) return MODE_CLOCK_HIGH; ``` The new code: ```c const struct rzg2l_du_device_info *info =3D renc->info; if (info->mode_clock_min && mode->clock < info->mode_clock_min) return MODE_CLOCK_LOW; if (info->mode_clock_max && mode->clock > info->mode_clock_max) return MODE_CLOCK_HIGH; ``` **Issue 1 =E2=80=94 Only r9a07g043u gets clock limits; r9a07g044 and r9a09g= 057 are left at 0:** ```c static const struct rzg2l_du_device_info rzg2l_du_r9a07g043u_info =3D { ... + .mode_clock_min =3D 20875, + .mode_clock_max =3D 83500, }; ``` But `rzg2l_du_r9a07g044_info` and `rzg2l_du_r9a09g057_info` have no clock l= imits added, so `.mode_clock_min` and `.mode_clock_max` remain zero. The `0= ` check skips validation when these fields are zero, which means **the 83.5= MHz limit is silently removed for r9a07g044** (RZ/G2L). The old code appli= ed the 83500 cap to all DPAD0 outputs. If RZ/G2L has the same pixel clock c= onstraint (it likely does =E2=80=94 it's the same DU IP), this is a regress= ion. The r9a09g057 (RZ/V2H) only has DSI0 output, so the old code never capped i= t anyway =E2=80=94 no change there. **The r9a07g044 needs `.mode_clock_max =3D 83500` added** to preserve the e= xisting behavior. **Issue 2 =E2=80=94 The output-type check is dropped:** The old code only applied the cap to `RZG2L_DU_OUTPUT_DPAD0`, not `DSI0`. T= he new code applies the clock limits regardless of output type. For r9a07g0= 44 which has both DSI0 and DPAD0, the DSI output may have different clock c= onstraints than DPAD. Moving to per-SoC limits loses the per-output-type gr= anularity. This may be fine if the clock limits truly apply to the DU block= itself rather than the output interface, but the commit message should add= ress this design choice. **Issue 3 =E2=80=94 Storing `info` pointer in encoder:** ```c + renc->info =3D rcdu->info; ``` The encoder could access the device info through `renc->base.dev->dev_priva= te` or similar DRM device traversal instead of storing a redundant pointer.= However, this is a minor style point =E2=80=94 the direct pointer is clean= er for the `mode_valid` hot path and matches patterns seen in other drivers= . Acceptable. --- Generated by Claude Code Patch Reviewer