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

[PATCH drm-misc-next v2 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.

I got 2 type of suggestions from reviewers:
1. Write the panic screen to guest_memory_bo and send dirty/flush
command
2. Write it to VRAM directly and then switch back to legacy mode

Since option 2 is simpler than another one and ldu might be able
to re-use some codes if ldu supports drm_panic, I re-write this
v2 patch with option 2.

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

 drivers/gpu/drm/vmwgfx/vmwgfx_cmd.c  | 73 ++++++++++++++++++++++++++++
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.h  | 18 +++++++
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c  |  9 ++++
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.h  |  1 +
 drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c  | 43 ++++++++++++++++
 drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 34 +++++++++++++
 6 files changed, 178 insertions(+)


base-commit: 306c8959b5fdfdc3900aebe4e56257a349f5765f
-- 
2.51.0

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

[Ryosuke Yasuoka]

Add drm_panic module for vmwgfx stdu so that panic screen can be
displayed on panic.

Signed-off-by: Ryosuke Yasuoka <ryasuoka@redhat.com>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_cmd.c  | 73 ++++++++++++++++++++++++++++
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.h  | 18 +++++++
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c  |  9 ++++
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.h  |  1 +
 drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c  | 43 ++++++++++++++++
 drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 34 +++++++++++++
 6 files changed, 178 insertions(+)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_cmd.c b/drivers/gpu/drm/vmwgfx/vmwgfx_cmd.c
index 8fe02131a6c4..fe6275a6cc31 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_cmd.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_cmd.c
@@ -156,6 +156,16 @@ struct vmw_fifo_state *vmw_fifo_create(struct vmw_private *dev_priv)
 	return fifo;
 }
 
+/* For drm_panic */
+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);
+
+}
+
 void vmw_fifo_ping_host(struct vmw_private *dev_priv, uint32_t reason)
 {
 	u32 *fifo_mem = dev_priv->fifo_mem;
@@ -264,6 +274,46 @@ static int vmw_fifo_wait(struct vmw_private *dev_priv,
 	return ret;
 }
 
+/* For drm_panic */
+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));
+}
+
 /*
  * Reserve @bytes number of bytes in the fifo.
  *
@@ -424,6 +474,29 @@ static void vmw_fifo_slow_copy(struct vmw_fifo_state *fifo_state,
 	}
 }
 
+/* For drm_panic */
+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);
+}
+
 static void vmw_local_fifo_commit(struct vmw_private *dev_priv, uint32_t bytes)
 {
 	struct vmw_fifo_state *fifo_state = dev_priv->fifo;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
index eda5b6f8f4c4..a1dc6d63c079 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
@@ -635,6 +635,17 @@ static inline bool vmw_is_svga_v3(const struct vmw_private *dev)
 	return dev->pci_id == VMWGFX_PCI_ID_SVGA3;
 }
 
+/*
+ * 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);
+}
+
 /*
  * The locking here is fine-grained, so that it is performed once
  * for every read- and write operation. This is of course costly, but we
@@ -854,16 +865,19 @@ extern void vmw_fifo_destroy(struct vmw_private *dev_priv);
 extern bool vmw_cmd_supported(struct vmw_private *vmw);
 extern void *
 vmw_cmd_ctx_reserve(struct vmw_private *dev_priv, uint32_t bytes, int ctx_id);
+extern void vmw_panic_fifo_commit(struct vmw_private *dev_priv, uint32_t bytes);
 extern void vmw_cmd_commit(struct vmw_private *dev_priv, uint32_t bytes);
 extern void vmw_cmd_commit_flush(struct vmw_private *dev_priv, uint32_t bytes);
 extern int vmw_cmd_send_fence(struct vmw_private *dev_priv, uint32_t *seqno);
 extern bool vmw_supports_3d(struct vmw_private *dev_priv);
+extern void vmw_panic_fifo_ping_host(struct vmw_private *dev_priv, uint32_t reason);
 extern void vmw_fifo_ping_host(struct vmw_private *dev_priv, uint32_t reason);
 extern bool vmw_fifo_have_pitchlock(struct vmw_private *dev_priv);
 extern int vmw_cmd_emit_dummy_query(struct vmw_private *dev_priv,
 				    uint32_t cid);
 extern int vmw_cmd_flush(struct vmw_private *dev_priv,
 			 bool interruptible);
+extern void *vmw_panic_fifo_reserve(struct vmw_private *dev_priv, uint32_t bytes);
 
 #define VMW_CMD_CTX_RESERVE(__priv, __bytes, __ctx_id)                        \
 ({                                                                            \
@@ -1027,6 +1041,8 @@ void vmw_kms_cursor_snoop(struct vmw_surface *srf,
 			  struct ttm_object_file *tfile,
 			  struct ttm_buffer_object *bo,
 			  SVGA3dCmdHeader *header);
+void vmw_kms_panic_write_svga(struct vmw_private *vmw_priv,
+			      unsigned int width, unsigned int height, unsigned int pitch);
 int vmw_kms_write_svga(struct vmw_private *vmw_priv,
 		       unsigned width, unsigned height, unsigned pitch,
 		       unsigned bpp, unsigned depth);
@@ -1349,6 +1365,8 @@ int vmw_mksstat_remove_ioctl(struct drm_device *dev, void *data,
 		      struct drm_file *file_priv);
 int vmw_mksstat_remove_all(struct vmw_private *dev_priv);
 
+void vmw_ldu_primary_plane_panic_flush(struct drm_plane *plane);
+
 /* VMW logging */
 
 /**
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index 54ea1b513950..89d04d6be83e 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -1203,6 +1203,15 @@ int vmw_kms_close(struct vmw_private *dev_priv)
 	return ret;
 }
 
+/* For drm_panic */
+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);
+}
+
 int vmw_kms_write_svga(struct vmw_private *vmw_priv,
 			unsigned width, unsigned height, unsigned pitch,
 			unsigned bpp, unsigned depth)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
index 445471fe9be6..e6299390ffea 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
@@ -499,6 +499,7 @@ int vmw_kms_stdu_readback(struct vmw_private *dev_priv,
 			  struct drm_crtc *crtc);
 
 int vmw_du_helper_plane_update(struct vmw_du_update_plane *update);
+int vmw_du_panic_helper_plane_update(struct vmw_du_update_plane *update);
 
 /**
  * vmw_du_translate_to_crtc - Translate a rect from framebuffer to crtc
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c
index c23c9195f0dc..b7c3dfbab541 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c
@@ -304,6 +304,49 @@ static int vmw_kms_ldu_do_bo_dirty(struct vmw_private *dev_priv,
 				   struct drm_mode_rect *clips,
 				   unsigned int num_clips);
 
+/* For drm_panic */
+static int vmw_kms_ldu_panic_do_bo_dirty(struct vmw_private *dev_priv,
+					 struct drm_framebuffer *fb)
+{
+	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 = fb->width;
+	cmd[0].body.height = fb->height;
+
+	vmw_panic_fifo_commit(dev_priv, fifo_size);
+	return 0;
+}
+
+/* For drm_panic */
+void vmw_ldu_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;
+	int ret;
+
+	vmw_kms_panic_write_svga(dev_priv, fb->width, fb->height, fb->pitches[0]);
+
+	ret = vmw_kms_ldu_panic_do_bo_dirty(dev_priv, fb);
+	if (ret)
+		pr_warn("Failed to vmw_kms_ldu_panic_do_bo_dirty\n");
+}
+
 /*
  * Legacy Display Plane Functions
  */
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
index 20aab725e53a..faa7135bd699 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
@@ -36,6 +36,7 @@
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_damage_helper.h>
 #include <drm/drm_fourcc.h>
+#include <drm/drm_panic.h>
 #include <drm/drm_vblank.h>
 
 #define vmw_crtc_to_stdu(x) \
@@ -1458,6 +1459,37 @@ vmw_stdu_primary_plane_atomic_update(struct drm_plane *plane,
 		vmw_fence_obj_unreference(&fence);
 }
 
+static int
+vmw_stdu_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);
+
+	if (!plane->state || !plane->state->fb || !plane->state->visible)
+		return -ENODEV;
+
+	sb->format = plane->state->fb->format;
+	sb->width = plane->state->fb->width;
+	sb->height = plane->state->fb->height;
+	sb->pitch[0] = plane->state->fb->pitches[0];
+
+	u64 size = sb->height * sb->pitch[0];
+
+	sb->map[0].vaddr = memremap(dev_priv->vram_start, size, MEMREMAP_WT);
+
+	if (!sb->map[0].vaddr)
+		return -ENOMEM;
+
+	return 0;
+}
+
+static void vmw_stdu_primary_plane_panic_flush(struct drm_plane *plane)
+{
+	vmw_ldu_primary_plane_panic_flush(plane);
+}
+
 static void
 vmw_stdu_crtc_atomic_flush(struct drm_crtc *crtc,
 			   struct drm_atomic_state *state)
@@ -1506,6 +1538,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_stdu_primary_plane_get_scanout_buffer,
+	.panic_flush = vmw_stdu_primary_plane_panic_flush,
 };
 
 static const struct drm_crtc_helper_funcs vmw_stdu_crtc_helper_funcs = {
-- 
2.51.0

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

[Ryosuke Yasuoka]

On Mon, Sep 8, 2025 at 11:12 PM Ryosuke Yasuoka <ryasuoka@redhat.com> wrote:
>
> Add drm_panic support for stdu in vmwgfx. This patch was tested in
> a VM with VMSVGA on Virtual Box.
>
> I got 2 type of suggestions from reviewers:
> 1. Write the panic screen to guest_memory_bo and send dirty/flush
> command
> 2. Write it to VRAM directly and then switch back to legacy mode
>
> Since option 2 is simpler than another one and ldu might be able
> to re-use some codes if ldu supports drm_panic, I re-write this
> v2 patch with option 2.
>
> Ryosuke Yasuoka (1):
>   drm/vmwgfx: add drm_panic support for stdu
>
>  drivers/gpu/drm/vmwgfx/vmwgfx_cmd.c  | 73 ++++++++++++++++++++++++++++
>  drivers/gpu/drm/vmwgfx/vmwgfx_drv.h  | 18 +++++++
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c  |  9 ++++
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.h  |  1 +
>  drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c  | 43 ++++++++++++++++
>  drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 34 +++++++++++++
>  6 files changed, 178 insertions(+)
>
>
> base-commit: 306c8959b5fdfdc3900aebe4e56257a349f5765f
> --
> 2.51.0
>

This is the history of this patch.

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

v2:
- 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.

Regards,
Ryosuke

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

[Ian Forbes]

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

On Mon, Sep 8, 2025 at 9:12 AM Ryosuke Yasuoka <ryasuoka@redhat.com> wrote:

> +static int
> +vmw_stdu_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);
> +
> +       if (!plane->state || !plane->state->fb || !plane->state->visible)
> +               return -ENODEV;
> +
> +       sb->format = plane->state->fb->format;
> +       sb->width = plane->state->fb->width;
> +       sb->height = plane->state->fb->height;
> +       sb->pitch[0] = plane->state->fb->pitches[0];
> +
> +       u64 size = sb->height * sb->pitch[0];
> +
> +       sb->map[0].vaddr = memremap(dev_priv->vram_start, size, MEMREMAP_WT);
> +
> +       if (!sb->map[0].vaddr)
> +               return -ENOMEM;
> +
> +       return 0;
> +}
> +
> +static void vmw_stdu_primary_plane_panic_flush(struct drm_plane *plane)
> +{
> +       vmw_ldu_primary_plane_panic_flush(plane);
> +}
> +
>  static void
>  vmw_stdu_crtc_atomic_flush(struct drm_crtc *crtc,
>                            struct drm_atomic_state *state)
> @@ -1506,6 +1538,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_stdu_primary_plane_get_scanout_buffer,
> +       .panic_flush = vmw_stdu_primary_plane_panic_flush,
>  };
>
>  static const struct drm_crtc_helper_funcs vmw_stdu_crtc_helper_funcs = {
> --
> 2.51.0
>

You only need the code I've highlighted in this reply with some minor changes.

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.
2. The format should be hardcoded to RGB565 to minimize the risk of
overrunning VRAM. Adjust the pitch accordingly to 2x width.
3. The sizes should be vmw_priv->initial_width, and
vmw_priv->initial_height. These are the safe sizes for VRAM in early
boot and will work in panic as well.
4. You want to ensure `get_scanout_buffer` only succeeds once when
using display unit 0 since all calls to this function will return the
same VRAM.
5. memremap flags should be `MEMREMAP_WB | MEMREMAP_DEC`
6. Move the panic related functions to the vmwgfx_kms.c files since
they work in all DU modes.

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

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

[Ryosuke Yasuoka]

On Tue, Sep 09, 2025 at 03:48:01PM -0500, Ian Forbes wrote:
> On Mon, Sep 8, 2025 at 9:12 AM Ryosuke Yasuoka <ryasuoka@redhat.com> wrote:
> 
> > +static int
> > +vmw_stdu_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);
> > +
> > +       if (!plane->state || !plane->state->fb || !plane->state->visible)
> > +               return -ENODEV;
> > +
> > +       sb->format = plane->state->fb->format;
> > +       sb->width = plane->state->fb->width;
> > +       sb->height = plane->state->fb->height;
> > +       sb->pitch[0] = plane->state->fb->pitches[0];
> > +
> > +       u64 size = sb->height * sb->pitch[0];
> > +
> > +       sb->map[0].vaddr = memremap(dev_priv->vram_start, size, MEMREMAP_WT);
> > +
> > +       if (!sb->map[0].vaddr)
> > +               return -ENOMEM;
> > +
> > +       return 0;
> > +}
> > +
> > +static void vmw_stdu_primary_plane_panic_flush(struct drm_plane *plane)
> > +{
> > +       vmw_ldu_primary_plane_panic_flush(plane);
> > +}
> > +
> >  static void
> >  vmw_stdu_crtc_atomic_flush(struct drm_crtc *crtc,
> >                            struct drm_atomic_state *state)
> > @@ -1506,6 +1538,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_stdu_primary_plane_get_scanout_buffer,
> > +       .panic_flush = vmw_stdu_primary_plane_panic_flush,
> >  };
> >
> >  static const struct drm_crtc_helper_funcs vmw_stdu_crtc_helper_funcs = {
> > --
> > 2.51.0
> >
> 
> You only need the code I've highlighted in this reply with some minor changes.
> 
> 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, it will be unstable without dirty command's
submission. In test environment, which is Virtual box, I've
observed serveral instances of the panic screen appearing.
It sometimes appears immediately as expected, and at other times,
there's a delay of about 15 to 20 seconds to appear the screen.
Another pattern, it's necessary to switch back and forth between
the Virtual Box console window and other windows.

With command submission, we can stably get a panic screen. Even if it
failed due to some reasons, we may get the panic screen ultimately. So I
think we should leave vmw_kms_ldu_panic_do_bo_dirty() to submit
commands.

What do you think?

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

I see. drm panic supports multiple pitches. Checking pitch type 
in get_scanout_buffer is not difficult.

> 3. The sizes should be vmw_priv->initial_width, and
> vmw_priv->initial_height. These are the safe sizes for VRAM in early
> boot and will work in panic as well.

OK. I'll try to change this.

> 4. You want to ensure `get_scanout_buffer` only succeeds once when
> using display unit 0 since all calls to this function will return the
> same VRAM.

OK. I'll add a check in get_scanout_buffer. To confirm this, Is it
sufficient to check if lds->num_active is 1, or do I need to check
other places or values?

> 5. memremap flags should be `MEMREMAP_WB | MEMREMAP_DEC`

OK. I'll chenge the flag like this.

> 6. Move the panic related functions to the vmwgfx_kms.c files since
> they work in all DU modes.

OK. I'll try to move them in vmwgfx_kms.c

Thank you for your review and detailed comment.

Ryosuke