All of lore.kernel.org
 help / color / mirror / Atom feed
From: Paulo Zanoni <paulo.r.zanoni@intel.com>
To: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>,
	intel-gfx@lists.freedesktop.org
Subject: Re: [RFC 02/10] drm/i915: use intel_uncore in fw get/put internal paths
Date: Fri, 15 Mar 2019 14:00:48 -0700	[thread overview]
Message-ID: <26cfe9334a761d34e87f9f4d8c835fd5b18a8077.camel@intel.com> (raw)
In-Reply-To: <20190313231319.711-3-daniele.ceraolospurio@intel.com>

Em qua, 2019-03-13 às 16:13 -0700, Daniele Ceraolo Spurio escreveu:
> Get/put functions used outside of uncore.c are updated in the next
> patch for a nicer split
> 
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>

I really like this one, replacing a gazillion i915->uncore.x with only
uncore->x and just very few instances where you need to go from uncore
back to i915. IMHO this patch is worth it on its own, not considering
the rest of the series.

A single comment below:

> ---
>  drivers/gpu/drm/i915/i915_debugfs.c           |   5 +-
>  drivers/gpu/drm/i915/i915_drv.c               |   2 +-
>  drivers/gpu/drm/i915/i915_drv.h               |   5 +
>  drivers/gpu/drm/i915/intel_uncore.c           | 203 +++++++++---------
>  drivers/gpu/drm/i915/intel_uncore.h           |  17 +-
>  drivers/gpu/drm/i915/selftests/intel_uncore.c |   2 +-
>  6 files changed, 125 insertions(+), 109 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 6a90558de213..3bcf0e07b614 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1414,13 +1414,14 @@ static int ironlake_drpc_info(struct seq_file *m)
>  static int i915_forcewake_domains(struct seq_file *m, void *data)
>  {
>  	struct drm_i915_private *i915 = node_to_i915(m->private);
> +	struct intel_uncore *uncore = &i915->uncore;
>  	struct intel_uncore_forcewake_domain *fw_domain;
>  	unsigned int tmp;
>  
>  	seq_printf(m, "user.bypass_count = %u\n",
> -		   i915->uncore.user_forcewake.count);
> +		   uncore->user_forcewake.count);
>  
> -	for_each_fw_domain(fw_domain, i915, tmp)
> +	for_each_fw_domain(fw_domain, uncore, tmp)
>  		seq_printf(m, "%s.wake_count = %u\n",
>  			   intel_uncore_forcewake_domain_to_str(fw_domain->id),
>  			   READ_ONCE(fw_domain->wake_count));
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 0d743907e7bc..4ace6fadfbc2 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -2914,7 +2914,7 @@ static int intel_runtime_suspend(struct device *kdev)
>  		intel_opregion_notify_adapter(dev_priv, PCI_D1);
>  	}
>  
> -	assert_forcewakes_inactive(dev_priv);
> +	assert_forcewakes_inactive(&dev_priv->uncore);
>  
>  	if (!IS_VALLEYVIEW(dev_priv) && !IS_CHERRYVIEW(dev_priv))
>  		intel_hpd_poll_init(dev_priv);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index dccb6006aabf..8a4b5bbac02e 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2102,6 +2102,11 @@ static inline struct drm_i915_private *huc_to_i915(struct intel_huc *huc)
>  	return container_of(huc, struct drm_i915_private, huc);
>  }
>  
> +static inline struct drm_i915_private *uncore_to_i915(struct intel_uncore *uncore)
> +{
> +	return container_of(uncore, struct drm_i915_private, uncore);
> +}
> +
>  /* Simple iterator over all initialised engines */
>  #define for_each_engine(engine__, dev_priv__, id__) \
>  	for ((id__) = 0; \
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index cb78dcddc9cb..a2d9490739a0 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -205,60 +205,60 @@ fw_domain_put(const struct intel_uncore_forcewake_domain *d)
>  }
>  
>  static void
> -fw_domains_get(struct drm_i915_private *i915, enum forcewake_domains fw_domains)
> +fw_domains_get(struct intel_uncore *uncore, enum forcewake_domains fw_domains)
>  {
>  	struct intel_uncore_forcewake_domain *d;
>  	unsigned int tmp;
>  
> -	GEM_BUG_ON(fw_domains & ~i915->uncore.fw_domains);
> +	GEM_BUG_ON(fw_domains & ~uncore->fw_domains);
>  
> -	for_each_fw_domain_masked(d, fw_domains, i915, tmp) {
> +	for_each_fw_domain_masked(d, fw_domains, uncore, tmp) {
>  		fw_domain_wait_ack_clear(d);
>  		fw_domain_get(d);
>  	}
>  
> -	for_each_fw_domain_masked(d, fw_domains, i915, tmp)
> +	for_each_fw_domain_masked(d, fw_domains, uncore, tmp)
>  		fw_domain_wait_ack_set(d);
>  
> -	i915->uncore.fw_domains_active |= fw_domains;
> +	uncore->fw_domains_active |= fw_domains;
>  }
>  
>  static void
> -fw_domains_get_with_fallback(struct drm_i915_private *i915,
> +fw_domains_get_with_fallback(struct intel_uncore *uncore,
>  			     enum forcewake_domains fw_domains)
>  {
>  	struct intel_uncore_forcewake_domain *d;
>  	unsigned int tmp;
>  
> -	GEM_BUG_ON(fw_domains & ~i915->uncore.fw_domains);
> +	GEM_BUG_ON(fw_domains & ~uncore->fw_domains);
>  
> -	for_each_fw_domain_masked(d, fw_domains, i915, tmp) {
> +	for_each_fw_domain_masked(d, fw_domains, uncore, tmp) {
>  		fw_domain_wait_ack_clear_fallback(d);
>  		fw_domain_get(d);
>  	}
>  
> -	for_each_fw_domain_masked(d, fw_domains, i915, tmp)
> +	for_each_fw_domain_masked(d, fw_domains, uncore, tmp)
>  		fw_domain_wait_ack_set_fallback(d);
>  
> -	i915->uncore.fw_domains_active |= fw_domains;
> +	uncore->fw_domains_active |= fw_domains;
>  }
>  
>  static void
> -fw_domains_put(struct drm_i915_private *i915, enum forcewake_domains fw_domains)
> +fw_domains_put(struct intel_uncore *uncore, enum forcewake_domains fw_domains)
>  {
>  	struct intel_uncore_forcewake_domain *d;
>  	unsigned int tmp;
>  
> -	GEM_BUG_ON(fw_domains & ~i915->uncore.fw_domains);
> +	GEM_BUG_ON(fw_domains & ~uncore->fw_domains);
>  
> -	for_each_fw_domain_masked(d, fw_domains, i915, tmp)
> +	for_each_fw_domain_masked(d, fw_domains, uncore, tmp)
>  		fw_domain_put(d);
>  
> -	i915->uncore.fw_domains_active &= ~fw_domains;
> +	uncore->fw_domains_active &= ~fw_domains;
>  }
>  
>  static void
> -fw_domains_reset(struct drm_i915_private *i915,
> +fw_domains_reset(struct intel_uncore *uncore,
>  		 enum forcewake_domains fw_domains)
>  {
>  	struct intel_uncore_forcewake_domain *d;
> @@ -267,9 +267,9 @@ fw_domains_reset(struct drm_i915_private *i915,
>  	if (!fw_domains)
>  		return;
>  
> -	GEM_BUG_ON(fw_domains & ~i915->uncore.fw_domains);
> +	GEM_BUG_ON(fw_domains & ~uncore->fw_domains);
>  
> -	for_each_fw_domain_masked(d, fw_domains, i915, tmp)
> +	for_each_fw_domain_masked(d, fw_domains, uncore, tmp)
>  		fw_domain_reset(d);
>  }
>  
> @@ -293,13 +293,13 @@ static void __gen6_gt_wait_for_thread_c0(struct drm_i915_private *dev_priv)
>  		  "GT thread status wait timed out\n");
>  }
>  
> -static void fw_domains_get_with_thread_status(struct drm_i915_private *dev_priv,
> +static void fw_domains_get_with_thread_status(struct intel_uncore *uncore,
>  					      enum forcewake_domains fw_domains)
>  {
> -	fw_domains_get(dev_priv, fw_domains);
> +	fw_domains_get(uncore, fw_domains);
>  
>  	/* WaRsForcewakeWaitTC0:snb,ivb,hsw,bdw,vlv */
> -	__gen6_gt_wait_for_thread_c0(dev_priv);
> +	__gen6_gt_wait_for_thread_c0(uncore_to_i915(uncore));
>  }
>  
>  static inline u32 fifo_free_entries(struct drm_i915_private *dev_priv)
> @@ -337,30 +337,29 @@ intel_uncore_fw_release_timer(struct hrtimer *timer)
>  {
>  	struct intel_uncore_forcewake_domain *domain =
>  	       container_of(timer, struct intel_uncore_forcewake_domain, timer);
> -	struct drm_i915_private *dev_priv =
> -		container_of(domain, struct drm_i915_private, uncore.fw_domain[domain->id]);
> +	struct intel_uncore *uncore = forcewake_domain_to_uncore(domain);
>  	unsigned long irqflags;
>  
> -	assert_rpm_device_not_suspended(dev_priv);
> +	assert_rpm_device_not_suspended(uncore_to_i915(uncore));
>  
>  	if (xchg(&domain->active, false))
>  		return HRTIMER_RESTART;
>  
> -	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
> +	spin_lock_irqsave(&uncore->lock, irqflags);
>  	if (WARN_ON(domain->wake_count == 0))
>  		domain->wake_count++;
>  
>  	if (--domain->wake_count == 0)
> -		dev_priv->uncore.funcs.force_wake_put(dev_priv, domain->mask);
> +		uncore->funcs.force_wake_put(uncore, domain->mask);
>  
> -	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
> +	spin_unlock_irqrestore(&uncore->lock, irqflags);
>  
>  	return HRTIMER_NORESTART;
>  }
>  
>  /* Note callers must have acquired the PUNIT->PMIC bus, before calling this. */
>  static unsigned int
> -intel_uncore_forcewake_reset(struct drm_i915_private *dev_priv)
> +intel_uncore_forcewake_reset(struct intel_uncore *uncore)
>  {
>  	unsigned long irqflags;
>  	struct intel_uncore_forcewake_domain *domain;
> @@ -378,7 +377,7 @@ intel_uncore_forcewake_reset(struct drm_i915_private *dev_priv)
>  
>  		active_domains = 0;
>  
> -		for_each_fw_domain(domain, dev_priv, tmp) {
> +		for_each_fw_domain(domain, uncore, tmp) {
>  			smp_store_mb(domain->active, false);
>  			if (hrtimer_cancel(&domain->timer) == 0)
>  				continue;
> @@ -386,9 +385,9 @@ intel_uncore_forcewake_reset(struct drm_i915_private *dev_priv)
>  			intel_uncore_fw_release_timer(&domain->timer);
>  		}
>  
> -		spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
> +		spin_lock_irqsave(&uncore->lock, irqflags);
>  
> -		for_each_fw_domain(domain, dev_priv, tmp) {
> +		for_each_fw_domain(domain, uncore, tmp) {
>  			if (hrtimer_active(&domain->timer))
>  				active_domains |= domain->mask;
>  		}
> @@ -401,20 +400,20 @@ intel_uncore_forcewake_reset(struct drm_i915_private *dev_priv)
>  			break;
>  		}
>  
> -		spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
> +		spin_unlock_irqrestore(&uncore->lock, irqflags);
>  		cond_resched();
>  	}
>  
>  	WARN_ON(active_domains);
>  
> -	fw = dev_priv->uncore.fw_domains_active;
> +	fw = uncore->fw_domains_active;
>  	if (fw)
> -		dev_priv->uncore.funcs.force_wake_put(dev_priv, fw);
> +		uncore->funcs.force_wake_put(uncore, fw);
>  
> -	fw_domains_reset(dev_priv, dev_priv->uncore.fw_domains);
> -	assert_forcewakes_inactive(dev_priv);
> +	fw_domains_reset(uncore, uncore->fw_domains);
> +	assert_forcewakes_inactive(uncore);
>  
> -	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
> +	spin_unlock_irqrestore(&uncore->lock, irqflags);
>  
>  	return fw; /* track the lost user forcewake domains */
>  }
> @@ -540,10 +539,10 @@ static void __intel_uncore_early_sanitize(struct drm_i915_private *dev_priv,
>  	}
>  
>  	iosf_mbi_punit_acquire();
> -	intel_uncore_forcewake_reset(dev_priv);
> +	intel_uncore_forcewake_reset(&dev_priv->uncore);
>  	if (restore_forcewake) {
>  		spin_lock_irq(&dev_priv->uncore.lock);
> -		dev_priv->uncore.funcs.force_wake_get(dev_priv,
> +		dev_priv->uncore.funcs.force_wake_get(&dev_priv->uncore,
>  						      restore_forcewake);
>  
>  		if (IS_GEN_RANGE(dev_priv, 6, 7))
> @@ -560,7 +559,7 @@ void intel_uncore_suspend(struct drm_i915_private *dev_priv)
>  	iosf_mbi_unregister_pmic_bus_access_notifier_unlocked(
>  		&dev_priv->uncore.pmic_bus_access_nb);
>  	dev_priv->uncore.fw_domains_saved =
> -		intel_uncore_forcewake_reset(dev_priv);
> +		intel_uncore_forcewake_reset(&dev_priv->uncore);
>  	iosf_mbi_punit_release();
>  }
>  
> @@ -588,15 +587,15 @@ void intel_uncore_sanitize(struct drm_i915_private *dev_priv)
>  	intel_sanitize_gt_powersave(dev_priv);
>  }
>  
> -static void __intel_uncore_forcewake_get(struct drm_i915_private *dev_priv,
> +static void __intel_uncore_forcewake_get(struct intel_uncore *uncore,
>  					 enum forcewake_domains fw_domains)
>  {
>  	struct intel_uncore_forcewake_domain *domain;
>  	unsigned int tmp;
>  
> -	fw_domains &= dev_priv->uncore.fw_domains;
> +	fw_domains &= uncore->fw_domains;
>  
> -	for_each_fw_domain_masked(domain, fw_domains, dev_priv, tmp) {
> +	for_each_fw_domain_masked(domain, fw_domains, uncore, tmp) {
>  		if (domain->wake_count++) {
>  			fw_domains &= ~domain->mask;
>  			domain->active = true;
> @@ -604,7 +603,7 @@ static void __intel_uncore_forcewake_get(struct drm_i915_private *dev_priv,
>  	}
>  
>  	if (fw_domains)
> -		dev_priv->uncore.funcs.force_wake_get(dev_priv, fw_domains);
> +		uncore->funcs.force_wake_get(uncore, fw_domains);
>  }
>  
>  /**
> @@ -623,16 +622,17 @@ static void __intel_uncore_forcewake_get(struct drm_i915_private *dev_priv,
>  void intel_uncore_forcewake_get(struct drm_i915_private *dev_priv,
>  				enum forcewake_domains fw_domains)
>  {
> +	struct intel_uncore *uncore = &dev_priv->uncore;
>  	unsigned long irqflags;
>  
> -	if (!dev_priv->uncore.funcs.force_wake_get)
> +	if (!uncore->funcs.force_wake_get)
>  		return;
>  
> -	assert_rpm_wakelock_held(dev_priv);
> +	assert_rpm_wakelock_held(uncore_to_i915(uncore));

You still have dev_priv here, there's no need for this change.

With that:

Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

>  
> -	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
> -	__intel_uncore_forcewake_get(dev_priv, fw_domains);
> -	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
> +	spin_lock_irqsave(&uncore->lock, irqflags);
> +	__intel_uncore_forcewake_get(uncore, fw_domains);
> +	spin_unlock_irqrestore(&uncore->lock, irqflags);
>  }
>  
>  /**
> @@ -645,20 +645,22 @@ void intel_uncore_forcewake_get(struct drm_i915_private *dev_priv,
>   */
>  void intel_uncore_forcewake_user_get(struct drm_i915_private *dev_priv)
>  {
> -	spin_lock_irq(&dev_priv->uncore.lock);
> -	if (!dev_priv->uncore.user_forcewake.count++) {
> +	struct intel_uncore *uncore = &dev_priv->uncore;
> +
> +	spin_lock_irq(&uncore->lock);
> +	if (!uncore->user_forcewake.count++) {
>  		intel_uncore_forcewake_get__locked(dev_priv, FORCEWAKE_ALL);
>  
>  		/* Save and disable mmio debugging for the user bypass */
> -		dev_priv->uncore.user_forcewake.saved_mmio_check =
> -			dev_priv->uncore.unclaimed_mmio_check;
> -		dev_priv->uncore.user_forcewake.saved_mmio_debug =
> +		uncore->user_forcewake.saved_mmio_check =
> +			uncore->unclaimed_mmio_check;
> +		uncore->user_forcewake.saved_mmio_debug =
>  			i915_modparams.mmio_debug;
>  
> -		dev_priv->uncore.unclaimed_mmio_check = 0;
> +		uncore->unclaimed_mmio_check = 0;
>  		i915_modparams.mmio_debug = 0;
>  	}
> -	spin_unlock_irq(&dev_priv->uncore.lock);
> +	spin_unlock_irq(&uncore->lock);
>  }
>  
>  /**
> @@ -670,20 +672,22 @@ void intel_uncore_forcewake_user_get(struct drm_i915_private *dev_priv)
>   */
>  void intel_uncore_forcewake_user_put(struct drm_i915_private *dev_priv)
>  {
> -	spin_lock_irq(&dev_priv->uncore.lock);
> -	if (!--dev_priv->uncore.user_forcewake.count) {
> +	struct intel_uncore *uncore = &dev_priv->uncore;
> +
> +	spin_lock_irq(&uncore->lock);
> +	if (!--uncore->user_forcewake.count) {
>  		if (intel_uncore_unclaimed_mmio(dev_priv))
>  			dev_info(dev_priv->drm.dev,
>  				 "Invalid mmio detected during user access\n");
>  
> -		dev_priv->uncore.unclaimed_mmio_check =
> -			dev_priv->uncore.user_forcewake.saved_mmio_check;
> +		uncore->unclaimed_mmio_check =
> +			uncore->user_forcewake.saved_mmio_check;
>  		i915_modparams.mmio_debug =
> -			dev_priv->uncore.user_forcewake.saved_mmio_debug;
> +			uncore->user_forcewake.saved_mmio_debug;
>  
>  		intel_uncore_forcewake_put__locked(dev_priv, FORCEWAKE_ALL);
>  	}
> -	spin_unlock_irq(&dev_priv->uncore.lock);
> +	spin_unlock_irq(&uncore->lock);
>  }
>  
>  /**
> @@ -697,23 +701,25 @@ void intel_uncore_forcewake_user_put(struct drm_i915_private *dev_priv)
>  void intel_uncore_forcewake_get__locked(struct drm_i915_private *dev_priv,
>  					enum forcewake_domains fw_domains)
>  {
> -	lockdep_assert_held(&dev_priv->uncore.lock);
> +	struct intel_uncore *uncore = &dev_priv->uncore;
> +
> +	lockdep_assert_held(&uncore->lock);
>  
> -	if (!dev_priv->uncore.funcs.force_wake_get)
> +	if (!uncore->funcs.force_wake_get)
>  		return;
>  
> -	__intel_uncore_forcewake_get(dev_priv, fw_domains);
> +	__intel_uncore_forcewake_get(uncore, fw_domains);
>  }
>  
> -static void __intel_uncore_forcewake_put(struct drm_i915_private *dev_priv,
> +static void __intel_uncore_forcewake_put(struct intel_uncore *uncore,
>  					 enum forcewake_domains fw_domains)
>  {
>  	struct intel_uncore_forcewake_domain *domain;
>  	unsigned int tmp;
>  
> -	fw_domains &= dev_priv->uncore.fw_domains;
> +	fw_domains &= uncore->fw_domains;
>  
> -	for_each_fw_domain_masked(domain, fw_domains, dev_priv, tmp) {
> +	for_each_fw_domain_masked(domain, fw_domains, uncore, tmp) {
>  		if (WARN_ON(domain->wake_count == 0))
>  			continue;
>  
> @@ -737,14 +743,15 @@ static void __intel_uncore_forcewake_put(struct drm_i915_private *dev_priv,
>  void intel_uncore_forcewake_put(struct drm_i915_private *dev_priv,
>  				enum forcewake_domains fw_domains)
>  {
> +	struct intel_uncore *uncore = &dev_priv->uncore;
>  	unsigned long irqflags;
>  
> -	if (!dev_priv->uncore.funcs.force_wake_put)
> +	if (!uncore->funcs.force_wake_put)
>  		return;
>  
> -	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
> -	__intel_uncore_forcewake_put(dev_priv, fw_domains);
> -	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
> +	spin_lock_irqsave(&uncore->lock, irqflags);
> +	__intel_uncore_forcewake_put(uncore, fw_domains);
> +	spin_unlock_irqrestore(&uncore->lock, irqflags);
>  }
>  
>  /**
> @@ -758,36 +765,38 @@ void intel_uncore_forcewake_put(struct drm_i915_private *dev_priv,
>  void intel_uncore_forcewake_put__locked(struct drm_i915_private *dev_priv,
>  					enum forcewake_domains fw_domains)
>  {
> -	lockdep_assert_held(&dev_priv->uncore.lock);
> +	struct intel_uncore *uncore = &dev_priv->uncore;
>  
> -	if (!dev_priv->uncore.funcs.force_wake_put)
> +	lockdep_assert_held(&uncore->lock);
> +
> +	if (!uncore->funcs.force_wake_put)
>  		return;
>  
> -	__intel_uncore_forcewake_put(dev_priv, fw_domains);
> +	__intel_uncore_forcewake_put(uncore, fw_domains);
>  }
>  
> -void assert_forcewakes_inactive(struct drm_i915_private *dev_priv)
> +void assert_forcewakes_inactive(struct intel_uncore *uncore)
>  {
> -	if (!dev_priv->uncore.funcs.force_wake_get)
> +	if (!uncore->funcs.force_wake_get)
>  		return;
>  
> -	WARN(dev_priv->uncore.fw_domains_active,
> +	WARN(uncore->fw_domains_active,
>  	     "Expected all fw_domains to be inactive, but %08x are still on\n",
> -	     dev_priv->uncore.fw_domains_active);
> +	     uncore->fw_domains_active);
>  }
>  
> -void assert_forcewakes_active(struct drm_i915_private *dev_priv,
> +void assert_forcewakes_active(struct intel_uncore *uncore,
>  			      enum forcewake_domains fw_domains)
>  {
> -	if (!dev_priv->uncore.funcs.force_wake_get)
> +	if (!uncore->funcs.force_wake_get)
>  		return;
>  
> -	assert_rpm_wakelock_held(dev_priv);
> +	assert_rpm_wakelock_held(uncore_to_i915(uncore));
>  
> -	fw_domains &= dev_priv->uncore.fw_domains;
> -	WARN(fw_domains & ~dev_priv->uncore.fw_domains_active,
> +	fw_domains &= uncore->fw_domains;
> +	WARN(fw_domains & ~uncore->fw_domains_active,
>  	     "Expected %08x fw_domains to be active, but %08x are off\n",
> -	     fw_domains, fw_domains & ~dev_priv->uncore.fw_domains_active);
> +	     fw_domains, fw_domains & ~uncore->fw_domains_active);
>  }
>  
>  /* We give fast paths for the really cool registers */
> @@ -1151,32 +1160,32 @@ __gen2_read(64)
>  	trace_i915_reg_rw(false, reg, val, sizeof(val), trace); \
>  	return val
>  
> -static noinline void ___force_wake_auto(struct drm_i915_private *dev_priv,
> +static noinline void ___force_wake_auto(struct intel_uncore *uncore,
>  					enum forcewake_domains fw_domains)
>  {
>  	struct intel_uncore_forcewake_domain *domain;
>  	unsigned int tmp;
>  
> -	GEM_BUG_ON(fw_domains & ~dev_priv->uncore.fw_domains);
> +	GEM_BUG_ON(fw_domains & ~uncore->fw_domains);
>  
> -	for_each_fw_domain_masked(domain, fw_domains, dev_priv, tmp)
> +	for_each_fw_domain_masked(domain, fw_domains, uncore, tmp)
>  		fw_domain_arm_timer(domain);
>  
> -	dev_priv->uncore.funcs.force_wake_get(dev_priv, fw_domains);
> +	uncore->funcs.force_wake_get(uncore, fw_domains);
>  }
>  
> -static inline void __force_wake_auto(struct drm_i915_private *dev_priv,
> +static inline void __force_wake_auto(struct intel_uncore *uncore,
>  				     enum forcewake_domains fw_domains)
>  {
>  	if (WARN_ON(!fw_domains))
>  		return;
>  
>  	/* Turn on all requested but inactive supported forcewake domains. */
> -	fw_domains &= dev_priv->uncore.fw_domains;
> -	fw_domains &= ~dev_priv->uncore.fw_domains_active;
> +	fw_domains &= uncore->fw_domains;
> +	fw_domains &= ~uncore->fw_domains_active;
>  
>  	if (fw_domains)
> -		___force_wake_auto(dev_priv, fw_domains);
> +		___force_wake_auto(uncore, fw_domains);
>  }
>  
>  #define __gen_read(func, x) \
> @@ -1186,7 +1195,7 @@ func##_read##x(struct drm_i915_private *dev_priv, i915_reg_t reg, bool trace) {
>  	GEN6_READ_HEADER(x); \
>  	fw_engine = __##func##_reg_read_fw_domains(offset); \
>  	if (fw_engine) \
> -		__force_wake_auto(dev_priv, fw_engine); \
> +		__force_wake_auto(&dev_priv->uncore, fw_engine); \
>  	val = __raw_i915_read##x(dev_priv, reg); \
>  	GEN6_READ_FOOTER; \
>  }
> @@ -1278,7 +1287,7 @@ func##_write##x(struct drm_i915_private *dev_priv, i915_reg_t reg, u##x val, boo
>  	GEN6_WRITE_HEADER; \
>  	fw_engine = __##func##_reg_write_fw_domains(offset); \
>  	if (fw_engine) \
> -		__force_wake_auto(dev_priv, fw_engine); \
> +		__force_wake_auto(&dev_priv->uncore, fw_engine); \
>  	__raw_i915_write##x(dev_priv, reg, val); \
>  	GEN6_WRITE_FOOTER; \
>  }
> @@ -1482,9 +1491,9 @@ static void intel_uncore_fw_domains_init(struct drm_i915_private *dev_priv)
>  			       FORCEWAKE_MT, FORCEWAKE_MT_ACK);
>  
>  		spin_lock_irq(&dev_priv->uncore.lock);
> -		fw_domains_get_with_thread_status(dev_priv, FORCEWAKE_RENDER);
> +		fw_domains_get_with_thread_status(&dev_priv->uncore, FORCEWAKE_RENDER);
>  		ecobus = __raw_i915_read32(dev_priv, ECOBUS);
> -		fw_domains_put(dev_priv, FORCEWAKE_RENDER);
> +		fw_domains_put(&dev_priv->uncore, FORCEWAKE_RENDER);
>  		spin_unlock_irq(&dev_priv->uncore.lock);
>  
>  		if (!(ecobus & FORCEWAKE_MT_ENABLE)) {
> @@ -1638,7 +1647,7 @@ void intel_uncore_fini(struct drm_i915_private *dev_priv)
>  	iosf_mbi_punit_acquire();
>  	iosf_mbi_unregister_pmic_bus_access_notifier_unlocked(
>  		&dev_priv->uncore.pmic_bus_access_nb);
> -	intel_uncore_forcewake_reset(dev_priv);
> +	intel_uncore_forcewake_reset(&dev_priv->uncore);
>  	iosf_mbi_punit_release();
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_uncore.h b/drivers/gpu/drm/i915/intel_uncore.h
> index b0a95469babf..86e397be9ae0 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.h
> +++ b/drivers/gpu/drm/i915/intel_uncore.h
> @@ -32,6 +32,7 @@
>  #include "i915_reg.h"
>  
>  struct drm_i915_private;
> +struct intel_uncore;
>  
>  enum forcewake_domain_id {
>  	FW_DOMAIN_ID_RENDER = 0,
> @@ -62,9 +63,9 @@ enum forcewake_domains {
>  };
>  
>  struct intel_uncore_funcs {
> -	void (*force_wake_get)(struct drm_i915_private *dev_priv,
> +	void (*force_wake_get)(struct intel_uncore *uncore,
>  			       enum forcewake_domains domains);
> -	void (*force_wake_put)(struct drm_i915_private *dev_priv,
> +	void (*force_wake_put)(struct intel_uncore *uncore,
>  			       enum forcewake_domains domains);
>  
>  	u8 (*mmio_readb)(struct drm_i915_private *dev_priv,
> @@ -131,12 +132,12 @@ struct intel_uncore {
>  };
>  
>  /* Iterate over initialised fw domains */
> -#define for_each_fw_domain_masked(domain__, mask__, dev_priv__, tmp__) \
> +#define for_each_fw_domain_masked(domain__, mask__, uncore__, tmp__) \
>  	for (tmp__ = (mask__); \
> -	     tmp__ ? (domain__ = &(dev_priv__)->uncore.fw_domain[__mask_next_bit(tmp__)]), 1 : 0;)
> +	     tmp__ ? (domain__ = &(uncore__)->fw_domain[__mask_next_bit(tmp__)]), 1 : 0;)
>  
> -#define for_each_fw_domain(domain__, dev_priv__, tmp__) \
> -	for_each_fw_domain_masked(domain__, (dev_priv__)->uncore.fw_domains, dev_priv__, tmp__)
> +#define for_each_fw_domain(domain__, uncore__, tmp__) \
> +	for_each_fw_domain_masked(domain__, (uncore__)->fw_domains, uncore__, tmp__)
>  
>  static inline struct intel_uncore *
>  forcewake_domain_to_uncore(const struct intel_uncore_forcewake_domain *d)
> @@ -155,8 +156,8 @@ void intel_uncore_resume_early(struct drm_i915_private *dev_priv);
>  void intel_uncore_runtime_resume(struct drm_i915_private *dev_priv);
>  
>  u64 intel_uncore_edram_size(struct drm_i915_private *dev_priv);
> -void assert_forcewakes_inactive(struct drm_i915_private *dev_priv);
> -void assert_forcewakes_active(struct drm_i915_private *dev_priv,
> +void assert_forcewakes_inactive(struct intel_uncore *uncore);
> +void assert_forcewakes_active(struct intel_uncore *uncore,
>  			      enum forcewake_domains fw_domains);
>  const char *intel_uncore_forcewake_domain_to_str(const enum forcewake_domain_id id);
>  
> diff --git a/drivers/gpu/drm/i915/selftests/intel_uncore.c b/drivers/gpu/drm/i915/selftests/intel_uncore.c
> index 81d9d31042a9..1c15f8d146aa 100644
> --- a/drivers/gpu/drm/i915/selftests/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/selftests/intel_uncore.c
> @@ -160,7 +160,7 @@ static int intel_uncore_check_forcewake_domains(struct drm_i915_private *dev_pri
>  		i915_reg_t reg = { offset };
>  
>  		iosf_mbi_punit_acquire();
> -		intel_uncore_forcewake_reset(dev_priv);
> +		intel_uncore_forcewake_reset(&dev_priv->uncore);
>  		iosf_mbi_punit_release();
>  
>  		check_for_unclaimed_mmio(dev_priv);

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2019-03-15 21:00 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-13 23:13 [RFC 00/10] Compartmentalize uncore code Daniele Ceraolo Spurio
2019-03-13 23:13 ` [RFC 01/10] drm/i915: do not pass dev_priv to low-level forcewake functions Daniele Ceraolo Spurio
2019-03-15 20:07   ` Paulo Zanoni
2019-03-15 23:41   ` Chris Wilson
2019-03-13 23:13 ` [RFC 02/10] drm/i915: use intel_uncore in fw get/put internal paths Daniele Ceraolo Spurio
2019-03-15 21:00   ` Paulo Zanoni [this message]
2019-03-13 23:13 ` [RFC 03/10] drm/i915: use intel_uncore for all forcewake get/put Daniele Ceraolo Spurio
2019-03-15 21:41   ` Paulo Zanoni
2019-03-13 23:13 ` [RFC 04/10] drm/i915: make more uncore function work on intel_uncore Daniele Ceraolo Spurio
2019-03-15 22:17   ` Paulo Zanoni
2019-03-13 23:13 ` [RFC 05/10] drm/i915: make find_fw_domain " Daniele Ceraolo Spurio
2019-03-15 22:57   ` Paulo Zanoni
2019-03-13 23:13 ` [RFC 06/10] drm/i915: reduce the dev_priv->uncore dance in uncore.c Daniele Ceraolo Spurio
2019-03-15 23:07   ` Paulo Zanoni
2019-03-13 23:13 ` [RFC 07/10] drm/i915: move regs pointer inside the uncore structure Daniele Ceraolo Spurio
2019-03-15 20:50   ` Chris Wilson
2019-03-18 18:08     ` Daniele Ceraolo Spurio
2019-03-13 23:13 ` [RFC 08/10] drm/i915: make raw access function work on uncore Daniele Ceraolo Spurio
2019-03-15 23:33   ` Paulo Zanoni
2019-03-13 23:13 ` [RFC 09/10] drm/i915: add uncore flags Daniele Ceraolo Spurio
2019-03-13 23:13 ` [RFC 10/10] drm/i915: switch uncore mmio funcs to use intel_uncore Daniele Ceraolo Spurio
2019-03-14  4:09 ` ✗ Fi.CI.BAT: failure for Compartmentalize uncore code Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=26cfe9334a761d34e87f9f4d8c835fd5b18a8077.camel@intel.com \
    --to=paulo.r.zanoni@intel.com \
    --cc=daniele.ceraolospurio@intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.