Re: [PATCH 5/7] drm/i915: add pipe name to pipe mismatch logs

Ville Syrjälä
2019-10-11T12:05:59Z

On Thu, Oct 10, 2019 at 06:09:05PM -0700, Lucas De Marchi wrote:
> This way it's easier to figure out what didn't match when we have
> multiple pipes enabled.
>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_display.c | 33 +++++++++++---------
> 1 file changed, 18 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index c5985f0b5bbf..fbc1f01f70d8 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -12526,8 +12526,9 @@ pipe_config_infoframe_mismatch(struct drm_i915_private *dev_priv,
> }
> }
>
> -static void __printf(3, 4)
> -pipe_config_mismatch(bool fastset, const char *name, const char *format, ...)
> +static void __printf(4, 5)
> +pipe_config_mismatch(bool fastset, const char *crtc_name, const char *name,
> + const char *format, ...)

I would just pass the crtc and use the canonical [CRTC:%d:%s] format.

> {
> struct va_format vaf;
> va_list args;
> @@ -12537,9 +12538,10 @@ pipe_config_mismatch(bool fastset, const char *name, const char *format, ...)
> vaf.va = &args;
>
> if (fastset)
> - DRM_DEBUG_KMS("fastset mismatch in %s %pV\n", name, &vaf);
> + DRM_DEBUG_KMS("[%s] fastset mismatch in %s %pV\n",
> + crtc_name, name, &vaf);
> else
> - DRM_ERROR("mismatch in %s %pV\n", name, &vaf);
> + DRM_ERROR("[%s] mismatch in %s %pV\n", crtc_name, name, &vaf);
>
> va_end(args);
> }
> @@ -12567,6 +12569,7 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
> bool fastset)
> {
> struct drm_i915_private *dev_priv = to_i915(current_config->base.crtc->dev);
> + const char *crtc_name = pipe_config->base.crtc->name;
> bool ret = true;
> u32 bp_gamma = 0;
> bool fixup_inherited = fastset &&
> @@ -12580,7 +12583,7 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
>
> #define PIPE_CONF_CHECK_X(name) do { \
> if (current_config->name != pipe_config->name) { \
> - pipe_config_mismatch(fastset, __stringify(name), \
> + pipe_config_mismatch(fastset, crtc_name, __stringify(name), \
> "(expected 0x%08x, found 0x%08x)", \
> current_config->name, \
> pipe_config->name); \
> @@ -12590,7 +12593,7 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
>
> #define PIPE_CONF_CHECK_I(name) do { \
> if (current_config->name != pipe_config->name) { \
> - pipe_config_mismatch(fastset, __stringify(name), \
> + pipe_config_mismatch(fastset, crtc_name, __stringify(name), \
> "(expected %i, found %i)", \
> current_config->name, \
> pipe_config->name); \
> @@ -12600,7 +12603,7 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
>
> #define PIPE_CONF_CHECK_BOOL(name) do { \
> if (current_config->name != pipe_config->name) { \
> - pipe_config_mismatch(fastset, __stringify(name), \
> + pipe_config_mismatch(fastset, crtc_name, __stringify(name), \
> "(expected %s, found %s)", \
> yesno(current_config->name), \
> yesno(pipe_config->name)); \
> @@ -12617,7 +12620,7 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
> if (!fixup_inherited || (!current_config->name && !pipe_config->name)) { \
> PIPE_CONF_CHECK_BOOL(name); \
> } else { \
> - pipe_config_mismatch(fastset, __stringify(name), \
> + pipe_config_mismatch(fastset, crtc_name, __stringify(name), \
> "unable to verify whether state matches exactly, forcing modeset (expected %s, found %s)", \
> yesno(current_config->name), \
> yesno(pipe_config->name)); \
> @@ -12627,7 +12630,7 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
>
> #define PIPE_CONF_CHECK_P(name) do { \
> if (current_config->name != pipe_config->name) { \
> - pipe_config_mismatch(fastset, __stringify(name), \
> + pipe_config_mismatch(fastset, crtc_name, __stringify(name), \
> "(expected %p, found %p)", \
> current_config->name, \
> pipe_config->name); \
> @@ -12639,7 +12642,7 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
> if (!intel_compare_link_m_n(&current_config->name, \
> &pipe_config->name,\
> !fastset)) { \
> - pipe_config_mismatch(fastset, __stringify(name), \
> + pipe_config_mismatch(fastset, crtc_name, __stringify(name), \
> "(expected tu %i gmch %i/%i link %i/%i, " \
> "found tu %i, gmch %i/%i link %i/%i)", \
> current_config->name.tu, \
> @@ -12666,7 +12669,7 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
> &pipe_config->name, !fastset) && \
> !intel_compare_link_m_n(&current_config->alt_name, \
> &pipe_config->name, !fastset)) { \
> - pipe_config_mismatch(fastset, __stringify(name), \
> + pipe_config_mismatch(fastset, crtc_name, __stringify(name), \
> "(expected tu %i gmch %i/%i link %i/%i, " \
> "or tu %i gmch %i/%i link %i/%i, " \
> "found tu %i, gmch %i/%i link %i/%i)", \
> @@ -12691,7 +12694,7 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
>
> #define PIPE_CONF_CHECK_FLAGS(name, mask) do { \
> if ((current_config->name ^ pipe_config->name) & (mask)) { \
> - pipe_config_mismatch(fastset, __stringify(name), \
> + pipe_config_mismatch(fastset, crtc_name, __stringify(name), \
> "(%x) (expected %i, found %i)", \
> (mask), \
> current_config->name & (mask), \
> @@ -12702,7 +12705,7 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
>
> #define PIPE_CONF_CHECK_CLOCK_FUZZY(name) do { \
> if (!intel_fuzzy_clock_check(current_config->name, pipe_config->name)) { \
> - pipe_config_mismatch(fastset, __stringify(name), \
> + pipe_config_mismatch(fastset, crtc_name, __stringify(name), \
> "(expected %i, found %i)", \
> current_config->name, \
> pipe_config->name); \
> @@ -12722,7 +12725,7 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
>
> #define PIPE_CONF_CHECK_COLOR_LUT(name1, name2, bit_precision) do { \
> if (current_config->name1 != pipe_config->name1) { \
> - pipe_config_mismatch(fastset, __stringify(name1), \
> + pipe_config_mismatch(fastset, crtc_name, __stringify(name1), \
> "(expected %i, found %i, won't compare lut values)", \
> current_config->name1, \
> pipe_config->name1); \
> @@ -12731,7 +12734,7 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
> if (!intel_color_lut_equal(current_config->name2, \
> pipe_config->name2, pipe_config->name1, \
> bit_precision)) { \
> - pipe_config_mismatch(fastset, __stringify(name2), \
> + pipe_config_mismatch(fastset, crtc_name, __stringify(name2), \
> "hw_state doesn't match sw_state"); \
> ret = false; \
> } \
> --
> 2.23.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

--
Ville Syrjälä
Intel