diff mbox series

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

Message ID e6bef9acfb4b146c8763c5b4e319285f96872710.1725372032.git.jani.nikula@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915/display: add snapshot capture/print infra | expand

Commit Message

Jani Nikula Sept. 3, 2024, 2:03 p.m. UTC 
Convert dmc error state printing to new snapshot capture/print division.

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(-)

Comments

Rodrigo Vivi Sept. 5, 2024, 4:59 p.m. UTC | #1 
On Tue, Sep 03, 2024 at 05:03:00PM +0300, Jani Nikula wrote:
> Convert dmc error state printing to new snapshot capture/print division.
> 
> 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));

I was wondering if we really need to stash the version, but since the
print can happen later and perhaps when it is gone, let's go with the safe.

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

>  }
>  
>  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
>
diff mbox series

Patch

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));