VT switchless v3

VT switchless v3

[Jesse Barnes]

A few more fixes from Daniel.

[PATCH 1/6] drm/i915: don't restore LVDS enable state blindly v2

[Jesse Barnes]

We still rely on a few LVDS bits, but restoring the enable bit can cause
trouble at this point, so don't.

v2: use the right mask to prevent restore (Daniel)
    conditionalize on KMS support (Denial)

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/i915_suspend.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_suspend.c b/drivers/gpu/drm/i915/i915_suspend.c
index 2135f21..c1e02b0 100644
--- a/drivers/gpu/drm/i915/i915_suspend.c
+++ b/drivers/gpu/drm/i915/i915_suspend.c
@@ -255,6 +255,7 @@ static void i915_save_display(struct drm_device *dev)
 static void i915_restore_display(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
+	u32 mask = 0xffffffff;
 
 	/* Display arbitration */
 	if (INTEL_INFO(dev)->gen <= 4)
@@ -267,10 +268,13 @@ static void i915_restore_display(struct drm_device *dev)
 	if (INTEL_INFO(dev)->gen >= 4 && !HAS_PCH_SPLIT(dev))
 		I915_WRITE(BLC_PWM_CTL2, dev_priv->regfile.saveBLC_PWM_CTL2);
 
+	if (drm_core_check_feature(dev, DRIVER_MODESET))
+		mask = ~LVDS_PORT_EN;
+
 	if (HAS_PCH_SPLIT(dev)) {
-		I915_WRITE(PCH_LVDS, dev_priv->regfile.saveLVDS);
+		I915_WRITE(PCH_LVDS, dev_priv->regfile.saveLVDS & mask);
 	} else if (IS_MOBILE(dev) && !IS_I830(dev))
-		I915_WRITE(LVDS, dev_priv->regfile.saveLVDS);
+		I915_WRITE(LVDS, dev_priv->regfile.saveLVDS & mask);
 
 	if (!IS_I830(dev) && !IS_845G(dev) && !HAS_PCH_SPLIT(dev))
 		I915_WRITE(PFIT_CONTROL, dev_priv->regfile.savePFIT_CONTROL);
-- 
1.7.9.5

[PATCH 2/6] drm/i915: add sprite restore function v2

[Jesse Barnes]

To be used to restore sprite state on resume.

v2: move sprite tracking bits up so we don't track modified sprite state

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/intel_drv.h    |    5 +++++
 drivers/gpu/drm/i915/intel_sprite.c |   23 +++++++++++++++++++++++
 2 files changed, 28 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 005a91f..1b548e0 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -247,6 +247,10 @@ struct intel_plane {
 	bool can_scale;
 	int max_downscale;
 	u32 lut_r[1024], lut_g[1024], lut_b[1024];
+	int crtc_x, crtc_y;
+	unsigned int crtc_w, crtc_h;
+	uint32_t x, y;
+	uint32_t src_w, src_h;
 	void (*update_plane)(struct drm_plane *plane,
 			     struct drm_framebuffer *fb,
 			     struct drm_i915_gem_object *obj,
@@ -532,6 +536,7 @@ extern bool intel_encoder_check_is_cloned(struct intel_encoder *encoder);
 extern void intel_connector_dpms(struct drm_connector *, int mode);
 extern bool intel_connector_get_hw_state(struct intel_connector *connector);
 extern void intel_modeset_check_state(struct drm_device *dev);
+extern void intel_plane_restore(struct drm_plane *plane);
 
 
 static inline struct intel_encoder *intel_attached_encoder(struct drm_connector *connector)
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 03cfd62..ca171af 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -438,6 +438,15 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 
 	old_obj = intel_plane->obj;
 
+	intel_plane->crtc_x = crtc_x;
+	intel_plane->crtc_y = crtc_y;
+	intel_plane->crtc_w = crtc_w;
+	intel_plane->crtc_h = crtc_h;
+	intel_plane->x = x;
+	intel_plane->y = y;
+	intel_plane->src_w = src_w;
+	intel_plane->src_h = src_h;
+
 	src_w = src_w >> 16;
 	src_h = src_h >> 16;
 
@@ -644,6 +653,20 @@ out_unlock:
 	return ret;
 }
 
+void intel_plane_restore(struct drm_plane *plane)
+{
+	struct intel_plane *intel_plane = to_intel_plane(plane);
+
+	if (!plane->crtc || !plane->fb)
+		return;
+
+	intel_update_plane(plane, plane->crtc, plane->fb,
+			   intel_plane->crtc_x, intel_plane->crtc_y,
+			   intel_plane->crtc_w, intel_plane->crtc_h,
+			   intel_plane->x, intel_plane->y,
+			   intel_plane->src_w, intel_plane->src_h);
+}
+
 static const struct drm_plane_funcs intel_plane_funcs = {
 	.update_plane = intel_update_plane,
 	.disable_plane = intel_disable_plane,
-- 
1.7.9.5

[PATCH 3/6] drm/i915: restore cursor and sprite state when forcing a config restore

[Jesse Barnes]

Needed for VT switchless resume.

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/intel_display.c |   15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 3e6dadf..2bf076e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8985,6 +8985,7 @@ void intel_modeset_setup_hw_state(struct drm_device *dev,
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	enum pipe pipe;
 	u32 tmp;
+	struct drm_plane *plane;
 	struct intel_crtc *crtc;
 	struct intel_encoder *encoder;
 	struct intel_connector *connector;
@@ -9081,8 +9082,20 @@ void intel_modeset_setup_hw_state(struct drm_device *dev,
 
 	if (force_restore) {
 		for_each_pipe(pipe) {
-			intel_crtc_restore_mode(dev_priv->pipe_to_crtc_mapping[pipe]);
+			struct drm_crtc *crtc =
+				dev_priv->pipe_to_crtc_mapping[pipe];
+			struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+
+			intel_crtc_restore_mode(crtc);
+			if (intel_crtc->cursor_visible) {
+				/* Force update for previously enabled cursor */
+				intel_crtc->cursor_visible = false;
+				intel_crtc_update_cursor(&intel_crtc->base,
+							 true);
+			}
 		}
+		list_for_each_entry(plane, &dev->mode_config.plane_list, head)
+			intel_plane_restore(plane);
 
 		i915_redisable_vga(dev);
 	} else {
-- 
1.7.9.5

[PATCH 4/6] drm/i915: enable VT switchless resume v2

[Jesse Barnes]

With the other bits in place, we can do this safely.

v2: disable backlight on suspend to prevent premature enablement on resume

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/i915_drv.c |   12 +++++++++---
 drivers/gpu/drm/i915/intel_fb.c |    3 +++
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index c5b8c81..e76b038 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -492,9 +492,10 @@ static int i915_drm_freeze(struct drm_device *dev)
 
 		cancel_delayed_work_sync(&dev_priv->rps.delayed_resume_work);
 
-		intel_modeset_disable(dev);
-
 		drm_irq_uninstall(dev);
+
+		if (dev_priv->backlight)
+			intel_panel_disable_backlight(dev);
 	}
 
 	i915_save_state(dev);
@@ -569,9 +570,14 @@ 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, false);
+
 		drm_irq_install(dev);
 		intel_hpd_init(dev);
+
+		/* Resume the modeset for every activated CRTC */
+		drm_modeset_lock_all(dev);
+		intel_modeset_setup_hw_state(dev, true);
+		drm_modeset_unlock_all(dev);
 	}
 
 	intel_opregion_init(dev);
diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c
index 1c510da..987bc33 100644
--- a/drivers/gpu/drm/i915/intel_fb.c
+++ b/drivers/gpu/drm/i915/intel_fb.c
@@ -149,6 +149,9 @@ static int intelfb_create(struct intel_fbdev *ifbdev,
 	}
 	info->screen_size = size;
 
+	/* This driver doesn't need a VT switch to restore the mode on resume */
+	info->skip_vt_switch = true;
+
 //	memset(info->screen_base, 0, size);
 
 	drm_fb_helper_fill_fix(info, fb->pitches[0], fb->depth);
-- 
1.7.9.5

[PATCH 5/6] drm/i915: emit a hotplug event on resume

[Jesse Barnes]

This will poke userspace into probing for configuration changes that may
have occurred across suspend/resume.

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/i915_drv.c |   20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index e76b038..1b37eec 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -551,6 +551,24 @@ void intel_console_resume(struct work_struct *work)
 	console_unlock();
 }
 
+static void intel_resume_hotplug(struct drm_device *dev)
+{
+	struct drm_mode_config *mode_config = &dev->mode_config;
+	struct intel_encoder *encoder;
+
+	mutex_lock(&mode_config->mutex);
+	DRM_DEBUG_KMS("running encoder hotplug functions\n");
+
+	list_for_each_entry(encoder, &mode_config->encoder_list, base.head)
+		if (encoder->hot_plug)
+			encoder->hot_plug(encoder);
+
+	mutex_unlock(&mode_config->mutex);
+
+	/* Just fire off a uevent and let userspace tell us what to do */
+	drm_helper_hpd_irq_event(dev);
+}
+
 static int __i915_drm_thaw(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -578,6 +596,8 @@ static int __i915_drm_thaw(struct drm_device *dev)
 		drm_modeset_lock_all(dev);
 		intel_modeset_setup_hw_state(dev, true);
 		drm_modeset_unlock_all(dev);
+
+		intel_resume_hotplug(dev);
 	}
 
 	intel_opregion_init(dev);
-- 
1.7.9.5

[PATCH 6/6] drm/i915: remove disabled memset of framebuffer from intel_fb

[Jesse Barnes]

Commented out and unneeded.

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/intel_fb.c |    2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c
index 987bc33..f4e0b88 100644
--- a/drivers/gpu/drm/i915/intel_fb.c
+++ b/drivers/gpu/drm/i915/intel_fb.c
@@ -152,8 +152,6 @@ static int intelfb_create(struct intel_fbdev *ifbdev,
 	/* This driver doesn't need a VT switch to restore the mode on resume */
 	info->skip_vt_switch = true;
 
-//	memset(info->screen_base, 0, size);
-
 	drm_fb_helper_fill_fix(info, fb->pitches[0], fb->depth);
 	drm_fb_helper_fill_var(info, &ifbdev->helper, sizes->fb_width, sizes->fb_height);
 
-- 
1.7.9.5

Re: VT switchless v3

[Daniel Vetter]

On Fri, Feb 15, 2013 at 01:23:08PM -0800, Jesse Barnes wrote:
> A few more fixes from Daniel.

So one thing that crossed my mind which we should at least quickly
discuss: How is userspace supposed to notice the resume? On a lot of
desktops (and even Androids) the normal thing seems to be to draw the
screensave/lock after resume, which means that we'll show the desktop for
a frame or so. Which is not too nice. Is userspace simply supposed to draw
the screen lock before sleep, or does it need a helping hand from the
kernel to fix this issue?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 2/6] drm/i915: add sprite restore function v2

[Ville Syrjälä]

On Fri, Feb 15, 2013 at 01:23:10PM -0800, Jesse Barnes wrote:
> To be used to restore sprite state on resume.
> 
> v2: move sprite tracking bits up so we don't track modified sprite state
> 
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> ---
>  drivers/gpu/drm/i915/intel_drv.h    |    5 +++++
>  drivers/gpu/drm/i915/intel_sprite.c |   23 +++++++++++++++++++++++
>  2 files changed, 28 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 005a91f..1b548e0 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -247,6 +247,10 @@ struct intel_plane {
>  	bool can_scale;
>  	int max_downscale;
>  	u32 lut_r[1024], lut_g[1024], lut_b[1024];
> +	int crtc_x, crtc_y;
> +	unsigned int crtc_w, crtc_h;
> +	uint32_t x, y;

Can we call just them src_x/src_y instead?

> +	uint32_t src_w, src_h;
>  	void (*update_plane)(struct drm_plane *plane,
>  			     struct drm_framebuffer *fb,
>  			     struct drm_i915_gem_object *obj,
> @@ -532,6 +536,7 @@ extern bool intel_encoder_check_is_cloned(struct intel_encoder *encoder);
>  extern void intel_connector_dpms(struct drm_connector *, int mode);
>  extern bool intel_connector_get_hw_state(struct intel_connector *connector);
>  extern void intel_modeset_check_state(struct drm_device *dev);
> +extern void intel_plane_restore(struct drm_plane *plane);
>  
>  
>  static inline struct intel_encoder *intel_attached_encoder(struct drm_connector *connector)
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 03cfd62..ca171af 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -438,6 +438,15 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
>  
>  	old_obj = intel_plane->obj;
>  
> +	intel_plane->crtc_x = crtc_x;
> +	intel_plane->crtc_y = crtc_y;
> +	intel_plane->crtc_w = crtc_w;
> +	intel_plane->crtc_h = crtc_h;
> +	intel_plane->x = x;
> +	intel_plane->y = y;

x and y are not fixed point numbers. They just contain the integer parts
of src_x and src_y. So you need to use src_x and src_y here instead.

> +	intel_plane->src_w = src_w;
> +	intel_plane->src_h = src_h;
> +
>  	src_w = src_w >> 16;
>  	src_h = src_h >> 16;
>  
> @@ -644,6 +653,20 @@ out_unlock:
>  	return ret;
>  }
>  
> +void intel_plane_restore(struct drm_plane *plane)
> +{
> +	struct intel_plane *intel_plane = to_intel_plane(plane);
> +
> +	if (!plane->crtc || !plane->fb)
> +		return;
> +
> +	intel_update_plane(plane, plane->crtc, plane->fb,
> +			   intel_plane->crtc_x, intel_plane->crtc_y,
> +			   intel_plane->crtc_w, intel_plane->crtc_h,
> +			   intel_plane->x, intel_plane->y,
> +			   intel_plane->src_w, intel_plane->src_h);
> +}
> +
>  static const struct drm_plane_funcs intel_plane_funcs = {
>  	.update_plane = intel_update_plane,
>  	.disable_plane = intel_disable_plane,
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC

Re: VT switchless v3

[Kristian Høgsberg]

On Mon, Feb 18, 2013 at 10:03 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Fri, Feb 15, 2013 at 01:23:08PM -0800, Jesse Barnes wrote:
>> A few more fixes from Daniel.
>
> So one thing that crossed my mind which we should at least quickly
> discuss: How is userspace supposed to notice the resume? On a lot of
> desktops (and even Androids) the normal thing seems to be to draw the
> screensave/lock after resume, which means that we'll show the desktop for
> a frame or so. Which is not too nice. Is userspace simply supposed to draw
> the screen lock before sleep, or does it need a helping hand from the
> kernel to fix this issue?

Yup, I second that and I've mentioned it to Jesse before. VTs in
KD_GRAPHICS mode are already responsible for re-setting their mode
when you switch back to them after having switched away.  Surely they
should have the option to handle setting mode and potentially present
a lockscreen when coming back from resume.  In case of weston, we have
the option to handle this very well, since the compositor can launch
prepare a lock screen (potentailly launching a helper client to do
that) and just not set a mode or dpms on until the UI is ready.

Kristian

> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: VT switchless v3

[Jesse Barnes]

On Mon, 18 Feb 2013 16:03:20 +0100
Daniel Vetter <daniel@ffwll.ch> wrote:

> On Fri, Feb 15, 2013 at 01:23:08PM -0800, Jesse Barnes wrote:
> > A few more fixes from Daniel.
> 
> So one thing that crossed my mind which we should at least quickly
> discuss: How is userspace supposed to notice the resume? On a lot of
> desktops (and even Androids) the normal thing seems to be to draw the
> screensave/lock after resume, which means that we'll show the desktop for
> a frame or so. Which is not too nice. Is userspace simply supposed to draw
> the screen lock before sleep, or does it need a helping hand from the
> kernel to fix this issue?

Userspace initiates the suspend, so GNOME or whatever is supposed to
lock things at that point.  I've heard talk of a uevent or dbus thing
to broadcast it as well, but I'm not sure of the status of that.

IIRC Android already has a hack like this; they don't want the fbcon to
ever be visible during usage...

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: VT switchless v3

[Jesse Barnes]

On Mon, 18 Feb 2013 19:58:26 -0500
Kristian Høgsberg <krh@bitplanet.net> wrote:

> On Mon, Feb 18, 2013 at 10:03 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
> > On Fri, Feb 15, 2013 at 01:23:08PM -0800, Jesse Barnes wrote:
> >> A few more fixes from Daniel.
> >
> > So one thing that crossed my mind which we should at least quickly
> > discuss: How is userspace supposed to notice the resume? On a lot of
> > desktops (and even Androids) the normal thing seems to be to draw the
> > screensave/lock after resume, which means that we'll show the desktop for
> > a frame or so. Which is not too nice. Is userspace simply supposed to draw
> > the screen lock before sleep, or does it need a helping hand from the
> > kernel to fix this issue?
> 
> Yup, I second that and I've mentioned it to Jesse before. VTs in
> KD_GRAPHICS mode are already responsible for re-setting their mode
> when you switch back to them after having switched away.  Surely they
> should have the option to handle setting mode and potentially present
> a lockscreen when coming back from resume.  In case of weston, we have
> the option to handle this very well, since the compositor can launch
> prepare a lock screen (potentailly launching a helper client to do
> that) and just not set a mode or dpms on until the UI is ready.

Rob Bradford mentioned that GNOME had a bug here where on suspend the
lock screen daemon would be notified, but the suspend would happen too
quickly for it to actually receive the notification and take action.
So when you resumed, you'd see your desktop until the daemon was re-run
and then the lock screen would appear.  That's been fixed apparently,
but it illustrates that you don't want to do things on resume, you want
to do them on suspend like Kristian points out.

-- 
Jesse Barnes, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/6] drm/i915: add sprite restore function v2

[Jesse Barnes]

On Mon, 18 Feb 2013 19:19:55 +0200
Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:

> On Fri, Feb 15, 2013 at 01:23:10PM -0800, Jesse Barnes wrote:
> > To be used to restore sprite state on resume.
> > 
> > v2: move sprite tracking bits up so we don't track modified sprite state
> > 
> > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> > ---
> >  drivers/gpu/drm/i915/intel_drv.h    |    5 +++++
> >  drivers/gpu/drm/i915/intel_sprite.c |   23 +++++++++++++++++++++++
> >  2 files changed, 28 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index 005a91f..1b548e0 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -247,6 +247,10 @@ struct intel_plane {
> >  	bool can_scale;
> >  	int max_downscale;
> >  	u32 lut_r[1024], lut_g[1024], lut_b[1024];
> > +	int crtc_x, crtc_y;
> > +	unsigned int crtc_w, crtc_h;
> > +	uint32_t x, y;
> 
> Can we call just them src_x/src_y instead?
> 
> > +	uint32_t src_w, src_h;
> >  	void (*update_plane)(struct drm_plane *plane,
> >  			     struct drm_framebuffer *fb,
> >  			     struct drm_i915_gem_object *obj,
> > @@ -532,6 +536,7 @@ extern bool intel_encoder_check_is_cloned(struct intel_encoder *encoder);
> >  extern void intel_connector_dpms(struct drm_connector *, int mode);
> >  extern bool intel_connector_get_hw_state(struct intel_connector *connector);
> >  extern void intel_modeset_check_state(struct drm_device *dev);
> > +extern void intel_plane_restore(struct drm_plane *plane);
> >  
> >  
> >  static inline struct intel_encoder *intel_attached_encoder(struct drm_connector *connector)
> > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> > index 03cfd62..ca171af 100644
> > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > @@ -438,6 +438,15 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
> >  
> >  	old_obj = intel_plane->obj;
> >  
> > +	intel_plane->crtc_x = crtc_x;
> > +	intel_plane->crtc_y = crtc_y;
> > +	intel_plane->crtc_w = crtc_w;
> > +	intel_plane->crtc_h = crtc_h;
> > +	intel_plane->x = x;
> > +	intel_plane->y = y;
> 
> x and y are not fixed point numbers. They just contain the integer parts
> of src_x and src_y. So you need to use src_x and src_y here instead.
> 

Oops, thanks.  Will fix and use the suggested terminology to avoid
future confusion.

-- 
Jesse Barnes, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 4/6] drm/i915: enable VT switchless resume v2

[Jesse Barnes]

With the other bits in place, we can do this safely.

v2: disable backlight on suspend to prevent premature enablement on resume

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/i915_drv.c |   12 +++++++++---
 drivers/gpu/drm/i915/intel_fb.c |    3 +++
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index c5b8c81..e76b038 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -492,9 +492,10 @@ static int i915_drm_freeze(struct drm_device *dev)
 
 		cancel_delayed_work_sync(&dev_priv->rps.delayed_resume_work);
 
-		intel_modeset_disable(dev);
-
 		drm_irq_uninstall(dev);
+
+		if (dev_priv->backlight)
+			intel_panel_disable_backlight(dev);
 	}
 
 	i915_save_state(dev);
@@ -569,9 +570,14 @@ 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, false);
+
 		drm_irq_install(dev);
 		intel_hpd_init(dev);
+
+		/* Resume the modeset for every activated CRTC */
+		drm_modeset_lock_all(dev);
+		intel_modeset_setup_hw_state(dev, true);
+		drm_modeset_unlock_all(dev);
 	}
 
 	intel_opregion_init(dev);
diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c
index 1c510da..987bc33 100644
--- a/drivers/gpu/drm/i915/intel_fb.c
+++ b/drivers/gpu/drm/i915/intel_fb.c
@@ -149,6 +149,9 @@ static int intelfb_create(struct intel_fbdev *ifbdev,
 	}
 	info->screen_size = size;
 
+	/* This driver doesn't need a VT switch to restore the mode on resume */
+	info->skip_vt_switch = true;
+
 //	memset(info->screen_base, 0, size);
 
 	drm_fb_helper_fill_fix(info, fb->pitches[0], fb->depth);
-- 
1.7.9.5

Re: [PATCH 4/6] drm/i915: enable VT switchless resume v2

[Daniel Vetter]

On Tue, Feb 19, 2013 at 12:11:41PM -0800, Jesse Barnes wrote:
> With the other bits in place, we can do this safely.
> 
> v2: disable backlight on suspend to prevent premature enablement on resume
> 
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> ---
>  drivers/gpu/drm/i915/i915_drv.c |   12 +++++++++---
>  drivers/gpu/drm/i915/intel_fb.c |    3 +++
>  2 files changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index c5b8c81..e76b038 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -492,9 +492,10 @@ static int i915_drm_freeze(struct drm_device *dev)
>  
>  		cancel_delayed_work_sync(&dev_priv->rps.delayed_resume_work);
>  
> -		intel_modeset_disable(dev);

As discussed in person last week, simply dropping this will probably kill
S0i3 support.
-Daniel

> -
>  		drm_irq_uninstall(dev);
> +
> +		if (dev_priv->backlight)
> +			intel_panel_disable_backlight(dev);
>  	}
>  
>  	i915_save_state(dev);
> @@ -569,9 +570,14 @@ 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, false);
> +
>  		drm_irq_install(dev);
>  		intel_hpd_init(dev);
> +
> +		/* Resume the modeset for every activated CRTC */
> +		drm_modeset_lock_all(dev);
> +		intel_modeset_setup_hw_state(dev, true);
> +		drm_modeset_unlock_all(dev);
>  	}
>  
>  	intel_opregion_init(dev);
> diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c
> index 1c510da..987bc33 100644
> --- a/drivers/gpu/drm/i915/intel_fb.c
> +++ b/drivers/gpu/drm/i915/intel_fb.c
> @@ -149,6 +149,9 @@ static int intelfb_create(struct intel_fbdev *ifbdev,
>  	}
>  	info->screen_size = size;
>  
> +	/* This driver doesn't need a VT switch to restore the mode on resume */
> +	info->skip_vt_switch = true;
> +
>  //	memset(info->screen_base, 0, size);
>  
>  	drm_fb_helper_fill_fix(info, fb->pitches[0], fb->depth);
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 4/6] drm/i915: enable VT switchless resume v2

[Jesse Barnes]

On Mon, 18 Mar 2013 08:49:07 +0100
Daniel Vetter <daniel@ffwll.ch> wrote:

> On Tue, Feb 19, 2013 at 12:11:41PM -0800, Jesse Barnes wrote:
> > With the other bits in place, we can do this safely.
> > 
> > v2: disable backlight on suspend to prevent premature enablement on resume
> > 
> > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.c |   12 +++++++++---
> >  drivers/gpu/drm/i915/intel_fb.c |    3 +++
> >  2 files changed, 12 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > index c5b8c81..e76b038 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -492,9 +492,10 @@ static int i915_drm_freeze(struct drm_device *dev)
> >  
> >  		cancel_delayed_work_sync(&dev_priv->rps.delayed_resume_work);
> >  
> > -		intel_modeset_disable(dev);
> 
> As discussed in person last week, simply dropping this will probably kill
> S0i3 support.

Not really, since DPMS will be off in that case too generally, but it
does make testing harder.

I think it just needs to be a low level call to crtc disable on each
pipe, otherwise we'll zap the state we're trying to save.

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [PATCH 4/6] drm/i915: enable VT switchless resume v2

[Daniel Vetter]

On Mon, Mar 18, 2013 at 6:42 PM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> On Mon, 18 Mar 2013 08:49:07 +0100
> Daniel Vetter <daniel@ffwll.ch> wrote:
>
>> On Tue, Feb 19, 2013 at 12:11:41PM -0800, Jesse Barnes wrote:
>> > With the other bits in place, we can do this safely.
>> >
>> > v2: disable backlight on suspend to prevent premature enablement on resume
>> >
>> > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
>> > ---
>> >  drivers/gpu/drm/i915/i915_drv.c |   12 +++++++++---
>> >  drivers/gpu/drm/i915/intel_fb.c |    3 +++
>> >  2 files changed, 12 insertions(+), 3 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
>> > index c5b8c81..e76b038 100644
>> > --- a/drivers/gpu/drm/i915/i915_drv.c
>> > +++ b/drivers/gpu/drm/i915/i915_drv.c
>> > @@ -492,9 +492,10 @@ static int i915_drm_freeze(struct drm_device *dev)
>> >
>> >             cancel_delayed_work_sync(&dev_priv->rps.delayed_resume_work);
>> >
>> > -           intel_modeset_disable(dev);
>>
>> As discussed in person last week, simply dropping this will probably kill
>> S0i3 support.
>
> Not really, since DPMS will be off in that case too generally, but it
> does make testing harder.

Hm, where do we do that currently? Without fbcon I don't see it, and
we can't really rely on userspace to get this right I guess ...

> I think it just needs to be a low level call to crtc disable on each
> pipe, otherwise we'll zap the state we're trying to save.

That just reminded me that we also should restore the right dpms state
I think. At least I'm not too sure whether we'll currently do that
(and whether the modeset state tracker would catch it). Otoh dpms
standby/suspend died with gen4 ;-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 4/6] drm/i915: enable VT switchless resume v2

[Jesse Barnes]

On Mon, 18 Mar 2013 19:50:37 +0100
Daniel Vetter <daniel@ffwll.ch> wrote:

> On Mon, Mar 18, 2013 at 6:42 PM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> > On Mon, 18 Mar 2013 08:49:07 +0100
> > Daniel Vetter <daniel@ffwll.ch> wrote:
> >
> >> On Tue, Feb 19, 2013 at 12:11:41PM -0800, Jesse Barnes wrote:
> >> > With the other bits in place, we can do this safely.
> >> >
> >> > v2: disable backlight on suspend to prevent premature enablement on resume
> >> >
> >> > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> >> > ---
> >> >  drivers/gpu/drm/i915/i915_drv.c |   12 +++++++++---
> >> >  drivers/gpu/drm/i915/intel_fb.c |    3 +++
> >> >  2 files changed, 12 insertions(+), 3 deletions(-)
> >> >
> >> > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> >> > index c5b8c81..e76b038 100644
> >> > --- a/drivers/gpu/drm/i915/i915_drv.c
> >> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> >> > @@ -492,9 +492,10 @@ static int i915_drm_freeze(struct drm_device *dev)
> >> >
> >> >             cancel_delayed_work_sync(&dev_priv->rps.delayed_resume_work);
> >> >
> >> > -           intel_modeset_disable(dev);
> >>
> >> As discussed in person last week, simply dropping this will probably kill
> >> S0i3 support.
> >
> > Not really, since DPMS will be off in that case too generally, but it
> > does make testing harder.
> 
> Hm, where do we do that currently? Without fbcon I don't see it, and
> we can't really rely on userspace to get this right I guess ...

I was assuming userspace would do it, but yes we shouldn't require that.

> > I think it just needs to be a low level call to crtc disable on each
> > pipe, otherwise we'll zap the state we're trying to save.
> 
> That just reminded me that we also should restore the right dpms state
> I think. At least I'm not too sure whether we'll currently do that
> (and whether the modeset state tracker would catch it). Otoh dpms
> standby/suspend died with gen4 ;-)

Hm yeah haven't tested that at all.  One typical kind of suspend will
happen after DPMS off when the machine has been idle for some period.
When it comes back up the user will probably want to see the display.
But we don't have to enforce that on the kernel side; we can leave it
to userspace.

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [PATCH 4/6] drm/i915: enable VT switchless resume v2

[Daniel Vetter]

On Tue, Mar 19, 2013 at 5:56 PM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
>> > I think it just needs to be a low level call to crtc disable on each
>> > pipe, otherwise we'll zap the state we're trying to save.
>>
>> That just reminded me that we also should restore the right dpms state
>> I think. At least I'm not too sure whether we'll currently do that
>> (and whether the modeset state tracker would catch it). Otoh dpms
>> standby/suspend died with gen4 ;-)
>
> Hm yeah haven't tested that at all.  One typical kind of suspend will
> happen after DPMS off when the machine has been idle for some period.
> When it comes back up the user will probably want to see the display.
> But we don't have to enforce that on the kernel side; we can leave it
> to userspace.

Note that this isn't about dpms state in general, we'll take care of
that. The problem is with intermediate dpms levels, which requires us
to keep the pipe partially running. If you force-restore such a thing
we'll end up at dpms ON. Which isn't quite what we want.

Otoh it's old hw, so I don't think we need to spill too many brain
cycles on this issue. But if we do fix it, I think we should implement
proper support to read out that hw state and cross-check it -
otherwise it's pretty much guaranteed to be broken.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 4/6] drm/i915: enable VT switchless resume v2

[Daniel Vetter]

On Tue, Mar 19, 2013 at 06:13:09PM +0100, Daniel Vetter wrote:
> On Tue, Mar 19, 2013 at 5:56 PM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> >> > I think it just needs to be a low level call to crtc disable on each
> >> > pipe, otherwise we'll zap the state we're trying to save.
> >>
> >> That just reminded me that we also should restore the right dpms state
> >> I think. At least I'm not too sure whether we'll currently do that
> >> (and whether the modeset state tracker would catch it). Otoh dpms
> >> standby/suspend died with gen4 ;-)
> >
> > Hm yeah haven't tested that at all.  One typical kind of suspend will
> > happen after DPMS off when the machine has been idle for some period.
> > When it comes back up the user will probably want to see the display.
> > But we don't have to enforce that on the kernel side; we can leave it
> > to userspace.
> 
> Note that this isn't about dpms state in general, we'll take care of
> that. The problem is with intermediate dpms levels, which requires us
> to keep the pipe partially running. If you force-restore such a thing
> we'll end up at dpms ON. Which isn't quite what we want.
> 
> Otoh it's old hw, so I don't think we need to spill too many brain
> cycles on this issue. But if we do fix it, I think we should implement
> proper support to read out that hw state and cross-check it -
> otherwise it's pretty much guaranteed to be broken.

Ajax just submitted a patch to fix restoring of intermediate dpms levels,
so we should be good here. Another idea for safe/restore around suspend
which should just work is loop over all connectors and adjust the dpms
state (and safe just that piece somewhere). That way we get nice implicit
coverag of our dpms code while doing suspends and suspend doesn't need
anything in addition infrastructure wise.

The only downside is that we need to make the power wells stuff work with
dpms, too. But that's a requirement, anyway.

Now the real reason for writing this mail: 3.10 feature merge is drawing
to a close (in about a month I think) and imo we should get switchless
resume in a few weeks ahead. That way we can give it a decent beating
before it reaches Linus' upstream git. And I like switchless resume a lot.

So what's your plan here?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 4/6] drm/i915: enable VT switchless resume v2

[Jesse Barnes]

On Tue, 26 Mar 2013 13:14:48 +0100
Daniel Vetter <daniel@ffwll.ch> wrote:

> On Tue, Mar 19, 2013 at 06:13:09PM +0100, Daniel Vetter wrote:
> > On Tue, Mar 19, 2013 at 5:56 PM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> > >> > I think it just needs to be a low level call to crtc disable on each
> > >> > pipe, otherwise we'll zap the state we're trying to save.
> > >>
> > >> That just reminded me that we also should restore the right dpms state
> > >> I think. At least I'm not too sure whether we'll currently do that
> > >> (and whether the modeset state tracker would catch it). Otoh dpms
> > >> standby/suspend died with gen4 ;-)
> > >
> > > Hm yeah haven't tested that at all.  One typical kind of suspend will
> > > happen after DPMS off when the machine has been idle for some period.
> > > When it comes back up the user will probably want to see the display.
> > > But we don't have to enforce that on the kernel side; we can leave it
> > > to userspace.
> > 
> > Note that this isn't about dpms state in general, we'll take care of
> > that. The problem is with intermediate dpms levels, which requires us
> > to keep the pipe partially running. If you force-restore such a thing
> > we'll end up at dpms ON. Which isn't quite what we want.
> > 
> > Otoh it's old hw, so I don't think we need to spill too many brain
> > cycles on this issue. But if we do fix it, I think we should implement
> > proper support to read out that hw state and cross-check it -
> > otherwise it's pretty much guaranteed to be broken.
> 
> Ajax just submitted a patch to fix restoring of intermediate dpms levels,
> so we should be good here. Another idea for safe/restore around suspend
> which should just work is loop over all connectors and adjust the dpms
> state (and safe just that piece somewhere). That way we get nice implicit
> coverag of our dpms code while doing suspends and suspend doesn't need
> anything in addition infrastructure wise.
> 
> The only downside is that we need to make the power wells stuff work with
> dpms, too. But that's a requirement, anyway.
> 
> Now the real reason for writing this mail: 3.10 feature merge is drawing
> to a close (in about a month I think) and imo we should get switchless
> resume in a few weeks ahead. That way we can give it a decent beating
> before it reaches Linus' upstream git. And I like switchless resume a lot.
> 
> So what's your plan here?

What's left?  Just dealing with the shut down at suspend time (so that
suspend testing and RTD3 works) and Paulo's issue?

I'm not sure what to do about the latter, but I can spin up a new patch
with the shutdown bits in place.

-- 
Jesse Barnes, Intel Open Source Technology Center