* [PATCH v2 1/4] drm/i915: Decouple struct i915_params i915 into i915_params.h
2015-12-18 11:08 [PATCH v2 0/4] Optionally display recurring warning messages Joonas Lahtinen
@ 2015-12-18 11:08 ` Joonas Lahtinen
2015-12-18 11:08 ` [PATCH v2 2/4] drm/i915: Reorder i915_params struct Joonas Lahtinen
` (2 subsequent siblings)
3 siblings, 0 replies; 7+ messages in thread
From: Joonas Lahtinen @ 2015-12-18 11:08 UTC (permalink / raw)
To: Intel graphics driver community testing & development
Otherwise usage in the i915 debug macros yields problems due to
i915_drv.h <-> i915_trace.h <-> intel_drv.h include loops.
v2:
- Document not-so-obvious need for linux/cache.h (Chris)
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
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 1ccd137..3ce609f 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"
@@ -2645,44 +2646,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..353951c
--- /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> /* for __read_mostly */
+
+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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v2 2/4] drm/i915: Reorder i915_params struct.
2015-12-18 11:08 [PATCH v2 0/4] Optionally display recurring warning messages Joonas Lahtinen
2015-12-18 11:08 ` [PATCH v2 1/4] drm/i915: Decouple struct i915_params i915 into i915_params.h Joonas Lahtinen
@ 2015-12-18 11:08 ` Joonas Lahtinen
2015-12-21 13:35 ` Daniel Vetter
2015-12-18 11:08 ` [PATCH v2 3/4] drm/i915: Add WARN_RECUR and i915.recur_warnings Joonas Lahtinen
2015-12-18 11:08 ` [PATCH v2 4/4] drm/i915: Use WARN_RECUR for recurring warning messages Joonas Lahtinen
3 siblings, 1 reply; 7+ messages in thread
From: Joonas Lahtinen @ 2015-12-18 11:08 UTC (permalink / raw)
To: Intel graphics driver community testing & development
Move all the bool variables to the end as per the comment.
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
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 353951c..5299290 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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/4] drm/i915: Reorder i915_params struct.
2015-12-18 11:08 ` [PATCH v2 2/4] drm/i915: Reorder i915_params struct Joonas Lahtinen
@ 2015-12-21 13:35 ` Daniel Vetter
0 siblings, 0 replies; 7+ messages in thread
From: Daniel Vetter @ 2015-12-21 13:35 UTC (permalink / raw)
To: Joonas Lahtinen; +Cc: Intel graphics driver community testing & development
On Fri, Dec 18, 2015 at 01:08:16PM +0200, Joonas Lahtinen wrote:
> Move all the bool variables to the end as per the comment.
>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Merged the first 2 patches from this series, thanks.
-Daniel
> ---
> 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 353951c..5299290 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
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 3/4] drm/i915: Add WARN_RECUR and i915.recur_warnings
2015-12-18 11:08 [PATCH v2 0/4] Optionally display recurring warning messages Joonas Lahtinen
2015-12-18 11:08 ` [PATCH v2 1/4] drm/i915: Decouple struct i915_params i915 into i915_params.h Joonas Lahtinen
2015-12-18 11:08 ` [PATCH v2 2/4] drm/i915: Reorder i915_params struct Joonas Lahtinen
@ 2015-12-18 11:08 ` Joonas Lahtinen
2015-12-18 11:30 ` Chris Wilson
2015-12-18 11:08 ` [PATCH v2 4/4] drm/i915: Use WARN_RECUR for recurring warning messages Joonas Lahtinen
3 siblings, 1 reply; 7+ messages in thread
From: Joonas Lahtinen @ 2015-12-18 11:08 UTC (permalink / raw)
To: Intel graphics driver community testing & development
Add i915.recur_warnings parameter to control output in cases where the warning
is of recurring type and is potentially called from multiple paths. Using just
WARN_ONCE would mask out other calling paths but one, this is not desireable
on developer machine or CI system, but is a compromise to be made on end user
system not to flood the message and overflow all possible kernel log buffers.
When the recur_warnings option is false (default), WARN_RECUR will reduce to
WARN_ONCE.
v2:
- More upstreamable macro name and parameter (Chris)
- Squash a hunk that slipped to next patch
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 | 10 ++++++++++
drivers/gpu/drm/i915/i915_params.c | 6 ++++++
drivers/gpu/drm/i915/i915_params.h | 1 +
3 files changed, 17 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 3ce609f..e1ca61f 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -76,6 +76,16 @@
#undef WARN_ON_ONCE
#define WARN_ON_ONCE(x) WARN_ONCE((x), "WARN_ON_ONCE(" __stringify(x) ")")
+#define 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.recur_warnings), format)) \
+ __warned = true; \
+ unlikely(__ret_warn_on); \
+})
+
#define MISSING_CASE(x) WARN(1, "Missing switch case (%lu) in %s\n", \
(long) (x), __func__);
diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index 8d90c25..f75dac9 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,
+ .recur_warnings = 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(recur_warnings, i915.recur_warnings, bool, 0600);
+MODULE_PARM_DESC(recur_warnings,
+ "Display all warnings of recurring nature (default: false). "
+ "For developers only.");
diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
index 5299290..23be479 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 recur_warnings;
};
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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 3/4] drm/i915: Add WARN_RECUR and i915.recur_warnings
2015-12-18 11:08 ` [PATCH v2 3/4] drm/i915: Add WARN_RECUR and i915.recur_warnings Joonas Lahtinen
@ 2015-12-18 11:30 ` Chris Wilson
0 siblings, 0 replies; 7+ messages in thread
From: Chris Wilson @ 2015-12-18 11:30 UTC (permalink / raw)
To: Joonas Lahtinen; +Cc: Intel graphics driver community testing & development
On Fri, Dec 18, 2015 at 01:08:17PM +0200, Joonas Lahtinen wrote:
> Add i915.recur_warnings parameter to control output in cases where the warning
> is of recurring type and is potentially called from multiple paths. Using just
> WARN_ONCE would mask out other calling paths but one, this is not desireable
> on developer machine or CI system, but is a compromise to be made on end user
> system not to flood the message and overflow all possible kernel log buffers.
>
> When the recur_warnings option is false (default), WARN_RECUR will reduce to
> WARN_ONCE.
>
> v2:
> - More upstreamable macro name and parameter (Chris)
> - Squash a hunk that slipped to next patch
>
> 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 | 10 ++++++++++
> drivers/gpu/drm/i915/i915_params.c | 6 ++++++
> drivers/gpu/drm/i915/i915_params.h | 1 +
> 3 files changed, 17 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 3ce609f..e1ca61f 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -76,6 +76,16 @@
> #undef WARN_ON_ONCE
> #define WARN_ON_ONCE(x) WARN_ONCE((x), "WARN_ON_ONCE(" __stringify(x) ")")
>
> +#define 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.recur_warnings), format)) \
> + __warned = true; \
> + unlikely(__ret_warn_on); \
> +})
Ah, see include/linux/ratelimit.h
Just wondering if we can reuse that, extend it in someway to cover a
control variable?
Similarly, how to fold in I915_STATE_WARN (if possible)? i.e. to have
the optional error message instead of the WARN.
-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
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 4/4] drm/i915: Use WARN_RECUR for recurring warning messages
2015-12-18 11:08 [PATCH v2 0/4] Optionally display recurring warning messages Joonas Lahtinen
` (2 preceding siblings ...)
2015-12-18 11:08 ` [PATCH v2 3/4] drm/i915: Add WARN_RECUR and i915.recur_warnings Joonas Lahtinen
@ 2015-12-18 11:08 ` Joonas Lahtinen
3 siblings, 0 replies; 7+ messages in thread
From: Joonas Lahtinen @ 2015-12-18 11:08 UTC (permalink / raw)
To: Intel graphics driver community testing & development
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 but
displaying it each time would make end user system log overflow
easily.
v2:
- Rebase for new macro name
- Remove a hunk belonging to previous patch
Cc: Imre Deak <imre.deak@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
drivers/gpu/drm/i915/intel_drv.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d523ebb..baac83c 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,
+ 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),
+ 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,
+ 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
^ permalink raw reply related [flat|nested] 7+ messages in thread