Re: Re: drm/i915: new warning (regression) in 3.7.10 and 3.8.3

[Daniel Vetter <daniel@ffwll.ch>]

On Tue, Apr 09, 2013 at 03:21:35PM +0200, Richard Cochran wrote:
> On Tue, Apr 09, 2013 at 02:50:03PM +0200, Daniel Vetter wrote:
> > 
> > This should be fixed with the above mentioned patch. The issue is that the
> > bios fumbles around with the output configuration behind our backs, so the
> > new paranoid modeset code in 3.7+ freaks out about the state mismatch
> > between sw and hw.
> > 
> > The patch above should detect this situation and undo any bios-induced
> > damage.
> 
> Even with the patch, I still am seeing the issue (in 3.8).

Indeed, there's still a bug there with the state checking. Can you please
test the below patch?

Thanks, Daniel

commit d206df2685801a832a728c7dca13428a6f5bf3ef
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date:   Tue Apr 9 19:25:12 2013 +0200

    drm/i915: don't check inconstent modeset state when force-restoring
    
    It will be only consistent once we've restored all the crtcs. Since a
    bunch of other callers also want to just restore a single crtc, add a
    boolean to disable checking only where it doesn't make sense.
    
    Note that intel_modeset_setup_hw_state already has a call to
    intel_modeset_check_state at the end, so we don't reduce the amount of
    checking.
    
    References: https://lkml.org/lkml/2013/3/16/60
    Cc: Tomas Melin <tomas.melin@iki.fi>
    Cc: Richard Cochran <richardcochran@gmail.com>
    Cc: stable@vger.kernel.org
    Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 8809813..1e29201 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7916,9 +7916,9 @@ intel_modeset_check_state(struct drm_device *dev)
 	}
 }
 
-int intel_set_mode(struct drm_crtc *crtc,
-		   struct drm_display_mode *mode,
-		   int x, int y, struct drm_framebuffer *fb)
+static int __intel_set_mode(struct drm_crtc *crtc,
+			    struct drm_display_mode *mode,
+			    int x, int y, struct drm_framebuffer *fb)
 {
 	struct drm_device *dev = crtc->dev;
 	drm_i915_private_t *dev_priv = dev->dev_private;
@@ -8012,8 +8012,6 @@ done:
 	if (ret && crtc->enabled) {
 		crtc->hwmode = *saved_hwmode;
 		crtc->mode = *saved_mode;
-	} else {
-		intel_modeset_check_state(dev);
 	}
 
 out:
@@ -8022,9 +8020,27 @@ out:
 	return ret;
 }
 
-void intel_crtc_restore_mode(struct drm_crtc *crtc)
+int intel_set_mode(struct drm_crtc *crtc,
+		     struct drm_display_mode *mode,
+		     int x, int y, struct drm_framebuffer *fb)
+{
+	int ret;
+
+	ret = __intel_set_mode(crtc, mode, x, y, fb);
+
+	if (ret == 0)
+		intel_modeset_check_state(crtc->dev);
+
+	return ret;
+}
+
+void intel_crtc_restore_mode(struct drm_crtc *crtc,
+			     bool check)
 {
-	intel_set_mode(crtc, &crtc->mode, crtc->x, crtc->y, crtc->fb);
+	__intel_set_mode(crtc, &crtc->mode, crtc->x, crtc->y, crtc->fb);
+
+	if (check)
+		intel_modeset_check_state(crtc->dev);
 }
 
 #undef for_each_intel_crtc_masked
@@ -9336,7 +9352,7 @@ setup_pipes:
 		for_each_pipe(pipe) {
 			struct drm_crtc *crtc =
 				dev_priv->pipe_to_crtc_mapping[pipe];
-			intel_crtc_restore_mode(crtc);
+			intel_crtc_restore_mode(crtc, false);
 		}
 		list_for_each_entry(plane, &dev->mode_config.plane_list, head)
 			intel_plane_restore(plane);
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 173add1..99b9f09 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2466,7 +2466,7 @@ intel_dp_set_property(struct drm_connector *connector,
 
 done:
 	if (intel_encoder->base.crtc)
-		intel_crtc_restore_mode(intel_encoder->base.crtc);
+		intel_crtc_restore_mode(intel_encoder->base.crtc, true);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d7bd031..d7b1094 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -540,7 +540,7 @@ struct intel_set_config {
 extern int intel_set_mode(struct drm_crtc *crtc, struct drm_display_mode *mode,
 			  int x, int y, struct drm_framebuffer *old_fb);
 extern void intel_modeset_disable(struct drm_device *dev);
-extern void intel_crtc_restore_mode(struct drm_crtc *crtc);
+extern void intel_crtc_restore_mode(struct drm_crtc *crtc, bool check);
 extern void intel_crtc_load_lut(struct drm_crtc *crtc);
 extern void intel_crtc_update_dpms(struct drm_crtc *crtc);
 extern void intel_encoder_destroy(struct drm_encoder *encoder);
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index ee4a8da..de07bd4 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -942,7 +942,7 @@ intel_hdmi_set_property(struct drm_connector *connector,
 
 done:
 	if (intel_dig_port->base.base.crtc)
-		intel_crtc_restore_mode(intel_dig_port->base.base.crtc);
+		intel_crtc_restore_mode(intel_dig_port->base.base.crtc, true);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index ca2d903..4e80e76 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -666,7 +666,7 @@ static int intel_lvds_set_property(struct drm_connector *connector,
 			 * If the CRTC is enabled, the display will be changed
 			 * according to the new panel fitting mode.
 			 */
-			intel_crtc_restore_mode(crtc);
+			intel_crtc_restore_mode(crtc, true);
 		}
 	}
 
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index f6a9f4a..777d0d4 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -2044,7 +2044,7 @@ set_value:
 
 done:
 	if (intel_sdvo->base.base.crtc)
-		intel_crtc_restore_mode(intel_sdvo->base.base.crtc);
+		intel_crtc_restore_mode(intel_sdvo->base.base.crtc, true);
 
 	return 0;
 #undef CHECK_PROPERTY
diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
index 6673726..bbe2925 100644
--- a/drivers/gpu/drm/i915/intel_tv.c
+++ b/drivers/gpu/drm/i915/intel_tv.c
@@ -1481,7 +1481,7 @@ intel_tv_set_property(struct drm_connector *connector, struct drm_property *prop
 	}
 
 	if (changed && crtc)
-		intel_crtc_restore_mode(crtc);
+		intel_crtc_restore_mode(crtc, true);
 out:
 	return ret;
 }
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch