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 10:17:49 +1000 Message-ID: In-Reply-To: <20260514224819.3631763-3-maaz.mombasawala@broadcom.com> References: <20260514224819.3631763-1-maaz.mombasawala@broadcom.com> <20260514224819.3631763-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 fix for missing mode_valid callback.** The `drm_mode_config_funcs.mo= de_valid` callback is invoked from `drm_mode_setcrtc` but was not previousl= y set for vmwgfx, meaning modes that only go through the legacy setcrtc pat= h bypassed validation. **Duplicated validation logic:** The new `vmw_kms_config_mode_valid()` dupl= icates a significant portion of the checks from `vmw_stdu_connector_mode_va= lid()` (size validation, memory checks, vrefresh). The stdu connector path = checks additional constraints (stdu_max_width/height, max_mob_pages, max_mo= b_size) that the new config-level callback does not. This is reasonable sin= ce the config-level check applies to all display units (sou, ldu, stdu), bu= t it would be worth a comment noting the relationship between the two valid= ation levels. **GPU_TILE_SIZE move is correct.** Moving the `#define GPU_TILE_SIZE 64` fr= om `vmwgfx_stdu.c` to `vmwgfx_kms.h` makes sense since it's now used in `vm= wgfx_kms.c` as well. **vrefresh check added to stdu connector callback too** =E2=80=94 good for = consistency, covering both the atomic (connector mode_valid) and legacy (co= nfig mode_valid) paths. --- Generated by Claude Code Patch Reviewer