On 03/04/2019 09:15, Lucas De Marchi wrote:
On Tue, Apr 2, 2019 at 11:58 PM Tvrtko Ursulin
<tvrtko.ursulin@xxxxxxxxxxxxxxx> wrote:
On 03/04/2019 02:46, Lucas De Marchi wrote:
No reason to stick to u32 for platform mask if we can just use more bits
on 64 bit platforms.
$ size drivers/gpu/drm/i915/i915.ko*
text data bss dec hex filename
1884779 41334 5408 1931521 1d7901 drivers/gpu/drm/i915/i915.ko
1886693 41358 5408 1933459 1d8093 drivers/gpu/drm/i915/i915.ko.old
How did you get such a large difference, and decrease even? Could you
check in the code what is happening? Because I get an increase with this
patch:
text data bss dec hex filename
1905314 43903 7424 1956641 1ddb21 i915.ko.orig
1905796 43903 7424 1957123 1ddd03 i915.ko.patch
the only explanation I really have is that my measurement was bogus.
Some possible explanations...
1) I compared a i386 to a x86-64 build; 2) somehow a config changed
between the builds;
3) when preparing the patch I rebased on upstream between the builds.
Checking (1), no... that's in the ~400k range. So no idea, sorry.
I was worried you'd say you compiler just behaves differently. To
eliminate this option it would still be good to double check if you can
find the time.
So I think the only useful thing in this patch is to make the array to
grow automatically. Or maybe not even that?
I really liked that and then started thinking that it can still sneak up
a mistake if one changes the type of the member and forgets to change
the type in size calculation BITS_PER_TYPE. So I ended up a little less
sure. Could the calculation self-reference the struct member?
Regards,
Tvrtko
Lucas De Marchi
Regards,
Tvrtko
Now on 64 bits we have only one long as opposed to 2 u32:
$ pahole -C intel_runtime_info drivers/gpu/drm/i915/i915.ko
struct intel_runtime_info {
long unsigned int platform_mask[1]; /* 0 8 */
...
}
On 32 bits we still have the same thing as before:
$ size drivers/gpu/drm/i915/i915.ko*
text data bss dec hex filename
1489839 32485 2816 1525140 174594 drivers/gpu/drm/i915/i915.ko
1489839 32485 2816 1525140 174594 drivers/gpu/drm/i915/i915.ko.old
Besides reducing the code on x86-64 now the array size is automatically
calculated and we don't have to worry about extending it anymore.
v2: fix sparse and checkpatch warnings
Signed-off-by: Lucas De Marchi <lucas.demarchi@xxxxxxxxx>
---
drivers/gpu/drm/i915/i915_drv.h | 6 +-----
drivers/gpu/drm/i915/intel_device_info.h | 7 +++----
2 files changed, 4 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 0ab4826921f7..9fe765ffe878 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2309,10 +2309,6 @@ __platform_mask_index(const struct intel_runtime_info *info,
const unsigned int pbits =
BITS_PER_TYPE(info->platform_mask[0]) - INTEL_SUBPLATFORM_BITS;
- /* Expand the platform_mask array if this fails. */
- BUILD_BUG_ON(INTEL_MAX_PLATFORMS >
- pbits * ARRAY_SIZE(info->platform_mask));
-
return p / pbits;
}
@@ -2354,7 +2350,7 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
const unsigned int pi = __platform_mask_index(info, p);
const unsigned int pb = __platform_mask_bit(info, p);
const unsigned int msb = BITS_PER_TYPE(info->platform_mask[0]) - 1;
- const u32 mask = info->platform_mask[pi];
+ const unsigned long mask = info->platform_mask[pi];
BUILD_BUG_ON(!__builtin_constant_p(p));
BUILD_BUG_ON(!__builtin_constant_p(s));
diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
index 0e579f158016..2f5ca2b6f094 100644
--- a/drivers/gpu/drm/i915/intel_device_info.h
+++ b/drivers/gpu/drm/i915/intel_device_info.h
@@ -214,11 +214,10 @@ struct intel_runtime_info {
* Platform mask is used for optimizing or-ed IS_PLATFORM calls into
* into single runtime conditionals, and also to provide groundwork
* for future per platform, or per SKU build optimizations.
- *
- * Array can be extended when necessary if the corresponding
- * BUILD_BUG_ON is hit.
*/
- u32 platform_mask[2];
+ unsigned long platform_mask[DIV_ROUND_UP(INTEL_MAX_PLATFORMS,
+ BITS_PER_TYPE(unsigned long)
+ - INTEL_SUBPLATFORM_BITS)];
u16 device_id;
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
