All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/4] drm/i915: Store gen_mask inside the static device info
@ 2018-02-09 22:35 Chris Wilson
  2018-02-09 22:35 ` [PATCH 2/4] drm/i915: Always define GEN as part of GENx_FEATURES Chris Wilson
                   ` (6 more replies)
  0 siblings, 7 replies; 15+ messages in thread
From: Chris Wilson @ 2018-02-09 22:35 UTC (permalink / raw)
  To: intel-gfx

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)
+
 #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
 
-- 
2.16.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* [PATCH 2/4] drm/i915: Always define GEN as part of GENx_FEATURES
  2018-02-09 22:35 [PATCH 1/4] drm/i915: Store gen_mask inside the static device info Chris Wilson
@ 2018-02-09 22:35 ` Chris Wilson
  2018-02-14 22:40   ` Rodrigo Vivi
  2018-02-09 22:35 ` [PATCH 3/4] drm/i915: Store platform_mask inside the static device info Chris Wilson
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 15+ messages in thread
From: Chris Wilson @ 2018-02-09 22:35 UTC (permalink / raw)
  To: intel-gfx

Be consistent and define the device's GEN as part of the GENx_FEATURE.
It will be overridden by the next gen upon inheriting, as per usual.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_pci.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index 3b4516d7f9a4..364d3b41f816 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -383,6 +383,7 @@ static const struct intel_device_info intel_haswell_gt3_info = {
 
 #define GEN8_FEATURES \
 	G75_FEATURES, \
+	GEN(8), \
 	BDW_COLORS, \
 	.page_sizes = I915_GTT_PAGE_SIZE_4K | \
 		      I915_GTT_PAGE_SIZE_2M, \
@@ -393,7 +394,6 @@ static const struct intel_device_info intel_haswell_gt3_info = {
 
 #define BDW_PLATFORM \
 	GEN8_FEATURES, \
-	GEN(8), \
 	.platform = INTEL_BROADWELL
 
 static const struct intel_device_info intel_broadwell_gt1_info = {
@@ -452,6 +452,7 @@ static const struct intel_device_info intel_cherryview_info = {
 
 #define GEN9_FEATURES \
 	GEN8_FEATURES, \
+	GEN(9), \
 	GEN9_DEFAULT_PAGE_SIZES, \
 	.has_logical_ring_preemption = 1, \
 	.has_csr = 1, \
@@ -461,7 +462,6 @@ static const struct intel_device_info intel_cherryview_info = {
 
 #define SKL_PLATFORM \
 	GEN9_FEATURES, \
-	GEN(9), \
 	.platform = INTEL_SKYLAKE
 
 static const struct intel_device_info intel_skylake_gt1_info = {
@@ -535,7 +535,6 @@ static const struct intel_device_info intel_geminilake_info = {
 
 #define KBL_PLATFORM \
 	GEN9_FEATURES, \
-	GEN(9),  \
 	.platform = INTEL_KABYLAKE
 
 static const struct intel_device_info intel_kabylake_gt1_info = {
@@ -556,7 +555,6 @@ static const struct intel_device_info intel_kabylake_gt3_info = {
 
 #define CFL_PLATFORM \
 	GEN9_FEATURES, \
-	GEN(9), \
 	.platform = INTEL_COFFEELAKE
 
 static const struct intel_device_info intel_coffeelake_gt1_info = {
@@ -577,12 +575,12 @@ static const struct intel_device_info intel_coffeelake_gt3_info = {
 
 #define GEN10_FEATURES \
 	GEN9_FEATURES, \
+	GEN(10), \
 	.ddb_size = 1024, \
 	GLK_COLORS
 
 static const struct intel_device_info intel_cannonlake_info = {
 	GEN10_FEATURES,
-	GEN(10),
 	.is_alpha_support = 1,
 	.platform = INTEL_CANNONLAKE,
 	.gt = 2,
-- 
2.16.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* [PATCH 3/4] drm/i915: Store platform_mask inside the static device info
  2018-02-09 22:35 [PATCH 1/4] drm/i915: Store gen_mask inside the static device info Chris Wilson
  2018-02-09 22:35 ` [PATCH 2/4] drm/i915: Always define GEN as part of GENx_FEATURES Chris Wilson
@ 2018-02-09 22:35 ` Chris Wilson
  2018-02-14 22:45   ` Rodrigo Vivi
  2018-02-09 22:35 ` [PATCH 4/4] drm/i915: Disable dynamic setup of device_info->num_rings Chris Wilson
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 15+ messages in thread
From: Chris Wilson @ 2018-02-09 22:35 UTC (permalink / raw)
  To: intel-gfx

Rather than deriving the platform_mask from the
intel_device_static_info->platform at runtime, prefill it in the static
data.

 baseline.ko drivers/gpu/drm/i915/i915.ko
add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-20 (-20)
Function                                     old     new   delta
i915_driver_load                            5027    5007     -20
Total: Before=1331200, After=1331180, chg -0.00%

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 | 69 ++++++++++++++++++++++-------------------
 2 files changed, 37 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 90f4adbbff28..a81be8506797 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -899,8 +899,6 @@ static int i915_driver_init_early(struct drm_i915_private *dev_priv,
 
 	BUILD_BUG_ON(INTEL_MAX_PLATFORMS >
 		     sizeof(device_info->platform_mask) * BITS_PER_BYTE);
-	device_info->platform_mask = BIT(device_info->platform);
-
 	BUG_ON(device_info->gen > sizeof(device_info->gen_mask) * BITS_PER_BYTE);
 	spin_lock_init(&dev_priv->irq_lock);
 	spin_lock_init(&dev_priv->gpu_error.lock);
diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index 364d3b41f816..a10404686710 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -30,6 +30,7 @@
 #include "i915_selftest.h"
 
 #define GEN(x) .gen = (x), .gen_mask = BIT((x) - 1)
+#define PLATFORM(x) .platform = (x), .platform_mask = BIT(x)
 
 #define GEN_DEFAULT_PIPEOFFSETS \
 	.pipe_offsets = { PIPE_A_OFFSET, PIPE_B_OFFSET, \
@@ -79,19 +80,20 @@
 
 static const struct intel_device_info intel_i830_info = {
 	GEN2_FEATURES,
-	.platform = INTEL_I830,
+	PLATFORM(INTEL_I830),
 	.is_mobile = 1, .cursor_needs_physical = 1,
 	.num_pipes = 2, /* legal, last one wins */
 };
 
 static const struct intel_device_info intel_i845g_info = {
 	GEN2_FEATURES,
-	.platform = INTEL_I845G,
+	PLATFORM(INTEL_I845G),
 };
 
 static const struct intel_device_info intel_i85x_info = {
 	GEN2_FEATURES,
-	.platform = INTEL_I85X, .is_mobile = 1,
+	PLATFORM(INTEL_I85X),
+	.is_mobile = 1,
 	.num_pipes = 2, /* legal, last one wins */
 	.cursor_needs_physical = 1,
 	.has_fbc = 1,
@@ -99,7 +101,7 @@ static const struct intel_device_info intel_i85x_info = {
 
 static const struct intel_device_info intel_i865g_info = {
 	GEN2_FEATURES,
-	.platform = INTEL_I865G,
+	PLATFORM(INTEL_I865G),
 };
 
 #define GEN3_FEATURES \
@@ -114,7 +116,8 @@ static const struct intel_device_info intel_i865g_info = {
 
 static const struct intel_device_info intel_i915g_info = {
 	GEN3_FEATURES,
-	.platform = INTEL_I915G, .cursor_needs_physical = 1,
+	PLATFORM(INTEL_I915G),
+	.cursor_needs_physical = 1,
 	.has_overlay = 1, .overlay_needs_physical = 1,
 	.hws_needs_physical = 1,
 	.unfenced_needs_alignment = 1,
@@ -122,7 +125,7 @@ static const struct intel_device_info intel_i915g_info = {
 
 static const struct intel_device_info intel_i915gm_info = {
 	GEN3_FEATURES,
-	.platform = INTEL_I915GM,
+	PLATFORM(INTEL_I915GM),
 	.is_mobile = 1,
 	.cursor_needs_physical = 1,
 	.has_overlay = 1, .overlay_needs_physical = 1,
@@ -134,7 +137,7 @@ static const struct intel_device_info intel_i915gm_info = {
 
 static const struct intel_device_info intel_i945g_info = {
 	GEN3_FEATURES,
-	.platform = INTEL_I945G,
+	PLATFORM(INTEL_I945G),
 	.has_hotplug = 1, .cursor_needs_physical = 1,
 	.has_overlay = 1, .overlay_needs_physical = 1,
 	.hws_needs_physical = 1,
@@ -143,7 +146,8 @@ static const struct intel_device_info intel_i945g_info = {
 
 static const struct intel_device_info intel_i945gm_info = {
 	GEN3_FEATURES,
-	.platform = INTEL_I945GM, .is_mobile = 1,
+	PLATFORM(INTEL_I945GM),
+	.is_mobile = 1,
 	.has_hotplug = 1, .cursor_needs_physical = 1,
 	.has_overlay = 1, .overlay_needs_physical = 1,
 	.supports_tv = 1,
@@ -154,14 +158,15 @@ static const struct intel_device_info intel_i945gm_info = {
 
 static const struct intel_device_info intel_g33_info = {
 	GEN3_FEATURES,
-	.platform = INTEL_G33,
+	PLATFORM(INTEL_G33),
 	.has_hotplug = 1,
 	.has_overlay = 1,
 };
 
 static const struct intel_device_info intel_pineview_info = {
 	GEN3_FEATURES,
-	.platform = INTEL_PINEVIEW, .is_mobile = 1,
+	PLATFORM(INTEL_PINEVIEW),
+	.is_mobile = 1,
 	.has_hotplug = 1,
 	.has_overlay = 1,
 };
@@ -179,7 +184,7 @@ static const struct intel_device_info intel_pineview_info = {
 
 static const struct intel_device_info intel_i965g_info = {
 	GEN4_FEATURES,
-	.platform = INTEL_I965G,
+	PLATFORM(INTEL_I965G),
 	.has_overlay = 1,
 	.hws_needs_physical = 1,
 	.has_snoop = false,
@@ -187,7 +192,7 @@ static const struct intel_device_info intel_i965g_info = {
 
 static const struct intel_device_info intel_i965gm_info = {
 	GEN4_FEATURES,
-	.platform = INTEL_I965GM,
+	PLATFORM(INTEL_I965GM),
 	.is_mobile = 1, .has_fbc = 1,
 	.has_overlay = 1,
 	.supports_tv = 1,
@@ -197,13 +202,13 @@ static const struct intel_device_info intel_i965gm_info = {
 
 static const struct intel_device_info intel_g45_info = {
 	GEN4_FEATURES,
-	.platform = INTEL_G45,
+	PLATFORM(INTEL_G45),
 	.ring_mask = RENDER_RING | BSD_RING,
 };
 
 static const struct intel_device_info intel_gm45_info = {
 	GEN4_FEATURES,
-	.platform = INTEL_GM45,
+	PLATFORM(INTEL_GM45),
 	.is_mobile = 1, .has_fbc = 1,
 	.supports_tv = 1,
 	.ring_mask = RENDER_RING | BSD_RING,
@@ -223,12 +228,12 @@ static const struct intel_device_info intel_gm45_info = {
 
 static const struct intel_device_info intel_ironlake_d_info = {
 	GEN5_FEATURES,
-	.platform = INTEL_IRONLAKE,
+	PLATFORM(INTEL_IRONLAKE),
 };
 
 static const struct intel_device_info intel_ironlake_m_info = {
 	GEN5_FEATURES,
-	.platform = INTEL_IRONLAKE,
+	PLATFORM(INTEL_IRONLAKE),
 	.is_mobile = 1, .has_fbc = 1,
 };
 
@@ -248,7 +253,7 @@ static const struct intel_device_info intel_ironlake_m_info = {
 
 #define SNB_D_PLATFORM \
 	GEN6_FEATURES, \
-	.platform = INTEL_SANDYBRIDGE
+	PLATFORM(INTEL_SANDYBRIDGE)
 
 static const struct intel_device_info intel_sandybridge_d_gt1_info = {
 	SNB_D_PLATFORM,
@@ -262,7 +267,7 @@ static const struct intel_device_info intel_sandybridge_d_gt2_info = {
 
 #define SNB_M_PLATFORM \
 	GEN6_FEATURES, \
-	.platform = INTEL_SANDYBRIDGE, \
+	PLATFORM(INTEL_SANDYBRIDGE), \
 	.is_mobile = 1
 
 
@@ -293,7 +298,7 @@ static const struct intel_device_info intel_sandybridge_m_gt2_info = {
 
 #define IVB_D_PLATFORM \
 	GEN7_FEATURES, \
-	.platform = INTEL_IVYBRIDGE, \
+	PLATFORM(INTEL_IVYBRIDGE), \
 	.has_l3_dpf = 1
 
 static const struct intel_device_info intel_ivybridge_d_gt1_info = {
@@ -308,7 +313,7 @@ static const struct intel_device_info intel_ivybridge_d_gt2_info = {
 
 #define IVB_M_PLATFORM \
 	GEN7_FEATURES, \
-	.platform = INTEL_IVYBRIDGE, \
+	PLATFORM(INTEL_IVYBRIDGE), \
 	.is_mobile = 1, \
 	.has_l3_dpf = 1
 
@@ -324,14 +329,14 @@ static const struct intel_device_info intel_ivybridge_m_gt2_info = {
 
 static const struct intel_device_info intel_ivybridge_q_info = {
 	GEN7_FEATURES,
-	.platform = INTEL_IVYBRIDGE,
+	PLATFORM(INTEL_IVYBRIDGE),
 	.gt = 2,
 	.num_pipes = 0, /* legal, last one wins */
 	.has_l3_dpf = 1,
 };
 
 static const struct intel_device_info intel_valleyview_info = {
-	.platform = INTEL_VALLEYVIEW,
+	PLATFORM(INTEL_VALLEYVIEW),
 	GEN(7),
 	.is_lp = 1,
 	.num_pipes = 2,
@@ -363,7 +368,7 @@ static const struct intel_device_info intel_valleyview_info = {
 
 #define HSW_PLATFORM \
 	G75_FEATURES, \
-	.platform = INTEL_HASWELL, \
+	PLATFORM(INTEL_HASWELL), \
 	.has_l3_dpf = 1
 
 static const struct intel_device_info intel_haswell_gt1_info = {
@@ -394,7 +399,7 @@ static const struct intel_device_info intel_haswell_gt3_info = {
 
 #define BDW_PLATFORM \
 	GEN8_FEATURES, \
-	.platform = INTEL_BROADWELL
+	PLATFORM(INTEL_BROADWELL)
 
 static const struct intel_device_info intel_broadwell_gt1_info = {
 	BDW_PLATFORM,
@@ -421,12 +426,12 @@ static const struct intel_device_info intel_broadwell_gt3_info = {
 };
 
 static const struct intel_device_info intel_cherryview_info = {
+	PLATFORM(INTEL_CHERRYVIEW),
 	GEN(8),
 	.num_pipes = 3,
 	.has_hotplug = 1,
 	.is_lp = 1,
 	.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING,
-	.platform = INTEL_CHERRYVIEW,
 	.has_64bit_reloc = 1,
 	.has_psr = 1,
 	.has_runtime_pm = 1,
@@ -462,7 +467,7 @@ static const struct intel_device_info intel_cherryview_info = {
 
 #define SKL_PLATFORM \
 	GEN9_FEATURES, \
-	.platform = INTEL_SKYLAKE
+	PLATFORM(INTEL_SKYLAKE)
 
 static const struct intel_device_info intel_skylake_gt1_info = {
 	SKL_PLATFORM,
@@ -522,20 +527,20 @@ static const struct intel_device_info intel_skylake_gt4_info = {
 
 static const struct intel_device_info intel_broxton_info = {
 	GEN9_LP_FEATURES,
-	.platform = INTEL_BROXTON,
+	PLATFORM(INTEL_BROXTON),
 	.ddb_size = 512,
 };
 
 static const struct intel_device_info intel_geminilake_info = {
 	GEN9_LP_FEATURES,
-	.platform = INTEL_GEMINILAKE,
+	PLATFORM(INTEL_GEMINILAKE),
 	.ddb_size = 1024,
 	GLK_COLORS,
 };
 
 #define KBL_PLATFORM \
 	GEN9_FEATURES, \
-	.platform = INTEL_KABYLAKE
+	PLATFORM(INTEL_KABYLAKE)
 
 static const struct intel_device_info intel_kabylake_gt1_info = {
 	KBL_PLATFORM,
@@ -555,7 +560,7 @@ static const struct intel_device_info intel_kabylake_gt3_info = {
 
 #define CFL_PLATFORM \
 	GEN9_FEATURES, \
-	.platform = INTEL_COFFEELAKE
+	PLATFORM(INTEL_COFFEELAKE)
 
 static const struct intel_device_info intel_coffeelake_gt1_info = {
 	CFL_PLATFORM,
@@ -581,8 +586,8 @@ static const struct intel_device_info intel_coffeelake_gt3_info = {
 
 static const struct intel_device_info intel_cannonlake_info = {
 	GEN10_FEATURES,
+	PLATFORM(INTEL_CANNONLAKE),
 	.is_alpha_support = 1,
-	.platform = INTEL_CANNONLAKE,
 	.gt = 2,
 };
 
@@ -594,7 +599,7 @@ static const struct intel_device_info intel_cannonlake_info = {
 
 static const struct intel_device_info intel_icelake_11_info = {
 	GEN11_FEATURES,
-	.platform = INTEL_ICELAKE,
+	PLATFORM(INTEL_ICELAKE),
 	.is_alpha_support = 1,
 	.has_resource_streamer = 0,
 };
-- 
2.16.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* [PATCH 4/4] drm/i915: Disable dynamic setup of device_info->num_rings
  2018-02-09 22:35 [PATCH 1/4] drm/i915: Store gen_mask inside the static device info Chris Wilson
  2018-02-09 22:35 ` [PATCH 2/4] drm/i915: Always define GEN as part of GENx_FEATURES Chris Wilson
  2018-02-09 22:35 ` [PATCH 3/4] drm/i915: Store platform_mask inside the static device info Chris Wilson
@ 2018-02-09 22:35 ` Chris Wilson
  2018-02-09 23:46   ` Oscar Mateo
  2018-02-09 23:13 ` ✓ Fi.CI.BAT: success for series starting with [1/4] drm/i915: Store gen_mask inside the static device info Patchwork
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 15+ messages in thread
From: Chris Wilson @ 2018-02-09 22:35 UTC (permalink / raw)
  To: intel-gfx

In order to allow the compiler to use a known constant number of
available engines, disable modification of intel_device_static_info
during engine bring up. Instead of trying to gracefully hide the broken
setup, error out -- in theory, this should be caught during power on.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_pci.c        | 34 +++++++++++++++++-----------------
 drivers/gpu/drm/i915/intel_engine_cs.c | 16 +++++-----------
 2 files changed, 22 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index a10404686710..1e5bc971e975 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -31,6 +31,7 @@
 
 #define GEN(x) .gen = (x), .gen_mask = BIT((x) - 1)
 #define PLATFORM(x) .platform = (x), .platform_mask = BIT(x)
+#define RINGS(x) .ring_mask = (x), .num_rings = hweight32(x)
 
 #define GEN_DEFAULT_PIPEOFFSETS \
 	.pipe_offsets = { PIPE_A_OFFSET, PIPE_B_OFFSET, \
@@ -67,12 +68,12 @@
 
 #define GEN2_FEATURES \
 	GEN(2), \
+	RINGS(RENDER_RING), \
 	.num_pipes = 1, \
 	.has_overlay = 1, .overlay_needs_physical = 1, \
 	.has_gmch_display = 1, \
 	.hws_needs_physical = 1, \
 	.unfenced_needs_alignment = 1, \
-	.ring_mask = RENDER_RING, \
 	.has_snoop = true, \
 	GEN_DEFAULT_PIPEOFFSETS, \
 	GEN_DEFAULT_PAGE_SIZES, \
@@ -106,9 +107,9 @@ static const struct intel_device_info intel_i865g_info = {
 
 #define GEN3_FEATURES \
 	GEN(3), \
+	RINGS(RENDER_RING), \
 	.num_pipes = 2, \
 	.has_gmch_display = 1, \
-	.ring_mask = RENDER_RING, \
 	.has_snoop = true, \
 	GEN_DEFAULT_PIPEOFFSETS, \
 	GEN_DEFAULT_PAGE_SIZES, \
@@ -173,10 +174,10 @@ static const struct intel_device_info intel_pineview_info = {
 
 #define GEN4_FEATURES \
 	GEN(4), \
+	RINGS(RENDER_RING), \
 	.num_pipes = 2, \
 	.has_hotplug = 1, \
 	.has_gmch_display = 1, \
-	.ring_mask = RENDER_RING, \
 	.has_snoop = true, \
 	GEN_DEFAULT_PIPEOFFSETS, \
 	GEN_DEFAULT_PAGE_SIZES, \
@@ -203,22 +204,22 @@ static const struct intel_device_info intel_i965gm_info = {
 static const struct intel_device_info intel_g45_info = {
 	GEN4_FEATURES,
 	PLATFORM(INTEL_G45),
-	.ring_mask = RENDER_RING | BSD_RING,
+	RINGS(RENDER_RING | BSD_RING),
 };
 
 static const struct intel_device_info intel_gm45_info = {
 	GEN4_FEATURES,
 	PLATFORM(INTEL_GM45),
+	RINGS(RENDER_RING | BSD_RING),
 	.is_mobile = 1, .has_fbc = 1,
 	.supports_tv = 1,
-	.ring_mask = RENDER_RING | BSD_RING,
 };
 
 #define GEN5_FEATURES \
 	GEN(5), \
+	RINGS(RENDER_RING | BSD_RING), \
 	.num_pipes = 2, \
 	.has_hotplug = 1, \
-	.ring_mask = RENDER_RING | BSD_RING, \
 	.has_snoop = true, \
 	/* ilk does support rc6, but we do not implement [power] contexts */ \
 	.has_rc6 = 0, \
@@ -239,10 +240,10 @@ static const struct intel_device_info intel_ironlake_m_info = {
 
 #define GEN6_FEATURES \
 	GEN(6), \
+	RINGS(RENDER_RING | BSD_RING | BLT_RING), \
 	.num_pipes = 2, \
 	.has_hotplug = 1, \
 	.has_fbc = 1, \
-	.ring_mask = RENDER_RING | BSD_RING | BLT_RING, \
 	.has_llc = 1, \
 	.has_rc6 = 1, \
 	.has_rc6p = 1, \
@@ -283,10 +284,10 @@ static const struct intel_device_info intel_sandybridge_m_gt2_info = {
 
 #define GEN7_FEATURES  \
 	GEN(7), \
+	RINGS(RENDER_RING | BSD_RING | BLT_RING), \
 	.num_pipes = 3, \
 	.has_hotplug = 1, \
 	.has_fbc = 1, \
-	.ring_mask = RENDER_RING | BSD_RING | BLT_RING, \
 	.has_llc = 1, \
 	.has_rc6 = 1, \
 	.has_rc6p = 1, \
@@ -338,6 +339,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),
+	RINGS(RENDER_RING | BSD_RING | BLT_RING),
 	.is_lp = 1,
 	.num_pipes = 2,
 	.has_psr = 1,
@@ -348,7 +350,6 @@ static const struct intel_device_info intel_valleyview_info = {
 	.has_aliasing_ppgtt = 1,
 	.has_full_ppgtt = 1,
 	.has_snoop = true,
-	.ring_mask = RENDER_RING | BSD_RING | BLT_RING,
 	.display_mmio_offset = VLV_DISPLAY_BASE,
 	GEN_DEFAULT_PAGE_SIZES,
 	GEN_DEFAULT_PIPEOFFSETS,
@@ -357,7 +358,7 @@ static const struct intel_device_info intel_valleyview_info = {
 
 #define G75_FEATURES  \
 	GEN7_FEATURES, \
-	.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING, \
+	RINGS(RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING), \
 	.has_ddi = 1, \
 	.has_fpga_dbg = 1, \
 	.has_psr = 1, \
@@ -421,17 +422,17 @@ static const struct intel_device_info intel_broadwell_rsvd_info = {
 
 static const struct intel_device_info intel_broadwell_gt3_info = {
 	BDW_PLATFORM,
+	RINGS(RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING),
 	.gt = 3,
-	.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
 };
 
 static const struct intel_device_info intel_cherryview_info = {
 	PLATFORM(INTEL_CHERRYVIEW),
 	GEN(8),
+	RINGS(RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING),
 	.num_pipes = 3,
 	.has_hotplug = 1,
 	.is_lp = 1,
-	.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING,
 	.has_64bit_reloc = 1,
 	.has_psr = 1,
 	.has_runtime_pm = 1,
@@ -481,8 +482,7 @@ static const struct intel_device_info intel_skylake_gt2_info = {
 
 #define SKL_GT3_PLUS_PLATFORM \
 	SKL_PLATFORM, \
-	.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING
-
+	RINGS(RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING)
 
 static const struct intel_device_info intel_skylake_gt3_info = {
 	SKL_GT3_PLUS_PLATFORM,
@@ -496,9 +496,9 @@ static const struct intel_device_info intel_skylake_gt4_info = {
 
 #define GEN9_LP_FEATURES \
 	GEN(9), \
+	RINGS(RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING), \
 	.is_lp = 1, \
 	.has_hotplug = 1, \
-	.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING, \
 	.num_pipes = 3, \
 	.has_64bit_reloc = 1, \
 	.has_ddi = 1, \
@@ -554,8 +554,8 @@ static const struct intel_device_info intel_kabylake_gt2_info = {
 
 static const struct intel_device_info intel_kabylake_gt3_info = {
 	KBL_PLATFORM,
+	RINGS(RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING),
 	.gt = 3,
-	.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
 };
 
 #define CFL_PLATFORM \
@@ -575,7 +575,7 @@ static const struct intel_device_info intel_coffeelake_gt2_info = {
 static const struct intel_device_info intel_coffeelake_gt3_info = {
 	CFL_PLATFORM,
 	.gt = 3,
-	.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
+	RINGS(RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING),
 };
 
 #define GEN10_FEATURES \
diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index 0ad9184eba97..c65fce102d65 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -263,18 +263,12 @@ intel_engine_setup(struct drm_i915_private *dev_priv,
  */
 int intel_engines_init_mmio(struct drm_i915_private *dev_priv)
 {
-	struct intel_device_info *device_info = mkwrite_device_info(dev_priv);
-	const unsigned int ring_mask = INTEL_INFO(dev_priv)->ring_mask;
 	struct intel_engine_cs *engine;
 	enum intel_engine_id id;
 	unsigned int mask = 0;
 	unsigned int i;
 	int err;
 
-	WARN_ON(ring_mask == 0);
-	WARN_ON(ring_mask &
-		GENMASK(sizeof(mask) * BITS_PER_BYTE - 1, I915_NUM_ENGINES));
-
 	for (i = 0; i < ARRAY_SIZE(intel_engines); i++) {
 		if (!HAS_ENGINE(dev_priv, i))
 			continue;
@@ -291,17 +285,17 @@ int intel_engines_init_mmio(struct drm_i915_private *dev_priv)
 	 * are added to the driver by a warning and disabling the forgotten
 	 * engines.
 	 */
-	if (WARN_ON(mask != ring_mask))
-		device_info->ring_mask = mask;
+	if (GEM_WARN_ON(mask != INTEL_INFO(dev_priv)->ring_mask)) {
+		err = -ENODEV;
+		goto cleanup;
+	}
 
 	/* We always presume we have at least RCS available for later probing */
-	if (WARN_ON(!HAS_ENGINE(dev_priv, RCS))) {
+	if (GEM_WARN_ON(!HAS_ENGINE(dev_priv, RCS))) {
 		err = -ENODEV;
 		goto cleanup;
 	}
 
-	device_info->num_rings = hweight32(mask);
-
 	i915_check_and_clear_faults(dev_priv);
 
 	return 0;
-- 
2.16.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* ✓ Fi.CI.BAT: success for series starting with [1/4] drm/i915: Store gen_mask inside the static device info
  2018-02-09 22:35 [PATCH 1/4] drm/i915: Store gen_mask inside the static device info Chris Wilson
                   ` (2 preceding siblings ...)
  2018-02-09 22:35 ` [PATCH 4/4] drm/i915: Disable dynamic setup of device_info->num_rings Chris Wilson
@ 2018-02-09 23:13 ` Patchwork
  2018-02-10  0:24 ` ✗ Fi.CI.IGT: failure " Patchwork
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 15+ messages in thread
From: Patchwork @ 2018-02-09 23:13 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/4] drm/i915: Store gen_mask inside the static device info
URL   : https://patchwork.freedesktop.org/series/38026/
State : success

== Summary ==

Series 38026v1 series starting with [1/4] drm/i915: Store gen_mask inside the static device info
https://patchwork.freedesktop.org/api/1.0/series/38026/revisions/1/mbox/

Test kms_psr_sink_crc:
        Subgroup psr_basic:
                incomplete -> SKIP       (fi-elk-e7500)

fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:420s
fi-bdw-gvtdvm    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:421s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:375s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:487s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:286s
fi-bxt-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:481s
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:484s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:467s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:457s
fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:569s
fi-cnl-y3        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:583s
fi-elk-e7500     total:288  pass:229  dwarn:0   dfail:0   fail:0   skip:59  time:415s
fi-gdg-551       total:288  pass:179  dwarn:0   dfail:0   fail:1   skip:108 time:284s
fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:515s
fi-hsw-4770      total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:394s
fi-ilk-650       total:288  pass:228  dwarn:0   dfail:0   fail:0   skip:60  time:410s
fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:460s
fi-ivb-3770      total:288  pass:255  dwarn:0   dfail:0   fail:0   skip:33  time:412s
fi-kbl-7500u     total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:459s
fi-kbl-7560u     total:288  pass:269  dwarn:0   dfail:0   fail:0   skip:19  time:496s
fi-kbl-r         total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:499s
fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:600s
fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:426s
fi-skl-6600u     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:510s
fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:524s
fi-skl-6700k2    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:489s
fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:487s
fi-skl-guc       total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:411s
fi-skl-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:428s
fi-snb-2520m     total:3    pass:2    dwarn:0   dfail:0   fail:0   skip:0  
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:394s
Blacklisted hosts:
fi-glk-dsi       total:149  pass:135  dwarn:0   dfail:0   fail:0   skip:13 

2bea3467efbacf91c06fb231d066d05507a5b453 drm-tip: 2018y-02m-09d-22h-30m-41s UTC integration manifest
81619b2c8cc6 drm/i915: Disable dynamic setup of device_info->num_rings
3287d0a413df drm/i915: Store platform_mask inside the static device info
6c96db54ee14 drm/i915: Always define GEN as part of GENx_FEATURES
450c0e8bbdc1 drm/i915: Store gen_mask inside the static device info

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7973/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 4/4] drm/i915: Disable dynamic setup of device_info->num_rings
  2018-02-09 22:35 ` [PATCH 4/4] drm/i915: Disable dynamic setup of device_info->num_rings Chris Wilson
@ 2018-02-09 23:46   ` Oscar Mateo
  2018-02-10  8:39     ` Chris Wilson
  0 siblings, 1 reply; 15+ messages in thread
From: Oscar Mateo @ 2018-02-09 23:46 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx



On 02/09/2018 02:35 PM, Chris Wilson wrote:
> In order to allow the compiler to use a known constant number of
> available engines, disable modification of intel_device_static_info
> during engine bring up. Instead of trying to gracefully hide the broken
> setup, error out -- in theory, this should be caught during power on.

We are about to have a case for dynamic number of available engines. 
It's one of the ICL patches:

drm/i915/icl: Check for fused-off VDBOX and VEBOX instances

intel_device_runtime_info as well?

> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_pci.c        | 34 +++++++++++++++++-----------------
>   drivers/gpu/drm/i915/intel_engine_cs.c | 16 +++++-----------
>   2 files changed, 22 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> index a10404686710..1e5bc971e975 100644
> --- a/drivers/gpu/drm/i915/i915_pci.c
> +++ b/drivers/gpu/drm/i915/i915_pci.c
> @@ -31,6 +31,7 @@
>   
>   #define GEN(x) .gen = (x), .gen_mask = BIT((x) - 1)
>   #define PLATFORM(x) .platform = (x), .platform_mask = BIT(x)
> +#define RINGS(x) .ring_mask = (x), .num_rings = hweight32(x)
>   
>   #define GEN_DEFAULT_PIPEOFFSETS \
>   	.pipe_offsets = { PIPE_A_OFFSET, PIPE_B_OFFSET, \
> @@ -67,12 +68,12 @@
>   
>   #define GEN2_FEATURES \
>   	GEN(2), \
> +	RINGS(RENDER_RING), \
>   	.num_pipes = 1, \
>   	.has_overlay = 1, .overlay_needs_physical = 1, \
>   	.has_gmch_display = 1, \
>   	.hws_needs_physical = 1, \
>   	.unfenced_needs_alignment = 1, \
> -	.ring_mask = RENDER_RING, \
>   	.has_snoop = true, \
>   	GEN_DEFAULT_PIPEOFFSETS, \
>   	GEN_DEFAULT_PAGE_SIZES, \
> @@ -106,9 +107,9 @@ static const struct intel_device_info intel_i865g_info = {
>   
>   #define GEN3_FEATURES \
>   	GEN(3), \
> +	RINGS(RENDER_RING), \
>   	.num_pipes = 2, \
>   	.has_gmch_display = 1, \
> -	.ring_mask = RENDER_RING, \
>   	.has_snoop = true, \
>   	GEN_DEFAULT_PIPEOFFSETS, \
>   	GEN_DEFAULT_PAGE_SIZES, \
> @@ -173,10 +174,10 @@ static const struct intel_device_info intel_pineview_info = {
>   
>   #define GEN4_FEATURES \
>   	GEN(4), \
> +	RINGS(RENDER_RING), \
>   	.num_pipes = 2, \
>   	.has_hotplug = 1, \
>   	.has_gmch_display = 1, \
> -	.ring_mask = RENDER_RING, \
>   	.has_snoop = true, \
>   	GEN_DEFAULT_PIPEOFFSETS, \
>   	GEN_DEFAULT_PAGE_SIZES, \
> @@ -203,22 +204,22 @@ static const struct intel_device_info intel_i965gm_info = {
>   static const struct intel_device_info intel_g45_info = {
>   	GEN4_FEATURES,
>   	PLATFORM(INTEL_G45),
> -	.ring_mask = RENDER_RING | BSD_RING,
> +	RINGS(RENDER_RING | BSD_RING),
>   };
>   
>   static const struct intel_device_info intel_gm45_info = {
>   	GEN4_FEATURES,
>   	PLATFORM(INTEL_GM45),
> +	RINGS(RENDER_RING | BSD_RING),
>   	.is_mobile = 1, .has_fbc = 1,
>   	.supports_tv = 1,
> -	.ring_mask = RENDER_RING | BSD_RING,
>   };
>   
>   #define GEN5_FEATURES \
>   	GEN(5), \
> +	RINGS(RENDER_RING | BSD_RING), \
>   	.num_pipes = 2, \
>   	.has_hotplug = 1, \
> -	.ring_mask = RENDER_RING | BSD_RING, \
>   	.has_snoop = true, \
>   	/* ilk does support rc6, but we do not implement [power] contexts */ \
>   	.has_rc6 = 0, \
> @@ -239,10 +240,10 @@ static const struct intel_device_info intel_ironlake_m_info = {
>   
>   #define GEN6_FEATURES \
>   	GEN(6), \
> +	RINGS(RENDER_RING | BSD_RING | BLT_RING), \
>   	.num_pipes = 2, \
>   	.has_hotplug = 1, \
>   	.has_fbc = 1, \
> -	.ring_mask = RENDER_RING | BSD_RING | BLT_RING, \
>   	.has_llc = 1, \
>   	.has_rc6 = 1, \
>   	.has_rc6p = 1, \
> @@ -283,10 +284,10 @@ static const struct intel_device_info intel_sandybridge_m_gt2_info = {
>   
>   #define GEN7_FEATURES  \
>   	GEN(7), \
> +	RINGS(RENDER_RING | BSD_RING | BLT_RING), \
>   	.num_pipes = 3, \
>   	.has_hotplug = 1, \
>   	.has_fbc = 1, \
> -	.ring_mask = RENDER_RING | BSD_RING | BLT_RING, \
>   	.has_llc = 1, \
>   	.has_rc6 = 1, \
>   	.has_rc6p = 1, \
> @@ -338,6 +339,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),
> +	RINGS(RENDER_RING | BSD_RING | BLT_RING),
>   	.is_lp = 1,
>   	.num_pipes = 2,
>   	.has_psr = 1,
> @@ -348,7 +350,6 @@ static const struct intel_device_info intel_valleyview_info = {
>   	.has_aliasing_ppgtt = 1,
>   	.has_full_ppgtt = 1,
>   	.has_snoop = true,
> -	.ring_mask = RENDER_RING | BSD_RING | BLT_RING,
>   	.display_mmio_offset = VLV_DISPLAY_BASE,
>   	GEN_DEFAULT_PAGE_SIZES,
>   	GEN_DEFAULT_PIPEOFFSETS,
> @@ -357,7 +358,7 @@ static const struct intel_device_info intel_valleyview_info = {
>   
>   #define G75_FEATURES  \
>   	GEN7_FEATURES, \
> -	.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING, \
> +	RINGS(RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING), \
>   	.has_ddi = 1, \
>   	.has_fpga_dbg = 1, \
>   	.has_psr = 1, \
> @@ -421,17 +422,17 @@ static const struct intel_device_info intel_broadwell_rsvd_info = {
>   
>   static const struct intel_device_info intel_broadwell_gt3_info = {
>   	BDW_PLATFORM,
> +	RINGS(RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING),
>   	.gt = 3,
> -	.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
>   };
>   
>   static const struct intel_device_info intel_cherryview_info = {
>   	PLATFORM(INTEL_CHERRYVIEW),
>   	GEN(8),
> +	RINGS(RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING),
>   	.num_pipes = 3,
>   	.has_hotplug = 1,
>   	.is_lp = 1,
> -	.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING,
>   	.has_64bit_reloc = 1,
>   	.has_psr = 1,
>   	.has_runtime_pm = 1,
> @@ -481,8 +482,7 @@ static const struct intel_device_info intel_skylake_gt2_info = {
>   
>   #define SKL_GT3_PLUS_PLATFORM \
>   	SKL_PLATFORM, \
> -	.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING
> -
> +	RINGS(RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING)
>   
>   static const struct intel_device_info intel_skylake_gt3_info = {
>   	SKL_GT3_PLUS_PLATFORM,
> @@ -496,9 +496,9 @@ static const struct intel_device_info intel_skylake_gt4_info = {
>   
>   #define GEN9_LP_FEATURES \
>   	GEN(9), \
> +	RINGS(RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING), \
>   	.is_lp = 1, \
>   	.has_hotplug = 1, \
> -	.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING, \
>   	.num_pipes = 3, \
>   	.has_64bit_reloc = 1, \
>   	.has_ddi = 1, \
> @@ -554,8 +554,8 @@ static const struct intel_device_info intel_kabylake_gt2_info = {
>   
>   static const struct intel_device_info intel_kabylake_gt3_info = {
>   	KBL_PLATFORM,
> +	RINGS(RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING),
>   	.gt = 3,
> -	.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
>   };
>   
>   #define CFL_PLATFORM \
> @@ -575,7 +575,7 @@ static const struct intel_device_info intel_coffeelake_gt2_info = {
>   static const struct intel_device_info intel_coffeelake_gt3_info = {
>   	CFL_PLATFORM,
>   	.gt = 3,
> -	.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
> +	RINGS(RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING),
>   };
>   
>   #define GEN10_FEATURES \
> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
> index 0ad9184eba97..c65fce102d65 100644
> --- a/drivers/gpu/drm/i915/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
> @@ -263,18 +263,12 @@ intel_engine_setup(struct drm_i915_private *dev_priv,
>    */
>   int intel_engines_init_mmio(struct drm_i915_private *dev_priv)
>   {
> -	struct intel_device_info *device_info = mkwrite_device_info(dev_priv);
> -	const unsigned int ring_mask = INTEL_INFO(dev_priv)->ring_mask;
>   	struct intel_engine_cs *engine;
>   	enum intel_engine_id id;
>   	unsigned int mask = 0;
>   	unsigned int i;
>   	int err;
>   
> -	WARN_ON(ring_mask == 0);
> -	WARN_ON(ring_mask &
> -		GENMASK(sizeof(mask) * BITS_PER_BYTE - 1, I915_NUM_ENGINES));
> -
>   	for (i = 0; i < ARRAY_SIZE(intel_engines); i++) {
>   		if (!HAS_ENGINE(dev_priv, i))
>   			continue;
> @@ -291,17 +285,17 @@ int intel_engines_init_mmio(struct drm_i915_private *dev_priv)
>   	 * are added to the driver by a warning and disabling the forgotten
>   	 * engines.
>   	 */
> -	if (WARN_ON(mask != ring_mask))
> -		device_info->ring_mask = mask;
> +	if (GEM_WARN_ON(mask != INTEL_INFO(dev_priv)->ring_mask)) {
> +		err = -ENODEV;
> +		goto cleanup;
> +	}
>   
>   	/* We always presume we have at least RCS available for later probing */
> -	if (WARN_ON(!HAS_ENGINE(dev_priv, RCS))) {
> +	if (GEM_WARN_ON(!HAS_ENGINE(dev_priv, RCS))) {
>   		err = -ENODEV;
>   		goto cleanup;
>   	}
>   
> -	device_info->num_rings = hweight32(mask);
> -
>   	i915_check_and_clear_faults(dev_priv);
>   
>   	return 0;

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* ✗ Fi.CI.IGT: failure for series starting with [1/4] drm/i915: Store gen_mask inside the static device info
  2018-02-09 22:35 [PATCH 1/4] drm/i915: Store gen_mask inside the static device info Chris Wilson
                   ` (3 preceding siblings ...)
  2018-02-09 23:13 ` ✓ Fi.CI.BAT: success for series starting with [1/4] drm/i915: Store gen_mask inside the static device info Patchwork
@ 2018-02-10  0:24 ` Patchwork
  2018-02-12  9:58 ` [PATCH 1/4] " Jani Nikula
  2018-02-14 22:39 ` Rodrigo Vivi
  6 siblings, 0 replies; 15+ messages in thread
From: Patchwork @ 2018-02-10  0:24 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/4] drm/i915: Store gen_mask inside the static device info
URL   : https://patchwork.freedesktop.org/series/38026/
State : failure

== Summary ==

Test kms_setmode:
        Subgroup basic:
                fail       -> PASS       (shard-hsw) fdo#99912
Test kms_flip:
        Subgroup plain-flip-fb-recreate-interruptible:
                pass       -> FAIL       (shard-hsw) fdo#100368
Test perf_pmu:
        Subgroup rc6:
                skip       -> PASS       (shard-hsw)
Test kms_plane:
        Subgroup plane-panning-bottom-right-suspend-pipe-a-planes:
                skip       -> PASS       (shard-snb) fdo#102365
        Subgroup plane-panning-bottom-right-suspend-pipe-c-planes:
                incomplete -> PASS       (shard-hsw) fdo#103375
Test kms_cursor_legacy:
        Subgroup flip-vs-cursor-varying-size:
                fail       -> PASS       (shard-hsw) fdo#102670
Test kms_vblank:
        Subgroup pipe-b-accuracy-idle:
                fail       -> PASS       (shard-snb)
                pass       -> FAIL       (shard-apl)

fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#102365 https://bugs.freedesktop.org/show_bug.cgi?id=102365
fdo#103375 https://bugs.freedesktop.org/show_bug.cgi?id=103375
fdo#102670 https://bugs.freedesktop.org/show_bug.cgi?id=102670

shard-apl        total:3439 pass:1784 dwarn:1   dfail:0   fail:21  skip:1632 time:12457s
shard-hsw        total:3444 pass:1761 dwarn:1   dfail:0   fail:10  skip:1671 time:11859s
shard-snb        total:3444 pass:1351 dwarn:1   dfail:0   fail:10  skip:2082 time:6621s
Blacklisted hosts:
shard-kbl        total:3444 pass:1913 dwarn:2   dfail:0   fail:23  skip:1506 time:9640s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7973/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 4/4] drm/i915: Disable dynamic setup of device_info->num_rings
  2018-02-09 23:46   ` Oscar Mateo
@ 2018-02-10  8:39     ` Chris Wilson
  2018-02-12 17:48       ` Ville Syrjälä
  0 siblings, 1 reply; 15+ messages in thread
From: Chris Wilson @ 2018-02-10  8:39 UTC (permalink / raw)
  To: Oscar Mateo, intel-gfx

Quoting Oscar Mateo (2018-02-09 23:46:31)
> 
> On 02/09/2018 02:35 PM, Chris Wilson wrote:
> > In order to allow the compiler to use a known constant number of
> > available engines, disable modification of intel_device_static_info
> > during engine bring up. Instead of trying to gracefully hide the broken
> > setup, error out -- in theory, this should be caught during power on.
> 
> We are about to have a case for dynamic number of available engines. 
> It's one of the ICL patches:
> 
> drm/i915/icl: Check for fused-off VDBOX and VEBOX instances
> 
> intel_device_runtime_info as well?

Hmm, ring_mask is more widely used than I was expecting. I think we want
both, static_info if we ever think we can benefit from single-platform
LTO of the engines, but whether to use runtime_info or i915->gt.engine_mask
(and move the engine maps to i915->gt as well).

Advantage of runtime_info, centralised place for debugging.
Disadvantage of runtime_info, centralised place far from code.

Maybe we don't need to say everything is inside runtime_info (just
anything that doesn't fit elsewhere?), but use the hooks for debugging?
Maybe having a central runtime_info is simply a bad idea?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 1/4] drm/i915: Store gen_mask inside the static device info
  2018-02-09 22:35 [PATCH 1/4] drm/i915: Store gen_mask inside the static device info Chris Wilson
                   ` (4 preceding siblings ...)
  2018-02-10  0:24 ` ✗ Fi.CI.IGT: failure " Patchwork
@ 2018-02-12  9:58 ` Jani Nikula
  2018-02-12 10:05   ` Chris Wilson
  2018-02-14 22:39 ` Rodrigo Vivi
  6 siblings, 1 reply; 15+ messages in thread
From: Jani Nikula @ 2018-02-12  9:58 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx

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

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

* Re: [PATCH 1/4] drm/i915: Store gen_mask inside the static device info
  2018-02-12  9:58 ` [PATCH 1/4] " Jani Nikula
@ 2018-02-12 10:05   ` Chris Wilson
  0 siblings, 0 replies; 15+ messages in thread
From: Chris Wilson @ 2018-02-12 10:05 UTC (permalink / raw)
  To: Jani Nikula, intel-gfx

Quoting Jani Nikula (2018-02-12 09:58:20)
> 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.

I was working on this being local to the file, but sure :)
 
> 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 know clang emits a warn, and I've vague memories about gcc warning for
large constants. What I did contemplate is that given the macro, we
could use BUILD_BUG_ON_OR_ZERO() and incorporate the overflow checks
into the macro.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 4/4] drm/i915: Disable dynamic setup of device_info->num_rings
  2018-02-10  8:39     ` Chris Wilson
@ 2018-02-12 17:48       ` Ville Syrjälä
  0 siblings, 0 replies; 15+ messages in thread
From: Ville Syrjälä @ 2018-02-12 17:48 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

On Sat, Feb 10, 2018 at 08:39:08AM +0000, Chris Wilson wrote:
> Quoting Oscar Mateo (2018-02-09 23:46:31)
> > 
> > On 02/09/2018 02:35 PM, Chris Wilson wrote:
> > > In order to allow the compiler to use a known constant number of
> > > available engines, disable modification of intel_device_static_info
> > > during engine bring up. Instead of trying to gracefully hide the broken
> > > setup, error out -- in theory, this should be caught during power on.
> > 
> > We are about to have a case for dynamic number of available engines. 
> > It's one of the ICL patches:
> > 
> > drm/i915/icl: Check for fused-off VDBOX and VEBOX instances
> > 
> > intel_device_runtime_info as well?
> 
> Hmm, ring_mask is more widely used than I was expecting. I think we want
> both, static_info if we ever think we can benefit from single-platform
> LTO of the engines, but whether to use runtime_info or i915->gt.engine_mask
> (and move the engine maps to i915->gt as well).
> 
> Advantage of runtime_info, centralised place for debugging.
> Disadvantage of runtime_info, centralised place far from code.
> 
> Maybe we don't need to say everything is inside runtime_info (just
> anything that doesn't fit elsewhere?), but use the hooks for debugging?
> Maybe having a central runtime_info is simply a bad idea?

Yeah, I don't like the "far from the code" aspect of runtime_info
(or even the static info in some cases).

Another counter argument is perhaps that people are more likely to
update the info for new platforms if it's in a central location, as
opposed having to trawl the entire codebase.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 1/4] drm/i915: Store gen_mask inside the static device info
  2018-02-09 22:35 [PATCH 1/4] drm/i915: Store gen_mask inside the static device info Chris Wilson
                   ` (5 preceding siblings ...)
  2018-02-12  9:58 ` [PATCH 1/4] " Jani Nikula
@ 2018-02-14 22:39 ` Rodrigo Vivi
  6 siblings, 0 replies; 15+ messages in thread
From: Rodrigo Vivi @ 2018-02-14 22:39 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx

Chris Wilson <chris@chris-wilson.co.uk> writes:

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

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> ---
>  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)
> +
>  #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
>  
> -- 
> 2.16.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 2/4] drm/i915: Always define GEN as part of GENx_FEATURES
  2018-02-09 22:35 ` [PATCH 2/4] drm/i915: Always define GEN as part of GENx_FEATURES Chris Wilson
@ 2018-02-14 22:40   ` Rodrigo Vivi
  0 siblings, 0 replies; 15+ messages in thread
From: Rodrigo Vivi @ 2018-02-14 22:40 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx

Chris Wilson <chris@chris-wilson.co.uk> writes:

> Be consistent and define the device's GEN as part of the GENx_FEATURE.
> It will be overridden by the next gen upon inheriting, as per usual.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> ---
>  drivers/gpu/drm/i915/i915_pci.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> index 3b4516d7f9a4..364d3b41f816 100644
> --- a/drivers/gpu/drm/i915/i915_pci.c
> +++ b/drivers/gpu/drm/i915/i915_pci.c
> @@ -383,6 +383,7 @@ static const struct intel_device_info intel_haswell_gt3_info = {
>  
>  #define GEN8_FEATURES \
>  	G75_FEATURES, \
> +	GEN(8), \
>  	BDW_COLORS, \
>  	.page_sizes = I915_GTT_PAGE_SIZE_4K | \
>  		      I915_GTT_PAGE_SIZE_2M, \
> @@ -393,7 +394,6 @@ static const struct intel_device_info intel_haswell_gt3_info = {
>  
>  #define BDW_PLATFORM \
>  	GEN8_FEATURES, \
> -	GEN(8), \
>  	.platform = INTEL_BROADWELL
>  
>  static const struct intel_device_info intel_broadwell_gt1_info = {
> @@ -452,6 +452,7 @@ static const struct intel_device_info intel_cherryview_info = {
>  
>  #define GEN9_FEATURES \
>  	GEN8_FEATURES, \
> +	GEN(9), \
>  	GEN9_DEFAULT_PAGE_SIZES, \
>  	.has_logical_ring_preemption = 1, \
>  	.has_csr = 1, \
> @@ -461,7 +462,6 @@ static const struct intel_device_info intel_cherryview_info = {
>  
>  #define SKL_PLATFORM \
>  	GEN9_FEATURES, \
> -	GEN(9), \
>  	.platform = INTEL_SKYLAKE
>  
>  static const struct intel_device_info intel_skylake_gt1_info = {
> @@ -535,7 +535,6 @@ static const struct intel_device_info intel_geminilake_info = {
>  
>  #define KBL_PLATFORM \
>  	GEN9_FEATURES, \
> -	GEN(9),  \
>  	.platform = INTEL_KABYLAKE
>  
>  static const struct intel_device_info intel_kabylake_gt1_info = {
> @@ -556,7 +555,6 @@ static const struct intel_device_info intel_kabylake_gt3_info = {
>  
>  #define CFL_PLATFORM \
>  	GEN9_FEATURES, \
> -	GEN(9), \
>  	.platform = INTEL_COFFEELAKE
>  
>  static const struct intel_device_info intel_coffeelake_gt1_info = {
> @@ -577,12 +575,12 @@ static const struct intel_device_info intel_coffeelake_gt3_info = {
>  
>  #define GEN10_FEATURES \
>  	GEN9_FEATURES, \
> +	GEN(10), \
>  	.ddb_size = 1024, \
>  	GLK_COLORS
>  
>  static const struct intel_device_info intel_cannonlake_info = {
>  	GEN10_FEATURES,
> -	GEN(10),
>  	.is_alpha_support = 1,
>  	.platform = INTEL_CANNONLAKE,
>  	.gt = 2,
> -- 
> 2.16.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 3/4] drm/i915: Store platform_mask inside the static device info
  2018-02-09 22:35 ` [PATCH 3/4] drm/i915: Store platform_mask inside the static device info Chris Wilson
@ 2018-02-14 22:45   ` Rodrigo Vivi
  2018-02-15  7:55     ` Chris Wilson
  0 siblings, 1 reply; 15+ messages in thread
From: Rodrigo Vivi @ 2018-02-14 22:45 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx

Chris Wilson <chris@chris-wilson.co.uk> writes:

> Rather than deriving the platform_mask from the
> intel_device_static_info->platform at runtime, prefill it in the static
> data.
>
>  baseline.ko drivers/gpu/drm/i915/i915.ko
> add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-20 (-20)
> Function                                     old     new   delta
> i915_driver_load                            5027    5007     -20
> Total: Before=1331200, After=1331180, chg -0.00%
>
> 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 | 69 ++++++++++++++++++++++-------------------
>  2 files changed, 37 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 90f4adbbff28..a81be8506797 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -899,8 +899,6 @@ static int i915_driver_init_early(struct drm_i915_private *dev_priv,
>  
>  	BUILD_BUG_ON(INTEL_MAX_PLATFORMS >
>  		     sizeof(device_info->platform_mask) * BITS_PER_BYTE);
> -	device_info->platform_mask = BIT(device_info->platform);
> -
>  	BUG_ON(device_info->gen > sizeof(device_info->gen_mask) * BITS_PER_BYTE);
>  	spin_lock_init(&dev_priv->irq_lock);
>  	spin_lock_init(&dev_priv->gpu_error.lock);
> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> index 364d3b41f816..a10404686710 100644
> --- a/drivers/gpu/drm/i915/i915_pci.c
> +++ b/drivers/gpu/drm/i915/i915_pci.c
> @@ -30,6 +30,7 @@
>  #include "i915_selftest.h"
>  
>  #define GEN(x) .gen = (x), .gen_mask = BIT((x) - 1)
> +#define PLATFORM(x) .platform = (x), .platform_mask = BIT(x)
>  
>  #define GEN_DEFAULT_PIPEOFFSETS \
>  	.pipe_offsets = { PIPE_A_OFFSET, PIPE_B_OFFSET, \
> @@ -79,19 +80,20 @@
>  
>  static const struct intel_device_info intel_i830_info = {
>  	GEN2_FEATURES,
> -	.platform = INTEL_I830,
> +	PLATFORM(INTEL_I830),
>  	.is_mobile = 1, .cursor_needs_physical = 1,
>  	.num_pipes = 2, /* legal, last one wins */
>  };
>  
>  static const struct intel_device_info intel_i845g_info = {
>  	GEN2_FEATURES,
> -	.platform = INTEL_I845G,
> +	PLATFORM(INTEL_I845G),
>  };
>  
>  static const struct intel_device_info intel_i85x_info = {
>  	GEN2_FEATURES,
> -	.platform = INTEL_I85X, .is_mobile = 1,
> +	PLATFORM(INTEL_I85X),
> +	.is_mobile = 1,
>  	.num_pipes = 2, /* legal, last one wins */
>  	.cursor_needs_physical = 1,
>  	.has_fbc = 1,
> @@ -99,7 +101,7 @@ static const struct intel_device_info intel_i85x_info = {
>  
>  static const struct intel_device_info intel_i865g_info = {
>  	GEN2_FEATURES,
> -	.platform = INTEL_I865G,
> +	PLATFORM(INTEL_I865G),
>  };
>  
>  #define GEN3_FEATURES \
> @@ -114,7 +116,8 @@ static const struct intel_device_info intel_i865g_info = {
>  
>  static const struct intel_device_info intel_i915g_info = {
>  	GEN3_FEATURES,
> -	.platform = INTEL_I915G, .cursor_needs_physical = 1,
> +	PLATFORM(INTEL_I915G),
> +	.cursor_needs_physical = 1,
>  	.has_overlay = 1, .overlay_needs_physical = 1,
>  	.hws_needs_physical = 1,
>  	.unfenced_needs_alignment = 1,
> @@ -122,7 +125,7 @@ static const struct intel_device_info intel_i915g_info = {
>  
>  static const struct intel_device_info intel_i915gm_info = {
>  	GEN3_FEATURES,
> -	.platform = INTEL_I915GM,
> +	PLATFORM(INTEL_I915GM),
>  	.is_mobile = 1,
>  	.cursor_needs_physical = 1,
>  	.has_overlay = 1, .overlay_needs_physical = 1,
> @@ -134,7 +137,7 @@ static const struct intel_device_info intel_i915gm_info = {
>  
>  static const struct intel_device_info intel_i945g_info = {
>  	GEN3_FEATURES,
> -	.platform = INTEL_I945G,
> +	PLATFORM(INTEL_I945G),
>  	.has_hotplug = 1, .cursor_needs_physical = 1,
>  	.has_overlay = 1, .overlay_needs_physical = 1,
>  	.hws_needs_physical = 1,
> @@ -143,7 +146,8 @@ static const struct intel_device_info intel_i945g_info = {
>  
>  static const struct intel_device_info intel_i945gm_info = {
>  	GEN3_FEATURES,
> -	.platform = INTEL_I945GM, .is_mobile = 1,
> +	PLATFORM(INTEL_I945GM),
> +	.is_mobile = 1,
>  	.has_hotplug = 1, .cursor_needs_physical = 1,
>  	.has_overlay = 1, .overlay_needs_physical = 1,
>  	.supports_tv = 1,
> @@ -154,14 +158,15 @@ static const struct intel_device_info intel_i945gm_info = {
>  
>  static const struct intel_device_info intel_g33_info = {
>  	GEN3_FEATURES,
> -	.platform = INTEL_G33,
> +	PLATFORM(INTEL_G33),
>  	.has_hotplug = 1,
>  	.has_overlay = 1,
>  };
>  
>  static const struct intel_device_info intel_pineview_info = {
>  	GEN3_FEATURES,
> -	.platform = INTEL_PINEVIEW, .is_mobile = 1,
> +	PLATFORM(INTEL_PINEVIEW),
> +	.is_mobile = 1,
>  	.has_hotplug = 1,
>  	.has_overlay = 1,
>  };
> @@ -179,7 +184,7 @@ static const struct intel_device_info intel_pineview_info = {
>  
>  static const struct intel_device_info intel_i965g_info = {
>  	GEN4_FEATURES,
> -	.platform = INTEL_I965G,
> +	PLATFORM(INTEL_I965G),
>  	.has_overlay = 1,
>  	.hws_needs_physical = 1,
>  	.has_snoop = false,
> @@ -187,7 +192,7 @@ static const struct intel_device_info intel_i965g_info = {
>  
>  static const struct intel_device_info intel_i965gm_info = {
>  	GEN4_FEATURES,
> -	.platform = INTEL_I965GM,
> +	PLATFORM(INTEL_I965GM),
>  	.is_mobile = 1, .has_fbc = 1,
>  	.has_overlay = 1,
>  	.supports_tv = 1,
> @@ -197,13 +202,13 @@ static const struct intel_device_info intel_i965gm_info = {
>  
>  static const struct intel_device_info intel_g45_info = {
>  	GEN4_FEATURES,
> -	.platform = INTEL_G45,
> +	PLATFORM(INTEL_G45),
>  	.ring_mask = RENDER_RING | BSD_RING,
>  };
>  
>  static const struct intel_device_info intel_gm45_info = {
>  	GEN4_FEATURES,
> -	.platform = INTEL_GM45,
> +	PLATFORM(INTEL_GM45),
>  	.is_mobile = 1, .has_fbc = 1,
>  	.supports_tv = 1,
>  	.ring_mask = RENDER_RING | BSD_RING,
> @@ -223,12 +228,12 @@ static const struct intel_device_info intel_gm45_info = {
>  
>  static const struct intel_device_info intel_ironlake_d_info = {
>  	GEN5_FEATURES,
> -	.platform = INTEL_IRONLAKE,
> +	PLATFORM(INTEL_IRONLAKE),
>  };
>  
>  static const struct intel_device_info intel_ironlake_m_info = {
>  	GEN5_FEATURES,
> -	.platform = INTEL_IRONLAKE,
> +	PLATFORM(INTEL_IRONLAKE),
>  	.is_mobile = 1, .has_fbc = 1,
>  };
>  
> @@ -248,7 +253,7 @@ static const struct intel_device_info intel_ironlake_m_info = {
>  
>  #define SNB_D_PLATFORM \
>  	GEN6_FEATURES, \
> -	.platform = INTEL_SANDYBRIDGE
> +	PLATFORM(INTEL_SANDYBRIDGE)
>  
>  static const struct intel_device_info intel_sandybridge_d_gt1_info = {
>  	SNB_D_PLATFORM,
> @@ -262,7 +267,7 @@ static const struct intel_device_info intel_sandybridge_d_gt2_info = {
>  
>  #define SNB_M_PLATFORM \
>  	GEN6_FEATURES, \
> -	.platform = INTEL_SANDYBRIDGE, \
> +	PLATFORM(INTEL_SANDYBRIDGE), \
>  	.is_mobile = 1
>  
>  
> @@ -293,7 +298,7 @@ static const struct intel_device_info intel_sandybridge_m_gt2_info = {
>  
>  #define IVB_D_PLATFORM \
>  	GEN7_FEATURES, \
> -	.platform = INTEL_IVYBRIDGE, \
> +	PLATFORM(INTEL_IVYBRIDGE), \
>  	.has_l3_dpf = 1
>  
>  static const struct intel_device_info intel_ivybridge_d_gt1_info = {
> @@ -308,7 +313,7 @@ static const struct intel_device_info intel_ivybridge_d_gt2_info = {
>  
>  #define IVB_M_PLATFORM \
>  	GEN7_FEATURES, \
> -	.platform = INTEL_IVYBRIDGE, \
> +	PLATFORM(INTEL_IVYBRIDGE), \
>  	.is_mobile = 1, \
>  	.has_l3_dpf = 1
>  
> @@ -324,14 +329,14 @@ static const struct intel_device_info intel_ivybridge_m_gt2_info = {
>  
>  static const struct intel_device_info intel_ivybridge_q_info = {
>  	GEN7_FEATURES,
> -	.platform = INTEL_IVYBRIDGE,
> +	PLATFORM(INTEL_IVYBRIDGE),
>  	.gt = 2,
>  	.num_pipes = 0, /* legal, last one wins */
>  	.has_l3_dpf = 1,
>  };
>  
>  static const struct intel_device_info intel_valleyview_info = {
> -	.platform = INTEL_VALLEYVIEW,
> +	PLATFORM(INTEL_VALLEYVIEW),
>  	GEN(7),
>  	.is_lp = 1,
>  	.num_pipes = 2,
> @@ -363,7 +368,7 @@ static const struct intel_device_info intel_valleyview_info = {
>  
>  #define HSW_PLATFORM \
>  	G75_FEATURES, \
> -	.platform = INTEL_HASWELL, \
> +	PLATFORM(INTEL_HASWELL), \
>  	.has_l3_dpf = 1
>  
>  static const struct intel_device_info intel_haswell_gt1_info = {
> @@ -394,7 +399,7 @@ static const struct intel_device_info intel_haswell_gt3_info = {
>  
>  #define BDW_PLATFORM \
>  	GEN8_FEATURES, \
> -	.platform = INTEL_BROADWELL
> +	PLATFORM(INTEL_BROADWELL)
>  
>  static const struct intel_device_info intel_broadwell_gt1_info = {
>  	BDW_PLATFORM,
> @@ -421,12 +426,12 @@ static const struct intel_device_info intel_broadwell_gt3_info = {
>  };
>  
>  static const struct intel_device_info intel_cherryview_info = {
> +	PLATFORM(INTEL_CHERRYVIEW),
>  	GEN(8),

The order seems a bit strange... here it comes before the GEN
and in other cases it comes after the GEN_FEATURES which includes GEN...

probably we could make PLATFORM the very first thing on any case
but up to you...

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

>  	.num_pipes = 3,
>  	.has_hotplug = 1,
>  	.is_lp = 1,
>  	.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING,
> -	.platform = INTEL_CHERRYVIEW,
>  	.has_64bit_reloc = 1,
>  	.has_psr = 1,
>  	.has_runtime_pm = 1,
> @@ -462,7 +467,7 @@ static const struct intel_device_info intel_cherryview_info = {
>  
>  #define SKL_PLATFORM \
>  	GEN9_FEATURES, \
> -	.platform = INTEL_SKYLAKE
> +	PLATFORM(INTEL_SKYLAKE)
>  
>  static const struct intel_device_info intel_skylake_gt1_info = {
>  	SKL_PLATFORM,
> @@ -522,20 +527,20 @@ static const struct intel_device_info intel_skylake_gt4_info = {
>  
>  static const struct intel_device_info intel_broxton_info = {
>  	GEN9_LP_FEATURES,
> -	.platform = INTEL_BROXTON,
> +	PLATFORM(INTEL_BROXTON),
>  	.ddb_size = 512,
>  };
>  
>  static const struct intel_device_info intel_geminilake_info = {
>  	GEN9_LP_FEATURES,
> -	.platform = INTEL_GEMINILAKE,
> +	PLATFORM(INTEL_GEMINILAKE),
>  	.ddb_size = 1024,
>  	GLK_COLORS,
>  };
>  
>  #define KBL_PLATFORM \
>  	GEN9_FEATURES, \
> -	.platform = INTEL_KABYLAKE
> +	PLATFORM(INTEL_KABYLAKE)
>  
>  static const struct intel_device_info intel_kabylake_gt1_info = {
>  	KBL_PLATFORM,
> @@ -555,7 +560,7 @@ static const struct intel_device_info intel_kabylake_gt3_info = {
>  
>  #define CFL_PLATFORM \
>  	GEN9_FEATURES, \
> -	.platform = INTEL_COFFEELAKE
> +	PLATFORM(INTEL_COFFEELAKE)
>  
>  static const struct intel_device_info intel_coffeelake_gt1_info = {
>  	CFL_PLATFORM,
> @@ -581,8 +586,8 @@ static const struct intel_device_info intel_coffeelake_gt3_info = {
>  
>  static const struct intel_device_info intel_cannonlake_info = {
>  	GEN10_FEATURES,
> +	PLATFORM(INTEL_CANNONLAKE),
>  	.is_alpha_support = 1,
> -	.platform = INTEL_CANNONLAKE,
>  	.gt = 2,
>  };
>  
> @@ -594,7 +599,7 @@ static const struct intel_device_info intel_cannonlake_info = {
>  
>  static const struct intel_device_info intel_icelake_11_info = {
>  	GEN11_FEATURES,
> -	.platform = INTEL_ICELAKE,
> +	PLATFORM(INTEL_ICELAKE),
>  	.is_alpha_support = 1,
>  	.has_resource_streamer = 0,
>  };
> -- 
> 2.16.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 3/4] drm/i915: Store platform_mask inside the static device info
  2018-02-14 22:45   ` Rodrigo Vivi
@ 2018-02-15  7:55     ` Chris Wilson
  0 siblings, 0 replies; 15+ messages in thread
From: Chris Wilson @ 2018-02-15  7:55 UTC (permalink / raw)
  To: Rodrigo Vivi, intel-gfx

Quoting Rodrigo Vivi (2018-02-14 22:45:33)
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > Rather than deriving the platform_mask from the
> > intel_device_static_info->platform at runtime, prefill it in the static
> > data.
> >
> >  baseline.ko drivers/gpu/drm/i915/i915.ko
> > add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-20 (-20)
> > Function                                     old     new   delta
> > i915_driver_load                            5027    5007     -20
> > Total: Before=1331200, After=1331180, chg -0.00%
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> >  static const struct intel_device_info intel_cherryview_info = {
> > +     PLATFORM(INTEL_CHERRYVIEW),
> >       GEN(8),
> 
> The order seems a bit strange... here it comes before the GEN
> and in other cases it comes after the GEN_FEATURES which includes GEN...
> 
> probably we could make PLATFORM the very first thing on any case
> but up to you...

I was trying to have PLATFORM be the first non-inherited value in each
device_info. So the pattern I had in mind was 
	[GENx_FEATURES,]
	PLATFORM(foo)
I was tempted to add feature defines for the atoms as well just so it
looked more consistent. Later on it was looking more like
	static const PLATFORM_INFO(cherryview) = {
		PLATFORM(INTEL_CHERRYVIEW),
		...
	};
where the repetition is more obvious (and easier to check)
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

end of thread, other threads:[~2018-02-15  7:55 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-09 22:35 [PATCH 1/4] drm/i915: Store gen_mask inside the static device info Chris Wilson
2018-02-09 22:35 ` [PATCH 2/4] drm/i915: Always define GEN as part of GENx_FEATURES Chris Wilson
2018-02-14 22:40   ` Rodrigo Vivi
2018-02-09 22:35 ` [PATCH 3/4] drm/i915: Store platform_mask inside the static device info Chris Wilson
2018-02-14 22:45   ` Rodrigo Vivi
2018-02-15  7:55     ` Chris Wilson
2018-02-09 22:35 ` [PATCH 4/4] drm/i915: Disable dynamic setup of device_info->num_rings Chris Wilson
2018-02-09 23:46   ` Oscar Mateo
2018-02-10  8:39     ` Chris Wilson
2018-02-12 17:48       ` Ville Syrjälä
2018-02-09 23:13 ` ✓ Fi.CI.BAT: success for series starting with [1/4] drm/i915: Store gen_mask inside the static device info Patchwork
2018-02-10  0:24 ` ✗ Fi.CI.IGT: failure " Patchwork
2018-02-12  9:58 ` [PATCH 1/4] " Jani Nikula
2018-02-12 10:05   ` Chris Wilson
2018-02-14 22:39 ` Rodrigo Vivi

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.