All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/vmwgfx: Add drm_panic support
@ 2025-10-23 20:04 Ian Forbes
  2025-10-27 13:56 ` Jocelyn Falempe
  2025-11-07 20:46 ` [PATCH v2] " Ian Forbes
  0 siblings, 2 replies; 9+ messages in thread
From: Ian Forbes @ 2025-10-23 20:04 UTC (permalink / raw)
  To: dri-devel
  Cc: bcm-kernel-feedback-list, zack.rusin, maaz.mombasawala,
	Ian Forbes, Ryosuke Yasuoka

Sets up VRAM as the scanout buffer then switches to legacy mode.

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);
+	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);
+}
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 = {
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] drm/vmwgfx: Add drm_panic support
  2025-10-23 20:04 [PATCH] drm/vmwgfx: Add drm_panic support Ian Forbes
@ 2025-10-27 13:56 ` Jocelyn Falempe
  2025-10-28 11:51   ` Ryosuke Yasuoka
  2025-11-07 20:46 ` [PATCH v2] " Ian Forbes
  1 sibling, 1 reply; 9+ messages in thread
From: Jocelyn Falempe @ 2025-10-27 13:56 UTC (permalink / raw)
  To: Ian Forbes, dri-devel
  Cc: bcm-kernel-feedback-list, zack.rusin, maaz.mombasawala,
	Ryosuke Yasuoka

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);
> +	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);
> +}
> 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 = {



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] drm/vmwgfx: Add drm_panic support
  2025-10-27 13:56 ` Jocelyn Falempe
@ 2025-10-28 11:51   ` Ryosuke Yasuoka
  2025-10-28 13:55     ` Jocelyn Falempe
  0 siblings, 1 reply; 9+ messages in thread
From: Ryosuke Yasuoka @ 2025-10-28 11:51 UTC (permalink / raw)
  To: Ian Forbes
  Cc: dri-devel, bcm-kernel-feedback-list, zack.rusin, maaz.mombasawala,
	jfalempe

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 = {
> 
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] drm/vmwgfx: Add drm_panic support
  2025-10-28 11:51   ` Ryosuke Yasuoka
@ 2025-10-28 13:55     ` Jocelyn Falempe
  0 siblings, 0 replies; 9+ messages in thread
From: Jocelyn Falempe @ 2025-10-28 13:55 UTC (permalink / raw)
  To: Ryosuke Yasuoka, Ian Forbes
  Cc: dri-devel, bcm-kernel-feedback-list, zack.rusin, maaz.mombasawala

On 28/10/2025 12:51, Ryosuke Yasuoka wrote:
> 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?

The debugfs interface is broken by design, it's just here to help adding 
new device support to drm_panic, so not a problem if there are garbage 
after you trigger it. (it's the case on most driver anyway).

> 
>>> +	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]!

Maybe another solution is to restrict the panic handler to 
VMWGFX_PCI_ID_SVGA3, that doesn't need locks in vmw_write().

I don't know if there are still a lot of hosts with VMWGFX_PCI_ID_SVGA2 
in the open, and if we want to add drm_panic support for them.

Best regards,

-- 

Jocelyn

