[PATCH v2 1/5] drm/i915: Eliminate per-fw_domain i915 backpointer

[PATCH v2 1/5] drm/i915: Eliminate per-fw_domain i915 backpointer

[Chris Wilson]

Pass along the drm_i915_private pointer from the caller, rather than
looking it up from each fw_domain during fw_domains_get/_put. This
allows us to then eliminate the backpointer, in exchange for a more
complicated unwrapping procedure in the rare
intel_uncore_fw_release_timer().

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h     | 33 ++++++++--------
 drivers/gpu/drm/i915/intel_uncore.c | 77 ++++++++++++++++++++-----------------
 2 files changed, 60 insertions(+), 50 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index a5947a496d0a..4c9de7d00eaf 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -732,21 +732,25 @@ intel_uncore_forcewake_for_reg(struct drm_i915_private *dev_priv,
 
 struct intel_uncore_funcs {
 	void (*force_wake_get)(struct drm_i915_private *dev_priv,
-							enum forcewake_domains domains);
+			       enum forcewake_domains domains);
 	void (*force_wake_put)(struct drm_i915_private *dev_priv,
-							enum forcewake_domains domains);
-
-	uint8_t  (*mmio_readb)(struct drm_i915_private *dev_priv, i915_reg_t r, bool trace);
-	uint16_t (*mmio_readw)(struct drm_i915_private *dev_priv, i915_reg_t r, bool trace);
-	uint32_t (*mmio_readl)(struct drm_i915_private *dev_priv, i915_reg_t r, bool trace);
-	uint64_t (*mmio_readq)(struct drm_i915_private *dev_priv, i915_reg_t r, bool trace);
-
-	void (*mmio_writeb)(struct drm_i915_private *dev_priv, i915_reg_t r,
-				uint8_t val, bool trace);
-	void (*mmio_writew)(struct drm_i915_private *dev_priv, i915_reg_t r,
-				uint16_t val, bool trace);
-	void (*mmio_writel)(struct drm_i915_private *dev_priv, i915_reg_t r,
-				uint32_t val, bool trace);
+			       enum forcewake_domains domains);
+
+	uint8_t  (*mmio_readb)(struct drm_i915_private *dev_priv,
+			       i915_reg_t r, bool trace);
+	uint16_t (*mmio_readw)(struct drm_i915_private *dev_priv,
+			       i915_reg_t r, bool trace);
+	uint32_t (*mmio_readl)(struct drm_i915_private *dev_priv,
+			       i915_reg_t r, bool trace);
+	uint64_t (*mmio_readq)(struct drm_i915_private *dev_priv,
+			       i915_reg_t r, bool trace);
+
+	void (*mmio_writeb)(struct drm_i915_private *dev_priv,
+			    i915_reg_t r, uint8_t val, bool trace);
+	void (*mmio_writew)(struct drm_i915_private *dev_priv,
+			    i915_reg_t r, uint16_t val, bool trace);
+	void (*mmio_writel)(struct drm_i915_private *dev_priv,
+			    i915_reg_t r, uint32_t val, bool trace);
 };
 
 struct intel_forcewake_range {
@@ -771,7 +775,6 @@ struct intel_uncore {
 	enum forcewake_domains fw_domains_active;
 
 	struct intel_uncore_forcewake_domain {
-		struct drm_i915_private *i915;
 		enum forcewake_domain_id id;
 		enum forcewake_domains mask;
 		unsigned wake_count;
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index 09f5f02d7901..aa2e7401efdc 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -52,10 +52,11 @@ intel_uncore_forcewake_domain_to_str(const enum forcewake_domain_id id)
 }
 
 static inline void
-fw_domain_reset(const struct intel_uncore_forcewake_domain *d)
+fw_domain_reset(struct drm_i915_private *i915,
+		const struct intel_uncore_forcewake_domain *d)
 {
 	WARN_ON(!i915_mmio_reg_valid(d->reg_set));
-	__raw_i915_write32(d->i915, d->reg_set, d->val_reset);
+	__raw_i915_write32(i915, d->reg_set, d->val_reset);
 }
 
 static inline void
@@ -69,9 +70,10 @@ fw_domain_arm_timer(struct intel_uncore_forcewake_domain *d)
 }
 
 static inline void
-fw_domain_wait_ack_clear(const struct intel_uncore_forcewake_domain *d)
+fw_domain_wait_ack_clear(struct drm_i915_private *i915,
+			 const struct intel_uncore_forcewake_domain *d)
 {
-	if (wait_for_atomic((__raw_i915_read32(d->i915, d->reg_ack) &
+	if (wait_for_atomic((__raw_i915_read32(i915, d->reg_ack) &
 			     FORCEWAKE_KERNEL) == 0,
 			    FORCEWAKE_ACK_TIMEOUT_MS))
 		DRM_ERROR("%s: timed out waiting for forcewake ack to clear.\n",
@@ -79,15 +81,17 @@ fw_domain_wait_ack_clear(const struct intel_uncore_forcewake_domain *d)
 }
 
 static inline void
-fw_domain_get(const struct intel_uncore_forcewake_domain *d)
+fw_domain_get(struct drm_i915_private *i915,
+	      const struct intel_uncore_forcewake_domain *d)
 {
-	__raw_i915_write32(d->i915, d->reg_set, d->val_set);
+	__raw_i915_write32(i915, d->reg_set, d->val_set);
 }
 
 static inline void
-fw_domain_wait_ack(const struct intel_uncore_forcewake_domain *d)
+fw_domain_wait_ack(struct drm_i915_private *i915,
+		   const struct intel_uncore_forcewake_domain *d)
 {
-	if (wait_for_atomic((__raw_i915_read32(d->i915, d->reg_ack) &
+	if (wait_for_atomic((__raw_i915_read32(i915, d->reg_ack) &
 			     FORCEWAKE_KERNEL),
 			    FORCEWAKE_ACK_TIMEOUT_MS))
 		DRM_ERROR("%s: timed out waiting for forcewake ack request.\n",
@@ -95,72 +99,75 @@ fw_domain_wait_ack(const struct intel_uncore_forcewake_domain *d)
 }
 
 static inline void
-fw_domain_put(const struct intel_uncore_forcewake_domain *d)
+fw_domain_put(struct drm_i915_private *i915,
+	      const struct intel_uncore_forcewake_domain *d)
 {
-	__raw_i915_write32(d->i915, d->reg_set, d->val_clear);
+	__raw_i915_write32(i915, d->reg_set, d->val_clear);
 }
 
 static inline void
-fw_domain_posting_read(const struct intel_uncore_forcewake_domain *d)
+fw_domain_posting_read(struct drm_i915_private *i915,
+		       const struct intel_uncore_forcewake_domain *d)
 {
 	/* something from same cacheline, but not from the set register */
 	if (i915_mmio_reg_valid(d->reg_post))
-		__raw_posting_read(d->i915, d->reg_post);
+		__raw_posting_read(i915, d->reg_post);
 }
 
 static void
-fw_domains_get(struct drm_i915_private *dev_priv, enum forcewake_domains fw_domains)
+fw_domains_get(struct drm_i915_private *i915, enum forcewake_domains fw_domains)
 {
 	struct intel_uncore_forcewake_domain *d;
 
-	for_each_fw_domain_masked(d, fw_domains, dev_priv) {
-		fw_domain_wait_ack_clear(d);
-		fw_domain_get(d);
+	for_each_fw_domain_masked(d, fw_domains, i915) {
+		fw_domain_wait_ack_clear(i915, d);
+		fw_domain_get(i915, d);
 	}
 
-	for_each_fw_domain_masked(d, fw_domains, dev_priv)
-		fw_domain_wait_ack(d);
+	for_each_fw_domain_masked(d, fw_domains, i915)
+		fw_domain_wait_ack(i915, d);
 
-	dev_priv->uncore.fw_domains_active |= fw_domains;
+	i915->uncore.fw_domains_active |= fw_domains;
 }
 
 static void
-fw_domains_put(struct drm_i915_private *dev_priv, enum forcewake_domains fw_domains)
+fw_domains_put(struct drm_i915_private *i915, enum forcewake_domains fw_domains)
 {
 	struct intel_uncore_forcewake_domain *d;
 
-	for_each_fw_domain_masked(d, fw_domains, dev_priv) {
-		fw_domain_put(d);
-		fw_domain_posting_read(d);
+	for_each_fw_domain_masked(d, fw_domains, i915) {
+		fw_domain_put(i915, d);
+		fw_domain_posting_read(i915, d);
 	}
 
-	dev_priv->uncore.fw_domains_active &= ~fw_domains;
+	i915->uncore.fw_domains_active &= ~fw_domains;
 }
 
 static void
-fw_domains_posting_read(struct drm_i915_private *dev_priv)
+fw_domains_posting_read(struct drm_i915_private *i915)
 {
 	struct intel_uncore_forcewake_domain *d;
 
 	/* No need to do for all, just do for first found */
-	for_each_fw_domain(d, dev_priv) {
-		fw_domain_posting_read(d);
+	for_each_fw_domain(d, i915) {
+		fw_domain_posting_read(i915, d);
 		break;
 	}
 }
 
 static void
-fw_domains_reset(struct drm_i915_private *dev_priv, enum forcewake_domains fw_domains)
+fw_domains_reset(struct drm_i915_private *i915,
+		 enum forcewake_domains fw_domains)
 {
 	struct intel_uncore_forcewake_domain *d;
 
-	if (dev_priv->uncore.fw_domains == 0)
+	if (i915->uncore.fw_domains == 0)
 		return;
 
-	for_each_fw_domain_masked(d, fw_domains, dev_priv)
-		fw_domain_reset(d);
+	for_each_fw_domain_masked(d, fw_domains, i915)
+		fw_domain_reset(i915, d);
 
-	fw_domains_posting_read(dev_priv);
+	fw_domains_posting_read(i915);
 }
 
 static void __gen6_gt_wait_for_thread_c0(struct drm_i915_private *dev_priv)
@@ -236,7 +243,8 @@ intel_uncore_fw_release_timer(struct hrtimer *timer)
 {
 	struct intel_uncore_forcewake_domain *domain =
 	       container_of(timer, struct intel_uncore_forcewake_domain, timer);
-	struct drm_i915_private *dev_priv = domain->i915;
+	struct drm_i915_private *dev_priv =
+		container_of(domain, struct drm_i915_private, uncore.fw_domain[domain->id]);
 	unsigned long irqflags;
 
 	assert_rpm_device_not_suspended(dev_priv);
@@ -1161,7 +1169,6 @@ static void fw_domain_init(struct drm_i915_private *dev_priv,
 	else if (IS_GEN6(dev_priv) || IS_GEN7(dev_priv) || IS_GEN8(dev_priv))
 		d->reg_post = ECOBUS;
 
-	d->i915 = dev_priv;
 	d->id = domain_id;
 
 	BUILD_BUG_ON(FORCEWAKE_RENDER != (1 << FW_DOMAIN_ID_RENDER));
@@ -1175,7 +1182,7 @@ static void fw_domain_init(struct drm_i915_private *dev_priv,
 
 	dev_priv->uncore.fw_domains |= (1 << domain_id);
 
-	fw_domain_reset(d);
+	fw_domain_reset(dev_priv, d);
 }
 
 static void intel_uncore_fw_domains_init(struct drm_i915_private *dev_priv)
-- 
2.11.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 2/5] drm/i915: Skip unused fw_domains

[Chris Wilson]

Use find-first-set bitop to quickly scan through the fw_domains mask and
skip iterating over unused domains.

v2: Move the WARN into the caller, to prevent compiler warnings in
normal builds.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_debugfs.c |  3 ++-
 drivers/gpu/drm/i915/i915_drv.h     | 30 +++++++++++------------
 drivers/gpu/drm/i915/intel_uncore.c | 48 ++++++++++++++++++++++++-------------
 3 files changed, 48 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 29bf11d8b620..fcac795d4396 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1461,9 +1461,10 @@ static int i915_forcewake_domains(struct seq_file *m, void *data)
 {
 	struct drm_i915_private *dev_priv = node_to_i915(m->private);
 	struct intel_uncore_forcewake_domain *fw_domain;
+	unsigned int tmp;
 
 	spin_lock_irq(&dev_priv->uncore.lock);
-	for_each_fw_domain(fw_domain, dev_priv) {
+	for_each_fw_domain(fw_domain, dev_priv, tmp) {
 		seq_printf(m, "%s.wake_count = %u\n",
 			   intel_uncore_forcewake_domain_to_str(fw_domain->id),
 			   fw_domain->wake_count);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 4c9de7d00eaf..589f91c4e585 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -703,9 +703,9 @@ enum forcewake_domain_id {
 };
 
 enum forcewake_domains {
-	FORCEWAKE_RENDER = (1 << FW_DOMAIN_ID_RENDER),
-	FORCEWAKE_BLITTER = (1 << FW_DOMAIN_ID_BLITTER),
-	FORCEWAKE_MEDIA	= (1 << FW_DOMAIN_ID_MEDIA),
+	FORCEWAKE_RENDER = BIT(FW_DOMAIN_ID_RENDER),
+	FORCEWAKE_BLITTER = BIT(FW_DOMAIN_ID_BLITTER),
+	FORCEWAKE_MEDIA	= BIT(FW_DOMAIN_ID_MEDIA),
 	FORCEWAKE_ALL = (FORCEWAKE_RENDER |
 			 FORCEWAKE_BLITTER |
 			 FORCEWAKE_MEDIA)
@@ -790,15 +790,19 @@ struct intel_uncore {
 	int unclaimed_mmio_check;
 };
 
+#define __mask_next_bit(mask) ({					\
+	int __idx = ffs(mask) - 1;					\
+	mask &= ~BIT(__idx);						\
+	__idx;								\
+})
+
 /* Iterate over initialised fw domains */
-#define for_each_fw_domain_masked(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_masked(domain__, mask__, dev_priv__, tmp__) \
+	for (tmp__ = (mask__); \
+	     tmp__ ? (domain__ = &(dev_priv__)->uncore.fw_domain[__mask_next_bit(tmp__)]), 1 : 0;)
 
-#define for_each_fw_domain(domain__, dev_priv__) \
-	for_each_fw_domain_masked(domain__, FORCEWAKE_ALL, dev_priv__)
+#define for_each_fw_domain(domain__, dev_priv__, tmp__) \
+	for_each_fw_domain_masked(domain__, (dev_priv__)->uncore.fw_domains, dev_priv__, tmp__)
 
 #define CSR_VERSION(major, minor)	((major) << 16 | (minor))
 #define CSR_VERSION_MAJOR(version)	((version) >> 16)
@@ -2581,12 +2585,6 @@ static inline struct drm_i915_private *huc_to_i915(struct intel_huc *huc)
 	     (id__)++) \
 		for_each_if ((engine__) = (dev_priv__)->engine[(id__)])
 
-#define __mask_next_bit(mask) ({					\
-	int __idx = ffs(mask) - 1;					\
-	mask &= ~BIT(__idx);						\
-	__idx;								\
-})
-
 /* Iterator over subset of engines selected by mask */
 #define for_each_engine_masked(engine__, dev_priv__, mask__, tmp__) \
 	for (tmp__ = mask__ & INTEL_INFO(dev_priv__)->ring_mask;	\
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index aa2e7401efdc..2e79ca129202 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -118,13 +118,16 @@ static void
 fw_domains_get(struct drm_i915_private *i915, enum forcewake_domains fw_domains)
 {
 	struct intel_uncore_forcewake_domain *d;
+	unsigned int tmp;
+
+	GEM_BUG_ON(fw_domains & ~i915->uncore.fw_domains);
 
-	for_each_fw_domain_masked(d, fw_domains, i915) {
+	for_each_fw_domain_masked(d, fw_domains, i915, tmp) {
 		fw_domain_wait_ack_clear(i915, d);
 		fw_domain_get(i915, d);
 	}
 
-	for_each_fw_domain_masked(d, fw_domains, i915)
+	for_each_fw_domain_masked(d, fw_domains, i915, tmp)
 		fw_domain_wait_ack(i915, d);
 
 	i915->uncore.fw_domains_active |= fw_domains;
@@ -134,8 +137,11 @@ static void
 fw_domains_put(struct drm_i915_private *i915, enum forcewake_domains fw_domains)
 {
 	struct intel_uncore_forcewake_domain *d;
+	unsigned int tmp;
+
+	GEM_BUG_ON(fw_domains & ~i915->uncore.fw_domains);
 
-	for_each_fw_domain_masked(d, fw_domains, i915) {
+	for_each_fw_domain_masked(d, fw_domains, i915, tmp) {
 		fw_domain_put(i915, d);
 		fw_domain_posting_read(i915, d);
 	}
@@ -147,9 +153,10 @@ static void
 fw_domains_posting_read(struct drm_i915_private *i915)
 {
 	struct intel_uncore_forcewake_domain *d;
+	unsigned int tmp;
 
 	/* No need to do for all, just do for first found */
-	for_each_fw_domain(d, i915) {
+	for_each_fw_domain(d, i915, tmp) {
 		fw_domain_posting_read(i915, d);
 		break;
 	}
@@ -160,11 +167,14 @@ fw_domains_reset(struct drm_i915_private *i915,
 		 enum forcewake_domains fw_domains)
 {
 	struct intel_uncore_forcewake_domain *d;
+	unsigned int tmp;
 
-	if (i915->uncore.fw_domains == 0)
+	if (!fw_domains)
 		return;
 
-	for_each_fw_domain_masked(d, fw_domains, i915)
+	GEM_BUG_ON(fw_domains & ~i915->uncore.fw_domains);
+
+	for_each_fw_domain_masked(d, fw_domains, i915, tmp)
 		fw_domain_reset(i915, d);
 
 	fw_domains_posting_read(i915);
@@ -274,9 +284,11 @@ static void intel_uncore_forcewake_reset(struct drm_i915_private *dev_priv,
 	 * timers are run before holding.
 	 */
 	while (1) {
+		unsigned int tmp;
+
 		active_domains = 0;
 
-		for_each_fw_domain(domain, dev_priv) {
+		for_each_fw_domain(domain, dev_priv, tmp) {
 			if (hrtimer_cancel(&domain->timer) == 0)
 				continue;
 
@@ -285,7 +297,7 @@ static void intel_uncore_forcewake_reset(struct drm_i915_private *dev_priv,
 
 		spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
 
-		for_each_fw_domain(domain, dev_priv) {
+		for_each_fw_domain(domain, dev_priv, tmp) {
 			if (hrtimer_active(&domain->timer))
 				active_domains |= domain->mask;
 		}
@@ -308,7 +320,7 @@ static void intel_uncore_forcewake_reset(struct drm_i915_private *dev_priv,
 	if (fw)
 		dev_priv->uncore.funcs.force_wake_put(dev_priv, fw);
 
-	fw_domains_reset(dev_priv, FORCEWAKE_ALL);
+	fw_domains_reset(dev_priv, dev_priv->uncore.fw_domains);
 
 	if (restore) { /* If reset with a user forcewake, try to restore */
 		if (fw)
@@ -465,13 +477,13 @@ static void __intel_uncore_forcewake_get(struct drm_i915_private *dev_priv,
 					 enum forcewake_domains fw_domains)
 {
 	struct intel_uncore_forcewake_domain *domain;
+	unsigned int tmp;
 
 	fw_domains &= dev_priv->uncore.fw_domains;
 
-	for_each_fw_domain_masked(domain, fw_domains, dev_priv) {
+	for_each_fw_domain_masked(domain, fw_domains, dev_priv, tmp)
 		if (domain->wake_count++)
 			fw_domains &= ~domain->mask;
-	}
 
 	if (fw_domains)
 		dev_priv->uncore.funcs.force_wake_get(dev_priv, fw_domains);
@@ -528,10 +540,11 @@ static void __intel_uncore_forcewake_put(struct drm_i915_private *dev_priv,
 					 enum forcewake_domains fw_domains)
 {
 	struct intel_uncore_forcewake_domain *domain;
+	unsigned int tmp;
 
 	fw_domains &= dev_priv->uncore.fw_domains;
 
-	for_each_fw_domain_masked(domain, fw_domains, dev_priv) {
+	for_each_fw_domain_masked(domain, fw_domains, dev_priv, tmp) {
 		if (WARN_ON(domain->wake_count == 0))
 			continue;
 
@@ -936,8 +949,11 @@ static noinline void ___force_wake_auto(struct drm_i915_private *dev_priv,
 					enum forcewake_domains fw_domains)
 {
 	struct intel_uncore_forcewake_domain *domain;
+	unsigned int tmp;
+
+	GEM_BUG_ON(fw_domains & ~dev_priv->uncore.fw_domains);
 
-	for_each_fw_domain_masked(domain, fw_domains, dev_priv)
+	for_each_fw_domain_masked(domain, fw_domains, dev_priv, tmp)
 		fw_domain_arm_timer(domain);
 
 	dev_priv->uncore.funcs.force_wake_get(dev_priv, fw_domains);
@@ -1175,7 +1191,7 @@ static void fw_domain_init(struct drm_i915_private *dev_priv,
 	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;
+	d->mask = BIT(domain_id);
 
 	hrtimer_init(&d->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
 	d->timer.function = intel_uncore_fw_release_timer;
@@ -1253,9 +1269,9 @@ static void intel_uncore_fw_domains_init(struct drm_i915_private *dev_priv)
 			       FORCEWAKE_MT, FORCEWAKE_MT_ACK);
 
 		spin_lock_irq(&dev_priv->uncore.lock);
-		fw_domains_get_with_thread_status(dev_priv, FORCEWAKE_ALL);
+		fw_domains_get_with_thread_status(dev_priv, FORCEWAKE_RENDER);
 		ecobus = __raw_i915_read32(dev_priv, ECOBUS);
-		fw_domains_put_with_fifo(dev_priv, FORCEWAKE_ALL);
+		fw_domains_put_with_fifo(dev_priv, FORCEWAKE_RENDER);
 		spin_unlock_irq(&dev_priv->uncore.lock);
 
 		if (!(ecobus & FORCEWAKE_MT_ENABLE)) {
-- 
2.11.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 3/5] drm/i915: Remove posting-read for forcewake put

[Chris Wilson]

We can relax the requirement upon ourselves that the forcewake is
released immediately and just allow it to occur naturally following our
mmio request.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h     |  1 -
 drivers/gpu/drm/i915/intel_uncore.c | 33 +--------------------------------
 2 files changed, 1 insertion(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 589f91c4e585..bb0e89d6255c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -783,7 +783,6 @@ struct intel_uncore {
 		u32 val_set;
 		u32 val_clear;
 		i915_reg_t reg_ack;
-		i915_reg_t reg_post;
 		u32 val_reset;
 	} fw_domain[FW_DOMAIN_ID_COUNT];
 
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index 2e79ca129202..5464c0418bab 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -105,15 +105,6 @@ fw_domain_put(struct drm_i915_private *i915,
 	__raw_i915_write32(i915, d->reg_set, d->val_clear);
 }
 
-static inline void
-fw_domain_posting_read(struct drm_i915_private *i915,
-		       const struct intel_uncore_forcewake_domain *d)
-{
-	/* something from same cacheline, but not from the set register */
-	if (i915_mmio_reg_valid(d->reg_post))
-		__raw_posting_read(i915, d->reg_post);
-}
-
 static void
 fw_domains_get(struct drm_i915_private *i915, enum forcewake_domains fw_domains)
 {
@@ -141,28 +132,13 @@ fw_domains_put(struct drm_i915_private *i915, enum forcewake_domains fw_domains)
 
 	GEM_BUG_ON(fw_domains & ~i915->uncore.fw_domains);
 
-	for_each_fw_domain_masked(d, fw_domains, i915, tmp) {
+	for_each_fw_domain_masked(d, fw_domains, i915, tmp)
 		fw_domain_put(i915, d);
-		fw_domain_posting_read(i915, d);
-	}
 
 	i915->uncore.fw_domains_active &= ~fw_domains;
 }
 
 static void
-fw_domains_posting_read(struct drm_i915_private *i915)
-{
-	struct intel_uncore_forcewake_domain *d;
-	unsigned int tmp;
-
-	/* No need to do for all, just do for first found */
-	for_each_fw_domain(d, i915, tmp) {
-		fw_domain_posting_read(i915, d);
-		break;
-	}
-}
-
-static void
 fw_domains_reset(struct drm_i915_private *i915,
 		 enum forcewake_domains fw_domains)
 {
@@ -176,8 +152,6 @@ fw_domains_reset(struct drm_i915_private *i915,
 
 	for_each_fw_domain_masked(d, fw_domains, i915, tmp)
 		fw_domain_reset(i915, d);
-
-	fw_domains_posting_read(i915);
 }
 
 static void __gen6_gt_wait_for_thread_c0(struct drm_i915_private *dev_priv)
@@ -1180,11 +1154,6 @@ static void fw_domain_init(struct drm_i915_private *dev_priv,
 		d->val_clear = _MASKED_BIT_DISABLE(FORCEWAKE_KERNEL);
 	}
 
-	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
-		d->reg_post = FORCEWAKE_ACK_VLV;
-	else if (IS_GEN6(dev_priv) || IS_GEN7(dev_priv) || IS_GEN8(dev_priv))
-		d->reg_post = ECOBUS;
-
 	d->id = domain_id;
 
 	BUILD_BUG_ON(FORCEWAKE_RENDER != (1 << FW_DOMAIN_ID_RENDER));
-- 
2.11.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 4/5] drm/i915: All fw_domains share the same set/clear/reset values

[Chris Wilson]

Since we reuse the same values for each fw_domain, move them onto
uncore.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h     | 11 +++++-----
 drivers/gpu/drm/i915/intel_uncore.c | 40 +++++++++++++++++++------------------
 2 files changed, 27 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index bb0e89d6255c..2911c49113b0 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -774,16 +774,17 @@ struct intel_uncore {
 	enum forcewake_domains fw_domains;
 	enum forcewake_domains fw_domains_active;
 
+	u32 fw_set;
+	u32 fw_clear;
+	u32 fw_reset;
+
 	struct intel_uncore_forcewake_domain {
 		enum forcewake_domain_id id;
 		enum forcewake_domains mask;
 		unsigned wake_count;
 		struct hrtimer timer;
 		i915_reg_t reg_set;
-		u32 val_set;
-		u32 val_clear;
 		i915_reg_t reg_ack;
-		u32 val_reset;
 	} fw_domain[FW_DOMAIN_ID_COUNT];
 
 	int unclaimed_mmio_check;
@@ -3956,14 +3957,14 @@ u64 intel_rc6_residency_us(struct drm_i915_private *dev_priv,
 #define POSTING_READ16(reg)	(void)I915_READ16_NOTRACE(reg)
 
 #define __raw_read(x, s) \
-static inline uint##x##_t __raw_i915_read##x(struct drm_i915_private *dev_priv, \
+static inline uint##x##_t __raw_i915_read##x(const struct drm_i915_private *dev_priv, \
 					     i915_reg_t reg) \
 { \
 	return read##s(dev_priv->regs + i915_mmio_reg_offset(reg)); \
 }
 
 #define __raw_write(x, s) \
-static inline void __raw_i915_write##x(struct drm_i915_private *dev_priv, \
+static inline void __raw_i915_write##x(const struct drm_i915_private *dev_priv, \
 				       i915_reg_t reg, uint##x##_t val) \
 { \
 	write##s(val, dev_priv->regs + i915_mmio_reg_offset(reg)); \
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index 5464c0418bab..6d1ea26b2493 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -55,8 +55,7 @@ static inline void
 fw_domain_reset(struct drm_i915_private *i915,
 		const struct intel_uncore_forcewake_domain *d)
 {
-	WARN_ON(!i915_mmio_reg_valid(d->reg_set));
-	__raw_i915_write32(i915, d->reg_set, d->val_reset);
+	__raw_i915_write32(i915, d->reg_set, i915->uncore.fw_reset);
 }
 
 static inline void
@@ -70,7 +69,7 @@ fw_domain_arm_timer(struct intel_uncore_forcewake_domain *d)
 }
 
 static inline void
-fw_domain_wait_ack_clear(struct drm_i915_private *i915,
+fw_domain_wait_ack_clear(const struct drm_i915_private *i915,
 			 const struct intel_uncore_forcewake_domain *d)
 {
 	if (wait_for_atomic((__raw_i915_read32(i915, d->reg_ack) &
@@ -84,11 +83,11 @@ static inline void
 fw_domain_get(struct drm_i915_private *i915,
 	      const struct intel_uncore_forcewake_domain *d)
 {
-	__raw_i915_write32(i915, d->reg_set, d->val_set);
+	__raw_i915_write32(i915, d->reg_set, i915->uncore.fw_set);
 }
 
 static inline void
-fw_domain_wait_ack(struct drm_i915_private *i915,
+fw_domain_wait_ack(const struct drm_i915_private *i915,
 		   const struct intel_uncore_forcewake_domain *d)
 {
 	if (wait_for_atomic((__raw_i915_read32(i915, d->reg_ack) &
@@ -99,10 +98,10 @@ fw_domain_wait_ack(struct drm_i915_private *i915,
 }
 
 static inline void
-fw_domain_put(struct drm_i915_private *i915,
+fw_domain_put(const struct drm_i915_private *i915,
 	      const struct intel_uncore_forcewake_domain *d)
 {
-	__raw_i915_write32(i915, d->reg_set, d->val_clear);
+	__raw_i915_write32(i915, d->reg_set, i915->uncore.fw_clear);
 }
 
 static void
@@ -1139,21 +1138,13 @@ static void fw_domain_init(struct drm_i915_private *dev_priv,
 
 	WARN_ON(d->wake_count);
 
+	WARN_ON(!i915_mmio_reg_valid(reg_set));
+	WARN_ON(!i915_mmio_reg_valid(reg_ack));
+
 	d->wake_count = 0;
 	d->reg_set = reg_set;
 	d->reg_ack = reg_ack;
 
-	if (IS_GEN6(dev_priv)) {
-		d->val_reset = 0;
-		d->val_set = FORCEWAKE_KERNEL;
-		d->val_clear = 0;
-	} else {
-		/* WaRsClearFWBitsAtReset:bdw,skl */
-		d->val_reset = _MASKED_BIT_DISABLE(0xffff);
-		d->val_set = _MASKED_BIT_ENABLE(FORCEWAKE_KERNEL);
-		d->val_clear = _MASKED_BIT_DISABLE(FORCEWAKE_KERNEL);
-	}
-
 	d->id = domain_id;
 
 	BUILD_BUG_ON(FORCEWAKE_RENDER != (1 << FW_DOMAIN_ID_RENDER));
@@ -1165,7 +1156,7 @@ static void fw_domain_init(struct drm_i915_private *dev_priv,
 	hrtimer_init(&d->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
 	d->timer.function = intel_uncore_fw_release_timer;
 
-	dev_priv->uncore.fw_domains |= (1 << domain_id);
+	dev_priv->uncore.fw_domains |= BIT(domain_id);
 
 	fw_domain_reset(dev_priv, d);
 }
@@ -1175,6 +1166,17 @@ static void intel_uncore_fw_domains_init(struct drm_i915_private *dev_priv)
 	if (INTEL_GEN(dev_priv) <= 5 || intel_vgpu_active(dev_priv))
 		return;
 
+	if (IS_GEN6(dev_priv)) {
+		dev_priv->uncore.fw_reset = 0;
+		dev_priv->uncore.fw_set = FORCEWAKE_KERNEL;
+		dev_priv->uncore.fw_clear = 0;
+	} else {
+		/* WaRsClearFWBitsAtReset:bdw,skl */
+		dev_priv->uncore.fw_reset = _MASKED_BIT_DISABLE(0xffff);
+		dev_priv->uncore.fw_set = _MASKED_BIT_ENABLE(FORCEWAKE_KERNEL);
+		dev_priv->uncore.fw_clear = _MASKED_BIT_DISABLE(FORCEWAKE_KERNEL);
+	}
+
 	if (IS_GEN9(dev_priv)) {
 		dev_priv->uncore.funcs.force_wake_get = fw_domains_get;
 		dev_priv->uncore.funcs.force_wake_put = fw_domains_put;
-- 
2.11.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 5/5] drm/i915: Drop uncore spinlock for reading forcewake counters

[Chris Wilson]

The set of available structs is not protected by the spinlock, and for
the single read we can use READ_ONCE instead.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index fcac795d4396..f30591f44d3a 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1459,17 +1459,14 @@ static int ironlake_drpc_info(struct seq_file *m)
 
 static int i915_forcewake_domains(struct seq_file *m, void *data)
 {
-	struct drm_i915_private *dev_priv = node_to_i915(m->private);
+	struct drm_i915_private *i915 = node_to_i915(m->private);
 	struct intel_uncore_forcewake_domain *fw_domain;
 	unsigned int tmp;
 
-	spin_lock_irq(&dev_priv->uncore.lock);
-	for_each_fw_domain(fw_domain, dev_priv, tmp) {
+	for_each_fw_domain(fw_domain, i915, tmp)
 		seq_printf(m, "%s.wake_count = %u\n",
 			   intel_uncore_forcewake_domain_to_str(fw_domain->id),
-			   fw_domain->wake_count);
-	}
-	spin_unlock_irq(&dev_priv->uncore.lock);
+			   READ_ONCE(fw_domain->wake_count));
 
 	return 0;
 }
-- 
2.11.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 5/5] drm/i915: Drop uncore spinlock for reading forcewake counters

[Joonas Lahtinen]

On ke, 2017-03-22 at 12:37 +0000, Chris Wilson wrote:
> The set of available structs is not protected by the spinlock, and for
> the single read we can use READ_ONCE instead.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: warning for series starting with [v2,1/5] drm/i915: Eliminate per-fw_domain i915 backpointer

[Patchwork]

== Series Details ==

Series: series starting with [v2,1/5] drm/i915: Eliminate per-fw_domain i915 backpointer
URL   : https://patchwork.freedesktop.org/series/21686/
State : warning

== Summary ==

Series 21686v1 Series without cover letter
https://patchwork.freedesktop.org/api/1.0/series/21686/revisions/1/mbox/

Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-uc:
                fail       -> PASS       (fi-snb-2600) fdo#100007
Test kms_force_connector_basic:
        Subgroup force-connector-state:
                pass       -> SKIP       (fi-ivb-3520m)
        Subgroup force-edid:
                pass       -> SKIP       (fi-ivb-3520m)
        Subgroup force-load-detect:
                pass       -> SKIP       (fi-ivb-3520m)
        Subgroup prune-stale-modes:
                pass       -> SKIP       (fi-ivb-3520m)

fdo#100007 https://bugs.freedesktop.org/show_bug.cgi?id=100007

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time: 458s
fi-bdw-gvtdvm    total:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  time: 466s
fi-bsw-n3050     total:278  pass:239  dwarn:0   dfail:0   fail:0   skip:39  time: 576s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time: 545s
fi-byt-j1900     total:278  pass:251  dwarn:0   dfail:0   fail:0   skip:27  time: 508s
fi-byt-n2820     total:278  pass:247  dwarn:0   dfail:0   fail:0   skip:31  time: 504s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time: 442s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time: 437s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time: 440s
fi-ivb-3520m     total:278  pass:256  dwarn:0   dfail:0   fail:0   skip:22  time: 517s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 497s
fi-kbl-7500u     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 478s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 489s
fi-skl-6700hq    total:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17  time: 596s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 519s
fi-skl-gvtdvm    total:278  pass:265  dwarn:0   dfail:0   fail:0   skip:13  time: 461s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time: 548s
fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29  time: 427s
fi-skl-6700k failed to collect. IGT log at Patchwork_4262/fi-skl-6700k/igt.log

1595c9b1bafa002ccfaa484f1dd3b2d7b9303a64 drm-tip: 2017y-03m-22d-09h-09m-59s UTC integration manifest
8b2459b drm/i915: Drop uncore spinlock for reading forcewake counters
307cf40 drm/i915: All fw_domains share the same set/clear/reset values
85a2834 drm/i915: Remove posting-read for forcewake put
0d766f0 drm/i915: Skip unused fw_domains
4c0323c drm/i915: Eliminate per-fw_domain i915 backpointer

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4262/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 2/5] drm/i915: Skip unused fw_domains

[Mika Kuoppala]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> Use find-first-set bitop to quickly scan through the fw_domains mask and
> skip iterating over unused domains.
>
> v2: Move the WARN into the caller, to prevent compiler warnings in
> normal builds.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c |  3 ++-
>  drivers/gpu/drm/i915/i915_drv.h     | 30 +++++++++++------------
>  drivers/gpu/drm/i915/intel_uncore.c | 48 ++++++++++++++++++++++++-------------
>  3 files changed, 48 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 29bf11d8b620..fcac795d4396 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1461,9 +1461,10 @@ static int i915_forcewake_domains(struct seq_file *m, void *data)
>  {
>  	struct drm_i915_private *dev_priv = node_to_i915(m->private);
>  	struct intel_uncore_forcewake_domain *fw_domain;
> +	unsigned int tmp;
>  
>  	spin_lock_irq(&dev_priv->uncore.lock);
> -	for_each_fw_domain(fw_domain, dev_priv) {
> +	for_each_fw_domain(fw_domain, dev_priv, tmp) {
>  		seq_printf(m, "%s.wake_count = %u\n",
>  			   intel_uncore_forcewake_domain_to_str(fw_domain->id),
>  			   fw_domain->wake_count);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 4c9de7d00eaf..589f91c4e585 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -703,9 +703,9 @@ enum forcewake_domain_id {
>  };
>  
>  enum forcewake_domains {
> -	FORCEWAKE_RENDER = (1 << FW_DOMAIN_ID_RENDER),
> -	FORCEWAKE_BLITTER = (1 << FW_DOMAIN_ID_BLITTER),
> -	FORCEWAKE_MEDIA	= (1 << FW_DOMAIN_ID_MEDIA),
> +	FORCEWAKE_RENDER = BIT(FW_DOMAIN_ID_RENDER),
> +	FORCEWAKE_BLITTER = BIT(FW_DOMAIN_ID_BLITTER),
> +	FORCEWAKE_MEDIA	= BIT(FW_DOMAIN_ID_MEDIA),
>  	FORCEWAKE_ALL = (FORCEWAKE_RENDER |
>  			 FORCEWAKE_BLITTER |
>  			 FORCEWAKE_MEDIA)
> @@ -790,15 +790,19 @@ struct intel_uncore {
>  	int unclaimed_mmio_check;
>  };
>  
> +#define __mask_next_bit(mask) ({					\
> +	int __idx = ffs(mask) - 1;					\
> +	mask &= ~BIT(__idx);						\
> +	__idx;								\
> +})
> +
>  /* Iterate over initialised fw domains */
> -#define for_each_fw_domain_masked(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_masked(domain__, mask__, dev_priv__, tmp__) \
> +	for (tmp__ = (mask__); \
> +	     tmp__ ? (domain__ = &(dev_priv__)->uncore.fw_domain[__mask_next_bit(tmp__)]), 1 : 0;)
>  
> -#define for_each_fw_domain(domain__, dev_priv__) \
> -	for_each_fw_domain_masked(domain__, FORCEWAKE_ALL, dev_priv__)
> +#define for_each_fw_domain(domain__, dev_priv__, tmp__) \
> +	for_each_fw_domain_masked(domain__, (dev_priv__)->uncore.fw_domains, dev_priv__, tmp__)
>  
>  #define CSR_VERSION(major, minor)	((major) << 16 | (minor))
>  #define CSR_VERSION_MAJOR(version)	((version) >> 16)
> @@ -2581,12 +2585,6 @@ static inline struct drm_i915_private *huc_to_i915(struct intel_huc *huc)
>  	     (id__)++) \
>  		for_each_if ((engine__) = (dev_priv__)->engine[(id__)])
>  
> -#define __mask_next_bit(mask) ({					\
> -	int __idx = ffs(mask) - 1;					\
> -	mask &= ~BIT(__idx);						\
> -	__idx;								\
> -})
> -
>  /* Iterator over subset of engines selected by mask */
>  #define for_each_engine_masked(engine__, dev_priv__, mask__, tmp__) \
>  	for (tmp__ = mask__ & INTEL_INFO(dev_priv__)->ring_mask;	\
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index aa2e7401efdc..2e79ca129202 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -118,13 +118,16 @@ static void
>  fw_domains_get(struct drm_i915_private *i915, enum forcewake_domains fw_domains)
>  {
>  	struct intel_uncore_forcewake_domain *d;
> +	unsigned int tmp;
> +
> +	GEM_BUG_ON(fw_domains & ~i915->uncore.fw_domains);
>  
> -	for_each_fw_domain_masked(d, fw_domains, i915) {
> +	for_each_fw_domain_masked(d, fw_domains, i915, tmp) {
>  		fw_domain_wait_ack_clear(i915, d);
>  		fw_domain_get(i915, d);
>  	}
>  
> -	for_each_fw_domain_masked(d, fw_domains, i915)
> +	for_each_fw_domain_masked(d, fw_domains, i915, tmp)
>  		fw_domain_wait_ack(i915, d);
>  
>  	i915->uncore.fw_domains_active |= fw_domains;
> @@ -134,8 +137,11 @@ static void
>  fw_domains_put(struct drm_i915_private *i915, enum forcewake_domains fw_domains)
>  {
>  	struct intel_uncore_forcewake_domain *d;
> +	unsigned int tmp;
> +
> +	GEM_BUG_ON(fw_domains & ~i915->uncore.fw_domains);
>  
> -	for_each_fw_domain_masked(d, fw_domains, i915) {
> +	for_each_fw_domain_masked(d, fw_domains, i915, tmp) {
>  		fw_domain_put(i915, d);
>  		fw_domain_posting_read(i915, d);
>  	}
> @@ -147,9 +153,10 @@ static void
>  fw_domains_posting_read(struct drm_i915_private *i915)
>  {
>  	struct intel_uncore_forcewake_domain *d;
> +	unsigned int tmp;
>  
>  	/* No need to do for all, just do for first found */
> -	for_each_fw_domain(d, i915) {
> +	for_each_fw_domain(d, i915, tmp) {
>  		fw_domain_posting_read(i915, d);
>  		break;
>  	}
> @@ -160,11 +167,14 @@ fw_domains_reset(struct drm_i915_private *i915,
>  		 enum forcewake_domains fw_domains)
>  {
>  	struct intel_uncore_forcewake_domain *d;
> +	unsigned int tmp;
>  
> -	if (i915->uncore.fw_domains == 0)
> +	if (!fw_domains)
>  		return;
>  
> -	for_each_fw_domain_masked(d, fw_domains, i915)
> +	GEM_BUG_ON(fw_domains & ~i915->uncore.fw_domains);
> +
> +	for_each_fw_domain_masked(d, fw_domains, i915, tmp)
>  		fw_domain_reset(i915, d);
>  
>  	fw_domains_posting_read(i915);
> @@ -274,9 +284,11 @@ static void intel_uncore_forcewake_reset(struct drm_i915_private *dev_priv,
>  	 * timers are run before holding.
>  	 */
>  	while (1) {
> +		unsigned int tmp;
> +
>  		active_domains = 0;
>  
> -		for_each_fw_domain(domain, dev_priv) {
> +		for_each_fw_domain(domain, dev_priv, tmp) {
>  			if (hrtimer_cancel(&domain->timer) == 0)
>  				continue;
>  
> @@ -285,7 +297,7 @@ static void intel_uncore_forcewake_reset(struct drm_i915_private *dev_priv,
>  
>  		spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
>  
> -		for_each_fw_domain(domain, dev_priv) {
> +		for_each_fw_domain(domain, dev_priv, tmp) {
>  			if (hrtimer_active(&domain->timer))
>  				active_domains |= domain->mask;
>  		}
> @@ -308,7 +320,7 @@ static void intel_uncore_forcewake_reset(struct drm_i915_private *dev_priv,
>  	if (fw)
>  		dev_priv->uncore.funcs.force_wake_put(dev_priv, fw);
>  
> -	fw_domains_reset(dev_priv, FORCEWAKE_ALL);
> +	fw_domains_reset(dev_priv, dev_priv->uncore.fw_domains);
>  
>  	if (restore) { /* If reset with a user forcewake, try to restore */
>  		if (fw)
> @@ -465,13 +477,13 @@ static void __intel_uncore_forcewake_get(struct drm_i915_private *dev_priv,
>  					 enum forcewake_domains fw_domains)
>  {
>  	struct intel_uncore_forcewake_domain *domain;
> +	unsigned int tmp;
>  
>  	fw_domains &= dev_priv->uncore.fw_domains;
>  
> -	for_each_fw_domain_masked(domain, fw_domains, dev_priv) {
> +	for_each_fw_domain_masked(domain, fw_domains, dev_priv, tmp)
>  		if (domain->wake_count++)
>  			fw_domains &= ~domain->mask;
> -	}
>  
>  	if (fw_domains)
>  		dev_priv->uncore.funcs.force_wake_get(dev_priv, fw_domains);
> @@ -528,10 +540,11 @@ static void __intel_uncore_forcewake_put(struct drm_i915_private *dev_priv,
>  					 enum forcewake_domains fw_domains)
>  {
>  	struct intel_uncore_forcewake_domain *domain;
> +	unsigned int tmp;
>  
>  	fw_domains &= dev_priv->uncore.fw_domains;
>  
> -	for_each_fw_domain_masked(domain, fw_domains, dev_priv) {
> +	for_each_fw_domain_masked(domain, fw_domains, dev_priv, tmp) {
>  		if (WARN_ON(domain->wake_count == 0))
>  			continue;
>  
> @@ -936,8 +949,11 @@ static noinline void ___force_wake_auto(struct drm_i915_private *dev_priv,
>  					enum forcewake_domains fw_domains)
>  {
>  	struct intel_uncore_forcewake_domain *domain;
> +	unsigned int tmp;
> +
> +	GEM_BUG_ON(fw_domains & ~dev_priv->uncore.fw_domains);
>  
> -	for_each_fw_domain_masked(domain, fw_domains, dev_priv)
> +	for_each_fw_domain_masked(domain, fw_domains, dev_priv, tmp)
>  		fw_domain_arm_timer(domain);
>  
>  	dev_priv->uncore.funcs.force_wake_get(dev_priv, fw_domains);
> @@ -1175,7 +1191,7 @@ static void fw_domain_init(struct drm_i915_private *dev_priv,
>  	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;
> +	d->mask = BIT(domain_id);
>  
>  	hrtimer_init(&d->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
>  	d->timer.function = intel_uncore_fw_release_timer;
> @@ -1253,9 +1269,9 @@ static void intel_uncore_fw_domains_init(struct drm_i915_private *dev_priv)
>  			       FORCEWAKE_MT, FORCEWAKE_MT_ACK);
>  
>  		spin_lock_irq(&dev_priv->uncore.lock);
> -		fw_domains_get_with_thread_status(dev_priv, FORCEWAKE_ALL);
> +		fw_domains_get_with_thread_status(dev_priv, FORCEWAKE_RENDER);

Well we are skipping here the unused ones so I guess it fits
to the commit desc.

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>

>  		ecobus = __raw_i915_read32(dev_priv, ECOBUS);
> -		fw_domains_put_with_fifo(dev_priv, FORCEWAKE_ALL);
> +		fw_domains_put_with_fifo(dev_priv, FORCEWAKE_RENDER);
>  		spin_unlock_irq(&dev_priv->uncore.lock);
>  
>  		if (!(ecobus & FORCEWAKE_MT_ENABLE)) {
> -- 
> 2.11.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 2/5] drm/i915: Skip unused fw_domains

[Chris Wilson]

On Thu, Mar 23, 2017 at 11:36:53AM +0200, Mika Kuoppala wrote:
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> > @@ -1253,9 +1269,9 @@ static void intel_uncore_fw_domains_init(struct drm_i915_private *dev_priv)
> >  			       FORCEWAKE_MT, FORCEWAKE_MT_ACK);
> >  
> >  		spin_lock_irq(&dev_priv->uncore.lock);
> > -		fw_domains_get_with_thread_status(dev_priv, FORCEWAKE_ALL);
> > +		fw_domains_get_with_thread_status(dev_priv, FORCEWAKE_RENDER);
> 
> Well we are skipping here the unused ones so I guess it fits
> to the commit desc.

This patch added the bug on to catch use with an invalid fw_domain (that
would have just been iterated over previously). I suppose I could have
split this chunk into a prep patch...

"In the next patch we will begin to sanity check that we do not attempt
to obtain the forcewake on an unsupport domain. However, that is exactly
what we do during our actual initialisation of fw_domains - rectify it
before it explodes."
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx