From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jani Nikula Subject: Re: [PATCH 3/3] [RFC] Revert "drm/i915: use variadic macros and arrays to choose port/pipe based registers" Date: Mon, 20 Mar 2017 12:08:00 +0200 Message-ID: <87shm8b6rj.fsf@intel.com> References: <20170320094335.1266306-1-arnd@arndb.de> <20170320094335.1266306-3-arnd@arndb.de> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <20170320094335.1266306-3-arnd@arndb.de> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Daniel Vetter , David Airlie Cc: Ander Conselvan de Oliveira , Arnd Bergmann , Mika Kuoppala , linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, intel-gfx@lists.freedesktop.org, Robert Bragg List-Id: dri-devel@lists.freedesktop.org T24gTW9uLCAyMCBNYXIgMjAxNywgQXJuZCBCZXJnbWFubiA8YXJuZEBhcm5kYi5kZT4gd3JvdGU6 Cj4gVGhlIHZhcmFyZ3MgbWFjcm8gdHJpY2sgaW4gX1BJUEUzL19QSFkzL19QT1JUMyB3YXMgbWVh bnQgYXMgYW4gb3B0aW1pemF0aW9uCj4gdG8gc2hyaW5rIHRoZSBpOTE1IGtlcm5lbCBtb2R1bGUg YnkgYXJvdW5kIDEwMDAgYnl0ZXMuCgpUbyBiZSBjbGVhciwgaXQgd2FzIG5vdCBhdCBhbGwgaW50 ZW5kZWQgdG8gYmUgYW4gb3B0aW1pemF0aW9uLCBub3RoaW5nCm9mIHRoZSBzb3J0LiBUaGUgaW50 ZW50aW9uIHdhcyB0byBtYWtlIGl0IGVhc2llciBhbmQgbGVzcyBlcnJvciBwcm9uZSB0bwphZGQg bW9yZSBwYXJhbWV0ZXJzIHRvIHNhaWQgbWFjcm9zLiBUaGUgdGV4dCBzaXplIHNocmluZyB3YXMg anVzdCBhCmJvbnVzLgoKPiBIb3dldmVyLCB0aGUKPiBkb3duc2lkZSBpcyBhIHNpemUgcmVncmVz c2lvbiB3aXRoIENPTkZJR19LQVNBTiwgYXMgSSBmb3VuZCBmcm9tIHN0YWNrIHNpemUKPiB3YXJu aW5ncyB3aXRoIGdjYy03LjAuMToKCkluIGhpcyByZXZpZXcgb2YgdGhlIG9yaWdpbmFsIGNoYW5n ZSwgQ2hyaXMgcHJvdmlkZWQgdGhpcyBjb21wYXJpc29uCmh0dHBzOi8vZ29kYm9sdC5vcmcvZy9Z Q0sxb2QKCkhvdyBkb2VzIENPTkZJR19LQVNBTiBjaGFuZ2UgdGhpcz8gV291bGQgYmUgbmljZSB0 byBzZWUgaG93IHRoZQpnZW5lcmF0ZWQgY29kZSBibG93cyB1cC4KCgpCUiwKSmFuaS4KCgo+Cj4g YmVmb3JlOgo+IGRyaXZlcnMvZ3B1L2RybS9pOTE1L2ludGVsX2RwbGxfbWdyLmM6IEluIGZ1bmN0 aW9uICdieHRfZGRpX3BsbF9nZXRfaHdfc3RhdGUnOgo+IGRyaXZlcnMvZ3B1L2RybS9pOTE1L2lu dGVsX2RwbGxfbWdyLmM6MTY0NDoxOiBlcnJvcjogdGhlIGZyYW1lIHNpemUgb2YgMTc2IGJ5dGVz IGlzIGxhcmdlciB0aGFuIDEwMCBieXRlcyBbLVdlcnJvcj1mcmFtZS1sYXJnZXItdGhhbj1dCj4g ZHJpdmVycy9ncHUvZHJtL2k5MTUvaW50ZWxfZHBsbF9tZ3IuYzogSW4gZnVuY3Rpb24gJ2J4dF9k ZGlfcGxsX2VuYWJsZSc6Cj4gZHJpdmVycy9ncHUvZHJtL2k5MTUvaW50ZWxfZHBsbF9tZ3IuYzox NTQ4OjE6IGVycm9yOiB0aGUgZnJhbWUgc2l6ZSBvZiAyMjQgYnl0ZXMgaXMgbGFyZ2VyIHRoYW4g MTAwIGJ5dGVzIFstV2Vycm9yPWZyYW1lLWxhcmdlci10aGFuPV0KPgo+IGFmdGVyOgo+IGRyaXZl cnMvZ3B1L2RybS9pOTE1L2ludGVsX2RwbGxfbWdyLmM6IEluIGZ1bmN0aW9uICdieHRfZGRpX3Bs bF9nZXRfaHdfc3RhdGUnOgo+IGRyaXZlcnMvZ3B1L2RybS9pOTE1L2ludGVsX2RwbGxfbWdyLmM6 MTY0NDoxOiBlcnJvcjogdGhlIGZyYW1lIHNpemUgb2YgMTAxNiBieXRlcyBpcyBsYXJnZXIgdGhh biAxMDAwIGJ5dGVzIFstV2Vycm9yPWZyYW1lLWxhcmdlci10aGFuPV0KPiBkcml2ZXJzL2dwdS9k cm0vaTkxNS9pbnRlbF9kcGxsX21nci5jOiBJbiBmdW5jdGlvbiAnYnh0X2RkaV9wbGxfZW5hYmxl JzoKPiBkcml2ZXJzL2dwdS9kcm0vaTkxNS9pbnRlbF9kcGxsX21nci5jOjE1NDg6MTogZXJyb3I6 IHRoZSBmcmFtZSBzaXplIG9mIDE5NjAgYnl0ZXMgaXMgbGFyZ2VyIHRoYW4gMTAwMCBieXRlcyBb LVdlcnJvcj1mcmFtZS1sYXJnZXItdGhhbj1dCj4KPiBJIGFsc28gY2hlY2tlZCB0aGUgbW9kdWxl IHNpemVzIGFuZCBnb3QKPgo+IGJlZm9yZToKPiAgICB0ZXh0CSAgIGRhdGEJICAgIGJzcwkgICAg ZGVjCSAgICBoZXgJZmlsZW5hbWUKPiAyNzA0NTkyCSA0MTIwMjUJICAxMTEwNAkzMTI3NzIxCSAy ZmI5YTkJZHJpdmVycy9ncHUvZHJtL2k5MTUvaTkxNS5vCj4KPiBhZnRlcjoKPiAgICB0ZXh0CSAg IGRhdGEJICAgIGJzcwkgICAgZGVjCSAgICBoZXgJZmlsZW5hbWUKPiAyNzE4NTM4CSA0MTIwMjUJ ICAxMTEwNAkzMTQxNjY3CSAyZmYwMjMJZHJpdmVycy9ncHUvZHJtL2k5MTUvaTkxNS5vCj4KPiBz aG93aW5nIGEgc2lnbmlmaWNhbnQgY29kZSBzaXplIGdyb3d0aC4gVGhpcyByZXZlcnRzIHRoZSBw YXRjaCB0byBhdm9pZAo+IHRoZSB3YXJuaW5nIGFuZCBnZXQgYmFjayB0byB0aGUgYmV0dGVyIGNv ZGUgd2l0aCBDT05GSUdfS0FTQU4uIElmIHNvbWVvbmUKPiBoYXMgYW5vdGhlciBpZGVhIHRvIGF2 b2lkIHRoZSB3YXJuaW5nIHdoaWxlIGtlZXBpbmcgdGhlIG1vcmUgZWZmaWNpZW50Cj4gY29kZSBm b3IgdGhlIG5vbi1LQVNBTiBjYXNlLCB0aGF0IHdvdWxkIG9idmlvdXNseSBiZSBiZXR0ZXIuCj4K PiBGaXhlczogY2U2NDY0NWQ4NmFjICgiZHJtL2k5MTU6IHVzZSB2YXJpYWRpYyBtYWNyb3MgYW5k IGFycmF5cyB0byBjaG9vc2UgcG9ydC9waXBlIGJhc2VkIHJlZ2lzdGVycyIpCj4gU2lnbmVkLW9m Zi1ieTogQXJuZCBCZXJnbWFubiA8YXJuZEBhcm5kYi5kZT4KPiAtLS0KPiAgZHJpdmVycy9ncHUv ZHJtL2k5MTUvaTkxNV9yZWcuaCB8IDExICsrKysrKy0tLS0tCj4gIDEgZmlsZSBjaGFuZ2VkLCA2 IGluc2VydGlvbnMoKyksIDUgZGVsZXRpb25zKC0pCj4KPiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9n cHUvZHJtL2k5MTUvaTkxNV9yZWcuaCBiL2RyaXZlcnMvZ3B1L2RybS9pOTE1L2k5MTVfcmVnLmgK PiBpbmRleCAwNGM4ZjY5ZmNjNjIuLmFhNzMyZWNjZGViNSAxMDA2NDQKPiAtLS0gYS9kcml2ZXJz L2dwdS9kcm0vaTkxNS9pOTE1X3JlZy5oCj4gKysrIGIvZHJpdmVycy9ncHUvZHJtL2k5MTUvaTkx NV9yZWcuaAo+IEBAIC00OCw4ICs0OCw2IEBAIHN0YXRpYyBpbmxpbmUgYm9vbCBpOTE1X21taW9f cmVnX3ZhbGlkKGk5MTVfcmVnX3QgcmVnKQo+ICAJcmV0dXJuICFpOTE1X21taW9fcmVnX2VxdWFs KHJlZywgSU5WQUxJRF9NTUlPX1JFRyk7Cj4gIH0KPiAgCj4gLSNkZWZpbmUgX1BJQ0soX19pbmRl eCwgLi4uKSAoKChjb25zdCB1MzIgW10peyBfX1ZBX0FSR1NfXyB9KVtfX2luZGV4XSkKPiAtCj4g ICNkZWZpbmUgX1BJUEUocGlwZSwgYSwgYikgKChhKSArIChwaXBlKSooKGIpLShhKSkpCj4gICNk ZWZpbmUgX01NSU9fUElQRShwaXBlLCBhLCBiKSBfTU1JTyhfUElQRShwaXBlLCBhLCBiKSkKPiAg I2RlZmluZSBfUExBTkUocGxhbmUsIGEsIGIpIF9QSVBFKHBsYW5lLCBhLCBiKQo+IEBAIC01OCwx MSArNTYsMTQgQEAgc3RhdGljIGlubGluZSBib29sIGk5MTVfbW1pb19yZWdfdmFsaWQoaTkxNV9y ZWdfdCByZWcpCj4gICNkZWZpbmUgX01NSU9fVFJBTlModHJhbiwgYSwgYikgX01NSU8oX1RSQU5T KHRyYW4sIGEsIGIpKQo+ICAjZGVmaW5lIF9QT1JUKHBvcnQsIGEsIGIpICgoYSkgKyAocG9ydCkq KChiKS0oYSkpKQo+ICAjZGVmaW5lIF9NTUlPX1BPUlQocG9ydCwgYSwgYikgX01NSU8oX1BPUlQo cG9ydCwgYSwgYikpCj4gLSNkZWZpbmUgX1BJUEUzKHBpcGUsIC4uLikgX1BJQ0socGlwZSwgX19W QV9BUkdTX18pCj4gKyNkZWZpbmUgX1BJUEUzKHBpcGUsIGEsIGIsIGMpICgocGlwZSkgPT0gUElQ RV9BID8gKGEpIDogXAo+ICsJCQkgICAgICAgKHBpcGUpID09IFBJUEVfQiA/IChiKSA6IChjKSkK PiAgI2RlZmluZSBfTU1JT19QSVBFMyhwaXBlLCBhLCBiLCBjKSBfTU1JTyhfUElQRTMocGlwZSwg YSwgYiwgYykpCj4gLSNkZWZpbmUgX1BPUlQzKHBvcnQsIC4uLikgX1BJQ0socG9ydCwgX19WQV9B UkdTX18pCj4gKyNkZWZpbmUgX1BPUlQzKHBvcnQsIGEsIGIsIGMpICgocG9ydCkgPT0gUE9SVF9B ID8gKGEpIDogXAo+ICsJCQkgICAgICAgKHBvcnQpID09IFBPUlRfQiA/IChiKSA6IChjKSkKPiAg I2RlZmluZSBfTU1JT19QT1JUMyhwaXBlLCBhLCBiLCBjKSBfTU1JTyhfUE9SVDMocGlwZSwgYSwg YiwgYykpCj4gLSNkZWZpbmUgX1BIWTMocGh5LCAuLi4pIF9QSUNLKHBoeSwgX19WQV9BUkdTX18p Cj4gKyNkZWZpbmUgX1BIWTMocGh5LCBhLCBiLCBjKSAoKHBoeSkgPT0gRFBJT19QSFkwID8gKGEp IDogXAo+ICsJCQkgICAgIChwaHkpID09IERQSU9fUEhZMSA/IChiKSA6IChjKSkKPiAgI2RlZmlu ZSBfTU1JT19QSFkzKHBoeSwgYSwgYiwgYykgX01NSU8oX1BIWTMocGh5LCBhLCBiLCBjKSkKPiAg Cj4gICNkZWZpbmUgX01BU0tFRF9GSUVMRChtYXNrLCB2YWx1ZSkgKHsJCQkJCSAgIFwKCi0tIApK YW5pIE5pa3VsYSwgSW50ZWwgT3BlbiBTb3VyY2UgVGVjaG5vbG9neSBDZW50ZXIKX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVsIG1haWxpbmcg bGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlzdHMuZnJlZWRl c2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753852AbdCTKIU (ORCPT ); Mon, 20 Mar 2017 06:08:20 -0400 Received: from mga14.intel.com ([192.55.52.115]:19506 "EHLO mga14.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753744AbdCTKIH (ORCPT ); Mon, 20 Mar 2017 06:08:07 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.36,193,1486454400"; d="scan'208";a="836581078" From: Jani Nikula To: Arnd Bergmann , Daniel Vetter , David Airlie Cc: Arnd Bergmann , Mika Kuoppala , Ville =?utf-8?B?U3lyasOkbMOk?= , Chris Wilson , Imre Deak , Ander Conselvan de Oliveira , Robert Bragg , intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 3/3] [RFC] Revert "drm/i915: use variadic macros and arrays to choose port/pipe based registers" In-Reply-To: <20170320094335.1266306-3-arnd@arndb.de> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo References: <20170320094335.1266306-1-arnd@arndb.de> <20170320094335.1266306-3-arnd@arndb.de> Date: Mon, 20 Mar 2017 12:08:00 +0200 Message-ID: <87shm8b6rj.fsf@intel.com> MIME-Version: 1.0 Content-Type: text/plain Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, 20 Mar 2017, Arnd Bergmann 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 > --- > 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