[Intel-gfx] [PATCH 00/22] .rodata diet
From: Tvrtko Ursulin
Dynamic typing in __I915__ (INTEL_INFO) has and unfortuante consequence that
for every time it is called under a WARN it generates a very verbose string
placed into the appropriate .rodata section.
Each instance of that can can several hundred bytes to the binary. One example
of such strings found in the binary, not word-wrapped is:
WARN_ON(!((&({ struct drm_i915_private *__p; if
(__builtin_types_compatible_p(typeof(*dev_priv), struct drm_i915_private)) __p
= (struct drm_i915_private *)dev_priv; else if
(__builtin_types_compatible_p(typeof(*dev_priv), struct drm_device)) __p =
to_i915((struct drm_device *)dev_priv); else do { bool __cond = !(!(1)); extern
void __compiletime_assert_1912(void) __attribute__((error("BUILD_BUG
failed"))); if (__cond) __compiletime_assert_1912(); do { } while (0); } while
(0); __p; })->info)->has_ddi) && (control & (0x << 16)) != (0xabcd << 16))
If we gradually remove dynamic typing abilities from individual macros we can
start bringing the size of the binary down.
For example after this series:
textdata bss dec hex filename
1067727 23256 576 1091559 10a7e7 i915.ko.nightly
1038202 23256 576 1062034 103492 i915.ko.diet
Which is a ~29KiB saving.
This is disruptive of course, but perhaps it is time to bite the bullet since we
now have a situation that even new platforms like Kabylake are adding code which
uses the wrong thing in those macros (dev instead of dev_priv).
The way I have done it here makes it impossible to use the converted macros in
a wrong way going forward.
P.S. Series starts with three diffrent type of .rodata shrinkage patches. I was
just lazy to split that up.
Tvrtko Ursulin (22):
drm/i915: Shrink cxsr_latency_table
drm/i915: Shrink sdvo_cmd_names
drm/i915: Shrink per-platform watermark configuration
drm/i915: Make HAS_DDI and HAS_PCH_LPT_LP only take dev_priv
drm/i915: Make INTEL_PCH_TYPE & co only take dev_priv
drm/i915: Make HAS_GMCH_DISPLAY only take dev_priv
drm/i915: Make HAS_RUNTIME_PM only take dev_priv
drm/i915: Do not use INTEL_INFO(dev_priv)->ring_mask inside WARNs
drm/i915: Make IS_GEN-range macro only take dev_priv
drm/i915: Make INTEL_DEVID only take dev_priv
drm/i915: Make IS_IVYBRIDGE only take dev_priv
drm/i915: Make IS_BROADWELL only take dev_priv
drm/i915: Make IS_HASWELL only take dev_priv
drm/i915: Make IS_KABYLAKE only take dev_priv
drm/i915: Make IS_SKYLAKE only take dev_priv
drm/i915: Make IS_BROXTON only take dev_priv
drm/i915: Make HAS_L3_DPF only take dev_priv
drm/i915: Make IS_G4X only take dev_priv
drm/i915: Make IS_CHERRYVIEW only take dev_priv
drm/i915: Make IS_VALLEYVIEW only take dev_priv
drm/i915: Make INTEL_GEN only take dev_priv
drm/i915: Make IS_GEN macros only take dev_priv
drivers/gpu/drm/i915/i915_debugfs.c | 4 +-
drivers/gpu/drm/i915/i915_drv.c | 63 ++--
drivers/gpu/drm/i915/i915_drv.h | 198 ++---
drivers/gpu/drm/i915/i915_gem.c | 55 ++--
drivers/gpu/drm/i915/i915_gem_context.c | 2 +-
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 4 +-
drivers/gpu/drm/i915/i915_gem_fence.c| 11 +-
drivers/gpu/drm/i915/i915_gem_gtt.c | 60 ++--
drivers/gpu/drm/i915/i915_gem_render_state.c | 6 +-
drivers/gpu/drm/i915/i915_gem_stolen.c | 17 +-
drivers/gpu/drm/i915/i915_gem_tiling.c | 7 +-
drivers/gpu/drm/i915/i915_gpu_error.c| 18 +-
drivers/gpu/drm/i915/i915_irq.c | 34 +--
drivers/gpu/drm/i915/i915_reg.h | 4 +-
drivers/gpu/drm/i915/i915_suspend.c | 8 +-
drivers/gpu/drm/i915/intel_audio.c | 6 +-
drivers/gpu/drm/i915/intel_color.c | 16 +-
drivers/gpu/drm/i915/intel_crt.c | 53 ++--
drivers/gpu/drm/i915/intel_ddi.c | 22 +-
drivers/gpu/drm/i915/intel_display.c | 416 ++-
drivers/gpu/drm/i915/intel_dp.c | 165 +--
drivers/gpu/drm/i915/intel_dpll_mgr.c| 10 +-
drivers/gpu/drm/i915/intel_drv.h | 28 +-
drivers/gpu/drm/i915/intel_dsi.c | 37 ++-
drivers/gpu/drm/i915/intel_dsi_pll.c | 26 +-
drivers/gpu/drm/i915/intel_engine_cs.c | 7 +-
drivers/gpu/drm/i915/intel_fifo_underrun.c | 8 +-
drivers/gpu/drm/i915/intel_guc_loader.c | 15 +-
drivers/gpu/drm/i915/intel_hdmi.c| 58 ++--
drivers/gpu/drm/i915/intel_i2c.c | 9 +-
drivers/gpu/drm/i915/intel_lvds.c| 29 +-
drivers/gpu/drm/i915/intel_pm.c | 166 +--
drivers/gpu/drm/i915/intel_psr.c | 22 +-
drivers/gpu/drm/i915/intel_runtime_pm.c | 15 +-
drivers/gpu/drm/i915/intel_sdvo.c| 25 +-
drivers/gpu/drm/i915/intel_sprite.c | 30 +-
drivers/gpu/drm/i915/intel_tv.c | 4 +-
37 files changed, 851 insertions(+), 807 deletions(-)
--
2.7.4
__
Re: [Intel-gfx] [PATCH 00/22] .rodata diet
On Wed, 05 Oct 2016, Tvrtko Ursulin wrote: > Dynamic typing in __I915__ (INTEL_INFO) has and unfortuante consequence that > for every time it is called under a WARN it generates a very verbose string > placed into the appropriate .rodata section. AFAIK the idea all along was to use this "dynamic typing" for a transition period. I'm in favor of moving towards only accepting dev_priv. I'm a bit hesitant to make the change in one go, though. BR, Jani. -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 00/22] .rodata diet
On 05/10/2016 14:01, Jani Nikula wrote: On Wed, 05 Oct 2016, Tvrtko Ursulin wrote: Dynamic typing in __I915__ (INTEL_INFO) has and unfortuante consequence that for every time it is called under a WARN it generates a very verbose string placed into the appropriate .rodata section. AFAIK the idea all along was to use this "dynamic typing" for a transition period. I'm in favor of moving towards only accepting dev_priv. I'm a bit hesitant to make the change in one go, though. Oh right, maybe polymorphism would have been a better term to use. I know that it was a temporary solution but did not realize until yesterday what unbearable log messages it produces for WARNs and the bloat it adds. Anyway, there are several arguments to be made here. First of all, it has probably been some months since the last time people tried this (maybe in a slightly different form) so one point is that there will probably never be a good time for this amount of churn. However you do it it is so sprinkled all over that it is probably true. Secondly, this series does not go all the way. The issue of converting things alike INTEL_INFO(*)->gen to INTEL_GEN remains. It would be another good chunk of work on top of this. Only when that is done could the __I915__ magic go away (more or less). Positive is that every patch in this series helps a bit by locking down a particular macro to dev_priv. This is since it seems we are not catching and stopping all the new code which does the wrong thing during review. So applying a sub part of this series is still a good step IMHO. Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 00/22] .rodata diet
On Wed, Oct 05, 2016 at 02:39:16PM +0100, Tvrtko Ursulin wrote: > > On 05/10/2016 14:01, Jani Nikula wrote: > > >On Wed, 05 Oct 2016, Tvrtko Ursulin wrote: > >>Dynamic typing in __I915__ (INTEL_INFO) has and unfortuante consequence that > >>for every time it is called under a WARN it generates a very verbose string > >>placed into the appropriate .rodata section. > >AFAIK the idea all along was to use this "dynamic typing" for a > >transition period. I'm in favor of moving towards only accepting > >dev_priv. I'm a bit hesitant to make the change in one go, though. > > Oh right, maybe polymorphism would have been a better term to use. I > know that it was a temporary solution but did not realize until > yesterday what unbearable log messages it produces for WARNs and the > bloat it adds. > > Anyway, there are several arguments to be made here. > > First of all, it has probably been some months since the last time > people tried this (maybe in a slightly different form) so one point > is that there will probably never be a good time for this amount of > churn. However you do it it is so sprinkled all over that it is > probably true. The challenge with such sweeping changes is coordination of the trees. David's fell flat because patches against -nightly couldn't be applied against dinq. So for those we really need a back-merge + application. > Secondly, this series does not go all the way. The issue of > converting things alike INTEL_INFO(*)->gen to INTEL_GEN remains. It > would be another good chunk of work on top of this. Only when that > is done could the __I915__ magic go away (more or less). > > Positive is that every patch in this series helps a bit by locking > down a particular macro to dev_priv. This is since it seems we are > not catching and stopping all the new code which does the wrong > thing during review. So applying a sub part of this series is still > a good step IMHO. Yes, it does look gentler, and the .rodata does expain where David was seeing the huge gains from the conversion. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 00/22] .rodata diet
On Wed, Oct 05, 2016 at 01:33:27PM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin
>
> Dynamic typing in __I915__ (INTEL_INFO) has and unfortuante consequence that
> for every time it is called under a WARN it generates a very verbose string
> placed into the appropriate .rodata section.
>
> Each instance of that can can several hundred bytes to the binary. One example
> of such strings found in the binary, not word-wrapped is:
>
> WARN_ON(!((&({ struct drm_i915_private *__p; if
> (__builtin_types_compatible_p(typeof(*dev_priv), struct drm_i915_private))
> __p = (struct drm_i915_private *)dev_priv; else if
> (__builtin_types_compatible_p(typeof(*dev_priv), struct drm_device)) __p =
> to_i915((struct drm_device *)dev_priv); else do { bool __cond = !(!(1));
> extern void __compiletime_assert_1912(void) __attribute__((error("BUILD_BUG
> failed"))); if (__cond) __compiletime_assert_1912(); do { } while (0); }
> while (0); __p; })->info)->has_ddi) && (control & (0x << 16)) != (0xabcd
> << 16))
>
> If we gradually remove dynamic typing abilities from individual macros we can
> start bringing the size of the binary down.
>
> For example after this series:
>
>textdata bss dec hex filename
> 1067727 23256 576 1091559 10a7e7 i915.ko.nightly
> 1038202 23256 576 1062034 103492 i915.ko.diet
>
> Which is a ~29KiB saving.
>
> This is disruptive of course, but perhaps it is time to bite the bullet since
> we
> now have a situation that even new platforms like Kabylake are adding code
> which
> uses the wrong thing in those macros (dev instead of dev_priv).
>
> The way I have done it here makes it impossible to use the converted macros in
> a wrong way going forward.
Amen, hallelujah!
I'll happily review the entire series (I've done this same job once
already). Is this still the latest version, or do you have an update?
I've also got a patch series for the remaining INTEL_INFO() fixes;
once your patches are merged I can submit those.
Kind regards, David
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 00/22] .rodata diet
On Wed, Oct 5, 2016 at 3:01 PM, Jani Nikula wrote: > On Wed, 05 Oct 2016, Tvrtko Ursulin wrote: >> Dynamic typing in __I915__ (INTEL_INFO) has and unfortuante consequence that >> for every time it is called under a WARN it generates a very verbose string >> placed into the appropriate .rodata section. > > AFAIK the idea all along was to use this "dynamic typing" for a > transition period. I'm in favor of moving towards only accepting > dev_priv. I'm a bit hesitant to make the change in one go, though. +1 on transitioning away from me too. I'll leave it up to the people involved whether staging this is required or not. Fixing up the fallout (in internal, in-flight patches or wherever) should at least be simple. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 00/22] .rodata diet
On 11/10/2016 13:17, David Weinehall wrote:
On Wed, Oct 05, 2016 at 01:33:27PM +0100, Tvrtko Ursulin wrote:
From: Tvrtko Ursulin
Dynamic typing in __I915__ (INTEL_INFO) has and unfortuante consequence that
for every time it is called under a WARN it generates a very verbose string
placed into the appropriate .rodata section.
Each instance of that can can several hundred bytes to the binary. One example
of such strings found in the binary, not word-wrapped is:
WARN_ON(!((&({ struct drm_i915_private *__p; if (__builtin_types_compatible_p(typeof(*dev_priv), struct
drm_i915_private)) __p = (struct drm_i915_private *)dev_priv; else if (__builtin_types_compatible_p(typeof(*dev_priv),
struct drm_device)) __p = to_i915((struct drm_device *)dev_priv); else do { bool __cond = !(!(1)); extern void
__compiletime_assert_1912(void) __attribute__((error("BUILD_BUG failed"))); if (__cond)
__compiletime_assert_1912(); do { } while (0); } while (0); __p; })->info)->has_ddi) && (control &
(0x << 16)) != (0xabcd << 16))
If we gradually remove dynamic typing abilities from individual macros we can
start bringing the size of the binary down.
For example after this series:
textdata bss dec hex filename
1067727 23256 576 1091559 10a7e7 i915.ko.nightly
1038202 23256 576 1062034 103492 i915.ko.diet
Which is a ~29KiB saving.
This is disruptive of course, but perhaps it is time to bite the bullet since we
now have a situation that even new platforms like Kabylake are adding code which
uses the wrong thing in those macros (dev instead of dev_priv).
The way I have done it here makes it impossible to use the converted macros in
a wrong way going forward.
Amen, hallelujah!
I'll happily review the entire series (I've done this same job once
already). Is this still the latest version, or do you have an update?
I've also got a patch series for the remaining INTEL_INFO() fixes;
once your patches are merged I can submit those.
I have an updated version with some review comments incorporated and
from which I have split out the unrelated const table shrinks.
Good news is that now both Daniel and Jani are in favour so we just need
to agree on merging dynamics.
I will send the updated series out today. Thanks for reviewing!
Regards,
Tvrtko
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
