[PATCH] drm/i915: s/intel_modeset_disable/intel_modeset_hw_freeze/

[PATCH] drm/i915: s/intel_modeset_disable/intel_modeset_hw_freeze/

[Daniel Vetter]

Chris Wilson rightfully complained that the name of this function is
misleading - it suggests that it's a generic means to disable all
outputs (and also update all the sw side tracking of the modeset
state). In effect it only disables the hw to quiescent it in the
freeze callbacks for suspend/hibernate - since thaw will read-out the
entire hw state again anyway and so scrap the sw tracking, we don't
need to update the driver state.

Also add a comment in the code.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_drv.c      |    2 +-
 drivers/gpu/drm/i915/intel_display.c |    4 +++-
 drivers/gpu/drm/i915/intel_drv.h     |    2 +-
 3 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 530db83..b2c4ee6 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -483,7 +483,7 @@ static int i915_drm_freeze(struct drm_device *dev)
 
 		cancel_delayed_work_sync(&dev_priv->rps.delayed_resume_work);
 
-		intel_modeset_disable(dev);
+		intel_modeset_hw_freeze(dev);
 
 		drm_irq_uninstall(dev);
 	}
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 31844fa..0a7c955 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3807,10 +3807,12 @@ static void intel_crtc_disable(struct drm_crtc *crtc)
 	}
 }
 
-void intel_modeset_disable(struct drm_device *dev)
+void intel_modeset_hw_freeze(struct drm_device *dev)
 {
 	struct drm_crtc *crtc;
 
+	/* No need to update the sw tracking of the hw state - at resume time
+	 * the hw state readout will overwrite it anyway. */
 	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
 		if (crtc->enabled)
 			intel_crtc_disable(crtc);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 8a1bd4a..951d0ab 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -504,7 +504,7 @@ struct intel_set_config {
 
 extern bool 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_modeset_hw_freeze(struct drm_device *dev);
 extern void intel_crtc_load_lut(struct drm_crtc *crtc);
 extern void intel_crtc_update_dpms(struct drm_crtc *crtc);
 extern void intel_encoder_noop(struct drm_encoder *encoder);
-- 
1.7.10.4

Re: [PATCH] drm/i915: s/intel_modeset_disable/intel_modeset_hw_freeze/

[Chris Wilson]

On Mon, 17 Dec 2012 15:51:57 +0100, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> Chris Wilson rightfully complained that the name of this function is
> misleading - it suggests that it's a generic means to disable all
> outputs (and also update all the sw side tracking of the modeset
> state). In effect it only disables the hw to quiescent it in the
> freeze callbacks for suspend/hibernate - since thaw will read-out the
> entire hw state again anyway and so scrap the sw tracking, we don't
> need to update the driver state.
> 
> Also add a comment in the code.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

A nice improvement and stops me from wanting to call it -- it would have
been a nasty mistake.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH] drm/i915: s/intel_modeset_disable/intel_modeset_hw_freeze/

[Chris Wilson]

On Mon, 17 Dec 2012 15:51:57 +0100, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> Chris Wilson rightfully complained that the name of this function is
> misleading - it suggests that it's a generic means to disable all
> outputs (and also update all the sw side tracking of the modeset
> state). In effect it only disables the hw to quiescent it in the
> freeze callbacks for suspend/hibernate - since thaw will read-out the
> entire hw state again anyway and so scrap the sw tracking, we don't
> need to update the driver state.
> 
> Also add a comment in the code.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Forgot the r-b,
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre