All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/4] drm/i915: Don't clobber M/N values during fastset check
@ 2019-06-12 13:07 Ville Syrjala
  2019-06-12 13:07 ` [PATCH 2/4] drm/i915: Constify intel_pipe_config_compare() Ville Syrjala
                   ` (8 more replies)
  0 siblings, 9 replies; 19+ messages in thread
From: Ville Syrjala @ 2019-06-12 13:07 UTC (permalink / raw)
  To: intel-gfx; +Cc: stable, Blubberbub, Maarten Lankhorst, Hans de Goede

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

We're now calling intel_pipe_config_compare(..., true) uncoditionally
which means we're always going clobber the calculated M/N values with
the old values if the fuzzy M/N check passes. That causes problems
because the fuzzy check allows for a huge difference in the values.

I'm actually tempted to just make the M/N checks exact, but that might
prevent fastboot from kicking in when people want it. So for now let's
overwrite the computed values with the old values only if decide to skip
the modeset.

Cc: stable@vger.kernel.org
Cc: Blubberbub@protonmail.com
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Hans de Goede <hdegoede@redhat.com>
Tested-by: Blubberbub@protonmail.com
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110782
Fixes: d19f958db23c ("drm/i915: Enable fastset for non-boot modesets.")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 35 +++++++++++++++++++++-------
 1 file changed, 27 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 1b1ddb48ca7a..73b3e92b7ed5 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12299,9 +12299,6 @@ intel_compare_link_m_n(const struct intel_link_m_n *m_n,
 			      m2_n2->gmch_m, m2_n2->gmch_n, !adjust) &&
 	    intel_compare_m_n(m_n->link_m, m_n->link_n,
 			      m2_n2->link_m, m2_n2->link_n, !adjust)) {
-		if (adjust)
-			*m2_n2 = *m_n;
-
 		return true;
 	}
 
@@ -13433,6 +13430,32 @@ static int calc_watermark_data(struct intel_atomic_state *state)
 	return 0;
 }
 
+static void intel_crtc_check_fastset(struct intel_crtc_state *old_crtc_state,
+				     struct intel_crtc_state *new_crtc_state)
+{
+	struct drm_i915_private *dev_priv =
+		to_i915(new_crtc_state->base.crtc->dev);
+
+	if (!intel_pipe_config_compare(dev_priv, old_crtc_state,
+				       new_crtc_state, true))
+		return;
+
+	new_crtc_state->base.mode_changed = false;
+	new_crtc_state->update_pipe = true;
+
+	/*
+	 * If we're not doing the full modeset we want to
+	 * keep the current M/N values as they may be
+	 * sufficiently different to the computed values
+	 * to cause problems.
+	 *
+	 * FIXME: should really copy more fuzzy state here
+	 */
+	new_crtc_state->fdi_m_n = old_crtc_state->fdi_m_n;
+	new_crtc_state->dp_m_n = old_crtc_state->dp_m_n;
+	new_crtc_state->dp_m2_n2 = old_crtc_state->dp_m2_n2;
+}
+
 /**
  * intel_atomic_check - validate state object
  * @dev: drm device
@@ -13474,11 +13497,7 @@ static int intel_atomic_check(struct drm_device *dev,
 		if (ret)
 			goto fail;
 
-		if (intel_pipe_config_compare(dev_priv, old_crtc_state,
-					      new_crtc_state, true)) {
-			new_crtc_state->base.mode_changed = false;
-			new_crtc_state->update_pipe = true;
-		}
+		intel_crtc_check_fastset(old_crtc_state, new_crtc_state);
 
 		if (needs_modeset(&new_crtc_state->base))
 			any_ms = true;
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 2/4] drm/i915: Constify intel_pipe_config_compare()
  2019-06-12 13:07 [PATCH 1/4] drm/i915: Don't clobber M/N values during fastset check Ville Syrjala
@ 2019-06-12 13:07 ` Ville Syrjala
  2019-06-18 12:25   ` Imre Deak
  2019-06-12 13:08 ` [PATCH 3/4] drm/i915: Make pipe_config_err() vs. fastset less confusing Ville Syrjala
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 19+ messages in thread
From: Ville Syrjala @ 2019-06-12 13:07 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Now that intel_pipe_config_compare() no longer clobbers the passed
in state we can make both crtc states const. And while at we simplify
the calling convention, and clean up intel_compare_link_m_n() a bit.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 43 +++++++++++-----------------
 1 file changed, 17 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 73b3e92b7ed5..dcc301df6174 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12291,18 +12291,14 @@ intel_compare_m_n(unsigned int m, unsigned int n,
 
 static bool
 intel_compare_link_m_n(const struct intel_link_m_n *m_n,
-		       struct intel_link_m_n *m2_n2,
-		       bool adjust)
-{
-	if (m_n->tu == m2_n2->tu &&
-	    intel_compare_m_n(m_n->gmch_m, m_n->gmch_n,
-			      m2_n2->gmch_m, m2_n2->gmch_n, !adjust) &&
-	    intel_compare_m_n(m_n->link_m, m_n->link_n,
-			      m2_n2->link_m, m2_n2->link_n, !adjust)) {
-		return true;
-	}
-
-	return false;
+		       const struct intel_link_m_n *m2_n2,
+		       bool exact)
+{
+	return m_n->tu == m2_n2->tu &&
+		intel_compare_m_n(m_n->gmch_m, m_n->gmch_n,
+				  m2_n2->gmch_m, m2_n2->gmch_n, exact) &&
+		intel_compare_m_n(m_n->link_m, m_n->link_n,
+				  m2_n2->link_m, m2_n2->link_n, exact);
 }
 
 static bool
@@ -12372,11 +12368,11 @@ static bool fastboot_enabled(struct drm_i915_private *dev_priv)
 }
 
 static bool
-intel_pipe_config_compare(struct drm_i915_private *dev_priv,
-			  struct intel_crtc_state *current_config,
-			  struct intel_crtc_state *pipe_config,
+intel_pipe_config_compare(const struct intel_crtc_state *current_config,
+			  const struct intel_crtc_state *pipe_config,
 			  bool adjust)
 {
+	struct drm_i915_private *dev_priv = to_i915(current_config->base.crtc->dev);
 	bool ret = true;
 	bool fixup_inherited = adjust &&
 		(current_config->base.mode.private_flags & I915_MODE_FLAG_INHERITED) &&
@@ -12447,7 +12443,7 @@ intel_pipe_config_compare(struct drm_i915_private *dev_priv,
 #define PIPE_CONF_CHECK_M_N(name) do { \
 	if (!intel_compare_link_m_n(&current_config->name, \
 				    &pipe_config->name,\
-				    adjust)) { \
+				    !adjust)) { \
 		pipe_config_err(adjust, __stringify(name), \
 			  "(expected tu %i gmch %i/%i link %i/%i, " \
 			  "found tu %i, gmch %i/%i link %i/%i)\n", \
@@ -12472,9 +12468,9 @@ intel_pipe_config_compare(struct drm_i915_private *dev_priv,
  */
 #define PIPE_CONF_CHECK_M_N_ALT(name, alt_name) do { \
 	if (!intel_compare_link_m_n(&current_config->name, \
-				    &pipe_config->name, adjust) && \
+				    &pipe_config->name, !adjust) && \
 	    !intel_compare_link_m_n(&current_config->alt_name, \
-				    &pipe_config->name, adjust)) { \
+				    &pipe_config->name, !adjust)) { \
 		pipe_config_err(adjust, __stringify(name), \
 			  "(expected tu %i gmch %i/%i link %i/%i, " \
 			  "or tu %i gmch %i/%i link %i/%i, " \
@@ -12984,8 +12980,7 @@ verify_crtc_state(struct drm_crtc *crtc,
 	intel_pipe_config_sanity_check(dev_priv, pipe_config);
 
 	sw_config = to_intel_crtc_state(new_crtc_state);
-	if (!intel_pipe_config_compare(dev_priv, sw_config,
-				       pipe_config, false)) {
+	if (!intel_pipe_config_compare(sw_config, pipe_config, false)) {
 		I915_STATE_WARN(1, "pipe state doesn't match!\n");
 		intel_dump_pipe_config(pipe_config, NULL, "[hw state]");
 		intel_dump_pipe_config(sw_config, NULL, "[sw state]");
@@ -13430,14 +13425,10 @@ static int calc_watermark_data(struct intel_atomic_state *state)
 	return 0;
 }
 
-static void intel_crtc_check_fastset(struct intel_crtc_state *old_crtc_state,
+static void intel_crtc_check_fastset(const struct intel_crtc_state *old_crtc_state,
 				     struct intel_crtc_state *new_crtc_state)
 {
-	struct drm_i915_private *dev_priv =
-		to_i915(new_crtc_state->base.crtc->dev);
-
-	if (!intel_pipe_config_compare(dev_priv, old_crtc_state,
-				       new_crtc_state, true))
+	if (!intel_pipe_config_compare(old_crtc_state, new_crtc_state, true))
 		return;
 
 	new_crtc_state->base.mode_changed = false;
-- 
2.21.0

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 3/4] drm/i915: Make pipe_config_err() vs. fastset less confusing
  2019-06-12 13:07 [PATCH 1/4] drm/i915: Don't clobber M/N values during fastset check Ville Syrjala
  2019-06-12 13:07 ` [PATCH 2/4] drm/i915: Constify intel_pipe_config_compare() Ville Syrjala
@ 2019-06-12 13:08 ` Ville Syrjala
  2019-06-12 15:43   ` Imre Deak
  2019-06-12 13:08 ` [PATCH 4/4] drm/i915: Drop the _INCOMPLETE for has_infoframe Ville Syrjala
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 19+ messages in thread
From: Ville Syrjala @ 2019-06-12 13:08 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Rename pipe_config_err() to pipe_config_mismatch(), and also print
whether we're doing the fastset check or the sw vs. hw state readout
check. Should make the logs a bit less confusing when they're not
filled with what looks like a real error.

Also rename the 'adjust' variable to 'fastset' to make it clear what
it means.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 158 +++++++++++++--------------
 1 file changed, 79 insertions(+), 79 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index dcc301df6174..ec3b7e825613 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12309,16 +12309,16 @@ intel_compare_infoframe(const union hdmi_infoframe *a,
 }
 
 static void
-pipe_config_infoframe_err(struct drm_i915_private *dev_priv,
-			  bool adjust, const char *name,
-			  const union hdmi_infoframe *a,
-			  const union hdmi_infoframe *b)
+pipe_config_infoframe_mismatch(struct drm_i915_private *dev_priv,
+			       bool fastset, const char *name,
+			       const union hdmi_infoframe *a,
+			       const union hdmi_infoframe *b)
 {
-	if (adjust) {
+	if (fastset) {
 		if ((drm_debug & DRM_UT_KMS) == 0)
 			return;
 
-		drm_dbg(DRM_UT_KMS, "mismatch in %s infoframe", name);
+		drm_dbg(DRM_UT_KMS, "fastset mismatch in %s infoframe", name);
 		drm_dbg(DRM_UT_KMS, "expected:");
 		hdmi_infoframe_log(KERN_DEBUG, dev_priv->drm.dev, a);
 		drm_dbg(DRM_UT_KMS, "found");
@@ -12333,7 +12333,7 @@ pipe_config_infoframe_err(struct drm_i915_private *dev_priv,
 }
 
 static void __printf(3, 4)
-pipe_config_err(bool adjust, const char *name, const char *format, ...)
+pipe_config_mismatch(bool fastset, const char *name, const char *format, ...)
 {
 	struct va_format vaf;
 	va_list args;
@@ -12342,8 +12342,8 @@ pipe_config_err(bool adjust, const char *name, const char *format, ...)
 	vaf.fmt = format;
 	vaf.va = &args;
 
-	if (adjust)
-		drm_dbg(DRM_UT_KMS, "mismatch in %s %pV", name, &vaf);
+	if (fastset)
+		drm_dbg(DRM_UT_KMS, "fastset mismatch in %s %pV", name, &vaf);
 	else
 		drm_err("mismatch in %s %pV", name, &vaf);
 
@@ -12370,11 +12370,11 @@ static bool fastboot_enabled(struct drm_i915_private *dev_priv)
 static bool
 intel_pipe_config_compare(const struct intel_crtc_state *current_config,
 			  const struct intel_crtc_state *pipe_config,
-			  bool adjust)
+			  bool fastset)
 {
 	struct drm_i915_private *dev_priv = to_i915(current_config->base.crtc->dev);
 	bool ret = true;
-	bool fixup_inherited = adjust &&
+	bool fixup_inherited = fastset &&
 		(current_config->base.mode.private_flags & I915_MODE_FLAG_INHERITED) &&
 		!(pipe_config->base.mode.private_flags & I915_MODE_FLAG_INHERITED);
 
@@ -12385,30 +12385,30 @@ 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_err(adjust, __stringify(name), \
-			  "(expected 0x%08x, found 0x%08x)\n", \
-			  current_config->name, \
-			  pipe_config->name); \
+		pipe_config_mismatch(fastset, __stringify(name), \
+				     "(expected 0x%08x, found 0x%08x)\n", \
+				     current_config->name, \
+				     pipe_config->name); \
 		ret = false; \
 	} \
 } while (0)
 
 #define PIPE_CONF_CHECK_I(name) do { \
 	if (current_config->name != pipe_config->name) { \
-		pipe_config_err(adjust, __stringify(name), \
-			  "(expected %i, found %i)\n", \
-			  current_config->name, \
-			  pipe_config->name); \
+		pipe_config_mismatch(fastset, __stringify(name), \
+				     "(expected %i, found %i)\n", \
+				     current_config->name, \
+				     pipe_config->name); \
 		ret = false; \
 	} \
 } while (0)
 
 #define PIPE_CONF_CHECK_BOOL(name) do { \
 	if (current_config->name != pipe_config->name) { \
-		pipe_config_err(adjust, __stringify(name), \
-			  "(expected %s, found %s)\n", \
-			  yesno(current_config->name), \
-			  yesno(pipe_config->name)); \
+		pipe_config_mismatch(fastset, __stringify(name), \
+				     "(expected %s, found %s)\n", \
+				     yesno(current_config->name), \
+				     yesno(pipe_config->name)); \
 		ret = false; \
 	} \
 } while (0)
@@ -12422,20 +12422,20 @@ 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_err(adjust, __stringify(name), \
-			  "unable to verify whether state matches exactly, forcing modeset (expected %s, found %s)\n", \
-			  yesno(current_config->name), \
-			  yesno(pipe_config->name)); \
+		pipe_config_mismatch(fastset, __stringify(name), \
+				     "unable to verify whether state matches exactly, forcing modeset (expected %s, found %s)\n", \
+				     yesno(current_config->name), \
+				     yesno(pipe_config->name)); \
 		ret = false; \
 	} \
 } while (0)
 
 #define PIPE_CONF_CHECK_P(name) do { \
 	if (current_config->name != pipe_config->name) { \
-		pipe_config_err(adjust, __stringify(name), \
-			  "(expected %p, found %p)\n", \
-			  current_config->name, \
-			  pipe_config->name); \
+		pipe_config_mismatch(fastset, __stringify(name), \
+				     "(expected %p, found %p)\n", \
+				     current_config->name, \
+				     pipe_config->name); \
 		ret = false; \
 	} \
 } while (0)
@@ -12443,20 +12443,20 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
 #define PIPE_CONF_CHECK_M_N(name) do { \
 	if (!intel_compare_link_m_n(&current_config->name, \
 				    &pipe_config->name,\
-				    !adjust)) { \
-		pipe_config_err(adjust, __stringify(name), \
-			  "(expected tu %i gmch %i/%i link %i/%i, " \
-			  "found tu %i, gmch %i/%i link %i/%i)\n", \
-			  current_config->name.tu, \
-			  current_config->name.gmch_m, \
-			  current_config->name.gmch_n, \
-			  current_config->name.link_m, \
-			  current_config->name.link_n, \
-			  pipe_config->name.tu, \
-			  pipe_config->name.gmch_m, \
-			  pipe_config->name.gmch_n, \
-			  pipe_config->name.link_m, \
-			  pipe_config->name.link_n); \
+				    !fastset)) { \
+		pipe_config_mismatch(fastset, __stringify(name), \
+				     "(expected tu %i gmch %i/%i link %i/%i, " \
+				     "found tu %i, gmch %i/%i link %i/%i)\n", \
+				     current_config->name.tu, \
+				     current_config->name.gmch_m, \
+				     current_config->name.gmch_n, \
+				     current_config->name.link_m, \
+				     current_config->name.link_n, \
+				     pipe_config->name.tu, \
+				     pipe_config->name.gmch_m, \
+				     pipe_config->name.gmch_n, \
+				     pipe_config->name.link_m, \
+				     pipe_config->name.link_n); \
 		ret = false; \
 	} \
 } while (0)
@@ -12468,49 +12468,49 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
  */
 #define PIPE_CONF_CHECK_M_N_ALT(name, alt_name) do { \
 	if (!intel_compare_link_m_n(&current_config->name, \
-				    &pipe_config->name, !adjust) && \
+				    &pipe_config->name, !fastset) && \
 	    !intel_compare_link_m_n(&current_config->alt_name, \
-				    &pipe_config->name, !adjust)) { \
-		pipe_config_err(adjust, __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)\n", \
-			  current_config->name.tu, \
-			  current_config->name.gmch_m, \
-			  current_config->name.gmch_n, \
-			  current_config->name.link_m, \
-			  current_config->name.link_n, \
-			  current_config->alt_name.tu, \
-			  current_config->alt_name.gmch_m, \
-			  current_config->alt_name.gmch_n, \
-			  current_config->alt_name.link_m, \
-			  current_config->alt_name.link_n, \
-			  pipe_config->name.tu, \
-			  pipe_config->name.gmch_m, \
-			  pipe_config->name.gmch_n, \
-			  pipe_config->name.link_m, \
-			  pipe_config->name.link_n); \
+				    &pipe_config->name, !fastset)) { \
+		pipe_config_mismatch(fastset, __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)\n", \
+				     current_config->name.tu, \
+				     current_config->name.gmch_m, \
+				     current_config->name.gmch_n, \
+				     current_config->name.link_m, \
+				     current_config->name.link_n, \
+				     current_config->alt_name.tu, \
+				     current_config->alt_name.gmch_m, \
+				     current_config->alt_name.gmch_n, \
+				     current_config->alt_name.link_m, \
+				     current_config->alt_name.link_n, \
+				     pipe_config->name.tu, \
+				     pipe_config->name.gmch_m, \
+				     pipe_config->name.gmch_n, \
+				     pipe_config->name.link_m, \
+				     pipe_config->name.link_n); \
 		ret = false; \
 	} \
 } while (0)
 
 #define PIPE_CONF_CHECK_FLAGS(name, mask) do { \
 	if ((current_config->name ^ pipe_config->name) & (mask)) { \
-		pipe_config_err(adjust, __stringify(name), \
-			  "(%x) (expected %i, found %i)\n", \
-			  (mask), \
-			  current_config->name & (mask), \
-			  pipe_config->name & (mask)); \
+		pipe_config_mismatch(fastset, __stringify(name), \
+				     "(%x) (expected %i, found %i)\n", \
+				     (mask), \
+				     current_config->name & (mask), \
+				     pipe_config->name & (mask)); \
 		ret = false; \
 	} \
 } while (0)
 
 #define PIPE_CONF_CHECK_CLOCK_FUZZY(name) do { \
 	if (!intel_fuzzy_clock_check(current_config->name, pipe_config->name)) { \
-		pipe_config_err(adjust, __stringify(name), \
-			  "(expected %i, found %i)\n", \
-			  current_config->name, \
-			  pipe_config->name); \
+		pipe_config_mismatch(fastset, __stringify(name), \
+				     "(expected %i, found %i)\n", \
+				     current_config->name, \
+				     pipe_config->name); \
 		ret = false; \
 	} \
 } while (0)
@@ -12518,9 +12518,9 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
 #define PIPE_CONF_CHECK_INFOFRAME(name) do { \
 	if (!intel_compare_infoframe(&current_config->infoframes.name, \
 				     &pipe_config->infoframes.name)) { \
-		pipe_config_infoframe_err(dev_priv, adjust, __stringify(name), \
-					  &current_config->infoframes.name, \
-					  &pipe_config->infoframes.name); \
+		pipe_config_infoframe_mismatch(dev_priv, fastset, __stringify(name), \
+					       &current_config->infoframes.name, \
+					       &pipe_config->infoframes.name); \
 		ret = false; \
 	} \
 } while (0)
@@ -12600,7 +12600,7 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
 	 */
 	PIPE_CONF_CHECK_BOOL(pch_pfit.force_thru);
 
-	if (!adjust) {
+	if (!fastset) {
 		PIPE_CONF_CHECK_I(pipe_src_w);
 		PIPE_CONF_CHECK_I(pipe_src_h);
 
-- 
2.21.0

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 4/4] drm/i915: Drop the _INCOMPLETE for has_infoframe
  2019-06-12 13:07 [PATCH 1/4] drm/i915: Don't clobber M/N values during fastset check Ville Syrjala
  2019-06-12 13:07 ` [PATCH 2/4] drm/i915: Constify intel_pipe_config_compare() Ville Syrjala
  2019-06-12 13:08 ` [PATCH 3/4] drm/i915: Make pipe_config_err() vs. fastset less confusing Ville Syrjala
@ 2019-06-12 13:08 ` Ville Syrjala
  2019-06-18 12:34   ` Imre Deak
  2019-06-12 13:15 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/4] drm/i915: Don't clobber M/N values during fastset check Patchwork
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 19+ messages in thread
From: Ville Syrjala @ 2019-06-12 13:08 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

We have full infoframe readout now so we can replace the
PIPE_CONF_CHECK_BOOL_INCOMPLETE(has_infoframe) with the normal
PIPE_CONF_CHECK_BOOL(has_infoframe).

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index ec3b7e825613..72fb04863acd 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12570,7 +12570,7 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
 
 	PIPE_CONF_CHECK_BOOL(hdmi_scrambling);
 	PIPE_CONF_CHECK_BOOL(hdmi_high_tmds_clock_ratio);
-	PIPE_CONF_CHECK_BOOL_INCOMPLETE(has_infoframe);
+	PIPE_CONF_CHECK_BOOL(has_infoframe);
 
 	PIPE_CONF_CHECK_BOOL_INCOMPLETE(has_audio);
 
-- 
2.21.0

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/4] drm/i915: Don't clobber M/N values during fastset check
  2019-06-12 13:07 [PATCH 1/4] drm/i915: Don't clobber M/N values during fastset check Ville Syrjala
                   ` (2 preceding siblings ...)
  2019-06-12 13:08 ` [PATCH 4/4] drm/i915: Drop the _INCOMPLETE for has_infoframe Ville Syrjala
@ 2019-06-12 13:15 ` Patchwork
  2019-06-12 13:52 ` ✗ Fi.CI.BAT: failure " Patchwork
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 19+ messages in thread
From: Patchwork @ 2019-06-12 13:15 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/4] drm/i915: Don't clobber M/N values during fastset check
URL   : https://patchwork.freedesktop.org/series/61960/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
bc29623ab5e2 drm/i915: Don't clobber M/N values during fastset check
2e4eb369f0c4 drm/i915: Constify intel_pipe_config_compare()
2b517a2a0592 drm/i915: Make pipe_config_err() vs. fastset less confusing
-:132: WARNING:LONG_LINE: line over 100 characters
#132: FILE: drivers/gpu/drm/i915/intel_display.c:12426:
+				     "unable to verify whether state matches exactly, forcing modeset (expected %s, found %s)\n", \

total: 0 errors, 1 warnings, 0 checks, 256 lines checked
90a1b356ee44 drm/i915: Drop the _INCOMPLETE for has_infoframe

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* ✗ Fi.CI.BAT: failure for series starting with [1/4] drm/i915: Don't clobber M/N values during fastset check
  2019-06-12 13:07 [PATCH 1/4] drm/i915: Don't clobber M/N values during fastset check Ville Syrjala
                   ` (3 preceding siblings ...)
  2019-06-12 13:15 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/4] drm/i915: Don't clobber M/N values during fastset check Patchwork
@ 2019-06-12 13:52 ` Patchwork
  2019-06-12 17:24 ` [PATCH v2 1/4] " Ville Syrjala
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 19+ messages in thread
From: Patchwork @ 2019-06-12 13:52 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/4] drm/i915: Don't clobber M/N values during fastset check
URL   : https://patchwork.freedesktop.org/series/61960/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_6248 -> Patchwork_13252
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_13252 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_13252, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13252/

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_13252:

### IGT changes ###

#### Possible regressions ####

  * igt@gem_close_race@basic-threads:
    - fi-icl-y:           [PASS][1] -> [DMESG-WARN][2] +8 similar issues
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6248/fi-icl-y/igt@gem_close_race@basic-threads.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13252/fi-icl-y/igt@gem_close_race@basic-threads.html

  
Known issues
------------

  Here are the changes found in Patchwork_13252 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@kms_chamelium@dp-crc-fast:
    - fi-cml-u2:          [PASS][3] -> [FAIL][4] ([fdo#110627])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6248/fi-cml-u2/igt@kms_chamelium@dp-crc-fast.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13252/fi-cml-u2/igt@kms_chamelium@dp-crc-fast.html

  * igt@kms_flip@basic-flip-vs-dpms:
    - fi-bxt-dsi:         [PASS][5] -> [INCOMPLETE][6] ([fdo#103927])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6248/fi-bxt-dsi/igt@kms_flip@basic-flip-vs-dpms.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13252/fi-bxt-dsi/igt@kms_flip@basic-flip-vs-dpms.html

  * igt@kms_frontbuffer_tracking@basic:
    - fi-hsw-peppy:       [PASS][7] -> [DMESG-WARN][8] ([fdo#102614])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6248/fi-hsw-peppy/igt@kms_frontbuffer_tracking@basic.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13252/fi-hsw-peppy/igt@kms_frontbuffer_tracking@basic.html

  
#### Possible fixes ####

  * igt@gem_ctx_switch@basic-default:
    - fi-icl-u2:          [INCOMPLETE][9] ([fdo#107713] / [fdo#108569]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6248/fi-icl-u2/igt@gem_ctx_switch@basic-default.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13252/fi-icl-u2/igt@gem_ctx_switch@basic-default.html

  * igt@i915_module_load@reload:
    - fi-ilk-650:         [DMESG-WARN][11] ([fdo#106387]) -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6248/fi-ilk-650/igt@i915_module_load@reload.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13252/fi-ilk-650/igt@i915_module_load@reload.html

  
#### Warnings ####

  * igt@i915_pm_rpm@basic-pci-d3-state:
    - fi-kbl-guc:         [FAIL][13] ([fdo#110829]) -> [SKIP][14] ([fdo#109271])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6248/fi-kbl-guc/igt@i915_pm_rpm@basic-pci-d3-state.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13252/fi-kbl-guc/igt@i915_pm_rpm@basic-pci-d3-state.html

  
  [fdo#102614]: https://bugs.freedesktop.org/show_bug.cgi?id=102614
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#106387]: https://bugs.freedesktop.org/show_bug.cgi?id=106387
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#110627]: https://bugs.freedesktop.org/show_bug.cgi?id=110627
  [fdo#110829]: https://bugs.freedesktop.org/show_bug.cgi?id=110829


Participating hosts (53 -> 45)
------------------------------

  Additional (1): fi-cml-u 
  Missing    (9): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-pnv-d510 fi-byt-n2820 fi-byt-clapper fi-bdw-samus 


Build changes
-------------

  * Linux: CI_DRM_6248 -> Patchwork_13252

  CI_DRM_6248: 4f407039f4a40b1a7de2974a87f8aebc1782b74b @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5053: b90ebd9c21518f305a61ee50aea38462ef01e65c @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_13252: 90a1b356ee441124c988a62160e5f9caa276c190 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

90a1b356ee44 drm/i915: Drop the _INCOMPLETE for has_infoframe
2b517a2a0592 drm/i915: Make pipe_config_err() vs. fastset less confusing
2e4eb369f0c4 drm/i915: Constify intel_pipe_config_compare()
bc29623ab5e2 drm/i915: Don't clobber M/N values during fastset check

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13252/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 3/4] drm/i915: Make pipe_config_err() vs. fastset less confusing
  2019-06-12 13:08 ` [PATCH 3/4] drm/i915: Make pipe_config_err() vs. fastset less confusing Ville Syrjala
