[PATCH v2 0/4] drm/i915/display: add snapshot capture/print infra

[PATCH v2 0/4] drm/i915/display: add snapshot capture/print infra

[Jani Nikula]

Another spin of [1].

BR,
Jani.

[1] https://lore.kernel.org/r/cover.1725372032.git.jani.nikula@intel.com


Jani Nikula (4):
  drm/i915: dump display parameters captured in error state, not current
  drm/i915/display: add intel_display_snapshot abstraction
  drm/i915/display: move device info and params handling to snapshot
  drm/i915/display: move dmc snapshotting to new display snapshot

 drivers/gpu/drm/i915/Makefile                 |  1 +
 .../drm/i915/display/intel_display_params.c   |  8 ++-
 .../drm/i915/display/intel_display_params.h   |  5 +-
 .../drm/i915/display/intel_display_snapshot.c | 72 +++++++++++++++++++
 .../drm/i915/display/intel_display_snapshot.h | 16 +++++
 drivers/gpu/drm/i915/display/intel_dmc.c      | 39 +++++++---
 drivers/gpu/drm/i915/display/intel_dmc.h      |  7 +-
 drivers/gpu/drm/i915/display/intel_overlay.c  | 16 +++--
 drivers/gpu/drm/i915/display/intel_overlay.h  | 25 ++++---
 drivers/gpu/drm/i915/i915_debugfs.c           |  2 +-
 drivers/gpu/drm/i915/i915_gpu_error.c         | 25 ++-----
 drivers/gpu/drm/i915/i915_gpu_error.h         | 11 +--
 12 files changed, 168 insertions(+), 59 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/display/intel_display_snapshot.c
 create mode 100644 drivers/gpu/drm/i915/display/intel_display_snapshot.h

-- 
2.39.2

[PATCH v2 1/4] drm/i915: dump display parameters captured in error state, not current

[Jani Nikula]

intel_display_params_dump() prints the current display parameters, not
the ones captured during error capture. It's not likely the params get
changed in between, but make it pedantically correct anyway. Pass in the
parameters and driver name to intel_display_params_dump().

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display_params.c | 8 +++++---
 drivers/gpu/drm/i915/display/intel_display_params.h | 5 ++---
 drivers/gpu/drm/i915/i915_debugfs.c                 | 2 +-
 drivers/gpu/drm/i915/i915_gpu_error.c               | 2 +-
 4 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_params.c b/drivers/gpu/drm/i915/display/intel_display_params.c
index 1a45d300b6f0..024de8abcb1a 100644
--- a/drivers/gpu/drm/i915/display/intel_display_params.c
+++ b/drivers/gpu/drm/i915/display/intel_display_params.c
@@ -173,14 +173,16 @@ static void _param_print_charp(struct drm_printer *p, const char *driver_name,
 
 /**
  * intel_display_params_dump - dump intel display modparams
- * @display: display device
+ * @params: display params
+ * @driver_name: driver name to use for printing
  * @p: the &drm_printer
  *
  * Pretty printer for i915 modparams.
  */
-void intel_display_params_dump(struct intel_display *display, struct drm_printer *p)
+void intel_display_params_dump(const struct intel_display_params *params,
+			       const char *driver_name, struct drm_printer *p)
 {
-#define PRINT(T, x, ...) _param_print(p, display->drm->driver->name, #x, display->params.x);
+#define PRINT(T, x, ...) _param_print(p, driver_name, #x, params->x);
 	INTEL_DISPLAY_PARAMS_FOR_EACH(PRINT);
 #undef PRINT
 }
diff --git a/drivers/gpu/drm/i915/display/intel_display_params.h b/drivers/gpu/drm/i915/display/intel_display_params.h
index da8dc943234b..dcb6face936a 100644
--- a/drivers/gpu/drm/i915/display/intel_display_params.h
+++ b/drivers/gpu/drm/i915/display/intel_display_params.h
@@ -9,7 +9,6 @@
 #include <linux/types.h>
 
 struct drm_printer;
-struct intel_display;
 
 /*
  * Invoke param, a function-like macro, for each intel display param, with
@@ -56,8 +55,8 @@ struct intel_display_params {
 };
 #undef MEMBER
 
-void intel_display_params_dump(struct intel_display *display,
-			       struct drm_printer *p);
+void intel_display_params_dump(const struct intel_display_params *params,
+			       const char *driver_name, struct drm_printer *p);
 void intel_display_params_copy(struct intel_display_params *dest);
 void intel_display_params_free(struct intel_display_params *params);
 
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index f969f585d07b..246fece628d6 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -78,7 +78,7 @@ static int i915_capabilities(struct seq_file *m, void *data)
 
 	kernel_param_lock(THIS_MODULE);
 	i915_params_dump(&i915->params, &p);
-	intel_display_params_dump(display, &p);
+	intel_display_params_dump(&display->params, display->drm->driver->name, &p);
 	kernel_param_unlock(THIS_MODULE);
 
 	return 0;
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index 6469b9bcf2ec..f23769ccf050 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -663,7 +663,7 @@ static void err_print_params(struct drm_i915_error_state_buf *m,
 	struct intel_display *display = &m->i915->display;
 
 	i915_params_dump(params, &p);
-	intel_display_params_dump(display, &p);
+	intel_display_params_dump(&display->params, display->drm->driver->name, &p);
 }
 
 static void err_print_pciid(struct drm_i915_error_state_buf *m,
-- 
2.39.2

[PATCH v2 2/4] drm/i915/display: add intel_display_snapshot abstraction

[Jani Nikula]

The error state capture still handles display info at a too detailed
level. Start abstracting the whole display snapshot capture and printing
at a higher level. Move overlay to display snapshot first.

Use the same nomenclature and style as in xe devcoredump, in preparation
for perhaps some day bolting the snapshots there as well.

v3: Fix build harder for CONFIG_DRM_I915_CAPTURE_ERROR=n

v2: Fix build for CONFIG_DRM_I915_CAPTURE_ERROR=n (kernel test robot)

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/Makefile                 |  1 +
 .../drm/i915/display/intel_display_snapshot.c | 42 +++++++++++++++++++
 .../drm/i915/display/intel_display_snapshot.h | 16 +++++++
 drivers/gpu/drm/i915/display/intel_overlay.c  | 16 ++++---
 drivers/gpu/drm/i915/display/intel_overlay.h  | 25 +++++++----
 drivers/gpu/drm/i915/i915_gpu_error.c         | 12 +++---
 drivers/gpu/drm/i915/i915_gpu_error.h         |  6 +--
 7 files changed, 94 insertions(+), 24 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/display/intel_display_snapshot.c
 create mode 100644 drivers/gpu/drm/i915/display/intel_display_snapshot.h

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index c63fa2133ccb..9fcd9e09bc0b 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -242,6 +242,7 @@ i915-y += \
 	display/intel_display_power_well.o \
 	display/intel_display_reset.o \
 	display/intel_display_rps.o \
+	display/intel_display_snapshot.o \
 	display/intel_display_wa.o \
 	display/intel_dmc.o \
 	display/intel_dmc_wl.o \
diff --git a/drivers/gpu/drm/i915/display/intel_display_snapshot.c b/drivers/gpu/drm/i915/display/intel_display_snapshot.c
new file mode 100644
index 000000000000..78b019dcd41d
--- /dev/null
+++ b/drivers/gpu/drm/i915/display/intel_display_snapshot.c
@@ -0,0 +1,42 @@
+// SPDX-License-Identifier: MIT
+/* Copyright © 2024 Intel Corporation */
+
+#include <linux/slab.h>
+
+#include "intel_display_snapshot.h"
+#include "intel_overlay.h"
+
+struct intel_display_snapshot {
+	struct intel_overlay_snapshot *overlay;
+};
+
+struct intel_display_snapshot *intel_display_snapshot_capture(struct intel_display *display)
+{
+	struct intel_display_snapshot *snapshot;
+
+	snapshot = kzalloc(sizeof(*snapshot), GFP_ATOMIC);
+	if (!snapshot)
+		return NULL;
+
+	snapshot->overlay = intel_overlay_snapshot_capture(display);
+
+	return snapshot;
+}
+
+void intel_display_snapshot_print(const struct intel_display_snapshot *snapshot,
+				  struct drm_printer *p)
+{
+	if (!snapshot)
+		return;
+
+	intel_overlay_snapshot_print(snapshot->overlay, p);
+}
+
+void intel_display_snapshot_free(struct intel_display_snapshot *snapshot)
+{
+	if (!snapshot)
+		return;
+
+	kfree(snapshot->overlay);
+	kfree(snapshot);
+}
diff --git a/drivers/gpu/drm/i915/display/intel_display_snapshot.h b/drivers/gpu/drm/i915/display/intel_display_snapshot.h
new file mode 100644
index 000000000000..7ed27cdea644
--- /dev/null
+++ b/drivers/gpu/drm/i915/display/intel_display_snapshot.h
@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: MIT */
+/* Copyright © 2024 Intel Corporation */
+
+#ifndef __INTEL_DISPLAY_SNAPSHOT_H__
+#define __INTEL_DISPLAY_SNAPSHOT_H__
+
+struct drm_printer;
+struct intel_display;
+struct intel_display_snapshot;
+
+struct intel_display_snapshot *intel_display_snapshot_capture(struct intel_display *display);
+void intel_display_snapshot_print(const struct intel_display_snapshot *snapshot,
+				  struct drm_printer *p);
+void intel_display_snapshot_free(struct intel_display_snapshot *snapshot);
+
+#endif /* __INTEL_DISPLAY_SNAPSHOT_H__ */
diff --git a/drivers/gpu/drm/i915/display/intel_overlay.c b/drivers/gpu/drm/i915/display/intel_overlay.c
index 06b1122ec13e..b89541458765 100644
--- a/drivers/gpu/drm/i915/display/intel_overlay.c
+++ b/drivers/gpu/drm/i915/display/intel_overlay.c
@@ -1457,18 +1457,19 @@ void intel_overlay_cleanup(struct drm_i915_private *dev_priv)
 
 #if IS_ENABLED(CONFIG_DRM_I915_CAPTURE_ERROR)
 
-struct intel_overlay_error_state {
+struct intel_overlay_snapshot {
 	struct overlay_registers regs;
 	unsigned long base;
 	u32 dovsta;
 	u32 isr;
 };
 
-struct intel_overlay_error_state *
-intel_overlay_capture_error_state(struct drm_i915_private *dev_priv)
+struct intel_overlay_snapshot *
+intel_overlay_snapshot_capture(struct intel_display *display)
 {
+	struct drm_i915_private *dev_priv = to_i915(display->drm);
 	struct intel_overlay *overlay = dev_priv->display.overlay;
-	struct intel_overlay_error_state *error;
+	struct intel_overlay_snapshot *error;
 
 	if (!overlay || !overlay->active)
 		return NULL;
@@ -1487,9 +1488,12 @@ intel_overlay_capture_error_state(struct drm_i915_private *dev_priv)
 }
 
 void
-intel_overlay_print_error_state(struct drm_printer *p,
-				struct intel_overlay_error_state *error)
+intel_overlay_snapshot_print(const struct intel_overlay_snapshot *error,
+			     struct drm_printer *p)
 {
+	if (!error)
+		return;
+
 	drm_printf(p, "Overlay, status: 0x%08x, interrupt: 0x%08x\n",
 		   error->dovsta, error->isr);
 	drm_printf(p, "  Register file at 0x%08lx:\n", error->base);
diff --git a/drivers/gpu/drm/i915/display/intel_overlay.h b/drivers/gpu/drm/i915/display/intel_overlay.h
index f28a09c062d0..eafac24d1de8 100644
--- a/drivers/gpu/drm/i915/display/intel_overlay.h
+++ b/drivers/gpu/drm/i915/display/intel_overlay.h
@@ -6,12 +6,15 @@
 #ifndef __INTEL_OVERLAY_H__
 #define __INTEL_OVERLAY_H__
 
+#include <linux/types.h>
+
 struct drm_device;
 struct drm_file;
 struct drm_i915_private;
 struct drm_printer;
+struct intel_display;
 struct intel_overlay;
-struct intel_overlay_error_state;
+struct intel_overlay_snapshot;
 
 #ifdef I915
 void intel_overlay_setup(struct drm_i915_private *dev_priv);
@@ -22,10 +25,6 @@ int intel_overlay_put_image_ioctl(struct drm_device *dev, void *data,
 int intel_overlay_attrs_ioctl(struct drm_device *dev, void *data,
 			      struct drm_file *file_priv);
 void intel_overlay_reset(struct drm_i915_private *dev_priv);
-struct intel_overlay_error_state *
-intel_overlay_capture_error_state(struct drm_i915_private *dev_priv);
-void intel_overlay_print_error_state(struct drm_printer *p,
-				     struct intel_overlay_error_state *error);
 #else
 static inline void intel_overlay_setup(struct drm_i915_private *dev_priv)
 {
@@ -50,13 +49,21 @@ static inline int intel_overlay_attrs_ioctl(struct drm_device *dev, void *data,
 static inline void intel_overlay_reset(struct drm_i915_private *dev_priv)
 {
 }
-static inline struct intel_overlay_error_state *
-intel_overlay_capture_error_state(struct drm_i915_private *dev_priv)
+#endif
+
+#if IS_ENABLED(CONFIG_DRM_I915_CAPTURE_ERROR) && defined(I915)
+struct intel_overlay_snapshot *
+intel_overlay_snapshot_capture(struct intel_display *display);
+void intel_overlay_snapshot_print(const struct intel_overlay_snapshot *error,
+				  struct drm_printer *p);
+#else
+static inline struct intel_overlay_snapshot *
+intel_overlay_snapshot_capture(struct intel_display *display)
 {
 	return NULL;
 }
-static inline void intel_overlay_print_error_state(struct drm_printer *p,
-						   struct intel_overlay_error_state *error)
+static inline void intel_overlay_snapshot_print(const struct intel_overlay_snapshot *error,
+						struct drm_printer *p)
 {
 }
 #endif
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index f23769ccf050..b047b24a90d5 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -40,8 +40,8 @@
 #include <drm/drm_cache.h>
 #include <drm/drm_print.h>
 
+#include "display/intel_display_snapshot.h"
 #include "display/intel_dmc.h"
-#include "display/intel_overlay.h"
 
 #include "gem/i915_gem_context.h"
 #include "gem/i915_gem_lmem.h"
@@ -905,11 +905,10 @@ static void __err_print_to_sgl(struct drm_i915_error_state_buf *m,
 		err_print_gt_info(m, error->gt);
 	}
 
-	if (error->overlay)
-		intel_overlay_print_error_state(&p, error->overlay);
-
 	err_print_capabilities(m, error);
 	err_print_params(m, &error->params);
+
+	intel_display_snapshot_print(error->display_snapshot, &p);
 }
 
 static int err_print_to_sgl(struct i915_gpu_coredump *error)
@@ -1077,7 +1076,7 @@ void __i915_gpu_coredump_free(struct kref *error_ref)
 		cleanup_gt(gt);
 	}
 
-	kfree(error->overlay);
+	intel_display_snapshot_free(error->display_snapshot);
 
 	cleanup_params(error);
 
@@ -2097,6 +2096,7 @@ static struct i915_gpu_coredump *
 __i915_gpu_coredump(struct intel_gt *gt, intel_engine_mask_t engine_mask, u32 dump_flags)
 {
 	struct drm_i915_private *i915 = gt->i915;
+	struct intel_display *display = &i915->display;
 	struct i915_gpu_coredump *error;
 
 	/* Check if GPU capture has been disabled */
@@ -2138,7 +2138,7 @@ __i915_gpu_coredump(struct intel_gt *gt, intel_engine_mask_t engine_mask, u32 du
 		error->simulated |= error->gt->simulated;
 	}
 
-	error->overlay = intel_overlay_capture_error_state(i915);
+	error->display_snapshot = intel_display_snapshot_capture(display);
 
 	return error;
 }
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.h b/drivers/gpu/drm/i915/i915_gpu_error.h
index 7c255bb1c319..1a11942d7800 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.h
+++ b/drivers/gpu/drm/i915/i915_gpu_error.h
@@ -31,7 +31,7 @@
 struct drm_i915_private;
 struct i915_vma_compress;
 struct intel_engine_capture_vma;
-struct intel_overlay_error_state;
+struct intel_display_snapshot;
 
 struct i915_vma_coredump {
 	struct i915_vma_coredump *next;
@@ -218,9 +218,9 @@ struct i915_gpu_coredump {
 	struct i915_params params;
 	struct intel_display_params display_params;
 
-	struct intel_overlay_error_state *overlay;
-
 	struct scatterlist *sgl, *fit;
+
+	struct intel_display_snapshot *display_snapshot;
 };
 
 struct i915_gpu_error {
-- 
2.39.2

[PATCH v2 3/4] drm/i915/display: move device info and params handling to snapshot

[Jani Nikula]

Snapshot display device and runtime info as well as display parameters
in display snapshot.

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 .../drm/i915/display/intel_display_snapshot.c | 25 +++++++++++++++++++
 drivers/gpu/drm/i915/i915_gpu_error.c         | 10 --------
 drivers/gpu/drm/i915/i915_gpu_error.h         |  5 ----
 3 files changed, 25 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_snapshot.c b/drivers/gpu/drm/i915/display/intel_display_snapshot.c
index 78b019dcd41d..a61ff0f81397 100644
--- a/drivers/gpu/drm/i915/display/intel_display_snapshot.c
+++ b/drivers/gpu/drm/i915/display/intel_display_snapshot.c
@@ -3,10 +3,18 @@
 
 #include <linux/slab.h>
 
+#include "i915_drv.h"
+#include "intel_display_device.h"
+#include "intel_display_params.h"
 #include "intel_display_snapshot.h"
 #include "intel_overlay.h"
 
 struct intel_display_snapshot {
+	struct intel_display *display;
+
+	struct intel_display_device_info info;
+	struct intel_display_runtime_info runtime_info;
+	struct intel_display_params params;
 	struct intel_overlay_snapshot *overlay;
 };
 
@@ -18,6 +26,14 @@ struct intel_display_snapshot *intel_display_snapshot_capture(struct intel_displ
 	if (!snapshot)
 		return NULL;
 
+	snapshot->display = display;
+
+	memcpy(&snapshot->info, DISPLAY_INFO(display), sizeof(snapshot->info));
+	memcpy(&snapshot->runtime_info, DISPLAY_RUNTIME_INFO(display),
+	       sizeof(snapshot->runtime_info));
+
+	intel_display_params_copy(&snapshot->params);
+
 	snapshot->overlay = intel_overlay_snapshot_capture(display);
 
 	return snapshot;
@@ -26,9 +42,16 @@ struct intel_display_snapshot *intel_display_snapshot_capture(struct intel_displ
 void intel_display_snapshot_print(const struct intel_display_snapshot *snapshot,
 				  struct drm_printer *p)
 {
+	struct intel_display *display;
+
 	if (!snapshot)
 		return;
 
+	display = snapshot->display;
+
+	intel_display_device_info_print(&snapshot->info, &snapshot->runtime_info, p);
+	intel_display_params_dump(&snapshot->params, display->drm->driver->name, p);
+
 	intel_overlay_snapshot_print(snapshot->overlay, p);
 }
 
@@ -37,6 +60,8 @@ void intel_display_snapshot_free(struct intel_display_snapshot *snapshot)
 	if (!snapshot)
 		return;
 
+	intel_display_params_free(&snapshot->params);
+
 	kfree(snapshot->overlay);
 	kfree(snapshot);
 }
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index b047b24a90d5..15d57206b281 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -651,8 +651,6 @@ static void err_print_capabilities(struct drm_i915_error_state_buf *m,
 	struct drm_printer p = i915_error_printer(m);
 
 	intel_device_info_print(&error->device_info, &error->runtime_info, &p);
-	intel_display_device_info_print(&error->display_device_info,
-					&error->display_runtime_info, &p);
 	intel_driver_caps_print(&error->driver_caps, &p);
 }
 
@@ -660,10 +658,8 @@ static void err_print_params(struct drm_i915_error_state_buf *m,
 			     const struct i915_params *params)
 {
 	struct drm_printer p = i915_error_printer(m);
-	struct intel_display *display = &m->i915->display;
 
 	i915_params_dump(params, &p);
-	intel_display_params_dump(&display->params, display->drm->driver->name, &p);
 }
 
 static void err_print_pciid(struct drm_i915_error_state_buf *m,
@@ -1031,7 +1027,6 @@ static void i915_vma_coredump_free(struct i915_vma_coredump *vma)
 static void cleanup_params(struct i915_gpu_coredump *error)
 {
 	i915_params_free(&error->params);
-	intel_display_params_free(&error->display_params);
 }
 
 static void cleanup_uc(struct intel_uc_coredump *uc)
@@ -1992,17 +1987,12 @@ static void capture_gen(struct i915_gpu_coredump *error)
 	error->suspend_count = i915->suspend_count;
 
 	i915_params_copy(&error->params, &i915->params);
-	intel_display_params_copy(&error->display_params);
 	memcpy(&error->device_info,
 	       INTEL_INFO(i915),
 	       sizeof(error->device_info));
 	memcpy(&error->runtime_info,
 	       RUNTIME_INFO(i915),
 	       sizeof(error->runtime_info));
-	memcpy(&error->display_device_info, DISPLAY_INFO(i915),
-	       sizeof(error->display_device_info));
-	memcpy(&error->display_runtime_info, DISPLAY_RUNTIME_INFO(i915),
-	       sizeof(error->display_runtime_info));
 	error->driver_caps = i915->caps;
 }
 
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.h b/drivers/gpu/drm/i915/i915_gpu_error.h
index 1a11942d7800..78a8928562a9 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.h
+++ b/drivers/gpu/drm/i915/i915_gpu_error.h
@@ -14,8 +14,6 @@
 
 #include <drm/drm_mm.h>
 
-#include "display/intel_display_device.h"
-#include "display/intel_display_params.h"
 #include "gt/intel_engine.h"
 #include "gt/intel_engine_types.h"
 #include "gt/intel_gt_types.h"
@@ -212,11 +210,8 @@ struct i915_gpu_coredump {
 
 	struct intel_device_info device_info;
 	struct intel_runtime_info runtime_info;
-	struct intel_display_device_info display_device_info;
-	struct intel_display_runtime_info display_runtime_info;
 	struct intel_driver_caps driver_caps;
 	struct i915_params params;
-	struct intel_display_params display_params;
 
 	struct scatterlist *sgl, *fit;
 
-- 
2.39.2

[PATCH v2 4/4] drm/i915/display: move dmc snapshotting to new display snapshot

[Jani Nikula]

Convert dmc error state printing to new snapshot capture/print division.

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 .../drm/i915/display/intel_display_snapshot.c |  5 +++
 drivers/gpu/drm/i915/display/intel_dmc.c      | 39 +++++++++++++++----
 drivers/gpu/drm/i915/display/intel_dmc.h      |  7 +++-
 drivers/gpu/drm/i915/i915_gpu_error.c         |  3 --
 4 files changed, 41 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_snapshot.c b/drivers/gpu/drm/i915/display/intel_display_snapshot.c
index a61ff0f81397..030c4f873da1 100644
--- a/drivers/gpu/drm/i915/display/intel_display_snapshot.c
+++ b/drivers/gpu/drm/i915/display/intel_display_snapshot.c
@@ -7,6 +7,7 @@
 #include "intel_display_device.h"
 #include "intel_display_params.h"
 #include "intel_display_snapshot.h"
+#include "intel_dmc.h"
 #include "intel_overlay.h"
 
 struct intel_display_snapshot {
@@ -16,6 +17,7 @@ struct intel_display_snapshot {
 	struct intel_display_runtime_info runtime_info;
 	struct intel_display_params params;
 	struct intel_overlay_snapshot *overlay;
+	struct intel_dmc_snapshot *dmc;
 };
 
 struct intel_display_snapshot *intel_display_snapshot_capture(struct intel_display *display)
@@ -35,6 +37,7 @@ struct intel_display_snapshot *intel_display_snapshot_capture(struct intel_displ
 	intel_display_params_copy(&snapshot->params);
 
 	snapshot->overlay = intel_overlay_snapshot_capture(display);
+	snapshot->dmc = intel_dmc_snapshot_capture(display);
 
 	return snapshot;
 }
@@ -53,6 +56,7 @@ void intel_display_snapshot_print(const struct intel_display_snapshot *snapshot,
 	intel_display_params_dump(&snapshot->params, display->drm->driver->name, p);
 
 	intel_overlay_snapshot_print(snapshot->overlay, p);
+	intel_dmc_snapshot_print(snapshot->dmc, p);
 }
 
 void intel_display_snapshot_free(struct intel_display_snapshot *snapshot)
@@ -63,5 +67,6 @@ void intel_display_snapshot_free(struct intel_display_snapshot *snapshot)
 	intel_display_params_free(&snapshot->params);
 
 	kfree(snapshot->overlay);
+	kfree(snapshot->dmc);
 	kfree(snapshot);
 }
diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c b/drivers/gpu/drm/i915/display/intel_dmc.c
index 7c756d5ba2a2..d2f9684c8b0e 100644
--- a/drivers/gpu/drm/i915/display/intel_dmc.c
+++ b/drivers/gpu/drm/i915/display/intel_dmc.c
@@ -1184,21 +1184,44 @@ void intel_dmc_fini(struct drm_i915_private *i915)
 	}
 }
 
-void intel_dmc_print_error_state(struct drm_printer *p,
-				 struct drm_i915_private *i915)
+struct intel_dmc_snapshot {
+	bool initialized;
+	bool loaded;
+	u32 version;
+};
+
+struct intel_dmc_snapshot *intel_dmc_snapshot_capture(struct intel_display *display)
 {
+	struct drm_i915_private *i915 = to_i915(display->drm);
 	struct intel_dmc *dmc = i915_to_dmc(i915);
+	struct intel_dmc_snapshot *snapshot;
 
 	if (!HAS_DMC(i915))
-		return;
+		return NULL;
 
-	drm_printf(p, "DMC initialized: %s\n", str_yes_no(dmc));
-	drm_printf(p, "DMC loaded: %s\n",
-		   str_yes_no(intel_dmc_has_payload(i915)));
+	snapshot = kzalloc(sizeof(*snapshot), GFP_ATOMIC);
+	if (!snapshot)
+		return NULL;
+
+	snapshot->initialized = dmc;
+	snapshot->loaded = intel_dmc_has_payload(i915);
 	if (dmc)
+		snapshot->version = dmc->version;
+
+	return snapshot;
+}
+
+void intel_dmc_snapshot_print(const struct intel_dmc_snapshot *snapshot, struct drm_printer *p)
+{
+	if (!snapshot)
+		return;
+
+	drm_printf(p, "DMC initialized: %s\n", str_yes_no(snapshot->initialized));
+	drm_printf(p, "DMC loaded: %s\n", str_yes_no(snapshot->loaded));
+	if (snapshot->initialized)
 		drm_printf(p, "DMC fw version: %d.%d\n",
-			   DMC_VERSION_MAJOR(dmc->version),
-			   DMC_VERSION_MINOR(dmc->version));
+			   DMC_VERSION_MAJOR(snapshot->version),
+			   DMC_VERSION_MINOR(snapshot->version));
 }
 
 static int intel_dmc_debugfs_status_show(struct seq_file *m, void *unused)
diff --git a/drivers/gpu/drm/i915/display/intel_dmc.h b/drivers/gpu/drm/i915/display/intel_dmc.h
index 54cff6002e31..e2186c900505 100644
--- a/drivers/gpu/drm/i915/display/intel_dmc.h
+++ b/drivers/gpu/drm/i915/display/intel_dmc.h
@@ -11,6 +11,8 @@
 enum pipe;
 struct drm_i915_private;
 struct drm_printer;
+struct intel_display;
+struct intel_dmc_snapshot;
 
 void intel_dmc_init(struct drm_i915_private *i915);
 void intel_dmc_load_program(struct drm_i915_private *i915);
@@ -22,8 +24,9 @@ void intel_dmc_suspend(struct drm_i915_private *i915);
 void intel_dmc_resume(struct drm_i915_private *i915);
 bool intel_dmc_has_payload(struct drm_i915_private *i915);
 void intel_dmc_debugfs_register(struct drm_i915_private *i915);
-void intel_dmc_print_error_state(struct drm_printer *p,
-				 struct drm_i915_private *i915);
+
+struct intel_dmc_snapshot *intel_dmc_snapshot_capture(struct intel_display *display);
+void intel_dmc_snapshot_print(const struct intel_dmc_snapshot *snapshot, struct drm_printer *p);
 
 void assert_dmc_loaded(struct drm_i915_private *i915);
 
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index 15d57206b281..135ded17334e 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -41,7 +41,6 @@
 #include <drm/drm_print.h>
 
 #include "display/intel_display_snapshot.h"
-#include "display/intel_dmc.h"
 
 #include "gem/i915_gem_context.h"
 #include "gem/i915_gem_lmem.h"
@@ -871,8 +870,6 @@ static void __err_print_to_sgl(struct drm_i915_error_state_buf *m,
 
 	err_printf(m, "IOMMU enabled?: %d\n", error->iommu);
 
-	intel_dmc_print_error_state(&p, m->i915);
-
 	err_printf(m, "RPM wakelock: %s\n", str_yes_no(error->wakelock));
 	err_printf(m, "PM suspended: %s\n", str_yes_no(error->suspended));
 
-- 
2.39.2

✗ Fi.CI.CHECKPATCH: warning for drm/i915/display: add snapshot capture/print infra (rev3)

[Patchwork]

== Series Details ==

Series: drm/i915/display: add snapshot capture/print infra (rev3)
URL   : https://patchwork.freedesktop.org/series/138151/
State : warning

== Summary ==

Error: dim checkpatch failed
648fcb624b42 drm/i915: dump display parameters captured in error state, not current
-:35: WARNING:MACRO_ARG_UNUSED: Argument 'T' is not used in function-like macro
#35: FILE: drivers/gpu/drm/i915/display/intel_display_params.c:185:
+#define PRINT(T, x, ...) _param_print(p, driver_name, #x, params->x);

-:35: CHECK:MACRO_ARG_PRECEDENCE: Macro argument 'x' may be better as '(x)' to avoid precedence issues
#35: FILE: drivers/gpu/drm/i915/display/intel_display_params.c:185:
+#define PRINT(T, x, ...) _param_print(p, driver_name, #x, params->x);

-:35: WARNING:TRAILING_SEMICOLON: macros should not use a trailing semicolon
#35: FILE: drivers/gpu/drm/i915/display/intel_display_params.c:185:
+#define PRINT(T, x, ...) _param_print(p, driver_name, #x, params->x);

total: 0 errors, 2 warnings, 1 checks, 52 lines checked
c4df7e175813 drm/i915/display: add intel_display_snapshot abstraction
-:32: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating?
#32: 
new file mode 100644

-:197: CHECK:LINE_SPACING: Please use a blank line after function/struct/union/enum declarations
#197: FILE: drivers/gpu/drm/i915/display/intel_overlay.h:65:
 }
+static inline void intel_overlay_snapshot_print(const struct intel_overlay_snapshot *error,

total: 0 errors, 1 warnings, 1 checks, 217 lines checked
b784a1421f7a drm/i915/display: move device info and params handling to snapshot
51f36d65ac61 drm/i915/display: move dmc snapshotting to new display snapshot

✗ Fi.CI.SPARSE: warning for drm/i915/display: add snapshot capture/print infra (rev3)

[Patchwork]

== Series Details ==

Series: drm/i915/display: add snapshot capture/print infra (rev3)
URL   : https://patchwork.freedesktop.org/series/138151/
State : warning

== Summary ==

Error: dim sparse failed
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.

✗ Fi.CI.BAT: failure for drm/i915/display: add snapshot capture/print infra (rev3)

[Patchwork]

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

== Series Details ==

Series: drm/i915/display: add snapshot capture/print infra (rev3)
URL   : https://patchwork.freedesktop.org/series/138151/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_15385 -> Patchwork_138151v3
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_138151v3 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_138151v3, please notify your bug team (I915-ci-infra@lists.freedesktop.org) to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_138151v3/index.html

Participating hosts (38 -> 41)
------------------------------

  Additional (5): bat-arlh-3 fi-bsw-n3050 fi-kbl-8809g bat-jsl-1 bat-mtlp-6 
  Missing    (2): bat-dg2-11 fi-snb-2520m 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_138151v3:

### IGT changes ###

#### Possible regressions ####

  * igt@gem_exec_create@basic@smem:
    - bat-arlh-2:         [PASS][1] -> [INCOMPLETE][2] +1 other test incomplete
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15385/bat-arlh-2/igt@gem_exec_create@basic@smem.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_138151v3/bat-arlh-2/igt@gem_exec_create@basic@smem.html

  
Known issues
------------

  Here are the changes found in Patchwork_138151v3 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@debugfs_test@basic-hwmon:
    - bat-jsl-1:          NOTRUN -> [SKIP][3] ([i915#9318])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_138151v3/bat-jsl-1/igt@debugfs_test@basic-hwmon.html
    - bat-mtlp-6:         NOTRUN -> [SKIP][4] ([i915#9318])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_138151v3/bat-mtlp-6/igt@debugfs_test@basic-hwmon.html

  * igt@fbdev@info:
    - bat-mtlp-6:         NOTRUN -> [SKIP][5] ([i915#1849] / [i915#2582])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_138151v3/bat-mtlp-6/igt@fbdev@info.html

  * igt@fbdev@write:
    - bat-mtlp-6:         NOTRUN -> [SKIP][6] ([i915#2582]) +3 other tests skip
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_138151v3/bat-mtlp-6/igt@fbdev@write.html

  * igt@gem_huc_copy@huc-copy:
    - fi-kbl-8809g:       NOTRUN -> [SKIP][7] ([i915#2190])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_138151v3/fi-kbl-8809g/igt@gem_huc_copy@huc-copy.html
    - bat-jsl-1:          NOTRUN -> [SKIP][8] ([i915#2190])
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_138151v3/bat-jsl-1/igt@gem_huc_copy@huc-copy.html

  * igt@gem_lmem_swapping@parallel-random-engines:
    - bat-jsl-1:          NOTRUN -> [SKIP][9] ([i915#4613]) +3 other tests skip
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_138151v3/bat-jsl-1/igt@gem_lmem_swapping@parallel-random-engines.html
    - fi-kbl-8809g:       NOTRUN -> [SKIP][10] ([i915#4613]) +3 other tests skip
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_138151v3/fi-kbl-8809g/igt@gem_lmem_swapping@parallel-random-engines.html

  * igt@gem_lmem_swapping@verify-random:
    - bat-mtlp-6:         NOTRUN -> [SKIP][11] ([i915#4613]) +3 other tests skip
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_138151v3/bat-mtlp-6/igt@gem_lmem_swapping@verify-random.html

  * igt@gem_mmap@basic:
    - bat-mtlp-6:         NOTRUN -> [SKIP][12] ([i915#4083])
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_138151v3/bat-mtlp-6/igt@gem_mmap@basic.html

  * igt@gem_tiled_blits@basic:
    - bat-mtlp-6:         NOTRUN -> [SKIP][13] ([i915#4077]) +2 other tests skip
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_138151v3/bat-mtlp-6/igt@gem_tiled_blits@basic.html

  * igt@gem_tiled_pread_basic:
    - bat-mtlp-6:         NOTRUN -> [SKIP][14] ([i915#4079]) +1 other test skip
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_138151v3/bat-mtlp-6/igt@gem_tiled_pread_basic.html

  * igt@i915_pm_rps@basic-api:
    - bat-mtlp-6:         NOTRUN -> [SKIP][15] ([i915#11681] / [i915#6621])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_138151v3/bat-mtlp-6/igt@i915_pm_rps@basic-api.html

  * igt@i915_selftest@live:
    - bat-arls-2:         [PASS][16] -> [DMESG-WARN][17] ([i915#10341])
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15385/bat-arls-2/igt@i915_selftest@live.html
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_138151v3/bat-arls-2/igt@i915_selftest@live.html

  * igt@i915_selftest@live@hangcheck:
    - bat-arls-1:         NOTRUN -> [DMESG-WARN][18] ([i915#11349])
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_138151v3/bat-arls-1/igt@i915_selftest@live@hangcheck.html
    - bat-arls-2:         [PASS][19] -> [DMESG-WARN][20] ([i915#11349])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15385/bat-arls-2/igt@i915_selftest@live@hangcheck.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_138151v3/bat-arls-2/igt@i915_selftest@live@hangcheck.html

  * igt@kms_addfb_basic@addfb25-x-tiled-legacy:
    - bat-mtlp-6:         NOTRUN -> [SKIP][21] ([i915#4212] / [i915#9792]) +8 other tests skip
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_138151v3/bat-mtlp-6/igt@kms_addfb_basic@addfb25-x-tiled-legacy.html

  * igt@kms_addfb_basic@addfb25-y-tiled-small-legacy:
    - bat-mtlp-6:         NOTRUN -> [SKIP][22] ([i915#5190] / [i915#9792])
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_138151v3/bat-mtlp-6/igt@kms_addfb_basic@addfb25-y-tiled-small-legacy.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy:
    - bat-jsl-1:          NOTRUN -> [SKIP][23] ([i915#4103]) +1 other test skip
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_138151v3/bat-jsl-1/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy.html

  * igt@kms_cursor_legacy@basic-flip-after-cursor-legacy:
    - bat-mtlp-6:         NOTRUN -> [SKIP][24] ([i915#9792]) +17 other tests skip
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_138151v3/bat-mtlp-6/igt@kms_cursor_legacy@basic-flip-after-cursor-legacy.html

  * igt@kms_dsc@dsc-basic:
    - fi-kbl-8809g:       NOTRUN -> [SKIP][25] +30 other tests skip
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_138151v3/fi-kbl-8809g/igt@kms_dsc@dsc-basic.html
    - bat-jsl-1:          NOTRUN -> [SKIP][26] ([i915#3555] / [i915#9886])
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_138151v3/bat-jsl-1/igt@kms_dsc@dsc-basic.html

  * igt@kms_flip@basic-flip-vs-dpms:
    - bat-mtlp-6:         NOTRUN -> [SKIP][27] ([i915#3637] / [i915#9792]) +3 other tests skip
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_138151v3/bat-mtlp-6/igt@kms_flip@basic-flip-vs-dpms.html

  * igt@kms_force_connector_basic@force-load-detect:
    - bat-jsl-1:          NOTRUN -> [SKIP][28]
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_138151v3/bat-jsl-1/igt@kms_force_connector_basic@force-load-detect.html

  * igt@kms_force_connector_basic@prune-stale-modes:
    - bat-mtlp-6:         NOTRUN -> [SKIP][29] ([i915#5274] / [i915#9792])
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_138151v3/bat-mtlp-6/igt@kms_force_connector_basic@prune-stale-modes.html

  * igt@kms_frontbuffer_tracking@basic:
    - bat-mtlp-6:         NOTRUN -> [SKIP][30] ([i915#4342] / [i915#5354] / [i915#9792])
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_138151v3/bat-mtlp-6/igt@kms_frontbuffer_tracking@basic.html

  * igt@kms_pm_backlight@basic-brightness:
    - bat-mtlp-6:         NOTRUN -> [SKIP][31] ([i915#5354] / [i915#9792])
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_138151v3/bat-mtlp-6/igt@kms_pm_backlight@basic-brightness.html

  * igt@kms_psr@psr-cursor-plane-move:
    - bat-mtlp-6:         NOTRUN -> [SKIP][32] ([i915#1072] / [i915#9673] / [i915#9732] / [i915#9792]) +3 other tests skip
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_138151v3/bat-mtlp-6/igt@kms_psr@psr-cursor-plane-move.html

  * igt@kms_psr@psr-primary-mmap-gtt:
    - fi-bsw-n3050:       NOTRUN -> [SKIP][33] +20 other tests skip
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_138151v3/fi-bsw-n3050/igt@kms_psr@psr-primary-mmap-gtt.html

  * igt@kms_setmode@basic-clone-single-crtc:
    - bat-mtlp-6:         NOTRUN -> [SKIP][34] ([i915#3555] / [i915#8809] / [i915#9792])
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_138151v3/bat-mtlp-6/igt@kms_setmode@basic-clone-single-crtc.html
    - bat-jsl-1:          NOTRUN -> [SKIP][35] ([i915#3555])
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_138151v3/bat-jsl-1/igt@kms_setmode@basic-clone-single-crtc.html

  * igt@prime_vgem@basic-fence-flip:
    - bat-mtlp-6:         NOTRUN -> [SKIP][36] ([i915#3708] / [i915#9792])
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_138151v3/bat-mtlp-6/igt@prime_vgem@basic-fence-flip.html

  * igt@prime_vgem@basic-fence-mmap:
    - bat-mtlp-6:         NOTRUN -> [SKIP][37] ([i915#3708] / [i915#4077]) +1 other test skip
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_138151v3/bat-mtlp-6/igt@prime_vgem@basic-fence-mmap.html

  * igt@prime_vgem@basic-read:
    - bat-mtlp-6:         NOTRUN -> [SKIP][38] ([i915#3708]) +1 other test skip
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_138151v3/bat-mtlp-6/igt@prime_vgem@basic-read.html

  * igt@prime_vgem@basic-write:
    - bat-mtlp-6:         NOTRUN -> [SKIP][39] ([i915#10216] / [i915#3708])
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_138151v3/bat-mtlp-6/igt@prime_vgem@basic-write.html

  
#### Possible fixes ####

  * igt@i915_selftest@live@workarounds:
    - bat-arls-1:         [ABORT][40] ([i915#12061]) -> [PASS][41]
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15385/bat-arls-1/igt@i915_selftest@live@workarounds.html
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_138151v3/bat-arls-1/igt@i915_selftest@live@workarounds.html

  * igt@kms_pipe_crc_basic@nonblocking-crc:
    - bat-arls-5:         [INCOMPLETE][42] ([i915#11320]) -> [PASS][43]
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15385/bat-arls-5/igt@kms_pipe_crc_basic@nonblocking-crc.html
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_138151v3/bat-arls-5/igt@kms_pipe_crc_basic@nonblocking-crc.html

  
#### Warnings ####

  * igt@fbdev@read:
    - bat-arls-1:         [DMESG-WARN][44] ([i915#12102]) -> [DMESG-FAIL][45] ([i915#12102])
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15385/bat-arls-1/igt@fbdev@read.html
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_138151v3/bat-arls-1/igt@fbdev@read.html

  * igt@i915_selftest@live:
    - bat-arls-1:         [ABORT][46] ([i915#12175]) -> [DMESG-WARN][47] ([i915#10341])
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15385/bat-arls-1/igt@i915_selftest@live.html
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_138151v3/bat-arls-1/igt@i915_selftest@live.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [i915#10196]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/10196
  [i915#10216]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/10216
  [i915#10341]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/10341
  [i915#1072]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/1072
  [i915#11320]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/11320
  [i915#11343]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/11343
  [i915#11346]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/11346
  [i915#11349]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/11349
  [i915#11666]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/11666
  [i915#11671]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/11671
  [i915#11681]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/11681
  [i915#11723]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/11723
  [i915#11724]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/11724
  [i915#11725]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/11725
  [i915#11726]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/11726
  [i915#11981]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/11981
  [i915#12061]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12061
  [i915#12102]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12102
  [i915#12175]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12175
  [i915#1849]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/1849
  [i915#2190]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/2190
  [i915#2582]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/2582
  [i915#3555]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/3555
  [i915#3637]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/3637
  [i915#3708]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/3708
  [i915#4077]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/4077
  [i915#4079]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/4079
  [i915#4083]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/4083
  [i915#4103]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/4103
  [i915#4212]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/4212
  [i915#4342]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/4342
  [i915#4613]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/4613
  [i915#5190]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/5190
  [i915#5274]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/5274
  [i915#5354]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/5354
  [i915#6621]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/6621
  [i915#8809]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/8809
  [i915#9318]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/9318
  [i915#9673]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/9673
  [i915#9732]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/9732
  [i915#9792]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/9792
  [i915#9886]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/9886


Build changes
-------------

  * Linux: CI_DRM_15385 -> Patchwork_138151v3

  CI-20190529: 20190529
  CI_DRM_15385: bd6c4b2266ebc908d55e8ec3f419b832ebdc3945 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_8011: 26dca87f1252b7f6f0c0f833050256f0244d61e9 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_138151v3: bd6c4b2266ebc908d55e8ec3f419b832ebdc3945 @ git://anongit.freedesktop.org/gfx-ci/linux

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_138151v3/index.html

[-- Attachment #2: Type: text/html, Size: 17601 bytes --]

Re: [PATCH v2 2/4] drm/i915/display: add intel_display_snapshot abstraction

[Rodrigo Vivi]

On Mon, Sep 09, 2024 at 04:32:57PM +0300, Jani Nikula wrote:
> The error state capture still handles display info at a too detailed
> level. Start abstracting the whole display snapshot capture and printing
> at a higher level. Move overlay to display snapshot first.
> 
> Use the same nomenclature and style as in xe devcoredump, in preparation
> for perhaps some day bolting the snapshots there as well.
> 
> v3: Fix build harder for CONFIG_DRM_I915_CAPTURE_ERROR=n
> 
> v2: Fix build for CONFIG_DRM_I915_CAPTURE_ERROR=n (kernel test robot)
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/Makefile                 |  1 +
>  .../drm/i915/display/intel_display_snapshot.c | 42 +++++++++++++++++++
>  .../drm/i915/display/intel_display_snapshot.h | 16 +++++++
>  drivers/gpu/drm/i915/display/intel_overlay.c  | 16 ++++---
>  drivers/gpu/drm/i915/display/intel_overlay.h  | 25 +++++++----
>  drivers/gpu/drm/i915/i915_gpu_error.c         | 12 +++---
>  drivers/gpu/drm/i915/i915_gpu_error.h         |  6 +--
>  7 files changed, 94 insertions(+), 24 deletions(-)
>  create mode 100644 drivers/gpu/drm/i915/display/intel_display_snapshot.c
>  create mode 100644 drivers/gpu/drm/i915/display/intel_display_snapshot.h
> 
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index c63fa2133ccb..9fcd9e09bc0b 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -242,6 +242,7 @@ i915-y += \
>  	display/intel_display_power_well.o \
>  	display/intel_display_reset.o \
>  	display/intel_display_rps.o \
> +	display/intel_display_snapshot.o \
>  	display/intel_display_wa.o \
>  	display/intel_dmc.o \
>  	display/intel_dmc_wl.o \
> diff --git a/drivers/gpu/drm/i915/display/intel_display_snapshot.c b/drivers/gpu/drm/i915/display/intel_display_snapshot.c
> new file mode 100644
> index 000000000000..78b019dcd41d
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/display/intel_display_snapshot.c
> @@ -0,0 +1,42 @@
> +// SPDX-License-Identifier: MIT
> +/* Copyright © 2024 Intel Corporation */
> +
> +#include <linux/slab.h>
> +
> +#include "intel_display_snapshot.h"
> +#include "intel_overlay.h"
> +
> +struct intel_display_snapshot {
> +	struct intel_overlay_snapshot *overlay;
> +};
> +
> +struct intel_display_snapshot *intel_display_snapshot_capture(struct intel_display *display)
> +{
> +	struct intel_display_snapshot *snapshot;
> +
> +	snapshot = kzalloc(sizeof(*snapshot), GFP_ATOMIC);
> +	if (!snapshot)
> +		return NULL;
> +
> +	snapshot->overlay = intel_overlay_snapshot_capture(display);
> +
> +	return snapshot;
> +}
> +
> +void intel_display_snapshot_print(const struct intel_display_snapshot *snapshot,
> +				  struct drm_printer *p)
> +{
> +	if (!snapshot)
> +		return;
> +
> +	intel_overlay_snapshot_print(snapshot->overlay, p);
> +}
> +
> +void intel_display_snapshot_free(struct intel_display_snapshot *snapshot)
> +{
> +	if (!snapshot)
> +		return;
> +
> +	kfree(snapshot->overlay);
> +	kfree(snapshot);
> +}
> diff --git a/drivers/gpu/drm/i915/display/intel_display_snapshot.h b/drivers/gpu/drm/i915/display/intel_display_snapshot.h
> new file mode 100644
> index 000000000000..7ed27cdea644
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/display/intel_display_snapshot.h
> @@ -0,0 +1,16 @@
> +/* SPDX-License-Identifier: MIT */
> +/* Copyright © 2024 Intel Corporation */
> +
> +#ifndef __INTEL_DISPLAY_SNAPSHOT_H__
> +#define __INTEL_DISPLAY_SNAPSHOT_H__
> +
> +struct drm_printer;
> +struct intel_display;
> +struct intel_display_snapshot;
> +
> +struct intel_display_snapshot *intel_display_snapshot_capture(struct intel_display *display);
> +void intel_display_snapshot_print(const struct intel_display_snapshot *snapshot,
> +				  struct drm_printer *p);
> +void intel_display_snapshot_free(struct intel_display_snapshot *snapshot);
> +
> +#endif /* __INTEL_DISPLAY_SNAPSHOT_H__ */
> diff --git a/drivers/gpu/drm/i915/display/intel_overlay.c b/drivers/gpu/drm/i915/display/intel_overlay.c
> index 06b1122ec13e..b89541458765 100644
> --- a/drivers/gpu/drm/i915/display/intel_overlay.c
> +++ b/drivers/gpu/drm/i915/display/intel_overlay.c
> @@ -1457,18 +1457,19 @@ void intel_overlay_cleanup(struct drm_i915_private *dev_priv)
>  
>  #if IS_ENABLED(CONFIG_DRM_I915_CAPTURE_ERROR)
>  
> -struct intel_overlay_error_state {
> +struct intel_overlay_snapshot {
>  	struct overlay_registers regs;
>  	unsigned long base;
>  	u32 dovsta;
>  	u32 isr;
>  };
>  
> -struct intel_overlay_error_state *
> -intel_overlay_capture_error_state(struct drm_i915_private *dev_priv)
> +struct intel_overlay_snapshot *
> +intel_overlay_snapshot_capture(struct intel_display *display)
>  {
> +	struct drm_i915_private *dev_priv = to_i915(display->drm);
>  	struct intel_overlay *overlay = dev_priv->display.overlay;
> -	struct intel_overlay_error_state *error;
> +	struct intel_overlay_snapshot *error;
>  
>  	if (!overlay || !overlay->active)
>  		return NULL;
> @@ -1487,9 +1488,12 @@ intel_overlay_capture_error_state(struct drm_i915_private *dev_priv)
>  }
>  
>  void
> -intel_overlay_print_error_state(struct drm_printer *p,
> -				struct intel_overlay_error_state *error)
> +intel_overlay_snapshot_print(const struct intel_overlay_snapshot *error,
> +			     struct drm_printer *p)
>  {
> +	if (!error)
> +		return;
> +
>  	drm_printf(p, "Overlay, status: 0x%08x, interrupt: 0x%08x\n",
>  		   error->dovsta, error->isr);
>  	drm_printf(p, "  Register file at 0x%08lx:\n", error->base);
> diff --git a/drivers/gpu/drm/i915/display/intel_overlay.h b/drivers/gpu/drm/i915/display/intel_overlay.h
> index f28a09c062d0..eafac24d1de8 100644
> --- a/drivers/gpu/drm/i915/display/intel_overlay.h
> +++ b/drivers/gpu/drm/i915/display/intel_overlay.h
> @@ -6,12 +6,15 @@
>  #ifndef __INTEL_OVERLAY_H__
>  #define __INTEL_OVERLAY_H__
>  
> +#include <linux/types.h>

so, that was it?
I cannot spot any other difference between the v3 and v2.
But I also cannot correlate this to the reported errors.

> +
>  struct drm_device;
>  struct drm_file;
>  struct drm_i915_private;
>  struct drm_printer;
> +struct intel_display;
>  struct intel_overlay;
> -struct intel_overlay_error_state;
> +struct intel_overlay_snapshot;
>  
>  #ifdef I915
>  void intel_overlay_setup(struct drm_i915_private *dev_priv);
> @@ -22,10 +25,6 @@ int intel_overlay_put_image_ioctl(struct drm_device *dev, void *data,
>  int intel_overlay_attrs_ioctl(struct drm_device *dev, void *data,
>  			      struct drm_file *file_priv);
>  void intel_overlay_reset(struct drm_i915_private *dev_priv);
> -struct intel_overlay_error_state *
> -intel_overlay_capture_error_state(struct drm_i915_private *dev_priv);
> -void intel_overlay_print_error_state(struct drm_printer *p,
> -				     struct intel_overlay_error_state *error);
>  #else
>  static inline void intel_overlay_setup(struct drm_i915_private *dev_priv)
>  {
> @@ -50,13 +49,21 @@ static inline int intel_overlay_attrs_ioctl(struct drm_device *dev, void *data,
>  static inline void intel_overlay_reset(struct drm_i915_private *dev_priv)
>  {
>  }
> -static inline struct intel_overlay_error_state *
> -intel_overlay_capture_error_state(struct drm_i915_private *dev_priv)
> +#endif
> +
> +#if IS_ENABLED(CONFIG_DRM_I915_CAPTURE_ERROR) && defined(I915)
> +struct intel_overlay_snapshot *
> +intel_overlay_snapshot_capture(struct intel_display *display);
> +void intel_overlay_snapshot_print(const struct intel_overlay_snapshot *error,
> +				  struct drm_printer *p);
> +#else
> +static inline struct intel_overlay_snapshot *
> +intel_overlay_snapshot_capture(struct intel_display *display)
>  {
>  	return NULL;
>  }
> -static inline void intel_overlay_print_error_state(struct drm_printer *p,
> -						   struct intel_overlay_error_state *error)
> +static inline void intel_overlay_snapshot_print(const struct intel_overlay_snapshot *error,
> +						struct drm_printer *p)
>  {
>  }
>  #endif
> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> index f23769ccf050..b047b24a90d5 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> @@ -40,8 +40,8 @@
>  #include <drm/drm_cache.h>
>  #include <drm/drm_print.h>
>  
> +#include "display/intel_display_snapshot.h"
>  #include "display/intel_dmc.h"
> -#include "display/intel_overlay.h"
>  
>  #include "gem/i915_gem_context.h"
>  #include "gem/i915_gem_lmem.h"
> @@ -905,11 +905,10 @@ static void __err_print_to_sgl(struct drm_i915_error_state_buf *m,
>  		err_print_gt_info(m, error->gt);
>  	}
>  
> -	if (error->overlay)
> -		intel_overlay_print_error_state(&p, error->overlay);
> -
>  	err_print_capabilities(m, error);
>  	err_print_params(m, &error->params);
> +
> +	intel_display_snapshot_print(error->display_snapshot, &p);
>  }
>  
>  static int err_print_to_sgl(struct i915_gpu_coredump *error)
> @@ -1077,7 +1076,7 @@ void __i915_gpu_coredump_free(struct kref *error_ref)
>  		cleanup_gt(gt);
>  	}
>  
> -	kfree(error->overlay);
> +	intel_display_snapshot_free(error->display_snapshot);
>  
>  	cleanup_params(error);
>  
> @@ -2097,6 +2096,7 @@ static struct i915_gpu_coredump *
>  __i915_gpu_coredump(struct intel_gt *gt, intel_engine_mask_t engine_mask, u32 dump_flags)
>  {
>  	struct drm_i915_private *i915 = gt->i915;
> +	struct intel_display *display = &i915->display;
>  	struct i915_gpu_coredump *error;
>  
>  	/* Check if GPU capture has been disabled */
> @@ -2138,7 +2138,7 @@ __i915_gpu_coredump(struct intel_gt *gt, intel_engine_mask_t engine_mask, u32 du
>  		error->simulated |= error->gt->simulated;
>  	}
>  
> -	error->overlay = intel_overlay_capture_error_state(i915);
> +	error->display_snapshot = intel_display_snapshot_capture(display);
>  
>  	return error;
>  }
> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.h b/drivers/gpu/drm/i915/i915_gpu_error.h
> index 7c255bb1c319..1a11942d7800 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.h
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.h
> @@ -31,7 +31,7 @@
>  struct drm_i915_private;
>  struct i915_vma_compress;
>  struct intel_engine_capture_vma;
> -struct intel_overlay_error_state;
> +struct intel_display_snapshot;
>  
>  struct i915_vma_coredump {
>  	struct i915_vma_coredump *next;
> @@ -218,9 +218,9 @@ struct i915_gpu_coredump {
>  	struct i915_params params;
>  	struct intel_display_params display_params;
>  
> -	struct intel_overlay_error_state *overlay;
> -
>  	struct scatterlist *sgl, *fit;
> +
> +	struct intel_display_snapshot *display_snapshot;
>  };
>  
>  struct i915_gpu_error {
> -- 
> 2.39.2
> 

Re: [PATCH v2 2/4] drm/i915/display: add intel_display_snapshot abstraction

[Jani Nikula]

On Mon, 09 Sep 2024, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> On Mon, Sep 09, 2024 at 04:32:57PM +0300, Jani Nikula wrote:
>> The error state capture still handles display info at a too detailed
>> level. Start abstracting the whole display snapshot capture and printing
>> at a higher level. Move overlay to display snapshot first.
>> 
>> Use the same nomenclature and style as in xe devcoredump, in preparation
>> for perhaps some day bolting the snapshots there as well.
>> 
>> v3: Fix build harder for CONFIG_DRM_I915_CAPTURE_ERROR=n
>> 
>> v2: Fix build for CONFIG_DRM_I915_CAPTURE_ERROR=n (kernel test robot)
>> 
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>  drivers/gpu/drm/i915/Makefile                 |  1 +
>>  .../drm/i915/display/intel_display_snapshot.c | 42 +++++++++++++++++++
>>  .../drm/i915/display/intel_display_snapshot.h | 16 +++++++
>>  drivers/gpu/drm/i915/display/intel_overlay.c  | 16 ++++---
>>  drivers/gpu/drm/i915/display/intel_overlay.h  | 25 +++++++----
>>  drivers/gpu/drm/i915/i915_gpu_error.c         | 12 +++---
>>  drivers/gpu/drm/i915/i915_gpu_error.h         |  6 +--
>>  7 files changed, 94 insertions(+), 24 deletions(-)
>>  create mode 100644 drivers/gpu/drm/i915/display/intel_display_snapshot.c
>>  create mode 100644 drivers/gpu/drm/i915/display/intel_display_snapshot.h
>> 
>> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
>> index c63fa2133ccb..9fcd9e09bc0b 100644
>> --- a/drivers/gpu/drm/i915/Makefile
>> +++ b/drivers/gpu/drm/i915/Makefile
>> @@ -242,6 +242,7 @@ i915-y += \
>>  	display/intel_display_power_well.o \
>>  	display/intel_display_reset.o \
>>  	display/intel_display_rps.o \
>> +	display/intel_display_snapshot.o \
>>  	display/intel_display_wa.o \
>>  	display/intel_dmc.o \
>>  	display/intel_dmc_wl.o \
>> diff --git a/drivers/gpu/drm/i915/display/intel_display_snapshot.c b/drivers/gpu/drm/i915/display/intel_display_snapshot.c
>> new file mode 100644
>> index 000000000000..78b019dcd41d
>> --- /dev/null
>> +++ b/drivers/gpu/drm/i915/display/intel_display_snapshot.c
>> @@ -0,0 +1,42 @@
>> +// SPDX-License-Identifier: MIT
>> +/* Copyright © 2024 Intel Corporation */
>> +
>> +#include <linux/slab.h>
>> +
>> +#include "intel_display_snapshot.h"
>> +#include "intel_overlay.h"
>> +
>> +struct intel_display_snapshot {
>> +	struct intel_overlay_snapshot *overlay;
>> +};
>> +
>> +struct intel_display_snapshot *intel_display_snapshot_capture(struct intel_display *display)
>> +{
>> +	struct intel_display_snapshot *snapshot;
>> +
>> +	snapshot = kzalloc(sizeof(*snapshot), GFP_ATOMIC);
>> +	if (!snapshot)
>> +		return NULL;
>> +
>> +	snapshot->overlay = intel_overlay_snapshot_capture(display);
>> +
>> +	return snapshot;
>> +}
>> +
>> +void intel_display_snapshot_print(const struct intel_display_snapshot *snapshot,
>> +				  struct drm_printer *p)
>> +{
>> +	if (!snapshot)
>> +		return;
>> +
>> +	intel_overlay_snapshot_print(snapshot->overlay, p);
>> +}
>> +
>> +void intel_display_snapshot_free(struct intel_display_snapshot *snapshot)
>> +{
>> +	if (!snapshot)
>> +		return;
>> +
>> +	kfree(snapshot->overlay);
>> +	kfree(snapshot);
>> +}
>> diff --git a/drivers/gpu/drm/i915/display/intel_display_snapshot.h b/drivers/gpu/drm/i915/display/intel_display_snapshot.h
>> new file mode 100644
>> index 000000000000..7ed27cdea644
>> --- /dev/null
>> +++ b/drivers/gpu/drm/i915/display/intel_display_snapshot.h
>> @@ -0,0 +1,16 @@
>> +/* SPDX-License-Identifier: MIT */
>> +/* Copyright © 2024 Intel Corporation */
>> +
>> +#ifndef __INTEL_DISPLAY_SNAPSHOT_H__
>> +#define __INTEL_DISPLAY_SNAPSHOT_H__
>> +
>> +struct drm_printer;
>> +struct intel_display;
>> +struct intel_display_snapshot;
>> +
>> +struct intel_display_snapshot *intel_display_snapshot_capture(struct intel_display *display);
>> +void intel_display_snapshot_print(const struct intel_display_snapshot *snapshot,
>> +				  struct drm_printer *p);
>> +void intel_display_snapshot_free(struct intel_display_snapshot *snapshot);
>> +
>> +#endif /* __INTEL_DISPLAY_SNAPSHOT_H__ */
>> diff --git a/drivers/gpu/drm/i915/display/intel_overlay.c b/drivers/gpu/drm/i915/display/intel_overlay.c
>> index 06b1122ec13e..b89541458765 100644
>> --- a/drivers/gpu/drm/i915/display/intel_overlay.c
>> +++ b/drivers/gpu/drm/i915/display/intel_overlay.c
>> @@ -1457,18 +1457,19 @@ void intel_overlay_cleanup(struct drm_i915_private *dev_priv)
>>  
>>  #if IS_ENABLED(CONFIG_DRM_I915_CAPTURE_ERROR)
>>  
>> -struct intel_overlay_error_state {
>> +struct intel_overlay_snapshot {
>>  	struct overlay_registers regs;
>>  	unsigned long base;
>>  	u32 dovsta;
>>  	u32 isr;
>>  };
>>  
>> -struct intel_overlay_error_state *
>> -intel_overlay_capture_error_state(struct drm_i915_private *dev_priv)
>> +struct intel_overlay_snapshot *
>> +intel_overlay_snapshot_capture(struct intel_display *display)
>>  {
>> +	struct drm_i915_private *dev_priv = to_i915(display->drm);
>>  	struct intel_overlay *overlay = dev_priv->display.overlay;
>> -	struct intel_overlay_error_state *error;
>> +	struct intel_overlay_snapshot *error;
>>  
>>  	if (!overlay || !overlay->active)
>>  		return NULL;
>> @@ -1487,9 +1488,12 @@ intel_overlay_capture_error_state(struct drm_i915_private *dev_priv)
>>  }
>>  
>>  void
>> -intel_overlay_print_error_state(struct drm_printer *p,
>> -				struct intel_overlay_error_state *error)
>> +intel_overlay_snapshot_print(const struct intel_overlay_snapshot *error,
>> +			     struct drm_printer *p)
>>  {
>> +	if (!error)
>> +		return;
>> +
>>  	drm_printf(p, "Overlay, status: 0x%08x, interrupt: 0x%08x\n",
>>  		   error->dovsta, error->isr);
>>  	drm_printf(p, "  Register file at 0x%08lx:\n", error->base);
>> diff --git a/drivers/gpu/drm/i915/display/intel_overlay.h b/drivers/gpu/drm/i915/display/intel_overlay.h
>> index f28a09c062d0..eafac24d1de8 100644
>> --- a/drivers/gpu/drm/i915/display/intel_overlay.h
>> +++ b/drivers/gpu/drm/i915/display/intel_overlay.h
>> @@ -6,12 +6,15 @@
>>  #ifndef __INTEL_OVERLAY_H__
>>  #define __INTEL_OVERLAY_H__
>>  
>> +#include <linux/types.h>
>
> so, that was it?
> I cannot spot any other difference between the v3 and v2.
> But I also cannot correlate this to the reported errors.

I'm not sure if the test robot actually tested v2, it just sent the same
results for gcc and clang. But I found this myself when trying locally
with CONFIG_DRM_I915_CAPTURE_ERROR=n. It's needed for returning NULL in
the stub...

BR,
Jani.

>
>> +
>>  struct drm_device;
>>  struct drm_file;
>>  struct drm_i915_private;
>>  struct drm_printer;
>> +struct intel_display;
>>  struct intel_overlay;
>> -struct intel_overlay_error_state;
>> +struct intel_overlay_snapshot;
>>  
>>  #ifdef I915
>>  void intel_overlay_setup(struct drm_i915_private *dev_priv);
>> @@ -22,10 +25,6 @@ int intel_overlay_put_image_ioctl(struct drm_device *dev, void *data,
>>  int intel_overlay_attrs_ioctl(struct drm_device *dev, void *data,
>>  			      struct drm_file *file_priv);
>>  void intel_overlay_reset(struct drm_i915_private *dev_priv);
>> -struct intel_overlay_error_state *
>> -intel_overlay_capture_error_state(struct drm_i915_private *dev_priv);
>> -void intel_overlay_print_error_state(struct drm_printer *p,
>> -				     struct intel_overlay_error_state *error);
>>  #else
>>  static inline void intel_overlay_setup(struct drm_i915_private *dev_priv)
>>  {
>> @@ -50,13 +49,21 @@ static inline int intel_overlay_attrs_ioctl(struct drm_device *dev, void *data,
>>  static inline void intel_overlay_reset(struct drm_i915_private *dev_priv)
>>  {
>>  }
>> -static inline struct intel_overlay_error_state *
>> -intel_overlay_capture_error_state(struct drm_i915_private *dev_priv)
>> +#endif
>> +
>> +#if IS_ENABLED(CONFIG_DRM_I915_CAPTURE_ERROR) && defined(I915)
>> +struct intel_overlay_snapshot *
>> +intel_overlay_snapshot_capture(struct intel_display *display);
>> +void intel_overlay_snapshot_print(const struct intel_overlay_snapshot *error,
>> +				  struct drm_printer *p);
>> +#else
>> +static inline struct intel_overlay_snapshot *
>> +intel_overlay_snapshot_capture(struct intel_display *display)
>>  {
>>  	return NULL;
>>  }
>> -static inline void intel_overlay_print_error_state(struct drm_printer *p,
>> -						   struct intel_overlay_error_state *error)
>> +static inline void intel_overlay_snapshot_print(const struct intel_overlay_snapshot *error,
>> +						struct drm_printer *p)
>>  {
>>  }
>>  #endif
>> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
>> index f23769ccf050..b047b24a90d5 100644
>> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
>> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
>> @@ -40,8 +40,8 @@
>>  #include <drm/drm_cache.h>
>>  #include <drm/drm_print.h>
>>  
>> +#include "display/intel_display_snapshot.h"
>>  #include "display/intel_dmc.h"
>> -#include "display/intel_overlay.h"
>>  
>>  #include "gem/i915_gem_context.h"
>>  #include "gem/i915_gem_lmem.h"
>> @@ -905,11 +905,10 @@ static void __err_print_to_sgl(struct drm_i915_error_state_buf *m,
>>  		err_print_gt_info(m, error->gt);
>>  	}
>>  
>> -	if (error->overlay)
>> -		intel_overlay_print_error_state(&p, error->overlay);
>> -
>>  	err_print_capabilities(m, error);
>>  	err_print_params(m, &error->params);
>> +
>> +	intel_display_snapshot_print(error->display_snapshot, &p);
>>  }
>>  
>>  static int err_print_to_sgl(struct i915_gpu_coredump *error)
>> @@ -1077,7 +1076,7 @@ void __i915_gpu_coredump_free(struct kref *error_ref)
>>  		cleanup_gt(gt);
>>  	}
>>  
>> -	kfree(error->overlay);
>> +	intel_display_snapshot_free(error->display_snapshot);
>>  
>>  	cleanup_params(error);
>>  
>> @@ -2097,6 +2096,7 @@ static struct i915_gpu_coredump *
>>  __i915_gpu_coredump(struct intel_gt *gt, intel_engine_mask_t engine_mask, u32 dump_flags)
>>  {
>>  	struct drm_i915_private *i915 = gt->i915;
>> +	struct intel_display *display = &i915->display;
>>  	struct i915_gpu_coredump *error;
>>  
>>  	/* Check if GPU capture has been disabled */
>> @@ -2138,7 +2138,7 @@ __i915_gpu_coredump(struct intel_gt *gt, intel_engine_mask_t engine_mask, u32 du
>>  		error->simulated |= error->gt->simulated;
>>  	}
>>  
>> -	error->overlay = intel_overlay_capture_error_state(i915);
>> +	error->display_snapshot = intel_display_snapshot_capture(display);
>>  
>>  	return error;
>>  }
>> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.h b/drivers/gpu/drm/i915/i915_gpu_error.h
>> index 7c255bb1c319..1a11942d7800 100644
>> --- a/drivers/gpu/drm/i915/i915_gpu_error.h
>> +++ b/drivers/gpu/drm/i915/i915_gpu_error.h
>> @@ -31,7 +31,7 @@
>>  struct drm_i915_private;
>>  struct i915_vma_compress;
>>  struct intel_engine_capture_vma;
>> -struct intel_overlay_error_state;
>> +struct intel_display_snapshot;
>>  
>>  struct i915_vma_coredump {
>>  	struct i915_vma_coredump *next;
>> @@ -218,9 +218,9 @@ struct i915_gpu_coredump {
>>  	struct i915_params params;
>>  	struct intel_display_params display_params;
>>  
>> -	struct intel_overlay_error_state *overlay;
>> -
>>  	struct scatterlist *sgl, *fit;
>> +
>> +	struct intel_display_snapshot *display_snapshot;
>>  };
>>  
>>  struct i915_gpu_error {
>> -- 
>> 2.39.2
>> 

-- 
Jani Nikula, Intel

Re: [PATCH v2 2/4] drm/i915/display: add intel_display_snapshot abstraction

[Rodrigo Vivi]

On Mon, Sep 09, 2024 at 11:53:02PM +0300, Jani Nikula wrote:
> On Mon, 09 Sep 2024, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> > On Mon, Sep 09, 2024 at 04:32:57PM +0300, Jani Nikula wrote:
> >> The error state capture still handles display info at a too detailed
> >> level. Start abstracting the whole display snapshot capture and printing
> >> at a higher level. Move overlay to display snapshot first.
> >> 
> >> Use the same nomenclature and style as in xe devcoredump, in preparation
> >> for perhaps some day bolting the snapshots there as well.
> >> 
> >> v3: Fix build harder for CONFIG_DRM_I915_CAPTURE_ERROR=n
> >> 
> >> v2: Fix build for CONFIG_DRM_I915_CAPTURE_ERROR=n (kernel test robot)
> >> 
> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/Makefile                 |  1 +
> >>  .../drm/i915/display/intel_display_snapshot.c | 42 +++++++++++++++++++
> >>  .../drm/i915/display/intel_display_snapshot.h | 16 +++++++
> >>  drivers/gpu/drm/i915/display/intel_overlay.c  | 16 ++++---
> >>  drivers/gpu/drm/i915/display/intel_overlay.h  | 25 +++++++----
> >>  drivers/gpu/drm/i915/i915_gpu_error.c         | 12 +++---
> >>  drivers/gpu/drm/i915/i915_gpu_error.h         |  6 +--
> >>  7 files changed, 94 insertions(+), 24 deletions(-)
> >>  create mode 100644 drivers/gpu/drm/i915/display/intel_display_snapshot.c
> >>  create mode 100644 drivers/gpu/drm/i915/display/intel_display_snapshot.h
> >> 
> >> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> >> index c63fa2133ccb..9fcd9e09bc0b 100644
> >> --- a/drivers/gpu/drm/i915/Makefile
> >> +++ b/drivers/gpu/drm/i915/Makefile
> >> @@ -242,6 +242,7 @@ i915-y += \
> >>  	display/intel_display_power_well.o \
> >>  	display/intel_display_reset.o \
> >>  	display/intel_display_rps.o \
> >> +	display/intel_display_snapshot.o \
> >>  	display/intel_display_wa.o \
> >>  	display/intel_dmc.o \
> >>  	display/intel_dmc_wl.o \
> >> diff --git a/drivers/gpu/drm/i915/display/intel_display_snapshot.c b/drivers/gpu/drm/i915/display/intel_display_snapshot.c
> >> new file mode 100644
> >> index 000000000000..78b019dcd41d
> >> --- /dev/null
> >> +++ b/drivers/gpu/drm/i915/display/intel_display_snapshot.c
> >> @@ -0,0 +1,42 @@
> >> +// SPDX-License-Identifier: MIT
> >> +/* Copyright © 2024 Intel Corporation */
> >> +
> >> +#include <linux/slab.h>
> >> +
> >> +#include "intel_display_snapshot.h"
> >> +#include "intel_overlay.h"
> >> +
> >> +struct intel_display_snapshot {
> >> +	struct intel_overlay_snapshot *overlay;
> >> +};
> >> +
> >> +struct intel_display_snapshot *intel_display_snapshot_capture(struct intel_display *display)
> >> +{
> >> +	struct intel_display_snapshot *snapshot;
> >> +
> >> +	snapshot = kzalloc(sizeof(*snapshot), GFP_ATOMIC);
> >> +	if (!snapshot)
> >> +		return NULL;
> >> +
> >> +	snapshot->overlay = intel_overlay_snapshot_capture(display);
> >> +
> >> +	return snapshot;
> >> +}
> >> +
> >> +void intel_display_snapshot_print(const struct intel_display_snapshot *snapshot,
> >> +				  struct drm_printer *p)
> >> +{
> >> +	if (!snapshot)
> >> +		return;
> >> +
> >> +	intel_overlay_snapshot_print(snapshot->overlay, p);
> >> +}
> >> +
> >> +void intel_display_snapshot_free(struct intel_display_snapshot *snapshot)
> >> +{
> >> +	if (!snapshot)
> >> +		return;
> >> +
> >> +	kfree(snapshot->overlay);
> >> +	kfree(snapshot);
> >> +}
> >> diff --git a/drivers/gpu/drm/i915/display/intel_display_snapshot.h b/drivers/gpu/drm/i915/display/intel_display_snapshot.h
> >> new file mode 100644
> >> index 000000000000..7ed27cdea644
> >> --- /dev/null
> >> +++ b/drivers/gpu/drm/i915/display/intel_display_snapshot.h
> >> @@ -0,0 +1,16 @@
> >> +/* SPDX-License-Identifier: MIT */
> >> +/* Copyright © 2024 Intel Corporation */
> >> +
> >> +#ifndef __INTEL_DISPLAY_SNAPSHOT_H__
> >> +#define __INTEL_DISPLAY_SNAPSHOT_H__
> >> +
> >> +struct drm_printer;
> >> +struct intel_display;
> >> +struct intel_display_snapshot;
> >> +
> >> +struct intel_display_snapshot *intel_display_snapshot_capture(struct intel_display *display);
> >> +void intel_display_snapshot_print(const struct intel_display_snapshot *snapshot,
> >> +				  struct drm_printer *p);
> >> +void intel_display_snapshot_free(struct intel_display_snapshot *snapshot);
> >> +
> >> +#endif /* __INTEL_DISPLAY_SNAPSHOT_H__ */
> >> diff --git a/drivers/gpu/drm/i915/display/intel_overlay.c b/drivers/gpu/drm/i915/display/intel_overlay.c
> >> index 06b1122ec13e..b89541458765 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_overlay.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_overlay.c
> >> @@ -1457,18 +1457,19 @@ void intel_overlay_cleanup(struct drm_i915_private *dev_priv)
> >>  
> >>  #if IS_ENABLED(CONFIG_DRM_I915_CAPTURE_ERROR)
> >>  
> >> -struct intel_overlay_error_state {
> >> +struct intel_overlay_snapshot {
> >>  	struct overlay_registers regs;
> >>  	unsigned long base;
> >>  	u32 dovsta;
> >>  	u32 isr;
> >>  };
> >>  
> >> -struct intel_overlay_error_state *
> >> -intel_overlay_capture_error_state(struct drm_i915_private *dev_priv)
> >> +struct intel_overlay_snapshot *
> >> +intel_overlay_snapshot_capture(struct intel_display *display)
> >>  {
> >> +	struct drm_i915_private *dev_priv = to_i915(display->drm);
> >>  	struct intel_overlay *overlay = dev_priv->display.overlay;
> >> -	struct intel_overlay_error_state *error;
> >> +	struct intel_overlay_snapshot *error;
> >>  
> >>  	if (!overlay || !overlay->active)
> >>  		return NULL;
> >> @@ -1487,9 +1488,12 @@ intel_overlay_capture_error_state(struct drm_i915_private *dev_priv)
> >>  }
> >>  
> >>  void
> >> -intel_overlay_print_error_state(struct drm_printer *p,
> >> -				struct intel_overlay_error_state *error)
> >> +intel_overlay_snapshot_print(const struct intel_overlay_snapshot *error,
> >> +			     struct drm_printer *p)
> >>  {
> >> +	if (!error)
> >> +		return;
> >> +
> >>  	drm_printf(p, "Overlay, status: 0x%08x, interrupt: 0x%08x\n",
> >>  		   error->dovsta, error->isr);
> >>  	drm_printf(p, "  Register file at 0x%08lx:\n", error->base);
> >> diff --git a/drivers/gpu/drm/i915/display/intel_overlay.h b/drivers/gpu/drm/i915/display/intel_overlay.h
> >> index f28a09c062d0..eafac24d1de8 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_overlay.h
> >> +++ b/drivers/gpu/drm/i915/display/intel_overlay.h
> >> @@ -6,12 +6,15 @@
> >>  #ifndef __INTEL_OVERLAY_H__
> >>  #define __INTEL_OVERLAY_H__
> >>  
> >> +#include <linux/types.h>
> >
> > so, that was it?
> > I cannot spot any other difference between the v3 and v2.
> > But I also cannot correlate this to the reported errors.
> 
> I'm not sure if the test robot actually tested v2, it just sent the same
> results for gcc and clang. But I found this myself when trying locally
> with CONFIG_DRM_I915_CAPTURE_ERROR=n. It's needed for returning NULL in
> the stub...

fair enough. the code looks right to me and if build-bots are okay
now:

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> 
> BR,
> Jani.
> 
> >
> >> +
> >>  struct drm_device;
> >>  struct drm_file;
> >>  struct drm_i915_private;
> >>  struct drm_printer;
> >> +struct intel_display;
> >>  struct intel_overlay;
> >> -struct intel_overlay_error_state;
> >> +struct intel_overlay_snapshot;
> >>  
> >>  #ifdef I915
> >>  void intel_overlay_setup(struct drm_i915_private *dev_priv);
> >> @@ -22,10 +25,6 @@ int intel_overlay_put_image_ioctl(struct drm_device *dev, void *data,
> >>  int intel_overlay_attrs_ioctl(struct drm_device *dev, void *data,
> >>  			      struct drm_file *file_priv);
> >>  void intel_overlay_reset(struct drm_i915_private *dev_priv);
> >> -struct intel_overlay_error_state *
> >> -intel_overlay_capture_error_state(struct drm_i915_private *dev_priv);
> >> -void intel_overlay_print_error_state(struct drm_printer *p,
> >> -				     struct intel_overlay_error_state *error);
> >>  #else
> >>  static inline void intel_overlay_setup(struct drm_i915_private *dev_priv)
> >>  {
> >> @@ -50,13 +49,21 @@ static inline int intel_overlay_attrs_ioctl(struct drm_device *dev, void *data,
> >>  static inline void intel_overlay_reset(struct drm_i915_private *dev_priv)
> >>  {
> >>  }
> >> -static inline struct intel_overlay_error_state *
> >> -intel_overlay_capture_error_state(struct drm_i915_private *dev_priv)
> >> +#endif
> >> +
> >> +#if IS_ENABLED(CONFIG_DRM_I915_CAPTURE_ERROR) && defined(I915)
> >> +struct intel_overlay_snapshot *
> >> +intel_overlay_snapshot_capture(struct intel_display *display);
> >> +void intel_overlay_snapshot_print(const struct intel_overlay_snapshot *error,
> >> +				  struct drm_printer *p);
> >> +#else
> >> +static inline struct intel_overlay_snapshot *
> >> +intel_overlay_snapshot_capture(struct intel_display *display)
> >>  {
> >>  	return NULL;
> >>  }
> >> -static inline void intel_overlay_print_error_state(struct drm_printer *p,
> >> -						   struct intel_overlay_error_state *error)
> >> +static inline void intel_overlay_snapshot_print(const struct intel_overlay_snapshot *error,
> >> +						struct drm_printer *p)
> >>  {
> >>  }
> >>  #endif
> >> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> >> index f23769ccf050..b047b24a90d5 100644
> >> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> >> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> >> @@ -40,8 +40,8 @@
> >>  #include <drm/drm_cache.h>
> >>  #include <drm/drm_print.h>
> >>  
> >> +#include "display/intel_display_snapshot.h"
> >>  #include "display/intel_dmc.h"
> >> -#include "display/intel_overlay.h"
> >>  
> >>  #include "gem/i915_gem_context.h"
> >>  #include "gem/i915_gem_lmem.h"
> >> @@ -905,11 +905,10 @@ static void __err_print_to_sgl(struct drm_i915_error_state_buf *m,
> >>  		err_print_gt_info(m, error->gt);
> >>  	}
> >>  
> >> -	if (error->overlay)
> >> -		intel_overlay_print_error_state(&p, error->overlay);
> >> -
> >>  	err_print_capabilities(m, error);
> >>  	err_print_params(m, &error->params);
> >> +
> >> +	intel_display_snapshot_print(error->display_snapshot, &p);
> >>  }
> >>  
> >>  static int err_print_to_sgl(struct i915_gpu_coredump *error)
> >> @@ -1077,7 +1076,7 @@ void __i915_gpu_coredump_free(struct kref *error_ref)
> >>  		cleanup_gt(gt);
> >>  	}
> >>  
> >> -	kfree(error->overlay);
> >> +	intel_display_snapshot_free(error->display_snapshot);
> >>  
> >>  	cleanup_params(error);
> >>  
> >> @@ -2097,6 +2096,7 @@ static struct i915_gpu_coredump *
> >>  __i915_gpu_coredump(struct intel_gt *gt, intel_engine_mask_t engine_mask, u32 dump_flags)
> >>  {
> >>  	struct drm_i915_private *i915 = gt->i915;
> >> +	struct intel_display *display = &i915->display;
> >>  	struct i915_gpu_coredump *error;
> >>  
> >>  	/* Check if GPU capture has been disabled */
> >> @@ -2138,7 +2138,7 @@ __i915_gpu_coredump(struct intel_gt *gt, intel_engine_mask_t engine_mask, u32 du
> >>  		error->simulated |= error->gt->simulated;
> >>  	}
> >>  
> >> -	error->overlay = intel_overlay_capture_error_state(i915);
> >> +	error->display_snapshot = intel_display_snapshot_capture(display);
> >>  
> >>  	return error;
> >>  }
> >> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.h b/drivers/gpu/drm/i915/i915_gpu_error.h
> >> index 7c255bb1c319..1a11942d7800 100644
> >> --- a/drivers/gpu/drm/i915/i915_gpu_error.h
> >> +++ b/drivers/gpu/drm/i915/i915_gpu_error.h
> >> @@ -31,7 +31,7 @@
> >>  struct drm_i915_private;
> >>  struct i915_vma_compress;
> >>  struct intel_engine_capture_vma;
> >> -struct intel_overlay_error_state;
> >> +struct intel_display_snapshot;
> >>  
> >>  struct i915_vma_coredump {
> >>  	struct i915_vma_coredump *next;
> >> @@ -218,9 +218,9 @@ struct i915_gpu_coredump {
> >>  	struct i915_params params;
> >>  	struct intel_display_params display_params;
> >>  
> >> -	struct intel_overlay_error_state *overlay;
> >> -
> >>  	struct scatterlist *sgl, *fit;
> >> +
> >> +	struct intel_display_snapshot *display_snapshot;
> >>  };
> >>  
> >>  struct i915_gpu_error {
> >> -- 
> >> 2.39.2
> >> 
> 
> -- 
> Jani Nikula, Intel