[Intel-gfx] [PATCH v3 4/4] drm/i915: rename i915_init_power_well to i915_init_power_domains

Paulo Zanoni przanoni at gmail.com
Fri Oct 25 22:10:31 CEST 2013


2013/10/25 Imre Deak <imre.deak at intel.com>:
> Similarly rename the other related functions in the power domain
> interface.
>
> Higher level driver code calling these functions knows only about power
> domains, not the underlying power wells which may be different on
> different platforms. Also these functions really init/cleanup/resume
> power domains and only through that all related power wells, so rename
> them accordingly.
>
> Signed-off-by: Imre Deak <imre.deak at intel.com>

I agree with the "_domains" rename, I think it's an improvement, but
since you're already renaming things, I have to drop my bikeshed: we
have intel_init_power_{well,domains} and
i915_init_power_{well,domains}. IMHO this is really super annoyingly
confusing, because they sound like they do the same thing. I know it's
not your fault, but while you're at it, could you please propose names
to unconfuse this?

i915_init_power_well takes care of initializing the structs and
pointers, while intel_init_power_well is the only one that touches
hardware. A possible suggestion:

- i915_init_power_well becomes intel_init_power_domains (just because
I don't like the "i915_" prefix, since the PM code uses "intel_" for
everything).
- i915_remove_power_well becomes intel_remove_power_domains (to match
the one above)
- intel_init_power_well becomes intel_init_power_domains_hardware or
intel_init_power_wells (since on the HW these things are actually
called "power wells") or intel_init_hw_power_wells (to combine both
suggestions)

Thanks,
Paulo

> ---
>  drivers/gpu/drm/i915/i915_dma.c  |  8 ++++----
>  drivers/gpu/drm/i915/i915_drv.c  |  2 +-
>  drivers/gpu/drm/i915/intel_drv.h |  6 +++---
>  drivers/gpu/drm/i915/intel_pm.c  | 10 +++++-----
>  4 files changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index b722b35..75f02e4 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1311,7 +1311,7 @@ static int i915_load_modeset_init(struct drm_device *dev)
>         if (ret)
>                 goto cleanup_gem_stolen;
>
> -       intel_init_power_well(dev);
> +       intel_init_power_domains(dev);
>
>         /* Important: The output setup functions called by modeset_init need
>          * working irqs for e.g. gmbus and dp aux transfers. */
> @@ -1640,7 +1640,7 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
>         }
>
>         if (HAS_POWER_WELL(dev))
> -               i915_init_power_well(dev);
> +               i915_init_power_domains(dev);
>
>         if (drm_core_check_feature(dev, DRIVER_MODESET)) {
>                 ret = i915_load_modeset_init(dev);
> @@ -1668,7 +1668,7 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
>
>  out_power_well:
>         if (HAS_POWER_WELL(dev))
> -               i915_remove_power_well(dev);
> +               i915_remove_power_domains(dev);
>         drm_vblank_cleanup(dev);
>  out_gem_unload:
>         if (dev_priv->mm.inactive_shrinker.scan_objects)
> @@ -1711,7 +1711,7 @@ int i915_driver_unload(struct drm_device *dev)
>                  * the power well is not enabled, so just enable it in case
>                  * we're going to unload/reload. */
>                 intel_display_set_init_power(dev, true);
> -               i915_remove_power_well(dev);
> +               i915_remove_power_domains(dev);
>         }
>
>         i915_teardown_sysfs(dev);
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 770c9f8..e4474c6 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -597,7 +597,7 @@ static int __i915_drm_thaw(struct drm_device *dev, bool restore_gtt_mappings)
>                 mutex_unlock(&dev->struct_mutex);
>         }
>
> -       intel_init_power_well(dev);
> +       intel_init_power_domains(dev);
>
>         i915_restore_state(dev);
>         intel_opregion_setup(dev);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index bf4394a..ae6d362 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -823,15 +823,15 @@ bool intel_fbc_enabled(struct drm_device *dev);
>  void intel_update_fbc(struct drm_device *dev);
>  void intel_gpu_ips_init(struct drm_i915_private *dev_priv);
>  void intel_gpu_ips_teardown(void);
> -int i915_init_power_well(struct drm_device *dev);
> -void i915_remove_power_well(struct drm_device *dev);
> +int i915_init_power_domains(struct drm_device *dev);
> +void i915_remove_power_domains(struct drm_device *dev);
>  bool intel_display_power_enabled(struct drm_device *dev,
>                                  enum intel_display_power_domain domain);
>  void intel_display_power_get(struct drm_device *dev,
>                              enum intel_display_power_domain domain);
>  void intel_display_power_put(struct drm_device *dev,
>                              enum intel_display_power_domain domain);
> -void intel_init_power_well(struct drm_device *dev);
> +void intel_init_power_domains(struct drm_device *dev);
>  void intel_set_power_well(struct drm_device *dev, bool enable);
>  void intel_enable_gt_powersave(struct drm_device *dev);
>  void intel_disable_gt_powersave(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 4f84a4b..9afec23 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5697,7 +5697,7 @@ void i915_release_power_well(void)
>  }
>  EXPORT_SYMBOL_GPL(i915_release_power_well);
>
> -int i915_init_power_well(struct drm_device *dev)
> +int i915_init_power_domains(struct drm_device *dev)
>  {
>         struct drm_i915_private *dev_priv = dev->dev_private;
>         struct i915_power_domains *power_domains = &dev_priv->power_domains;
> @@ -5712,12 +5712,12 @@ int i915_init_power_well(struct drm_device *dev)
>         return 0;
>  }
>
> -void i915_remove_power_well(struct drm_device *dev)
> +void i915_remove_power_domains(struct drm_device *dev)
>  {
>         hsw_pwr = NULL;
>  }
>
> -static void intel_resume_power_well(struct drm_device *dev)
> +static void intel_resume_power_domains(struct drm_device *dev)
>  {
>         struct drm_i915_private *dev_priv = dev->dev_private;
>         struct i915_power_domains *power_domains = &dev_priv->power_domains;
> @@ -5740,7 +5740,7 @@ static void intel_resume_power_well(struct drm_device *dev)
>   * to be enabled, and it will only be disabled if none of the registers is
>   * requesting it to be enabled.
>   */
> -void intel_init_power_well(struct drm_device *dev)
> +void intel_init_power_domains(struct drm_device *dev)
>  {
>         struct drm_i915_private *dev_priv = dev->dev_private;
>
> @@ -5749,7 +5749,7 @@ void intel_init_power_well(struct drm_device *dev)
>
>         /* For now, we need the power well to be always enabled. */
>         intel_display_set_init_power(dev, true);
> -       intel_resume_power_well(dev);
> +       intel_resume_power_domains(dev);
>
>         /* We're taking over the BIOS, so clear any requests made by it since
>          * the driver is in charge now. */
> --
> 1.8.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni



More information about the Intel-gfx mailing list