All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: force restore on lid open
@ 2012-11-23 17:16 Daniel Vetter
  2012-11-27 16:14 ` Jesse Barnes
  0 siblings, 1 reply; 3+ messages in thread
From: Daniel Vetter @ 2012-11-23 17:16 UTC (permalink / raw)
  To: Intel Graphics Development; +Cc: Daniel Vetter

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;
-- 
1.7.10.4

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

* Re: [PATCH] drm/i915: force restore on lid open
  2012-11-23 17:16 [PATCH] drm/i915: force restore on lid open Daniel Vetter
@ 2012-11-27 16:14 ` Jesse Barnes
  2012-11-27 17:04   ` Daniel Vetter
  0 siblings, 1 reply; 3+ messages in thread
From: Jesse Barnes @ 2012-11-27 16:14 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: Intel Graphics Development

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>

-- 
Jesse Barnes, Intel Open Source Technology Center

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

* Re: [PATCH] drm/i915: force restore on lid open
  2012-11-27 16:14 ` Jesse Barnes
@ 2012-11-27 17:04   ` Daniel Vetter
  0 siblings, 0 replies; 3+ messages in thread
From: Daniel Vetter @ 2012-11-27 17:04 UTC (permalink / raw)
  To: Jesse Barnes; +Cc: Daniel Vetter, Intel Graphics Development

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

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

end of thread, other threads:[~2012-11-27 17:02 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-11-23 17:16 [PATCH] drm/i915: force restore on lid open Daniel Vetter
2012-11-27 16:14 ` Jesse Barnes
2012-11-27 17:04   ` Daniel Vetter

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.