Re: [PATCH] drm/vmwgfx: Add drm_panic support

[Ryosuke Yasuoka <ryasuoka@redhat.com>]

On Mon, Oct 27, 2025 at 02:56:21PM +0100, Jocelyn Falempe wrote:
> On 23/10/2025 22:04, Ian Forbes wrote:
> > Sets up VRAM as the scanout buffer then switches to legacy mode.
> 
> Thank you and Ryosuke for working on drm_panic support on vmwgfx.
> For the use of the drm_panic API, it looks good to me.
> 
> Acked-by: Jocelyn Falempe <jfalempe@redhat.com>
> > 
> > Suggested-by: Ryosuke Yasuoka <ryasuoka@redhat.com>
> > Signed-off-by: Ian Forbes <ian.forbes@broadcom.com>
> > ---
> >   drivers/gpu/drm/vmwgfx/vmwgfx_kms.c  | 33 ++++++++++++++++++++++++++++
> >   drivers/gpu/drm/vmwgfx/vmwgfx_kms.h  |  5 +++++
> >   drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c |  2 ++
> >   3 files changed, 40 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > index 54ea1b513950..4ff4ae041236 100644
> > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > @@ -20,6 +20,7 @@
> >   #include <drm/drm_rect.h>
> >   #include <drm/drm_sysfs.h>
> >   #include <drm/drm_edid.h>
> > +#include <drm/drm_panic.h>
> >   void vmw_du_init(struct vmw_display_unit *du)
> >   {
> > @@ -2022,3 +2023,35 @@ bool vmw_user_object_is_null(struct vmw_user_object *uo)
> >   {
> >   	return !uo->buffer && !uo->surface;
> >   }
> > +
> > +int
> > +vmw_get_scanout_buffer(struct drm_plane *plane, struct drm_scanout_buffer *sb)
> > +{
> > +	void  *vram;
> > +	struct vmw_private *vmw_priv = container_of(plane->dev, struct vmw_private, drm);
> > +
> > +	// Only call on the primary display
> > +	if (container_of(plane, struct vmw_display_unit, primary)->unit != 0)
> > +		return -EINVAL;
> > +
> > +	vram = memremap(vmw_priv->vram_start, vmw_priv->vram_size,
> > +			MEMREMAP_WB | MEMREMAP_DEC);
> > +	if (!vram)
> > +		return -ENOMEM;
> > +
> > +	sb->map[0].vaddr = vram;
> > +	sb->format = drm_format_info(DRM_FORMAT_RGB565);

Let me confirm whether debugfs feature works correctly. As I mentioned
in my original patch [1], modifying this format will allow to display
the panic screen by debugfs only one time. In your environment, can you
trigger panic screen by debugfs multiple times?

> > +	sb->width  = vmw_priv->initial_width;
> > +	sb->height = vmw_priv->initial_height;
> > +	sb->pitch[0] = sb->width * 2;
> > +	return 0;
> > +}
> > +
> > +void vmw_panic_flush(struct drm_plane *plane)
> > +{
> > +	struct vmw_private *vmw_priv = container_of(plane->dev, struct vmw_private, drm);
> > +
> > +	vmw_kms_write_svga(vmw_priv,
> > +			   vmw_priv->initial_width, vmw_priv->initial_height,
> > +			   vmw_priv->initial_width * 2, 16, 16);

vmw_kms_write_svga() calls vmw_write() which locks spin lock. Since
these functions are called in panic handler, we should avoid them. You
can find some idea in my original patch [1]!

[1] https://lore.kernel.org/all/20250919032936.2267240-1-ryasuoka@redhat.com/

Thank you
Ryosuke

> > +}
> > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
> > index 445471fe9be6..8e37561cd527 100644
> > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
> > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
> > @@ -500,6 +500,11 @@ int vmw_kms_stdu_readback(struct vmw_private *dev_priv,
> >   int vmw_du_helper_plane_update(struct vmw_du_update_plane *update);
> > +struct drm_scanout_buffer;
> > +
> > +int vmw_get_scanout_buffer(struct drm_plane *pl, struct drm_scanout_buffer *sb);
> > +void vmw_panic_flush(struct drm_plane *plane);
> > +
> >   /**
> >    * vmw_du_translate_to_crtc - Translate a rect from framebuffer to crtc
> >    * @state: Plane state.
> > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> > index 20aab725e53a..37cb742ba1d9 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_get_scanout_buffer,
> > +	.panic_flush = vmw_panic_flush,
> >   };
> >   static const struct drm_crtc_helper_funcs vmw_stdu_crtc_helper_funcs = {
> 
>