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

Ville Syrjälä ville.syrjala at linux.intel.com
Fri Nov 7 16:46:55 CET 2014


On Wed, Sep 10, 2014 at 07:34:54PM +0100, Chris Wilson wrote:
> Introduce a structure to track the individual forcewake domains and use
> that to eliminated duplicate logic.
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c |  41 +++---
>  drivers/gpu/drm/i915/i915_drv.h     |  32 +++--
>  drivers/gpu/drm/i915/intel_uncore.c | 265 ++++++++++++++++--------------------
>  3 files changed, 157 insertions(+), 181 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index a72d8b8..c3176f1 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1317,7 +1317,6 @@ static int vlv_drpc_info(struct seq_file *m)
>  	struct drm_device *dev = node->minor->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	u32 rpmodectl1, rcctl1;
> -	unsigned fw_rendercount = 0, fw_mediacount = 0;
>  
>  	intel_runtime_pm_get(dev_priv);
>  
> @@ -1350,22 +1349,12 @@ 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, data);
>  }
>  
>  
>  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;
> @@ -1379,7 +1368,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_RENDER].wake_count;
>  	spin_unlock_irq(&dev_priv->uncore.lock);
>  
>  	if (forcewake_count) {
> @@ -1976,21 +1965,23 @@ 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;
> +	const char *domain_names[] = {

static const char * const domain_names[] ?

Hmm. Maybe someone should fire up coccinelle to go through the entire
codebase with this...

> +		"render",
> +		"media",
> +	};
> +	int i;
>  
>  	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);
> +	for (i = 0; i < FW_DOMAIN_COUNT; i++) {
> +		if ((dev_priv->uncore.fw_domains & (1 << i)) == 0)
> +			continue;
>  
> -	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);
> +		seq_printf(m, "%s.wake_count = %u\n",
> +			   domain_names[i],
> +			   dev_priv->uncore.fw_domain[i].wake_count);
> +	}
> +
> +	spin_unlock_irq(&dev_priv->uncore.lock);
>  
>  	return 0;
>  }
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 5d0b38c..6424191 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -528,18 +528,30 @@ struct intel_uncore_funcs {
>  				uint64_t val, bool trace);
>  };
>  
> +enum {
> +	FW_DOMAIN_RENDER = 0,
> +	FW_DOMAIN_MEDIA,
> +
> +	FW_DOMAIN_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_domains;
>  
> -	struct timer_list force_wake_timer;
> +	struct intel_uncore_forcewake_domain {
> +		struct drm_i915_private *i915;
> +		int id;
> +		unsigned wake_count;
> +		struct timer_list timer;
> +	} fw_domain[FW_DOMAIN_COUNT];
> +#define FORCEWAKE_RENDER	(1 << FW_DOMAIN_RENDER)
> +#define FORCEWAKE_MEDIA		(1 << FW_DOMAIN_MEDIA)
> +#define FORCEWAKE_ALL		(FORCEWAKE_RENDER | FORCEWAKE_MEDIA)
>  };
>  
>  #define DEV_INFO_FOR_EACH_FLAG(func, sep) \
> @@ -2948,8 +2960,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, u8 mbox, u32 *val);
> @@ -2981,10 +2995,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_ALL		(FORCEWAKE_RENDER | FORCEWAKE_MEDIA)
> -
>  
>  #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 3b3d3e0..641950b 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -73,7 +73,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))
> @@ -99,7 +99,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;
>  
> @@ -136,7 +136,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 */
> @@ -145,7 +145,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));
> @@ -194,7 +194,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) {
> @@ -238,9 +238,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,
> @@ -258,59 +257,35 @@ 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 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;
> +	int i;
>  
> -	if (del_timer_sync(&dev_priv->uncore.force_wake_timer))
> -		gen6_force_wake_timer((unsigned long)dev_priv);
> +	for (i = 0; i < FW_DOMAIN_COUNT; i++ ) {
> +		if ((dev_priv->uncore.fw_domains & (1 << i)) == 0)
> +			continue;
> +
> +		if (del_timer_sync(&dev_priv->uncore.fw_domain[i].timer))
> +			gen6_force_wake_timer((unsigned long)&dev_priv->uncore.fw_domain[i]);
> +	}
>  
>  	/* Hold uncore.lock across reset to prevent any register access
>  	 * with forcewake not set correctly
> @@ -327,18 +302,12 @@ void intel_uncore_forcewake_reset(struct drm_device *dev, bool restore)
>  
>  	if (restore) { /* If reset with a user forcewake, try to restore */
>  		unsigned fw = 0;
> +		int i;
>  
> -		if (IS_VALLEYVIEW(dev)) {
> -			if (dev_priv->uncore.fw_rendercount)
> -				fw |= FORCEWAKE_RENDER;
> -
> -			if (dev_priv->uncore.fw_mediacount)
> -				fw |= FORCEWAKE_MEDIA;
> -		} else {
> -			if (dev_priv->uncore.forcewake_count)
> -				fw = FORCEWAKE_ALL;
> +		for (i = 0; i < FW_DOMAIN_COUNT; i++) {
> +			if (dev_priv->uncore.fw_domain[i].wake_count)
> +				fw |= 1 << i;
>  		}
> -
>  		if (fw)
>  			dev_priv->uncore.funcs.force_wake_get(dev_priv, fw);
>  
> @@ -389,50 +358,60 @@ 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;
> +	int i;
>  
>  	if (!dev_priv->uncore.funcs.force_wake_get)
>  		return;
>  
>  	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);
>  
> -	/* Redirect to VLV specific routine */
> -	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 (i = 0; i < FW_DOMAIN_COUNT; i++) {
> +		if ((fw_domains & (1 << i)) == 0)
> +			continue;
> +
> +		if (dev_priv->uncore.fw_domain[i].wake_count++)
> +			fw_domains &= ~(1 << i);
>  	}
> +
> +	if (fw_domains)
> +		dev_priv->uncore.funcs.force_wake_get(dev_priv, fw_domains);
> +
>  	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
>  }
>  
>  /*
>   * 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;
> +	int i;
>  
>  	if (!dev_priv->uncore.funcs.force_wake_put)
>  		return;

Aren't we missing this

fw_domains &= dev_priv->uncore.fw_domains;

here? I would expect to see the WARN triggering below due to
wake_count==0 when the domain doesn't exist.

>  
>  	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
>  
> -	/* Redirect to VLV specific routine */
> -	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 (i = 0; i < FW_DOMAIN_COUNT; i++) {
> +		if ((fw_domains & (1 << i)) == 0)
> +			continue;
> +
> +		WARN_ON(!dev_priv->uncore.fw_domain[i].wake_count);
> +		if (--dev_priv->uncore.fw_domain[i].wake_count)
> +			continue;
> +

Maybe WARN_ON(timer_pending()) here?

> +		dev_priv->uncore.fw_domain[i].wake_count++;
> +		mod_timer_pinned(&dev_priv->uncore.fw_domain[i].timer,
> +				 jiffies + 1);
>  	}
>  
>  	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
> @@ -441,10 +420,13 @@ 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)
>  {
> +	int i;
> +
>  	if (!dev_priv->uncore.funcs.force_wake_get)
>  		return;
>  
> -	WARN_ON(dev_priv->uncore.forcewake_count > 0);
> +	for (i = 0; i < FW_DOMAIN_COUNT; i++)
> +		WARN_ON(dev_priv->uncore.fw_domain[i].wake_count > 0);
>  }
>  
>  /* We give fast paths for the really cool registers */
> @@ -578,19 +560,38 @@ __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)
> +{
> +	int i;
> +
> +	/* Ideally GCC would be constant-fold and eliminate this loop */
> +
> +	for (i = 0; i < FW_DOMAIN_COUNT; i++) {
> +		if ((fw_domains & (1 << i)) == 0)
> +			continue;
> +
> +		if (dev_priv->uncore.fw_domain[i].wake_count) {
> +			fw_domains &= ~(1 << i);
> +			continue;
> +		}
> +

And here?

I think I had some concerns about the wake_count vs. timer handling when
I saw the first version of this patch, but now that I look at this I
can't see anything wrong with it.

> +		dev_priv->uncore.fw_domain[i].wake_count++;
> +		mod_timer_pinned(&dev_priv->uncore.fw_domain[i].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); \

This seems to be against some other tree where the timer is already used
for register accesses. That's not the case in nightly AFAICS.

I'm a bit worried about making the timer change at the same time as the
code refactoring. I would suggest refactoring first, and tossing in another
patch on top which expands the use of the timer to include register
access. That way we could at least revert the timer part if necessary
without losing the much cleaner code.

Also the code in nightly still has the runtime pm stuff in the forcewake
code. I guess you cleaned those out in your tree, but I don't remeber
seeing a recent patch on that front.

Otherwise this lookd very nice IMO, so would be great to get it in
finally.

> -	} \
> +	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; \
> @@ -599,45 +600,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; \
>  }
>  
> @@ -766,17 +749,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; \
> @@ -785,28 +760,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; \
>  }
>  
> @@ -837,18 +801,18 @@ __gen6_write(64)
>  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);
> +	int i;
>  
>  	intel_uncore_early_sanitize(dev, false);
>  
>  	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;
>  
> @@ -880,11 +844,22 @@ 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 (i = 0; i < FW_DOMAIN_COUNT; i++) {
> +		dev_priv->uncore.fw_domain[i].i915 = dev_priv;
> +		dev_priv->uncore.fw_domain[i].id = i;
> +
> +		setup_timer(&dev_priv->uncore.fw_domain[i].timer,
> +			    gen6_force_wake_timer,
> +			    (unsigned long)&dev_priv->uncore.fw_domain[i]);
>  	}
>  
>  	switch (INTEL_INFO(dev)->gen) {
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC



More information about the Intel-gfx mailing list