public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH 1/2] drm/i915: Make the device_info structure __initconst
@ 2014-07-10 13:52 Damien Lespiau
  2014-07-10 13:52 ` [PATCH 2/2] drm/i915: Don't cast a pointer to void* unnecessarily Damien Lespiau
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Damien Lespiau @ 2014-07-10 13:52 UTC (permalink / raw)
  To: intel-gfx

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>
---
 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

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/2] drm/i915: Don't cast a pointer to void* unnecessarily
  2014-07-10 13:52 [PATCH 1/2] drm/i915: Make the device_info structure __initconst Damien Lespiau
@ 2014-07-10 13:52 ` Damien Lespiau
  2014-07-10 19:26   ` Paulo Zanoni
  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
  2 siblings, 1 reply; 8+ messages in thread
From: Damien Lespiau @ 2014-07-10 13:52 UTC (permalink / raw)
  To: intel-gfx

C is super happy to asign anything pointer to void *. Don't pretend
otherwise.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index ce69185..2c0bad6 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1596,7 +1596,7 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 	if (dev_priv == NULL)
 		return -ENOMEM;
 
-	dev->dev_private = (void *)dev_priv;
+	dev->dev_private = dev_priv;
 	dev_priv->dev = dev;
 
 	/* copy initial configuration to dev_priv->info */
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] drm/i915: Make the device_info structure __initconst
  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:18 ` Paulo Zanoni
  2014-07-10 20:25 ` Daniel Vetter
  2 siblings, 0 replies; 8+ messages in thread
From: Paulo Zanoni @ 2014-07-10 19:18 UTC (permalink / raw)
  To: Damien Lespiau; +Cc: Intel Graphics Development

2014-07-10 10:52 GMT-03:00 Damien Lespiau <damien.lespiau@intel.com>:
> 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.

I'm not an expert on this area, but it looks correct.
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

>
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> ---
>  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



-- 
Paulo Zanoni

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] drm/i915: Don't cast a pointer to void* unnecessarily
  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
  0 siblings, 1 reply; 8+ messages in thread
From: Paulo Zanoni @ 2014-07-10 19:26 UTC (permalink / raw)
  To: Damien Lespiau; +Cc: Intel Graphics Development

2014-07-10 10:52 GMT-03:00 Damien Lespiau <damien.lespiau@intel.com>:
> C is super happy to asign anything pointer to void *. Don't pretend
> otherwise.

Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

>
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_dma.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index ce69185..2c0bad6 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1596,7 +1596,7 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
>         if (dev_priv == NULL)
>                 return -ENOMEM;
>
> -       dev->dev_private = (void *)dev_priv;
> +       dev->dev_private = dev_priv;
>         dev_priv->dev = dev;
>
>         /* copy initial configuration to dev_priv->info */
> --
> 1.8.3.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] drm/i915: Make the device_info structure __initconst
  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:18 ` [PATCH 1/2] drm/i915: Make the device_info structure __initconst Paulo Zanoni
@ 2014-07-10 20:25 ` Daniel Vetter
  2014-07-10 21:47   ` Damien Lespiau
  2 siblings, 1 reply; 8+ messages in thread
From: Daniel Vetter @ 2014-07-10 20:25 UTC (permalink / raw)
  To: Damien Lespiau; +Cc: intel-gfx

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] drm/i915: Don't cast a pointer to void* unnecessarily
  2014-07-10 19:26   ` Paulo Zanoni
@ 2014-07-10 20:26     ` Daniel Vetter
  0 siblings, 0 replies; 8+ messages in thread
From: Daniel Vetter @ 2014-07-10 20:26 UTC (permalink / raw)
  To: Paulo Zanoni; +Cc: Intel Graphics Development

On Thu, Jul 10, 2014 at 04:26:53PM -0300, Paulo Zanoni wrote:
> 2014-07-10 10:52 GMT-03:00 Damien Lespiau <damien.lespiau@intel.com>:
> > C is super happy to asign anything pointer to void *. Don't pretend
> > otherwise.
> 
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

Queued for -next, thanks for the patch.
-Daniel

> 
> >
> > Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_dma.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> > index ce69185..2c0bad6 100644
> > --- a/drivers/gpu/drm/i915/i915_dma.c
> > +++ b/drivers/gpu/drm/i915/i915_dma.c
> > @@ -1596,7 +1596,7 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
> >         if (dev_priv == NULL)
> >                 return -ENOMEM;
> >
> > -       dev->dev_private = (void *)dev_priv;
> > +       dev->dev_private = dev_priv;
> >         dev_priv->dev = dev;
> >
> >         /* copy initial configuration to dev_priv->info */
> > --
> > 1.8.3.1
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 
> 
> -- 
> Paulo Zanoni
> _______________________________________________
> 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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] drm/i915: Make the device_info structure __initconst
  2014-07-10 20:25 ` Daniel Vetter
@ 2014-07-10 21:47   ` Damien Lespiau
  2014-07-11  6:33     ` Daniel Vetter
  0 siblings, 1 reply; 8+ messages in thread
From: Damien Lespiau @ 2014-07-10 21:47 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx

On Thu, Jul 10, 2014 at 10:25:27PM +0200, Daniel Vetter wrote:
> 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 :(

Sigh, I followed the !DRIVER_MODESET code path, I am very sad now.

I was thinking about having a Kconfig option to select a specific
platform to compile the driver for and, by trying to make that work, we
could end up with a nice per-platform split of the code. Would people be
totally opposed to such a thing?

-- 
Damien

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] drm/i915: Make the device_info structure __initconst
  2014-07-10 21:47   ` Damien Lespiau
@ 2014-07-11  6:33     ` Daniel Vetter
  0 siblings, 0 replies; 8+ messages in thread
From: Daniel Vetter @ 2014-07-11  6:33 UTC (permalink / raw)
  To: Damien Lespiau; +Cc: intel-gfx

On Thu, Jul 10, 2014 at 10:47:21PM +0100, Damien Lespiau wrote:
> On Thu, Jul 10, 2014 at 10:25:27PM +0200, Daniel Vetter wrote:
> > 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 :(
> 
> Sigh, I followed the !DRIVER_MODESET code path, I am very sad now.
> 
> I was thinking about having a Kconfig option to select a specific
> platform to compile the driver for and, by trying to make that work, we
> could end up with a nice per-platform split of the code. Would people be
> totally opposed to such a thing?

Not opposed if we're going to sign up the compiler for the resulting dce.
If it'll result in #ifdef hell then no.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2014-07-11  6:33 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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
2014-07-10 21:47   ` Damien Lespiau
2014-07-11  6:33     ` Daniel Vetter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox