All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jani Nikula <jani.nikula@linux.intel.com>
To: intel-gfx@lists.freedesktop.org
Cc: lucas.demarchi@intel.com, Rodrigo Vivi <rodrigo.vivi@intel.com>
Subject: Re: [RFC 3/8] drm/i915/gen10: Prefer gen number than platform codename.
Date: Fri, 19 Oct 2018 10:48:57 +0300	[thread overview]
Message-ID: <87tvligyvq.fsf@intel.com> (raw)
In-Reply-To: <20181018233447.5187-4-rodrigo.vivi@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

  reply	other threads:[~2018-10-19  7:49 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-18 23:34 [RFC 0/8] re-organize a bit gen10 and gen11 Rodrigo Vivi
2018-10-18 23:34 ` [RFC 1/8] drm/i915: Make number of ddi ports explicit Rodrigo Vivi
2018-10-18 23:34 ` [RFC 2/8] drm/i915: Use the ddi_ports info to kill ugly CNL_WITH_PORT_F Rodrigo Vivi
2018-10-19  7:39   ` Jani Nikula
2018-10-19  8:27     ` Daniel Vetter
2018-10-19 16:46     ` Lucas De Marchi
2018-10-19 17:40       ` Rodrigo Vivi
2018-10-18 23:34 ` [RFC 3/8] drm/i915/gen10: Prefer gen number than platform codename Rodrigo Vivi
2018-10-19  7:48   ` Jani Nikula [this message]
2018-10-19 10:37   ` Ville Syrjälä
2018-10-18 23:34 ` [RFC 4/8] drm/i915: Group gen 10 display Rodrigo Vivi
2018-10-19  8:03   ` Jani Nikula
2018-10-19  8:30     ` Daniel Vetter
2018-10-19  9:32       ` Joonas Lahtinen
2018-10-19 10:33       ` Ville Syrjälä
2018-10-19 16:41         ` Rodrigo Vivi
2018-10-19 17:45           ` Ville Syrjälä
2018-10-19 16:59         ` Lucas De Marchi
2018-10-19 16:52     ` Lucas De Marchi
2018-10-18 23:34 ` [RFC 5/8] drm/i915/gen11+: Prefer gen over platform codename Rodrigo Vivi
2018-10-19  8:05   ` Jani Nikula
2018-10-18 23:34 ` [RFC 6/8] drm/i915: Consolidate cdclk hooks Rodrigo Vivi
2018-10-18 23:34 ` [RFC 7/8] drm/i915: Sort platform if cases from newer-to-older Rodrigo Vivi
2018-10-19  8:07   ` Jani Nikula
2018-10-18 23:34 ` [RFC 8/8] drm/i915: Simplify intel_has_sagv function Rodrigo Vivi
2018-10-19  8:15   ` Jani Nikula
2018-10-18 23:47 ` ✗ Fi.CI.CHECKPATCH: warning for re-organize a bit gen10 and gen11 Patchwork
2018-10-18 23:50 ` ✗ Fi.CI.SPARSE: " Patchwork
2018-10-19  0:04 ` ✗ Fi.CI.BAT: failure " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87tvligyvq.fsf@intel.com \
    --to=jani.nikula@linux.intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=lucas.demarchi@intel.com \
    --cc=rodrigo.vivi@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
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.