On 20/09/2017 10:56, Tvrtko Ursulin wrote:
On 20/09/2017 10:39, Jani Nikula wrote:
On Wed, 20 Sep 2017, Tvrtko Ursulin <tursulin@xxxxxxxxxxx> wrote:
From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
If we store the platform as a bitmask, and convert the
IS_PLATFORM macro to use it, we allow the compiler to
merge the IS_PLATFORM(a) || IS_PLATFORM(b) || ... checks
into a single conditional.
Even with the added BUG_ON this saves almost 1k of text:
text data bss dec hex filename
-1460254 60014 3656 1523924 1740d4
drivers/gpu/drm/i915/i915.ko
+1459260 60026 3656 1522942 173cfe
drivers/gpu/drm/i915/i915.ko
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
Cc: Jani Nikula <jani.nikula@xxxxxxxxx>
---
drivers/gpu/drm/i915/i915_drv.c | 6 ++++++
drivers/gpu/drm/i915/i915_drv.h | 3 ++-
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c
b/drivers/gpu/drm/i915/i915_drv.c
index b1f96eb1be16..c3bd4b7cb19b 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -869,6 +869,12 @@ static int i915_driver_init_early(struct
drm_i915_private *dev_priv,
memcpy(device_info, match_info, sizeof(*device_info));
device_info->device_id = dev_priv->drm.pdev->device;
+ BUILD_BUG_ON(sizeof(device_info->platform_mask) * BITS_PER_BYTE <
+ (INTEL_MAX_PLATFORMS - 1));
+ BUG_ON(device_info->platform == 0 >>> + device_info->platform >= INTEL_MAX_PLATFORMS);
+ device_info->platform_mask = BIT(device_info->platform - 1);
Please just lose the -1, pretty please?
+
BUG_ON(device_info->gen > sizeof(device_info->gen_mask) *
BITS_PER_BYTE);
device_info->gen_mask = BIT(device_info->gen - 1);
diff --git a/drivers/gpu/drm/i915/i915_drv.h
b/drivers/gpu/drm/i915/i915_drv.h
index 950aa109f8cb..81211f23326a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -861,6 +861,7 @@ struct intel_device_info {
u8 ring_mask; /* Rings supported by the HW */
enum intel_platform platform;
+ u32 platform_mask;
u32 display_mmio_offset;
@@ -2984,7 +2985,7 @@ intel_info(const struct drm_i915_private
*dev_priv)
#define IS_REVID(p, since, until) \
(INTEL_REVID(p) >= (since) && INTEL_REVID(p) <= (until))
-#define IS_PLATFORM(dev_priv, p) ((dev_priv)->info.platform == (p))
+#define IS_PLATFORM(dev_priv, p) ((dev_priv)->info.platform_mask &
BIT((p) - 1))
What would the result be without platform_mask and just:
#define IS_PLATFORM(dev_priv, p) (BIT((dev_priv)->info.platform) &
BIT(p))
More code I'm afraid. But the problem of 32 platforms limit makes it
problematic for me. Because I checked, and going to u64 for the
platform_mask grows the code 100-200 bytes over the starting point. We'd
keep the single conditional advantage but I don't know, feels like not
worth it in that case. It could only last for a couple years before we
would need to go to u64.
On the other hand we could take this, and by the time would need to
grown the mask field, we can a) enjoy the size saving, b) by that time
new platform code will dwarf the 100-200 bytes going to u64 will add,
and c) 100-200 bytes might even turn to a gain again, since new or-ed
IS_platform checks will appear. Thoughts?
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
