On Thu, 12 Oct 2023, Arun R Murthy <arun.r.mur...@intel.com> wrote:
> By default warn_on are enabled, hence removing this module parameter.
I'm uneasy about this change, though.
Do we really want to force people to see WARNs (and oops if
panic_on_warn=1) on state checker? On the other hand, that's the way to
get bug reports and fix the stuff...
I wonder if distros actually use the module parameter.
An alternative might be to add a debug config option.
> Signed-off-by: Arun R Murthy <arun.r.mur...@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_display.h | 2 +-
> drivers/gpu/drm/i915/i915_params.c | 4 ----
> drivers/gpu/drm/i915/i915_params.h | 1 -
> 3 files changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.h
> b/drivers/gpu/drm/i915/display/intel_display.h
> index 0e5dffe8f018..8e2453e010a7 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.h
> +++ b/drivers/gpu/drm/i915/display/intel_display.h
> @@ -552,7 +552,7 @@ bool assert_port_valid(struct drm_i915_private *i915,
> enum port port);
> struct drm_device *drm = &(__i915)->drm; \
> int __ret_warn_on = !!(condition); \
> if (unlikely(__ret_warn_on)) \
> - if (!drm_WARN(drm, i915_modparams.verbose_state_checks,
> format)) \
> + if (!drm_WARN(drm, true, format)) \
> drm_err(drm, format); \
Everything in the whole macro is about that one condition on the verbose
state checks, and with that taken away, it all simplifies to something
along the lines of:
#define I915_STATE_WARN(__i915, condition, ...) \
drm_WARN(&(__i915)->drm, condition, ## __VA_ARGS__)
I didn't build check that I got the va arg stuff right, but it's
something like that *if* we want to remove the condition altogether.
The comment also becomes stale and needs to be fixed.
BR,
Jani.
> unlikely(__ret_warn_on); \
> })
> diff --git a/drivers/gpu/drm/i915/i915_params.c
> b/drivers/gpu/drm/i915/i915_params.c
> index 036c4c3ed6ed..23453d9be175 100644
> --- a/drivers/gpu/drm/i915/i915_params.c
> +++ b/drivers/gpu/drm/i915/i915_params.c
> @@ -162,10 +162,6 @@ i915_param_named(mmio_debug, int, 0400,
> "Enable the MMIO debug code for the first N failures (default: off). "
> "This may negatively affect performance.");
>
> -/* Special case writable file */
> -i915_param_named(verbose_state_checks, bool, 0600,
> - "Enable verbose logs (ie. WARN_ON()) in case of unexpected hw state
> conditions.");
> -
> i915_param_named_unsafe(nuclear_pageflip, bool, 0400,
> "Force enable atomic functionality on platforms that don't have full
> support yet.");
>
> diff --git a/drivers/gpu/drm/i915/i915_params.h
> b/drivers/gpu/drm/i915/i915_params.h
> index d5194b039aab..af675618ab07 100644
> --- a/drivers/gpu/drm/i915/i915_params.h
> +++ b/drivers/gpu/drm/i915/i915_params.h
> @@ -83,7 +83,6 @@ struct drm_printer;
> param(bool, force_reset_modeset_test, false, 0600) \
> param(bool, error_capture, true,
> IS_ENABLED(CONFIG_DRM_I915_CAPTURE_ERROR) ? 0600 : 0) \
> param(bool, disable_display, false, 0400) \
> - param(bool, verbose_state_checks, true, 0) \
> param(bool, nuclear_pageflip, false, 0400) \
> param(bool, enable_dp_mst, true, 0600) \
> param(bool, enable_gvt, false, IS_ENABLED(CONFIG_DRM_I915_GVT) ? 0400 :
> 0)
--
Jani Nikula, Intel
