Re: [PATCH v1 1/1] drm/i915: Perform modeset based on DPMS state during resume

[Daniel Vetter <daniel@ffwll.ch>]

On Mon, Dec 01, 2014 at 12:29:34PM +0530, sagar.a.kamble@intel.com wrote:
> From: Akash Goel <akash.goel@intel.com>
> 
> During resume, modeset was being performed independent of DPMS state which
> increased resume time as well as it kept display wells ON. With this change
> this modeset will be skipped.
> 
> Signed-off-by: Akash Goel <akash.goel@intel.com>
> Signed-off-by: Sagar Kamble <sagar.a.kamble@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.c | 33 ++++++++++++++++++++++++++++++---
>  1 file changed, 30 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 71be3c9..919e552 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -671,6 +671,31 @@ int i915_suspend_legacy(struct drm_device *dev, pm_message_t state)
>  	return i915_drm_suspend_late(dev);
>  }
>  
> +static bool display_is_on(struct drm_device *dev)
> +{
> +	struct drm_connector *connector;
> +	bool display_is_on = false;
> +
> +	drm_modeset_lock_all(dev);
> +	list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
> +		if (!connector->encoder || !connector->encoder->crtc)
> +			continue;
> +		/*
> +		 * If Display wasn't turned off, before going to suspend then
> +		 * it should be re-enabled now, as we don't expect the DPMS ON
> +		 * call to come in that case
> +		 */
> +		if (connector->dpms != DRM_MODE_DPMS_OFF) {
> +			DRM_DEBUG_KMS("Display was on before suspend\n");
> +			display_is_on = true;
> +			break;
> +		}
> +	}
> +	drm_modeset_unlock_all(dev);
> +
> +	return display_is_on;
> +}

Imo this is at the wrong level - we should recompute what
intel_crtc->active should be like in intel_crtc_update_dpms. Maybe we need
to recompute intel_encoder->connectors_active, but I think that should
work out without any other changes.

And that logic should be in the setup_hw_state restore logic.
-Daniel

> +
>  static int i915_drm_resume(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -707,9 +732,11 @@ static int i915_drm_resume(struct drm_device *dev)
>  		spin_unlock_irq(&dev_priv->irq_lock);
>  
>  		intel_dp_mst_resume(dev);
> -		drm_modeset_lock_all(dev);
> -		intel_modeset_setup_hw_state(dev, true);
> -		drm_modeset_unlock_all(dev);
> +		if (display_is_on(dev)) {
> +			drm_modeset_lock_all(dev);
> +			intel_modeset_setup_hw_state(dev, true);
> +			drm_modeset_unlock_all(dev);
> +		}
>  
>  		/*
>  		 * ... but also need to make sure that hotplug processing
> -- 
> 1.8.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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx