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

Mika Kuoppala mika.kuoppala at linux.intel.com
Tue Dec 16 07:26:17 PST 2014


Deepak S <deepak.s at linux.intel.com> writes:

> 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?

1. We read the register, wake_count = 1 and domain timer is armed.
2. In reset we see which domains are awake and store them in fw
3. We do a put for all domains with the hw callback,
   this doesn't touch the wake_count. I wanted this so that
   we first try with the soft approach by doing the disable and
   all the related fifodbg dance here like this was a normal put.
4. Regardless if the above did the trick, we then we take the gloves
   off and use more blunt approach by resetting the register.
5. We restore all the forcewakes

But while I was reading this through, there is one other
problem which is that if we don't restore, we mess the counts
and the timers will be still running.

I posted a new series with a fix for this.

-Mika

>
>
> 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) {
>
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx


More information about the Intel-gfx mailing list