On Sun, 24 Jun 2012 16:42:32 +0200
Daniel Vetter <daniel.vetter at ffwll.ch> wrote:
> ... instead of calling each one for each generation indiviudally.
>
> Notice that we've already managed to be inconsistent, the resume path
> is missing an IS_VLV check. As a nice benefit we can mark all the
> platform specific enable/disable functions as static and hide them in
> intel_pm.c
>
> Signed-Off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
I like what you've done here. Some comments inline, still.
> ---
> drivers/gpu/drm/i915/i915_suspend.c | 5 +----
> drivers/gpu/drm/i915/intel_display.c | 21 ++-----------------
> drivers/gpu/drm/i915/intel_drv.h | 9 ++-------
> drivers/gpu/drm/i915/intel_pm.c | 37 +++++++++++++++++++++++++++-------
> 4 files changed, 35 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_suspend.c b/drivers/gpu/drm/i915/i915_suspend.c
> index 0ede02a..740c076 100644
> --- a/drivers/gpu/drm/i915/i915_suspend.c
> +++ b/drivers/gpu/drm/i915/i915_suspend.c
> @@ -825,10 +825,7 @@ int i915_save_state(struct drm_device *dev)
> dev_priv->saveIMR = I915_READ(IMR);
> }
>
> - if (IS_IRONLAKE_M(dev))
> - ironlake_disable_drps(dev);
> - if (INTEL_INFO(dev)->gen >= 6)
> - gen6_disable_rps(dev);
> + intel_disable_gt_powersave(dev);
>
> /* Cache mode state */
> dev_priv->saveCACHE_MODE_0 = I915_READ(CACHE_MODE_0);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index b3052ef..fdca5b9 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -7172,20 +7172,9 @@ static void ivb_pch_pwm_override(struct drm_device *dev)
>
> void intel_modeset_init_hw(struct drm_device *dev)
> {
> - struct drm_i915_private *dev_priv = dev->dev_private;
> -
> intel_init_clock_gating(dev);
>
> - if (IS_IRONLAKE_M(dev)) {
> - ironlake_enable_drps(dev);
> - ironlake_enable_rc6(dev);
> - intel_init_emon(dev);
> - }
> -
> - if ((IS_GEN6(dev) || IS_GEN7(dev)) && !IS_VALLEYVIEW(dev)) {
> - gen6_enable_rps(dev_priv);
> - gen6_update_ring_freq(dev_priv);
> - }
> + intel_enable_gt_powersave(dev);
>
> if (IS_IVYBRIDGE(dev))
> ivb_pch_pwm_override(dev);
> @@ -7277,13 +7266,7 @@ void intel_modeset_cleanup(struct drm_device *dev)
>
> intel_disable_fbc(dev);
>
> - if (IS_IRONLAKE_M(dev))
> - ironlake_disable_drps(dev);
> - if ((IS_GEN6(dev) || IS_GEN7(dev)) && !IS_VALLEYVIEW(dev))
> - gen6_disable_rps(dev);
> -
> - if (IS_IRONLAKE_M(dev))
> - ironlake_disable_rc6(dev);
> + intel_disable_gt_powersave(dev);
>
> if (IS_VALLEYVIEW(dev))
> vlv_init_dpio(dev);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 5290e9d..cc1573b 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -425,9 +425,6 @@ extern void intel_crtc_fb_gamma_set(struct drm_crtc *crtc, u16 red, u16 green,
> extern void intel_crtc_fb_gamma_get(struct drm_crtc *crtc, u16 *red, u16 *green,
> u16 *blue, int regno);
> extern void intel_enable_clock_gating(struct drm_device *dev);
> -extern void ironlake_disable_rc6(struct drm_device *dev);
> -extern void ironlake_enable_drps(struct drm_device *dev);
> -extern void ironlake_disable_drps(struct drm_device *dev);
>
> extern int intel_pin_and_fence_fb_obj(struct drm_device *dev,
> struct drm_i915_gem_object *obj,
> @@ -494,10 +491,8 @@ extern void intel_update_fbc(struct drm_device *dev);
> extern void intel_gpu_ips_init(struct drm_i915_private *dev_priv);
> extern void intel_gpu_ips_teardown(void);
>
> -extern void gen6_enable_rps(struct drm_i915_private *dev_priv);
> -extern void gen6_update_ring_freq(struct drm_i915_private *dev_priv);
> -extern void gen6_disable_rps(struct drm_device *dev);
> -extern void intel_init_emon(struct drm_device *dev);
> +extern void intel_enable_gt_powersave(struct drm_device *dev);
> +extern void intel_disable_gt_powersave(struct drm_device *dev);
>
> extern void intel_ddi_dpms(struct drm_encoder *encoder, int mode);
> extern void intel_ddi_mode_set(struct drm_encoder *encoder,
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 7504fbc..2baba10 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2184,7 +2184,7 @@ bool ironlake_set_drps(struct drm_device *dev, u8 val)
> return true;
> }
>
> -void ironlake_enable_drps(struct drm_device *dev)
> +static void ironlake_enable_drps(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> u32 rgvmodectl = I915_READ(MEMMODECTL);
> @@ -2248,7 +2248,7 @@ void ironlake_enable_drps(struct drm_device *dev)
> getrawmonotonic(&dev_priv->last_time2);
> }
>
> -void ironlake_disable_drps(struct drm_device *dev)
> +static void ironlake_disable_drps(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> u16 rgvswctl = I915_READ16(MEMSWCTL);
> @@ -2301,7 +2301,7 @@ void gen6_set_rps(struct drm_device *dev, u8 val)
> dev_priv->cur_delay = val;
> }
>
> -void gen6_disable_rps(struct drm_device *dev)
> +static void gen6_disable_rps(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
>
> @@ -2349,7 +2349,7 @@ int intel_enable_rc6(const struct drm_device *dev)
> return (INTEL_RC6_ENABLE | INTEL_RC6p_ENABLE);
> }
>
> -void gen6_enable_rps(struct drm_i915_private *dev_priv)
> +static void gen6_enable_rps(struct drm_i915_private *dev_priv)
> {
> struct intel_ring_buffer *ring;
> u32 rp_state_cap;
> @@ -2494,7 +2494,7 @@ void gen6_enable_rps(struct drm_i915_private *dev_priv)
> mutex_unlock(&dev_priv->dev->struct_mutex);
> }
>
> -void gen6_update_ring_freq(struct drm_i915_private *dev_priv)
> +static void gen6_update_ring_freq(struct drm_i915_private *dev_priv)
> {
> int min_freq = 15;
> int gpu_freq, ia_freq, max_ia_freq;
> @@ -3156,8 +3156,7 @@ void intel_gpu_ips_teardown(void)
> i915_mch_dev = NULL;
> spin_unlock(&mchdev_lock);
> }
> -
> -void intel_init_emon(struct drm_device *dev)
> +static void intel_init_emon(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> u32 lcfuse;
> @@ -3228,6 +3227,30 @@ void intel_init_emon(struct drm_device *dev)
> dev_priv->corr = (lcfuse & LCFUSE_HIV_MASK);
> }
>
> +void intel_disable_gt_powersave(struct drm_device *dev)
> +{
> + if (IS_IRONLAKE_M(dev))
> + ironlake_disable_drps(dev);
> + if (INTEL_INFO(dev)->gen >= 6 && !IS_VALLEYVIEW(dev))
> + gen6_disable_rps(dev);
> +}
What happened to ironlake_disable_rc6?
> +
> +void intel_enable_gt_powersave(struct drm_device *dev)
> +{
> + struct drm_i915_private *dev_priv = dev->dev_private;
> +
> + if (IS_IRONLAKE_M(dev)) {
> + ironlake_enable_drps(dev);
> + ironlake_enable_rc6(dev);
> + intel_init_emon(dev);
> + }
> +
> + if ((IS_GEN6(dev) || IS_GEN7(dev)) && !IS_VALLEYVIEW(dev)) {
> + gen6_enable_rps(dev_priv);
> + gen6_update_ring_freq(dev_priv);
> + }
> +}
> +
> static void ironlake_init_clock_gating(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
It'd be nice if there was some explanation of why disable doesn't do the
reverse of enable.
--
Ben Widawsky, Intel Open Source Technology Center
