[PATCH drm-misc-next v3 0/1] add drm_panic_support for vmwgfx-stdu

[PATCH drm-misc-next v3 0/1] add drm_panic_support for vmwgfx-stdu

[Ryosuke Yasuoka]

Add drm_panic support for stdu in vmwgfx. This patch was tested in a VM
with VMSVGA on Virtual Box.

Based on the feedback for v2 patch, I've made the following updates in
my v3 patch.
- Use MEMREMAP_WB | MEMREMAP_DEC flags for memremap
- Use vmw_priv->initial_height and initial_width for sb and VRAM
- Move all panic related functions to the vmwgfx_kms.c file
- Ensure if the current display unit is 0 in get_scanout_buffer()

I did not apply all of Ian's feedback in this v3 patch for the reasons
outlined below.

> 1. You can call `vmw_kms_write_svga` directly in `panic_flush`. There
> is no need to mark the buffer as dirty or send any commands.

In my test environment (VirtualBox), the panic screen appears unstably 
without dirty command's submission. Without it, the screen sometimes
appears immediately as expected, and at other times, there's a delay
of 15 to 20 seconds. I've also observed that it sometimes only appears
after switching between the VirtualBox console window and other windows.

With command submission, we can stably get a panic screen. Even if it
fails, we may get the panic screen ultimately. Therefore, I think we
should retain vmw_kms_panic_do_bo_dirty() to submit commands.

> 2. The format should be hardcoded to RGB565 to minimize the risk of
> overrunning VRAM. Adjust the pitch accordingly to 2x width.

While it's possible to output the panic screen with RGB565 by adjusting
pitch and width, and BPP on the (virtual) hardware side, this approach
causes debugfs to fail. It appears that the BPP must be reset after the
panic screen is displayed, and there is no way to wait for the screen
to be fully displayed within the drm_panic handler code.

In my test environment, as VRAM has enough space even when using
XRGB8888 (32 bits), I think the risk of overruning VRAM is low. So I've
kept the default format and CPP size.

v1:
https://lore.kernel.org/all/20250901083701.32365-1-ryasuoka@redhat.com/

v2:
https://lore.kernel.org/all/20250908141152.221291-2-ryasuoka@redhat.com/
- Map a scanout_buffer to VRAM in .get_scanout_buffer
- And then write to VRAM directly using fifo in legacy mode to avoid
allocations or command submissions.


Ryosuke Yasuoka (1):
  drm/vmwgfx: add drm_panic support for stdu mode

 drivers/gpu/drm/vmwgfx/vmwgfx_drv.h  |   4 +
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c  | 165 +++++++++++++++++++++++++++
 drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c |   2 +
 3 files changed, 171 insertions(+)


base-commit: d41c79838c47bc822534cd53628fe5e0f8ad2424
-- 
2.51.0

[PATCH drm-misc-next v3 1/1] drm/vmwgfx: add drm_panic support for stdu mode

[Ryosuke Yasuoka]

Add drm_panic module for vmwgfx stdu mode so that panic screen can be
displayed on panic. Since implemented by writing to VRAM and switching
back to legacy mode, it would expect to work in all DU modes.

Signed-off-by: Ryosuke Yasuoka <ryasuoka@redhat.com>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.h  |   4 +
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c  | 165 +++++++++++++++++++++++++++
 drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c |   2 +
 3 files changed, 171 insertions(+)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
index eda5b6f8f4c4..1259112ee2c2 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
@@ -16,6 +16,7 @@
 #include <drm/drm_auth.h>
 #include <drm/drm_device.h>
 #include <drm/drm_file.h>
+#include <drm/drm_panic.h>
 #include <drm/drm_rect.h>
 
 #include <drm/ttm/ttm_execbuf_util.h>
@@ -1046,6 +1047,9 @@ void vmw_kms_lost_device(struct drm_device *dev);
 extern int vmw_resource_pin(struct vmw_resource *res, bool interruptible);
 extern void vmw_resource_unpin(struct vmw_resource *res);
 extern enum vmw_res_type vmw_res_type(const struct vmw_resource *res);
+int vmw_primary_plane_get_scanout_buffer(struct drm_plane *plane,
+					 struct drm_scanout_buffer *sb);
+void vmw_primary_plane_panic_flush(struct drm_plane *plane);
 
 /**
  * Overlay control - vmwgfx_overlay.c
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index 54ea1b513950..c79e39c1845e 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -2022,3 +2022,168 @@ bool vmw_user_object_is_null(struct vmw_user_object *uo)
 {
 	return !uo->buffer && !uo->surface;
 }
+
+/*
+ * For drm_panic
+ * Lockless vmw_write() because drm_panic calls this in panic handler
+ */
+static inline void vmw_panic_write(struct vmw_private *dev_priv,
+				   unsigned int offset, uint32_t value)
+{
+	outl(offset, dev_priv->io_start + SVGA_INDEX_PORT);
+	outl(value, dev_priv->io_start + SVGA_VALUE_PORT);
+}
+
+/* For drm_panic */
+static void
+vmw_kms_panic_write_svga(struct vmw_private *vmw_priv, unsigned int width,
+			 unsigned int height, unsigned int pitch)
+{
+	vmw_panic_write(vmw_priv, SVGA_REG_PITCHLOCK, pitch);
+	vmw_panic_write(vmw_priv, SVGA_REG_WIDTH, width);
+	vmw_panic_write(vmw_priv, SVGA_REG_HEIGHT, height);
+}
+
+/* For drm_panic */
+static void *vmw_panic_fifo_reserve(struct vmw_private *dev_priv, uint32_t bytes)
+{
+	struct vmw_fifo_state *fifo_state = dev_priv->fifo;
+	u32  *fifo_mem = dev_priv->fifo_mem;
+	uint32_t reserveable = fifo_state->capabilities & SVGA_FIFO_CAP_RESERVE;
+
+	/*
+	 * Access to fifo registers without mutex lock because it is only called is
+	 * panic handler
+	 */
+	uint32_t max = vmw_fifo_mem_read(dev_priv, SVGA_FIFO_MAX);
+	uint32_t min = vmw_fifo_mem_read(dev_priv, SVGA_FIFO_MIN);
+	uint32_t stop = vmw_fifo_mem_read(dev_priv, SVGA_FIFO_STOP);
+	uint32_t next_cmd = vmw_fifo_mem_read(dev_priv, SVGA_FIFO_NEXT_CMD);
+
+	if (unlikely(bytes >= (max - min)))
+		return NULL;
+
+	bool has_space;
+
+	if (next_cmd >= stop) {
+		has_space = (next_cmd + bytes < max ||
+			     (next_cmd + bytes == max && stop > min));
+	} else {
+		has_space = (next_cmd + bytes < stop);
+	}
+
+	if (unlikely(!has_space || (!reserveable && bytes > sizeof(uint32_t))))
+		return NULL;
+
+	fifo_state->reserved_size = bytes;
+	fifo_state->using_bounce_buffer = false;
+
+	if (reserveable)
+		vmw_fifo_mem_write(dev_priv, SVGA_FIFO_RESERVED, bytes);
+
+	return (void __force *) (fifo_mem + (next_cmd >> 2));
+}
+
+/* For drm_panic */
+static void
+vmw_panic_fifo_ping_host(struct vmw_private *dev_priv, uint32_t reason)
+{
+	u32 *fifo_mem = dev_priv->fifo_mem;
+
+	if (fifo_mem && cmpxchg(fifo_mem + SVGA_FIFO_BUSY, 0, 1) == 0)
+		vmw_panic_write(dev_priv, SVGA_REG_SYNC, reason);
+
+}
+
+/* For drm_panic */
+static void vmw_panic_fifo_commit(struct vmw_private *dev_priv, uint32_t bytes)
+{
+	struct vmw_fifo_state *fifo_state = dev_priv->fifo;
+	uint32_t next_cmd = vmw_fifo_mem_read(dev_priv, SVGA_FIFO_NEXT_CMD);
+	uint32_t max = vmw_fifo_mem_read(dev_priv, SVGA_FIFO_MAX);
+	uint32_t min = vmw_fifo_mem_read(dev_priv, SVGA_FIFO_MIN);
+	bool reserveable = fifo_state->capabilities & SVGA_FIFO_CAP_RESERVE;
+
+	fifo_state->reserved_size = 0;
+
+	if (reserveable) {
+		next_cmd += bytes;
+		if (next_cmd >= max)
+			next_cmd -= max - min;
+		mb();
+		vmw_fifo_mem_write(dev_priv, SVGA_FIFO_NEXT_CMD, next_cmd);
+		vmw_fifo_mem_write(dev_priv, SVGA_FIFO_RESERVED, 0);
+	}
+	mb();
+	vmw_panic_fifo_ping_host(dev_priv, SVGA_SYNC_GENERIC);
+}
+
+/* For drm_panic */
+static int vmw_kms_panic_do_bo_dirty(struct vmw_private *dev_priv)
+{
+	size_t fifo_size;
+	struct {
+		uint32_t header;
+		SVGAFifoCmdUpdate body;
+	} *cmd;
+
+	fifo_size = sizeof(*cmd);
+	cmd = vmw_panic_fifo_reserve(dev_priv, fifo_size);
+	if (IS_ERR_OR_NULL(cmd))
+		return -ENOMEM;
+
+	memset(cmd, 0, fifo_size);
+
+	cmd[0].header = SVGA_CMD_UPDATE;
+	cmd[0].body.x = 0;
+	cmd[0].body.y = 0;
+	cmd[0].body.width = dev_priv->initial_width;
+	cmd[0].body.height = dev_priv->initial_height;
+
+	vmw_panic_fifo_commit(dev_priv, fifo_size);
+	return 0;
+}
+
+int vmw_primary_plane_get_scanout_buffer(struct drm_plane *plane,
+					 struct drm_scanout_buffer *sb)
+{
+	struct drm_plane_state *state = plane->state;
+	struct drm_crtc *crtc = state->crtc;
+	struct vmw_private *dev_priv = vmw_priv(crtc->dev);
+	struct vmw_display_unit *du = vmw_crtc_to_du(crtc);
+
+	if (!plane->state || !plane->state->fb || !plane->state->visible || du->unit != 0)
+		return -ENODEV;
+
+	sb->format = state->fb->format;
+	sb->width = dev_priv->initial_width;
+	sb->height = dev_priv->initial_height;
+	sb->pitch[0] = dev_priv->initial_width * state->fb->format->cpp[0];
+
+	u64 size = sb->height * sb->pitch[0];
+
+	sb->map[0].vaddr = memremap(dev_priv->vram_start, size, MEMREMAP_WB | MEMREMAP_DEC);
+
+	if (!sb->map[0].vaddr)
+		return -ENOMEM;
+
+	return 0;
+}
+
+/* For drm_panic */
+void vmw_primary_plane_panic_flush(struct drm_plane *plane)
+{
+	struct drm_plane_state *state = plane->state;
+	struct drm_crtc *crtc = state->crtc;
+	struct vmw_private *dev_priv = vmw_priv(crtc->dev);
+	struct drm_framebuffer *fb = state->fb;
+	unsigned int pitch = dev_priv->initial_width * fb->format->cpp[0];
+	int ret;
+
+	vmw_kms_panic_write_svga(dev_priv, dev_priv->initial_width,
+				 dev_priv->initial_height, pitch);
+
+	ret = vmw_kms_panic_do_bo_dirty(dev_priv);
+	if (ret)
+		pr_warn("Failed to vmw_kms_ldu_panic_do_bo_dirty\n");
+}
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
index 20aab725e53a..93c02308d44b 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
@@ -1506,6 +1506,8 @@ drm_plane_helper_funcs vmw_stdu_primary_plane_helper_funcs = {
 	.atomic_update = vmw_stdu_primary_plane_atomic_update,
 	.prepare_fb = vmw_stdu_primary_plane_prepare_fb,
 	.cleanup_fb = vmw_stdu_primary_plane_cleanup_fb,
+	.get_scanout_buffer = vmw_primary_plane_get_scanout_buffer,
+	.panic_flush = vmw_primary_plane_panic_flush,
 };
 
 static const struct drm_crtc_helper_funcs vmw_stdu_crtc_helper_funcs = {
-- 
2.51.0

Re: [PATCH drm-misc-next v3 0/1] add drm_panic_support for vmwgfx-stdu

[Ryosuke Yasuoka]

On Fri, Sep 19, 2025 at 12:29:29PM +0900, Ryosuke Yasuoka wrote:
> Add drm_panic support for stdu in vmwgfx. This patch was tested in a VM
> with VMSVGA on Virtual Box.
> 
> Based on the feedback for v2 patch, I've made the following updates in
> my v3 patch.
> - Use MEMREMAP_WB | MEMREMAP_DEC flags for memremap
> - Use vmw_priv->initial_height and initial_width for sb and VRAM
> - Move all panic related functions to the vmwgfx_kms.c file
> - Ensure if the current display unit is 0 in get_scanout_buffer()
> 
> I did not apply all of Ian's feedback in this v3 patch for the reasons
> outlined below.
> 
> > 1. You can call `vmw_kms_write_svga` directly in `panic_flush`. There
> > is no need to mark the buffer as dirty or send any commands.
> 
> In my test environment (VirtualBox), the panic screen appears unstably 
> without dirty command's submission. Without it, the screen sometimes
> appears immediately as expected, and at other times, there's a delay
> of 15 to 20 seconds. I've also observed that it sometimes only appears
> after switching between the VirtualBox console window and other windows.
> 
> With command submission, we can stably get a panic screen. Even if it
> fails, we may get the panic screen ultimately. Therefore, I think we
> should retain vmw_kms_panic_do_bo_dirty() to submit commands.
> 
> > 2. The format should be hardcoded to RGB565 to minimize the risk of
> > overrunning VRAM. Adjust the pitch accordingly to 2x width.
> 
> While it's possible to output the panic screen with RGB565 by adjusting
> pitch and width, and BPP on the (virtual) hardware side, this approach
> causes debugfs to fail. It appears that the BPP must be reset after the
> panic screen is displayed, and there is no way to wait for the screen
> to be fully displayed within the drm_panic handler code.
> 
> In my test environment, as VRAM has enough space even when using
> XRGB8888 (32 bits), I think the risk of overruning VRAM is low. So I've
> kept the default format and CPP size.
> 
> v1:
> https://lore.kernel.org/all/20250901083701.32365-1-ryasuoka@redhat.com/
> 
> v2:
> https://lore.kernel.org/all/20250908141152.221291-2-ryasuoka@redhat.com/
> - Map a scanout_buffer to VRAM in .get_scanout_buffer
> - And then write to VRAM directly using fifo in legacy mode to avoid
> allocations or command submissions.
> 
> 
> Ryosuke Yasuoka (1):
>   drm/vmwgfx: add drm_panic support for stdu mode
> 
>  drivers/gpu/drm/vmwgfx/vmwgfx_drv.h  |   4 +
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c  | 165 +++++++++++++++++++++++++++
>  drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c |   2 +
>  3 files changed, 171 insertions(+)
> 
> 
> base-commit: d41c79838c47bc822534cd53628fe5e0f8ad2424
> -- 
> 2.51.0

Hi all,

This is a gentle reminder for this v3 patch.
I would appreciate any feedback when you have a chance.

Ian, your feedback on v1 and v2 were very helpful. I would appreciate it
if you could take another look when you have a moment.

Any comments are welcome.

Thank you
Ryosuke

Re: [PATCH drm-misc-next v3 0/1] add drm_panic_support for vmwgfx-stdu

[Ian Forbes]

[-- Attachment #1: Type: text/plain, Size: 3699 bytes --]

If it doesn't work on vbox that's not really our problem. I've sent my
version [1] that works on Workstation and ESXI.
Since it has less code it has a much higher chance of succeeding in a
panic situation which is what we care about.
Feel free to add whatever tags you feel are appropriate on that patch review.

[1] https://lore.kernel.org/dri-devel/20251023200447.206834-1-ian.forbes@broadcom.com/T/#u

Ian,

On Thu, Oct 16, 2025 at 9:48 AM Ryosuke Yasuoka <ryasuoka@redhat.com> wrote:
>
> On Fri, Sep 19, 2025 at 12:29:29PM +0900, Ryosuke Yasuoka wrote:
> > Add drm_panic support for stdu in vmwgfx. This patch was tested in a VM
> > with VMSVGA on Virtual Box.
> >
> > Based on the feedback for v2 patch, I've made the following updates in
> > my v3 patch.
> > - Use MEMREMAP_WB | MEMREMAP_DEC flags for memremap
> > - Use vmw_priv->initial_height and initial_width for sb and VRAM
> > - Move all panic related functions to the vmwgfx_kms.c file
> > - Ensure if the current display unit is 0 in get_scanout_buffer()
> >
> > I did not apply all of Ian's feedback in this v3 patch for the reasons
> > outlined below.
> >
> > > 1. You can call `vmw_kms_write_svga` directly in `panic_flush`. There
> > > is no need to mark the buffer as dirty or send any commands.
> >
> > In my test environment (VirtualBox), the panic screen appears unstably
> > without dirty command's submission. Without it, the screen sometimes
> > appears immediately as expected, and at other times, there's a delay
> > of 15 to 20 seconds. I've also observed that it sometimes only appears
> > after switching between the VirtualBox console window and other windows.
> >
> > With command submission, we can stably get a panic screen. Even if it
> > fails, we may get the panic screen ultimately. Therefore, I think we
> > should retain vmw_kms_panic_do_bo_dirty() to submit commands.
> >
> > > 2. The format should be hardcoded to RGB565 to minimize the risk of
> > > overrunning VRAM. Adjust the pitch accordingly to 2x width.
> >
> > While it's possible to output the panic screen with RGB565 by adjusting
> > pitch and width, and BPP on the (virtual) hardware side, this approach
> > causes debugfs to fail. It appears that the BPP must be reset after the
> > panic screen is displayed, and there is no way to wait for the screen
> > to be fully displayed within the drm_panic handler code.
> >
> > In my test environment, as VRAM has enough space even when using
> > XRGB8888 (32 bits), I think the risk of overruning VRAM is low. So I've
> > kept the default format and CPP size.
> >
> > v1:
> > https://lore.kernel.org/all/20250901083701.32365-1-ryasuoka@redhat.com/
> >
> > v2:
> > https://lore.kernel.org/all/20250908141152.221291-2-ryasuoka@redhat.com/
> > - Map a scanout_buffer to VRAM in .get_scanout_buffer
> > - And then write to VRAM directly using fifo in legacy mode to avoid
> > allocations or command submissions.
> >
> >
> > Ryosuke Yasuoka (1):
> >   drm/vmwgfx: add drm_panic support for stdu mode
> >
> >  drivers/gpu/drm/vmwgfx/vmwgfx_drv.h  |   4 +
> >  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c  | 165 +++++++++++++++++++++++++++
> >  drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c |   2 +
> >  3 files changed, 171 insertions(+)
> >
> >
> > base-commit: d41c79838c47bc822534cd53628fe5e0f8ad2424
> > --
> > 2.51.0
>
> Hi all,
>
> This is a gentle reminder for this v3 patch.
> I would appreciate any feedback when you have a chance.
>
> Ian, your feedback on v1 and v2 were very helpful. I would appreciate it
> if you could take another look when you have a moment.
>
> Any comments are welcome.
>
> Thank you
> Ryosuke
>

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5414 bytes --]