[Intel-gfx] [PATCH 4/8] drm/i915: Reduce duplicated forcewake logic

Deepak S deepak.s at linux.intel.com
Fri Dec 12 04:48:16 PST 2014


On Monday 08 December 2014 11:57 PM, Mika Kuoppala wrote:
> From: Chris Wilson <chris at chris-wilson.co.uk>
>
> Introduce a structure to track the individual forcewake domains and use
> that to eliminate duplicate logic.
>
> v2: - Rebase on latest dinq (Mika)
>      - for_each_fw_domain macro (Mika)
>      - Handle reset atomically, keeping the timer running (Mika)
>      - for_each_fw_domain parameter ordering (Chris)
>      - defer timer on new register access (Mika)
>
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk> (v1)
> Signed-off-by: Mika Kuoppala <mika.kuoppala at intel.com>
> ---
>   drivers/gpu/drm/i915/i915_debugfs.c |  65 +++---
>   drivers/gpu/drm/i915/i915_drv.h     |  54 +++--
>   drivers/gpu/drm/i915/intel_uncore.c | 410 +++++++++++++-----------------------
>   3 files changed, 208 insertions(+), 321 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index e142629..5cc838b 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1235,14 +1235,36 @@ static int ironlake_drpc_info(struct seq_file *m)
>   	return 0;
>   }
>   
> -static int vlv_drpc_info(struct seq_file *m)
> +static int i915_gen6_forcewake_count_info(struct seq_file *m, void *data)
>   {
> +	struct drm_info_node *node = m->private;
> +	struct drm_device *dev = node->minor->dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct intel_uncore_forcewake_domain *fw_domain;
> +	const char *domain_names[] = {
> +		"render",
> +		"blitter",
> +		"media",
> +	};
> +	int i;
> +
> +	spin_lock_irq(&dev_priv->uncore.lock);
> +	for_each_fw_domain(fw_domain, dev_priv, i) {
> +		seq_printf(m, "%s.wake_count = %u\n",
> +			   domain_names[i],
> +			   fw_domain->wake_count);
> +	}
> +	spin_unlock_irq(&dev_priv->uncore.lock);
>   
> +	return 0;
> +}
> +
> +static int vlv_drpc_info(struct seq_file *m)
> +{
>   	struct drm_info_node *node = m->private;
>   	struct drm_device *dev = node->minor->dev;
>   	struct drm_i915_private *dev_priv = dev->dev_private;
>   	u32 rpmodectl1, rcctl1, pw_status;
> -	unsigned fw_rendercount = 0, fw_mediacount = 0;
>   
>   	intel_runtime_pm_get(dev_priv);
>   
> @@ -1274,22 +1296,11 @@ static int vlv_drpc_info(struct seq_file *m)
>   	seq_printf(m, "Media RC6 residency since boot: %u\n",
>   		   I915_READ(VLV_GT_MEDIA_RC6));
>   
> -	spin_lock_irq(&dev_priv->uncore.lock);
> -	fw_rendercount = dev_priv->uncore.fw_rendercount;
> -	fw_mediacount = dev_priv->uncore.fw_mediacount;
> -	spin_unlock_irq(&dev_priv->uncore.lock);
> -
> -	seq_printf(m, "Forcewake Render Count = %u\n", fw_rendercount);
> -	seq_printf(m, "Forcewake Media Count = %u\n", fw_mediacount);
> -
> -
> -	return 0;
> +	return i915_gen6_forcewake_count_info(m, NULL);
>   }
>   
> -
>   static int gen6_drpc_info(struct seq_file *m)
>   {
> -
>   	struct drm_info_node *node = m->private;
>   	struct drm_device *dev = node->minor->dev;
>   	struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -1303,7 +1314,7 @@ static int gen6_drpc_info(struct seq_file *m)
>   	intel_runtime_pm_get(dev_priv);
>   
>   	spin_lock_irq(&dev_priv->uncore.lock);
> -	forcewake_count = dev_priv->uncore.forcewake_count;
> +	forcewake_count = dev_priv->uncore.fw_domain[FW_DOMAIN_ID_RENDER].wake_count;
>   	spin_unlock_irq(&dev_priv->uncore.lock);
>   
>   	if (forcewake_count) {
> @@ -1931,30 +1942,6 @@ static int i915_execlists(struct seq_file *m, void *data)
>   	return 0;
>   }
>   
> -static int i915_gen6_forcewake_count_info(struct seq_file *m, void *data)
> -{
> -	struct drm_info_node *node = m->private;
> -	struct drm_device *dev = node->minor->dev;
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> -	unsigned forcewake_count = 0, fw_rendercount = 0, fw_mediacount = 0;
> -
> -	spin_lock_irq(&dev_priv->uncore.lock);
> -	if (IS_VALLEYVIEW(dev)) {
> -		fw_rendercount = dev_priv->uncore.fw_rendercount;
> -		fw_mediacount = dev_priv->uncore.fw_mediacount;
> -	} else
> -		forcewake_count = dev_priv->uncore.forcewake_count;
> -	spin_unlock_irq(&dev_priv->uncore.lock);
> -
> -	if (IS_VALLEYVIEW(dev)) {
> -		seq_printf(m, "fw_rendercount = %u\n", fw_rendercount);
> -		seq_printf(m, "fw_mediacount = %u\n", fw_mediacount);
> -	} else
> -		seq_printf(m, "forcewake count = %u\n", forcewake_count);
> -
> -	return 0;
> -}
> -
>   static const char *swizzle_string(unsigned swizzle)
>   {
>   	switch (swizzle) {
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 95dfa2d..410558a 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -554,20 +554,45 @@ struct intel_uncore_funcs {
>   				uint64_t val, bool trace);
>   };
>   
> +enum {
> +	FW_DOMAIN_ID_RENDER = 0,
> +	FW_DOMAIN_ID_BLITTER,
> +	FW_DOMAIN_ID_MEDIA,
> +
> +	FW_DOMAIN_ID_COUNT
> +};
> +
>   struct intel_uncore {
>   	spinlock_t lock; /** lock is also taken in irq contexts. */
>   
>   	struct intel_uncore_funcs funcs;
>   
>   	unsigned fifo_count;
> -	unsigned forcewake_count;
> -
> -	unsigned fw_rendercount;
> -	unsigned fw_mediacount;
> -	unsigned fw_blittercount;
> -
> -	struct timer_list force_wake_timer;
> -};
> +	unsigned fw_domains;
> +
> +	struct intel_uncore_forcewake_domain {
> +		struct drm_i915_private *i915;
> +		int id;
> +		unsigned wake_count;
> +		struct timer_list timer;
> +	} fw_domain[FW_DOMAIN_ID_COUNT];
> +#define FORCEWAKE_RENDER	(1 << FW_DOMAIN_ID_RENDER)
> +#define FORCEWAKE_BLITTER	(1 << FW_DOMAIN_ID_BLITTER)
> +#define FORCEWAKE_MEDIA		(1 << FW_DOMAIN_ID_MEDIA)
> +#define FORCEWAKE_ALL		(FORCEWAKE_RENDER | \
> +				 FORCEWAKE_BLITTER | \
> +				 FORCEWAKE_MEDIA)
> +};
> +
> +/* Iterate over initialised fw domains */
> +#define for_each_fw_domain_mask(domain__, mask__, dev_priv__, i__) \
> +	for ((i__) = 0, (domain__) = &(dev_priv__)->uncore.fw_domain[0]; \
> +	     (i__) < FW_DOMAIN_ID_COUNT; \
> +	     (i__)++, (domain__) = &(dev_priv__)->uncore.fw_domain[i__]) \
> +		if (((mask__) & (dev_priv__)->uncore.fw_domains) & (1 << (i__)))
> +
> +#define for_each_fw_domain(domain__, dev_priv__, i__) \
> +	for_each_fw_domain_mask(domain__, FORCEWAKE_ALL, dev_priv__, i__)
>   
>   #define DEV_INFO_FOR_EACH_FLAG(func, sep) \
>   	func(is_mobile) sep \
> @@ -2998,8 +3023,10 @@ extern void intel_display_print_error_state(struct drm_i915_error_state_buf *e,
>    * must be set to prevent GT core from power down and stale values being
>    * returned.
>    */
> -void gen6_gt_force_wake_get(struct drm_i915_private *dev_priv, int fw_engine);
> -void gen6_gt_force_wake_put(struct drm_i915_private *dev_priv, int fw_engine);
> +void gen6_gt_force_wake_get(struct drm_i915_private *dev_priv,
> +			    unsigned fw_domains);
> +void gen6_gt_force_wake_put(struct drm_i915_private *dev_priv,
> +			    unsigned fw_domains);
>   void assert_force_wake_inactive(struct drm_i915_private *dev_priv);
>   
>   int sandybridge_pcode_read(struct drm_i915_private *dev_priv, u32 mbox, u32 *val);
> @@ -3031,13 +3058,6 @@ void vlv_flisdsi_write(struct drm_i915_private *dev_priv, u32 reg, u32 val);
>   int vlv_gpu_freq(struct drm_i915_private *dev_priv, int val);
>   int vlv_freq_opcode(struct drm_i915_private *dev_priv, int val);
>   
> -#define FORCEWAKE_RENDER	(1 << 0)
> -#define FORCEWAKE_MEDIA		(1 << 1)
> -#define FORCEWAKE_BLITTER	(1 << 2)
> -#define FORCEWAKE_ALL		(FORCEWAKE_RENDER | FORCEWAKE_MEDIA | \
> -					FORCEWAKE_BLITTER)
> -
> -
>   #define I915_READ8(reg)		dev_priv->uncore.funcs.mmio_readb(dev_priv, (reg), true)
>   #define I915_WRITE8(reg, val)	dev_priv->uncore.funcs.mmio_writeb(dev_priv, (reg), (val), true)
>   
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index 069fe7a..85e46b0 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -67,7 +67,7 @@ static void __gen6_gt_force_wake_reset(struct drm_i915_private *dev_priv)
>   }
>   
>   static void __gen6_gt_force_wake_get(struct drm_i915_private *dev_priv,
> -							int fw_engine)
> +				     int fw_engine)
>   {
>   	if (wait_for_atomic((__raw_i915_read32(dev_priv, FORCEWAKE_ACK) & 1) == 0,
>   			    FORCEWAKE_ACK_TIMEOUT_MS))
> @@ -93,7 +93,7 @@ static void __gen7_gt_force_wake_mt_reset(struct drm_i915_private *dev_priv)
>   }
>   
>   static void __gen7_gt_force_wake_mt_get(struct drm_i915_private *dev_priv,
> -							int fw_engine)
> +					int fw_engine)
>   {
>   	u32 forcewake_ack;
>   
> @@ -129,7 +129,7 @@ static void gen6_gt_check_fifodbg(struct drm_i915_private *dev_priv)
>   }
>   
>   static void __gen6_gt_force_wake_put(struct drm_i915_private *dev_priv,
> -							int fw_engine)
> +				     int fw_engine)
>   {
>   	__raw_i915_write32(dev_priv, FORCEWAKE, 0);
>   	/* something from same cacheline, but !FORCEWAKE */
> @@ -138,7 +138,7 @@ static void __gen6_gt_force_wake_put(struct drm_i915_private *dev_priv,
>   }
>   
>   static void __gen7_gt_force_wake_mt_put(struct drm_i915_private *dev_priv,
> -							int fw_engine)
> +					int fw_engine)
>   {
>   	__raw_i915_write32(dev_priv, FORCEWAKE_MT,
>   			   _MASKED_BIT_DISABLE(FORCEWAKE_KERNEL));
> @@ -187,7 +187,7 @@ static void vlv_force_wake_reset(struct drm_i915_private *dev_priv)
>   }
>   
>   static void __vlv_force_wake_get(struct drm_i915_private *dev_priv,
> -						int fw_engine)
> +				 int fw_engine)
>   {
>   	/* Check for Render Engine */
>   	if (FORCEWAKE_RENDER & fw_engine) {
> @@ -227,9 +227,8 @@ static void __vlv_force_wake_get(struct drm_i915_private *dev_priv,
>   }
>   
>   static void __vlv_force_wake_put(struct drm_i915_private *dev_priv,
> -					int fw_engine)
> +				 int fw_engine)
>   {
> -
>   	/* Check for Render Engine */
>   	if (FORCEWAKE_RENDER & fw_engine)
>   		__raw_i915_write32(dev_priv, FORCEWAKE_VLV,
> @@ -247,37 +246,6 @@ static void __vlv_force_wake_put(struct drm_i915_private *dev_priv,
>   		gen6_gt_check_fifodbg(dev_priv);
>   }
>   
> -static void vlv_force_wake_get(struct drm_i915_private *dev_priv, int fw_engine)
> -{
> -	if (fw_engine & FORCEWAKE_RENDER &&
> -	    dev_priv->uncore.fw_rendercount++ != 0)
> -		fw_engine &= ~FORCEWAKE_RENDER;
> -	if (fw_engine & FORCEWAKE_MEDIA &&
> -	    dev_priv->uncore.fw_mediacount++ != 0)
> -		fw_engine &= ~FORCEWAKE_MEDIA;
> -
> -	if (fw_engine)
> -		dev_priv->uncore.funcs.force_wake_get(dev_priv, fw_engine);
> -}
> -
> -static void vlv_force_wake_put(struct drm_i915_private *dev_priv, int fw_engine)
> -{
> -	if (fw_engine & FORCEWAKE_RENDER) {
> -		WARN_ON(!dev_priv->uncore.fw_rendercount);
> -		if (--dev_priv->uncore.fw_rendercount != 0)
> -			fw_engine &= ~FORCEWAKE_RENDER;
> -	}
> -
> -	if (fw_engine & FORCEWAKE_MEDIA) {
> -		WARN_ON(!dev_priv->uncore.fw_mediacount);
> -		if (--dev_priv->uncore.fw_mediacount != 0)
> -			fw_engine &= ~FORCEWAKE_MEDIA;
> -	}
> -
> -	if (fw_engine)
> -		dev_priv->uncore.funcs.force_wake_put(dev_priv, fw_engine);
> -}
> -
>   static void __gen9_gt_force_wake_mt_reset(struct drm_i915_private *dev_priv)
>   {
>   	__raw_i915_write32(dev_priv, FORCEWAKE_RENDER_GEN9,
> @@ -367,80 +335,40 @@ __gen9_force_wake_put(struct drm_i915_private *dev_priv, int fw_engine)
>   				_MASKED_BIT_DISABLE(FORCEWAKE_KERNEL));
>   }
>   
> -static void
> -gen9_force_wake_get(struct drm_i915_private *dev_priv, int fw_engine)
> -{
> -	if (FORCEWAKE_RENDER & fw_engine) {
> -		if (dev_priv->uncore.fw_rendercount++ == 0)
> -			dev_priv->uncore.funcs.force_wake_get(dev_priv,
> -							FORCEWAKE_RENDER);
> -	}
> -
> -	if (FORCEWAKE_MEDIA & fw_engine) {
> -		if (dev_priv->uncore.fw_mediacount++ == 0)
> -			dev_priv->uncore.funcs.force_wake_get(dev_priv,
> -							FORCEWAKE_MEDIA);
> -	}
> -
> -	if (FORCEWAKE_BLITTER & fw_engine) {
> -		if (dev_priv->uncore.fw_blittercount++ == 0)
> -			dev_priv->uncore.funcs.force_wake_get(dev_priv,
> -							FORCEWAKE_BLITTER);
> -	}
> -}
> -
> -static void
> -gen9_force_wake_put(struct drm_i915_private *dev_priv, int fw_engine)
> -{
> -	if (FORCEWAKE_RENDER & fw_engine) {
> -		WARN_ON(dev_priv->uncore.fw_rendercount == 0);
> -		if (--dev_priv->uncore.fw_rendercount == 0)
> -			dev_priv->uncore.funcs.force_wake_put(dev_priv,
> -							FORCEWAKE_RENDER);
> -	}
> -
> -	if (FORCEWAKE_MEDIA & fw_engine) {
> -		WARN_ON(dev_priv->uncore.fw_mediacount == 0);
> -		if (--dev_priv->uncore.fw_mediacount == 0)
> -			dev_priv->uncore.funcs.force_wake_put(dev_priv,
> -							FORCEWAKE_MEDIA);
> -	}
> -
> -	if (FORCEWAKE_BLITTER & fw_engine) {
> -		WARN_ON(dev_priv->uncore.fw_blittercount == 0);
> -		if (--dev_priv->uncore.fw_blittercount == 0)
> -			dev_priv->uncore.funcs.force_wake_put(dev_priv,
> -							FORCEWAKE_BLITTER);
> -	}
> -}
> -
>   static void gen6_force_wake_timer(unsigned long arg)
>   {
> -	struct drm_i915_private *dev_priv = (void *)arg;
> +	struct intel_uncore_forcewake_domain *domain = (void *)arg;
>   	unsigned long irqflags;
>   
> -	assert_device_not_suspended(dev_priv);
> +	assert_device_not_suspended(domain->i915);
>   
> -	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
> -	WARN_ON(!dev_priv->uncore.forcewake_count);
> +	spin_lock_irqsave(&domain->i915->uncore.lock, irqflags);
> +	WARN_ON(!domain->wake_count);
>   
> -	if (--dev_priv->uncore.forcewake_count == 0)
> -		dev_priv->uncore.funcs.force_wake_put(dev_priv, FORCEWAKE_ALL);
> -	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
> +	if (--domain->wake_count == 0)
> +		domain->i915->uncore.funcs.force_wake_put(domain->i915,
> +							  1 << domain->id);
> +	spin_unlock_irqrestore(&domain->i915->uncore.lock, irqflags);
>   }
>   
>   void intel_uncore_forcewake_reset(struct drm_device *dev, bool restore)
>   {
>   	struct drm_i915_private *dev_priv = dev->dev_private;
> -	unsigned long irqflags;
> -
> -	if (del_timer_sync(&dev_priv->uncore.force_wake_timer))
> -		gen6_force_wake_timer((unsigned long)dev_priv);
> +	unsigned long irqflags, fw = 0;
> +	struct intel_uncore_forcewake_domain *domain;
> +	int id;
>   
> +	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
>   	/* Hold uncore.lock across reset to prevent any register access
>   	 * with forcewake not set correctly
>   	 */
> -	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
> +
> +	for_each_fw_domain(domain, dev_priv, id)
> +		if (domain->wake_count)
> +			fw |= 1 << id;
> +
> +	if (fw)
> +		dev_priv->uncore.funcs.force_wake_put(dev_priv, fw);
>   

Hi Mika,

Why do put?

Is there a possibility of getting get & put count mismatch? for example,
1. We Read the register
    -> forcewake_get & start domain_timer. (count = 1)
2. Now we call "intel_uncore_forcewake_reset" which does a put. (count = 0)
3. now the domain_timer from step #1 comes (count -1)? right?
    or i am missing something here?


Others Looks fine to me.
Acked-by: Deepak S <deepak.s at linux.intel.com>

>   	if (IS_VALLEYVIEW(dev))
>   		vlv_force_wake_reset(dev_priv);
> @@ -454,28 +382,6 @@ void intel_uncore_forcewake_reset(struct drm_device *dev, bool restore)
>   		__gen9_gt_force_wake_mt_reset(dev_priv);
>   
>   	if (restore) { /* If reset with a user forcewake, try to restore */
> -		unsigned fw = 0;
> -
> -		if (IS_VALLEYVIEW(dev)) {
> -			if (dev_priv->uncore.fw_rendercount)
> -				fw |= FORCEWAKE_RENDER;
> -
> -			if (dev_priv->uncore.fw_mediacount)
> -				fw |= FORCEWAKE_MEDIA;
> -		} else if (IS_GEN9(dev)) {
> -			if (dev_priv->uncore.fw_rendercount)
> -				fw |= FORCEWAKE_RENDER;
> -
> -			if (dev_priv->uncore.fw_mediacount)
> -				fw |= FORCEWAKE_MEDIA;
> -
> -			if (dev_priv->uncore.fw_blittercount)
> -				fw |= FORCEWAKE_BLITTER;
> -		} else {
> -			if (dev_priv->uncore.forcewake_count)
> -				fw = FORCEWAKE_ALL;
> -		}
> -
>   		if (fw)
>   			dev_priv->uncore.funcs.force_wake_get(dev_priv, fw);
>   
> @@ -533,28 +439,28 @@ void intel_uncore_sanitize(struct drm_device *dev)
>    * be called at the beginning of the sequence followed by a call to
>    * gen6_gt_force_wake_put() at the end of the sequence.
>    */
> -void gen6_gt_force_wake_get(struct drm_i915_private *dev_priv, int fw_engine)
> +void gen6_gt_force_wake_get(struct drm_i915_private *dev_priv,
> +			    unsigned fw_domains)
>   {
>   	unsigned long irqflags;
> +	struct intel_uncore_forcewake_domain *domain;
> +	int id;
>   
>   	if (!dev_priv->uncore.funcs.force_wake_get)
>   		return;
>   
> -	intel_runtime_pm_get(dev_priv);
> -

you need to change this in Patch #2

>   	WARN_ON(!pm_runtime_active(&dev_priv->dev->pdev->dev));
>   
> +	fw_domains &= dev_priv->uncore.fw_domains;
> +
>   	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
>   
> -	if (IS_GEN9(dev_priv->dev)) {
> -		gen9_force_wake_get(dev_priv, fw_engine);
> -	} else if (IS_VALLEYVIEW(dev_priv->dev)) {
> -		vlv_force_wake_get(dev_priv, fw_engine);
> -	} else {
> -		if (dev_priv->uncore.forcewake_count++ == 0)
> -			dev_priv->uncore.funcs.force_wake_get(dev_priv,
> -							      FORCEWAKE_ALL);
> -	}
> +	for_each_fw_domain_mask(domain, fw_domains, dev_priv, id)
> +		if (domain->wake_count++)
> +			fw_domains &= ~(1 << id);
> +
> +	if (fw_domains)
> +		dev_priv->uncore.funcs.force_wake_get(dev_priv, fw_domains);
>   
>   	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
>   }
> @@ -562,26 +468,27 @@ void gen6_gt_force_wake_get(struct drm_i915_private *dev_priv, int fw_engine)
>   /*
>    * see gen6_gt_force_wake_get()
>    */
> -void gen6_gt_force_wake_put(struct drm_i915_private *dev_priv, int fw_engine)
> +void gen6_gt_force_wake_put(struct drm_i915_private *dev_priv,
> +			    unsigned fw_domains)
>   {
>   	unsigned long irqflags;
> +	struct intel_uncore_forcewake_domain *domain;
> +	int id;
>   
>   	if (!dev_priv->uncore.funcs.force_wake_put)
>   		return;
>   
> +	fw_domains &= dev_priv->uncore.fw_domains;
> +
>   	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
>   
> -	if (IS_GEN9(dev_priv->dev)) {
> -		gen9_force_wake_put(dev_priv, fw_engine);
> -	} else if (IS_VALLEYVIEW(dev_priv->dev)) {
> -		vlv_force_wake_put(dev_priv, fw_engine);
> -	} else {
> -		WARN_ON(!dev_priv->uncore.forcewake_count);
> -		if (--dev_priv->uncore.forcewake_count == 0) {
> -			dev_priv->uncore.forcewake_count++;
> -			mod_timer_pinned(&dev_priv->uncore.force_wake_timer,
> -					 jiffies + 1);
> -		}
> +	for_each_fw_domain_mask(domain, fw_domains, dev_priv, id) {
> +		WARN_ON(!domain->wake_count);
> +		if (--domain->wake_count)
> +			continue;
> +
> +		domain->wake_count++;
> +		mod_timer_pinned(&domain->timer, jiffies + 1);
>   	}
>   
>   	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
> @@ -589,10 +496,14 @@ void gen6_gt_force_wake_put(struct drm_i915_private *dev_priv, int fw_engine)
>   
>   void assert_force_wake_inactive(struct drm_i915_private *dev_priv)
>   {
> +	struct intel_uncore_forcewake_domain *domain;
> +	int i;
> +
>   	if (!dev_priv->uncore.funcs.force_wake_get)
>   		return;
>   
> -	WARN_ON(dev_priv->uncore.forcewake_count > 0);
> +	for_each_fw_domain(domain, dev_priv, i)
> +		WARN_ON(domain->wake_count);
>   }
>   
>   /* We give fast paths for the really cool registers */
> @@ -758,19 +669,36 @@ __gen2_read(64)
>   	trace_i915_reg_rw(false, reg, val, sizeof(val), trace); \
>   	return val
>   
> +static inline void __force_wake_get(struct drm_i915_private *dev_priv,
> +				    unsigned fw_domains)
> +{
> +	struct intel_uncore_forcewake_domain *domain;
> +	int i;
> +
> +	if (WARN_ON(!fw_domains))
> +		return;
> +
> +	/* Ideally GCC would be constant-fold and eliminate this loop */
> +	for_each_fw_domain_mask(domain, fw_domains, dev_priv, i) {
> +		if (domain->wake_count)
> +			fw_domains &= ~(1 << i);
> +		else
> +			domain->wake_count++;
> +
> +		mod_timer_pinned(&domain->timer, jiffies + 1);
> +	}
> +
> +	if (fw_domains)
> +		dev_priv->uncore.funcs.force_wake_get(dev_priv, fw_domains);
> +}
> +
>   #define __gen6_read(x) \
>   static u##x \
>   gen6_read##x(struct drm_i915_private *dev_priv, off_t reg, bool trace) { \
>   	GEN6_READ_HEADER(x); \
>   	hsw_unclaimed_reg_debug(dev_priv, reg, true, true); \
> -	if (dev_priv->uncore.forcewake_count == 0 && \
> -	    NEEDS_FORCE_WAKE((dev_priv), (reg))) { \
> -		dev_priv->uncore.funcs.force_wake_get(dev_priv, \
> -						      FORCEWAKE_ALL); \
> -		dev_priv->uncore.forcewake_count++; \
> -		mod_timer_pinned(&dev_priv->uncore.force_wake_timer, \
> -				 jiffies + 1); \
> -	} \
> +	if (NEEDS_FORCE_WAKE((dev_priv), (reg))) \
> +		__force_wake_get(dev_priv, FORCEWAKE_RENDER); \
>   	val = __raw_i915_read##x(dev_priv, reg); \
>   	hsw_unclaimed_reg_debug(dev_priv, reg, true, false); \
>   	GEN6_READ_FOOTER; \
> @@ -779,45 +707,27 @@ gen6_read##x(struct drm_i915_private *dev_priv, off_t reg, bool trace) { \
>   #define __vlv_read(x) \
>   static u##x \
>   vlv_read##x(struct drm_i915_private *dev_priv, off_t reg, bool trace) { \
> -	unsigned fwengine = 0; \
>   	GEN6_READ_HEADER(x); \
> -	if (FORCEWAKE_VLV_RENDER_RANGE_OFFSET(reg)) { \
> -		if (dev_priv->uncore.fw_rendercount == 0) \
> -			fwengine = FORCEWAKE_RENDER; \
> -	} else if (FORCEWAKE_VLV_MEDIA_RANGE_OFFSET(reg)) { \
> -		if (dev_priv->uncore.fw_mediacount == 0) \
> -			fwengine = FORCEWAKE_MEDIA; \
> -	}  \
> -	if (fwengine) \
> -		dev_priv->uncore.funcs.force_wake_get(dev_priv, fwengine); \
> +	if (FORCEWAKE_VLV_RENDER_RANGE_OFFSET(reg)) \
> +		__force_wake_get(dev_priv, FORCEWAKE_RENDER); \
> +	else if (FORCEWAKE_VLV_MEDIA_RANGE_OFFSET(reg)) \
> +		__force_wake_get(dev_priv, FORCEWAKE_MEDIA); \
>   	val = __raw_i915_read##x(dev_priv, reg); \
> -	if (fwengine) \
> -		dev_priv->uncore.funcs.force_wake_put(dev_priv, fwengine); \
>   	GEN6_READ_FOOTER; \
>   }
>   
>   #define __chv_read(x) \
>   static u##x \
>   chv_read##x(struct drm_i915_private *dev_priv, off_t reg, bool trace) { \
> -	unsigned fwengine = 0; \
>   	GEN6_READ_HEADER(x); \
> -	if (FORCEWAKE_CHV_RENDER_RANGE_OFFSET(reg)) { \
> -		if (dev_priv->uncore.fw_rendercount == 0) \
> -			fwengine = FORCEWAKE_RENDER; \
> -	} else if (FORCEWAKE_CHV_MEDIA_RANGE_OFFSET(reg)) { \
> -		if (dev_priv->uncore.fw_mediacount == 0) \
> -			fwengine = FORCEWAKE_MEDIA; \
> -	} else if (FORCEWAKE_CHV_COMMON_RANGE_OFFSET(reg)) { \
> -		if (dev_priv->uncore.fw_rendercount == 0) \
> -			fwengine |= FORCEWAKE_RENDER; \
> -		if (dev_priv->uncore.fw_mediacount == 0) \
> -			fwengine |= FORCEWAKE_MEDIA; \
> -	} \
> -	if (fwengine) \
> -		dev_priv->uncore.funcs.force_wake_get(dev_priv, fwengine); \
> +	if (FORCEWAKE_CHV_RENDER_RANGE_OFFSET(reg)) \
> +		__force_wake_get(dev_priv, FORCEWAKE_RENDER); \
> +	else if (FORCEWAKE_CHV_MEDIA_RANGE_OFFSET(reg)) \
> +		__force_wake_get(dev_priv, FORCEWAKE_MEDIA); \
> +	else if (FORCEWAKE_CHV_COMMON_RANGE_OFFSET(reg)) \
> +		__force_wake_get(dev_priv, \
> +				 FORCEWAKE_RENDER | FORCEWAKE_MEDIA); \
>   	val = __raw_i915_read##x(dev_priv, reg); \
> -	if (fwengine) \
> -		dev_priv->uncore.funcs.force_wake_put(dev_priv, fwengine); \
>   	GEN6_READ_FOOTER; \
>   }
>   
> @@ -827,32 +737,21 @@ chv_read##x(struct drm_i915_private *dev_priv, off_t reg, bool trace) { \
>   #define __gen9_read(x) \
>   static u##x \
>   gen9_read##x(struct drm_i915_private *dev_priv, off_t reg, bool trace) { \
> +	unsigned fw_engine; \
>   	GEN6_READ_HEADER(x); \
> -	if (!SKL_NEEDS_FORCE_WAKE((dev_priv), (reg))) { \
> -		val = __raw_i915_read##x(dev_priv, reg); \
> -	} else { \
> -		unsigned fwengine = 0; \
> -		if (FORCEWAKE_GEN9_RENDER_RANGE_OFFSET(reg)) { \
> -			if (dev_priv->uncore.fw_rendercount == 0) \
> -				fwengine = FORCEWAKE_RENDER; \
> -		} else if (FORCEWAKE_GEN9_MEDIA_RANGE_OFFSET(reg)) { \
> -			if (dev_priv->uncore.fw_mediacount == 0) \
> -				fwengine = FORCEWAKE_MEDIA; \
> -		} else if (FORCEWAKE_GEN9_COMMON_RANGE_OFFSET(reg)) { \
> -			if (dev_priv->uncore.fw_rendercount == 0) \
> -				fwengine |= FORCEWAKE_RENDER; \
> -			if (dev_priv->uncore.fw_mediacount == 0) \
> -				fwengine |= FORCEWAKE_MEDIA; \
> -		} else { \
> -			if (dev_priv->uncore.fw_blittercount == 0) \
> -				fwengine = FORCEWAKE_BLITTER; \
> -		} \
> -		if (fwengine) \
> -			dev_priv->uncore.funcs.force_wake_get(dev_priv, fwengine); \
> -		val = __raw_i915_read##x(dev_priv, reg); \
> -		if (fwengine) \
> -			dev_priv->uncore.funcs.force_wake_put(dev_priv, fwengine); \
> -	} \
> +	if (!SKL_NEEDS_FORCE_WAKE((dev_priv), (reg)))	\
> +		fw_engine = 0; \
> +	else if (FORCEWAKE_GEN9_RENDER_RANGE_OFFSET(reg))	\
> +		fw_engine = FORCEWAKE_RENDER; \
> +	else if (FORCEWAKE_GEN9_MEDIA_RANGE_OFFSET(reg)) \
> +		fw_engine = FORCEWAKE_MEDIA; \
> +	else if (FORCEWAKE_GEN9_COMMON_RANGE_OFFSET(reg)) \
> +		fw_engine = FORCEWAKE_RENDER | FORCEWAKE_MEDIA; \
> +	else \
> +		fw_engine = FORCEWAKE_BLITTER; \
> +	if (fw_engine) \
> +		__force_wake_get(dev_priv, fw_engine); \
> +	val = __raw_i915_read##x(dev_priv, reg); \
>   	GEN6_READ_FOOTER; \
>   }
>   
> @@ -986,17 +885,9 @@ static void \
>   gen8_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool trace) { \
>   	GEN6_WRITE_HEADER; \
>   	hsw_unclaimed_reg_debug(dev_priv, reg, false, true); \
> -	if (reg < 0x40000 && !is_gen8_shadowed(dev_priv, reg)) { \
> -		if (dev_priv->uncore.forcewake_count == 0) \
> -			dev_priv->uncore.funcs.force_wake_get(dev_priv,	\
> -							      FORCEWAKE_ALL); \
> -		__raw_i915_write##x(dev_priv, reg, val); \
> -		if (dev_priv->uncore.forcewake_count == 0) \
> -			dev_priv->uncore.funcs.force_wake_put(dev_priv, \
> -							      FORCEWAKE_ALL); \
> -	} else { \
> -		__raw_i915_write##x(dev_priv, reg, val); \
> -	} \
> +	if (reg < 0x40000 && !is_gen8_shadowed(dev_priv, reg)) \
> +		__force_wake_get(dev_priv, FORCEWAKE_RENDER); \
> +	__raw_i915_write##x(dev_priv, reg, val); \
>   	hsw_unclaimed_reg_debug(dev_priv, reg, false, false); \
>   	hsw_unclaimed_reg_detect(dev_priv); \
>   	GEN6_WRITE_FOOTER; \
> @@ -1005,28 +896,17 @@ gen8_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool trace
>   #define __chv_write(x) \
>   static void \
>   chv_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool trace) { \
> -	unsigned fwengine = 0; \
>   	bool shadowed = is_gen8_shadowed(dev_priv, reg); \
>   	GEN6_WRITE_HEADER; \
>   	if (!shadowed) { \
> -		if (FORCEWAKE_CHV_RENDER_RANGE_OFFSET(reg)) { \
> -			if (dev_priv->uncore.fw_rendercount == 0) \
> -				fwengine = FORCEWAKE_RENDER; \
> -		} else if (FORCEWAKE_CHV_MEDIA_RANGE_OFFSET(reg)) { \
> -			if (dev_priv->uncore.fw_mediacount == 0) \
> -				fwengine = FORCEWAKE_MEDIA; \
> -		} else if (FORCEWAKE_CHV_COMMON_RANGE_OFFSET(reg)) { \
> -			if (dev_priv->uncore.fw_rendercount == 0) \
> -				fwengine |= FORCEWAKE_RENDER; \
> -			if (dev_priv->uncore.fw_mediacount == 0) \
> -				fwengine |= FORCEWAKE_MEDIA; \
> -		} \
> +		if (FORCEWAKE_CHV_RENDER_RANGE_OFFSET(reg)) \
> +			__force_wake_get(dev_priv, FORCEWAKE_RENDER); \
> +		else if (FORCEWAKE_CHV_MEDIA_RANGE_OFFSET(reg)) \
> +			__force_wake_get(dev_priv, FORCEWAKE_MEDIA); \
> +		else if (FORCEWAKE_CHV_COMMON_RANGE_OFFSET(reg)) \
> +			__force_wake_get(dev_priv, FORCEWAKE_RENDER | FORCEWAKE_MEDIA); \
>   	} \
> -	if (fwengine) \
> -		dev_priv->uncore.funcs.force_wake_get(dev_priv, fwengine); \
>   	__raw_i915_write##x(dev_priv, reg, val); \
> -	if (fwengine) \
> -		dev_priv->uncore.funcs.force_wake_put(dev_priv, fwengine); \
>   	GEN6_WRITE_FOOTER; \
>   }
>   
> @@ -1057,35 +937,22 @@ static bool is_gen9_shadowed(struct drm_i915_private *dev_priv, u32 reg)
>   static void \
>   gen9_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, \
>   		bool trace) { \
> +	unsigned fw_engine; \
>   	GEN6_WRITE_HEADER; \
> -	if (!SKL_NEEDS_FORCE_WAKE((dev_priv), (reg)) || \
> -			is_gen9_shadowed(dev_priv, reg)) { \
> -		__raw_i915_write##x(dev_priv, reg, val); \
> -	} else { \
> -		unsigned fwengine = 0; \
> -		if (FORCEWAKE_GEN9_RENDER_RANGE_OFFSET(reg)) { \
> -			if (dev_priv->uncore.fw_rendercount == 0) \
> -				fwengine = FORCEWAKE_RENDER; \
> -		} else if (FORCEWAKE_GEN9_MEDIA_RANGE_OFFSET(reg)) { \
> -			if (dev_priv->uncore.fw_mediacount == 0) \
> -				fwengine = FORCEWAKE_MEDIA; \
> -		} else if (FORCEWAKE_GEN9_COMMON_RANGE_OFFSET(reg)) { \
> -			if (dev_priv->uncore.fw_rendercount == 0) \
> -				fwengine |= FORCEWAKE_RENDER; \
> -			if (dev_priv->uncore.fw_mediacount == 0) \
> -				fwengine |= FORCEWAKE_MEDIA; \
> -		} else { \
> -			if (dev_priv->uncore.fw_blittercount == 0) \
> -				fwengine = FORCEWAKE_BLITTER; \
> -		} \
> -		if (fwengine) \
> -			dev_priv->uncore.funcs.force_wake_get(dev_priv, \
> -					fwengine); \
> -		__raw_i915_write##x(dev_priv, reg, val); \
> -		if (fwengine) \
> -			dev_priv->uncore.funcs.force_wake_put(dev_priv, \
> -					fwengine); \
> -	} \
> +	if (!SKL_NEEDS_FORCE_WAKE((dev_priv), (reg)) ||	\
> +	    is_gen9_shadowed(dev_priv, reg)) \
> +		fw_engine = 0; \
> +	else if (FORCEWAKE_GEN9_RENDER_RANGE_OFFSET(reg)) \
> +		fw_engine = FORCEWAKE_RENDER; \
> +	else if (FORCEWAKE_GEN9_MEDIA_RANGE_OFFSET(reg)) \
> +		fw_engine = FORCEWAKE_MEDIA; \
> +	else if (FORCEWAKE_GEN9_COMMON_RANGE_OFFSET(reg)) \
> +		fw_engine = FORCEWAKE_RENDER | FORCEWAKE_MEDIA; \
> +	else \
> +		fw_engine = FORCEWAKE_BLITTER; \
> +	if (fw_engine) \
> +		__force_wake_get(dev_priv, fw_engine); \
> +	__raw_i915_write##x(dev_priv, reg, val); \
>   	GEN6_WRITE_FOOTER; \
>   }
>   
> @@ -1137,21 +1004,24 @@ do { \
>   void intel_uncore_init(struct drm_device *dev)
>   {
>   	struct drm_i915_private *dev_priv = dev->dev_private;
> -
> -	setup_timer(&dev_priv->uncore.force_wake_timer,
> -		    gen6_force_wake_timer, (unsigned long)dev_priv);
> +	struct intel_uncore_forcewake_domain *domain;
> +	int i;
>   
>   	__intel_uncore_early_sanitize(dev, false);
>   
>   	if (IS_GEN9(dev)) {
>   		dev_priv->uncore.funcs.force_wake_get = __gen9_force_wake_get;
>   		dev_priv->uncore.funcs.force_wake_put = __gen9_force_wake_put;
> +		dev_priv->uncore.fw_domains = FORCEWAKE_RENDER |
> +			FORCEWAKE_BLITTER | FORCEWAKE_MEDIA;
>   	} else if (IS_VALLEYVIEW(dev)) {
>   		dev_priv->uncore.funcs.force_wake_get = __vlv_force_wake_get;
>   		dev_priv->uncore.funcs.force_wake_put = __vlv_force_wake_put;
> +		dev_priv->uncore.fw_domains = FORCEWAKE_RENDER | FORCEWAKE_MEDIA;
>   	} else if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
>   		dev_priv->uncore.funcs.force_wake_get = __gen7_gt_force_wake_mt_get;
>   		dev_priv->uncore.funcs.force_wake_put = __gen7_gt_force_wake_mt_put;
> +		dev_priv->uncore.fw_domains = FORCEWAKE_RENDER;
>   	} else if (IS_IVYBRIDGE(dev)) {
>   		u32 ecobus;
>   
> @@ -1183,11 +1053,21 @@ void intel_uncore_init(struct drm_device *dev)
>   			dev_priv->uncore.funcs.force_wake_put =
>   				__gen6_gt_force_wake_put;
>   		}
> +		dev_priv->uncore.fw_domains = FORCEWAKE_RENDER;
>   	} else if (IS_GEN6(dev)) {
>   		dev_priv->uncore.funcs.force_wake_get =
>   			__gen6_gt_force_wake_get;
>   		dev_priv->uncore.funcs.force_wake_put =
>   			__gen6_gt_force_wake_put;
> +		dev_priv->uncore.fw_domains = FORCEWAKE_RENDER;
> +	}
> +
> +	for_each_fw_domain(domain, dev_priv, i) {
> +		domain->i915 = dev_priv;
> +		domain->id = i;
> +
> +		setup_timer(&domain->timer, gen6_force_wake_timer,
> +			    (unsigned long)domain);
>   	}
>   
>   	switch (INTEL_INFO(dev)->gen) {




More information about the Intel-gfx mailing list