[Intel-gfx] [DRM/I915]: Restore the modeset for every activated CRTC

[Jesse Barnes]

On Wed, 08 Jul 2009 09:25:44 +0800
yakui <yakui.zhao at intel.com> wrote:

> On Wed, 2009-07-08 at 02:40 +0800, Eric Anholt wrote:
> > On Thu, 2009-07-02 at 15:16 +0800, yakui.zhao at intel.com wrote:
> > > From: Zhao Yakui <yakui.zhao at intel.com>
> > > 
> > > Restore the modeset for every activated CRTC in course of resume.
> > > This is realized by calling the function of
> > > drm_helper_resume_force_mode. Note: it is meaningful only for the
> > > KMS mode.
> > > 
> > > Signed-off-by: Zhao Yakui <yakui.zhao at intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/i915_drv.c |    5 +++++
> > >  1 file changed, 5 insertions(+)
> > > 
> > > Index: linux-2.6/drivers/gpu/drm/i915/i915_drv.c
> > > ===================================================================
> > > --- linux-2.6.orig/drivers/gpu/drm/i915/i915_drv.c
> > > 2009-07-02 14:24:44.000000000 +0800 +++
> > > linux-2.6/drivers/gpu/drm/i915/i915_drv.c	2009-07-02
> > > 14:25:52.000000000 +0800 @@ -35,6 +35,7 @@ 
> > >  #include "drm_pciids.h"
> > >  #include <linux/console.h>
> > > +#include "drm_crtc_helper.h"
> > >  
> > >  static unsigned int i915_modeset = -1;
> > >  module_param_named(modeset, i915_modeset, int, 0400);
> > > @@ -115,6 +116,10 @@
> > >  
> > >  		drm_irq_install(dev);
> > >  	}
> > > +	if (drm_core_check_feature(dev, DRIVER_MODESET)) {
> > > +		/* Resume the modeset for every activated CRTC */
> > > +		drm_helper_resume_force_mode(dev);
> > > +	}
> > >  
> > >  	return ret;
> > >  }
> > 
> > So, this patch does a full modeset on top of the almost-full modeset
> > done by the suspend/resume code.  That seems bogus -- if we're
> > going to do this, we shouldn't do the rest of the save/restore for
> > mdoesetting.
> In course of suspend we will save some registers related with
> modesetting. Of course the registers related with interrupt/vga are
> also saved.
> And in course of resume we will restore the registers.
> After doing so, many boxes can work well.
> But if the modesetting registers are touched by BIOS in course of
> suspend/resume, maybe it is not enough to only restore the registers
> related with modesetting. It will be better that we restore the full
> modeset for the activated crtc. 
> And when the system is resumed from S4, the registers related with
> modesetting are not initialized correctly. In such case it will be
> better that we restore the modeset.

Right, we want to do the mode set.  But we don't want to touch the mode
setting regs twice at resume time (the current resume function
basically does mode sets on each output, since it restores the previous
config).

So your patch should split the suspend & resume functions into code
that executes in a non-KMS config and code that executes in a KMS
config.  In the non-KMS case, we'll need to save/restore everything,
like we do currently.  In the non-KMS case we only need to save/restore
certain global chip state.  Anything that the existing mode set
functions set doesn't have to be saved & restored, since the resume
mode set will take care of it.

Doing it that way will speed up suspend/resume and reduce flicker.

> This patch has no side effect for the box that can work well after
> using save/restore. And it can assure that the modeset before entering
> suspended state is restored correctly.
> 
> If the patch can be shipped, we can remove the register related with
> modesetting in save/restore when it works in KMS mode. In fact it
> doesn't matter if the we don't remove them. The side effect is that
> some registers are restored twice.
> > However, note that drm_helper_resume_force_mode doesn't turn off
> > disabled connectors, which seems wrong.
> the drm_helper_resume_force_mode doesn't touch the disabled
> connectors. Why is it necessary to turn off the disabled connectors
> again?

Two main reasons: one is that it restores the actual config of the
machine prior to suspend, and the other is that it helps save power.
If you resume your machine and an extra, unused pipe is turned on, your
battery will drain faster.

-- 
Jesse Barnes, Intel Open Source Technology Center