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

[Jesse Barnes <jbarnes@virtuousgeek.org>]

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