| Message ID | 1459788671-17501-2-git-send-email-tvrtko.ursulin@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Apr 04, 2016 at 05:51:10PM +0100, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > As the vast majority of users does not use the domain id variable, > we can eliminate it from the iterator and also change the latter > using the same principle as was recently done for for_each_engine. > > For a couple of callers which do need the domain id, or the domain > mask, store the later in the domain itself at init time and use > both through it. > > Result is clearer code and smaller generated binary, especially > in the tight fw get/put loops. Also, relationship between domain > id and mask is no longer assumed in the macro. > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Hah, I made exactly (admittedly I didn't think of adding BUILD_BUG_ON) the same patch just as you went off on holiday and weren't around to comment! Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> -Chris
On 04/04/16 17:51, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > As the vast majority of users does not use the domain id variable, "do not use" > we can eliminate it from the iterator and also change the latter > using the same principle as was recently done for for_each_engine. > > For a couple of callers which do need the domain id, or the domain > mask, store the later in the domain itself at init time and use > both through it. "For a couple of callers which do need the domain mask, store it in the domain array (which already has the domain id), then both can be retrieved thence." ? > Result is clearer code and smaller generated binary, especially > in the tight fw get/put loops. Also, relationship between domain > id and mask is no longer assumed in the macro. "in the macro or elsewhere" ? > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> A few typos & clarifications above, plus a minor quibble about a macro name (which you haven't actually changed, but might as well fix). Otherwise LGTM, so Reviewed-by: Dave Gordon <david.s.gordon@intel.com> (even if you don't change the macro name) > --- > drivers/gpu/drm/i915/i915_debugfs.c | 5 ++--- > drivers/gpu/drm/i915/i915_drv.h | 17 +++++++------- > drivers/gpu/drm/i915/intel_uncore.c | 45 +++++++++++++++++-------------------- > 3 files changed, 32 insertions(+), 35 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c > index a2e3af02c292..06fce014d0b4 100644 > --- a/drivers/gpu/drm/i915/i915_debugfs.c > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > @@ -1464,12 +1464,11 @@ static int i915_forcewake_domains(struct seq_file *m, void *data) > struct drm_device *dev = node->minor->dev; > struct drm_i915_private *dev_priv = dev->dev_private; > struct intel_uncore_forcewake_domain *fw_domain; > - int i; > > spin_lock_irq(&dev_priv->uncore.lock); > - for_each_fw_domain(fw_domain, dev_priv, i) { > + for_each_fw_domain(fw_domain, dev_priv) { > seq_printf(m, "%s.wake_count = %u\n", > - intel_uncore_forcewake_domain_to_str(i), > + intel_uncore_forcewake_domain_to_str(fw_domain->id), > fw_domain->wake_count); > } > spin_unlock_irq(&dev_priv->uncore.lock); > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 7d4c704d7d75..160f980f0368 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -665,6 +665,7 @@ struct intel_uncore { > struct intel_uncore_forcewake_domain { > struct drm_i915_private *i915; > enum forcewake_domain_id id; > + enum forcewake_domains mask; At present this mask will always have only one bit set, but I suppose there might be some utility in allowing multiple bits (virtual domains?) > unsigned wake_count; > struct hrtimer timer; > i915_reg_t reg_set; > @@ -679,14 +680,14 @@ struct intel_uncore { > }; > > /* 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__]) \ > - for_each_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 for_each_fw_domain_mask(domain__, mask__, dev_priv__) \ > + for ((domain__) = &(dev_priv__)->uncore.fw_domain[0]; \ > + (domain__) < &(dev_priv__)->uncore.fw_domain[FW_DOMAIN_ID_COUNT]; \ > + (domain__)++) \ > + for_each_if ((mask__) & (domain__)->mask) > + For consistency with the for_each_engine() macros, the name ought to be "for_each_fw_domain_mask*ed*()", showing that we're iterating over a subset selected by the 'mask' parameter, rather than iterating over all possible masks. .Dave. > +#define for_each_fw_domain(domain__, dev_priv__) \ > + for_each_fw_domain_mask(domain__, FORCEWAKE_ALL, dev_priv__) > > #define CSR_VERSION(major, minor) ((major) << 16 | (minor)) > #define CSR_VERSION_MAJOR(version) ((version) >> 16) > diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c > index 76ac990de354..49edd641b434 100644 > --- a/drivers/gpu/drm/i915/intel_uncore.c > +++ b/drivers/gpu/drm/i915/intel_uncore.c > @@ -111,9 +111,8 @@ static void > fw_domains_get(struct drm_i915_private *dev_priv, enum forcewake_domains fw_domains) > { > struct intel_uncore_forcewake_domain *d; > - enum forcewake_domain_id id; > > - for_each_fw_domain_mask(d, fw_domains, dev_priv, id) { > + for_each_fw_domain_mask(d, fw_domains, dev_priv) { > fw_domain_wait_ack_clear(d); > fw_domain_get(d); > fw_domain_wait_ack(d); > @@ -124,9 +123,8 @@ static void > fw_domains_put(struct drm_i915_private *dev_priv, enum forcewake_domains fw_domains) > { > struct intel_uncore_forcewake_domain *d; > - enum forcewake_domain_id id; > > - for_each_fw_domain_mask(d, fw_domains, dev_priv, id) { > + for_each_fw_domain_mask(d, fw_domains, dev_priv) { > fw_domain_put(d); > fw_domain_posting_read(d); > } > @@ -136,10 +134,9 @@ static void > fw_domains_posting_read(struct drm_i915_private *dev_priv) > { > struct intel_uncore_forcewake_domain *d; > - enum forcewake_domain_id id; > > /* No need to do for all, just do for first found */ > - for_each_fw_domain(d, dev_priv, id) { > + for_each_fw_domain(d, dev_priv) { > fw_domain_posting_read(d); > break; > } > @@ -149,12 +146,11 @@ static void > fw_domains_reset(struct drm_i915_private *dev_priv, enum forcewake_domains fw_domains) > { > struct intel_uncore_forcewake_domain *d; > - enum forcewake_domain_id id; > > if (dev_priv->uncore.fw_domains == 0) > return; > > - for_each_fw_domain_mask(d, fw_domains, dev_priv, id) > + for_each_fw_domain_mask(d, fw_domains, dev_priv) > fw_domain_reset(d); > > fw_domains_posting_read(dev_priv); > @@ -256,7 +252,6 @@ void intel_uncore_forcewake_reset(struct drm_device *dev, bool restore) > unsigned long irqflags; > struct intel_uncore_forcewake_domain *domain; > int retry_count = 100; > - enum forcewake_domain_id id; > enum forcewake_domains fw = 0, active_domains; > > /* Hold uncore.lock across reset to prevent any register access > @@ -266,7 +261,7 @@ void intel_uncore_forcewake_reset(struct drm_device *dev, bool restore) > while (1) { > active_domains = 0; > > - for_each_fw_domain(domain, dev_priv, id) { > + for_each_fw_domain(domain, dev_priv) { > if (hrtimer_cancel(&domain->timer) == 0) > continue; > > @@ -275,9 +270,9 @@ void intel_uncore_forcewake_reset(struct drm_device *dev, bool restore) > > spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); > > - for_each_fw_domain(domain, dev_priv, id) { > + for_each_fw_domain(domain, dev_priv) { > if (hrtimer_active(&domain->timer)) > - active_domains |= (1 << id); > + active_domains |= domain->mask; > } > > if (active_domains == 0) > @@ -294,9 +289,9 @@ void intel_uncore_forcewake_reset(struct drm_device *dev, bool restore) > > WARN_ON(active_domains); > > - for_each_fw_domain(domain, dev_priv, id) > + for_each_fw_domain(domain, dev_priv) > if (domain->wake_count) > - fw |= 1 << id; > + fw |= domain->mask; > > if (fw) > dev_priv->uncore.funcs.force_wake_put(dev_priv, fw); > @@ -418,16 +413,15 @@ static void __intel_uncore_forcewake_get(struct drm_i915_private *dev_priv, > enum forcewake_domains fw_domains) > { > struct intel_uncore_forcewake_domain *domain; > - enum forcewake_domain_id id; > > if (!dev_priv->uncore.funcs.force_wake_get) > return; > > fw_domains &= dev_priv->uncore.fw_domains; > > - for_each_fw_domain_mask(domain, fw_domains, dev_priv, id) { > + for_each_fw_domain_mask(domain, fw_domains, dev_priv) { > if (domain->wake_count++) > - fw_domains &= ~(1 << id); > + fw_domains &= ~domain->mask; > } > > if (fw_domains) > @@ -485,14 +479,13 @@ static void __intel_uncore_forcewake_put(struct drm_i915_private *dev_priv, > enum forcewake_domains fw_domains) > { > struct intel_uncore_forcewake_domain *domain; > - enum forcewake_domain_id id; > > if (!dev_priv->uncore.funcs.force_wake_put) > return; > > fw_domains &= dev_priv->uncore.fw_domains; > > - for_each_fw_domain_mask(domain, fw_domains, dev_priv, id) { > + for_each_fw_domain_mask(domain, fw_domains, dev_priv) { > if (WARN_ON(domain->wake_count == 0)) > continue; > > @@ -546,12 +539,11 @@ void intel_uncore_forcewake_put__locked(struct drm_i915_private *dev_priv, > void assert_forcewakes_inactive(struct drm_i915_private *dev_priv) > { > struct intel_uncore_forcewake_domain *domain; > - enum forcewake_domain_id id; > > if (!dev_priv->uncore.funcs.force_wake_get) > return; > > - for_each_fw_domain(domain, dev_priv, id) > + for_each_fw_domain(domain, dev_priv) > WARN_ON(domain->wake_count); > } > > @@ -727,15 +719,14 @@ static inline void __force_wake_auto(struct drm_i915_private *dev_priv, > enum forcewake_domains fw_domains) > { > struct intel_uncore_forcewake_domain *domain; > - enum forcewake_domain_id id; > > 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, id) { > + for_each_fw_domain_mask(domain, fw_domains, dev_priv) { > if (domain->wake_count) { > - fw_domains &= ~(1 << id); > + fw_domains &= ~domain->mask; > continue; > } > > @@ -1156,6 +1147,12 @@ static void fw_domain_init(struct drm_i915_private *dev_priv, > d->i915 = dev_priv; > d->id = domain_id; > > + BUILD_BUG_ON(FORCEWAKE_RENDER != (1 << FW_DOMAIN_ID_RENDER)); > + BUILD_BUG_ON(FORCEWAKE_BLITTER != (1 << FW_DOMAIN_ID_BLITTER)); > + BUILD_BUG_ON(FORCEWAKE_MEDIA != (1 << FW_DOMAIN_ID_MEDIA)); > + > + d->mask = 1 << domain_id; > + > hrtimer_init(&d->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL); > d->timer.function = intel_uncore_fw_release_timer; > >
On 04/04/16 20:14, Dave Gordon wrote: > On 04/04/16 17:51, Tvrtko Ursulin wrote: >> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >> >> As the vast majority of users does not use the domain id variable, > > "do not use" Yep. > >> we can eliminate it from the iterator and also change the latter >> using the same principle as was recently done for for_each_engine. >> >> For a couple of callers which do need the domain id, or the domain >> mask, store the later in the domain itself at init time and use >> both through it. > > "For a couple of callers which do need the domain mask, store it > in the domain array (which already has the domain id), then both > can be retrieved thence." ? Better yes. >> Result is clearer code and smaller generated binary, especially >> in the tight fw get/put loops. Also, relationship between domain >> id and mask is no longer assumed in the macro. > > "in the macro or elsewhere" ? Just in the macro, it is still hardcoded in fw_domain_init. >> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > A few typos & clarifications above, plus a minor quibble about a macro > name (which you haven't actually changed, but might as well fix). > Otherwise LGTM, so > > Reviewed-by: Dave Gordon <david.s.gordon@intel.com> > > (even if you don't change the macro name) Thanks, consistency sounds good so I will change it. >> --- >> drivers/gpu/drm/i915/i915_debugfs.c | 5 ++--- >> drivers/gpu/drm/i915/i915_drv.h | 17 +++++++------- >> drivers/gpu/drm/i915/intel_uncore.c | 45 >> +++++++++++++++++-------------------- >> 3 files changed, 32 insertions(+), 35 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c >> b/drivers/gpu/drm/i915/i915_debugfs.c >> index a2e3af02c292..06fce014d0b4 100644 >> --- a/drivers/gpu/drm/i915/i915_debugfs.c >> +++ b/drivers/gpu/drm/i915/i915_debugfs.c >> @@ -1464,12 +1464,11 @@ static int i915_forcewake_domains(struct >> seq_file *m, void *data) >> struct drm_device *dev = node->minor->dev; >> struct drm_i915_private *dev_priv = dev->dev_private; >> struct intel_uncore_forcewake_domain *fw_domain; >> - int i; >> >> spin_lock_irq(&dev_priv->uncore.lock); >> - for_each_fw_domain(fw_domain, dev_priv, i) { >> + for_each_fw_domain(fw_domain, dev_priv) { >> seq_printf(m, "%s.wake_count = %u\n", >> - intel_uncore_forcewake_domain_to_str(i), >> + intel_uncore_forcewake_domain_to_str(fw_domain->id), >> fw_domain->wake_count); >> } >> spin_unlock_irq(&dev_priv->uncore.lock); >> diff --git a/drivers/gpu/drm/i915/i915_drv.h >> b/drivers/gpu/drm/i915/i915_drv.h >> index 7d4c704d7d75..160f980f0368 100644 >> --- a/drivers/gpu/drm/i915/i915_drv.h >> +++ b/drivers/gpu/drm/i915/i915_drv.h >> @@ -665,6 +665,7 @@ struct intel_uncore { >> struct intel_uncore_forcewake_domain { >> struct drm_i915_private *i915; >> enum forcewake_domain_id id; >> + enum forcewake_domains mask; > > At present this mask will always have only one bit set, but I suppose > there might be some utility in allowing multiple bits (virtual domains?) I did not like the name mask myself but couldn't think of anything better. Do you have a suggestion? Regards, Tvrtko
On Tue, Apr 05, 2016 at 10:05:24AM +0100, Tvrtko Ursulin wrote: > On 04/04/16 20:14, Dave Gordon wrote: > >On 04/04/16 17:51, Tvrtko Ursulin wrote: > >>diff --git a/drivers/gpu/drm/i915/i915_drv.h > >>b/drivers/gpu/drm/i915/i915_drv.h > >>index 7d4c704d7d75..160f980f0368 100644 > >>--- a/drivers/gpu/drm/i915/i915_drv.h > >>+++ b/drivers/gpu/drm/i915/i915_drv.h > >>@@ -665,6 +665,7 @@ struct intel_uncore { > >> struct intel_uncore_forcewake_domain { > >> struct drm_i915_private *i915; > >> enum forcewake_domain_id id; > >>+ enum forcewake_domains mask; > > > >At present this mask will always have only one bit set, but I suppose > >there might be some utility in allowing multiple bits (virtual domains?) > > I did not like the name mask myself but couldn't think of anything > better. Do you have a suggestion? mask is fine as we use it as a mask. We've called it intel_engine_flag() which is worse imo. If there is a much better name for this, we should also fixup that name as well. -Chris
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index a2e3af02c292..06fce014d0b4 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -1464,12 +1464,11 @@ static int i915_forcewake_domains(struct seq_file *m, void *data) struct drm_device *dev = node->minor->dev; struct drm_i915_private *dev_priv = dev->dev_private; struct intel_uncore_forcewake_domain *fw_domain; - int i; spin_lock_irq(&dev_priv->uncore.lock); - for_each_fw_domain(fw_domain, dev_priv, i) { + for_each_fw_domain(fw_domain, dev_priv) { seq_printf(m, "%s.wake_count = %u\n", - intel_uncore_forcewake_domain_to_str(i), + intel_uncore_forcewake_domain_to_str(fw_domain->id), fw_domain->wake_count); } spin_unlock_irq(&dev_priv->uncore.lock); diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 7d4c704d7d75..160f980f0368 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -665,6 +665,7 @@ struct intel_uncore { struct intel_uncore_forcewake_domain { struct drm_i915_private *i915; enum forcewake_domain_id id; + enum forcewake_domains mask; unsigned wake_count; struct hrtimer timer; i915_reg_t reg_set; @@ -679,14 +680,14 @@ struct intel_uncore { }; /* 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__]) \ - for_each_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 for_each_fw_domain_mask(domain__, mask__, dev_priv__) \ + for ((domain__) = &(dev_priv__)->uncore.fw_domain[0]; \ + (domain__) < &(dev_priv__)->uncore.fw_domain[FW_DOMAIN_ID_COUNT]; \ + (domain__)++) \ + for_each_if ((mask__) & (domain__)->mask) + +#define for_each_fw_domain(domain__, dev_priv__) \ + for_each_fw_domain_mask(domain__, FORCEWAKE_ALL, dev_priv__) #define CSR_VERSION(major, minor) ((major) << 16 | (minor)) #define CSR_VERSION_MAJOR(version) ((version) >> 16) diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c index 76ac990de354..49edd641b434 100644 --- a/drivers/gpu/drm/i915/intel_uncore.c +++ b/drivers/gpu/drm/i915/intel_uncore.c @@ -111,9 +111,8 @@ static void fw_domains_get(struct drm_i915_private *dev_priv, enum forcewake_domains fw_domains) { struct intel_uncore_forcewake_domain *d; - enum forcewake_domain_id id; - for_each_fw_domain_mask(d, fw_domains, dev_priv, id) { + for_each_fw_domain_mask(d, fw_domains, dev_priv) { fw_domain_wait_ack_clear(d); fw_domain_get(d); fw_domain_wait_ack(d); @@ -124,9 +123,8 @@ static void fw_domains_put(struct drm_i915_private *dev_priv, enum forcewake_domains fw_domains) { struct intel_uncore_forcewake_domain *d; - enum forcewake_domain_id id; - for_each_fw_domain_mask(d, fw_domains, dev_priv, id) { + for_each_fw_domain_mask(d, fw_domains, dev_priv) { fw_domain_put(d); fw_domain_posting_read(d); } @@ -136,10 +134,9 @@ static void fw_domains_posting_read(struct drm_i915_private *dev_priv) { struct intel_uncore_forcewake_domain *d; - enum forcewake_domain_id id; /* No need to do for all, just do for first found */ - for_each_fw_domain(d, dev_priv, id) { + for_each_fw_domain(d, dev_priv) { fw_domain_posting_read(d); break; } @@ -149,12 +146,11 @@ static void fw_domains_reset(struct drm_i915_private *dev_priv, enum forcewake_domains fw_domains) { struct intel_uncore_forcewake_domain *d; - enum forcewake_domain_id id; if (dev_priv->uncore.fw_domains == 0) return; - for_each_fw_domain_mask(d, fw_domains, dev_priv, id) + for_each_fw_domain_mask(d, fw_domains, dev_priv) fw_domain_reset(d); fw_domains_posting_read(dev_priv); @@ -256,7 +252,6 @@ void intel_uncore_forcewake_reset(struct drm_device *dev, bool restore) unsigned long irqflags; struct intel_uncore_forcewake_domain *domain; int retry_count = 100; - enum forcewake_domain_id id; enum forcewake_domains fw = 0, active_domains; /* Hold uncore.lock across reset to prevent any register access @@ -266,7 +261,7 @@ void intel_uncore_forcewake_reset(struct drm_device *dev, bool restore) while (1) { active_domains = 0; - for_each_fw_domain(domain, dev_priv, id) { + for_each_fw_domain(domain, dev_priv) { if (hrtimer_cancel(&domain->timer) == 0) continue; @@ -275,9 +270,9 @@ void intel_uncore_forcewake_reset(struct drm_device *dev, bool restore) spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); - for_each_fw_domain(domain, dev_priv, id) { + for_each_fw_domain(domain, dev_priv) { if (hrtimer_active(&domain->timer)) - active_domains |= (1 << id); + active_domains |= domain->mask; } if (active_domains == 0) @@ -294,9 +289,9 @@ void intel_uncore_forcewake_reset(struct drm_device *dev, bool restore) WARN_ON(active_domains); - for_each_fw_domain(domain, dev_priv, id) + for_each_fw_domain(domain, dev_priv) if (domain->wake_count) - fw |= 1 << id; + fw |= domain->mask; if (fw) dev_priv->uncore.funcs.force_wake_put(dev_priv, fw); @@ -418,16 +413,15 @@ static void __intel_uncore_forcewake_get(struct drm_i915_private *dev_priv, enum forcewake_domains fw_domains) { struct intel_uncore_forcewake_domain *domain; - enum forcewake_domain_id id; if (!dev_priv->uncore.funcs.force_wake_get) return; fw_domains &= dev_priv->uncore.fw_domains; - for_each_fw_domain_mask(domain, fw_domains, dev_priv, id) { + for_each_fw_domain_mask(domain, fw_domains, dev_priv) { if (domain->wake_count++) - fw_domains &= ~(1 << id); + fw_domains &= ~domain->mask; } if (fw_domains) @@ -485,14 +479,13 @@ static void __intel_uncore_forcewake_put(struct drm_i915_private *dev_priv, enum forcewake_domains fw_domains) { struct intel_uncore_forcewake_domain *domain; - enum forcewake_domain_id id; if (!dev_priv->uncore.funcs.force_wake_put) return; fw_domains &= dev_priv->uncore.fw_domains; - for_each_fw_domain_mask(domain, fw_domains, dev_priv, id) { + for_each_fw_domain_mask(domain, fw_domains, dev_priv) { if (WARN_ON(domain->wake_count == 0)) continue; @@ -546,12 +539,11 @@ void intel_uncore_forcewake_put__locked(struct drm_i915_private *dev_priv, void assert_forcewakes_inactive(struct drm_i915_private *dev_priv) { struct intel_uncore_forcewake_domain *domain; - enum forcewake_domain_id id; if (!dev_priv->uncore.funcs.force_wake_get) return; - for_each_fw_domain(domain, dev_priv, id) + for_each_fw_domain(domain, dev_priv) WARN_ON(domain->wake_count); } @@ -727,15 +719,14 @@ static inline void __force_wake_auto(struct drm_i915_private *dev_priv, enum forcewake_domains fw_domains) { struct intel_uncore_forcewake_domain *domain; - enum forcewake_domain_id id; 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, id) { + for_each_fw_domain_mask(domain, fw_domains, dev_priv) { if (domain->wake_count) { - fw_domains &= ~(1 << id); + fw_domains &= ~domain->mask; continue; } @@ -1156,6 +1147,12 @@ static void fw_domain_init(struct drm_i915_private *dev_priv, d->i915 = dev_priv; d->id = domain_id; + BUILD_BUG_ON(FORCEWAKE_RENDER != (1 << FW_DOMAIN_ID_RENDER)); + BUILD_BUG_ON(FORCEWAKE_BLITTER != (1 << FW_DOMAIN_ID_BLITTER)); + BUILD_BUG_ON(FORCEWAKE_MEDIA != (1 << FW_DOMAIN_ID_MEDIA)); + + d->mask = 1 << domain_id; + hrtimer_init(&d->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL); d->timer.function = intel_uncore_fw_release_timer;