On Mon, 20 Mar 2017, Arnd Bergmann <arnd@xxxxxxxx> wrote: > The varargs macro trick in _PIPE3/_PHY3/_PORT3 was meant as an optimization > to shrink the i915 kernel module by around 1000 bytes. To be clear, it was not at all intended to be an optimization, nothing of the sort. The intention was to make it easier and less error prone to add more parameters to said macros. The text size shring was just a bonus. > However, the > downside is a size regression with CONFIG_KASAN, as I found from stack size > warnings with gcc-7.0.1: In his review of the original change, Chris provided this comparison https://godbolt.org/g/YCK1od How does CONFIG_KASAN change this? Would be nice to see how the generated code blows up. BR, Jani. > > before: > drivers/gpu/drm/i915/intel_dpll_mgr.c: In function 'bxt_ddi_pll_get_hw_state': > drivers/gpu/drm/i915/intel_dpll_mgr.c:1644:1: error: the frame size of 176 bytes is larger than 100 bytes [-Werror=frame-larger-than=] > drivers/gpu/drm/i915/intel_dpll_mgr.c: In function 'bxt_ddi_pll_enable': > drivers/gpu/drm/i915/intel_dpll_mgr.c:1548:1: error: the frame size of 224 bytes is larger than 100 bytes [-Werror=frame-larger-than=] > > after: > drivers/gpu/drm/i915/intel_dpll_mgr.c: In function 'bxt_ddi_pll_get_hw_state': > drivers/gpu/drm/i915/intel_dpll_mgr.c:1644:1: error: the frame size of 1016 bytes is larger than 1000 bytes [-Werror=frame-larger-than=] > drivers/gpu/drm/i915/intel_dpll_mgr.c: In function 'bxt_ddi_pll_enable': > drivers/gpu/drm/i915/intel_dpll_mgr.c:1548:1: error: the frame size of 1960 bytes is larger than 1000 bytes [-Werror=frame-larger-than=] > > I also checked the module sizes and got > > before: > text data bss dec hex filename > 2704592 412025 11104 3127721 2fb9a9 drivers/gpu/drm/i915/i915.o > > after: > text data bss dec hex filename > 2718538 412025 11104 3141667 2ff023 drivers/gpu/drm/i915/i915.o > > showing a significant code size growth. This reverts the patch to avoid > the warning and get back to the better code with CONFIG_KASAN. If someone > has another idea to avoid the warning while keeping the more efficient > code for the non-KASAN case, that would obviously be better. > > Fixes: ce64645d86ac ("drm/i915: use variadic macros and arrays to choose port/pipe based registers") > Signed-off-by: Arnd Bergmann <arnd@xxxxxxxx> > --- > drivers/gpu/drm/i915/i915_reg.h | 11 ++++++----- > 1 file changed, 6 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 04c8f69fcc62..aa732eccdeb5 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -48,8 +48,6 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg) > return !i915_mmio_reg_equal(reg, INVALID_MMIO_REG); > } > > -#define _PICK(__index, ...) (((const u32 []){ __VA_ARGS__ })[__index]) > - > #define _PIPE(pipe, a, b) ((a) + (pipe)*((b)-(a))) > #define _MMIO_PIPE(pipe, a, b) _MMIO(_PIPE(pipe, a, b)) > #define _PLANE(plane, a, b) _PIPE(plane, a, b) > @@ -58,11 +56,14 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg) > #define _MMIO_TRANS(tran, a, b) _MMIO(_TRANS(tran, a, b)) > #define _PORT(port, a, b) ((a) + (port)*((b)-(a))) > #define _MMIO_PORT(port, a, b) _MMIO(_PORT(port, a, b)) > -#define _PIPE3(pipe, ...) _PICK(pipe, __VA_ARGS__) > +#define _PIPE3(pipe, a, b, c) ((pipe) == PIPE_A ? (a) : \ > + (pipe) == PIPE_B ? (b) : (c)) > #define _MMIO_PIPE3(pipe, a, b, c) _MMIO(_PIPE3(pipe, a, b, c)) > -#define _PORT3(port, ...) _PICK(port, __VA_ARGS__) > +#define _PORT3(port, a, b, c) ((port) == PORT_A ? (a) : \ > + (port) == PORT_B ? (b) : (c)) > #define _MMIO_PORT3(pipe, a, b, c) _MMIO(_PORT3(pipe, a, b, c)) > -#define _PHY3(phy, ...) _PICK(phy, __VA_ARGS__) > +#define _PHY3(phy, a, b, c) ((phy) == DPIO_PHY0 ? (a) : \ > + (phy) == DPIO_PHY1 ? (b) : (c)) > #define _MMIO_PHY3(phy, a, b, c) _MMIO(_PHY3(phy, a, b, c)) > > #define _MASKED_FIELD(mask, value) ({ \ -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
