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

Ryosuke Yasuoka posted 1 patch 4 months, 3 weeks ago
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(+)
[PATCH drm-misc-next v3 0/1] add drm_panic_support for vmwgfx-stdu
Posted by Ryosuke Yasuoka 4 months, 3 weeks ago
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
Re: [PATCH drm-misc-next v3 0/1] add drm_panic_support for vmwgfx-stdu
Posted by Ryosuke Yasuoka 3 months, 3 weeks ago
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
Posted by Ian Forbes 3 months, 2 weeks ago
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
>