Re: [PATCH v2 2/5] drm/i915: Skip unused fw_domains

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> writes:

> Use find-first-set bitop to quickly scan through the fw_domains mask and
> skip iterating over unused domains.
>
> v2: Move the WARN into the caller, to prevent compiler warnings in
> normal builds.
>
> Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c |  3 ++-
>  drivers/gpu/drm/i915/i915_drv.h     | 30 +++++++++++------------
>  drivers/gpu/drm/i915/intel_uncore.c | 48 ++++++++++++++++++++++++-------------
>  3 files changed, 48 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 29bf11d8b620..fcac795d4396 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1461,9 +1461,10 @@ static int i915_forcewake_domains(struct seq_file *m, void *data)
>  {
>  	struct drm_i915_private *dev_priv = node_to_i915(m->private);
>  	struct intel_uncore_forcewake_domain *fw_domain;
> +	unsigned int tmp;
>  
>  	spin_lock_irq(&dev_priv->uncore.lock);
> -	for_each_fw_domain(fw_domain, dev_priv) {
> +	for_each_fw_domain(fw_domain, dev_priv, tmp) {
>  		seq_printf(m, "%s.wake_count = %u\n",
>  			   intel_uncore_forcewake_domain_to_str(fw_domain->id),
>  			   fw_domain->wake_count);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 4c9de7d00eaf..589f91c4e585 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -703,9 +703,9 @@ enum forcewake_domain_id {
>  };
>  
>  enum forcewake_domains {
> -	FORCEWAKE_RENDER = (1 << FW_DOMAIN_ID_RENDER),
> -	FORCEWAKE_BLITTER = (1 << FW_DOMAIN_ID_BLITTER),
> -	FORCEWAKE_MEDIA	= (1 << FW_DOMAIN_ID_MEDIA),
> +	FORCEWAKE_RENDER = BIT(FW_DOMAIN_ID_RENDER),
> +	FORCEWAKE_BLITTER = BIT(FW_DOMAIN_ID_BLITTER),
> +	FORCEWAKE_MEDIA	= BIT(FW_DOMAIN_ID_MEDIA),
>  	FORCEWAKE_ALL = (FORCEWAKE_RENDER |
>  			 FORCEWAKE_BLITTER |
>  			 FORCEWAKE_MEDIA)
> @@ -790,15 +790,19 @@ struct intel_uncore {
>  	int unclaimed_mmio_check;
>  };
>  
> +#define __mask_next_bit(mask) ({					\
> +	int __idx = ffs(mask) - 1;					\
> +	mask &= ~BIT(__idx);						\
> +	__idx;								\
> +})
> +
>  /* Iterate over initialised fw domains */
> -#define for_each_fw_domain_masked(domain__, mask__, dev_priv__) \
> -	for ((domain__) = &(dev_priv__)->uncore.fw_domain[0]; \
> -	     (domain__) < &(dev_priv__)->uncore.fw_domain[FW_DOMAIN_ID_COUNT]; \
> -	     (domain__)++) \
> -		for_each_if ((mask__) & (domain__)->mask)
> +#define for_each_fw_domain_masked(domain__, mask__, dev_priv__, tmp__) \
> +	for (tmp__ = (mask__); \
> +	     tmp__ ? (domain__ = &(dev_priv__)->uncore.fw_domain[__mask_next_bit(tmp__)]), 1 : 0;)
>  
> -#define for_each_fw_domain(domain__, dev_priv__) \
> -	for_each_fw_domain_masked(domain__, FORCEWAKE_ALL, dev_priv__)
> +#define for_each_fw_domain(domain__, dev_priv__, tmp__) \
> +	for_each_fw_domain_masked(domain__, (dev_priv__)->uncore.fw_domains, dev_priv__, tmp__)
>  
>  #define CSR_VERSION(major, minor)	((major) << 16 | (minor))
>  #define CSR_VERSION_MAJOR(version)	((version) >> 16)
> @@ -2581,12 +2585,6 @@ static inline struct drm_i915_private *huc_to_i915(struct intel_huc *huc)
>  	     (id__)++) \
>  		for_each_if ((engine__) = (dev_priv__)->engine[(id__)])
>  
> -#define __mask_next_bit(mask) ({					\
> -	int __idx = ffs(mask) - 1;					\
> -	mask &= ~BIT(__idx);						\
> -	__idx;								\
> -})
> -
>  /* Iterator over subset of engines selected by mask */
>  #define for_each_engine_masked(engine__, dev_priv__, mask__, tmp__) \
>  	for (tmp__ = mask__ & INTEL_INFO(dev_priv__)->ring_mask;	\
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index aa2e7401efdc..2e79ca129202 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -118,13 +118,16 @@ static void
>  fw_domains_get(struct drm_i915_private *i915, enum forcewake_domains fw_domains)
>  {
>  	struct intel_uncore_forcewake_domain *d;
> +	unsigned int tmp;
> +
> +	GEM_BUG_ON(fw_domains & ~i915->uncore.fw_domains);
>  
> -	for_each_fw_domain_masked(d, fw_domains, i915) {
> +	for_each_fw_domain_masked(d, fw_domains, i915, tmp) {
>  		fw_domain_wait_ack_clear(i915, d);
>  		fw_domain_get(i915, d);
>  	}
>  
> -	for_each_fw_domain_masked(d, fw_domains, i915)
> +	for_each_fw_domain_masked(d, fw_domains, i915, tmp)
>  		fw_domain_wait_ack(i915, d);
>  
>  	i915->uncore.fw_domains_active |= fw_domains;
> @@ -134,8 +137,11 @@ static void
>  fw_domains_put(struct drm_i915_private *i915, enum forcewake_domains fw_domains)
>  {
>  	struct intel_uncore_forcewake_domain *d;
> +	unsigned int tmp;
> +
> +	GEM_BUG_ON(fw_domains & ~i915->uncore.fw_domains);
>  
> -	for_each_fw_domain_masked(d, fw_domains, i915) {
> +	for_each_fw_domain_masked(d, fw_domains, i915, tmp) {
>  		fw_domain_put(i915, d);
>  		fw_domain_posting_read(i915, d);
>  	}
> @@ -147,9 +153,10 @@ static void
>  fw_domains_posting_read(struct drm_i915_private *i915)
>  {
>  	struct intel_uncore_forcewake_domain *d;
> +	unsigned int tmp;
>  
>  	/* No need to do for all, just do for first found */
> -	for_each_fw_domain(d, i915) {
> +	for_each_fw_domain(d, i915, tmp) {
>  		fw_domain_posting_read(i915, d);
>  		break;
>  	}
> @@ -160,11 +167,14 @@ fw_domains_reset(struct drm_i915_private *i915,
>  		 enum forcewake_domains fw_domains)
>  {
>  	struct intel_uncore_forcewake_domain *d;
> +	unsigned int tmp;
>  
> -	if (i915->uncore.fw_domains == 0)
> +	if (!fw_domains)
>  		return;
>  
> -	for_each_fw_domain_masked(d, fw_domains, i915)
> +	GEM_BUG_ON(fw_domains & ~i915->uncore.fw_domains);
> +
> +	for_each_fw_domain_masked(d, fw_domains, i915, tmp)
>  		fw_domain_reset(i915, d);
>  
>  	fw_domains_posting_read(i915);
> @@ -274,9 +284,11 @@ static void intel_uncore_forcewake_reset(struct drm_i915_private *dev_priv,
>  	 * timers are run before holding.
>  	 */
>  	while (1) {
> +		unsigned int tmp;
> +
>  		active_domains = 0;
>  
> -		for_each_fw_domain(domain, dev_priv) {
> +		for_each_fw_domain(domain, dev_priv, tmp) {
>  			if (hrtimer_cancel(&domain->timer) == 0)
>  				continue;
>  
> @@ -285,7 +297,7 @@ static void intel_uncore_forcewake_reset(struct drm_i915_private *dev_priv,
>  
>  		spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
>  
> -		for_each_fw_domain(domain, dev_priv) {
> +		for_each_fw_domain(domain, dev_priv, tmp) {
>  			if (hrtimer_active(&domain->timer))
>  				active_domains |= domain->mask;
>  		}
> @@ -308,7 +320,7 @@ static void intel_uncore_forcewake_reset(struct drm_i915_private *dev_priv,
>  	if (fw)
>  		dev_priv->uncore.funcs.force_wake_put(dev_priv, fw);
>  
> -	fw_domains_reset(dev_priv, FORCEWAKE_ALL);
> +	fw_domains_reset(dev_priv, dev_priv->uncore.fw_domains);
>  
>  	if (restore) { /* If reset with a user forcewake, try to restore */
>  		if (fw)
> @@ -465,13 +477,13 @@ static void __intel_uncore_forcewake_get(struct drm_i915_private *dev_priv,
>  					 enum forcewake_domains fw_domains)
>  {
>  	struct intel_uncore_forcewake_domain *domain;
> +	unsigned int tmp;
>  
>  	fw_domains &= dev_priv->uncore.fw_domains;
>  
> -	for_each_fw_domain_masked(domain, fw_domains, dev_priv) {
> +	for_each_fw_domain_masked(domain, fw_domains, dev_priv, tmp)
>  		if (domain->wake_count++)
>  			fw_domains &= ~domain->mask;
> -	}
>  
>  	if (fw_domains)
>  		dev_priv->uncore.funcs.force_wake_get(dev_priv, fw_domains);
> @@ -528,10 +540,11 @@ static void __intel_uncore_forcewake_put(struct drm_i915_private *dev_priv,
>  					 enum forcewake_domains fw_domains)
>  {
>  	struct intel_uncore_forcewake_domain *domain;
> +	unsigned int tmp;
>  
>  	fw_domains &= dev_priv->uncore.fw_domains;
>  
> -	for_each_fw_domain_masked(domain, fw_domains, dev_priv) {
> +	for_each_fw_domain_masked(domain, fw_domains, dev_priv, tmp) {
>  		if (WARN_ON(domain->wake_count == 0))
>  			continue;
>  
> @@ -936,8 +949,11 @@ static noinline void ___force_wake_auto(struct drm_i915_private *dev_priv,
>  					enum forcewake_domains fw_domains)
>  {
>  	struct intel_uncore_forcewake_domain *domain;
> +	unsigned int tmp;
> +
> +	GEM_BUG_ON(fw_domains & ~dev_priv->uncore.fw_domains);
>  
> -	for_each_fw_domain_masked(domain, fw_domains, dev_priv)
> +	for_each_fw_domain_masked(domain, fw_domains, dev_priv, tmp)
>  		fw_domain_arm_timer(domain);
>  
>  	dev_priv->uncore.funcs.force_wake_get(dev_priv, fw_domains);
> @@ -1175,7 +1191,7 @@ static void fw_domain_init(struct drm_i915_private *dev_priv,
>  	BUILD_BUG_ON(FORCEWAKE_BLITTER != (1 << FW_DOMAIN_ID_BLITTER));
>  	BUILD_BUG_ON(FORCEWAKE_MEDIA != (1 << FW_DOMAIN_ID_MEDIA));
>  
> -	d->mask = 1 << domain_id;
> +	d->mask = BIT(domain_id);
>  
>  	hrtimer_init(&d->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
>  	d->timer.function = intel_uncore_fw_release_timer;
> @@ -1253,9 +1269,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_ALL);
> +		fw_domains_get_with_thread_status(dev_priv, FORCEWAKE_RENDER);

Well we are skipping here the unused ones so I guess it fits
to the commit desc.

Reviewed-by: Mika Kuoppala <mika.kuoppala@xxxxxxxxx>

>  		ecobus = __raw_i915_read32(dev_priv, ECOBUS);
> -		fw_domains_put_with_fifo(dev_priv, FORCEWAKE_ALL);
> +		fw_domains_put_with_fifo(dev_priv, FORCEWAKE_RENDER);
>  		spin_unlock_irq(&dev_priv->uncore.lock);
>  
>  		if (!(ecobus & FORCEWAKE_MT_ENABLE)) {
> -- 
> 2.11.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux