From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tvrtko Ursulin Subject: Re: [PATCH] drm/i915: Reduce duplicated forcewake logic Date: Wed, 01 Oct 2014 16:53:07 +0100 Message-ID: <542C2363.1080603@linux.intel.com> References: <20140910155116.GD31074@nuc-i3427.alporthouse.com> <1410374094-5210-1-git-send-email-chris@chris-wilson.co.uk> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by gabe.freedesktop.org (Postfix) with ESMTP id A964E6E23D for ; Wed, 1 Oct 2014 08:53:09 -0700 (PDT) In-Reply-To: <1410374094-5210-1-git-send-email-chris@chris-wilson.co.uk> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Chris Wilson , intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On 09/10/2014 07:34 PM, 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(-) As said yesterday I like the idea, some comments and questions inline. > 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[] = { > + "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; What do you think about something like for_each_forcewake_domain (and perhaps for_each_possible_forcewake_domain) for readability since there is a fair number of these loops? > - 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 > +}; Would there be any need to attempt hiding these somehow so people don't try to pass these instead of FORCEWAKE_... ones into relevant functions? No idea how though. Maybe gcc will complain if passing in enum into int? > + > 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; Should this be named dev_priv for consistency? > + 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) > { What's up with the above series of indentation changes? > - > /* 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) Is gen6_ still the best prefix for these two? Should you also use the opportunity to add kerneldoc now that Daniel is making a bit push in that direction? > { > unsigned long irqflags; > + int i; Not sure if it makes any difference nowadays to use unsigned for loops like this. > > 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; > > 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; > + > + dev_priv->uncore.fw_domain[i].wake_count++; I would put a comment here about why we are incrementing after decrementing. Possibly also what is the purpose of the timer if not already documented somewhere. > + 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) This function is to be used by what type of callers compared to gen6_gt_force_wake_get? > +{ > + 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; > + } > + > + 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); \ > - } \ > + 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); \ This changes from immediate to delayed put - would it make sense to move it into a follow up patch? > 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) { > Regards, Tvrtko