[PATCH v2 0/5] Fix some issues from igt runs.

[PATCH v2 0/5] Fix some issues from igt runs.

[Maaz Mombasawala]

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

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 (5):
  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.
  drm/vmwgfx: Remove duplicates in vmwgfx_bo.h

 drivers/gpu/drm/vmwgfx/ttm_object.c          |  7 +++--
 drivers/gpu/drm/vmwgfx/vmwgfx_bo.c           |  6 ++--
 drivers/gpu/drm/vmwgfx/vmwgfx_bo.h           | 15 ---------
 drivers/gpu/drm/vmwgfx/vmwgfx_cotable.c      |  4 +--
 drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c | 16 +++++++---
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.h          |  1 -
 drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c      |  4 +--
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c          | 32 ++++++++++++++++++--
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.h          |  9 ++++++
 drivers/gpu/drm/vmwgfx/vmwgfx_overlay.c      |  2 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_prime.c        |  2 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_resource.c     | 12 ++++----
 drivers/gpu/drm/vmwgfx/vmwgfx_shader.c       |  4 +--
 drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c         | 12 ++------
 drivers/gpu/drm/vmwgfx/vmwgfx_surface.c      | 11 ++++---
 drivers/gpu/drm/vmwgfx/vmwgfx_vkms.c         | 12 ++++++++
 16 files changed, 94 insertions(+), 55 deletions(-)

-- 
2.54.0

[PATCH v2 1/5] 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.
Add a check to fix this and also do an early return if there is no
new_state or crtc since there is nothing to do in that case.
This fixes igt test kms_cursor_legacy@torture-bo.

Signed-off-by: Maaz Mombasawala <maaz.mombasawala@broadcom.com>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c b/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c
index c46f17ba7236..cb7c4d33c1b1 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c
@@ -742,11 +742,17 @@ vmw_cursor_plane_atomic_update(struct drm_plane *plane,
 			       struct drm_atomic_state *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;
+	struct drm_plane_state *old_state;
+
+	new_state = drm_atomic_get_new_plane_state(state, plane);
+	if (!new_state)
+		return;
+	old_state = drm_atomic_get_old_plane_state(state, plane);
+	struct drm_crtc *crtc = new_state->crtc ? new_state->crtc :
+		(old_state ? old_state->crtc : NULL);
+	if (!crtc)
+		return;
 	struct vmw_private *dev_priv = vmw_priv(plane->dev);
 	struct vmw_display_unit *du = vmw_crtc_to_du(crtc);
 	struct vmw_plane_state *vps = vmw_plane_state_to_vps(new_state);
-- 
2.54.0

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

[Claude Code Review Bot]

Patch Review

**C99 mixed declarations issue.** The patch adds early-return guards for null `new_state` and `old_state`, which is correct defensively, but introduces a variable declaration after a statement, which is problematic for kernel coding style:

```c
	new_state = drm_atomic_get_new_plane_state(state, plane);
	if (!new_state)
		return;
	old_state = drm_atomic_get_old_plane_state(state, plane);
	struct drm_crtc *crtc = new_state->crtc ? new_state->crtc :
		(old_state ? old_state->crtc : NULL);
```

The `struct drm_crtc *crtc` declaration appears after the `if (!new_state) return;` statement. While C99/C11 allows this, mixing declarations and statements like this after control flow is poor style. `crtc` should be declared at the top of the function alongside the other variables, and assigned later.

**Questionable whether `new_state` can actually be NULL.** In the atomic commit path, `drm_atomic_get_new_plane_state` should always return a valid state for a plane that is part of the atomic update. If this function is being called, the plane was part of the commit. If the null dereference in the IGT test is real, it might indicate a deeper issue worth investigating rather than just papering over with null checks. That said, defensive programming here is not unreasonable as a crash fix.

**The `old_state` null handling is good.** Guarding `old_state->crtc` with `old_state ? old_state->crtc : NULL` is the right fix for the originally reported null dereference — on first enable there may be no old state, and the `?:` operator unconditionally dereferences `old_state`.

---
Generated by Claude Code Patch Reviewer

[PATCH v2 2/5] 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  | 28 ++++++++++++++++++++++++++++
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.h  |  9 +++++++++
 drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 12 +++---------
 3 files changed, 40 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index 55730e29d3ae..121a4f006804 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -1062,8 +1062,36 @@ 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 = ALIGN(mode->hdisplay, GPU_TILE_SIZE) *
+			   ALIGN(mode->vdisplay, GPU_TILE_SIZE) *
+			   assumed_cpp;
+	required_mem = ALIGN(required_mem, PAGE_SIZE);
+
+	ret = drm_mode_validate_size(mode, dev_priv->texture_max_width,
+				     dev_priv->texture_max_height);
+	if (ret != MODE_OK)
+		return ret;
+
+	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 445471fe9be6..ea12f6c77f2d 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 dcbacee97f61..17dc20b2ec64 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

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

[PATCH v2 3/5] 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   |  2 ++
 drivers/gpu/drm/vmwgfx/vmwgfx_vkms.c | 12 ++++++++++++
 2 files changed, 14 insertions(+)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
index 9c7a73c0b0dc..6817e0b8ce99 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
@@ -55,7 +55,9 @@ static void vmw_bo_free(struct ttm_buffer_object *bo)
 			/* Reserve and switch the backing mob. */
 			mutex_lock(&res->dev_priv->cmdbuf_mutex);
 			(void)vmw_resource_reserve(res, false, true);
+			ttm_bo_reserve(bo, false, false, NULL);
 			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 5abd7f5ad2db..21cdfc6a1641 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,18 @@ 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

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

[Claude Code Review Bot]

Patch Review

**vmw_bo_free fix is correct.** Looking at `vmw_resource_mob_detach`, it calls `dma_resv_assert_held(gbo->tbo.base.resv)`, which requires the TTM BO reservation lock to be held. `vmw_resource_reserve` does NOT acquire the BO reservation lock — it only takes the resource off the LRU list and optionally allocates a guest memory buffer. So the added `ttm_bo_reserve`/`ttm_bo_unreserve` around `vmw_resource_mob_detach` in `vmw_bo_free` is necessary:

```c
			(void)vmw_resource_reserve(res, false, true);
+			ttm_bo_reserve(bo, false, false, NULL);
			vmw_resource_mob_detach(res);
+			ttm_bo_unreserve(bo);
```

However, the return value of `ttm_bo_reserve` is not checked. It's called with `interruptible=false` and `no_wait=false`, so it should only fail if the BO is being destroyed — but since we're *in* the BO's free callback, this is the BO being destroyed. This is a potential concern: can you successfully reserve a BO that is in the process of being freed (refcount == 0, as asserted at line 45)? The `vmw_resource_release` path does `BUG_ON(ret)` for the same call, so at minimum the error should not be silently ignored.

**vkms crc_generate_worker fix — goto skips vmw_surface_unreference.** The error path jumps to `done:` which falls through to `vmw_surface_unreference(&surf)`, so the surface reference is properly released. However, the blank line after `done:` is unnecessary style-wise:

```c
+done:
+
 		vmw_surface_unreference(&surf);
```

**The reservation scope is wider than necessary in vkms.** The lock is held across the entire `compute_crc` call. This is functionally correct (the BO must be reserved during access), but worth noting if `compute_crc` is expensive.

---
Generated by Claude Code Patch Reviewer

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

[Maaz Mombasawala]

Preserve a ttm reference 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.

Signed-off-by: Maaz Mombasawala <maaz.mombasawala@broadcom.com>
---
 drivers/gpu/drm/vmwgfx/ttm_object.c     | 7 +++++--
 drivers/gpu/drm/vmwgfx/vmwgfx_surface.c | 5 +++--
 2 files changed, 8 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_surface.c b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
index b2d3927b5567..256e7b89daea 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
@@ -2328,11 +2328,12 @@ 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;
+
 err:
 	if (res)
 		vmw_resource_unreference(&res);
-
-	ttm_ref_object_base_unref(tfile, arg.rep.handle);
+	if (ret)
+		ttm_ref_object_base_unref(tfile, arg.rep.handle);
 
 	return ret;
 }
-- 
2.54.0

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

[Claude Code Review Bot]

Patch Review

**The dma_buf leak fix in ttm_prime_fd_to_handle is clearly correct.** The original code returns `-ENOSYS` without calling `dma_buf_put(dma_buf)`, leaking the reference obtained from `dma_buf_get(fd)`:

```c
-	if (dma_buf->ops != &tdev->ops)
-		return -ENOSYS;
+	if (dma_buf->ops != &tdev->ops) {
+		ret = -ENOSYS;
+		goto out;
+	}
```

This is a straightforward and correct fix.

**The vmw_dumb_create change needs careful consideration.** The patch changes the error path to conditionally unref:

```c
-	ttm_ref_object_base_unref(tfile, arg.rep.handle);
+	if (ret)
+		ttm_ref_object_base_unref(tfile, arg.rep.handle);
```

Combined with `usurf->prime.base.refcount_release = NULL`, this preserves the TTM reference for dumb buffers on the success path. The idea is that the GEM object holds the surface alive, and the TTM refcount_release callback is nulled out so that when the TTM ref is eventually dropped, it doesn't try to destroy the surface (since the GEM path handles that). This is a somewhat subtle lifetime management change — the commit message should explain the ownership model more clearly.

**Setting `refcount_release = NULL` is outside the error check.** The line `usurf->prime.base.refcount_release = NULL;` executes before the `err:` label, meaning it only runs on the success path. This is correct, but the code flow would be clearer if there were a comment explaining why.

---
Generated by Claude Code Patch Reviewer

[PATCH v2 5/5] drm/vmwgfx: Remove duplicates in vmwgfx_bo.h

[Maaz Mombasawala]

The vmw_user_bo_ref and vmw_user_bo_unref functions are identical to
vmw_bo_reference and vmw_bo_unreference respectively, remove them.

Signed-off-by: Maaz Mombasawala <maaz.mombasawala@broadcom.com>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_bo.c       |  4 ++--
 drivers/gpu/drm/vmwgfx/vmwgfx_bo.h       | 15 ---------------
 drivers/gpu/drm/vmwgfx/vmwgfx_cotable.c  |  4 ++--
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.h      |  1 -
 drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c  |  4 ++--
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c      |  4 ++--
 drivers/gpu/drm/vmwgfx/vmwgfx_overlay.c  |  2 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_prime.c    |  2 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_resource.c | 12 ++++++------
 drivers/gpu/drm/vmwgfx/vmwgfx_shader.c   |  4 ++--
 drivers/gpu/drm/vmwgfx/vmwgfx_surface.c  |  6 +++---
 11 files changed, 21 insertions(+), 37 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
index 6817e0b8ce99..4071190408b5 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
@@ -539,7 +539,7 @@ static int vmw_user_bo_synccpu_release(struct drm_file *filp,
 		if (!(flags & drm_vmw_synccpu_allow_cs)) {
 			atomic_dec(&vmw_bo->cpu_writers);
 		}
-		vmw_user_bo_unref(&vmw_bo);
+		vmw_bo_unreference(&vmw_bo);
 	}
 
 	return ret;
@@ -581,7 +581,7 @@ int vmw_user_bo_synccpu_ioctl(struct drm_device *dev, void *data,
 			return ret;
 
 		ret = vmw_user_bo_synccpu_grab(vbo, arg->flags);
-		vmw_user_bo_unref(&vbo);
+		vmw_bo_unreference(&vbo);
 		if (unlikely(ret != 0)) {
 			if (ret == -ERESTARTSYS || ret == -EBUSY)
 				return -EBUSY;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.h b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.h
index cf84a163bfcb..f3c2cfb42588 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.h
@@ -213,21 +213,6 @@ static inline struct vmw_bo *vmw_bo_reference(struct vmw_bo *buf)
 	return buf;
 }
 
-static inline struct vmw_bo *vmw_user_bo_ref(struct vmw_bo *vbo)
-{
-	drm_gem_object_get(&vbo->tbo.base);
-	return vbo;
-}
-
-static inline void vmw_user_bo_unref(struct vmw_bo **buf)
-{
-	struct vmw_bo *tmp_buf = *buf;
-
-	*buf = NULL;
-	if (tmp_buf)
-		drm_gem_object_put(&tmp_buf->tbo.base);
-}
-
 static inline struct vmw_bo *to_vmw_bo(struct drm_gem_object *gobj)
 {
 	return container_of((gobj), struct vmw_bo, tbo.base);
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_cotable.c b/drivers/gpu/drm/vmwgfx/vmwgfx_cotable.c
index 091f1039a052..9c882e746e3c 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_cotable.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_cotable.c
@@ -502,7 +502,7 @@ static int vmw_cotable_resize(struct vmw_resource *res, size_t new_size)
 
 	vmw_resource_mob_attach(res);
 	/* Let go of the old mob. */
-	vmw_user_bo_unref(&old_buf);
+	vmw_bo_unreference(&old_buf);
 	res->id = vcotbl->type;
 
 	ret = dma_resv_reserve_fences(bo->base.resv, 1);
@@ -521,7 +521,7 @@ static int vmw_cotable_resize(struct vmw_resource *res, size_t new_size)
 out_wait:
 	ttm_bo_unpin(bo);
 	ttm_bo_unreserve(bo);
-	vmw_user_bo_unref(&buf);
+	vmw_bo_unreference(&buf);
 
 out_done:
 	MKS_STAT_TIME_POP(MKSSTAT_KERN_COTABLE_RESIZE);
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
index f2abaf1bda6a..6fbc4c321158 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
@@ -127,7 +127,6 @@ struct vmw_res_func;
  * resource destruction.
  */
 struct vmw_bo;
-struct vmw_bo;
 struct vmw_resource_dirty;
 struct vmw_resource {
 	struct kref kref;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
index e1f18020170a..f3fd45f28140 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
@@ -1148,7 +1148,7 @@ static int vmw_translate_mob_ptr(struct vmw_private *dev_priv,
 	vmw_bo_placement_set(vmw_bo, VMW_BO_DOMAIN_MOB, VMW_BO_DOMAIN_MOB);
 	ret = vmw_validation_add_bo(sw_context->ctx, vmw_bo);
 	tmp_bo = vmw_bo;
-	vmw_user_bo_unref(&tmp_bo);
+	vmw_bo_unreference(&tmp_bo);
 	if (unlikely(ret != 0))
 		return ret;
 
@@ -1205,7 +1205,7 @@ static int vmw_translate_guest_ptr(struct vmw_private *dev_priv,
 			     VMW_BO_DOMAIN_GMR | VMW_BO_DOMAIN_VRAM);
 	ret = vmw_validation_add_bo(sw_context->ctx, vmw_bo);
 	tmp_bo = vmw_bo;
-	vmw_user_bo_unref(&tmp_bo);
+	vmw_bo_unreference(&tmp_bo);
 	if (unlikely(ret != 0))
 		return ret;
 
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index 121a4f006804..230dd9c281a2 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -1967,7 +1967,7 @@ int vmw_connector_get_modes(struct drm_connector *connector)
 struct vmw_user_object *vmw_user_object_ref(struct vmw_user_object *uo)
 {
 	if (uo->buffer)
-		vmw_user_bo_ref(uo->buffer);
+		vmw_bo_reference(uo->buffer);
 	else if (uo->surface)
 		vmw_surface_reference(uo->surface);
 	return uo;
@@ -1976,7 +1976,7 @@ struct vmw_user_object *vmw_user_object_ref(struct vmw_user_object *uo)
 void vmw_user_object_unref(struct vmw_user_object *uo)
 {
 	if (uo->buffer)
-		vmw_user_bo_unref(&uo->buffer);
+		vmw_bo_unreference(&uo->buffer);
 	else if (uo->surface)
 		vmw_surface_unreference(&uo->surface);
 }
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_overlay.c b/drivers/gpu/drm/vmwgfx/vmwgfx_overlay.c
index 679adf7c7183..d4a1cd50c325 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_overlay.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_overlay.c
@@ -451,7 +451,7 @@ int vmw_overlay_ioctl(struct drm_device *dev, void *data,
 
 	ret = vmw_overlay_update_stream(dev_priv, buf, arg, true);
 
-	vmw_user_bo_unref(&buf);
+	vmw_bo_unreference(&buf);
 
 out_unlock:
 	mutex_unlock(&overlay->mutex);
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_prime.c b/drivers/gpu/drm/vmwgfx/vmwgfx_prime.c
index 598b90ac7590..6abeb1ee1f8f 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_prime.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_prime.c
@@ -117,7 +117,7 @@ int vmw_prime_handle_to_fd(struct drm_device *dev,
 								 handle, flags,
 								 prime_fd);
 		}
-		vmw_user_bo_unref(&vbo);
+		vmw_bo_unreference(&vbo);
 	}
 
 	return ret;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
index 388011696941..1391bed79f7b 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
@@ -143,7 +143,7 @@ static void vmw_resource_release(struct kref *kref)
 		if (res->coherent)
 			vmw_bo_dirty_release(res->guest_memory_bo);
 		ttm_bo_unreserve(bo);
-		vmw_user_bo_unref(&res->guest_memory_bo);
+		vmw_bo_unreference(&res->guest_memory_bo);
 	}
 
 	if (likely(res->hw_destroy != NULL)) {
@@ -315,7 +315,7 @@ int vmw_user_object_lookup(struct vmw_private *dev_priv,
 							    uo->buffer,
 							    handle);
 		if (uo->surface)
-			vmw_user_bo_unref(&uo->buffer);
+			vmw_bo_unreference(&uo->buffer);
 	}
 
 	return ret;
@@ -466,11 +466,11 @@ void vmw_resource_unreserve(struct vmw_resource *res,
 			vmw_resource_mob_detach(res);
 			if (res->coherent)
 				vmw_bo_dirty_release(res->guest_memory_bo);
-			vmw_user_bo_unref(&res->guest_memory_bo);
+			vmw_bo_unreference(&res->guest_memory_bo);
 		}
 
 		if (new_guest_memory_bo) {
-			res->guest_memory_bo = vmw_user_bo_ref(new_guest_memory_bo);
+			res->guest_memory_bo = vmw_bo_reference(new_guest_memory_bo);
 
 			/*
 			 * The validation code should already have added a
@@ -560,7 +560,7 @@ vmw_resource_check_buffer(struct ww_acquire_ctx *ticket,
 	drm_gem_object_put(&val_buf->bo->base);
 	val_buf->bo = NULL;
 	if (guest_memory_dirty)
-		vmw_user_bo_unref(&res->guest_memory_bo);
+		vmw_bo_unreference(&res->guest_memory_bo);
 
 	return ret;
 }
@@ -736,7 +736,7 @@ int vmw_resource_validate(struct vmw_resource *res, bool intr,
 		goto out_no_validate;
 	else if (!res->func->needs_guest_memory && res->guest_memory_bo) {
 		WARN_ON_ONCE(vmw_resource_mob_attached(res));
-		vmw_user_bo_unref(&res->guest_memory_bo);
+		vmw_bo_unreference(&res->guest_memory_bo);
 	}
 
 	return 0;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_shader.c b/drivers/gpu/drm/vmwgfx/vmwgfx_shader.c
index eca4e3e97eb4..ede4de9dc8b0 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_shader.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_shader.c
@@ -180,7 +180,7 @@ static int vmw_gb_shader_init(struct vmw_private *dev_priv,
 
 	res->guest_memory_size = size;
 	if (byte_code) {
-		res->guest_memory_bo = vmw_user_bo_ref(byte_code);
+		res->guest_memory_bo = vmw_bo_reference(byte_code);
 		res->guest_memory_offset = offset;
 	}
 	shader->size = size;
@@ -809,7 +809,7 @@ static int vmw_shader_define(struct drm_device *dev, struct drm_file *file_priv,
 				    shader_type, num_input_sig,
 				    num_output_sig, tfile, shader_handle);
 out_bad_arg:
-	vmw_user_bo_unref(&buffer);
+	vmw_bo_unreference(&buffer);
 	return ret;
 }
 
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
index 256e7b89daea..3525d1388f33 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
@@ -972,7 +972,7 @@ static int vmw_buffer_prime_to_surface_base(struct vmw_private *dev_priv,
 
 	*base_p = base;
 out:
-	vmw_user_bo_unref(&bo);
+	vmw_bo_unreference(&bo);
 
 	return ret;
 }
@@ -1593,12 +1593,12 @@ vmw_gb_surface_define_internal(struct drm_device *dev,
 		if (ret == 0) {
 			if (res->guest_memory_bo->is_dumb) {
 				VMW_DEBUG_USER("Can't backup surface with a dumb buffer.\n");
-				vmw_user_bo_unref(&res->guest_memory_bo);
+				vmw_bo_unreference(&res->guest_memory_bo);
 				ret = -EINVAL;
 				goto out_unlock;
 			} else if (res->guest_memory_bo->tbo.base.size < res->guest_memory_size) {
 				VMW_DEBUG_USER("Surface backup buffer too small.\n");
-				vmw_user_bo_unref(&res->guest_memory_bo);
+				vmw_bo_unreference(&res->guest_memory_bo);
 				ret = -EINVAL;
 				goto out_unlock;
 			} else {
-- 
2.54.0

Claude review: drm/vmwgfx: Remove duplicates in vmwgfx_bo.h

[Claude Code Review Bot]

Patch Review

**Confirmed the functions are truly identical.** Both `vmw_user_bo_ref`/`vmw_bo_reference` call `drm_gem_object_get`, and both `vmw_user_bo_unref`/`vmw_bo_unreference` null the pointer then call `drm_gem_object_put`. The removal is correct.

**Also removes a duplicate `struct vmw_bo;` forward declaration** in `vmwgfx_drv.h`:
```c
 struct vmw_bo;
-struct vmw_bo;
```
Good cleanup.

**This is a straightforward mechanical substitution** across 11 files. The patch is clean and complete — all callers are updated. This should be the lowest-risk patch in the series, but it would be better placed first or second in the series to avoid depending on patches 3/4's changes to the same files.

---
Generated by Claude Code Patch Reviewer

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: 6
Reviewed: 2026-05-16T14:26:30.462768

---

This is a 5-patch series fixing issues found via igt-gpu-tools testing on vmwgfx. The fixes address: a null dereference in cursor plane updates, missing vrefresh validation, missing TTM BO reservation lock acquisition, dumb buffer reference count leaks, and removal of duplicate helper functions. The fixes are generally reasonable and address real bugs, but there are some coding style issues (patch 1) and a locking concern (patch 3) worth discussing.

---

---
Generated by Claude Code Patch Reviewer

[PATCH v3 0/5] Fix some issues from igt runs.

[Maaz Mombasawala]

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

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 (5):
  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.
  drm/vmwgfx: Remove duplicates in vmwgfx_bo.h

 drivers/gpu/drm/vmwgfx/ttm_object.c          |  7 +++--
 drivers/gpu/drm/vmwgfx/vmwgfx_bo.c           |  6 ++--
 drivers/gpu/drm/vmwgfx/vmwgfx_bo.h           | 15 ---------
 drivers/gpu/drm/vmwgfx/vmwgfx_cotable.c      |  4 +--
 drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c | 16 +++++++---
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.h          |  1 -
 drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c      |  4 +--
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c          | 32 ++++++++++++++++++--
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.h          |  9 ++++++
 drivers/gpu/drm/vmwgfx/vmwgfx_overlay.c      |  2 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_prime.c        |  2 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_resource.c     | 12 ++++----
 drivers/gpu/drm/vmwgfx/vmwgfx_shader.c       |  4 +--
 drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c         | 12 ++------
 drivers/gpu/drm/vmwgfx/vmwgfx_surface.c      | 11 ++++---
 drivers/gpu/drm/vmwgfx/vmwgfx_vkms.c         | 12 ++++++++
 16 files changed, 94 insertions(+), 55 deletions(-)

-- 
2.54.0

[PATCH v3 1/5] 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.
Add a check to fix this and also do an early return if there is no
new_state or crtc since there is nothing to do in that case.
This fixes igt test kms_cursor_legacy@torture-bo.

Signed-off-by: Maaz Mombasawala <maaz.mombasawala@broadcom.com>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c b/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c
index b010fc7ca68e..583115c94800 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c
@@ -742,11 +742,17 @@ 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;
+	struct drm_plane_state *old_state;
+
+	new_state = drm_atomic_get_new_plane_state(state, plane);
+	if (!new_state)
+		return;
+	old_state = drm_atomic_get_old_plane_state(state, plane);
+	struct drm_crtc *crtc =  new_state->crtc ? new_state->crtc :
+		(old_state ? old_state->crtc : NULL);
+	if (!crtc)
+		return;
 	struct vmw_private *dev_priv = vmw_priv(plane->dev);
 	struct vmw_display_unit *du = vmw_crtc_to_du(crtc);
 	struct vmw_plane_state *vps = vmw_plane_state_to_vps(new_state);
-- 
2.54.0

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

[Claude Code Review Bot]

Patch Review

**Coding style issue — declaration after statement:**

```c
+	new_state = drm_atomic_get_new_plane_state(state, plane);
+	if (!new_state)
+		return;
+	old_state = drm_atomic_get_old_plane_state(state, plane);
+	struct drm_crtc *crtc =  new_state->crtc ? new_state->crtc :
+		(old_state ? old_state->crtc : NULL);
+	if (!crtc)
+		return;
+	struct vmw_private *dev_priv = vmw_priv(plane->dev);
```

The `struct drm_crtc *crtc` and `struct vmw_private *dev_priv` declarations are interspersed with code. While C99/GNU extensions allow this, Linux kernel style strongly prefers all declarations at the top of the block. Move `crtc` and `dev_priv` declarations to the top (alongside `new_state`, `old_state`) and assign them after the NULL checks. Also there's a double space (`=  new_state`) on the crtc assignment line.

**Questionable NULL check on new_state:** In the atomic commit path, `drm_atomic_get_new_plane_state()` should always return a valid state for a plane that is part of the commit — this function is called from `atomic_update`, which only runs for planes included in the commit. If `new_state` is NULL here, something has gone seriously wrong at a higher layer. A `WARN_ON(!new_state)` plus return might be more appropriate to flag the unexpected condition rather than silently returning.

**Logic correctness:** The ternary for `crtc` is correct — it safely handles `old_state` being NULL. The original code `new_state->crtc ?: old_state->crtc` would crash if `new_state->crtc` was NULL and `old_state` was also NULL.

---
Generated by Claude Code Patch Reviewer

[PATCH] 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.
Add a check to fix this and also do an early return if there is no
new_state or crtc since there is nothing to do in that case.
This fixes igt test kms_cursor_legacy@torture-bo.

Signed-off-by: Maaz Mombasawala <maaz.mombasawala@broadcom.com>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c b/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c
index c46f17ba7236..88b7185a8371 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c
@@ -744,9 +744,14 @@ vmw_cursor_plane_atomic_update(struct drm_plane *plane,
 	struct vmw_bo *bo;
 	struct drm_plane_state *new_state =
 		drm_atomic_get_new_plane_state(state, plane);
+	if (!new_state)
+		return;
 	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_crtc *crtc = new_state->crtc ? new_state->crtc :
+		(old_state ? old_state->crtc : NULL);
+	if (!crtc)
+		return;
 	struct vmw_private *dev_priv = vmw_priv(plane->dev);
 	struct vmw_display_unit *du = vmw_crtc_to_du(crtc);
 	struct vmw_plane_state *vps = vmw_plane_state_to_vps(new_state);
-- 
2.53.0

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

[Claude Code Review Bot]

Patch Review

**Coding style: Declarations after statements (build error / warning)**

The patch inserts a `return` statement between variable declarations:

```c
struct drm_plane_state *new_state =
    drm_atomic_get_new_plane_state(state, plane);
if (!new_state)
    return;
struct drm_plane_state *old_state =
    drm_atomic_get_old_plane_state(state, plane);
```

This interleaves statements (`if (!new_state) return;`) with declarations (`struct drm_plane_state *old_state = ...`). While C99/C11 technically allows this, the Linux kernel coding style strongly discourages mixing declarations and statements, and `-Wdeclaration-after-statement` (which is enabled in kernel builds) will produce a warning. The same issue occurs with the `if (!crtc) return;` followed by `struct vmw_private *dev_priv = ...`.

The fix should move all declarations to the top of the function and perform the null checks afterward. Something like:

```c
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;
struct vmw_private *dev_priv;
...

if (!new_state)
    return;

crtc = new_state->crtc ? new_state->crtc :
    (old_state ? old_state->crtc : NULL);
if (!crtc)
    return;

dev_priv = vmw_priv(plane->dev);
```

**Questionable whether `new_state` can actually be NULL**

The `atomic_update` callback is invoked by the atomic commit path only for planes that are part of the commit. `drm_atomic_get_new_plane_state()` should always return a valid state for such a plane. If `new_state` is truly NULL here, it would indicate a deeper framework bug. The commit message says this fixes `kms_cursor_legacy@torture-bo` but doesn't explain the actual root cause — was the plane being committed without state, or was the issue specifically with `old_state` being NULL? A more detailed analysis in the commit message would help reviewers understand if this is papering over a real bug elsewhere.

**The `old_state` NULL check is the more plausible fix**

The original `old_state->crtc` dereference without a NULL check is the more likely actual bug. If a cursor plane is being enabled for the first time (no previous state), `old_state` could legitimately be NULL. Protecting against that case specifically seems correct.

**Minor: ternary replacement**

The original `?:` (GNU extension) was replaced with a fully spelled-out ternary `? new_state->crtc : ...`. This is fine and arguably more portable, though the GNU extension is widely used in the kernel.

**Summary**: The intent is good but the implementation needs rework to comply with kernel coding style (no declarations after statements). The commit message should also clarify the actual root cause more precisely — specifically which pointer was NULL when the crash occurred.

---
Generated by Claude Code Patch Reviewer

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

[Claude Code Review Bot]

Overall Series Review

Subject: drm/vmwgfx: Add some checks to vmw_cursor_plane_atomic_update
Author: Maaz Mombasawala <maaz.mombasawala@broadcom.com>
Patches: 1
Reviewed: 2026-03-22T03:57:47.499306

---

This is a single-patch series adding null-pointer checks to `vmw_cursor_plane_atomic_update()` in the vmwgfx driver to fix a crash seen in the `kms_cursor_legacy@torture-bo` IGT test. The intent is reasonable — guarding against null dereferences when `old_state` or `new_state` could be absent. However, the patch has coding style issues that would prevent it from compiling cleanly under strict C standards and violate kernel coding conventions.

---
Generated by Claude Code Patch Reviewer