2013/10/25 Imre Deak <imre.deak@xxxxxxxxx>:
> 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@xxxxxxxxx>
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@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Paulo Zanoni
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
