Re: [PATCH 1/4] drm/i915: Store gen_mask inside the static device info

[Jani Nikula <jani.nikula@linux.intel.com>]

On Fri, 09 Feb 2018, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> Rather than deriving the gen_mask from the static intel_device_info->gen
> at runtime, prefill it in the static data.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_drv.c |  2 --
>  drivers/gpu/drm/i915/i915_pci.c | 39 ++++++++++++++++++++++++---------------
>  2 files changed, 24 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index daa9060bdfcb..90f4adbbff28 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -902,8 +902,6 @@ static int i915_driver_init_early(struct drm_i915_private *dev_priv,
>  	device_info->platform_mask = BIT(device_info->platform);
>  
>  	BUG_ON(device_info->gen > sizeof(device_info->gen_mask) * BITS_PER_BYTE);
> -	device_info->gen_mask = BIT(device_info->gen - 1);
> -
>  	spin_lock_init(&dev_priv->irq_lock);
>  	spin_lock_init(&dev_priv->gpu_error.lock);
>  	mutex_init(&dev_priv->backlight_lock);
> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> index 4e7a10c89782..3b4516d7f9a4 100644
> --- a/drivers/gpu/drm/i915/i915_pci.c
> +++ b/drivers/gpu/drm/i915/i915_pci.c
> @@ -29,6 +29,8 @@
>  #include "i915_drv.h"
>  #include "i915_selftest.h"
>  
> +#define GEN(x) .gen = (x), .gen_mask = BIT((x) - 1)

Please #undef GEN afterwards, ditto for the other macros in the other
patches.

Does the compiler warn if you overflow .gen_mask? Can we drop the
runtime overflow check now? Or better be safe than sorry with the
BUG_ON?

I like the destination in the series.

BR,
Jani.


> +
>  #define GEN_DEFAULT_PIPEOFFSETS \
>  	.pipe_offsets = { PIPE_A_OFFSET, PIPE_B_OFFSET, \
>  			  PIPE_C_OFFSET, PIPE_EDP_OFFSET }, \
> @@ -63,7 +65,8 @@
>  	.page_sizes = I915_GTT_PAGE_SIZE_4K
>  
>  #define GEN2_FEATURES \
> -	.gen = 2, .num_pipes = 1, \
> +	GEN(2), \
> +	.num_pipes = 1, \
>  	.has_overlay = 1, .overlay_needs_physical = 1, \
>  	.has_gmch_display = 1, \
>  	.hws_needs_physical = 1, \
> @@ -100,7 +103,8 @@ static const struct intel_device_info intel_i865g_info = {
>  };
>  
>  #define GEN3_FEATURES \
> -	.gen = 3, .num_pipes = 2, \
> +	GEN(3), \
> +	.num_pipes = 2, \
>  	.has_gmch_display = 1, \
>  	.ring_mask = RENDER_RING, \
>  	.has_snoop = true, \
> @@ -163,7 +167,8 @@ static const struct intel_device_info intel_pineview_info = {
>  };
>  
>  #define GEN4_FEATURES \
> -	.gen = 4, .num_pipes = 2, \
> +	GEN(4), \
> +	.num_pipes = 2, \
>  	.has_hotplug = 1, \
>  	.has_gmch_display = 1, \
>  	.ring_mask = RENDER_RING, \
> @@ -205,7 +210,8 @@ static const struct intel_device_info intel_gm45_info = {
>  };
>  
>  #define GEN5_FEATURES \
> -	.gen = 5, .num_pipes = 2, \
> +	GEN(5), \
> +	.num_pipes = 2, \
>  	.has_hotplug = 1, \
>  	.ring_mask = RENDER_RING | BSD_RING, \
>  	.has_snoop = true, \
> @@ -227,7 +233,8 @@ static const struct intel_device_info intel_ironlake_m_info = {
>  };
>  
>  #define GEN6_FEATURES \
> -	.gen = 6, .num_pipes = 2, \
> +	GEN(6), \
> +	.num_pipes = 2, \
>  	.has_hotplug = 1, \
>  	.has_fbc = 1, \
>  	.ring_mask = RENDER_RING | BSD_RING | BLT_RING, \
> @@ -270,7 +277,8 @@ static const struct intel_device_info intel_sandybridge_m_gt2_info = {
>  };
>  
>  #define GEN7_FEATURES  \
> -	.gen = 7, .num_pipes = 3, \
> +	GEN(7), \
> +	.num_pipes = 3, \
>  	.has_hotplug = 1, \
>  	.has_fbc = 1, \
>  	.ring_mask = RENDER_RING | BSD_RING | BLT_RING, \
> @@ -324,7 +332,7 @@ static const struct intel_device_info intel_ivybridge_q_info = {
>  
>  static const struct intel_device_info intel_valleyview_info = {
>  	.platform = INTEL_VALLEYVIEW,
> -	.gen = 7,
> +	GEN(7),
>  	.is_lp = 1,
>  	.num_pipes = 2,
>  	.has_psr = 1,
> @@ -385,7 +393,7 @@ static const struct intel_device_info intel_haswell_gt3_info = {
>  
>  #define BDW_PLATFORM \
>  	GEN8_FEATURES, \
> -	.gen = 8, \
> +	GEN(8), \
>  	.platform = INTEL_BROADWELL
>  
>  static const struct intel_device_info intel_broadwell_gt1_info = {
> @@ -413,7 +421,8 @@ static const struct intel_device_info intel_broadwell_gt3_info = {
>  };
>  
>  static const struct intel_device_info intel_cherryview_info = {
> -	.gen = 8, .num_pipes = 3,
> +	GEN(8),
> +	.num_pipes = 3,
>  	.has_hotplug = 1,
>  	.is_lp = 1,
>  	.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING,
> @@ -452,7 +461,7 @@ static const struct intel_device_info intel_cherryview_info = {
>  
>  #define SKL_PLATFORM \
>  	GEN9_FEATURES, \
> -	.gen = 9, \
> +	GEN(9), \
>  	.platform = INTEL_SKYLAKE
>  
>  static const struct intel_device_info intel_skylake_gt1_info = {
> @@ -481,7 +490,7 @@ static const struct intel_device_info intel_skylake_gt4_info = {
>  };
>  
>  #define GEN9_LP_FEATURES \
> -	.gen = 9, \
> +	GEN(9), \
>  	.is_lp = 1, \
>  	.has_hotplug = 1, \
>  	.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING, \
> @@ -526,7 +535,7 @@ static const struct intel_device_info intel_geminilake_info = {
>  
>  #define KBL_PLATFORM \
>  	GEN9_FEATURES, \
> -	.gen = 9, \
> +	GEN(9),  \
>  	.platform = INTEL_KABYLAKE
>  
>  static const struct intel_device_info intel_kabylake_gt1_info = {
> @@ -547,7 +556,7 @@ static const struct intel_device_info intel_kabylake_gt3_info = {
>  
>  #define CFL_PLATFORM \
>  	GEN9_FEATURES, \
> -	.gen = 9, \
> +	GEN(9), \
>  	.platform = INTEL_COFFEELAKE
>  
>  static const struct intel_device_info intel_coffeelake_gt1_info = {
> @@ -573,15 +582,15 @@ static const struct intel_device_info intel_coffeelake_gt3_info = {
>  
>  static const struct intel_device_info intel_cannonlake_info = {
>  	GEN10_FEATURES,
> +	GEN(10),
>  	.is_alpha_support = 1,
>  	.platform = INTEL_CANNONLAKE,
> -	.gen = 10,
>  	.gt = 2,
>  };
>  
>  #define GEN11_FEATURES \
>  	GEN10_FEATURES, \
> -	.gen = 11, \
> +	GEN(11), \
>  	.ddb_size = 2048, \
>  	.has_csr = 0

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx