On Wed, 16 Oct 2013 17:25:53 +0300
Imre Deak <imre.deak@xxxxxxxxx> wrote:
> Currently we make sure that all power domains are enabled during driver
> init and turn off unneded ones only after the first modeset. Similarly
> during suspend we enable all power domains, which will remain on through
> the following resume until the first modeset.
>
> This logic is supported by intel_set_power_well() in the power domain
> framework. It would be nice to simplify the API, so that we only have
> get/put functions and make it more explicit on the higher level how this
> "power well on during init" logic works. This will make it also easier
> if in the future we want to shorten the time the power wells are on.
>
> For this add a new device private flag tracking whether we have the
> power wells on because of init/suspend and use only
> intel_display_power_get()/put(). As nothing else uses
> intel_set_power_well() we can remove it.
>
> Signed-off-by: Imre Deak <imre.deak@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_dma.c | 2 +-
> drivers/gpu/drm/i915/i915_drv.c | 2 +-
> drivers/gpu/drm/i915/i915_drv.h | 7 ++++++-
> drivers/gpu/drm/i915/intel_display.c | 17 +++++++++++++++++
> drivers/gpu/drm/i915/intel_drv.h | 1 +
> drivers/gpu/drm/i915/intel_pm.c | 35 +----------------------------------
> 6 files changed, 27 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index b8bc887..dd7f1f6 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1708,7 +1708,7 @@ int i915_driver_unload(struct drm_device *dev)
> /* The i915.ko module is still not prepared to be loaded when
> * the power well is not enabled, so just enable it in case
> * we're going to unload/reload. */
> - intel_set_power_well(dev, true);
> + intel_display_set_init_power(dev, true);
> i915_remove_power_well(dev);
> }
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 59649c0..7299a96 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -469,7 +469,7 @@ static int i915_drm_freeze(struct drm_device *dev)
> /* We do a lot of poking in a lot of registers, make sure they work
> * properly. */
> hsw_disable_package_c8(dev_priv);
> - intel_set_power_well(dev, true);
> + intel_display_set_init_power(dev, true);
>
> drm_kms_helper_poll_disable(dev);
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index e4354dd..0557c6b 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -100,6 +100,7 @@ enum intel_display_power_domain {
> POWER_DOMAIN_TRANSCODER_C,
> POWER_DOMAIN_TRANSCODER_EDP,
> POWER_DOMAIN_VGA,
> + POWER_DOMAIN_INIT,
>
> POWER_DOMAIN_NUM,
> };
> @@ -913,7 +914,6 @@ struct i915_power_well {
> struct mutex lock;
> /* power well enable/disable usage count */
> int count;
> - int i915_request;
> };
>
> struct i915_dri1_state {
> @@ -1369,6 +1369,11 @@ typedef struct drm_i915_private {
> * mchdev_lock in intel_pm.c */
> struct intel_ilk_power_mgmt ips;
>
> + /*
> + * Power wells needed for initialization at driver init and suspend
> + * time are on. They are kept on until after the first modeset.
> + */
> + bool init_power_on;
> /* Haswell power well */
> struct i915_power_well power_well;
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index e2a4f3b..e30db91 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6581,6 +6581,21 @@ static unsigned long get_pipe_power_domains(struct drm_device *dev,
> return mask;
> }
>
> +void intel_display_set_init_power(struct drm_device *dev, bool enable)
> +{
> + struct drm_i915_private *dev_priv = dev->dev_private;
> +
> + if (dev_priv->init_power_on == enable)
> + return;
> +
> + if (enable)
> + intel_display_power_get(dev, POWER_DOMAIN_INIT);
> + else
> + intel_display_power_put(dev, POWER_DOMAIN_INIT);
> +
> + dev_priv->init_power_on = enable;
> +}
> +
> static void modeset_update_power_wells(struct drm_device *dev)
> {
> unsigned long pipe_domains[I915_MAX_PIPES] = { 0, };
> @@ -6612,6 +6627,8 @@ static void modeset_update_power_wells(struct drm_device *dev)
>
> crtc->enabled_power_domains = pipe_domains[crtc->pipe];
> }
> +
> + intel_display_set_init_power(dev, false);
> }
>
> static void haswell_modeset_global_resources(struct drm_device *dev)
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 63a5bfd..e6306f5 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -680,6 +680,7 @@ bool intel_crtc_active(struct drm_crtc *crtc);
> void i915_disable_vga_mem(struct drm_device *dev);
> void hsw_enable_ips(struct intel_crtc *crtc);
> void hsw_disable_ips(struct intel_crtc *crtc);
> +void intel_display_set_init_power(struct drm_device *dev, bool enable);
>
>
> /* intel_dp.c */
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index f7363a8..9291a2e 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5542,39 +5542,6 @@ void i915_remove_power_well(struct drm_device *dev)
> hsw_pwr = NULL;
> }
>
> -void intel_set_power_well(struct drm_device *dev, bool enable)
> -{
> - struct drm_i915_private *dev_priv = dev->dev_private;
> - struct i915_power_well *power_well = &dev_priv->power_well;
> -
> - if (!HAS_POWER_WELL(dev))
> - return;
> -
> - if (!i915_disable_power_well && !enable)
> - return;
> -
> - mutex_lock(&power_well->lock);
> -
> - /*
> - * This function will only ever contribute one
> - * to the power well reference count. i915_request
> - * is what tracks whether we have or have not
> - * added the one to the reference count.
> - */
> - if (power_well->i915_request == enable)
> - goto out;
> -
> - power_well->i915_request = enable;
> -
> - if (enable)
> - __intel_power_well_get(power_well);
> - else
> - __intel_power_well_put(power_well);
> -
> - out:
> - mutex_unlock(&power_well->lock);
> -}
> -
> static void intel_resume_power_well(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -5602,7 +5569,7 @@ void intel_init_power_well(struct drm_device *dev)
> return;
>
> /* For now, we need the power well to be always enabled. */
> - intel_set_power_well(dev, true);
> + intel_display_set_init_power(dev, true);
> intel_resume_power_well(dev);
>
> /* We're taking over the BIOS, so clear any requests made by it since
And there it goes, yay! :) Now we just have to get rid of the _INIT
domain and actually shut things down before the first mode set if
nothing is in use (maybe with a delayed shutdown to avoid toggling too
fast).
Reviewed-by: Jesse Barnes <jbarnes@xxxxxxxxxxxxxxxx>
--
Jesse Barnes, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
