Re: [PATCH 01/19] drm/i915: use drm_i915_private everywhere in the power domain api

Jesse Barnes
2014-02-20T19:23:20Z

On Tue, 18 Feb 2014 00:02:02 +0200
Imre Deak <imre.deak@intel.com> wrote:

> The power domains framework is internal to the i915 driver, so pass
> drm_i915_private instead of drm_device to its functions.
>
> Also remove a dangling intel_set_power_well() declaration.
>
> No functional change.
>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
> drivers/gpu/drm/i915/i915_dma.c | 14 ++++----
> drivers/gpu/drm/i915/i915_drv.c | 4 +--
> drivers/gpu/drm/i915/i915_drv.h | 4 +--
> drivers/gpu/drm/i915/intel_display.c | 27 +++++++--------
> drivers/gpu/drm/i915/intel_drv.h | 17 +++++-----
> drivers/gpu/drm/i915/intel_pm.c | 65 ++++++++++++++----------------------
> 6 files changed, 58 insertions(+), 73 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 7688abc..8177c17 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1325,7 +1325,7 @@ static int i915_load_modeset_init(struct drm_device *dev)
> if (ret)
> goto cleanup_gem_stolen;
>
> - intel_power_domains_init_hw(dev);
> + intel_power_domains_init_hw(dev_priv);
>
> /* Important: The output setup functions called by modeset_init need
> * working irqs for e.g. gmbus and dp aux transfers. */
> @@ -1343,7 +1343,7 @@ static int i915_load_modeset_init(struct drm_device *dev)
> /* FIXME: do pre/post-mode set stuff in core KMS code */
> dev->vblank_disable_allowed = true;
> if (INTEL_INFO(dev)->num_pipes == 0) {
> - intel_display_power_put(dev, POWER_DOMAIN_VGA);
> + intel_display_power_put(dev_priv, POWER_DOMAIN_VGA);
> return 0;
> }
>
> @@ -1381,7 +1381,7 @@ cleanup_gem:
> WARN_ON(dev_priv->mm.aliasing_ppgtt);
> drm_mm_takedown(&dev_priv->gtt.base.mm);
> cleanup_power:
> - intel_display_power_put(dev, POWER_DOMAIN_VGA);
> + intel_display_power_put(dev_priv, POWER_DOMAIN_VGA);
> drm_irq_uninstall(dev);
> cleanup_gem_stolen:
> i915_gem_cleanup_stolen(dev);
> @@ -1702,7 +1702,7 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
> goto out_gem_unload;
> }
>
> - intel_power_domains_init(dev);
> + intel_power_domains_init(dev_priv);
>
> if (drm_core_check_feature(dev, DRIVER_MODESET)) {
> ret = i915_load_modeset_init(dev);
> @@ -1731,7 +1731,7 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
> return 0;
>
> out_power_well:
> - intel_power_domains_remove(dev);
> + intel_power_domains_remove(dev_priv);
> drm_vblank_cleanup(dev);
> out_gem_unload:
> if (dev_priv->mm.inactive_shrinker.scan_objects)
> @@ -1781,8 +1781,8 @@ 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_display_set_init_power(dev, true);
> - intel_power_domains_remove(dev);
> + intel_display_set_init_power(dev_priv, true);
> + intel_power_domains_remove(dev_priv);
>
> i915_teardown_sysfs(dev);
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 08052f3d..ce898af 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -429,7 +429,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_display_set_init_power(dev, true);
> + intel_display_set_init_power(dev_priv, true);
>
> drm_kms_helper_poll_disable(dev);
>
> @@ -551,7 +551,7 @@ static int __i915_drm_thaw(struct drm_device *dev, bool restore_gtt_mappings)
> mutex_unlock(&dev->struct_mutex);
> }
>
> - intel_power_domains_init_hw(dev);
> + intel_power_domains_init_hw(dev_priv);
>
> i915_restore_state(dev);
> intel_opregion_setup(dev);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 8c64831..796f971 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1024,9 +1024,9 @@ struct i915_power_well {
> int count;
> unsigned long domains;
> void *data;
> - void (*set)(struct drm_device *dev, struct i915_power_well *power_well,
> + void (*set)(struct drm_i915_private *dev_priv, struct i915_power_well *power_well,
> bool enable);
> - bool (*is_enabled)(struct drm_device *dev,
> + bool (*is_enabled)(struct drm_i915_private *dev_priv,
> 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 f19e6ea..17c2033 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -1122,7 +1122,7 @@ void assert_pipe(struct drm_i915_private *dev_priv,
> if (pipe == PIPE_A && dev_priv->quirks & QUIRK_PIPEA_FORCE)
> state = true;
>
> - if (!intel_display_power_enabled(dev_priv->dev,
> + if (!intel_display_power_enabled(dev_priv,
> POWER_DOMAIN_TRANSCODER(cpu_transcoder))) {
> cur_state = false;
> } else {
> @@ -6858,23 +6858,23 @@ 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)
> +void intel_display_set_init_power(struct drm_i915_private *dev_priv,
> + bool enable)
> {
> - struct drm_i915_private *dev_priv = dev->dev_private;
> -
> if (dev_priv->power_domains.init_power_on == enable)
> return;
>
> if (enable)
> - intel_display_power_get(dev, POWER_DOMAIN_INIT);
> + intel_display_power_get(dev_priv, POWER_DOMAIN_INIT);
> else
> - intel_display_power_put(dev, POWER_DOMAIN_INIT);
> + intel_display_power_put(dev_priv, POWER_DOMAIN_INIT);
>
> dev_priv->power_domains.init_power_on = enable;
> }
>
> static void modeset_update_power_wells(struct drm_device *dev)
> {
> + struct drm_i915_private *dev_priv = dev->dev_private;
> unsigned long pipe_domains[I915_MAX_PIPES] = { 0, };
> struct intel_crtc *crtc;
>
> @@ -6893,19 +6893,19 @@ static void modeset_update_power_wells(struct drm_device *dev)
> crtc->config.pch_pfit.enabled);
>
> for_each_power_domain(domain, pipe_domains[crtc->pipe])
> - intel_display_power_get(dev, domain);
> + intel_display_power_get(dev_priv, domain);
> }
>
> list_for_each_entry(crtc, &dev->mode_config.crtc_list, base.head) {
> enum intel_display_power_domain domain;
>
> for_each_power_domain(domain, crtc->enabled_power_domains)
> - intel_display_power_put(dev, domain);
> + intel_display_power_put(dev_priv, domain);
>
> crtc->enabled_power_domains = pipe_domains[crtc->pipe];
> }
>
> - intel_display_set_init_power(dev, false);
> + intel_display_set_init_power(dev_priv, false);
> }
>
> static void haswell_modeset_global_resources(struct drm_device *dev)
> @@ -6986,7 +6986,7 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
> pipe_config->cpu_transcoder = TRANSCODER_EDP;
> }
>
> - if (!intel_display_power_enabled(dev,
> + if (!intel_display_power_enabled(dev_priv,
> POWER_DOMAIN_TRANSCODER(pipe_config->cpu_transcoder)))
> return false;
>
> @@ -7014,7 +7014,7 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
> intel_get_pipe_timings(crtc, pipe_config);
>
> pfit_domain = POWER_DOMAIN_PIPE_PANEL_FITTER(crtc->pipe);
> - if (intel_display_power_enabled(dev, pfit_domain))
> + if (intel_display_power_enabled(dev_priv, pfit_domain))
> ironlake_get_pfit_config(crtc, pipe_config);
>
> if (IS_HASWELL(dev))
> @@ -11602,7 +11602,8 @@ intel_display_capture_error_state(struct drm_device *dev)
>
> for_each_pipe(i) {
> error->pipe[i].power_domain_on =
> - intel_display_power_enabled_sw(dev, POWER_DOMAIN_PIPE(i));
> + intel_display_power_enabled_sw(dev_priv,
> + POWER_DOMAIN_PIPE(i));
> if (!error->pipe[i].power_domain_on)
> continue;
>
> @@ -11640,7 +11641,7 @@ intel_display_capture_error_state(struct drm_device *dev)
> enum transcoder cpu_transcoder = transcoders[i];
>
> error->transcoder[i].power_domain_on =
> - intel_display_power_enabled_sw(dev,
> + intel_display_power_enabled_sw(dev_priv,
> POWER_DOMAIN_TRANSCODER(cpu_transcoder));
> if (!error->transcoder[i].power_domain_on)
> continue;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index a4ffc02..6042797 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -732,7 +732,7 @@ ironlake_check_encoder_dotclock(const struct intel_crtc_config *pipe_config,
> bool intel_crtc_active(struct drm_crtc *crtc);
> 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);
> +void intel_display_set_init_power(struct drm_i915_private *dev, bool enable);
> int valleyview_get_vco(struct drm_i915_private *dev_priv);
> void intel_mode_from_pipe_config(struct drm_display_mode *mode,
> struct intel_crtc_config *pipe_config);
> @@ -871,18 +871,17 @@ 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 intel_power_domains_init(struct drm_device *dev);
> -void intel_power_domains_remove(struct drm_device *dev);
> -bool intel_display_power_enabled(struct drm_device *dev,
> +int intel_power_domains_init(struct drm_i915_private *);
> +void intel_power_domains_remove(struct drm_i915_private *);
> +bool intel_display_power_enabled(struct drm_i915_private *dev_priv,
> enum intel_display_power_domain domain);
> -bool intel_display_power_enabled_sw(struct drm_device *dev,
> +bool intel_display_power_enabled_sw(struct drm_i915_private *dev_priv,
> enum intel_display_power_domain domain);
> -void intel_display_power_get(struct drm_device *dev,
> +void intel_display_power_get(struct drm_i915_private *dev_priv,
> enum intel_display_power_domain domain);
> -void intel_display_power_put(struct drm_device *dev,
> +void intel_display_power_put(struct drm_i915_private *dev_priv,
> enum intel_display_power_domain domain);
> -void intel_power_domains_init_hw(struct drm_device *dev);
> -void intel_set_power_well(struct drm_device *dev, bool enable);
> +void intel_power_domains_init_hw(struct drm_i915_private *dev_priv);
> void intel_enable_gt_powersave(struct drm_device *dev);
> void intel_disable_gt_powersave(struct drm_device *dev);
> void ironlake_teardown_rc6(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index c8347ae..aa9c2df 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5169,19 +5169,16 @@ void intel_suspend_hw(struct drm_device *dev)
> * enable it, so check if it's enabled and also check if we've requested it to
> * be enabled.
> */
> -static bool hsw_power_well_enabled(struct drm_device *dev,
> +static bool hsw_power_well_enabled(struct drm_i915_private *dev_priv,
> struct i915_power_well *power_well)
> {
> - struct drm_i915_private *dev_priv = dev->dev_private;
> -
> return I915_READ(HSW_PWR_WELL_DRIVER) ==
> (HSW_PWR_WELL_ENABLE_REQUEST | HSW_PWR_WELL_STATE_ENABLED);
> }
>
> -bool intel_display_power_enabled_sw(struct drm_device *dev,
> +bool intel_display_power_enabled_sw(struct drm_i915_private *dev_priv,
> enum intel_display_power_domain domain)
> {
> - struct drm_i915_private *dev_priv = dev->dev_private;
> struct i915_power_domains *power_domains;
>
> power_domains = &dev_priv->power_domains;
> @@ -5189,10 +5186,9 @@ bool intel_display_power_enabled_sw(struct drm_device *dev,
> return power_domains->domain_use_count[domain];
> }
>
> -bool intel_display_power_enabled(struct drm_device *dev,
> +bool intel_display_power_enabled(struct drm_i915_private *dev_priv,
> enum intel_display_power_domain domain)
> {
> - struct drm_i915_private *dev_priv = dev->dev_private;
> struct i915_power_domains *power_domains;
> struct i915_power_well *power_well;
> bool is_enabled;
> @@ -5207,7 +5203,7 @@ bool intel_display_power_enabled(struct drm_device *dev,
> if (power_well->always_on)
> continue;
>
> - if (!power_well->is_enabled(dev, power_well)) {
> + if (!power_well->is_enabled(dev_priv, power_well)) {
> is_enabled = false;
> break;
> }
> @@ -5273,10 +5269,9 @@ static void hsw_power_well_post_disable(struct drm_i915_private *dev_priv)
> spin_unlock_irqrestore(&dev->vbl_lock, irqflags);
> }
>
> -static void hsw_set_power_well(struct drm_device *dev,
> +static void hsw_set_power_well(struct drm_i915_private *dev_priv,
> struct i915_power_well *power_well, bool enable)
> {
> - struct drm_i915_private *dev_priv = dev->dev_private;
> bool is_enabled, enable_requested;
> uint32_t tmp;
>
> @@ -5310,35 +5305,30 @@ static void hsw_set_power_well(struct drm_device *dev,
> }
> }
>
> -static void __intel_power_well_get(struct drm_device *dev,
> +static void __intel_power_well_get(struct drm_i915_private *dev_priv,
> struct i915_power_well *power_well)
> {
> - struct drm_i915_private *dev_priv = dev->dev_private;
> -
> if (!power_well->count++ && power_well->set) {
> hsw_disable_package_c8(dev_priv);
> - power_well->set(dev, power_well, true);
> + power_well->set(dev_priv, power_well, true);
> }
> }
>
> -static void __intel_power_well_put(struct drm_device *dev,
> +static void __intel_power_well_put(struct drm_i915_private *dev_priv,
> struct i915_power_well *power_well)
> {
> - struct drm_i915_private *dev_priv = dev->dev_private;
> -
> WARN_ON(!power_well->count);
>
> if (!--power_well->count && power_well->set &&
> i915.disable_power_well) {
> - power_well->set(dev, power_well, false);
> + power_well->set(dev_priv, power_well, false);
> hsw_enable_package_c8(dev_priv);
> }
> }
>
> -void intel_display_power_get(struct drm_device *dev,
> +void intel_display_power_get(struct drm_i915_private *dev_priv,
> enum intel_display_power_domain domain)
> {
> - struct drm_i915_private *dev_priv = dev->dev_private;
> struct i915_power_domains *power_domains;
> struct i915_power_well *power_well;
> int i;
> @@ -5348,17 +5338,16 @@ void intel_display_power_get(struct drm_device *dev,
> mutex_lock(&power_domains->lock);
>
> for_each_power_well(i, power_well, BIT(domain), power_domains)
> - __intel_power_well_get(dev, power_well);
> + __intel_power_well_get(dev_priv, power_well);
>
> power_domains->domain_use_count[domain]++;
>
> mutex_unlock(&power_domains->lock);
> }
>
> -void intel_display_power_put(struct drm_device *dev,
> +void intel_display_power_put(struct drm_i915_private *dev_priv,
> enum intel_display_power_domain domain)
> {
> - struct drm_i915_private *dev_priv = dev->dev_private;
> struct i915_power_domains *power_domains;
> struct i915_power_well *power_well;
> int i;
> @@ -5371,7 +5360,7 @@ void intel_display_power_put(struct drm_device *dev,
> power_domains->domain_use_count[domain]--;
>
> for_each_power_well_rev(i, power_well, BIT(domain), power_domains)
> - __intel_power_well_put(dev, power_well);
> + __intel_power_well_put(dev_priv, power_well);
>
> mutex_unlock(&power_domains->lock);
> }
> @@ -5388,7 +5377,7 @@ void i915_request_power_well(void)
>
> dev_priv = container_of(hsw_pwr, struct drm_i915_private,
> power_domains);
> - intel_display_power_get(dev_priv->dev, POWER_DOMAIN_AUDIO);
> + intel_display_power_get(dev_priv, POWER_DOMAIN_AUDIO);
> }
> EXPORT_SYMBOL_GPL(i915_request_power_well);
>
> @@ -5402,7 +5391,7 @@ void i915_release_power_well(void)
>
> dev_priv = container_of(hsw_pwr, struct drm_i915_private,
> power_domains);
> - intel_display_power_put(dev_priv->dev, POWER_DOMAIN_AUDIO);
> + intel_display_power_put(dev_priv, POWER_DOMAIN_AUDIO);
> }
> EXPORT_SYMBOL_GPL(i915_release_power_well);
>
> @@ -5447,9 +5436,8 @@ static struct i915_power_well bdw_power_wells[] = {
> (power_domains)->power_well_count = ARRAY_SIZE(__power_wells); \
> })
>
> -int intel_power_domains_init(struct drm_device *dev)
> +int intel_power_domains_init(struct drm_i915_private *dev_priv)
> {
> - struct drm_i915_private *dev_priv = dev->dev_private;
> struct i915_power_domains *power_domains = &dev_priv->power_domains;
>
> mutex_init(&power_domains->lock);
> @@ -5458,10 +5446,10 @@ int intel_power_domains_init(struct drm_device *dev)
> * The enabling order will be from lower to higher indexed wells,
> * the disabling order is reversed.
> */
> - if (IS_HASWELL(dev)) {
> + if (IS_HASWELL(dev_priv->dev)) {
> set_power_wells(power_domains, hsw_power_wells);
> hsw_pwr = power_domains;
> - } else if (IS_BROADWELL(dev)) {
> + } else if (IS_BROADWELL(dev_priv->dev)) {
> set_power_wells(power_domains, bdw_power_wells);
> hsw_pwr = power_domains;
> } else {
> @@ -5471,14 +5459,13 @@ int intel_power_domains_init(struct drm_device *dev)
> return 0;
> }
>
> -void intel_power_domains_remove(struct drm_device *dev)
> +void intel_power_domains_remove(struct drm_i915_private *dev_priv)
> {
> hsw_pwr = NULL;
> }
>
> -static void intel_power_domains_resume(struct drm_device *dev)
> +static void intel_power_domains_resume(struct drm_i915_private *dev_priv)
> {
> - struct drm_i915_private *dev_priv = dev->dev_private;
> struct i915_power_domains *power_domains = &dev_priv->power_domains;
> struct i915_power_well *power_well;
> int i;
> @@ -5486,7 +5473,7 @@ static void intel_power_domains_resume(struct drm_device *dev)
> mutex_lock(&power_domains->lock);
> for_each_power_well(i, power_well, POWER_DOMAIN_MASK, power_domains) {
> if (power_well->set)
> - power_well->set(dev, power_well, power_well->count > 0);
> + power_well->set(dev_priv, power_well, power_well->count > 0);
> }
> mutex_unlock(&power_domains->lock);
> }
> @@ -5497,15 +5484,13 @@ static void intel_power_domains_resume(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_power_domains_init_hw(struct drm_device *dev)
> +void intel_power_domains_init_hw(struct drm_i915_private *dev_priv)
> {
> - struct drm_i915_private *dev_priv = dev->dev_private;
> -
> /* For now, we need the power well to be always enabled. */
> - intel_display_set_init_power(dev, true);
> - intel_power_domains_resume(dev);
> + intel_display_set_init_power(dev_priv, true);
> + intel_power_domains_resume(dev_priv);
>
> - if (!(IS_HASWELL(dev) || IS_BROADWELL(dev)))
> + if (!(IS_HASWELL(dev_priv->dev) || IS_BROADWELL(dev_priv->dev)))
> return;
>
> /* We're taking over the BIOS, so clear any requests made by it since

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>

--
Jesse Barnes, Intel Open Source Technology Center