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/vmwgfx: Check vrefresh in drm_mode_setcrtc. Date: Sat, 16 May 2026 14:26:31 +1000 Message-ID: In-Reply-To: <20260512002752.2826685-3-maaz.mombasawala@broadcom.com> References: <20260512002752.2826685-1-maaz.mombasawala@broadcom.com> <20260512002752.2826685-3-maaz.mombasawala@broadcom.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 of GPU_TILE_SIZE.** Moving the `GPU_TILE_SIZE` define an= d associated logic from `vmwgfx_stdu.c` to the shared `vmwgfx_kms.h`/`vmwgf= x_kms.c` is the right approach since the mode validation now applies to all= display unit types, not just STDU. **The `mode_valid` callback is the correct hook.** Adding `vmw_kms_config_m= ode_valid` to `drm_mode_config_funcs` makes this validation apply during `d= rm_mode_setcrtc`, which is exactly what the failing IGT test exercises. **Potential duplicate validation for STDU.** With this patch, STDU connecto= rs will validate vrefresh twice =E2=80=94 once in `vmw_stdu_connector_mode_= valid` and once in the new `vmw_kms_config_mode_valid`. The duplication is = harmless but worth noting. Also, the config-level `mode_valid` checks `dev_= priv->max_primary_mem` while the STDU connector checks `dev_priv->max_mob_s= ize` =E2=80=94 these are intentionally different limits applied at differen= t levels, which is correct. **The vrefresh check is correct:** ```c if (!drm_mode_vrefresh(mode)) return MODE_BAD; ``` This rejects modes where the calculated vrefresh is zero, which handles the= overflow case the IGT test checks. --- Generated by Claude Code Patch Reviewer