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

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

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

> 
> Signed-off-by: Rodrigo Vivi 
> ---
>  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, _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)
>   _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, _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 

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

2018-10-19 Thread Jani Nikula
On Thu, 18 Oct 2018, Rodrigo Vivi  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 
> ---
>  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, _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)
>   _entries);
>   else
>   n_entries = ARRAY_SIZE(icl_mg_phy_ddi_translations);
> - } else if (IS_CANNONLAKE(dev_priv)) {
> + } else if 

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

2018-10-18 Thread Rodrigo Vivi
Whenever possible let's move towards preferring gen number
and or features instead of hard coding platform codename
everywhere.

Signed-off-by: Rodrigo Vivi 
---
 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, _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)
_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, _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,