[PATCH 0/6] drm/i915: modparam rework prep work

[PATCH 0/6] drm/i915: modparam rework prep work

[Jani Nikula]

The first six prep patches of the module params to device params series
[1]. These should be fairly non-controversial and pass CI.

BR,
Jani.

[1] https://patchwork.freedesktop.org/series/54414/



Jani Nikula (6):
  drm/i915: add a helper to make a copy of i915_params
  drm/i915: add a helper to free the members of i915_params
  drm/i915/uc: add dev_priv parameter to intel_uc_is_using_* functions
  drm/i915/params: set i915.enable_hangcheck permissions to 0600
  drm/i915: move load failure injection to selftests
  drm/i915/params: document I915_PARAMS_FOR_EACH()

 drivers/gpu/drm/i915/i915_drv.c                | 25 -----------------
 drivers/gpu/drm/i915/i915_drv.h                | 21 +++------------
 drivers/gpu/drm/i915/i915_gpu_error.c          | 20 ++------------
 drivers/gpu/drm/i915/i915_params.c             | 37 +++++++++++++++++++++-----
 drivers/gpu/drm/i915/i915_params.h             | 12 ++++++++-
 drivers/gpu/drm/i915/i915_selftest.h           | 10 +++++++
 drivers/gpu/drm/i915/intel_uc.c                | 12 ++++-----
 drivers/gpu/drm/i915/intel_uc.h                |  6 ++---
 drivers/gpu/drm/i915/intel_wopcm.c             |  2 +-
 drivers/gpu/drm/i915/selftests/i915_selftest.c | 26 ++++++++++++++++++
 10 files changed, 93 insertions(+), 78 deletions(-)

-- 
2.11.0

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

[PATCH 1/6] drm/i915: add a helper to make a copy of i915_params

[Jani Nikula]

Abstract the one user in anticipation of more.

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_gpu_error.c | 11 +----------
 drivers/gpu/drm/i915/i915_params.c    | 14 ++++++++++++++
 drivers/gpu/drm/i915/i915_params.h    |  1 +
 3 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index 63df41d93379..8c04479c1586 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -1834,18 +1834,9 @@ static void capture_gen_state(struct i915_gpu_state *error)
 	error->driver_caps = i915->caps;
 }
 
-static __always_inline void dup_param(const char *type, void *x)
-{
-	if (!__builtin_strcmp(type, "char *"))
-		*(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
+	i915_params_copy(&error->params, &i915_modparams);
 }
 
 static unsigned long capture_find_epoch(const struct i915_gpu_state *error)
diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index 2e0356561839..ae3ece4ec7ab 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -203,3 +203,17 @@ void i915_params_dump(const struct i915_params *params, struct drm_printer *p)
 	I915_PARAMS_FOR_EACH(PRINT);
 #undef PRINT
 }
+
+static __always_inline void dup_param(const char *type, void *x)
+{
+	if (!__builtin_strcmp(type, "char *"))
+		*(void **)x = kstrdup(*(void **)x, GFP_ATOMIC);
+}
+
+void i915_params_copy(struct i915_params *dest, const struct i915_params *src)
+{
+	*dest = *src;
+#define DUP(T, x, ...) dup_param(#T, &dest->x);
+	I915_PARAMS_FOR_EACH(DUP);
+#undef DUP
+}
diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
index 7e56c516c815..fd1cf9415e60 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -78,6 +78,7 @@ struct i915_params {
 extern struct i915_params i915_modparams __read_mostly;
 
 void i915_params_dump(const struct i915_params *params, struct drm_printer *p);
+void i915_params_copy(struct i915_params *dest, const struct i915_params *src);
 
 #endif
 
-- 
2.11.0

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

[PATCH 2/6] drm/i915: add a helper to free the members of i915_params

[Jani Nikula]

Abstract the one user in anticipation of more. Set the dangling pointers
to NULL while at it.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_gpu_error.c |  9 +--------
 drivers/gpu/drm/i915/i915_params.c    | 16 ++++++++++++++++
 drivers/gpu/drm/i915/i915_params.h    |  1 +
 3 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index 8c04479c1586..2bd7991ec9af 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -963,17 +963,10 @@ static void i915_error_object_free(struct drm_i915_error_object *obj)
 	kfree(obj);
 }
 
-static __always_inline void free_param(const char *type, void *x)
-{
-	if (!__builtin_strcmp(type, "char *"))
-		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
+	i915_params_free(&error->params);
 }
 
 static void cleanup_uc_state(struct i915_gpu_state *error)
diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index ae3ece4ec7ab..81c73bfc7991 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -217,3 +217,19 @@ void i915_params_copy(struct i915_params *dest, const struct i915_params *src)
 	I915_PARAMS_FOR_EACH(DUP);
 #undef DUP
 }
+
+static __always_inline void free_param(const char *type, void *x)
+{
+	if (!__builtin_strcmp(type, "char *")) {
+		kfree(*(void **)x);
+		*(void **)x = NULL;
+	}
+}
+
+/* free the allocated members, *not* the passed in params itself */
+void i915_params_free(struct i915_params *params)
+{
+#define FREE(T, x, ...) free_param(#T, &params->x);
+	I915_PARAMS_FOR_EACH(FREE);
+#undef FREE
+}
diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
index fd1cf9415e60..93f665eced16 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -79,6 +79,7 @@ extern struct i915_params i915_modparams __read_mostly;
 
 void i915_params_dump(const struct i915_params *params, struct drm_printer *p);
 void i915_params_copy(struct i915_params *dest, const struct i915_params *src);
+void i915_params_free(struct i915_params *params);
 
 #endif
 
-- 
2.11.0

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

[PATCH 3/6] drm/i915/uc: add dev_priv parameter to intel_uc_is_using_* functions

[Jani Nikula]

Reveals the build fail fixed in the last hunk. Also prep work.

v2: name it i915 instead of dev_priv (Michal)

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h    |  6 +++---
 drivers/gpu/drm/i915/intel_uc.c    | 12 ++++++------
 drivers/gpu/drm/i915/intel_uc.h    |  6 +++---
 drivers/gpu/drm/i915/intel_wopcm.c |  2 +-
 4 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index d44255a8655e..c60cf17e50a0 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2490,9 +2490,9 @@ intel_info(const struct drm_i915_private *dev_priv)
 #define HAS_HUC_UCODE(dev_priv)	(HAS_GUC(dev_priv))
 
 /* Having a GuC is not the same as using a GuC */
-#define USES_GUC(dev_priv)		intel_uc_is_using_guc()
-#define USES_GUC_SUBMISSION(dev_priv)	intel_uc_is_using_guc_submission()
-#define USES_HUC(dev_priv)		intel_uc_is_using_huc()
+#define USES_GUC(dev_priv)		intel_uc_is_using_guc(dev_priv)
+#define USES_GUC_SUBMISSION(dev_priv)	intel_uc_is_using_guc_submission(dev_priv)
+#define USES_HUC(dev_priv)		intel_uc_is_using_huc(dev_priv)
 
 #define HAS_POOLED_EU(dev_priv)	((dev_priv)->info.has_pooled_eu)
 
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 447b1de77cc7..731b82afe636 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -71,7 +71,7 @@ static int __get_default_guc_log_level(struct drm_i915_private *i915)
 {
 	int guc_log_level;
 
-	if (!HAS_GUC(i915) || !intel_uc_is_using_guc())
+	if (!HAS_GUC(i915) || !intel_uc_is_using_guc(i915))
 		guc_log_level = GUC_LOG_LEVEL_DISABLED;
 	else if (IS_ENABLED(CONFIG_DRM_I915_DEBUG) ||
 		 IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM))
@@ -112,11 +112,11 @@ static void sanitize_options_early(struct drm_i915_private *i915)
 
 	DRM_DEBUG_DRIVER("enable_guc=%d (submission:%s huc:%s)\n",
 			 i915_modparams.enable_guc,
-			 yesno(intel_uc_is_using_guc_submission()),
-			 yesno(intel_uc_is_using_huc()));
+			 yesno(intel_uc_is_using_guc_submission(i915)),
+			 yesno(intel_uc_is_using_huc(i915)));
 
 	/* Verify GuC firmware availability */
-	if (intel_uc_is_using_guc() && !intel_uc_fw_is_selected(guc_fw)) {
+	if (intel_uc_is_using_guc(i915) && !intel_uc_fw_is_selected(guc_fw)) {
 		DRM_WARN("Incompatible option detected: %s=%d, %s!\n",
 			 "enable_guc", i915_modparams.enable_guc,
 			 !HAS_GUC(i915) ? "no GuC hardware" :
@@ -124,7 +124,7 @@ static void sanitize_options_early(struct drm_i915_private *i915)
 	}
 
 	/* Verify HuC firmware availability */
-	if (intel_uc_is_using_huc() && !intel_uc_fw_is_selected(huc_fw)) {
+	if (intel_uc_is_using_huc(i915) && !intel_uc_fw_is_selected(huc_fw)) {
 		DRM_WARN("Incompatible option detected: %s=%d, %s!\n",
 			 "enable_guc", i915_modparams.enable_guc,
 			 !HAS_HUC(i915) ? "no HuC hardware" :
@@ -136,7 +136,7 @@ static void sanitize_options_early(struct drm_i915_private *i915)
 		i915_modparams.guc_log_level =
 			__get_default_guc_log_level(i915);
 
-	if (i915_modparams.guc_log_level > 0 && !intel_uc_is_using_guc()) {
+	if (i915_modparams.guc_log_level > 0 && !intel_uc_is_using_guc(i915)) {
 		DRM_WARN("Incompatible option detected: %s=%d, %s!\n",
 			 "guc_log_level", i915_modparams.guc_log_level,
 			 !HAS_GUC(i915) ? "no GuC hardware" :
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 25d73ada74ae..870faf9011b9 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -41,19 +41,19 @@ void intel_uc_fini(struct drm_i915_private *dev_priv);
 int intel_uc_suspend(struct drm_i915_private *dev_priv);
 int intel_uc_resume(struct drm_i915_private *dev_priv);
 
-static inline bool intel_uc_is_using_guc(void)
+static inline bool intel_uc_is_using_guc(struct drm_i915_private *i915)
 {
 	GEM_BUG_ON(i915_modparams.enable_guc < 0);
 	return i915_modparams.enable_guc > 0;
 }
 
-static inline bool intel_uc_is_using_guc_submission(void)
+static inline bool intel_uc_is_using_guc_submission(struct drm_i915_private *i915)
 {
 	GEM_BUG_ON(i915_modparams.enable_guc < 0);
 	return i915_modparams.enable_guc & ENABLE_GUC_SUBMISSION;
 }
 
-static inline bool intel_uc_is_using_huc(void)
+static inline bool intel_uc_is_using_huc(struct drm_i915_private *i915)
 {
 	GEM_BUG_ON(i915_modparams.enable_guc < 0);
 	return i915_modparams.enable_guc & ENABLE_GUC_LOAD_HUC;
diff --git a/drivers/gpu/drm/i915/intel_wopcm.c b/drivers/gpu/drm/i915/intel_wopcm.c
index 630c887682e8..f82a415ea2ba 100644
--- a/drivers/gpu/drm/i915/intel_wopcm.c
+++ b/drivers/gpu/drm/i915/intel_wopcm.c
@@ -163,7 +163,7 @@ int intel_wopcm_init(struct intel_wopcm *wopcm)
 	u32 guc_wopcm_rsvd;
 	int err;
 
-	if (!USES_GUC(dev_priv))
+	if (!USES_GUC(i915))
 		return 0;
 
 	GEM_BUG_ON(!wopcm->size);
-- 
2.11.0

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

[PATCH 4/6] drm/i915/params: set i915.enable_hangcheck permissions to 0600

[Jani Nikula]

i915.enable_hangcheck has been an outlier since its introduction in
commit 3e0dc6b01f53 ("drm/i915: hangcheck disable parameter") with 0644
permissions, while all the rest are either 0400 or 0600. Follow suit
with 0600.

IGT never reads the value, so there should be no impact.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_params.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index 81c73bfc7991..9f0539bdaa39 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -77,7 +77,7 @@ i915_param_named(error_capture, bool, 0600,
 	"triaging and debugging hangs.");
 #endif
 
-i915_param_named_unsafe(enable_hangcheck, bool, 0644,
+i915_param_named_unsafe(enable_hangcheck, bool, 0600,
 	"Periodically check GPU activity for detecting hangs. "
 	"WARNING: Disabling this can cause system wide hangs. "
 	"(default: true)");
-- 
2.11.0

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

[PATCH 5/6] drm/i915: move load failure injection to selftests

[Jani Nikula]

Seems like selftests is a better home for everything related to load
failure injection, including the module parameter.

The failure injection code gets moved from under DRM_I915_DEBUG to
DRM_I915_SELFTEST config option. This should be of no consequence, as
the former selects the latter.

With the parameter no longer part of i915_params, its value will not be
recorded in error capture or debugfs param dump. Note that the value
would have been zeroed anyway if a selftest had been hit, so there
should not be meaningful information loss here, especially with all the
logging around failure injection.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c                | 25 -------------------------
 drivers/gpu/drm/i915/i915_drv.h                | 15 ---------------
 drivers/gpu/drm/i915/i915_params.c             |  5 -----
 drivers/gpu/drm/i915/i915_params.h             |  1 -
 drivers/gpu/drm/i915/i915_selftest.h           | 10 ++++++++++
 drivers/gpu/drm/i915/selftests/i915_selftest.c | 26 ++++++++++++++++++++++++++
 6 files changed, 36 insertions(+), 46 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index caa055ac9472..559a1b11f3e4 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -57,31 +57,6 @@
 
 static struct drm_driver driver;
 
-#if IS_ENABLED(CONFIG_DRM_I915_DEBUG)
-static unsigned int i915_load_fail_count;
-
-bool __i915_inject_load_failure(const char *func, int line)
-{
-	if (i915_load_fail_count >= i915_modparams.inject_load_failure)
-		return false;
-
-	if (++i915_load_fail_count == i915_modparams.inject_load_failure) {
-		DRM_INFO("Injecting failure at checkpoint %u [%s:%d]\n",
-			 i915_modparams.inject_load_failure, func, line);
-		i915_modparams.inject_load_failure = 0;
-		return true;
-	}
-
-	return false;
-}
-
-bool i915_error_injected(void)
-{
-	return i915_load_fail_count && !i915_modparams.inject_load_failure;
-}
-
-#endif
-
 #define FDO_BUG_URL "https://bugs.freedesktop.org/enter_bug.cgi?product=DRI"
 #define FDO_BUG_MSG "Please file a bug at " FDO_BUG_URL " against DRM/Intel " \
 		    "providing the dmesg log by booting with drm.debug=0xf"
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index c60cf17e50a0..1f29f3933625 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -111,21 +111,6 @@
 #define I915_STATE_WARN_ON(x)						\
 	I915_STATE_WARN((x), "%s", "WARN_ON(" __stringify(x) ")")
 
-#if IS_ENABLED(CONFIG_DRM_I915_DEBUG)
-
-bool __i915_inject_load_failure(const char *func, int line);
-#define i915_inject_load_failure() \
-	__i915_inject_load_failure(__func__, __LINE__)
-
-bool i915_error_injected(void);
-
-#else
-
-#define i915_inject_load_failure() false
-#define i915_error_injected() false
-
-#endif
-
 #define i915_load_error(i915, fmt, ...)					 \
 	__i915_printk(i915, i915_error_injected() ? KERN_DEBUG : KERN_ERR, \
 		      fmt, ##__VA_ARGS__)
diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index 9f0539bdaa39..2853b54570eb 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -159,11 +159,6 @@ i915_param_named_unsafe(dmc_firmware_path, charp, 0400,
 i915_param_named_unsafe(enable_dp_mst, bool, 0600,
 	"Enable multi-stream transport (MST) for new DisplayPort sinks. (default: true)");
 
-#if IS_ENABLED(CONFIG_DRM_I915_DEBUG)
-i915_param_named_unsafe(inject_load_failure, uint, 0400,
-	"Force an error after a number of failure check points (0:disabled (default), N:force failure at the Nth failure check point)");
-#endif
-
 i915_param_named(enable_dpcd_backlight, bool, 0600,
 	"Enable support for DPCD backlight control (default:false)");
 
diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
index 93f665eced16..85a9007c0ed6 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -53,7 +53,6 @@ struct drm_printer;
 	param(int, mmio_debug, 0) \
 	param(int, edp_vswing, 0) \
 	param(int, reset, 2) \
-	param(unsigned int, inject_load_failure, 0) \
 	/* leave bools at the end to not create holes */ \
 	param(bool, alpha_support, IS_ENABLED(CONFIG_DRM_I915_ALPHA_SUPPORT)) \
 	param(bool, enable_hangcheck, true) \
diff --git a/drivers/gpu/drm/i915/i915_selftest.h b/drivers/gpu/drm/i915/i915_selftest.h
index a73472dd12fd..1266cef8bf17 100644
--- a/drivers/gpu/drm/i915/i915_selftest.h
+++ b/drivers/gpu/drm/i915/i915_selftest.h
@@ -33,6 +33,7 @@ struct i915_selftest {
 	unsigned int random_seed;
 	int mock;
 	int live;
+	unsigned int inject_load_failure;
 };
 
 #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
@@ -77,6 +78,12 @@ int __i915_subtests(const char *caller,
 #define I915_SELFTEST_DECLARE(x) x
 #define I915_SELFTEST_ONLY(x) unlikely(x)
 
+bool __i915_inject_load_failure(const char *func, int line);
+#define i915_inject_load_failure() \
+	__i915_inject_load_failure(__func__, __LINE__)
+
+bool i915_error_injected(void);
+
 #else /* !IS_ENABLED(CONFIG_DRM_I915_SELFTEST) */
 
 static inline int i915_mock_selftests(void) { return 0; }
@@ -85,6 +92,9 @@ static inline int i915_live_selftests(struct pci_dev *pdev) { return 0; }
 #define I915_SELFTEST_DECLARE(x)
 #define I915_SELFTEST_ONLY(x) 0
 
+static inline int i915_inject_load_failure(void) { return false; }
+static inline int i915_error_injected(void) { return false; }
+
 #endif
 
 /* Using the i915_selftest_ prefix becomes a little unwieldy with the helpers.
diff --git a/drivers/gpu/drm/i915/selftests/i915_selftest.c b/drivers/gpu/drm/i915/selftests/i915_selftest.c
index 86c54ea37f48..9e556fc4707d 100644
--- a/drivers/gpu/drm/i915/selftests/i915_selftest.c
+++ b/drivers/gpu/drm/i915/selftests/i915_selftest.c
@@ -250,3 +250,29 @@ MODULE_PARM_DESC(mock_selftests, "Run selftests before loading, using mock hardw
 
 module_param_named_unsafe(live_selftests, i915_selftest.live, int, 0400);
 MODULE_PARM_DESC(live_selftests, "Run selftests after driver initialisation on the live system (0:disabled [default], 1:run tests then continue, -1:run tests then exit module)");
+
+module_param_named_unsafe(inject_load_failure, i915_selftest.inject_load_failure, uint, 0400);
+MODULE_PARM_DESC(inject_load_failure,
+		 "Force an error after a number of failure check points (0:disabled (default), N:force failure at the Nth failure check point)");
+
+static unsigned int i915_load_fail_count;
+
+bool __i915_inject_load_failure(const char *func, int line)
+{
+	if (i915_load_fail_count >= i915_selftest.inject_load_failure)
+		return false;
+
+	if (++i915_load_fail_count == i915_selftest.inject_load_failure) {
+		DRM_INFO("Injecting failure at checkpoint %u [%s:%d]\n",
+			 i915_selftest.inject_load_failure, func, line);
+		i915_selftest.inject_load_failure = 0;
+		return true;
+	}
+
+	return false;
+}
+
+bool i915_error_injected(void)
+{
+	return i915_load_fail_count && !i915_selftest.inject_load_failure;
+}
-- 
2.11.0

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

[PATCH 6/6] drm/i915/params: document I915_PARAMS_FOR_EACH()

[Jani Nikula]

Macros with this much magic in them deserve some explanatory text.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_params.h | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
index 85a9007c0ed6..98eba6728095 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -33,6 +33,15 @@ struct drm_printer;
 #define ENABLE_GUC_SUBMISSION		BIT(0)
 #define ENABLE_GUC_LOAD_HUC		BIT(1)
 
+/*
+ * Invoke param, a function-like macro, for each i915 param, with arguments:
+ *
+ * param(type, name, value)
+ *
+ * type: parameter type, one of {bool, int, unsigned int, char *}
+ * name: name of the parameter
+ * value: initial/default value of the parameter
+ */
 #define I915_PARAMS_FOR_EACH(param) \
 	param(char *, vbt_firmware, NULL) \
 	param(int, modeset, -1) \
-- 
2.11.0

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

✗ Fi.CI.CHECKPATCH: warning for drm/i915: modparam rework prep work

[Patchwork]

== Series Details ==

Series: drm/i915: modparam rework prep work
URL   : https://patchwork.freedesktop.org/series/54499/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
ef92c4788911 drm/i915: add a helper to make a copy of i915_params
-:53: CHECK:MACRO_ARG_PRECEDENCE: Macro argument 'x' may be better as '(x)' to avoid precedence issues
#53: FILE: drivers/gpu/drm/i915/i915_params.c:216:
+#define DUP(T, x, ...) dup_param(#T, &dest->x);

-:53: WARNING:TRAILING_SEMICOLON: macros should not use a trailing semicolon
#53: FILE: drivers/gpu/drm/i915/i915_params.c:216:
+#define DUP(T, x, ...) dup_param(#T, &dest->x);

total: 0 errors, 1 warnings, 1 checks, 43 lines checked
446aa1f787ee drm/i915: add a helper to free the members of i915_params
-:54: CHECK:MACRO_ARG_PRECEDENCE: Macro argument 'x' may be better as '(x)' to avoid precedence issues
#54: FILE: drivers/gpu/drm/i915/i915_params.c:232:
+#define FREE(T, x, ...) free_param(#T, &params->x);

-:54: WARNING:TRAILING_SEMICOLON: macros should not use a trailing semicolon
#54: FILE: drivers/gpu/drm/i915/i915_params.c:232:
+#define FREE(T, x, ...) free_param(#T, &params->x);

total: 0 errors, 1 warnings, 1 checks, 44 lines checked
1116230810cd drm/i915/uc: add dev_priv parameter to intel_uc_is_using_* functions
e41b8248dea6 drm/i915/params: set i915.enable_hangcheck permissions to 0600
-:26: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#26: FILE: drivers/gpu/drm/i915/i915_params.c:81:
+i915_param_named_unsafe(enable_hangcheck, bool, 0600,
 	"Periodically check GPU activity for detecting hangs. "

total: 0 errors, 0 warnings, 1 checks, 8 lines checked
f21e97b58b40 drm/i915: move load failure injection to selftests
cef50b357096 drm/i915/params: document I915_PARAMS_FOR_EACH()

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

✗ Fi.CI.SPARSE: warning for drm/i915: modparam rework prep work

[Patchwork]

== Series Details ==

Series: drm/i915: modparam rework prep work
URL   : https://patchwork.freedesktop.org/series/54499/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Sparse version: v0.5.2
Commit: drm/i915: add a helper to make a copy of i915_params
Okay!

Commit: drm/i915: add a helper to free the members of i915_params
Okay!

Commit: drm/i915/uc: add dev_priv parameter to intel_uc_is_using_* functions
Okay!

Commit: drm/i915/params: set i915.enable_hangcheck permissions to 0600
Okay!

Commit: drm/i915: move load failure injection to selftests
-drivers/gpu/drm/i915/selftests/../i915_drv.h:3550:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/selftests/../i915_drv.h:3535:16: warning: expression using sizeof(void)

Commit: drm/i915/params: document I915_PARAMS_FOR_EACH()
Okay!

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

✗ Fi.CI.BAT: failure for drm/i915: modparam rework prep work

[Patchwork]

== Series Details ==

Series: drm/i915: modparam rework prep work
URL   : https://patchwork.freedesktop.org/series/54499/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_5343 -> Patchwork_11156
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_11156 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_11156, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/54499/revisions/1/mbox/

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@i915_selftest@live_contexts:
    - fi-bsw-kefka:       NOTRUN -> DMESG-FAIL

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

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

### IGT changes ###

#### Issues hit ####

  * igt@i915_module_load@reload:
    - fi-blb-e6850:       NOTRUN -> INCOMPLETE [fdo#107718]

  * {igt@runner@aborted}:
    - fi-icl-y:           NOTRUN -> FAIL [fdo#108915]

  
#### Possible fixes ####

  * igt@gem_ctx_create@basic-files:
    - fi-bsw-kefka:       INCOMPLETE [fdo#105876] / [fdo#108714] -> PASS

  * igt@gem_exec_suspend@basic-s3:
    - fi-blb-e6850:       INCOMPLETE [fdo#107718] -> PASS

  * igt@i915_selftest@live_contexts:
    - fi-byt-j1900:       DMESG-WARN -> PASS

  
#### Warnings ####

  * igt@i915_selftest@live_contexts:
    - fi-icl-u2:          INCOMPLETE [fdo#108315] -> DMESG-FAIL [fdo#108569]

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

  [fdo#105876]: https://bugs.freedesktop.org/show_bug.cgi?id=105876
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#108315]: https://bugs.freedesktop.org/show_bug.cgi?id=108315
  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
  [fdo#108714]: https://bugs.freedesktop.org/show_bug.cgi?id=108714
  [fdo#108915]: https://bugs.freedesktop.org/show_bug.cgi?id=108915


Participating hosts (46 -> 42)
------------------------------

  Additional (2): fi-icl-y fi-pnv-d510 
  Missing    (6): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 


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

    * Linux: CI_DRM_5343 -> Patchwork_11156

  CI_DRM_5343: e7a162c10f4ab5e4338927ec5bc5be726ecf3e8b @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4754: a176905d46d072300ba57f29ac2b98a0228e0e2d @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_11156: cef50b3570963ec3321c7bda92dfd80d159a0c02 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

cef50b357096 drm/i915/params: document I915_PARAMS_FOR_EACH()
f21e97b58b40 drm/i915: move load failure injection to selftests
e41b8248dea6 drm/i915/params: set i915.enable_hangcheck permissions to 0600
1116230810cd drm/i915/uc: add dev_priv parameter to intel_uc_is_using_* functions
446aa1f787ee drm/i915: add a helper to free the members of i915_params
ef92c4788911 drm/i915: add a helper to make a copy of i915_params

== Logs ==

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

✗ Fi.CI.CHECKPATCH: warning for drm/i915: modparam rework prep work (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: modparam rework prep work (rev2)
URL   : https://patchwork.freedesktop.org/series/54499/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
5ee0abd94977 drm/i915: add a helper to make a copy of i915_params
-:53: CHECK:MACRO_ARG_PRECEDENCE: Macro argument 'x' may be better as '(x)' to avoid precedence issues
#53: FILE: drivers/gpu/drm/i915/i915_params.c:216:
+#define DUP(T, x, ...) dup_param(#T, &dest->x);

-:53: WARNING:TRAILING_SEMICOLON: macros should not use a trailing semicolon
#53: FILE: drivers/gpu/drm/i915/i915_params.c:216:
+#define DUP(T, x, ...) dup_param(#T, &dest->x);

total: 0 errors, 1 warnings, 1 checks, 43 lines checked
c829d2117b4a drm/i915: add a helper to free the members of i915_params
-:54: CHECK:MACRO_ARG_PRECEDENCE: Macro argument 'x' may be better as '(x)' to avoid precedence issues
#54: FILE: drivers/gpu/drm/i915/i915_params.c:232:
+#define FREE(T, x, ...) free_param(#T, &params->x);

-:54: WARNING:TRAILING_SEMICOLON: macros should not use a trailing semicolon
#54: FILE: drivers/gpu/drm/i915/i915_params.c:232:
+#define FREE(T, x, ...) free_param(#T, &params->x);

total: 0 errors, 1 warnings, 1 checks, 44 lines checked
1c32c8a8f4a5 drm/i915/uc: add dev_priv parameter to intel_uc_is_using_* functions
cf2343abe4a6 drm/i915/params: set i915.enable_hangcheck permissions to 0600
-:26: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#26: FILE: drivers/gpu/drm/i915/i915_params.c:81:
+i915_param_named_unsafe(enable_hangcheck, bool, 0600,
 	"Periodically check GPU activity for detecting hangs. "

total: 0 errors, 0 warnings, 1 checks, 8 lines checked
adbd54b825e1 drm/i915: move load failure injection to selftests
11491dfc6f63 drm/i915/params: document I915_PARAMS_FOR_EACH()

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

✗ Fi.CI.SPARSE: warning for drm/i915: modparam rework prep work (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: modparam rework prep work (rev2)
URL   : https://patchwork.freedesktop.org/series/54499/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Sparse version: v0.5.2
Commit: drm/i915: add a helper to make a copy of i915_params
Okay!

Commit: drm/i915: add a helper to free the members of i915_params
Okay!

Commit: drm/i915/uc: add dev_priv parameter to intel_uc_is_using_* functions
Okay!

Commit: drm/i915/params: set i915.enable_hangcheck permissions to 0600
Okay!

Commit: drm/i915: move load failure injection to selftests
-drivers/gpu/drm/i915/selftests/../i915_drv.h:3550:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/selftests/../i915_drv.h:3535:16: warning: expression using sizeof(void)

Commit: drm/i915/params: document I915_PARAMS_FOR_EACH()
Okay!

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

✓ Fi.CI.BAT: success for drm/i915: modparam rework prep work (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: modparam rework prep work (rev2)
URL   : https://patchwork.freedesktop.org/series/54499/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5345 -> Patchwork_11160
====================================================

Summary
-------

  **WARNING**

  Minor unknown changes coming with Patchwork_11160 need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_11160, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/54499/revisions/2/mbox/

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

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

### IGT changes ###

#### Warnings ####

  * igt@pm_rpm@basic-pci-d3-state:
    - fi-byt-j1900:       SKIP -> PASS

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

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

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live_hangcheck:
    - fi-bwr-2160:        PASS -> DMESG-FAIL [fdo#108735]

  * igt@kms_frontbuffer_tracking@basic:
    - fi-hsw-peppy:       PASS -> DMESG-WARN [fdo#102614]
    - fi-icl-u2:          PASS -> FAIL [fdo#103167]

  
#### Possible fixes ####

  * igt@gem_exec_suspend@basic-s3:
    - fi-blb-e6850:       INCOMPLETE [fdo#107718] -> PASS

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-kbl-7500u:       FAIL [fdo#108767] -> PASS

  * igt@pm_rpm@basic-rte:
    - fi-byt-j1900:       FAIL [fdo#108800] -> PASS

  
  [fdo#102614]: https://bugs.freedesktop.org/show_bug.cgi?id=102614
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#108735]: https://bugs.freedesktop.org/show_bug.cgi?id=108735
  [fdo#108767]: https://bugs.freedesktop.org/show_bug.cgi?id=108767
  [fdo#108800]: https://bugs.freedesktop.org/show_bug.cgi?id=108800


Participating hosts (46 -> 42)
------------------------------

  Additional (2): fi-kbl-7560u fi-pnv-d510 
  Missing    (6): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 


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

    * Linux: CI_DRM_5345 -> Patchwork_11160

  CI_DRM_5345: 401703d974c357af84537c0d5f1e07ebeaaa99fe @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4754: a176905d46d072300ba57f29ac2b98a0228e0e2d @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_11160: 11491dfc6f63a23916eb93b4548cb83f7b702405 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

11491dfc6f63 drm/i915/params: document I915_PARAMS_FOR_EACH()
adbd54b825e1 drm/i915: move load failure injection to selftests
cf2343abe4a6 drm/i915/params: set i915.enable_hangcheck permissions to 0600
1c32c8a8f4a5 drm/i915/uc: add dev_priv parameter to intel_uc_is_using_* functions
c829d2117b4a drm/i915: add a helper to free the members of i915_params
5ee0abd94977 drm/i915: add a helper to make a copy of i915_params

== Logs ==

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

✓ Fi.CI.IGT: success for drm/i915: modparam rework prep work (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: modparam rework prep work (rev2)
URL   : https://patchwork.freedesktop.org/series/54499/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5345_full -> Patchwork_11160_full
====================================================

Summary
-------

  **WARNING**

  Minor unknown changes coming with Patchwork_11160_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_11160_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

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

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

### IGT changes ###

#### Warnings ####

  * igt@tools_test@tools_test:
    - shard-iclb:         PASS -> SKIP

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_ctx_isolation@vcs1-s3:
    - shard-kbl:          PASS -> INCOMPLETE [fdo#103665]

  * igt@gem_workarounds@suspend-resume-fd:
    - shard-skl:          PASS -> INCOMPLETE [fdo#104108] / [fdo#107773] +1

  * igt@i915_suspend@shrink:
    - shard-skl:          NOTRUN -> INCOMPLETE [fdo#106886]

  * igt@kms_available_modes_crc@available_mode_test_crc:
    - shard-apl:          PASS -> FAIL [fdo#106641]

  * igt@kms_busy@extended-modeset-hang-newfb-render-a:
    - shard-iclb:         NOTRUN -> DMESG-WARN [fdo#107956]

  * igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-a:
    - shard-skl:          NOTRUN -> DMESG-WARN [fdo#107956]

  * igt@kms_chv_cursor_fail@pipe-c-128x128-bottom-edge:
    - shard-skl:          PASS -> FAIL [fdo#104671] +1

  * igt@kms_cursor_crc@cursor-64x64-suspend:
    - shard-glk:          PASS -> FAIL [fdo#103232]

  * igt@kms_cursor_crc@cursor-size-change:
    - shard-skl:          NOTRUN -> FAIL [fdo#103232]

  * igt@kms_cursor_legacy@cursor-vs-flip-atomic-transitions:
    - shard-iclb:         PASS -> FAIL [fdo#103355]

  * igt@kms_flip@flip-vs-expired-vblank-interruptible:
    - shard-skl:          NOTRUN -> FAIL [fdo#105363]

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-render:
    - shard-glk:          PASS -> FAIL [fdo#103167]

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-move:
    - shard-iclb:         PASS -> FAIL [fdo#103167] +7

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-fullscreen:
    - shard-iclb:         NOTRUN -> FAIL [fdo#103167]

  * igt@kms_panel_fitting@legacy:
    - shard-skl:          NOTRUN -> FAIL [fdo#105456]

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-c-frame-sequence:
    - shard-skl:          PASS -> FAIL [fdo#103191] / [fdo#107362]

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
    - shard-kbl:          PASS -> DMESG-WARN [fdo#103558] / [fdo#105602] +6

  * igt@kms_plane@pixel-format-pipe-b-planes:
    - shard-skl:          NOTRUN -> DMESG-WARN [fdo#106885] +1

  * igt@kms_plane@plane-position-covered-pipe-b-planes:
    - shard-apl:          PASS -> FAIL [fdo#103166]

  * igt@kms_plane_alpha_blend@pipe-a-alpha-basic:
    - shard-skl:          NOTRUN -> FAIL [fdo#107815] / [fdo#108145]

  * igt@kms_plane_alpha_blend@pipe-a-alpha-opaque-fb:
    - shard-skl:          NOTRUN -> FAIL [fdo#108145] +1

  * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
    - shard-skl:          PASS -> FAIL [fdo#107815]

  * igt@kms_plane_multiple@atomic-pipe-b-tiling-yf:
    - shard-iclb:         PASS -> FAIL [fdo#103166]

  * igt@kms_plane_multiple@atomic-pipe-c-tiling-x:
    - shard-glk:          PASS -> FAIL [fdo#103166]

  * igt@kms_plane_scaling@pipe-b-scaler-with-rotation:
    - shard-iclb:         NOTRUN -> DMESG-WARN [fdo#107724]

  * igt@kms_psr@suspend:
    - shard-skl:          NOTRUN -> INCOMPLETE [fdo#107773]

  * igt@kms_rmfb@close-fd:
    - shard-iclb:         PASS -> DMESG-WARN [fdo#107724] +1

  * igt@kms_rotation_crc@multiplane-rotation-cropping-top:
    - shard-glk:          PASS -> DMESG-FAIL [fdo#105763] / [fdo#106538]

  * igt@kms_setmode@basic:
    - shard-skl:          NOTRUN -> FAIL [fdo#99912]
    - shard-hsw:          PASS -> FAIL [fdo#99912]

  * igt@kms_vblank@pipe-c-ts-continuation-idle-hang:
    - shard-kbl:          PASS -> DMESG-WARN [fdo#103313] / [fdo#103558] / [fdo#105602] +5

  * igt@perf_pmu@rc6-runtime-pm:
    - shard-iclb:         PASS -> FAIL [fdo#105010]

  * igt@pm_rpm@cursor-dpms:
    - shard-skl:          PASS -> INCOMPLETE [fdo#107807]

  * igt@pm_rpm@universal-planes:
    - shard-iclb:         PASS -> DMESG-WARN [fdo#108654] / [fdo#108756]

  * igt@pm_rps@waitboost:
    - shard-iclb:         NOTRUN -> FAIL [fdo#102250] / [fdo#108059]

  * {igt@runner@aborted}:
    - shard-iclb:         NOTRUN -> FAIL [fdo#108654] / [fdo#108756]

  
#### Possible fixes ####

  * igt@gem_softpin@noreloc-s3:
    - shard-iclb:         INCOMPLETE [fdo#107713] -> PASS

  * igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-b:
    - shard-snb:          DMESG-WARN [fdo#107956] -> SKIP

  * igt@kms_cursor_crc@cursor-128x128-offscreen:
    - shard-skl:          FAIL [fdo#103232] -> PASS +1

  * igt@kms_cursor_crc@cursor-128x42-random:
    - shard-apl:          FAIL [fdo#103232] -> PASS

  * igt@kms_cursor_crc@cursor-256x256-dpms:
    - shard-glk:          FAIL [fdo#103232] -> PASS +1

  * igt@kms_draw_crc@draw-method-xrgb8888-pwrite-untiled:
    - shard-iclb:         WARN [fdo#108336] -> PASS

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-fullscreen:
    - shard-apl:          FAIL [fdo#103167] -> PASS

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-draw-mmap-gtt:
    - shard-glk:          FAIL [fdo#103167] -> PASS

  * igt@kms_frontbuffer_tracking@fbc-suspend:
    - shard-skl:          INCOMPLETE [fdo#104108] / [fdo#105959] / [fdo#107773] -> PASS

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-pwrite:
    - shard-iclb:         DMESG-FAIL [fdo#107724] -> PASS +3

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-cur-indfb-draw-mmap-cpu:
    - shard-iclb:         DMESG-WARN [fdo#107724] / [fdo#108336] -> PASS +4

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-move:
    - shard-iclb:         FAIL [fdo#103167] -> PASS +2

  * igt@kms_plane_alpha_blend@pipe-b-coverage-7efc:
    - shard-skl:          FAIL [fdo#107815] -> PASS

  * igt@kms_plane_multiple@atomic-pipe-c-tiling-y:
    - shard-glk:          FAIL [fdo#103166] -> PASS

  * igt@kms_rmfb@rmfb-ioctl:
    - shard-iclb:         DMESG-WARN [fdo#107724] -> PASS +17

  * igt@kms_rotation_crc@multiplane-rotation-cropping-top:
    - shard-kbl:          DMESG-FAIL [fdo#108950] -> PASS

  * igt@kms_setmode@basic:
    - shard-kbl:          FAIL [fdo#99912] -> PASS

  * igt@prime_busy@hang-bsd:
    - shard-apl:          FAIL -> PASS

  
#### Warnings ####

  * igt@i915_selftest@live_contexts:
    - shard-iclb:         DMESG-FAIL [fdo#108569] -> INCOMPLETE [fdo#108315]

  * igt@kms_cursor_crc@cursor-256x256-sliding:
    - shard-iclb:         DMESG-WARN [fdo#107724] / [fdo#108336] -> FAIL [fdo#103232]

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-move:
    - shard-iclb:         DMESG-FAIL [fdo#107724] -> FAIL [fdo#103167]

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

  [fdo#102250]: https://bugs.freedesktop.org/show_bug.cgi?id=102250
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103313]: https://bugs.freedesktop.org/show_bug.cgi?id=103313
  [fdo#103355]: https://bugs.freedesktop.org/show_bug.cgi?id=103355
  [fdo#103558]: https://bugs.freedesktop.org/show_bug.cgi?id=103558
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
  [fdo#104671]: https://bugs.freedesktop.org/show_bug.cgi?id=104671
  [fdo#105010]: https://bugs.freedesktop.org/show_bug.cgi?id=105010
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#105456]: https://bugs.freedesktop.org/show_bug.cgi?id=105456
  [fdo#105602]: https://bugs.freedesktop.org/show_bug.cgi?id=105602
  [fdo#105763]: https://bugs.freedesktop.org/show_bug.cgi?id=105763
  [fdo#105959]: https://bugs.freedesktop.org/show_bug.cgi?id=105959
  [fdo#106538]: https://bugs.freedesktop.org/show_bug.cgi?id=106538
  [fdo#106641]: https://bugs.freedesktop.org/show_bug.cgi?id=106641
  [fdo#106885]: https://bugs.freedesktop.org/show_bug.cgi?id=106885
  [fdo#106886]: https://bugs.freedesktop.org/show_bug.cgi?id=106886
  [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#107773]: https://bugs.freedesktop.org/show_bug.cgi?id=107773
  [fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807
  [fdo#107815]: https://bugs.freedesktop.org/show_bug.cgi?id=107815
  [fdo#107956]: https://bugs.freedesktop.org/show_bug.cgi?id=107956
  [fdo#108059]: https://bugs.freedesktop.org/show_bug.cgi?id=108059
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108315]: https://bugs.freedesktop.org/show_bug.cgi?id=108315
  [fdo#108336]: https://bugs.freedesktop.org/show_bug.cgi?id=108336
  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
  [fdo#108654]: https://bugs.freedesktop.org/show_bug.cgi?id=108654
  [fdo#108756]: https://bugs.freedesktop.org/show_bug.cgi?id=108756
  [fdo#108950]: https://bugs.freedesktop.org/show_bug.cgi?id=108950
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


Participating hosts (7 -> 7)
------------------------------

  No changes in participating hosts


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

    * Linux: CI_DRM_5345 -> Patchwork_11160

  CI_DRM_5345: 401703d974c357af84537c0d5f1e07ebeaaa99fe @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4754: a176905d46d072300ba57f29ac2b98a0228e0e2d @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_11160: 11491dfc6f63a23916eb93b4548cb83f7b702405 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

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

Re: [PATCH 2/6] drm/i915: add a helper to free the members of i915_params

[Tvrtko Ursulin]

On 27/12/2018 14:33, Jani Nikula wrote:
> Abstract the one user in anticipation of more. Set the dangling pointers
> to NULL while at it.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_gpu_error.c |  9 +--------
>   drivers/gpu/drm/i915/i915_params.c    | 16 ++++++++++++++++
>   drivers/gpu/drm/i915/i915_params.h    |  1 +
>   3 files changed, 18 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> index 8c04479c1586..2bd7991ec9af 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> @@ -963,17 +963,10 @@ static void i915_error_object_free(struct drm_i915_error_object *obj)
>   	kfree(obj);
>   }
>   
> -static __always_inline void free_param(const char *type, void *x)
> -{
> -	if (!__builtin_strcmp(type, "char *"))
> -		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
> +	i915_params_free(&error->params);
>   }
>   
>   static void cleanup_uc_state(struct i915_gpu_state *error)
> diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
> index ae3ece4ec7ab..81c73bfc7991 100644
> --- a/drivers/gpu/drm/i915/i915_params.c
> +++ b/drivers/gpu/drm/i915/i915_params.c
> @@ -217,3 +217,19 @@ void i915_params_copy(struct i915_params *dest, const struct i915_params *src)
>   	I915_PARAMS_FOR_EACH(DUP);
>   #undef DUP
>   }
> +
> +static __always_inline void free_param(const char *type, void *x)
> +{
> +	if (!__builtin_strcmp(type, "char *")) {
> +		kfree(*(void **)x);
> +		*(void **)x = NULL;
> +	}
> +}
> +
> +/* free the allocated members, *not* the passed in params itself */
> +void i915_params_free(struct i915_params *params)
> +{
> +#define FREE(T, x, ...) free_param(#T, &params->x);
> +	I915_PARAMS_FOR_EACH(FREE);
> +#undef FREE
> +}
> diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
> index fd1cf9415e60..93f665eced16 100644
> --- a/drivers/gpu/drm/i915/i915_params.h
> +++ b/drivers/gpu/drm/i915/i915_params.h
> @@ -79,6 +79,7 @@ extern struct i915_params i915_modparams __read_mostly;
>   
>   void i915_params_dump(const struct i915_params *params, struct drm_printer *p);
>   void i915_params_copy(struct i915_params *dest, const struct i915_params *src);
> +void i915_params_free(struct i915_params *params);
>   
>   #endif
>   
> 

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

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

Re: [PATCH 3/6] drm/i915/uc: add dev_priv parameter to intel_uc_is_using_* functions

[Tvrtko Ursulin]

On 27/12/2018 14:33, Jani Nikula wrote:
> Reveals the build fail fixed in the last hunk. Also prep work.
> 
> v2: name it i915 instead of dev_priv (Michal)
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_drv.h    |  6 +++---
>   drivers/gpu/drm/i915/intel_uc.c    | 12 ++++++------
>   drivers/gpu/drm/i915/intel_uc.h    |  6 +++---
>   drivers/gpu/drm/i915/intel_wopcm.c |  2 +-
>   4 files changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index d44255a8655e..c60cf17e50a0 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2490,9 +2490,9 @@ intel_info(const struct drm_i915_private *dev_priv)
>   #define HAS_HUC_UCODE(dev_priv)	(HAS_GUC(dev_priv))
>   
>   /* Having a GuC is not the same as using a GuC */
> -#define USES_GUC(dev_priv)		intel_uc_is_using_guc()
> -#define USES_GUC_SUBMISSION(dev_priv)	intel_uc_is_using_guc_submission()
> -#define USES_HUC(dev_priv)		intel_uc_is_using_huc()
> +#define USES_GUC(dev_priv)		intel_uc_is_using_guc(dev_priv)
> +#define USES_GUC_SUBMISSION(dev_priv)	intel_uc_is_using_guc_submission(dev_priv)
> +#define USES_HUC(dev_priv)		intel_uc_is_using_huc(dev_priv)
>   
>   #define HAS_POOLED_EU(dev_priv)	((dev_priv)->info.has_pooled_eu)
>   
> diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
> index 447b1de77cc7..731b82afe636 100644
> --- a/drivers/gpu/drm/i915/intel_uc.c
> +++ b/drivers/gpu/drm/i915/intel_uc.c
> @@ -71,7 +71,7 @@ static int __get_default_guc_log_level(struct drm_i915_private *i915)
>   {
>   	int guc_log_level;
>   
> -	if (!HAS_GUC(i915) || !intel_uc_is_using_guc())
> +	if (!HAS_GUC(i915) || !intel_uc_is_using_guc(i915))
>   		guc_log_level = GUC_LOG_LEVEL_DISABLED;
>   	else if (IS_ENABLED(CONFIG_DRM_I915_DEBUG) ||
>   		 IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM))
> @@ -112,11 +112,11 @@ static void sanitize_options_early(struct drm_i915_private *i915)
>   
>   	DRM_DEBUG_DRIVER("enable_guc=%d (submission:%s huc:%s)\n",
>   			 i915_modparams.enable_guc,
> -			 yesno(intel_uc_is_using_guc_submission()),
> -			 yesno(intel_uc_is_using_huc()));
> +			 yesno(intel_uc_is_using_guc_submission(i915)),
> +			 yesno(intel_uc_is_using_huc(i915)));
>   
>   	/* Verify GuC firmware availability */
> -	if (intel_uc_is_using_guc() && !intel_uc_fw_is_selected(guc_fw)) {
> +	if (intel_uc_is_using_guc(i915) && !intel_uc_fw_is_selected(guc_fw)) {
>   		DRM_WARN("Incompatible option detected: %s=%d, %s!\n",
>   			 "enable_guc", i915_modparams.enable_guc,
>   			 !HAS_GUC(i915) ? "no GuC hardware" :
> @@ -124,7 +124,7 @@ static void sanitize_options_early(struct drm_i915_private *i915)
>   	}
>   
>   	/* Verify HuC firmware availability */
> -	if (intel_uc_is_using_huc() && !intel_uc_fw_is_selected(huc_fw)) {
> +	if (intel_uc_is_using_huc(i915) && !intel_uc_fw_is_selected(huc_fw)) {
>   		DRM_WARN("Incompatible option detected: %s=%d, %s!\n",
>   			 "enable_guc", i915_modparams.enable_guc,
>   			 !HAS_HUC(i915) ? "no HuC hardware" :
> @@ -136,7 +136,7 @@ static void sanitize_options_early(struct drm_i915_private *i915)
>   		i915_modparams.guc_log_level =
>   			__get_default_guc_log_level(i915);
>   
> -	if (i915_modparams.guc_log_level > 0 && !intel_uc_is_using_guc()) {
> +	if (i915_modparams.guc_log_level > 0 && !intel_uc_is_using_guc(i915)) {
>   		DRM_WARN("Incompatible option detected: %s=%d, %s!\n",
>   			 "guc_log_level", i915_modparams.guc_log_level,
>   			 !HAS_GUC(i915) ? "no GuC hardware" :
> diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
> index 25d73ada74ae..870faf9011b9 100644
> --- a/drivers/gpu/drm/i915/intel_uc.h
> +++ b/drivers/gpu/drm/i915/intel_uc.h
> @@ -41,19 +41,19 @@ void intel_uc_fini(struct drm_i915_private *dev_priv);
>   int intel_uc_suspend(struct drm_i915_private *dev_priv);
>   int intel_uc_resume(struct drm_i915_private *dev_priv);
>   
> -static inline bool intel_uc_is_using_guc(void)
> +static inline bool intel_uc_is_using_guc(struct drm_i915_private *i915)
>   {
>   	GEM_BUG_ON(i915_modparams.enable_guc < 0);
>   	return i915_modparams.enable_guc > 0;
>   }
>   
> -static inline bool intel_uc_is_using_guc_submission(void)
> +static inline bool intel_uc_is_using_guc_submission(struct drm_i915_private *i915)
>   {
>   	GEM_BUG_ON(i915_modparams.enable_guc < 0);
>   	return i915_modparams.enable_guc & ENABLE_GUC_SUBMISSION;
>   }
>   
> -static inline bool intel_uc_is_using_huc(void)
> +static inline bool intel_uc_is_using_huc(struct drm_i915_private *i915)
>   {
>   	GEM_BUG_ON(i915_modparams.enable_guc < 0);
>   	return i915_modparams.enable_guc & ENABLE_GUC_LOAD_HUC;
> diff --git a/drivers/gpu/drm/i915/intel_wopcm.c b/drivers/gpu/drm/i915/intel_wopcm.c
> index 630c887682e8..f82a415ea2ba 100644
> --- a/drivers/gpu/drm/i915/intel_wopcm.c
> +++ b/drivers/gpu/drm/i915/intel_wopcm.c
> @@ -163,7 +163,7 @@ int intel_wopcm_init(struct intel_wopcm *wopcm)
>   	u32 guc_wopcm_rsvd;
>   	int err;
>   
> -	if (!USES_GUC(dev_priv))
> +	if (!USES_GUC(i915))

Sadly even in this short file there is no consistency. 
(intel_wopcm_init_hw uses dev_priv.)

>   		return 0;
>   
>   	GEM_BUG_ON(!wopcm->size);
> 

Anyways,

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

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

Re: [PATCH 4/6] drm/i915/params: set i915.enable_hangcheck permissions to 0600

[Tvrtko Ursulin]

On 27/12/2018 14:33, Jani Nikula wrote:
> i915.enable_hangcheck has been an outlier since its introduction in
> commit 3e0dc6b01f53 ("drm/i915: hangcheck disable parameter") with 0644
> permissions, while all the rest are either 0400 or 0600. Follow suit
> with 0600.
> 
> IGT never reads the value, so there should be no impact.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_params.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
> index 81c73bfc7991..9f0539bdaa39 100644
> --- a/drivers/gpu/drm/i915/i915_params.c
> +++ b/drivers/gpu/drm/i915/i915_params.c
> @@ -77,7 +77,7 @@ i915_param_named(error_capture, bool, 0600,
>   	"triaging and debugging hangs.");
>   #endif
>   
> -i915_param_named_unsafe(enable_hangcheck, bool, 0644,
> +i915_param_named_unsafe(enable_hangcheck, bool, 0600,
>   	"Periodically check GPU activity for detecting hangs. "
>   	"WARNING: Disabling this can cause system wide hangs. "
>   	"(default: true)");
> 

Having dug out the introducing commit, there doesn't seem to be a 
special reason for it to be 0644 indeed:

   commit 3e0dc6b01f5301d63046f6deddde2c7f5c57d67a
   Author: Ben Widawsky <ben@bwidawsk.net>
   Date:   Wed Jun 29 10:26:42 2011 -0700

       drm/i915: hangcheck disable parameter

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

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

Re: [PATCH 6/6] drm/i915/params: document I915_PARAMS_FOR_EACH()

[Tvrtko Ursulin]

On 27/12/2018 14:33, Jani Nikula wrote:
> Macros with this much magic in them deserve some explanatory text.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_params.h | 9 +++++++++
>   1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
> index 85a9007c0ed6..98eba6728095 100644
> --- a/drivers/gpu/drm/i915/i915_params.h
> +++ b/drivers/gpu/drm/i915/i915_params.h
> @@ -33,6 +33,15 @@ struct drm_printer;
>   #define ENABLE_GUC_SUBMISSION		BIT(0)
>   #define ENABLE_GUC_LOAD_HUC		BIT(1)
>   
> +/*
> + * Invoke param, a function-like macro, for each i915 param, with arguments:
> + *
> + * param(type, name, value)
> + *
> + * type: parameter type, one of {bool, int, unsigned int, char *}
> + * name: name of the parameter
> + * value: initial/default value of the parameter

Deliberately not in kerneldoc format, or kerneldoc doesn't work for 
macros, or I am missing something else?

Regards,

Tvrtko

> + */
>   #define I915_PARAMS_FOR_EACH(param) \
>   	param(char *, vbt_firmware, NULL) \
>   	param(int, modeset, -1) \
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 5/6] drm/i915: move load failure injection to selftests

[Tvrtko Ursulin]

On 27/12/2018 14:33, Jani Nikula wrote:
> Seems like selftests is a better home for everything related to load
> failure injection, including the module parameter.

Hm not sure I would immediately want to couple the two, however..

> The failure injection code gets moved from under DRM_I915_DEBUG to
> DRM_I915_SELFTEST config option. This should be of no consequence, as
> the former selects the latter.

... I also don't see why debug would auto-select self-tests.

So don't know. Personally I'd have selftests separate from debug, and 
load failure injection separate from selftests. But I don't feel that 
strongly about it. At least I agree that losing failure injection from 
the error state is not an issue. (Since all of them are strictly during 
driver load phase.)

Regards,

Tvrtko

> 
> With the parameter no longer part of i915_params, its value will not be
> recorded in error capture or debugfs param dump. Note that the value
> would have been zeroed anyway if a selftest had been hit, so there
> should not be meaningful information loss here, especially with all the
> logging around failure injection.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_drv.c                | 25 -------------------------
>   drivers/gpu/drm/i915/i915_drv.h                | 15 ---------------
>   drivers/gpu/drm/i915/i915_params.c             |  5 -----
>   drivers/gpu/drm/i915/i915_params.h             |  1 -
>   drivers/gpu/drm/i915/i915_selftest.h           | 10 ++++++++++
>   drivers/gpu/drm/i915/selftests/i915_selftest.c | 26 ++++++++++++++++++++++++++
>   6 files changed, 36 insertions(+), 46 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index caa055ac9472..559a1b11f3e4 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -57,31 +57,6 @@
>   
>   static struct drm_driver driver;
>   
> -#if IS_ENABLED(CONFIG_DRM_I915_DEBUG)
> -static unsigned int i915_load_fail_count;
> -
> -bool __i915_inject_load_failure(const char *func, int line)
> -{
> -	if (i915_load_fail_count >= i915_modparams.inject_load_failure)
> -		return false;
> -
> -	if (++i915_load_fail_count == i915_modparams.inject_load_failure) {
> -		DRM_INFO("Injecting failure at checkpoint %u [%s:%d]\n",
> -			 i915_modparams.inject_load_failure, func, line);
> -		i915_modparams.inject_load_failure = 0;
> -		return true;
> -	}
> -
> -	return false;
> -}
> -
> -bool i915_error_injected(void)
> -{
> -	return i915_load_fail_count && !i915_modparams.inject_load_failure;
> -}
> -
> -#endif
> -
>   #define FDO_BUG_URL "https://bugs.freedesktop.org/enter_bug.cgi?product=DRI"
>   #define FDO_BUG_MSG "Please file a bug at " FDO_BUG_URL " against DRM/Intel " \
>   		    "providing the dmesg log by booting with drm.debug=0xf"
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index c60cf17e50a0..1f29f3933625 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -111,21 +111,6 @@
>   #define I915_STATE_WARN_ON(x)						\
>   	I915_STATE_WARN((x), "%s", "WARN_ON(" __stringify(x) ")")
>   
> -#if IS_ENABLED(CONFIG_DRM_I915_DEBUG)
> -
> -bool __i915_inject_load_failure(const char *func, int line);
> -#define i915_inject_load_failure() \
> -	__i915_inject_load_failure(__func__, __LINE__)
> -
> -bool i915_error_injected(void);
> -
> -#else
> -
> -#define i915_inject_load_failure() false
> -#define i915_error_injected() false
> -
> -#endif
> -
>   #define i915_load_error(i915, fmt, ...)					 \
>   	__i915_printk(i915, i915_error_injected() ? KERN_DEBUG : KERN_ERR, \
>   		      fmt, ##__VA_ARGS__)
> diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
> index 9f0539bdaa39..2853b54570eb 100644
> --- a/drivers/gpu/drm/i915/i915_params.c
> +++ b/drivers/gpu/drm/i915/i915_params.c
> @@ -159,11 +159,6 @@ i915_param_named_unsafe(dmc_firmware_path, charp, 0400,
>   i915_param_named_unsafe(enable_dp_mst, bool, 0600,
>   	"Enable multi-stream transport (MST) for new DisplayPort sinks. (default: true)");
>   
> -#if IS_ENABLED(CONFIG_DRM_I915_DEBUG)
> -i915_param_named_unsafe(inject_load_failure, uint, 0400,
> -	"Force an error after a number of failure check points (0:disabled (default), N:force failure at the Nth failure check point)");
> -#endif
> -
>   i915_param_named(enable_dpcd_backlight, bool, 0600,
>   	"Enable support for DPCD backlight control (default:false)");
>   
> diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
> index 93f665eced16..85a9007c0ed6 100644
> --- a/drivers/gpu/drm/i915/i915_params.h
> +++ b/drivers/gpu/drm/i915/i915_params.h
> @@ -53,7 +53,6 @@ struct drm_printer;
>   	param(int, mmio_debug, 0) \
>   	param(int, edp_vswing, 0) \
>   	param(int, reset, 2) \
> -	param(unsigned int, inject_load_failure, 0) \
>   	/* leave bools at the end to not create holes */ \
>   	param(bool, alpha_support, IS_ENABLED(CONFIG_DRM_I915_ALPHA_SUPPORT)) \
>   	param(bool, enable_hangcheck, true) \
> diff --git a/drivers/gpu/drm/i915/i915_selftest.h b/drivers/gpu/drm/i915/i915_selftest.h
> index a73472dd12fd..1266cef8bf17 100644
> --- a/drivers/gpu/drm/i915/i915_selftest.h
> +++ b/drivers/gpu/drm/i915/i915_selftest.h
> @@ -33,6 +33,7 @@ struct i915_selftest {
>   	unsigned int random_seed;
>   	int mock;
>   	int live;
> +	unsigned int inject_load_failure;
>   };
>   
>   #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
> @@ -77,6 +78,12 @@ int __i915_subtests(const char *caller,
>   #define I915_SELFTEST_DECLARE(x) x
>   #define I915_SELFTEST_ONLY(x) unlikely(x)
>   
> +bool __i915_inject_load_failure(const char *func, int line);
> +#define i915_inject_load_failure() \
> +	__i915_inject_load_failure(__func__, __LINE__)
> +
> +bool i915_error_injected(void);
> +
>   #else /* !IS_ENABLED(CONFIG_DRM_I915_SELFTEST) */
>   
>   static inline int i915_mock_selftests(void) { return 0; }
> @@ -85,6 +92,9 @@ static inline int i915_live_selftests(struct pci_dev *pdev) { return 0; }
>   #define I915_SELFTEST_DECLARE(x)
>   #define I915_SELFTEST_ONLY(x) 0
>   
> +static inline int i915_inject_load_failure(void) { return false; }
> +static inline int i915_error_injected(void) { return false; }
> +
>   #endif
>   
>   /* Using the i915_selftest_ prefix becomes a little unwieldy with the helpers.
> diff --git a/drivers/gpu/drm/i915/selftests/i915_selftest.c b/drivers/gpu/drm/i915/selftests/i915_selftest.c
> index 86c54ea37f48..9e556fc4707d 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_selftest.c
> +++ b/drivers/gpu/drm/i915/selftests/i915_selftest.c
> @@ -250,3 +250,29 @@ MODULE_PARM_DESC(mock_selftests, "Run selftests before loading, using mock hardw
>   
>   module_param_named_unsafe(live_selftests, i915_selftest.live, int, 0400);
>   MODULE_PARM_DESC(live_selftests, "Run selftests after driver initialisation on the live system (0:disabled [default], 1:run tests then continue, -1:run tests then exit module)");
> +
> +module_param_named_unsafe(inject_load_failure, i915_selftest.inject_load_failure, uint, 0400);
> +MODULE_PARM_DESC(inject_load_failure,
> +		 "Force an error after a number of failure check points (0:disabled (default), N:force failure at the Nth failure check point)");
> +
> +static unsigned int i915_load_fail_count;
> +
> +bool __i915_inject_load_failure(const char *func, int line)
> +{
> +	if (i915_load_fail_count >= i915_selftest.inject_load_failure)
> +		return false;
> +
> +	if (++i915_load_fail_count == i915_selftest.inject_load_failure) {
> +		DRM_INFO("Injecting failure at checkpoint %u [%s:%d]\n",
> +			 i915_selftest.inject_load_failure, func, line);
> +		i915_selftest.inject_load_failure = 0;
> +		return true;
> +	}
> +
> +	return false;
> +}
> +
> +bool i915_error_injected(void)
> +{
> +	return i915_load_fail_count && !i915_selftest.inject_load_failure;
> +}
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 5/6] drm/i915: move load failure injection to selftests

[Chris Wilson]

Quoting Tvrtko Ursulin (2018-12-31 13:13:26)
> 
> On 27/12/2018 14:33, Jani Nikula wrote:
> > Seems like selftests is a better home for everything related to load
> > failure injection, including the module parameter.
> 
> Hm not sure I would immediately want to couple the two, however..
> 
> > The failure injection code gets moved from under DRM_I915_DEBUG to
> > DRM_I915_SELFTEST config option. This should be of no consequence, as
> > the former selects the latter.
> 
> ... I also don't see why debug would auto-select self-tests.

DRM_I915_DEBUG is now the list of things we use for igt
s/DRM_I915_DEBUG/DRM_I915_IGT/

Just so we can have a single switch for "if you want to run igt, enable
this CONFIG to enable all debug features used by igt". Handy for CI
testing when more features are required.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 6/6] drm/i915/params: document I915_PARAMS_FOR_EACH()

[Jani Nikula]

On Mon, 31 Dec 2018, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote:
> On 27/12/2018 14:33, Jani Nikula wrote:
>> Macros with this much magic in them deserve some explanatory text.
>> 
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_params.h | 9 +++++++++
>>   1 file changed, 9 insertions(+)
>> 
>> diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
>> index 85a9007c0ed6..98eba6728095 100644
>> --- a/drivers/gpu/drm/i915/i915_params.h
>> +++ b/drivers/gpu/drm/i915/i915_params.h
>> @@ -33,6 +33,15 @@ struct drm_printer;
>>   #define ENABLE_GUC_SUBMISSION		BIT(0)
>>   #define ENABLE_GUC_LOAD_HUC		BIT(1)
>>   
>> +/*
>> + * Invoke param, a function-like macro, for each i915 param, with arguments:
>> + *
>> + * param(type, name, value)
>> + *
>> + * type: parameter type, one of {bool, int, unsigned int, char *}
>> + * name: name of the parameter
>> + * value: initial/default value of the parameter
>
> Deliberately not in kerneldoc format, or kerneldoc doesn't work for 
> macros, or I am missing something else?

The comment describes the parameters passed to the macro passed in as
param, so it's too many levels of abstraction for poor kernel-doc. ;)

BR,
Jani.

>
> Regards,
>
> Tvrtko
>
>> + */
>>   #define I915_PARAMS_FOR_EACH(param) \
>>   	param(char *, vbt_firmware, NULL) \
>>   	param(int, modeset, -1) \
>> 

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/6] drm/i915/params: set i915.enable_hangcheck permissions to 0600

[Jani Nikula]

On Mon, 31 Dec 2018, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote:
> On 27/12/2018 14:33, Jani Nikula wrote:
>> i915.enable_hangcheck has been an outlier since its introduction in
>> commit 3e0dc6b01f53 ("drm/i915: hangcheck disable parameter") with 0644
>> permissions, while all the rest are either 0400 or 0600. Follow suit
>> with 0600.
>> 
>> IGT never reads the value, so there should be no impact.
>> 
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_params.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
>> index 81c73bfc7991..9f0539bdaa39 100644
>> --- a/drivers/gpu/drm/i915/i915_params.c
>> +++ b/drivers/gpu/drm/i915/i915_params.c
>> @@ -77,7 +77,7 @@ i915_param_named(error_capture, bool, 0600,
>>   	"triaging and debugging hangs.");
>>   #endif
>>   
>> -i915_param_named_unsafe(enable_hangcheck, bool, 0644,
>> +i915_param_named_unsafe(enable_hangcheck, bool, 0600,
>>   	"Periodically check GPU activity for detecting hangs. "
>>   	"WARNING: Disabling this can cause system wide hangs. "
>>   	"(default: true)");
>> 
>
> Having dug out the introducing commit, there doesn't seem to be a 
> special reason for it to be 0644 indeed:
>
>    commit 3e0dc6b01f5301d63046f6deddde2c7f5c57d67a
>    Author: Ben Widawsky <ben@bwidawsk.net>
>    Date:   Wed Jun 29 10:26:42 2011 -0700
>
>        drm/i915: hangcheck disable parameter
>
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Thanks for the reviews, pushed patches 1-4 in the series.

BR,
Jani.


>
> Regards,
>
> Tvrtko

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 6/6] drm/i915/params: document I915_PARAMS_FOR_EACH()

[Tvrtko Ursulin]

On 31/12/2018 13:24, Jani Nikula wrote:
> On Mon, 31 Dec 2018, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote:
>> On 27/12/2018 14:33, Jani Nikula wrote:
>>> Macros with this much magic in them deserve some explanatory text.
>>>
>>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>>> ---
>>>    drivers/gpu/drm/i915/i915_params.h | 9 +++++++++
>>>    1 file changed, 9 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
>>> index 85a9007c0ed6..98eba6728095 100644
>>> --- a/drivers/gpu/drm/i915/i915_params.h
>>> +++ b/drivers/gpu/drm/i915/i915_params.h
>>> @@ -33,6 +33,15 @@ struct drm_printer;
>>>    #define ENABLE_GUC_SUBMISSION		BIT(0)
>>>    #define ENABLE_GUC_LOAD_HUC		BIT(1)
>>>    
>>> +/*
>>> + * Invoke param, a function-like macro, for each i915 param, with arguments:
>>> + *
>>> + * param(type, name, value)
>>> + *
>>> + * type: parameter type, one of {bool, int, unsigned int, char *}
>>> + * name: name of the parameter
>>> + * value: initial/default value of the parameter
>>
>> Deliberately not in kerneldoc format, or kerneldoc doesn't work for
>> macros, or I am missing something else?
> 
> The comment describes the parameters passed to the macro passed in as
> param, so it's too many levels of abstraction for poor kernel-doc. ;)

Ha! :)

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko

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

Re: [PATCH 5/6] drm/i915: move load failure injection to selftests

[Jani Nikula]

On Mon, 31 Dec 2018, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> Quoting Tvrtko Ursulin (2018-12-31 13:13:26)
>> 
>> On 27/12/2018 14:33, Jani Nikula wrote:
>> > Seems like selftests is a better home for everything related to load
>> > failure injection, including the module parameter.
>> 
>> Hm not sure I would immediately want to couple the two, however..
>> 
>> > The failure injection code gets moved from under DRM_I915_DEBUG to
>> > DRM_I915_SELFTEST config option. This should be of no consequence, as
>> > the former selects the latter.
>> 
>> ... I also don't see why debug would auto-select self-tests.
>
> DRM_I915_DEBUG is now the list of things we use for igt
> s/DRM_I915_DEBUG/DRM_I915_IGT/
>
> Just so we can have a single switch for "if you want to run igt, enable
> this CONFIG to enable all debug features used by igt". Handy for CI
> testing when more features are required.

So what say you on the patch? I can keep it in i915_params.c too, it
just needs some extra juggling because it gets used before i915 gets
allocated. So not very easy as a "device param".

BR,
Jani.

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 5/6] drm/i915: move load failure injection to selftests

[Chris Wilson]

Quoting Jani Nikula (2018-12-31 15:12:41)
> On Mon, 31 Dec 2018, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > Quoting Tvrtko Ursulin (2018-12-31 13:13:26)
> >> 
> >> On 27/12/2018 14:33, Jani Nikula wrote:
> >> > Seems like selftests is a better home for everything related to load
> >> > failure injection, including the module parameter.
> >> 
> >> Hm not sure I would immediately want to couple the two, however..
> >> 
> >> > The failure injection code gets moved from under DRM_I915_DEBUG to
> >> > DRM_I915_SELFTEST config option. This should be of no consequence, as
> >> > the former selects the latter.
> >> 
> >> ... I also don't see why debug would auto-select self-tests.
> >
> > DRM_I915_DEBUG is now the list of things we use for igt
> > s/DRM_I915_DEBUG/DRM_I915_IGT/
> >
> > Just so we can have a single switch for "if you want to run igt, enable
> > this CONFIG to enable all debug features used by igt". Handy for CI
> > testing when more features are required.
> 
> So what say you on the patch? I can keep it in i915_params.c too, it
> just needs some extra juggling because it gets used before i915 gets
> allocated. So not very easy as a "device param".

I had no objections to the patch. When you raised the idea, I was
thinking of doing the entire iterative fault injection inside a selftest
and remove the kernel parameter entirely :)
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 6/6] drm/i915/params: document I915_PARAMS_FOR_EACH()

[Jani Nikula]

On Mon, 31 Dec 2018, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote:
> On 31/12/2018 13:24, Jani Nikula wrote:
>> On Mon, 31 Dec 2018, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote:
>>> On 27/12/2018 14:33, Jani Nikula wrote:
>>>> Macros with this much magic in them deserve some explanatory text.
>>>>
>>>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>>>> ---
>>>>    drivers/gpu/drm/i915/i915_params.h | 9 +++++++++
>>>>    1 file changed, 9 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
>>>> index 85a9007c0ed6..98eba6728095 100644
>>>> --- a/drivers/gpu/drm/i915/i915_params.h
>>>> +++ b/drivers/gpu/drm/i915/i915_params.h
>>>> @@ -33,6 +33,15 @@ struct drm_printer;
>>>>    #define ENABLE_GUC_SUBMISSION		BIT(0)
>>>>    #define ENABLE_GUC_LOAD_HUC		BIT(1)
>>>>    
>>>> +/*
>>>> + * Invoke param, a function-like macro, for each i915 param, with arguments:
>>>> + *
>>>> + * param(type, name, value)
>>>> + *
>>>> + * type: parameter type, one of {bool, int, unsigned int, char *}
>>>> + * name: name of the parameter
>>>> + * value: initial/default value of the parameter
>>>
>>> Deliberately not in kerneldoc format, or kerneldoc doesn't work for
>>> macros, or I am missing something else?
>> 
>> The comment describes the parameters passed to the macro passed in as
>> param, so it's too many levels of abstraction for poor kernel-doc. ;)
>
> Ha! :)
>
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Thanks, and pushed.

BR,
Jani.

>
> Regards,
>
> Tvrtko
>

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx