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: Mon, 25 May 2026 19:32:33 +1000 Message-ID: In-Reply-To: <20260521223756.627311-3-maaz.mombasawala@broadcom.com> References: <20260521223756.627311-1-maaz.mombasawala@broadcom.com> <20260521223756.627311-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 **Purpose:** Add `mode_valid` callback to `drm_mode_config_funcs` to reject= modes with zero vrefresh during `drm_mode_setcrtc`, and add vrefresh check= s to STDU connector mode validation. Fixes IGT test `kms_invalid_mode@overf= low-vrefresh`. **Issues:** 1. **Config `mode_valid` checks `max_primary_mem` but connector checks diff= erent limits:** The new `vmw_kms_config_mode_valid()` validates against `de= v_priv->max_primary_mem`, while the STDU connector's `vmw_stdu_connector_mo= de_valid()` also checks `max_mob_pages` and `max_mob_size` plus STDU-specif= ic width/height limits. This is correct =E2=80=94 the config-level check is= necessarily more permissive since it's display-unit agnostic =E2=80=94 but= it's worth noting that the config-level check could pass modes that the co= nnector level rejects. 2. **`GPU_TILE_SIZE` moved to header =E2=80=94 good refactoring:** Moving t= he `GPU_TILE_SIZE` define from `vmwgfx_stdu.c` to `vmwgfx_kms.h` is the rig= ht thing since it's now used in both files. 3. **Missing Fixes tag:** Unlike patches 1 and 4, this patch has no `Fixes:= ` tag. The cover letter mentions adding Fixes messages, but this patch does= n't have one. If this is a regression fix, it should have one. 4. **Overall approach looks correct:** Using `drm_mode_config_funcs.mode_va= lid` is the correct hook for validating modes during `drm_mode_setcrtc` for= all display types (SOU, LDU, STDU). The vrefresh check with `drm_mode_vref= resh()` returning 0 is a sensible rejection criterion. --- Generated by Claude Code Patch Reviewer