@ 2019-06-12 15:43   ` Imre Deak
  0 siblings, 0 replies; 19+ messages in thread
From: Imre Deak @ 2019-06-12 15:43 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

On Wed, Jun 12, 2019 at 04:08:00PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Rename pipe_config_err() to pipe_config_mismatch(), and also print
> whether we're doing the fastset check or the sw vs. hw state readout
> check. Should make the logs a bit less confusing when they're not
> filled with what looks like a real error.
> 
> Also rename the 'adjust' variable to 'fastset' to make it clear what
> it means.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Makes it easier to differentiate the two reason for a comparison
mismatch in the log at a glance:

Reviewed-by: Imre Deak <imre.deak@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_display.c | 158 +++++++++++++--------------
>  1 file changed, 79 insertions(+), 79 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index dcc301df6174..ec3b7e825613 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -12309,16 +12309,16 @@ intel_compare_infoframe(const union hdmi_infoframe *a,
>  }
>  
>  static void
> -pipe_config_infoframe_err(struct drm_i915_private *dev_priv,
> -			  bool adjust, const char *name,
> -			  const union hdmi_infoframe *a,
> -			  const union hdmi_infoframe *b)
> +pipe_config_infoframe_mismatch(struct drm_i915_private *dev_priv,
> +			       bool fastset, const char *name,
> +			       const union hdmi_infoframe *a,
> +			       const union hdmi_infoframe *b)
>  {
> -	if (adjust) {
> +	if (fastset) {
>  		if ((drm_debug & DRM_UT_KMS) == 0)
>  			return;
>  
> -		drm_dbg(DRM_UT_KMS, "mismatch in %s infoframe", name);
> +		drm_dbg(DRM_UT_KMS, "fastset mismatch in %s infoframe", name);
>  		drm_dbg(DRM_UT_KMS, "expected:");
>  		hdmi_infoframe_log(KERN_DEBUG, dev_priv->drm.dev, a);
>  		drm_dbg(DRM_UT_KMS, "found");
> @@ -12333,7 +12333,7 @@ pipe_config_infoframe_err(struct drm_i915_private *dev_priv,
>  }
>  
>  static void __printf(3, 4)
> -pipe_config_err(bool adjust, const char *name, const char *format, ...)
> +pipe_config_mismatch(bool fastset, const char *name, const char *format, ...)
>  {
>  	struct va_format vaf;
>  	va_list args;
> @@ -12342,8 +12342,8 @@ pipe_config_err(bool adjust, const char *name, const char *format, ...)
>  	vaf.fmt = format;
>  	vaf.va = &args;
>  
> -	if (adjust)
> -		drm_dbg(DRM_UT_KMS, "mismatch in %s %pV", name, &vaf);
> +	if (fastset)
> +		drm_dbg(DRM_UT_KMS, "fastset mismatch in %s %pV", name, &vaf);
>  	else
>  		drm_err("mismatch in %s %pV", name, &vaf);
>  
> @@ -12370,11 +12370,11 @@ static bool fastboot_enabled(struct drm_i915_private *dev_priv)
>  static bool
>  intel_pipe_config_compare(const struct intel_crtc_state *current_config,
>  			  const struct intel_crtc_state *pipe_config,
> -			  bool adjust)
> +			  bool fastset)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(current_config->base.crtc->dev);
>  	bool ret = true;
> -	bool fixup_inherited = adjust &&
> +	bool fixup_inherited = fastset &&
>  		(current_config->base.mode.private_flags & I915_MODE_FLAG_INHERITED) &&
>  		!(pipe_config->base.mode.private_flags & I915_MODE_FLAG_INHERITED);
>  
> @@ -12385,30 +12385,30 @@ 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_err(adjust, __stringify(name), \
> -			  "(expected 0x%08x, found 0x%08x)\n", \
> -			  current_config->name, \
> -			  pipe_config->name); \
> +		pipe_config_mismatch(fastset, __stringify(name), \
> +				     "(expected 0x%08x, found 0x%08x)\n", \
> +				     current_config->name, \
> +				     pipe_config->name); \
>  		ret = false; \
>  	} \
>  } while (0)
>  
>  #define PIPE_CONF_CHECK_I(name) do { \
>  	if (current_config->name != pipe_config->name) { \
> -		pipe_config_err(adjust, __stringify(name), \
> -			  "(expected %i, found %i)\n", \
> -			  current_config->name, \
> -			  pipe_config->name); \
> +		pipe_config_mismatch(fastset, __stringify(name), \
> +				     "(expected %i, found %i)\n", \
> +				     current_config->name, \
> +				     pipe_config->name); \
>  		ret = false; \
>  	} \
>  } while (0)
>  
>  #define PIPE_CONF_CHECK_BOOL(name) do { \
>  	if (current_config->name != pipe_config->name) { \
> -		pipe_config_err(adjust, __stringify(name), \
> -			  "(expected %s, found %s)\n", \
> -			  yesno(current_config->name), \
> -			  yesno(pipe_config->name)); \
> +		pipe_config_mismatch(fastset, __stringify(name), \
> +				     "(expected %s, found %s)\n", \
> +				     yesno(current_config->name), \
> +				     yesno(pipe_config->name)); \
>  		ret = false; \
>  	} \
>  } while (0)
> @@ -12422,20 +12422,20 @@ 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_err(adjust, __stringify(name), \
> -			  "unable to verify whether state matches exactly, forcing modeset (expected %s, found %s)\n", \
> -			  yesno(current_config->name), \
> -			  yesno(pipe_config->name)); \
> +		pipe_config_mismatch(fastset, __stringify(name), \
> +				     "unable to verify whether state matches exactly, forcing modeset (expected %s, found %s)\n", \
> +				     yesno(current_config->name), \
> +				     yesno(pipe_config->name)); \
>  		ret = false; \
>  	} \
>  } while (0)
>  
>  #define PIPE_CONF_CHECK_P(name) do { \
>  	if (current_config->name != pipe_config->name) { \
> -		pipe_config_err(adjust, __stringify(name), \
> -			  "(expected %p, found %p)\n", \
> -			  current_config->name, \
> -			  pipe_config->name); \
> +		pipe_config_mismatch(fastset, __stringify(name), \
> +				     "(expected %p, found %p)\n", \
> +				     current_config->name, \
> +				     pipe_config->name); \
>  		ret = false; \
>  	} \
>  } while (0)
> @@ -12443,20 +12443,20 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
>  #define PIPE_CONF_CHECK_M_N(name) do { \
>  	if (!intel_compare_link_m_n(&current_config->name, \
>  				    &pipe_config->name,\
> -				    !adjust)) { \
> -		pipe_config_err(adjust, __stringify(name), \
> -			  "(expected tu %i gmch %i/%i link %i/%i, " \
> -			  "found tu %i, gmch %i/%i link %i/%i)\n", \
> -			  current_config->name.tu, \
> -			  current_config->name.gmch_m, \
> -			  current_config->name.gmch_n, \
> -			  current_config->name.link_m, \
> -			  current_config->name.link_n, \
> -			  pipe_config->name.tu, \
> -			  pipe_config->name.gmch_m, \
> -			  pipe_config->name.gmch_n, \
> -			  pipe_config->name.link_m, \
> -			  pipe_config->name.link_n); \
> +				    !fastset)) { \
> +		pipe_config_mismatch(fastset, __stringify(name), \
> +				     "(expected tu %i gmch %i/%i link %i/%i, " \
> +				     "found tu %i, gmch %i/%i link %i/%i)\n", \
> +				     current_config->name.tu, \
> +				     current_config->name.gmch_m, \
> +				     current_config->name.gmch_n, \
> +				     current_config->name.link_m, \
> +				     current_config->name.link_n, \
> +				     pipe_config->name.tu, \
> +				     pipe_config->name.gmch_m, \
> +				     pipe_config->name.gmch_n, \
> +				     pipe_config->name.link_m, \
> +				     pipe_config->name.link_n); \
>  		ret = false; \
>  	} \
>  } while (0)
> @@ -12468,49 +12468,49 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
>   */
>  #define PIPE_CONF_CHECK_M_N_ALT(name, alt_name) do { \
>  	if (!intel_compare_link_m_n(&current_config->name, \
> -				    &pipe_config->name, !adjust) && \
> +				    &pipe_config->name, !fastset) && \
>  	    !intel_compare_link_m_n(&current_config->alt_name, \
> -				    &pipe_config->name, !adjust)) { \
> -		pipe_config_err(adjust, __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)\n", \
> -			  current_config->name.tu, \
> -			  current_config->name.gmch_m, \
> -			  current_config->name.gmch_n, \
> -			  current_config->name.link_m, \
> -			  current_config->name.link_n, \
> -			  current_config->alt_name.tu, \
> -			  current_config->alt_name.gmch_m, \
> -			  current_config->alt_name.gmch_n, \
> -			  current_config->alt_name.link_m, \
> -			  current_config->alt_name.link_n, \
> -			  pipe_config->name.tu, \
> -			  pipe_config->name.gmch_m, \
> -			  pipe_config->name.gmch_n, \
> -			  pipe_config->name.link_m, \
> -			  pipe_config->name.link_n); \
> +				    &pipe_config->name, !fastset)) { \
> +		pipe_config_mismatch(fastset, __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)\n", \
> +				     current_config->name.tu, \
> +				     current_config->name.gmch_m, \
> +				     current_config->name.gmch_n, \
> +				     current_config->name.link_m, \
> +				     current_config->name.link_n, \
> +				     current_config->alt_name.tu, \
> +				     current_config->alt_name.gmch_m, \
> +				     current_config->alt_name.gmch_n, \
> +				     current_config->alt_name.link_m, \
> +				     current_config->alt_name.link_n, \
> +				     pipe_config->name.tu, \
> +				     pipe_config->name.gmch_m, \
> +				     pipe_config->name.gmch_n, \
> +				     pipe_config->name.link_m, \
> +				     pipe_config->name.link_n); \
>  		ret = false; \
>  	} \
>  } while (0)
>  
>  #define PIPE_CONF_CHECK_FLAGS(name, mask) do { \
>  	if ((current_config->name ^ pipe_config->name) & (mask)) { \
> -		pipe_config_err(adjust, __stringify(name), \
> -			  "(%x) (expected %i, found %i)\n", \
> -			  (mask), \
> -			  current_config->name & (mask), \
> -			  pipe_config->name & (mask)); \
> +		pipe_config_mismatch(fastset, __stringify(name), \
> +				     "(%x) (expected %i, found %i)\n", \
> +				     (mask), \
> +				     current_config->name & (mask), \
> +				     pipe_config->name & (mask)); \
>  		ret = false; \
>  	} \
>  } while (0)
>  
>  #define PIPE_CONF_CHECK_CLOCK_FUZZY(name) do { \
>  	if (!intel_fuzzy_clock_check(current_config->name, pipe_config->name)) { \
> -		pipe_config_err(adjust, __stringify(name), \
> -			  "(expected %i, found %i)\n", \
> -			  current_config->name, \
> -			  pipe_config->name); \
> +		pipe_config_mismatch(fastset, __stringify(name), \
> +				     "(expected %i, found %i)\n", \
> +				     current_config->name, \
> +				     pipe_config->name); \
>  		ret = false; \
>  	} \
>  } while (0)
> @@ -12518,9 +12518,9 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
>  #define PIPE_CONF_CHECK_INFOFRAME(name) do { \
>  	if (!intel_compare_infoframe(&current_config->infoframes.name, \
>  				     &pipe_config->infoframes.name)) { \
> -		pipe_config_infoframe_err(dev_priv, adjust, __stringify(name), \
> -					  &current_config->infoframes.name, \
> -					  &pipe_config->infoframes.name); \
> +		pipe_config_infoframe_mismatch(dev_priv, fastset, __stringify(name), \
> +					       &current_config->infoframes.name, \
> +					       &pipe_config->infoframes.name); \
>  		ret = false; \
>  	} \
>  } while (0)
> @@ -12600,7 +12600,7 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
>  	 */
>  	PIPE_CONF_CHECK_BOOL(pch_pfit.force_thru);
>  
> -	if (!adjust) {
> +	if (!fastset) {
>  		PIPE_CONF_CHECK_I(pipe_src_w);
>  		PIPE_CONF_CHECK_I(pipe_src_h);
>  
> -- 
> 2.21.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v2 1/4] drm/i915: Don't clobber M/N values during fastset check
  2019-06-12 13:07 [PATCH 1/4] drm/i915: Don't clobber M/N values during fastset check Ville Syrjala
                   ` (4 preceding siblings ...)
  2019-06-12 13:52 ` ✗ Fi.CI.BAT: failure " Patchwork
@ 2019-06-12 17:24 ` Ville Syrjala
  2019-06-13  9:24     ` Ville Syrjälä
  2019-06-15 22:16   ` Sasha Levin
  2019-06-12 18:35 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [v2,1/4] drm/i915: Don't clobber M/N values during fastset check (rev2) Patchwork
                   ` (2 subsequent siblings)
  8 siblings, 2 replies; 19+ messages in thread
From: Ville Syrjala @ 2019-06-12 17:24 UTC (permalink / raw)
  To: intel-gfx; +Cc: stable, Blubberbub, Maarten Lankhorst, Hans de Goede

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

We're now calling intel_pipe_config_compare(..., true) uncoditionally
which means we're always going clobber the calculated M/N values with
the old values if the fuzzy M/N check passes. That causes problems
because the fuzzy check allows for a huge difference in the values.

I'm actually tempted to just make the M/N checks exact, but that might
prevent fastboot from kicking in when people want it. So for now let's
overwrite the computed values with the old values only if decide to skip
the modeset.

v2: Copy has_drrs along with M/N M2/N2 values

Cc: stable@vger.kernel.org
Cc: Blubberbub@protonmail.com
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Hans de Goede <hdegoede@redhat.com>
Tested-by: Blubberbub@protonmail.com
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110782
Fixes: d19f958db23c ("drm/i915: Enable fastset for non-boot modesets.")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 36 +++++++++++++++++++++-------
 1 file changed, 28 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 1b1ddb48ca7a..3d8ed1cf0ab7 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12299,9 +12299,6 @@ intel_compare_link_m_n(const struct intel_link_m_n *m_n,
 			      m2_n2->gmch_m, m2_n2->gmch_n, !adjust) &&
 	    intel_compare_m_n(m_n->link_m, m_n->link_n,
 			      m2_n2->link_m, m2_n2->link_n, !adjust)) {
-		if (adjust)
-			*m2_n2 = *m_n;
-
 		return true;
 	}
 
@@ -13433,6 +13430,33 @@ static int calc_watermark_data(struct intel_atomic_state *state)
 	return 0;
 }
 
+static void intel_crtc_check_fastset(struct intel_crtc_state *old_crtc_state,
+				     struct intel_crtc_state *new_crtc_state)
+{
+	struct drm_i915_private *dev_priv =
+		to_i915(new_crtc_state->base.crtc->dev);
+
+	if (!intel_pipe_config_compare(dev_priv, old_crtc_state,
+				       new_crtc_state, true))
+		return;
+
+	new_crtc_state->base.mode_changed = false;
+	new_crtc_state->update_pipe = true;
+
+	/*
+	 * If we're not doing the full modeset we want to
+	 * keep the current M/N values as they may be
+	 * sufficiently different to the computed values
+	 * to cause problems.
+	 *
+	 * FIXME: should really copy more fuzzy state here
+	 */
+	new_crtc_state->fdi_m_n = old_crtc_state->fdi_m_n;
+	new_crtc_state->dp_m_n = old_crtc_state->dp_m_n;
+	new_crtc_state->dp_m2_n2 = old_crtc_state->dp_m2_n2;
+	new_crtc_state->has_drrs = old_crtc_state->has_drrs;
+}
+
 /**
  * intel_atomic_check - validate state object
  * @dev: drm device
@@ -13474,11 +13498,7 @@ static int intel_atomic_check(struct drm_device *dev,
 		if (ret)
 			goto fail;
 
-		if (intel_pipe_config_compare(dev_priv, old_crtc_state,
-					      new_crtc_state, true)) {
-			new_crtc_state->base.mode_changed = false;
-			new_crtc_state->update_pipe = true;
-		}
+		intel_crtc_check_fastset(old_crtc_state, new_crtc_state);
 
 		if (needs_modeset(&new_crtc_state->base))
 			any_ms = true;
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* ✗ Fi.CI.CHECKPATCH: warning for series starting with [v2,1/4] drm/i915: Don't clobber M/N values during fastset check (rev2)
  2019-06-12 13:07 [PATCH 1/4] drm/i915: Don't clobber M/N values during fastset check Ville Syrjala
                   ` (5 preceding siblings ...)
  2019-06-12 17:24 ` [PATCH v2 1/4] " Ville Syrjala
@ 2019-06-12 18:35 ` Patchwork
  2019-06-12 19:06 ` ✓ Fi.CI.BAT: success " Patchwork
  2019-06-14 13:14 ` ✓ Fi.CI.IGT: " Patchwork
  8 siblings, 0 replies; 19+ messages in thread
From: Patchwork @ 2019-06-12 18:35 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

== Series Details ==

Series: series starting with [v2,1/4] drm/i915: Don't clobber M/N values during fastset check (rev2)
URL   : https://patchwork.freedesktop.org/series/61960/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
f31b23cc1b1a drm/i915: Don't clobber M/N values during fastset check
51ce57812d1c drm/i915: Constify intel_pipe_config_compare()
b7b9d9ff1aa9 drm/i915: Make pipe_config_err() vs. fastset less confusing
-:133: WARNING:LONG_LINE: line over 100 characters
#133: FILE: drivers/gpu/drm/i915/intel_display.c:12426:
+				     "unable to verify whether state matches exactly, forcing modeset (expected %s, found %s)\n", \

total: 0 errors, 1 warnings, 0 checks, 256 lines checked
0bdea677061d drm/i915: Drop the _INCOMPLETE for has_infoframe

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* ✓ Fi.CI.BAT: success for series starting with [v2,1/4] drm/i915: Don't clobber M/N values during fastset check (rev2)
  2019-06-12 13:07 [PATCH 1/4] drm/i915: Don't clobber M/N values during fastset check Ville Syrjala
                   ` (6 preceding siblings ...)
  2019-06-12 18:35 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [v2,1/4] drm/i915: Don't clobber M/N values during fastset check (rev2) Patchwork
@ 2019-06-12 19:06 ` Patchwork
  2019-06-14 13:14 ` ✓ Fi.CI.IGT: " Patchwork
  8 siblings, 0 replies; 19+ messages in thread
From: Patchwork @ 2019-06-12 19:06 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

== Series Details ==

Series: series starting with [v2,1/4] drm/i915: Don't clobber M/N values during fastset check (rev2)
URL   : https://patchwork.freedesktop.org/series/61960/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6253 -> Patchwork_13257
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13257/

Known issues
------------

  Here are the changes found in Patchwork_13257 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_basic@bad-close:
    - fi-icl-y:           [PASS][1] -> [INCOMPLETE][2] ([fdo#107713])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6253/fi-icl-y/igt@gem_basic@bad-close.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13257/fi-icl-y/igt@gem_basic@bad-close.html

  
#### Possible fixes ####

  * igt@gem_busy@busy-all:
    - fi-icl-guc:         [INCOMPLETE][3] ([fdo#107713]) -> [PASS][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6253/fi-icl-guc/igt@gem_busy@busy-all.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13257/fi-icl-guc/igt@gem_busy@busy-all.html

  * igt@gem_ctx_create@basic-files:
    - fi-icl-u2:          [INCOMPLETE][5] ([fdo#107713] / [fdo#109100]) -> [PASS][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6253/fi-icl-u2/igt@gem_ctx_create@basic-files.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13257/fi-icl-u2/igt@gem_ctx_create@basic-files.html

  
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#109100]: https://bugs.freedesktop.org/show_bug.cgi?id=109100


Participating hosts (53 -> 47)
------------------------------

  Additional (1): fi-icl-dsi 
  Missing    (7): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-byt-clapper fi-bdw-samus 


Build changes
-------------

  * Linux: CI_DRM_6253 -> Patchwork_13257

  CI_DRM_6253: 83fdc69645c5c6b511e36e171f1c75a6132f007c @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5054: 7a295df596fdf71e5c28ecb1fbfec002060e9293 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_13257: 0bdea677061d4717d0130af46e6afb28c0bdcbb6 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

0bdea677061d drm/i915: Drop the _INCOMPLETE for has_infoframe
b7b9d9ff1aa9 drm/i915: Make pipe_config_err() vs. fastset less confusing
51ce57812d1c drm/i915: Constify intel_pipe_config_compare()
f31b23cc1b1a drm/i915: Don't clobber M/N values during fastset check

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13257/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 1/4] drm/i915: Don't clobber M/N values during fastset check
  2019-06-12 17:24 ` [PATCH v2 1/4] " Ville Syrjala
@ 2019-06-13  9:24     ` Ville Syrjälä
  2019-06-15 22:16   ` Sasha Levin
  1 sibling, 0 replies; 19+ messages in thread
From: Ville Syrjälä @ 2019-06-13  9:24 UTC (permalink / raw)
  To: intel-gfx; +Cc: stable, Blubberbub, Maarten Lankhorst, Hans de Goede

On Wed, Jun 12, 2019 at 08:24:23PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> We're now calling intel_pipe_config_compare(..., true) uncoditionally
> which means we're always going clobber the calculated M/N values with
> the old values if the fuzzy M/N check passes. That causes problems
> because the fuzzy check allows for a huge difference in the values.
> 
> I'm actually tempted to just make the M/N checks exact, but that might
> prevent fastboot from kicking in when people want it. So for now let's
> overwrite the computed values with the old values only if decide to skip
> the modeset.
> 
> v2: Copy has_drrs along with M/N M2/N2 values
> 
> Cc: stable@vger.kernel.org
> Cc: Blubberbub@protonmail.com
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Hans de Goede <hdegoede@redhat.com>
> Tested-by: Blubberbub@protonmail.com
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110782
> Fixes: d19f958db23c ("drm/i915: Enable fastset for non-boot modesets.")
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Looks like also
https://bugs.freedesktop.org/show_bug.cgi?id=110675

