i915: restore only the mode of this driver on lastclose

Commit Message

Dave Airlie April 11, 2011, 5:23 a.m. UTC

From: Dave Airlie <[e-mail]>

This has always used a big hammer, but that hammer is probably
too big, I'm also not sure its necessary but at least this
should be safe.

Should fix: https://bugzilla.kernel.org/show_bug.cgi?id=23592

Signed-off-by: Dave Airlie <[e-mail]>
---
 drivers/gpu/drm/i915/i915_dma.c  |    2 +-
 drivers/gpu/drm/i915/intel_drv.h |    1 +
 drivers/gpu/drm/i915/intel_fb.c  |   16 ++++++++++++++++
 3 files changed, 18 insertions(+), 1 deletions(-)

Comments

Chris Wilson April 11, 2011, 7:21 a.m. UTC | #1

On Mon, 11 Apr 2011 15:23:17 +1000, Dave Airlie <[e-mail]> wrote:
> From: Dave Airlie <[e-mail]>
> 
> This has always used a big hammer, but that hammer is probably
> too big, I'm also not sure its necessary but at least this
> should be safe.

So the difference appears to be that the patch only restores the fbcon
of a device for the lastclose of that device. That makes sense.

However, would it not be better to make this a generic
drm_fb_helper_restore_mode() as it sounds like something every driver
should consider?
-Chris

Keith Packard April 11, 2011, 4:06 p.m. UTC | #2

On Mon, 11 Apr 2011 15:23:17 +1000, Dave Airlie <[e-mail]> wrote:
> From: Dave Airlie <[e-mail]>
> 
> This has always used a big hammer, but that hammer is probably
> too big, I'm also not sure its necessary but at least this
> should be safe.

Am I correct in reading the drm_fb_helper_restore path as restoring
the mode of each fbdev device in turn? If so, why would that ever be
useful (aside from doing it for each inserted card)?

Chris Wilson April 11, 2011, 5:31 p.m. UTC | #3

On Mon, 11 Apr 2011 09:06:38 -0700, Keith Packard <[e-mail]> wrote:
> On Mon, 11 Apr 2011 15:23:17 +1000, Dave Airlie <[e-mail]> wrote:
> > From: Dave Airlie <[e-mail]>
> > 
> > This has always used a big hammer, but that hammer is probably
> > too big, I'm also not sure its necessary but at least this
> > should be safe.
> 
> Am I correct in reading the drm_fb_helper_restore path as restoring
> the mode of each fbdev device in turn? If so, why would that ever be
> useful (aside from doing it for each inserted card)?

During panics? Just not lastclose. :)
-Chris

Keith Packard April 11, 2011, 5:55 p.m. UTC | #4

On Mon, 11 Apr 2011 18:31:21 +0100, Chris Wilson <[e-mail]> wrote:

> During panics? Just not lastclose. :)

I'm still mystified -- why is it useful to do more than one modesetting
operation on the video card?

Patch

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 7273037..12876f2 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -2207,7 +2207,7 @@  void i915_driver_lastclose(struct drm_device * dev)
 	drm_i915_private_t *dev_priv = dev->dev_private;
 
 	if (!dev_priv || drm_core_check_feature(dev, DRIVER_MODESET)) {
-		drm_fb_helper_restore();
+		intel_fb_restore_mode(dev);
 		vga_switcheroo_process_delayed_switch();
 		return;
 	}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index f5b0d83..1d20712 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -338,4 +338,5 @@  extern int intel_overlay_attrs(struct drm_device *dev, void *data,
 			       struct drm_file *file_priv);
 
 extern void intel_fb_output_poll_changed(struct drm_device *dev);
+extern void intel_fb_restore_mode(struct drm_device *dev);
 #endif /* __INTEL_DRV_H__ */
diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c
index 5127827..96a45c4 100644
--- a/drivers/gpu/drm/i915/intel_fb.c
+++ b/drivers/gpu/drm/i915/intel_fb.c
@@ -264,3 +264,19 @@  void intel_fb_output_poll_changed(struct drm_device *dev)
 	drm_i915_private_t *dev_priv = dev->dev_private;
 	drm_fb_helper_hotplug_event(&dev_priv->fbdev->helper);
 }
+
+void intel_fb_restore_mode(struct drm_device *dev)
+{
+	drm_i915_private_t *dev_priv = dev->dev_private;
+	int ret, i;
+
+	if (!dev_priv->fbdev)
+		return;
+
+	for (i = 0; i < dev_priv->fbdev->helper.crtc_count; i++) {
+		struct drm_mode_set *mode_set = &dev_priv->fbdev->helper.crtc_info[i].mode_set;
+		ret = drm_crtc_helper_set_config(mode_set);
+		if (ret)
+			DRM_DEBUG("failed to restore crtc mode\n");
+	}
+}