[PATCH 00/13] Forcewake binary search & code shrink

[PATCH 00/13] Forcewake binary search & code shrink

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

A bunch of cleaned up patches from one of my old branches. Just an idea I had
been toying about and probably more a RFC at this point. But it amazingly
passed trybot on (almost) the first try so I got tempted to post it.

Basically main motivation was to replace linear search comparison ladders in
all mmio accessors with something smarter - a binary search in this case.

Main enabler is moving the comparison ladders from the code into the data
tables. Then gradually, patch by patch, I was able to reduce the code and
end up with a total saving of approximately 14KiB for the series.

That is kind of nice, but whether the churn is worth it I leave open for
dicussion. It is slightly less code so perhaps more maintainable. Although
tastes will probably differ.

And I haven't been able to do any benchmarking to see if any performance
differece is at all detectable.

Tvrtko Ursulin (13):
  drm/i915: Remove redundant hsw_write* mmio functions
  drm/i915: Keep track of active forcewake domains in a bitmask
  drm/i915: Do not inline forcewake taking in mmio accessors
  drm/i915: Data driven register to forcewake domains lookup
  drm/i915: Sort forcewake mapping tables
  drm/i915: Use binary search when looking up forcewake domains
  drm/i915: Eliminate Gen9 special case
  drm/i915: Store the active forcewake range table pointer
  drm/i915: Remove identical macros
  drm/i915: Remove identical mmio read functions
  drm/i915: Remove identical write mmmio functions
  drm/i915: Sort the shadow register table
  drm/i915: Use binary search when looking for shadowed registers

 drivers/gpu/drm/i915/i915_drv.h     |  13 +
 drivers/gpu/drm/i915/intel_uncore.c | 525 ++++++++++++++++--------------------
 2 files changed, 247 insertions(+), 291 deletions(-)

-- 
2.7.4

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

[PATCH 01/13] drm/i915: Remove redundant hsw_write* mmio functions

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

They are completely identical to gen6_write* ones.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_uncore.c | 25 +------------------------
 1 file changed, 1 insertion(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index a9b6c936aadd..478364057812 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -1055,21 +1055,6 @@ gen6_write##x(struct drm_i915_private *dev_priv, i915_reg_t reg, u##x val, bool
 	GEN6_WRITE_FOOTER; \
 }
 
-#define __hsw_write(x) \
-static void \
-hsw_write##x(struct drm_i915_private *dev_priv, i915_reg_t reg, u##x val, bool trace) { \
-	u32 __fifo_ret = 0; \
-	GEN6_WRITE_HEADER; \
-	if (NEEDS_FORCE_WAKE(offset)) { \
-		__fifo_ret = __gen6_gt_wait_for_fifo(dev_priv); \
-	} \
-	__raw_i915_write##x(dev_priv, reg, val); \
-	if (unlikely(__fifo_ret)) { \
-		gen6_gt_check_fifodbg(dev_priv); \
-	} \
-	GEN6_WRITE_FOOTER; \
-}
-
 #define __gen8_write(x) \
 static void \
 gen8_write##x(struct drm_i915_private *dev_priv, i915_reg_t reg, u##x val, bool trace) { \
@@ -1116,9 +1101,6 @@ __chv_write(32)
 __gen8_write(8)
 __gen8_write(16)
 __gen8_write(32)
-__hsw_write(8)
-__hsw_write(16)
-__hsw_write(32)
 __gen6_write(8)
 __gen6_write(16)
 __gen6_write(32)
@@ -1126,7 +1108,6 @@ __gen6_write(32)
 #undef __gen9_write
 #undef __chv_write
 #undef __gen8_write
-#undef __hsw_write
 #undef __gen6_write
 #undef GEN6_WRITE_FOOTER
 #undef GEN6_WRITE_HEADER
@@ -1343,11 +1324,7 @@ void intel_uncore_init(struct drm_i915_private *dev_priv)
 		break;
 	case 7:
 	case 6:
-		if (IS_HASWELL(dev_priv)) {
-			ASSIGN_WRITE_MMIO_VFUNCS(hsw);
-		} else {
-			ASSIGN_WRITE_MMIO_VFUNCS(gen6);
-		}
+		ASSIGN_WRITE_MMIO_VFUNCS(gen6);
 
 		if (IS_VALLEYVIEW(dev_priv)) {
 			ASSIGN_READ_MMIO_VFUNCS(vlv);
-- 
2.7.4

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

[PATCH 02/13] drm/i915: Keep track of active forcewake domains in a bitmask

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

There are current places in the code, and there will be more in the
future, which iterate the forcewake domains to find out which ones
are currently active.

To save them from doing this iteration, we can cheaply keep a mask
of active domains in dev_priv->uncore.fw_domains_active.

This has no cost in terms of object size, even manages to shrink it
overall by 368 bytes on my config.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: "Paneri, Praveen" <praveen.paneri@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_drv.h     |  2 ++
 drivers/gpu/drm/i915/intel_uncore.c | 54 ++++++++++++++++---------------------
 2 files changed, 25 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 23bc43d23d2c..4cd727376d1d 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -588,7 +588,9 @@ struct intel_uncore {
 	struct intel_uncore_funcs funcs;
 
 	unsigned fifo_count;
+
 	enum forcewake_domains fw_domains;
+	enum forcewake_domains fw_domains_active;
 
 	struct intel_uncore_forcewake_domain {
 		struct drm_i915_private *i915;
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index 478364057812..079102b46fd3 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -231,19 +231,21 @@ 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;
 	unsigned long irqflags;
 
-	assert_rpm_device_not_suspended(domain->i915);
+	assert_rpm_device_not_suspended(dev_priv);
 
-	spin_lock_irqsave(&domain->i915->uncore.lock, irqflags);
+	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
 	if (WARN_ON(domain->wake_count == 0))
 		domain->wake_count++;
 
-	if (--domain->wake_count == 0)
-		domain->i915->uncore.funcs.force_wake_put(domain->i915,
-							  1 << domain->id);
+	if (--domain->wake_count == 0) {
+		dev_priv->uncore.funcs.force_wake_put(dev_priv, domain->mask);
+		dev_priv->uncore.fw_domains_active &= ~domain->mask;
+	}
 
-	spin_unlock_irqrestore(&domain->i915->uncore.lock, irqflags);
+	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 
 	return HRTIMER_NORESTART;
 }
@@ -254,7 +256,7 @@ void intel_uncore_forcewake_reset(struct drm_i915_private *dev_priv,
 	unsigned long irqflags;
 	struct intel_uncore_forcewake_domain *domain;
 	int retry_count = 100;
-	enum forcewake_domains fw = 0, active_domains;
+	enum forcewake_domains fw, active_domains;
 
 	/* Hold uncore.lock across reset to prevent any register access
 	 * with forcewake not set correctly. Wait until all pending
@@ -291,10 +293,7 @@ void intel_uncore_forcewake_reset(struct drm_i915_private *dev_priv,
 
 	WARN_ON(active_domains);
 
-	for_each_fw_domain(domain, dev_priv)
-		if (domain->wake_count)
-			fw |= domain->mask;
-
+	fw = dev_priv->uncore.fw_domains_active;
 	if (fw)
 		dev_priv->uncore.funcs.force_wake_put(dev_priv, fw);
 
@@ -443,9 +442,6 @@ static void __intel_uncore_forcewake_get(struct drm_i915_private *dev_priv,
 {
 	struct intel_uncore_forcewake_domain *domain;
 
-	if (!dev_priv->uncore.funcs.force_wake_get)
-		return;
-
 	fw_domains &= dev_priv->uncore.fw_domains;
 
 	for_each_fw_domain_masked(domain, fw_domains, dev_priv) {
@@ -453,8 +449,10 @@ static void __intel_uncore_forcewake_get(struct drm_i915_private *dev_priv,
 			fw_domains &= ~domain->mask;
 	}
 
-	if (fw_domains)
+	if (fw_domains) {
 		dev_priv->uncore.funcs.force_wake_get(dev_priv, fw_domains);
+		dev_priv->uncore.fw_domains_active |= fw_domains;
+	}
 }
 
 /**
@@ -509,9 +507,6 @@ static void __intel_uncore_forcewake_put(struct drm_i915_private *dev_priv,
 {
 	struct intel_uncore_forcewake_domain *domain;
 
-	if (!dev_priv->uncore.funcs.force_wake_put)
-		return;
-
 	fw_domains &= dev_priv->uncore.fw_domains;
 
 	for_each_fw_domain_masked(domain, fw_domains, dev_priv) {
@@ -567,13 +562,10 @@ 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;
-
 	if (!dev_priv->uncore.funcs.force_wake_get)
 		return;
 
-	for_each_fw_domain(domain, dev_priv)
-		WARN_ON(domain->wake_count);
+	WARN_ON(dev_priv->uncore.fw_domains_active);
 }
 
 /* We give fast paths for the really cool registers */
@@ -878,18 +870,18 @@ static inline void __force_wake_auto(struct drm_i915_private *dev_priv,
 	if (WARN_ON(!fw_domains))
 		return;
 
-	/* Ideally GCC would be constant-fold and eliminate this loop */
-	for_each_fw_domain_masked(domain, fw_domains, dev_priv) {
-		if (domain->wake_count) {
-			fw_domains &= ~domain->mask;
-			continue;
-		}
+	/* Turn on all requested but inactive supported forcewake domains. */
+	fw_domains &= dev_priv->uncore.fw_domains;
+	fw_domains &= ~dev_priv->uncore.fw_domains_active;
 
-		fw_domain_arm_timer(domain);
-	}
+	if (fw_domains) {
+		/* Ideally GCC would be constant-fold and eliminate this loop */
+		for_each_fw_domain_masked(domain, fw_domains, dev_priv)
+			fw_domain_arm_timer(domain);
 
-	if (fw_domains)
 		dev_priv->uncore.funcs.force_wake_get(dev_priv, fw_domains);
+		dev_priv->uncore.fw_domains_active |= fw_domains;
+	}
 }
 
 #define __gen6_read(x) \
-- 
2.7.4

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

[PATCH 03/13] drm/i915: Do not inline forcewake taking in mmio accessors

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Once we know we need to take new forcewakes, that being
a slow operation, it does not make sense to inline that
code into every mmio accessor.

Move it to a separate function and save some code.

v2: Be explicit with noinline and remove stale comment.
    (Chris Wilson)

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_uncore.c | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index 079102b46fd3..60137a164094 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -862,11 +862,21 @@ __gen2_read(64)
 	trace_i915_reg_rw(false, reg, val, sizeof(val), trace); \
 	return val
 
-static inline void __force_wake_auto(struct drm_i915_private *dev_priv,
-				     enum forcewake_domains fw_domains)
+static noinline void ___force_wake_auto(struct drm_i915_private *dev_priv,
+					enum forcewake_domains fw_domains)
 {
 	struct intel_uncore_forcewake_domain *domain;
 
+	for_each_fw_domain_masked(domain, fw_domains, dev_priv)
+		fw_domain_arm_timer(domain);
+
+	dev_priv->uncore.funcs.force_wake_get(dev_priv, fw_domains);
+	dev_priv->uncore.fw_domains_active |= fw_domains;
+}
+
+static inline void __force_wake_auto(struct drm_i915_private *dev_priv,
+				     enum forcewake_domains fw_domains)
+{
 	if (WARN_ON(!fw_domains))
 		return;
 
@@ -874,14 +884,8 @@ static inline void __force_wake_auto(struct drm_i915_private *dev_priv,
 	fw_domains &= dev_priv->uncore.fw_domains;
 	fw_domains &= ~dev_priv->uncore.fw_domains_active;
 
-	if (fw_domains) {
-		/* Ideally GCC would be constant-fold and eliminate this loop */
-		for_each_fw_domain_masked(domain, fw_domains, dev_priv)
-			fw_domain_arm_timer(domain);
-
-		dev_priv->uncore.funcs.force_wake_get(dev_priv, fw_domains);
-		dev_priv->uncore.fw_domains_active |= fw_domains;
-	}
+	if (fw_domains)
+		___force_wake_auto(dev_priv, fw_domains);
 }
 
 #define __gen6_read(x) \
-- 
2.7.4

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

[PATCH 04/13] drm/i915: Data driven register to forcewake domains lookup

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Move finding the correct forcewake domains to take for
register access from code to a mapping table. This will
allow more interesting work in the following patches
and is easier to review if singled out early.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/intel_uncore.c | 212 ++++++++++++++++++------------------
 1 file changed, 103 insertions(+), 109 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index 60137a164094..e21e65ab2a16 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -581,28 +581,52 @@ void assert_forcewakes_inactive(struct drm_i915_private *dev_priv)
 	__fwd; \
 })
 
-#define REG_RANGE(reg, start, end) ((reg) >= (start) && (reg) < (end))
+struct intel_forcewake_range
+{
+	u32 start;
+	u32 end;
+
+	enum forcewake_domains domains;
+};
 
-#define FORCEWAKE_VLV_RENDER_RANGE_OFFSET(reg) \
-	(REG_RANGE((reg), 0x2000, 0x4000) || \
-	 REG_RANGE((reg), 0x5000, 0x8000) || \
-	 REG_RANGE((reg), 0xB000, 0x12000) || \
-	 REG_RANGE((reg), 0x2E000, 0x30000))
+static enum forcewake_domains
+find_fw_domain(u32 offset, const struct intel_forcewake_range *ranges,
+	       unsigned int num_ranges)
+{
+	unsigned int i;
+	struct intel_forcewake_range *entry =
+		(struct intel_forcewake_range *)ranges;
+
+	for (i = 0; i < num_ranges; i++, entry++) {
+		if (offset >= entry->start && offset <= entry->end)
+			return entry->domains;
+	}
+
+	return -1;
+}
 
-#define FORCEWAKE_VLV_MEDIA_RANGE_OFFSET(reg) \
-	(REG_RANGE((reg), 0x12000, 0x14000) || \
-	 REG_RANGE((reg), 0x22000, 0x24000) || \
-	 REG_RANGE((reg), 0x30000, 0x40000))
+#define GEN_FW_RANGE(s, e, d) \
+	{ .start = (s), .end = (e), .domains = (d) }
+
+static const struct intel_forcewake_range __vlv_fw_ranges[] = {
+	GEN_FW_RANGE(0x2000, 0x3fff, FORCEWAKE_RENDER),
+	GEN_FW_RANGE(0x5000, 0x7fff, FORCEWAKE_RENDER),
+	GEN_FW_RANGE(0xb000, 0x11fff, FORCEWAKE_RENDER),
+	GEN_FW_RANGE(0x2e000, 0x2ffff, FORCEWAKE_RENDER),
+	GEN_FW_RANGE(0x12000, 0x13fff, FORCEWAKE_MEDIA),
+	GEN_FW_RANGE(0x22000, 0x23fff, FORCEWAKE_MEDIA),
+	GEN_FW_RANGE(0x30000, 0x3ffff, FORCEWAKE_MEDIA),
+};
 
 #define __vlv_reg_read_fw_domains(offset) \
 ({ \
 	enum forcewake_domains __fwd = 0; \
-	if (!NEEDS_FORCE_WAKE(offset)) \
-		__fwd = 0; \
-	else if (FORCEWAKE_VLV_RENDER_RANGE_OFFSET(offset)) \
-		__fwd = FORCEWAKE_RENDER; \
-	else if (FORCEWAKE_VLV_MEDIA_RANGE_OFFSET(offset)) \
-		__fwd = FORCEWAKE_MEDIA; \
+	if (NEEDS_FORCE_WAKE((offset))) { \
+		__fwd = find_fw_domain(offset, __vlv_fw_ranges, \
+				       ARRAY_SIZE(__vlv_fw_ranges)); \
+		if (__fwd == -1 ) \
+			__fwd = 0; \
+	} \
 	__fwd; \
 })
 
@@ -636,104 +660,78 @@ static bool is_gen8_shadowed(u32 offset)
 	__fwd; \
 })
 
-#define FORCEWAKE_CHV_RENDER_RANGE_OFFSET(reg) \
-	(REG_RANGE((reg), 0x2000, 0x4000) || \
-	 REG_RANGE((reg), 0x5200, 0x8000) || \
-	 REG_RANGE((reg), 0x8300, 0x8500) || \
-	 REG_RANGE((reg), 0xB000, 0xB480) || \
-	 REG_RANGE((reg), 0xE000, 0xE800))
-
-#define FORCEWAKE_CHV_MEDIA_RANGE_OFFSET(reg) \
-	(REG_RANGE((reg), 0x8800, 0x8900) || \
-	 REG_RANGE((reg), 0xD000, 0xD800) || \
-	 REG_RANGE((reg), 0x12000, 0x14000) || \
-	 REG_RANGE((reg), 0x1A000, 0x1C000) || \
-	 REG_RANGE((reg), 0x1E800, 0x1EA00) || \
-	 REG_RANGE((reg), 0x30000, 0x38000))
-
-#define FORCEWAKE_CHV_COMMON_RANGE_OFFSET(reg) \
-	(REG_RANGE((reg), 0x4000, 0x5000) || \
-	 REG_RANGE((reg), 0x8000, 0x8300) || \
-	 REG_RANGE((reg), 0x8500, 0x8600) || \
-	 REG_RANGE((reg), 0x9000, 0xB000) || \
-	 REG_RANGE((reg), 0xF000, 0x10000))
+static const struct intel_forcewake_range __chv_fw_ranges[] = {
+	GEN_FW_RANGE(0x2000, 0x3fff, FORCEWAKE_RENDER),
+	GEN_FW_RANGE(0x5200, 0x7fff, FORCEWAKE_RENDER),
+	GEN_FW_RANGE(0x8300, 0x84ff, FORCEWAKE_RENDER),
+	GEN_FW_RANGE(0xb000, 0xb47f, FORCEWAKE_RENDER),
+	GEN_FW_RANGE(0xe000, 0xe7ff, FORCEWAKE_RENDER),
+	GEN_FW_RANGE(0x8800, 0x88ff, FORCEWAKE_MEDIA),
+	GEN_FW_RANGE(0xd000, 0xd7ff, FORCEWAKE_MEDIA),
+	GEN_FW_RANGE(0x12000, 0x13fff, FORCEWAKE_MEDIA),
+	GEN_FW_RANGE(0x1a000, 0x1bfff, FORCEWAKE_MEDIA),
+	GEN_FW_RANGE(0x1e800, 0x1e9ff, FORCEWAKE_MEDIA),
+	GEN_FW_RANGE(0x30000, 0x37fff, FORCEWAKE_MEDIA),
+	GEN_FW_RANGE(0x4000, 0x4fff, FORCEWAKE_RENDER | FORCEWAKE_MEDIA),
+	GEN_FW_RANGE(0x8000, 0x82ff, FORCEWAKE_RENDER | FORCEWAKE_MEDIA),
+	GEN_FW_RANGE(0x8500, 0x85ff, FORCEWAKE_RENDER | FORCEWAKE_MEDIA),
+	GEN_FW_RANGE(0x9000, 0xafff, FORCEWAKE_RENDER | FORCEWAKE_MEDIA),
+	GEN_FW_RANGE(0xf000, 0xffff, FORCEWAKE_RENDER | FORCEWAKE_MEDIA),
+};
 
 #define __chv_reg_read_fw_domains(offset) \
 ({ \
 	enum forcewake_domains __fwd = 0; \
-	if (!NEEDS_FORCE_WAKE(offset)) \
-		__fwd = 0; \
-	else if (FORCEWAKE_CHV_RENDER_RANGE_OFFSET(offset)) \
-		__fwd = FORCEWAKE_RENDER; \
-	else if (FORCEWAKE_CHV_MEDIA_RANGE_OFFSET(offset)) \
-		__fwd = FORCEWAKE_MEDIA; \
-	else if (FORCEWAKE_CHV_COMMON_RANGE_OFFSET(offset)) \
-		__fwd = FORCEWAKE_RENDER | FORCEWAKE_MEDIA; \
+	if (NEEDS_FORCE_WAKE((offset))) { \
+		__fwd = find_fw_domain(offset, __chv_fw_ranges, \
+				       ARRAY_SIZE(__chv_fw_ranges)); \
+		if (__fwd == -1 ) \
+			__fwd = 0; \
+	} \
 	__fwd; \
 })
 
 #define __chv_reg_write_fw_domains(offset) \
 ({ \
 	enum forcewake_domains __fwd = 0; \
-	if (!NEEDS_FORCE_WAKE(offset) || is_gen8_shadowed(offset)) \
-		__fwd = 0; \
-	else if (FORCEWAKE_CHV_RENDER_RANGE_OFFSET(offset)) \
-		__fwd = FORCEWAKE_RENDER; \
-	else if (FORCEWAKE_CHV_MEDIA_RANGE_OFFSET(offset)) \
-		__fwd = FORCEWAKE_MEDIA; \
-	else if (FORCEWAKE_CHV_COMMON_RANGE_OFFSET(offset)) \
-		__fwd = FORCEWAKE_RENDER | FORCEWAKE_MEDIA; \
+	if (NEEDS_FORCE_WAKE((offset)) && !is_gen8_shadowed(offset)) { \
+		__fwd = find_fw_domain(offset, __chv_fw_ranges, \
+				       ARRAY_SIZE(__chv_fw_ranges)); \
+		if (__fwd == -1 ) \
+			__fwd = 0; \
+	} \
 	__fwd; \
 })
 
-#define FORCEWAKE_GEN9_UNCORE_RANGE_OFFSET(reg) \
-	REG_RANGE((reg), 0xB00,  0x2000)
-
-#define FORCEWAKE_GEN9_RENDER_RANGE_OFFSET(reg) \
-	(REG_RANGE((reg), 0x2000, 0x2700) || \
-	 REG_RANGE((reg), 0x3000, 0x4000) || \
-	 REG_RANGE((reg), 0x5200, 0x8000) || \
-	 REG_RANGE((reg), 0x8140, 0x8160) || \
-	 REG_RANGE((reg), 0x8300, 0x8500) || \
-	 REG_RANGE((reg), 0x8C00, 0x8D00) || \
-	 REG_RANGE((reg), 0xB000, 0xB480) || \
-	 REG_RANGE((reg), 0xE000, 0xE900) || \
-	 REG_RANGE((reg), 0x24400, 0x24800))
-
-#define FORCEWAKE_GEN9_MEDIA_RANGE_OFFSET(reg) \
-	(REG_RANGE((reg), 0x8130, 0x8140) || \
-	 REG_RANGE((reg), 0x8800, 0x8A00) || \
-	 REG_RANGE((reg), 0xD000, 0xD800) || \
-	 REG_RANGE((reg), 0x12000, 0x14000) || \
-	 REG_RANGE((reg), 0x1A000, 0x1EA00) || \
-	 REG_RANGE((reg), 0x30000, 0x40000))
-
-#define FORCEWAKE_GEN9_COMMON_RANGE_OFFSET(reg) \
-	REG_RANGE((reg), 0x9400, 0x9800)
-
-#define FORCEWAKE_GEN9_BLITTER_RANGE_OFFSET(reg) \
-	((reg) < 0x40000 && \
-	 !FORCEWAKE_GEN9_UNCORE_RANGE_OFFSET(reg) && \
-	 !FORCEWAKE_GEN9_RENDER_RANGE_OFFSET(reg) && \
-	 !FORCEWAKE_GEN9_MEDIA_RANGE_OFFSET(reg) && \
-	 !FORCEWAKE_GEN9_COMMON_RANGE_OFFSET(reg))
-
-#define SKL_NEEDS_FORCE_WAKE(reg) \
-	((reg) < 0x40000 && !FORCEWAKE_GEN9_UNCORE_RANGE_OFFSET(reg))
+static const struct intel_forcewake_range __gen9_fw_ranges[] = {
+	GEN_FW_RANGE(0xb00, 0x1fff, 0), /* uncore range */
+	GEN_FW_RANGE(0x2000, 0x26ff, FORCEWAKE_RENDER),
+	GEN_FW_RANGE(0x3000, 0x3fff, FORCEWAKE_RENDER),
+	GEN_FW_RANGE(0x5200, 0x7fff, FORCEWAKE_RENDER),
+	GEN_FW_RANGE(0x8140, 0x815f, FORCEWAKE_RENDER),
+	GEN_FW_RANGE(0x8300, 0x84ff, FORCEWAKE_RENDER),
+	GEN_FW_RANGE(0x8c00, 0x8cff, FORCEWAKE_RENDER),
+	GEN_FW_RANGE(0xb000, 0xb47f, FORCEWAKE_RENDER),
+	GEN_FW_RANGE(0xe000, 0xe8ff, FORCEWAKE_RENDER),
+	GEN_FW_RANGE(0x24400, 0x247ff, FORCEWAKE_RENDER),
+	GEN_FW_RANGE(0x9400, 0x97ff, FORCEWAKE_RENDER | FORCEWAKE_MEDIA),
+	GEN_FW_RANGE(0x8130, 0x813f, FORCEWAKE_MEDIA),
+	GEN_FW_RANGE(0x8800, 0x89ff, FORCEWAKE_MEDIA),
+	GEN_FW_RANGE(0xd000, 0xd7ff, FORCEWAKE_MEDIA),
+	GEN_FW_RANGE(0x12000, 0x13fff, FORCEWAKE_MEDIA),
+	GEN_FW_RANGE(0x1a000, 0x1e9ff, FORCEWAKE_MEDIA),
+	GEN_FW_RANGE(0x30000, 0x3ffff, FORCEWAKE_MEDIA),
+};
 
 #define __gen9_reg_read_fw_domains(offset) \
 ({ \
-	enum forcewake_domains __fwd; \
-	if (!SKL_NEEDS_FORCE_WAKE(offset)) \
-		__fwd = 0; \
-	else if (FORCEWAKE_GEN9_RENDER_RANGE_OFFSET(offset)) \
-		__fwd = FORCEWAKE_RENDER; \
-	else if (FORCEWAKE_GEN9_MEDIA_RANGE_OFFSET(offset)) \
-		__fwd = FORCEWAKE_MEDIA; \
-	else if (FORCEWAKE_GEN9_COMMON_RANGE_OFFSET(offset)) \
-		__fwd = FORCEWAKE_RENDER | FORCEWAKE_MEDIA; \
-	else \
-		__fwd = FORCEWAKE_BLITTER; \
+	enum forcewake_domains __fwd = 0; \
+	if (NEEDS_FORCE_WAKE((offset))) { \
+		__fwd = find_fw_domain(offset, __gen9_fw_ranges, \
+				       ARRAY_SIZE(__gen9_fw_ranges)); \
+		if (__fwd == -1 ) \
+			__fwd = FORCEWAKE_BLITTER; \
+	} \
 	__fwd; \
 })
 
@@ -759,17 +757,13 @@ static bool is_gen9_shadowed(u32 offset)
 
 #define __gen9_reg_write_fw_domains(offset) \
 ({ \
-	enum forcewake_domains __fwd; \
-	if (!SKL_NEEDS_FORCE_WAKE(offset) || is_gen9_shadowed(offset)) \
-		__fwd = 0; \
-	else if (FORCEWAKE_GEN9_RENDER_RANGE_OFFSET(offset)) \
-		__fwd = FORCEWAKE_RENDER; \
-	else if (FORCEWAKE_GEN9_MEDIA_RANGE_OFFSET(offset)) \
-		__fwd = FORCEWAKE_MEDIA; \
-	else if (FORCEWAKE_GEN9_COMMON_RANGE_OFFSET(offset)) \
-		__fwd = FORCEWAKE_RENDER | FORCEWAKE_MEDIA; \
-	else \
-		__fwd = FORCEWAKE_BLITTER; \
+	enum forcewake_domains __fwd = 0; \
+	if (NEEDS_FORCE_WAKE((offset)) && !is_gen9_shadowed(offset)) { \
+		__fwd = find_fw_domain(offset, __gen9_fw_ranges, \
+				       ARRAY_SIZE(__gen9_fw_ranges)); \
+		if (__fwd == -1 ) \
+			__fwd = FORCEWAKE_BLITTER; \
+	} \
 	__fwd; \
 })
 
-- 
2.7.4

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

[PATCH 05/13] drm/i915: Sort forcewake mapping tables

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Sorting the tables (verified at runtime to help during
development) is another prerequisite for interesting
work which will follow.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/intel_uncore.c | 55 ++++++++++++++++++++++++++++---------
 1 file changed, 42 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index e21e65ab2a16..bee1482a5ece 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -605,16 +605,34 @@ find_fw_domain(u32 offset, const struct intel_forcewake_range *ranges,
 	return -1;
 }
 
+static void
+intel_fw_table_check(const struct intel_forcewake_range *ranges,
+		     unsigned int num_ranges)
+{
+	unsigned int i;
+	struct intel_forcewake_range *entry =
+		(struct intel_forcewake_range *)ranges;
+	s32 prev = -1;
+
+	for (i = 0; i < num_ranges; i++, entry++) {
+		WARN_ON_ONCE(prev >= (s32)entry->start);
+		prev = entry->start;
+		WARN_ON_ONCE(prev >= (s32)entry->end);
+		prev = entry->end;
+	}
+}
+
 #define GEN_FW_RANGE(s, e, d) \
 	{ .start = (s), .end = (e), .domains = (d) }
 
+/* *Must* be sorted by offset ranges! */
 static const struct intel_forcewake_range __vlv_fw_ranges[] = {
 	GEN_FW_RANGE(0x2000, 0x3fff, FORCEWAKE_RENDER),
 	GEN_FW_RANGE(0x5000, 0x7fff, FORCEWAKE_RENDER),
 	GEN_FW_RANGE(0xb000, 0x11fff, FORCEWAKE_RENDER),
-	GEN_FW_RANGE(0x2e000, 0x2ffff, FORCEWAKE_RENDER),
 	GEN_FW_RANGE(0x12000, 0x13fff, FORCEWAKE_MEDIA),
 	GEN_FW_RANGE(0x22000, 0x23fff, FORCEWAKE_MEDIA),
+	GEN_FW_RANGE(0x2e000, 0x2ffff, FORCEWAKE_RENDER),
 	GEN_FW_RANGE(0x30000, 0x3ffff, FORCEWAKE_MEDIA),
 };
 
@@ -660,23 +678,24 @@ static bool is_gen8_shadowed(u32 offset)
 	__fwd; \
 })
 
+/* *Must* be sorted by offset ranges! */
 static const struct intel_forcewake_range __chv_fw_ranges[] = {
 	GEN_FW_RANGE(0x2000, 0x3fff, FORCEWAKE_RENDER),
+	GEN_FW_RANGE(0x4000, 0x4fff, FORCEWAKE_RENDER | FORCEWAKE_MEDIA),
 	GEN_FW_RANGE(0x5200, 0x7fff, FORCEWAKE_RENDER),
+	GEN_FW_RANGE(0x8000, 0x82ff, FORCEWAKE_RENDER | FORCEWAKE_MEDIA),
 	GEN_FW_RANGE(0x8300, 0x84ff, FORCEWAKE_RENDER),
-	GEN_FW_RANGE(0xb000, 0xb47f, FORCEWAKE_RENDER),
-	GEN_FW_RANGE(0xe000, 0xe7ff, FORCEWAKE_RENDER),
+	GEN_FW_RANGE(0x8500, 0x85ff, FORCEWAKE_RENDER | FORCEWAKE_MEDIA),
 	GEN_FW_RANGE(0x8800, 0x88ff, FORCEWAKE_MEDIA),
+	GEN_FW_RANGE(0x9000, 0xafff, FORCEWAKE_RENDER | FORCEWAKE_MEDIA),
+	GEN_FW_RANGE(0xb000, 0xb47f, FORCEWAKE_RENDER),
 	GEN_FW_RANGE(0xd000, 0xd7ff, FORCEWAKE_MEDIA),
+	GEN_FW_RANGE(0xe000, 0xe7ff, FORCEWAKE_RENDER),
+	GEN_FW_RANGE(0xf000, 0xffff, FORCEWAKE_RENDER | FORCEWAKE_MEDIA),
 	GEN_FW_RANGE(0x12000, 0x13fff, FORCEWAKE_MEDIA),
 	GEN_FW_RANGE(0x1a000, 0x1bfff, FORCEWAKE_MEDIA),
 	GEN_FW_RANGE(0x1e800, 0x1e9ff, FORCEWAKE_MEDIA),
 	GEN_FW_RANGE(0x30000, 0x37fff, FORCEWAKE_MEDIA),
-	GEN_FW_RANGE(0x4000, 0x4fff, FORCEWAKE_RENDER | FORCEWAKE_MEDIA),
-	GEN_FW_RANGE(0x8000, 0x82ff, FORCEWAKE_RENDER | FORCEWAKE_MEDIA),
-	GEN_FW_RANGE(0x8500, 0x85ff, FORCEWAKE_RENDER | FORCEWAKE_MEDIA),
-	GEN_FW_RANGE(0x9000, 0xafff, FORCEWAKE_RENDER | FORCEWAKE_MEDIA),
-	GEN_FW_RANGE(0xf000, 0xffff, FORCEWAKE_RENDER | FORCEWAKE_MEDIA),
 };
 
 #define __chv_reg_read_fw_domains(offset) \
@@ -703,23 +722,24 @@ static const struct intel_forcewake_range __chv_fw_ranges[] = {
 	__fwd; \
 })
 
+/* *Must* be sorted by offset ranges! */
 static const struct intel_forcewake_range __gen9_fw_ranges[] = {
 	GEN_FW_RANGE(0xb00, 0x1fff, 0), /* uncore range */
 	GEN_FW_RANGE(0x2000, 0x26ff, FORCEWAKE_RENDER),
 	GEN_FW_RANGE(0x3000, 0x3fff, FORCEWAKE_RENDER),
 	GEN_FW_RANGE(0x5200, 0x7fff, FORCEWAKE_RENDER),
+	GEN_FW_RANGE(0x8130, 0x813f, FORCEWAKE_MEDIA),
 	GEN_FW_RANGE(0x8140, 0x815f, FORCEWAKE_RENDER),
 	GEN_FW_RANGE(0x8300, 0x84ff, FORCEWAKE_RENDER),
+	GEN_FW_RANGE(0x8800, 0x89ff, FORCEWAKE_MEDIA),
 	GEN_FW_RANGE(0x8c00, 0x8cff, FORCEWAKE_RENDER),
-	GEN_FW_RANGE(0xb000, 0xb47f, FORCEWAKE_RENDER),
-	GEN_FW_RANGE(0xe000, 0xe8ff, FORCEWAKE_RENDER),
-	GEN_FW_RANGE(0x24400, 0x247ff, FORCEWAKE_RENDER),
 	GEN_FW_RANGE(0x9400, 0x97ff, FORCEWAKE_RENDER | FORCEWAKE_MEDIA),
-	GEN_FW_RANGE(0x8130, 0x813f, FORCEWAKE_MEDIA),
-	GEN_FW_RANGE(0x8800, 0x89ff, FORCEWAKE_MEDIA),
+	GEN_FW_RANGE(0xb000, 0xb47f, FORCEWAKE_RENDER),
 	GEN_FW_RANGE(0xd000, 0xd7ff, FORCEWAKE_MEDIA),
+	GEN_FW_RANGE(0xe000, 0xe8ff, FORCEWAKE_RENDER),
 	GEN_FW_RANGE(0x12000, 0x13fff, FORCEWAKE_MEDIA),
 	GEN_FW_RANGE(0x1a000, 0x1e9ff, FORCEWAKE_MEDIA),
+	GEN_FW_RANGE(0x24400, 0x247ff, FORCEWAKE_RENDER),
 	GEN_FW_RANGE(0x30000, 0x3ffff, FORCEWAKE_MEDIA),
 };
 
@@ -1299,11 +1319,17 @@ void intel_uncore_init(struct drm_i915_private *dev_priv)
 	switch (INTEL_INFO(dev_priv)->gen) {
 	default:
 	case 9:
+		intel_fw_table_check(__gen9_fw_ranges,
+				     ARRAY_SIZE(__gen9_fw_ranges));
+
 		ASSIGN_WRITE_MMIO_VFUNCS(gen9);
 		ASSIGN_READ_MMIO_VFUNCS(gen9);
 		break;
 	case 8:
 		if (IS_CHERRYVIEW(dev_priv)) {
+			intel_fw_table_check(__chv_fw_ranges,
+					     ARRAY_SIZE(__chv_fw_ranges));
+
 			ASSIGN_WRITE_MMIO_VFUNCS(chv);
 			ASSIGN_READ_MMIO_VFUNCS(chv);
 
@@ -1317,6 +1343,9 @@ void intel_uncore_init(struct drm_i915_private *dev_priv)
 		ASSIGN_WRITE_MMIO_VFUNCS(gen6);
 
 		if (IS_VALLEYVIEW(dev_priv)) {
+			intel_fw_table_check(__vlv_fw_ranges,
+					     ARRAY_SIZE(__vlv_fw_ranges));
+
 			ASSIGN_READ_MMIO_VFUNCS(vlv);
 		} else {
 			ASSIGN_READ_MMIO_VFUNCS(gen6);
-- 
2.7.4

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

Re: [PATCH 05/13] drm/i915: Sort forcewake mapping tables

[Chris Wilson]

On Thu, Sep 29, 2016 at 04:35:48PM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Sorting the tables (verified at runtime to help during
> development) is another prerequisite for interesting
> work which will follow.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_uncore.c | 55 ++++++++++++++++++++++++++++---------
>  1 file changed, 42 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index e21e65ab2a16..bee1482a5ece 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -605,16 +605,34 @@ find_fw_domain(u32 offset, const struct intel_forcewake_range *ranges,
>  	return -1;
>  }
>  
> +static void
> +intel_fw_table_check(const struct intel_forcewake_range *ranges,
> +		     unsigned int num_ranges)
> +{
> +	unsigned int i;
> +	struct intel_forcewake_range *entry =
> +		(struct intel_forcewake_range *)ranges;

Why the const cast away?

> +	s32 prev = -1;
> +
> +	for (i = 0; i < num_ranges; i++, entry++) {
> +		WARN_ON_ONCE(prev >= (s32)entry->start);
> +		prev = entry->start;
> +		WARN_ON_ONCE(prev >= (s32)entry->end);
> +		prev = entry->end;
> +	}

> +/* *Must* be sorted by offset ranges! */

/* *Must* be sorted by offset ranges! See intel_fw_table_check() */

>  static const struct intel_forcewake_range __vlv_fw_ranges[] = {
>  	GEN_FW_RANGE(0x2000, 0x3fff, FORCEWAKE_RENDER),
>  	GEN_FW_RANGE(0x5000, 0x7fff, FORCEWAKE_RENDER),
>  	GEN_FW_RANGE(0xb000, 0x11fff, FORCEWAKE_RENDER),
> -	GEN_FW_RANGE(0x2e000, 0x2ffff, FORCEWAKE_RENDER),
>  	GEN_FW_RANGE(0x12000, 0x13fff, FORCEWAKE_MEDIA),
>  	GEN_FW_RANGE(0x22000, 0x23fff, FORCEWAKE_MEDIA),
> +	GEN_FW_RANGE(0x2e000, 0x2ffff, FORCEWAKE_RENDER),
>  	GEN_FW_RANGE(0x30000, 0x3ffff, FORCEWAKE_MEDIA),
>  };

Ok.

> @@ -660,23 +678,24 @@ static bool is_gen8_shadowed(u32 offset)
>  	__fwd; \
>  })
>  
> +/* *Must* be sorted by offset ranges! */
>  static const struct intel_forcewake_range __chv_fw_ranges[] = {
>  	GEN_FW_RANGE(0x2000, 0x3fff, FORCEWAKE_RENDER),
> +	GEN_FW_RANGE(0x4000, 0x4fff, FORCEWAKE_RENDER | FORCEWAKE_MEDIA),
>  	GEN_FW_RANGE(0x5200, 0x7fff, FORCEWAKE_RENDER),
> +	GEN_FW_RANGE(0x8000, 0x82ff, FORCEWAKE_RENDER | FORCEWAKE_MEDIA),
>  	GEN_FW_RANGE(0x8300, 0x84ff, FORCEWAKE_RENDER),
> -	GEN_FW_RANGE(0xb000, 0xb47f, FORCEWAKE_RENDER),
> -	GEN_FW_RANGE(0xe000, 0xe7ff, FORCEWAKE_RENDER),
> +	GEN_FW_RANGE(0x8500, 0x85ff, FORCEWAKE_RENDER | FORCEWAKE_MEDIA),
>  	GEN_FW_RANGE(0x8800, 0x88ff, FORCEWAKE_MEDIA),
> +	GEN_FW_RANGE(0x9000, 0xafff, FORCEWAKE_RENDER | FORCEWAKE_MEDIA),
> +	GEN_FW_RANGE(0xb000, 0xb47f, FORCEWAKE_RENDER),
>  	GEN_FW_RANGE(0xd000, 0xd7ff, FORCEWAKE_MEDIA),
> +	GEN_FW_RANGE(0xe000, 0xe7ff, FORCEWAKE_RENDER),
> +	GEN_FW_RANGE(0xf000, 0xffff, FORCEWAKE_RENDER | FORCEWAKE_MEDIA),
>  	GEN_FW_RANGE(0x12000, 0x13fff, FORCEWAKE_MEDIA),
>  	GEN_FW_RANGE(0x1a000, 0x1bfff, FORCEWAKE_MEDIA),
>  	GEN_FW_RANGE(0x1e800, 0x1e9ff, FORCEWAKE_MEDIA),
>  	GEN_FW_RANGE(0x30000, 0x37fff, FORCEWAKE_MEDIA),
> -	GEN_FW_RANGE(0x4000, 0x4fff, FORCEWAKE_RENDER | FORCEWAKE_MEDIA),
> -	GEN_FW_RANGE(0x8000, 0x82ff, FORCEWAKE_RENDER | FORCEWAKE_MEDIA),
> -	GEN_FW_RANGE(0x8500, 0x85ff, FORCEWAKE_RENDER | FORCEWAKE_MEDIA),
> -	GEN_FW_RANGE(0x9000, 0xafff, FORCEWAKE_RENDER | FORCEWAKE_MEDIA),
> -	GEN_FW_RANGE(0xf000, 0xffff, FORCEWAKE_RENDER | FORCEWAKE_MEDIA),
>  };

Ok.

>  #define __chv_reg_read_fw_domains(offset) \
> @@ -703,23 +722,24 @@ static const struct intel_forcewake_range __chv_fw_ranges[] = {
>  	__fwd; \
>  })
>  
> +/* *Must* be sorted by offset ranges! */
>  static const struct intel_forcewake_range __gen9_fw_ranges[] = {
>  	GEN_FW_RANGE(0xb00, 0x1fff, 0), /* uncore range */
>  	GEN_FW_RANGE(0x2000, 0x26ff, FORCEWAKE_RENDER),
>  	GEN_FW_RANGE(0x3000, 0x3fff, FORCEWAKE_RENDER),
>  	GEN_FW_RANGE(0x5200, 0x7fff, FORCEWAKE_RENDER),
> +	GEN_FW_RANGE(0x8130, 0x813f, FORCEWAKE_MEDIA),
>  	GEN_FW_RANGE(0x8140, 0x815f, FORCEWAKE_RENDER),
>  	GEN_FW_RANGE(0x8300, 0x84ff, FORCEWAKE_RENDER),
> +	GEN_FW_RANGE(0x8800, 0x89ff, FORCEWAKE_MEDIA),
>  	GEN_FW_RANGE(0x8c00, 0x8cff, FORCEWAKE_RENDER),
> -	GEN_FW_RANGE(0xb000, 0xb47f, FORCEWAKE_RENDER),
> -	GEN_FW_RANGE(0xe000, 0xe8ff, FORCEWAKE_RENDER),
> -	GEN_FW_RANGE(0x24400, 0x247ff, FORCEWAKE_RENDER),
>  	GEN_FW_RANGE(0x9400, 0x97ff, FORCEWAKE_RENDER | FORCEWAKE_MEDIA),
> -	GEN_FW_RANGE(0x8130, 0x813f, FORCEWAKE_MEDIA),
> -	GEN_FW_RANGE(0x8800, 0x89ff, FORCEWAKE_MEDIA),
> +	GEN_FW_RANGE(0xb000, 0xb47f, FORCEWAKE_RENDER),
>  	GEN_FW_RANGE(0xd000, 0xd7ff, FORCEWAKE_MEDIA),
> +	GEN_FW_RANGE(0xe000, 0xe8ff, FORCEWAKE_RENDER),
>  	GEN_FW_RANGE(0x12000, 0x13fff, FORCEWAKE_MEDIA),
>  	GEN_FW_RANGE(0x1a000, 0x1e9ff, FORCEWAKE_MEDIA),
> +	GEN_FW_RANGE(0x24400, 0x247ff, FORCEWAKE_RENDER),
>  	GEN_FW_RANGE(0x30000, 0x3ffff, FORCEWAKE_MEDIA),
>  };

Ok.
  

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

[PATCH 06/13] drm/i915: Use binary search when looking up forcewake domains

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Instead of the existing linear seach, now that we have sorted
range tables, we can do a binary search on them for some
potential miniscule performance gain, but more importantly
for elegance and code size. Hopefully the perfomance gain is
sufficient to offset the function calls which were not there
before.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/intel_uncore.c | 28 ++++++++++++++++++++--------
 1 file changed, 20 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index bee1482a5ece..ae5edaea16f7 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -26,6 +26,7 @@
 #include "i915_vgpu.h"
 
 #include <linux/pm_runtime.h>
+#include <linux/bsearch.h>
 
 #define FORCEWAKE_ACK_TIMEOUT_MS 50
 
@@ -589,20 +590,31 @@ struct intel_forcewake_range
 	enum forcewake_domains domains;
 };
 
+static int fw_range_cmp(const void *key, const void *elt)
+{
+	struct intel_forcewake_range *entry =
+		(struct intel_forcewake_range *)elt;
+	u32 offset = (u32)((unsigned long)key);
+
+	if (offset < entry->start)
+		return -1;
+	else if (offset > entry->end)
+		return 1;
+	else
+		return 0;
+}
+
 static enum forcewake_domains
 find_fw_domain(u32 offset, const struct intel_forcewake_range *ranges,
 	       unsigned int num_ranges)
 {
-	unsigned int i;
-	struct intel_forcewake_range *entry =
-		(struct intel_forcewake_range *)ranges;
+	struct intel_forcewake_range *entry;
 
-	for (i = 0; i < num_ranges; i++, entry++) {
-		if (offset >= entry->start && offset <= entry->end)
-			return entry->domains;
-	}
+	entry = bsearch((void *)(unsigned long)offset, (const void *)ranges,
+			num_ranges, sizeof(struct intel_forcewake_range),
+			fw_range_cmp);
 
-	return -1;
+	return entry ? entry->domains : -1;
 }
 
 static void
-- 
2.7.4

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

Re: [PATCH 06/13] drm/i915: Use binary search when looking up forcewake domains

[Chris Wilson]

On Thu, Sep 29, 2016 at 04:35:49PM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Instead of the existing linear seach, now that we have sorted
> range tables, we can do a binary search on them for some
> potential miniscule performance gain, but more importantly
> for elegance and code size. Hopefully the perfomance gain is
> sufficient to offset the function calls which were not there
> before.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_uncore.c | 28 ++++++++++++++++++++--------
>  1 file changed, 20 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index bee1482a5ece..ae5edaea16f7 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -26,6 +26,7 @@
>  #include "i915_vgpu.h"
>  
>  #include <linux/pm_runtime.h>
> +#include <linux/bsearch.h>
>  
>  #define FORCEWAKE_ACK_TIMEOUT_MS 50
>  
> @@ -589,20 +590,31 @@ struct intel_forcewake_range
>  	enum forcewake_domains domains;
>  };
>  
> +static int fw_range_cmp(const void *key, const void *elt)
> +{
> +	struct intel_forcewake_range *entry =
> +		(struct intel_forcewake_range *)elt;
> +	u32 offset = (u32)((unsigned long)key);
> +
> +	if (offset < entry->start)
> +		return -1;
> +	else if (offset > entry->end)
> +		return 1;
> +	else
> +		return 0;
> +}
> +
>  static enum forcewake_domains
>  find_fw_domain(u32 offset, const struct intel_forcewake_range *ranges,
>  	       unsigned int num_ranges)
>  {
> -	unsigned int i;
> -	struct intel_forcewake_range *entry =
> -		(struct intel_forcewake_range *)ranges;
> +	struct intel_forcewake_range *entry;
>  
> -	for (i = 0; i < num_ranges; i++, entry++) {
> -		if (offset >= entry->start && offset <= entry->end)
> -			return entry->domains;
> -	}
> +	entry = bsearch((void *)(unsigned long)offset, (const void *)ranges,
> +			num_ranges, sizeof(struct intel_forcewake_range),
> +			fw_range_cmp);

How much for bsearch() to be turned into a generator macro?

> -	return -1;
> +	return entry ? entry->domains : -1;
>  }

Looks ok, maybe pass in the default value to return if !entry, saves the
double check.
-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

[PATCH] lib: Typesafe generator macro for bsearch

[Chris Wilson]

Sometimes a callout to a generic bsearch() library function is
substantial overhead for a small search utility. For these situations,
macro generate a type-specific bsearch routine.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: Alessio Igor Bogani <abogani@kernel.org>
---
 include/linux/bsearch.h | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/include/linux/bsearch.h b/include/linux/bsearch.h
index 90b1aa867224..b5669e945c10 100644
--- a/include/linux/bsearch.h
+++ b/include/linux/bsearch.h
@@ -6,4 +6,22 @@
 void *bsearch(const void *key, const void *base, size_t num, size_t size,
 	      int (*cmp)(const void *key, const void *elt));
 
+#define BSEARCH(key, base, num, cmp) ({					\
+	unsigned long start__ = 0, end__ = (num);			\
+	typeof(base) result__ = NULL;					\
+	while (start__ < end__) {					\
+		unsigned long mid__ = (start__ + end__) / 2;		\
+		int ret__ = (cmp)((key), (base) + mid__);		\
+		if (ret__ < 0) {					\
+			end__ = mid__;					\
+		} else if (ret__ > 0) {					\
+			start__ = mid__ + 1;				\
+		} else {						\
+			result__ = (base) + mid__;			\
+			break;						\
+		}							\
+	}								\
+	result__;							\
+})
+
 #endif /* _LINUX_BSEARCH_H */
-- 
2.9.3

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

Re: [PATCH 06/13] drm/i915: Use binary search when looking up forcewake domains

[Tvrtko Ursulin]

On 29/09/2016 17:16, Chris Wilson wrote:
> On Thu, Sep 29, 2016 at 04:35:49PM +0100, Tvrtko Ursulin wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> Instead of the existing linear seach, now that we have sorted
>> range tables, we can do a binary search on them for some
>> potential miniscule performance gain, but more importantly
>> for elegance and code size. Hopefully the perfomance gain is
>> sufficient to offset the function calls which were not there
>> before.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_uncore.c | 28 ++++++++++++++++++++--------
>>   1 file changed, 20 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
>> index bee1482a5ece..ae5edaea16f7 100644
>> --- a/drivers/gpu/drm/i915/intel_uncore.c
>> +++ b/drivers/gpu/drm/i915/intel_uncore.c
>> @@ -26,6 +26,7 @@
>>   #include "i915_vgpu.h"
>>   
>>   #include <linux/pm_runtime.h>
>> +#include <linux/bsearch.h>
>>   
>>   #define FORCEWAKE_ACK_TIMEOUT_MS 50
>>   
>> @@ -589,20 +590,31 @@ struct intel_forcewake_range
>>   	enum forcewake_domains domains;
>>   };
>>   
>> +static int fw_range_cmp(const void *key, const void *elt)
>> +{
>> +	struct intel_forcewake_range *entry =
>> +		(struct intel_forcewake_range *)elt;
>> +	u32 offset = (u32)((unsigned long)key);
>> +
>> +	if (offset < entry->start)
>> +		return -1;
>> +	else if (offset > entry->end)
>> +		return 1;
>> +	else
>> +		return 0;
>> +}
>> +
>>   static enum forcewake_domains
>>   find_fw_domain(u32 offset, const struct intel_forcewake_range *ranges,
>>   	       unsigned int num_ranges)
>>   {
>> -	unsigned int i;
>> -	struct intel_forcewake_range *entry =
>> -		(struct intel_forcewake_range *)ranges;
>> +	struct intel_forcewake_range *entry;
>>   
>> -	for (i = 0; i < num_ranges; i++, entry++) {
>> -		if (offset >= entry->start && offset <= entry->end)
>> -			return entry->domains;
>> -	}
>> +	entry = bsearch((void *)(unsigned long)offset, (const void *)ranges,
>> +			num_ranges, sizeof(struct intel_forcewake_range),
>> +			fw_range_cmp);
> How much for bsearch() to be turned into a generator macro?

By default it is a small code size win (128 bytes). It makes 
find_fw_domain a function with an inlined comparator (so one function 
call less per search iteration than using library bsearch) and inlines 
is_gen8_shadowed completely.

Forcing find_fw_domain to be fully inline adds approximately 1k.

I am not sure - you think it is worth doing some of the above? Function 
calls are supposed to be cheap so perhaps just with the default 
inlining, but then it is either pushing the core patch or having a local 
copy of a macro.

>> -	return -1;
>> +	return entry ? entry->domains : -1;
>>   }
> Looks ok, maybe pass in the default value to return if !entry, saves the
> double check.

It goes away later in the series so it is fine. I just wanted to be 
gradual so it is easy to review.

Regards,

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

Re: [PATCH 06/13] drm/i915: Use binary search when looking up forcewake domains

[Chris Wilson]

On Fri, Sep 30, 2016 at 12:08:26PM +0100, Tvrtko Ursulin wrote:
> 
> On 29/09/2016 17:16, Chris Wilson wrote:
> >On Thu, Sep 29, 2016 at 04:35:49PM +0100, Tvrtko Ursulin wrote:
> >>+	entry = bsearch((void *)(unsigned long)offset, (const void *)ranges,
> >>+			num_ranges, sizeof(struct intel_forcewake_range),
> >>+			fw_range_cmp);
> >How much for bsearch() to be turned into a generator macro?
> 
> By default it is a small code size win (128 bytes). It makes
> find_fw_domain a function with an inlined comparator (so one
> function call less per search iteration than using library bsearch)
> and inlines is_gen8_shadowed completely.
> 
> Forcing find_fw_domain to be fully inline adds approximately 1k.
> 
> I am not sure - you think it is worth doing some of the above?
> Function calls are supposed to be cheap so perhaps just with the
> default inlining, but then it is either pushing the core patch or
> having a local copy of a macro.

But I wonder if we get better branch predictor from inlining.

Happy enough with the default inlining, whilst we are in the noise
compared to the actual fw and mmio, we might as well try to keep the irq
paths trim at least.
-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

[PATCH 07/13] drm/i915: Eliminate Gen9 special case

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

If we insert blitter forcewake domain entries in the range
table we can eliminate that special case and simplify the
code in a few macros. This will enable more unification later.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/intel_uncore.c | 42 ++++++++++++++++++-------------------
 1 file changed, 21 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index ae5edaea16f7..a41a6a2019be 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -614,7 +614,7 @@ find_fw_domain(u32 offset, const struct intel_forcewake_range *ranges,
 			num_ranges, sizeof(struct intel_forcewake_range),
 			fw_range_cmp);
 
-	return entry ? entry->domains : -1;
+	return entry ? entry->domains : 0;
 }
 
 static void
@@ -651,12 +651,9 @@ static const struct intel_forcewake_range __vlv_fw_ranges[] = {
 #define __vlv_reg_read_fw_domains(offset) \
 ({ \
 	enum forcewake_domains __fwd = 0; \
-	if (NEEDS_FORCE_WAKE((offset))) { \
+	if (NEEDS_FORCE_WAKE((offset))) \
 		__fwd = find_fw_domain(offset, __vlv_fw_ranges, \
 				       ARRAY_SIZE(__vlv_fw_ranges)); \
-		if (__fwd == -1 ) \
-			__fwd = 0; \
-	} \
 	__fwd; \
 })
 
@@ -713,57 +710,63 @@ static const struct intel_forcewake_range __chv_fw_ranges[] = {
 #define __chv_reg_read_fw_domains(offset) \
 ({ \
 	enum forcewake_domains __fwd = 0; \
-	if (NEEDS_FORCE_WAKE((offset))) { \
+	if (NEEDS_FORCE_WAKE((offset))) \
 		__fwd = find_fw_domain(offset, __chv_fw_ranges, \
 				       ARRAY_SIZE(__chv_fw_ranges)); \
-		if (__fwd == -1 ) \
-			__fwd = 0; \
-	} \
 	__fwd; \
 })
 
 #define __chv_reg_write_fw_domains(offset) \
 ({ \
 	enum forcewake_domains __fwd = 0; \
-	if (NEEDS_FORCE_WAKE((offset)) && !is_gen8_shadowed(offset)) { \
+	if (NEEDS_FORCE_WAKE((offset)) && !is_gen8_shadowed(offset)) \
 		__fwd = find_fw_domain(offset, __chv_fw_ranges, \
 				       ARRAY_SIZE(__chv_fw_ranges)); \
-		if (__fwd == -1 ) \
-			__fwd = 0; \
-	} \
 	__fwd; \
 })
 
 /* *Must* be sorted by offset ranges! */
 static const struct intel_forcewake_range __gen9_fw_ranges[] = {
+	GEN_FW_RANGE(0x0, 0xaff, FORCEWAKE_BLITTER),
 	GEN_FW_RANGE(0xb00, 0x1fff, 0), /* uncore range */
 	GEN_FW_RANGE(0x2000, 0x26ff, FORCEWAKE_RENDER),
+	GEN_FW_RANGE(0x2700, 0x2fff, FORCEWAKE_BLITTER),
 	GEN_FW_RANGE(0x3000, 0x3fff, FORCEWAKE_RENDER),
+	GEN_FW_RANGE(0x4000, 0x51ff, FORCEWAKE_BLITTER),
 	GEN_FW_RANGE(0x5200, 0x7fff, FORCEWAKE_RENDER),
+	GEN_FW_RANGE(0x8000, 0x812f, FORCEWAKE_BLITTER),
 	GEN_FW_RANGE(0x8130, 0x813f, FORCEWAKE_MEDIA),
 	GEN_FW_RANGE(0x8140, 0x815f, FORCEWAKE_RENDER),
+	GEN_FW_RANGE(0x8160, 0x82ff, FORCEWAKE_BLITTER),
 	GEN_FW_RANGE(0x8300, 0x84ff, FORCEWAKE_RENDER),
+	GEN_FW_RANGE(0x8500, 0x87ff, FORCEWAKE_BLITTER),
 	GEN_FW_RANGE(0x8800, 0x89ff, FORCEWAKE_MEDIA),
+	GEN_FW_RANGE(0x8a00, 0x8bff, FORCEWAKE_BLITTER),
 	GEN_FW_RANGE(0x8c00, 0x8cff, FORCEWAKE_RENDER),
+	GEN_FW_RANGE(0x8d00, 0x93ff, FORCEWAKE_BLITTER),
 	GEN_FW_RANGE(0x9400, 0x97ff, FORCEWAKE_RENDER | FORCEWAKE_MEDIA),
+	GEN_FW_RANGE(0x9800, 0xafff, FORCEWAKE_BLITTER),
 	GEN_FW_RANGE(0xb000, 0xb47f, FORCEWAKE_RENDER),
+	GEN_FW_RANGE(0xb480, 0xbfff, FORCEWAKE_BLITTER),
 	GEN_FW_RANGE(0xd000, 0xd7ff, FORCEWAKE_MEDIA),
+	GEN_FW_RANGE(0xd800, 0xdfff, FORCEWAKE_BLITTER),
 	GEN_FW_RANGE(0xe000, 0xe8ff, FORCEWAKE_RENDER),
+	GEN_FW_RANGE(0xe900, 0x11fff, FORCEWAKE_BLITTER),
 	GEN_FW_RANGE(0x12000, 0x13fff, FORCEWAKE_MEDIA),
+	GEN_FW_RANGE(0x14000, 0x19fff, FORCEWAKE_BLITTER),
 	GEN_FW_RANGE(0x1a000, 0x1e9ff, FORCEWAKE_MEDIA),
+	GEN_FW_RANGE(0x1ea00, 0x243ff, FORCEWAKE_BLITTER),
 	GEN_FW_RANGE(0x24400, 0x247ff, FORCEWAKE_RENDER),
+	GEN_FW_RANGE(0x24800, 0x2ffff, FORCEWAKE_BLITTER),
 	GEN_FW_RANGE(0x30000, 0x3ffff, FORCEWAKE_MEDIA),
 };
 
 #define __gen9_reg_read_fw_domains(offset) \
 ({ \
 	enum forcewake_domains __fwd = 0; \
-	if (NEEDS_FORCE_WAKE((offset))) { \
+	if (NEEDS_FORCE_WAKE((offset))) \
 		__fwd = find_fw_domain(offset, __gen9_fw_ranges, \
 				       ARRAY_SIZE(__gen9_fw_ranges)); \
-		if (__fwd == -1 ) \
-			__fwd = FORCEWAKE_BLITTER; \
-	} \
 	__fwd; \
 })
 
@@ -790,12 +793,9 @@ static bool is_gen9_shadowed(u32 offset)
 #define __gen9_reg_write_fw_domains(offset) \
 ({ \
 	enum forcewake_domains __fwd = 0; \
-	if (NEEDS_FORCE_WAKE((offset)) && !is_gen9_shadowed(offset)) { \
+	if (NEEDS_FORCE_WAKE((offset)) && !is_gen9_shadowed(offset)) \
 		__fwd = find_fw_domain(offset, __gen9_fw_ranges, \
 				       ARRAY_SIZE(__gen9_fw_ranges)); \
-		if (__fwd == -1 ) \
-			__fwd = FORCEWAKE_BLITTER; \
-	} \
 	__fwd; \
 })
 
-- 
2.7.4

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

Re: [PATCH 07/13] drm/i915: Eliminate Gen9 special case

[Chris Wilson]

On Thu, Sep 29, 2016 at 04:35:50PM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> If we insert blitter forcewake domain entries in the range
> table we can eliminate that special case and simplify the
> code in a few macros. This will enable more unification later.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_uncore.c | 42 ++++++++++++++++++-------------------
>  1 file changed, 21 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index ae5edaea16f7..a41a6a2019be 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -614,7 +614,7 @@ find_fw_domain(u32 offset, const struct intel_forcewake_range *ranges,
>  			num_ranges, sizeof(struct intel_forcewake_range),
>  			fw_range_cmp);
>  
> -	return entry ? entry->domains : -1;
> +	return entry ? entry->domains : 0;
>  }
>  
>  static void
> @@ -651,12 +651,9 @@ static const struct intel_forcewake_range __vlv_fw_ranges[] = {
>  #define __vlv_reg_read_fw_domains(offset) \
>  ({ \
>  	enum forcewake_domains __fwd = 0; \
> -	if (NEEDS_FORCE_WAKE((offset))) { \
> +	if (NEEDS_FORCE_WAKE((offset))) \
>  		__fwd = find_fw_domain(offset, __vlv_fw_ranges, \
>  				       ARRAY_SIZE(__vlv_fw_ranges)); \
> -		if (__fwd == -1 ) \
> -			__fwd = 0; \
> -	} \
>  	__fwd; \
>  })
>  
> @@ -713,57 +710,63 @@ static const struct intel_forcewake_range __chv_fw_ranges[] = {
>  #define __chv_reg_read_fw_domains(offset) \
>  ({ \
>  	enum forcewake_domains __fwd = 0; \
> -	if (NEEDS_FORCE_WAKE((offset))) { \
> +	if (NEEDS_FORCE_WAKE((offset))) \
>  		__fwd = find_fw_domain(offset, __chv_fw_ranges, \
>  				       ARRAY_SIZE(__chv_fw_ranges)); \
> -		if (__fwd == -1 ) \
> -			__fwd = 0; \
> -	} \
>  	__fwd; \
>  })
>  
>  #define __chv_reg_write_fw_domains(offset) \
>  ({ \
>  	enum forcewake_domains __fwd = 0; \
> -	if (NEEDS_FORCE_WAKE((offset)) && !is_gen8_shadowed(offset)) { \
> +	if (NEEDS_FORCE_WAKE((offset)) && !is_gen8_shadowed(offset)) \
>  		__fwd = find_fw_domain(offset, __chv_fw_ranges, \
>  				       ARRAY_SIZE(__chv_fw_ranges)); \
> -		if (__fwd == -1 ) \
> -			__fwd = 0; \
> -	} \
>  	__fwd; \
>  })
>  
>  /* *Must* be sorted by offset ranges! */
>  static const struct intel_forcewake_range __gen9_fw_ranges[] = {
> +	GEN_FW_RANGE(0x0, 0xaff, FORCEWAKE_BLITTER),
>  	GEN_FW_RANGE(0xb00, 0x1fff, 0), /* uncore range */
>  	GEN_FW_RANGE(0x2000, 0x26ff, FORCEWAKE_RENDER),
> +	GEN_FW_RANGE(0x2700, 0x2fff, FORCEWAKE_BLITTER),
>  	GEN_FW_RANGE(0x3000, 0x3fff, FORCEWAKE_RENDER),
> +	GEN_FW_RANGE(0x4000, 0x51ff, FORCEWAKE_BLITTER),
>  	GEN_FW_RANGE(0x5200, 0x7fff, FORCEWAKE_RENDER),
> +	GEN_FW_RANGE(0x8000, 0x812f, FORCEWAKE_BLITTER),
>  	GEN_FW_RANGE(0x8130, 0x813f, FORCEWAKE_MEDIA),
>  	GEN_FW_RANGE(0x8140, 0x815f, FORCEWAKE_RENDER),
> +	GEN_FW_RANGE(0x8160, 0x82ff, FORCEWAKE_BLITTER),
>  	GEN_FW_RANGE(0x8300, 0x84ff, FORCEWAKE_RENDER),
> +	GEN_FW_RANGE(0x8500, 0x87ff, FORCEWAKE_BLITTER),
>  	GEN_FW_RANGE(0x8800, 0x89ff, FORCEWAKE_MEDIA),
> +	GEN_FW_RANGE(0x8a00, 0x8bff, FORCEWAKE_BLITTER),
>  	GEN_FW_RANGE(0x8c00, 0x8cff, FORCEWAKE_RENDER),
> +	GEN_FW_RANGE(0x8d00, 0x93ff, FORCEWAKE_BLITTER),
>  	GEN_FW_RANGE(0x9400, 0x97ff, FORCEWAKE_RENDER | FORCEWAKE_MEDIA),
> +	GEN_FW_RANGE(0x9800, 0xafff, FORCEWAKE_BLITTER),
>  	GEN_FW_RANGE(0xb000, 0xb47f, FORCEWAKE_RENDER),
> +	GEN_FW_RANGE(0xb480, 0xbfff, FORCEWAKE_BLITTER),
>  	GEN_FW_RANGE(0xd000, 0xd7ff, FORCEWAKE_MEDIA),
> +	GEN_FW_RANGE(0xd800, 0xdfff, FORCEWAKE_BLITTER),
>  	GEN_FW_RANGE(0xe000, 0xe8ff, FORCEWAKE_RENDER),
> +	GEN_FW_RANGE(0xe900, 0x11fff, FORCEWAKE_BLITTER),
>  	GEN_FW_RANGE(0x12000, 0x13fff, FORCEWAKE_MEDIA),
> +	GEN_FW_RANGE(0x14000, 0x19fff, FORCEWAKE_BLITTER),
>  	GEN_FW_RANGE(0x1a000, 0x1e9ff, FORCEWAKE_MEDIA),
> +	GEN_FW_RANGE(0x1ea00, 0x243ff, FORCEWAKE_BLITTER),
>  	GEN_FW_RANGE(0x24400, 0x247ff, FORCEWAKE_RENDER),
> +	GEN_FW_RANGE(0x24800, 0x2ffff, FORCEWAKE_BLITTER),
>  	GEN_FW_RANGE(0x30000, 0x3ffff, FORCEWAKE_MEDIA),
>  };
>  
>  #define __gen9_reg_read_fw_domains(offset) \
>  ({ \
>  	enum forcewake_domains __fwd = 0; \
> -	if (NEEDS_FORCE_WAKE((offset))) { \
> +	if (NEEDS_FORCE_WAKE((offset))) \
>  		__fwd = find_fw_domain(offset, __gen9_fw_ranges, \
>  				       ARRAY_SIZE(__gen9_fw_ranges)); \
> -		if (__fwd == -1 ) \
> -			__fwd = FORCEWAKE_BLITTER; \

Hmm. Leaving the holes halves the table size, and saves approximately
one pass... :|

Being explicit in the declaration is better, so lgtm.
-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

[PATCH 08/13] drm/i915: Store the active forcewake range table pointer

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

If we store this in the uncore structure we are on a good way to
show more commonality between the per-platform implementations.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h     | 11 ++++++
 drivers/gpu/drm/i915/intel_uncore.c | 74 ++++++++++++++++++-------------------
 2 files changed, 47 insertions(+), 38 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 4cd727376d1d..7c8bd07767fe 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -582,9 +582,20 @@ struct intel_uncore_funcs {
 				uint32_t val, bool trace);
 };
 
+struct intel_forcewake_range
+{
+	u32 start;
+	u32 end;
+
+	enum forcewake_domains domains;
+};
+
 struct intel_uncore {
 	spinlock_t lock; /** lock is also taken in irq contexts. */
 
+	struct intel_forcewake_range *fw_domains_table;
+	unsigned int fw_domains_table_entries;
+
 	struct intel_uncore_funcs funcs;
 
 	unsigned fifo_count;
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index a41a6a2019be..2197c0a43dda 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -582,14 +582,6 @@ void assert_forcewakes_inactive(struct drm_i915_private *dev_priv)
 	__fwd; \
 })
 
-struct intel_forcewake_range
-{
-	u32 start;
-	u32 end;
-
-	enum forcewake_domains domains;
-};
-
 static int fw_range_cmp(const void *key, const void *elt)
 {
 	struct intel_forcewake_range *entry =
@@ -605,28 +597,36 @@ static int fw_range_cmp(const void *key, const void *elt)
 }
 
 static enum forcewake_domains
-find_fw_domain(u32 offset, const struct intel_forcewake_range *ranges,
-	       unsigned int num_ranges)
+find_fw_domain(struct drm_i915_private *dev_priv, u32 offset)
 {
-	struct intel_forcewake_range *entry;
+	struct intel_forcewake_range *table, *entry;
+	unsigned int num_entries;
 
-	entry = bsearch((void *)(unsigned long)offset, (const void *)ranges,
-			num_ranges, sizeof(struct intel_forcewake_range),
+	table = dev_priv->uncore.fw_domains_table;
+	num_entries = dev_priv->uncore.fw_domains_table_entries;
+
+	entry = bsearch((void *)(unsigned long)offset, (const void *)table,
+			num_entries, sizeof(struct intel_forcewake_range),
 			fw_range_cmp);
 
 	return entry ? entry->domains : 0;
 }
 
 static void
-intel_fw_table_check(const struct intel_forcewake_range *ranges,
-		     unsigned int num_ranges)
+intel_fw_table_check(struct drm_i915_private *dev_priv)
 {
 	unsigned int i;
-	struct intel_forcewake_range *entry =
-		(struct intel_forcewake_range *)ranges;
-	s32 prev = -1;
+	struct intel_forcewake_range *entry;
+	unsigned int num_ranges;
+	s32 prev;
 
-	for (i = 0; i < num_ranges; i++, entry++) {
+	entry = dev_priv->uncore.fw_domains_table;
+	if (!entry)
+		return;
+
+	num_ranges = dev_priv->uncore.fw_domains_table_entries;
+
+	for (i = 0, prev = -1; i < num_ranges; i++, entry++) {
 		WARN_ON_ONCE(prev >= (s32)entry->start);
 		prev = entry->start;
 		WARN_ON_ONCE(prev >= (s32)entry->end);
@@ -652,8 +652,7 @@ static const struct intel_forcewake_range __vlv_fw_ranges[] = {
 ({ \
 	enum forcewake_domains __fwd = 0; \
 	if (NEEDS_FORCE_WAKE((offset))) \
-		__fwd = find_fw_domain(offset, __vlv_fw_ranges, \
-				       ARRAY_SIZE(__vlv_fw_ranges)); \
+		__fwd = find_fw_domain(dev_priv, offset); \
 	__fwd; \
 })
 
@@ -711,8 +710,7 @@ static const struct intel_forcewake_range __chv_fw_ranges[] = {
 ({ \
 	enum forcewake_domains __fwd = 0; \
 	if (NEEDS_FORCE_WAKE((offset))) \
-		__fwd = find_fw_domain(offset, __chv_fw_ranges, \
-				       ARRAY_SIZE(__chv_fw_ranges)); \
+		__fwd = find_fw_domain(dev_priv, offset); \
 	__fwd; \
 })
 
@@ -720,8 +718,7 @@ static const struct intel_forcewake_range __chv_fw_ranges[] = {
 ({ \
 	enum forcewake_domains __fwd = 0; \
 	if (NEEDS_FORCE_WAKE((offset)) && !is_gen8_shadowed(offset)) \
-		__fwd = find_fw_domain(offset, __chv_fw_ranges, \
-				       ARRAY_SIZE(__chv_fw_ranges)); \
+		__fwd = find_fw_domain(dev_priv, offset); \
 	__fwd; \
 })
 
@@ -765,8 +762,7 @@ static const struct intel_forcewake_range __gen9_fw_ranges[] = {
 ({ \
 	enum forcewake_domains __fwd = 0; \
 	if (NEEDS_FORCE_WAKE((offset))) \
-		__fwd = find_fw_domain(offset, __gen9_fw_ranges, \
-				       ARRAY_SIZE(__gen9_fw_ranges)); \
+		__fwd = find_fw_domain(dev_priv, offset); \
 	__fwd; \
 })
 
@@ -794,8 +790,7 @@ static bool is_gen9_shadowed(u32 offset)
 ({ \
 	enum forcewake_domains __fwd = 0; \
 	if (NEEDS_FORCE_WAKE((offset)) && !is_gen9_shadowed(offset)) \
-		__fwd = find_fw_domain(offset, __gen9_fw_ranges, \
-				       ARRAY_SIZE(__gen9_fw_ranges)); \
+		__fwd = find_fw_domain(dev_priv, offset); \
 	__fwd; \
 })
 
@@ -1318,6 +1313,13 @@ static void intel_uncore_fw_domains_init(struct drm_i915_private *dev_priv)
 	WARN_ON(dev_priv->uncore.fw_domains == 0);
 }
 
+#define ASSIGN_FW_DOMAINS_TABLE(d) \
+{ \
+	dev_priv->uncore.fw_domains_table = \
+			(struct intel_forcewake_range *)(d); \
+	dev_priv->uncore.fw_domains_table_entries = ARRAY_SIZE((d)); \
+}
+
 void intel_uncore_init(struct drm_i915_private *dev_priv)
 {
 	i915_check_vgpu(dev_priv);
@@ -1331,17 +1333,13 @@ void intel_uncore_init(struct drm_i915_private *dev_priv)
 	switch (INTEL_INFO(dev_priv)->gen) {
 	default:
 	case 9:
-		intel_fw_table_check(__gen9_fw_ranges,
-				     ARRAY_SIZE(__gen9_fw_ranges));
-
+		ASSIGN_FW_DOMAINS_TABLE(__gen9_fw_ranges);
 		ASSIGN_WRITE_MMIO_VFUNCS(gen9);
 		ASSIGN_READ_MMIO_VFUNCS(gen9);
 		break;
 	case 8:
 		if (IS_CHERRYVIEW(dev_priv)) {
-			intel_fw_table_check(__chv_fw_ranges,
-					     ARRAY_SIZE(__chv_fw_ranges));
-
+			ASSIGN_FW_DOMAINS_TABLE(__chv_fw_ranges);
 			ASSIGN_WRITE_MMIO_VFUNCS(chv);
 			ASSIGN_READ_MMIO_VFUNCS(chv);
 
@@ -1355,9 +1353,7 @@ void intel_uncore_init(struct drm_i915_private *dev_priv)
 		ASSIGN_WRITE_MMIO_VFUNCS(gen6);
 
 		if (IS_VALLEYVIEW(dev_priv)) {
-			intel_fw_table_check(__vlv_fw_ranges,
-					     ARRAY_SIZE(__vlv_fw_ranges));
-
+			ASSIGN_FW_DOMAINS_TABLE(__vlv_fw_ranges);
 			ASSIGN_READ_MMIO_VFUNCS(vlv);
 		} else {
 			ASSIGN_READ_MMIO_VFUNCS(gen6);
@@ -1375,6 +1371,8 @@ void intel_uncore_init(struct drm_i915_private *dev_priv)
 		break;
 	}
 
+	intel_fw_table_check(dev_priv);
+
 	if (intel_vgpu_active(dev_priv)) {
 		ASSIGN_WRITE_MMIO_VFUNCS(vgpu);
 		ASSIGN_READ_MMIO_VFUNCS(vgpu);
-- 
2.7.4

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

Re: [PATCH 08/13] drm/i915: Store the active forcewake range table pointer

[Chris Wilson]

On Thu, Sep 29, 2016 at 04:35:51PM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> If we store this in the uncore structure we are on a good way to
> show more commonality between the per-platform implementations.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h     | 11 ++++++
>  drivers/gpu/drm/i915/intel_uncore.c | 74 ++++++++++++++++++-------------------
>  2 files changed, 47 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 4cd727376d1d..7c8bd07767fe 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -582,9 +582,20 @@ struct intel_uncore_funcs {
>  				uint32_t val, bool trace);
>  };
>  
> +struct intel_forcewake_range
> +{

struct intel_forcewake_range {

> +	u32 start;
> +	u32 end;
> +
> +	enum forcewake_domains domains;
> +};
> +
>  struct intel_uncore {
>  	spinlock_t lock; /** lock is also taken in irq contexts. */
>  
> +	struct intel_forcewake_range *fw_domains_table;
> +	unsigned int fw_domains_table_entries;

constify?

> +
>  	struct intel_uncore_funcs funcs;
>  
>  	unsigned fifo_count;
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index a41a6a2019be..2197c0a43dda 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -582,14 +582,6 @@ void assert_forcewakes_inactive(struct drm_i915_private *dev_priv)
>  	__fwd; \
>  })
>  
> -struct intel_forcewake_range
> -{
> -	u32 start;
> -	u32 end;
> -
> -	enum forcewake_domains domains;
> -};
> -
>  static int fw_range_cmp(const void *key, const void *elt)
>  {
>  	struct intel_forcewake_range *entry =
> @@ -605,28 +597,36 @@ static int fw_range_cmp(const void *key, const void *elt)
>  }
>  
>  static enum forcewake_domains
> -find_fw_domain(u32 offset, const struct intel_forcewake_range *ranges,
> -	       unsigned int num_ranges)
> +find_fw_domain(struct drm_i915_private *dev_priv, u32 offset)
>  {
> -	struct intel_forcewake_range *entry;
> +	struct intel_forcewake_range *table, *entry;
> +	unsigned int num_entries;
>  
> -	entry = bsearch((void *)(unsigned long)offset, (const void *)ranges,
> -			num_ranges, sizeof(struct intel_forcewake_range),
> +	table = dev_priv->uncore.fw_domains_table;
> +	num_entries = dev_priv->uncore.fw_domains_table_entries;
> +
> +	entry = bsearch((void *)(unsigned long)offset, (const void *)table,
> +			num_entries, sizeof(struct intel_forcewake_range),
>  			fw_range_cmp);
>  
>  	return entry ? entry->domains : 0;
>  }
>  
>  static void
> -intel_fw_table_check(const struct intel_forcewake_range *ranges,
> -		     unsigned int num_ranges)
> +intel_fw_table_check(struct drm_i915_private *dev_priv)
>  {
>  	unsigned int i;
> -	struct intel_forcewake_range *entry =
> -		(struct intel_forcewake_range *)ranges;
> -	s32 prev = -1;
> +	struct intel_forcewake_range *entry;
> +	unsigned int num_ranges;
> +	s32 prev;
>  
> -	for (i = 0; i < num_ranges; i++, entry++) {
> +	entry = dev_priv->uncore.fw_domains_table;
> +	if (!entry)
> +		return;
> +
> +	num_ranges = dev_priv->uncore.fw_domains_table_entries;
> +
> +	for (i = 0, prev = -1; i < num_ranges; i++, entry++) {
>  		WARN_ON_ONCE(prev >= (s32)entry->start);
>  		prev = entry->start;
>  		WARN_ON_ONCE(prev >= (s32)entry->end);
> @@ -652,8 +652,7 @@ static const struct intel_forcewake_range __vlv_fw_ranges[] = {
>  ({ \
>  	enum forcewake_domains __fwd = 0; \
>  	if (NEEDS_FORCE_WAKE((offset))) \
> -		__fwd = find_fw_domain(offset, __vlv_fw_ranges, \
> -				       ARRAY_SIZE(__vlv_fw_ranges)); \
> +		__fwd = find_fw_domain(dev_priv, offset); \
>  	__fwd; \
>  })
>  
> @@ -711,8 +710,7 @@ static const struct intel_forcewake_range __chv_fw_ranges[] = {
>  ({ \
>  	enum forcewake_domains __fwd = 0; \
>  	if (NEEDS_FORCE_WAKE((offset))) \
> -		__fwd = find_fw_domain(offset, __chv_fw_ranges, \
> -				       ARRAY_SIZE(__chv_fw_ranges)); \
> +		__fwd = find_fw_domain(dev_priv, offset); \
>  	__fwd; \
>  })
>  
> @@ -720,8 +718,7 @@ static const struct intel_forcewake_range __chv_fw_ranges[] = {
>  ({ \
>  	enum forcewake_domains __fwd = 0; \
>  	if (NEEDS_FORCE_WAKE((offset)) && !is_gen8_shadowed(offset)) \
> -		__fwd = find_fw_domain(offset, __chv_fw_ranges, \
> -				       ARRAY_SIZE(__chv_fw_ranges)); \
> +		__fwd = find_fw_domain(dev_priv, offset); \
>  	__fwd; \
>  })
>  
> @@ -765,8 +762,7 @@ static const struct intel_forcewake_range __gen9_fw_ranges[] = {
>  ({ \
>  	enum forcewake_domains __fwd = 0; \
>  	if (NEEDS_FORCE_WAKE((offset))) \
> -		__fwd = find_fw_domain(offset, __gen9_fw_ranges, \
> -				       ARRAY_SIZE(__gen9_fw_ranges)); \
> +		__fwd = find_fw_domain(dev_priv, offset); \
>  	__fwd; \
>  })
>  
> @@ -794,8 +790,7 @@ static bool is_gen9_shadowed(u32 offset)
>  ({ \
>  	enum forcewake_domains __fwd = 0; \
>  	if (NEEDS_FORCE_WAKE((offset)) && !is_gen9_shadowed(offset)) \
> -		__fwd = find_fw_domain(offset, __gen9_fw_ranges, \
> -				       ARRAY_SIZE(__gen9_fw_ranges)); \
> +		__fwd = find_fw_domain(dev_priv, offset); \
>  	__fwd; \
>  })
>  
> @@ -1318,6 +1313,13 @@ static void intel_uncore_fw_domains_init(struct drm_i915_private *dev_priv)
>  	WARN_ON(dev_priv->uncore.fw_domains == 0);
>  }
>  
> +#define ASSIGN_FW_DOMAINS_TABLE(d) \
> +{ \
> +	dev_priv->uncore.fw_domains_table = \
> +			(struct intel_forcewake_range *)(d); \
> +	dev_priv->uncore.fw_domains_table_entries = ARRAY_SIZE((d)); \
> +}
> +
>  void intel_uncore_init(struct drm_i915_private *dev_priv)
>  {
>  	i915_check_vgpu(dev_priv);
> @@ -1331,17 +1333,13 @@ void intel_uncore_init(struct drm_i915_private *dev_priv)
>  	switch (INTEL_INFO(dev_priv)->gen) {
>  	default:
>  	case 9:
> -		intel_fw_table_check(__gen9_fw_ranges,
> -				     ARRAY_SIZE(__gen9_fw_ranges));
> -
> +		ASSIGN_FW_DOMAINS_TABLE(__gen9_fw_ranges);
>  		ASSIGN_WRITE_MMIO_VFUNCS(gen9);
>  		ASSIGN_READ_MMIO_VFUNCS(gen9);
>  		break;
>  	case 8:
>  		if (IS_CHERRYVIEW(dev_priv)) {
> -			intel_fw_table_check(__chv_fw_ranges,
> -					     ARRAY_SIZE(__chv_fw_ranges));
> -
> +			ASSIGN_FW_DOMAINS_TABLE(__chv_fw_ranges);
>  			ASSIGN_WRITE_MMIO_VFUNCS(chv);
>  			ASSIGN_READ_MMIO_VFUNCS(chv);
>  
> @@ -1355,9 +1353,7 @@ void intel_uncore_init(struct drm_i915_private *dev_priv)
>  		ASSIGN_WRITE_MMIO_VFUNCS(gen6);
>  
>  		if (IS_VALLEYVIEW(dev_priv)) {
> -			intel_fw_table_check(__vlv_fw_ranges,
> -					     ARRAY_SIZE(__vlv_fw_ranges));
> -
> +			ASSIGN_FW_DOMAINS_TABLE(__vlv_fw_ranges);
>  			ASSIGN_READ_MMIO_VFUNCS(vlv);
>  		} else {
>  			ASSIGN_READ_MMIO_VFUNCS(gen6);
> @@ -1375,6 +1371,8 @@ void intel_uncore_init(struct drm_i915_private *dev_priv)
>  		break;
>  	}
>  
> +	intel_fw_table_check(dev_priv);

Looking better...
-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

[PATCH 09/13] drm/i915: Remove identical macros

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Remove some macros which are now obviously identical.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/intel_uncore.c | 30 +++++++-----------------------
 1 file changed, 7 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index 2197c0a43dda..a4ce3576f8d4 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -648,7 +648,7 @@ static const struct intel_forcewake_range __vlv_fw_ranges[] = {
 	GEN_FW_RANGE(0x30000, 0x3ffff, FORCEWAKE_MEDIA),
 };
 
-#define __vlv_reg_read_fw_domains(offset) \
+#define __fwtable_reg_read_fw_domains(offset) \
 ({ \
 	enum forcewake_domains __fwd = 0; \
 	if (NEEDS_FORCE_WAKE((offset))) \
@@ -706,14 +706,6 @@ static const struct intel_forcewake_range __chv_fw_ranges[] = {
 	GEN_FW_RANGE(0x30000, 0x37fff, FORCEWAKE_MEDIA),
 };
 
-#define __chv_reg_read_fw_domains(offset) \
-({ \
-	enum forcewake_domains __fwd = 0; \
-	if (NEEDS_FORCE_WAKE((offset))) \
-		__fwd = find_fw_domain(dev_priv, offset); \
-	__fwd; \
-})
-
 #define __chv_reg_write_fw_domains(offset) \
 ({ \
 	enum forcewake_domains __fwd = 0; \
@@ -758,14 +750,6 @@ static const struct intel_forcewake_range __gen9_fw_ranges[] = {
 	GEN_FW_RANGE(0x30000, 0x3ffff, FORCEWAKE_MEDIA),
 };
 
-#define __gen9_reg_read_fw_domains(offset) \
-({ \
-	enum forcewake_domains __fwd = 0; \
-	if (NEEDS_FORCE_WAKE((offset))) \
-		__fwd = find_fw_domain(dev_priv, offset); \
-	__fwd; \
-})
-
 static const i915_reg_t gen9_shadowed_regs[] = {
 	RING_TAIL(RENDER_RING_BASE),
 	RING_TAIL(GEN6_BSD_RING_BASE),
@@ -926,7 +910,7 @@ static u##x \
 vlv_read##x(struct drm_i915_private *dev_priv, i915_reg_t reg, bool trace) { \
 	enum forcewake_domains fw_engine; \
 	GEN6_READ_HEADER(x); \
-	fw_engine = __vlv_reg_read_fw_domains(offset); \
+	fw_engine = __fwtable_reg_read_fw_domains(offset); \
 	if (fw_engine) \
 		__force_wake_auto(dev_priv, fw_engine); \
 	val = __raw_i915_read##x(dev_priv, reg); \
@@ -938,7 +922,7 @@ static u##x \
 chv_read##x(struct drm_i915_private *dev_priv, i915_reg_t reg, bool trace) { \
 	enum forcewake_domains fw_engine; \
 	GEN6_READ_HEADER(x); \
-	fw_engine = __chv_reg_read_fw_domains(offset); \
+	fw_engine = __fwtable_reg_read_fw_domains(offset); \
 	if (fw_engine) \
 		__force_wake_auto(dev_priv, fw_engine); \
 	val = __raw_i915_read##x(dev_priv, reg); \
@@ -950,7 +934,7 @@ static u##x \
 gen9_read##x(struct drm_i915_private *dev_priv, i915_reg_t reg, bool trace) { \
 	enum forcewake_domains fw_engine; \
 	GEN6_READ_HEADER(x); \
-	fw_engine = __gen9_reg_read_fw_domains(offset); \
+	fw_engine = __fwtable_reg_read_fw_domains(offset); \
 	if (fw_engine) \
 		__force_wake_auto(dev_priv, fw_engine); \
 	val = __raw_i915_read##x(dev_priv, reg); \
@@ -1829,18 +1813,18 @@ intel_uncore_forcewake_for_read(struct drm_i915_private *dev_priv,
 
 	switch (INTEL_GEN(dev_priv)) {
 	case 9:
-		fw_domains = __gen9_reg_read_fw_domains(i915_mmio_reg_offset(reg));
+		fw_domains = __fwtable_reg_read_fw_domains(i915_mmio_reg_offset(reg));
 		break;
 	case 8:
 		if (IS_CHERRYVIEW(dev_priv))
-			fw_domains = __chv_reg_read_fw_domains(i915_mmio_reg_offset(reg));
+			fw_domains = __fwtable_reg_read_fw_domains(i915_mmio_reg_offset(reg));
 		else
 			fw_domains = __gen6_reg_read_fw_domains(i915_mmio_reg_offset(reg));
 		break;
 	case 7:
 	case 6:
 		if (IS_VALLEYVIEW(dev_priv))
-			fw_domains = __vlv_reg_read_fw_domains(i915_mmio_reg_offset(reg));
+			fw_domains = __fwtable_reg_read_fw_domains(i915_mmio_reg_offset(reg));
 		else
 			fw_domains = __gen6_reg_read_fw_domains(i915_mmio_reg_offset(reg));
 		break;
-- 
2.7.4

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

Re: [PATCH 09/13] drm/i915: Remove identical macros

[Joonas Lahtinen]

On to, 2016-09-29 at 16:35 +0100, Tvrtko Ursulin wrote:
> vlv_read##x(struct drm_i915_private *dev_priv, i915_reg_t reg, bool trace) { \
> 
>  	enum forcewake_domains fw_engine; \
>  	GEN6_READ_HEADER(x); \
> -	fw_engine = __vlv_reg_read_fw_domains(offset); \
> +	fw_engine = __fwtable_reg_read_fw_domains(offset); \
>  	if (fw_engine) \
>  		__force_wake_auto(dev_priv, fw_engine); \
>  	val = __raw_i915_read##x(dev_priv, reg); \

I notice you in later patch remove the now equal funcs.

> @@ -1829,18 +1813,18 @@ intel_uncore_forcewake_for_read(struct drm_i915_private *dev_priv,
>  
>  	switch (INTEL_GEN(dev_priv)) {
>  	case 9:
> -		fw_domains = __gen9_reg_read_fw_domains(i915_mmio_reg_offset(reg));
> +		fw_domains = __fwtable_reg_read_fw_domains(i915_mmio_reg_offset(reg));
>  		break;
>  	case 8:
>  		if (IS_CHERRYVIEW(dev_priv))
> -			fw_domains = __chv_reg_read_fw_domains(i915_mmio_reg_offset(reg));
> +			fw_domains = __fwtable_reg_read_fw_domains(i915_mmio_reg_offset(reg));
>  		else
>  			fw_domains = __gen6_reg_read_fw_domains(i915_mmio_reg_offset(reg));
>  		break;
>  	case 7:
>  	case 6:
>  		if (IS_VALLEYVIEW(dev_priv))
> -			fw_domains = __vlv_reg_read_fw_domains(i915_mmio_reg_offset(reg));
> +			fw_domains = __fwtable_reg_read_fw_domains(i915_mmio_reg_offset(reg));
>  		else
>  			fw_domains = __gen6_reg_read_fw_domains(i915_mmio_reg_offset(reg));
>  		break;

Drop the whole switch, just do if (IS_CHERRYVIEW || IS_VALLEYVIEW)
fwtable_ else gen6_. Maybe some HAS_FWTABLE() prop would work too, but
that's not must.

With switch removed,

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

[PATCH 10/13] drm/i915: Remove identical mmio read functions

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

It is now obvious VLV, CHV and Gen9 mmio read fcuntions are
completely identical so we can remove the three copies and
just keep the newly named generic implementation.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/intel_uncore.c | 54 +++++++------------------------------
 1 file changed, 10 insertions(+), 44 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index a4ce3576f8d4..4f9f57774e2a 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -905,9 +905,9 @@ gen6_read##x(struct drm_i915_private *dev_priv, i915_reg_t reg, bool trace) { \
 	GEN6_READ_FOOTER; \
 }
 
-#define __vlv_read(x) \
+#define __fwtbl_read(x) \
 static u##x \
-vlv_read##x(struct drm_i915_private *dev_priv, i915_reg_t reg, bool trace) { \
+fwtbl_read##x(struct drm_i915_private *dev_priv, i915_reg_t reg, bool trace) { \
 	enum forcewake_domains fw_engine; \
 	GEN6_READ_HEADER(x); \
 	fw_engine = __fwtable_reg_read_fw_domains(offset); \
@@ -917,50 +917,16 @@ vlv_read##x(struct drm_i915_private *dev_priv, i915_reg_t reg, bool trace) { \
 	GEN6_READ_FOOTER; \
 }
 
-#define __chv_read(x) \
-static u##x \
-chv_read##x(struct drm_i915_private *dev_priv, i915_reg_t reg, bool trace) { \
-	enum forcewake_domains fw_engine; \
-	GEN6_READ_HEADER(x); \
-	fw_engine = __fwtable_reg_read_fw_domains(offset); \
-	if (fw_engine) \
-		__force_wake_auto(dev_priv, fw_engine); \
-	val = __raw_i915_read##x(dev_priv, reg); \
-	GEN6_READ_FOOTER; \
-}
-
-#define __gen9_read(x) \
-static u##x \
-gen9_read##x(struct drm_i915_private *dev_priv, i915_reg_t reg, bool trace) { \
-	enum forcewake_domains fw_engine; \
-	GEN6_READ_HEADER(x); \
-	fw_engine = __fwtable_reg_read_fw_domains(offset); \
-	if (fw_engine) \
-		__force_wake_auto(dev_priv, fw_engine); \
-	val = __raw_i915_read##x(dev_priv, reg); \
-	GEN6_READ_FOOTER; \
-}
-
-__gen9_read(8)
-__gen9_read(16)
-__gen9_read(32)
-__gen9_read(64)
-__chv_read(8)
-__chv_read(16)
-__chv_read(32)
-__chv_read(64)
-__vlv_read(8)
-__vlv_read(16)
-__vlv_read(32)
-__vlv_read(64)
+__fwtbl_read(8)
+__fwtbl_read(16)
+__fwtbl_read(32)
+__fwtbl_read(64)
 __gen6_read(8)
 __gen6_read(16)
 __gen6_read(32)
 __gen6_read(64)
 
-#undef __gen9_read
-#undef __chv_read
-#undef __vlv_read
+#undef __fwtbl_read
 #undef __gen6_read
 #undef GEN6_READ_FOOTER
 #undef GEN6_READ_HEADER
@@ -1319,13 +1285,13 @@ void intel_uncore_init(struct drm_i915_private *dev_priv)
 	case 9:
 		ASSIGN_FW_DOMAINS_TABLE(__gen9_fw_ranges);
 		ASSIGN_WRITE_MMIO_VFUNCS(gen9);
-		ASSIGN_READ_MMIO_VFUNCS(gen9);
+		ASSIGN_READ_MMIO_VFUNCS(fwtbl);
 		break;
 	case 8:
 		if (IS_CHERRYVIEW(dev_priv)) {
 			ASSIGN_FW_DOMAINS_TABLE(__chv_fw_ranges);
 			ASSIGN_WRITE_MMIO_VFUNCS(chv);
-			ASSIGN_READ_MMIO_VFUNCS(chv);
+			ASSIGN_READ_MMIO_VFUNCS(fwtbl);
 
 		} else {
 			ASSIGN_WRITE_MMIO_VFUNCS(gen8);
@@ -1338,7 +1304,7 @@ void intel_uncore_init(struct drm_i915_private *dev_priv)
 
 		if (IS_VALLEYVIEW(dev_priv)) {
 			ASSIGN_FW_DOMAINS_TABLE(__vlv_fw_ranges);
-			ASSIGN_READ_MMIO_VFUNCS(vlv);
+			ASSIGN_READ_MMIO_VFUNCS(fwtbl);
 		} else {
 			ASSIGN_READ_MMIO_VFUNCS(gen6);
 		}
-- 
2.7.4

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

Re: [PATCH 10/13] drm/i915: Remove identical mmio read functions

[Joonas Lahtinen]

On to, 2016-09-29 at 16:35 +0100, Tvrtko Ursulin wrote:
> @@ -905,9 +905,9 @@ gen6_read##x(struct drm_i915_private *dev_priv, i915_reg_t reg, bool trace) { \
> 
> >  	GEN6_READ_FOOTER; \
>  }
>  
> -#define __vlv_read(x) \
> +#define __fwtbl_read(x) \

Consistency is the key, use __fwtable or __fwtbl in the series, not
both.

With the unification,

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

[PATCH 11/13] drm/i915: Remove identical write mmmio functions

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

We notice two identical copies of the shadow register table and
following from that removal can also unify CHV and Gen9 write
mmio functions and macros into a single implementation.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/intel_uncore.c | 69 +++++++------------------------------
 1 file changed, 12 insertions(+), 57 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index 4f9f57774e2a..893d73c7368a 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -706,7 +706,7 @@ static const struct intel_forcewake_range __chv_fw_ranges[] = {
 	GEN_FW_RANGE(0x30000, 0x37fff, FORCEWAKE_MEDIA),
 };
 
-#define __chv_reg_write_fw_domains(offset) \
+#define __fwtbl_reg_write_fw_domains(offset) \
 ({ \
 	enum forcewake_domains __fwd = 0; \
 	if (NEEDS_FORCE_WAKE((offset)) && !is_gen8_shadowed(offset)) \
@@ -750,34 +750,6 @@ static const struct intel_forcewake_range __gen9_fw_ranges[] = {
 	GEN_FW_RANGE(0x30000, 0x3ffff, FORCEWAKE_MEDIA),
 };
 
-static const i915_reg_t gen9_shadowed_regs[] = {
-	RING_TAIL(RENDER_RING_BASE),
-	RING_TAIL(GEN6_BSD_RING_BASE),
-	RING_TAIL(VEBOX_RING_BASE),
-	RING_TAIL(BLT_RING_BASE),
-	GEN6_RPNSWREQ,
-	GEN6_RC_VIDEO_FREQ,
-	/* TODO: Other registers are not yet used */
-};
-
-static bool is_gen9_shadowed(u32 offset)
-{
-	int i;
-	for (i = 0; i < ARRAY_SIZE(gen9_shadowed_regs); i++)
-		if (offset == gen9_shadowed_regs[i].reg)
-			return true;
-
-	return false;
-}
-
-#define __gen9_reg_write_fw_domains(offset) \
-({ \
-	enum forcewake_domains __fwd = 0; \
-	if (NEEDS_FORCE_WAKE((offset)) && !is_gen9_shadowed(offset)) \
-		__fwd = find_fw_domain(dev_priv, offset); \
-	__fwd; \
-})
-
 static void
 ilk_dummy_write(struct drm_i915_private *dev_priv)
 {
@@ -1034,37 +1006,21 @@ gen8_write##x(struct drm_i915_private *dev_priv, i915_reg_t reg, u##x val, bool
 	GEN6_WRITE_FOOTER; \
 }
 
-#define __chv_write(x) \
-static void \
-chv_write##x(struct drm_i915_private *dev_priv, i915_reg_t reg, u##x val, bool trace) { \
-	enum forcewake_domains fw_engine; \
-	GEN6_WRITE_HEADER; \
-	fw_engine = __chv_reg_write_fw_domains(offset); \
-	if (fw_engine) \
-		__force_wake_auto(dev_priv, fw_engine); \
-	__raw_i915_write##x(dev_priv, reg, val); \
-	GEN6_WRITE_FOOTER; \
-}
-
-#define __gen9_write(x) \
+#define __fwtbl_write(x) \
 static void \
-gen9_write##x(struct drm_i915_private *dev_priv, i915_reg_t reg, u##x val, \
-		bool trace) { \
+fwtbl_write##x(struct drm_i915_private *dev_priv, i915_reg_t reg, u##x val, bool trace) { \
 	enum forcewake_domains fw_engine; \
 	GEN6_WRITE_HEADER; \
-	fw_engine = __gen9_reg_write_fw_domains(offset); \
+	fw_engine = __fwtbl_reg_write_fw_domains(offset); \
 	if (fw_engine) \
 		__force_wake_auto(dev_priv, fw_engine); \
 	__raw_i915_write##x(dev_priv, reg, val); \
 	GEN6_WRITE_FOOTER; \
 }
 
-__gen9_write(8)
-__gen9_write(16)
-__gen9_write(32)
-__chv_write(8)
-__chv_write(16)
-__chv_write(32)
+__fwtbl_write(8)
+__fwtbl_write(16)
+__fwtbl_write(32)
 __gen8_write(8)
 __gen8_write(16)
 __gen8_write(32)
@@ -1072,8 +1028,7 @@ __gen6_write(8)
 __gen6_write(16)
 __gen6_write(32)
 
-#undef __gen9_write
-#undef __chv_write
+#undef __fwtbl_write
 #undef __gen8_write
 #undef __gen6_write
 #undef GEN6_WRITE_FOOTER
@@ -1284,13 +1239,13 @@ void intel_uncore_init(struct drm_i915_private *dev_priv)
 	default:
 	case 9:
 		ASSIGN_FW_DOMAINS_TABLE(__gen9_fw_ranges);
-		ASSIGN_WRITE_MMIO_VFUNCS(gen9);
+		ASSIGN_WRITE_MMIO_VFUNCS(fwtbl);
 		ASSIGN_READ_MMIO_VFUNCS(fwtbl);
 		break;
 	case 8:
 		if (IS_CHERRYVIEW(dev_priv)) {
 			ASSIGN_FW_DOMAINS_TABLE(__chv_fw_ranges);
-			ASSIGN_WRITE_MMIO_VFUNCS(chv);
+			ASSIGN_WRITE_MMIO_VFUNCS(fwtbl);
 			ASSIGN_READ_MMIO_VFUNCS(fwtbl);
 
 		} else {
@@ -1819,11 +1774,11 @@ intel_uncore_forcewake_for_write(struct drm_i915_private *dev_priv,
 
 	switch (INTEL_GEN(dev_priv)) {
 	case 9:
-		fw_domains = __gen9_reg_write_fw_domains(i915_mmio_reg_offset(reg));
+		fw_domains = __fwtbl_reg_write_fw_domains(i915_mmio_reg_offset(reg));
 		break;
 	case 8:
 		if (IS_CHERRYVIEW(dev_priv))
-			fw_domains = __chv_reg_write_fw_domains(i915_mmio_reg_offset(reg));
+			fw_domains = __fwtbl_reg_write_fw_domains(i915_mmio_reg_offset(reg));
 		else
 			fw_domains = __gen8_reg_write_fw_domains(i915_mmio_reg_offset(reg));
 		break;
-- 
2.7.4

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

Re: [PATCH 11/13] drm/i915: Remove identical write mmmio functions

[Joonas Lahtinen]

On to, 2016-09-29 at 16:35 +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> We notice two identical copies of the shadow register table and
> following from that removal can also unify CHV and Gen9 write
> mmio functions and macros into a single implementation.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

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

[PATCH 12/13] drm/i915: Sort the shadow register table

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Also verify the order at runtime. This was we can start using
binary search on it in a following patch.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/intel_uncore.c | 28 ++++++++++++++++++++++------
 1 file changed, 22 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index 893d73c7368a..8f25cd7ab50f 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -657,15 +657,29 @@ static const struct intel_forcewake_range __vlv_fw_ranges[] = {
 })
 
 static const i915_reg_t gen8_shadowed_regs[] = {
-	GEN6_RPNSWREQ,
-	GEN6_RC_VIDEO_FREQ,
-	RING_TAIL(RENDER_RING_BASE),
-	RING_TAIL(GEN6_BSD_RING_BASE),
-	RING_TAIL(VEBOX_RING_BASE),
-	RING_TAIL(BLT_RING_BASE),
+	RING_TAIL(RENDER_RING_BASE),	/* 0x2000 (base) */
+	GEN6_RPNSWREQ,			/* 0xA008 */
+	GEN6_RC_VIDEO_FREQ,		/* 0xA00C */
+	RING_TAIL(GEN6_BSD_RING_BASE),	/* 0x12000 (base) */
+	RING_TAIL(VEBOX_RING_BASE),	/* 0x1a000 (base) */
+	RING_TAIL(BLT_RING_BASE),	/* 0x22000 (base) */
 	/* TODO: Other registers are not yet used */
 };
 
+static void intel_shadow_table_check(void)
+{
+	i915_reg_t *reg = (i915_reg_t *)gen8_shadowed_regs;
+	s32 prev = -1;
+	u32 offset;
+	unsigned int i;
+
+	for (i = 0; i < ARRAY_SIZE(gen8_shadowed_regs); i++, reg++) {
+		offset = i915_mmio_reg_offset(*reg);
+		WARN_ON_ONCE(prev >= (s32)offset);
+		prev = offset;
+	}
+}
+
 static bool is_gen8_shadowed(u32 offset)
 {
 	int i;
@@ -1277,6 +1291,8 @@ void intel_uncore_init(struct drm_i915_private *dev_priv)
 	}
 
 	intel_fw_table_check(dev_priv);
+	if (INTEL_GEN(dev_priv) >= 8)
+		intel_shadow_table_check();
 
 	if (intel_vgpu_active(dev_priv)) {
 		ASSIGN_WRITE_MMIO_VFUNCS(vgpu);
-- 
2.7.4

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

Re: [PATCH 12/13] drm/i915: Sort the shadow register table

[Chris Wilson]

On Thu, Sep 29, 2016 at 04:35:55PM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Also verify the order at runtime. This was we can start using
> binary search on it in a following patch.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_uncore.c | 28 ++++++++++++++++++++++------
>  1 file changed, 22 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index 893d73c7368a..8f25cd7ab50f 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -657,15 +657,29 @@ static const struct intel_forcewake_range __vlv_fw_ranges[] = {
>  })
>  
>  static const i915_reg_t gen8_shadowed_regs[] = {
> -	GEN6_RPNSWREQ,
> -	GEN6_RC_VIDEO_FREQ,
> -	RING_TAIL(RENDER_RING_BASE),
> -	RING_TAIL(GEN6_BSD_RING_BASE),
> -	RING_TAIL(VEBOX_RING_BASE),
> -	RING_TAIL(BLT_RING_BASE),
> +	RING_TAIL(RENDER_RING_BASE),	/* 0x2000 (base) */
> +	GEN6_RPNSWREQ,			/* 0xA008 */
> +	GEN6_RC_VIDEO_FREQ,		/* 0xA00C */
> +	RING_TAIL(GEN6_BSD_RING_BASE),	/* 0x12000 (base) */
> +	RING_TAIL(VEBOX_RING_BASE),	/* 0x1a000 (base) */
> +	RING_TAIL(BLT_RING_BASE),	/* 0x22000 (base) */
>  	/* TODO: Other registers are not yet used */
>  };

Ok.
-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

Re: [PATCH 12/13] drm/i915: Sort the shadow register table

[Joonas Lahtinen]

On to, 2016-09-29 at 16:35 +0100, Tvrtko Ursulin wrote:
> +static void intel_shadow_table_check(void)
> +{
> +	i915_reg_t *reg = (i915_reg_t *)gen8_shadowed_regs;
> +	s32 prev = -1;
> +	u32 offset;
> +	unsigned int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(gen8_shadowed_regs); i++, reg++) {
> +		offset = i915_mmio_reg_offset(*reg);
> +		WARN_ON_ONCE(prev >= (s32)offset);
> +		prev = offset;
> +	}
> +}
> +

BUILD_BUG_ON would gain extra points, but this is fine too :)

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

Re: [PATCH 12/13] drm/i915: Sort the shadow register table

[Tvrtko Ursulin]

On 30/09/2016 09:09, Joonas Lahtinen wrote:
> On to, 2016-09-29 at 16:35 +0100, Tvrtko Ursulin wrote:
>> +static void intel_shadow_table_check(void)
>> +{
>> +	i915_reg_t *reg = (i915_reg_t *)gen8_shadowed_regs;
>> +	s32 prev = -1;
>> +	u32 offset;
>> +	unsigned int i;
>> +
>> +	for (i = 0; i < ARRAY_SIZE(gen8_shadowed_regs); i++, reg++) {
>> +		offset = i915_mmio_reg_offset(*reg);
>> +		WARN_ON_ONCE(prev >= (s32)offset);
>> +		prev = offset;
>> +	}
>> +}
>> +
> BUILD_BUG_ON would gain extra points, but this is fine too :)

It is possible to do that, how?

Opinions of whether these checks should always be there or maybe under 
#ifdef CONFIG_DRM_I915_DEBUG perhaps?

Regards,

Tvrtko

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

Re: [PATCH 12/13] drm/i915: Sort the shadow register table

[Joonas Lahtinen]

On pe, 2016-09-30 at 10:06 +0100, Tvrtko Ursulin wrote:
> 
> It is possible to do that, how?
> 

I think it'd enable uncomfortable amount of macros. So maybe not worth
the bother :)

> Opinions of whether these checks should always be there or maybe
> under 
> #ifdef CONFIG_DRM_I915_DEBUG perhaps?
> 

I think I915_DEBUG around would be good idea, just omit the function
body if not enabled.

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

[PATCH 13/13] drm/i915: Use binary search when looking for shadowed registers

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Simply replace the linear search with the kernel's binary
search implementation. There is only six registers currently
in that table so this may not be that interesting. It adds a
function call so hopefully remains performance neutral for now.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/intel_uncore.c | 26 +++++++++++++++++++++-----
 1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index 8f25cd7ab50f..24e2b167b8b6 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -680,14 +680,30 @@ static void intel_shadow_table_check(void)
 	}
 }
 
+static int mmio_reg_cmp(const void *key, const void *elt)
+{
+	u32 offset = (u32)(unsigned long)key;
+	i915_reg_t *reg = (i915_reg_t *)elt;
+
+	if (offset < i915_mmio_reg_offset(*reg))
+		return -1;
+	else if (offset > i915_mmio_reg_offset(*reg))
+		return 1;
+	else
+		return 0;
+}
+
 static bool is_gen8_shadowed(u32 offset)
 {
-	int i;
-	for (i = 0; i < ARRAY_SIZE(gen8_shadowed_regs); i++)
-		if (offset == gen8_shadowed_regs[i].reg)
-			return true;
+	i915_reg_t *reg;
 
-	return false;
+	reg = bsearch((void *)(unsigned long)offset,
+		      (const void *)gen8_shadowed_regs,
+		      ARRAY_SIZE(gen8_shadowed_regs),
+		      sizeof(i915_reg_t),
+		      mmio_reg_cmp);
+
+	return reg ? true : false;
 }
 
 #define __gen8_reg_write_fw_domains(offset) \
-- 
2.7.4

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

Re: [PATCH 13/13] drm/i915: Use binary search when looking for shadowed registers

[Joonas Lahtinen]

On to, 2016-09-29 at 16:35 +0100, Tvrtko Ursulin wrote:
> +static int mmio_reg_cmp(const void *key, const void *elt)
+ {
<SNIP>
> 
> +	if (offset < i915_mmio_reg_offset(*reg))
> +		return -1;
> +	else if (offset > i915_mmio_reg_offset(*reg))
> +		return 1;
> +	else
> +		return 0;

I was about to suggest return a - b; but then realized it might
overflow. After a bit of searching, that is done in
regcache_default_cmp though. So not sure if it's a bug in there or did
I miss something. This is safe way at least.

>  static bool is_gen8_shadowed(u32 offset)
>  {
> > -	int i;
> > -	for (i = 0; i < ARRAY_SIZE(gen8_shadowed_regs); i++)
> > -		if (offset == gen8_shadowed_regs[i].reg)
> > -			return true;
> > +	i915_reg_t *reg;
>  
> > -	return false;
> +	reg = bsearch((void *)(unsigned long)offset,

(unsigned long) is not required, as we don't have sign bits to extend.

> +		      (const void *)gen8_shadowed_regs,
> +		      ARRAY_SIZE(gen8_shadowed_regs),
> +		      sizeof(i915_reg_t),
> +		      mmio_reg_cmp);
> +
> +	return reg ? true : false;

As I previously learnt, no need to explicitly convert bool types
anymore, so this can just be return bsearch(...);

With the last two fixed,

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

Re: [PATCH 13/13] drm/i915: Use binary search when looking for shadowed registers

[Tvrtko Ursulin]

On 30/09/2016 08:54, Joonas Lahtinen wrote:
> On to, 2016-09-29 at 16:35 +0100, Tvrtko Ursulin wrote:
>> +static int mmio_reg_cmp(const void *key, const void *elt)
> + {
> <SNIP>
>> +	if (offset < i915_mmio_reg_offset(*reg))
>> +		return -1;
>> +	else if (offset > i915_mmio_reg_offset(*reg))
>> +		return 1;
>> +	else
>> +		return 0;
> I was about to suggest return a - b; but then realized it might
> overflow. After a bit of searching, that is done in
> regcache_default_cmp though. So not sure if it's a bug in there or did
> I miss something. This is safe way at least.
>
>>   static bool is_gen8_shadowed(u32 offset)
>>   {
>>> -	int i;
>>> -	for (i = 0; i < ARRAY_SIZE(gen8_shadowed_regs); i++)
>>> -		if (offset == gen8_shadowed_regs[i].reg)
>>> -			return true;
>>> +	i915_reg_t *reg;
>>   
>>> -	return false;
>> +	reg = bsearch((void *)(unsigned long)offset,
> (unsigned long) is not required, as we don't have sign bits to extend.

Gcc warns if I omit that.
>
>> +		      (const void *)gen8_shadowed_regs,
>> +		      ARRAY_SIZE(gen8_shadowed_regs),
>> +		      sizeof(i915_reg_t),
>> +		      mmio_reg_cmp);
>> +
>> +	return reg ? true : false;
> As I previously learnt, no need to explicitly convert bool types
> anymore, so this can just be return bsearch(...);
>
> With the last two fixed,

I've done this one.

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

Thanks!

Tvrtko

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

Re: [PATCH 00/13] Forcewake binary search & code shrink

[Jani Nikula]

On Thu, 29 Sep 2016, Tvrtko Ursulin <tursulin@ursulin.net> wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> A bunch of cleaned up patches from one of my old branches. Just an idea I had
> been toying about and probably more a RFC at this point. But it amazingly
> passed trybot on (almost) the first try so I got tempted to post it.
>
> Basically main motivation was to replace linear search comparison ladders in
> all mmio accessors with something smarter - a binary search in this case.
>
> Main enabler is moving the comparison ladders from the code into the data
> tables. Then gradually, patch by patch, I was able to reduce the code and
> end up with a total saving of approximately 14KiB for the series.
>
> That is kind of nice, but whether the churn is worth it I leave open for
> dicussion. It is slightly less code so perhaps more maintainable. Although
> tastes will probably differ.

Glanced through it quickly, I think the new stuff looks prettier and
more manageable. I'm not opposed.

BR,
Jani.


>
> And I haven't been able to do any benchmarking to see if any performance
> differece is at all detectable.
>
> Tvrtko Ursulin (13):
>   drm/i915: Remove redundant hsw_write* mmio functions
>   drm/i915: Keep track of active forcewake domains in a bitmask
>   drm/i915: Do not inline forcewake taking in mmio accessors
>   drm/i915: Data driven register to forcewake domains lookup
>   drm/i915: Sort forcewake mapping tables
>   drm/i915: Use binary search when looking up forcewake domains
>   drm/i915: Eliminate Gen9 special case
>   drm/i915: Store the active forcewake range table pointer
>   drm/i915: Remove identical macros
>   drm/i915: Remove identical mmio read functions
>   drm/i915: Remove identical write mmmio functions
>   drm/i915: Sort the shadow register table
>   drm/i915: Use binary search when looking for shadowed registers
>
>  drivers/gpu/drm/i915/i915_drv.h     |  13 +
>  drivers/gpu/drm/i915/intel_uncore.c | 525 ++++++++++++++++--------------------
>  2 files changed, 247 insertions(+), 291 deletions(-)

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: warning for Forcewake binary search & code shrink

[Patchwork]

== Series Details ==

Series: Forcewake binary search & code shrink
URL   : https://patchwork.freedesktop.org/series/13080/
State : warning

== Summary ==

Series 13080v1 Forcewake binary search & code shrink
https://patchwork.freedesktop.org/api/1.0/series/13080/revisions/1/mbox/

Test kms_pipe_crc_basic:
        Subgroup read-crc-pipe-b:
                pass       -> DMESG-WARN (fi-ilk-650)

fi-bdw-5557u     total:244  pass:229  dwarn:0   dfail:0   fail:0   skip:15 
fi-bsw-n3050     total:244  pass:202  dwarn:0   dfail:0   fail:0   skip:42 
fi-bxt-t5700     total:244  pass:214  dwarn:0   dfail:0   fail:0   skip:30 
fi-byt-n2820     total:244  pass:208  dwarn:0   dfail:0   fail:1   skip:35 
fi-hsw-4770      total:244  pass:222  dwarn:0   dfail:0   fail:0   skip:22 
fi-hsw-4770r     total:244  pass:222  dwarn:0   dfail:0   fail:0   skip:22 
fi-ilk-650       total:244  pass:181  dwarn:1   dfail:0   fail:2   skip:60 
fi-ivb-3520m     total:244  pass:219  dwarn:0   dfail:0   fail:0   skip:25 
fi-ivb-3770      total:244  pass:207  dwarn:0   dfail:0   fail:0   skip:37 
fi-skl-6260u     total:244  pass:230  dwarn:0   dfail:0   fail:0   skip:14 
fi-skl-6700hq    total:244  pass:221  dwarn:1   dfail:0   fail:0   skip:22 
fi-skl-6700k     total:244  pass:219  dwarn:1   dfail:0   fail:0   skip:24 
fi-skl-6770hq    total:244  pass:228  dwarn:1   dfail:0   fail:1   skip:14 
fi-snb-2520m     total:244  pass:208  dwarn:0   dfail:0   fail:0   skip:36 
fi-snb-2600      total:244  pass:207  dwarn:0   dfail:0   fail:0   skip:37 
fi-byt-j1900 failed to connect after reboot

Results at /archive/results/CI_IGT_test/Patchwork_2594/

2fbc239494fa3883e164cc89d35f54d22f0c9e1f drm-intel-nightly: 2016y-09m-29d-13h-31m-39s UTC integration manifest
d042c9c drm/i915: Use binary search when looking for shadowed registers
dbafe06 drm/i915: Sort the shadow register table
b9121e6 drm/i915: Remove identical write mmmio functions
0780db3 drm/i915: Remove identical mmio read functions
b1f3e47 drm/i915: Remove identical macros
777472d drm/i915: Store the active forcewake range table pointer
20718aa drm/i915: Eliminate Gen9 special case
09c6e3b drm/i915: Use binary search when looking up forcewake domains
98dd975 drm/i915: Sort forcewake mapping tables
f89fd58 drm/i915: Data driven register to forcewake domains lookup
bac5870 drm/i915: Do not inline forcewake taking in mmio accessors
6b67148 drm/i915: Keep track of active forcewake domains in a bitmask
cf28017 drm/i915: Remove redundant hsw_write* mmio functions

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

Re: [PATCH 00/13] Forcewake binary search & code shrink

[Chris Wilson]

On Thu, Sep 29, 2016 at 04:35:43PM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> A bunch of cleaned up patches from one of my old branches. Just an idea I had
> been toying about and probably more a RFC at this point. But it amazingly
> passed trybot on (almost) the first try so I got tempted to post it.
> 
> Basically main motivation was to replace linear search comparison ladders in
> all mmio accessors with something smarter - a binary search in this case.
> 
> Main enabler is moving the comparison ladders from the code into the data
> tables. Then gradually, patch by patch, I was able to reduce the code and
> end up with a total saving of approximately 14KiB for the series.
> 
> That is kind of nice, but whether the churn is worth it I leave open for
> dicussion. It is slightly less code so perhaps more maintainable. Although
> tastes will probably differ.

More declarative approach, yes please. Looks really nice.
-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

✗ Fi.CI.BAT: failure for Forcewake binary search & code shrink (rev2)

[Patchwork]

== Series Details ==

Series: Forcewake binary search & code shrink (rev2)
URL   : https://patchwork.freedesktop.org/series/13080/
State : failure

== Summary ==

  LD      fs/btrfs/built-in.o
  CC      arch/x86/kernel/cpu/capflags.o
  LD      arch/x86/kernel/cpu/built-in.o
  LD      arch/x86/kernel/built-in.o
  LD      drivers/md/md-mod.o
  LD      drivers/md/built-in.o
  LD [M]  drivers/net/ethernet/intel/igb/igb.o
  LD      drivers/usb/core/usbcore.o
  LD      drivers/usb/core/built-in.o
  LD      arch/x86/built-in.o
  LD      drivers/usb/host/xhci-hcd.o
  LD      fs/ext4/ext4.o
  LD      fs/ext4/built-in.o
  LD      net/core/built-in.o
  LD      fs/built-in.o
  LD      drivers/usb/host/built-in.o
  LD      drivers/usb/built-in.o
  LD [M]  drivers/net/ethernet/intel/e1000e/e1000e.o
  LD      net/ipv4/built-in.o
  LD      net/built-in.o
  LD      drivers/net/ethernet/built-in.o
  LD      drivers/net/built-in.o
scripts/Makefile.build:440: recipe for target 'drivers/gpu/drm/i915' failed
make[3]: *** [drivers/gpu/drm/i915] Error 2
scripts/Makefile.build:440: recipe for target 'drivers/gpu/drm' failed
make[2]: *** [drivers/gpu/drm] Error 2
scripts/Makefile.build:440: recipe for target 'drivers/gpu' failed
make[1]: *** [drivers/gpu] Error 2
Makefile:968: recipe for target 'drivers' failed
make: *** [drivers] Error 2

Full logs at /archive/deploy/logs/Patchwork_2598

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