[RFC 0/8] re-organize a bit gen10 and gen11

[RFC 0/8] re-organize a bit gen10 and gen11

[Rodrigo Vivi]

The goal off this series is to start the trend of prefering
gen number and feature checks instead of hardcoding
individual platform names everywhere.

I tried to use coccinelle but I failed badly because
there are places where it makes sense to have platform
codenames still. On sort part it was many different cases
that manually changing got easier than creating rules
(at least for me).

Please let me know what you think.

Thanks in advance,
Rodrigo.

Rodrigo Vivi (8):
  drm/i915: Make number of ddi ports explicit.
  drm/i915: Use the ddi_ports info to kill ugly CNL_WITH_PORT_F.
  drm/i915/gen10: Prefer gen number than platform codename.
  drm/i915: Group gen 10 display.
  drm/i915/gen11+: Prefer gen over platform codename.
  drm/i915: Consolidate cdclk hooks.
  drm/i915: Sort platform if cases from newer-to-older.
  drm/i915: Simplify intel_has_sagv function.

 drivers/gpu/drm/i915/i915_drv.c          | 10 +++
 drivers/gpu/drm/i915/i915_drv.h          |  8 +-
 drivers/gpu/drm/i915/i915_irq.c          |  5 +-
 drivers/gpu/drm/i915/i915_pci.c          |  5 +-
 drivers/gpu/drm/i915/intel_cdclk.c       | 97 +++++++++++-------------
 drivers/gpu/drm/i915/intel_color.c       |  2 +-
 drivers/gpu/drm/i915/intel_ddi.c         | 48 ++++++------
 drivers/gpu/drm/i915/intel_device_info.h |  2 +-
 drivers/gpu/drm/i915/intel_display.c     | 16 ++--
 drivers/gpu/drm/i915/intel_dp.c          | 26 ++++---
 drivers/gpu/drm/i915/intel_dpll_mgr.c    |  4 +-
 drivers/gpu/drm/i915/intel_hdmi.c        |  2 +-
 drivers/gpu/drm/i915/intel_hotplug.c     |  2 +-
 drivers/gpu/drm/i915/intel_mocs.c        |  3 +-
 drivers/gpu/drm/i915/intel_pm.c          | 12 +--
 drivers/gpu/drm/i915/intel_runtime_pm.c  | 26 +++----
 drivers/gpu/drm/i915/intel_sprite.c      |  2 +-
 drivers/gpu/drm/i915/intel_uncore.c      |  2 +-
 18 files changed, 136 insertions(+), 136 deletions(-)

-- 
2.19.1

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

[RFC 1/8] drm/i915: Make number of ddi ports explicit.

[Rodrigo Vivi]

Instead of a simple bool that shows if we have ddi ports
or not, let's highlight the number of ddi ports.

So we can use this information to determine the code
path instead of using platforms codenames.

Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h          | 2 +-
 drivers/gpu/drm/i915/i915_pci.c          | 5 +++--
 drivers/gpu/drm/i915/intel_device_info.h | 2 +-
 3 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 3017ef037fed..7ad232849268 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2647,7 +2647,7 @@ intel_info(const struct drm_i915_private *dev_priv)
 
 #define HAS_DP_MST(dev_priv)	((dev_priv)->info.has_dp_mst)
 
-#define HAS_DDI(dev_priv)		 ((dev_priv)->info.has_ddi)
+#define HAS_DDI(dev_priv)		 ((dev_priv)->info.ddi_ports > 0)
 #define HAS_FPGA_DBG_UNCLAIMED(dev_priv) ((dev_priv)->info.has_fpga_dbg)
 #define HAS_PSR(dev_priv)		 ((dev_priv)->info.has_psr)
 
diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index 0a05cc7ace14..0427486f63d0 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -363,7 +363,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, \
-	.has_ddi = 1, \
+	.ddi_ports = 5, \
 	.has_fpga_dbg = 1, \
 	.has_psr = 1, \
 	.has_dp_mst = 1, \
@@ -505,7 +505,7 @@ static const struct intel_device_info intel_skylake_gt4_info = {
 	.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING, \
 	.num_pipes = 3, \
 	.has_64bit_reloc = 1, \
-	.has_ddi = 1, \
+	.ddi_ports = 3, \
 	.has_fpga_dbg = 1, \
 	.has_fbc = 1, \
 	.has_psr = 1, \
@@ -596,6 +596,7 @@ static const struct intel_device_info intel_cannonlake_info = {
 #define GEN11_FEATURES \
 	GEN10_FEATURES, \
 	GEN(11), \
+	.ddi_ports = 6, \
 	.ddb_size = 2048, \
 	.has_logical_ring_elsq = 1
 
diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
index af7002640cdf..1be941222ed0 100644
--- a/drivers/gpu/drm/i915/intel_device_info.h
+++ b/drivers/gpu/drm/i915/intel_device_info.h
@@ -90,7 +90,6 @@ enum intel_ppgtt {
 	/* Keep has_* in alphabetical order */ \
 	func(has_64bit_reloc); \
 	func(has_csr); \
-	func(has_ddi); \
 	func(has_dp_mst); \
 	func(has_reset_engine); \
 	func(has_fbc); \
@@ -165,6 +164,7 @@ struct intel_device_info {
 
 	u32 display_mmio_offset;
 
+	u8 ddi_ports;
 	u8 num_pipes;
 	u8 num_sprites[I915_MAX_PIPES];
 	u8 num_scalers[I915_MAX_PIPES];
-- 
2.19.1

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

[RFC 2/8] drm/i915: Use the ddi_ports info to kill ugly CNL_WITH_PORT_F.

[Rodrigo Vivi]

Now that we have the number of ddi ports information available
let's use it instead of that ugly platform macro.

Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c         | 10 ++++++++++
 drivers/gpu/drm/i915/i915_drv.h         |  2 --
 drivers/gpu/drm/i915/i915_irq.c         |  5 ++---
 drivers/gpu/drm/i915/intel_dp.c         |  2 +-
 drivers/gpu/drm/i915/intel_hotplug.c    |  2 +-
 drivers/gpu/drm/i915/intel_runtime_pm.c |  2 +-
 6 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index baac35f698f9..83ab325c6675 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -239,6 +239,14 @@ intel_virt_detect_pch(const struct drm_i915_private *dev_priv)
 	return id;
 }
 
+static void intel_adjust_skus_differences(struct drm_i915_private *dev_priv)
+{
+	/* Some Cannonlake SKUs have Port F */
+	if (IS_CANNONLAKE(dev_priv) &&
+	    (INTEL_DEVID(dev_priv) & 0x0004) == 0x0004)
+		mkwrite_device_info(dev_priv)->ddi_ports = 6;
+}
+
 static void intel_detect_pch(struct drm_i915_private *dev_priv)
 {
 	struct pci_dev *pch = NULL;
@@ -904,6 +912,8 @@ static int i915_driver_init_early(struct drm_i915_private *dev_priv)
 	if (ret < 0)
 		goto err_workqueues;
 
+	intel_adjust_skus_differences(dev_priv);
+
 	/* This must be called before any calls to HAS_PCH_* */
 	intel_detect_pch(dev_priv);
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 7ad232849268..99e42df79ed8 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2485,8 +2485,6 @@ intel_info(const struct drm_i915_private *dev_priv)
 				 (dev_priv)->info.gt == 2)
 #define IS_CFL_GT3(dev_priv)	(IS_COFFEELAKE(dev_priv) && \
 				 (dev_priv)->info.gt == 3)
-#define IS_CNL_WITH_PORT_F(dev_priv)   (IS_CANNONLAKE(dev_priv) && \
-					(INTEL_DEVID(dev_priv) & 0x0004) == 0x0004)
 
 #define IS_ALPHA_SUPPORT(intel_info) ((intel_info)->is_alpha_support)
 
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 5d1f53723388..63d676de3840 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2782,8 +2782,7 @@ gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl)
 			if (INTEL_GEN(dev_priv) >= 11)
 				tmp_mask |= ICL_AUX_CHANNEL_E;
 
-			if (IS_CNL_WITH_PORT_F(dev_priv) ||
-			    INTEL_GEN(dev_priv) >= 11)
+			if (INTEL_INFO(dev_priv)->ddi_ports >= 6)
 				tmp_mask |= CNL_AUX_CHANNEL_F;
 
 			if (iir & tmp_mask) {
@@ -4220,7 +4219,7 @@ static void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv)
 	if (INTEL_GEN(dev_priv) >= 11)
 		de_port_masked |= ICL_AUX_CHANNEL_E;
 
-	if (IS_CNL_WITH_PORT_F(dev_priv) || INTEL_GEN(dev_priv) >= 11)
+	if (INTEL_INFO(dev_priv)->ddi_ports >= 6)
 		de_port_masked |= CNL_AUX_CHANNEL_F;
 
 	de_pipe_enables = de_pipe_masked | GEN8_PIPE_VBLANK |
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 3384a9bbdafd..0ea0414ccef4 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -402,7 +402,7 @@ static int cnl_max_source_rate(struct intel_dp *intel_dp)
 		return 540000;
 
 	/* For this SKU 8.1G is supported in all ports */
-	if (IS_CNL_WITH_PORT_F(dev_priv))
+	if (INTEL_INFO(dev_priv)->ddi_ports == 6)
 		return 810000;
 
 	/* For other SKUs, max rate on ports A and D is 5.4G */
diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c
index 648a13c6043c..05d1faf77048 100644
--- a/drivers/gpu/drm/i915/intel_hotplug.c
+++ b/drivers/gpu/drm/i915/intel_hotplug.c
@@ -101,7 +101,7 @@ enum hpd_pin intel_hpd_pin_default(struct drm_i915_private *dev_priv,
 	case PORT_E:
 		return HPD_PORT_E;
 	case PORT_F:
-		if (IS_CNL_WITH_PORT_F(dev_priv))
+		if (IS_GEN10(dev_priv))
 			return HPD_PORT_E;
 		return HPD_PORT_F;
 	default:
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 31a49bdcf193..80e14be11279 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -3099,7 +3099,7 @@ int intel_power_domains_init(struct drm_i915_private *dev_priv)
 		 * timeouts, lets remove them from the list
 		 * for the SKUs without port F.
 		 */
-		if (!IS_CNL_WITH_PORT_F(dev_priv))
+		if (INTEL_INFO(dev_priv)->ddi_ports == 5)
 			power_domains->power_well_count -= 2;
 
 	} else if (IS_BROXTON(dev_priv)) {
-- 
2.19.1

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

[RFC 3/8] drm/i915/gen10: Prefer gen number than platform codename.

[Rodrigo Vivi]

Whenever possible let's move towards preferring gen number
and or features instead of hard coding platform codename
everywhere.

Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h         |  2 +-
 drivers/gpu/drm/i915/intel_cdclk.c      |  6 +++---
 drivers/gpu/drm/i915/intel_ddi.c        | 20 ++++++++++----------
 drivers/gpu/drm/i915/intel_display.c    |  4 ++--
 drivers/gpu/drm/i915/intel_dpll_mgr.c   |  2 +-
 drivers/gpu/drm/i915/intel_mocs.c       |  2 +-
 drivers/gpu/drm/i915/intel_pm.c         |  2 +-
 drivers/gpu/drm/i915/intel_runtime_pm.c |  8 ++++----
 8 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 99e42df79ed8..d19b38ef145b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2620,7 +2620,7 @@ intel_info(const struct drm_i915_private *dev_priv)
 
 /* WaRsDisableCoarsePowerGating:skl,cnl */
 #define NEEDS_WaRsDisableCoarsePowerGating(dev_priv) \
-	(IS_CANNONLAKE(dev_priv) || \
+	(IS_GEN10(dev_priv) || \
 	 IS_SKL_GT3(dev_priv) || IS_SKL_GT4(dev_priv))
 
 #define HAS_GMBUS_IRQ(dev_priv) (INTEL_GEN(dev_priv) >= 4)
diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
index 29075c763428..4aa6500604cc 100644
--- a/drivers/gpu/drm/i915/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/intel_cdclk.c
@@ -2577,7 +2577,7 @@ void intel_update_max_cdclk(struct drm_i915_private *dev_priv)
 			dev_priv->max_cdclk_freq = 648000;
 		else
 			dev_priv->max_cdclk_freq = 652800;
-	} else if (IS_CANNONLAKE(dev_priv)) {
+	} else if (IS_GEN10(dev_priv)) {
 		dev_priv->max_cdclk_freq = 528000;
 	} else if (IS_GEN9_BC(dev_priv)) {
 		u32 limit = I915_READ(SKL_DFSM) & SKL_DFSM_CDCLK_LIMIT_MASK;
@@ -2797,7 +2797,7 @@ void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv)
 		dev_priv->display.set_cdclk = skl_set_cdclk;
 		dev_priv->display.modeset_calc_cdclk =
 			skl_modeset_calc_cdclk;
-	} else if (IS_CANNONLAKE(dev_priv)) {
+	} else if (IS_GEN10(dev_priv)) {
 		dev_priv->display.set_cdclk = cnl_set_cdclk;
 		dev_priv->display.modeset_calc_cdclk =
 			cnl_modeset_calc_cdclk;
@@ -2808,7 +2808,7 @@ void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv)
 
 	if (IS_ICELAKE(dev_priv))
 		dev_priv->display.get_cdclk = icl_get_cdclk;
-	else if (IS_CANNONLAKE(dev_priv))
+	else if (IS_GEN10(dev_priv))
 		dev_priv->display.get_cdclk = cnl_get_cdclk;
 	else if (IS_GEN9_BC(dev_priv))
 		dev_priv->display.get_cdclk = skl_get_cdclk;
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 6b9742baa5f2..cd627851f2a5 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -922,7 +922,7 @@ static int intel_ddi_hdmi_level(struct drm_i915_private *dev_priv, enum port por
 		else
 			n_entries = ARRAY_SIZE(icl_mg_phy_ddi_translations);
 		default_entry = n_entries - 1;
-	} else if (IS_CANNONLAKE(dev_priv)) {
+	} else if (IS_GEN10(dev_priv)) {
 		cnl_get_buf_trans_hdmi(dev_priv, &n_entries);
 		default_entry = n_entries - 1;
 	} else if (IS_GEN9_LP(dev_priv)) {
@@ -1743,7 +1743,7 @@ static void intel_ddi_clock_get(struct intel_encoder *encoder,
 		skl_ddi_clock_get(encoder, pipe_config);
 	else if (IS_GEN9_LP(dev_priv))
 		bxt_ddi_clock_get(encoder, pipe_config);
-	else if (IS_CANNONLAKE(dev_priv))
+	else if (IS_GEN10(dev_priv))
 		cnl_ddi_clock_get(encoder, pipe_config);
 	else if (IS_ICELAKE(dev_priv))
 		icl_ddi_clock_get(encoder, pipe_config);
@@ -2247,7 +2247,7 @@ u8 intel_ddi_dp_voltage_max(struct intel_encoder *encoder)
 						&n_entries);
 		else
 			n_entries = ARRAY_SIZE(icl_mg_phy_ddi_translations);
-	} else if (IS_CANNONLAKE(dev_priv)) {
+	} else if (IS_GEN10(dev_priv)) {
 		if (encoder->type == INTEL_OUTPUT_EDP)
 			cnl_get_buf_trans_edp(dev_priv, &n_entries);
 		else
@@ -2719,7 +2719,7 @@ u32 bxt_signal_levels(struct intel_dp *intel_dp)
 	if (IS_ICELAKE(dev_priv))
 		icl_ddi_vswing_sequence(encoder, intel_dp->link_rate,
 					level, encoder->type);
-	else if (IS_CANNONLAKE(dev_priv))
+	else if (IS_GEN10(dev_priv))
 		cnl_ddi_vswing_sequence(encoder, level, encoder->type);
 	else
 		bxt_ddi_vswing_sequence(encoder, level, encoder->type);
@@ -2837,7 +2837,7 @@ static void intel_ddi_clk_select(struct intel_encoder *encoder,
 		if (!intel_port_is_combophy(dev_priv, port))
 			I915_WRITE(DDI_CLK_SEL(port),
 				   icl_pll_to_ddi_pll_sel(encoder, crtc_state));
-	} else if (IS_CANNONLAKE(dev_priv)) {
+	} else if (IS_GEN10(dev_priv)) {
 		/* Configure DPCLKA_CFGCR0 to map the DPLL to the DDI. */
 		val = I915_READ(DPCLKA_CFGCR0);
 		val &= ~DPCLKA_CFGCR0_DDI_CLK_SEL_MASK(port);
@@ -2878,7 +2878,7 @@ static void intel_ddi_clk_disable(struct intel_encoder *encoder)
 	if (IS_ICELAKE(dev_priv)) {
 		if (!intel_port_is_combophy(dev_priv, port))
 			I915_WRITE(DDI_CLK_SEL(port), DDI_CLK_SEL_NONE);
-	} else if (IS_CANNONLAKE(dev_priv)) {
+	} else if (IS_GEN10(dev_priv)) {
 		I915_WRITE(DPCLKA_CFGCR0, I915_READ(DPCLKA_CFGCR0) |
 			   DPCLKA_CFGCR0_DDI_CLK_OFF(port));
 	} else if (IS_GEN9_BC(dev_priv)) {
@@ -2920,7 +2920,7 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
 	if (IS_ICELAKE(dev_priv))
 		icl_ddi_vswing_sequence(encoder, crtc_state->port_clock,
 					level, encoder->type);
-	else if (IS_CANNONLAKE(dev_priv))
+	else if (IS_GEN10(dev_priv))
 		cnl_ddi_vswing_sequence(encoder, level, encoder->type);
 	else if (IS_GEN9_LP(dev_priv))
 		bxt_ddi_vswing_sequence(encoder, level, encoder->type);
@@ -2959,7 +2959,7 @@ static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder,
 	if (IS_ICELAKE(dev_priv))
 		icl_ddi_vswing_sequence(encoder, crtc_state->port_clock,
 					level, INTEL_OUTPUT_HDMI);
-	else if (IS_CANNONLAKE(dev_priv))
+	else if (IS_GEN10(dev_priv))
 		cnl_ddi_vswing_sequence(encoder, level, INTEL_OUTPUT_HDMI);
 	else if (IS_GEN9_LP(dev_priv))
 		bxt_ddi_vswing_sequence(encoder, level, INTEL_OUTPUT_HDMI);
@@ -3373,7 +3373,7 @@ static bool intel_ddi_is_audio_enabled(struct drm_i915_private *dev_priv,
 void intel_ddi_compute_min_voltage_level(struct drm_i915_private *dev_priv,
 					 struct intel_crtc_state *crtc_state)
 {
-	if (IS_CANNONLAKE(dev_priv) && crtc_state->port_clock > 594000)
+	if (IS_GEN10(dev_priv) && crtc_state->port_clock > 594000)
 		crtc_state->min_voltage_level = 2;
 	else if (IS_ICELAKE(dev_priv) && crtc_state->port_clock > 594000)
 		crtc_state->min_voltage_level = 1;
@@ -3742,7 +3742,7 @@ static bool intel_ddi_a_force_4_lanes(struct intel_digital_port *dport)
 	 *             DDI_F what makes DDI_E useless. However for this
 	 *             case let's trust VBT info.
 	 */
-	if (IS_CANNONLAKE(dev_priv) &&
+	if (IS_GEN10(dev_priv) &&
 	    !intel_bios_is_port_present(dev_priv, PORT_E))
 		return true;
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index fc7e3b0bd95c..7b04b8436b6d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5269,7 +5269,7 @@ static bool needs_nv12_wa(struct drm_i915_private *dev_priv,
 		return false;
 
 	if ((INTEL_GEN(dev_priv) == 9 && !IS_GEMINILAKE(dev_priv)) ||
-	    IS_CANNONLAKE(dev_priv))
+	    IS_GEN10(dev_priv))
 		return true;
 
 	return false;
@@ -9498,7 +9498,7 @@ static void haswell_get_ddi_port_state(struct intel_crtc *crtc,
 
 	if (IS_ICELAKE(dev_priv))
 		icelake_get_ddi_pll(dev_priv, port, pipe_config);
-	else if (IS_CANNONLAKE(dev_priv))
+	else if (IS_GEN10(dev_priv))
 		cannonlake_get_ddi_pll(dev_priv, port, pipe_config);
 	else if (IS_GEN9_BC(dev_priv))
 		skylake_get_ddi_pll(dev_priv, port, pipe_config);
diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
index 7bdff5ba58b9..ae92dc97d5aa 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
@@ -3202,7 +3202,7 @@ void intel_shared_dpll_init(struct drm_device *dev)
 
 	if (IS_ICELAKE(dev_priv))
 		dpll_mgr = &icl_pll_mgr;
-	else if (IS_CANNONLAKE(dev_priv))
+	else if (IS_GEN10(dev_priv))
 		dpll_mgr = &cnl_pll_mgr;
 	else if (IS_GEN9_BC(dev_priv))
 		dpll_mgr = &skl_pll_mgr;
diff --git a/drivers/gpu/drm/i915/intel_mocs.c b/drivers/gpu/drm/i915/intel_mocs.c
index 77e9871a8c9a..662eb087bb2e 100644
--- a/drivers/gpu/drm/i915/intel_mocs.c
+++ b/drivers/gpu/drm/i915/intel_mocs.c
@@ -178,7 +178,7 @@ static bool get_mocs_settings(struct drm_i915_private *dev_priv,
 {
 	bool result = false;
 
-	if (IS_GEN9_BC(dev_priv) || IS_CANNONLAKE(dev_priv) ||
+	if (IS_GEN9_BC(dev_priv) || IS_GEN10(dev_priv) ||
 	    IS_ICELAKE(dev_priv)) {
 		table->size  = ARRAY_SIZE(skylake_mocs_table);
 		table->table = skylake_mocs_table;
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 67a4d0735291..7234b2272481 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3613,7 +3613,7 @@ static bool
 intel_has_sagv(struct drm_i915_private *dev_priv)
 {
 	if (IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv) ||
-	    IS_CANNONLAKE(dev_priv) || IS_ICELAKE(dev_priv))
+	    IS_GEN10(dev_priv) || IS_ICELAKE(dev_priv))
 		return true;
 
 	if (IS_SKYLAKE(dev_priv) &&
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 80e14be11279..63f0b1c0bf77 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -387,7 +387,7 @@ static void hsw_power_well_enable(struct drm_i915_private *dev_priv,
 	hsw_wait_for_power_well_enable(dev_priv, power_well);
 
 	/* Display WA #1178: cnl */
-	if (IS_CANNONLAKE(dev_priv) &&
+	if (IS_GEN10(dev_priv) &&
 	    pw_idx >= GLK_PW_CTL_IDX_AUX_B &&
 	    pw_idx <= CNL_PW_CTL_IDX_AUX_F) {
 		val = I915_READ(CNL_AUX_ANAOVRD1(pw_idx));
@@ -3090,7 +3090,7 @@ int intel_power_domains_init(struct drm_i915_private *dev_priv)
 		err = set_power_wells(power_domains, bdw_power_wells);
 	} else if (IS_GEN9_BC(dev_priv)) {
 		err = set_power_wells(power_domains, skl_power_wells);
-	} else if (IS_CANNONLAKE(dev_priv)) {
+	} else if (IS_GEN10(dev_priv)) {
 		err = set_power_wells(power_domains, cnl_power_wells);
 
 		/*
@@ -3769,7 +3769,7 @@ void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool resume)
 
 	if (IS_ICELAKE(dev_priv)) {
 		icl_display_core_init(dev_priv, resume);
-	} else if (IS_CANNONLAKE(dev_priv)) {
+	} else if (IS_GEN10(dev_priv)) {
 		cnl_display_core_init(dev_priv, resume);
 	} else if (IS_GEN9_BC(dev_priv)) {
 		skl_display_core_init(dev_priv, resume);
@@ -3900,7 +3900,7 @@ void intel_power_domains_suspend(struct drm_i915_private *dev_priv,
 
 	if (IS_ICELAKE(dev_priv))
 		icl_display_core_uninit(dev_priv);
-	else if (IS_CANNONLAKE(dev_priv))
+	else if (IS_GEN10(dev_priv))
 		cnl_display_core_uninit(dev_priv);
 	else if (IS_GEN9_BC(dev_priv))
 		skl_display_core_uninit(dev_priv);
-- 
2.19.1

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

[RFC 4/8] drm/i915: Group gen 10 display.

[Rodrigo Vivi]

Continuing with the goal of use less platform codenames:
let's group platforms who has gen10 display.

Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      | 2 ++
 drivers/gpu/drm/i915/intel_cdclk.c   | 2 +-
 drivers/gpu/drm/i915/intel_color.c   | 2 +-
 drivers/gpu/drm/i915/intel_display.c | 4 ++--
 drivers/gpu/drm/i915/intel_sprite.c  | 2 +-
 5 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index d19b38ef145b..6436fedfe561 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2637,6 +2637,8 @@ intel_info(const struct drm_i915_private *dev_priv)
 #define SUPPORTS_TV(dev_priv)		((dev_priv)->info.supports_tv)
 #define I915_HAS_HOTPLUG(dev_priv)	((dev_priv)->info.has_hotplug)
 
+#define HAS_DISPLAY_10(dev_priv) (IS_GEN10(dev_priv) || IS_GEMINILAKE(dev_priv))
+
 #define HAS_FW_BLC(dev_priv) 	(INTEL_GEN(dev_priv) > 2)
 #define HAS_FBC(dev_priv)	((dev_priv)->info.has_fbc)
 #define HAS_CUR_FBC(dev_priv)	(!HAS_GMCH_DISPLAY(dev_priv) && INTEL_GEN(dev_priv) >= 7)
diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
index 4aa6500604cc..b315b70fd49c 100644
--- a/drivers/gpu/drm/i915/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/intel_cdclk.c
@@ -2181,7 +2181,7 @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state)
 	    crtc_state->has_audio &&
 	    crtc_state->port_clock >= 540000 &&
 	    crtc_state->lane_count == 4) {
-		if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv)) {
+		if (HAS_DISPLAY_10(dev_priv)) {
 			/* Display WA #1145: glk,cnl */
 			min_cdclk = max(316800, min_cdclk);
 		} else if (IS_GEN9(dev_priv) || IS_BROADWELL(dev_priv)) {
diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index 5127da286a2b..d5f67b9c9698 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -659,7 +659,7 @@ void intel_color_init(struct drm_crtc *crtc)
 		   IS_BROXTON(dev_priv)) {
 		dev_priv->display.load_csc_matrix = ilk_load_csc_matrix;
 		dev_priv->display.load_luts = broadwell_load_luts;
-	} else if (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) {
+	} else if (HAS_DISPLAY_10(dev_priv)) {
 		dev_priv->display.load_csc_matrix = ilk_load_csc_matrix;
 		dev_priv->display.load_luts = glk_load_luts;
 	} else {
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 7b04b8436b6d..1abf79a4ee91 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5730,7 +5730,7 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
 	intel_crtc->active = true;
 
 	/* Display WA #1180: WaDisableScalarClockGating: glk, cnl */
-	psl_clkgate_wa = (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) &&
+	psl_clkgate_wa = (HAS_DISPLAY_10(dev_priv)) &&
 			 pipe_config->pch_pfit.enabled;
 	if (psl_clkgate_wa)
 		glk_pipe_scaler_clock_gating_wa(dev_priv, pipe, true);
@@ -15565,7 +15565,7 @@ get_encoder_power_domains(struct drm_i915_private *dev_priv)
 static void intel_early_display_was(struct drm_i915_private *dev_priv)
 {
 	/* Display WA #1185 WaDisableDARBFClkGating:cnl,glk */
-	if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
+	if (HAS_DISPLAY_10(dev_priv))
 		I915_WRITE(GEN9_CLKGATE_DIS_0, I915_READ(GEN9_CLKGATE_DIS_0) |
 			   DARBF_GATING_DIS);
 
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 7cd59eee5cad..912ab7b9d89a 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1330,7 +1330,7 @@ static int skl_plane_check_dst_coordinates(const struct intel_crtc_state *crtc_s
 	 * than the cursor ending less than 4 pixels from the left edge of the
 	 * screen may cause FIFO underflow and display corruption.
 	 */
-	if ((IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) &&
+	if (HAS_DISPLAY_10(dev_priv) &&
 	    (crtc_x + crtc_w < 4 || crtc_x > pipe_src_w - 4)) {
 		DRM_DEBUG_KMS("requested plane X %s position %d invalid (valid range %d-%d)\n",
 			      crtc_x + crtc_w < 4 ? "end" : "start",
-- 
2.19.1

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

[RFC 5/8] drm/i915/gen11+: Prefer gen over platform codename.

[Rodrigo Vivi]

Also let's always consider next platform follows
the most recent one. Like we have done for transitioning
gen9 to gen10 and gent10 to gen11.

Let's use same approach for gen11+ and only introduce
changes later as needed.

Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_cdclk.c      |  6 +++---
 drivers/gpu/drm/i915/intel_ddi.c        | 18 +++++++++---------
 drivers/gpu/drm/i915/intel_display.c    |  8 ++++----
 drivers/gpu/drm/i915/intel_dpll_mgr.c   |  2 +-
 drivers/gpu/drm/i915/intel_hdmi.c       |  2 +-
 drivers/gpu/drm/i915/intel_mocs.c       |  3 +--
 drivers/gpu/drm/i915/intel_pm.c         |  4 ++--
 drivers/gpu/drm/i915/intel_runtime_pm.c |  6 +++---
 8 files changed, 24 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
index b315b70fd49c..915e2c93412b 100644
--- a/drivers/gpu/drm/i915/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/intel_cdclk.c
@@ -2572,7 +2572,7 @@ static int intel_compute_max_dotclk(struct drm_i915_private *dev_priv)
  */
 void intel_update_max_cdclk(struct drm_i915_private *dev_priv)
 {
-	if (IS_ICELAKE(dev_priv)) {
+	if (INTEL_GEN(dev_priv) >= 11) {
 		if (dev_priv->cdclk.hw.ref == 24000)
 			dev_priv->max_cdclk_freq = 648000;
 		else
@@ -2801,12 +2801,12 @@ void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv)
 		dev_priv->display.set_cdclk = cnl_set_cdclk;
 		dev_priv->display.modeset_calc_cdclk =
 			cnl_modeset_calc_cdclk;
-	} else if (IS_ICELAKE(dev_priv)) {
+	} else if (INTEL_GEN(dev_priv) >= 11) {
 		dev_priv->display.set_cdclk = icl_set_cdclk;
 		dev_priv->display.modeset_calc_cdclk = icl_modeset_calc_cdclk;
 	}
 
-	if (IS_ICELAKE(dev_priv))
+	if (INTEL_GEN(dev_priv) >= 11)
 		dev_priv->display.get_cdclk = icl_get_cdclk;
 	else if (IS_GEN10(dev_priv))
 		dev_priv->display.get_cdclk = cnl_get_cdclk;
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index cd627851f2a5..10b5314f266c 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -915,7 +915,7 @@ static int intel_ddi_hdmi_level(struct drm_i915_private *dev_priv, enum port por
 
 	level = dev_priv->vbt.ddi_port_info[port].hdmi_level_shift;
 
-	if (IS_ICELAKE(dev_priv)) {
+	if (INTEL_GEN(dev_priv) >= 11) {
 		if (intel_port_is_combophy(dev_priv, port))
 			icl_get_combo_buf_trans(dev_priv, port,
 						INTEL_OUTPUT_HDMI, &n_entries);
@@ -1745,7 +1745,7 @@ static void intel_ddi_clock_get(struct intel_encoder *encoder,
 		bxt_ddi_clock_get(encoder, pipe_config);
 	else if (IS_GEN10(dev_priv))
 		cnl_ddi_clock_get(encoder, pipe_config);
-	else if (IS_ICELAKE(dev_priv))
+	else if (INTEL_GEN(dev_priv) >= 11)
 		icl_ddi_clock_get(encoder, pipe_config);
 }
 
@@ -2241,7 +2241,7 @@ u8 intel_ddi_dp_voltage_max(struct intel_encoder *encoder)
 	enum port port = encoder->port;
 	int n_entries;
 
-	if (IS_ICELAKE(dev_priv)) {
+	if (INTEL_GEN(dev_priv) >= 11) {
 		if (intel_port_is_combophy(dev_priv, port))
 			icl_get_combo_buf_trans(dev_priv, port, encoder->type,
 						&n_entries);
@@ -2716,7 +2716,7 @@ u32 bxt_signal_levels(struct intel_dp *intel_dp)
 	struct intel_encoder *encoder = &dport->base;
 	int level = intel_ddi_dp_level(intel_dp);
 
-	if (IS_ICELAKE(dev_priv))
+	if (INTEL_GEN(dev_priv) >= 11)
 		icl_ddi_vswing_sequence(encoder, intel_dp->link_rate,
 					level, encoder->type);
 	else if (IS_GEN10(dev_priv))
@@ -2833,7 +2833,7 @@ static void intel_ddi_clk_select(struct intel_encoder *encoder,
 
 	mutex_lock(&dev_priv->dpll_lock);
 
-	if (IS_ICELAKE(dev_priv)) {
+	if (INTEL_GEN(dev_priv) >= 11) {
 		if (!intel_port_is_combophy(dev_priv, port))
 			I915_WRITE(DDI_CLK_SEL(port),
 				   icl_pll_to_ddi_pll_sel(encoder, crtc_state));
@@ -2875,7 +2875,7 @@ static void intel_ddi_clk_disable(struct intel_encoder *encoder)
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	enum port port = encoder->port;
 
-	if (IS_ICELAKE(dev_priv)) {
+	if (INTEL_GEN(dev_priv) >= 11) {
 		if (!intel_port_is_combophy(dev_priv, port))
 			I915_WRITE(DDI_CLK_SEL(port), DDI_CLK_SEL_NONE);
 	} else if (IS_GEN10(dev_priv)) {
@@ -2917,7 +2917,7 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
 	icl_program_mg_dp_mode(intel_dp);
 	icl_disable_phy_clock_gating(dig_port);
 
-	if (IS_ICELAKE(dev_priv))
+	if (INTEL_GEN(dev_priv) >= 11)
 		icl_ddi_vswing_sequence(encoder, crtc_state->port_clock,
 					level, encoder->type);
 	else if (IS_GEN10(dev_priv))
@@ -2956,7 +2956,7 @@ static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder,
 
 	intel_display_power_get(dev_priv, dig_port->ddi_io_power_domain);
 
-	if (IS_ICELAKE(dev_priv))
+	if (INTEL_GEN(dev_priv) >= 11)
 		icl_ddi_vswing_sequence(encoder, crtc_state->port_clock,
 					level, INTEL_OUTPUT_HDMI);
 	else if (IS_GEN10(dev_priv))
@@ -3375,7 +3375,7 @@ void intel_ddi_compute_min_voltage_level(struct drm_i915_private *dev_priv,
 {
 	if (IS_GEN10(dev_priv) && crtc_state->port_clock > 594000)
 		crtc_state->min_voltage_level = 2;
-	else if (IS_ICELAKE(dev_priv) && crtc_state->port_clock > 594000)
+	else if (INTEL_GEN(dev_priv) >= 11 && crtc_state->port_clock > 594000)
 		crtc_state->min_voltage_level = 1;
 }
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 1abf79a4ee91..08d0cd802e48 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5932,7 +5932,7 @@ bool intel_port_is_combophy(struct drm_i915_private *dev_priv, enum port port)
 	if (port == PORT_NONE)
 		return false;
 
-	if (IS_ICELAKE(dev_priv))
+	if (INTEL_GEN(dev_priv) >= 11)
 		return port <= PORT_B;
 
 	return false;
@@ -5940,7 +5940,7 @@ bool intel_port_is_combophy(struct drm_i915_private *dev_priv, enum port port)
 
 bool intel_port_is_tc(struct drm_i915_private *dev_priv, enum port port)
 {
-	if (IS_ICELAKE(dev_priv))
+	if (INTEL_GEN(dev_priv) >= 11)
 		return port >= PORT_C && port <= PORT_F;
 
 	return false;
@@ -9496,7 +9496,7 @@ static void haswell_get_ddi_port_state(struct intel_crtc *crtc,
 
 	port = (tmp & TRANS_DDI_PORT_MASK) >> TRANS_DDI_PORT_SHIFT;
 
-	if (IS_ICELAKE(dev_priv))
+	if (INTEL_GEN(dev_priv) >= 11)
 		icelake_get_ddi_pll(dev_priv, port, pipe_config);
 	else if (IS_GEN10(dev_priv))
 		cannonlake_get_ddi_pll(dev_priv, port, pipe_config);
@@ -14049,7 +14049,7 @@ static void intel_setup_outputs(struct drm_i915_private *dev_priv)
 	if (intel_crt_present(dev_priv))
 		intel_crt_init(dev_priv);
 
-	if (IS_ICELAKE(dev_priv)) {
+	if (INTEL_GEN(dev_priv) >= 11) {
 		intel_ddi_init(dev_priv, PORT_A);
 		intel_ddi_init(dev_priv, PORT_B);
 		intel_ddi_init(dev_priv, PORT_C);
diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
index ae92dc97d5aa..b0ba775706b3 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
@@ -3200,7 +3200,7 @@ void intel_shared_dpll_init(struct drm_device *dev)
 	const struct dpll_info *dpll_info;
 	int i;
 
-	if (IS_ICELAKE(dev_priv))
+	if (INTEL_GEN(dev_priv) >= 11)
 		dpll_mgr = &icl_pll_mgr;
 	else if (IS_GEN10(dev_priv))
 		dpll_mgr = &cnl_pll_mgr;
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 89d5e3984452..ab835370df19 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1919,7 +1919,7 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
 
 	intel_display_power_get(dev_priv, POWER_DOMAIN_GMBUS);
 
-	if (IS_ICELAKE(dev_priv) &&
+	if (INTEL_GEN(dev_priv) >= 11 &&
 	    !intel_digital_port_connected(encoder))
 		goto out;
 
diff --git a/drivers/gpu/drm/i915/intel_mocs.c b/drivers/gpu/drm/i915/intel_mocs.c
index 662eb087bb2e..d75b9a0c2bcc 100644
--- a/drivers/gpu/drm/i915/intel_mocs.c
+++ b/drivers/gpu/drm/i915/intel_mocs.c
@@ -178,8 +178,7 @@ static bool get_mocs_settings(struct drm_i915_private *dev_priv,
 {
 	bool result = false;
 
-	if (IS_GEN9_BC(dev_priv) || IS_GEN10(dev_priv) ||
-	    IS_ICELAKE(dev_priv)) {
+	if (INTEL_GEN(dev_priv) >= 10 || IS_GEN9_BC(dev_priv)) {
 		table->size  = ARRAY_SIZE(skylake_mocs_table);
 		table->table = skylake_mocs_table;
 		result = true;
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 7234b2272481..f4b7fd132173 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3612,8 +3612,8 @@ static bool skl_needs_memory_bw_wa(struct intel_atomic_state *state)
 static bool
 intel_has_sagv(struct drm_i915_private *dev_priv)
 {
-	if (IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv) ||
-	    IS_GEN10(dev_priv) || IS_ICELAKE(dev_priv))
+	if (INTEL_GEN(dev_priv) >= 10 ||
+	    IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv))
 		return true;
 
 	if (IS_SKYLAKE(dev_priv) &&
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 63f0b1c0bf77..1d7dd506708a 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -3082,7 +3082,7 @@ int intel_power_domains_init(struct drm_i915_private *dev_priv)
 	 * The enabling order will be from lower to higher indexed wells,
 	 * the disabling order is reversed.
 	 */
-	if (IS_ICELAKE(dev_priv)) {
+	if (INTEL_GEN(dev_priv) >= 11) {
 		err = set_power_wells(power_domains, icl_power_wells);
 	} else if (IS_HASWELL(dev_priv)) {
 		err = set_power_wells(power_domains, hsw_power_wells);
@@ -3767,7 +3767,7 @@ void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool resume)
 
 	power_domains->initializing = true;
 
-	if (IS_ICELAKE(dev_priv)) {
+	if (INTEL_GEN(dev_priv) >= 11) {
 		icl_display_core_init(dev_priv, resume);
 	} else if (IS_GEN10(dev_priv)) {
 		cnl_display_core_init(dev_priv, resume);
@@ -3898,7 +3898,7 @@ void intel_power_domains_suspend(struct drm_i915_private *dev_priv,
 		intel_power_domains_verify_state(dev_priv);
 	}
 
-	if (IS_ICELAKE(dev_priv))
+	if (INTEL_GEN(dev_priv) >= 11)
 		icl_display_core_uninit(dev_priv);
 	else if (IS_GEN10(dev_priv))
 		cnl_display_core_uninit(dev_priv);
-- 
2.19.1

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

[RFC 6/8] drm/i915: Consolidate cdclk hooks.

[Rodrigo Vivi]

We don't need 2 different blocks.
Specially with on in ordered older-to-newer and the other
one newer-to-older.

Let's start always using newer-to-older order
when it makes sense.

Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_cdclk.c | 91 ++++++++++++++----------------
 1 file changed, 42 insertions(+), 49 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
index 915e2c93412b..36db1fad160b 100644
--- a/drivers/gpu/drm/i915/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/intel_cdclk.c
@@ -2777,80 +2777,73 @@ void intel_update_rawclk(struct drm_i915_private *dev_priv)
  */
 void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv)
 {
-	if (IS_CHERRYVIEW(dev_priv)) {
-		dev_priv->display.set_cdclk = chv_set_cdclk;
-		dev_priv->display.modeset_calc_cdclk =
-			vlv_modeset_calc_cdclk;
-	} else if (IS_VALLEYVIEW(dev_priv)) {
-		dev_priv->display.set_cdclk = vlv_set_cdclk;
-		dev_priv->display.modeset_calc_cdclk =
-			vlv_modeset_calc_cdclk;
-	} else if (IS_BROADWELL(dev_priv)) {
-		dev_priv->display.set_cdclk = bdw_set_cdclk;
-		dev_priv->display.modeset_calc_cdclk =
-			bdw_modeset_calc_cdclk;
-	} else if (IS_GEN9_LP(dev_priv)) {
-		dev_priv->display.set_cdclk = bxt_set_cdclk;
-		dev_priv->display.modeset_calc_cdclk =
-			bxt_modeset_calc_cdclk;
-	} else if (IS_GEN9_BC(dev_priv)) {
-		dev_priv->display.set_cdclk = skl_set_cdclk;
-		dev_priv->display.modeset_calc_cdclk =
-			skl_modeset_calc_cdclk;
+	if (INTEL_GEN(dev_priv) >= 11) {
+		dev_priv->display.set_cdclk = icl_set_cdclk;
+		dev_priv->display.modeset_calc_cdclk = icl_modeset_calc_cdclk;
+		dev_priv->display.get_cdclk = icl_get_cdclk;
 	} else if (IS_GEN10(dev_priv)) {
 		dev_priv->display.set_cdclk = cnl_set_cdclk;
 		dev_priv->display.modeset_calc_cdclk =
 			cnl_modeset_calc_cdclk;
-	} else if (INTEL_GEN(dev_priv) >= 11) {
-		dev_priv->display.set_cdclk = icl_set_cdclk;
-		dev_priv->display.modeset_calc_cdclk = icl_modeset_calc_cdclk;
-	}
-
-	if (INTEL_GEN(dev_priv) >= 11)
-		dev_priv->display.get_cdclk = icl_get_cdclk;
-	else if (IS_GEN10(dev_priv))
 		dev_priv->display.get_cdclk = cnl_get_cdclk;
-	else if (IS_GEN9_BC(dev_priv))
+	} else if (IS_GEN9_BC(dev_priv)) {
+		dev_priv->display.set_cdclk = skl_set_cdclk;
+		dev_priv->display.modeset_calc_cdclk =
+			skl_modeset_calc_cdclk;
 		dev_priv->display.get_cdclk = skl_get_cdclk;
-	else if (IS_GEN9_LP(dev_priv))
+	} else if (IS_GEN9_LP(dev_priv)) {
+		dev_priv->display.set_cdclk = bxt_set_cdclk;
+		dev_priv->display.modeset_calc_cdclk =
+			bxt_modeset_calc_cdclk;
 		dev_priv->display.get_cdclk = bxt_get_cdclk;
-	else if (IS_BROADWELL(dev_priv))
+	} else if (IS_BROADWELL(dev_priv)) {
+		dev_priv->display.set_cdclk = bdw_set_cdclk;
+		dev_priv->display.modeset_calc_cdclk =
+			bdw_modeset_calc_cdclk;
 		dev_priv->display.get_cdclk = bdw_get_cdclk;
-	else if (IS_HASWELL(dev_priv))
+	} else if (IS_HASWELL(dev_priv)) {
 		dev_priv->display.get_cdclk = hsw_get_cdclk;
-	else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
+	} else if (IS_CHERRYVIEW(dev_priv)) {
+		dev_priv->display.set_cdclk = chv_set_cdclk;
+		dev_priv->display.modeset_calc_cdclk =
+			vlv_modeset_calc_cdclk;
 		dev_priv->display.get_cdclk = vlv_get_cdclk;
-	else if (IS_GEN6(dev_priv) || IS_IVYBRIDGE(dev_priv))
+	} else if (IS_VALLEYVIEW(dev_priv)) {
+		dev_priv->display.set_cdclk = vlv_set_cdclk;
+		dev_priv->display.modeset_calc_cdclk =
+			vlv_modeset_calc_cdclk;
+		dev_priv->display.get_cdclk = vlv_get_cdclk;
+	} else if (IS_GEN6(dev_priv) || IS_IVYBRIDGE(dev_priv)) {
 		dev_priv->display.get_cdclk = fixed_400mhz_get_cdclk;
-	else if (IS_GEN5(dev_priv))
+	} else if (IS_GEN5(dev_priv)) {
 		dev_priv->display.get_cdclk = fixed_450mhz_get_cdclk;
-	else if (IS_GM45(dev_priv))
+	} else if (IS_GM45(dev_priv)) {
 		dev_priv->display.get_cdclk = gm45_get_cdclk;
-	else if (IS_G45(dev_priv))
+	} else if (IS_G45(dev_priv)) {
 		dev_priv->display.get_cdclk = g33_get_cdclk;
-	else if (IS_I965GM(dev_priv))
+	} else if (IS_I965GM(dev_priv)) {
 		dev_priv->display.get_cdclk = i965gm_get_cdclk;
-	else if (IS_I965G(dev_priv))
+	} else if (IS_I965G(dev_priv)) {
 		dev_priv->display.get_cdclk = fixed_400mhz_get_cdclk;
-	else if (IS_PINEVIEW(dev_priv))
+	} else if (IS_PINEVIEW(dev_priv)) {
 		dev_priv->display.get_cdclk = pnv_get_cdclk;
-	else if (IS_G33(dev_priv))
+	} else if (IS_G33(dev_priv)) {
 		dev_priv->display.get_cdclk = g33_get_cdclk;
-	else if (IS_I945GM(dev_priv))
+	} else if (IS_I945GM(dev_priv)) {
 		dev_priv->display.get_cdclk = i945gm_get_cdclk;
-	else if (IS_I945G(dev_priv))
+	} else if (IS_I945G(dev_priv)) {
 		dev_priv->display.get_cdclk = fixed_400mhz_get_cdclk;
-	else if (IS_I915GM(dev_priv))
+	} else if (IS_I915GM(dev_priv)) {
 		dev_priv->display.get_cdclk = i915gm_get_cdclk;
-	else if (IS_I915G(dev_priv))
+	} else if (IS_I915G(dev_priv)) {
 		dev_priv->display.get_cdclk = fixed_333mhz_get_cdclk;
-	else if (IS_I865G(dev_priv))
+	} else if (IS_I865G(dev_priv)) {
 		dev_priv->display.get_cdclk = fixed_266mhz_get_cdclk;
-	else if (IS_I85X(dev_priv))
+	} else if (IS_I85X(dev_priv)) {
 		dev_priv->display.get_cdclk = i85x_get_cdclk;
-	else if (IS_I845G(dev_priv))
+	} else if (IS_I845G(dev_priv)) {
 		dev_priv->display.get_cdclk = fixed_200mhz_get_cdclk;
-	else { /* 830 */
+	} else { /* 830 */
 		WARN(!IS_I830(dev_priv),
 		     "Unknown platform. Assuming 133 MHz CDCLK\n");
 		dev_priv->display.get_cdclk = fixed_133mhz_get_cdclk;
-- 
2.19.1

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

[RFC 7/8] drm/i915: Sort platform if cases from newer-to-older.

[Rodrigo Vivi]

Let's use this whenever it makes sense and code gets
easier to read.

Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c        | 18 +++++++++---------
 drivers/gpu/drm/i915/intel_dp.c         | 24 +++++++++++++-----------
 drivers/gpu/drm/i915/intel_pm.c         |  6 +++---
 drivers/gpu/drm/i915/intel_runtime_pm.c | 12 ++++++------
 drivers/gpu/drm/i915/intel_uncore.c     |  2 +-
 5 files changed, 32 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 10b5314f266c..498521a3bc21 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1737,16 +1737,16 @@ static void intel_ddi_clock_get(struct intel_encoder *encoder,
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 
-	if (INTEL_GEN(dev_priv) <= 8)
-		hsw_ddi_clock_get(encoder, pipe_config);
+	if (INTEL_GEN(dev_priv) >= 11)
+		icl_ddi_clock_get(encoder, pipe_config);
+	else if (IS_GEN10(dev_priv))
+		cnl_ddi_clock_get(encoder, pipe_config);
 	else if (IS_GEN9_BC(dev_priv))
 		skl_ddi_clock_get(encoder, pipe_config);
 	else if (IS_GEN9_LP(dev_priv))
 		bxt_ddi_clock_get(encoder, pipe_config);
-	else if (IS_GEN10(dev_priv))
-		cnl_ddi_clock_get(encoder, pipe_config);
-	else if (INTEL_GEN(dev_priv) >= 11)
-		icl_ddi_clock_get(encoder, pipe_config);
+	else if (INTEL_GEN(dev_priv) <= 8)
+		hsw_ddi_clock_get(encoder, pipe_config);
 }
 
 void intel_ddi_set_pipe_settings(const struct intel_crtc_state *crtc_state)
@@ -3373,10 +3373,10 @@ static bool intel_ddi_is_audio_enabled(struct drm_i915_private *dev_priv,
 void intel_ddi_compute_min_voltage_level(struct drm_i915_private *dev_priv,
 					 struct intel_crtc_state *crtc_state)
 {
-	if (IS_GEN10(dev_priv) && crtc_state->port_clock > 594000)
-		crtc_state->min_voltage_level = 2;
-	else if (INTEL_GEN(dev_priv) >= 11 && crtc_state->port_clock > 594000)
+	if (INTEL_GEN(dev_priv) >= 11 && crtc_state->port_clock > 594000)
 		crtc_state->min_voltage_level = 1;
+	else if (IS_GEN10(dev_priv) && crtc_state->port_clock > 594000)
+		crtc_state->min_voltage_level = 2;
 }
 
 void intel_ddi_get_config(struct intel_encoder *encoder,
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 0ea0414ccef4..3c13a49b4a7a 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -5035,20 +5035,22 @@ bool intel_digital_port_connected(struct intel_encoder *encoder)
 			return g4x_digital_port_connected(encoder);
 	}
 
-	if (IS_GEN5(dev_priv))
-		return ilk_digital_port_connected(encoder);
-	else if (IS_GEN6(dev_priv))
-		return snb_digital_port_connected(encoder);
-	else if (IS_GEN7(dev_priv))
-		return ivb_digital_port_connected(encoder);
-	else if (IS_GEN8(dev_priv))
-		return bdw_digital_port_connected(encoder);
+	if (INTEL_GEN(dev_priv) >= 11)
+		return icl_digital_port_connected(encoder);
+	else if (IS_GEN10(dev_priv) || IS_GEN9_BC(dev_priv))
+		return spt_digital_port_connected(encoder);
 	else if (IS_GEN9_LP(dev_priv))
 		return bxt_digital_port_connected(encoder);
-	else if (IS_GEN9_BC(dev_priv) || IS_GEN10(dev_priv))
-		return spt_digital_port_connected(encoder);
+	else if (IS_GEN8(dev_priv))
+		return bdw_digital_port_connected(encoder);
+	else if (IS_GEN7(dev_priv))
+		return ivb_digital_port_connected(encoder);
+	else if (IS_GEN6(dev_priv))
+		return snb_digital_port_connected(encoder);
+	else if (IS_GEN5(dev_priv))
+		return ilk_digital_port_connected(encoder);
 	else
-		return icl_digital_port_connected(encoder);
+		return false;
 }
 
 static struct edid *
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index f4b7fd132173..5663b7059467 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3724,12 +3724,12 @@ bool intel_can_enable_sagv(struct drm_atomic_state *state)
 	if (!intel_has_sagv(dev_priv))
 		return false;
 
-	if (IS_GEN9(dev_priv))
-		sagv_block_time_us = 30;
+	if (INTEL_GEN(dev_priv) >= 11)
+		sagv_block_time_us = 10;
 	else if (IS_GEN10(dev_priv))
 		sagv_block_time_us = 20;
 	else
-		sagv_block_time_us = 10;
+		sagv_block_time_us = 30;
 
 	/*
 	 * SKL+ workaround: bspec recommends we disable the SAGV when we have
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 1d7dd506708a..9472cd895ea6 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -3084,14 +3084,10 @@ int intel_power_domains_init(struct drm_i915_private *dev_priv)
 	 */
 	if (INTEL_GEN(dev_priv) >= 11) {
 		err = set_power_wells(power_domains, icl_power_wells);
-	} else if (IS_HASWELL(dev_priv)) {
-		err = set_power_wells(power_domains, hsw_power_wells);
-	} else if (IS_BROADWELL(dev_priv)) {
-		err = set_power_wells(power_domains, bdw_power_wells);
-	} else if (IS_GEN9_BC(dev_priv)) {
-		err = set_power_wells(power_domains, skl_power_wells);
 	} else if (IS_GEN10(dev_priv)) {
 		err = set_power_wells(power_domains, cnl_power_wells);
+	} else if (IS_GEN9_BC(dev_priv)) {
+		err = set_power_wells(power_domains, skl_power_wells);
 
 		/*
 		 * DDI and Aux IO are getting enabled for all ports
@@ -3106,8 +3102,12 @@ int intel_power_domains_init(struct drm_i915_private *dev_priv)
 		err = set_power_wells(power_domains, bxt_power_wells);
 	} else if (IS_GEMINILAKE(dev_priv)) {
 		err = set_power_wells(power_domains, glk_power_wells);
+	} else if (IS_BROADWELL(dev_priv)) {
+		err = set_power_wells(power_domains, bdw_power_wells);
 	} else if (IS_CHERRYVIEW(dev_priv)) {
 		err = set_power_wells(power_domains, chv_power_wells);
+	} else if (IS_HASWELL(dev_priv)) {
+		err = set_power_wells(power_domains, hsw_power_wells);
 	} else if (IS_VALLEYVIEW(dev_priv)) {
 		err = set_power_wells(power_domains, vlv_power_wells);
 	} else if (IS_I830(dev_priv)) {
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index 3ad302c66254..9289515108c3 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -1437,7 +1437,7 @@ static void intel_uncore_fw_domains_init(struct drm_i915_private *dev_priv)
 				       FORCEWAKE_MEDIA_VEBOX_GEN11(i),
 				       FORCEWAKE_ACK_MEDIA_VEBOX_GEN11(i));
 		}
-	} else if (IS_GEN9(dev_priv) || IS_GEN10(dev_priv)) {
+	} else if (IS_GEN10(dev_priv) || IS_GEN9(dev_priv)) {
 		dev_priv->uncore.funcs.force_wake_get =
 			fw_domains_get_with_fallback;
 		dev_priv->uncore.funcs.force_wake_put = fw_domains_put;
-- 
2.19.1

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

[RFC 8/8] drm/i915: Simplify intel_has_sagv function.

[Rodrigo Vivi]

Let's just handle SKL as special case instead of listing
platform by platform.

Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 5663b7059467..97f191b081e0 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3612,15 +3612,11 @@ static bool skl_needs_memory_bw_wa(struct intel_atomic_state *state)
 static bool
 intel_has_sagv(struct drm_i915_private *dev_priv)
 {
-	if (INTEL_GEN(dev_priv) >= 10 ||
-	    IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv))
-		return true;
-
 	if (IS_SKYLAKE(dev_priv) &&
 	    dev_priv->sagv_status != I915_SAGV_NOT_CONTROLLED)
 		return true;
 
-	return false;
+	return IS_GEN9_BC(dev_priv) || INTEL_GEN(dev_priv) >= 10;
 }
 
 /*
-- 
2.19.1

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

✗ Fi.CI.CHECKPATCH: warning for re-organize a bit gen10 and gen11

[Patchwork]

== Series Details ==

Series: re-organize a bit gen10 and gen11
URL   : https://patchwork.freedesktop.org/series/51230/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
8c7a79b33eb4 drm/i915: Make number of ddi ports explicit.
e86a18876ae8 drm/i915: Use the ddi_ports info to kill ugly CNL_WITH_PORT_F.
5c375fe14a84 drm/i915/gen10: Prefer gen number than platform codename.
aa3ea0538b89 drm/i915: Group gen 10 display.
-:19: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'dev_priv' - possible side-effects?
#19: FILE: drivers/gpu/drm/i915/i915_drv.h:2640:
+#define HAS_DISPLAY_10(dev_priv) (IS_GEN10(dev_priv) || IS_GEMINILAKE(dev_priv))

total: 0 errors, 0 warnings, 1 checks, 48 lines checked
e9a131a18829 drm/i915/gen11+: Prefer gen over platform codename.
c68022b6c0bb drm/i915: Consolidate cdclk hooks.
7d7a58575b77 drm/i915: Sort platform if cases from newer-to-older.
3b5b7d09a783 drm/i915: Simplify intel_has_sagv function.

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

✗ Fi.CI.SPARSE: warning for re-organize a bit gen10 and gen11

[Patchwork]

== Series Details ==

Series: re-organize a bit gen10 and gen11
URL   : https://patchwork.freedesktop.org/series/51230/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Sparse version: v0.5.2
Commit: drm/i915: Make number of ddi ports explicit.
Okay!

Commit: drm/i915: Use the ddi_ports info to kill ugly CNL_WITH_PORT_F.
-drivers/gpu/drm/i915/selftests/../i915_drv.h:3725:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/selftests/../i915_drv.h:3723:16: warning: expression using sizeof(void)

Commit: drm/i915/gen10: Prefer gen number than platform codename.
Okay!

Commit: drm/i915: Group gen 10 display.
-O:drivers/gpu/drm/i915/intel_cdclk.c:2186:37: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_cdclk.c:2186:37: warning: expression using sizeof(void)
-drivers/gpu/drm/i915/selftests/../i915_drv.h:3723:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/selftests/../i915_drv.h:3725:16: warning: expression using sizeof(void)

Commit: drm/i915/gen11+: Prefer gen over platform codename.
Okay!

Commit: drm/i915: Consolidate cdclk hooks.
Okay!

Commit: drm/i915: Sort platform if cases from newer-to-older.
Okay!

Commit: drm/i915: Simplify intel_has_sagv function.
Okay!

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

✗ Fi.CI.BAT: failure for re-organize a bit gen10 and gen11

[Patchwork]

== Series Details ==

Series: re-organize a bit gen10 and gen11
URL   : https://patchwork.freedesktop.org/series/51230/
State : failure

== Summary ==

= CI Bug Log - changes from CI_DRM_5011 -> Patchwork_10511 =

== Summary - FAILURE ==

  Serious unknown changes coming with Patchwork_10511 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_10511, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/51230/revisions/1/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_10511:

  === IGT changes ===

    ==== Possible regressions ====

    igt@drv_module_reload@basic-reload:
      fi-cnl-u:           PASS -> DMESG-WARN +24

    
== Known issues ==

  Here are the changes found in Patchwork_10511 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_exec_suspend@basic-s3:
      fi-cfl-8109u:       PASS -> INCOMPLETE (fdo#108126, fdo#107187)
      fi-kbl-7500u:       PASS -> INCOMPLETE (fdo#105524, k.org#199541)
      fi-skl-6700k2:      PASS -> INCOMPLETE (fdo#105524, fdo#104108, fdo#107773, k.org#199541)

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-blb-e6850:       PASS -> INCOMPLETE (fdo#107718)

    
    ==== Possible fixes ====

    igt@drv_getparams_basic@basic-subslice-total:
      fi-snb-2520m:       DMESG-WARN (fdo#103713) -> PASS +10

    igt@kms_pipe_crc_basic@read-crc-pipe-b:
      fi-byt-clapper:     FAIL (fdo#107362) -> PASS +1

    
  fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
  fdo#104108 https://bugs.freedesktop.org/show_bug.cgi?id=104108
  fdo#105524 https://bugs.freedesktop.org/show_bug.cgi?id=105524
  fdo#107187 https://bugs.freedesktop.org/show_bug.cgi?id=107187
  fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
  fdo#107718 https://bugs.freedesktop.org/show_bug.cgi?id=107718
  fdo#107773 https://bugs.freedesktop.org/show_bug.cgi?id=107773
  fdo#108126 https://bugs.freedesktop.org/show_bug.cgi?id=108126
  k.org#199541 https://bugzilla.kernel.org/show_bug.cgi?id=199541


== Participating hosts (42 -> 38) ==

  Missing    (4): fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-skl-6700hq 


== Build changes ==

    * Linux: CI_DRM_5011 -> Patchwork_10511

  CI_DRM_5011: f66154472620fdd1f364c577ade311ce2b9d445a @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4684: 6f27fddc6dd79c0486181b64201c6773c5c42a24 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10511: 3b5b7d09a783829df0ff1c824025b7dce40f1959 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

3b5b7d09a783 drm/i915: Simplify intel_has_sagv function.
7d7a58575b77 drm/i915: Sort platform if cases from newer-to-older.
c68022b6c0bb drm/i915: Consolidate cdclk hooks.
e9a131a18829 drm/i915/gen11+: Prefer gen over platform codename.
aa3ea0538b89 drm/i915: Group gen 10 display.
5c375fe14a84 drm/i915/gen10: Prefer gen number than platform codename.
e86a18876ae8 drm/i915: Use the ddi_ports info to kill ugly CNL_WITH_PORT_F.
8c7a79b33eb4 drm/i915: Make number of ddi ports explicit.

== Logs ==

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

Re: [RFC 2/8] drm/i915: Use the ddi_ports info to kill ugly CNL_WITH_PORT_F.

[Jani Nikula]

On Thu, 18 Oct 2018, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> Now that we have the number of ddi ports information available
> let's use it instead of that ugly platform macro.
>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.c         | 10 ++++++++++
>  drivers/gpu/drm/i915/i915_drv.h         |  2 --
>  drivers/gpu/drm/i915/i915_irq.c         |  5 ++---
>  drivers/gpu/drm/i915/intel_dp.c         |  2 +-
>  drivers/gpu/drm/i915/intel_hotplug.c    |  2 +-
>  drivers/gpu/drm/i915/intel_runtime_pm.c |  2 +-
>  6 files changed, 15 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index baac35f698f9..83ab325c6675 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -239,6 +239,14 @@ intel_virt_detect_pch(const struct drm_i915_private *dev_priv)
>  	return id;
>  }
>  
> +static void intel_adjust_skus_differences(struct drm_i915_private *dev_priv)
> +{
> +	/* Some Cannonlake SKUs have Port F */
> +	if (IS_CANNONLAKE(dev_priv) &&
> +	    (INTEL_DEVID(dev_priv) & 0x0004) == 0x0004)
> +		mkwrite_device_info(dev_priv)->ddi_ports = 6;
> +}

I'd like reduce the amount of device info stuff we adjust at probe, not
increase.

Long term, I'd like to make dev_priv->info a const pointer to the const
data in i915_pci.c, and make it truly immutable. We could have an
additional runtime device info to complement that, but this approach
calls for a more concrete split in const and non-const data, not "mostly
const but sometimes mutable".

I think I've chatted about this with Chris too.

BR,
Jani.

> +
>  static void intel_detect_pch(struct drm_i915_private *dev_priv)
>  {
>  	struct pci_dev *pch = NULL;
> @@ -904,6 +912,8 @@ static int i915_driver_init_early(struct drm_i915_private *dev_priv)
>  	if (ret < 0)
>  		goto err_workqueues;
>  
> +	intel_adjust_skus_differences(dev_priv);
> +
>  	/* This must be called before any calls to HAS_PCH_* */
>  	intel_detect_pch(dev_priv);
>  
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 7ad232849268..99e42df79ed8 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2485,8 +2485,6 @@ intel_info(const struct drm_i915_private *dev_priv)
>  				 (dev_priv)->info.gt == 2)
>  #define IS_CFL_GT3(dev_priv)	(IS_COFFEELAKE(dev_priv) && \
>  				 (dev_priv)->info.gt == 3)
> -#define IS_CNL_WITH_PORT_F(dev_priv)   (IS_CANNONLAKE(dev_priv) && \
> -					(INTEL_DEVID(dev_priv) & 0x0004) == 0x0004)
>  
>  #define IS_ALPHA_SUPPORT(intel_info) ((intel_info)->is_alpha_support)
>  
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 5d1f53723388..63d676de3840 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -2782,8 +2782,7 @@ gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl)
>  			if (INTEL_GEN(dev_priv) >= 11)
>  				tmp_mask |= ICL_AUX_CHANNEL_E;
>  
> -			if (IS_CNL_WITH_PORT_F(dev_priv) ||
> -			    INTEL_GEN(dev_priv) >= 11)
> +			if (INTEL_INFO(dev_priv)->ddi_ports >= 6)
>  				tmp_mask |= CNL_AUX_CHANNEL_F;
>  
>  			if (iir & tmp_mask) {
> @@ -4220,7 +4219,7 @@ static void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv)
>  	if (INTEL_GEN(dev_priv) >= 11)
>  		de_port_masked |= ICL_AUX_CHANNEL_E;
>  
> -	if (IS_CNL_WITH_PORT_F(dev_priv) || INTEL_GEN(dev_priv) >= 11)
> +	if (INTEL_INFO(dev_priv)->ddi_ports >= 6)
>  		de_port_masked |= CNL_AUX_CHANNEL_F;
>  
>  	de_pipe_enables = de_pipe_masked | GEN8_PIPE_VBLANK |
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 3384a9bbdafd..0ea0414ccef4 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -402,7 +402,7 @@ static int cnl_max_source_rate(struct intel_dp *intel_dp)
>  		return 540000;
>  
>  	/* For this SKU 8.1G is supported in all ports */
> -	if (IS_CNL_WITH_PORT_F(dev_priv))
> +	if (INTEL_INFO(dev_priv)->ddi_ports == 6)
>  		return 810000;
>  
>  	/* For other SKUs, max rate on ports A and D is 5.4G */
> diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c
> index 648a13c6043c..05d1faf77048 100644
> --- a/drivers/gpu/drm/i915/intel_hotplug.c
> +++ b/drivers/gpu/drm/i915/intel_hotplug.c
> @@ -101,7 +101,7 @@ enum hpd_pin intel_hpd_pin_default(struct drm_i915_private *dev_priv,
>  	case PORT_E:
>  		return HPD_PORT_E;
>  	case PORT_F:
> -		if (IS_CNL_WITH_PORT_F(dev_priv))
> +		if (IS_GEN10(dev_priv))
>  			return HPD_PORT_E;
>  		return HPD_PORT_F;
>  	default:
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 31a49bdcf193..80e14be11279 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -3099,7 +3099,7 @@ int intel_power_domains_init(struct drm_i915_private *dev_priv)
>  		 * timeouts, lets remove them from the list
>  		 * for the SKUs without port F.
>  		 */
> -		if (!IS_CNL_WITH_PORT_F(dev_priv))
> +		if (INTEL_INFO(dev_priv)->ddi_ports == 5)
>  			power_domains->power_well_count -= 2;
>  
>  	} else if (IS_BROXTON(dev_priv)) {

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

Re: [RFC 3/8] drm/i915/gen10: Prefer gen number than platform codename.

[Jani Nikula]

On Thu, 18 Oct 2018, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> Whenever possible let's move towards preferring gen number
> and or features instead of hard coding platform codename
> everywhere.

Rationale missing.

But for gem, agreed, it largely makes sense. For display, I'm not sure
if this is a good idea. Consider the likes of Geminilake. It's gen 9,
but largely gen 10 display. There'll be more stuff like that.

Now I don't know what the answer should be. But you need to consider the
conditions like

	if (IS_CANNONLAKE() || IS_GEMINILAKE())

and

        if (INTEL_GEN() == 9 && !IS_GEMINILAKE())

and figure out how to do that in a sensible way for display. Just
changing stuff from platforms to gen numbers isn't helpful for the
humans reading the code.

BR,
Jani.


>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h         |  2 +-
>  drivers/gpu/drm/i915/intel_cdclk.c      |  6 +++---
>  drivers/gpu/drm/i915/intel_ddi.c        | 20 ++++++++++----------
>  drivers/gpu/drm/i915/intel_display.c    |  4 ++--
>  drivers/gpu/drm/i915/intel_dpll_mgr.c   |  2 +-
>  drivers/gpu/drm/i915/intel_mocs.c       |  2 +-
>  drivers/gpu/drm/i915/intel_pm.c         |  2 +-
>  drivers/gpu/drm/i915/intel_runtime_pm.c |  8 ++++----
>  8 files changed, 23 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 99e42df79ed8..d19b38ef145b 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2620,7 +2620,7 @@ intel_info(const struct drm_i915_private *dev_priv)
>  
>  /* WaRsDisableCoarsePowerGating:skl,cnl */
>  #define NEEDS_WaRsDisableCoarsePowerGating(dev_priv) \
> -	(IS_CANNONLAKE(dev_priv) || \
> +	(IS_GEN10(dev_priv) || \
>  	 IS_SKL_GT3(dev_priv) || IS_SKL_GT4(dev_priv))
>  
>  #define HAS_GMBUS_IRQ(dev_priv) (INTEL_GEN(dev_priv) >= 4)
> diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
> index 29075c763428..4aa6500604cc 100644
> --- a/drivers/gpu/drm/i915/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> @@ -2577,7 +2577,7 @@ void intel_update_max_cdclk(struct drm_i915_private *dev_priv)
>  			dev_priv->max_cdclk_freq = 648000;
>  		else
>  			dev_priv->max_cdclk_freq = 652800;
> -	} else if (IS_CANNONLAKE(dev_priv)) {
> +	} else if (IS_GEN10(dev_priv)) {
>  		dev_priv->max_cdclk_freq = 528000;
>  	} else if (IS_GEN9_BC(dev_priv)) {
>  		u32 limit = I915_READ(SKL_DFSM) & SKL_DFSM_CDCLK_LIMIT_MASK;
> @@ -2797,7 +2797,7 @@ void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv)
>  		dev_priv->display.set_cdclk = skl_set_cdclk;
>  		dev_priv->display.modeset_calc_cdclk =
>  			skl_modeset_calc_cdclk;
> -	} else if (IS_CANNONLAKE(dev_priv)) {
> +	} else if (IS_GEN10(dev_priv)) {
>  		dev_priv->display.set_cdclk = cnl_set_cdclk;
>  		dev_priv->display.modeset_calc_cdclk =
>  			cnl_modeset_calc_cdclk;
> @@ -2808,7 +2808,7 @@ void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv)
>  
>  	if (IS_ICELAKE(dev_priv))
>  		dev_priv->display.get_cdclk = icl_get_cdclk;
> -	else if (IS_CANNONLAKE(dev_priv))
> +	else if (IS_GEN10(dev_priv))
>  		dev_priv->display.get_cdclk = cnl_get_cdclk;
>  	else if (IS_GEN9_BC(dev_priv))
>  		dev_priv->display.get_cdclk = skl_get_cdclk;
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 6b9742baa5f2..cd627851f2a5 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -922,7 +922,7 @@ static int intel_ddi_hdmi_level(struct drm_i915_private *dev_priv, enum port por
>  		else
>  			n_entries = ARRAY_SIZE(icl_mg_phy_ddi_translations);
>  		default_entry = n_entries - 1;
> -	} else if (IS_CANNONLAKE(dev_priv)) {
> +	} else if (IS_GEN10(dev_priv)) {
>  		cnl_get_buf_trans_hdmi(dev_priv, &n_entries);
>  		default_entry = n_entries - 1;
>  	} else if (IS_GEN9_LP(dev_priv)) {
> @@ -1743,7 +1743,7 @@ static void intel_ddi_clock_get(struct intel_encoder *encoder,
>  		skl_ddi_clock_get(encoder, pipe_config);
>  	else if (IS_GEN9_LP(dev_priv))
>  		bxt_ddi_clock_get(encoder, pipe_config);
> -	else if (IS_CANNONLAKE(dev_priv))
> +	else if (IS_GEN10(dev_priv))
>  		cnl_ddi_clock_get(encoder, pipe_config);
>  	else if (IS_ICELAKE(dev_priv))
>  		icl_ddi_clock_get(encoder, pipe_config);
> @@ -2247,7 +2247,7 @@ u8 intel_ddi_dp_voltage_max(struct intel_encoder *encoder)
>  						&n_entries);
>  		else
>  			n_entries = ARRAY_SIZE(icl_mg_phy_ddi_translations);
> -	} else if (IS_CANNONLAKE(dev_priv)) {
> +	} else if (IS_GEN10(dev_priv)) {
>  		if (encoder->type == INTEL_OUTPUT_EDP)
>  			cnl_get_buf_trans_edp(dev_priv, &n_entries);
>  		else
> @@ -2719,7 +2719,7 @@ u32 bxt_signal_levels(struct intel_dp *intel_dp)
>  	if (IS_ICELAKE(dev_priv))
>  		icl_ddi_vswing_sequence(encoder, intel_dp->link_rate,
>  					level, encoder->type);
> -	else if (IS_CANNONLAKE(dev_priv))
> +	else if (IS_GEN10(dev_priv))
>  		cnl_ddi_vswing_sequence(encoder, level, encoder->type);
>  	else
>  		bxt_ddi_vswing_sequence(encoder, level, encoder->type);
> @@ -2837,7 +2837,7 @@ static void intel_ddi_clk_select(struct intel_encoder *encoder,
>  		if (!intel_port_is_combophy(dev_priv, port))
>  			I915_WRITE(DDI_CLK_SEL(port),
>  				   icl_pll_to_ddi_pll_sel(encoder, crtc_state));
> -	} else if (IS_CANNONLAKE(dev_priv)) {
> +	} else if (IS_GEN10(dev_priv)) {
>  		/* Configure DPCLKA_CFGCR0 to map the DPLL to the DDI. */
>  		val = I915_READ(DPCLKA_CFGCR0);
>  		val &= ~DPCLKA_CFGCR0_DDI_CLK_SEL_MASK(port);
> @@ -2878,7 +2878,7 @@ static void intel_ddi_clk_disable(struct intel_encoder *encoder)
>  	if (IS_ICELAKE(dev_priv)) {
>  		if (!intel_port_is_combophy(dev_priv, port))
>  			I915_WRITE(DDI_CLK_SEL(port), DDI_CLK_SEL_NONE);
> -	} else if (IS_CANNONLAKE(dev_priv)) {
> +	} else if (IS_GEN10(dev_priv)) {
>  		I915_WRITE(DPCLKA_CFGCR0, I915_READ(DPCLKA_CFGCR0) |
>  			   DPCLKA_CFGCR0_DDI_CLK_OFF(port));
>  	} else if (IS_GEN9_BC(dev_priv)) {
> @@ -2920,7 +2920,7 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
>  	if (IS_ICELAKE(dev_priv))
>  		icl_ddi_vswing_sequence(encoder, crtc_state->port_clock,
>  					level, encoder->type);
> -	else if (IS_CANNONLAKE(dev_priv))
> +	else if (IS_GEN10(dev_priv))
>  		cnl_ddi_vswing_sequence(encoder, level, encoder->type);
>  	else if (IS_GEN9_LP(dev_priv))
>  		bxt_ddi_vswing_sequence(encoder, level, encoder->type);
> @@ -2959,7 +2959,7 @@ static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder,
>  	if (IS_ICELAKE(dev_priv))
>  		icl_ddi_vswing_sequence(encoder, crtc_state->port_clock,
>  					level, INTEL_OUTPUT_HDMI);
> -	else if (IS_CANNONLAKE(dev_priv))
> +	else if (IS_GEN10(dev_priv))
>  		cnl_ddi_vswing_sequence(encoder, level, INTEL_OUTPUT_HDMI);
>  	else if (IS_GEN9_LP(dev_priv))
>  		bxt_ddi_vswing_sequence(encoder, level, INTEL_OUTPUT_HDMI);
> @@ -3373,7 +3373,7 @@ static bool intel_ddi_is_audio_enabled(struct drm_i915_private *dev_priv,
>  void intel_ddi_compute_min_voltage_level(struct drm_i915_private *dev_priv,
>  					 struct intel_crtc_state *crtc_state)
>  {
> -	if (IS_CANNONLAKE(dev_priv) && crtc_state->port_clock > 594000)
> +	if (IS_GEN10(dev_priv) && crtc_state->port_clock > 594000)
>  		crtc_state->min_voltage_level = 2;
>  	else if (IS_ICELAKE(dev_priv) && crtc_state->port_clock > 594000)
>  		crtc_state->min_voltage_level = 1;
> @@ -3742,7 +3742,7 @@ static bool intel_ddi_a_force_4_lanes(struct intel_digital_port *dport)
>  	 *             DDI_F what makes DDI_E useless. However for this
>  	 *             case let's trust VBT info.
>  	 */
> -	if (IS_CANNONLAKE(dev_priv) &&
> +	if (IS_GEN10(dev_priv) &&
>  	    !intel_bios_is_port_present(dev_priv, PORT_E))
>  		return true;
>  
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index fc7e3b0bd95c..7b04b8436b6d 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5269,7 +5269,7 @@ static bool needs_nv12_wa(struct drm_i915_private *dev_priv,
>  		return false;
>  
>  	if ((INTEL_GEN(dev_priv) == 9 && !IS_GEMINILAKE(dev_priv)) ||
> -	    IS_CANNONLAKE(dev_priv))
> +	    IS_GEN10(dev_priv))
>  		return true;
>  
>  	return false;
> @@ -9498,7 +9498,7 @@ static void haswell_get_ddi_port_state(struct intel_crtc *crtc,
>  
>  	if (IS_ICELAKE(dev_priv))
>  		icelake_get_ddi_pll(dev_priv, port, pipe_config);
> -	else if (IS_CANNONLAKE(dev_priv))
> +	else if (IS_GEN10(dev_priv))
>  		cannonlake_get_ddi_pll(dev_priv, port, pipe_config);
>  	else if (IS_GEN9_BC(dev_priv))
>  		skylake_get_ddi_pll(dev_priv, port, pipe_config);
> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> index 7bdff5ba58b9..ae92dc97d5aa 100644
> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> @@ -3202,7 +3202,7 @@ void intel_shared_dpll_init(struct drm_device *dev)
>  
>  	if (IS_ICELAKE(dev_priv))
>  		dpll_mgr = &icl_pll_mgr;
> -	else if (IS_CANNONLAKE(dev_priv))
> +	else if (IS_GEN10(dev_priv))
>  		dpll_mgr = &cnl_pll_mgr;
>  	else if (IS_GEN9_BC(dev_priv))
>  		dpll_mgr = &skl_pll_mgr;
> diff --git a/drivers/gpu/drm/i915/intel_mocs.c b/drivers/gpu/drm/i915/intel_mocs.c
> index 77e9871a8c9a..662eb087bb2e 100644
> --- a/drivers/gpu/drm/i915/intel_mocs.c
> +++ b/drivers/gpu/drm/i915/intel_mocs.c
> @@ -178,7 +178,7 @@ static bool get_mocs_settings(struct drm_i915_private *dev_priv,
>  {
>  	bool result = false;
>  
> -	if (IS_GEN9_BC(dev_priv) || IS_CANNONLAKE(dev_priv) ||
> +	if (IS_GEN9_BC(dev_priv) || IS_GEN10(dev_priv) ||
>  	    IS_ICELAKE(dev_priv)) {
>  		table->size  = ARRAY_SIZE(skylake_mocs_table);
>  		table->table = skylake_mocs_table;
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 67a4d0735291..7234b2272481 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3613,7 +3613,7 @@ static bool
>  intel_has_sagv(struct drm_i915_private *dev_priv)
>  {
>  	if (IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv) ||
> -	    IS_CANNONLAKE(dev_priv) || IS_ICELAKE(dev_priv))
> +	    IS_GEN10(dev_priv) || IS_ICELAKE(dev_priv))
>  		return true;
>  
>  	if (IS_SKYLAKE(dev_priv) &&
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 80e14be11279..63f0b1c0bf77 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -387,7 +387,7 @@ static void hsw_power_well_enable(struct drm_i915_private *dev_priv,
>  	hsw_wait_for_power_well_enable(dev_priv, power_well);
>  
>  	/* Display WA #1178: cnl */
> -	if (IS_CANNONLAKE(dev_priv) &&
> +	if (IS_GEN10(dev_priv) &&
>  	    pw_idx >= GLK_PW_CTL_IDX_AUX_B &&
>  	    pw_idx <= CNL_PW_CTL_IDX_AUX_F) {
>  		val = I915_READ(CNL_AUX_ANAOVRD1(pw_idx));
> @@ -3090,7 +3090,7 @@ int intel_power_domains_init(struct drm_i915_private *dev_priv)
>  		err = set_power_wells(power_domains, bdw_power_wells);
>  	} else if (IS_GEN9_BC(dev_priv)) {
>  		err = set_power_wells(power_domains, skl_power_wells);
> -	} else if (IS_CANNONLAKE(dev_priv)) {
> +	} else if (IS_GEN10(dev_priv)) {
>  		err = set_power_wells(power_domains, cnl_power_wells);
>  
>  		/*
> @@ -3769,7 +3769,7 @@ void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool resume)
>  
>  	if (IS_ICELAKE(dev_priv)) {
>  		icl_display_core_init(dev_priv, resume);
> -	} else if (IS_CANNONLAKE(dev_priv)) {
> +	} else if (IS_GEN10(dev_priv)) {
>  		cnl_display_core_init(dev_priv, resume);
>  	} else if (IS_GEN9_BC(dev_priv)) {
>  		skl_display_core_init(dev_priv, resume);
> @@ -3900,7 +3900,7 @@ void intel_power_domains_suspend(struct drm_i915_private *dev_priv,
>  
>  	if (IS_ICELAKE(dev_priv))
>  		icl_display_core_uninit(dev_priv);
> -	else if (IS_CANNONLAKE(dev_priv))
> +	else if (IS_GEN10(dev_priv))
>  		cnl_display_core_uninit(dev_priv);
>  	else if (IS_GEN9_BC(dev_priv))
>  		skl_display_core_uninit(dev_priv);

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

Re: [RFC 4/8] drm/i915: Group gen 10 display.

[Jani Nikula]

On Thu, 18 Oct 2018, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> Continuing with the goal of use less platform codenames:
> let's group platforms who has gen10 display.

Ahah, so this answers my question in the previous patch. ;)

So we already have HAS_GMCH_DISPLAY().

If we're going to make gen10 display a thing, should we not generalize
this to have an actual display gen in device info? Of course for most
platforms it's going to be identical to the gem gen.

The question becomes, is display gen accurate enough? It's not enough to
cover all of Geminilake I believe. Which makes me think, should we just
add tons more device info flags to cover features at a detailed level? I
think that's what Joonas advocates. Perhaps it bloats the device info,
and causes increase in the number of device info blocks. But my gut
feeling says together with truly immutable device info, there are
compiler optimization benefits to be had.

Also, currently we "inherit" device info using truly obnoxious macro
layering where you have to work hard to trace the macros to find out
what is set for a given platform. It doesn't have to be that way. We
could move parts of it to separate but shared features structs, and add
pointers to them.

Anyway, thanks for rolling this series as a starting point for
discussion. Even if I'm not buying the changes as-is. ;)


BR,
Jani.

>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h      | 2 ++
>  drivers/gpu/drm/i915/intel_cdclk.c   | 2 +-
>  drivers/gpu/drm/i915/intel_color.c   | 2 +-
>  drivers/gpu/drm/i915/intel_display.c | 4 ++--
>  drivers/gpu/drm/i915/intel_sprite.c  | 2 +-
>  5 files changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index d19b38ef145b..6436fedfe561 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2637,6 +2637,8 @@ intel_info(const struct drm_i915_private *dev_priv)
>  #define SUPPORTS_TV(dev_priv)		((dev_priv)->info.supports_tv)
>  #define I915_HAS_HOTPLUG(dev_priv)	((dev_priv)->info.has_hotplug)
>  
> +#define HAS_DISPLAY_10(dev_priv) (IS_GEN10(dev_priv) || IS_GEMINILAKE(dev_priv))
> +
>  #define HAS_FW_BLC(dev_priv) 	(INTEL_GEN(dev_priv) > 2)
>  #define HAS_FBC(dev_priv)	((dev_priv)->info.has_fbc)
>  #define HAS_CUR_FBC(dev_priv)	(!HAS_GMCH_DISPLAY(dev_priv) && INTEL_GEN(dev_priv) >= 7)
> diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
> index 4aa6500604cc..b315b70fd49c 100644
> --- a/drivers/gpu/drm/i915/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> @@ -2181,7 +2181,7 @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state)
>  	    crtc_state->has_audio &&
>  	    crtc_state->port_clock >= 540000 &&
>  	    crtc_state->lane_count == 4) {
> -		if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv)) {
> +		if (HAS_DISPLAY_10(dev_priv)) {
>  			/* Display WA #1145: glk,cnl */
>  			min_cdclk = max(316800, min_cdclk);
>  		} else if (IS_GEN9(dev_priv) || IS_BROADWELL(dev_priv)) {
> diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
> index 5127da286a2b..d5f67b9c9698 100644
> --- a/drivers/gpu/drm/i915/intel_color.c
> +++ b/drivers/gpu/drm/i915/intel_color.c
> @@ -659,7 +659,7 @@ void intel_color_init(struct drm_crtc *crtc)
>  		   IS_BROXTON(dev_priv)) {
>  		dev_priv->display.load_csc_matrix = ilk_load_csc_matrix;
>  		dev_priv->display.load_luts = broadwell_load_luts;
> -	} else if (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) {
> +	} else if (HAS_DISPLAY_10(dev_priv)) {
>  		dev_priv->display.load_csc_matrix = ilk_load_csc_matrix;
>  		dev_priv->display.load_luts = glk_load_luts;
>  	} else {
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 7b04b8436b6d..1abf79a4ee91 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5730,7 +5730,7 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
>  	intel_crtc->active = true;
>  
>  	/* Display WA #1180: WaDisableScalarClockGating: glk, cnl */
> -	psl_clkgate_wa = (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) &&
> +	psl_clkgate_wa = (HAS_DISPLAY_10(dev_priv)) &&
>  			 pipe_config->pch_pfit.enabled;
>  	if (psl_clkgate_wa)
>  		glk_pipe_scaler_clock_gating_wa(dev_priv, pipe, true);
> @@ -15565,7 +15565,7 @@ get_encoder_power_domains(struct drm_i915_private *dev_priv)
>  static void intel_early_display_was(struct drm_i915_private *dev_priv)
>  {
>  	/* Display WA #1185 WaDisableDARBFClkGating:cnl,glk */
> -	if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
> +	if (HAS_DISPLAY_10(dev_priv))
>  		I915_WRITE(GEN9_CLKGATE_DIS_0, I915_READ(GEN9_CLKGATE_DIS_0) |
>  			   DARBF_GATING_DIS);
>  
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 7cd59eee5cad..912ab7b9d89a 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -1330,7 +1330,7 @@ static int skl_plane_check_dst_coordinates(const struct intel_crtc_state *crtc_s
>  	 * than the cursor ending less than 4 pixels from the left edge of the
>  	 * screen may cause FIFO underflow and display corruption.
>  	 */
> -	if ((IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) &&
> +	if (HAS_DISPLAY_10(dev_priv) &&
>  	    (crtc_x + crtc_w < 4 || crtc_x > pipe_src_w - 4)) {
>  		DRM_DEBUG_KMS("requested plane X %s position %d invalid (valid range %d-%d)\n",
>  			      crtc_x + crtc_w < 4 ? "end" : "start",

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

Re: [RFC 5/8] drm/i915/gen11+: Prefer gen over platform codename.

[Jani Nikula]

On Thu, 18 Oct 2018, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> Also let's always consider next platform follows
> the most recent one. Like we have done for transitioning
> gen9 to gen10 and gent10 to gen11.
>
> Let's use same approach for gen11+ and only introduce
> changes later as needed.

Same worry as with Geminilake. The gen is essentially the gem gen, not
display gen.

BR,
Jani.

>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_cdclk.c      |  6 +++---
>  drivers/gpu/drm/i915/intel_ddi.c        | 18 +++++++++---------
>  drivers/gpu/drm/i915/intel_display.c    |  8 ++++----
>  drivers/gpu/drm/i915/intel_dpll_mgr.c   |  2 +-
>  drivers/gpu/drm/i915/intel_hdmi.c       |  2 +-
>  drivers/gpu/drm/i915/intel_mocs.c       |  3 +--
>  drivers/gpu/drm/i915/intel_pm.c         |  4 ++--
>  drivers/gpu/drm/i915/intel_runtime_pm.c |  6 +++---
>  8 files changed, 24 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
> index b315b70fd49c..915e2c93412b 100644
> --- a/drivers/gpu/drm/i915/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> @@ -2572,7 +2572,7 @@ static int intel_compute_max_dotclk(struct drm_i915_private *dev_priv)
>   */
>  void intel_update_max_cdclk(struct drm_i915_private *dev_priv)
>  {
> -	if (IS_ICELAKE(dev_priv)) {
> +	if (INTEL_GEN(dev_priv) >= 11) {
>  		if (dev_priv->cdclk.hw.ref == 24000)
>  			dev_priv->max_cdclk_freq = 648000;
>  		else
> @@ -2801,12 +2801,12 @@ void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv)
>  		dev_priv->display.set_cdclk = cnl_set_cdclk;
>  		dev_priv->display.modeset_calc_cdclk =
>  			cnl_modeset_calc_cdclk;
> -	} else if (IS_ICELAKE(dev_priv)) {
> +	} else if (INTEL_GEN(dev_priv) >= 11) {
>  		dev_priv->display.set_cdclk = icl_set_cdclk;
>  		dev_priv->display.modeset_calc_cdclk = icl_modeset_calc_cdclk;
>  	}
>  
> -	if (IS_ICELAKE(dev_priv))
> +	if (INTEL_GEN(dev_priv) >= 11)
>  		dev_priv->display.get_cdclk = icl_get_cdclk;
>  	else if (IS_GEN10(dev_priv))
>  		dev_priv->display.get_cdclk = cnl_get_cdclk;
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index cd627851f2a5..10b5314f266c 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -915,7 +915,7 @@ static int intel_ddi_hdmi_level(struct drm_i915_private *dev_priv, enum port por
>  
>  	level = dev_priv->vbt.ddi_port_info[port].hdmi_level_shift;
>  
> -	if (IS_ICELAKE(dev_priv)) {
> +	if (INTEL_GEN(dev_priv) >= 11) {
>  		if (intel_port_is_combophy(dev_priv, port))
>  			icl_get_combo_buf_trans(dev_priv, port,
>  						INTEL_OUTPUT_HDMI, &n_entries);
> @@ -1745,7 +1745,7 @@ static void intel_ddi_clock_get(struct intel_encoder *encoder,
>  		bxt_ddi_clock_get(encoder, pipe_config);
>  	else if (IS_GEN10(dev_priv))
>  		cnl_ddi_clock_get(encoder, pipe_config);
> -	else if (IS_ICELAKE(dev_priv))
> +	else if (INTEL_GEN(dev_priv) >= 11)
>  		icl_ddi_clock_get(encoder, pipe_config);
>  }
>  
> @@ -2241,7 +2241,7 @@ u8 intel_ddi_dp_voltage_max(struct intel_encoder *encoder)
>  	enum port port = encoder->port;
>  	int n_entries;
>  
> -	if (IS_ICELAKE(dev_priv)) {
> +	if (INTEL_GEN(dev_priv) >= 11) {
>  		if (intel_port_is_combophy(dev_priv, port))
>  			icl_get_combo_buf_trans(dev_priv, port, encoder->type,
>  						&n_entries);
> @@ -2716,7 +2716,7 @@ u32 bxt_signal_levels(struct intel_dp *intel_dp)
>  	struct intel_encoder *encoder = &dport->base;
>  	int level = intel_ddi_dp_level(intel_dp);
>  
> -	if (IS_ICELAKE(dev_priv))
> +	if (INTEL_GEN(dev_priv) >= 11)
>  		icl_ddi_vswing_sequence(encoder, intel_dp->link_rate,
>  					level, encoder->type);
>  	else if (IS_GEN10(dev_priv))
> @@ -2833,7 +2833,7 @@ static void intel_ddi_clk_select(struct intel_encoder *encoder,
>  
>  	mutex_lock(&dev_priv->dpll_lock);
>  
> -	if (IS_ICELAKE(dev_priv)) {
> +	if (INTEL_GEN(dev_priv) >= 11) {
>  		if (!intel_port_is_combophy(dev_priv, port))
>  			I915_WRITE(DDI_CLK_SEL(port),
>  				   icl_pll_to_ddi_pll_sel(encoder, crtc_state));
> @@ -2875,7 +2875,7 @@ static void intel_ddi_clk_disable(struct intel_encoder *encoder)
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>  	enum port port = encoder->port;
>  
> -	if (IS_ICELAKE(dev_priv)) {
> +	if (INTEL_GEN(dev_priv) >= 11) {
>  		if (!intel_port_is_combophy(dev_priv, port))
>  			I915_WRITE(DDI_CLK_SEL(port), DDI_CLK_SEL_NONE);
>  	} else if (IS_GEN10(dev_priv)) {
> @@ -2917,7 +2917,7 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
>  	icl_program_mg_dp_mode(intel_dp);
>  	icl_disable_phy_clock_gating(dig_port);
>  
> -	if (IS_ICELAKE(dev_priv))
> +	if (INTEL_GEN(dev_priv) >= 11)
>  		icl_ddi_vswing_sequence(encoder, crtc_state->port_clock,
>  					level, encoder->type);
>  	else if (IS_GEN10(dev_priv))
> @@ -2956,7 +2956,7 @@ static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder,
>  
>  	intel_display_power_get(dev_priv, dig_port->ddi_io_power_domain);
>  
> -	if (IS_ICELAKE(dev_priv))
> +	if (INTEL_GEN(dev_priv) >= 11)
>  		icl_ddi_vswing_sequence(encoder, crtc_state->port_clock,
>  					level, INTEL_OUTPUT_HDMI);
>  	else if (IS_GEN10(dev_priv))
> @@ -3375,7 +3375,7 @@ void intel_ddi_compute_min_voltage_level(struct drm_i915_private *dev_priv,
>  {
>  	if (IS_GEN10(dev_priv) && crtc_state->port_clock > 594000)
>  		crtc_state->min_voltage_level = 2;
> -	else if (IS_ICELAKE(dev_priv) && crtc_state->port_clock > 594000)
> +	else if (INTEL_GEN(dev_priv) >= 11 && crtc_state->port_clock > 594000)
>  		crtc_state->min_voltage_level = 1;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 1abf79a4ee91..08d0cd802e48 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5932,7 +5932,7 @@ bool intel_port_is_combophy(struct drm_i915_private *dev_priv, enum port port)
>  	if (port == PORT_NONE)
>  		return false;
>  
> -	if (IS_ICELAKE(dev_priv))
> +	if (INTEL_GEN(dev_priv) >= 11)
>  		return port <= PORT_B;
>  
>  	return false;
> @@ -5940,7 +5940,7 @@ bool intel_port_is_combophy(struct drm_i915_private *dev_priv, enum port port)
>  
>  bool intel_port_is_tc(struct drm_i915_private *dev_priv, enum port port)
>  {
> -	if (IS_ICELAKE(dev_priv))
> +	if (INTEL_GEN(dev_priv) >= 11)
>  		return port >= PORT_C && port <= PORT_F;
>  
>  	return false;
> @@ -9496,7 +9496,7 @@ static void haswell_get_ddi_port_state(struct intel_crtc *crtc,
>  
>  	port = (tmp & TRANS_DDI_PORT_MASK) >> TRANS_DDI_PORT_SHIFT;
>  
> -	if (IS_ICELAKE(dev_priv))
> +	if (INTEL_GEN(dev_priv) >= 11)
>  		icelake_get_ddi_pll(dev_priv, port, pipe_config);
>  	else if (IS_GEN10(dev_priv))
>  		cannonlake_get_ddi_pll(dev_priv, port, pipe_config);
> @@ -14049,7 +14049,7 @@ static void intel_setup_outputs(struct drm_i915_private *dev_priv)
>  	if (intel_crt_present(dev_priv))
>  		intel_crt_init(dev_priv);
>  
> -	if (IS_ICELAKE(dev_priv)) {
> +	if (INTEL_GEN(dev_priv) >= 11) {
>  		intel_ddi_init(dev_priv, PORT_A);
>  		intel_ddi_init(dev_priv, PORT_B);
>  		intel_ddi_init(dev_priv, PORT_C);
> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> index ae92dc97d5aa..b0ba775706b3 100644
> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> @@ -3200,7 +3200,7 @@ void intel_shared_dpll_init(struct drm_device *dev)
>  	const struct dpll_info *dpll_info;
>  	int i;
>  
> -	if (IS_ICELAKE(dev_priv))
> +	if (INTEL_GEN(dev_priv) >= 11)
>  		dpll_mgr = &icl_pll_mgr;
>  	else if (IS_GEN10(dev_priv))
>  		dpll_mgr = &cnl_pll_mgr;
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 89d5e3984452..ab835370df19 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1919,7 +1919,7 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
>  
>  	intel_display_power_get(dev_priv, POWER_DOMAIN_GMBUS);
>  
> -	if (IS_ICELAKE(dev_priv) &&
> +	if (INTEL_GEN(dev_priv) >= 11 &&
>  	    !intel_digital_port_connected(encoder))
>  		goto out;
>  
> diff --git a/drivers/gpu/drm/i915/intel_mocs.c b/drivers/gpu/drm/i915/intel_mocs.c
> index 662eb087bb2e..d75b9a0c2bcc 100644
> --- a/drivers/gpu/drm/i915/intel_mocs.c
> +++ b/drivers/gpu/drm/i915/intel_mocs.c
> @@ -178,8 +178,7 @@ static bool get_mocs_settings(struct drm_i915_private *dev_priv,
>  {
>  	bool result = false;
>  
> -	if (IS_GEN9_BC(dev_priv) || IS_GEN10(dev_priv) ||
> -	    IS_ICELAKE(dev_priv)) {
> +	if (INTEL_GEN(dev_priv) >= 10 || IS_GEN9_BC(dev_priv)) {
>  		table->size  = ARRAY_SIZE(skylake_mocs_table);
>  		table->table = skylake_mocs_table;
>  		result = true;
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 7234b2272481..f4b7fd132173 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3612,8 +3612,8 @@ static bool skl_needs_memory_bw_wa(struct intel_atomic_state *state)
>  static bool
>  intel_has_sagv(struct drm_i915_private *dev_priv)
>  {
> -	if (IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv) ||
> -	    IS_GEN10(dev_priv) || IS_ICELAKE(dev_priv))
> +	if (INTEL_GEN(dev_priv) >= 10 ||
> +	    IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv))
>  		return true;
>  
>  	if (IS_SKYLAKE(dev_priv) &&
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 63f0b1c0bf77..1d7dd506708a 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -3082,7 +3082,7 @@ int intel_power_domains_init(struct drm_i915_private *dev_priv)
>  	 * The enabling order will be from lower to higher indexed wells,
>  	 * the disabling order is reversed.
>  	 */
> -	if (IS_ICELAKE(dev_priv)) {
> +	if (INTEL_GEN(dev_priv) >= 11) {
>  		err = set_power_wells(power_domains, icl_power_wells);
>  	} else if (IS_HASWELL(dev_priv)) {
>  		err = set_power_wells(power_domains, hsw_power_wells);
> @@ -3767,7 +3767,7 @@ void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool resume)
>  
>  	power_domains->initializing = true;
>  
> -	if (IS_ICELAKE(dev_priv)) {
> +	if (INTEL_GEN(dev_priv) >= 11) {
>  		icl_display_core_init(dev_priv, resume);
>  	} else if (IS_GEN10(dev_priv)) {
>  		cnl_display_core_init(dev_priv, resume);
> @@ -3898,7 +3898,7 @@ void intel_power_domains_suspend(struct drm_i915_private *dev_priv,
>  		intel_power_domains_verify_state(dev_priv);
>  	}
>  
> -	if (IS_ICELAKE(dev_priv))
> +	if (INTEL_GEN(dev_priv) >= 11)
>  		icl_display_core_uninit(dev_priv);
>  	else if (IS_GEN10(dev_priv))
>  		cnl_display_core_uninit(dev_priv);

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

Re: [RFC 7/8] drm/i915: Sort platform if cases from newer-to-older.

[Jani Nikula]

On Thu, 18 Oct 2018, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> Let's use this whenever it makes sense and code gets
> easier to read.

Ack on this general direction.

BR,
Jani.


>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c        | 18 +++++++++---------
>  drivers/gpu/drm/i915/intel_dp.c         | 24 +++++++++++++-----------
>  drivers/gpu/drm/i915/intel_pm.c         |  6 +++---
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 12 ++++++------
>  drivers/gpu/drm/i915/intel_uncore.c     |  2 +-
>  5 files changed, 32 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 10b5314f266c..498521a3bc21 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1737,16 +1737,16 @@ static void intel_ddi_clock_get(struct intel_encoder *encoder,
>  {
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>  
> -	if (INTEL_GEN(dev_priv) <= 8)
> -		hsw_ddi_clock_get(encoder, pipe_config);
> +	if (INTEL_GEN(dev_priv) >= 11)
> +		icl_ddi_clock_get(encoder, pipe_config);
> +	else if (IS_GEN10(dev_priv))
> +		cnl_ddi_clock_get(encoder, pipe_config);
>  	else if (IS_GEN9_BC(dev_priv))
>  		skl_ddi_clock_get(encoder, pipe_config);
>  	else if (IS_GEN9_LP(dev_priv))
>  		bxt_ddi_clock_get(encoder, pipe_config);
> -	else if (IS_GEN10(dev_priv))
> -		cnl_ddi_clock_get(encoder, pipe_config);
> -	else if (INTEL_GEN(dev_priv) >= 11)
> -		icl_ddi_clock_get(encoder, pipe_config);
> +	else if (INTEL_GEN(dev_priv) <= 8)
> +		hsw_ddi_clock_get(encoder, pipe_config);
>  }
>  
>  void intel_ddi_set_pipe_settings(const struct intel_crtc_state *crtc_state)
> @@ -3373,10 +3373,10 @@ static bool intel_ddi_is_audio_enabled(struct drm_i915_private *dev_priv,
>  void intel_ddi_compute_min_voltage_level(struct drm_i915_private *dev_priv,
>  					 struct intel_crtc_state *crtc_state)
>  {
> -	if (IS_GEN10(dev_priv) && crtc_state->port_clock > 594000)
> -		crtc_state->min_voltage_level = 2;
> -	else if (INTEL_GEN(dev_priv) >= 11 && crtc_state->port_clock > 594000)
> +	if (INTEL_GEN(dev_priv) >= 11 && crtc_state->port_clock > 594000)
>  		crtc_state->min_voltage_level = 1;
> +	else if (IS_GEN10(dev_priv) && crtc_state->port_clock > 594000)
> +		crtc_state->min_voltage_level = 2;
>  }
>  
>  void intel_ddi_get_config(struct intel_encoder *encoder,
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 0ea0414ccef4..3c13a49b4a7a 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -5035,20 +5035,22 @@ bool intel_digital_port_connected(struct intel_encoder *encoder)
>  			return g4x_digital_port_connected(encoder);
>  	}
>  
> -	if (IS_GEN5(dev_priv))
> -		return ilk_digital_port_connected(encoder);
> -	else if (IS_GEN6(dev_priv))
> -		return snb_digital_port_connected(encoder);
> -	else if (IS_GEN7(dev_priv))
> -		return ivb_digital_port_connected(encoder);
> -	else if (IS_GEN8(dev_priv))
> -		return bdw_digital_port_connected(encoder);
> +	if (INTEL_GEN(dev_priv) >= 11)
> +		return icl_digital_port_connected(encoder);
> +	else if (IS_GEN10(dev_priv) || IS_GEN9_BC(dev_priv))
> +		return spt_digital_port_connected(encoder);
>  	else if (IS_GEN9_LP(dev_priv))
>  		return bxt_digital_port_connected(encoder);
> -	else if (IS_GEN9_BC(dev_priv) || IS_GEN10(dev_priv))
> -		return spt_digital_port_connected(encoder);
> +	else if (IS_GEN8(dev_priv))
> +		return bdw_digital_port_connected(encoder);
> +	else if (IS_GEN7(dev_priv))
> +		return ivb_digital_port_connected(encoder);
> +	else if (IS_GEN6(dev_priv))
> +		return snb_digital_port_connected(encoder);
> +	else if (IS_GEN5(dev_priv))
> +		return ilk_digital_port_connected(encoder);
>  	else
> -		return icl_digital_port_connected(encoder);
> +		return false;
>  }
>  
>  static struct edid *
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index f4b7fd132173..5663b7059467 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3724,12 +3724,12 @@ bool intel_can_enable_sagv(struct drm_atomic_state *state)
>  	if (!intel_has_sagv(dev_priv))
>  		return false;
>  
> -	if (IS_GEN9(dev_priv))
> -		sagv_block_time_us = 30;
> +	if (INTEL_GEN(dev_priv) >= 11)
> +		sagv_block_time_us = 10;
>  	else if (IS_GEN10(dev_priv))
>  		sagv_block_time_us = 20;
>  	else
> -		sagv_block_time_us = 10;
> +		sagv_block_time_us = 30;
>  
>  	/*
>  	 * SKL+ workaround: bspec recommends we disable the SAGV when we have
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 1d7dd506708a..9472cd895ea6 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -3084,14 +3084,10 @@ int intel_power_domains_init(struct drm_i915_private *dev_priv)
>  	 */
>  	if (INTEL_GEN(dev_priv) >= 11) {
>  		err = set_power_wells(power_domains, icl_power_wells);
> -	} else if (IS_HASWELL(dev_priv)) {
> -		err = set_power_wells(power_domains, hsw_power_wells);
> -	} else if (IS_BROADWELL(dev_priv)) {
> -		err = set_power_wells(power_domains, bdw_power_wells);
> -	} else if (IS_GEN9_BC(dev_priv)) {
> -		err = set_power_wells(power_domains, skl_power_wells);
>  	} else if (IS_GEN10(dev_priv)) {
>  		err = set_power_wells(power_domains, cnl_power_wells);
> +	} else if (IS_GEN9_BC(dev_priv)) {
> +		err = set_power_wells(power_domains, skl_power_wells);
>  
>  		/*
>  		 * DDI and Aux IO are getting enabled for all ports
> @@ -3106,8 +3102,12 @@ int intel_power_domains_init(struct drm_i915_private *dev_priv)
>  		err = set_power_wells(power_domains, bxt_power_wells);
>  	} else if (IS_GEMINILAKE(dev_priv)) {
>  		err = set_power_wells(power_domains, glk_power_wells);
> +	} else if (IS_BROADWELL(dev_priv)) {
> +		err = set_power_wells(power_domains, bdw_power_wells);
>  	} else if (IS_CHERRYVIEW(dev_priv)) {
>  		err = set_power_wells(power_domains, chv_power_wells);
> +	} else if (IS_HASWELL(dev_priv)) {
> +		err = set_power_wells(power_domains, hsw_power_wells);
>  	} else if (IS_VALLEYVIEW(dev_priv)) {
>  		err = set_power_wells(power_domains, vlv_power_wells);
>  	} else if (IS_I830(dev_priv)) {
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index 3ad302c66254..9289515108c3 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -1437,7 +1437,7 @@ static void intel_uncore_fw_domains_init(struct drm_i915_private *dev_priv)
>  				       FORCEWAKE_MEDIA_VEBOX_GEN11(i),
>  				       FORCEWAKE_ACK_MEDIA_VEBOX_GEN11(i));
>  		}
> -	} else if (IS_GEN9(dev_priv) || IS_GEN10(dev_priv)) {
> +	} else if (IS_GEN10(dev_priv) || IS_GEN9(dev_priv)) {
>  		dev_priv->uncore.funcs.force_wake_get =
>  			fw_domains_get_with_fallback;
>  		dev_priv->uncore.funcs.force_wake_put = fw_domains_put;

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

Re: [RFC 8/8] drm/i915: Simplify intel_has_sagv function.

[Jani Nikula]

On Thu, 18 Oct 2018, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> Let's just handle SKL as special case instead of listing
> platform by platform.
>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 5663b7059467..97f191b081e0 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3612,15 +3612,11 @@ static bool skl_needs_memory_bw_wa(struct intel_atomic_state *state)
>  static bool
>  intel_has_sagv(struct drm_i915_private *dev_priv)
>  {
> -	if (INTEL_GEN(dev_priv) >= 10 ||
> -	    IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv))
> -		return true;
> -
>  	if (IS_SKYLAKE(dev_priv) &&
>  	    dev_priv->sagv_status != I915_SAGV_NOT_CONTROLLED)
>  		return true;
>  
> -	return false;
> +	return IS_GEN9_BC(dev_priv) || INTEL_GEN(dev_priv) >= 10;
>  }

You can go one step further and remove the Skylake special case even for
I915_SAGV_NOT_CONTROLLED:

	return (IS_GEN9_BC(dev_priv) || INTEL_GEN(dev_priv) >= 10) && 
		dev_priv->sagv_status != I915_SAGV_NOT_CONTROLLED;

dev_priv->sagv_status == I915_SAGV_NOT_CONTROLLED only on SKL, but this
function doesn't have to know that.

BR,
Jani.

>  
>  /*

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

Re: [RFC 2/8] drm/i915: Use the ddi_ports info to kill ugly CNL_WITH_PORT_F.

[Daniel Vetter]

On Fri, Oct 19, 2018 at 10:39:46AM +0300, Jani Nikula wrote:
> On Thu, 18 Oct 2018, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> > Now that we have the number of ddi ports information available
> > let's use it instead of that ugly platform macro.
> >
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.c         | 10 ++++++++++
> >  drivers/gpu/drm/i915/i915_drv.h         |  2 --
> >  drivers/gpu/drm/i915/i915_irq.c         |  5 ++---
> >  drivers/gpu/drm/i915/intel_dp.c         |  2 +-
> >  drivers/gpu/drm/i915/intel_hotplug.c    |  2 +-
> >  drivers/gpu/drm/i915/intel_runtime_pm.c |  2 +-
> >  6 files changed, 15 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > index baac35f698f9..83ab325c6675 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -239,6 +239,14 @@ intel_virt_detect_pch(const struct drm_i915_private *dev_priv)
> >  	return id;
> >  }
> >  
> > +static void intel_adjust_skus_differences(struct drm_i915_private *dev_priv)
> > +{
> > +	/* Some Cannonlake SKUs have Port F */
> > +	if (IS_CANNONLAKE(dev_priv) &&
> > +	    (INTEL_DEVID(dev_priv) & 0x0004) == 0x0004)
> > +		mkwrite_device_info(dev_priv)->ddi_ports = 6;
> > +}
> 
> I'd like reduce the amount of device info stuff we adjust at probe, not
> increase.
> 
> Long term, I'd like to make dev_priv->info a const pointer to the const
> data in i915_pci.c, and make it truly immutable. We could have an
> additional runtime device info to complement that, but this approach
> calls for a more concrete split in const and non-const data, not "mostly
> const but sometimes mutable".
> 
> I think I've chatted about this with Chris too.

This should be easy to fix since it's encoded in the pci id. Just need a
new separate entry just for this one.
-Daniel

> 
> BR,
> Jani.
> 
> > +
> >  static void intel_detect_pch(struct drm_i915_private *dev_priv)
> >  {
> >  	struct pci_dev *pch = NULL;
> > @@ -904,6 +912,8 @@ static int i915_driver_init_early(struct drm_i915_private *dev_priv)
> >  	if (ret < 0)
> >  		goto err_workqueues;
> >  
> > +	intel_adjust_skus_differences(dev_priv);
> > +
> >  	/* This must be called before any calls to HAS_PCH_* */
> >  	intel_detect_pch(dev_priv);
> >  
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 7ad232849268..99e42df79ed8 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -2485,8 +2485,6 @@ intel_info(const struct drm_i915_private *dev_priv)
> >  				 (dev_priv)->info.gt == 2)
> >  #define IS_CFL_GT3(dev_priv)	(IS_COFFEELAKE(dev_priv) && \
> >  				 (dev_priv)->info.gt == 3)
> > -#define IS_CNL_WITH_PORT_F(dev_priv)   (IS_CANNONLAKE(dev_priv) && \
> > -					(INTEL_DEVID(dev_priv) & 0x0004) == 0x0004)
> >  
> >  #define IS_ALPHA_SUPPORT(intel_info) ((intel_info)->is_alpha_support)
> >  
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > index 5d1f53723388..63d676de3840 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -2782,8 +2782,7 @@ gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl)
> >  			if (INTEL_GEN(dev_priv) >= 11)
> >  				tmp_mask |= ICL_AUX_CHANNEL_E;
> >  
> > -			if (IS_CNL_WITH_PORT_F(dev_priv) ||
> > -			    INTEL_GEN(dev_priv) >= 11)
> > +			if (INTEL_INFO(dev_priv)->ddi_ports >= 6)
> >  				tmp_mask |= CNL_AUX_CHANNEL_F;
> >  
> >  			if (iir & tmp_mask) {
> > @@ -4220,7 +4219,7 @@ static void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv)
> >  	if (INTEL_GEN(dev_priv) >= 11)
> >  		de_port_masked |= ICL_AUX_CHANNEL_E;
> >  
> > -	if (IS_CNL_WITH_PORT_F(dev_priv) || INTEL_GEN(dev_priv) >= 11)
> > +	if (INTEL_INFO(dev_priv)->ddi_ports >= 6)
> >  		de_port_masked |= CNL_AUX_CHANNEL_F;
> >  
> >  	de_pipe_enables = de_pipe_masked | GEN8_PIPE_VBLANK |
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index 3384a9bbdafd..0ea0414ccef4 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -402,7 +402,7 @@ static int cnl_max_source_rate(struct intel_dp *intel_dp)
> >  		return 540000;
> >  
> >  	/* For this SKU 8.1G is supported in all ports */
> > -	if (IS_CNL_WITH_PORT_F(dev_priv))
> > +	if (INTEL_INFO(dev_priv)->ddi_ports == 6)
> >  		return 810000;
> >  
> >  	/* For other SKUs, max rate on ports A and D is 5.4G */
> > diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c
> > index 648a13c6043c..05d1faf77048 100644
> > --- a/drivers/gpu/drm/i915/intel_hotplug.c
> > +++ b/drivers/gpu/drm/i915/intel_hotplug.c
> > @@ -101,7 +101,7 @@ enum hpd_pin intel_hpd_pin_default(struct drm_i915_private *dev_priv,
> >  	case PORT_E:
> >  		return HPD_PORT_E;
> >  	case PORT_F:
> > -		if (IS_CNL_WITH_PORT_F(dev_priv))
> > +		if (IS_GEN10(dev_priv))
> >  			return HPD_PORT_E;
> >  		return HPD_PORT_F;
> >  	default:
> > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > index 31a49bdcf193..80e14be11279 100644
> > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > @@ -3099,7 +3099,7 @@ int intel_power_domains_init(struct drm_i915_private *dev_priv)
> >  		 * timeouts, lets remove them from the list
> >  		 * for the SKUs without port F.
> >  		 */
> > -		if (!IS_CNL_WITH_PORT_F(dev_priv))
> > +		if (INTEL_INFO(dev_priv)->ddi_ports == 5)
> >  			power_domains->power_well_count -= 2;
> >  
> >  	} else if (IS_BROXTON(dev_priv)) {
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 4/8] drm/i915: Group gen 10 display.

[Daniel Vetter]

On Fri, Oct 19, 2018 at 11:03:53AM +0300, Jani Nikula wrote:
> On Thu, 18 Oct 2018, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> > Continuing with the goal of use less platform codenames:
> > let's group platforms who has gen10 display.
> 
> Ahah, so this answers my question in the previous patch. ;)
> 
> So we already have HAS_GMCH_DISPLAY().

We also have HAS_DDI, which I guess you could call gen8 display :-)

> If we're going to make gen10 display a thing, should we not generalize
> this to have an actual display gen in device info? Of course for most
> platforms it's going to be identical to the gem gen.
> 
> The question becomes, is display gen accurate enough? It's not enough to
> cover all of Geminilake I believe. Which makes me think, should we just
> add tons more device info flags to cover features at a detailed level? I
> think that's what Joonas advocates. Perhaps it bloats the device info,
> and causes increase in the number of device info blocks. But my gut
> feeling says together with truly immutable device info, there are
> compiler optimization benefits to be had.
> 
> Also, currently we "inherit" device info using truly obnoxious macro
> layering where you have to work hard to trace the macros to find out
> what is set for a given platform. It doesn't have to be that way. We
> could move parts of it to separate but shared features structs, and add
> pointers to them.
> 
> Anyway, thanks for rolling this series as a starting point for
> discussion. Even if I'm not buying the changes as-is. ;)

Yeah, stuffing this into intel_info imo makes sense. As long as it's
booleans they should be fairly cheap.
-Daniel
> 
> 
> BR,
> Jani.
> 
> >
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h      | 2 ++
> >  drivers/gpu/drm/i915/intel_cdclk.c   | 2 +-
> >  drivers/gpu/drm/i915/intel_color.c   | 2 +-
> >  drivers/gpu/drm/i915/intel_display.c | 4 ++--
> >  drivers/gpu/drm/i915/intel_sprite.c  | 2 +-
> >  5 files changed, 7 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index d19b38ef145b..6436fedfe561 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -2637,6 +2637,8 @@ intel_info(const struct drm_i915_private *dev_priv)
> >  #define SUPPORTS_TV(dev_priv)		((dev_priv)->info.supports_tv)
> >  #define I915_HAS_HOTPLUG(dev_priv)	((dev_priv)->info.has_hotplug)
> >  
> > +#define HAS_DISPLAY_10(dev_priv) (IS_GEN10(dev_priv) || IS_GEMINILAKE(dev_priv))
> > +
> >  #define HAS_FW_BLC(dev_priv) 	(INTEL_GEN(dev_priv) > 2)
> >  #define HAS_FBC(dev_priv)	((dev_priv)->info.has_fbc)
> >  #define HAS_CUR_FBC(dev_priv)	(!HAS_GMCH_DISPLAY(dev_priv) && INTEL_GEN(dev_priv) >= 7)
> > diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
> > index 4aa6500604cc..b315b70fd49c 100644
> > --- a/drivers/gpu/drm/i915/intel_cdclk.c
> > +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> > @@ -2181,7 +2181,7 @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state)
> >  	    crtc_state->has_audio &&
> >  	    crtc_state->port_clock >= 540000 &&
> >  	    crtc_state->lane_count == 4) {
> > -		if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv)) {
> > +		if (HAS_DISPLAY_10(dev_priv)) {
> >  			/* Display WA #1145: glk,cnl */
> >  			min_cdclk = max(316800, min_cdclk);
> >  		} else if (IS_GEN9(dev_priv) || IS_BROADWELL(dev_priv)) {
> > diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
> > index 5127da286a2b..d5f67b9c9698 100644
> > --- a/drivers/gpu/drm/i915/intel_color.c
> > +++ b/drivers/gpu/drm/i915/intel_color.c
> > @@ -659,7 +659,7 @@ void intel_color_init(struct drm_crtc *crtc)
> >  		   IS_BROXTON(dev_priv)) {
> >  		dev_priv->display.load_csc_matrix = ilk_load_csc_matrix;
> >  		dev_priv->display.load_luts = broadwell_load_luts;
> > -	} else if (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) {
> > +	} else if (HAS_DISPLAY_10(dev_priv)) {
> >  		dev_priv->display.load_csc_matrix = ilk_load_csc_matrix;
> >  		dev_priv->display.load_luts = glk_load_luts;
> >  	} else {
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 7b04b8436b6d..1abf79a4ee91 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -5730,7 +5730,7 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
> >  	intel_crtc->active = true;
> >  
> >  	/* Display WA #1180: WaDisableScalarClockGating: glk, cnl */
> > -	psl_clkgate_wa = (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) &&
> > +	psl_clkgate_wa = (HAS_DISPLAY_10(dev_priv)) &&
> >  			 pipe_config->pch_pfit.enabled;
> >  	if (psl_clkgate_wa)
> >  		glk_pipe_scaler_clock_gating_wa(dev_priv, pipe, true);
> > @@ -15565,7 +15565,7 @@ get_encoder_power_domains(struct drm_i915_private *dev_priv)
> >  static void intel_early_display_was(struct drm_i915_private *dev_priv)
> >  {
> >  	/* Display WA #1185 WaDisableDARBFClkGating:cnl,glk */
> > -	if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
> > +	if (HAS_DISPLAY_10(dev_priv))
> >  		I915_WRITE(GEN9_CLKGATE_DIS_0, I915_READ(GEN9_CLKGATE_DIS_0) |
> >  			   DARBF_GATING_DIS);
> >  
> > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> > index 7cd59eee5cad..912ab7b9d89a 100644
> > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > @@ -1330,7 +1330,7 @@ static int skl_plane_check_dst_coordinates(const struct intel_crtc_state *crtc_s
> >  	 * than the cursor ending less than 4 pixels from the left edge of the
> >  	 * screen may cause FIFO underflow and display corruption.
> >  	 */
> > -	if ((IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) &&
> > +	if (HAS_DISPLAY_10(dev_priv) &&
> >  	    (crtc_x + crtc_w < 4 || crtc_x > pipe_src_w - 4)) {
> >  		DRM_DEBUG_KMS("requested plane X %s position %d invalid (valid range %d-%d)\n",
> >  			      crtc_x + crtc_w < 4 ? "end" : "start",
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 4/8] drm/i915: Group gen 10 display.

[Joonas Lahtinen]

Quoting Daniel Vetter (2018-10-19 11:30:46)
> On Fri, Oct 19, 2018 at 11:03:53AM +0300, Jani Nikula wrote:
> > On Thu, 18 Oct 2018, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> > > Continuing with the goal of use less platform codenames:
> > > let's group platforms who has gen10 display.
> > 
> > Ahah, so this answers my question in the previous patch. ;)
> > 
> > So we already have HAS_GMCH_DISPLAY().
> 
> We also have HAS_DDI, which I guess you could call gen8 display :-)
> 
> > If we're going to make gen10 display a thing, should we not generalize
> > this to have an actual display gen in device info? Of course for most
> > platforms it's going to be identical to the gem gen.
> > 
> > The question becomes, is display gen accurate enough? It's not enough to
> > cover all of Geminilake I believe. Which makes me think, should we just
> > add tons more device info flags to cover features at a detailed level? I
> > think that's what Joonas advocates. Perhaps it bloats the device info,
> > and causes increase in the number of device info blocks. But my gut
> > feeling says together with truly immutable device info, there are
> > compiler optimization benefits to be had.
> > 
> > Also, currently we "inherit" device info using truly obnoxious macro
> > layering where you have to work hard to trace the macros to find out
> > what is set for a given platform. It doesn't have to be that way. We
> > could move parts of it to separate but shared features structs, and add
> > pointers to them.
> > 
> > Anyway, thanks for rolling this series as a starting point for
> > discussion. Even if I'm not buying the changes as-is. ;)
> 
> Yeah, stuffing this into intel_info imo makes sense. As long as it's
> booleans they should be fairly cheap.

Like Jani mentioned. I'm all in favour of going with a number of has_*
flags, when there is mix and match between different gens.

Regards, Joonas

> -Daniel
> > 
> > 
> > BR,
> > Jani.
> > 
> > >
> > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/i915_drv.h      | 2 ++
> > >  drivers/gpu/drm/i915/intel_cdclk.c   | 2 +-
> > >  drivers/gpu/drm/i915/intel_color.c   | 2 +-
> > >  drivers/gpu/drm/i915/intel_display.c | 4 ++--
> > >  drivers/gpu/drm/i915/intel_sprite.c  | 2 +-
> > >  5 files changed, 7 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > > index d19b38ef145b..6436fedfe561 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > @@ -2637,6 +2637,8 @@ intel_info(const struct drm_i915_private *dev_priv)
> > >  #define SUPPORTS_TV(dev_priv)              ((dev_priv)->info.supports_tv)
> > >  #define I915_HAS_HOTPLUG(dev_priv) ((dev_priv)->info.has_hotplug)
> > >  
> > > +#define HAS_DISPLAY_10(dev_priv) (IS_GEN10(dev_priv) || IS_GEMINILAKE(dev_priv))
> > > +
> > >  #define HAS_FW_BLC(dev_priv)       (INTEL_GEN(dev_priv) > 2)
> > >  #define HAS_FBC(dev_priv)  ((dev_priv)->info.has_fbc)
> > >  #define HAS_CUR_FBC(dev_priv)      (!HAS_GMCH_DISPLAY(dev_priv) && INTEL_GEN(dev_priv) >= 7)
> > > diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
> > > index 4aa6500604cc..b315b70fd49c 100644
> > > --- a/drivers/gpu/drm/i915/intel_cdclk.c
> > > +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> > > @@ -2181,7 +2181,7 @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state)
> > >         crtc_state->has_audio &&
> > >         crtc_state->port_clock >= 540000 &&
> > >         crtc_state->lane_count == 4) {
> > > -           if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv)) {
> > > +           if (HAS_DISPLAY_10(dev_priv)) {
> > >                     /* Display WA #1145: glk,cnl */
> > >                     min_cdclk = max(316800, min_cdclk);
> > >             } else if (IS_GEN9(dev_priv) || IS_BROADWELL(dev_priv)) {
> > > diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
> > > index 5127da286a2b..d5f67b9c9698 100644
> > > --- a/drivers/gpu/drm/i915/intel_color.c
> > > +++ b/drivers/gpu/drm/i915/intel_color.c
> > > @@ -659,7 +659,7 @@ void intel_color_init(struct drm_crtc *crtc)
> > >                IS_BROXTON(dev_priv)) {
> > >             dev_priv->display.load_csc_matrix = ilk_load_csc_matrix;
> > >             dev_priv->display.load_luts = broadwell_load_luts;
> > > -   } else if (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) {
> > > +   } else if (HAS_DISPLAY_10(dev_priv)) {
> > >             dev_priv->display.load_csc_matrix = ilk_load_csc_matrix;
> > >             dev_priv->display.load_luts = glk_load_luts;
> > >     } else {
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > index 7b04b8436b6d..1abf79a4ee91 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -5730,7 +5730,7 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
> > >     intel_crtc->active = true;
> > >  
> > >     /* Display WA #1180: WaDisableScalarClockGating: glk, cnl */
> > > -   psl_clkgate_wa = (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) &&
> > > +   psl_clkgate_wa = (HAS_DISPLAY_10(dev_priv)) &&
> > >                      pipe_config->pch_pfit.enabled;
> > >     if (psl_clkgate_wa)
> > >             glk_pipe_scaler_clock_gating_wa(dev_priv, pipe, true);
> > > @@ -15565,7 +15565,7 @@ get_encoder_power_domains(struct drm_i915_private *dev_priv)
> > >  static void intel_early_display_was(struct drm_i915_private *dev_priv)
> > >  {
> > >     /* Display WA #1185 WaDisableDARBFClkGating:cnl,glk */
> > > -   if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
> > > +   if (HAS_DISPLAY_10(dev_priv))
> > >             I915_WRITE(GEN9_CLKGATE_DIS_0, I915_READ(GEN9_CLKGATE_DIS_0) |
> > >                        DARBF_GATING_DIS);
> > >  
> > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> > > index 7cd59eee5cad..912ab7b9d89a 100644
> > > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > > @@ -1330,7 +1330,7 @@ static int skl_plane_check_dst_coordinates(const struct intel_crtc_state *crtc_s
> > >      * than the cursor ending less than 4 pixels from the left edge of the
> > >      * screen may cause FIFO underflow and display corruption.
> > >      */
> > > -   if ((IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) &&
> > > +   if (HAS_DISPLAY_10(dev_priv) &&
> > >         (crtc_x + crtc_w < 4 || crtc_x > pipe_src_w - 4)) {
> > >             DRM_DEBUG_KMS("requested plane X %s position %d invalid (valid range %d-%d)\n",
> > >                           crtc_x + crtc_w < 4 ? "end" : "start",
> > 
> > -- 
> > Jani Nikula, Intel Open Source Graphics Center
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
> _______________________________________________
> 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

Re: [RFC 4/8] drm/i915: Group gen 10 display.

[Ville Syrjälä]

On Fri, Oct 19, 2018 at 10:30:46AM +0200, Daniel Vetter wrote:
> On Fri, Oct 19, 2018 at 11:03:53AM +0300, Jani Nikula wrote:
> > On Thu, 18 Oct 2018, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> > > Continuing with the goal of use less platform codenames:
> > > let's group platforms who has gen10 display.
> > 
> > Ahah, so this answers my question in the previous patch. ;)
> > 
> > So we already have HAS_GMCH_DISPLAY().
> 
> We also have HAS_DDI, which I guess you could call gen8 display :-)

There's also these interesting gen designations in some old docs:
ctg/elk = gen5.5
ilk = gen5.75
I guess we could more or less call all of that as gen5 display.

And of course we have the other oddballs like vlv/chv which are
sort of mismash of i965, ctg/elk, ibx, and custom stuff. Also
pnv is mostly gen3 display except for a few bits that were
snatched from i965.

Not sure we have enough numbers for all that without resotring to
fractions. And no one could anyway remember what all the different
numbers mean.

> 
> > If we're going to make gen10 display a thing, should we not generalize
> > this to have an actual display gen in device info? Of course for most
> > platforms it's going to be identical to the gem gen.
> > 
> > The question becomes, is display gen accurate enough? It's not enough to
> > cover all of Geminilake I believe. Which makes me think, should we just
> > add tons more device info flags to cover features at a detailed level? I
> > think that's what Joonas advocates. Perhaps it bloats the device info,
> > and causes increase in the number of device info blocks. But my gut
> > feeling says together with truly immutable device info, there are
> > compiler optimization benefits to be had.
> > 
> > Also, currently we "inherit" device info using truly obnoxious macro
> > layering where you have to work hard to trace the macros to find out
> > what is set for a given platform. It doesn't have to be that way. We
> > could move parts of it to separate but shared features structs, and add
> > pointers to them.
> > 
> > Anyway, thanks for rolling this series as a starting point for
> > discussion. Even if I'm not buying the changes as-is. ;)
> 
> Yeah, stuffing this into intel_info imo makes sense. As long as it's
> booleans they should be fairly cheap.
> -Daniel
> > 
> > 
> > BR,
> > Jani.
> > 
> > >
> > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/i915_drv.h      | 2 ++
> > >  drivers/gpu/drm/i915/intel_cdclk.c   | 2 +-
> > >  drivers/gpu/drm/i915/intel_color.c   | 2 +-
> > >  drivers/gpu/drm/i915/intel_display.c | 4 ++--
> > >  drivers/gpu/drm/i915/intel_sprite.c  | 2 +-
> > >  5 files changed, 7 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > > index d19b38ef145b..6436fedfe561 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > @@ -2637,6 +2637,8 @@ intel_info(const struct drm_i915_private *dev_priv)
> > >  #define SUPPORTS_TV(dev_priv)		((dev_priv)->info.supports_tv)
> > >  #define I915_HAS_HOTPLUG(dev_priv)	((dev_priv)->info.has_hotplug)
> > >  
> > > +#define HAS_DISPLAY_10(dev_priv) (IS_GEN10(dev_priv) || IS_GEMINILAKE(dev_priv))
> > > +
> > >  #define HAS_FW_BLC(dev_priv) 	(INTEL_GEN(dev_priv) > 2)
> > >  #define HAS_FBC(dev_priv)	((dev_priv)->info.has_fbc)
> > >  #define HAS_CUR_FBC(dev_priv)	(!HAS_GMCH_DISPLAY(dev_priv) && INTEL_GEN(dev_priv) >= 7)
> > > diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
> > > index 4aa6500604cc..b315b70fd49c 100644
> > > --- a/drivers/gpu/drm/i915/intel_cdclk.c
> > > +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> > > @@ -2181,7 +2181,7 @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state)
> > >  	    crtc_state->has_audio &&
> > >  	    crtc_state->port_clock >= 540000 &&
> > >  	    crtc_state->lane_count == 4) {
> > > -		if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv)) {
> > > +		if (HAS_DISPLAY_10(dev_priv)) {
> > >  			/* Display WA #1145: glk,cnl */
> > >  			min_cdclk = max(316800, min_cdclk);
> > >  		} else if (IS_GEN9(dev_priv) || IS_BROADWELL(dev_priv)) {
> > > diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
> > > index 5127da286a2b..d5f67b9c9698 100644
> > > --- a/drivers/gpu/drm/i915/intel_color.c
> > > +++ b/drivers/gpu/drm/i915/intel_color.c
> > > @@ -659,7 +659,7 @@ void intel_color_init(struct drm_crtc *crtc)
> > >  		   IS_BROXTON(dev_priv)) {
> > >  		dev_priv->display.load_csc_matrix = ilk_load_csc_matrix;
> > >  		dev_priv->display.load_luts = broadwell_load_luts;
> > > -	} else if (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) {
> > > +	} else if (HAS_DISPLAY_10(dev_priv)) {
> > >  		dev_priv->display.load_csc_matrix = ilk_load_csc_matrix;
> > >  		dev_priv->display.load_luts = glk_load_luts;
> > >  	} else {
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > index 7b04b8436b6d..1abf79a4ee91 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -5730,7 +5730,7 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
> > >  	intel_crtc->active = true;
> > >  
> > >  	/* Display WA #1180: WaDisableScalarClockGating: glk, cnl */
> > > -	psl_clkgate_wa = (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) &&
> > > +	psl_clkgate_wa = (HAS_DISPLAY_10(dev_priv)) &&
> > >  			 pipe_config->pch_pfit.enabled;
> > >  	if (psl_clkgate_wa)
> > >  		glk_pipe_scaler_clock_gating_wa(dev_priv, pipe, true);
> > > @@ -15565,7 +15565,7 @@ get_encoder_power_domains(struct drm_i915_private *dev_priv)
> > >  static void intel_early_display_was(struct drm_i915_private *dev_priv)
> > >  {
> > >  	/* Display WA #1185 WaDisableDARBFClkGating:cnl,glk */
> > > -	if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
> > > +	if (HAS_DISPLAY_10(dev_priv))
> > >  		I915_WRITE(GEN9_CLKGATE_DIS_0, I915_READ(GEN9_CLKGATE_DIS_0) |
> > >  			   DARBF_GATING_DIS);
> > >  
> > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> > > index 7cd59eee5cad..912ab7b9d89a 100644
> > > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > > @@ -1330,7 +1330,7 @@ static int skl_plane_check_dst_coordinates(const struct intel_crtc_state *crtc_s
> > >  	 * than the cursor ending less than 4 pixels from the left edge of the
> > >  	 * screen may cause FIFO underflow and display corruption.
> > >  	 */
> > > -	if ((IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) &&
> > > +	if (HAS_DISPLAY_10(dev_priv) &&
> > >  	    (crtc_x + crtc_w < 4 || crtc_x > pipe_src_w - 4)) {
> > >  		DRM_DEBUG_KMS("requested plane X %s position %d invalid (valid range %d-%d)\n",
> > >  			      crtc_x + crtc_w < 4 ? "end" : "start",
> > 
> > -- 
> > Jani Nikula, Intel Open Source Graphics Center
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [RFC 3/8] drm/i915/gen10: Prefer gen number than platform codename.

[Ville Syrjälä]

On Thu, Oct 18, 2018 at 04:34:42PM -0700, Rodrigo Vivi wrote:
> Whenever possible let's move towards preferring gen number
> and or features instead of hard coding platform codename
> everywhere.

Not a big fan of this idea. The gen numbers are simply
confusing when talking about the display.

> 
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h         |  2 +-
>  drivers/gpu/drm/i915/intel_cdclk.c      |  6 +++---
>  drivers/gpu/drm/i915/intel_ddi.c        | 20 ++++++++++----------
>  drivers/gpu/drm/i915/intel_display.c    |  4 ++--
>  drivers/gpu/drm/i915/intel_dpll_mgr.c   |  2 +-
>  drivers/gpu/drm/i915/intel_mocs.c       |  2 +-
>  drivers/gpu/drm/i915/intel_pm.c         |  2 +-
>  drivers/gpu/drm/i915/intel_runtime_pm.c |  8 ++++----
>  8 files changed, 23 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 99e42df79ed8..d19b38ef145b 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2620,7 +2620,7 @@ intel_info(const struct drm_i915_private *dev_priv)
>  
>  /* WaRsDisableCoarsePowerGating:skl,cnl */
>  #define NEEDS_WaRsDisableCoarsePowerGating(dev_priv) \
> -	(IS_CANNONLAKE(dev_priv) || \
> +	(IS_GEN10(dev_priv) || \
>  	 IS_SKL_GT3(dev_priv) || IS_SKL_GT4(dev_priv))
>  
>  #define HAS_GMBUS_IRQ(dev_priv) (INTEL_GEN(dev_priv) >= 4)
> diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
> index 29075c763428..4aa6500604cc 100644
> --- a/drivers/gpu/drm/i915/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> @@ -2577,7 +2577,7 @@ void intel_update_max_cdclk(struct drm_i915_private *dev_priv)
>  			dev_priv->max_cdclk_freq = 648000;
>  		else
>  			dev_priv->max_cdclk_freq = 652800;
> -	} else if (IS_CANNONLAKE(dev_priv)) {
> +	} else if (IS_GEN10(dev_priv)) {
>  		dev_priv->max_cdclk_freq = 528000;
>  	} else if (IS_GEN9_BC(dev_priv)) {
>  		u32 limit = I915_READ(SKL_DFSM) & SKL_DFSM_CDCLK_LIMIT_MASK;
> @@ -2797,7 +2797,7 @@ void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv)
>  		dev_priv->display.set_cdclk = skl_set_cdclk;
>  		dev_priv->display.modeset_calc_cdclk =
>  			skl_modeset_calc_cdclk;
> -	} else if (IS_CANNONLAKE(dev_priv)) {
> +	} else if (IS_GEN10(dev_priv)) {
>  		dev_priv->display.set_cdclk = cnl_set_cdclk;
>  		dev_priv->display.modeset_calc_cdclk =
>  			cnl_modeset_calc_cdclk;
> @@ -2808,7 +2808,7 @@ void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv)
>  
>  	if (IS_ICELAKE(dev_priv))
>  		dev_priv->display.get_cdclk = icl_get_cdclk;
> -	else if (IS_CANNONLAKE(dev_priv))
> +	else if (IS_GEN10(dev_priv))
>  		dev_priv->display.get_cdclk = cnl_get_cdclk;
>  	else if (IS_GEN9_BC(dev_priv))
>  		dev_priv->display.get_cdclk = skl_get_cdclk;
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 6b9742baa5f2..cd627851f2a5 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -922,7 +922,7 @@ static int intel_ddi_hdmi_level(struct drm_i915_private *dev_priv, enum port por
>  		else
>  			n_entries = ARRAY_SIZE(icl_mg_phy_ddi_translations);
>  		default_entry = n_entries - 1;
> -	} else if (IS_CANNONLAKE(dev_priv)) {
> +	} else if (IS_GEN10(dev_priv)) {
>  		cnl_get_buf_trans_hdmi(dev_priv, &n_entries);
>  		default_entry = n_entries - 1;
>  	} else if (IS_GEN9_LP(dev_priv)) {
> @@ -1743,7 +1743,7 @@ static void intel_ddi_clock_get(struct intel_encoder *encoder,
>  		skl_ddi_clock_get(encoder, pipe_config);
>  	else if (IS_GEN9_LP(dev_priv))
>  		bxt_ddi_clock_get(encoder, pipe_config);
> -	else if (IS_CANNONLAKE(dev_priv))
> +	else if (IS_GEN10(dev_priv))
>  		cnl_ddi_clock_get(encoder, pipe_config);
>  	else if (IS_ICELAKE(dev_priv))
>  		icl_ddi_clock_get(encoder, pipe_config);
> @@ -2247,7 +2247,7 @@ u8 intel_ddi_dp_voltage_max(struct intel_encoder *encoder)
>  						&n_entries);
>  		else
>  			n_entries = ARRAY_SIZE(icl_mg_phy_ddi_translations);
> -	} else if (IS_CANNONLAKE(dev_priv)) {
> +	} else if (IS_GEN10(dev_priv)) {
>  		if (encoder->type == INTEL_OUTPUT_EDP)
>  			cnl_get_buf_trans_edp(dev_priv, &n_entries);
>  		else
> @@ -2719,7 +2719,7 @@ u32 bxt_signal_levels(struct intel_dp *intel_dp)
>  	if (IS_ICELAKE(dev_priv))
>  		icl_ddi_vswing_sequence(encoder, intel_dp->link_rate,
>  					level, encoder->type);
> -	else if (IS_CANNONLAKE(dev_priv))
> +	else if (IS_GEN10(dev_priv))
>  		cnl_ddi_vswing_sequence(encoder, level, encoder->type);
>  	else
>  		bxt_ddi_vswing_sequence(encoder, level, encoder->type);
> @@ -2837,7 +2837,7 @@ static void intel_ddi_clk_select(struct intel_encoder *encoder,
>  		if (!intel_port_is_combophy(dev_priv, port))
>  			I915_WRITE(DDI_CLK_SEL(port),
>  				   icl_pll_to_ddi_pll_sel(encoder, crtc_state));
> -	} else if (IS_CANNONLAKE(dev_priv)) {
> +	} else if (IS_GEN10(dev_priv)) {
>  		/* Configure DPCLKA_CFGCR0 to map the DPLL to the DDI. */
>  		val = I915_READ(DPCLKA_CFGCR0);
>  		val &= ~DPCLKA_CFGCR0_DDI_CLK_SEL_MASK(port);
> @@ -2878,7 +2878,7 @@ static void intel_ddi_clk_disable(struct intel_encoder *encoder)
>  	if (IS_ICELAKE(dev_priv)) {
>  		if (!intel_port_is_combophy(dev_priv, port))
>  			I915_WRITE(DDI_CLK_SEL(port), DDI_CLK_SEL_NONE);
> -	} else if (IS_CANNONLAKE(dev_priv)) {
> +	} else if (IS_GEN10(dev_priv)) {
>  		I915_WRITE(DPCLKA_CFGCR0, I915_READ(DPCLKA_CFGCR0) |
>  			   DPCLKA_CFGCR0_DDI_CLK_OFF(port));
>  	} else if (IS_GEN9_BC(dev_priv)) {
> @@ -2920,7 +2920,7 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
>  	if (IS_ICELAKE(dev_priv))
>  		icl_ddi_vswing_sequence(encoder, crtc_state->port_clock,
>  					level, encoder->type);
> -	else if (IS_CANNONLAKE(dev_priv))
> +	else if (IS_GEN10(dev_priv))
>  		cnl_ddi_vswing_sequence(encoder, level, encoder->type);
>  	else if (IS_GEN9_LP(dev_priv))
>  		bxt_ddi_vswing_sequence(encoder, level, encoder->type);
> @@ -2959,7 +2959,7 @@ static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder,
>  	if (IS_ICELAKE(dev_priv))
>  		icl_ddi_vswing_sequence(encoder, crtc_state->port_clock,
>  					level, INTEL_OUTPUT_HDMI);
> -	else if (IS_CANNONLAKE(dev_priv))
> +	else if (IS_GEN10(dev_priv))
>  		cnl_ddi_vswing_sequence(encoder, level, INTEL_OUTPUT_HDMI);
>  	else if (IS_GEN9_LP(dev_priv))
>  		bxt_ddi_vswing_sequence(encoder, level, INTEL_OUTPUT_HDMI);
> @@ -3373,7 +3373,7 @@ static bool intel_ddi_is_audio_enabled(struct drm_i915_private *dev_priv,
>  void intel_ddi_compute_min_voltage_level(struct drm_i915_private *dev_priv,
>  					 struct intel_crtc_state *crtc_state)
>  {
> -	if (IS_CANNONLAKE(dev_priv) && crtc_state->port_clock > 594000)
> +	if (IS_GEN10(dev_priv) && crtc_state->port_clock > 594000)
>  		crtc_state->min_voltage_level = 2;
>  	else if (IS_ICELAKE(dev_priv) && crtc_state->port_clock > 594000)
>  		crtc_state->min_voltage_level = 1;
> @@ -3742,7 +3742,7 @@ static bool intel_ddi_a_force_4_lanes(struct intel_digital_port *dport)
>  	 *             DDI_F what makes DDI_E useless. However for this
>  	 *             case let's trust VBT info.
>  	 */
> -	if (IS_CANNONLAKE(dev_priv) &&
> +	if (IS_GEN10(dev_priv) &&
>  	    !intel_bios_is_port_present(dev_priv, PORT_E))
>  		return true;
>  
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index fc7e3b0bd95c..7b04b8436b6d 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5269,7 +5269,7 @@ static bool needs_nv12_wa(struct drm_i915_private *dev_priv,
>  		return false;
>  
>  	if ((INTEL_GEN(dev_priv) == 9 && !IS_GEMINILAKE(dev_priv)) ||
> -	    IS_CANNONLAKE(dev_priv))
> +	    IS_GEN10(dev_priv))
>  		return true;
>  
>  	return false;
> @@ -9498,7 +9498,7 @@ static void haswell_get_ddi_port_state(struct intel_crtc *crtc,
>  
>  	if (IS_ICELAKE(dev_priv))
>  		icelake_get_ddi_pll(dev_priv, port, pipe_config);
> -	else if (IS_CANNONLAKE(dev_priv))
> +	else if (IS_GEN10(dev_priv))
>  		cannonlake_get_ddi_pll(dev_priv, port, pipe_config);
>  	else if (IS_GEN9_BC(dev_priv))
>  		skylake_get_ddi_pll(dev_priv, port, pipe_config);
> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> index 7bdff5ba58b9..ae92dc97d5aa 100644
> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> @@ -3202,7 +3202,7 @@ void intel_shared_dpll_init(struct drm_device *dev)
>  
>  	if (IS_ICELAKE(dev_priv))
>  		dpll_mgr = &icl_pll_mgr;
> -	else if (IS_CANNONLAKE(dev_priv))
> +	else if (IS_GEN10(dev_priv))
>  		dpll_mgr = &cnl_pll_mgr;
>  	else if (IS_GEN9_BC(dev_priv))
>  		dpll_mgr = &skl_pll_mgr;
> diff --git a/drivers/gpu/drm/i915/intel_mocs.c b/drivers/gpu/drm/i915/intel_mocs.c
> index 77e9871a8c9a..662eb087bb2e 100644
> --- a/drivers/gpu/drm/i915/intel_mocs.c
> +++ b/drivers/gpu/drm/i915/intel_mocs.c
> @@ -178,7 +178,7 @@ static bool get_mocs_settings(struct drm_i915_private *dev_priv,
>  {
>  	bool result = false;
>  
> -	if (IS_GEN9_BC(dev_priv) || IS_CANNONLAKE(dev_priv) ||
> +	if (IS_GEN9_BC(dev_priv) || IS_GEN10(dev_priv) ||
>  	    IS_ICELAKE(dev_priv)) {
>  		table->size  = ARRAY_SIZE(skylake_mocs_table);
>  		table->table = skylake_mocs_table;
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 67a4d0735291..7234b2272481 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3613,7 +3613,7 @@ static bool
>  intel_has_sagv(struct drm_i915_private *dev_priv)
>  {
>  	if (IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv) ||
> -	    IS_CANNONLAKE(dev_priv) || IS_ICELAKE(dev_priv))
> +	    IS_GEN10(dev_priv) || IS_ICELAKE(dev_priv))
>  		return true;
>  
>  	if (IS_SKYLAKE(dev_priv) &&
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 80e14be11279..63f0b1c0bf77 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -387,7 +387,7 @@ static void hsw_power_well_enable(struct drm_i915_private *dev_priv,
>  	hsw_wait_for_power_well_enable(dev_priv, power_well);
>  
>  	/* Display WA #1178: cnl */
> -	if (IS_CANNONLAKE(dev_priv) &&
> +	if (IS_GEN10(dev_priv) &&
>  	    pw_idx >= GLK_PW_CTL_IDX_AUX_B &&
>  	    pw_idx <= CNL_PW_CTL_IDX_AUX_F) {
>  		val = I915_READ(CNL_AUX_ANAOVRD1(pw_idx));
> @@ -3090,7 +3090,7 @@ int intel_power_domains_init(struct drm_i915_private *dev_priv)
>  		err = set_power_wells(power_domains, bdw_power_wells);
>  	} else if (IS_GEN9_BC(dev_priv)) {
>  		err = set_power_wells(power_domains, skl_power_wells);
> -	} else if (IS_CANNONLAKE(dev_priv)) {
> +	} else if (IS_GEN10(dev_priv)) {
>  		err = set_power_wells(power_domains, cnl_power_wells);
>  
>  		/*
> @@ -3769,7 +3769,7 @@ void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool resume)
>  
>  	if (IS_ICELAKE(dev_priv)) {
>  		icl_display_core_init(dev_priv, resume);
> -	} else if (IS_CANNONLAKE(dev_priv)) {
> +	} else if (IS_GEN10(dev_priv)) {
>  		cnl_display_core_init(dev_priv, resume);
>  	} else if (IS_GEN9_BC(dev_priv)) {
>  		skl_display_core_init(dev_priv, resume);
> @@ -3900,7 +3900,7 @@ void intel_power_domains_suspend(struct drm_i915_private *dev_priv,
>  
>  	if (IS_ICELAKE(dev_priv))
>  		icl_display_core_uninit(dev_priv);
> -	else if (IS_CANNONLAKE(dev_priv))
> +	else if (IS_GEN10(dev_priv))
>  		cnl_display_core_uninit(dev_priv);
>  	else if (IS_GEN9_BC(dev_priv))
>  		skl_display_core_uninit(dev_priv);
> -- 
> 2.19.1

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

Re: [RFC 4/8] drm/i915: Group gen 10 display.

[Rodrigo Vivi]

On Fri, Oct 19, 2018 at 01:33:08PM +0300, Ville Syrjälä wrote:
> On Fri, Oct 19, 2018 at 10:30:46AM +0200, Daniel Vetter wrote:
> > On Fri, Oct 19, 2018 at 11:03:53AM +0300, Jani Nikula wrote:
> > > On Thu, 18 Oct 2018, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> > > > Continuing with the goal of use less platform codenames:
> > > > let's group platforms who has gen10 display.
> > > 
> > > Ahah, so this answers my question in the previous patch. ;)
> > > 
> > > So we already have HAS_GMCH_DISPLAY().
> > 
> > We also have HAS_DDI, which I guess you could call gen8 display :-)
> 
> There's also these interesting gen designations in some old docs:
> ctg/elk = gen5.5
> ilk = gen5.75
> I guess we could more or less call all of that as gen5 display.
> 
> And of course we have the other oddballs like vlv/chv which are
> sort of mismash of i965, ctg/elk, ibx, and custom stuff. Also
> pnv is mostly gen3 display except for a few bits that were
> snatched from i965.
> 
> Not sure we have enough numbers for all that without resotring to
> fractions. And no one could anyway remember what all the different
> numbers mean.

Thanks for all the comments. Yeap, the idea is not to use this series
as is but just start the discussion and evolve it.

Gen number by itself doesn't fit to display indeed, but neither
display-gen-number because we have the fraction and cases like glk
that is gen10-stripped-display+bxt :/

Also platform names are not enough by itself, like cannnonlake-with-port-f
because the sku lacks on having a different name. If you take a look to our
internal branch the same is about to happen with icl soon....

So my idea was that we first use feature {name, or number of something}
whenever possible. On the secondary case we use groups of things like
HAS_GMCH_DISPLAY, HAS_GEN10_DISPLAY, HAS_VLV_DISPLAY. And on last case
we use gen numbers.

My idea of preferring gen over platform names in general was that for
platform name we cannot use >= ICELAKE... :/

Well, maybe having at least the basic display number to use in cases
like this would solve this case, but I'm afraid it can get tricky
in some common areas and the use of IS_GENx or IS_DISPLAY_x could become
blur and mixed up.

I will play more with this series, removing the controversial parts
and see what I can do with boolean "has_"

but for now still using gen >= 11 instead of icelake while we don't
have a better suggestion that covers that...

Thanks,
Rodrigo.

> 
> > 
> > > If we're going to make gen10 display a thing, should we not generalize
> > > this to have an actual display gen in device info? Of course for most
> > > platforms it's going to be identical to the gem gen.
> > > 
> > > The question becomes, is display gen accurate enough? It's not enough to
> > > cover all of Geminilake I believe. Which makes me think, should we just
> > > add tons more device info flags to cover features at a detailed level? I
> > > think that's what Joonas advocates. Perhaps it bloats the device info,
> > > and causes increase in the number of device info blocks. But my gut
> > > feeling says together with truly immutable device info, there are
> > > compiler optimization benefits to be had.
> > > 
> > > Also, currently we "inherit" device info using truly obnoxious macro
> > > layering where you have to work hard to trace the macros to find out
> > > what is set for a given platform. It doesn't have to be that way. We
> > > could move parts of it to separate but shared features structs, and add
> > > pointers to them.
> > > 
> > > Anyway, thanks for rolling this series as a starting point for
> > > discussion. Even if I'm not buying the changes as-is. ;)
> > 
> > Yeah, stuffing this into intel_info imo makes sense. As long as it's
> > booleans they should be fairly cheap.
> > -Daniel
> > > 
> > > 
> > > BR,
> > > Jani.
> > > 
> > > >
> > > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/i915_drv.h      | 2 ++
> > > >  drivers/gpu/drm/i915/intel_cdclk.c   | 2 +-
> > > >  drivers/gpu/drm/i915/intel_color.c   | 2 +-
> > > >  drivers/gpu/drm/i915/intel_display.c | 4 ++--
> > > >  drivers/gpu/drm/i915/intel_sprite.c  | 2 +-
> > > >  5 files changed, 7 insertions(+), 5 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > > > index d19b38ef145b..6436fedfe561 100644
> > > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > > @@ -2637,6 +2637,8 @@ intel_info(const struct drm_i915_private *dev_priv)
> > > >  #define SUPPORTS_TV(dev_priv)		((dev_priv)->info.supports_tv)
> > > >  #define I915_HAS_HOTPLUG(dev_priv)	((dev_priv)->info.has_hotplug)
> > > >  
> > > > +#define HAS_DISPLAY_10(dev_priv) (IS_GEN10(dev_priv) || IS_GEMINILAKE(dev_priv))
> > > > +
> > > >  #define HAS_FW_BLC(dev_priv) 	(INTEL_GEN(dev_priv) > 2)
> > > >  #define HAS_FBC(dev_priv)	((dev_priv)->info.has_fbc)
> > > >  #define HAS_CUR_FBC(dev_priv)	(!HAS_GMCH_DISPLAY(dev_priv) && INTEL_GEN(dev_priv) >= 7)
> > > > diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
> > > > index 4aa6500604cc..b315b70fd49c 100644
> > > > --- a/drivers/gpu/drm/i915/intel_cdclk.c
> > > > +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> > > > @@ -2181,7 +2181,7 @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state)
> > > >  	    crtc_state->has_audio &&
> > > >  	    crtc_state->port_clock >= 540000 &&
> > > >  	    crtc_state->lane_count == 4) {
> > > > -		if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv)) {
> > > > +		if (HAS_DISPLAY_10(dev_priv)) {
> > > >  			/* Display WA #1145: glk,cnl */
> > > >  			min_cdclk = max(316800, min_cdclk);
> > > >  		} else if (IS_GEN9(dev_priv) || IS_BROADWELL(dev_priv)) {
> > > > diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
> > > > index 5127da286a2b..d5f67b9c9698 100644
> > > > --- a/drivers/gpu/drm/i915/intel_color.c
> > > > +++ b/drivers/gpu/drm/i915/intel_color.c
> > > > @@ -659,7 +659,7 @@ void intel_color_init(struct drm_crtc *crtc)
> > > >  		   IS_BROXTON(dev_priv)) {
> > > >  		dev_priv->display.load_csc_matrix = ilk_load_csc_matrix;
> > > >  		dev_priv->display.load_luts = broadwell_load_luts;
> > > > -	} else if (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) {
> > > > +	} else if (HAS_DISPLAY_10(dev_priv)) {
> > > >  		dev_priv->display.load_csc_matrix = ilk_load_csc_matrix;
> > > >  		dev_priv->display.load_luts = glk_load_luts;
> > > >  	} else {
> > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > > index 7b04b8436b6d..1abf79a4ee91 100644
> > > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > > @@ -5730,7 +5730,7 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
> > > >  	intel_crtc->active = true;
> > > >  
> > > >  	/* Display WA #1180: WaDisableScalarClockGating: glk, cnl */
> > > > -	psl_clkgate_wa = (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) &&
> > > > +	psl_clkgate_wa = (HAS_DISPLAY_10(dev_priv)) &&
> > > >  			 pipe_config->pch_pfit.enabled;
> > > >  	if (psl_clkgate_wa)
> > > >  		glk_pipe_scaler_clock_gating_wa(dev_priv, pipe, true);
> > > > @@ -15565,7 +15565,7 @@ get_encoder_power_domains(struct drm_i915_private *dev_priv)
> > > >  static void intel_early_display_was(struct drm_i915_private *dev_priv)
> > > >  {
> > > >  	/* Display WA #1185 WaDisableDARBFClkGating:cnl,glk */
> > > > -	if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
> > > > +	if (HAS_DISPLAY_10(dev_priv))
> > > >  		I915_WRITE(GEN9_CLKGATE_DIS_0, I915_READ(GEN9_CLKGATE_DIS_0) |
> > > >  			   DARBF_GATING_DIS);
> > > >  
> > > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> > > > index 7cd59eee5cad..912ab7b9d89a 100644
> > > > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > > > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > > > @@ -1330,7 +1330,7 @@ static int skl_plane_check_dst_coordinates(const struct intel_crtc_state *crtc_s
> > > >  	 * than the cursor ending less than 4 pixels from the left edge of the
> > > >  	 * screen may cause FIFO underflow and display corruption.
> > > >  	 */
> > > > -	if ((IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) &&
> > > > +	if (HAS_DISPLAY_10(dev_priv) &&
> > > >  	    (crtc_x + crtc_w < 4 || crtc_x > pipe_src_w - 4)) {
> > > >  		DRM_DEBUG_KMS("requested plane X %s position %d invalid (valid range %d-%d)\n",
> > > >  			      crtc_x + crtc_w < 4 ? "end" : "start",
> > > 
> > > -- 
> > > Jani Nikula, Intel Open Source Graphics Center
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 
> > -- 
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 2/8] drm/i915: Use the ddi_ports info to kill ugly CNL_WITH_PORT_F.

[Lucas De Marchi]

On Fri, Oct 19, 2018 at 10:39:46AM +0300, Jani Nikula wrote:
> On Thu, 18 Oct 2018, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> > Now that we have the number of ddi ports information available
> > let's use it instead of that ugly platform macro.
> >
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.c         | 10 ++++++++++
> >  drivers/gpu/drm/i915/i915_drv.h         |  2 --
> >  drivers/gpu/drm/i915/i915_irq.c         |  5 ++---
> >  drivers/gpu/drm/i915/intel_dp.c         |  2 +-
> >  drivers/gpu/drm/i915/intel_hotplug.c    |  2 +-
> >  drivers/gpu/drm/i915/intel_runtime_pm.c |  2 +-
> >  6 files changed, 15 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > index baac35f698f9..83ab325c6675 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -239,6 +239,14 @@ intel_virt_detect_pch(const struct drm_i915_private *dev_priv)
> >  	return id;
> >  }
> >  
> > +static void intel_adjust_skus_differences(struct drm_i915_private *dev_priv)
> > +{
> > +	/* Some Cannonlake SKUs have Port F */
> > +	if (IS_CANNONLAKE(dev_priv) &&
> > +	    (INTEL_DEVID(dev_priv) & 0x0004) == 0x0004)
> > +		mkwrite_device_info(dev_priv)->ddi_ports = 6;
> > +}
> 
> I'd like reduce the amount of device info stuff we adjust at probe, not
> increase.
> 
> Long term, I'd like to make dev_priv->info a const pointer to the const
> data in i915_pci.c, and make it truly immutable. We could have an
> additional runtime device info to complement that, but this approach
> calls for a more concrete split in const and non-const data, not "mostly
> const but sometimes mutable".

I agree with this. I'd also like to have a const dev_info. What I think we need is to split
in what is const what can be changed in runtime/initialization.

However, I'm not fond on using the number of ddi ports to take decisions in the code not
related to the number of ports, but rather with other things of that sku. See below.

> 
> I think I've chatted about this with Chris too.
> 
> BR,
> Jani.
> 
> > +
> >  static void intel_detect_pch(struct drm_i915_private *dev_priv)
> >  {
> >  	struct pci_dev *pch = NULL;
> > @@ -904,6 +912,8 @@ static int i915_driver_init_early(struct drm_i915_private *dev_priv)
> >  	if (ret < 0)
> >  		goto err_workqueues;
> >  
> > +	intel_adjust_skus_differences(dev_priv);
> > +
> >  	/* This must be called before any calls to HAS_PCH_* */
> >  	intel_detect_pch(dev_priv);
> >  
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 7ad232849268..99e42df79ed8 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -2485,8 +2485,6 @@ intel_info(const struct drm_i915_private *dev_priv)
> >  				 (dev_priv)->info.gt == 2)
> >  #define IS_CFL_GT3(dev_priv)	(IS_COFFEELAKE(dev_priv) && \
> >  				 (dev_priv)->info.gt == 3)
> > -#define IS_CNL_WITH_PORT_F(dev_priv)   (IS_CANNONLAKE(dev_priv) && \
> > -					(INTEL_DEVID(dev_priv) & 0x0004) == 0x0004)
> >  
> >  #define IS_ALPHA_SUPPORT(intel_info) ((intel_info)->is_alpha_support)
> >  
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > index 5d1f53723388..63d676de3840 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -2782,8 +2782,7 @@ gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl)
> >  			if (INTEL_GEN(dev_priv) >= 11)
> >  				tmp_mask |= ICL_AUX_CHANNEL_E;
> >  
> > -			if (IS_CNL_WITH_PORT_F(dev_priv) ||
> > -			    INTEL_GEN(dev_priv) >= 11)
> > +			if (INTEL_INFO(dev_priv)->ddi_ports >= 6)
> >  				tmp_mask |= CNL_AUX_CHANNEL_F;

ok, this is directly related to the number of ports

> >  
> >  			if (iir & tmp_mask) {
> > @@ -4220,7 +4219,7 @@ static void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv)
> >  	if (INTEL_GEN(dev_priv) >= 11)
> >  		de_port_masked |= ICL_AUX_CHANNEL_E;
> >  
> > -	if (IS_CNL_WITH_PORT_F(dev_priv) || INTEL_GEN(dev_priv) >= 11)
> > +	if (INTEL_INFO(dev_priv)->ddi_ports >= 6)
> >  		de_port_masked |= CNL_AUX_CHANNEL_F;
> >  
> >  	de_pipe_enables = de_pipe_masked | GEN8_PIPE_VBLANK |
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index 3384a9bbdafd..0ea0414ccef4 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -402,7 +402,7 @@ static int cnl_max_source_rate(struct intel_dp *intel_dp)
> >  		return 540000;
> >  
> >  	/* For this SKU 8.1G is supported in all ports */
> > -	if (IS_CNL_WITH_PORT_F(dev_priv))
> > +	if (INTEL_INFO(dev_priv)->ddi_ports == 6)

this is not. And anyone reading this will have to chase back what it means to have 6 ddi ports here.
So we may need to create a more useful macro (that can in fact check by the number of ports).
Places in which the number of ports is not related to what's being checked will be much more
difficult to check to support newer platforms. Explicit variables in dev_info won't.


> >  		return 810000;
> >  
> >  	/* For other SKUs, max rate on ports A and D is 5.4G */
> > diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c
> > index 648a13c6043c..05d1faf77048 100644
> > --- a/drivers/gpu/drm/i915/intel_hotplug.c
> > +++ b/drivers/gpu/drm/i915/intel_hotplug.c
> > @@ -101,7 +101,7 @@ enum hpd_pin intel_hpd_pin_default(struct drm_i915_private *dev_priv,
> >  	case PORT_E:
> >  		return HPD_PORT_E;
> >  	case PORT_F:
> > -		if (IS_CNL_WITH_PORT_F(dev_priv))
> > +		if (IS_GEN10(dev_priv))

this seems wrong, no? missing a && ddi_ports == 6?

Lucas De Marchi

> >  			return HPD_PORT_E;
> >  		return HPD_PORT_F;
> >  	default:
> > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > index 31a49bdcf193..80e14be11279 100644
> > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > @@ -3099,7 +3099,7 @@ int intel_power_domains_init(struct drm_i915_private *dev_priv)
> >  		 * timeouts, lets remove them from the list
> >  		 * for the SKUs without port F.
> >  		 */
> > -		if (!IS_CNL_WITH_PORT_F(dev_priv))
> > +		if (INTEL_INFO(dev_priv)->ddi_ports == 5)
> >  			power_domains->power_well_count -= 2;
> >  
> >  	} else if (IS_BROXTON(dev_priv)) {
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 4/8] drm/i915: Group gen 10 display.

[Lucas De Marchi]

On Fri, Oct 19, 2018 at 11:03:53AM +0300, Jani Nikula wrote:
> On Thu, 18 Oct 2018, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> > Continuing with the goal of use less platform codenames:
> > let's group platforms who has gen10 display.
> 
> Ahah, so this answers my question in the previous patch. ;)
> 
> So we already have HAS_GMCH_DISPLAY().
> 
> If we're going to make gen10 display a thing, should we not generalize
> this to have an actual display gen in device info? Of course for most
> platforms it's going to be identical to the gem gen.

I think we should have indeed a display_gen in device info. It's a way to group features
that are particular to that gen. Just like we do for gem.  When the spec is clear about
this being something from that gen, I think it's fine.

We will need more fine grained flags in the device info, but IMO that should be in addition
to this display gen number.

Lucas De Marchi

> 
> The question becomes, is display gen accurate enough? It's not enough to
> cover all of Geminilake I believe. Which makes me think, should we just
> add tons more device info flags to cover features at a detailed level? I
> think that's what Joonas advocates. Perhaps it bloats the device info,
> and causes increase in the number of device info blocks. But my gut
> feeling says together with truly immutable device info, there are
> compiler optimization benefits to be had.
> 
> Also, currently we "inherit" device info using truly obnoxious macro
> layering where you have to work hard to trace the macros to find out
> what is set for a given platform. It doesn't have to be that way. We
> could move parts of it to separate but shared features structs, and add
> pointers to them.
> 
> Anyway, thanks for rolling this series as a starting point for
> discussion. Even if I'm not buying the changes as-is. ;)
> 
> 
> BR,
> Jani.
> 
> >
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h      | 2 ++
> >  drivers/gpu/drm/i915/intel_cdclk.c   | 2 +-
> >  drivers/gpu/drm/i915/intel_color.c   | 2 +-
> >  drivers/gpu/drm/i915/intel_display.c | 4 ++--
> >  drivers/gpu/drm/i915/intel_sprite.c  | 2 +-
> >  5 files changed, 7 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index d19b38ef145b..6436fedfe561 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -2637,6 +2637,8 @@ intel_info(const struct drm_i915_private *dev_priv)
> >  #define SUPPORTS_TV(dev_priv)		((dev_priv)->info.supports_tv)
> >  #define I915_HAS_HOTPLUG(dev_priv)	((dev_priv)->info.has_hotplug)
> >  
> > +#define HAS_DISPLAY_10(dev_priv) (IS_GEN10(dev_priv) || IS_GEMINILAKE(dev_priv))
> > +
> >  #define HAS_FW_BLC(dev_priv) 	(INTEL_GEN(dev_priv) > 2)
> >  #define HAS_FBC(dev_priv)	((dev_priv)->info.has_fbc)
> >  #define HAS_CUR_FBC(dev_priv)	(!HAS_GMCH_DISPLAY(dev_priv) && INTEL_GEN(dev_priv) >= 7)
> > diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
> > index 4aa6500604cc..b315b70fd49c 100644
> > --- a/drivers/gpu/drm/i915/intel_cdclk.c
> > +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> > @@ -2181,7 +2181,7 @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state)
> >  	    crtc_state->has_audio &&
> >  	    crtc_state->port_clock >= 540000 &&
> >  	    crtc_state->lane_count == 4) {
> > -		if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv)) {
> > +		if (HAS_DISPLAY_10(dev_priv)) {
> >  			/* Display WA #1145: glk,cnl */
> >  			min_cdclk = max(316800, min_cdclk);
> >  		} else if (IS_GEN9(dev_priv) || IS_BROADWELL(dev_priv)) {
> > diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
> > index 5127da286a2b..d5f67b9c9698 100644
> > --- a/drivers/gpu/drm/i915/intel_color.c
> > +++ b/drivers/gpu/drm/i915/intel_color.c
> > @@ -659,7 +659,7 @@ void intel_color_init(struct drm_crtc *crtc)
> >  		   IS_BROXTON(dev_priv)) {
> >  		dev_priv->display.load_csc_matrix = ilk_load_csc_matrix;
> >  		dev_priv->display.load_luts = broadwell_load_luts;
> > -	} else if (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) {
> > +	} else if (HAS_DISPLAY_10(dev_priv)) {
> >  		dev_priv->display.load_csc_matrix = ilk_load_csc_matrix;
> >  		dev_priv->display.load_luts = glk_load_luts;
> >  	} else {
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 7b04b8436b6d..1abf79a4ee91 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -5730,7 +5730,7 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
> >  	intel_crtc->active = true;
> >  
> >  	/* Display WA #1180: WaDisableScalarClockGating: glk, cnl */
> > -	psl_clkgate_wa = (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) &&
> > +	psl_clkgate_wa = (HAS_DISPLAY_10(dev_priv)) &&
> >  			 pipe_config->pch_pfit.enabled;
> >  	if (psl_clkgate_wa)
> >  		glk_pipe_scaler_clock_gating_wa(dev_priv, pipe, true);
> > @@ -15565,7 +15565,7 @@ get_encoder_power_domains(struct drm_i915_private *dev_priv)
> >  static void intel_early_display_was(struct drm_i915_private *dev_priv)
> >  {
> >  	/* Display WA #1185 WaDisableDARBFClkGating:cnl,glk */
> > -	if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
> > +	if (HAS_DISPLAY_10(dev_priv))
> >  		I915_WRITE(GEN9_CLKGATE_DIS_0, I915_READ(GEN9_CLKGATE_DIS_0) |
> >  			   DARBF_GATING_DIS);
> >  
> > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> > index 7cd59eee5cad..912ab7b9d89a 100644
> > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > @@ -1330,7 +1330,7 @@ static int skl_plane_check_dst_coordinates(const struct intel_crtc_state *crtc_s
> >  	 * than the cursor ending less than 4 pixels from the left edge of the
> >  	 * screen may cause FIFO underflow and display corruption.
> >  	 */
> > -	if ((IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) &&
> > +	if (HAS_DISPLAY_10(dev_priv) &&
> >  	    (crtc_x + crtc_w < 4 || crtc_x > pipe_src_w - 4)) {
> >  		DRM_DEBUG_KMS("requested plane X %s position %d invalid (valid range %d-%d)\n",
> >  			      crtc_x + crtc_w < 4 ? "end" : "start",
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 4/8] drm/i915: Group gen 10 display.

[Lucas De Marchi]

On Fri, Oct 19, 2018 at 01:33:08PM +0300, Ville Syrjälä wrote:
> On Fri, Oct 19, 2018 at 10:30:46AM +0200, Daniel Vetter wrote:
> > On Fri, Oct 19, 2018 at 11:03:53AM +0300, Jani Nikula wrote:
> > > On Thu, 18 Oct 2018, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> > > > Continuing with the goal of use less platform codenames:
> > > > let's group platforms who has gen10 display.
> > > 
> > > Ahah, so this answers my question in the previous patch. ;)
> > > 
> > > So we already have HAS_GMCH_DISPLAY().
> > 
> > We also have HAS_DDI, which I guess you could call gen8 display :-)
> 
> There's also these interesting gen designations in some old docs:
> ctg/elk = gen5.5
> ilk = gen5.75
> I guess we could more or less call all of that as gen5 display.
> 
> And of course we have the other oddballs like vlv/chv which are
> sort of mismash of i965, ctg/elk, ibx, and custom stuff. Also
> pnv is mostly gen3 display except for a few bits that were
> snatched from i965.
> 
> Not sure we have enough numbers for all that without resotring to
> fractions. And no one could anyway remember what all the different
> numbers mean.

I think we should approximate to the closest one, but prefer using gen_display number
checks for cases in which it's clear from the spec the separation.

For other things we will need more fine grained flags, that can be used across
gens or to differentiate features within a single gen.



Lucas De Marchi


> 
> > 
> > > If we're going to make gen10 display a thing, should we not generalize
> > > this to have an actual display gen in device info? Of course for most
> > > platforms it's going to be identical to the gem gen.
> > > 
> > > The question becomes, is display gen accurate enough? It's not enough to
> > > cover all of Geminilake I believe. Which makes me think, should we just
> > > add tons more device info flags to cover features at a detailed level? I
> > > think that's what Joonas advocates. Perhaps it bloats the device info,
> > > and causes increase in the number of device info blocks. But my gut
> > > feeling says together with truly immutable device info, there are
> > > compiler optimization benefits to be had.
> > > 
> > > Also, currently we "inherit" device info using truly obnoxious macro
> > > layering where you have to work hard to trace the macros to find out
> > > what is set for a given platform. It doesn't have to be that way. We
> > > could move parts of it to separate but shared features structs, and add
> > > pointers to them.
> > > 
> > > Anyway, thanks for rolling this series as a starting point for
> > > discussion. Even if I'm not buying the changes as-is. ;)
> > 
> > Yeah, stuffing this into intel_info imo makes sense. As long as it's
> > booleans they should be fairly cheap.
> > -Daniel
> > > 
> > > 
> > > BR,
> > > Jani.
> > > 
> > > >
> > > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/i915_drv.h      | 2 ++
> > > >  drivers/gpu/drm/i915/intel_cdclk.c   | 2 +-
> > > >  drivers/gpu/drm/i915/intel_color.c   | 2 +-
> > > >  drivers/gpu/drm/i915/intel_display.c | 4 ++--
> > > >  drivers/gpu/drm/i915/intel_sprite.c  | 2 +-
> > > >  5 files changed, 7 insertions(+), 5 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > > > index d19b38ef145b..6436fedfe561 100644
> > > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > > @@ -2637,6 +2637,8 @@ intel_info(const struct drm_i915_private *dev_priv)
> > > >  #define SUPPORTS_TV(dev_priv)		((dev_priv)->info.supports_tv)
> > > >  #define I915_HAS_HOTPLUG(dev_priv)	((dev_priv)->info.has_hotplug)
> > > >  
> > > > +#define HAS_DISPLAY_10(dev_priv) (IS_GEN10(dev_priv) || IS_GEMINILAKE(dev_priv))
> > > > +
> > > >  #define HAS_FW_BLC(dev_priv) 	(INTEL_GEN(dev_priv) > 2)
> > > >  #define HAS_FBC(dev_priv)	((dev_priv)->info.has_fbc)
> > > >  #define HAS_CUR_FBC(dev_priv)	(!HAS_GMCH_DISPLAY(dev_priv) && INTEL_GEN(dev_priv) >= 7)
> > > > diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
> > > > index 4aa6500604cc..b315b70fd49c 100644
> > > > --- a/drivers/gpu/drm/i915/intel_cdclk.c
> > > > +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> > > > @@ -2181,7 +2181,7 @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state)
> > > >  	    crtc_state->has_audio &&
> > > >  	    crtc_state->port_clock >= 540000 &&
> > > >  	    crtc_state->lane_count == 4) {
> > > > -		if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv)) {
> > > > +		if (HAS_DISPLAY_10(dev_priv)) {
> > > >  			/* Display WA #1145: glk,cnl */
> > > >  			min_cdclk = max(316800, min_cdclk);
> > > >  		} else if (IS_GEN9(dev_priv) || IS_BROADWELL(dev_priv)) {
> > > > diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
> > > > index 5127da286a2b..d5f67b9c9698 100644
> > > > --- a/drivers/gpu/drm/i915/intel_color.c
> > > > +++ b/drivers/gpu/drm/i915/intel_color.c
> > > > @@ -659,7 +659,7 @@ void intel_color_init(struct drm_crtc *crtc)
> > > >  		   IS_BROXTON(dev_priv)) {
> > > >  		dev_priv->display.load_csc_matrix = ilk_load_csc_matrix;
> > > >  		dev_priv->display.load_luts = broadwell_load_luts;
> > > > -	} else if (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) {
> > > > +	} else if (HAS_DISPLAY_10(dev_priv)) {
> > > >  		dev_priv->display.load_csc_matrix = ilk_load_csc_matrix;
> > > >  		dev_priv->display.load_luts = glk_load_luts;
> > > >  	} else {
> > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > > index 7b04b8436b6d..1abf79a4ee91 100644
> > > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > > @@ -5730,7 +5730,7 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
> > > >  	intel_crtc->active = true;
> > > >  
> > > >  	/* Display WA #1180: WaDisableScalarClockGating: glk, cnl */
> > > > -	psl_clkgate_wa = (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) &&
> > > > +	psl_clkgate_wa = (HAS_DISPLAY_10(dev_priv)) &&
> > > >  			 pipe_config->pch_pfit.enabled;
> > > >  	if (psl_clkgate_wa)
> > > >  		glk_pipe_scaler_clock_gating_wa(dev_priv, pipe, true);
> > > > @@ -15565,7 +15565,7 @@ get_encoder_power_domains(struct drm_i915_private *dev_priv)
> > > >  static void intel_early_display_was(struct drm_i915_private *dev_priv)
> > > >  {
> > > >  	/* Display WA #1185 WaDisableDARBFClkGating:cnl,glk */
> > > > -	if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
> > > > +	if (HAS_DISPLAY_10(dev_priv))
> > > >  		I915_WRITE(GEN9_CLKGATE_DIS_0, I915_READ(GEN9_CLKGATE_DIS_0) |
> > > >  			   DARBF_GATING_DIS);
> > > >  
> > > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> > > > index 7cd59eee5cad..912ab7b9d89a 100644
> > > > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > > > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > > > @@ -1330,7 +1330,7 @@ static int skl_plane_check_dst_coordinates(const struct intel_crtc_state *crtc_s
> > > >  	 * than the cursor ending less than 4 pixels from the left edge of the
> > > >  	 * screen may cause FIFO underflow and display corruption.
> > > >  	 */
> > > > -	if ((IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) &&
> > > > +	if (HAS_DISPLAY_10(dev_priv) &&
> > > >  	    (crtc_x + crtc_w < 4 || crtc_x > pipe_src_w - 4)) {
> > > >  		DRM_DEBUG_KMS("requested plane X %s position %d invalid (valid range %d-%d)\n",
> > > >  			      crtc_x + crtc_w < 4 ? "end" : "start",
> > > 
> > > -- 
> > > Jani Nikula, Intel Open Source Graphics Center
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 
> > -- 
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 2/8] drm/i915: Use the ddi_ports info to kill ugly CNL_WITH_PORT_F.

[Rodrigo Vivi]

On Fri, Oct 19, 2018 at 09:46:13AM -0700, Lucas De Marchi wrote:
> On Fri, Oct 19, 2018 at 10:39:46AM +0300, Jani Nikula wrote:
> > On Thu, 18 Oct 2018, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> > > Now that we have the number of ddi ports information available
> > > let's use it instead of that ugly platform macro.
> > >
> > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/i915_drv.c         | 10 ++++++++++
> > >  drivers/gpu/drm/i915/i915_drv.h         |  2 --
> > >  drivers/gpu/drm/i915/i915_irq.c         |  5 ++---
> > >  drivers/gpu/drm/i915/intel_dp.c         |  2 +-
> > >  drivers/gpu/drm/i915/intel_hotplug.c    |  2 +-
> > >  drivers/gpu/drm/i915/intel_runtime_pm.c |  2 +-
> > >  6 files changed, 15 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > > index baac35f698f9..83ab325c6675 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.c
> > > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > > @@ -239,6 +239,14 @@ intel_virt_detect_pch(const struct drm_i915_private *dev_priv)
> > >  	return id;
> > >  }
> > >  
> > > +static void intel_adjust_skus_differences(struct drm_i915_private *dev_priv)
> > > +{
> > > +	/* Some Cannonlake SKUs have Port F */
> > > +	if (IS_CANNONLAKE(dev_priv) &&
> > > +	    (INTEL_DEVID(dev_priv) & 0x0004) == 0x0004)
> > > +		mkwrite_device_info(dev_priv)->ddi_ports = 6;
> > > +}
> > 
> > I'd like reduce the amount of device info stuff we adjust at probe, not
> > increase.
> > 
> > Long term, I'd like to make dev_priv->info a const pointer to the const
> > data in i915_pci.c, and make it truly immutable. We could have an
> > additional runtime device info to complement that, but this approach
> > calls for a more concrete split in const and non-const data, not "mostly
> > const but sometimes mutable".
> 
> I agree with this. I'd also like to have a const dev_info. What I think we need is to split
> in what is const what can be changed in runtime/initialization.
> 
> However, I'm not fond on using the number of ddi ports to take decisions in the code not
> related to the number of ports, but rather with other things of that sku. See below.
> 
> > 
> > I think I've chatted about this with Chris too.
> > 
> > BR,
> > Jani.
> > 
> > > +
> > >  static void intel_detect_pch(struct drm_i915_private *dev_priv)
> > >  {
> > >  	struct pci_dev *pch = NULL;
> > > @@ -904,6 +912,8 @@ static int i915_driver_init_early(struct drm_i915_private *dev_priv)
> > >  	if (ret < 0)
> > >  		goto err_workqueues;
> > >  
> > > +	intel_adjust_skus_differences(dev_priv);
> > > +
> > >  	/* This must be called before any calls to HAS_PCH_* */
> > >  	intel_detect_pch(dev_priv);
> > >  
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > > index 7ad232849268..99e42df79ed8 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > @@ -2485,8 +2485,6 @@ intel_info(const struct drm_i915_private *dev_priv)
> > >  				 (dev_priv)->info.gt == 2)
> > >  #define IS_CFL_GT3(dev_priv)	(IS_COFFEELAKE(dev_priv) && \
> > >  				 (dev_priv)->info.gt == 3)
> > > -#define IS_CNL_WITH_PORT_F(dev_priv)   (IS_CANNONLAKE(dev_priv) && \
> > > -					(INTEL_DEVID(dev_priv) & 0x0004) == 0x0004)
> > >  
> > >  #define IS_ALPHA_SUPPORT(intel_info) ((intel_info)->is_alpha_support)
> > >  
> > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > > index 5d1f53723388..63d676de3840 100644
> > > --- a/drivers/gpu/drm/i915/i915_irq.c
> > > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > > @@ -2782,8 +2782,7 @@ gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl)
> > >  			if (INTEL_GEN(dev_priv) >= 11)
> > >  				tmp_mask |= ICL_AUX_CHANNEL_E;
> > >  
> > > -			if (IS_CNL_WITH_PORT_F(dev_priv) ||
> > > -			    INTEL_GEN(dev_priv) >= 11)
> > > +			if (INTEL_INFO(dev_priv)->ddi_ports >= 6)
> > >  				tmp_mask |= CNL_AUX_CHANNEL_F;
> 
> ok, this is directly related to the number of ports
> 
> > >  
> > >  			if (iir & tmp_mask) {
> > > @@ -4220,7 +4219,7 @@ static void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv)
> > >  	if (INTEL_GEN(dev_priv) >= 11)
> > >  		de_port_masked |= ICL_AUX_CHANNEL_E;
> > >  
> > > -	if (IS_CNL_WITH_PORT_F(dev_priv) || INTEL_GEN(dev_priv) >= 11)
> > > +	if (INTEL_INFO(dev_priv)->ddi_ports >= 6)
> > >  		de_port_masked |= CNL_AUX_CHANNEL_F;
> > >  
> > >  	de_pipe_enables = de_pipe_masked | GEN8_PIPE_VBLANK |
> > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > > index 3384a9bbdafd..0ea0414ccef4 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > @@ -402,7 +402,7 @@ static int cnl_max_source_rate(struct intel_dp *intel_dp)
> > >  		return 540000;
> > >  
> > >  	/* For this SKU 8.1G is supported in all ports */
> > > -	if (IS_CNL_WITH_PORT_F(dev_priv))
> > > +	if (INTEL_INFO(dev_priv)->ddi_ports == 6)
> 
> this is not. And anyone reading this will have to chase back what it means to have 6 ddi ports here.
> So we may need to create a more useful macro (that can in fact check by the number of ports).
> Places in which the number of ports is not related to what's being checked will be much more
> difficult to check to support newer platforms. Explicit variables in dev_info won't.

makes sense. I will change.

> 
> 
> > >  		return 810000;
> > >  
> > >  	/* For other SKUs, max rate on ports A and D is 5.4G */
> > > diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c
> > > index 648a13c6043c..05d1faf77048 100644
> > > --- a/drivers/gpu/drm/i915/intel_hotplug.c
> > > +++ b/drivers/gpu/drm/i915/intel_hotplug.c
> > > @@ -101,7 +101,7 @@ enum hpd_pin intel_hpd_pin_default(struct drm_i915_private *dev_priv,
> > >  	case PORT_E:
> > >  		return HPD_PORT_E;
> > >  	case PORT_F:
> > > -		if (IS_CNL_WITH_PORT_F(dev_priv))
> > > +		if (IS_GEN10(dev_priv))
> 
> this seems wrong, no? missing a && ddi_ports == 6?

maybe probably deserves a separated patch with proper explanation?

but the only case were this case PORT_F can be reached
is that this is a CNL_WITH_PORT_F or this is an ICELAKE...

So the IS_CANNONLAKE or IS_GEN10 is enough here.

If regular CNL was taking this path before this patch it
would end with HPD_PORT_F which doesn't exist in any cnl
anyway.

> 
> Lucas De Marchi
> 
> > >  			return HPD_PORT_E;
> > >  		return HPD_PORT_F;
> > >  	default:
> > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > index 31a49bdcf193..80e14be11279 100644
> > > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > @@ -3099,7 +3099,7 @@ int intel_power_domains_init(struct drm_i915_private *dev_priv)
> > >  		 * timeouts, lets remove them from the list
> > >  		 * for the SKUs without port F.
> > >  		 */
> > > -		if (!IS_CNL_WITH_PORT_F(dev_priv))
> > > +		if (INTEL_INFO(dev_priv)->ddi_ports == 5)
> > >  			power_domains->power_well_count -= 2;
> > >  
> > >  	} else if (IS_BROXTON(dev_priv)) {
> > 
> > -- 
> > Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 4/8] drm/i915: Group gen 10 display.

[Ville Syrjälä]

On Fri, Oct 19, 2018 at 09:41:37AM -0700, Rodrigo Vivi wrote:
> On Fri, Oct 19, 2018 at 01:33:08PM +0300, Ville Syrjälä wrote:
> > On Fri, Oct 19, 2018 at 10:30:46AM +0200, Daniel Vetter wrote:
> > > On Fri, Oct 19, 2018 at 11:03:53AM +0300, Jani Nikula wrote:
> > > > On Thu, 18 Oct 2018, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> > > > > Continuing with the goal of use less platform codenames:
> > > > > let's group platforms who has gen10 display.
> > > > 
> > > > Ahah, so this answers my question in the previous patch. ;)
> > > > 
> > > > So we already have HAS_GMCH_DISPLAY().
> > > 
> > > We also have HAS_DDI, which I guess you could call gen8 display :-)
> > 
> > There's also these interesting gen designations in some old docs:
> > ctg/elk = gen5.5
> > ilk = gen5.75
> > I guess we could more or less call all of that as gen5 display.
> > 
> > And of course we have the other oddballs like vlv/chv which are
> > sort of mismash of i965, ctg/elk, ibx, and custom stuff. Also
> > pnv is mostly gen3 display except for a few bits that were
> > snatched from i965.
> > 
> > Not sure we have enough numbers for all that without resotring to
> > fractions. And no one could anyway remember what all the different
> > numbers mean.
> 
> Thanks for all the comments. Yeap, the idea is not to use this series
> as is but just start the discussion and evolve it.
> 
> Gen number by itself doesn't fit to display indeed, but neither
> display-gen-number because we have the fraction and cases like glk
> that is gen10-stripped-display+bxt :/
> 
> Also platform names are not enough by itself, like cannnonlake-with-port-f
> because the sku lacks on having a different name. If you take a look to our
> internal branch the same is about to happen with icl soon....
> 
> So my idea was that we first use feature {name, or number of something}
> whenever possible. On the secondary case we use groups of things like
> HAS_GMCH_DISPLAY, HAS_GEN10_DISPLAY, HAS_VLV_DISPLAY. And on last case
> we use gen numbers.
> 
> My idea of preferring gen over platform names in general was that for
> platform name we cannot use >= ICELAKE... :/

Well... we could. Assuming we order the enum suitably. Problem
is that there may not be a single order that works for all cases.
But it might be good enough for a lot of the cases.

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