> ---
>  drivers/gpu/drm/i915/intel_display.c | 36 +++++++++++++++++++++-------
>  1 file changed, 28 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 1b1ddb48ca7a..3d8ed1cf0ab7 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -12299,9 +12299,6 @@ intel_compare_link_m_n(const struct intel_link_m_n *m_n,
>  			      m2_n2->gmch_m, m2_n2->gmch_n, !adjust) &&
>  	    intel_compare_m_n(m_n->link_m, m_n->link_n,
>  			      m2_n2->link_m, m2_n2->link_n, !adjust)) {
> -		if (adjust)
> -			*m2_n2 = *m_n;
> -
>  		return true;
>  	}
>  
> @@ -13433,6 +13430,33 @@ static int calc_watermark_data(struct intel_atomic_state *state)
>  	return 0;
>  }
>  
> +static void intel_crtc_check_fastset(struct intel_crtc_state *old_crtc_state,
> +				     struct intel_crtc_state *new_crtc_state)
> +{
> +	struct drm_i915_private *dev_priv =
> +		to_i915(new_crtc_state->base.crtc->dev);
> +
> +	if (!intel_pipe_config_compare(dev_priv, old_crtc_state,
> +				       new_crtc_state, true))
> +		return;
> +
> +	new_crtc_state->base.mode_changed = false;
> +	new_crtc_state->update_pipe = true;
> +
> +	/*
> +	 * If we're not doing the full modeset we want to
> +	 * keep the current M/N values as they may be
> +	 * sufficiently different to the computed values
> +	 * to cause problems.
> +	 *
> +	 * FIXME: should really copy more fuzzy state here
> +	 */
> +	new_crtc_state->fdi_m_n = old_crtc_state->fdi_m_n;
> +	new_crtc_state->dp_m_n = old_crtc_state->dp_m_n;
> +	new_crtc_state->dp_m2_n2 = old_crtc_state->dp_m2_n2;
> +	new_crtc_state->has_drrs = old_crtc_state->has_drrs;
> +}
> +
>  /**
>   * intel_atomic_check - validate state object
>   * @dev: drm device
> @@ -13474,11 +13498,7 @@ static int intel_atomic_check(struct drm_device *dev,
>  		if (ret)
>  			goto fail;
>  
> -		if (intel_pipe_config_compare(dev_priv, old_crtc_state,
> -					      new_crtc_state, true)) {
> -			new_crtc_state->base.mode_changed = false;
> -			new_crtc_state->update_pipe = true;
> -		}
> +		intel_crtc_check_fastset(old_crtc_state, new_crtc_state);
>  
>  		if (needs_modeset(&new_crtc_state->base))
>  			any_ms = true;
> -- 
> 2.21.0

-- 
Ville Syrjälä
Intel

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 1/4] drm/i915: Don't clobber M/N values during fastset check
@ 2019-06-13  9:24     ` Ville Syrjälä
  0 siblings, 0 replies; 19+ messages in thread
From: Ville Syrjälä @ 2019-06-13  9:24 UTC (permalink / raw)
  To: intel-gfx; +Cc: Blubberbub, stable

On Wed, Jun 12, 2019 at 08:24:23PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> We're now calling intel_pipe_config_compare(..., true) uncoditionally
> which means we're always going clobber the calculated M/N values with
> the old values if the fuzzy M/N check passes. That causes problems
> because the fuzzy check allows for a huge difference in the values.
> 
> I'm actually tempted to just make the M/N checks exact, but that might
> prevent fastboot from kicking in when people want it. So for now let's
> overwrite the computed values with the old values only if decide to skip
> the modeset.
> 
> v2: Copy has_drrs along with M/N M2/N2 values
> 
> Cc: stable@vger.kernel.org
> Cc: Blubberbub@protonmail.com
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Hans de Goede <hdegoede@redhat.com>
> Tested-by: Blubberbub@protonmail.com
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110782
> Fixes: d19f958db23c ("drm/i915: Enable fastset for non-boot modesets.")
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Looks like also
https://bugs.freedesktop.org/show_bug.cgi?id=110675

> ---
>  drivers/gpu/drm/i915/intel_display.c | 36 +++++++++++++++++++++-------
>  1 file changed, 28 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 1b1ddb48ca7a..3d8ed1cf0ab7 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -12299,9 +12299,6 @@ intel_compare_link_m_n(const struct intel_link_m_n *m_n,
>  			      m2_n2->gmch_m, m2_n2->gmch_n, !adjust) &&
>  	    intel_compare_m_n(m_n->link_m, m_n->link_n,
>  			      m2_n2->link_m, m2_n2->link_n, !adjust)) {
> -		if (adjust)
> -			*m2_n2 = *m_n;
> -
>  		return true;
>  	}
>  
> @@ -13433,6 +13430,33 @@ static int calc_watermark_data(struct intel_atomic_state *state)
>  	return 0;
>  }
>  
> +static void intel_crtc_check_fastset(struct intel_crtc_state *old_crtc_state,
> +				     struct intel_crtc_state *new_crtc_state)
> +{
> +	struct drm_i915_private *dev_priv =
> +		to_i915(new_crtc_state->base.crtc->dev);
> +
> +	if (!intel_pipe_config_compare(dev_priv, old_crtc_state,
> +				       new_crtc_state, true))
> +		return;
> +
> +	new_crtc_state->base.mode_changed = false;
> +	new_crtc_state->update_pipe = true;
> +
> +	/*
> +	 * If we're not doing the full modeset we want to
> +	 * keep the current M/N values as they may be
> +	 * sufficiently different to the computed values
> +	 * to cause problems.
> +	 *
> +	 * FIXME: should really copy more fuzzy state here
> +	 */
> +	new_crtc_state->fdi_m_n = old_crtc_state->fdi_m_n;
> +	new_crtc_state->dp_m_n = old_crtc_state->dp_m_n;
> +	new_crtc_state->dp_m2_n2 = old_crtc_state->dp_m2_n2;
> +	new_crtc_state->has_drrs = old_crtc_state->has_drrs;
> +}
> +
>  /**
>   * intel_atomic_check - validate state object
>   * @dev: drm device
> @@ -13474,11 +13498,7 @@ static int intel_atomic_check(struct drm_device *dev,
>  		if (ret)
>  			goto fail;
>  
> -		if (intel_pipe_config_compare(dev_priv, old_crtc_state,
> -					      new_crtc_state, true)) {
> -			new_crtc_state->base.mode_changed = false;
> -			new_crtc_state->update_pipe = true;
> -		}
> +		intel_crtc_check_fastset(old_crtc_state, new_crtc_state);
>  
>  		if (needs_modeset(&new_crtc_state->base))
>  			any_ms = true;
> -- 
> 2.21.0

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* ✓ Fi.CI.IGT: success for series starting with [v2,1/4] drm/i915: Don't clobber M/N values during fastset check (rev2)
  2019-06-12 13:07 [PATCH 1/4] drm/i915: Don't clobber M/N values during fastset check Ville Syrjala
                   ` (7 preceding siblings ...)
  2019-06-12 19:06 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2019-06-14 13:14 ` Patchwork
  8 siblings, 0 replies; 19+ messages in thread
From: Patchwork @ 2019-06-14 13:14 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

== Series Details ==

Series: series starting with [v2,1/4] drm/i915: Don't clobber M/N values during fastset check (rev2)
URL   : https://patchwork.freedesktop.org/series/61960/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6253_full -> Patchwork_13257_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Known issues
------------

  Here are the changes found in Patchwork_13257_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_eio@in-flight-contexts-1us:
    - shard-apl:          [PASS][1] -> [DMESG-WARN][2] ([fdo#110913 ]) +1 similar issue
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6253/shard-apl6/igt@gem_eio@in-flight-contexts-1us.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13257/shard-apl7/igt@gem_eio@in-flight-contexts-1us.html

  * igt@gem_persistent_relocs@forked-faulting-reloc-thrash-inactive:
    - shard-apl:          [PASS][3] -> [INCOMPLETE][4] ([fdo#103927])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6253/shard-apl8/igt@gem_persistent_relocs@forked-faulting-reloc-thrash-inactive.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13257/shard-apl8/igt@gem_persistent_relocs@forked-faulting-reloc-thrash-inactive.html

  * igt@gem_persistent_relocs@forked-faulting-reloc-thrashing:
    - shard-glk:          [PASS][5] -> [DMESG-WARN][6] ([fdo#110913 ])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6253/shard-glk9/igt@gem_persistent_relocs@forked-faulting-reloc-thrashing.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13257/shard-glk1/igt@gem_persistent_relocs@forked-faulting-reloc-thrashing.html

  * igt@gem_persistent_relocs@forked-interruptible-faulting-reloc-thrashing:
    - shard-hsw:          [PASS][7] -> [DMESG-WARN][8] ([fdo#110789] / [fdo#110913 ])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6253/shard-hsw4/igt@gem_persistent_relocs@forked-interruptible-faulting-reloc-thrashing.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13257/shard-hsw5/igt@gem_persistent_relocs@forked-interruptible-faulting-reloc-thrashing.html

  * igt@gem_persistent_relocs@forked-interruptible-thrashing:
    - shard-snb:          [PASS][9] -> [DMESG-WARN][10] ([fdo#110789] / [fdo#110913 ]) +2 similar issues
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6253/shard-snb1/igt@gem_persistent_relocs@forked-interruptible-thrashing.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13257/shard-snb5/igt@gem_persistent_relocs@forked-interruptible-thrashing.html
    - shard-skl:          [PASS][11] -> [DMESG-WARN][12] ([fdo#110913 ]) +3 similar issues
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6253/shard-skl4/igt@gem_persistent_relocs@forked-interruptible-thrashing.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13257/shard-skl8/igt@gem_persistent_relocs@forked-interruptible-thrashing.html

  * igt@gem_softpin@noreloc-s3:
    - shard-apl:          [PASS][13] -> [DMESG-WARN][14] ([fdo#108566]) +1 similar issue
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6253/shard-apl3/igt@gem_softpin@noreloc-s3.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13257/shard-apl8/igt@gem_softpin@noreloc-s3.html

  * igt@gem_userptr_blits@map-fixed-invalidate-busy-gup:
    - shard-kbl:          [PASS][15] -> [DMESG-WARN][16] ([fdo#110913 ]) +1 similar issue
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6253/shard-kbl4/igt@gem_userptr_blits@map-fixed-invalidate-busy-gup.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13257/shard-kbl2/igt@gem_userptr_blits@map-fixed-invalidate-busy-gup.html

  * igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy-gup:
    - shard-iclb:         [PASS][17] -> [DMESG-WARN][18] ([fdo#110913 ])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6253/shard-iclb5/igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy-gup.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13257/shard-iclb4/igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy-gup.html

  * igt@i915_selftest@live_hangcheck:
    - shard-snb:          [PASS][19] -> [INCOMPLETE][20] ([fdo#105411])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6253/shard-snb2/igt@i915_selftest@live_hangcheck.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13257/shard-snb5/igt@i915_selftest@live_hangcheck.html

  * igt@kms_cursor_crc@pipe-a-cursor-suspend:
    - shard-kbl:          [PASS][21] -> [DMESG-WARN][22] ([fdo#108566])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6253/shard-kbl2/igt@kms_cursor_crc@pipe-a-cursor-suspend.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13257/shard-kbl1/igt@kms_cursor_crc@pipe-a-cursor-suspend.html

  * igt@kms_cursor_crc@pipe-b-cursor-256x85-random:
    - shard-apl:          [PASS][23] -> [FAIL][24] ([fdo#103232]) +1 similar issue
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6253/shard-apl6/igt@kms_cursor_crc@pipe-b-cursor-256x85-random.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13257/shard-apl7/igt@kms_cursor_crc@pipe-b-cursor-256x85-random.html

  * igt@kms_cursor_crc@pipe-b-cursor-suspend:
    - shard-skl:          [PASS][25] -> [FAIL][26] ([fdo#103232]) +1 similar issue
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6253/shard-skl1/igt@kms_cursor_crc@pipe-b-cursor-suspend.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13257/shard-skl9/igt@kms_cursor_crc@pipe-b-cursor-suspend.html
    - shard-kbl:          [PASS][27] -> [FAIL][28] ([fdo#103232]) +1 similar issue
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6253/shard-kbl4/igt@kms_cursor_crc@pipe-b-cursor-suspend.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13257/shard-kbl3/igt@kms_cursor_crc@pipe-b-cursor-suspend.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-indfb-pgflip-blt:
    - shard-iclb:         [PASS][29] -> [FAIL][30] ([fdo#103167]) +5 similar issues
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6253/shard-iclb4/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-indfb-pgflip-blt.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13257/shard-iclb7/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-indfb-pgflip-blt.html

  * igt@kms_psr@psr2_sprite_mmap_gtt:
    - shard-iclb:         [PASS][31] -> [SKIP][32] ([fdo#109441]) +2 similar issues
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6253/shard-iclb2/igt@kms_psr@psr2_sprite_mmap_gtt.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13257/shard-iclb1/igt@kms_psr@psr2_sprite_mmap_gtt.html

  
#### Possible fixes ####

  * igt@gem_eio@suspend:
    - shard-skl:          [DMESG-WARN][33] ([fdo#110913 ]) -> [PASS][34] +4 similar issues
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6253/shard-skl1/igt@gem_eio@suspend.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13257/shard-skl1/igt@gem_eio@suspend.html

  * igt@gem_persistent_relocs@forked-faulting-reloc-thrashing:
    - shard-hsw:          [DMESG-WARN][35] ([fdo#110789] / [fdo#110913 ]) -> [PASS][36] +1 similar issue
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6253/shard-hsw2/igt@gem_persistent_relocs@forked-faulting-reloc-thrashing.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13257/shard-hsw8/igt@gem_persistent_relocs@forked-faulting-reloc-thrashing.html

  * igt@gem_persistent_relocs@forked-interruptible-faulting-reloc-thrashing:
    - shard-snb:          [DMESG-WARN][37] ([fdo#110789] / [fdo#110913 ]) -> [PASS][38] +1 similar issue
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6253/shard-snb4/igt@gem_persistent_relocs@forked-interruptible-faulting-reloc-thrashing.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13257/shard-snb7/igt@gem_persistent_relocs@forked-interruptible-faulting-reloc-thrashing.html
    - shard-kbl:          [DMESG-WARN][39] ([fdo#110913 ]) -> [PASS][40]
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6253/shard-kbl4/igt@gem_persistent_relocs@forked-interruptible-faulting-reloc-thrashing.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13257/shard-kbl3/igt@gem_persistent_relocs@forked-interruptible-faulting-reloc-thrashing.html

  * igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy:
    - shard-iclb:         [DMESG-WARN][41] ([fdo#110913 ]) -> [PASS][42]
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6253/shard-iclb5/igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13257/shard-iclb4/igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy.html
    - shard-hsw:          [DMESG-WARN][43] ([fdo#110913 ]) -> [PASS][44]
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6253/shard-hsw6/igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13257/shard-hsw2/igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy.html

  * igt@i915_suspend@fence-restore-untiled:
    - shard-skl:          [INCOMPLETE][45] ([fdo#104108]) -> [PASS][46]
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6253/shard-skl6/igt@i915_suspend@fence-restore-untiled.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13257/shard-skl4/igt@i915_suspend@fence-restore-untiled.html

  * igt@kms_cursor_crc@pipe-b-cursor-64x21-random:
    - shard-apl:          [FAIL][47] ([fdo#103232]) -> [PASS][48]
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6253/shard-apl2/igt@kms_cursor_crc@pipe-b-cursor-64x21-random.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13257/shard-apl2/igt@kms_cursor_crc@pipe-b-cursor-64x21-random.html

  * igt@kms_flip@2x-flip-vs-modeset-vs-hang-interruptible:
    - shard-hsw:          [INCOMPLETE][49] ([fdo#103540]) -> [PASS][50]
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6253/shard-hsw4/igt@kms_flip@2x-flip-vs-modeset-vs-hang-interruptible.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13257/shard-hsw4/igt@kms_flip@2x-flip-vs-modeset-vs-hang-interruptible.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-blt:
    - shard-iclb:         [FAIL][51] ([fdo#103167]) -> [PASS][52] +4 similar issues
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6253/shard-iclb7/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-blt.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13257/shard-iclb3/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-blt.html

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-cur-indfb-draw-pwrite:
    - shard-hsw:          [SKIP][53] ([fdo#109271]) -> [PASS][54] +28 similar issues
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6253/shard-hsw1/igt@kms_frontbuffer_tracking@fbc-2p-primscrn-cur-indfb-draw-pwrite.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13257/shard-hsw7/igt@kms_frontbuffer_tracking@fbc-2p-primscrn-cur-indfb-draw-pwrite.html

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
    - shard-apl:          [DMESG-WARN][55] ([fdo#108566]) -> [PASS][56] +2 similar issues
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6253/shard-apl3/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13257/shard-apl2/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c.html

  * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
    - shard-skl:          [FAIL][57] ([fdo#108145] / [fdo#110403]) -> [PASS][58] +1 similar issue
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6253/shard-skl6/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13257/shard-skl8/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html

  * igt@kms_plane_lowres@pipe-a-tiling-x:
    - shard-iclb:         [FAIL][59] ([fdo#103166]) -> [PASS][60]
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6253/shard-iclb7/igt@kms_plane_lowres@pipe-a-tiling-x.html
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13257/shard-iclb3/igt@kms_plane_lowres@pipe-a-tiling-x.html

  * igt@kms_psr@no_drrs:
    - shard-iclb:         [FAIL][61] ([fdo#108341]) -> [PASS][62]
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6253/shard-iclb1/igt@kms_psr@no_drrs.html
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13257/shard-iclb6/igt@kms_psr@no_drrs.html

  * igt@kms_psr@psr2_sprite_plane_move:
    - shard-iclb:         [SKIP][63] ([fdo#109441]) -> [PASS][64]
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6253/shard-iclb6/igt@kms_psr@psr2_sprite_plane_move.html
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13257/shard-iclb2/igt@kms_psr@psr2_sprite_plane_move.html

  * igt@perf_pmu@rc6-runtime-pm-long:
    - shard-kbl:          [FAIL][65] ([fdo#105010]) -> [PASS][66]
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6253/shard-kbl7/igt@perf_pmu@rc6-runtime-pm-long.html
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13257/shard-kbl3/igt@perf_pmu@rc6-runtime-pm-long.html

  
#### Warnings ####

  * igt@kms_frontbuffer_tracking@psr-2p-primscrn-shrfb-pgflip-blt:
    - shard-apl:          [INCOMPLETE][67] ([fdo#103927]) -> [SKIP][68] ([fdo#109271])
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6253/shard-apl2/igt@kms_frontbuffer_tracking@psr-2p-primscrn-shrfb-pgflip-blt.html
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13257/shard-apl7/igt@kms_frontbuffer_tracking@psr-2p-primscrn-shrfb-pgflip-blt.html

  
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103540]: https://bugs.freedesktop.org/show_bug.cgi?id=103540
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
  [fdo#105010]: https://bugs.freedesktop.org/show_bug.cgi?id=105010
  [fdo#105411]: https://bugs.freedesktop.org/show_bug.cgi?id=105411
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108341]: https://bugs.freedesktop.org/show_bug.cgi?id=108341
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#110403]: https://bugs.freedesktop.org/show_bug.cgi?id=110403
  [fdo#110789]: https://bugs.freedesktop.org/show_bug.cgi?id=110789
  [fdo#110913 ]: https://bugs.freedesktop.org/show_bug.cgi?id=110913 


Participating hosts (10 -> 10)
------------------------------

  No changes in participating hosts


Build changes
-------------

  * Linux: CI_DRM_6253 -> Patchwork_13257

  CI_DRM_6253: 83fdc69645c5c6b511e36e171f1c75a6132f007c @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5054: 7a295df596fdf71e5c28ecb1fbfec002060e9293 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_13257: 0bdea677061d4717d0130af46e6afb28c0bdcbb6 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13257/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 1/4] drm/i915: Don't clobber M/N values during fastset check
  2019-06-12 17:24 ` [PATCH v2 1/4] " Ville Syrjala
  2019-06-13  9:24     ` Ville Syrjälä
@ 2019-06-15 22:16   ` Sasha Levin
  1 sibling, 0 replies; 19+ messages in thread
From: Sasha Levin @ 2019-06-15 22:16 UTC (permalink / raw)
  To: Sasha Levin, Ville Syrjala, intel-gfx; +Cc: Blubberbub, stable

Hi,

[This is an automated email]

This commit has been processed because it contains a "Fixes:" tag,
fixing commit: d19f958db23c drm/i915: Enable fastset for non-boot modesets..

The bot has tested the following trees: v5.1.9.

v5.1.9: Failed to apply! Possible dependencies:
    1b9994c78977 ("drm/i915: Don't pass the crtc to intel_dump_pipe_config()")
    48d9f87ddd21 ("drm/i915: Save the old CDCLK atomic state")
    5643dd9c7af4 ("drm/i915: Use intel_ types in intel_modeset_checks()")
    59f9e9cab3a1 ("drm/i915: Skip modeset for cdclk changes if possible")
    85829eb5ee1a ("drm/i915: Pass intel_atomic state to check_digital_port_conflicts()")
    905801fe7237 ("drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio power is enabled")
    9a86a07c7e94 ("drm/i915: Use intel_ types in intel_atomic_check()")
    d31c85fc8642 ("snd/hda, drm/i915: Track the display_power_status using a cookie")
    f239b7998507 ("drm/i915: Don't pass the crtc to intel_modeset_pipe_config()")


How should we proceed with this patch?

--
Thanks,
Sasha
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Intel-gfx] [PATCH v2 1/4] drm/i915: Don't clobber M/N values during fastset check
  2019-06-13  9:24     ` Ville Syrjälä
  (?)
@ 2019-06-18 12:19     ` Imre Deak
  -1 siblings, 0 replies; 19+ messages in thread
From: Imre Deak @ 2019-06-18 12:19 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx, Blubberbub, stable

On Thu, Jun 13, 2019 at 12:24:59PM +0300, Ville Syrjälä wrote:
> On Wed, Jun 12, 2019 at 08:24:23PM +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > We're now calling intel_pipe_config_compare(..., true) uncoditionally
> > which means we're always going clobber the calculated M/N values with
> > the old values if the fuzzy M/N check passes. That causes problems
> > because the fuzzy check allows for a huge difference in the values.
> > 
> > I'm actually tempted to just make the M/N checks exact, but that might
> > prevent fastboot from kicking in when people want it. So for now let's
> > overwrite the computed values with the old values only if decide to skip
> > the modeset.
> > 
> > v2: Copy has_drrs along with M/N M2/N2 values
> > 
> > Cc: stable@vger.kernel.org
> > Cc: Blubberbub@protonmail.com
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Cc: Hans de Goede <hdegoede@redhat.com>
> > Tested-by: Blubberbub@protonmail.com
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110782
> > Fixes: d19f958db23c ("drm/i915: Enable fastset for non-boot modesets.")
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Looks like also
> https://bugs.freedesktop.org/show_bug.cgi?id=110675

Ok, the copying from old-state to new-state is needed to keep HW/SW
state verification later pass, but we want to preserve the calculated
state if we'll need to reprogram everything based on that. Makes sense:

Reviewed-by: Imre Deak <imre.deak@intel.com>

> 
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 36 +++++++++++++++++++++-------
> >  1 file changed, 28 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 1b1ddb48ca7a..3d8ed1cf0ab7 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -12299,9 +12299,6 @@ intel_compare_link_m_n(const struct intel_link_m_n *m_n,
> >  			      m2_n2->gmch_m, m2_n2->gmch_n, !adjust) &&
> >  	    intel_compare_m_n(m_n->link_m, m_n->link_n,
> >  			      m2_n2->link_m, m2_n2->link_n, !adjust)) {
> > -		if (adjust)
> > -			*m2_n2 = *m_n;
> > -
> >  		return true;
> >  	}
> >  
> > @@ -13433,6 +13430,33 @@ static int calc_watermark_data(struct intel_atomic_state *state)
> >  	return 0;
> >  }
> >  
> > +static void intel_crtc_check_fastset(struct intel_crtc_state *old_crtc_state,
> > +				     struct intel_crtc_state *new_crtc_state)
> > +{
> > +	struct drm_i915_private *dev_priv =
> > +		to_i915(new_crtc_state->base.crtc->dev);
> > +
> > +	if (!intel_pipe_config_compare(dev_priv, old_crtc_state,
> > +				       new_crtc_state, true))
> > +		return;
> > +
> > +	new_crtc_state->base.mode_changed = false;
> > +	new_crtc_state->update_pipe = true;
> > +
> > +	/*
> > +	 * If we're not doing the full modeset we want to
> > +	 * keep the current M/N values as they may be
> > +	 * sufficiently different to the computed values
> > +	 * to cause problems.
> > +	 *
> > +	 * FIXME: should really copy more fuzzy state here
> > +	 */
> > +	new_crtc_state->fdi_m_n = old_crtc_state->fdi_m_n;
> > +	new_crtc_state->dp_m_n = old_crtc_state->dp_m_n;
> > +	new_crtc_state->dp_m2_n2 = old_crtc_state->dp_m2_n2;
> > +	new_crtc_state->has_drrs = old_crtc_state->has_drrs;
> > +}
> > +
> >  /**
> >   * intel_atomic_check - validate state object
> >   * @dev: drm device
> > @@ -13474,11 +13498,7 @@ static int intel_atomic_check(struct drm_device *dev,
> >  		if (ret)
> >  			goto fail;
> >  
> > -		if (intel_pipe_config_compare(dev_priv, old_crtc_state,
> > -					      new_crtc_state, true)) {
> > -			new_crtc_state->base.mode_changed = false;
> > -			new_crtc_state->update_pipe = true;
> > -		}
> > +		intel_crtc_check_fastset(old_crtc_state, new_crtc_state);
> >  
> >  		if (needs_modeset(&new_crtc_state->base))
> >  			any_ms = true;
> > -- 
> > 2.21.0
> 
> -- 
> Ville Syrjälä
> Intel
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/4] drm/i915: Constify intel_pipe_config_compare()
  2019-06-12 13:07 ` [PATCH 2/4] drm/i915: Constify intel_pipe_config_compare() Ville Syrjala
@ 2019-06-18 12:25   ` Imre Deak
  0 siblings, 0 replies; 19+ messages in thread
From: Imre Deak @ 2019-06-18 12:25 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

On Wed, Jun 12, 2019 at 04:07:59PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Now that intel_pipe_config_compare() no longer clobbers the passed
> in state we can make both crtc states const. And while at we simplify
> the calling convention, and clean up intel_compare_link_m_n() a bit.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Imre Deak <imre.deak@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_display.c | 43 +++++++++++-----------------
>  1 file changed, 17 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 73b3e92b7ed5..dcc301df6174 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -12291,18 +12291,14 @@ intel_compare_m_n(unsigned int m, unsigned int n,
>  
>  static bool
>  intel_compare_link_m_n(const struct intel_link_m_n *m_n,
> -		       struct intel_link_m_n *m2_n2,
> -		       bool adjust)
> -{
> -	if (m_n->tu == m2_n2->tu &&
> -	    intel_compare_m_n(m_n->gmch_m, m_n->gmch_n,
> -			      m2_n2->gmch_m, m2_n2->gmch_n, !adjust) &&
> -	    intel_compare_m_n(m_n->link_m, m_n->link_n,
> -			      m2_n2->link_m, m2_n2->link_n, !adjust)) {
> -		return true;
> -	}
> -
> -	return false;
> +		       const struct intel_link_m_n *m2_n2,
> +		       bool exact)
> +{
> +	return m_n->tu == m2_n2->tu &&
> +		intel_compare_m_n(m_n->gmch_m, m_n->gmch_n,
> +				  m2_n2->gmch_m, m2_n2->gmch_n, exact) &&
> +		intel_compare_m_n(m_n->link_m, m_n->link_n,
> +				  m2_n2->link_m, m2_n2->link_n, exact);
>  }
>  
>  static bool
> @@ -12372,11 +12368,11 @@ static bool fastboot_enabled(struct drm_i915_private *dev_priv)
>  }
>  
>  static bool
> -intel_pipe_config_compare(struct drm_i915_private *dev_priv,
> -			  struct intel_crtc_state *current_config,
> -			  struct intel_crtc_state *pipe_config,
> +intel_pipe_config_compare(const struct intel_crtc_state *current_config,
> +			  const struct intel_crtc_state *pipe_config,
>  			  bool adjust)
>  {
> +	struct drm_i915_private *dev_priv = to_i915(current_config->base.crtc->dev);
>  	bool ret = true;
>  	bool fixup_inherited = adjust &&
>  		(current_config->base.mode.private_flags & I915_MODE_FLAG_INHERITED) &&
> @@ -12447,7 +12443,7 @@ intel_pipe_config_compare(struct drm_i915_private *dev_priv,
>  #define PIPE_CONF_CHECK_M_N(name) do { \
>  	if (!intel_compare_link_m_n(&current_config->name, \
>  				    &pipe_config->name,\
> -				    adjust)) { \
> +				    !adjust)) { \
>  		pipe_config_err(adjust, __stringify(name), \
>  			  "(expected tu %i gmch %i/%i link %i/%i, " \
>  			  "found tu %i, gmch %i/%i link %i/%i)\n", \
> @@ -12472,9 +12468,9 @@ intel_pipe_config_compare(struct drm_i915_private *dev_priv,
>   */
>  #define PIPE_CONF_CHECK_M_N_ALT(name, alt_name) do { \
>  	if (!intel_compare_link_m_n(&current_config->name, \
> -				    &pipe_config->name, adjust) && \
> +				    &pipe_config->name, !adjust) && \
>  	    !intel_compare_link_m_n(&current_config->alt_name, \
> -				    &pipe_config->name, adjust)) { \
> +				    &pipe_config->name, !adjust)) { \
>  		pipe_config_err(adjust, __stringify(name), \
>  			  "(expected tu %i gmch %i/%i link %i/%i, " \
>  			  "or tu %i gmch %i/%i link %i/%i, " \
> @@ -12984,8 +12980,7 @@ verify_crtc_state(struct drm_crtc *crtc,
>  	intel_pipe_config_sanity_check(dev_priv, pipe_config);
>  
>  	sw_config = to_intel_crtc_state(new_crtc_state);
> -	if (!intel_pipe_config_compare(dev_priv, sw_config,
> -				       pipe_config, false)) {
> +	if (!intel_pipe_config_compare(sw_config, pipe_config, false)) {
>  		I915_STATE_WARN(1, "pipe state doesn't match!\n");
>  		intel_dump_pipe_config(pipe_config, NULL, "[hw state]");
>  		intel_dump_pipe_config(sw_config, NULL, "[sw state]");
> @@ -13430,14 +13425,10 @@ static int calc_watermark_data(struct intel_atomic_state *state)
>  	return 0;
>  }
>  
> -static void intel_crtc_check_fastset(struct intel_crtc_state *old_crtc_state,
> +static void intel_crtc_check_fastset(const struct intel_crtc_state *old_crtc_state,
>  				     struct intel_crtc_state *new_crtc_state)
>  {
> -	struct drm_i915_private *dev_priv =
> -		to_i915(new_crtc_state->base.crtc->dev);
> -
> -	if (!intel_pipe_config_compare(dev_priv, old_crtc_state,
> -				       new_crtc_state, true))
> +	if (!intel_pipe_config_compare(old_crtc_state, new_crtc_state, true))
>  		return;
>  
>  	new_crtc_state->base.mode_changed = false;
> -- 
> 2.21.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 4/4] drm/i915: Drop the _INCOMPLETE for has_infoframe
  2019-06-12 13:08 ` [PATCH 4/4] drm/i915: Drop the _INCOMPLETE for has_infoframe Ville Syrjala
@ 2019-06-18 12:34   ` Imre Deak
  2019-06-18 12:55     ` Ville Syrjälä
  0 siblings, 1 reply; 19+ messages in thread
From: Imre Deak @ 2019-06-18 12:34 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

On Wed, Jun 12, 2019 at 04:08:01PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> We have full infoframe readout now so we can replace the
> PIPE_CONF_CHECK_BOOL_INCOMPLETE(has_infoframe) with the normal
> PIPE_CONF_CHECK_BOOL(has_infoframe).
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Imre Deak <imre.deak@intel.com>

Looks like has_audio has the readout too in place.

> ---
>  drivers/gpu/drm/i915/intel_display.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index ec3b7e825613..72fb04863acd 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -12570,7 +12570,7 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
>  
>  	PIPE_CONF_CHECK_BOOL(hdmi_scrambling);
>  	PIPE_CONF_CHECK_BOOL(hdmi_high_tmds_clock_ratio);
> -	PIPE_CONF_CHECK_BOOL_INCOMPLETE(has_infoframe);
> +	PIPE_CONF_CHECK_BOOL(has_infoframe);
>  
>  	PIPE_CONF_CHECK_BOOL_INCOMPLETE(has_audio);
>  
> -- 
> 2.21.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 4/4] drm/i915: Drop the _INCOMPLETE for has_infoframe
  2019-06-18 12:34   ` Imre Deak
@ 2019-06-18 12:55     ` Ville Syrjälä
  2019-06-18 13:28       ` Imre Deak
  0 siblings, 1 reply; 19+ messages in thread
From: Ville Syrjälä @ 2019-06-18 12:55 UTC (permalink / raw)
  To: Imre Deak; +Cc: intel-gfx

On Tue, Jun 18, 2019 at 03:34:46PM +0300, Imre Deak wrote:
> On Wed, Jun 12, 2019 at 04:08:01PM +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > We have full infoframe readout now so we can replace the
> > PIPE_CONF_CHECK_BOOL_INCOMPLETE(has_infoframe) with the normal
> > PIPE_CONF_CHECK_BOOL(has_infoframe).
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Reviewed-by: Imre Deak <imre.deak@intel.com>
> 
> Looks like has_audio has the readout too in place.

I suppose audio is using INCOMPLETE due to lack of ELD readout?

> 
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index ec3b7e825613..72fb04863acd 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -12570,7 +12570,7 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
> >  
> >  	PIPE_CONF_CHECK_BOOL(hdmi_scrambling);
> >  	PIPE_CONF_CHECK_BOOL(hdmi_high_tmds_clock_ratio);
> > -	PIPE_CONF_CHECK_BOOL_INCOMPLETE(has_infoframe);
> > +	PIPE_CONF_CHECK_BOOL(has_infoframe);
> >  
> >  	PIPE_CONF_CHECK_BOOL_INCOMPLETE(has_audio);
> >  
> > -- 
> > 2.21.0
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 4/4] drm/i915: Drop the _INCOMPLETE for has_infoframe
  2019-06-18 12:55     ` Ville Syrjälä
@ 2019-06-18 13:28       ` Imre Deak
  0 siblings, 0 replies; 19+ messages in thread
From: Imre Deak @ 2019-06-18 13:28 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

On Tue, Jun 18, 2019 at 03:55:53PM +0300, Ville Syrjälä wrote:
> On Tue, Jun 18, 2019 at 03:34:46PM +0300, Imre Deak wrote:
> > On Wed, Jun 12, 2019 at 04:08:01PM +0300, Ville Syrjala wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > We have full infoframe readout now so we can replace the
> > > PIPE_CONF_CHECK_BOOL_INCOMPLETE(has_infoframe) with the normal
> > > PIPE_CONF_CHECK_BOOL(has_infoframe).
> > > 
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Reviewed-by: Imre Deak <imre.deak@intel.com>
> > 
> > Looks like has_audio has the readout too in place.
> 
> I suppose audio is using INCOMPLETE due to lack of ELD readout?

Ah ok, so the computed has_audio state depending on ELD won't be
correct.

> 
> > 
> > > ---
> > >  drivers/gpu/drm/i915/intel_display.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > index ec3b7e825613..72fb04863acd 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -12570,7 +12570,7 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
> > >  
> > >  	PIPE_CONF_CHECK_BOOL(hdmi_scrambling);
> > >  	PIPE_CONF_CHECK_BOOL(hdmi_high_tmds_clock_ratio);
> > > -	PIPE_CONF_CHECK_BOOL_INCOMPLETE(has_infoframe);
> > > +	PIPE_CONF_CHECK_BOOL(has_infoframe);
> > >  
> > >  	PIPE_CONF_CHECK_BOOL_INCOMPLETE(has_audio);
> > >  
> > > -- 
> > > 2.21.0
> > > 
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2019-06-18 13:28 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-12 13:07 [PATCH 1/4] drm/i915: Don't clobber M/N values during fastset check Ville Syrjala
2019-06-12 13:07 ` [PATCH 2/4] drm/i915: Constify intel_pipe_config_compare() Ville Syrjala
2019-06-18 12:25   ` Imre Deak
2019-06-12 13:08 ` [PATCH 3/4] drm/i915: Make pipe_config_err() vs. fastset less confusing Ville Syrjala
2019-06-12 15:43   ` Imre Deak
2019-06-12 13:08 ` [PATCH 4/4] drm/i915: Drop the _INCOMPLETE for has_infoframe Ville Syrjala
2019-06-18 12:34   ` Imre Deak
2019-06-18 12:55     ` Ville Syrjälä
2019-06-18 13:28       ` Imre Deak
2019-06-12 13:15 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/4] drm/i915: Don't clobber M/N values during fastset check Patchwork
2019-06-12 13:52 ` ✗ Fi.CI.BAT: failure " Patchwork
2019-06-12 17:24 ` [PATCH v2 1/4] " Ville Syrjala
2019-06-13  9:24   ` Ville Syrjälä
2019-06-13  9:24     ` Ville Syrjälä
2019-06-18 12:19     ` [Intel-gfx] " Imre Deak
2019-06-15 22:16   ` Sasha Levin
2019-06-12 18:35 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [v2,1/4] drm/i915: Don't clobber M/N values during fastset check (rev2) Patchwork
2019-06-12 19:06 ` ✓ Fi.CI.BAT: success " Patchwork
2019-06-14 13:14 ` ✓ Fi.CI.IGT: " Patchwork

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.