dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v10 00/10] drm/i915: Add drm_panic support
@ 2025-06-18  9:31 Jocelyn Falempe
  2025-06-18  9:31 ` [PATCH v10 01/10] drm/panic: Add a private field to struct drm_scanout_buffer Jocelyn Falempe
                   ` (10 more replies)
  0 siblings, 11 replies; 20+ messages in thread
From: Jocelyn Falempe @ 2025-06-18  9:31 UTC (permalink / raw)
  To: Maarten Lankhorst, Jani Nikula, Rodrigo Vivi, Joonas Lahtinen,
	Tvrtko Ursulin, Ville Syrjälä, David Airlie,
	Simona Vetter, Christian Koenig, Huang Rui, Matthew Auld,
	Matthew Brost, Maxime Ripard, Thomas Zimmermann, intel-gfx,
	intel-xe, dri-devel, linux-kernel
  Cc: Jocelyn Falempe

This adds drm_panic support for i915 and xe driver.

I've tested it on the 4 intel laptops I have at my disposal.
 * Haswell with 128MB of eDRAM.
 * Comet Lake  i7-10850H
 * Raptor Lake i7-1370P (with DPT, and Y-tiling).
 * Lunar Lake Ultra 5 228V (with DPT, and 4-tiling, and using the Xe driver.

I tested panic in both fbdev console and gnome desktop.
I think it won't work yet on discrete GPU, but that can be added later.

Best regards,

v2:
 * Add the proper abstractions to build also for Xe.
 * Fix dim checkpatch issues.

v3:
 * Add support for Y-tiled framebuffer when DPT is enabled.

v4:
 * Add support for Xe driver, which shares most of the code.
 * Add support for 4-tiled framebuffer found in newest GPU.

v5:
 * Rebase on top of git@gitlab.freedesktop.org:drm/i915/kernel.git drm-intel-next
 * Use struct intel_display instead of drm_i915_private.
 * Use iosys_map for intel_bo_panic_map().

v6:
 * Rebase on top of git@gitlab.freedesktop.org:drm/i915/kernel.git drm-intel-next
 * Use struct intel_display instead of drm_i915_private for intel_atomic_plane.c

v7:
 * Fix mismatch {} in intel_panic_flush() (Jani Nikula)
 * Return int for i915_gem_object_panic_map() (Ville Syrjälä)
 * Reword commit message about alignment/size when disabling tiling (Ville Syrjälä)

v8:
 * Use kmap_try_from_panic() instead of vmap, to access the framebuffer.
 * Add ttm_bo_kmap_try_from_panic() for the xe driver, that uses ttm.
 * Replace intel_bo_panic_map() with a setup() and finish() function,
   to allow mapping only one page of teh framebuffer at a time.
 * Configure psr to send the full framebuffer update.

v9:
 * Fix comment in ttm_bo_kmap_try_from_panic(), this can *only* be called
   from the panic handler (Christian König)
 * Fix missing kfree() for i915_panic_pages in i915_gem_object_panic_finish()
   Also change i915_panic_pages allocation to kmalloc, as kvmalloc is not
   safe to call from the panic handler.
 * Fix dim checkpatch warnings.

v10:
 * Add a private field to struct drm_scanout_buffer
 * Replace static variables with new fields in struct intel_framebuffer
   (Maarten Lankhorst)
 * Add error handling if i915_gem_object_panic_pages() returns NULL
 * Declare struct drm_scanout_buffer instead of including <drm/drm_panic.h>
   in intel_bo.h

Jocelyn Falempe (10):
  drm/panic: Add a private field to struct drm_scanout_buffer
  drm/i915/fbdev: Add intel_fbdev_get_map()
  drm/i915/display/i9xx: Add a disable_tiling() for i9xx planes
  drm/i915/display: Add a disable_tiling() for skl planes
  drm/ttm: Add ttm_bo_kmap_try_from_panic()
  drm/i915: Add intel_bo_panic_setup and intel_bo_panic_finish
  drm/i915/display: Add drm_panic support
  drm/i915/display: Add drm_panic support for Y-tiling with DPT
  drm/i915/display: Add drm_panic support for 4-tiling with DPT
  drm/i915/psr: Add intel_psr2_panic_force_full_update

 drivers/gpu/drm/i915/display/i9xx_plane.c     |  23 +++
 .../gpu/drm/i915/display/intel_atomic_plane.c | 170 +++++++++++++++++-
 drivers/gpu/drm/i915/display/intel_bo.c       |  12 ++
 drivers/gpu/drm/i915/display/intel_bo.h       |   4 +
 .../drm/i915/display/intel_display_types.h    |  11 ++
 drivers/gpu/drm/i915/display/intel_fb_pin.c   |   5 +
 drivers/gpu/drm/i915/display/intel_fb_pin.h   |   2 +
 drivers/gpu/drm/i915/display/intel_fbdev.c    |   5 +
 drivers/gpu/drm/i915/display/intel_fbdev.h    |   6 +-
 drivers/gpu/drm/i915/display/intel_psr.c      |  20 +++
 drivers/gpu/drm/i915/display/intel_psr.h      |   2 +
 .../drm/i915/display/skl_universal_plane.c    |  27 +++
 drivers/gpu/drm/i915/gem/i915_gem_object.h    |   5 +
 drivers/gpu/drm/i915/gem/i915_gem_pages.c     | 112 ++++++++++++
 drivers/gpu/drm/i915/i915_vma.h               |   5 +
 drivers/gpu/drm/ttm/ttm_bo_util.c             |  27 +++
 drivers/gpu/drm/xe/display/intel_bo.c         |  61 +++++++
 drivers/gpu/drm/xe/display/xe_fb_pin.c        |   5 +
 include/drm/drm_panic.h                       |   6 +
 include/drm/ttm/ttm_bo.h                      |   1 +
 20 files changed, 507 insertions(+), 2 deletions(-)


base-commit: b2f7e30d2e4a34fcee8111d713bef4f29dc23c77
-- 
2.49.0


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

* [PATCH v10 01/10] drm/panic: Add a private field to struct drm_scanout_buffer
  2025-06-18  9:31 [PATCH v10 00/10] drm/i915: Add drm_panic support Jocelyn Falempe
@ 2025-06-18  9:31 ` Jocelyn Falempe
  2025-06-18  9:31 ` [PATCH v10 02/10] drm/i915/fbdev: Add intel_fbdev_get_map() Jocelyn Falempe
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 20+ messages in thread
From: Jocelyn Falempe @ 2025-06-18  9:31 UTC (permalink / raw)
  To: Maarten Lankhorst, Jani Nikula, Rodrigo Vivi, Joonas Lahtinen,
	Tvrtko Ursulin, Ville Syrjälä, David Airlie,
	Simona Vetter, Christian Koenig, Huang Rui, Matthew Auld,
	Matthew Brost, Maxime Ripard, Thomas Zimmermann, intel-gfx,
	intel-xe, dri-devel, linux-kernel
  Cc: Jocelyn Falempe

This allows driver to set some private data in get_scanout_buffer(),
and re-use them in set_pixel() callback.

Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
---

v10:
 * Added in v10, to avoid static variables

 include/drm/drm_panic.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/include/drm/drm_panic.h b/include/drm/drm_panic.h
index 310c88c4d336..ac0e46b73436 100644
--- a/include/drm/drm_panic.h
+++ b/include/drm/drm_panic.h
@@ -72,6 +72,12 @@ struct drm_scanout_buffer {
 	void (*set_pixel)(struct drm_scanout_buffer *sb, unsigned int x,
 			  unsigned int y, u32 color);
 
+	/**
+	 * @private: private pointer that you can use in the callbacks
+	 * set_pixel()
+	 */
+	void *private;
+
 };
 
 #ifdef CONFIG_DRM_PANIC
-- 
2.49.0


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

* [PATCH v10 02/10] drm/i915/fbdev: Add intel_fbdev_get_map()
  2025-06-18  9:31 [PATCH v10 00/10] drm/i915: Add drm_panic support Jocelyn Falempe
  2025-06-18  9:31 ` [PATCH v10 01/10] drm/panic: Add a private field to struct drm_scanout_buffer Jocelyn Falempe
@ 2025-06-18  9:31 ` Jocelyn Falempe
  2025-06-18  9:31 ` [PATCH v10 03/10] drm/i915/display/i9xx: Add a disable_tiling() for i9xx planes Jocelyn Falempe
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 20+ messages in thread
From: Jocelyn Falempe @ 2025-06-18  9:31 UTC (permalink / raw)
  To: Maarten Lankhorst, Jani Nikula, Rodrigo Vivi, Joonas Lahtinen,
	Tvrtko Ursulin, Ville Syrjälä, David Airlie,
	Simona Vetter, Christian Koenig, Huang Rui, Matthew Auld,
	Matthew Brost, Maxime Ripard, Thomas Zimmermann, intel-gfx,
	intel-xe, dri-devel, linux-kernel
  Cc: Jocelyn Falempe

The vaddr of the fbdev framebuffer is private to the struct
intel_fbdev, so this function is needed to access it for drm_panic.
Also the struct i915_vma is different between i915 and xe, so it
requires a few functions to access fbdev->vma->iomap.

Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
---

v2:
 * Add intel_fb_get_vaddr() and i915_vma_get_iomap() to build with Xe driver.
 
v4:
 * rename to get_map(), and return the struct iosys_map mapping.
 * implement the Xe variant.

 drivers/gpu/drm/i915/display/intel_fb_pin.c | 5 +++++
 drivers/gpu/drm/i915/display/intel_fb_pin.h | 2 ++
 drivers/gpu/drm/i915/display/intel_fbdev.c  | 5 +++++
 drivers/gpu/drm/i915/display/intel_fbdev.h  | 6 +++++-
 drivers/gpu/drm/i915/i915_vma.h             | 5 +++++
 drivers/gpu/drm/xe/display/xe_fb_pin.c      | 5 +++++
 6 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_fb_pin.c b/drivers/gpu/drm/i915/display/intel_fb_pin.c
index 98a61a7b0b93..b792e9b062d8 100644
--- a/drivers/gpu/drm/i915/display/intel_fb_pin.c
+++ b/drivers/gpu/drm/i915/display/intel_fb_pin.c
@@ -334,3 +334,8 @@ void intel_plane_unpin_fb(struct intel_plane_state *old_plane_state)
 			intel_dpt_unpin_from_ggtt(fb->dpt_vm);
 	}
 }
+
+void intel_fb_get_map(struct i915_vma *vma, struct iosys_map *map)
+{
+	iosys_map_set_vaddr_iomem(map, i915_vma_get_iomap(vma));
+}
diff --git a/drivers/gpu/drm/i915/display/intel_fb_pin.h b/drivers/gpu/drm/i915/display/intel_fb_pin.h
index 01770dbba2e0..81ab79da1af7 100644
--- a/drivers/gpu/drm/i915/display/intel_fb_pin.h
+++ b/drivers/gpu/drm/i915/display/intel_fb_pin.h
@@ -12,6 +12,7 @@ struct drm_framebuffer;
 struct i915_vma;
 struct intel_plane_state;
 struct i915_gtt_view;
+struct iosys_map;
 
 struct i915_vma *
 intel_fb_pin_to_ggtt(const struct drm_framebuffer *fb,
@@ -27,5 +28,6 @@ void intel_fb_unpin_vma(struct i915_vma *vma, unsigned long flags);
 int intel_plane_pin_fb(struct intel_plane_state *new_plane_state,
 		       const struct intel_plane_state *old_plane_state);
 void intel_plane_unpin_fb(struct intel_plane_state *old_plane_state);
+void intel_fb_get_map(struct i915_vma *vma, struct iosys_map *map);
 
 #endif
diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c b/drivers/gpu/drm/i915/display/intel_fbdev.c
index 2dc4029d71ed..7c4709d58aa3 100644
--- a/drivers/gpu/drm/i915/display/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
@@ -512,3 +512,8 @@ struct i915_vma *intel_fbdev_vma_pointer(struct intel_fbdev *fbdev)
 {
 	return fbdev ? fbdev->vma : NULL;
 }
+
+void intel_fbdev_get_map(struct intel_fbdev *fbdev, struct iosys_map *map)
+{
+	intel_fb_get_map(fbdev->vma, map);
+}
diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.h b/drivers/gpu/drm/i915/display/intel_fbdev.h
index a15e3e222a0c..150cc5f45bb3 100644
--- a/drivers/gpu/drm/i915/display/intel_fbdev.h
+++ b/drivers/gpu/drm/i915/display/intel_fbdev.h
@@ -13,6 +13,7 @@ struct drm_fb_helper_surface_size;
 struct intel_display;
 struct intel_fbdev;
 struct intel_framebuffer;
+struct iosys_map;
 
 #ifdef CONFIG_DRM_FBDEV_EMULATION
 int intel_fbdev_driver_fbdev_probe(struct drm_fb_helper *helper,
@@ -22,7 +23,7 @@ int intel_fbdev_driver_fbdev_probe(struct drm_fb_helper *helper,
 void intel_fbdev_setup(struct intel_display *display);
 struct intel_framebuffer *intel_fbdev_framebuffer(struct intel_fbdev *fbdev);
 struct i915_vma *intel_fbdev_vma_pointer(struct intel_fbdev *fbdev);
-
+void intel_fbdev_get_map(struct intel_fbdev *fbdev, struct iosys_map *map);
 #else
 #define INTEL_FBDEV_DRIVER_OPS \
 	.fbdev_probe = NULL
@@ -39,6 +40,9 @@ static inline struct i915_vma *intel_fbdev_vma_pointer(struct intel_fbdev *fbdev
 	return NULL;
 }
 
+static inline void intel_fbdev_get_map(struct intel_fbdev *fbdev, struct iosys_map *map)
+{
+}
 #endif
 
 #endif /* __INTEL_FBDEV_H__ */
diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h
index 6a6be8048aa8..4ae610927fa7 100644
--- a/drivers/gpu/drm/i915/i915_vma.h
+++ b/drivers/gpu/drm/i915/i915_vma.h
@@ -353,6 +353,11 @@ static inline bool i915_node_color_differs(const struct drm_mm_node *node,
 	return drm_mm_node_allocated(node) && node->color != color;
 }
 
+static inline void __iomem *i915_vma_get_iomap(struct i915_vma *vma)
+{
+	return READ_ONCE(vma->iomap);
+}
+
 /**
  * i915_vma_pin_iomap - calls ioremap_wc to map the GGTT VMA via the aperture
  * @vma: VMA to iomap
diff --git a/drivers/gpu/drm/xe/display/xe_fb_pin.c b/drivers/gpu/drm/xe/display/xe_fb_pin.c
index 461ecdfdb742..ac9a5ba363b2 100644
--- a/drivers/gpu/drm/xe/display/xe_fb_pin.c
+++ b/drivers/gpu/drm/xe/display/xe_fb_pin.c
@@ -465,3 +465,8 @@ u64 intel_dpt_offset(struct i915_vma *dpt_vma)
 {
 	return 0;
 }
+
+void intel_fb_get_map(struct i915_vma *vma, struct iosys_map *map)
+{
+	*map = vma->bo->vmap;
+}
-- 
2.49.0


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

* [PATCH v10 03/10] drm/i915/display/i9xx: Add a disable_tiling() for i9xx planes
  2025-06-18  9:31 [PATCH v10 00/10] drm/i915: Add drm_panic support Jocelyn Falempe
  2025-06-18  9:31 ` [PATCH v10 01/10] drm/panic: Add a private field to struct drm_scanout_buffer Jocelyn Falempe
  2025-06-18  9:31 ` [PATCH v10 02/10] drm/i915/fbdev: Add intel_fbdev_get_map() Jocelyn Falempe
@ 2025-06-18  9:31 ` Jocelyn Falempe
  2025-07-19 18:23   ` Ville Syrjälä
  2025-06-18  9:31 ` [PATCH v10 04/10] drm/i915/display: Add a disable_tiling() for skl planes Jocelyn Falempe
                   ` (7 subsequent siblings)
  10 siblings, 1 reply; 20+ messages in thread
From: Jocelyn Falempe @ 2025-06-18  9:31 UTC (permalink / raw)
  To: Maarten Lankhorst, Jani Nikula, Rodrigo Vivi, Joonas Lahtinen,
	Tvrtko Ursulin, Ville Syrjälä, David Airlie,
	Simona Vetter, Christian Koenig, Huang Rui, Matthew Auld,
	Matthew Brost, Maxime Ripard, Thomas Zimmermann, intel-gfx,
	intel-xe, dri-devel, linux-kernel
  Cc: Jocelyn Falempe

drm_panic draws in linear framebuffer, so it's easier to re-use the
current framebuffer, and disable tiling in the panic handler, to show
the panic screen.
This assumes that the alignment restriction is always smaller in
linear than in tiled.
It also assumes that the linear framebuffer size is always smaller
than the tiled.

Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
---

v7:
 * Reword commit message about alignment/size when disabling tiling (Ville Syrjälä)

 drivers/gpu/drm/i915/display/i9xx_plane.c     | 23 +++++++++++++++++++
 .../drm/i915/display/intel_display_types.h    |  2 ++
 2 files changed, 25 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/i9xx_plane.c b/drivers/gpu/drm/i915/display/i9xx_plane.c
index 8f15333a4b07..0807fae12450 100644
--- a/drivers/gpu/drm/i915/display/i9xx_plane.c
+++ b/drivers/gpu/drm/i915/display/i9xx_plane.c
@@ -905,6 +905,27 @@ static const struct drm_plane_funcs i8xx_plane_funcs = {
 	.format_mod_supported_async = intel_plane_format_mod_supported_async,
 };
 
+static void i9xx_disable_tiling(struct intel_plane *plane)
+{
+	struct intel_display *display = to_intel_display(plane);
+	enum i9xx_plane_id i9xx_plane = plane->i9xx_plane;
+	u32 dspcntr;
+	u32 reg;
+
+	dspcntr = intel_de_read_fw(display, DSPCNTR(display, i9xx_plane));
+	dspcntr &= ~DISP_TILED;
+	intel_de_write_fw(display, DSPCNTR(display, i9xx_plane), dspcntr);
+
+	if (DISPLAY_VER(display) >= 4) {
+		reg = intel_de_read_fw(display, DSPSURF(display, i9xx_plane));
+		intel_de_write_fw(display, DSPSURF(display, i9xx_plane), reg);
+
+	} else {
+		reg = intel_de_read_fw(display, DSPADDR(display, i9xx_plane));
+		intel_de_write_fw(display, DSPADDR(display, i9xx_plane), reg);
+	}
+}
+
 struct intel_plane *
 intel_primary_plane_create(struct intel_display *display, enum pipe pipe)
 {
@@ -1047,6 +1068,8 @@ intel_primary_plane_create(struct intel_display *display, enum pipe pipe)
 		}
 	}
 
+	plane->disable_tiling = i9xx_disable_tiling;
+
 	modifiers = intel_fb_plane_get_modifiers(display, INTEL_PLANE_CAP_TILING_X);
 
 	if (DISPLAY_VER(display) >= 5 || display->platform.g4x)
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index ed4d743fc7c5..3654d88e9c5f 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1517,6 +1517,8 @@ struct intel_plane {
 			   bool async_flip);
 	void (*enable_flip_done)(struct intel_plane *plane);
 	void (*disable_flip_done)(struct intel_plane *plane);
+	/* For drm_panic */
+	void (*disable_tiling)(struct intel_plane *plane);
 };
 
 #define to_intel_atomic_state(x) container_of(x, struct intel_atomic_state, base)
-- 
2.49.0


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

* [PATCH v10 04/10] drm/i915/display: Add a disable_tiling() for skl planes
  2025-06-18  9:31 [PATCH v10 00/10] drm/i915: Add drm_panic support Jocelyn Falempe
                   ` (2 preceding siblings ...)
  2025-06-18  9:31 ` [PATCH v10 03/10] drm/i915/display/i9xx: Add a disable_tiling() for i9xx planes Jocelyn Falempe
@ 2025-06-18  9:31 ` Jocelyn Falempe
  2025-06-18  9:31 ` [PATCH v10 05/10] drm/ttm: Add ttm_bo_kmap_try_from_panic() Jocelyn Falempe
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 20+ messages in thread
From: Jocelyn Falempe @ 2025-06-18  9:31 UTC (permalink / raw)
  To: Maarten Lankhorst, Jani Nikula, Rodrigo Vivi, Joonas Lahtinen,
	Tvrtko Ursulin, Ville Syrjälä, David Airlie,
	Simona Vetter, Christian Koenig, Huang Rui, Matthew Auld,
	Matthew Brost, Maxime Ripard, Thomas Zimmermann, intel-gfx,
	intel-xe, dri-devel, linux-kernel
  Cc: Jocelyn Falempe

drm_panic draws in linear framebuffer, so it's easier to re-use the
current framebuffer, and disable tiling in the panic handler, to show
the panic screen.

This assumes that the alignment restriction is always smaller in
linear than in tiled.
It also assumes that the linear framebuffer size is always smaller
than the tiled.

Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
---

v7:
 * Reword commit message about alignment/size when disabling tiling (Ville Syrjälä)

 .../drm/i915/display/skl_universal_plane.c    | 20 +++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
index 2aa64482d44b..ef09ef7b101f 100644
--- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
+++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
@@ -2792,6 +2792,25 @@ static u8 tgl_plane_caps(struct intel_display *display,
 	return caps;
 }
 
+static void skl_disable_tiling(struct intel_plane *plane)
+{
+	struct intel_plane_state *state = to_intel_plane_state(plane->base.state);
+	struct intel_display *display = to_intel_display(plane);
+	u32 stride = state->view.color_plane[0].scanout_stride / 64;
+	u32 plane_ctl;
+
+	plane_ctl = intel_de_read(display, PLANE_CTL(plane->pipe, plane->id));
+	plane_ctl &= ~PLANE_CTL_TILED_MASK;
+
+	intel_de_write_fw(display, PLANE_STRIDE(plane->pipe, plane->id),
+			  PLANE_STRIDE_(stride));
+
+	intel_de_write_fw(display, PLANE_CTL(plane->pipe, plane->id), plane_ctl);
+
+	intel_de_write_fw(display, PLANE_SURF(plane->pipe, plane->id),
+			  skl_plane_surf(state, 0));
+}
+
 struct intel_plane *
 skl_universal_plane_create(struct intel_display *display,
 			   enum pipe pipe, enum plane_id plane_id)
@@ -2838,6 +2857,7 @@ skl_universal_plane_create(struct intel_display *display,
 		plane->max_height = skl_plane_max_height;
 		plane->min_cdclk = skl_plane_min_cdclk;
 	}
+	plane->disable_tiling = skl_disable_tiling;
 
 	if (DISPLAY_VER(display) >= 13)
 		plane->max_stride = adl_plane_max_stride;
-- 
2.49.0


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

* [PATCH v10 05/10] drm/ttm: Add ttm_bo_kmap_try_from_panic()
  2025-06-18  9:31 [PATCH v10 00/10] drm/i915: Add drm_panic support Jocelyn Falempe
                   ` (3 preceding siblings ...)
  2025-06-18  9:31 ` [PATCH v10 04/10] drm/i915/display: Add a disable_tiling() for skl planes Jocelyn Falempe
@ 2025-06-18  9:31 ` Jocelyn Falempe
  2025-06-18 13:55   ` Christian König
  2025-06-18  9:31 ` [PATCH v10 06/10] drm/i915: Add intel_bo_panic_setup and intel_bo_panic_finish Jocelyn Falempe
                   ` (5 subsequent siblings)
  10 siblings, 1 reply; 20+ messages in thread
From: Jocelyn Falempe @ 2025-06-18  9:31 UTC (permalink / raw)
  To: Maarten Lankhorst, Jani Nikula, Rodrigo Vivi, Joonas Lahtinen,
	Tvrtko Ursulin, Ville Syrjälä, David Airlie,
	Simona Vetter, Christian Koenig, Huang Rui, Matthew Auld,
	Matthew Brost, Maxime Ripard, Thomas Zimmermann, intel-gfx,
	intel-xe, dri-devel, linux-kernel
  Cc: Jocelyn Falempe

If the ttm bo is backed by pages, then it's possible to safely kmap
one page at a time, using kmap_try_from_panic().
Unfortunately there is no way to do the same with ioremap, so it
only supports the kmap case.
This is needed for proper drm_panic support with xe driver.

Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
---

v8:
 * Added in v8

v9:
 * Fix comment in ttm_bo_kmap_try_from_panic(), this can *only* be called
   from the panic handler (Christian König)

 drivers/gpu/drm/ttm/ttm_bo_util.c | 27 +++++++++++++++++++++++++++
 include/drm/ttm/ttm_bo.h          |  1 +
 2 files changed, 28 insertions(+)

diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
index 15cab9bda17f..6912e6dfda25 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -377,6 +377,33 @@ static int ttm_bo_kmap_ttm(struct ttm_buffer_object *bo,
 	return (!map->virtual) ? -ENOMEM : 0;
 }
 
+/**
+ *
+ * ttm_bo_kmap_try_from_panic
+ *
+ * @bo: The buffer object
+ * @page: The page to map
+ *
+ * Sets up a kernel virtual mapping using kmap_local_page_try_from_panic().
+ * This should only be called from the panic handler, if you make sure the bo
+ * is the one being displayed, so is properly allocated, and protected.
+ *
+ * Returns the vaddr, that you can use to write to the bo, and that you should
+ * pass to kunmap_local() when you're done with this page, or NULL if the bo
+ * is in iomem.
+ */
+void *ttm_bo_kmap_try_from_panic(struct ttm_buffer_object *bo, unsigned long page)
+{
+	if (page + 1 > PFN_UP(bo->resource->size))
+		return NULL;
+
+	if (!bo->resource->bus.is_iomem && bo->ttm->pages && bo->ttm->pages[page])
+		return kmap_local_page_try_from_panic(bo->ttm->pages[page]);
+
+	return NULL;
+}
+EXPORT_SYMBOL(ttm_bo_kmap_try_from_panic);
+
 /**
  * ttm_bo_kmap
  *
diff --git a/include/drm/ttm/ttm_bo.h b/include/drm/ttm/ttm_bo.h
index cf027558b6db..8c0ce3fa077f 100644
--- a/include/drm/ttm/ttm_bo.h
+++ b/include/drm/ttm/ttm_bo.h
@@ -429,6 +429,7 @@ int ttm_bo_init_validate(struct ttm_device *bdev, struct ttm_buffer_object *bo,
 int ttm_bo_kmap(struct ttm_buffer_object *bo, unsigned long start_page,
 		unsigned long num_pages, struct ttm_bo_kmap_obj *map);
 void ttm_bo_kunmap(struct ttm_bo_kmap_obj *map);
+void *ttm_bo_kmap_try_from_panic(struct ttm_buffer_object *bo, unsigned long page);
 int ttm_bo_vmap(struct ttm_buffer_object *bo, struct iosys_map *map);
 void ttm_bo_vunmap(struct ttm_buffer_object *bo, struct iosys_map *map);
 int ttm_bo_mmap_obj(struct vm_area_struct *vma, struct ttm_buffer_object *bo);
-- 
2.49.0


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

* [PATCH v10 06/10] drm/i915: Add intel_bo_panic_setup and intel_bo_panic_finish
  2025-06-18  9:31 [PATCH v10 00/10] drm/i915: Add drm_panic support Jocelyn Falempe
                   ` (4 preceding siblings ...)
  2025-06-18  9:31 ` [PATCH v10 05/10] drm/ttm: Add ttm_bo_kmap_try_from_panic() Jocelyn Falempe
@ 2025-06-18  9:31 ` Jocelyn Falempe
  2025-06-18  9:31 ` [PATCH v10 07/10] drm/i915/display: Add drm_panic support Jocelyn Falempe
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 20+ messages in thread
From: Jocelyn Falempe @ 2025-06-18  9:31 UTC (permalink / raw)
  To: Maarten Lankhorst, Jani Nikula, Rodrigo Vivi, Joonas Lahtinen,
	Tvrtko Ursulin, Ville Syrjälä, David Airlie,
	Simona Vetter, Christian Koenig, Huang Rui, Matthew Auld,
	Matthew Brost, Maxime Ripard, Thomas Zimmermann, intel-gfx,
	intel-xe, dri-devel, linux-kernel
  Cc: Jocelyn Falempe

Implement both functions for i915 and xe, they prepare the work for
drm_panic support.
They both use kmap_try_from_panic(), and map one page at a time, to
write the panic screen on the framebuffer.

Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
---

v5:
 * Use iosys_map for intel_bo_panic_map().

v7:
 * Return int for i915_gem_object_panic_map() (Ville Syrjälä)

v8:
 * Complete rewrite, to use kmap_try_from_panic() which is safe
   to call from a panic handler.

v9:
 * Fix missing kfree() for i915_panic_pages in i915_gem_object_panic_finish()
   Also change i915_panic_pages allocation to kmalloc, as kvmalloc is not
   safe to call from the panic handler.

v10:
 * Replace static variables with new fields in struct intel_framebuffer
   (Maarten Lankhorst)
 * Add error handling if i915_gem_object_panic_pages() returns NULL
 * Declare struct drm_scanout_buffer instead of including <drm/drm_panic.h>
   in intel_bo.h

 drivers/gpu/drm/i915/display/intel_bo.c       | 12 +++
 drivers/gpu/drm/i915/display/intel_bo.h       |  4 +
 .../drm/i915/display/intel_display_types.h    |  8 ++
 drivers/gpu/drm/i915/gem/i915_gem_object.h    |  5 +
 drivers/gpu/drm/i915/gem/i915_gem_pages.c     | 98 +++++++++++++++++++
 drivers/gpu/drm/xe/display/intel_bo.c         | 58 +++++++++++
 6 files changed, 185 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_bo.c b/drivers/gpu/drm/i915/display/intel_bo.c
index fbd16d7b58d9..4e8ffeec6180 100644
--- a/drivers/gpu/drm/i915/display/intel_bo.c
+++ b/drivers/gpu/drm/i915/display/intel_bo.c
@@ -1,6 +1,8 @@
 // SPDX-License-Identifier: MIT
 /* Copyright © 2024 Intel Corporation */
 
+#include <drm/drm_panic.h>
+#include "display/intel_display_types.h"
 #include "gem/i915_gem_mman.h"
 #include "gem/i915_gem_object.h"
 #include "gem/i915_gem_object_frontbuffer.h"
@@ -57,3 +59,13 @@ void intel_bo_describe(struct seq_file *m, struct drm_gem_object *obj)
 {
 	i915_debugfs_describe_obj(m, to_intel_bo(obj));
 }
+
+int intel_bo_panic_setup(struct drm_scanout_buffer *sb)
+{
+	return i915_gem_object_panic_setup(sb);
+}
+
+void intel_bo_panic_finish(struct intel_framebuffer *fb)
+{
+	return i915_gem_object_panic_finish(fb);
+}
diff --git a/drivers/gpu/drm/i915/display/intel_bo.h b/drivers/gpu/drm/i915/display/intel_bo.h
index ea7a2253aaa5..a1a6a5329f91 100644
--- a/drivers/gpu/drm/i915/display/intel_bo.h
+++ b/drivers/gpu/drm/i915/display/intel_bo.h
@@ -7,6 +7,8 @@
 #include <linux/types.h>
 
 struct drm_gem_object;
+struct drm_scanout_buffer;
+struct intel_framebuffer;
 struct seq_file;
 struct vm_area_struct;
 
@@ -23,5 +25,7 @@ struct intel_frontbuffer *intel_bo_set_frontbuffer(struct drm_gem_object *obj,
 						   struct intel_frontbuffer *front);
 
 void intel_bo_describe(struct seq_file *m, struct drm_gem_object *obj);
+int intel_bo_panic_setup(struct drm_scanout_buffer *sb);
+void intel_bo_panic_finish(struct intel_framebuffer *fb);
 
 #endif /* __INTEL_BO__ */
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 3654d88e9c5f..6d6d2d948886 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -130,6 +130,12 @@ struct intel_fb_view {
 	} color_plane[4];
 };
 
+struct intel_panic_data {
+	struct page **pages;
+	int page;
+	void *vaddr;
+};
+
 struct intel_framebuffer {
 	struct drm_framebuffer base;
 	struct intel_frontbuffer *frontbuffer;
@@ -145,6 +151,8 @@ struct intel_framebuffer {
 
 	unsigned int min_alignment;
 	unsigned int vtd_guard;
+
+	struct intel_panic_data panic;
 };
 
 enum intel_hotplug_state {
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h b/drivers/gpu/drm/i915/gem/i915_gem_object.h
index c34f41605b46..0fd8faa174ad 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h
@@ -16,7 +16,9 @@
 #include "i915_gem_ww.h"
 #include "i915_vma_types.h"
 
+struct drm_scanout_buffer;
 enum intel_region_id;
+struct intel_framebuffer;
 
 #define obj_to_i915(obj__) to_i915((obj__)->base.dev)
 
@@ -691,6 +693,9 @@ i915_gem_object_unpin_pages(struct drm_i915_gem_object *obj)
 int __i915_gem_object_put_pages(struct drm_i915_gem_object *obj);
 int i915_gem_object_truncate(struct drm_i915_gem_object *obj);
 
+int i915_gem_object_panic_setup(struct drm_scanout_buffer *sb);
+void i915_gem_object_panic_finish(struct intel_framebuffer *fb);
+
 /**
  * i915_gem_object_pin_map - return a contiguous mapping of the entire object
  * @obj: the object to map into kernel address space
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_pages.c b/drivers/gpu/drm/i915/gem/i915_gem_pages.c
index 7f83f8bdc8fb..0bcf4647a2a0 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_pages.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_pages.c
@@ -4,8 +4,11 @@
  */
 
 #include <drm/drm_cache.h>
+#include <drm/drm_panic.h>
 #include <linux/vmalloc.h>
 
+#include "display/intel_fb.h"
+#include "display/intel_display_types.h"
 #include "gt/intel_gt.h"
 #include "gt/intel_tlb.h"
 
@@ -354,6 +357,101 @@ static void *i915_gem_object_map_pfn(struct drm_i915_gem_object *obj,
 	return vaddr ?: ERR_PTR(-ENOMEM);
 }
 
+static void i915_panic_kunmap(struct intel_panic_data *panic)
+{
+	if (panic->vaddr) {
+		drm_clflush_virt_range(panic->vaddr, PAGE_SIZE);
+		kunmap_local(panic->vaddr);
+		panic->vaddr = NULL;
+	}
+}
+
+static struct page **i915_gem_object_panic_pages(struct drm_i915_gem_object *obj)
+{
+	unsigned long n_pages = obj->base.size >> PAGE_SHIFT, i;
+	struct page *page;
+	struct page **pages;
+	struct sgt_iter iter;
+
+	/* For a 3840x2160 32 bits Framebuffer, this should require ~64K */
+	pages = kmalloc_array(n_pages, sizeof(*pages), GFP_ATOMIC);
+	if (!pages)
+		return NULL;
+
+	i = 0;
+	for_each_sgt_page(page, iter, obj->mm.pages)
+		pages[i++] = page;
+	return pages;
+}
+
+/*
+ * The scanout buffer pages are not mapped, so for each pixel,
+ * use kmap_local_page_try_from_panic() to map the page, and write the pixel.
+ * Try to keep the map from the previous pixel, to avoid too much map/unmap.
+ */
+static void i915_gem_object_panic_page_set_pixel(struct drm_scanout_buffer *sb, unsigned int x,
+						 unsigned int y, u32 color)
+{
+	unsigned int new_page;
+	unsigned int offset;
+	struct intel_framebuffer *fb = (struct intel_framebuffer *)sb->private;
+
+	offset = y * sb->pitch[0] + x * sb->format->cpp[0];
+
+	new_page = offset >> PAGE_SHIFT;
+	offset = offset % PAGE_SIZE;
+	if (new_page != fb->panic.page) {
+		i915_panic_kunmap(&fb->panic);
+		fb->panic.page = new_page;
+		fb->panic.vaddr =
+			kmap_local_page_try_from_panic(fb->panic.pages[fb->panic.page]);
+	}
+	if (fb->panic.vaddr) {
+		u32 *pix = fb->panic.vaddr + offset;
+		*pix = color;
+	}
+}
+
+/*
+ * Setup the gem framebuffer for drm_panic access.
+ * Use current vaddr if it exists, or setup a list of pages.
+ * pfn is not supported yet.
+ */
+int i915_gem_object_panic_setup(struct drm_scanout_buffer *sb)
+{
+	enum i915_map_type has_type;
+	struct intel_framebuffer *fb = (struct intel_framebuffer *)sb->private;
+	struct drm_i915_gem_object *obj = to_intel_bo(intel_fb_bo(&fb->base));
+	void *ptr;
+
+	ptr = page_unpack_bits(obj->mm.mapping, &has_type);
+	if (ptr) {
+		if (i915_gem_object_has_iomem(obj))
+			iosys_map_set_vaddr_iomem(&sb->map[0], (void __iomem *)ptr);
+		else
+			iosys_map_set_vaddr(&sb->map[0], ptr);
+
+		return 0;
+	}
+	if (i915_gem_object_has_struct_page(obj)) {
+		fb->panic.pages = i915_gem_object_panic_pages(obj);
+		if (!fb->panic.pages)
+			return -ENOMEM;
+		fb->panic.page = -1;
+		sb->set_pixel = i915_gem_object_panic_page_set_pixel;
+		return 0;
+	}
+	return -EOPNOTSUPP;
+}
+
+void i915_gem_object_panic_finish(struct intel_framebuffer *fb)
+{
+	i915_panic_kunmap(&fb->panic);
+	fb->panic.page = -1;
+	kfree(fb->panic.pages);
+	fb->panic.pages = NULL;
+}
+
 /* get, pin, and map the pages of the object into kernel space */
 void *i915_gem_object_pin_map(struct drm_i915_gem_object *obj,
 			      enum i915_map_type type)
diff --git a/drivers/gpu/drm/xe/display/intel_bo.c b/drivers/gpu/drm/xe/display/intel_bo.c
index 27437c22bd70..8a6de2dda88c 100644
--- a/drivers/gpu/drm/xe/display/intel_bo.c
+++ b/drivers/gpu/drm/xe/display/intel_bo.c
@@ -1,7 +1,12 @@
 // SPDX-License-Identifier: MIT
 /* Copyright © 2024 Intel Corporation */
 
+#include <drm/drm_cache.h>
 #include <drm/drm_gem.h>
+#include <drm/drm_panic.h>
+
+#include "intel_fb.h"
+#include "intel_display_types.h"
 
 #include "xe_bo.h"
 #include "intel_bo.h"
@@ -59,3 +64,56 @@ void intel_bo_describe(struct seq_file *m, struct drm_gem_object *obj)
 {
 	/* FIXME */
 }
+
+static void xe_panic_kunmap(struct intel_panic_data *panic)
+{
+	if (panic->vaddr) {
+		drm_clflush_virt_range(panic->vaddr, PAGE_SIZE);
+		kunmap_local(panic->vaddr);
+		panic->vaddr = NULL;
+	}
+}
+
+/*
+ * The scanout buffer pages are not mapped, so for each pixel,
+ * use kmap_local_page_try_from_panic() to map the page, and write the pixel.
+ * Try to keep the map from the previous pixel, to avoid too much map/unmap.
+ */
+static void xe_panic_page_set_pixel(struct drm_scanout_buffer *sb, unsigned int x,
+				    unsigned int y, u32 color)
+{
+	struct intel_framebuffer *fb = (struct intel_framebuffer *)sb->private;
+	struct xe_bo *bo = gem_to_xe_bo(intel_fb_bo(&fb->base));
+	unsigned int new_page;
+	unsigned int offset;
+
+	offset = y * sb->pitch[0] + x * sb->format->cpp[0];
+
+	new_page = offset >> PAGE_SHIFT;
+	offset = offset % PAGE_SIZE;
+	if (new_page != fb->panic.page) {
+		xe_panic_kunmap(&fb->panic);
+		fb->panic.page = new_page;
+		fb->panic.vaddr = ttm_bo_kmap_try_from_panic(&bo->ttm,
+							     fb->panic.page);
+	}
+	if (fb->panic.vaddr) {
+		u32 *pix = fb->panic.vaddr + offset;
+		*pix = color;
+	}
+}
+
+int intel_bo_panic_setup(struct drm_scanout_buffer *sb)
+{
+	struct intel_framebuffer *fb = (struct intel_framebuffer *)sb->private;
+
+	fb->panic.page = -1;
+	sb->set_pixel = xe_panic_page_set_pixel;
+	return 0;
+}
+
+void intel_bo_panic_finish(struct intel_framebuffer *fb)
+{
+	xe_panic_kunmap(&fb->panic);
+	fb->panic.page = -1;
+}
-- 
2.49.0


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

* [PATCH v10 07/10] drm/i915/display: Add drm_panic support
  2025-06-18  9:31 [PATCH v10 00/10] drm/i915: Add drm_panic support Jocelyn Falempe
                   ` (5 preceding siblings ...)
  2025-06-18  9:31 ` [PATCH v10 06/10] drm/i915: Add intel_bo_panic_setup and intel_bo_panic_finish Jocelyn Falempe
@ 2025-06-18  9:31 ` Jocelyn Falempe
  2025-06-18  9:31 ` [PATCH v10 08/10] drm/i915/display: Add drm_panic support for Y-tiling with DPT Jocelyn Falempe
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 20+ messages in thread
From: Jocelyn Falempe @ 2025-06-18  9:31 UTC (permalink / raw)
  To: Maarten Lankhorst, Jani Nikula, Rodrigo Vivi, Joonas Lahtinen,
	Tvrtko Ursulin, Ville Syrjälä, David Airlie,
	Simona Vetter, Christian Koenig, Huang Rui, Matthew Auld,
	Matthew Brost, Maxime Ripard, Thomas Zimmermann, intel-gfx,
	intel-xe, dri-devel, linux-kernel
  Cc: Jocelyn Falempe

This adds drm_panic support for a wide range of Intel GPU. I've
tested it only on 4 laptops, Haswell (with 128MB of eDRAM),
Comet Lake, Raptor Lake, and Lunar Lake.
For hardware using DPT, it's not possible to disable tiling, as you
will need to reconfigure the way the GPU is accessing the
framebuffer, so this will be handled by the following patches.

Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
---

v4:
 * Add support for Xe driver.
 
v6:
 * Use struct intel_display instead of drm_i915_private for intel_atomic_plane.c
 
v7:
 * Fix mismatch {} in intel_panic_flush() (Jani Nikula)

v8:
 * Use intel_bo_panic_setup() and intel_bo_panic_finish().
 
v10:
 * Use struct intel_framebuffer to store the panic variables (Maarten Lankhorst)
 
 .../gpu/drm/i915/display/intel_atomic_plane.c | 83 ++++++++++++++++++-
 1 file changed, 82 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
index 15ede7678636..9e1c9d1e015f 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
@@ -33,19 +33,23 @@
 
 #include <linux/dma-fence-chain.h>
 #include <linux/dma-resv.h>
+#include <linux/iosys-map.h>
 
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_blend.h>
 #include <drm/drm_damage_helper.h>
+#include <drm/drm_cache.h>
 #include <drm/drm_fourcc.h>
 #include <drm/drm_gem.h>
 #include <drm/drm_gem_atomic_helper.h>
+#include <drm/drm_panic.h>
 
 #include "gem/i915_gem_object.h"
 #include "i915_scheduler_types.h"
 #include "i915_vma.h"
 #include "i9xx_plane_regs.h"
 #include "intel_atomic_plane.h"
+#include "intel_bo.h"
 #include "intel_cdclk.h"
 #include "intel_cursor.h"
 #include "intel_display_rps.h"
@@ -53,6 +57,7 @@
 #include "intel_display_types.h"
 #include "intel_fb.h"
 #include "intel_fb_pin.h"
+#include "intel_fbdev.h"
 #include "skl_scaler.h"
 #include "skl_universal_plane.h"
 #include "skl_watermark.h"
@@ -1266,14 +1271,90 @@ intel_cleanup_plane_fb(struct drm_plane *plane,
 	intel_plane_unpin_fb(old_plane_state);
 }
 
+static void intel_panic_flush(struct drm_plane *plane)
+{
+	struct intel_plane_state *plane_state = to_intel_plane_state(plane->state);
+	struct intel_plane *iplane = to_intel_plane(plane);
+	struct intel_display *display = to_intel_display(iplane);
+	struct drm_framebuffer *fb = plane_state->hw.fb;
+	struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb);
+
+	intel_bo_panic_finish(intel_fb);
+
+	/* Flush the cache and don't disable tiling if it's the fbdev framebuffer.*/
+	if (intel_fb == intel_fbdev_framebuffer(display->fbdev.fbdev)) {
+		struct iosys_map map;
+
+		intel_fbdev_get_map(display->fbdev.fbdev, &map);
+		drm_clflush_virt_range(map.vaddr, fb->pitches[0] * fb->height);
+		return;
+	}
+
+	if (fb->modifier && iplane->disable_tiling)
+		iplane->disable_tiling(iplane);
+}
+
+static int intel_get_scanout_buffer(struct drm_plane *plane,
+				    struct drm_scanout_buffer *sb)
+{
+	struct intel_plane_state *plane_state;
+	struct drm_gem_object *obj;
+	struct drm_framebuffer *fb;
+	struct intel_framebuffer *intel_fb;
+	struct intel_display *display = to_intel_display(plane->dev);
+
+	if (!plane->state || !plane->state->fb || !plane->state->visible)
+		return -ENODEV;
+
+	plane_state = to_intel_plane_state(plane->state);
+	fb = plane_state->hw.fb;
+	intel_fb = to_intel_framebuffer(fb);
+
+	obj = intel_fb_bo(fb);
+	if (!obj)
+		return -ENODEV;
+
+	if (intel_fb == intel_fbdev_framebuffer(display->fbdev.fbdev)) {
+		intel_fbdev_get_map(display->fbdev.fbdev, &sb->map[0]);
+	} else {
+		int ret;
+		/* Can't disable tiling if DPT is in use */
+		if (intel_fb_uses_dpt(fb))
+			return -EOPNOTSUPP;
+		sb->private = intel_fb;
+		ret = intel_bo_panic_setup(sb);
+		if (ret)
+			return ret;
+	}
+	sb->width = fb->width;
+	sb->height = fb->height;
+	/* Use the generic linear format, because tiling, RC, CCS, CC
+	 * will be disabled in disable_tiling()
+	 */
+	sb->format = drm_format_info(fb->format->format);
+	sb->pitch[0] = fb->pitches[0];
+
+	return 0;
+}
+
 static const struct drm_plane_helper_funcs intel_plane_helper_funcs = {
 	.prepare_fb = intel_prepare_plane_fb,
 	.cleanup_fb = intel_cleanup_plane_fb,
 };
 
+static const struct drm_plane_helper_funcs intel_primary_plane_helper_funcs = {
+	.prepare_fb = intel_prepare_plane_fb,
+	.cleanup_fb = intel_cleanup_plane_fb,
+	.get_scanout_buffer = intel_get_scanout_buffer,
+	.panic_flush = intel_panic_flush,
+};
+
 void intel_plane_helper_add(struct intel_plane *plane)
 {
-	drm_plane_helper_add(&plane->base, &intel_plane_helper_funcs);
+	if (plane->base.type == DRM_PLANE_TYPE_PRIMARY)
+		drm_plane_helper_add(&plane->base, &intel_primary_plane_helper_funcs);
+	else
+		drm_plane_helper_add(&plane->base, &intel_plane_helper_funcs);
 }
 
 void intel_plane_init_cursor_vblank_work(struct intel_plane_state *old_plane_state,
-- 
2.49.0


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

* [PATCH v10 08/10] drm/i915/display: Add drm_panic support for Y-tiling with DPT
  2025-06-18  9:31 [PATCH v10 00/10] drm/i915: Add drm_panic support Jocelyn Falempe
                   ` (6 preceding siblings ...)
  2025-06-18  9:31 ` [PATCH v10 07/10] drm/i915/display: Add drm_panic support Jocelyn Falempe
@ 2025-06-18  9:31 ` Jocelyn Falempe
  2025-06-18  9:31 ` [PATCH v10 09/10] drm/i915/display: Add drm_panic support for 4-tiling " Jocelyn Falempe
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 20+ messages in thread
From: Jocelyn Falempe @ 2025-06-18  9:31 UTC (permalink / raw)
  To: Maarten Lankhorst, Jani Nikula, Rodrigo Vivi, Joonas Lahtinen,
	Tvrtko Ursulin, Ville Syrjälä, David Airlie,
	Simona Vetter, Christian Koenig, Huang Rui, Matthew Auld,
	Matthew Brost, Maxime Ripard, Thomas Zimmermann, intel-gfx,
	intel-xe, dri-devel, linux-kernel
  Cc: Jocelyn Falempe

On Alder Lake and later, it's not possible to disable tiling when DPT
is enabled.
So this commit implements Y-Tiling support, to still be able to draw
the panic screen.

Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
---

v8:
 * Pass the tiling function to intel_bo_panic_setup()

 .../gpu/drm/i915/display/intel_atomic_plane.c | 64 ++++++++++++++++++-
 .../drm/i915/display/intel_display_types.h    |  1 +
 .../drm/i915/display/skl_universal_plane.c    | 15 +++--
 drivers/gpu/drm/i915/gem/i915_gem_pages.c     | 16 ++++-
 drivers/gpu/drm/xe/display/intel_bo.c         |  5 +-
 5 files changed, 93 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
index 9e1c9d1e015f..044328d83df9 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
@@ -1271,6 +1271,32 @@ intel_cleanup_plane_fb(struct drm_plane *plane,
 	intel_plane_unpin_fb(old_plane_state);
 }
 
+/* Handle Y-tiling, only if DPT is enabled (otherwise disabling tiling is easier)
+ * All DPT hardware have 128-bytes width tiling, so Y-tile dimension is 32x32
+ * pixels for 32bits pixels.
+ */
+#define YTILE_WIDTH	32
+#define YTILE_HEIGHT	32
+#define YTILE_SIZE (YTILE_WIDTH * YTILE_HEIGHT * 4)
+
+static unsigned int intel_ytile_get_offset(unsigned int width, unsigned int x, unsigned int y)
+{
+	u32 offset;
+	unsigned int swizzle;
+	unsigned int width_in_blocks = DIV_ROUND_UP(width, 32);
+
+	/* Block offset */
+	offset = ((y / YTILE_HEIGHT) * width_in_blocks + (x / YTILE_WIDTH)) * YTILE_SIZE;
+
+	x = x % YTILE_WIDTH;
+	y = y % YTILE_HEIGHT;
+
+	/* bit order inside a block is x4 x3 x2 y4 y3 y2 y1 y0 x1 x0 */
+	swizzle = (x & 3) | ((y & 0x1f) << 2) | ((x & 0x1c) << 5);
+	offset += swizzle * 4;
+	return offset;
+}
+
 static void intel_panic_flush(struct drm_plane *plane)
 {
 	struct intel_plane_state *plane_state = to_intel_plane_state(plane->state);
@@ -1294,6 +1320,35 @@ static void intel_panic_flush(struct drm_plane *plane)
 		iplane->disable_tiling(iplane);
 }
 
+static unsigned int (*intel_get_tiling_func(u64 fb_modifier))(unsigned int width,
+							      unsigned int x,
+							      unsigned int y)
+{
+	switch (fb_modifier) {
+	case I915_FORMAT_MOD_Y_TILED:
+	case I915_FORMAT_MOD_Y_TILED_CCS:
+	case I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC:
+	case I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS:
+	case I915_FORMAT_MOD_Y_TILED_GEN12_MC_CCS:
+		return intel_ytile_get_offset;
+	case I915_FORMAT_MOD_4_TILED:
+	case I915_FORMAT_MOD_4_TILED_DG2_RC_CCS:
+	case I915_FORMAT_MOD_4_TILED_DG2_MC_CCS:
+	case I915_FORMAT_MOD_4_TILED_DG2_RC_CCS_CC:
+	case I915_FORMAT_MOD_4_TILED_MTL_RC_CCS:
+	case I915_FORMAT_MOD_4_TILED_MTL_RC_CCS_CC:
+	case I915_FORMAT_MOD_4_TILED_MTL_MC_CCS:
+	case I915_FORMAT_MOD_4_TILED_BMG_CCS:
+	case I915_FORMAT_MOD_4_TILED_LNL_CCS:
+	case I915_FORMAT_MOD_X_TILED:
+	case I915_FORMAT_MOD_Yf_TILED:
+	case I915_FORMAT_MOD_Yf_TILED_CCS:
+	default:
+	/* Not supported yet */
+		return NULL;
+	}
+}
+
 static int intel_get_scanout_buffer(struct drm_plane *plane,
 				    struct drm_scanout_buffer *sb)
 {
@@ -1319,8 +1374,13 @@ static int intel_get_scanout_buffer(struct drm_plane *plane,
 	} else {
 		int ret;
 		/* Can't disable tiling if DPT is in use */
-		if (intel_fb_uses_dpt(fb))
-			return -EOPNOTSUPP;
+		if (intel_fb_uses_dpt(fb)) {
+			if (fb->format->cpp[0] != 4)
+				return -EOPNOTSUPP;
+			intel_fb->panic.tiling = intel_get_tiling_func(fb->modifier);
+			if (!intel_fb->panic.tiling)
+				return -EOPNOTSUPP;
+		}
 		sb->private = intel_fb;
 		ret = intel_bo_panic_setup(sb);
 		if (ret)
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 6d6d2d948886..8db6a8c0686c 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -134,6 +134,7 @@ struct intel_panic_data {
 	struct page **pages;
 	int page;
 	void *vaddr;
+	unsigned int (*tiling)(unsigned int x, unsigned int y, unsigned int width);
 };
 
 struct intel_framebuffer {
diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
index ef09ef7b101f..623eff23d9bc 100644
--- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
+++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
@@ -2796,15 +2796,22 @@ static void skl_disable_tiling(struct intel_plane *plane)
 {
 	struct intel_plane_state *state = to_intel_plane_state(plane->base.state);
 	struct intel_display *display = to_intel_display(plane);
-	u32 stride = state->view.color_plane[0].scanout_stride / 64;
+	const struct drm_framebuffer *fb = state->hw.fb;
 	u32 plane_ctl;
 
 	plane_ctl = intel_de_read(display, PLANE_CTL(plane->pipe, plane->id));
-	plane_ctl &= ~PLANE_CTL_TILED_MASK;
 
-	intel_de_write_fw(display, PLANE_STRIDE(plane->pipe, plane->id),
-			  PLANE_STRIDE_(stride));
+	if (intel_fb_uses_dpt(fb)) {
+		/* if DPT is enabled, keep tiling, but disable compression */
+		plane_ctl &= ~PLANE_CTL_RENDER_DECOMPRESSION_ENABLE;
+	} else {
+		/* if DPT is not supported, disable tiling, and update stride */
+		u32 stride = state->view.color_plane[0].scanout_stride / 64;
 
+		plane_ctl &= ~PLANE_CTL_TILED_MASK;
+		intel_de_write_fw(display, PLANE_STRIDE(plane->pipe, plane->id),
+				  PLANE_STRIDE_(stride));
+	}
 	intel_de_write_fw(display, PLANE_CTL(plane->pipe, plane->id), plane_ctl);
 
 	intel_de_write_fw(display, PLANE_SURF(plane->pipe, plane->id),
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_pages.c b/drivers/gpu/drm/i915/gem/i915_gem_pages.c
index 0bcf4647a2a0..bf7cd7555777 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_pages.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_pages.c
@@ -384,6 +384,15 @@ static struct page **i915_gem_object_panic_pages(struct drm_i915_gem_object *obj
 	return pages;
 }
 
+static void i915_gem_object_panic_map_set_pixel(struct drm_scanout_buffer *sb, unsigned int x,
+						unsigned int y, u32 color)
+{
+	struct intel_framebuffer *fb = (struct intel_framebuffer *)sb->private;
+	unsigned int offset = fb->panic.tiling(sb->width, x, y);
+
+	iosys_map_wr(&sb->map[0], offset, u32, color);
+}
+
 /*
  * The scanout buffer pages are not mapped, so for each pixel,
  * use kmap_local_page_try_from_panic() to map the page, and write the pixel.
@@ -396,7 +405,10 @@ static void i915_gem_object_panic_page_set_pixel(struct drm_scanout_buffer *sb,
 	unsigned int offset;
 	struct intel_framebuffer *fb = (struct intel_framebuffer *)sb->private;
 
-	offset = y * sb->pitch[0] + x * sb->format->cpp[0];
+	if (fb->panic.tiling)
+		offset = fb->panic.tiling(sb->width, x, y);
+	else
+		offset = y * sb->pitch[0] + x * sb->format->cpp[0];
 
 	new_page = offset >> PAGE_SHIFT;
 	offset = offset % PAGE_SIZE;
@@ -431,6 +443,8 @@ int i915_gem_object_panic_setup(struct drm_scanout_buffer *sb)
 		else
 			iosys_map_set_vaddr(&sb->map[0], ptr);
 
+		if (fb->panic.tiling)
+			sb->set_pixel = i915_gem_object_panic_map_set_pixel;
 		return 0;
 	}
 	if (i915_gem_object_has_struct_page(obj)) {
diff --git a/drivers/gpu/drm/xe/display/intel_bo.c b/drivers/gpu/drm/xe/display/intel_bo.c
index 8a6de2dda88c..26db10fc2f48 100644
--- a/drivers/gpu/drm/xe/display/intel_bo.c
+++ b/drivers/gpu/drm/xe/display/intel_bo.c
@@ -87,7 +87,10 @@ static void xe_panic_page_set_pixel(struct drm_scanout_buffer *sb, unsigned int
 	unsigned int new_page;
 	unsigned int offset;
 
-	offset = y * sb->pitch[0] + x * sb->format->cpp[0];
+	if (fb->panic.tiling)
+		offset = fb->panic.tiling(sb->width, x, y);
+	else
+		offset = y * sb->pitch[0] + x * sb->format->cpp[0];
 
 	new_page = offset >> PAGE_SHIFT;
 	offset = offset % PAGE_SIZE;
-- 
2.49.0


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

* [PATCH v10 09/10] drm/i915/display: Add drm_panic support for 4-tiling with DPT
  2025-06-18  9:31 [PATCH v10 00/10] drm/i915: Add drm_panic support Jocelyn Falempe
                   ` (7 preceding siblings ...)
  2025-06-18  9:31 ` [PATCH v10 08/10] drm/i915/display: Add drm_panic support for Y-tiling with DPT Jocelyn Falempe
@ 2025-06-18  9:31 ` Jocelyn Falempe
  2025-06-18  9:31 ` [PATCH v10 10/10] drm/i915/psr: Add intel_psr2_panic_force_full_update Jocelyn Falempe
  2025-06-23  7:40 ` [PATCH v10 00/10] drm/i915: Add drm_panic support Maarten Lankhorst
  10 siblings, 0 replies; 20+ messages in thread
From: Jocelyn Falempe @ 2025-06-18  9:31 UTC (permalink / raw)
  To: Maarten Lankhorst, Jani Nikula, Rodrigo Vivi, Joonas Lahtinen,
	Tvrtko Ursulin, Ville Syrjälä, David Airlie,
	Simona Vetter, Christian Koenig, Huang Rui, Matthew Auld,
	Matthew Brost, Maxime Ripard, Thomas Zimmermann, intel-gfx,
	intel-xe, dri-devel, linux-kernel
  Cc: Jocelyn Falempe

On Alder Lake and later, it's not possible to disable tiling when DPT
is enabled.
So this commit implements 4-Tiling support, to still be able to draw
the panic screen.

Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
---

 .../gpu/drm/i915/display/intel_atomic_plane.c | 20 +++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
index 044328d83df9..086195a11af9 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
@@ -1297,6 +1297,25 @@ static unsigned int intel_ytile_get_offset(unsigned int width, unsigned int x, u
 	return offset;
 }
 
+static unsigned int intel_4tile_get_offset(unsigned int width, unsigned int x, unsigned int y)
+{
+	u32 offset;
+	unsigned int swizzle;
+	unsigned int width_in_blocks = DIV_ROUND_UP(width, 32);
+
+	/* Block offset */
+	offset = ((y / YTILE_HEIGHT) * width_in_blocks + (x / YTILE_WIDTH)) * YTILE_SIZE;
+
+	x = x % YTILE_WIDTH;
+	y = y % YTILE_HEIGHT;
+
+	/* bit order inside a block is y4 y3 x4 y2 x3 x2 y1 y0 x1 x0 */
+	swizzle = (x & 3) | ((y & 3) << 2) | ((x & 0xc) << 2) | (y & 4) << 4 |
+		  ((x & 0x10) << 3) | ((y & 0x18) << 5);
+	offset += swizzle * 4;
+	return offset;
+}
+
 static void intel_panic_flush(struct drm_plane *plane)
 {
 	struct intel_plane_state *plane_state = to_intel_plane_state(plane->state);
@@ -1340,6 +1359,7 @@ static unsigned int (*intel_get_tiling_func(u64 fb_modifier))(unsigned int width
 	case I915_FORMAT_MOD_4_TILED_MTL_MC_CCS:
 	case I915_FORMAT_MOD_4_TILED_BMG_CCS:
 	case I915_FORMAT_MOD_4_TILED_LNL_CCS:
+		return intel_4tile_get_offset;
 	case I915_FORMAT_MOD_X_TILED:
 	case I915_FORMAT_MOD_Yf_TILED:
 	case I915_FORMAT_MOD_Yf_TILED_CCS:
-- 
2.49.0


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

* [PATCH v10 10/10] drm/i915/psr: Add intel_psr2_panic_force_full_update
  2025-06-18  9:31 [PATCH v10 00/10] drm/i915: Add drm_panic support Jocelyn Falempe
                   ` (8 preceding siblings ...)
  2025-06-18  9:31 ` [PATCH v10 09/10] drm/i915/display: Add drm_panic support for 4-tiling " Jocelyn Falempe
@ 2025-06-18  9:31 ` Jocelyn Falempe
  2025-06-23  7:40 ` [PATCH v10 00/10] drm/i915: Add drm_panic support Maarten Lankhorst
  10 siblings, 0 replies; 20+ messages in thread
From: Jocelyn Falempe @ 2025-06-18  9:31 UTC (permalink / raw)
  To: Maarten Lankhorst, Jani Nikula, Rodrigo Vivi, Joonas Lahtinen,
	Tvrtko Ursulin, Ville Syrjälä, David Airlie,
	Simona Vetter, Christian Koenig, Huang Rui, Matthew Auld,
	Matthew Brost, Maxime Ripard, Thomas Zimmermann, intel-gfx,
	intel-xe, dri-devel, linux-kernel
  Cc: Jocelyn Falempe

When the panic handler is called, configure the psr to send the full
framebuffer to the monitor, otherwise the panic screen is only
partially visible.

Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
---

v8:
 * Added in v8

 .../gpu/drm/i915/display/intel_atomic_plane.c |  7 +++++++
 drivers/gpu/drm/i915/display/intel_psr.c      | 20 +++++++++++++++++++
 drivers/gpu/drm/i915/display/intel_psr.h      |  2 ++
 3 files changed, 29 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
index 086195a11af9..82669367647a 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
@@ -58,6 +58,7 @@
 #include "intel_fb.h"
 #include "intel_fb_pin.h"
 #include "intel_fbdev.h"
+#include "intel_psr.h"
 #include "skl_scaler.h"
 #include "skl_universal_plane.h"
 #include "skl_watermark.h"
@@ -1319,6 +1320,7 @@ static unsigned int intel_4tile_get_offset(unsigned int width, unsigned int x, u
 static void intel_panic_flush(struct drm_plane *plane)
 {
 	struct intel_plane_state *plane_state = to_intel_plane_state(plane->state);
+	struct intel_crtc_state *crtc_state = to_intel_crtc_state(plane->state->crtc->state);
 	struct intel_plane *iplane = to_intel_plane(plane);
 	struct intel_display *display = to_intel_display(iplane);
 	struct drm_framebuffer *fb = plane_state->hw.fb;
@@ -1326,6 +1328,11 @@ static void intel_panic_flush(struct drm_plane *plane)
 
 	intel_bo_panic_finish(intel_fb);
 
+	if (crtc_state->enable_psr2_sel_fetch) {
+		/* Force a full update for psr2 */
+		intel_psr2_panic_force_full_update(display, crtc_state);
+	}
+
 	/* Flush the cache and don't disable tiling if it's the fbdev framebuffer.*/
 	if (intel_fb == intel_fbdev_framebuffer(display->fbdev.fbdev)) {
 		struct iosys_map map;
diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
index 8bee2f592ae7..73bdc54d7831 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -2889,6 +2889,26 @@ int intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
 	return 0;
 }
 
+void intel_psr2_panic_force_full_update(struct intel_display *display,
+					struct intel_crtc_state *crtc_state)
+{
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
+	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
+	u32 val = man_trk_ctl_enable_bit_get(display);
+
+	/* SF partial frame enable has to be set even on full update */
+	val |= man_trk_ctl_partial_frame_bit_get(display);
+	val |= man_trk_ctl_continuos_full_frame(display);
+
+	/* Directly write the register */
+	intel_de_write_fw(display, PSR2_MAN_TRK_CTL(display, cpu_transcoder), val);
+
+	if (!crtc_state->enable_psr2_su_region_et)
+		return;
+
+	intel_de_write_fw(display, PIPE_SRCSZ_ERLY_TPT(crtc->pipe), 0);
+}
+
 void intel_psr_pre_plane_update(struct intel_atomic_state *state,
 				struct intel_crtc *crtc)
 {
diff --git a/drivers/gpu/drm/i915/display/intel_psr.h b/drivers/gpu/drm/i915/display/intel_psr.h
index 0cf53184f13f..9b061a22361f 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.h
+++ b/drivers/gpu/drm/i915/display/intel_psr.h
@@ -57,6 +57,8 @@ int intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
 				struct intel_crtc *crtc);
 void intel_psr2_program_trans_man_trk_ctl(struct intel_dsb *dsb,
 					  const struct intel_crtc_state *crtc_state);
+void intel_psr2_panic_force_full_update(struct intel_display *display,
+					struct intel_crtc_state *crtc_state);
 void intel_psr_pause(struct intel_dp *intel_dp);
 void intel_psr_resume(struct intel_dp *intel_dp);
 bool intel_psr_needs_vblank_notification(const struct intel_crtc_state *crtc_state);
-- 
2.49.0


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

* Re: [PATCH v10 05/10] drm/ttm: Add ttm_bo_kmap_try_from_panic()
  2025-06-18  9:31 ` [PATCH v10 05/10] drm/ttm: Add ttm_bo_kmap_try_from_panic() Jocelyn Falempe
@ 2025-06-18 13:55   ` Christian König
  2025-06-18 15:38     ` Jocelyn Falempe
  2025-06-27 10:05     ` Maarten Lankhorst
  0 siblings, 2 replies; 20+ messages in thread
From: Christian König @ 2025-06-18 13:55 UTC (permalink / raw)
  To: Jocelyn Falempe, Maarten Lankhorst, Jani Nikula, Rodrigo Vivi,
	Joonas Lahtinen, Tvrtko Ursulin, Ville Syrjälä,
	David Airlie, Simona Vetter, Huang Rui, Matthew Auld,
	Matthew Brost, Maxime Ripard, Thomas Zimmermann, intel-gfx,
	intel-xe, dri-devel, linux-kernel



On 6/18/25 11:31, Jocelyn Falempe wrote:
> If the ttm bo is backed by pages, then it's possible to safely kmap
> one page at a time, using kmap_try_from_panic().
> Unfortunately there is no way to do the same with ioremap, so it
> only supports the kmap case.
> This is needed for proper drm_panic support with xe driver.
> 
> Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>

Reviewed-by: Christian König <christian.koenig@amd.com>

Preferred through drm-misc-next, but feel free to merge it through every branch you want if it makes thinks easier for you.

Regards,
Christian.

> ---
> 
> v8:
>  * Added in v8
> 
> v9:
>  * Fix comment in ttm_bo_kmap_try_from_panic(), this can *only* be called
>    from the panic handler (Christian König)
> 
>  drivers/gpu/drm/ttm/ttm_bo_util.c | 27 +++++++++++++++++++++++++++
>  include/drm/ttm/ttm_bo.h          |  1 +
>  2 files changed, 28 insertions(+)
> 
> diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
> index 15cab9bda17f..6912e6dfda25 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo_util.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
> @@ -377,6 +377,33 @@ static int ttm_bo_kmap_ttm(struct ttm_buffer_object *bo,
>  	return (!map->virtual) ? -ENOMEM : 0;
>  }
>  
> +/**
> + *
> + * ttm_bo_kmap_try_from_panic
> + *
> + * @bo: The buffer object
> + * @page: The page to map
> + *
> + * Sets up a kernel virtual mapping using kmap_local_page_try_from_panic().
> + * This should only be called from the panic handler, if you make sure the bo
> + * is the one being displayed, so is properly allocated, and protected.
> + *
> + * Returns the vaddr, that you can use to write to the bo, and that you should
> + * pass to kunmap_local() when you're done with this page, or NULL if the bo
> + * is in iomem.
> + */
> +void *ttm_bo_kmap_try_from_panic(struct ttm_buffer_object *bo, unsigned long page)
> +{
> +	if (page + 1 > PFN_UP(bo->resource->size))
> +		return NULL;
> +
> +	if (!bo->resource->bus.is_iomem && bo->ttm->pages && bo->ttm->pages[page])
> +		return kmap_local_page_try_from_panic(bo->ttm->pages[page]);
> +
> +	return NULL;
> +}
> +EXPORT_SYMBOL(ttm_bo_kmap_try_from_panic);
> +
>  /**
>   * ttm_bo_kmap
>   *
> diff --git a/include/drm/ttm/ttm_bo.h b/include/drm/ttm/ttm_bo.h
> index cf027558b6db..8c0ce3fa077f 100644
> --- a/include/drm/ttm/ttm_bo.h
> +++ b/include/drm/ttm/ttm_bo.h
> @@ -429,6 +429,7 @@ int ttm_bo_init_validate(struct ttm_device *bdev, struct ttm_buffer_object *bo,
>  int ttm_bo_kmap(struct ttm_buffer_object *bo, unsigned long start_page,
>  		unsigned long num_pages, struct ttm_bo_kmap_obj *map);
>  void ttm_bo_kunmap(struct ttm_bo_kmap_obj *map);
> +void *ttm_bo_kmap_try_from_panic(struct ttm_buffer_object *bo, unsigned long page);
>  int ttm_bo_vmap(struct ttm_buffer_object *bo, struct iosys_map *map);
>  void ttm_bo_vunmap(struct ttm_buffer_object *bo, struct iosys_map *map);
>  int ttm_bo_mmap_obj(struct vm_area_struct *vma, struct ttm_buffer_object *bo);


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

* Re: [PATCH v10 05/10] drm/ttm: Add ttm_bo_kmap_try_from_panic()
  2025-06-18 13:55   ` Christian König
@ 2025-06-18 15:38     ` Jocelyn Falempe
  2025-06-27 10:05     ` Maarten Lankhorst
  1 sibling, 0 replies; 20+ messages in thread
From: Jocelyn Falempe @ 2025-06-18 15:38 UTC (permalink / raw)
  To: Christian König, Maarten Lankhorst, Jani Nikula,
	Rodrigo Vivi, Joonas Lahtinen, Tvrtko Ursulin,
	Ville Syrjälä, David Airlie, Simona Vetter, Huang Rui,
	Matthew Auld, Matthew Brost, Maxime Ripard, Thomas Zimmermann,
	intel-gfx, intel-xe, dri-devel, linux-kernel

On 18/06/2025 15:55, Christian König wrote:
> 
> 
> On 6/18/25 11:31, Jocelyn Falempe wrote:
>> If the ttm bo is backed by pages, then it's possible to safely kmap
>> one page at a time, using kmap_try_from_panic().
>> Unfortunately there is no way to do the same with ioremap, so it
>> only supports the kmap case.
>> This is needed for proper drm_panic support with xe driver.
>>
>> Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
> 
> Reviewed-by: Christian König <christian.koenig@amd.com>
> 
> Preferred through drm-misc-next, but feel free to merge it through every branch you want if it makes thinks easier for you.

Thanks, I will see if I can get the whole series through drm-intel-next, 
or if I can merge #1 and #5 first in drm-misc-next.

Best regards,

-- 

Jocelyn


> 
> Regards,
> Christian.
> 
>> ---
>>
>> v8:
>>   * Added in v8
>>
>> v9:
>>   * Fix comment in ttm_bo_kmap_try_from_panic(), this can *only* be called
>>     from the panic handler (Christian König)
>>
>>   drivers/gpu/drm/ttm/ttm_bo_util.c | 27 +++++++++++++++++++++++++++
>>   include/drm/ttm/ttm_bo.h          |  1 +
>>   2 files changed, 28 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
>> index 15cab9bda17f..6912e6dfda25 100644
>> --- a/drivers/gpu/drm/ttm/ttm_bo_util.c
>> +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
>> @@ -377,6 +377,33 @@ static int ttm_bo_kmap_ttm(struct ttm_buffer_object *bo,
>>   	return (!map->virtual) ? -ENOMEM : 0;
>>   }
>>   
>> +/**
>> + *
>> + * ttm_bo_kmap_try_from_panic
>> + *
>> + * @bo: The buffer object
>> + * @page: The page to map
>> + *
>> + * Sets up a kernel virtual mapping using kmap_local_page_try_from_panic().
>> + * This should only be called from the panic handler, if you make sure the bo
>> + * is the one being displayed, so is properly allocated, and protected.
>> + *
>> + * Returns the vaddr, that you can use to write to the bo, and that you should
>> + * pass to kunmap_local() when you're done with this page, or NULL if the bo
>> + * is in iomem.
>> + */
>> +void *ttm_bo_kmap_try_from_panic(struct ttm_buffer_object *bo, unsigned long page)
>> +{
>> +	if (page + 1 > PFN_UP(bo->resource->size))
>> +		return NULL;
>> +
>> +	if (!bo->resource->bus.is_iomem && bo->ttm->pages && bo->ttm->pages[page])
>> +		return kmap_local_page_try_from_panic(bo->ttm->pages[page]);
>> +
>> +	return NULL;
>> +}
>> +EXPORT_SYMBOL(ttm_bo_kmap_try_from_panic);
>> +
>>   /**
>>    * ttm_bo_kmap
>>    *
>> diff --git a/include/drm/ttm/ttm_bo.h b/include/drm/ttm/ttm_bo.h
>> index cf027558b6db..8c0ce3fa077f 100644
>> --- a/include/drm/ttm/ttm_bo.h
>> +++ b/include/drm/ttm/ttm_bo.h
>> @@ -429,6 +429,7 @@ int ttm_bo_init_validate(struct ttm_device *bdev, struct ttm_buffer_object *bo,
>>   int ttm_bo_kmap(struct ttm_buffer_object *bo, unsigned long start_page,
>>   		unsigned long num_pages, struct ttm_bo_kmap_obj *map);
>>   void ttm_bo_kunmap(struct ttm_bo_kmap_obj *map);
>> +void *ttm_bo_kmap_try_from_panic(struct ttm_buffer_object *bo, unsigned long page);
>>   int ttm_bo_vmap(struct ttm_buffer_object *bo, struct iosys_map *map);
>>   void ttm_bo_vunmap(struct ttm_buffer_object *bo, struct iosys_map *map);
>>   int ttm_bo_mmap_obj(struct vm_area_struct *vma, struct ttm_buffer_object *bo);
> 


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

* Re: [PATCH v10 00/10] drm/i915: Add drm_panic support
  2025-06-18  9:31 [PATCH v10 00/10] drm/i915: Add drm_panic support Jocelyn Falempe
                   ` (9 preceding siblings ...)
  2025-06-18  9:31 ` [PATCH v10 10/10] drm/i915/psr: Add intel_psr2_panic_force_full_update Jocelyn Falempe
@ 2025-06-23  7:40 ` Maarten Lankhorst
  2025-06-23 10:10   ` Jocelyn Falempe
  10 siblings, 1 reply; 20+ messages in thread
From: Maarten Lankhorst @ 2025-06-23  7:40 UTC (permalink / raw)
  To: Jocelyn Falempe, Jani Nikula, Rodrigo Vivi, Joonas Lahtinen,
	Tvrtko Ursulin, Ville Syrjälä, David Airlie,
	Simona Vetter, Christian Koenig, Huang Rui, Matthew Auld,
	Matthew Brost, Maxime Ripard, Thomas Zimmermann, intel-gfx,
	intel-xe, dri-devel, linux-kernel

Hey,

Thanks for the series. I didn't see you on irc so I wanted to ask if you are planning to send a v11 with
the changes from void * to struct intel_panic_data and adding the VRAM support?


Other than that, I think the series looks good and I'll be able to test it on my battlemage.

Best regards,
~Maarten

On 2025-06-18 11:31, Jocelyn Falempe wrote:
> This adds drm_panic support for i915 and xe driver.
> 
> I've tested it on the 4 intel laptops I have at my disposal.
>  * Haswell with 128MB of eDRAM.
>  * Comet Lake  i7-10850H
>  * Raptor Lake i7-1370P (with DPT, and Y-tiling).
>  * Lunar Lake Ultra 5 228V (with DPT, and 4-tiling, and using the Xe driver.
> 
> I tested panic in both fbdev console and gnome desktop.
> I think it won't work yet on discrete GPU, but that can be added later.
> 
> Best regards,
> 
> v2:
>  * Add the proper abstractions to build also for Xe.
>  * Fix dim checkpatch issues.
> 
> v3:
>  * Add support for Y-tiled framebuffer when DPT is enabled.
> 
> v4:
>  * Add support for Xe driver, which shares most of the code.
>  * Add support for 4-tiled framebuffer found in newest GPU.
> 
> v5:
>  * Rebase on top of git@gitlab.freedesktop.org:drm/i915/kernel.git drm-intel-next
>  * Use struct intel_display instead of drm_i915_private.
>  * Use iosys_map for intel_bo_panic_map().
> 
> v6:
>  * Rebase on top of git@gitlab.freedesktop.org:drm/i915/kernel.git drm-intel-next
>  * Use struct intel_display instead of drm_i915_private for intel_atomic_plane.c
> 
> v7:
>  * Fix mismatch {} in intel_panic_flush() (Jani Nikula)
>  * Return int for i915_gem_object_panic_map() (Ville Syrjälä)
>  * Reword commit message about alignment/size when disabling tiling (Ville Syrjälä)
> 
> v8:
>  * Use kmap_try_from_panic() instead of vmap, to access the framebuffer.
>  * Add ttm_bo_kmap_try_from_panic() for the xe driver, that uses ttm.
>  * Replace intel_bo_panic_map() with a setup() and finish() function,
>    to allow mapping only one page of teh framebuffer at a time.
>  * Configure psr to send the full framebuffer update.
> 
> v9:
>  * Fix comment in ttm_bo_kmap_try_from_panic(), this can *only* be called
>    from the panic handler (Christian König)
>  * Fix missing kfree() for i915_panic_pages in i915_gem_object_panic_finish()
>    Also change i915_panic_pages allocation to kmalloc, as kvmalloc is not
>    safe to call from the panic handler.
>  * Fix dim checkpatch warnings.
> 
> v10:
>  * Add a private field to struct drm_scanout_buffer
>  * Replace static variables with new fields in struct intel_framebuffer
>    (Maarten Lankhorst)
>  * Add error handling if i915_gem_object_panic_pages() returns NULL
>  * Declare struct drm_scanout_buffer instead of including <drm/drm_panic.h>
>    in intel_bo.h
> 
> Jocelyn Falempe (10):
>   drm/panic: Add a private field to struct drm_scanout_buffer
>   drm/i915/fbdev: Add intel_fbdev_get_map()
>   drm/i915/display/i9xx: Add a disable_tiling() for i9xx planes
>   drm/i915/display: Add a disable_tiling() for skl planes
>   drm/ttm: Add ttm_bo_kmap_try_from_panic()
>   drm/i915: Add intel_bo_panic_setup and intel_bo_panic_finish
>   drm/i915/display: Add drm_panic support
>   drm/i915/display: Add drm_panic support for Y-tiling with DPT
>   drm/i915/display: Add drm_panic support for 4-tiling with DPT
>   drm/i915/psr: Add intel_psr2_panic_force_full_update
> 
>  drivers/gpu/drm/i915/display/i9xx_plane.c     |  23 +++
>  .../gpu/drm/i915/display/intel_atomic_plane.c | 170 +++++++++++++++++-
>  drivers/gpu/drm/i915/display/intel_bo.c       |  12 ++
>  drivers/gpu/drm/i915/display/intel_bo.h       |   4 +
>  .../drm/i915/display/intel_display_types.h    |  11 ++
>  drivers/gpu/drm/i915/display/intel_fb_pin.c   |   5 +
>  drivers/gpu/drm/i915/display/intel_fb_pin.h   |   2 +
>  drivers/gpu/drm/i915/display/intel_fbdev.c    |   5 +
>  drivers/gpu/drm/i915/display/intel_fbdev.h    |   6 +-
>  drivers/gpu/drm/i915/display/intel_psr.c      |  20 +++
>  drivers/gpu/drm/i915/display/intel_psr.h      |   2 +
>  .../drm/i915/display/skl_universal_plane.c    |  27 +++
>  drivers/gpu/drm/i915/gem/i915_gem_object.h    |   5 +
>  drivers/gpu/drm/i915/gem/i915_gem_pages.c     | 112 ++++++++++++
>  drivers/gpu/drm/i915/i915_vma.h               |   5 +
>  drivers/gpu/drm/ttm/ttm_bo_util.c             |  27 +++
>  drivers/gpu/drm/xe/display/intel_bo.c         |  61 +++++++
>  drivers/gpu/drm/xe/display/xe_fb_pin.c        |   5 +
>  include/drm/drm_panic.h                       |   6 +
>  include/drm/ttm/ttm_bo.h                      |   1 +
>  20 files changed, 507 insertions(+), 2 deletions(-)
> 
> 
> base-commit: b2f7e30d2e4a34fcee8111d713bef4f29dc23c77


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

* Re: [PATCH v10 00/10] drm/i915: Add drm_panic support
  2025-06-23  7:40 ` [PATCH v10 00/10] drm/i915: Add drm_panic support Maarten Lankhorst
@ 2025-06-23 10:10   ` Jocelyn Falempe
  2025-06-23 12:02     ` Maarten Lankhorst
  0 siblings, 1 reply; 20+ messages in thread
From: Jocelyn Falempe @ 2025-06-23 10:10 UTC (permalink / raw)
  To: Maarten Lankhorst, Jani Nikula, Rodrigo Vivi, Joonas Lahtinen,
	Tvrtko Ursulin, Ville Syrjälä, David Airlie,
	Simona Vetter, Christian Koenig, Huang Rui, Matthew Auld,
	Matthew Brost, Maxime Ripard, Thomas Zimmermann, intel-gfx,
	intel-xe, dri-devel, linux-kernel

On 23/06/2025 09:40, Maarten Lankhorst wrote:
> Hey,
> 
> Thanks for the series. I didn't see you on irc so I wanted to ask if you are planning to send a v11 with
> the changes from void * to struct intel_panic_data and adding the VRAM support?

Yes, I'm preparing a v11, and I'm considering to do something like this, 
to allocate the panic data with the struct intel_framebuffer:

struct xe_framebuffer {
	struct intel_framebuffer base;
	struct xe_panic_data panic;
};

struct intel_framebuffer *intel_bo_alloc_framebuffer(void)
{
	struct xe_framebuffer *xe_fb;

	xe_fb = kmalloc(sizeof(struct xe_framebuffer), GFP_KERNEL);
	return &xe_fb->base;
}

(And the same for i915).
That should allow you to add battlemage support.

> 
> 
> Other than that, I think the series looks good and I'll be able to test it on my battlemage.
> 

Thanks

Best regards,

-- 

Jocelyn

> Best regards,
> ~Maarten
> 
> On 2025-06-18 11:31, Jocelyn Falempe wrote:
>> This adds drm_panic support for i915 and xe driver.
>>
>> I've tested it on the 4 intel laptops I have at my disposal.
>>   * Haswell with 128MB of eDRAM.
>>   * Comet Lake  i7-10850H
>>   * Raptor Lake i7-1370P (with DPT, and Y-tiling).
>>   * Lunar Lake Ultra 5 228V (with DPT, and 4-tiling, and using the Xe driver.
>>
>> I tested panic in both fbdev console and gnome desktop.
>> I think it won't work yet on discrete GPU, but that can be added later.
>>
>> Best regards,
>>
>> v2:
>>   * Add the proper abstractions to build also for Xe.
>>   * Fix dim checkpatch issues.
>>
>> v3:
>>   * Add support for Y-tiled framebuffer when DPT is enabled.
>>
>> v4:
>>   * Add support for Xe driver, which shares most of the code.
>>   * Add support for 4-tiled framebuffer found in newest GPU.
>>
>> v5:
>>   * Rebase on top of git@gitlab.freedesktop.org:drm/i915/kernel.git drm-intel-next
>>   * Use struct intel_display instead of drm_i915_private.
>>   * Use iosys_map for intel_bo_panic_map().
>>
>> v6:
>>   * Rebase on top of git@gitlab.freedesktop.org:drm/i915/kernel.git drm-intel-next
>>   * Use struct intel_display instead of drm_i915_private for intel_atomic_plane.c
>>
>> v7:
>>   * Fix mismatch {} in intel_panic_flush() (Jani Nikula)
>>   * Return int for i915_gem_object_panic_map() (Ville Syrjälä)
>>   * Reword commit message about alignment/size when disabling tiling (Ville Syrjälä)
>>
>> v8:
>>   * Use kmap_try_from_panic() instead of vmap, to access the framebuffer.
>>   * Add ttm_bo_kmap_try_from_panic() for the xe driver, that uses ttm.
>>   * Replace intel_bo_panic_map() with a setup() and finish() function,
>>     to allow mapping only one page of teh framebuffer at a time.
>>   * Configure psr to send the full framebuffer update.
>>
>> v9:
>>   * Fix comment in ttm_bo_kmap_try_from_panic(), this can *only* be called
>>     from the panic handler (Christian König)
>>   * Fix missing kfree() for i915_panic_pages in i915_gem_object_panic_finish()
>>     Also change i915_panic_pages allocation to kmalloc, as kvmalloc is not
>>     safe to call from the panic handler.
>>   * Fix dim checkpatch warnings.
>>
>> v10:
>>   * Add a private field to struct drm_scanout_buffer
>>   * Replace static variables with new fields in struct intel_framebuffer
>>     (Maarten Lankhorst)
>>   * Add error handling if i915_gem_object_panic_pages() returns NULL
>>   * Declare struct drm_scanout_buffer instead of including <drm/drm_panic.h>
>>     in intel_bo.h
>>
>> Jocelyn Falempe (10):
>>    drm/panic: Add a private field to struct drm_scanout_buffer
>>    drm/i915/fbdev: Add intel_fbdev_get_map()
>>    drm/i915/display/i9xx: Add a disable_tiling() for i9xx planes
>>    drm/i915/display: Add a disable_tiling() for skl planes
>>    drm/ttm: Add ttm_bo_kmap_try_from_panic()
>>    drm/i915: Add intel_bo_panic_setup and intel_bo_panic_finish
>>    drm/i915/display: Add drm_panic support
>>    drm/i915/display: Add drm_panic support for Y-tiling with DPT
>>    drm/i915/display: Add drm_panic support for 4-tiling with DPT
>>    drm/i915/psr: Add intel_psr2_panic_force_full_update
>>
>>   drivers/gpu/drm/i915/display/i9xx_plane.c     |  23 +++
>>   .../gpu/drm/i915/display/intel_atomic_plane.c | 170 +++++++++++++++++-
>>   drivers/gpu/drm/i915/display/intel_bo.c       |  12 ++
>>   drivers/gpu/drm/i915/display/intel_bo.h       |   4 +
>>   .../drm/i915/display/intel_display_types.h    |  11 ++
>>   drivers/gpu/drm/i915/display/intel_fb_pin.c   |   5 +
>>   drivers/gpu/drm/i915/display/intel_fb_pin.h   |   2 +
>>   drivers/gpu/drm/i915/display/intel_fbdev.c    |   5 +
>>   drivers/gpu/drm/i915/display/intel_fbdev.h    |   6 +-
>>   drivers/gpu/drm/i915/display/intel_psr.c      |  20 +++
>>   drivers/gpu/drm/i915/display/intel_psr.h      |   2 +
>>   .../drm/i915/display/skl_universal_plane.c    |  27 +++
>>   drivers/gpu/drm/i915/gem/i915_gem_object.h    |   5 +
>>   drivers/gpu/drm/i915/gem/i915_gem_pages.c     | 112 ++++++++++++
>>   drivers/gpu/drm/i915/i915_vma.h               |   5 +
>>   drivers/gpu/drm/ttm/ttm_bo_util.c             |  27 +++
>>   drivers/gpu/drm/xe/display/intel_bo.c         |  61 +++++++
>>   drivers/gpu/drm/xe/display/xe_fb_pin.c        |   5 +
>>   include/drm/drm_panic.h                       |   6 +
>>   include/drm/ttm/ttm_bo.h                      |   1 +
>>   20 files changed, 507 insertions(+), 2 deletions(-)
>>
>>
>> base-commit: b2f7e30d2e4a34fcee8111d713bef4f29dc23c77
> 


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

* Re: [PATCH v10 00/10] drm/i915: Add drm_panic support
  2025-06-23 10:10   ` Jocelyn Falempe
@ 2025-06-23 12:02     ` Maarten Lankhorst
  0 siblings, 0 replies; 20+ messages in thread
From: Maarten Lankhorst @ 2025-06-23 12:02 UTC (permalink / raw)
  To: Jocelyn Falempe, Jani Nikula, Rodrigo Vivi, Joonas Lahtinen,
	Tvrtko Ursulin, Ville Syrjälä, David Airlie,
	Simona Vetter, Christian Koenig, Huang Rui, Matthew Auld,
	Matthew Brost, Maxime Ripard, Thomas Zimmermann, intel-gfx,
	intel-xe, dri-devel, linux-kernel

Hey,

On 2025-06-23 12:10, Jocelyn Falempe wrote:
> On 23/06/2025 09:40, Maarten Lankhorst wrote:
>> Hey,
>>
>> Thanks for the series. I didn't see you on irc so I wanted to ask if you are planning to send a v11 with
>> the changes from void * to struct intel_panic_data and adding the VRAM support?
> 
> Yes, I'm preparing a v11, and I'm considering to do something like this, to allocate the panic data with the struct intel_framebuffer:
> 
> struct xe_framebuffer {
>     struct intel_framebuffer base;
>     struct xe_panic_data panic;
> };
> 
> struct intel_framebuffer *intel_bo_alloc_framebuffer(void)
> {
>     struct xe_framebuffer *xe_fb;
> 
>     xe_fb = kmalloc(sizeof(struct xe_framebuffer), GFP_KERNEL);
>     return &xe_fb->base;
> }
> 
> (And the same for i915).
> That should allow you to add battlemage support.
> 
>>
>>
>> Other than that, I think the series looks good and I'll be able to test it on my battlemage.
>>

A private member is fine, but if you can get signoff from the i915 people for the xe_fb slpit, I would really like that for other things too. :)

Kind regards,
~Maarten


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

* Re: [PATCH v10 05/10] drm/ttm: Add ttm_bo_kmap_try_from_panic()
  2025-06-18 13:55   ` Christian König
  2025-06-18 15:38     ` Jocelyn Falempe
@ 2025-06-27 10:05     ` Maarten Lankhorst
  1 sibling, 0 replies; 20+ messages in thread
From: Maarten Lankhorst @ 2025-06-27 10:05 UTC (permalink / raw)
  To: Christian König, Jocelyn Falempe, Jani Nikula, Rodrigo Vivi,
	Joonas Lahtinen, Tvrtko Ursulin, Ville Syrjälä,
	David Airlie, Simona Vetter, Huang Rui, Matthew Auld,
	Matthew Brost, Maxime Ripard, Thomas Zimmermann, intel-gfx,
	intel-xe, dri-devel, linux-kernel

Hey,

On 2025-06-18 15:55, Christian König wrote:
> 
> 
> On 6/18/25 11:31, Jocelyn Falempe wrote:
>> If the ttm bo is backed by pages, then it's possible to safely kmap
>> one page at a time, using kmap_try_from_panic().
>> Unfortunately there is no way to do the same with ioremap, so it
>> only supports the kmap case.
>> This is needed for proper drm_panic support with xe driver.
>>
>> Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
> 
> Reviewed-by: Christian König <christian.koenig@amd.com>
> 
> Preferred through drm-misc-next, but feel free to merge it through every branch you want if it makes thinks easier for you.
> 
Thanks for the ack there. I had to merge this patch through drm-intel-next-queued because of a rework affecting the series.

Kind regards,
~Maarten
> Regards,
> Christian.
> 
>> ---
>>
>> v8:
>>  * Added in v8
>>
>> v9:
>>  * Fix comment in ttm_bo_kmap_try_from_panic(), this can *only* be called
>>    from the panic handler (Christian König)
>>
>>  drivers/gpu/drm/ttm/ttm_bo_util.c | 27 +++++++++++++++++++++++++++
>>  include/drm/ttm/ttm_bo.h          |  1 +
>>  2 files changed, 28 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
>> index 15cab9bda17f..6912e6dfda25 100644
>> --- a/drivers/gpu/drm/ttm/ttm_bo_util.c
>> +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
>> @@ -377,6 +377,33 @@ static int ttm_bo_kmap_ttm(struct ttm_buffer_object *bo,
>>  	return (!map->virtual) ? -ENOMEM : 0;
>>  }
>>  
>> +/**
>> + *
>> + * ttm_bo_kmap_try_from_panic
>> + *
>> + * @bo: The buffer object
>> + * @page: The page to map
>> + *
>> + * Sets up a kernel virtual mapping using kmap_local_page_try_from_panic().
>> + * This should only be called from the panic handler, if you make sure the bo
>> + * is the one being displayed, so is properly allocated, and protected.
>> + *
>> + * Returns the vaddr, that you can use to write to the bo, and that you should
>> + * pass to kunmap_local() when you're done with this page, or NULL if the bo
>> + * is in iomem.
>> + */
>> +void *ttm_bo_kmap_try_from_panic(struct ttm_buffer_object *bo, unsigned long page)
>> +{
>> +	if (page + 1 > PFN_UP(bo->resource->size))
>> +		return NULL;
>> +
>> +	if (!bo->resource->bus.is_iomem && bo->ttm->pages && bo->ttm->pages[page])
>> +		return kmap_local_page_try_from_panic(bo->ttm->pages[page]);
>> +
>> +	return NULL;
>> +}
>> +EXPORT_SYMBOL(ttm_bo_kmap_try_from_panic);
>> +
>>  /**
>>   * ttm_bo_kmap
>>   *
>> diff --git a/include/drm/ttm/ttm_bo.h b/include/drm/ttm/ttm_bo.h
>> index cf027558b6db..8c0ce3fa077f 100644
>> --- a/include/drm/ttm/ttm_bo.h
>> +++ b/include/drm/ttm/ttm_bo.h
>> @@ -429,6 +429,7 @@ int ttm_bo_init_validate(struct ttm_device *bdev, struct ttm_buffer_object *bo,
>>  int ttm_bo_kmap(struct ttm_buffer_object *bo, unsigned long start_page,
>>  		unsigned long num_pages, struct ttm_bo_kmap_obj *map);
>>  void ttm_bo_kunmap(struct ttm_bo_kmap_obj *map);
>> +void *ttm_bo_kmap_try_from_panic(struct ttm_buffer_object *bo, unsigned long page);
>>  int ttm_bo_vmap(struct ttm_buffer_object *bo, struct iosys_map *map);
>>  void ttm_bo_vunmap(struct ttm_buffer_object *bo, struct iosys_map *map);
>>  int ttm_bo_mmap_obj(struct vm_area_struct *vma, struct ttm_buffer_object *bo);
> 


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

* Re: [PATCH v10 03/10] drm/i915/display/i9xx: Add a disable_tiling() for i9xx planes
  2025-06-18  9:31 ` [PATCH v10 03/10] drm/i915/display/i9xx: Add a disable_tiling() for i9xx planes Jocelyn Falempe
@ 2025-07-19 18:23   ` Ville Syrjälä
  2025-07-19 18:30     ` Ville Syrjälä
  2025-07-28 11:19     ` Jocelyn Falempe
  0 siblings, 2 replies; 20+ messages in thread
From: Ville Syrjälä @ 2025-07-19 18:23 UTC (permalink / raw)
  To: Jocelyn Falempe
  Cc: Maarten Lankhorst, Jani Nikula, Rodrigo Vivi, Joonas Lahtinen,
	Tvrtko Ursulin, David Airlie, Simona Vetter, Christian Koenig,
	Huang Rui, Matthew Auld, Matthew Brost, Maxime Ripard,
	Thomas Zimmermann, intel-gfx, intel-xe, dri-devel, linux-kernel

On Wed, Jun 18, 2025 at 11:31:21AM +0200, Jocelyn Falempe wrote:
> drm_panic draws in linear framebuffer, so it's easier to re-use the
> current framebuffer, and disable tiling in the panic handler, to show
> the panic screen.
> This assumes that the alignment restriction is always smaller in
> linear than in tiled.
> It also assumes that the linear framebuffer size is always smaller
> than the tiled.
> 
> Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
> ---
> 
> v7:
>  * Reword commit message about alignment/size when disabling tiling (Ville Syrjälä)
> 
>  drivers/gpu/drm/i915/display/i9xx_plane.c     | 23 +++++++++++++++++++
>  .../drm/i915/display/intel_display_types.h    |  2 ++
>  2 files changed, 25 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/i9xx_plane.c b/drivers/gpu/drm/i915/display/i9xx_plane.c
> index 8f15333a4b07..0807fae12450 100644
> --- a/drivers/gpu/drm/i915/display/i9xx_plane.c
> +++ b/drivers/gpu/drm/i915/display/i9xx_plane.c
> @@ -905,6 +905,27 @@ static const struct drm_plane_funcs i8xx_plane_funcs = {
>  	.format_mod_supported_async = intel_plane_format_mod_supported_async,
>  };
>  
> +static void i9xx_disable_tiling(struct intel_plane *plane)
> +{
> +	struct intel_display *display = to_intel_display(plane);
> +	enum i9xx_plane_id i9xx_plane = plane->i9xx_plane;
> +	u32 dspcntr;
> +	u32 reg;
> +
> +	dspcntr = intel_de_read_fw(display, DSPCNTR(display, i9xx_plane));
> +	dspcntr &= ~DISP_TILED;
> +	intel_de_write_fw(display, DSPCNTR(display, i9xx_plane), dspcntr);
> +
> +	if (DISPLAY_VER(display) >= 4) {
> +		reg = intel_de_read_fw(display, DSPSURF(display, i9xx_plane));
> +		intel_de_write_fw(display, DSPSURF(display, i9xx_plane), reg);
> +
> +	} else {
> +		reg = intel_de_read_fw(display, DSPADDR(display, i9xx_plane));
> +		intel_de_write_fw(display, DSPADDR(display, i9xx_plane), reg);
> +	}
> +}

I thought I already shot this down before, but apparently this
got merged now :(

Just to reiterate why we don't want these 'disable tiling' hacks:
- different tiling formats have different stride/alignment/watermark
  requirements so one can't safely change from one tiling to another
- this completely fails to account for the TILEOFF vs. LINOFF stuff
- etc.

So IMO these hacks must be removed and instead the code must learn how
to propetly write the tiled data. igt has all the code for that btw
(twice over IIRC) so shouldn't be that hard.

I suppose the only hack we need to keep is to disable compression,
mainly because (IIRC) on flat CCS systems the CPU doesn't have access
to the AUX data to clear it manually.

I also wonder if there are actual igts for this? I think what is needed
is a test that sets random things (different panning, rotation, pixel
foramts, etc.) and triggers the dumper. Not quite sure how the test
could validate that the output is correct though. CRCs might be a bit
tricky since you need an identical reference image.

/me off to summer vacation. Good luck

-- 
Ville Syrjälä
Intel

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

* Re: [PATCH v10 03/10] drm/i915/display/i9xx: Add a disable_tiling() for i9xx planes
  2025-07-19 18:23   ` Ville Syrjälä
@ 2025-07-19 18:30     ` Ville Syrjälä
  2025-07-28 11:19     ` Jocelyn Falempe
  1 sibling, 0 replies; 20+ messages in thread
From: Ville Syrjälä @ 2025-07-19 18:30 UTC (permalink / raw)
  To: Jocelyn Falempe
  Cc: Maarten Lankhorst, Jani Nikula, Rodrigo Vivi, Joonas Lahtinen,
	Tvrtko Ursulin, David Airlie, Simona Vetter, Christian Koenig,
	Huang Rui, Matthew Auld, Matthew Brost, Maxime Ripard,
	Thomas Zimmermann, intel-gfx, intel-xe, dri-devel, linux-kernel

On Sat, Jul 19, 2025 at 09:23:04PM +0300, Ville Syrjälä wrote:
> On Wed, Jun 18, 2025 at 11:31:21AM +0200, Jocelyn Falempe wrote:
> > drm_panic draws in linear framebuffer, so it's easier to re-use the
> > current framebuffer, and disable tiling in the panic handler, to show
> > the panic screen.
> > This assumes that the alignment restriction is always smaller in
> > linear than in tiled.
> > It also assumes that the linear framebuffer size is always smaller
> > than the tiled.
> > 
> > Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
> > ---
> > 
> > v7:
> >  * Reword commit message about alignment/size when disabling tiling (Ville Syrjälä)
> > 
> >  drivers/gpu/drm/i915/display/i9xx_plane.c     | 23 +++++++++++++++++++
> >  .../drm/i915/display/intel_display_types.h    |  2 ++
> >  2 files changed, 25 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/i9xx_plane.c b/drivers/gpu/drm/i915/display/i9xx_plane.c
> > index 8f15333a4b07..0807fae12450 100644
> > --- a/drivers/gpu/drm/i915/display/i9xx_plane.c
> > +++ b/drivers/gpu/drm/i915/display/i9xx_plane.c
> > @@ -905,6 +905,27 @@ static const struct drm_plane_funcs i8xx_plane_funcs = {
> >  	.format_mod_supported_async = intel_plane_format_mod_supported_async,
> >  };
> >  
> > +static void i9xx_disable_tiling(struct intel_plane *plane)
> > +{
> > +	struct intel_display *display = to_intel_display(plane);
> > +	enum i9xx_plane_id i9xx_plane = plane->i9xx_plane;
> > +	u32 dspcntr;
> > +	u32 reg;
> > +
> > +	dspcntr = intel_de_read_fw(display, DSPCNTR(display, i9xx_plane));
> > +	dspcntr &= ~DISP_TILED;
> > +	intel_de_write_fw(display, DSPCNTR(display, i9xx_plane), dspcntr);
> > +
> > +	if (DISPLAY_VER(display) >= 4) {
> > +		reg = intel_de_read_fw(display, DSPSURF(display, i9xx_plane));
> > +		intel_de_write_fw(display, DSPSURF(display, i9xx_plane), reg);
> > +
> > +	} else {
> > +		reg = intel_de_read_fw(display, DSPADDR(display, i9xx_plane));
> > +		intel_de_write_fw(display, DSPADDR(display, i9xx_plane), reg);
> > +	}
> > +}
> 
> I thought I already shot this down before, but apparently this
> got merged now :(
> 
> Just to reiterate why we don't want these 'disable tiling' hacks:
> - different tiling formats have different stride/alignment/watermark
>   requirements so one can't safely change from one tiling to another
> - this completely fails to account for the TILEOFF vs. LINOFF stuff

Oh yeah, and rotation support of course is one really big difference
between different tiling formats.

> - etc.
> 
> So IMO these hacks must be removed and instead the code must learn how
> to propetly write the tiled data. igt has all the code for that btw
> (twice over IIRC) so shouldn't be that hard.
> 
> I suppose the only hack we need to keep is to disable compression,
> mainly because (IIRC) on flat CCS systems the CPU doesn't have access
> to the AUX data to clear it manually.
> 
> I also wonder if there are actual igts for this? I think what is needed
> is a test that sets random things (different panning, rotation, pixel
> foramts, etc.) and triggers the dumper. Not quite sure how the test
> could validate that the output is correct though. CRCs might be a bit
> tricky since you need an identical reference image.
> 
> /me off to summer vacation. Good luck
> 
> -- 
> Ville Syrjälä
> Intel

-- 
Ville Syrjälä
Intel

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

* Re: [PATCH v10 03/10] drm/i915/display/i9xx: Add a disable_tiling() for i9xx planes
  2025-07-19 18:23   ` Ville Syrjälä
  2025-07-19 18:30     ` Ville Syrjälä
@ 2025-07-28 11:19     ` Jocelyn Falempe
  1 sibling, 0 replies; 20+ messages in thread
From: Jocelyn Falempe @ 2025-07-28 11:19 UTC (permalink / raw)
  To: Ville Syrjälä
  Cc: Maarten Lankhorst, Jani Nikula, Rodrigo Vivi, Joonas Lahtinen,
	Tvrtko Ursulin, David Airlie, Simona Vetter, Christian Koenig,
	Huang Rui, Matthew Auld, Matthew Brost, Maxime Ripard,
	Thomas Zimmermann, intel-gfx, intel-xe, dri-devel, linux-kernel

On 19/07/2025 20:23, Ville Syrjälä wrote:
> On Wed, Jun 18, 2025 at 11:31:21AM +0200, Jocelyn Falempe wrote:
>> drm_panic draws in linear framebuffer, so it's easier to re-use the
>> current framebuffer, and disable tiling in the panic handler, to show
>> the panic screen.
>> This assumes that the alignment restriction is always smaller in
>> linear than in tiled.
>> It also assumes that the linear framebuffer size is always smaller
>> than the tiled.
>>
>> Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
>> ---
>>
>> v7:
>>   * Reword commit message about alignment/size when disabling tiling (Ville Syrjälä)
>>
>>   drivers/gpu/drm/i915/display/i9xx_plane.c     | 23 +++++++++++++++++++
>>   .../drm/i915/display/intel_display_types.h    |  2 ++
>>   2 files changed, 25 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/display/i9xx_plane.c b/drivers/gpu/drm/i915/display/i9xx_plane.c
>> index 8f15333a4b07..0807fae12450 100644
>> --- a/drivers/gpu/drm/i915/display/i9xx_plane.c
>> +++ b/drivers/gpu/drm/i915/display/i9xx_plane.c
>> @@ -905,6 +905,27 @@ static const struct drm_plane_funcs i8xx_plane_funcs = {
>>   	.format_mod_supported_async = intel_plane_format_mod_supported_async,
>>   };
>>   
>> +static void i9xx_disable_tiling(struct intel_plane *plane)
>> +{
>> +	struct intel_display *display = to_intel_display(plane);
>> +	enum i9xx_plane_id i9xx_plane = plane->i9xx_plane;
>> +	u32 dspcntr;
>> +	u32 reg;
>> +
>> +	dspcntr = intel_de_read_fw(display, DSPCNTR(display, i9xx_plane));
>> +	dspcntr &= ~DISP_TILED;
>> +	intel_de_write_fw(display, DSPCNTR(display, i9xx_plane), dspcntr);
>> +
>> +	if (DISPLAY_VER(display) >= 4) {
>> +		reg = intel_de_read_fw(display, DSPSURF(display, i9xx_plane));
>> +		intel_de_write_fw(display, DSPSURF(display, i9xx_plane), reg);
>> +
>> +	} else {
>> +		reg = intel_de_read_fw(display, DSPADDR(display, i9xx_plane));
>> +		intel_de_write_fw(display, DSPADDR(display, i9xx_plane), reg);
>> +	}
>> +}
> 
> I thought I already shot this down before, but apparently this
> got merged now :(

Sorry for that. I replied to that thread, but I didn't get answer [1]

> 
> Just to reiterate why we don't want these 'disable tiling' hacks:
> - different tiling formats have different stride/alignment/watermark
>    requirements so one can't safely change from one tiling to another

I agree that going from one tiling format to another is not safe. But 
from my understanding, going from tiling to linear should be possible.
Do you have an example, where the stride/alignment/watermark requirement 
in tiled would be incompatible in Linear (for the same resolution)?

> - this completely fails to account for the TILEOFF vs. LINOFF stuff

Pardon my ignorance, can you explain what it is, and how it can break or 
make the output unreadable?

> - etc.
> 
> So IMO these hacks must be removed and instead the code must learn how
> to propetly write the tiled data. igt has all the code for that btw
> (twice over IIRC) so shouldn't be that hard.

Regarding the tiling format, I usually test on hardware to check that 
the image is correct. But I have only a few of them, and as the format 
is platform dependent, and sometime also depends on the memory 
configuration. For me it looks very hard to get it right.
I've done it only for Y-tile and 4-tile, but only when DPT is enabled 
(which means it's only the few latest generations).

> 
> I suppose the only hack we need to keep is to disable compression,
> mainly because (IIRC) on flat CCS systems the CPU doesn't have access
> to the AUX data to clear it manually.
> 
> I also wonder if there are actual igts for this? I think what is needed
> is a test that sets random things (different panning, rotation, pixel
> foramts, etc.) and triggers the dumper. Not quite sure how the test
> could validate that the output is correct though. CRCs might be a bit
> tricky since you need an identical reference image.

No, I didn't write igts for this yet. I test by triggering a kernel 
panic, as it's the only way to make sure it works.
Also I didn't consider rotation yet, I think if the panic screen is not 
rotated, it's still useful.

> 
> /me off to summer vacation. Good luck
> 

Sorry for that, my goal is just to have drm panic working on intel GPU.
Enjoy your vacation, and let's find a solution when you're back.

[1] 
https://lore.kernel.org/intel-gfx/72fa1da6-caaa-41c9-aef1-4e780bde6acf@redhat.com/

Best regards,

-- 

Jocelyn


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

end of thread, other threads:[~2025-07-28 11:19 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-18  9:31 [PATCH v10 00/10] drm/i915: Add drm_panic support Jocelyn Falempe
2025-06-18  9:31 ` [PATCH v10 01/10] drm/panic: Add a private field to struct drm_scanout_buffer Jocelyn Falempe
2025-06-18  9:31 ` [PATCH v10 02/10] drm/i915/fbdev: Add intel_fbdev_get_map() Jocelyn Falempe
2025-06-18  9:31 ` [PATCH v10 03/10] drm/i915/display/i9xx: Add a disable_tiling() for i9xx planes Jocelyn Falempe
2025-07-19 18:23   ` Ville Syrjälä
2025-07-19 18:30     ` Ville Syrjälä
2025-07-28 11:19     ` Jocelyn Falempe
2025-06-18  9:31 ` [PATCH v10 04/10] drm/i915/display: Add a disable_tiling() for skl planes Jocelyn Falempe
2025-06-18  9:31 ` [PATCH v10 05/10] drm/ttm: Add ttm_bo_kmap_try_from_panic() Jocelyn Falempe
2025-06-18 13:55   ` Christian König
2025-06-18 15:38     ` Jocelyn Falempe
2025-06-27 10:05     ` Maarten Lankhorst
2025-06-18  9:31 ` [PATCH v10 06/10] drm/i915: Add intel_bo_panic_setup and intel_bo_panic_finish Jocelyn Falempe
2025-06-18  9:31 ` [PATCH v10 07/10] drm/i915/display: Add drm_panic support Jocelyn Falempe
2025-06-18  9:31 ` [PATCH v10 08/10] drm/i915/display: Add drm_panic support for Y-tiling with DPT Jocelyn Falempe
2025-06-18  9:31 ` [PATCH v10 09/10] drm/i915/display: Add drm_panic support for 4-tiling " Jocelyn Falempe
2025-06-18  9:31 ` [PATCH v10 10/10] drm/i915/psr: Add intel_psr2_panic_force_full_update Jocelyn Falempe
2025-06-23  7:40 ` [PATCH v10 00/10] drm/i915: Add drm_panic support Maarten Lankhorst
2025-06-23 10:10   ` Jocelyn Falempe
2025-06-23 12:02     ` Maarten Lankhorst

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).