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

[Jesse Barnes]

On Wed, 8 Jul 2009 06:08:31 +1000
Dave Airlie <airlied at gmail.com> wrote:

> On 08/07/2009, at 4:57, Jesse Barnes <jbarnes at virtuousgeek.org> wrote:
> 
> > On Tue, 07 Jul 2009 11:40:10 -0700
> > Eric Anholt <eric at anholt.net> 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. However, note that
> >> drm_helper_resume_force_mode doesn't turn off disabled connectors,
> >> which seems wrong.
> >
> > Yeah, that's one problem; I think the code should also be
> > structured a bit differently.  We really want to track whole KMS
> > configurations (output configuration and associated buffers), and
> > have functions for saving & restoring those (including forced
> > restoration, e.g. for lastclose or panic).  Right now the two uses
> > are a bit muddled, which means panic/lastclose can fail, and
> > force_mode doesn't do the same thing as a regular set_crtc call.
> 
> Force mode can't do the same thing unless you tear down the crtcs
> and store the current modes on suspend which is pointless. The whole
> point of force mode is to force the mode the sw currently thinks is
> in the hw but isn't due to us having suspended. It is not meant for
> panic paths etc.

Ok, but it doesn't look like it works for the resume case either, since
it won't turn off unused stuff like the normal mode set path does.  So
we still need to handle both cases better.

-- 
Jesse Barnes, Intel Open Source Technology Center