Claude review: drm/vmwgfx: Check vrefresh in drm_mode_setcrtc.

Claude review: drm/vmwgfx: Check vrefresh in drm_mode_setcrtc.

[Claude Code Review Bot]

Patch Review

**Good fix for missing mode_valid callback.** The `drm_mode_config_funcs.mode_valid` callback is invoked from `drm_mode_setcrtc` but was not previously set for vmwgfx, meaning modes that only go through the legacy setcrtc path bypassed validation.

**Duplicated validation logic:** The new `vmw_kms_config_mode_valid()` duplicates a significant portion of the checks from `vmw_stdu_connector_mode_valid()` (size validation, memory checks, vrefresh). The stdu connector path checks additional constraints (stdu_max_width/height, max_mob_pages, max_mob_size) that the new config-level callback does not. This is reasonable since the config-level check applies to all display units (sou, ldu, stdu), but it would be worth a comment noting the relationship between the two validation levels.

**GPU_TILE_SIZE move is correct.** Moving the `#define GPU_TILE_SIZE 64` from `vmwgfx_stdu.c` to `vmwgfx_kms.h` makes sense since it's now used in `vmwgfx_kms.c` as well.

**vrefresh check added to stdu connector callback too** — good for consistency, covering both the atomic (connector mode_valid) and legacy (config mode_valid) paths.

---
Generated by Claude Code Patch Reviewer

Claude review: drm/vmwgfx: Check vrefresh in drm_mode_setcrtc.

[Claude Code Review Bot]

Patch Review

**Good refactoring of GPU_TILE_SIZE.** Moving the `GPU_TILE_SIZE` define and associated logic from `vmwgfx_stdu.c` to the shared `vmwgfx_kms.h`/`vmwgfx_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_mode_valid` to `drm_mode_config_funcs` makes this validation apply during `drm_mode_setcrtc`, which is exactly what the failing IGT test exercises.

**Potential duplicate validation for STDU.** With this patch, STDU connectors will validate vrefresh twice — 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_size` — these are intentionally different limits applied at different 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

Claude review: drm/vmwgfx: Check vrefresh in drm_mode_setcrtc.

[Claude Code Review Bot]

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 checks to STDU connector mode validation. Fixes IGT test `kms_invalid_mode@overflow-vrefresh`.

**Issues:**

1. **Config `mode_valid` checks `max_primary_mem` but connector checks different limits:** The new `vmw_kms_config_mode_valid()` validates against `dev_priv->max_primary_mem`, while the STDU connector's `vmw_stdu_connector_mode_valid()` also checks `max_mob_pages` and `max_mob_size` plus STDU-specific width/height limits. This is correct — the config-level check is necessarily more permissive since it's display-unit agnostic — but it's worth noting that the config-level check could pass modes that the connector level rejects.

2. **`GPU_TILE_SIZE` moved to header — good refactoring:** Moving the `GPU_TILE_SIZE` define from `vmwgfx_stdu.c` to `vmwgfx_kms.h` is the right 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 doesn't have one. If this is a regression fix, it should have one.

4. **Overall approach looks correct:** Using `drm_mode_config_funcs.mode_valid` is the correct hook for validating modes during `drm_mode_setcrtc` for all display types (SOU, LDU, STDU). The vrefresh check with `drm_mode_vrefresh()` returning 0 is a sensible rejection criterion.

---
Generated by Claude Code Patch Reviewer

[PATCH v5 0/4] Fix some issues from igt runs.

[Maaz Mombasawala]

I ran igt-gpu-tools with vmwgfx and fixed some issues I found.

Changes in v5:
- Get display unit from cursor plane, so there is no need for
old_crtc.
- Cast ALIGN to u64.
- Check return value of ttm_bo_reserve().

Changes in v4:
- For ttm ref change, changed to incrementing the refcount on
the ttm base object of the dumb surface instead of holding a
ttm ref object so that it is not exposed to tfile.
- Added Fixes messages.
- Removed BO cleanup patch.

Changes in v3:
- Minor changes so series can apply cleanly on drm-misc-next.

Changes in v2:
- Changes based on Zack's comments.
- For vrefresh changes, removed stdu specific checks so that test
kms_invalid_mode@overflow-vrefresh passes with sou and ldu displays.
For ttm ref changes, fixed a ref leak in ttm_prime_fd_to_handle()
which fixes a memleak in test vmw_prime@tri-map-dmabuf and removed
release callback for dumb buffer surface since the gem buffer will
handle release of the surface.
Added commit to remove duplicate functions in vmwgfx_bo.h.

Maaz Mombasawala (4):
  drm/vmwgfx: Add some checks to vmw_cursor_plane_atomic_update
  drm/vmwgfx: Check vrefresh in drm_mode_setcrtc.
  drm/vmwgfx: Reserve ttm object before resv usage
  drm/vmwgfx: Change ttm refs for dumb buffers.

 drivers/gpu/drm/vmwgfx/ttm_object.c          |  7 +++--
 drivers/gpu/drm/vmwgfx/vmwgfx_bo.c           |  8 +++++-
 drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c | 16 +++++++----
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.h          |  1 +
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c          | 30 ++++++++++++++++++++
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.h          |  9 ++++++
 drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c         | 12 ++------
 drivers/gpu/drm/vmwgfx/vmwgfx_surface.c      | 14 ++++++++-
 drivers/gpu/drm/vmwgfx/vmwgfx_vkms.c         |  8 ++++++
 9 files changed, 86 insertions(+), 19 deletions(-)

-- 
2.54.0

[PATCH v5 1/4] drm/vmwgfx: Add some checks to vmw_cursor_plane_atomic_update

[Maaz Mombasawala]

In cases where old_state may not be present we hit a null dereference.
Get display unit from cursor plane instead to fix this and also do an
early return if there is no new_state since there is nothing to do in
that case.
This fixes igt test kms_cursor_legacy@torture-bo.

Fixes: 965544150d1c ("drm/vmwgfx: Refactor cursor handling")
Cc: Zack Rusin <zack.rusin@broadcom.com>
Cc: Broadcom internal kernel review list <bcm-kernel-feedback-list@broadcom.com>
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Maaz Mombasawala <maaz.mombasawala@broadcom.com>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c b/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c
index b010fc7ca68e..35e3fd2bbdad 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c
@@ -22,6 +22,9 @@
 #define VMW_CURSOR_SNOOP_WIDTH 64
 #define VMW_CURSOR_SNOOP_HEIGHT 64
 
+#define vmw_cursor_to_du(cursor_plane) \
+	container_of(cursor_plane, struct vmw_display_unit, cursor.base)
+
 struct vmw_svga_fifo_cmd_define_cursor {
 	u32 cmd;
 	SVGAFifoCmdDefineAlphaCursor cursor;
@@ -742,13 +745,14 @@ vmw_cursor_plane_atomic_update(struct drm_plane *plane,
 			       struct drm_atomic_commit *state)
 {
 	struct vmw_bo *bo;
-	struct drm_plane_state *new_state =
-		drm_atomic_get_new_plane_state(state, plane);
-	struct drm_plane_state *old_state =
-		drm_atomic_get_old_plane_state(state, plane);
-	struct drm_crtc *crtc = new_state->crtc ?: old_state->crtc;
+	struct drm_plane_state *new_state;
+
+	new_state = drm_atomic_get_new_plane_state(state, plane);
+	if (!new_state)
+		return;
+
 	struct vmw_private *dev_priv = vmw_priv(plane->dev);
-	struct vmw_display_unit *du = vmw_crtc_to_du(crtc);
+	struct vmw_display_unit *du = vmw_cursor_to_du(plane);
 	struct vmw_plane_state *vps = vmw_plane_state_to_vps(new_state);
 	s32 hotspot_x, hotspot_y, cursor_x, cursor_y;
 
-- 
2.54.0

[PATCH v5 2/4] drm/vmwgfx: Check vrefresh in drm_mode_setcrtc.

[Maaz Mombasawala]

Add the mode_valid() callback in drm_mode_config_funcs to check for valid
mode during drm_mode_setcrtc.
Both the mode_valid() callbacks also check if the vrefresh value is
correct.
This fixes igt test kms_invalid_mode@overflow-vrefresh.

Signed-off-by: Maaz Mombasawala <maaz.mombasawala@broadcom.com>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c  | 30 ++++++++++++++++++++++++++++
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.h  |  9 +++++++++
 drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 12 +++--------
 3 files changed, 42 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index a170e38c5223..11edb422d2ba 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -1063,8 +1063,38 @@ vmw_kms_atomic_check_modeset(struct drm_device *dev,
 	return ret;
 }
 
+static enum drm_mode_status
+vmw_kms_config_mode_valid(struct drm_device *dev,
+			  const struct drm_display_mode *mode)
+{
+	enum drm_mode_status ret;
+	struct vmw_private *dev_priv = vmw_priv(dev);
+	u64 assumed_cpp = dev_priv->assume_16bpp ? 2 : 4;
+	/* Align width and height to account for GPU tile over-alignment */
+	u64 required_mem = 0;
+
+	ret = drm_mode_validate_size(mode, dev_priv->texture_max_width,
+				     dev_priv->texture_max_height);
+	if (ret != MODE_OK)
+		return ret;
+
+	required_mem = (u64)ALIGN(mode->hdisplay, GPU_TILE_SIZE) *
+		       ALIGN(mode->vdisplay, GPU_TILE_SIZE) *
+		       assumed_cpp;
+	required_mem = ALIGN(required_mem, PAGE_SIZE);
+
+	if (required_mem > dev_priv->max_primary_mem)
+		return MODE_MEM;
+
+	if (!drm_mode_vrefresh(mode))
+		return MODE_BAD;
+
+	return MODE_OK;
+}
+
 static const struct drm_mode_config_funcs vmw_kms_funcs = {
 	.fb_create = vmw_kms_fb_create,
+	.mode_valid = vmw_kms_config_mode_valid,
 	.atomic_check = vmw_kms_atomic_check_modeset,
 	.atomic_commit = drm_atomic_helper_commit,
 };
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
index 2224d7d91d1b..1b717caecc78 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
@@ -16,6 +16,15 @@
 #include <drm/drm_framebuffer.h>
 #include <drm/drm_probe_helper.h>
 
+/*
+ * Some renderers such as llvmpipe will align the width and height of their
+ * buffers to match their tile size. We need to keep this in mind when exposing
+ * modes to userspace so that this possible over-allocation will not exceed
+ * graphics memory. 64x64 pixels seems to be a reasonable upper bound for the
+ * tile size of current renderers.
+ */
+#define GPU_TILE_SIZE 64
+
 /**
  * struct vmw_du_update_plane - Closure structure for vmw_du_helper_plane_update
  * @plane: Plane which is being updated.
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
index 4139837f4caf..04c5c20588d8 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
@@ -45,15 +45,6 @@
 #define vmw_connector_to_stdu(x) \
 	container_of(x, struct vmw_screen_target_display_unit, base.connector)
 
-/*
- * Some renderers such as llvmpipe will align the width and height of their
- * buffers to match their tile size. We need to keep this in mind when exposing
- * modes to userspace so that this possible over-allocation will not exceed
- * graphics memory. 64x64 pixels seems to be a reasonable upper bound for the
- * tile size of current renderers.
- */
-#define GPU_TILE_SIZE 64
-
 enum stdu_content_type {
 	SAME_AS_DISPLAY = 0,
 	SEPARATE_SURFACE,
@@ -870,6 +861,9 @@ vmw_stdu_connector_mode_valid(struct drm_connector *connector,
 	if (required_mem > dev_priv->max_mob_size)
 		return MODE_MEM;
 
+	if (!drm_mode_vrefresh(mode))
+		return MODE_BAD;
+
 	return MODE_OK;
 }
 
-- 
2.54.0

[PATCH v5 3/4] drm/vmwgfx: Reserve ttm object before resv usage

[Maaz Mombasawala]

We hit dma_resv_assert_held warnings in several places, reserve the ttm
base object when it's being used to avoid them.

Signed-off-by: Maaz Mombasawala <maaz.mombasawala@broadcom.com>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_bo.c   | 6 ++++++
 drivers/gpu/drm/vmwgfx/vmwgfx_vkms.c | 8 ++++++++
 2 files changed, 14 insertions(+)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
index 1264a9ac4241..a69a6764ead2 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
@@ -52,10 +52,16 @@ static void vmw_bo_free(struct ttm_buffer_object *bo)
 		WARN_ON(vbo != res->guest_memory_bo);
 		WARN_ON(!res->guest_memory_bo);
 		if (res->guest_memory_bo) {
+			int ret = 0;
 			/* Reserve and switch the backing mob. */
 			mutex_lock(&res->dev_priv->cmdbuf_mutex);
 			(void)vmw_resource_reserve(res, false, true);
+			ret = ttm_bo_reserve(bo, false, false, NULL);
+			if (ret != 0)
+				drm_warn(&res->dev_priv->drm,
+					 "%s: failed reserve\n", __func__);
 			vmw_resource_mob_detach(res);
+			ttm_bo_unreserve(bo);
 			if (res->dirty)
 				res->func->dirty_free(res);
 			if (res->coherent)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_vkms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_vkms.c
index 7b8163b5e501..d0c371df7e4c 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_vkms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_vkms.c
@@ -131,6 +131,7 @@ crc_generate_worker(struct work_struct *work)
 	spin_unlock_irq(&du->vkms.crc_state_lock);
 
 	if (surf) {
+		int ret;
 		if (vmw_surface_sync(vmw, surf)) {
 			drm_warn(
 				crtc->dev,
@@ -138,7 +139,14 @@ crc_generate_worker(struct work_struct *work)
 			return;
 		}
 
+		ret = ttm_bo_reserve(&surf->res.guest_memory_bo->tbo, false, false, NULL);
+		if (ret != 0) {
+			drm_warn(&vmw->drm, "%s: failed reserve\n", __func__);
+			goto done;
+		}
 		compute_crc(crtc, surf, &crc32);
+		ttm_bo_unreserve(&surf->res.guest_memory_bo->tbo);
+done:
 		vmw_surface_unreference(&surf);
 	}
 
-- 
2.54.0

[PATCH v5 4/4] drm/vmwgfx: Change ttm refs for dumb buffers.

[Maaz Mombasawala]

Preserve a ref to surface during dumb buffer creation. This keeps the dumb
buffer valid for framebuffer usage and fixes all igt tests that use dumb
buffers.
Also fix ttm_prime_fd_to_handle(), which in the error case was leaking a
dma_buf reference. During vmw_prime_fd_to_handle() this function
is expected to fail for dumb buffers since the fd is for a gem object,
the dma_buf would in turn hold a reference to the dumb buffer gem object
and cause a memory leak.

Fixes: f42c09e614f1 ("drm/vmwgfx: Fix dumb buffer leak")
Cc: Ian Forbes <ian.forbes@broadcom.com>
Cc: Zack Rusin <zack.rusin@broadcom.com>
Cc: Broadcom internal kernel review list <bcm-kernel-feedback-list@broadcom.com>
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Maaz Mombasawala <maaz.mombasawala@broadcom.com>
---
 drivers/gpu/drm/vmwgfx/ttm_object.c     |  7 +++++--
 drivers/gpu/drm/vmwgfx/vmwgfx_bo.c      |  2 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.h     |  1 +
 drivers/gpu/drm/vmwgfx/vmwgfx_surface.c | 14 +++++++++++++-
 4 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/ttm_object.c b/drivers/gpu/drm/vmwgfx/ttm_object.c
index 2421b0dd057c..f9042bafdc93 100644
--- a/drivers/gpu/drm/vmwgfx/ttm_object.c
+++ b/drivers/gpu/drm/vmwgfx/ttm_object.c
@@ -547,14 +547,17 @@ int ttm_prime_fd_to_handle(struct ttm_object_file *tfile,
 	if (IS_ERR(dma_buf))
 		return PTR_ERR(dma_buf);
 
-	if (dma_buf->ops != &tdev->ops)
-		return -ENOSYS;
+	if (dma_buf->ops != &tdev->ops) {
+		ret = -ENOSYS;
+		goto out;
+	}
 
 	prime = (struct ttm_prime_object *) dma_buf->priv;
 	base = &prime->base;
 	*handle = base->handle;
 	ret = ttm_ref_object_add(tfile, base, NULL, false);
 
+out:
 	dma_buf_put(dma_buf);
 
 	return ret;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
index a69a6764ead2..3de255a619a8 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
@@ -72,7 +72,7 @@ static void vmw_bo_free(struct ttm_buffer_object *bo)
 					       0);
 			mutex_unlock(&res->dev_priv->cmdbuf_mutex);
 		}
-		vmw_surface_unreference(&vbo->dumb_surface);
+		vmw_dumb_surface_unref(&vbo->dumb_surface);
 	}
 	WARN_ON(!RB_EMPTY_ROOT(&vbo->res_tree));
 	drm_gem_object_release(&vbo->tbo.base);
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
index 812f1224f83e..cd46d3995ade 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
@@ -1185,6 +1185,7 @@ u32 vmw_lookup_surface_handle_for_buffer(struct vmw_private *vmw,
 int vmw_dumb_create(struct drm_file *file_priv,
 		    struct drm_device *dev,
 		    struct drm_mode_create_dumb *args);
+void vmw_dumb_surface_unref(struct vmw_surface **dumb_surface);
 
 /*
  * Shader management - vmwgfx_shader.c
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
index ab611943d774..5a9c953eb73c 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
@@ -2328,11 +2328,23 @@ int vmw_dumb_create(struct drm_file *file_priv,
 	struct vmw_user_surface *usurf = container_of(vbo->dumb_surface,
 						struct vmw_user_surface, srf);
 	usurf->prime.base.refcount_release = NULL;
+
+	ttm_base_object_lookup_for_ref(dev_priv->tdev, arg.rep.handle);
+
 err:
 	if (res)
 		vmw_resource_unreference(&res);
-
 	ttm_ref_object_base_unref(tfile, arg.rep.handle);
 
 	return ret;
 }
+
+void vmw_dumb_surface_unref(struct vmw_surface **dumb_surface)
+{
+	struct vmw_user_surface *usurf = container_of(*dumb_surface,
+						struct vmw_user_surface, srf);
+	struct ttm_base_object *base = &usurf->prime.base;
+
+	ttm_base_object_unref(&base);
+	vmw_surface_unreference(dumb_surface);
+}
-- 
2.54.0

Claude review: Fix some issues from igt runs.

[Claude Code Review Bot]

Overall Series Review

Subject: Fix some issues from igt runs.
Author: Maaz Mombasawala <maaz.mombasawala@broadcom.com>
Patches: 5
Reviewed: 2026-06-04T13:30:06.941577

---

This is a 4-patch series from Maaz Mombasawala (Broadcom/VMware) fixing issues found by running igt-gpu-tools against the vmwgfx driver, now at v5. The series addresses a null deref in cursor updates, missing vrefresh validation, missing TTM reservation locks, and dumb buffer lifetime management.

**Patches 1 and 2** are clean and correct. **Patch 3** has a problematic error handling path in `vmw_bo_free` where a failed `ttm_bo_reserve` will still proceed to call `vmw_resource_mob_detach` (which asserts the lock is held) and `ttm_bo_unreserve` (on an unreserved BO). **Patch 4** has the most significant issue: the return value of `ttm_base_object_lookup_for_ref()` is not checked, which can lead to a refcount underflow in `vmw_dumb_surface_unref()` at cleanup time if the lookup fails. Additionally, Patch 3 is missing a Fixes tag even though it addresses specific dma_resv_assert_held warnings.

---

---
Generated by Claude Code Patch Reviewer

Claude review: drm/vmwgfx: Add some checks to vmw_cursor_plane_atomic_update

[Claude Code Review Bot]

Patch Review

**Status: Looks good, minor style nit**

The fix correctly addresses a null dereference when `old_state` is not present. The root cause is that the old code did:
```c
struct drm_crtc *crtc = new_state->crtc ?: old_state->crtc;
struct vmw_display_unit *du = vmw_crtc_to_du(crtc);
```
...which crashes if both are NULL or `old_state` itself is NULL.

The new approach of getting `du` via `container_of(plane, struct vmw_display_unit, cursor.base)` is correct — the cursor plane *is* `du->cursor.base`, so `container_of` is valid and avoids the dependency on any plane state. The early return on `!new_state` is also appropriate since there's nothing to update.

**Minor style nit**: The declarations of `dev_priv` and `du` appear after the `if (!new_state) return;` statement:
```c
new_state = drm_atomic_get_new_plane_state(state, plane);
if (!new_state)
    return;

struct vmw_private *dev_priv = vmw_priv(plane->dev);
struct vmw_display_unit *du = vmw_cursor_to_du(plane);
```
While this is valid C11 (which the kernel uses), it's a coding style preference issue. Some reviewers prefer all declarations before statements within a block. Not a correctness issue.

---

---
Generated by Claude Code Patch Reviewer

Claude review: drm/vmwgfx: Check vrefresh in drm_mode_setcrtc.

[Claude Code Review Bot]

Patch Review

**Status: Looks good, but missing Fixes/Cc stable tags**

The patch adds `drm_mode_config_funcs.mode_valid` to validate modes set via the legacy `drm_mode_setcrtc` path, which the existing per-connector `mode_valid` doesn't cover. The vrefresh check (`if (!drm_mode_vrefresh(mode)) return MODE_BAD`) is also added to the STDU connector's `vmw_stdu_connector_mode_valid`.

Good decisions in this patch:
- Moving `GPU_TILE_SIZE` from `vmwgfx_stdu.c` to `vmwgfx_kms.h` for reuse.
- Casting the `ALIGN` result to `u64` to avoid 32-bit overflow in `required_mem` calculation:
  ```c
  required_mem = (u64)ALIGN(mode->hdisplay, GPU_TILE_SIZE) *
                 ALIGN(mode->vdisplay, GPU_TILE_SIZE) *
                 assumed_cpp;
  ```

**Missing tags**: This patch fixes `kms_invalid_mode@overflow-vrefresh` but has no `Fixes:` tag or `Cc: stable@` annotation. If this is a regression from a specific commit, a Fixes tag would help with backporting. Even without a clear single culprit, a Cc: stable would be appropriate if this matters for older kernels.

---

---
Generated by Claude Code Patch Reviewer

Claude review: drm/vmwgfx: Reserve ttm object before resv usage

[Claude Code Review Bot]

Patch Review

**Status: Has issues in error handling**

The intent is correct — `vmw_resource_mob_detach()` calls `dma_resv_assert_held()`, so the buffer object's reservation must be held.

**Issue in `vmw_bo_free` (vmwgfx_bo.c)**: If `ttm_bo_reserve()` fails, the code warns but continues to call both `vmw_resource_mob_detach(res)` and `ttm_bo_unreserve(bo)`:
```c
ret = ttm_bo_reserve(bo, false, false, NULL);
if (ret != 0)
    drm_warn(&res->dev_priv->drm,
             "%s: failed reserve\n", __func__);
vmw_resource_mob_detach(res);   /* asserts lock held — will fire */
ttm_bo_unreserve(bo);           /* unreserves without holding reservation */
```

If the reserve fails:
1. `vmw_resource_mob_detach` will hit the `dma_resv_assert_held` assertion — the very thing this patch is meant to fix.
2. `ttm_bo_unreserve` on a non-reserved BO is undefined behavior (likely corrupts the ww_mutex state).

In practice, `ttm_bo_reserve(bo, false, false, NULL)` (non-interruptible, blocking) should essentially never fail, but since v5 explicitly added this check per review feedback, the error path should be correct. A simple approach would be to gate the unreserve:
```c
ret = ttm_bo_reserve(bo, false, false, NULL);
if (ret != 0)
    drm_warn(...);
vmw_resource_mob_detach(res);
if (ret == 0)
    ttm_bo_unreserve(bo);
```
Or skip the detach on failure if that's safe for the destroy path.

**The `crc_generate_worker` change (vmwgfx_vkms.c)** looks correct — the `goto done` properly skips `compute_crc` and falls through to `vmw_surface_unreference(&surf)`.

**Missing tags**: This patch is also missing `Fixes:` and `Cc: stable` tags despite fixing lockdep/dma_resv assertion warnings.

---

---
Generated by Claude Code Patch Reviewer

Claude review: drm/vmwgfx: Change ttm refs for dumb buffers.

[Claude Code Review Bot]

Patch Review

**Status: Has a significant unchecked return value**

**Critical issue — unchecked return value (vmwgfx_surface.c:2332)**:
```c
ttm_base_object_lookup_for_ref(dev_priv->tdev, arg.rep.handle);
```
`ttm_base_object_lookup_for_ref()` returns `struct ttm_base_object *` (or NULL on failure), but the return value is completely discarded. This function is called to acquire an extra reference on the base object that will later be dropped by `vmw_dumb_surface_unref → ttm_base_object_unref`. If the lookup fails and no reference is acquired, the subsequent `ttm_base_object_unref` in `vmw_dumb_surface_unref` will decrement a refcount that was never incremented, causing a refcount underflow and potential use-after-free.

The fix should check the return value:
```c
if (!ttm_base_object_lookup_for_ref(dev_priv->tdev, arg.rep.handle)) {
    ret = -EINVAL;
    goto err;
}
```

**The dma_buf leak fix in `ttm_prime_fd_to_handle` (ttm_object.c)** is correct and straightforward — the early return was missing a `dma_buf_put()`:
```c
// Before: leaked dma_buf ref on !tdev->ops
if (dma_buf->ops != &tdev->ops)
    return -ENOSYS;

// After: properly releases via goto
if (dma_buf->ops != &tdev->ops) {
    ret = -ENOSYS;
    goto out;
}
```

**The `vmw_dumb_surface_unref` function**: Setting `refcount_release = NULL` is safe because `ttm_release_base()` (ttm_object.c:241) checks for NULL before calling it. The function correctly pairs `ttm_base_object_unref` (drops the base object ref from `lookup_for_ref`) with `vmw_surface_unreference` (drops the resource ref). The ordering is safe since the base object destruction (with NULL release callback) only does IDR cleanup and tfile unref, not memory deallocation.

---
Generated by Claude Code Patch Reviewer