[Intel-gfx] [PATCH v2] drm/i915: Number the platform enum strategically

Tvrtko Ursulin tursulin at ursulin.net
Thu Dec 8 13:38:10 UTC 2016


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

If we use only power of two values for the platform enum
values we can let the compiler optimize some common
checks to a single conditional.

For example code like this:

	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))

Goes from this:

   5c3c5:       8b 83 d8 06 00 00       mov    0x6d8(%rbx),%eax
   5c3cb:       83 f8 12                cmp    $0x12,%eax
   5c3ce:       0f 84 f3 00 00 00       je     5c4c7 <fw_domain_init+0x1a7>
   5c3d4:       83 f8 15                cmp    $0x15,%eax
   5c3d7:       0f 84 ea 00 00 00       je     5c4c7 <fw_domain_init+0x1a7>

To this:

   5c1d5:       f7 83 d8 06 00 00 00    testl  $0x240000,0x6d8(%rbx)
   5c1dc:       00 24 00
   5c1df:       0f 85 da 00 00 00       jne    5c2bf <fw_domain_init+0x18f>

It is not much but there is value in this that as long as we
have to have conditions like the above sprinkled troughout the
code, we can at least have the generate binary a bit smarter.

Until we get to more than 32 platforms there is no downside
to this approach.

v2:
 * Remove !! from the comparison. (Chris Wilson)
 * Simplify the patch by moving the BIT() to the generator.
   (Michal Wajdeczko)

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
Cc: Chris Wilson <chris at chris-wilson.co.uk>
Cc: Jani Nikula <jani.nikula at intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko at linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h          |  4 ++--
 drivers/gpu/drm/i915/intel_device_info.c | 10 ++++++----
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index fb8f4b7cd1ae..3be6a19f22eb 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -826,7 +826,7 @@ static inline unsigned int sseu_subslice_total(const struct sseu_dev_info *sseu)
 }
 
 enum intel_platform {
-#define i915_platform(name, value) INTEL_##name = value,
+#define i915_platform(name, value) INTEL_##name = BIT(value),
 #include "i915_platforms.h"
 #undef i915_platform
 };
@@ -2590,7 +2590,7 @@ intel_info(const struct drm_i915_private *dev_priv)
 #define i915_platform(name, id) \
 static inline bool IS_##name(const struct drm_i915_private *dev_priv) \
 { \
-	return (dev_priv)->info.platform == INTEL_##name; \
+	return (dev_priv)->info.platform & INTEL_##name; \
 }
 #include "i915_platforms.h"
 #undef i915_platform
diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
index 5192d388d10e..26df6363e265 100644
--- a/drivers/gpu/drm/i915/intel_device_info.c
+++ b/drivers/gpu/drm/i915/intel_device_info.c
@@ -25,18 +25,20 @@
 #include "i915_drv.h"
 
 static const char * const platform_names[] = {
-#define i915_platform(name, id) [id] = #name,
+#define i915_platform(name, id) [__builtin_ffs(id)] = #name,
 #include "i915_platforms.h"
 #undef i915_platform
 };
 
 const char *intel_platform_name(enum intel_platform platform)
 {
-	if (WARN_ON_ONCE(platform >= ARRAY_SIZE(platform_names) ||
-			 platform_names[platform] == NULL))
+	unsigned int idx = ffs(platform);
+
+	if (WARN_ON_ONCE(idx >= ARRAY_SIZE(platform_names) ||
+			 platform_names[idx] == NULL))
 		return "<unknown>";
 
-	return platform_names[platform];
+	return platform_names[idx];
 }
 
 void intel_device_info_dump(struct drm_i915_private *dev_priv)
-- 
2.7.4



More information about the Intel-gfx mailing list