[Intel-gfx] [PATCH 2/3] drm/i915: Simplify for_each_fw_domain iterators

Tvrtko Ursulin tvrtko.ursulin at linux.intel.com
Mon Apr 4 16:51:10 UTC 2016


From: Tvrtko Ursulin <tvrtko.ursulin at 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 at intel.com>
---
 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;
 		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;
 
-- 
1.9.1



More information about the Intel-gfx mailing list