All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] drm/i915/gen11+: First assume next platforms will inherit stuff
@ 2019-03-04 22:48 Rodrigo Vivi
  2019-03-04 22:48 ` [PATCH 2/3] drm/i915: Move PCH_NOP to -1 Rodrigo Vivi
                   ` (5 more replies)
  0 siblings, 6 replies; 20+ messages in thread
From: Rodrigo Vivi @ 2019-03-04 22:48 UTC (permalink / raw)
  To: intel-gfx; +Cc: Lucas De Marchi

This exactly same approach was already used from gen9
to gen10 and from gen10 to gen11. Let's also use it
for gen11+.

Let's first assume that we inherit a similar platform
and than we apply the differences on top.

Different from the previous attempts this will be
done this time with coccinelle. We obviously need to
exclude some case that is really exclusive for gen11
like  PCH, Firmware, and few others. Luckly this was
easy to filter by selecting the files we are touching
with coccinelle as exposed below:

spatch -sp_file gen11\+.cocci --in-place i915_perf.c \
       intel_bios.c intel_cdclk.c intel_ddi.c \
       intel_device_info.c intel_display.c intel_dpll_mgr.c \
       intel_dsi_vbt.c intel_hdmi.c intel_lrc.c intel_mocs.c intel_color.c

@noticelake@ expression e; @@
-!IS_ICELAKE(e)
+INTEL_GEN(e) < 11
@notgen11@ expression e; @@
-!IS_GEN(e, 11)
+INTEL_GEN(e) < 11
@icelake@ expression e; @@
-IS_ICELAKE(e)
+INTEL_GEN(e) >= 11
@gen11@ expression e; @@
-IS_GEN(e, 11)
+INTEL_GEN(e) >= 11

No functional change.

Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/i915_perf.c         |  2 +-
 drivers/gpu/drm/i915/intel_bios.c        |  4 ++--
 drivers/gpu/drm/i915/intel_cdclk.c       |  6 +++---
 drivers/gpu/drm/i915/intel_color.c       |  2 +-
 drivers/gpu/drm/i915/intel_ddi.c         | 18 +++++++++---------
 drivers/gpu/drm/i915/intel_device_info.c |  2 +-
 drivers/gpu/drm/i915/intel_display.c     | 18 +++++++++---------
 drivers/gpu/drm/i915/intel_dpll_mgr.c    |  2 +-
 drivers/gpu/drm/i915/intel_dsi_vbt.c     |  6 +++---
 drivers/gpu/drm/i915/intel_hdmi.c        |  4 ++--
 drivers/gpu/drm/i915/intel_lrc.c         |  4 ++--
 drivers/gpu/drm/i915/intel_mocs.c        |  2 +-
 12 files changed, 35 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index 72a9a35b40e2..c81feb43da90 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -2881,7 +2881,7 @@ void i915_perf_register(struct drm_i915_private *dev_priv)
 
 	sysfs_attr_init(&dev_priv->perf.oa.test_config.sysfs_metric_id.attr);
 
-	if (IS_ICELAKE(dev_priv)) {
+	if (INTEL_GEN(dev_priv) >= 11) {
 		i915_perf_load_test_config_icl(dev_priv);
 	} else if (IS_CANNONLAKE(dev_priv)) {
 		i915_perf_load_test_config_cnl(dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index b508d8a735e0..48c62bea92cd 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -2093,8 +2093,8 @@ bool intel_bios_is_dsi_present(struct drm_i915_private *dev_priv,
 		dvo_port = child->dvo_port;
 
 		if (dvo_port == DVO_PORT_MIPIA ||
-		    (dvo_port == DVO_PORT_MIPIB && IS_ICELAKE(dev_priv)) ||
-		    (dvo_port == DVO_PORT_MIPIC && !IS_ICELAKE(dev_priv))) {
+		    (dvo_port == DVO_PORT_MIPIB && INTEL_GEN(dev_priv) >= 11) ||
+		    (dvo_port == DVO_PORT_MIPIC && INTEL_GEN(dev_priv) < 11)) {
 			if (port)
 				*port = dvo_port - DVO_PORT_MIPIA;
 			return true;
diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
index 5d266538036d..7e5132772477 100644
--- a/drivers/gpu/drm/i915/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/intel_cdclk.c
@@ -2560,7 +2560,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
@@ -2744,7 +2744,7 @@ void intel_update_rawclk(struct drm_i915_private *dev_priv)
  */
 void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv)
 {
-	if (IS_ICELAKE(dev_priv)) {
+	if (INTEL_GEN(dev_priv) >= 11) {
 		dev_priv->display.set_cdclk = icl_set_cdclk;
 		dev_priv->display.modeset_calc_cdclk = icl_modeset_calc_cdclk;
 	} else if (IS_CANNONLAKE(dev_priv)) {
@@ -2773,7 +2773,7 @@ void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv)
 			vlv_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_CANNONLAKE(dev_priv))
 		dev_priv->display.get_cdclk = cnl_get_cdclk;
diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index da7a07d5ccea..0173967ed593 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -841,7 +841,7 @@ void intel_color_init(struct intel_crtc *crtc)
 
 		dev_priv->display.color_commit = i9xx_color_commit;
 	} else {
-		if (IS_ICELAKE(dev_priv))
+		if (INTEL_GEN(dev_priv) >= 11)
 			dev_priv->display.load_luts = icl_load_luts;
 		else if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
 			dev_priv->display.load_luts = glk_load_luts;
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index d918be927fc2..5b132082a650 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -851,7 +851,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,
 						0, &n_entries);
@@ -1678,7 +1678,7 @@ static void intel_ddi_clock_get(struct intel_encoder *encoder,
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 
-	if (IS_ICELAKE(dev_priv))
+	if (INTEL_GEN(dev_priv) >= 11)
 		icl_ddi_clock_get(encoder, pipe_config);
 	else if (IS_CANNONLAKE(dev_priv))
 		cnl_ddi_clock_get(encoder, pipe_config);
@@ -2225,7 +2225,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,
 						intel_dp->link_rate, &n_entries);
@@ -2698,7 +2698,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_CANNONLAKE(dev_priv))
@@ -2867,7 +2867,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_clk_sel(encoder, crtc_state));
@@ -2909,7 +2909,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_CANNONLAKE(dev_priv)) {
@@ -3126,7 +3126,7 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
 	icl_program_mg_dp_mode(dig_port);
 	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_CANNONLAKE(dev_priv))
@@ -3175,7 +3175,7 @@ static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder,
 	icl_program_mg_dp_mode(dig_port);
 	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, INTEL_OUTPUT_HDMI);
 	else if (IS_CANNONLAKE(dev_priv))
@@ -3711,7 +3711,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_ICELAKE(dev_priv) && 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_CANNONLAKE(dev_priv) && crtc_state->port_clock > 594000)
 		crtc_state->min_voltage_level = 2;
diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
index 855a5074ad77..e34259989ead 100644
--- a/drivers/gpu/drm/i915/intel_device_info.c
+++ b/drivers/gpu/drm/i915/intel_device_info.c
@@ -740,7 +740,7 @@ void intel_device_info_runtime_init(struct drm_i915_private *dev_priv)
 
 	BUILD_BUG_ON(I915_NUM_ENGINES > BITS_PER_TYPE(intel_ring_mask_t));
 
-	if (IS_GEN(dev_priv, 11))
+	if (INTEL_GEN(dev_priv) >= 11)
 		for_each_pipe(dev_priv, pipe)
 			runtime->num_sprites[pipe] = 6;
 	else if (IS_GEN(dev_priv, 10) || IS_GEMINILAKE(dev_priv))
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 7c5e84ef5171..fdefdc33a0de 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5036,10 +5036,10 @@ skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach,
 	/* range checks */
 	if (src_w < SKL_MIN_SRC_W || src_h < SKL_MIN_SRC_H ||
 	    dst_w < SKL_MIN_DST_W || dst_h < SKL_MIN_DST_H ||
-	    (IS_GEN(dev_priv, 11) &&
+	    (INTEL_GEN(dev_priv) >= 11 &&
 	     (src_w > ICL_MAX_SRC_W || src_h > ICL_MAX_SRC_H ||
 	      dst_w > ICL_MAX_DST_W || dst_h > ICL_MAX_DST_H)) ||
-	    (!IS_GEN(dev_priv, 11) &&
+	    (INTEL_GEN(dev_priv) < 11 &&
 	     (src_w > SKL_MAX_SRC_W || src_h > SKL_MAX_SRC_H ||
 	      dst_w > SKL_MAX_DST_W || dst_h > SKL_MAX_DST_H)))	{
 		DRM_DEBUG_KMS("scaler_user index %u.%u: src %ux%u dst %ux%u "
@@ -6131,7 +6131,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;
@@ -6139,7 +6139,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;
@@ -9550,7 +9550,7 @@ static int haswell_crtc_compute_clock(struct intel_crtc *crtc,
 		to_intel_atomic_state(crtc_state->base.state);
 
 	if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI) ||
-	    IS_ICELAKE(dev_priv)) {
+	    INTEL_GEN(dev_priv) >= 11) {
 		struct intel_encoder *encoder =
 			intel_get_crtc_new_encoder(state, crtc_state);
 
@@ -9693,7 +9693,7 @@ static bool hsw_get_transcoder_state(struct intel_crtc *crtc,
 	enum transcoder panel_transcoder;
 	u32 tmp;
 
-	if (IS_ICELAKE(dev_priv))
+	if (INTEL_GEN(dev_priv) >= 11)
 		panel_transcoder_mask |=
 			BIT(TRANSCODER_DSI_0) | BIT(TRANSCODER_DSI_1);
 
@@ -9826,7 +9826,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_CANNONLAKE(dev_priv))
 		cannonlake_get_ddi_pll(dev_priv, port, pipe_config);
@@ -9889,7 +9889,7 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
 		goto out;
 
 	if (!transcoder_is_dsi(pipe_config->cpu_transcoder) ||
-	    IS_ICELAKE(dev_priv)) {
+	    INTEL_GEN(dev_priv) >= 11) {
 		haswell_get_ddi_port_state(crtc, pipe_config);
 		intel_get_pipe_timings(crtc, pipe_config);
 	}
@@ -14635,7 +14635,7 @@ static void intel_setup_outputs(struct drm_i915_private *dev_priv)
 	if (!HAS_DISPLAY(dev_priv))
 		return;
 
-	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 e4ec73d415d9..b3fb221c2532 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
@@ -3259,7 +3259,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_CANNONLAKE(dev_priv))
 		dpll_mgr = &cnl_pll_mgr;
diff --git a/drivers/gpu/drm/i915/intel_dsi_vbt.c b/drivers/gpu/drm/i915/intel_dsi_vbt.c
index 06a11c35a784..d1e00e4c7726 100644
--- a/drivers/gpu/drm/i915/intel_dsi_vbt.c
+++ b/drivers/gpu/drm/i915/intel_dsi_vbt.c
@@ -194,7 +194,7 @@ static const u8 *mipi_exec_send_packet(struct intel_dsi *intel_dsi,
 		break;
 	}
 
-	if (!IS_ICELAKE(dev_priv))
+	if (INTEL_GEN(dev_priv) < 11)
 		vlv_dsi_wait_for_fifo_empty(intel_dsi, port);
 
 out:
@@ -365,7 +365,7 @@ static const u8 *mipi_exec_gpio(struct intel_dsi *intel_dsi, const u8 *data)
 	/* pull up/down */
 	value = *data++ & 1;
 
-	if (IS_ICELAKE(dev_priv))
+	if (INTEL_GEN(dev_priv) >= 11)
 		icl_exec_gpio(dev_priv, gpio_source, gpio_index, value);
 	else if (IS_VALLEYVIEW(dev_priv))
 		vlv_exec_gpio(dev_priv, gpio_source, gpio_number, value);
@@ -890,7 +890,7 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id)
 
 	intel_dsi->burst_mode_ratio = burst_mode_ratio;
 
-	if (IS_ICELAKE(dev_priv))
+	if (INTEL_GEN(dev_priv) >= 11)
 		icl_dphy_param_init(intel_dsi);
 	else
 		vlv_dphy_param_init(intel_dsi);
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index cd422a7b4da0..5ccb305a6e1c 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -2206,7 +2206,7 @@ static bool hdmi_deep_color_possible(const struct intel_crtc_state *crtc_state,
 
 	/* Display Wa_1405510057:icl */
 	if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420 &&
-	    bpc == 10 && IS_ICELAKE(dev_priv) &&
+	    bpc == 10 && INTEL_GEN(dev_priv) >= 11 &&
 	    (adjusted_mode->crtc_hblank_end -
 	     adjusted_mode->crtc_hblank_start) % 8 == 2)
 		return false;
@@ -2500,7 +2500,7 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
 
 	wakeref = 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_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 578c8c98c718..fb599e11e7b9 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -2525,7 +2525,7 @@ u32 gen8_make_rpcs(struct drm_i915_private *i915, struct intel_sseu *req_sseu)
 	} else {
 		ctx_sseu = intel_device_default_sseu(i915);
 
-		if (IS_GEN(i915, 11)) {
+		if (INTEL_GEN(i915) >= 11) {
 			/*
 			 * We only need subslice count so it doesn't matter
 			 * which ones we select - just turn off low bits in the
@@ -2565,7 +2565,7 @@ u32 gen8_make_rpcs(struct drm_i915_private *i915, struct intel_sseu *req_sseu)
 	 * subslices are enabled, or a count between one and four on the first
 	 * slice.
 	 */
-	if (IS_GEN(i915, 11) &&
+	if (INTEL_GEN(i915) >= 11 &&
 	    slices == 1 &&
 	    subslices > min_t(u8, 4, hweight8(sseu->subslice_mask[0]) / 2)) {
 		GEM_BUG_ON(subslices & 1);
diff --git a/drivers/gpu/drm/i915/intel_mocs.c b/drivers/gpu/drm/i915/intel_mocs.c
index 331e7a678fb7..79913b06f455 100644
--- a/drivers/gpu/drm/i915/intel_mocs.c
+++ b/drivers/gpu/drm/i915/intel_mocs.c
@@ -252,7 +252,7 @@ static bool get_mocs_settings(struct drm_i915_private *dev_priv,
 {
 	bool result = false;
 
-	if (IS_ICELAKE(dev_priv)) {
+	if (INTEL_GEN(dev_priv) >= 11) {
 		table->size  = ARRAY_SIZE(icelake_mocs_table);
 		table->table = icelake_mocs_table;
 		table->n_entries = GEN11_NUM_MOCS_ENTRIES;
-- 
2.20.1

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

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

* [PATCH 2/3] drm/i915: Move PCH_NOP to -1
  2019-03-04 22:48 [PATCH 1/3] drm/i915/gen11+: First assume next platforms will inherit stuff Rodrigo Vivi
@ 2019-03-04 22:48 ` Rodrigo Vivi
  2019-03-05 17:38   ` Lucas De Marchi
  2019-03-04 22:48 ` [PATCH 3/3] drm/i915: Start using comparative INTEL_PCH_TYPE Rodrigo Vivi
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 20+ messages in thread
From: Rodrigo Vivi @ 2019-03-04 22:48 UTC (permalink / raw)
  To: intel-gfx; +Cc: Lucas De Marchi

So we can later use PCH >= comparisons. The ultimate goal
is to make it easier for us to introduce a new platform
with south display engine on PCH just by reusing the previous
one.

Suggested-by: Lucas De Marchi <lucas.demarchi@intel.com>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 453af7438e67..e6be327ba86d 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -524,6 +524,7 @@ struct i915_psr {
 };
 
 enum intel_pch {
+	PCH_NOP = -1,	/* PCH without south display */
 	PCH_NONE = 0,	/* No PCH present */
 	PCH_IBX,	/* Ibexpeak PCH */
 	PCH_CPT,	/* Cougarpoint/Pantherpoint PCH */
@@ -532,7 +533,6 @@ enum intel_pch {
 	PCH_KBP,        /* Kaby Lake PCH */
 	PCH_CNP,        /* Cannon Lake PCH */
 	PCH_ICP,	/* Ice Lake PCH */
-	PCH_NOP,	/* PCH without south display */
 };
 
 enum intel_sbi_destination {
-- 
2.20.1

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

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

* [PATCH 3/3] drm/i915: Start using comparative INTEL_PCH_TYPE
  2019-03-04 22:48 [PATCH 1/3] drm/i915/gen11+: First assume next platforms will inherit stuff Rodrigo Vivi
  2019-03-04 22:48 ` [PATCH 2/3] drm/i915: Move PCH_NOP to -1 Rodrigo Vivi
@ 2019-03-04 22:48 ` Rodrigo Vivi
  2019-03-05 14:10   ` Jani Nikula
  2019-03-05  0:05 ` ✗ Fi.CI.SPARSE: warning for series starting with [1/3] drm/i915/gen11+: First assume next platforms will inherit stuff Patchwork
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 20+ messages in thread
From: Rodrigo Vivi @ 2019-03-04 22:48 UTC (permalink / raw)
  To: intel-gfx; +Cc: Lucas De Marchi

In order to make it easier to bring up new platforms
without having to take care about all corner cases
that was previously taken care for previous platforms
we already use comparative INTEL_GEN statements.

Let's start doing the same with PCH.

The only caveats are:
 - for less-than comparisons we need to be careful
   and check PCH_NONE < pch < PCH_CNP.
 - It is not necessarily a chronological order, but a matter
   of south display compatibility/inheritance.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h    |  6 ++++++
 drivers/gpu/drm/i915/i915_irq.c    |  7 ++-----
 drivers/gpu/drm/i915/intel_cdclk.c |  2 +-
 drivers/gpu/drm/i915/intel_dp.c    | 21 +++++++++------------
 drivers/gpu/drm/i915/intel_panel.c |  5 ++---
 5 files changed, 20 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index e6be327ba86d..e327736c76a0 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -523,6 +523,12 @@ struct i915_psr {
 	u16 su_x_granularity;
 };
 
+/*
+ * Sorted by south display engine compatibility.
+ * If the new PCH comes with a south display engine that is not
+ * inherited from the latest item, please do not add it to the
+ * end. Instead, add it right after its "parent" PCH.
+ */
 enum intel_pch {
 	PCH_NOP = -1,	/* PCH without south display */
 	PCH_NONE = 0,	/* No PCH present */
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index a42eb6394b69..923135d6b781 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2833,9 +2833,7 @@ gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl)
 
 			if (HAS_PCH_ICP(dev_priv))
 				icp_irq_handler(dev_priv, iir);
-			else if (HAS_PCH_SPT(dev_priv) ||
-				 HAS_PCH_KBP(dev_priv) ||
-				 HAS_PCH_CNP(dev_priv))
+			else if (INTEL_PCH_TYPE(dev_priv) >= PCH_SPT)
 				spt_irq_handler(dev_priv, iir);
 			else
 				cpt_irq_handler(dev_priv, iir);
@@ -4620,8 +4618,7 @@ void intel_irq_init(struct drm_i915_private *dev_priv)
 		dev->driver->disable_vblank = gen8_disable_vblank;
 		if (IS_GEN9_LP(dev_priv))
 			dev_priv->display.hpd_irq_setup = bxt_hpd_irq_setup;
-		else if (HAS_PCH_SPT(dev_priv) || HAS_PCH_KBP(dev_priv) ||
-			 HAS_PCH_CNP(dev_priv))
+		else if (INTEL_PCH_TYPE(dev_priv) >= PCH_SPT)
 			dev_priv->display.hpd_irq_setup = spt_hpd_irq_setup;
 		else
 			dev_priv->display.hpd_irq_setup = ilk_hpd_irq_setup;
diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
index 7e5132772477..9d236e4ed26a 100644
--- a/drivers/gpu/drm/i915/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/intel_cdclk.c
@@ -2723,7 +2723,7 @@ static int g4x_hrawclk(struct drm_i915_private *dev_priv)
  */
 void intel_update_rawclk(struct drm_i915_private *dev_priv)
 {
-	if (HAS_PCH_CNP(dev_priv) || HAS_PCH_ICP(dev_priv))
+	if (INTEL_PCH_TYPE(dev_priv) >= PCH_CNP)
 		dev_priv->rawclk_freq = cnp_rawclk(dev_priv);
 	else if (HAS_PCH_SPLIT(dev_priv))
 		dev_priv->rawclk_freq = pch_rawclk(dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index e1a051c0fbfe..acd2336bb214 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -949,8 +949,8 @@ static void intel_pps_get_registers(struct intel_dp *intel_dp,
 	regs->pp_stat = PP_STATUS(pps_idx);
 	regs->pp_on = PP_ON_DELAYS(pps_idx);
 	regs->pp_off = PP_OFF_DELAYS(pps_idx);
-	if (!IS_GEN9_LP(dev_priv) && !HAS_PCH_CNP(dev_priv) &&
-	    !HAS_PCH_ICP(dev_priv))
+	if (INTEL_PCH_TYPE(dev_priv) > PCH_NONE &&
+	    INTEL_PCH_TYPE(dev_priv) < PCH_CNP)
 		regs->pp_div = PP_DIVISOR(pps_idx);
 }
 
@@ -6431,8 +6431,8 @@ intel_pps_readout_hw_state(struct intel_dp *intel_dp, struct edp_power_seq *seq)
 
 	pp_on = I915_READ(regs.pp_on);
 	pp_off = I915_READ(regs.pp_off);
-	if (!IS_GEN9_LP(dev_priv) && !HAS_PCH_CNP(dev_priv) &&
-	    !HAS_PCH_ICP(dev_priv)) {
+	if (INTEL_PCH_TYPE(dev_priv) > PCH_NONE &&
+	    INTEL_PCH_TYPE(dev_priv) < PCH_CNP) {
 		I915_WRITE(regs.pp_ctrl, pp_ctl);
 		pp_div = I915_READ(regs.pp_div);
 	}
@@ -6450,8 +6450,7 @@ intel_pps_readout_hw_state(struct intel_dp *intel_dp, struct edp_power_seq *seq)
 	seq->t10 = (pp_off & PANEL_POWER_DOWN_DELAY_MASK) >>
 		   PANEL_POWER_DOWN_DELAY_SHIFT;
 
-	if (IS_GEN9_LP(dev_priv) || HAS_PCH_CNP(dev_priv) ||
-	    HAS_PCH_ICP(dev_priv)) {
+	if (IS_GEN9_LP(dev_priv) || INTEL_PCH_TYPE(dev_priv) >= PCH_CNP) {
 		seq->t11_t12 = ((pp_ctl & BXT_POWER_CYCLE_DELAY_MASK) >>
 				BXT_POWER_CYCLE_DELAY_SHIFT) * 1000;
 	} else {
@@ -6622,8 +6621,7 @@ intel_dp_init_panel_power_sequencer_registers(struct intel_dp *intel_dp,
 		 (seq->t10 << PANEL_POWER_DOWN_DELAY_SHIFT);
 	/* Compute the divisor for the pp clock, simply match the Bspec
 	 * formula. */
-	if (IS_GEN9_LP(dev_priv) || HAS_PCH_CNP(dev_priv) ||
-	    HAS_PCH_ICP(dev_priv)) {
+	if (IS_GEN9_LP(dev_priv) || INTEL_PCH_TYPE(dev_priv) >= PCH_CNP) {
 		pp_div = I915_READ(regs.pp_ctrl);
 		pp_div &= ~BXT_POWER_CYCLE_DELAY_MASK;
 		pp_div |= (DIV_ROUND_UP(seq->t11_t12, 1000)
@@ -6659,8 +6657,7 @@ intel_dp_init_panel_power_sequencer_registers(struct intel_dp *intel_dp,
 
 	I915_WRITE(regs.pp_on, pp_on);
 	I915_WRITE(regs.pp_off, pp_off);
-	if (IS_GEN9_LP(dev_priv) || HAS_PCH_CNP(dev_priv) ||
-	    HAS_PCH_ICP(dev_priv))
+	if (IS_GEN9_LP(dev_priv) || INTEL_PCH_TYPE(dev_priv) >= PCH_CNP)
 		I915_WRITE(regs.pp_ctrl, pp_div);
 	else
 		I915_WRITE(regs.pp_div, pp_div);
@@ -6668,8 +6665,8 @@ intel_dp_init_panel_power_sequencer_registers(struct intel_dp *intel_dp,
 	DRM_DEBUG_KMS("panel power sequencer register settings: PP_ON %#x, PP_OFF %#x, PP_DIV %#x\n",
 		      I915_READ(regs.pp_on),
 		      I915_READ(regs.pp_off),
-		      (IS_GEN9_LP(dev_priv) || HAS_PCH_CNP(dev_priv)  ||
-		       HAS_PCH_ICP(dev_priv)) ?
+		      (IS_GEN9_LP(dev_priv) ||
+		       INTEL_PCH_TYPE(dev_priv) >= PCH_CNP) ?
 		      (I915_READ(regs.pp_ctrl) & BXT_POWER_CYCLE_DELAY_MASK) :
 		      I915_READ(regs.pp_div));
 }
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index beca98d2b035..edd5540639b0 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -1894,15 +1894,14 @@ intel_panel_init_backlight_funcs(struct intel_panel *panel)
 		panel->backlight.set = bxt_set_backlight;
 		panel->backlight.get = bxt_get_backlight;
 		panel->backlight.hz_to_pwm = bxt_hz_to_pwm;
-	} else if (HAS_PCH_CNP(dev_priv) || HAS_PCH_ICP(dev_priv)) {
+	} else if (INTEL_PCH_TYPE(dev_priv) >= PCH_CNP) {
 		panel->backlight.setup = cnp_setup_backlight;
 		panel->backlight.enable = cnp_enable_backlight;
 		panel->backlight.disable = cnp_disable_backlight;
 		panel->backlight.set = bxt_set_backlight;
 		panel->backlight.get = bxt_get_backlight;
 		panel->backlight.hz_to_pwm = cnp_hz_to_pwm;
-	} else if (HAS_PCH_LPT(dev_priv) || HAS_PCH_SPT(dev_priv) ||
-		   HAS_PCH_KBP(dev_priv)) {
+	} else if (INTEL_PCH_TYPE(dev_priv) >= PCH_LPT) {
 		panel->backlight.setup = lpt_setup_backlight;
 		panel->backlight.enable = lpt_enable_backlight;
 		panel->backlight.disable = lpt_disable_backlight;
-- 
2.20.1

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

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

* ✗ Fi.CI.SPARSE: warning for series starting with [1/3] drm/i915/gen11+: First assume next platforms will inherit stuff
  2019-03-04 22:48 [PATCH 1/3] drm/i915/gen11+: First assume next platforms will inherit stuff Rodrigo Vivi
  2019-03-04 22:48 ` [PATCH 2/3] drm/i915: Move PCH_NOP to -1 Rodrigo Vivi
  2019-03-04 22:48 ` [PATCH 3/3] drm/i915: Start using comparative INTEL_PCH_TYPE Rodrigo Vivi
@ 2019-03-05  0:05 ` Patchwork
  2019-03-05  1:12 ` ✗ Fi.CI.BAT: failure " Patchwork
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2019-03-05  0:05 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/3] drm/i915/gen11+: First assume next platforms will inherit stuff
URL   : https://patchwork.freedesktop.org/series/57545/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Sparse version: v0.5.2
Commit: drm/i915/gen11+: First assume next platforms will inherit stuff
-O:drivers/gpu/drm/i915/intel_lrc.c:2570:25: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_lrc.c:2570:25: warning: expression using sizeof(void)

Commit: drm/i915: Move PCH_NOP to -1
Okay!

Commit: drm/i915: Start using comparative INTEL_PCH_TYPE
-drivers/gpu/drm/i915/selftests/../i915_drv.h:3567:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/selftests/../i915_drv.h:3573:16: warning: expression using sizeof(void)

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

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

* ✗ Fi.CI.BAT: failure for series starting with [1/3] drm/i915/gen11+: First assume next platforms will inherit stuff
  2019-03-04 22:48 [PATCH 1/3] drm/i915/gen11+: First assume next platforms will inherit stuff Rodrigo Vivi
                   ` (2 preceding siblings ...)
  2019-03-05  0:05 ` ✗ Fi.CI.SPARSE: warning for series starting with [1/3] drm/i915/gen11+: First assume next platforms will inherit stuff Patchwork
@ 2019-03-05  1:12 ` Patchwork
  2019-03-05 17:12 ` [PATCH 1/3] " Lucas De Marchi
  2019-03-05 17:43 ` Tvrtko Ursulin
  5 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2019-03-05  1:12 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/3] drm/i915/gen11+: First assume next platforms will inherit stuff
URL   : https://patchwork.freedesktop.org/series/57545/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_5702 -> Patchwork_12362
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_12362 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_12362, 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/57545/revisions/1/mbox/

Possible new issues
-------------------

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

### IGT changes ###

#### Possible regressions ####

  * igt@gem_exec_suspend@basic-s3:
    - fi-bsw-kefka:       PASS -> INCOMPLETE

  * igt@kms_pipe_crc_basic@read-crc-pipe-b:
    - fi-byt-clapper:     PASS -> DMESG-WARN +21

  
Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@i915_pm_rpm@basic-pci-d3-state:
    - fi-byt-j1900:       PASS -> SKIP [fdo#109271]

  * igt@i915_pm_rpm@basic-rte:
    - fi-byt-j1900:       PASS -> FAIL [fdo#108800]

  * igt@kms_busy@basic-flip-b:
    - fi-gdg-551:         PASS -> FAIL [fdo#103182]

  
#### Possible fixes ####

  * igt@i915_pm_rpm@module-reload:
    - fi-skl-6770hq:      FAIL [fdo#108511] -> PASS

  * igt@kms_busy@basic-flip-a:
    - fi-kbl-7567u:       SKIP [fdo#109271] / [fdo#109278] -> PASS +2

  
  [fdo#103182]: https://bugs.freedesktop.org/show_bug.cgi?id=103182
  [fdo#108511]: https://bugs.freedesktop.org/show_bug.cgi?id=108511
  [fdo#108800]: https://bugs.freedesktop.org/show_bug.cgi?id=108800
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278


Participating hosts (44 -> 39)
------------------------------

  Missing    (5): fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-apl-guc fi-bdw-samus 


Build changes
-------------

    * Linux: CI_DRM_5702 -> Patchwork_12362

  CI_DRM_5702: 6e90cfc547f7145e1c3c057a8d5f117888e23d91 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4870: ed944b45563c694dc6373bc48dc83b8ba7edb19f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12362: 62c5c19cfc75470d64cbf7e8eff4c83a10de3003 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

62c5c19cfc75 drm/i915: Start using comparative INTEL_PCH_TYPE
cc780db2fe45 drm/i915: Move PCH_NOP to -1
30d47303fe8a drm/i915/gen11+: First assume next platforms will inherit stuff

== Logs ==

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

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

* Re: [PATCH 3/3] drm/i915: Start using comparative INTEL_PCH_TYPE
  2019-03-04 22:48 ` [PATCH 3/3] drm/i915: Start using comparative INTEL_PCH_TYPE Rodrigo Vivi
@ 2019-03-05 14:10   ` Jani Nikula
  2019-03-05 17:08     ` Lucas De Marchi
  0 siblings, 1 reply; 20+ messages in thread
From: Jani Nikula @ 2019-03-05 14:10 UTC (permalink / raw)
  To: Rodrigo Vivi, intel-gfx; +Cc: Lucas De Marchi

On Mon, 04 Mar 2019, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> In order to make it easier to bring up new platforms
> without having to take care about all corner cases
> that was previously taken care for previous platforms
> we already use comparative INTEL_GEN statements.
>
> Let's start doing the same with PCH.
>
> The only caveats are:
>  - for less-than comparisons we need to be careful
>    and check PCH_NONE < pch < PCH_CNP.
>  - It is not necessarily a chronological order, but a matter
>    of south display compatibility/inheritance.

This scares me a bit, but I understand the reasons. Maybe we need an
IS_PCH_RANGE() macro to complement IS_GEN_RANGE(). But that can come
later as we see how this evolves.

Some notes below.

>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h    |  6 ++++++
>  drivers/gpu/drm/i915/i915_irq.c    |  7 ++-----
>  drivers/gpu/drm/i915/intel_cdclk.c |  2 +-
>  drivers/gpu/drm/i915/intel_dp.c    | 21 +++++++++------------
>  drivers/gpu/drm/i915/intel_panel.c |  5 ++---
>  5 files changed, 20 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index e6be327ba86d..e327736c76a0 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -523,6 +523,12 @@ struct i915_psr {
>  	u16 su_x_granularity;
>  };
>  
> +/*
> + * Sorted by south display engine compatibility.
> + * If the new PCH comes with a south display engine that is not
> + * inherited from the latest item, please do not add it to the
> + * end. Instead, add it right after its "parent" PCH.
> + */
>  enum intel_pch {
>  	PCH_NOP = -1,	/* PCH without south display */
>  	PCH_NONE = 0,	/* No PCH present */
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index a42eb6394b69..923135d6b781 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -2833,9 +2833,7 @@ gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl)
>  
>  			if (HAS_PCH_ICP(dev_priv))

PCH_TYPE >= ICP?

>  				icp_irq_handler(dev_priv, iir);
> -			else if (HAS_PCH_SPT(dev_priv) ||
> -				 HAS_PCH_KBP(dev_priv) ||
> -				 HAS_PCH_CNP(dev_priv))
> +			else if (INTEL_PCH_TYPE(dev_priv) >= PCH_SPT)
>  				spt_irq_handler(dev_priv, iir);
>  			else
>  				cpt_irq_handler(dev_priv, iir);
> @@ -4620,8 +4618,7 @@ void intel_irq_init(struct drm_i915_private *dev_priv)
>  		dev->driver->disable_vblank = gen8_disable_vblank;
>  		if (IS_GEN9_LP(dev_priv))
>  			dev_priv->display.hpd_irq_setup = bxt_hpd_irq_setup;
> -		else if (HAS_PCH_SPT(dev_priv) || HAS_PCH_KBP(dev_priv) ||
> -			 HAS_PCH_CNP(dev_priv))
> +		else if (INTEL_PCH_TYPE(dev_priv) >= PCH_SPT)
>  			dev_priv->display.hpd_irq_setup = spt_hpd_irq_setup;
>  		else
>  			dev_priv->display.hpd_irq_setup = ilk_hpd_irq_setup;
> diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
> index 7e5132772477..9d236e4ed26a 100644
> --- a/drivers/gpu/drm/i915/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> @@ -2723,7 +2723,7 @@ static int g4x_hrawclk(struct drm_i915_private *dev_priv)
>   */
>  void intel_update_rawclk(struct drm_i915_private *dev_priv)
>  {
> -	if (HAS_PCH_CNP(dev_priv) || HAS_PCH_ICP(dev_priv))
> +	if (INTEL_PCH_TYPE(dev_priv) >= PCH_CNP)
>  		dev_priv->rawclk_freq = cnp_rawclk(dev_priv);
>  	else if (HAS_PCH_SPLIT(dev_priv))
>  		dev_priv->rawclk_freq = pch_rawclk(dev_priv);
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index e1a051c0fbfe..acd2336bb214 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -949,8 +949,8 @@ static void intel_pps_get_registers(struct intel_dp *intel_dp,
>  	regs->pp_stat = PP_STATUS(pps_idx);
>  	regs->pp_on = PP_ON_DELAYS(pps_idx);
>  	regs->pp_off = PP_OFF_DELAYS(pps_idx);
> -	if (!IS_GEN9_LP(dev_priv) && !HAS_PCH_CNP(dev_priv) &&
> -	    !HAS_PCH_ICP(dev_priv))
> +	if (INTEL_PCH_TYPE(dev_priv) > PCH_NONE &&
> +	    INTEL_PCH_TYPE(dev_priv) < PCH_CNP)

This is not right, starts to require PCH.

>  		regs->pp_div = PP_DIVISOR(pps_idx);
>  }
>  
> @@ -6431,8 +6431,8 @@ intel_pps_readout_hw_state(struct intel_dp *intel_dp, struct edp_power_seq *seq)
>  
>  	pp_on = I915_READ(regs.pp_on);
>  	pp_off = I915_READ(regs.pp_off);
> -	if (!IS_GEN9_LP(dev_priv) && !HAS_PCH_CNP(dev_priv) &&
> -	    !HAS_PCH_ICP(dev_priv)) {
> +	if (INTEL_PCH_TYPE(dev_priv) > PCH_NONE &&
> +	    INTEL_PCH_TYPE(dev_priv) < PCH_CNP) {

Ditto.

>  		I915_WRITE(regs.pp_ctrl, pp_ctl);
>  		pp_div = I915_READ(regs.pp_div);
>  	}
> @@ -6450,8 +6450,7 @@ intel_pps_readout_hw_state(struct intel_dp *intel_dp, struct edp_power_seq *seq)
>  	seq->t10 = (pp_off & PANEL_POWER_DOWN_DELAY_MASK) >>
>  		   PANEL_POWER_DOWN_DELAY_SHIFT;
>  
> -	if (IS_GEN9_LP(dev_priv) || HAS_PCH_CNP(dev_priv) ||
> -	    HAS_PCH_ICP(dev_priv)) {
> +	if (IS_GEN9_LP(dev_priv) || INTEL_PCH_TYPE(dev_priv) >= PCH_CNP) {
>  		seq->t11_t12 = ((pp_ctl & BXT_POWER_CYCLE_DELAY_MASK) >>
>  				BXT_POWER_CYCLE_DELAY_SHIFT) * 1000;
>  	} else {
> @@ -6622,8 +6621,7 @@ intel_dp_init_panel_power_sequencer_registers(struct intel_dp *intel_dp,
>  		 (seq->t10 << PANEL_POWER_DOWN_DELAY_SHIFT);
>  	/* Compute the divisor for the pp clock, simply match the Bspec
>  	 * formula. */
> -	if (IS_GEN9_LP(dev_priv) || HAS_PCH_CNP(dev_priv) ||
> -	    HAS_PCH_ICP(dev_priv)) {
> +	if (IS_GEN9_LP(dev_priv) || INTEL_PCH_TYPE(dev_priv) >= PCH_CNP) {
>  		pp_div = I915_READ(regs.pp_ctrl);
>  		pp_div &= ~BXT_POWER_CYCLE_DELAY_MASK;
>  		pp_div |= (DIV_ROUND_UP(seq->t11_t12, 1000)
> @@ -6659,8 +6657,7 @@ intel_dp_init_panel_power_sequencer_registers(struct intel_dp *intel_dp,
>  
>  	I915_WRITE(regs.pp_on, pp_on);
>  	I915_WRITE(regs.pp_off, pp_off);
> -	if (IS_GEN9_LP(dev_priv) || HAS_PCH_CNP(dev_priv) ||
> -	    HAS_PCH_ICP(dev_priv))
> +	if (IS_GEN9_LP(dev_priv) || INTEL_PCH_TYPE(dev_priv) >= PCH_CNP)
>  		I915_WRITE(regs.pp_ctrl, pp_div);
>  	else
>  		I915_WRITE(regs.pp_div, pp_div);
> @@ -6668,8 +6665,8 @@ intel_dp_init_panel_power_sequencer_registers(struct intel_dp *intel_dp,
>  	DRM_DEBUG_KMS("panel power sequencer register settings: PP_ON %#x, PP_OFF %#x, PP_DIV %#x\n",
>  		      I915_READ(regs.pp_on),
>  		      I915_READ(regs.pp_off),
> -		      (IS_GEN9_LP(dev_priv) || HAS_PCH_CNP(dev_priv)  ||
> -		       HAS_PCH_ICP(dev_priv)) ?
> +		      (IS_GEN9_LP(dev_priv) ||
> +		       INTEL_PCH_TYPE(dev_priv) >= PCH_CNP) ?
>  		      (I915_READ(regs.pp_ctrl) & BXT_POWER_CYCLE_DELAY_MASK) :
>  		      I915_READ(regs.pp_div));

I think this was all pretty ugly in intel_dp.c before, and this doesn't
make it much better.

I tried to clean it up [1], please consider reviewing those and having
them merged first, after which your change becomes a one-liner in this
file.

Other than that, seems fine.


BR,
Jani.

[1] https://patchwork.freedesktop.org/series/57579/


>  }
> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> index beca98d2b035..edd5540639b0 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -1894,15 +1894,14 @@ intel_panel_init_backlight_funcs(struct intel_panel *panel)
>  		panel->backlight.set = bxt_set_backlight;
>  		panel->backlight.get = bxt_get_backlight;
>  		panel->backlight.hz_to_pwm = bxt_hz_to_pwm;
> -	} else if (HAS_PCH_CNP(dev_priv) || HAS_PCH_ICP(dev_priv)) {
> +	} else if (INTEL_PCH_TYPE(dev_priv) >= PCH_CNP) {
>  		panel->backlight.setup = cnp_setup_backlight;
>  		panel->backlight.enable = cnp_enable_backlight;
>  		panel->backlight.disable = cnp_disable_backlight;
>  		panel->backlight.set = bxt_set_backlight;
>  		panel->backlight.get = bxt_get_backlight;
>  		panel->backlight.hz_to_pwm = cnp_hz_to_pwm;
> -	} else if (HAS_PCH_LPT(dev_priv) || HAS_PCH_SPT(dev_priv) ||
> -		   HAS_PCH_KBP(dev_priv)) {
> +	} else if (INTEL_PCH_TYPE(dev_priv) >= PCH_LPT) {
>  		panel->backlight.setup = lpt_setup_backlight;
>  		panel->backlight.enable = lpt_enable_backlight;
>  		panel->backlight.disable = lpt_disable_backlight;

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

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

* Re: [PATCH 3/3] drm/i915: Start using comparative INTEL_PCH_TYPE
  2019-03-05 14:10   ` Jani Nikula
@ 2019-03-05 17:08     ` Lucas De Marchi
  2019-03-05 17:16       ` Jani Nikula
  0 siblings, 1 reply; 20+ messages in thread
From: Lucas De Marchi @ 2019-03-05 17:08 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

On Tue, Mar 05, 2019 at 04:10:04PM +0200, Jani Nikula wrote:
>On Mon, 04 Mar 2019, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> index e1a051c0fbfe..acd2336bb214 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -949,8 +949,8 @@ static void intel_pps_get_registers(struct intel_dp *intel_dp,
>>  	regs->pp_stat = PP_STATUS(pps_idx);
>>  	regs->pp_on = PP_ON_DELAYS(pps_idx);
>>  	regs->pp_off = PP_OFF_DELAYS(pps_idx);
>> -	if (!IS_GEN9_LP(dev_priv) && !HAS_PCH_CNP(dev_priv) &&
>> -	    !HAS_PCH_ICP(dev_priv))
>> +	if (INTEL_PCH_TYPE(dev_priv) > PCH_NONE &&
>> +	    INTEL_PCH_TYPE(dev_priv) < PCH_CNP)
>
>This is not right, starts to require PCH.

But in those cases INTEL_PCH_TYPE() will return either NONE or NOP.

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

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

* Re: [PATCH 1/3] drm/i915/gen11+: First assume next platforms will inherit stuff
  2019-03-04 22:48 [PATCH 1/3] drm/i915/gen11+: First assume next platforms will inherit stuff Rodrigo Vivi
                   ` (3 preceding siblings ...)
  2019-03-05  1:12 ` ✗ Fi.CI.BAT: failure " Patchwork
@ 2019-03-05 17:12 ` Lucas De Marchi
  2019-03-05 17:43 ` Tvrtko Ursulin
  5 siblings, 0 replies; 20+ messages in thread
From: Lucas De Marchi @ 2019-03-05 17:12 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: intel-gfx

On Mon, Mar 04, 2019 at 02:48:28PM -0800, Rodrigo Vivi wrote:
>This exactly same approach was already used from gen9
>to gen10 and from gen10 to gen11. Let's also use it
>for gen11+.
>
>Let's first assume that we inherit a similar platform
>and than we apply the differences on top.
>
>Different from the previous attempts this will be
>done this time with coccinelle. We obviously need to
>exclude some case that is really exclusive for gen11
>like  PCH, Firmware, and few others. Luckly this was
>easy to filter by selecting the files we are touching
>with coccinelle as exposed below:
>
>spatch -sp_file gen11\+.cocci --in-place i915_perf.c \
>       intel_bios.c intel_cdclk.c intel_ddi.c \
>       intel_device_info.c intel_display.c intel_dpll_mgr.c \
>       intel_dsi_vbt.c intel_hdmi.c intel_lrc.c intel_mocs.c intel_color.c
>
>@noticelake@ expression e; @@
>-!IS_ICELAKE(e)
>+INTEL_GEN(e) < 11
>@notgen11@ expression e; @@
>-!IS_GEN(e, 11)
>+INTEL_GEN(e) < 11
>@icelake@ expression e; @@
>-IS_ICELAKE(e)
>+INTEL_GEN(e) >= 11
>@gen11@ expression e; @@
>-IS_GEN(e, 11)
>+INTEL_GEN(e) >= 11
>
>No functional change.
>
>Cc: Lucas De Marchi <lucas.demarchi@intel.com>
>Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>

Lucas De Marchi

>---
> drivers/gpu/drm/i915/i915_perf.c         |  2 +-
> drivers/gpu/drm/i915/intel_bios.c        |  4 ++--
> drivers/gpu/drm/i915/intel_cdclk.c       |  6 +++---
> drivers/gpu/drm/i915/intel_color.c       |  2 +-
> drivers/gpu/drm/i915/intel_ddi.c         | 18 +++++++++---------
> drivers/gpu/drm/i915/intel_device_info.c |  2 +-
> drivers/gpu/drm/i915/intel_display.c     | 18 +++++++++---------
> drivers/gpu/drm/i915/intel_dpll_mgr.c    |  2 +-
> drivers/gpu/drm/i915/intel_dsi_vbt.c     |  6 +++---
> drivers/gpu/drm/i915/intel_hdmi.c        |  4 ++--
> drivers/gpu/drm/i915/intel_lrc.c         |  4 ++--
> drivers/gpu/drm/i915/intel_mocs.c        |  2 +-
> 12 files changed, 35 insertions(+), 35 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
>index 72a9a35b40e2..c81feb43da90 100644
>--- a/drivers/gpu/drm/i915/i915_perf.c
>+++ b/drivers/gpu/drm/i915/i915_perf.c
>@@ -2881,7 +2881,7 @@ void i915_perf_register(struct drm_i915_private *dev_priv)
>
> 	sysfs_attr_init(&dev_priv->perf.oa.test_config.sysfs_metric_id.attr);
>
>-	if (IS_ICELAKE(dev_priv)) {
>+	if (INTEL_GEN(dev_priv) >= 11) {
> 		i915_perf_load_test_config_icl(dev_priv);
> 	} else if (IS_CANNONLAKE(dev_priv)) {
> 		i915_perf_load_test_config_cnl(dev_priv);
>diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
>index b508d8a735e0..48c62bea92cd 100644
>--- a/drivers/gpu/drm/i915/intel_bios.c
>+++ b/drivers/gpu/drm/i915/intel_bios.c
>@@ -2093,8 +2093,8 @@ bool intel_bios_is_dsi_present(struct drm_i915_private *dev_priv,
> 		dvo_port = child->dvo_port;
>
> 		if (dvo_port == DVO_PORT_MIPIA ||
>-		    (dvo_port == DVO_PORT_MIPIB && IS_ICELAKE(dev_priv)) ||
>-		    (dvo_port == DVO_PORT_MIPIC && !IS_ICELAKE(dev_priv))) {
>+		    (dvo_port == DVO_PORT_MIPIB && INTEL_GEN(dev_priv) >= 11) ||
>+		    (dvo_port == DVO_PORT_MIPIC && INTEL_GEN(dev_priv) < 11)) {
> 			if (port)
> 				*port = dvo_port - DVO_PORT_MIPIA;
> 			return true;
>diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
>index 5d266538036d..7e5132772477 100644
>--- a/drivers/gpu/drm/i915/intel_cdclk.c
>+++ b/drivers/gpu/drm/i915/intel_cdclk.c
>@@ -2560,7 +2560,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
>@@ -2744,7 +2744,7 @@ void intel_update_rawclk(struct drm_i915_private *dev_priv)
>  */
> void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv)
> {
>-	if (IS_ICELAKE(dev_priv)) {
>+	if (INTEL_GEN(dev_priv) >= 11) {
> 		dev_priv->display.set_cdclk = icl_set_cdclk;
> 		dev_priv->display.modeset_calc_cdclk = icl_modeset_calc_cdclk;
> 	} else if (IS_CANNONLAKE(dev_priv)) {
>@@ -2773,7 +2773,7 @@ void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv)
> 			vlv_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_CANNONLAKE(dev_priv))
> 		dev_priv->display.get_cdclk = cnl_get_cdclk;
>diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
>index da7a07d5ccea..0173967ed593 100644
>--- a/drivers/gpu/drm/i915/intel_color.c
>+++ b/drivers/gpu/drm/i915/intel_color.c
>@@ -841,7 +841,7 @@ void intel_color_init(struct intel_crtc *crtc)
>
> 		dev_priv->display.color_commit = i9xx_color_commit;
> 	} else {
>-		if (IS_ICELAKE(dev_priv))
>+		if (INTEL_GEN(dev_priv) >= 11)
> 			dev_priv->display.load_luts = icl_load_luts;
> 		else if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
> 			dev_priv->display.load_luts = glk_load_luts;
>diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
>index d918be927fc2..5b132082a650 100644
>--- a/drivers/gpu/drm/i915/intel_ddi.c
>+++ b/drivers/gpu/drm/i915/intel_ddi.c
>@@ -851,7 +851,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,
> 						0, &n_entries);
>@@ -1678,7 +1678,7 @@ static void intel_ddi_clock_get(struct intel_encoder *encoder,
> {
> 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>
>-	if (IS_ICELAKE(dev_priv))
>+	if (INTEL_GEN(dev_priv) >= 11)
> 		icl_ddi_clock_get(encoder, pipe_config);
> 	else if (IS_CANNONLAKE(dev_priv))
> 		cnl_ddi_clock_get(encoder, pipe_config);
>@@ -2225,7 +2225,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,
> 						intel_dp->link_rate, &n_entries);
>@@ -2698,7 +2698,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_CANNONLAKE(dev_priv))
>@@ -2867,7 +2867,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_clk_sel(encoder, crtc_state));
>@@ -2909,7 +2909,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_CANNONLAKE(dev_priv)) {
>@@ -3126,7 +3126,7 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
> 	icl_program_mg_dp_mode(dig_port);
> 	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_CANNONLAKE(dev_priv))
>@@ -3175,7 +3175,7 @@ static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder,
> 	icl_program_mg_dp_mode(dig_port);
> 	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, INTEL_OUTPUT_HDMI);
> 	else if (IS_CANNONLAKE(dev_priv))
>@@ -3711,7 +3711,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_ICELAKE(dev_priv) && 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_CANNONLAKE(dev_priv) && crtc_state->port_clock > 594000)
> 		crtc_state->min_voltage_level = 2;
>diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
>index 855a5074ad77..e34259989ead 100644
>--- a/drivers/gpu/drm/i915/intel_device_info.c
>+++ b/drivers/gpu/drm/i915/intel_device_info.c
>@@ -740,7 +740,7 @@ void intel_device_info_runtime_init(struct drm_i915_private *dev_priv)
>
> 	BUILD_BUG_ON(I915_NUM_ENGINES > BITS_PER_TYPE(intel_ring_mask_t));
>
>-	if (IS_GEN(dev_priv, 11))
>+	if (INTEL_GEN(dev_priv) >= 11)
> 		for_each_pipe(dev_priv, pipe)
> 			runtime->num_sprites[pipe] = 6;
> 	else if (IS_GEN(dev_priv, 10) || IS_GEMINILAKE(dev_priv))
>diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>index 7c5e84ef5171..fdefdc33a0de 100644
>--- a/drivers/gpu/drm/i915/intel_display.c
>+++ b/drivers/gpu/drm/i915/intel_display.c
>@@ -5036,10 +5036,10 @@ skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach,
> 	/* range checks */
> 	if (src_w < SKL_MIN_SRC_W || src_h < SKL_MIN_SRC_H ||
> 	    dst_w < SKL_MIN_DST_W || dst_h < SKL_MIN_DST_H ||
>-	    (IS_GEN(dev_priv, 11) &&
>+	    (INTEL_GEN(dev_priv) >= 11 &&
> 	     (src_w > ICL_MAX_SRC_W || src_h > ICL_MAX_SRC_H ||
> 	      dst_w > ICL_MAX_DST_W || dst_h > ICL_MAX_DST_H)) ||
>-	    (!IS_GEN(dev_priv, 11) &&
>+	    (INTEL_GEN(dev_priv) < 11 &&
> 	     (src_w > SKL_MAX_SRC_W || src_h > SKL_MAX_SRC_H ||
> 	      dst_w > SKL_MAX_DST_W || dst_h > SKL_MAX_DST_H)))	{
> 		DRM_DEBUG_KMS("scaler_user index %u.%u: src %ux%u dst %ux%u "
>@@ -6131,7 +6131,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;
>@@ -6139,7 +6139,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;
>@@ -9550,7 +9550,7 @@ static int haswell_crtc_compute_clock(struct intel_crtc *crtc,
> 		to_intel_atomic_state(crtc_state->base.state);
>
> 	if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI) ||
>-	    IS_ICELAKE(dev_priv)) {
>+	    INTEL_GEN(dev_priv) >= 11) {
> 		struct intel_encoder *encoder =
> 			intel_get_crtc_new_encoder(state, crtc_state);
>
>@@ -9693,7 +9693,7 @@ static bool hsw_get_transcoder_state(struct intel_crtc *crtc,
> 	enum transcoder panel_transcoder;
> 	u32 tmp;
>
>-	if (IS_ICELAKE(dev_priv))
>+	if (INTEL_GEN(dev_priv) >= 11)
> 		panel_transcoder_mask |=
> 			BIT(TRANSCODER_DSI_0) | BIT(TRANSCODER_DSI_1);
>
>@@ -9826,7 +9826,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_CANNONLAKE(dev_priv))
> 		cannonlake_get_ddi_pll(dev_priv, port, pipe_config);
>@@ -9889,7 +9889,7 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
> 		goto out;
>
> 	if (!transcoder_is_dsi(pipe_config->cpu_transcoder) ||
>-	    IS_ICELAKE(dev_priv)) {
>+	    INTEL_GEN(dev_priv) >= 11) {
> 		haswell_get_ddi_port_state(crtc, pipe_config);
> 		intel_get_pipe_timings(crtc, pipe_config);
> 	}
>@@ -14635,7 +14635,7 @@ static void intel_setup_outputs(struct drm_i915_private *dev_priv)
> 	if (!HAS_DISPLAY(dev_priv))
> 		return;
>
>-	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 e4ec73d415d9..b3fb221c2532 100644
>--- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
>+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
>@@ -3259,7 +3259,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_CANNONLAKE(dev_priv))
> 		dpll_mgr = &cnl_pll_mgr;
>diff --git a/drivers/gpu/drm/i915/intel_dsi_vbt.c b/drivers/gpu/drm/i915/intel_dsi_vbt.c
>index 06a11c35a784..d1e00e4c7726 100644
>--- a/drivers/gpu/drm/i915/intel_dsi_vbt.c
>+++ b/drivers/gpu/drm/i915/intel_dsi_vbt.c
>@@ -194,7 +194,7 @@ static const u8 *mipi_exec_send_packet(struct intel_dsi *intel_dsi,
> 		break;
> 	}
>
>-	if (!IS_ICELAKE(dev_priv))
>+	if (INTEL_GEN(dev_priv) < 11)
> 		vlv_dsi_wait_for_fifo_empty(intel_dsi, port);
>
> out:
>@@ -365,7 +365,7 @@ static const u8 *mipi_exec_gpio(struct intel_dsi *intel_dsi, const u8 *data)
> 	/* pull up/down */
> 	value = *data++ & 1;
>
>-	if (IS_ICELAKE(dev_priv))
>+	if (INTEL_GEN(dev_priv) >= 11)
> 		icl_exec_gpio(dev_priv, gpio_source, gpio_index, value);
> 	else if (IS_VALLEYVIEW(dev_priv))
> 		vlv_exec_gpio(dev_priv, gpio_source, gpio_number, value);
>@@ -890,7 +890,7 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id)
>
> 	intel_dsi->burst_mode_ratio = burst_mode_ratio;
>
>-	if (IS_ICELAKE(dev_priv))
>+	if (INTEL_GEN(dev_priv) >= 11)
> 		icl_dphy_param_init(intel_dsi);
> 	else
> 		vlv_dphy_param_init(intel_dsi);
>diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
>index cd422a7b4da0..5ccb305a6e1c 100644
>--- a/drivers/gpu/drm/i915/intel_hdmi.c
>+++ b/drivers/gpu/drm/i915/intel_hdmi.c
>@@ -2206,7 +2206,7 @@ static bool hdmi_deep_color_possible(const struct intel_crtc_state *crtc_state,
>
> 	/* Display Wa_1405510057:icl */
> 	if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420 &&
>-	    bpc == 10 && IS_ICELAKE(dev_priv) &&
>+	    bpc == 10 && INTEL_GEN(dev_priv) >= 11 &&
> 	    (adjusted_mode->crtc_hblank_end -
> 	     adjusted_mode->crtc_hblank_start) % 8 == 2)
> 		return false;
>@@ -2500,7 +2500,7 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
>
> 	wakeref = 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_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
>index 578c8c98c718..fb599e11e7b9 100644
>--- a/drivers/gpu/drm/i915/intel_lrc.c
>+++ b/drivers/gpu/drm/i915/intel_lrc.c
>@@ -2525,7 +2525,7 @@ u32 gen8_make_rpcs(struct drm_i915_private *i915, struct intel_sseu *req_sseu)
> 	} else {
> 		ctx_sseu = intel_device_default_sseu(i915);
>
>-		if (IS_GEN(i915, 11)) {
>+		if (INTEL_GEN(i915) >= 11) {
> 			/*
> 			 * We only need subslice count so it doesn't matter
> 			 * which ones we select - just turn off low bits in the
>@@ -2565,7 +2565,7 @@ u32 gen8_make_rpcs(struct drm_i915_private *i915, struct intel_sseu *req_sseu)
> 	 * subslices are enabled, or a count between one and four on the first
> 	 * slice.
> 	 */
>-	if (IS_GEN(i915, 11) &&
>+	if (INTEL_GEN(i915) >= 11 &&
> 	    slices == 1 &&
> 	    subslices > min_t(u8, 4, hweight8(sseu->subslice_mask[0]) / 2)) {
> 		GEM_BUG_ON(subslices & 1);
>diff --git a/drivers/gpu/drm/i915/intel_mocs.c b/drivers/gpu/drm/i915/intel_mocs.c
>index 331e7a678fb7..79913b06f455 100644
>--- a/drivers/gpu/drm/i915/intel_mocs.c
>+++ b/drivers/gpu/drm/i915/intel_mocs.c
>@@ -252,7 +252,7 @@ static bool get_mocs_settings(struct drm_i915_private *dev_priv,
> {
> 	bool result = false;
>
>-	if (IS_ICELAKE(dev_priv)) {
>+	if (INTEL_GEN(dev_priv) >= 11) {
> 		table->size  = ARRAY_SIZE(icelake_mocs_table);
> 		table->table = icelake_mocs_table;
> 		table->n_entries = GEN11_NUM_MOCS_ENTRIES;
>-- 
>2.20.1
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 3/3] drm/i915: Start using comparative INTEL_PCH_TYPE
  2019-03-05 17:08     ` Lucas De Marchi
@ 2019-03-05 17:16       ` Jani Nikula
  2019-03-05 17:42         ` Lucas De Marchi
  0 siblings, 1 reply; 20+ messages in thread
From: Jani Nikula @ 2019-03-05 17:16 UTC (permalink / raw)
  To: Lucas De Marchi; +Cc: intel-gfx

On Tue, 05 Mar 2019, Lucas De Marchi <lucas.demarchi@intel.com> wrote:
> On Tue, Mar 05, 2019 at 04:10:04PM +0200, Jani Nikula wrote:
>>On Mon, 04 Mar 2019, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
>>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>>> index e1a051c0fbfe..acd2336bb214 100644
>>> --- a/drivers/gpu/drm/i915/intel_dp.c
>>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>>> @@ -949,8 +949,8 @@ static void intel_pps_get_registers(struct intel_dp *intel_dp,
>>>  	regs->pp_stat = PP_STATUS(pps_idx);
>>>  	regs->pp_on = PP_ON_DELAYS(pps_idx);
>>>  	regs->pp_off = PP_OFF_DELAYS(pps_idx);
>>> -	if (!IS_GEN9_LP(dev_priv) && !HAS_PCH_CNP(dev_priv) &&
>>> -	    !HAS_PCH_ICP(dev_priv))
>>> +	if (INTEL_PCH_TYPE(dev_priv) > PCH_NONE &&
>>> +	    INTEL_PCH_TYPE(dev_priv) < PCH_CNP)
>>
>>This is not right, starts to require PCH.
>
> But in those cases INTEL_PCH_TYPE() will return either NONE or NOP.

Exactly. Non-PCH platforms before CNP should match, but won't.

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

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

* Re: [PATCH 2/3] drm/i915: Move PCH_NOP to -1
  2019-03-04 22:48 ` [PATCH 2/3] drm/i915: Move PCH_NOP to -1 Rodrigo Vivi
@ 2019-03-05 17:38   ` Lucas De Marchi
  0 siblings, 0 replies; 20+ messages in thread
From: Lucas De Marchi @ 2019-03-05 17:38 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: intel-gfx

On Mon, Mar 04, 2019 at 02:48:29PM -0800, Rodrigo Vivi wrote:
>So we can later use PCH >= comparisons. The ultimate goal
>is to make it easier for us to introduce a new platform
>with south display engine on PCH just by reusing the previous
>one.
>
>Suggested-by: Lucas De Marchi <lucas.demarchi@intel.com>
>Cc: Lucas De Marchi <lucas.demarchi@intel.com>
>Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>

Lucas De Marchi

>---
> drivers/gpu/drm/i915/i915_drv.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>index 453af7438e67..e6be327ba86d 100644
>--- a/drivers/gpu/drm/i915/i915_drv.h
>+++ b/drivers/gpu/drm/i915/i915_drv.h
>@@ -524,6 +524,7 @@ struct i915_psr {
> };
>
> enum intel_pch {
>+	PCH_NOP = -1,	/* PCH without south display */
> 	PCH_NONE = 0,	/* No PCH present */
> 	PCH_IBX,	/* Ibexpeak PCH */
> 	PCH_CPT,	/* Cougarpoint/Pantherpoint PCH */
>@@ -532,7 +533,6 @@ enum intel_pch {
> 	PCH_KBP,        /* Kaby Lake PCH */
> 	PCH_CNP,        /* Cannon Lake PCH */
> 	PCH_ICP,	/* Ice Lake PCH */
>-	PCH_NOP,	/* PCH without south display */
> };
>
> enum intel_sbi_destination {
>-- 
>2.20.1
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 3/3] drm/i915: Start using comparative INTEL_PCH_TYPE
  2019-03-05 17:16       ` Jani Nikula
@ 2019-03-05 17:42         ` Lucas De Marchi
  2019-03-05 18:38           ` Rodrigo Vivi
  0 siblings, 1 reply; 20+ messages in thread
From: Lucas De Marchi @ 2019-03-05 17:42 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

On Tue, Mar 05, 2019 at 07:16:47PM +0200, Jani Nikula wrote:
>On Tue, 05 Mar 2019, Lucas De Marchi <lucas.demarchi@intel.com> wrote:
>> On Tue, Mar 05, 2019 at 04:10:04PM +0200, Jani Nikula wrote:
>>>On Mon, 04 Mar 2019, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
>>>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>>>> index e1a051c0fbfe..acd2336bb214 100644
>>>> --- a/drivers/gpu/drm/i915/intel_dp.c
>>>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>>>> @@ -949,8 +949,8 @@ static void intel_pps_get_registers(struct intel_dp *intel_dp,
>>>>  	regs->pp_stat = PP_STATUS(pps_idx);
>>>>  	regs->pp_on = PP_ON_DELAYS(pps_idx);
>>>>  	regs->pp_off = PP_OFF_DELAYS(pps_idx);
>>>> -	if (!IS_GEN9_LP(dev_priv) && !HAS_PCH_CNP(dev_priv) &&
>>>> -	    !HAS_PCH_ICP(dev_priv))
>>>> +	if (INTEL_PCH_TYPE(dev_priv) > PCH_NONE &&
>>>> +	    INTEL_PCH_TYPE(dev_priv) < PCH_CNP)
>>>
>>>This is not right, starts to require PCH.
>>
>> But in those cases INTEL_PCH_TYPE() will return either NONE or NOP.
>
>Exactly. Non-PCH platforms before CNP should match, but won't.

yeah, right. I misread the !IS_GEN9_LP().

Lucas De Marchi

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

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

* Re: [PATCH 1/3] drm/i915/gen11+: First assume next platforms will inherit stuff
  2019-03-04 22:48 [PATCH 1/3] drm/i915/gen11+: First assume next platforms will inherit stuff Rodrigo Vivi
                   ` (4 preceding siblings ...)
  2019-03-05 17:12 ` [PATCH 1/3] " Lucas De Marchi
@ 2019-03-05 17:43 ` Tvrtko Ursulin
  2019-03-05 18:36   ` Lucas De Marchi
  5 siblings, 1 reply; 20+ messages in thread
From: Tvrtko Ursulin @ 2019-03-05 17:43 UTC (permalink / raw)
  To: Rodrigo Vivi, intel-gfx; +Cc: Lucas De Marchi


On 04/03/2019 22:48, Rodrigo Vivi wrote:
> This exactly same approach was already used from gen9
> to gen10 and from gen10 to gen11. Let's also use it
> for gen11+.
> 
> Let's first assume that we inherit a similar platform
> and than we apply the differences on top.
> 
> Different from the previous attempts this will be
> done this time with coccinelle. We obviously need to
> exclude some case that is really exclusive for gen11
> like  PCH, Firmware, and few others. Luckly this was
> easy to filter by selecting the files we are touching
> with coccinelle as exposed below:
> 
> spatch -sp_file gen11\+.cocci --in-place i915_perf.c \
>         intel_bios.c intel_cdclk.c intel_ddi.c \
>         intel_device_info.c intel_display.c intel_dpll_mgr.c \
>         intel_dsi_vbt.c intel_hdmi.c intel_lrc.c intel_mocs.c intel_color.c
> 
> @noticelake@ expression e; @@
> -!IS_ICELAKE(e)
> +INTEL_GEN(e) < 11
> @notgen11@ expression e; @@
> -!IS_GEN(e, 11)
> +INTEL_GEN(e) < 11
> @icelake@ expression e; @@
> -IS_ICELAKE(e)
> +INTEL_GEN(e) >= 11
> @gen11@ expression e; @@
> -IS_GEN(e, 11)
> +INTEL_GEN(e) >= 11
> 
> No functional change.
> 
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_perf.c         |  2 +-
>   drivers/gpu/drm/i915/intel_bios.c        |  4 ++--
>   drivers/gpu/drm/i915/intel_cdclk.c       |  6 +++---
>   drivers/gpu/drm/i915/intel_color.c       |  2 +-
>   drivers/gpu/drm/i915/intel_ddi.c         | 18 +++++++++---------
>   drivers/gpu/drm/i915/intel_device_info.c |  2 +-
>   drivers/gpu/drm/i915/intel_display.c     | 18 +++++++++---------
>   drivers/gpu/drm/i915/intel_dpll_mgr.c    |  2 +-
>   drivers/gpu/drm/i915/intel_dsi_vbt.c     |  6 +++---
>   drivers/gpu/drm/i915/intel_hdmi.c        |  4 ++--
>   drivers/gpu/drm/i915/intel_lrc.c         |  4 ++--
>   drivers/gpu/drm/i915/intel_mocs.c        |  2 +-
>   12 files changed, 35 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
> index 72a9a35b40e2..c81feb43da90 100644
> --- a/drivers/gpu/drm/i915/i915_perf.c
> +++ b/drivers/gpu/drm/i915/i915_perf.c
> @@ -2881,7 +2881,7 @@ void i915_perf_register(struct drm_i915_private *dev_priv)
>   
>   	sysfs_attr_init(&dev_priv->perf.oa.test_config.sysfs_metric_id.attr);
>   
> -	if (IS_ICELAKE(dev_priv)) {
> +	if (INTEL_GEN(dev_priv) >= 11) {
>   		i915_perf_load_test_config_icl(dev_priv);

Ping Lionel if this is OK?

>   	} else if (IS_CANNONLAKE(dev_priv)) {
>   		i915_perf_load_test_config_cnl(dev_priv);
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index b508d8a735e0..48c62bea92cd 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -2093,8 +2093,8 @@ bool intel_bios_is_dsi_present(struct drm_i915_private *dev_priv,
>   		dvo_port = child->dvo_port;
>   
>   		if (dvo_port == DVO_PORT_MIPIA ||
> -		    (dvo_port == DVO_PORT_MIPIB && IS_ICELAKE(dev_priv)) ||
> -		    (dvo_port == DVO_PORT_MIPIC && !IS_ICELAKE(dev_priv))) {
> +		    (dvo_port == DVO_PORT_MIPIB && INTEL_GEN(dev_priv) >= 11) ||
> +		    (dvo_port == DVO_PORT_MIPIC && INTEL_GEN(dev_priv) < 11)) {
>   			if (port)
>   				*port = dvo_port - DVO_PORT_MIPIA;
>   			return true;
> diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
> index 5d266538036d..7e5132772477 100644
> --- a/drivers/gpu/drm/i915/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> @@ -2560,7 +2560,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
> @@ -2744,7 +2744,7 @@ void intel_update_rawclk(struct drm_i915_private *dev_priv)
>    */
>   void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv)
>   {
> -	if (IS_ICELAKE(dev_priv)) {
> +	if (INTEL_GEN(dev_priv) >= 11) {
>   		dev_priv->display.set_cdclk = icl_set_cdclk;
>   		dev_priv->display.modeset_calc_cdclk = icl_modeset_calc_cdclk;
>   	} else if (IS_CANNONLAKE(dev_priv)) {
> @@ -2773,7 +2773,7 @@ void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv)
>   			vlv_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_CANNONLAKE(dev_priv))
>   		dev_priv->display.get_cdclk = cnl_get_cdclk;
> diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
> index da7a07d5ccea..0173967ed593 100644
> --- a/drivers/gpu/drm/i915/intel_color.c
> +++ b/drivers/gpu/drm/i915/intel_color.c
> @@ -841,7 +841,7 @@ void intel_color_init(struct intel_crtc *crtc)
>   
>   		dev_priv->display.color_commit = i9xx_color_commit;
>   	} else {
> -		if (IS_ICELAKE(dev_priv))
> +		if (INTEL_GEN(dev_priv) >= 11)
>   			dev_priv->display.load_luts = icl_load_luts;
>   		else if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
>   			dev_priv->display.load_luts = glk_load_luts;
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index d918be927fc2..5b132082a650 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -851,7 +851,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,
>   						0, &n_entries);
> @@ -1678,7 +1678,7 @@ static void intel_ddi_clock_get(struct intel_encoder *encoder,
>   {
>   	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>   
> -	if (IS_ICELAKE(dev_priv))
> +	if (INTEL_GEN(dev_priv) >= 11)
>   		icl_ddi_clock_get(encoder, pipe_config);
>   	else if (IS_CANNONLAKE(dev_priv))
>   		cnl_ddi_clock_get(encoder, pipe_config);
> @@ -2225,7 +2225,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,
>   						intel_dp->link_rate, &n_entries);
> @@ -2698,7 +2698,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_CANNONLAKE(dev_priv))
> @@ -2867,7 +2867,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_clk_sel(encoder, crtc_state));
> @@ -2909,7 +2909,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_CANNONLAKE(dev_priv)) {
> @@ -3126,7 +3126,7 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
>   	icl_program_mg_dp_mode(dig_port);
>   	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_CANNONLAKE(dev_priv))
> @@ -3175,7 +3175,7 @@ static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder,
>   	icl_program_mg_dp_mode(dig_port);
>   	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, INTEL_OUTPUT_HDMI);
>   	else if (IS_CANNONLAKE(dev_priv))
> @@ -3711,7 +3711,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_ICELAKE(dev_priv) && 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_CANNONLAKE(dev_priv) && crtc_state->port_clock > 594000)
>   		crtc_state->min_voltage_level = 2;
> diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
> index 855a5074ad77..e34259989ead 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.c
> +++ b/drivers/gpu/drm/i915/intel_device_info.c
> @@ -740,7 +740,7 @@ void intel_device_info_runtime_init(struct drm_i915_private *dev_priv)
>   
>   	BUILD_BUG_ON(I915_NUM_ENGINES > BITS_PER_TYPE(intel_ring_mask_t));
>   
> -	if (IS_GEN(dev_priv, 11))
> +	if (INTEL_GEN(dev_priv) >= 11)
>   		for_each_pipe(dev_priv, pipe)
>   			runtime->num_sprites[pipe] = 6;
>   	else if (IS_GEN(dev_priv, 10) || IS_GEMINILAKE(dev_priv))
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 7c5e84ef5171..fdefdc33a0de 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5036,10 +5036,10 @@ skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach,
>   	/* range checks */
>   	if (src_w < SKL_MIN_SRC_W || src_h < SKL_MIN_SRC_H ||
>   	    dst_w < SKL_MIN_DST_W || dst_h < SKL_MIN_DST_H ||
> -	    (IS_GEN(dev_priv, 11) &&
> +	    (INTEL_GEN(dev_priv) >= 11 &&
>   	     (src_w > ICL_MAX_SRC_W || src_h > ICL_MAX_SRC_H ||
>   	      dst_w > ICL_MAX_DST_W || dst_h > ICL_MAX_DST_H)) ||
> -	    (!IS_GEN(dev_priv, 11) &&
> +	    (INTEL_GEN(dev_priv) < 11 &&
>   	     (src_w > SKL_MAX_SRC_W || src_h > SKL_MAX_SRC_H ||
>   	      dst_w > SKL_MAX_DST_W || dst_h > SKL_MAX_DST_H)))	{
>   		DRM_DEBUG_KMS("scaler_user index %u.%u: src %ux%u dst %ux%u "
> @@ -6131,7 +6131,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;
> @@ -6139,7 +6139,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;
> @@ -9550,7 +9550,7 @@ static int haswell_crtc_compute_clock(struct intel_crtc *crtc,
>   		to_intel_atomic_state(crtc_state->base.state);
>   
>   	if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI) ||
> -	    IS_ICELAKE(dev_priv)) {
> +	    INTEL_GEN(dev_priv) >= 11) {
>   		struct intel_encoder *encoder =
>   			intel_get_crtc_new_encoder(state, crtc_state);
>   
> @@ -9693,7 +9693,7 @@ static bool hsw_get_transcoder_state(struct intel_crtc *crtc,
>   	enum transcoder panel_transcoder;
>   	u32 tmp;
>   
> -	if (IS_ICELAKE(dev_priv))
> +	if (INTEL_GEN(dev_priv) >= 11)
>   		panel_transcoder_mask |=
>   			BIT(TRANSCODER_DSI_0) | BIT(TRANSCODER_DSI_1);
>   
> @@ -9826,7 +9826,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_CANNONLAKE(dev_priv))
>   		cannonlake_get_ddi_pll(dev_priv, port, pipe_config);
> @@ -9889,7 +9889,7 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
>   		goto out;
>   
>   	if (!transcoder_is_dsi(pipe_config->cpu_transcoder) ||
> -	    IS_ICELAKE(dev_priv)) {
> +	    INTEL_GEN(dev_priv) >= 11) {
>   		haswell_get_ddi_port_state(crtc, pipe_config);
>   		intel_get_pipe_timings(crtc, pipe_config);
>   	}
> @@ -14635,7 +14635,7 @@ static void intel_setup_outputs(struct drm_i915_private *dev_priv)
>   	if (!HAS_DISPLAY(dev_priv))
>   		return;
>   
> -	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 e4ec73d415d9..b3fb221c2532 100644
> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> @@ -3259,7 +3259,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_CANNONLAKE(dev_priv))
>   		dpll_mgr = &cnl_pll_mgr;
> diff --git a/drivers/gpu/drm/i915/intel_dsi_vbt.c b/drivers/gpu/drm/i915/intel_dsi_vbt.c
> index 06a11c35a784..d1e00e4c7726 100644
> --- a/drivers/gpu/drm/i915/intel_dsi_vbt.c
> +++ b/drivers/gpu/drm/i915/intel_dsi_vbt.c
> @@ -194,7 +194,7 @@ static const u8 *mipi_exec_send_packet(struct intel_dsi *intel_dsi,
>   		break;
>   	}
>   
> -	if (!IS_ICELAKE(dev_priv))
> +	if (INTEL_GEN(dev_priv) < 11)
>   		vlv_dsi_wait_for_fifo_empty(intel_dsi, port);
>   
>   out:
> @@ -365,7 +365,7 @@ static const u8 *mipi_exec_gpio(struct intel_dsi *intel_dsi, const u8 *data)
>   	/* pull up/down */
>   	value = *data++ & 1;
>   
> -	if (IS_ICELAKE(dev_priv))
> +	if (INTEL_GEN(dev_priv) >= 11)
>   		icl_exec_gpio(dev_priv, gpio_source, gpio_index, value);
>   	else if (IS_VALLEYVIEW(dev_priv))
>   		vlv_exec_gpio(dev_priv, gpio_source, gpio_number, value);
> @@ -890,7 +890,7 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id)
>   
>   	intel_dsi->burst_mode_ratio = burst_mode_ratio;
>   
> -	if (IS_ICELAKE(dev_priv))
> +	if (INTEL_GEN(dev_priv) >= 11)
>   		icl_dphy_param_init(intel_dsi);
>   	else
>   		vlv_dphy_param_init(intel_dsi);
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index cd422a7b4da0..5ccb305a6e1c 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -2206,7 +2206,7 @@ static bool hdmi_deep_color_possible(const struct intel_crtc_state *crtc_state,
>   
>   	/* Display Wa_1405510057:icl */
>   	if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420 &&
> -	    bpc == 10 && IS_ICELAKE(dev_priv) &&
> +	    bpc == 10 && INTEL_GEN(dev_priv) >= 11 &&
>   	    (adjusted_mode->crtc_hblank_end -
>   	     adjusted_mode->crtc_hblank_start) % 8 == 2)
>   		return false;
> @@ -2500,7 +2500,7 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
>   
>   	wakeref = 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_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 578c8c98c718..fb599e11e7b9 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -2525,7 +2525,7 @@ u32 gen8_make_rpcs(struct drm_i915_private *i915, struct intel_sseu *req_sseu)
>   	} else {
>   		ctx_sseu = intel_device_default_sseu(i915);
>   
> -		if (IS_GEN(i915, 11)) {
> +		if (INTEL_GEN(i915) >= 11) {

This one needs to stay since it is handling a specific Icelake hw issue 
and media related configuration.

>   			/*
>   			 * We only need subslice count so it doesn't matter
>   			 * which ones we select - just turn off low bits in the
> @@ -2565,7 +2565,7 @@ u32 gen8_make_rpcs(struct drm_i915_private *i915, struct intel_sseu *req_sseu)
>   	 * subslices are enabled, or a count between one and four on the first
>   	 * slice.
>   	 */
> -	if (IS_GEN(i915, 11) &&
> +	if (INTEL_GEN(i915) >= 11 &&

This one needs to stay as well for now.

>   	    slices == 1 &&
>   	    subslices > min_t(u8, 4, hweight8(sseu->subslice_mask[0]) / 2)) {
>   		GEM_BUG_ON(subslices & 1);
> diff --git a/drivers/gpu/drm/i915/intel_mocs.c b/drivers/gpu/drm/i915/intel_mocs.c
> index 331e7a678fb7..79913b06f455 100644
> --- a/drivers/gpu/drm/i915/intel_mocs.c
> +++ b/drivers/gpu/drm/i915/intel_mocs.c
> @@ -252,7 +252,7 @@ static bool get_mocs_settings(struct drm_i915_private *dev_priv,
>   {
>   	bool result = false;
>   
> -	if (IS_ICELAKE(dev_priv)) {
> +	if (INTEL_GEN(dev_priv) >= 11) {
>   		table->size  = ARRAY_SIZE(icelake_mocs_table);
>   		table->table = icelake_mocs_table;
>   		table->n_entries = GEN11_NUM_MOCS_ENTRIES;
> 

Lucas should know if this is OK.

Regards,

Tvrtko


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

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

* Re: [PATCH 1/3] drm/i915/gen11+: First assume next platforms will inherit stuff
  2019-03-05 17:43 ` Tvrtko Ursulin
@ 2019-03-05 18:36   ` Lucas De Marchi
  0 siblings, 0 replies; 20+ messages in thread
From: Lucas De Marchi @ 2019-03-05 18:36 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: intel-gfx

On Tue, Mar 05, 2019 at 05:43:15PM +0000, Tvrtko Ursulin wrote:
>>diff --git a/drivers/gpu/drm/i915/intel_mocs.c b/drivers/gpu/drm/i915/intel_mocs.c
>>index 331e7a678fb7..79913b06f455 100644
>>--- a/drivers/gpu/drm/i915/intel_mocs.c
>>+++ b/drivers/gpu/drm/i915/intel_mocs.c
>>@@ -252,7 +252,7 @@ static bool get_mocs_settings(struct drm_i915_private *dev_priv,
>>  {
>>  	bool result = false;
>>-	if (IS_ICELAKE(dev_priv)) {
>>+	if (INTEL_GEN(dev_priv) >= 11) {
>>  		table->size  = ARRAY_SIZE(icelake_mocs_table);
>>  		table->table = icelake_mocs_table;
>>  		table->n_entries = GEN11_NUM_MOCS_ENTRIES;
>>
>
>Lucas should know if this is OK.

I prefer having it like this and later change if it's different for gen
12. It will make it more consistent on how to add a new one.

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

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

* Re: [PATCH 3/3] drm/i915: Start using comparative INTEL_PCH_TYPE
  2019-03-05 17:42         ` Lucas De Marchi
@ 2019-03-05 18:38           ` Rodrigo Vivi
  2019-03-05 18:48             ` Lucas De Marchi
  0 siblings, 1 reply; 20+ messages in thread
From: Rodrigo Vivi @ 2019-03-05 18:38 UTC (permalink / raw)
  To: Lucas De Marchi; +Cc: intel-gfx

On Tue, Mar 05, 2019 at 09:42:22AM -0800, Lucas De Marchi wrote:
> On Tue, Mar 05, 2019 at 07:16:47PM +0200, Jani Nikula wrote:
> > On Tue, 05 Mar 2019, Lucas De Marchi <lucas.demarchi@intel.com> wrote:
> > > On Tue, Mar 05, 2019 at 04:10:04PM +0200, Jani Nikula wrote:
> > > > On Mon, 04 Mar 2019, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > > > > index e1a051c0fbfe..acd2336bb214 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > > > @@ -949,8 +949,8 @@ static void intel_pps_get_registers(struct intel_dp *intel_dp,
> > > > >  	regs->pp_stat = PP_STATUS(pps_idx);
> > > > >  	regs->pp_on = PP_ON_DELAYS(pps_idx);
> > > > >  	regs->pp_off = PP_OFF_DELAYS(pps_idx);
> > > > > -	if (!IS_GEN9_LP(dev_priv) && !HAS_PCH_CNP(dev_priv) &&
> > > > > -	    !HAS_PCH_ICP(dev_priv))
> > > > > +	if (INTEL_PCH_TYPE(dev_priv) > PCH_NONE &&
> > > > > +	    INTEL_PCH_TYPE(dev_priv) < PCH_CNP)
> > > > 
> > > > This is not right, starts to require PCH.
> > > 
> > > But in those cases INTEL_PCH_TYPE() will return either NONE or NOP.
> > 
> > Exactly. Non-PCH platforms before CNP should match, but won't.
> 
> yeah, right. I misread the !IS_GEN9_LP().

ouch... indeed.
probably this explains failure on ci for bsw and byt

options:

1. if (!IS_GEN9_LP(dev_priv) && !(INTEL_PCH_TYPE(dev_priv) >= 10))

2. if (INTEL_GEN(dev_priv) <= 8 || IS_GEN9_BC(dev_priv))

3. other ideas?

> 
> Lucas De Marchi
> 
> > 
> > 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

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

* Re: [PATCH 3/3] drm/i915: Start using comparative INTEL_PCH_TYPE
  2019-03-05 18:38           ` Rodrigo Vivi
@ 2019-03-05 18:48             ` Lucas De Marchi
  2019-03-05 19:46               ` Jani Nikula
  0 siblings, 1 reply; 20+ messages in thread
From: Lucas De Marchi @ 2019-03-05 18:48 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: intel-gfx

On Tue, Mar 05, 2019 at 10:38:46AM -0800, Rodrigo Vivi wrote:
>On Tue, Mar 05, 2019 at 09:42:22AM -0800, Lucas De Marchi wrote:
>> On Tue, Mar 05, 2019 at 07:16:47PM +0200, Jani Nikula wrote:
>> > On Tue, 05 Mar 2019, Lucas De Marchi <lucas.demarchi@intel.com> wrote:
>> > > On Tue, Mar 05, 2019 at 04:10:04PM +0200, Jani Nikula wrote:
>> > > > On Mon, 04 Mar 2019, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
>> > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> > > > > index e1a051c0fbfe..acd2336bb214 100644
>> > > > > --- a/drivers/gpu/drm/i915/intel_dp.c
>> > > > > +++ b/drivers/gpu/drm/i915/intel_dp.c
>> > > > > @@ -949,8 +949,8 @@ static void intel_pps_get_registers(struct intel_dp *intel_dp,
>> > > > >  	regs->pp_stat = PP_STATUS(pps_idx);
>> > > > >  	regs->pp_on = PP_ON_DELAYS(pps_idx);
>> > > > >  	regs->pp_off = PP_OFF_DELAYS(pps_idx);
>> > > > > -	if (!IS_GEN9_LP(dev_priv) && !HAS_PCH_CNP(dev_priv) &&
>> > > > > -	    !HAS_PCH_ICP(dev_priv))
>> > > > > +	if (INTEL_PCH_TYPE(dev_priv) > PCH_NONE &&
>> > > > > +	    INTEL_PCH_TYPE(dev_priv) < PCH_CNP)
>> > > >
>> > > > This is not right, starts to require PCH.
>> > >
>> > > But in those cases INTEL_PCH_TYPE() will return either NONE or NOP.
>> >
>> > Exactly. Non-PCH platforms before CNP should match, but won't.
>>
>> yeah, right. I misread the !IS_GEN9_LP().
>
>ouch... indeed.
>probably this explains failure on ci for bsw and byt
>
>options:
>
>1. if (!IS_GEN9_LP(dev_priv) && !(INTEL_PCH_TYPE(dev_priv) >= 10))

10? I think you meant PCH_CNP

>
>2. if (INTEL_GEN(dev_priv) <= 8 || IS_GEN9_BC(dev_priv))
>
>3. other ideas?

"all PCHs before CNP, excluding GEN9_LP":

	if (INTEL_PCH_TYPE(dev_priv) < PCH_CNP && !IS_GEN9_LP(dev_priv))


Lucas De Marchi

>
>>
>> Lucas De Marchi
>>
>> >
>> > 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

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

* Re: [PATCH 3/3] drm/i915: Start using comparative INTEL_PCH_TYPE
  2019-03-05 18:48             ` Lucas De Marchi
@ 2019-03-05 19:46               ` Jani Nikula
  2019-03-05 20:24                 ` Rodrigo Vivi
  0 siblings, 1 reply; 20+ messages in thread
From: Jani Nikula @ 2019-03-05 19:46 UTC (permalink / raw)
  To: Lucas De Marchi, Rodrigo Vivi; +Cc: intel-gfx

On Tue, 05 Mar 2019, Lucas De Marchi <lucas.demarchi@intel.com> wrote:
> On Tue, Mar 05, 2019 at 10:38:46AM -0800, Rodrigo Vivi wrote:
>>On Tue, Mar 05, 2019 at 09:42:22AM -0800, Lucas De Marchi wrote:
>>> On Tue, Mar 05, 2019 at 07:16:47PM +0200, Jani Nikula wrote:
>>> > On Tue, 05 Mar 2019, Lucas De Marchi <lucas.demarchi@intel.com> wrote:
>>> > > On Tue, Mar 05, 2019 at 04:10:04PM +0200, Jani Nikula wrote:
>>> > > > On Mon, 04 Mar 2019, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
>>> > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>>> > > > > index e1a051c0fbfe..acd2336bb214 100644
>>> > > > > --- a/drivers/gpu/drm/i915/intel_dp.c
>>> > > > > +++ b/drivers/gpu/drm/i915/intel_dp.c
>>> > > > > @@ -949,8 +949,8 @@ static void intel_pps_get_registers(struct intel_dp *intel_dp,
>>> > > > >  	regs->pp_stat = PP_STATUS(pps_idx);
>>> > > > >  	regs->pp_on = PP_ON_DELAYS(pps_idx);
>>> > > > >  	regs->pp_off = PP_OFF_DELAYS(pps_idx);
>>> > > > > -	if (!IS_GEN9_LP(dev_priv) && !HAS_PCH_CNP(dev_priv) &&
>>> > > > > -	    !HAS_PCH_ICP(dev_priv))
>>> > > > > +	if (INTEL_PCH_TYPE(dev_priv) > PCH_NONE &&
>>> > > > > +	    INTEL_PCH_TYPE(dev_priv) < PCH_CNP)
>>> > > >
>>> > > > This is not right, starts to require PCH.
>>> > >
>>> > > But in those cases INTEL_PCH_TYPE() will return either NONE or NOP.
>>> >
>>> > Exactly. Non-PCH platforms before CNP should match, but won't.
>>>
>>> yeah, right. I misread the !IS_GEN9_LP().
>>
>>ouch... indeed.
>>probably this explains failure on ci for bsw and byt
>>
>>options:
>>
>>1. if (!IS_GEN9_LP(dev_priv) && !(INTEL_PCH_TYPE(dev_priv) >= 10))
>
> 10? I think you meant PCH_CNP
>
>>
>>2. if (INTEL_GEN(dev_priv) <= 8 || IS_GEN9_BC(dev_priv))
>>
>>3. other ideas?
>
> "all PCHs before CNP, excluding GEN9_LP":
>
> 	if (INTEL_PCH_TYPE(dev_priv) < PCH_CNP && !IS_GEN9_LP(dev_priv))

See the series I mentioned upthread [1], it reverses the condition
making this easier to write anyway.

BR,
Jani.


[1] https://patchwork.freedesktop.org/series/57579/


>
>
> Lucas De Marchi
>
>>
>>>
>>> Lucas De Marchi
>>>
>>> >
>>> > BR,
>>> > Jani.
>>> >
>>> >
>>> > --
>>> > Jani Nikula, Intel Open Source Graphics Center

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

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

* Re: [PATCH 3/3] drm/i915: Start using comparative INTEL_PCH_TYPE
  2019-03-05 19:46               ` Jani Nikula
@ 2019-03-05 20:24                 ` Rodrigo Vivi
  0 siblings, 0 replies; 20+ messages in thread
From: Rodrigo Vivi @ 2019-03-05 20:24 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx, Lucas De Marchi

On Tue, Mar 05, 2019 at 09:46:29PM +0200, Jani Nikula wrote:
> On Tue, 05 Mar 2019, Lucas De Marchi <lucas.demarchi@intel.com> wrote:
> > On Tue, Mar 05, 2019 at 10:38:46AM -0800, Rodrigo Vivi wrote:
> >>On Tue, Mar 05, 2019 at 09:42:22AM -0800, Lucas De Marchi wrote:
> >>> On Tue, Mar 05, 2019 at 07:16:47PM +0200, Jani Nikula wrote:
> >>> > On Tue, 05 Mar 2019, Lucas De Marchi <lucas.demarchi@intel.com> wrote:
> >>> > > On Tue, Mar 05, 2019 at 04:10:04PM +0200, Jani Nikula wrote:
> >>> > > > On Mon, 04 Mar 2019, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> >>> > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> >>> > > > > index e1a051c0fbfe..acd2336bb214 100644
> >>> > > > > --- a/drivers/gpu/drm/i915/intel_dp.c
> >>> > > > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> >>> > > > > @@ -949,8 +949,8 @@ static void intel_pps_get_registers(struct intel_dp *intel_dp,
> >>> > > > >  	regs->pp_stat = PP_STATUS(pps_idx);
> >>> > > > >  	regs->pp_on = PP_ON_DELAYS(pps_idx);
> >>> > > > >  	regs->pp_off = PP_OFF_DELAYS(pps_idx);
> >>> > > > > -	if (!IS_GEN9_LP(dev_priv) && !HAS_PCH_CNP(dev_priv) &&
> >>> > > > > -	    !HAS_PCH_ICP(dev_priv))
> >>> > > > > +	if (INTEL_PCH_TYPE(dev_priv) > PCH_NONE &&
> >>> > > > > +	    INTEL_PCH_TYPE(dev_priv) < PCH_CNP)
> >>> > > >
> >>> > > > This is not right, starts to require PCH.
> >>> > >
> >>> > > But in those cases INTEL_PCH_TYPE() will return either NONE or NOP.
> >>> >
> >>> > Exactly. Non-PCH platforms before CNP should match, but won't.
> >>>
> >>> yeah, right. I misread the !IS_GEN9_LP().
> >>
> >>ouch... indeed.
> >>probably this explains failure on ci for bsw and byt
> >>
> >>options:
> >>
> >>1. if (!IS_GEN9_LP(dev_priv) && !(INTEL_PCH_TYPE(dev_priv) >= 10))
> >
> > 10? I think you meant PCH_CNP
> >
> >>
> >>2. if (INTEL_GEN(dev_priv) <= 8 || IS_GEN9_BC(dev_priv))
> >>
> >>3. other ideas?
> >
> > "all PCHs before CNP, excluding GEN9_LP":
> >
> > 	if (INTEL_PCH_TYPE(dev_priv) < PCH_CNP && !IS_GEN9_LP(dev_priv))
> 
> See the series I mentioned upthread [1], it reverses the condition
> making this easier to write anyway.

Thanks a lot!
I will make a v2 on top of yours and resend after it gets pushed.

> 
> BR,
> Jani.
> 
> 
> [1] https://patchwork.freedesktop.org/series/57579/
> 
> 
> >
> >
> > Lucas De Marchi
> >
> >>
> >>>
> >>> Lucas De Marchi
> >>>
> >>> >
> >>> > BR,
> >>> > Jani.
> >>> >
> >>> >
> >>> > --
> >>> > Jani Nikula, Intel Open Source Graphics Center
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 3/3] drm/i915: Start using comparative INTEL_PCH_TYPE
  2019-03-13 17:30   ` Lucas De Marchi
@ 2019-03-13 20:02     ` Rodrigo Vivi
  0 siblings, 0 replies; 20+ messages in thread
From: Rodrigo Vivi @ 2019-03-13 20:02 UTC (permalink / raw)
  To: Lucas De Marchi; +Cc: Jani Nikula, intel-gfx

On Wed, Mar 13, 2019 at 10:30:52AM -0700, Lucas De Marchi wrote:
> On Fri, Mar 08, 2019 at 01:43:00PM -0800, Rodrigo Vivi wrote:
> > In order to make it easier to bring up new platforms
> > without having to take care about all corner cases
> > that was previously taken care for previous platforms
> > we already use comparative INTEL_GEN statements.
> > 
> > Let's start doing the same with PCH.
> > 
> > The only caveats are:
> > - less-than comparisons need to be avoided or done with
> >   attention and check > PCH_NONE as well.
> > - It is not necessarily a chronological order, but a matter
> >   of south display compatibility/inheritance.
> > 
> > v2: Rebased on top of Jani's clean-up which removed the
> >    need for less-than comparison
> > 
> > Cc: Jani Nikula <jani.nikula@intel.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> 
> 
> Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>

Thanks, series pushed.

> 
> thanks
> Lucas De Marchi
> 
> 
> > ---
> > drivers/gpu/drm/i915/i915_drv.h    | 6 ++++++
> > drivers/gpu/drm/i915/i915_irq.c    | 7 ++-----
> > drivers/gpu/drm/i915/intel_cdclk.c | 2 +-
> > drivers/gpu/drm/i915/intel_dp.c    | 3 +--
> > drivers/gpu/drm/i915/intel_panel.c | 5 ++---
> > 5 files changed, 12 insertions(+), 11 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 8a57cdde5385..9a93accbb2e1 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -523,6 +523,12 @@ struct i915_psr {
> > 	u16 su_x_granularity;
> > };
> > 
> > +/*
> > + * Sorted by south display engine compatibility.
> > + * If the new PCH comes with a south display engine that is not
> > + * inherited from the latest item, please do not add it to the
> > + * end. Instead, add it right after its "parent" PCH.
> > + */
> > enum intel_pch {
> > 	PCH_NOP = -1,	/* PCH without south display */
> > 	PCH_NONE = 0,	/* No PCH present */
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > index 1f4e984ce42f..c823d2e76852 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -2831,9 +2831,7 @@ gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl)
> > 
> > 			if (HAS_PCH_ICP(dev_priv))
> > 				icp_irq_handler(dev_priv, iir);
> > -			else if (HAS_PCH_SPT(dev_priv) ||
> > -				 HAS_PCH_KBP(dev_priv) ||
> > -				 HAS_PCH_CNP(dev_priv))
> > +			else if (INTEL_PCH_TYPE(dev_priv) >= PCH_SPT)
> > 				spt_irq_handler(dev_priv, iir);
> > 			else
> > 				cpt_irq_handler(dev_priv, iir);
> > @@ -4621,8 +4619,7 @@ void intel_irq_init(struct drm_i915_private *dev_priv)
> > 		dev->driver->disable_vblank = gen8_disable_vblank;
> > 		if (IS_GEN9_LP(dev_priv))
> > 			dev_priv->display.hpd_irq_setup = bxt_hpd_irq_setup;
> > -		else if (HAS_PCH_SPT(dev_priv) || HAS_PCH_KBP(dev_priv) ||
> > -			 HAS_PCH_CNP(dev_priv))
> > +		else if (INTEL_PCH_TYPE(dev_priv) >= PCH_SPT)
> > 			dev_priv->display.hpd_irq_setup = spt_hpd_irq_setup;
> > 		else
> > 			dev_priv->display.hpd_irq_setup = ilk_hpd_irq_setup;
> > diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
> > index 7e5132772477..9d236e4ed26a 100644
> > --- a/drivers/gpu/drm/i915/intel_cdclk.c
> > +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> > @@ -2723,7 +2723,7 @@ static int g4x_hrawclk(struct drm_i915_private *dev_priv)
> >  */
> > void intel_update_rawclk(struct drm_i915_private *dev_priv)
> > {
> > -	if (HAS_PCH_CNP(dev_priv) || HAS_PCH_ICP(dev_priv))
> > +	if (INTEL_PCH_TYPE(dev_priv) >= PCH_CNP)
> > 		dev_priv->rawclk_freq = cnp_rawclk(dev_priv);
> > 	else if (HAS_PCH_SPLIT(dev_priv))
> > 		dev_priv->rawclk_freq = pch_rawclk(dev_priv);
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index f40b3342d82a..47857f96c3b1 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -951,8 +951,7 @@ static void intel_pps_get_registers(struct intel_dp *intel_dp,
> > 	regs->pp_off = PP_OFF_DELAYS(pps_idx);
> > 
> > 	/* Cycle delay moved from PP_DIVISOR to PP_CONTROL */
> > -	if (IS_GEN9_LP(dev_priv) || HAS_PCH_CNP(dev_priv) ||
> > -	    HAS_PCH_ICP(dev_priv))
> > +	if (IS_GEN9_LP(dev_priv) || INTEL_PCH_TYPE(dev_priv) >= PCH_CNP)
> > 		regs->pp_div = INVALID_MMIO_REG;
> > 	else
> > 		regs->pp_div = PP_DIVISOR(pps_idx);
> > diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> > index beca98d2b035..edd5540639b0 100644
> > --- a/drivers/gpu/drm/i915/intel_panel.c
> > +++ b/drivers/gpu/drm/i915/intel_panel.c
> > @@ -1894,15 +1894,14 @@ intel_panel_init_backlight_funcs(struct intel_panel *panel)
> > 		panel->backlight.set = bxt_set_backlight;
> > 		panel->backlight.get = bxt_get_backlight;
> > 		panel->backlight.hz_to_pwm = bxt_hz_to_pwm;
> > -	} else if (HAS_PCH_CNP(dev_priv) || HAS_PCH_ICP(dev_priv)) {
> > +	} else if (INTEL_PCH_TYPE(dev_priv) >= PCH_CNP) {
> > 		panel->backlight.setup = cnp_setup_backlight;
> > 		panel->backlight.enable = cnp_enable_backlight;
> > 		panel->backlight.disable = cnp_disable_backlight;
> > 		panel->backlight.set = bxt_set_backlight;
> > 		panel->backlight.get = bxt_get_backlight;
> > 		panel->backlight.hz_to_pwm = cnp_hz_to_pwm;
> > -	} else if (HAS_PCH_LPT(dev_priv) || HAS_PCH_SPT(dev_priv) ||
> > -		   HAS_PCH_KBP(dev_priv)) {
> > +	} else if (INTEL_PCH_TYPE(dev_priv) >= PCH_LPT) {
> > 		panel->backlight.setup = lpt_setup_backlight;
> > 		panel->backlight.enable = lpt_enable_backlight;
> > 		panel->backlight.disable = lpt_disable_backlight;
> > -- 
> > 2.20.1
> > 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 3/3] drm/i915: Start using comparative INTEL_PCH_TYPE
  2019-03-08 21:43 ` [PATCH 3/3] drm/i915: Start using comparative INTEL_PCH_TYPE Rodrigo Vivi
@ 2019-03-13 17:30   ` Lucas De Marchi
  2019-03-13 20:02     ` Rodrigo Vivi
  0 siblings, 1 reply; 20+ messages in thread
From: Lucas De Marchi @ 2019-03-13 17:30 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: Jani Nikula, intel-gfx

On Fri, Mar 08, 2019 at 01:43:00PM -0800, Rodrigo Vivi wrote:
>In order to make it easier to bring up new platforms
>without having to take care about all corner cases
>that was previously taken care for previous platforms
>we already use comparative INTEL_GEN statements.
>
>Let's start doing the same with PCH.
>
>The only caveats are:
> - less-than comparisons need to be avoided or done with
>   attention and check > PCH_NONE as well.
> - It is not necessarily a chronological order, but a matter
>   of south display compatibility/inheritance.
>
>v2: Rebased on top of Jani's clean-up which removed the
>    need for less-than comparison
>
>Cc: Jani Nikula <jani.nikula@intel.com>
>Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>Cc: Lucas De Marchi <lucas.demarchi@intel.com>
>Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>


Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>

thanks
Lucas De Marchi


>---
> drivers/gpu/drm/i915/i915_drv.h    | 6 ++++++
> drivers/gpu/drm/i915/i915_irq.c    | 7 ++-----
> drivers/gpu/drm/i915/intel_cdclk.c | 2 +-
> drivers/gpu/drm/i915/intel_dp.c    | 3 +--
> drivers/gpu/drm/i915/intel_panel.c | 5 ++---
> 5 files changed, 12 insertions(+), 11 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>index 8a57cdde5385..9a93accbb2e1 100644
>--- a/drivers/gpu/drm/i915/i915_drv.h
>+++ b/drivers/gpu/drm/i915/i915_drv.h
>@@ -523,6 +523,12 @@ struct i915_psr {
> 	u16 su_x_granularity;
> };
>
>+/*
>+ * Sorted by south display engine compatibility.
>+ * If the new PCH comes with a south display engine that is not
>+ * inherited from the latest item, please do not add it to the
>+ * end. Instead, add it right after its "parent" PCH.
>+ */
> enum intel_pch {
> 	PCH_NOP = -1,	/* PCH without south display */
> 	PCH_NONE = 0,	/* No PCH present */
>diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
>index 1f4e984ce42f..c823d2e76852 100644
>--- a/drivers/gpu/drm/i915/i915_irq.c
>+++ b/drivers/gpu/drm/i915/i915_irq.c
>@@ -2831,9 +2831,7 @@ gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl)
>
> 			if (HAS_PCH_ICP(dev_priv))
> 				icp_irq_handler(dev_priv, iir);
>-			else if (HAS_PCH_SPT(dev_priv) ||
>-				 HAS_PCH_KBP(dev_priv) ||
>-				 HAS_PCH_CNP(dev_priv))
>+			else if (INTEL_PCH_TYPE(dev_priv) >= PCH_SPT)
> 				spt_irq_handler(dev_priv, iir);
> 			else
> 				cpt_irq_handler(dev_priv, iir);
>@@ -4621,8 +4619,7 @@ void intel_irq_init(struct drm_i915_private *dev_priv)
> 		dev->driver->disable_vblank = gen8_disable_vblank;
> 		if (IS_GEN9_LP(dev_priv))
> 			dev_priv->display.hpd_irq_setup = bxt_hpd_irq_setup;
>-		else if (HAS_PCH_SPT(dev_priv) || HAS_PCH_KBP(dev_priv) ||
>-			 HAS_PCH_CNP(dev_priv))
>+		else if (INTEL_PCH_TYPE(dev_priv) >= PCH_SPT)
> 			dev_priv->display.hpd_irq_setup = spt_hpd_irq_setup;
> 		else
> 			dev_priv->display.hpd_irq_setup = ilk_hpd_irq_setup;
>diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
>index 7e5132772477..9d236e4ed26a 100644
>--- a/drivers/gpu/drm/i915/intel_cdclk.c
>+++ b/drivers/gpu/drm/i915/intel_cdclk.c
>@@ -2723,7 +2723,7 @@ static int g4x_hrawclk(struct drm_i915_private *dev_priv)
>  */
> void intel_update_rawclk(struct drm_i915_private *dev_priv)
> {
>-	if (HAS_PCH_CNP(dev_priv) || HAS_PCH_ICP(dev_priv))
>+	if (INTEL_PCH_TYPE(dev_priv) >= PCH_CNP)
> 		dev_priv->rawclk_freq = cnp_rawclk(dev_priv);
> 	else if (HAS_PCH_SPLIT(dev_priv))
> 		dev_priv->rawclk_freq = pch_rawclk(dev_priv);
>diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>index f40b3342d82a..47857f96c3b1 100644
>--- a/drivers/gpu/drm/i915/intel_dp.c
>+++ b/drivers/gpu/drm/i915/intel_dp.c
>@@ -951,8 +951,7 @@ static void intel_pps_get_registers(struct intel_dp *intel_dp,
> 	regs->pp_off = PP_OFF_DELAYS(pps_idx);
>
> 	/* Cycle delay moved from PP_DIVISOR to PP_CONTROL */
>-	if (IS_GEN9_LP(dev_priv) || HAS_PCH_CNP(dev_priv) ||
>-	    HAS_PCH_ICP(dev_priv))
>+	if (IS_GEN9_LP(dev_priv) || INTEL_PCH_TYPE(dev_priv) >= PCH_CNP)
> 		regs->pp_div = INVALID_MMIO_REG;
> 	else
> 		regs->pp_div = PP_DIVISOR(pps_idx);
>diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
>index beca98d2b035..edd5540639b0 100644
>--- a/drivers/gpu/drm/i915/intel_panel.c
>+++ b/drivers/gpu/drm/i915/intel_panel.c
>@@ -1894,15 +1894,14 @@ intel_panel_init_backlight_funcs(struct intel_panel *panel)
> 		panel->backlight.set = bxt_set_backlight;
> 		panel->backlight.get = bxt_get_backlight;
> 		panel->backlight.hz_to_pwm = bxt_hz_to_pwm;
>-	} else if (HAS_PCH_CNP(dev_priv) || HAS_PCH_ICP(dev_priv)) {
>+	} else if (INTEL_PCH_TYPE(dev_priv) >= PCH_CNP) {
> 		panel->backlight.setup = cnp_setup_backlight;
> 		panel->backlight.enable = cnp_enable_backlight;
> 		panel->backlight.disable = cnp_disable_backlight;
> 		panel->backlight.set = bxt_set_backlight;
> 		panel->backlight.get = bxt_get_backlight;
> 		panel->backlight.hz_to_pwm = cnp_hz_to_pwm;
>-	} else if (HAS_PCH_LPT(dev_priv) || HAS_PCH_SPT(dev_priv) ||
>-		   HAS_PCH_KBP(dev_priv)) {
>+	} else if (INTEL_PCH_TYPE(dev_priv) >= PCH_LPT) {
> 		panel->backlight.setup = lpt_setup_backlight;
> 		panel->backlight.enable = lpt_enable_backlight;
> 		panel->backlight.disable = lpt_disable_backlight;
>-- 
>2.20.1
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* [PATCH 3/3] drm/i915: Start using comparative INTEL_PCH_TYPE
  2019-03-08 21:42 Rodrigo Vivi
@ 2019-03-08 21:43 ` Rodrigo Vivi
  2019-03-13 17:30   ` Lucas De Marchi
  0 siblings, 1 reply; 20+ messages in thread
From: Rodrigo Vivi @ 2019-03-08 21:43 UTC (permalink / raw)
  To: intel-gfx; +Cc: Jani Nikula, Lucas De Marchi

In order to make it easier to bring up new platforms
without having to take care about all corner cases
that was previously taken care for previous platforms
we already use comparative INTEL_GEN statements.

Let's start doing the same with PCH.

The only caveats are:
 - less-than comparisons need to be avoided or done with
   attention and check > PCH_NONE as well.
 - It is not necessarily a chronological order, but a matter
   of south display compatibility/inheritance.

v2: Rebased on top of Jani's clean-up which removed the
    need for less-than comparison

Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h    | 6 ++++++
 drivers/gpu/drm/i915/i915_irq.c    | 7 ++-----
 drivers/gpu/drm/i915/intel_cdclk.c | 2 +-
 drivers/gpu/drm/i915/intel_dp.c    | 3 +--
 drivers/gpu/drm/i915/intel_panel.c | 5 ++---
 5 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 8a57cdde5385..9a93accbb2e1 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -523,6 +523,12 @@ struct i915_psr {
 	u16 su_x_granularity;
 };
 
+/*
+ * Sorted by south display engine compatibility.
+ * If the new PCH comes with a south display engine that is not
+ * inherited from the latest item, please do not add it to the
+ * end. Instead, add it right after its "parent" PCH.
+ */
 enum intel_pch {
 	PCH_NOP = -1,	/* PCH without south display */
 	PCH_NONE = 0,	/* No PCH present */
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 1f4e984ce42f..c823d2e76852 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2831,9 +2831,7 @@ gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl)
 
 			if (HAS_PCH_ICP(dev_priv))
 				icp_irq_handler(dev_priv, iir);
-			else if (HAS_PCH_SPT(dev_priv) ||
-				 HAS_PCH_KBP(dev_priv) ||
-				 HAS_PCH_CNP(dev_priv))
+			else if (INTEL_PCH_TYPE(dev_priv) >= PCH_SPT)
 				spt_irq_handler(dev_priv, iir);
 			else
 				cpt_irq_handler(dev_priv, iir);
@@ -4621,8 +4619,7 @@ void intel_irq_init(struct drm_i915_private *dev_priv)
 		dev->driver->disable_vblank = gen8_disable_vblank;
 		if (IS_GEN9_LP(dev_priv))
 			dev_priv->display.hpd_irq_setup = bxt_hpd_irq_setup;
-		else if (HAS_PCH_SPT(dev_priv) || HAS_PCH_KBP(dev_priv) ||
-			 HAS_PCH_CNP(dev_priv))
+		else if (INTEL_PCH_TYPE(dev_priv) >= PCH_SPT)
 			dev_priv->display.hpd_irq_setup = spt_hpd_irq_setup;
 		else
 			dev_priv->display.hpd_irq_setup = ilk_hpd_irq_setup;
diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
index 7e5132772477..9d236e4ed26a 100644
--- a/drivers/gpu/drm/i915/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/intel_cdclk.c
@@ -2723,7 +2723,7 @@ static int g4x_hrawclk(struct drm_i915_private *dev_priv)
  */
 void intel_update_rawclk(struct drm_i915_private *dev_priv)
 {
-	if (HAS_PCH_CNP(dev_priv) || HAS_PCH_ICP(dev_priv))
+	if (INTEL_PCH_TYPE(dev_priv) >= PCH_CNP)
 		dev_priv->rawclk_freq = cnp_rawclk(dev_priv);
 	else if (HAS_PCH_SPLIT(dev_priv))
 		dev_priv->rawclk_freq = pch_rawclk(dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index f40b3342d82a..47857f96c3b1 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -951,8 +951,7 @@ static void intel_pps_get_registers(struct intel_dp *intel_dp,
 	regs->pp_off = PP_OFF_DELAYS(pps_idx);
 
 	/* Cycle delay moved from PP_DIVISOR to PP_CONTROL */
-	if (IS_GEN9_LP(dev_priv) || HAS_PCH_CNP(dev_priv) ||
-	    HAS_PCH_ICP(dev_priv))
+	if (IS_GEN9_LP(dev_priv) || INTEL_PCH_TYPE(dev_priv) >= PCH_CNP)
 		regs->pp_div = INVALID_MMIO_REG;
 	else
 		regs->pp_div = PP_DIVISOR(pps_idx);
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index beca98d2b035..edd5540639b0 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -1894,15 +1894,14 @@ intel_panel_init_backlight_funcs(struct intel_panel *panel)
 		panel->backlight.set = bxt_set_backlight;
 		panel->backlight.get = bxt_get_backlight;
 		panel->backlight.hz_to_pwm = bxt_hz_to_pwm;
-	} else if (HAS_PCH_CNP(dev_priv) || HAS_PCH_ICP(dev_priv)) {
+	} else if (INTEL_PCH_TYPE(dev_priv) >= PCH_CNP) {
 		panel->backlight.setup = cnp_setup_backlight;
 		panel->backlight.enable = cnp_enable_backlight;
 		panel->backlight.disable = cnp_disable_backlight;
 		panel->backlight.set = bxt_set_backlight;
 		panel->backlight.get = bxt_get_backlight;
 		panel->backlight.hz_to_pwm = cnp_hz_to_pwm;
-	} else if (HAS_PCH_LPT(dev_priv) || HAS_PCH_SPT(dev_priv) ||
-		   HAS_PCH_KBP(dev_priv)) {
+	} else if (INTEL_PCH_TYPE(dev_priv) >= PCH_LPT) {
 		panel->backlight.setup = lpt_setup_backlight;
 		panel->backlight.enable = lpt_enable_backlight;
 		panel->backlight.disable = lpt_disable_backlight;
-- 
2.20.1

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

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

end of thread, other threads:[~2019-03-13 20:02 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-04 22:48 [PATCH 1/3] drm/i915/gen11+: First assume next platforms will inherit stuff Rodrigo Vivi
2019-03-04 22:48 ` [PATCH 2/3] drm/i915: Move PCH_NOP to -1 Rodrigo Vivi
2019-03-05 17:38   ` Lucas De Marchi
2019-03-04 22:48 ` [PATCH 3/3] drm/i915: Start using comparative INTEL_PCH_TYPE Rodrigo Vivi
2019-03-05 14:10   ` Jani Nikula
2019-03-05 17:08     ` Lucas De Marchi
2019-03-05 17:16       ` Jani Nikula
2019-03-05 17:42         ` Lucas De Marchi
2019-03-05 18:38           ` Rodrigo Vivi
2019-03-05 18:48             ` Lucas De Marchi
2019-03-05 19:46               ` Jani Nikula
2019-03-05 20:24                 ` Rodrigo Vivi
2019-03-05  0:05 ` ✗ Fi.CI.SPARSE: warning for series starting with [1/3] drm/i915/gen11+: First assume next platforms will inherit stuff Patchwork
2019-03-05  1:12 ` ✗ Fi.CI.BAT: failure " Patchwork
2019-03-05 17:12 ` [PATCH 1/3] " Lucas De Marchi
2019-03-05 17:43 ` Tvrtko Ursulin
2019-03-05 18:36   ` Lucas De Marchi
2019-03-08 21:42 Rodrigo Vivi
2019-03-08 21:43 ` [PATCH 3/3] drm/i915: Start using comparative INTEL_PCH_TYPE Rodrigo Vivi
2019-03-13 17:30   ` Lucas De Marchi
2019-03-13 20:02     ` Rodrigo Vivi

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