> 
> [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 = {
>>
>>
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2] drm/vmwgfx: Add drm_panic support
  2025-10-23 20:04 [PATCH] drm/vmwgfx: Add drm_panic support Ian Forbes
  2025-10-27 13:56 ` Jocelyn Falempe
@ 2025-11-07 20:46 ` Ian Forbes
  2025-11-12  9:14   ` Thomas Zimmermann
  2025-11-12  9:23   ` Thomas Zimmermann
  1 sibling, 2 replies; 9+ messages in thread
From: Ian Forbes @ 2025-11-07 20:46 UTC (permalink / raw)
  To: dri-devel
  Cc: bcm-kernel-feedback-list, zack.rusin, maaz.mombasawala,
	Ian Forbes, Ryosuke Yasuoka

Sets up VRAM as the scanout buffer then switches to legacy mode.

Suggested-by: Ryosuke Yasuoka <ryasuoka@redhat.com>
Signed-off-by: Ian Forbes <ian.forbes@broadcom.com>
---

v2:
 - Set SVGA_REG_CONFIG_DONE=false so that SVGA3 works correctly

 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c  | 35 ++++++++++++++++++++++++++++
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.h  |  5 ++++
 drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c |  2 ++
 3 files changed, 42 insertions(+)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index efdbb67a4966..87448e86d3b3 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)
 {
@@ -2025,3 +2026,37 @@ 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;
+
+	vmw_write(vmw_priv, SVGA_REG_CONFIG_DONE, false);
+
+	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);
+	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);
+}
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 add13294fb7c..faacfef7baa5 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 = {
-- 
2.51.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] drm/vmwgfx: Add drm_panic support
  2025-11-07 20:46 ` [PATCH v2] " Ian Forbes
@ 2025-11-12  9:14   ` Thomas Zimmermann
  2025-11-12  9:23   ` Thomas Zimmermann
  1 sibling, 0 replies; 9+ messages in thread
From: Thomas Zimmermann @ 2025-11-12  9:14 UTC (permalink / raw)
  To: Ian Forbes, dri-devel
  Cc: bcm-kernel-feedback-list, zack.rusin, maaz.mombasawala,
	Ryosuke Yasuoka

Hi

Am 07.11.25 um 21:46 schrieb Ian Forbes:
> Sets up VRAM as the scanout buffer then switches to legacy mode.
>
> Suggested-by: Ryosuke Yasuoka <ryasuoka@redhat.com>
> Signed-off-by: Ian Forbes <ian.forbes@broadcom.com>
> ---
>
> v2:
>   - Set SVGA_REG_CONFIG_DONE=false so that SVGA3 works correctly
>
>   drivers/gpu/drm/vmwgfx/vmwgfx_kms.c  | 35 ++++++++++++++++++++++++++++
>   drivers/gpu/drm/vmwgfx/vmwgfx_kms.h  |  5 ++++
>   drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c |  2 ++
>   3 files changed, 42 insertions(+)
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> index efdbb67a4966..87448e86d3b3 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)
>   {
> @@ -2025,3 +2026,37 @@ 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;
> +
> +	vmw_write(vmw_priv, SVGA_REG_CONFIG_DONE, false);
> +
> +	vram = memremap(vmw_priv->vram_start, vmw_priv->vram_size,
> +			MEMREMAP_WB | MEMREMAP_DEC);

Does that really work? We just had a kernel panic, so remapping might be 
difficult.

> +	if (!vram)
> +		return -ENOMEM;
> +
> +	sb->map[0].vaddr = vram;
> +	sb->format = drm_format_info(DRM_FORMAT_RGB565);
> +	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);
> +}

I think we should consider adding a callback table for these operations. 
There's flush here and the vmap above is another possible candidate.

Best regards
Thomas

> 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 add13294fb7c..faacfef7baa5 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 = {

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] drm/vmwgfx: Add drm_panic support
  2025-11-07 20:46 ` [PATCH v2] " Ian Forbes
  2025-11-12  9:14   ` Thomas Zimmermann
@ 2025-11-12  9:23   ` Thomas Zimmermann
  2025-11-13 15:49     ` Ian Forbes
  1 sibling, 1 reply; 9+ messages in thread
From: Thomas Zimmermann @ 2025-11-12  9:23 UTC (permalink / raw)
  To: Ian Forbes, dri-devel
  Cc: bcm-kernel-feedback-list, zack.rusin, maaz.mombasawala,
	Ryosuke Yasuoka

Hi

Am 07.11.25 um 21:46 schrieb Ian Forbes:
> Sets up VRAM as the scanout buffer then switches to legacy mode.

Please use make this a bit more elaborate.

Please also note that you don't set up VRAM, but return the scanout 
information for the given plane. That could be a difference if there are 
multiple planes.

>
> Suggested-by: Ryosuke Yasuoka <ryasuoka@redhat.com>
> Signed-off-by: Ian Forbes <ian.forbes@broadcom.com>
> ---
>
> v2:
>   - Set SVGA_REG_CONFIG_DONE=false so that SVGA3 works correctly
>
>   drivers/gpu/drm/vmwgfx/vmwgfx_kms.c  | 35 ++++++++++++++++++++++++++++
>   drivers/gpu/drm/vmwgfx/vmwgfx_kms.h  |  5 ++++
>   drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c |  2 ++
>   3 files changed, 42 insertions(+)
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> index efdbb67a4966..87448e86d3b3 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)
>   {
> @@ -2025,3 +2026,37 @@ 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;
> +
> +	vmw_write(vmw_priv, SVGA_REG_CONFIG_DONE, false);
> +
> +	vram = memremap(vmw_priv->vram_start, vmw_priv->vram_size,
> +			MEMREMAP_WB | MEMREMAP_DEC);
> +	if (!vram)
> +		return -ENOMEM;
> +

> +	sb->map[0].vaddr = vram;

Please use iosys_map_set_vaddr() to initialize this field.

> +	sb->format = drm_format_info(DRM_FORMAT_RGB565);
> +	sb->width  = vmw_priv->initial_width;
> +	sb->height = vmw_priv->initial_height;

Usually, you want to look at the plane's current framebuffer for this 
information.

> +	sb->pitch[0] = sb->width * 2;


Please use drm_format_info_min_pitch() to compute this value or set 
whatever has been programmed to the hardware already.

Best regards
Thomas


> +	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);
> +}
> 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 add13294fb7c..faacfef7baa5 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 = {

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] drm/vmwgfx: Add drm_panic support
  2025-11-12  9:23   ` Thomas Zimmermann
@ 2025-11-13 15:49     ` Ian Forbes
  2025-11-14  7:04       ` Thomas Zimmermann
  0 siblings, 1 reply; 9+ messages in thread
From: Ian Forbes @ 2025-11-13 15:49 UTC (permalink / raw)
  To: Thomas Zimmermann
  Cc: dri-devel, bcm-kernel-feedback-list, zack.rusin, maaz.mombasawala,
	Ryosuke Yasuoka

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

On Wed, Nov 12, 2025 at 3:23 AM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> Hi
>
> Am 07.11.25 um 21:46 schrieb Ian Forbes:
> > Sets up VRAM as the scanout buffer then switches to legacy mode.
>
> Please use make this a bit more elaborate.
>
> Please also note that you don't set up VRAM, but return the scanout
> information for the given plane. That could be a difference if there are
> multiple planes.
>

"VRAM" is a per device. It's not tied to a plane which is why we check
the display
unit ID to call this only once. It requires setup by mapping the mmio
region and poking
a register (CONFIG_DONE = false). I'm not sure how else to describe it
other than
"setup".

>
> Usually, you want to look at the plane's current framebuffer for this
> information.
>

The Screen Target (default) mode allows resolutions larger than
legacy/register mode can handle.
These sizes are guaranteed to be safe for legacy mode.

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] drm/vmwgfx: Add drm_panic support
  2025-11-13 15:49     ` Ian Forbes
@ 2025-11-14  7:04       ` Thomas Zimmermann
  0 siblings, 0 replies; 9+ messages in thread
From: Thomas Zimmermann @ 2025-11-14  7:04 UTC (permalink / raw)
  To: Ian Forbes
  Cc: dri-devel, bcm-kernel-feedback-list, zack.rusin, maaz.mombasawala,
	Ryosuke Yasuoka

Hi

Am 13.11.25 um 16:49 schrieb Ian Forbes:
> On Wed, Nov 12, 2025 at 3:23 AM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>> Hi
>>
>> Am 07.11.25 um 21:46 schrieb Ian Forbes:
>>> Sets up VRAM as the scanout buffer then switches to legacy mode.
>> Please use make this a bit more elaborate.
>>
>> Please also note that you don't set up VRAM, but return the scanout
>> information for the given plane. That could be a difference if there are
>> multiple planes.
>>
> "VRAM" is a per device. It's not tied to a plane which is why we check
> the display
> unit ID to call this only once. It requires setup by mapping the mmio
> region and poking
> a register (CONFIG_DONE = false). I'm not sure how else to describe it
> other than
> "setup".

Yeah, sure. The question was: does it still work if you run this code 
for multiple planes? The DRM core will try to find multiple scanout 
buffers, if necessary.

>
>> Usually, you want to look at the plane's current framebuffer for this
>> information.
>>
> The Screen Target (default) mode allows resolutions larger than
> legacy/register mode can handle.
> These sizes are guaranteed to be safe for legacy mode.

Ok.

Best regards
Thomas


-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)



^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2025-11-14  7:04 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-23 20:04 [PATCH] drm/vmwgfx: Add drm_panic support Ian Forbes
2025-10-27 13:56 ` Jocelyn Falempe
2025-10-28 11:51   ` Ryosuke Yasuoka
2025-10-28 13:55     ` Jocelyn Falempe
2025-11-07 20:46 ` [PATCH v2] " Ian Forbes
2025-11-12  9:14   ` Thomas Zimmermann
2025-11-12  9:23   ` Thomas Zimmermann
2025-11-13 15:49     ` Ian Forbes
2025-11-14  7:04       ` Thomas Zimmermann

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.