All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Lucas De Marchi <lucas.demarchi@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH v3 v2 4/5] drm/i915: add pipe id/name to pipe mismatch logs
Date: Tue, 15 Oct 2019 21:24:37 +0300	[thread overview]
Message-ID: <20191015182437.GI1208@intel.com> (raw)
In-Reply-To: <20191015164029.18431-5-lucas.demarchi@intel.com>

On Tue, Oct 15, 2019 at 09:40:28AM -0700, Lucas De Marchi wrote:
> This way it's easier to figure out what didn't match when we have
> multiple pipes enabled.
> 
> v2: pass drm_crtc and use the more common [CRTC:%d:%s] format
>     (Ville)
> 
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 34 +++++++++++---------
>  1 file changed, 19 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index e71ec9d96c75..cd0f600e0205 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 struct drm_crtc *crtc,
> +		     const char *name, const char *format, ...)
>  {
>  	struct va_format vaf;
>  	va_list args;
> @@ -12537,9 +12538,11 @@ 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("[CRTC:%d:%s] fastset mismatch in %s %pV\n",
> +			      crtc->base.id, crtc->name, name, &vaf);
>  	else
> -		DRM_ERROR("mismatch in %s %pV\n", name, &vaf);
> +		DRM_ERROR("[CRTC:%d:%s] mismatch in %s %pV\n",
> +			  crtc->base.id, crtc->name, name, &vaf);
>  
>  	va_end(args);
>  }
> @@ -12567,6 +12570,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 struct drm_crtc *crtc = pipe_config->base.crtc;

The only thing we really const these days is the states. So I'd drop the
const here.

Also pls make it struct intel_crtc since we're trying to get rid of
this mess with both drm_ and intel_ types getting mixed around randomly.

with that
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

>  	bool ret = true;
>  	u32 bp_gamma = 0;
>  	bool fixup_inherited = fastset &&
> @@ -12580,7 +12584,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, __stringify(name), \
>  				     "(expected 0x%08x, found 0x%08x)", \
>  				     current_config->name, \
>  				     pipe_config->name); \
> @@ -12590,7 +12594,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, __stringify(name), \
>  				     "(expected %i, found %i)", \
>  				     current_config->name, \
>  				     pipe_config->name); \
> @@ -12600,7 +12604,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,  __stringify(name), \
>  				     "(expected %s, found %s)", \
>  				     yesno(current_config->name), \
>  				     yesno(pipe_config->name)); \
> @@ -12617,7 +12621,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, __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 +12631,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, __stringify(name), \
>  				     "(expected %p, found %p)", \
>  				     current_config->name, \
>  				     pipe_config->name); \
> @@ -12639,7 +12643,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, __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 +12670,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, __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 +12695,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, __stringify(name), \
>  				     "(%x) (expected %i, found %i)", \
>  				     (mask), \
>  				     current_config->name & (mask), \
> @@ -12702,7 +12706,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, __stringify(name), \
>  				     "(expected %i, found %i)", \
>  				     current_config->name, \
>  				     pipe_config->name); \
> @@ -12722,7 +12726,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, __stringify(name1), \
>  				"(expected %i, found %i, won't compare lut values)", \
>  				current_config->name1, \
>  				pipe_config->name1); \
> @@ -12731,7 +12735,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, __stringify(name2), \
>  					"hw_state doesn't match sw_state"); \
>  			ret = false; \
>  		} \
> -- 
> 2.23.0

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

  reply	other threads:[~2019-10-15 18:24 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-15 16:40 [PATCH v3 v2 0/5] Small fixes before fixing MST Lucas De Marchi
2019-10-15 16:40 ` [PATCH v3 v2 1/5] drm/i915: simplify setting of ddi_io_power_domain Lucas De Marchi
2019-10-15 16:40 ` [PATCH v3 v2 2/5] drm/i915: fix port checks for MST support on gen >= 11 Lucas De Marchi
2019-10-15 18:21   ` Ville Syrjälä
2019-10-15 16:40 ` [PATCH v3 v2 3/5] drm/i915: remove extra new line on pipe_config mismatch Lucas De Marchi
2019-10-15 16:40 ` [PATCH v3 v2 4/5] drm/i915: add pipe id/name to pipe mismatch logs Lucas De Marchi
2019-10-15 18:24   ` Ville Syrjälä [this message]
2019-10-15 16:40 ` [PATCH v3 v2 5/5] drm/i915: prettify MST debug message Lucas De Marchi
2019-10-15 19:55 ` ✗ Fi.CI.CHECKPATCH: warning for Small fixes before fixing MST (rev2) Patchwork
2019-10-15 20:25 ` ✗ Fi.CI.BAT: failure " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191015182437.GI1208@intel.com \
    --to=ville.syrjala@linux.intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=lucas.demarchi@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.