[Intel-gfx] [PATCH 66/66] drm/i915: runtime PM support for DPMS

[Paulo Zanoni]

2014-04-24 18:55 GMT-03:00 Daniel Vetter <daniel.vetter at ffwll.ch>:
> Keeping track of the power domains is a bit messy since crtc->active
> is currently updated by the platform hooks, but we need to be aware of
> which state transition exactly is going on. Maybe we simply need to
> shovel all the power domain handling down into platform code to
> simplify this. But doing that requires some more auditing since
> currently the ->mode_set callbacks still read some random registers
> (to e.g. figure out the reference clocks).
>
> Also note that intel_crtc_update_dpms is always call first/last even
> for encoders which have their own dpms functions. Hence we really only
> need to update this place here.
>
> Being a quick "does it blow up?" run not really tested yet.

Shouldn't we put some "if (is hsw or newer)" here? I was assuming your
series did not care about the SNB code paths, and we do have SNB
runtime PM support.

I also agree with Jesse's plans to push things to crtc_enable/disable,
then remove this code and the code from modeset_global_resources. But
if that's complicated, for now the current solution is fine.

>
> Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 26 ++++++++++++++++++++++----
>  1 file changed, 22 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index e0bd0f94e43e..1b5d6b099b37 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4478,16 +4478,34 @@ void intel_crtc_update_dpms(struct drm_crtc *crtc)
>  {
>         struct drm_device *dev = crtc->dev;
>         struct drm_i915_private *dev_priv = dev->dev_private;
> +       struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>         struct intel_encoder *intel_encoder;
> +       enum intel_display_power_domain domain;
> +       unsigned long domains;
>         bool enable = false;
>
>         for_each_encoder_on_crtc(dev, crtc, intel_encoder)
>                 enable |= intel_encoder->connectors_active;
>
> -       if (enable)
> -               dev_priv->display.crtc_enable(crtc);
> -       else
> -               dev_priv->display.crtc_disable(crtc);
> +       if (enable) {
> +               if (!intel_crtc->active) {
> +                       domains = get_crtc_power_domains(crtc);
> +                       for_each_power_domain(domain, domains)
> +                               intel_display_power_get(dev_priv, domain);
> +                       intel_crtc->enabled_power_domains = domains;
> +
> +                       dev_priv->display.crtc_enable(crtc);
> +               }
> +       } else {
> +               if (intel_crtc->active) {
> +                       dev_priv->display.crtc_disable(crtc);
> +
> +                       domains = intel_crtc->enabled_power_domains;
> +                       for_each_power_domain(domain, domains)
> +                               intel_display_power_put(dev_priv, domain);
> +                       intel_crtc->enabled_power_domains = 0;
> +               }
> +       }
>
>         intel_crtc_update_sarea(crtc, enable);
>  }
> --
> 1.8.1.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni