Skip to contentSkip to conversation

Claw back optimised IS_PLATFORM checks

PATCH In review Tvrtko Ursulin 4 participants 12 messages Sep 20, 2017 – Sep 25, 2017

Subsystems (2)
DRM drivers Intel i915
lore
Share & tools
lore

Tvrtko Ursulin

author maintainer
#0 lore raw

cover Claw back optimised IS_PLATFORM checks

From: Tvrtko Ursulin <[email protected]>

Some time ago we stopped letting the compiler merge code like IS_SKYLAKE || IS_KABYLAKE || IS_... into a single conditional in all cases. This series quickly restores that for a nice overall saving and hopefully with a bearable impact on the code base.

text data bss dec hex filename

-1460454          60014    3656 1524124  17419c drivers/gpu/drm/i915/i915.ko
+1459260          60026    3656 1522942  173cfe drivers/gpu/drm/i915/i915.ko

Tvrtko Ursulin (3):
drm/i915: Add IS_PLATFORM macro
drm/i915: Compact device info access by a small re-ordering drm/i915: Allow optimized platform checks

 drivers/gpu/drm/i915/i915_drv.c |  6 ++++
 drivers/gpu/drm/i915/i915_drv.h | 73 +++++++++++++++++++++++------------------
 2 files changed, 47 insertions(+), 32 deletions(-)
Signature · 6 lines
2.9.5

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
author maintainer
#1 lore raw

PATCH 1/3 drm/i915: Add IS_PLATFORM macro

From: Tvrtko Ursulin <[email protected]>

This will allow some code re-organization in a following patch.

Signed-off-by Tvrtko Ursulin <[email protected]>
Cc Jani Nikula <[email protected]>
---
 drivers/gpu/drm/i915/i915_drv.h | 52 +++++++++++++++++++++--------------------
 1 file changed, 27 insertions(+), 25 deletions(-)
drivers/gpu/drm/i915/i915_drv.h +27 -25
index bd364be2ba27..0f1fe0d58a73 100644
@@ -2978,37 +2978,39 @@ 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_I830(dev_priv)	((dev_priv)->info.platform == INTEL_I830)
-#define IS_I845G(dev_priv)	((dev_priv)->info.platform == INTEL_I845G)
-#define IS_I85X(dev_priv)	((dev_priv)->info.platform == INTEL_I85X)
-#define IS_I865G(dev_priv)	((dev_priv)->info.platform == INTEL_I865G)
-#define IS_I915G(dev_priv)	((dev_priv)->info.platform == INTEL_I915G)
-#define IS_I915GM(dev_priv)	((dev_priv)->info.platform == INTEL_I915GM)
-#define IS_I945G(dev_priv)	((dev_priv)->info.platform == INTEL_I945G)
-#define IS_I945GM(dev_priv)	((dev_priv)->info.platform == INTEL_I945GM)
-#define IS_I965G(dev_priv)	((dev_priv)->info.platform == INTEL_I965G)
-#define IS_I965GM(dev_priv)	((dev_priv)->info.platform == INTEL_I965GM)
-#define IS_G45(dev_priv)	((dev_priv)->info.platform == INTEL_G45)
-#define IS_GM45(dev_priv)	((dev_priv)->info.platform == INTEL_GM45)
+#define IS_PLATFORM(dev_priv, p) ((dev_priv)->info.platform == (p))
+
+#define IS_I830(dev_priv)	IS_PLATFORM(dev_priv, INTEL_I830)
+#define IS_I845G(dev_priv)	IS_PLATFORM(dev_priv, INTEL_I845G)
+#define IS_I85X(dev_priv)	IS_PLATFORM(dev_priv, INTEL_I85X)
+#define IS_I865G(dev_priv)	IS_PLATFORM(dev_priv, INTEL_I865G)
+#define IS_I915G(dev_priv)	IS_PLATFORM(dev_priv, INTEL_I915G)
+#define IS_I915GM(dev_priv)	IS_PLATFORM(dev_priv, INTEL_I915GM)
+#define IS_I945G(dev_priv)	IS_PLATFORM(dev_priv, INTEL_I945G)
+#define IS_I945GM(dev_priv)	IS_PLATFORM(dev_priv, INTEL_I945GM)
+#define IS_I965G(dev_priv)	IS_PLATFORM(dev_priv, INTEL_I965G)
+#define IS_I965GM(dev_priv)	IS_PLATFORM(dev_priv, INTEL_I965GM)
+#define IS_G45(dev_priv)	IS_PLATFORM(dev_priv, INTEL_G45)
+#define IS_GM45(dev_priv)	IS_PLATFORM(dev_priv, INTEL_GM45)
 #define IS_G4X(dev_priv)	(IS_G45(dev_priv) || IS_GM45(dev_priv))
 #define IS_PINEVIEW_G(dev_priv)	(INTEL_DEVID(dev_priv) == 0xa001)
 #define IS_PINEVIEW_M(dev_priv)	(INTEL_DEVID(dev_priv) == 0xa011)
-#define IS_PINEVIEW(dev_priv)	((dev_priv)->info.platform == INTEL_PINEVIEW)
-#define IS_G33(dev_priv)	((dev_priv)->info.platform == INTEL_G33)
+#define IS_PINEVIEW(dev_priv)	IS_PLATFORM(dev_priv, INTEL_PINEVIEW)
+#define IS_G33(dev_priv)	IS_PLATFORM(dev_priv, INTEL_G33)
 #define IS_IRONLAKE_M(dev_priv)	(INTEL_DEVID(dev_priv) == 0x0046)
-#define IS_IVYBRIDGE(dev_priv)	((dev_priv)->info.platform == INTEL_IVYBRIDGE)
+#define IS_IVYBRIDGE(dev_priv)	IS_PLATFORM(dev_priv, INTEL_IVYBRIDGE)
 #define IS_IVB_GT1(dev_priv)	(IS_IVYBRIDGE(dev_priv) && \
 				 (dev_priv)->info.gt == 1)
-#define IS_VALLEYVIEW(dev_priv)	((dev_priv)->info.platform == INTEL_VALLEYVIEW)
-#define IS_CHERRYVIEW(dev_priv)	((dev_priv)->info.platform == INTEL_CHERRYVIEW)
-#define IS_HASWELL(dev_priv)	((dev_priv)->info.platform == INTEL_HASWELL)
-#define IS_BROADWELL(dev_priv)	((dev_priv)->info.platform == INTEL_BROADWELL)
-#define IS_SKYLAKE(dev_priv)	((dev_priv)->info.platform == INTEL_SKYLAKE)
-#define IS_BROXTON(dev_priv)	((dev_priv)->info.platform == INTEL_BROXTON)
-#define IS_KABYLAKE(dev_priv)	((dev_priv)->info.platform == INTEL_KABYLAKE)
-#define IS_GEMINILAKE(dev_priv)	((dev_priv)->info.platform == INTEL_GEMINILAKE)
-#define IS_COFFEELAKE(dev_priv)	((dev_priv)->info.platform == INTEL_COFFEELAKE)
-#define IS_CANNONLAKE(dev_priv)	((dev_priv)->info.platform == INTEL_CANNONLAKE)
+#define IS_VALLEYVIEW(dev_priv)	IS_PLATFORM(dev_priv, INTEL_VALLEYVIEW)
+#define IS_CHERRYVIEW(dev_priv)	IS_PLATFORM(dev_priv, INTEL_CHERRYVIEW)
+#define IS_HASWELL(dev_priv)	IS_PLATFORM(dev_priv, INTEL_HASWELL)
+#define IS_BROADWELL(dev_priv)	IS_PLATFORM(dev_priv, INTEL_BROADWELL)
+#define IS_SKYLAKE(dev_priv)	IS_PLATFORM(dev_priv, INTEL_SKYLAKE)
+#define IS_BROXTON(dev_priv)	IS_PLATFORM(dev_priv, INTEL_BROXTON)
+#define IS_KABYLAKE(dev_priv)	IS_PLATFORM(dev_priv, INTEL_KABYLAKE)
+#define IS_GEMINILAKE(dev_priv)	IS_PLATFORM(dev_priv, INTEL_GEMINILAKE)
+#define IS_COFFEELAKE(dev_priv)	IS_PLATFORM(dev_priv, INTEL_COFFEELAKE)
+#define IS_CANNONLAKE(dev_priv)	IS_PLATFORM(dev_priv, INTEL_CANNONLAKE)
 #define IS_MOBILE(dev_priv)	((dev_priv)->info.is_mobile)
 #define IS_HSW_EARLY_SDV(dev_priv) (IS_HASWELL(dev_priv) && \
 				    (INTEL_DEVID(dev_priv) & 0xFF00) == 0x0C00)
Signature · 6 lines
2.9.5

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Reviewed-by on v1 1/3
#2 lore raw

Re: [PATCH 1/3] drm/i915: Add IS_PLATFORM macro

Quoted 6 lines · Tvrtko Ursulin

From: Tvrtko Ursulin <[email protected]>

This will allow some code re-organization in a following patch.

Signed-off-by Tvrtko Ursulin <[email protected]>
Cc Jani Nikula <[email protected]>
Reviewed-by Jani Nikula <[email protected]>
Quoted 73 lines
---
 drivers/gpu/drm/i915/i915_drv.h | 52 +++++++++++++++++++++--------------------
 1 file changed, 27 insertions(+), 25 deletions(-)
drivers/gpu/drm/i915/i915_drv.h +27 -25
index bd364be2ba27..0f1fe0d58a73 100644
@@ -2978,37 +2978,39 @@ 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_I830(dev_priv)	((dev_priv)->info.platform == INTEL_I830)
-#define IS_I845G(dev_priv)	((dev_priv)->info.platform == INTEL_I845G)
-#define IS_I85X(dev_priv)	((dev_priv)->info.platform == INTEL_I85X)
-#define IS_I865G(dev_priv)	((dev_priv)->info.platform == INTEL_I865G)
-#define IS_I915G(dev_priv)	((dev_priv)->info.platform == INTEL_I915G)
-#define IS_I915GM(dev_priv)	((dev_priv)->info.platform == INTEL_I915GM)
-#define IS_I945G(dev_priv)	((dev_priv)->info.platform == INTEL_I945G)
-#define IS_I945GM(dev_priv)	((dev_priv)->info.platform == INTEL_I945GM)
-#define IS_I965G(dev_priv)	((dev_priv)->info.platform == INTEL_I965G)
-#define IS_I965GM(dev_priv)	((dev_priv)->info.platform == INTEL_I965GM)
-#define IS_G45(dev_priv)	((dev_priv)->info.platform == INTEL_G45)
-#define IS_GM45(dev_priv)	((dev_priv)->info.platform == INTEL_GM45)
+#define IS_PLATFORM(dev_priv, p) ((dev_priv)->info.platform == (p))
+
+#define IS_I830(dev_priv)	IS_PLATFORM(dev_priv, INTEL_I830)
+#define IS_I845G(dev_priv)	IS_PLATFORM(dev_priv, INTEL_I845G)
+#define IS_I85X(dev_priv)	IS_PLATFORM(dev_priv, INTEL_I85X)
+#define IS_I865G(dev_priv)	IS_PLATFORM(dev_priv, INTEL_I865G)
+#define IS_I915G(dev_priv)	IS_PLATFORM(dev_priv, INTEL_I915G)
+#define IS_I915GM(dev_priv)	IS_PLATFORM(dev_priv, INTEL_I915GM)
+#define IS_I945G(dev_priv)	IS_PLATFORM(dev_priv, INTEL_I945G)
+#define IS_I945GM(dev_priv)	IS_PLATFORM(dev_priv, INTEL_I945GM)
+#define IS_I965G(dev_priv)	IS_PLATFORM(dev_priv, INTEL_I965G)
+#define IS_I965GM(dev_priv)	IS_PLATFORM(dev_priv, INTEL_I965GM)
+#define IS_G45(dev_priv)	IS_PLATFORM(dev_priv, INTEL_G45)
+#define IS_GM45(dev_priv)	IS_PLATFORM(dev_priv, INTEL_GM45)
 #define IS_G4X(dev_priv)	(IS_G45(dev_priv) || IS_GM45(dev_priv))
 #define IS_PINEVIEW_G(dev_priv)	(INTEL_DEVID(dev_priv) == 0xa001)
 #define IS_PINEVIEW_M(dev_priv)	(INTEL_DEVID(dev_priv) == 0xa011)
-#define IS_PINEVIEW(dev_priv)	((dev_priv)->info.platform == INTEL_PINEVIEW)
-#define IS_G33(dev_priv)	((dev_priv)->info.platform == INTEL_G33)
+#define IS_PINEVIEW(dev_priv)	IS_PLATFORM(dev_priv, INTEL_PINEVIEW)
+#define IS_G33(dev_priv)	IS_PLATFORM(dev_priv, INTEL_G33)
 #define IS_IRONLAKE_M(dev_priv)	(INTEL_DEVID(dev_priv) == 0x0046)
-#define IS_IVYBRIDGE(dev_priv)	((dev_priv)->info.platform == INTEL_IVYBRIDGE)
+#define IS_IVYBRIDGE(dev_priv)	IS_PLATFORM(dev_priv, INTEL_IVYBRIDGE)
 #define IS_IVB_GT1(dev_priv)	(IS_IVYBRIDGE(dev_priv) && \
 				 (dev_priv)->info.gt == 1)
-#define IS_VALLEYVIEW(dev_priv)	((dev_priv)->info.platform == INTEL_VALLEYVIEW)
-#define IS_CHERRYVIEW(dev_priv)	((dev_priv)->info.platform == INTEL_CHERRYVIEW)
-#define IS_HASWELL(dev_priv)	((dev_priv)->info.platform == INTEL_HASWELL)
-#define IS_BROADWELL(dev_priv)	((dev_priv)->info.platform == INTEL_BROADWELL)
-#define IS_SKYLAKE(dev_priv)	((dev_priv)->info.platform == INTEL_SKYLAKE)
-#define IS_BROXTON(dev_priv)	((dev_priv)->info.platform == INTEL_BROXTON)
-#define IS_KABYLAKE(dev_priv)	((dev_priv)->info.platform == INTEL_KABYLAKE)
-#define IS_GEMINILAKE(dev_priv)	((dev_priv)->info.platform == INTEL_GEMINILAKE)
-#define IS_COFFEELAKE(dev_priv)	((dev_priv)->info.platform == INTEL_COFFEELAKE)
-#define IS_CANNONLAKE(dev_priv)	((dev_priv)->info.platform == INTEL_CANNONLAKE)
+#define IS_VALLEYVIEW(dev_priv)	IS_PLATFORM(dev_priv, INTEL_VALLEYVIEW)
+#define IS_CHERRYVIEW(dev_priv)	IS_PLATFORM(dev_priv, INTEL_CHERRYVIEW)
+#define IS_HASWELL(dev_priv)	IS_PLATFORM(dev_priv, INTEL_HASWELL)
+#define IS_BROADWELL(dev_priv)	IS_PLATFORM(dev_priv, INTEL_BROADWELL)
+#define IS_SKYLAKE(dev_priv)	IS_PLATFORM(dev_priv, INTEL_SKYLAKE)
+#define IS_BROXTON(dev_priv)	IS_PLATFORM(dev_priv, INTEL_BROXTON)
+#define IS_KABYLAKE(dev_priv)	IS_PLATFORM(dev_priv, INTEL_KABYLAKE)
+#define IS_GEMINILAKE(dev_priv)	IS_PLATFORM(dev_priv, INTEL_GEMINILAKE)
+#define IS_COFFEELAKE(dev_priv)	IS_PLATFORM(dev_priv, INTEL_COFFEELAKE)
+#define IS_CANNONLAKE(dev_priv)	IS_PLATFORM(dev_priv, INTEL_CANNONLAKE)
 #define IS_MOBILE(dev_priv)	((dev_priv)->info.is_mobile)
 #define IS_HSW_EARLY_SDV(dev_priv) (IS_HASWELL(dev_priv) && \
 				    (INTEL_DEVID(dev_priv) & 0xFF00) == 0x0C00)
Signature · 5 lines
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
author maintainer
#3 lore raw

PATCH 2/3 drm/i915: Compact device info access by a small re-ordering

In reply to Tvrtko Ursulin

From: Tvrtko Ursulin <[email protected]>

More effort to align members on 4-byte boundary helps with code size a tiny bit:

text data bss dec hex filename

-1460454          60014    3656 1524124  17419c drivers/gpu/drm/i915/i915.ko
+1460254          60014    3656 1523924  1740d4 drivers/gpu/drm/i915/i915.ko
Signed-off-by Tvrtko Ursulin <[email protected]>
Cc Jani Nikula <[email protected]>
---
 drivers/gpu/drm/i915/i915_drv.h | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)
drivers/gpu/drm/i915/i915_drv.h +13 -7
index 0f1fe0d58a73..950aa109f8cb 100644
@@ -852,21 +852,27 @@ enum intel_platform {
 };
 
 struct intel_device_info {
-	u32 display_mmio_offset;
 	u16 device_id;
-	u8 num_pipes;
-	u8 num_sprites[I915_MAX_PIPES];
-	u8 num_scalers[I915_MAX_PIPES];
-	u8 gen;
 	u16 gen_mask;
-	enum intel_platform platform;
+
+	u8 gen;
 	u8 gt; /* GT number, 0 if undefined */
-	u8 ring_mask; /* Rings supported by the HW */
 	u8 num_rings;
+	u8 ring_mask; /* Rings supported by the HW */
+
+	enum intel_platform platform;
+
+	u32 display_mmio_offset;
+
+	u8 num_pipes;
+	u8 num_sprites[I915_MAX_PIPES];
+	u8 num_scalers[I915_MAX_PIPES];
+
 #define DEFINE_FLAG(name) u8 name:1
 	DEV_INFO_FOR_EACH_FLAG(DEFINE_FLAG);
 #undef DEFINE_FLAG
 	u16 ddb_size; /* in blocks */
+
 	/* Register offsets for the various display pipes and transcoders */
 	int pipe_offsets[I915_MAX_TRANSCODERS];
 	int trans_offsets[I915_MAX_TRANSCODERS];
Signature · 6 lines
2.9.5

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Reviewed-by on v1 2/3
#4 lore raw

Re: [PATCH 2/3] drm/i915: Compact device info access by a small re-ordering

Quoted 11 lines · Tvrtko Ursulin

From: Tvrtko Ursulin <[email protected]>

More effort to align members on 4-byte boundary helps with code size a tiny bit:

text data bss dec hex filename

-1460454          60014    3656 1524124  17419c drivers/gpu/drm/i915/i915.ko
+1460254          60014    3656 1523924  1740d4 drivers/gpu/drm/i915/i915.ko
Signed-off-by Tvrtko Ursulin <[email protected]>
Cc Jani Nikula <[email protected]>
Reviewed-by Jani Nikula <[email protected]>
Quoted 43 lines
---
 drivers/gpu/drm/i915/i915_drv.h | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)
drivers/gpu/drm/i915/i915_drv.h +13 -7
index 0f1fe0d58a73..950aa109f8cb 100644
@@ -852,21 +852,27 @@ enum intel_platform {
 };
 
 struct intel_device_info {
-	u32 display_mmio_offset;
 	u16 device_id;
-	u8 num_pipes;
-	u8 num_sprites[I915_MAX_PIPES];
-	u8 num_scalers[I915_MAX_PIPES];
-	u8 gen;
 	u16 gen_mask;
-	enum intel_platform platform;
+
+	u8 gen;
 	u8 gt; /* GT number, 0 if undefined */
-	u8 ring_mask; /* Rings supported by the HW */
 	u8 num_rings;
+	u8 ring_mask; /* Rings supported by the HW */
+
+	enum intel_platform platform;
+
+	u32 display_mmio_offset;
+
+	u8 num_pipes;
+	u8 num_sprites[I915_MAX_PIPES];
+	u8 num_scalers[I915_MAX_PIPES];
+
 #define DEFINE_FLAG(name) u8 name:1
 	DEV_INFO_FOR_EACH_FLAG(DEFINE_FLAG);
 #undef DEFINE_FLAG
 	u16 ddb_size; /* in blocks */
+
 	/* Register offsets for the various display pipes and transcoders */
 	int pipe_offsets[I915_MAX_TRANSCODERS];
 	int trans_offsets[I915_MAX_TRANSCODERS];
Signature · 5 lines
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
author maintainer
#5 lore raw

PATCH 3/3 drm/i915: Allow optimized platform checks

In reply to Tvrtko Ursulin

From: Tvrtko Ursulin <[email protected]>

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 <[email protected]>
Cc Jani Nikula <[email protected]>
---
 drivers/gpu/drm/i915/i915_drv.c | 6 ++++++
 drivers/gpu/drm/i915/i915_drv.h | 3 ++-
 2 files changed, 8 insertions(+), 1 deletion(-)
drivers/gpu/drm/i915/i915_drv.c +6 -0
index b1f96eb1be16..c3bd4b7cb19b 100644
@@ -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);
+
 	BUG_ON(device_info->gen > sizeof(device_info->gen_mask) * BITS_PER_BYTE);
 	device_info->gen_mask = BIT(device_info->gen - 1);
drivers/gpu/drm/i915/i915_drv.h +2 -1
index 950aa109f8cb..81211f23326a 100644
@@ -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))
 
 #define IS_I830(dev_priv)	IS_PLATFORM(dev_priv, INTEL_I830)
 #define IS_I845G(dev_priv)	IS_PLATFORM(dev_priv, INTEL_I845G)
Signature · 6 lines
2.9.5

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
#6 lore raw

Re: [PATCH 3/3] drm/i915: Allow optimized platform checks

Quoted 7 lines · Tvrtko Ursulin

From: Tvrtko Ursulin <[email protected]>

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.

Quoted 5 lines

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

Forgot to say, this patch could also be called "32 platforms should be enough for everyone". :) Once we go over it (at 27 at the moment) we'd have to bump the platform_mask to u64 and the overall effect might be negative then. I better check that..

Regards,

Tvrtko

Quoted 38 lines
Signed-off-by Tvrtko Ursulin <[email protected]>
Cc Jani Nikula <[email protected]>
---
  drivers/gpu/drm/i915/i915_drv.c | 6 ++++++
  drivers/gpu/drm/i915/i915_drv.h | 3 ++-
  2 files changed, 8 insertions(+), 1 deletion(-)
drivers/gpu/drm/i915/i915_drv.c +6 -0
index b1f96eb1be16..c3bd4b7cb19b 100644
@@ -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);
+
  	BUG_ON(device_info->gen > sizeof(device_info->gen_mask) * BITS_PER_BYTE);
  	device_info->gen_mask = BIT(device_info->gen - 1);
drivers/gpu/drm/i915/i915_drv.h +1 -0
index 950aa109f8cb..81211f23326a 100644
@@ -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;

drivers/gpu/drm/i915/i915_drv.h intel_info(const struct drm_i915_private *dev_priv) · line 2991

Quoted 8 lines
drivers/gpu/drm/i915/i915_drv.h +1 -1
@@ -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))
 
  #define IS_I830(dev_priv)	IS_PLATFORM(dev_priv, INTEL_I830)
  #define IS_I845G(dev_priv)	IS_PLATFORM(dev_priv, INTEL_I845G)

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

#7 lore raw

Re: [PATCH 3/3] drm/i915: Allow optimized platform checks

In reply to Tvrtko Ursulin
Quoted 25 lines · Tvrtko Ursulin

From: Tvrtko Ursulin <[email protected]>

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 <[email protected]>
Cc Jani Nikula <[email protected]>
---
 drivers/gpu/drm/i915/i915_drv.c | 6 ++++++
 drivers/gpu/drm/i915/i915_drv.h | 3 ++-
 2 files changed, 8 insertions(+), 1 deletion(-)
drivers/gpu/drm/i915/i915_drv.c +0 -0
index b1f96eb1be16..c3bd4b7cb19b 100644

drivers/gpu/drm/i915/i915_drv.c static int i915_driver_init_early(struct drm_i915_private *dev_priv, · line 876

Quoted 8 lines
drivers/gpu/drm/i915/i915_drv.c +5 -0
@@ -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?

Quoted 17 lines
drivers/gpu/drm/i915/i915_drv.c +1 -0
+
 	BUG_ON(device_info->gen > sizeof(device_info->gen_mask) * BITS_PER_BYTE);
 	device_info->gen_mask = BIT(device_info->gen - 1);
drivers/gpu/drm/i915/i915_drv.h +1 -0
index 950aa109f8cb..81211f23326a 100644
@@ -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;

drivers/gpu/drm/i915/i915_drv.h intel_info(const struct drm_i915_private *dev_priv) · line 2988

Quoted 5 lines
drivers/gpu/drm/i915/i915_drv.h +1 -1
@@ -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))

BR,
Jani.

Quoted 3 lines
drivers/gpu/drm/i915/i915_drv.h +0 -0
 #define IS_I830(dev_priv)	IS_PLATFORM(dev_priv, INTEL_I830)
 #define IS_I845G(dev_priv)	IS_PLATFORM(dev_priv, INTEL_I845G)
Signature · 5 lines
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
#8 lore raw

Re: [PATCH 3/3] drm/i915: Allow optimized platform checks

Quoted 57 lines · Jani Nikula
Quoted 25 lines · Tvrtko Ursulin

From: Tvrtko Ursulin <[email protected]>

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 <[email protected]>
Cc Jani Nikula <[email protected]>
---
  drivers/gpu/drm/i915/i915_drv.c | 6 ++++++
  drivers/gpu/drm/i915/i915_drv.h | 3 ++-
  2 files changed, 8 insertions(+), 1 deletion(-)
drivers/gpu/drm/i915/i915_drv.c +0 -0
index b1f96eb1be16..c3bd4b7cb19b 100644

drivers/gpu/drm/i915/i915_drv.c static int i915_driver_init_early(struct drm_i915_private *dev_priv, · line 876

Quoted 8 lines
drivers/gpu/drm/i915/i915_drv.c +5 -0
@@ -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?

Quoted 20 lines
drivers/gpu/drm/i915/i915_drv.c +1 -0
+
  	BUG_ON(device_info->gen > sizeof(device_info->gen_mask) * BITS_PER_BYTE);
  	device_info->gen_mask = BIT(device_info->gen - 1);
drivers/gpu/drm/i915/i915_drv.h +1 -0
index 950aa109f8cb..81211f23326a 100644
@@ -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))
Quoted 6 lines

drivers/gpu/drm/i915/i915_drv.h intel_info(const struct drm_i915_private *dev_priv) · line 2988

Quoted 2 lines
drivers/gpu/drm/i915/i915_drv.h +1 -1
-#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.

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

#9 lore raw

Re: [PATCH 3/3] drm/i915: Allow optimized platform checks

Quoted 61 lines · Tvrtko Ursulin
Quoted 60 lines · Jani Nikula
Quoted 27 lines · Tvrtko Ursulin

From: Tvrtko Ursulin <[email protected]>

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 <[email protected]>
Cc Jani Nikula <[email protected]>

---
  drivers/gpu/drm/i915/i915_drv.c | 6 ++++++ drivers/gpu/drm/i915/i915_drv.h | 3 ++-

  2 files changed, 8 insertions(+), 1 deletion(-)
drivers/gpu/drm/i915/i915_drv.c +0 -0
index b1f96eb1be16..c3bd4b7cb19b 100644

drivers/gpu/drm/i915/i915_drv.c static int i915_driver_init_early(struct · line 875

Quoted 7 lines
drivers/gpu/drm/i915/i915_drv.c +4 -0
@@ -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?

Quoted 13 lines
drivers/gpu/drm/i915/i915_drv.c +1 -0
+
       BUG_ON(device_info->gen > sizeof(device_info->gen_mask) * BITS_PER_BYTE);
       device_info->gen_mask = BIT(device_info->gen - 1);
drivers/gpu/drm/i915/i915_drv.h +0 -0
index 950aa109f8cb..81211f23326a 100644
@@ -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;

drivers/gpu/drm/i915/i915_drv.h intel_info(const struct drm_i915_private · line 2989

Quoted 6 lines
drivers/gpu/drm/i915/i915_drv.h +1 -1
@@ -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:

Quoted 3 lines
Quoted 1 line

#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
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

CI
#10 lore raw

✓ Fi.CI.BAT: success for Claw back optimised IS_PLATFORM checks

In reply to Tvrtko Ursulin
== Series Details ==

== Series Details ==

Series: Claw back optimised IS_PLATFORM checks
URL : https://patchwork.freedesktop.org/series/30647/
State : success

== Summary ==

Series 30647v1 Claw back optimised IS_PLATFORM checks
https://patchwork.freedesktop.org/api/1.0/series/30647/revisions/1/mbox/

Test chamelium:

        Subgroup dp-edid-read:
                pass       -> FAIL       (fi-kbl-7500u) fdo#102672 +1
        Subgroup dp-crc-fast:
                pass       -> DMESG-FAIL (fi-kbl-7500u) fdo#102514
Test kms_cursor_legacy:
        Subgroup basic-busy-flip-before-cursor-atomic:
                pass       -> FAIL       (fi-snb-2600) fdo#100215 +1
Test pm_rpm:
        Subgroup basic-rte:
                pass       -> DMESG-WARN (fi-cfl-s) fdo#102294
Test drv_module_reload:
        Subgroup basic-reload:
                dmesg-warn -> PASS       (fi-glk-1) fdo#102777

fdo#102672 https://bugs.freedesktop.org/show_bug.cgi?id=102672
fdo#102514 https://bugs.freedesktop.org/show_bug.cgi?id=102514
fdo#100215 https://bugs.freedesktop.org/show_bug.cgi?id=100215
fdo#102294 https://bugs.freedesktop.org/show_bug.cgi?id=102294
fdo#102777 https://bugs.freedesktop.org/show_bug.cgi?id=102777

fi-bdw-5557u     total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:452s
fi-bdw-gvtdvm    total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:466s
fi-blb-e6850     total:289  pass:224  dwarn:1   dfail:0   fail:0   skip:64  time:419s
fi-bsw-n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  time:507s
fi-bwr-2160      total:289  pass:184  dwarn:0   dfail:0   fail:0   skip:105 time:276s
fi-bxt-j4205     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:508s
fi-byt-j1900     total:289  pass:254  dwarn:1   dfail:0   fail:0   skip:34  time:491s
fi-byt-n2820     total:289  pass:250  dwarn:1   dfail:0   fail:0   skip:38  time:494s
fi-cfl-s         total:289  pass:222  dwarn:35  dfail:0   fail:0   skip:32  time:535s
fi-elk-e7500     total:289  pass:230  dwarn:0   dfail:0   fail:0   skip:59  time:419s
fi-glk-1         total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:561s
fi-hsw-4770      total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:426s
fi-hsw-4770r     total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:403s
fi-ilk-650       total:289  pass:229  dwarn:0   dfail:0   fail:0   skip:60  time:426s
fi-ivb-3520m     total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:489s
fi-ivb-3770      total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:463s
fi-kbl-7500u     total:289  pass:262  dwarn:1   dfail:1   fail:1   skip:24  time:473s
fi-kbl-7560u     total:289  pass:270  dwarn:0   dfail:0   fail:0   skip:19  time:573s
fi-kbl-r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:588s
fi-skl-6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:451s
fi-skl-6700k     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:746s
fi-skl-6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:494s
fi-skl-gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  time:469s
fi-snb-2520m     total:289  pass:251  dwarn:0   dfail:0   fail:0   skip:38  time:567s
fi-snb-2600      total:289  pass:249  dwarn:0   dfail:0   fail:1   skip:39  time:417s

85e28edde5bfb81a40cc95ce27cd84230114d6f0 drm-tip: 2017y-09m-19d-23h-07m-57s UTC integration manifest 2d28cb18ba11 drm/i915: Allow optimized platform checks
33348eb5a828 drm/i915: Compact device info access by a small re-ordering 1e6aa77c6f95 drm/i915: Add IS_PLATFORM macro

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5763/
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

CI
#11 lore raw

✗ Fi.CI.IGT: failure for Claw back optimised IS_PLATFORM checks

In reply to Tvrtko Ursulin
== Series Details ==

== Series Details ==

Series: Claw back optimised IS_PLATFORM checks
URL : https://patchwork.freedesktop.org/series/30647/
State : failure

== Summary ==

Test kms_cursor_legacy:

        Subgroup cursor-vs-flip-toggle:
                pass       -> FAIL       (shard-hsw)
Test kms_setmode:
        Subgroup basic:
                fail       -> PASS       (shard-hsw) fdo#99912
Test kms_frontbuffer_tracking:
        Subgroup fbc-1p-primscrn-pri-indfb-draw-pwrite:
                pass       -> SKIP       (shard-hsw)
Test perf:
        Subgroup blocking:
                fail       -> PASS       (shard-hsw) fdo#102252

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

shard-hsw total:2317 pass:1243 dwarn:3 dfail:0 fail:13 skip:1058 time:9555s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5763/shards.html
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx