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

[Dave Airlie]

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.

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