From: Daniel Vetter <daniel@ffwll.ch>
To: Damien Lespiau <damien.lespiau@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 1/2] drm/i915: Make the device_info structure __initconst
Date: Thu, 10 Jul 2014 22:25:27 +0200 [thread overview]
Message-ID: <20140710202527.GK17271@phenom.ffwll.local> (raw)
In-Reply-To: <1405000363-15950-1-git-send-email-damien.lespiau@intel.com>
On Thu, Jul 10, 2014 at 02:52:42PM +0100, Damien Lespiau wrote:
> We don't need them past the module initialization as the correct
> structure is copied into dev_priv in ->load(), called from
> drm_pci_init(), called from the module init funtion.
>
> I'm always hesitant about adding new members to struct intel_device_info
> because it will add 30+ * sizeof(member) bytes to the driver. However,
> if we can discard those table after init(), it changes everything.
>
> After this change, the driver has a new .init.rodata section contains
> the structures in question and .rodata has now 2848 fewer bytes.
>
> lsmod shows -5425 bytes in its size field between before and after this
> change. Not too sure why this (Vs the 2848 bytes lost in .rodata), but
> that's enough for me.
>
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
You want devinintconst which is being phased out afaik ... Currently the
driver gets probed synchronously but you kinda never know what the device
core people are up to:
- init = removed after the module is loaded.
- devinit = removed after the driver is initilialized, and never for
CONFIG_HOTPLUG=y.
If we want to trim down the size of our driver, especially on specific
platforms we imo should have a) link time optimization b) some
heavy-handed macro to return a static device info c) a much more clever
gcc since last time I've tried this it failed to kick out large swats of
code like all the dvo crap. Despite that it was clearly unreachable :(
Cheers, Daniel
> ---
> drivers/gpu/drm/i915/i915_drv.c | 60 ++++++++++++++++++++---------------------
> 1 file changed, 30 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index bc19623..9f75261 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -63,7 +63,7 @@ static struct drm_driver driver;
> #define IVB_CURSOR_OFFSETS \
> .cursor_offsets = { CURSOR_A_OFFSET, IVB_CURSOR_B_OFFSET, IVB_CURSOR_C_OFFSET }
>
> -static const struct intel_device_info intel_i830_info = {
> +static const struct intel_device_info intel_i830_info __initconst = {
> .gen = 2, .is_mobile = 1, .cursor_needs_physical = 1, .num_pipes = 2,
> .has_overlay = 1, .overlay_needs_physical = 1,
> .ring_mask = RENDER_RING,
> @@ -71,7 +71,7 @@ static const struct intel_device_info intel_i830_info = {
> CURSOR_OFFSETS,
> };
>
> -static const struct intel_device_info intel_845g_info = {
> +static const struct intel_device_info intel_845g_info __initconst = {
> .gen = 2, .num_pipes = 1,
> .has_overlay = 1, .overlay_needs_physical = 1,
> .ring_mask = RENDER_RING,
> @@ -79,7 +79,7 @@ static const struct intel_device_info intel_845g_info = {
> CURSOR_OFFSETS,
> };
>
> -static const struct intel_device_info intel_i85x_info = {
> +static const struct intel_device_info intel_i85x_info __initconst = {
> .gen = 2, .is_i85x = 1, .is_mobile = 1, .num_pipes = 2,
> .cursor_needs_physical = 1,
> .has_overlay = 1, .overlay_needs_physical = 1,
> @@ -89,7 +89,7 @@ static const struct intel_device_info intel_i85x_info = {
> CURSOR_OFFSETS,
> };
>
> -static const struct intel_device_info intel_i865g_info = {
> +static const struct intel_device_info intel_i865g_info __initconst = {
> .gen = 2, .num_pipes = 1,
> .has_overlay = 1, .overlay_needs_physical = 1,
> .ring_mask = RENDER_RING,
> @@ -97,14 +97,14 @@ static const struct intel_device_info intel_i865g_info = {
> CURSOR_OFFSETS,
> };
>
> -static const struct intel_device_info intel_i915g_info = {
> +static const struct intel_device_info intel_i915g_info __initconst = {
> .gen = 3, .is_i915g = 1, .cursor_needs_physical = 1, .num_pipes = 2,
> .has_overlay = 1, .overlay_needs_physical = 1,
> .ring_mask = RENDER_RING,
> GEN_DEFAULT_PIPEOFFSETS,
> CURSOR_OFFSETS,
> };
> -static const struct intel_device_info intel_i915gm_info = {
> +static const struct intel_device_info intel_i915gm_info __initconst = {
> .gen = 3, .is_mobile = 1, .num_pipes = 2,
> .cursor_needs_physical = 1,
> .has_overlay = 1, .overlay_needs_physical = 1,
> @@ -114,14 +114,14 @@ static const struct intel_device_info intel_i915gm_info = {
> GEN_DEFAULT_PIPEOFFSETS,
> CURSOR_OFFSETS,
> };
> -static const struct intel_device_info intel_i945g_info = {
> +static const struct intel_device_info intel_i945g_info __initconst = {
> .gen = 3, .has_hotplug = 1, .cursor_needs_physical = 1, .num_pipes = 2,
> .has_overlay = 1, .overlay_needs_physical = 1,
> .ring_mask = RENDER_RING,
> GEN_DEFAULT_PIPEOFFSETS,
> CURSOR_OFFSETS,
> };
> -static const struct intel_device_info intel_i945gm_info = {
> +static const struct intel_device_info intel_i945gm_info __initconst = {
> .gen = 3, .is_i945gm = 1, .is_mobile = 1, .num_pipes = 2,
> .has_hotplug = 1, .cursor_needs_physical = 1,
> .has_overlay = 1, .overlay_needs_physical = 1,
> @@ -132,7 +132,7 @@ static const struct intel_device_info intel_i945gm_info = {
> CURSOR_OFFSETS,
> };
>
> -static const struct intel_device_info intel_i965g_info = {
> +static const struct intel_device_info intel_i965g_info __initconst = {
> .gen = 4, .is_broadwater = 1, .num_pipes = 2,
> .has_hotplug = 1,
> .has_overlay = 1,
> @@ -141,7 +141,7 @@ static const struct intel_device_info intel_i965g_info = {
> CURSOR_OFFSETS,
> };
>
> -static const struct intel_device_info intel_i965gm_info = {
> +static const struct intel_device_info intel_i965gm_info __initconst = {
> .gen = 4, .is_crestline = 1, .num_pipes = 2,
> .is_mobile = 1, .has_fbc = 1, .has_hotplug = 1,
> .has_overlay = 1,
> @@ -151,7 +151,7 @@ static const struct intel_device_info intel_i965gm_info = {
> CURSOR_OFFSETS,
> };
>
> -static const struct intel_device_info intel_g33_info = {
> +static const struct intel_device_info intel_g33_info __initconst = {
> .gen = 3, .is_g33 = 1, .num_pipes = 2,
> .need_gfx_hws = 1, .has_hotplug = 1,
> .has_overlay = 1,
> @@ -160,7 +160,7 @@ static const struct intel_device_info intel_g33_info = {
> CURSOR_OFFSETS,
> };
>
> -static const struct intel_device_info intel_g45_info = {
> +static const struct intel_device_info intel_g45_info __initconst = {
> .gen = 4, .is_g4x = 1, .need_gfx_hws = 1, .num_pipes = 2,
> .has_pipe_cxsr = 1, .has_hotplug = 1,
> .ring_mask = RENDER_RING | BSD_RING,
> @@ -168,7 +168,7 @@ static const struct intel_device_info intel_g45_info = {
> CURSOR_OFFSETS,
> };
>
> -static const struct intel_device_info intel_gm45_info = {
> +static const struct intel_device_info intel_gm45_info __initconst = {
> .gen = 4, .is_g4x = 1, .num_pipes = 2,
> .is_mobile = 1, .need_gfx_hws = 1, .has_fbc = 1,
> .has_pipe_cxsr = 1, .has_hotplug = 1,
> @@ -178,7 +178,7 @@ static const struct intel_device_info intel_gm45_info = {
> CURSOR_OFFSETS,
> };
>
> -static const struct intel_device_info intel_pineview_info = {
> +static const struct intel_device_info intel_pineview_info __initconst = {
> .gen = 3, .is_g33 = 1, .is_pineview = 1, .is_mobile = 1, .num_pipes = 2,
> .need_gfx_hws = 1, .has_hotplug = 1,
> .has_overlay = 1,
> @@ -186,7 +186,7 @@ static const struct intel_device_info intel_pineview_info = {
> CURSOR_OFFSETS,
> };
>
> -static const struct intel_device_info intel_ironlake_d_info = {
> +static const struct intel_device_info intel_ironlake_d_info __initconst = {
> .gen = 5, .num_pipes = 2,
> .need_gfx_hws = 1, .has_hotplug = 1,
> .ring_mask = RENDER_RING | BSD_RING,
> @@ -194,7 +194,7 @@ static const struct intel_device_info intel_ironlake_d_info = {
> CURSOR_OFFSETS,
> };
>
> -static const struct intel_device_info intel_ironlake_m_info = {
> +static const struct intel_device_info intel_ironlake_m_info __initconst = {
> .gen = 5, .is_mobile = 1, .num_pipes = 2,
> .need_gfx_hws = 1, .has_hotplug = 1,
> .has_fbc = 1,
> @@ -203,7 +203,7 @@ static const struct intel_device_info intel_ironlake_m_info = {
> CURSOR_OFFSETS,
> };
>
> -static const struct intel_device_info intel_sandybridge_d_info = {
> +static const struct intel_device_info intel_sandybridge_d_info __initconst = {
> .gen = 6, .num_pipes = 2,
> .need_gfx_hws = 1, .has_hotplug = 1,
> .has_fbc = 1,
> @@ -213,7 +213,7 @@ static const struct intel_device_info intel_sandybridge_d_info = {
> CURSOR_OFFSETS,
> };
>
> -static const struct intel_device_info intel_sandybridge_m_info = {
> +static const struct intel_device_info intel_sandybridge_m_info __initconst = {
> .gen = 6, .is_mobile = 1, .num_pipes = 2,
> .need_gfx_hws = 1, .has_hotplug = 1,
> .has_fbc = 1,
> @@ -230,14 +230,14 @@ static const struct intel_device_info intel_sandybridge_m_info = {
> .ring_mask = RENDER_RING | BSD_RING | BLT_RING, \
> .has_llc = 1
>
> -static const struct intel_device_info intel_ivybridge_d_info = {
> +static const struct intel_device_info intel_ivybridge_d_info __initconst = {
> GEN7_FEATURES,
> .is_ivybridge = 1,
> GEN_DEFAULT_PIPEOFFSETS,
> IVB_CURSOR_OFFSETS,
> };
>
> -static const struct intel_device_info intel_ivybridge_m_info = {
> +static const struct intel_device_info intel_ivybridge_m_info __initconst = {
> GEN7_FEATURES,
> .is_ivybridge = 1,
> .is_mobile = 1,
> @@ -245,7 +245,7 @@ static const struct intel_device_info intel_ivybridge_m_info = {
> IVB_CURSOR_OFFSETS,
> };
>
> -static const struct intel_device_info intel_ivybridge_q_info = {
> +static const struct intel_device_info intel_ivybridge_q_info __initconst = {
> GEN7_FEATURES,
> .is_ivybridge = 1,
> .num_pipes = 0, /* legal, last one wins */
> @@ -253,7 +253,7 @@ static const struct intel_device_info intel_ivybridge_q_info = {
> IVB_CURSOR_OFFSETS,
> };
>
> -static const struct intel_device_info intel_valleyview_m_info = {
> +static const struct intel_device_info intel_valleyview_m_info __initconst = {
> GEN7_FEATURES,
> .is_mobile = 1,
> .num_pipes = 2,
> @@ -265,7 +265,7 @@ static const struct intel_device_info intel_valleyview_m_info = {
> CURSOR_OFFSETS,
> };
>
> -static const struct intel_device_info intel_valleyview_d_info = {
> +static const struct intel_device_info intel_valleyview_d_info __initconst = {
> GEN7_FEATURES,
> .num_pipes = 2,
> .is_valleyview = 1,
> @@ -276,7 +276,7 @@ static const struct intel_device_info intel_valleyview_d_info = {
> CURSOR_OFFSETS,
> };
>
> -static const struct intel_device_info intel_haswell_d_info = {
> +static const struct intel_device_info intel_haswell_d_info __initconst = {
> GEN7_FEATURES,
> .is_haswell = 1,
> .has_ddi = 1,
> @@ -286,7 +286,7 @@ static const struct intel_device_info intel_haswell_d_info = {
> IVB_CURSOR_OFFSETS,
> };
>
> -static const struct intel_device_info intel_haswell_m_info = {
> +static const struct intel_device_info intel_haswell_m_info __initconst = {
> GEN7_FEATURES,
> .is_haswell = 1,
> .is_mobile = 1,
> @@ -297,7 +297,7 @@ static const struct intel_device_info intel_haswell_m_info = {
> IVB_CURSOR_OFFSETS,
> };
>
> -static const struct intel_device_info intel_broadwell_d_info = {
> +static const struct intel_device_info intel_broadwell_d_info __initconst = {
> .gen = 8, .num_pipes = 3,
> .need_gfx_hws = 1, .has_hotplug = 1,
> .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING,
> @@ -308,7 +308,7 @@ static const struct intel_device_info intel_broadwell_d_info = {
> IVB_CURSOR_OFFSETS,
> };
>
> -static const struct intel_device_info intel_broadwell_m_info = {
> +static const struct intel_device_info intel_broadwell_m_info __initconst = {
> .gen = 8, .is_mobile = 1, .num_pipes = 3,
> .need_gfx_hws = 1, .has_hotplug = 1,
> .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING,
> @@ -319,7 +319,7 @@ static const struct intel_device_info intel_broadwell_m_info = {
> IVB_CURSOR_OFFSETS,
> };
>
> -static const struct intel_device_info intel_broadwell_gt3d_info = {
> +static const struct intel_device_info intel_broadwell_gt3d_info __initconst = {
> .gen = 8, .num_pipes = 3,
> .need_gfx_hws = 1, .has_hotplug = 1,
> .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
> @@ -330,7 +330,7 @@ static const struct intel_device_info intel_broadwell_gt3d_info = {
> IVB_CURSOR_OFFSETS,
> };
>
> -static const struct intel_device_info intel_broadwell_gt3m_info = {
> +static const struct intel_device_info intel_broadwell_gt3m_info __initconst = {
> .gen = 8, .is_mobile = 1, .num_pipes = 3,
> .need_gfx_hws = 1, .has_hotplug = 1,
> .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
> @@ -341,7 +341,7 @@ static const struct intel_device_info intel_broadwell_gt3m_info = {
> IVB_CURSOR_OFFSETS,
> };
>
> -static const struct intel_device_info intel_cherryview_info = {
> +static const struct intel_device_info intel_cherryview_info __initconst = {
> .is_preliminary = 1,
> .gen = 8, .num_pipes = 3,
> .need_gfx_hws = 1, .has_hotplug = 1,
> --
> 1.8.3.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
next prev parent reply other threads:[~2014-07-10 20:25 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-07-10 13:52 [PATCH 1/2] drm/i915: Make the device_info structure __initconst Damien Lespiau
2014-07-10 13:52 ` [PATCH 2/2] drm/i915: Don't cast a pointer to void* unnecessarily Damien Lespiau
2014-07-10 19:26 ` Paulo Zanoni
2014-07-10 20:26 ` Daniel Vetter
2014-07-10 19:18 ` [PATCH 1/2] drm/i915: Make the device_info structure __initconst Paulo Zanoni
2014-07-10 20:25 ` Daniel Vetter [this message]
2014-07-10 21:47 ` Damien Lespiau
2014-07-11 6:33 ` Daniel Vetter
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20140710202527.GK17271@phenom.ffwll.local \
--to=daniel@ffwll.ch \
--cc=damien.lespiau@intel.com \
--cc=intel-gfx@lists.freedesktop.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox