* [PATCH 0/3] Claw back optimised IS_PLATFORM checks
@ 2017-09-20 9:26 Tvrtko Ursulin
2017-09-20 9:26 ` [PATCH 1/3] drm/i915: Add IS_PLATFORM macro Tvrtko Ursulin
` (4 more replies)
0 siblings, 5 replies; 12+ messages in thread
From: Tvrtko Ursulin @ 2017-09-20 9:26 UTC (permalink / raw)
To: Intel-gfx
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
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(-)
--
2.9.5
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/3] drm/i915: Add IS_PLATFORM macro
2017-09-20 9:26 [PATCH 0/3] Claw back optimised IS_PLATFORM checks Tvrtko Ursulin
@ 2017-09-20 9:26 ` Tvrtko Ursulin
2017-09-20 9:34 ` Jani Nikula
2017-09-20 9:27 ` [PATCH 2/3] drm/i915: Compact device info access by a small re-ordering Tvrtko Ursulin
` (3 subsequent siblings)
4 siblings, 1 reply; 12+ messages in thread
From: Tvrtko Ursulin @ 2017-09-20 9:26 UTC (permalink / raw)
To: Intel-gfx; +Cc: Jani Nikula
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
This will allow some code re-organization in a following patch.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 52 +++++++++++++++++++++--------------------
1 file changed, 27 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index bd364be2ba27..0f1fe0d58a73 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -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)
--
2.9.5
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/3] drm/i915: Compact device info access by a small re-ordering
2017-09-20 9:26 [PATCH 0/3] Claw back optimised IS_PLATFORM checks Tvrtko Ursulin
2017-09-20 9:26 ` [PATCH 1/3] drm/i915: Add IS_PLATFORM macro Tvrtko Ursulin
@ 2017-09-20 9:27 ` Tvrtko Ursulin
2017-09-20 9:34 ` Jani Nikula
2017-09-20 9:27 ` [PATCH 3/3] drm/i915: Allow optimized platform checks Tvrtko Ursulin
` (2 subsequent siblings)
4 siblings, 1 reply; 12+ messages in thread
From: Tvrtko Ursulin @ 2017-09-20 9:27 UTC (permalink / raw)
To: Intel-gfx; +Cc: Jani Nikula
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
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 <tvrtko.ursulin@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 20 +++++++++++++-------
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 0f1fe0d58a73..950aa109f8cb 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -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];
--
2.9.5
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 3/3] drm/i915: Allow optimized platform checks
2017-09-20 9:26 [PATCH 0/3] Claw back optimised IS_PLATFORM checks Tvrtko Ursulin
2017-09-20 9:26 ` [PATCH 1/3] drm/i915: Add IS_PLATFORM macro Tvrtko Ursulin
2017-09-20 9:27 ` [PATCH 2/3] drm/i915: Compact device info access by a small re-ordering Tvrtko Ursulin
@ 2017-09-20 9:27 ` Tvrtko Ursulin
2017-09-20 9:39 ` Jani Nikula
2017-09-20 9:39 ` Tvrtko Ursulin
2017-09-20 9:48 ` ✓ Fi.CI.BAT: success for Claw back optimised IS_PLATFORM checks Patchwork
2017-09-20 10:52 ` ✗ Fi.CI.IGT: failure " Patchwork
4 siblings, 2 replies; 12+ messages in thread
From: Tvrtko Ursulin @ 2017-09-20 9:27 UTC (permalink / raw)
To: Intel-gfx; +Cc: Jani Nikula
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
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@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
---
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);
+
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))
#define IS_I830(dev_priv) IS_PLATFORM(dev_priv, INTEL_I830)
#define IS_I845G(dev_priv) IS_PLATFORM(dev_priv, INTEL_I845G)
--
2.9.5
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] drm/i915: Add IS_PLATFORM macro
2017-09-20 9:26 ` [PATCH 1/3] drm/i915: Add IS_PLATFORM macro Tvrtko Ursulin
@ 2017-09-20 9:34 ` Jani Nikula
0 siblings, 0 replies; 12+ messages in thread
From: Jani Nikula @ 2017-09-20 9:34 UTC (permalink / raw)
To: Tvrtko Ursulin, Intel-gfx
On Wed, 20 Sep 2017, Tvrtko Ursulin <tursulin@ursulin.net> wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> This will allow some code re-organization in a following patch.
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 52 +++++++++++++++++++++--------------------
> 1 file changed, 27 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index bd364be2ba27..0f1fe0d58a73 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -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)
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] drm/i915: Compact device info access by a small re-ordering
2017-09-20 9:27 ` [PATCH 2/3] drm/i915: Compact device info access by a small re-ordering Tvrtko Ursulin
@ 2017-09-20 9:34 ` Jani Nikula
0 siblings, 0 replies; 12+ messages in thread
From: Jani Nikula @ 2017-09-20 9:34 UTC (permalink / raw)
To: Tvrtko Ursulin, Intel-gfx
On Wed, 20 Sep 2017, Tvrtko Ursulin <tursulin@ursulin.net> wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> 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 <tvrtko.ursulin@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 20 +++++++++++++-------
> 1 file changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 0f1fe0d58a73..950aa109f8cb 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -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];
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] drm/i915: Allow optimized platform checks
2017-09-20 9:27 ` [PATCH 3/3] drm/i915: Allow optimized platform checks Tvrtko Ursulin
@ 2017-09-20 9:39 ` Jani Nikula
2017-09-20 9:56 ` Tvrtko Ursulin
2017-09-20 9:39 ` Tvrtko Ursulin
1 sibling, 1 reply; 12+ messages in thread
From: Jani Nikula @ 2017-09-20 9:39 UTC (permalink / raw)
To: Tvrtko Ursulin, Intel-gfx
On Wed, 20 Sep 2017, Tvrtko Ursulin <tursulin@ursulin.net> wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> 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@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> ---
> 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))
BR,
Jani.
>
> #define IS_I830(dev_priv) IS_PLATFORM(dev_priv, INTEL_I830)
> #define IS_I845G(dev_priv) IS_PLATFORM(dev_priv, INTEL_I845G)
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] drm/i915: Allow optimized platform checks
2017-09-20 9:27 ` [PATCH 3/3] drm/i915: Allow optimized platform checks Tvrtko Ursulin
2017-09-20 9:39 ` Jani Nikula
@ 2017-09-20 9:39 ` Tvrtko Ursulin
1 sibling, 0 replies; 12+ messages in thread
From: Tvrtko Ursulin @ 2017-09-20 9:39 UTC (permalink / raw)
To: Tvrtko Ursulin, Intel-gfx; +Cc: Jani Nikula
On 20/09/2017 10:27, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> 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
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
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> ---
> 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);
> +
> 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))
>
> #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
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* ✓ Fi.CI.BAT: success for Claw back optimised IS_PLATFORM checks
2017-09-20 9:26 [PATCH 0/3] Claw back optimised IS_PLATFORM checks Tvrtko Ursulin
` (2 preceding siblings ...)
2017-09-20 9:27 ` [PATCH 3/3] drm/i915: Allow optimized platform checks Tvrtko Ursulin
@ 2017-09-20 9:48 ` Patchwork
2017-09-20 10:52 ` ✗ Fi.CI.IGT: failure " Patchwork
4 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2017-09-20 9:48 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: intel-gfx
== 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
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] drm/i915: Allow optimized platform checks
2017-09-20 9:39 ` Jani Nikula
@ 2017-09-20 9:56 ` Tvrtko Ursulin
2017-09-25 8:22 ` Tvrtko Ursulin
0 siblings, 1 reply; 12+ messages in thread
From: Tvrtko Ursulin @ 2017-09-20 9:56 UTC (permalink / raw)
To: Jani Nikula, Tvrtko Ursulin, Intel-gfx
On 20/09/2017 10:39, Jani Nikula wrote:
> On Wed, 20 Sep 2017, Tvrtko Ursulin <tursulin@ursulin.net> wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> 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@intel.com>
>> Cc: Jani Nikula <jani.nikula@intel.com>
>> ---
>> 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.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* ✗ Fi.CI.IGT: failure for Claw back optimised IS_PLATFORM checks
2017-09-20 9:26 [PATCH 0/3] Claw back optimised IS_PLATFORM checks Tvrtko Ursulin
` (3 preceding siblings ...)
2017-09-20 9:48 ` ✓ Fi.CI.BAT: success for Claw back optimised IS_PLATFORM checks Patchwork
@ 2017-09-20 10:52 ` Patchwork
4 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2017-09-20 10:52 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: intel-gfx
== 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
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] drm/i915: Allow optimized platform checks
2017-09-20 9:56 ` Tvrtko Ursulin
@ 2017-09-25 8:22 ` Tvrtko Ursulin
0 siblings, 0 replies; 12+ messages in thread
From: Tvrtko Ursulin @ 2017-09-25 8:22 UTC (permalink / raw)
To: Jani Nikula, Tvrtko Ursulin, Intel-gfx
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@ursulin.net> wrote:
>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>
>>> 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@intel.com>
>>> Cc: Jani Nikula <jani.nikula@intel.com>
>>> ---
>>> 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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2017-09-25 8:22 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-09-20 9:26 [PATCH 0/3] Claw back optimised IS_PLATFORM checks Tvrtko Ursulin
2017-09-20 9:26 ` [PATCH 1/3] drm/i915: Add IS_PLATFORM macro Tvrtko Ursulin
2017-09-20 9:34 ` Jani Nikula
2017-09-20 9:27 ` [PATCH 2/3] drm/i915: Compact device info access by a small re-ordering Tvrtko Ursulin
2017-09-20 9:34 ` Jani Nikula
2017-09-20 9:27 ` [PATCH 3/3] drm/i915: Allow optimized platform checks Tvrtko Ursulin
2017-09-20 9:39 ` Jani Nikula
2017-09-20 9:56 ` Tvrtko Ursulin
2017-09-25 8:22 ` Tvrtko Ursulin
2017-09-20 9:39 ` Tvrtko Ursulin
2017-09-20 9:48 ` ✓ Fi.CI.BAT: success for Claw back optimised IS_PLATFORM checks Patchwork
2017-09-20 10:52 ` ✗ Fi.CI.IGT: failure " Patchwork
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox