Re: [PATCH] drm/i915: force restore on lid open

[Daniel Vetter <daniel@ffwll.ch>]

On Tue, Nov 27, 2012 at 08:14:29AM -0800, Jesse Barnes wrote:
> On Fri, 23 Nov 2012 18:16:34 +0100
> Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> 
> > There seem to be indeed some awkwards machines around, mostly those
> > without OpRegion support, where the firmware changes the display hw
> > state behind our backs when closing the lid.
> > 
> > This force-restore logic has been originally introduced in
> > 
> > commit c1c7af60892070e4b82ad63bbfb95ae745056de0
> > Author: Jesse Barnes <jbarnes@virtuousgeek.org>
> > Date:   Thu Sep 10 15:28:03 2009 -0700
> > 
> >     drm/i915: force mode set at lid open time
> > 
> > but after the modeset-rework we've disabled it in the vain hope that
> > it's no longer required:
> > 
> > commit 3b7a89fce3e3dc96b549d6d829387b4439044d0d
> > Author: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Date:   Mon Sep 17 22:27:21 2012 +0200
> > 
> >     drm/i915: fix OOPS in lid_notify
> > 
> > Alas, no.
> > 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=54677
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=57434
> > Tested-by: Krzysztof Mazur <krzysiek@podlesie.net>
> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.c      |    2 +-
> >  drivers/gpu/drm/i915/i915_drv.h      |    3 ++-
> >  drivers/gpu/drm/i915/intel_display.c |   15 ++++++++++++---
> >  drivers/gpu/drm/i915/intel_lvds.c    |    2 +-
> >  4 files changed, 16 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > index 9e7e647..5e3fff1 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -548,7 +548,7 @@ static int i915_drm_thaw(struct drm_device *dev)
> >  		mutex_unlock(&dev->struct_mutex);
> >  
> >  		intel_modeset_init_hw(dev);
> > -		intel_modeset_setup_hw_state(dev);
> > +		intel_modeset_setup_hw_state(dev, false);
> >  		drm_irq_install(dev);
> >  	}
> >  
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 4728d30..fd3832e 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1598,7 +1598,8 @@ extern void intel_modeset_init(struct drm_device *dev);
> >  extern void intel_modeset_gem_init(struct drm_device *dev);
> >  extern void intel_modeset_cleanup(struct drm_device *dev);
> >  extern int intel_modeset_vga_set_state(struct drm_device *dev, bool state);
> > -extern void intel_modeset_setup_hw_state(struct drm_device *dev);
> > +extern void intel_modeset_setup_hw_state(struct drm_device *dev,
> > +					 bool force_restore);
> >  extern bool intel_fbc_enabled(struct drm_device *dev);
> >  extern void intel_disable_fbc(struct drm_device *dev);
> >  extern bool ironlake_set_drps(struct drm_device *dev, u8 val);
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index c2c219b..346e78a 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -8522,7 +8522,8 @@ static void intel_sanitize_encoder(struct intel_encoder *encoder)
> >  
> >  /* Scan out the current hw modeset state, sanitizes it and maps it into the drm
> >   * and i915 state tracking structures. */
> > -void intel_modeset_setup_hw_state(struct drm_device *dev)
> > +void intel_modeset_setup_hw_state(struct drm_device *dev,
> > +				  bool force_restore)
> >  {
> >  	struct drm_i915_private *dev_priv = dev->dev_private;
> >  	enum pipe pipe;
> > @@ -8596,7 +8597,15 @@ void intel_modeset_setup_hw_state(struct drm_device *dev)
> >  		intel_sanitize_crtc(crtc);
> >  	}
> >  
> > -	intel_modeset_update_staged_output_state(dev);
> > +	if (force_restore) {
> > +		for_each_pipe(pipe) {
> > +			crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
> > +			intel_set_mode(&crtc->base, &crtc->base.mode,
> > +				       crtc->base.x, crtc->base.y, crtc->base.fb);
> > +		}
> > +	} else {
> > +		intel_modeset_update_staged_output_state(dev);
> > +	}
> >  
> >  	intel_modeset_check_state(dev);
> >  
> > @@ -8609,7 +8618,7 @@ void intel_modeset_gem_init(struct drm_device *dev)
> >  
> >  	intel_setup_overlay(dev);
> >  
> > -	intel_modeset_setup_hw_state(dev);
> > +	intel_modeset_setup_hw_state(dev, false);
> >  }
> >  
> >  void intel_modeset_cleanup(struct drm_device *dev)
> > diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> > index 40d72bd..589c917 100644
> > --- a/drivers/gpu/drm/i915/intel_lvds.c
> > +++ b/drivers/gpu/drm/i915/intel_lvds.c
> > @@ -527,7 +527,7 @@ static int intel_lid_notify(struct notifier_block *nb, unsigned long val,
> >  	dev_priv->modeset_on_lid = 0;
> >  
> >  	mutex_lock(&dev->mode_config.mutex);
> > -	intel_modeset_check_state(dev);
> > +	intel_modeset_setup_hw_state(dev, true);
> >  	mutex_unlock(&dev->mode_config.mutex);
> >  
> >  	return NOTIFY_OK;
> 
> Assuming this does the right thing when the firmware *hasn't* messed
> with the state (i.e. lots of testing):
> Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Queued for -next (3.8), thanks for the patch.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch