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

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

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

Rationale missing.

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

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

	if (IS_CANNONLAKE() || IS_GEMINILAKE())

and

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

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

BR,
Jani.


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

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