public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH v6 1/3] drm/i915: Add Guc/HuC firmware details to error state
@ 2017-10-26 17:36 Michal Wajdeczko
  2017-10-26 17:36 ` [PATCH v6 2/3] drm/i915: Make GuC log part of the uC " Michal Wajdeczko
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Michal Wajdeczko @ 2017-10-26 17:36 UTC (permalink / raw)
  To: intel-gfx

Include GuC and HuC firmware details in captured error state
to provide additional debug information. To reuse existing
uc firmware pretty printer, introduce new drm-printer variant
that works with our i915_error_state_buf output. Also update
uc firmware pretty printer to accept const input.

v2: don't rely on current caps (Chris)
    dump correct fw info (Michal)
v3: simplify capture of custom paths (Chris)
v4: improve 'why' comment (Joonas)
    trim output if no fw path (Michal)
    group code around uc error state (Michal)
v5: use error in cleanup_uc (Michal)

Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h       |  5 +++
 drivers/gpu/drm/i915/i915_gpu_error.c | 65 +++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_uc_fw.c    |  6 +++-
 drivers/gpu/drm/i915/intel_uc_fw.h    |  2 +-
 4 files changed, 76 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 366ba74..f19f0fa 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -911,6 +911,11 @@ struct i915_gpu_state {
 	struct intel_device_info device_info;
 	struct i915_params params;
 
+	struct i915_error_uc {
+		struct intel_uc_fw guc_fw;
+		struct intel_uc_fw huc_fw;
+	} uc;
+
 	/* Generic register state */
 	u32 eir;
 	u32 pgtbl_er;
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index 653fb69..4500fc8 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -30,6 +30,8 @@
 #include <generated/utsrelease.h>
 #include <linux/stop_machine.h>
 #include <linux/zlib.h>
+#include <drm/drm_print.h>
+
 #include "i915_drv.h"
 
 static const char *engine_str(int engine)
@@ -175,6 +177,21 @@ static void i915_error_puts(struct drm_i915_error_state_buf *e,
 #define err_printf(e, ...) i915_error_printf(e, __VA_ARGS__)
 #define err_puts(e, s) i915_error_puts(e, s)
 
+static void __i915_printfn_error(struct drm_printer *p, struct va_format *vaf)
+{
+	i915_error_vprintf(p->arg, vaf->fmt, *vaf->va);
+}
+
+static inline struct drm_printer
+i915_error_printer(struct drm_i915_error_state_buf *e)
+{
+	struct drm_printer p = {
+		.printfn = __i915_printfn_error,
+		.arg = e,
+	};
+	return p;
+}
+
 #ifdef CONFIG_DRM_I915_COMPRESS_ERROR
 
 struct compress {
@@ -589,11 +606,26 @@ static void err_print_pciid(struct drm_i915_error_state_buf *m,
 		   pdev->subsystem_device);
 }
 
+static void err_print_uc(struct drm_i915_error_state_buf *m,
+			 const struct i915_error_uc *error_uc)
+{
+	struct drm_printer p = i915_error_printer(m);
+	const struct i915_gpu_state *error =
+		container_of(error_uc, typeof(*error), uc);
+
+	if (!error->device_info.has_guc)
+		return;
+
+	intel_uc_fw_dump(&error_uc->guc_fw, &p);
+	intel_uc_fw_dump(&error_uc->huc_fw, &p);
+}
+
 int i915_error_state_to_str(struct drm_i915_error_state_buf *m,
 			    const struct i915_gpu_state *error)
 {
 	struct drm_i915_private *dev_priv = m->i915;
 	struct drm_i915_error_object *obj;
+
 	int i, j;
 
 	if (!error) {
@@ -773,6 +805,7 @@ int i915_error_state_to_str(struct drm_i915_error_state_buf *m,
 
 	err_print_capabilities(m, &error->device_info);
 	err_print_params(m, &error->params);
+	err_print_uc(m, &error->uc);
 
 	if (m->bytes == 0 && m->err)
 		return m->err;
@@ -831,6 +864,14 @@ static __always_inline void free_param(const char *type, void *x)
 		kfree(*(void **)x);
 }
 
+static void cleanup_uc_state(struct i915_gpu_state *error)
+{
+	struct i915_error_uc *error_uc = &error->uc;
+
+	kfree(error_uc->guc_fw.path);
+	kfree(error_uc->huc_fw.path);
+}
+
 void __i915_gpu_state_free(struct kref *error_ref)
 {
 	struct i915_gpu_state *error =
@@ -870,6 +911,8 @@ void __i915_gpu_state_free(struct kref *error_ref)
 	I915_PARAMS_FOR_EACH(FREE);
 #undef FREE
 
+	cleanup_uc_state(error);
+
 	kfree(error);
 }
 
@@ -1559,6 +1602,26 @@ static void i915_capture_pinned_buffers(struct drm_i915_private *dev_priv,
 	error->pinned_bo = bo;
 }
 
+static void capture_uc_state(struct i915_gpu_state *error)
+{
+	struct drm_i915_private *i915 = error->i915;
+	struct i915_error_uc *error_uc = &error->uc;
+
+	/* Capturing uC state won't be useful if there is no GuC */
+	if (!error->device_info.has_guc)
+		return;
+
+	error_uc->guc_fw = i915->guc.fw;
+	error_uc->huc_fw = i915->huc.fw;
+
+	/* Non-default firmware paths will be specified by the modparam.
+	 * As modparams are generally accesible from the userspace make
+	 * explicit copies of the firmware paths.
+	 */
+	error_uc->guc_fw.path = kstrdup(i915->guc.fw.path, GFP_ATOMIC);
+	error_uc->huc_fw.path = kstrdup(i915->huc.fw.path, GFP_ATOMIC);
+}
+
 static void i915_gem_capture_guc_log_buffer(struct drm_i915_private *dev_priv,
 					    struct i915_gpu_state *error)
 {
@@ -1710,6 +1773,8 @@ static int capture(void *data)
 	I915_PARAMS_FOR_EACH(DUP);
 #undef DUP
 
+	capture_uc_state(error);
+
 	i915_capture_gen_state(error->i915, error);
 	i915_capture_reg_state(error->i915, error);
 	i915_gem_record_fences(error->i915, error);
diff --git a/drivers/gpu/drm/i915/intel_uc_fw.c b/drivers/gpu/drm/i915/intel_uc_fw.c
index 973888e..79a8797 100644
--- a/drivers/gpu/drm/i915/intel_uc_fw.c
+++ b/drivers/gpu/drm/i915/intel_uc_fw.c
@@ -299,10 +299,14 @@ void intel_uc_fw_fini(struct intel_uc_fw *uc_fw)
  *
  * Pretty printer for uC firmware.
  */
-void intel_uc_fw_dump(struct intel_uc_fw *uc_fw, struct drm_printer *p)
+void intel_uc_fw_dump(const struct intel_uc_fw *uc_fw, struct drm_printer *p)
 {
 	drm_printf(p, "%s firmware: %s\n",
 		   intel_uc_fw_type_repr(uc_fw->type), uc_fw->path);
+
+	if (!uc_fw->path)
+		return;
+
 	drm_printf(p, "\tstatus: fetch %s, load %s\n",
 		   intel_uc_fw_status_repr(uc_fw->fetch_status),
 		   intel_uc_fw_status_repr(uc_fw->load_status));
diff --git a/drivers/gpu/drm/i915/intel_uc_fw.h b/drivers/gpu/drm/i915/intel_uc_fw.h
index 1329036..5394d9d 100644
--- a/drivers/gpu/drm/i915/intel_uc_fw.h
+++ b/drivers/gpu/drm/i915/intel_uc_fw.h
@@ -116,6 +116,6 @@ int intel_uc_fw_upload(struct intel_uc_fw *uc_fw,
 		       int (*xfer)(struct intel_uc_fw *uc_fw,
 				   struct i915_vma *vma));
 void intel_uc_fw_fini(struct intel_uc_fw *uc_fw);
-void intel_uc_fw_dump(struct intel_uc_fw *uc_fw, struct drm_printer *p);
+void intel_uc_fw_dump(const struct intel_uc_fw *uc_fw, struct drm_printer *p);
 
 #endif
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* [PATCH v6 2/3] drm/i915: Make GuC log part of the uC error state
  2017-10-26 17:36 [PATCH v6 1/3] drm/i915: Add Guc/HuC firmware details to error state Michal Wajdeczko
@ 2017-10-26 17:36 ` Michal Wajdeczko
  2017-10-26 17:36 ` [PATCH v6 3/3] drm/i915: Handle error-state modparams in dedicated functions Michal Wajdeczko
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Michal Wajdeczko @ 2017-10-26 17:36 UTC (permalink / raw)
  To: intel-gfx

We keep details of GuC and HuC in separate error state struct.
Make GuC log part of it to group all related data together.
Since we are printing uC details at the end, with this change
GuC log will be moved there too.

v2: comment on new placement of the log (Chris)

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h       |  2 +-
 drivers/gpu/drm/i915/i915_gpu_error.c | 18 +++---------------
 2 files changed, 4 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index f19f0fa..45a74d5 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -914,6 +914,7 @@ struct i915_gpu_state {
 	struct i915_error_uc {
 		struct intel_uc_fw guc_fw;
 		struct intel_uc_fw huc_fw;
+		struct drm_i915_error_object *guc_log;
 	} uc;
 
 	/* Generic register state */
@@ -939,7 +940,6 @@ struct i915_gpu_state {
 	struct intel_overlay_error_state *overlay;
 	struct intel_display_error_state *display;
 	struct drm_i915_error_object *semaphore;
-	struct drm_i915_error_object *guc_log;
 
 	struct drm_i915_error_engine {
 		int engine_id;
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index 4500fc8..f18f65e 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -618,6 +618,7 @@ static void err_print_uc(struct drm_i915_error_state_buf *m,
 
 	intel_uc_fw_dump(&error_uc->guc_fw, &p);
 	intel_uc_fw_dump(&error_uc->huc_fw, &p);
+	print_error_obj(m, NULL, "GuC log buffer", error_uc->guc_log);
 }
 
 int i915_error_state_to_str(struct drm_i915_error_state_buf *m,
@@ -795,8 +796,6 @@ int i915_error_state_to_str(struct drm_i915_error_state_buf *m,
 
 	print_error_obj(m, NULL, "Semaphores", error->semaphore);
 
-	print_error_obj(m, NULL, "GuC log buffer", error->guc_log);
-
 	if (error->overlay)
 		intel_overlay_print_error_state(m, error->overlay);
 
@@ -870,6 +869,7 @@ static void cleanup_uc_state(struct i915_gpu_state *error)
 
 	kfree(error_uc->guc_fw.path);
 	kfree(error_uc->huc_fw.path);
+	i915_error_object_free(error_uc->guc_log);
 }
 
 void __i915_gpu_state_free(struct kref *error_ref)
@@ -898,7 +898,6 @@ void __i915_gpu_state_free(struct kref *error_ref)
 	}
 
 	i915_error_object_free(error->semaphore);
-	i915_error_object_free(error->guc_log);
 
 	for (i = 0; i < ARRAY_SIZE(error->active_bo); i++)
 		kfree(error->active_bo[i]);
@@ -1620,17 +1619,7 @@ static void capture_uc_state(struct i915_gpu_state *error)
 	 */
 	error_uc->guc_fw.path = kstrdup(i915->guc.fw.path, GFP_ATOMIC);
 	error_uc->huc_fw.path = kstrdup(i915->huc.fw.path, GFP_ATOMIC);
-}
-
-static void i915_gem_capture_guc_log_buffer(struct drm_i915_private *dev_priv,
-					    struct i915_gpu_state *error)
-{
-	/* Capturing log buf contents won't be useful if logging was disabled */
-	if (!dev_priv->guc.log.vma || (i915_modparams.guc_log_level < 0))
-		return;
-
-	error->guc_log = i915_error_object_create(dev_priv,
-						  dev_priv->guc.log.vma);
+	error_uc->guc_log = i915_error_object_create(i915, i915->guc.log.vma);
 }
 
 /* Capture all registers which don't fit into another category. */
@@ -1781,7 +1770,6 @@ static int capture(void *data)
 	i915_gem_record_rings(error->i915, error);
 	i915_capture_active_buffers(error->i915, error);
 	i915_capture_pinned_buffers(error->i915, error);
-	i915_gem_capture_guc_log_buffer(error->i915, error);
 
 	error->overlay = intel_overlay_capture_error_state(error->i915);
 	error->display = intel_display_capture_error_state(error->i915);
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* [PATCH v6 3/3] drm/i915: Handle error-state modparams in dedicated functions
  2017-10-26 17:36 [PATCH v6 1/3] drm/i915: Add Guc/HuC firmware details to error state Michal Wajdeczko
  2017-10-26 17:36 ` [PATCH v6 2/3] drm/i915: Make GuC log part of the uC " Michal Wajdeczko
@ 2017-10-26 17:36 ` Michal Wajdeczko
  2017-10-26 18:01 ` ✓ Fi.CI.BAT: success for series starting with [v6,1/3] drm/i915: Add Guc/HuC firmware details to error state Patchwork
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Michal Wajdeczko @ 2017-10-26 17:36 UTC (permalink / raw)
  To: intel-gfx

Capturing and cleanup and modparams in error state requires
some macro tricks. Move that code into separated functions
for easier maintenance.

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_gpu_error.c | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index f18f65e..ee6233b 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -863,6 +863,13 @@ static __always_inline void free_param(const char *type, void *x)
 		kfree(*(void **)x);
 }
 
+static void cleanup_params(struct i915_gpu_state *error)
+{
+#define FREE(T, x, ...) free_param(#T, &error->params.x);
+	I915_PARAMS_FOR_EACH(FREE);
+#undef FREE
+}
+
 static void cleanup_uc_state(struct i915_gpu_state *error)
 {
 	struct i915_error_uc *error_uc = &error->uc;
@@ -906,10 +913,7 @@ void __i915_gpu_state_free(struct kref *error_ref)
 	kfree(error->overlay);
 	kfree(error->display);
 
-#define FREE(T, x, ...) free_param(#T, &error->params.x);
-	I915_PARAMS_FOR_EACH(FREE);
-#undef FREE
-
+	cleanup_params(error);
 	cleanup_uc_state(error);
 
 	kfree(error);
@@ -1747,6 +1751,14 @@ static __always_inline void dup_param(const char *type, void *x)
 		*(void **)x = kstrdup(*(void **)x, GFP_ATOMIC);
 }
 
+static void capture_params(struct i915_gpu_state *error)
+{
+	error->params = i915_modparams;
+#define DUP(T, x, ...) dup_param(#T, &error->params.x);
+	I915_PARAMS_FOR_EACH(DUP);
+#undef DUP
+}
+
 static int capture(void *data)
 {
 	struct i915_gpu_state *error = data;
@@ -1757,11 +1769,7 @@ static int capture(void *data)
 		ktime_to_timeval(ktime_sub(ktime_get(),
 					   error->i915->gt.last_init_time));
 
-	error->params = i915_modparams;
-#define DUP(T, x, ...) dup_param(#T, &error->params.x);
-	I915_PARAMS_FOR_EACH(DUP);
-#undef DUP
-
+	capture_params(error);
 	capture_uc_state(error);
 
 	i915_capture_gen_state(error->i915, error);
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* ✓ Fi.CI.BAT: success for series starting with [v6,1/3] drm/i915: Add Guc/HuC firmware details to error state
  2017-10-26 17:36 [PATCH v6 1/3] drm/i915: Add Guc/HuC firmware details to error state Michal Wajdeczko
  2017-10-26 17:36 ` [PATCH v6 2/3] drm/i915: Make GuC log part of the uC " Michal Wajdeczko
  2017-10-26 17:36 ` [PATCH v6 3/3] drm/i915: Handle error-state modparams in dedicated functions Michal Wajdeczko
@ 2017-10-26 18:01 ` Patchwork
  2017-10-26 19:10 ` ✓ Fi.CI.IGT: " Patchwork
  2017-11-06 14:15 ` [PATCH v6 1/3] " Chris Wilson
  4 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2017-10-26 18:01 UTC (permalink / raw)
  To: Michal Wajdeczko; +Cc: intel-gfx

== Series Details ==

Series: series starting with [v6,1/3] drm/i915: Add Guc/HuC firmware details to error state
URL   : https://patchwork.freedesktop.org/series/32710/
State : success

== Summary ==

Series 32710v1 series starting with [v6,1/3] drm/i915: Add Guc/HuC firmware details to error state
https://patchwork.freedesktop.org/api/1.0/series/32710/revisions/1/mbox/

Test chamelium:
        Subgroup hdmi-hpd-fast:
                skip       -> FAIL       (fi-kbl-7500u) fdo#102672
Test kms_pipe_crc_basic:
        Subgroup read-crc-pipe-b-frame-sequence:
                skip       -> PASS       (fi-hsw-4770r) fdo#102332
        Subgroup suspend-read-crc-pipe-b:
                pass       -> DMESG-WARN (fi-byt-j1900) fdo#101705

fdo#102672 https://bugs.freedesktop.org/show_bug.cgi?id=102672
fdo#102332 https://bugs.freedesktop.org/show_bug.cgi?id=102332
fdo#101705 https://bugs.freedesktop.org/show_bug.cgi?id=101705

fi-bdw-5557u     total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:443s
fi-bdw-gvtdvm    total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:453s
fi-blb-e6850     total:289  pass:223  dwarn:1   dfail:0   fail:0   skip:65  time:372s
fi-bsw-n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  time:527s
fi-bwr-2160      total:289  pass:183  dwarn:0   dfail:0   fail:0   skip:106 time:265s
fi-bxt-dsi       total:289  pass:259  dwarn:0   dfail:0   fail:0   skip:30  time:502s
fi-bxt-j4205     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:510s
fi-byt-j1900     total:289  pass:253  dwarn:1   dfail:0   fail:0   skip:35  time:499s
fi-byt-n2820     total:289  pass:249  dwarn:1   dfail:0   fail:0   skip:39  time:484s
fi-cfl-s         total:289  pass:253  dwarn:4   dfail:0   fail:0   skip:32  time:561s
fi-cnl-y         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:605s
fi-elk-e7500     total:289  pass:229  dwarn:0   dfail:0   fail:0   skip:60  time:421s
fi-gdg-551       total:289  pass:178  dwarn:1   dfail:0   fail:1   skip:109 time:249s
fi-glk-1         total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:581s
fi-glk-dsi       total:289  pass:258  dwarn:0   dfail:0   fail:1   skip:30  time:496s
fi-hsw-4770      total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:430s
fi-hsw-4770r     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:427s
fi-ilk-650       total:289  pass:228  dwarn:0   dfail:0   fail:0   skip:61  time:431s
fi-ivb-3520m     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:497s
fi-ivb-3770      total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:460s
fi-kbl-7500u     total:289  pass:264  dwarn:1   dfail:0   fail:1   skip:23  time:489s
fi-kbl-7560u     total:289  pass:270  dwarn:0   dfail:0   fail:0   skip:19  time:575s
fi-kbl-7567u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:480s
fi-kbl-r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:589s
fi-pnv-d510      total:289  pass:222  dwarn:1   dfail:0   fail:0   skip:66  time:545s
fi-skl-6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:454s
fi-skl-6600u     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:593s
fi-skl-6700hq    total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:646s
fi-skl-6700k     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:519s
fi-skl-6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:500s
fi-skl-gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  time:459s
fi-snb-2520m     total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:566s
fi-snb-2600      total:289  pass:249  dwarn:0   dfail:0   fail:0   skip:40  time:420s

df3033b174059a59aa0c890f81de8af037abd11f drm-tip: 2017y-10m-26d-11h-03m-59s UTC integration manifest
976ff837e64e drm/i915: Handle error-state modparams in dedicated functions
4c3a25127b82 drm/i915: Make GuC log part of the uC error state
26138fd008d5 drm/i915: Add Guc/HuC firmware details to error state

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6211/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* ✓ Fi.CI.IGT: success for series starting with [v6,1/3] drm/i915: Add Guc/HuC firmware details to error state
  2017-10-26 17:36 [PATCH v6 1/3] drm/i915: Add Guc/HuC firmware details to error state Michal Wajdeczko
                   ` (2 preceding siblings ...)
  2017-10-26 18:01 ` ✓ Fi.CI.BAT: success for series starting with [v6,1/3] drm/i915: Add Guc/HuC firmware details to error state Patchwork
@ 2017-10-26 19:10 ` Patchwork
  2017-11-06 14:15 ` [PATCH v6 1/3] " Chris Wilson
  4 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2017-10-26 19:10 UTC (permalink / raw)
  To: Michal Wajdeczko; +Cc: intel-gfx

== Series Details ==

Series: series starting with [v6,1/3] drm/i915: Add Guc/HuC firmware details to error state
URL   : https://patchwork.freedesktop.org/series/32710/
State : success

== Summary ==

Test kms_busy:
        Subgroup extended-modeset-hang-newfb-with-reset-render-B:
                pass       -> DMESG-WARN (shard-hsw) fdo#103038
        Subgroup extended-modeset-hang-oldfb-with-reset-render-B:
                dmesg-warn -> PASS       (shard-hsw) fdo#102249
Test kms_plane:
        Subgroup plane-panning-bottom-right-suspend-pipe-A-planes:
                skip       -> PASS       (shard-hsw)

fdo#103038 https://bugs.freedesktop.org/show_bug.cgi?id=103038
fdo#102249 https://bugs.freedesktop.org/show_bug.cgi?id=102249

shard-hsw        total:2539 pass:1433 dwarn:1   dfail:0   fail:8   skip:1097 time:9216s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6211/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH v6 1/3] drm/i915: Add Guc/HuC firmware details to error state
  2017-10-26 17:36 [PATCH v6 1/3] drm/i915: Add Guc/HuC firmware details to error state Michal Wajdeczko
                   ` (3 preceding siblings ...)
  2017-10-26 19:10 ` ✓ Fi.CI.IGT: " Patchwork
@ 2017-11-06 14:15 ` Chris Wilson
  2017-11-08 11:27   ` Joonas Lahtinen
  4 siblings, 1 reply; 9+ messages in thread
From: Chris Wilson @ 2017-11-06 14:15 UTC (permalink / raw)
  To: Michal Wajdeczko, intel-gfx

Quoting Michal Wajdeczko (2017-10-26 18:36:55)
> Include GuC and HuC firmware details in captured error state
> to provide additional debug information. To reuse existing
> uc firmware pretty printer, introduce new drm-printer variant
> that works with our i915_error_state_buf output. Also update
> uc firmware pretty printer to accept const input.
> 
> v2: don't rely on current caps (Chris)
>     dump correct fw info (Michal)
> v3: simplify capture of custom paths (Chris)
> v4: improve 'why' comment (Joonas)
>     trim output if no fw path (Michal)
>     group code around uc error state (Michal)
> v5: use error in cleanup_uc (Michal)
> 
> Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h       |  5 +++
>  drivers/gpu/drm/i915/i915_gpu_error.c | 65 +++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_uc_fw.c    |  6 +++-
>  drivers/gpu/drm/i915/intel_uc_fw.h    |  2 +-
>  4 files changed, 76 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 366ba74..f19f0fa 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -911,6 +911,11 @@ struct i915_gpu_state {
>         struct intel_device_info device_info;
>         struct i915_params params;
>  
> +       struct i915_error_uc {
> +               struct intel_uc_fw guc_fw;
> +               struct intel_uc_fw huc_fw;
> +       } uc;
> +
>         /* Generic register state */
>         u32 eir;
>         u32 pgtbl_er;
> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> index 653fb69..4500fc8 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> @@ -30,6 +30,8 @@
>  #include <generated/utsrelease.h>
>  #include <linux/stop_machine.h>
>  #include <linux/zlib.h>
> +#include <drm/drm_print.h>
> +
>  #include "i915_drv.h"
>  
>  static const char *engine_str(int engine)
> @@ -175,6 +177,21 @@ static void i915_error_puts(struct drm_i915_error_state_buf *e,
>  #define err_printf(e, ...) i915_error_printf(e, __VA_ARGS__)
>  #define err_puts(e, s) i915_error_puts(e, s)
>  
> +static void __i915_printfn_error(struct drm_printer *p, struct va_format *vaf)
> +{
> +       i915_error_vprintf(p->arg, vaf->fmt, *vaf->va);
> +}
> +
> +static inline struct drm_printer
> +i915_error_printer(struct drm_i915_error_state_buf *e)
> +{
> +       struct drm_printer p = {
> +               .printfn = __i915_printfn_error,
> +               .arg = e,
> +       };
> +       return p;
> +}
> +
>  #ifdef CONFIG_DRM_I915_COMPRESS_ERROR
>  
>  struct compress {
> @@ -589,11 +606,26 @@ static void err_print_pciid(struct drm_i915_error_state_buf *m,
>                    pdev->subsystem_device);
>  }
>  
> +static void err_print_uc(struct drm_i915_error_state_buf *m,
> +                        const struct i915_error_uc *error_uc)
> +{
> +       struct drm_printer p = i915_error_printer(m);
> +       const struct i915_gpu_state *error =
> +               container_of(error_uc, typeof(*error), uc);
> +
> +       if (!error->device_info.has_guc)
> +               return;

I am still not keen on how derived state is mixed in with checking
whether or not a piece of fw was presented to HW before the hang, it is
still better than before.

> +
> +       intel_uc_fw_dump(&error_uc->guc_fw, &p);
> +       intel_uc_fw_dump(&error_uc->huc_fw, &p);
> +}
> +
>  int i915_error_state_to_str(struct drm_i915_error_state_buf *m,
>                             const struct i915_gpu_state *error)
>  {
>         struct drm_i915_private *dev_priv = m->i915;
>         struct drm_i915_error_object *obj;
> +
>         int i, j;
>  
>         if (!error) {
> @@ -773,6 +805,7 @@ int i915_error_state_to_str(struct drm_i915_error_state_buf *m,
>  
>         err_print_capabilities(m, &error->device_info);
>         err_print_params(m, &error->params);
> +       err_print_uc(m, &error->uc);
>  
>         if (m->bytes == 0 && m->err)
>                 return m->err;
> @@ -831,6 +864,14 @@ static __always_inline void free_param(const char *type, void *x)
>                 kfree(*(void **)x);
>  }
>  
> +static void cleanup_uc_state(struct i915_gpu_state *error)
> +{
> +       struct i915_error_uc *error_uc = &error->uc;
> +
> +       kfree(error_uc->guc_fw.path);
> +       kfree(error_uc->huc_fw.path);
> +}
> +
>  void __i915_gpu_state_free(struct kref *error_ref)
>  {
>         struct i915_gpu_state *error =
> @@ -870,6 +911,8 @@ void __i915_gpu_state_free(struct kref *error_ref)
>         I915_PARAMS_FOR_EACH(FREE);
>  #undef FREE
>  
> +       cleanup_uc_state(error);
> +
>         kfree(error);
>  }
>  
> @@ -1559,6 +1602,26 @@ static void i915_capture_pinned_buffers(struct drm_i915_private *dev_priv,
>         error->pinned_bo = bo;
>  }
>  
> +static void capture_uc_state(struct i915_gpu_state *error)
> +{
> +       struct drm_i915_private *i915 = error->i915;
> +       struct i915_error_uc *error_uc = &error->uc;
> +
> +       /* Capturing uC state won't be useful if there is no GuC */
> +       if (!error->device_info.has_guc)
> +               return;
> +
> +       error_uc->guc_fw = i915->guc.fw;
> +       error_uc->huc_fw = i915->huc.fw;
> +
> +       /* Non-default firmware paths will be specified by the modparam.
> +        * As modparams are generally accesible from the userspace make
> +        * explicit copies of the firmware paths.
> +        */
> +       error_uc->guc_fw.path = kstrdup(i915->guc.fw.path, GFP_ATOMIC);
> +       error_uc->huc_fw.path = kstrdup(i915->huc.fw.path, GFP_ATOMIC);
> +}
> +
>  static void i915_gem_capture_guc_log_buffer(struct drm_i915_private *dev_priv,
>                                             struct i915_gpu_state *error)
>  {
> @@ -1710,6 +1773,8 @@ static int capture(void *data)
>         I915_PARAMS_FOR_EACH(DUP);
>  #undef DUP
>  
> +       capture_uc_state(error);
> +
>         i915_capture_gen_state(error->i915, error);
>         i915_capture_reg_state(error->i915, error);
>         i915_gem_record_fences(error->i915, error);
> diff --git a/drivers/gpu/drm/i915/intel_uc_fw.c b/drivers/gpu/drm/i915/intel_uc_fw.c
> index 973888e..79a8797 100644
> --- a/drivers/gpu/drm/i915/intel_uc_fw.c
> +++ b/drivers/gpu/drm/i915/intel_uc_fw.c
> @@ -299,10 +299,14 @@ void intel_uc_fw_fini(struct intel_uc_fw *uc_fw)
>   *
>   * Pretty printer for uC firmware.
>   */
> -void intel_uc_fw_dump(struct intel_uc_fw *uc_fw, struct drm_printer *p)
> +void intel_uc_fw_dump(const struct intel_uc_fw *uc_fw, struct drm_printer *p)
>  {
>         drm_printf(p, "%s firmware: %s\n",
>                    intel_uc_fw_type_repr(uc_fw->type), uc_fw->path);
> +
> +       if (!uc_fw->path)
> +               return;

This could be NULL simply due to allocation failure. You still want the
status and version info.

As the path isn't dereferenced here, it is safe enough to drop this
chunk, as you currently don't even try and pretty-print the error state
unless it is enabled.

Removed the chunk and applied.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH v6 1/3] drm/i915: Add Guc/HuC firmware details to error state
  2017-11-06 14:15 ` [PATCH v6 1/3] " Chris Wilson
@ 2017-11-08 11:27   ` Joonas Lahtinen
  2017-11-08 12:20     ` Michal Wajdeczko
  0 siblings, 1 reply; 9+ messages in thread
From: Joonas Lahtinen @ 2017-11-08 11:27 UTC (permalink / raw)
  To: Chris Wilson, Michal Wajdeczko, intel-gfx, Sagar Arun Kamble

On Mon, 2017-11-06 at 14:15 +0000, Chris Wilson wrote:
> Quoting Michal Wajdeczko (2017-10-26 18:36:55)
> > Include GuC and HuC firmware details in captured error state
> > to provide additional debug information. To reuse existing
> > uc firmware pretty printer, introduce new drm-printer variant
> > that works with our i915_error_state_buf output. Also update
> > uc firmware pretty printer to accept const input.
> > 
> > v2: don't rely on current caps (Chris)
> >     dump correct fw info (Michal)
> > v3: simplify capture of custom paths (Chris)
> > v4: improve 'why' comment (Joonas)
> >     trim output if no fw path (Michal)
> >     group code around uc error state (Michal)
> > v5: use error in cleanup_uc (Michal)
> > 
> > Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

<SNIP>

> 
> Removed the chunk and applied.

So, this got merged. What's is the next series to look forward to? I
again see multiple GuC series in my inbox and mailing list.

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH v6 1/3] drm/i915: Add Guc/HuC firmware details to error state
  2017-11-08 11:27   ` Joonas Lahtinen
@ 2017-11-08 12:20     ` Michal Wajdeczko
  2017-11-08 20:21       ` Michal Wajdeczko
  0 siblings, 1 reply; 9+ messages in thread
From: Michal Wajdeczko @ 2017-11-08 12:20 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx, Sagar Arun Kamble, Joonas Lahtinen

On Wed, 08 Nov 2017 12:27:20 +0100, Joonas Lahtinen  
<joonas.lahtinen@linux.intel.com> wrote:

> On Mon, 2017-11-06 at 14:15 +0000, Chris Wilson wrote:
>> Quoting Michal Wajdeczko (2017-10-26 18:36:55)
>> > Include GuC and HuC firmware details in captured error state
>> > to provide additional debug information. To reuse existing
>> > uc firmware pretty printer, introduce new drm-printer variant
>> > that works with our i915_error_state_buf output. Also update
>> > uc firmware pretty printer to accept const input.
>> >
>> > v2: don't rely on current caps (Chris)
>> >     dump correct fw info (Michal)
>> > v3: simplify capture of custom paths (Chris)
>> > v4: improve 'why' comment (Joonas)
>> >     trim output if no fw path (Michal)
>> >     group code around uc error state (Michal)
>> > v5: use error in cleanup_uc (Michal)
>> >
>> > Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
>> > Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>
> <SNIP>
>
>>
>> Removed the chunk and applied.
>
> So, this got merged. What's is the next series to look forward to? I
> again see multiple GuC series in my inbox and mailing list.
>

I think we should wait now for new series from Sujaritha

Michal
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH v6 1/3] drm/i915: Add Guc/HuC firmware details to error state
  2017-11-08 12:20     ` Michal Wajdeczko
@ 2017-11-08 20:21       ` Michal Wajdeczko
  0 siblings, 0 replies; 9+ messages in thread
From: Michal Wajdeczko @ 2017-11-08 20:21 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx, Sagar Arun Kamble, Joonas Lahtinen,
	Michal Wajdeczko

On Wed, 08 Nov 2017 13:20:16 +0100, Michal Wajdeczko  
<michal.wajdeczko@intel.com> wrote:

> On Wed, 08 Nov 2017 12:27:20 +0100, Joonas Lahtinen  
> <joonas.lahtinen@linux.intel.com> wrote:
>
>> On Mon, 2017-11-06 at 14:15 +0000, Chris Wilson wrote:
>>> Quoting Michal Wajdeczko (2017-10-26 18:36:55)
>>> > Include GuC and HuC firmware details in captured error state
>>> > to provide additional debug information. To reuse existing
>>> > uc firmware pretty printer, introduce new drm-printer variant
>>> > that works with our i915_error_state_buf output. Also update
>>> > uc firmware pretty printer to accept const input.
>>> >
>>> > v2: don't rely on current caps (Chris)
>>> >     dump correct fw info (Michal)
>>> > v3: simplify capture of custom paths (Chris)
>>> > v4: improve 'why' comment (Joonas)
>>> >     trim output if no fw path (Michal)
>>> >     group code around uc error state (Michal)
>>> > v5: use error in cleanup_uc (Michal)
>>> >
>>> > Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
>>> > Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
>>> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
>>> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>>
>> <SNIP>
>>
>>>
>>> Removed the chunk and applied.
>>
>> So, this got merged. What's is the next series to look forward to? I
>> again see multiple GuC series in my inbox and mailing list.
>>
>
> I think we should wait now for new series from Sujaritha
>

I forgot about this series (already reviewed by Sagar) that
you can look while waiting for Sujarita update:

https://patchwork.freedesktop.org/series/33135/

Michal
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

end of thread, other threads:[~2017-11-08 20:21 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-10-26 17:36 [PATCH v6 1/3] drm/i915: Add Guc/HuC firmware details to error state Michal Wajdeczko
2017-10-26 17:36 ` [PATCH v6 2/3] drm/i915: Make GuC log part of the uC " Michal Wajdeczko
2017-10-26 17:36 ` [PATCH v6 3/3] drm/i915: Handle error-state modparams in dedicated functions Michal Wajdeczko
2017-10-26 18:01 ` ✓ Fi.CI.BAT: success for series starting with [v6,1/3] drm/i915: Add Guc/HuC firmware details to error state Patchwork
2017-10-26 19:10 ` ✓ Fi.CI.IGT: " Patchwork
2017-11-06 14:15 ` [PATCH v6 1/3] " Chris Wilson
2017-11-08 11:27   ` Joonas Lahtinen
2017-11-08 12:20     ` Michal Wajdeczko
2017-11-08 20:21       ` Michal Wajdeczko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox