[RFC][PATCH 0/6] Optionally display recurring warning messages

[RFC][PATCH 0/6] Optionally display recurring warning messages

[Joonas Lahtinen]

Wakeref series introduced multiple WARN_ONCE clauses which might be
triggered from several sources (hangcheck or cache flusing for example),
this series adds an i915.debug option to make them always WARN which is
useful in the CI.

Had to reorder i915_params to get rid of circular include dependency.

Compile tested, comments welcome.

Joonas Lahtinen (6):
  drm/i915: Compile time concatenate WARN_ON macro strings
  drm/i915: Decouple struct i915_params i915 into i915_params.h
  drm/i915: Reorder i915_params struct.
  drm/i915: Add i915.debug parameter
  drm/i915: Add helpers to reduce (repetitive) noise
  drm/i915: Use I915_WARN_RECUR for recurring warning messages

 drivers/gpu/drm/i915/i915_drv.h    | 75 +++++++++++++++++---------------------
 drivers/gpu/drm/i915/i915_params.c |  7 ++++
 drivers/gpu/drm/i915/i915_params.h | 69 +++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_drv.h   |  6 +--
 4 files changed, 112 insertions(+), 45 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/i915_params.h

-- 
2.4.3

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

[PATCH 1/6] drm/i915: Compile time concatenate WARN_ON macro strings

[Joonas Lahtinen]

Using __stringify(x) instead of #x adds support for macros as
a parameter and reduces runtime overhead.

Slightly increases the .text size but should not matter.

Cc: Rob Clark <robdclark@gmail.com>
Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 1d28d90..fe3e76d 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -69,11 +69,11 @@
 		BUILD_BUG_ON(__i915_warn_cond); \
 	WARN(__i915_warn_cond, "WARN_ON(" #x ")"); })
 #else
-#define WARN_ON(x) WARN((x), "WARN_ON(%s)", #x )
+#define WARN_ON(x) WARN((x), "WARN_ON(" __stringify(x) ")")
 #endif
 
 #undef WARN_ON_ONCE
-#define WARN_ON_ONCE(x) WARN_ONCE((x), "WARN_ON_ONCE(%s)", #x )
+#define WARN_ON_ONCE(x) WARN_ONCE((x), "WARN_ON_ONCE(" __stringify(x) ")")
 
 #define MISSING_CASE(x) WARN(1, "Missing switch case (%lu) in %s\n", \
 			     (long) (x), __func__);
@@ -97,12 +97,14 @@
 })
 
 #define I915_STATE_WARN_ON(condition) ({				\
+	static const char __warn_on_txt[] =				\
+		"WARN_ON(" __stringify(condition) ")\n";		\
 	int __ret_warn_on = !!(condition);				\
 	if (unlikely(__ret_warn_on)) {					\
 		if (i915.verbose_state_checks)				\
-			WARN(1, "WARN_ON(" #condition ")\n");		\
+			WARN(1, __warn_on_txt);				\
 		else 							\
-			DRM_ERROR("WARN_ON(" #condition ")\n");		\
+			DRM_ERROR(__warn_on_txt);			\
 	}								\
 	unlikely(__ret_warn_on);					\
 })
-- 
2.4.3

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

[PATCH 2/6] drm/i915: Decouple struct i915_params i915 into i915_params.h

[Joonas Lahtinen]

Otherwise usage in the i915 debug macros yields problems due to
i915_drv.h <-> i915_trace.h <-> intel_drv.h include loops.

Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h    | 40 ++--------------------
 drivers/gpu/drm/i915/i915_params.c |  1 +
 drivers/gpu/drm/i915/i915_params.h | 68 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 71 insertions(+), 38 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/i915_params.h

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index fe3e76d..ae4b433 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -34,6 +34,7 @@
 #include <uapi/drm/drm_fourcc.h>
 
 #include <drm/drmP.h>
+#include "i915_params.h"
 #include "i915_reg.h"
 #include "intel_bios.h"
 #include "intel_ringbuffer.h"
@@ -2658,44 +2659,7 @@ extern int i915_max_ioctl;
 extern int i915_suspend_switcheroo(struct drm_device *dev, pm_message_t state);
 extern int i915_resume_switcheroo(struct drm_device *dev);
 
-/* i915_params.c */
-struct i915_params {
-	int modeset;
-	int panel_ignore_lid;
-	int semaphores;
-	int lvds_channel_mode;
-	int panel_use_ssc;
-	int vbt_sdvo_panel_type;
-	int enable_rc6;
-	int enable_dc;
-	int enable_fbc;
-	int enable_ppgtt;
-	int enable_execlists;
-	int enable_psr;
-	unsigned int preliminary_hw_support;
-	int disable_power_well;
-	int enable_ips;
-	int invert_brightness;
-	int enable_cmd_parser;
-	/* leave bools at the end to not create holes */
-	bool enable_hangcheck;
-	bool fastboot;
-	bool prefault_disable;
-	bool load_detect_test;
-	bool reset;
-	bool disable_display;
-	bool disable_vtd_wa;
-	bool enable_guc_submission;
-	int guc_log_level;
-	int use_mmio_flip;
-	int mmio_debug;
-	bool verbose_state_checks;
-	bool nuclear_pageflip;
-	int edp_vswing;
-};
-extern struct i915_params i915 __read_mostly;
-
-				/* i915_dma.c */
+/* i915_dma.c */
 extern int i915_driver_load(struct drm_device *, unsigned long flags);
 extern int i915_driver_unload(struct drm_device *);
 extern int i915_driver_open(struct drm_device *dev, struct drm_file *file);
diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index 835d609..8d90c25 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -22,6 +22,7 @@
  * IN THE SOFTWARE.
  */
 
+#include "i915_params.h"
 #include "i915_drv.h"
 
 struct i915_params i915 __read_mostly = {
diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
new file mode 100644
index 0000000..a341525
--- /dev/null
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -0,0 +1,68 @@
+/*
+ * Copyright © 2015 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+
+#ifndef _I915_PARAMS_H_
+#define _I915_PARAMS_H_
+
+#include <linux/cache.h>
+
+struct i915_params {
+	int modeset;
+	int panel_ignore_lid;
+	int semaphores;
+	int lvds_channel_mode;
+	int panel_use_ssc;
+	int vbt_sdvo_panel_type;
+	int enable_rc6;
+	int enable_dc;
+	int enable_fbc;
+	int enable_ppgtt;
+	int enable_execlists;
+	int enable_psr;
+	unsigned int preliminary_hw_support;
+	int disable_power_well;
+	int enable_ips;
+	int invert_brightness;
+	int enable_cmd_parser;
+	/* leave bools at the end to not create holes */
+	bool enable_hangcheck;
+	bool fastboot;
+	bool prefault_disable;
+	bool load_detect_test;
+	bool reset;
+	bool disable_display;
+	bool disable_vtd_wa;
+	bool enable_guc_submission;
+	int guc_log_level;
+	int use_mmio_flip;
+	int mmio_debug;
+	bool verbose_state_checks;
+	bool nuclear_pageflip;
+	int edp_vswing;
+};
+
+extern struct i915_params i915 __read_mostly;
+
+#endif
+
-- 
2.4.3

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

[PATCH 3/6] drm/i915: Reorder i915_params struct.

[Joonas Lahtinen]

Move all the bool variables to the end as per the comment.

Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_params.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
index a341525..a748ebc 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -45,6 +45,10 @@ struct i915_params {
 	int enable_ips;
 	int invert_brightness;
 	int enable_cmd_parser;
+	int guc_log_level;
+	int use_mmio_flip;
+	int mmio_debug;
+	int edp_vswing;
 	/* leave bools at the end to not create holes */
 	bool enable_hangcheck;
 	bool fastboot;
@@ -54,12 +58,8 @@ struct i915_params {
 	bool disable_display;
 	bool disable_vtd_wa;
 	bool enable_guc_submission;
-	int guc_log_level;
-	int use_mmio_flip;
-	int mmio_debug;
 	bool verbose_state_checks;
 	bool nuclear_pageflip;
-	int edp_vswing;
 };
 
 extern struct i915_params i915 __read_mostly;
-- 
2.4.3

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

[PATCH 4/6] drm/i915: Add i915.debug parameter

[Joonas Lahtinen]

Add i915.debug parameter to control output in cases where the debug output
amount will be massive and as such is unsuitable for everyday use, but still
significant in value when solving bugs.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h    | 1 -
 drivers/gpu/drm/i915/i915_params.c | 6 ++++++
 drivers/gpu/drm/i915/i915_params.h | 1 +
 3 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index ae4b433..37fce5a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2659,7 +2659,6 @@ extern int i915_max_ioctl;
 extern int i915_suspend_switcheroo(struct drm_device *dev, pm_message_t state);
 extern int i915_resume_switcheroo(struct drm_device *dev);
 
-/* i915_dma.c */
 extern int i915_driver_load(struct drm_device *, unsigned long flags);
 extern int i915_driver_unload(struct drm_device *);
 extern int i915_driver_open(struct drm_device *dev, struct drm_file *file);
diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index 8d90c25..e76af8d 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -57,6 +57,7 @@ struct i915_params i915 __read_mostly = {
 	.edp_vswing = 0,
 	.enable_guc_submission = false,
 	.guc_log_level = -1,
+	.debug = false,
 };
 
 module_param_named(modeset, i915.modeset, int, 0400);
@@ -203,3 +204,8 @@ MODULE_PARM_DESC(enable_guc_submission, "Enable GuC submission (default:false)")
 module_param_named(guc_log_level, i915.guc_log_level, int, 0400);
 MODULE_PARM_DESC(guc_log_level,
 	"GuC firmware logging level (-1:disabled (default), 0-3:enabled)");
+
+module_param_named(debug, i915.debug, bool, 0600);
+MODULE_PARM_DESC(debug,
+	"Enable verbose debug output unsuitable for everyday use (default: false). "
+	"For developers only.");
diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
index a748ebc..9a7c540 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -60,6 +60,7 @@ struct i915_params {
 	bool enable_guc_submission;
 	bool verbose_state_checks;
 	bool nuclear_pageflip;
+	bool debug;
 };
 
 extern struct i915_params i915 __read_mostly;
-- 
2.4.3

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

[PATCH 5/6] drm/i915: Add helpers to reduce (repetitive) noise

[Joonas Lahtinen]

Add helpers to reduce the amount of noise. Use the i915.debug parameter
introduced in the previous patch to decide on whether to display all
debug messages or just ones that are meaningful to end user.

Take for example the CI environment, we want to see all possible warning
messages even though the system continues to operate. Opposite to that is
environment in daily use, repeating errors should be displayed once to
indicate the need for a bug report, but if the machine continues to work,
we should not spam the user continuously.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 37fce5a..07240af 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -110,6 +110,32 @@
 	unlikely(__ret_warn_on);					\
 })
 
+#define I915_DEBUG(condition, format...) ({				\
+	int __ret_debug_on = !!(condition);				\
+	if (unlikely(__ret_debug_on))					\
+		WARN(unlikely(i915.debug), format);			\
+	unlikely(__ret_debug_on);					\
+})
+
+#define I915_DEBUG_ON(condition) ({					\
+	static const char __debug_on_txt[] =				\
+		"I915_DEBUG_ON(" __stringify(condition) ")\n";		\
+	int __ret_debug_on = !!(condition);				\
+	if (unlikely( __ret_debug_on)) 					\
+		WARN(unlikely(i915.debug), __debug_on_txt);		\
+	unlikely(__ret_debug_on);					\
+})
+
+#define I915_WARN_RECUR(condition, format...) ({			\
+	static bool __section(.data.unlikely) __warned;			\
+	int __ret_warn_on = !!(condition);				\
+	if (unlikely(__ret_warn_on)) {					\
+		if (WARN(unlikely(!__warned || i915.debug), format))	\
+			__warned = true;				\
+	}								\
+	unlikely(__ret_debug_on);					\
+})
+
 static inline const char *yesno(bool v)
 {
 	return v ? "yes" : "no";
-- 
2.4.3

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

[PATCH 6/6] drm/i915: Use I915_WARN_RECUR for recurring warning messages

[Joonas Lahtinen]

Avoid flooding end users kernel log with warnings and mark recurring
warnings as such. This is a case when multiple code paths trigger a
warning and only displaying it once masks multiple errors in CI.

Cc: Imre Deak <imre.deak@intel.com>
Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h  | 2 +-
 drivers/gpu/drm/i915/intel_drv.h | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 07240af..20306e3 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -133,7 +133,7 @@
 		if (WARN(unlikely(!__warned || i915.debug), format))	\
 			__warned = true;				\
 	}								\
-	unlikely(__ret_debug_on);					\
+	unlikely(__ret_warn_on);					\
 })
 
 static inline const char *yesno(bool v)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d523ebb..c9e9f42 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1434,7 +1434,7 @@ void intel_display_power_put(struct drm_i915_private *dev_priv,
 static inline void
 assert_rpm_device_not_suspended(struct drm_i915_private *dev_priv)
 {
-	WARN_ONCE(dev_priv->pm.suspended,
+	I915_WARN_RECUR(dev_priv->pm.suspended,
 		  "Device suspended during HW access\n");
 }
 
@@ -1442,7 +1442,7 @@ static inline void
 assert_rpm_wakelock_held(struct drm_i915_private *dev_priv)
 {
 	assert_rpm_device_not_suspended(dev_priv);
-	WARN_ONCE(!atomic_read(&dev_priv->pm.wakeref_count),
+	I915_WARN_RECUR(!atomic_read(&dev_priv->pm.wakeref_count),
 		  "RPM wakelock ref not held during HW access");
 }
 
@@ -1459,7 +1459,7 @@ assert_rpm_atomic_begin(struct drm_i915_private *dev_priv)
 static inline void
 assert_rpm_atomic_end(struct drm_i915_private *dev_priv, int begin_seq)
 {
-	WARN_ONCE(atomic_read(&dev_priv->pm.atomic_seq) != begin_seq,
+	I915_WARN_RECUR(atomic_read(&dev_priv->pm.atomic_seq) != begin_seq,
 		  "HW access outside of RPM atomic section\n");
 }
 
-- 
2.4.3

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

Re: [PATCH 2/6] drm/i915: Decouple struct i915_params i915 into i915_params.h

[Chris Wilson]

On Thu, Dec 17, 2015 at 09:25:47PM +0200, Joonas Lahtinen wrote:
> +#ifndef _I915_PARAMS_H_
> +#define _I915_PARAMS_H_
> +
Is it worth saying

#include <linux/cache.h> /* for __read_mostly */

or am I alone in not knowing that?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/6] drm/i915: Reorder i915_params struct.

[Chris Wilson]

On Thu, Dec 17, 2015 at 09:25:48PM +0200, Joonas Lahtinen wrote:
> Move all the bool variables to the end as per the comment.
> 
> Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/6] drm/i915: Add i915.debug parameter

[Chris Wilson]

On Thu, Dec 17, 2015 at 09:25:49PM +0200, Joonas Lahtinen wrote:
> Add i915.debug parameter to control output in cases where the debug output
> amount will be massive and as such is unsuitable for everyday use, but still
> significant in value when solving bugs.
> 
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Maybe i915.verbose_checks ?

Too similar to i915.verbose_state_checks though?

Just trying to differentiate from drm.debug which we will also be
settings.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 5/6] drm/i915: Add helpers to reduce (repetitive) noise

[Chris Wilson]

On Thu, Dec 17, 2015 at 09:25:50PM +0200, Joonas Lahtinen wrote:
> Add helpers to reduce the amount of noise. Use the i915.debug parameter
> introduced in the previous patch to decide on whether to display all
> debug messages or just ones that are meaningful to end user.
> 
> Take for example the CI environment, we want to see all possible warning
> messages even though the system continues to operate. Opposite to that is
> environment in daily use, repeating errors should be displayed once to
> indicate the need for a bug report, but if the machine continues to work,
> we should not spam the user continuously.

One thing, if we have a helper that we think will be generally useful
(outside of our module), we have taken the liberty in the past in giving
it a non-specific name, e.g. WARN_RECUR();

That would mean we would use WARN_RECUR(cond, i915.debug, "foo has barred\n"))
instead.

The theory being is that if it is useful we can then lift it to the
core.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 5/6] drm/i915: Add helpers to reduce (repetitive) noise

[Joonas Lahtinen]

On to, 2015-12-17 at 21:10 +0000, Chris Wilson wrote:
> On Thu, Dec 17, 2015 at 09:25:50PM +0200, Joonas Lahtinen wrote:
> > Add helpers to reduce the amount of noise. Use the i915.debug
> > parameter
> > introduced in the previous patch to decide on whether to display
> > all
> > debug messages or just ones that are meaningful to end user.
> > 
> > Take for example the CI environment, we want to see all possible
> > warning
> > messages even though the system continues to operate. Opposite to
> > that is
> > environment in daily use, repeating errors should be displayed once
> > to
> > indicate the need for a bug report, but if the machine continues to
> > work,
> > we should not spam the user continuously.
> 
> One thing, if we have a helper that we think will be generally useful
> (outside of our module), we have taken the liberty in the past in
> giving
> it a non-specific name, e.g. WARN_RECUR();
> 
> That would mean we would use WARN_RECUR(cond, i915.debug, "foo has
> barred\n"))
> instead.
> 
> The theory being is that if it is useful we can then lift it to the
> core.

This makes sense.

Regards, Joonas

> -Chris
> 
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation

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

✗ warning: Fi.CI.BAT

[Patchwork]

== Summary ==

Built on ac2305b6c91b9a84cc12566016ece257c3ebcba3 drm-intel-nightly: 2015y-12m-17d-16h-19m-23s UTC integration manifest

Test igt@kms_flip@basic-flip-vs-wf_vblank on bsw-nuc-2 pass -> dmesg-warn
Test igt@kms_flip@basic-flip-vs-wf_vblank on skl-i7k-2 dmesg-fail -> dmesg-warn
Test igt@kms_pipe_crc_basic@read-crc-pipe-c-frame-sequence on bsw-nuc-2 dmesg-warn -> pass
Test igt@kms_flip@basic-flip-vs-dpms on bsw-nuc-2 pass -> skip
Test igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b on snb-x220t dmesg-warn -> pass
Test igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b on bdw-nuci7 pass -> dmesg-warn
Test igt@kms_flip@basic-plain-flip on bdw-nuci7 dmesg-warn -> pass
Test igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a on snb-x220t pass -> dmesg-warn
Test igt@kms_pipe_crc_basic@read-crc-pipe-c on skl-i7k-2 dmesg-warn -> pass
Test igt@kms_pipe_crc_basic@read-crc-pipe-b on snb-dellxps dmesg-warn -> pass
Test igt@kms_setmode@basic-clone-single-crtc on snb-dellxps pass -> dmesg-warn
Test igt@kms_flip@basic-flip-vs-modeset on bsw-nuc-2 dmesg-warn -> pass
Test igt@kms_flip@basic-flip-vs-modeset on skl-i5k-2 dmesg-warn -> pass
Test igt@kms_pipe_crc_basic@read-crc-pipe-a-frame-sequence on byt-nuc dmesg-warn -> pass

bdw-nuci7        total:135  pass:124  dwarn:2   dfail:0   fail:0   skip:9  
bsw-nuc-2        total:135  pass:113  dwarn:1   dfail:0   fail:0   skip:21 
byt-nuc          total:135  pass:120  dwarn:2   dfail:0   fail:0   skip:13 
hsw-brixbox      total:135  pass:127  dwarn:1   dfail:0   fail:0   skip:7  
hsw-gt2          total:135  pass:130  dwarn:1   dfail:0   fail:0   skip:4  
ilk-hp8440p      total:135  pass:100  dwarn:0   dfail:0   fail:2   skip:33 
ivb-t430s        total:135  pass:128  dwarn:1   dfail:1   fail:1   skip:4  
skl-i5k-2        total:135  pass:123  dwarn:4   dfail:0   fail:0   skip:8  
skl-i7k-2        total:135  pass:123  dwarn:4   dfail:0   fail:0   skip:8  
snb-dellxps      total:135  pass:121  dwarn:2   dfail:0   fail:0   skip:12 
snb-x220t        total:135  pass:121  dwarn:2   dfail:0   fail:1   skip:11 

Results at /archive/results/CI_IGT_test/Patchwork_708/

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

Re: [PATCH 2/6] drm/i915: Decouple struct i915_params i915 into i915_params.h

[Chris Wilson]

On Thu, Dec 17, 2015 at 08:59:42PM +0000, Chris Wilson wrote:
> On Thu, Dec 17, 2015 at 09:25:47PM +0200, Joonas Lahtinen wrote:
> > +#ifndef _I915_PARAMS_H_
> > +#define _I915_PARAMS_H_
> > +
> Is it worth saying
> 
> #include <linux/cache.h> /* for __read_mostly */
> 
> or am I alone in not knowing that?

Anyway,
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/6] drm/i915: Compile time concatenate WARN_ON macro strings

[Chris Wilson]

On Thu, Dec 17, 2015 at 09:25:46PM +0200, Joonas Lahtinen wrote:
> Using __stringify(x) instead of #x adds support for macros as
> a parameter and reduces runtime overhead.
> 
> Slightly increases the .text size but should not matter.
> 
> Cc: Rob Clark <robdclark@gmail.com>
> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 1d28d90..fe3e76d 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -69,11 +69,11 @@
>  		BUILD_BUG_ON(__i915_warn_cond); \
>  	WARN(__i915_warn_cond, "WARN_ON(" #x ")"); })
>  #else
> -#define WARN_ON(x) WARN((x), "WARN_ON(%s)", #x )
> +#define WARN_ON(x) WARN((x), "WARN_ON(" __stringify(x) ")")
>  #endif
>  
>  #undef WARN_ON_ONCE
> -#define WARN_ON_ONCE(x) WARN_ONCE((x), "WARN_ON_ONCE(%s)", #x )
> +#define WARN_ON_ONCE(x) WARN_ONCE((x), "WARN_ON_ONCE(" __stringify(x) ")")
>  
>  #define MISSING_CASE(x) WARN(1, "Missing switch case (%lu) in %s\n", \
>  			     (long) (x), __func__);
> @@ -97,12 +97,14 @@
>  })
>  
>  #define I915_STATE_WARN_ON(condition) ({				\
> +	static const char __warn_on_txt[] =				\
> +		"WARN_ON(" __stringify(condition) ")\n";		\
>  	int __ret_warn_on = !!(condition);				\
>  	if (unlikely(__ret_warn_on)) {					\
>  		if (i915.verbose_state_checks)				\
> -			WARN(1, "WARN_ON(" #condition ")\n");		\
> +			WARN(1, __warn_on_txt);				\
>  		else 							\
> -			DRM_ERROR("WARN_ON(" #condition ")\n");		\
> +			DRM_ERROR(__warn_on_txt);			\
>  	}								\

Bikeshedding, sorry.

#define I915_STATE_WARN_ON(condition) ({				\
  	int __ret_warn_on = !!(condition);				\
  	if (unlikely(__ret_warn_on)) {					\
		static const char __warn_on_txt[] =			\
			"WARN_ON(" __stringify(condition) ")\n";	\
  		if (!WARN(i915.verbose_state_checks, __warn_on_txt);	\
			DRM_ERROR(__warn_on_txt);			\
  	}								\
  	unlikely(__ret_warn_on);					\
  })
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/6] drm/i915: Add i915.debug parameter

[Joonas Lahtinen]

On to, 2015-12-17 at 21:02 +0000, Chris Wilson wrote:
> On Thu, Dec 17, 2015 at 09:25:49PM +0200, Joonas Lahtinen wrote:
> > Add i915.debug parameter to control output in cases where the debug
> > output
> > amount will be massive and as such is unsuitable for everyday use,
> > but still
> > significant in value when solving bugs.
> > 
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> 
> Maybe i915.verbose_checks ?
> 
> Too similar to i915.verbose_state_checks though?
> 
> Just trying to differentiate from drm.debug which we will also be
> settings.

As discussed in IRC, I will go with WARN_RECUR and i915.warn_recurring,
those can potentially be pushed upstream all the way to kernel core.

Regards, Joonas

> -Chris
> 
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation

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