subject:"\"\\\\\\\[Intel\\\\\\\-gfx\\\\\\\] \\\\\\\[PATCH\\\\\\\] drm\\\\\\\/i915\\\\\\\: Consolidate cdclk hooks.\""

[Intel-gfx] [PATCH] drm/i915: Consolidate cdclk hooks.

2018-10-19 Thread Rodrigo Vivi

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

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

Cc: Ville Syrjälä 
Signed-off-by: Rodrigo Vivi 
---
 drivers/gpu/drm/i915/intel_cdclk.c | 91 ++
 1 file changed, 42 insertions(+), 49 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_cdclk.c 
b/drivers/gpu/drm/i915/intel_cdclk.c
index 29075c763428..76e69c4f154f 100644
--- a/drivers/gpu/drm/i915/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/intel_cdclk.c
@@ -2777,80 +2777,73 @@ void intel_update_rawclk(struct drm_i915_private 
*dev_priv)
  */
 void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv)
 {
-   if (IS_CHERRYVIEW(dev_priv)) {
-   dev_priv->display.set_cdclk = chv_set_cdclk;
-   dev_priv->display.modeset_calc_cdclk =
-   vlv_modeset_calc_cdclk;
-   } else if (IS_VALLEYVIEW(dev_priv)) {
-   dev_priv->display.set_cdclk = vlv_set_cdclk;
-   dev_priv->display.modeset_calc_cdclk =
-   vlv_modeset_calc_cdclk;
-   } else if (IS_BROADWELL(dev_priv)) {
-   dev_priv->display.set_cdclk = bdw_set_cdclk;
-   dev_priv->display.modeset_calc_cdclk =
-   bdw_modeset_calc_cdclk;
-   } else if (IS_GEN9_LP(dev_priv)) {
-   dev_priv->display.set_cdclk = bxt_set_cdclk;
-   dev_priv->display.modeset_calc_cdclk =
-   bxt_modeset_calc_cdclk;
-   } else if (IS_GEN9_BC(dev_priv)) {
-   dev_priv->display.set_cdclk = skl_set_cdclk;
-   dev_priv->display.modeset_calc_cdclk =
-   skl_modeset_calc_cdclk;
+   if (IS_ICELAKE(dev_priv)) {
+   dev_priv->display.set_cdclk = icl_set_cdclk;
+   dev_priv->display.modeset_calc_cdclk = icl_modeset_calc_cdclk;
+   dev_priv->display.get_cdclk = icl_get_cdclk;
} else if (IS_CANNONLAKE(dev_priv)) {
dev_priv->display.set_cdclk = cnl_set_cdclk;
dev_priv->display.modeset_calc_cdclk =
cnl_modeset_calc_cdclk;
-   } else if (IS_ICELAKE(dev_priv)) {
-   dev_priv->display.set_cdclk = icl_set_cdclk;
-   dev_priv->display.modeset_calc_cdclk = icl_modeset_calc_cdclk;
-   }
-
-   if (IS_ICELAKE(dev_priv))
-   dev_priv->display.get_cdclk = icl_get_cdclk;
-   else if (IS_CANNONLAKE(dev_priv))
dev_priv->display.get_cdclk = cnl_get_cdclk;
-   else if (IS_GEN9_BC(dev_priv))
+   } else if (IS_GEN9_BC(dev_priv)) {
+   dev_priv->display.set_cdclk = skl_set_cdclk;
+   dev_priv->display.modeset_calc_cdclk =
+   skl_modeset_calc_cdclk;
dev_priv->display.get_cdclk = skl_get_cdclk;
-   else if (IS_GEN9_LP(dev_priv))
+   } else if (IS_GEN9_LP(dev_priv)) {
+   dev_priv->display.set_cdclk = bxt_set_cdclk;
+   dev_priv->display.modeset_calc_cdclk =
+   bxt_modeset_calc_cdclk;
dev_priv->display.get_cdclk = bxt_get_cdclk;
-   else if (IS_BROADWELL(dev_priv))
+   } else if (IS_BROADWELL(dev_priv)) {
+   dev_priv->display.set_cdclk = bdw_set_cdclk;
+   dev_priv->display.modeset_calc_cdclk =
+   bdw_modeset_calc_cdclk;
dev_priv->display.get_cdclk = bdw_get_cdclk;
-   else if (IS_HASWELL(dev_priv))
+   } else if (IS_HASWELL(dev_priv)) {
dev_priv->display.get_cdclk = hsw_get_cdclk;
-   else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
+   } else if (IS_CHERRYVIEW(dev_priv)) {
+   dev_priv->display.set_cdclk = chv_set_cdclk;
+   dev_priv->display.modeset_calc_cdclk =
+   vlv_modeset_calc_cdclk;
dev_priv->display.get_cdclk = vlv_get_cdclk;
-   else if (IS_GEN6(dev_priv) || IS_IVYBRIDGE(dev_priv))
+   } else if (IS_VALLEYVIEW(dev_priv)) {
+   dev_priv->display.set_cdclk = vlv_set_cdclk;
+   dev_priv->display.modeset_calc_cdclk =
+   vlv_modeset_calc_cdclk;
+   dev_priv->display.get_cdclk = vlv_get_cdclk;
+   } else if (IS_GEN6(dev_priv) || IS_IVYBRIDGE(dev_priv)) {
dev_priv->display.get_cdclk = fixed_400mhz_get_cdclk;
-   else if (IS_GEN5(dev_priv))
+   } else if (IS_GEN5(dev_priv)) {
dev_priv->display.get_cdclk = fixed_450mhz_get_cdclk;
-   else if (IS_GM45(dev_priv))
+   } else if (IS_GM45(dev_priv)) {
dev_priv->display.get_cdclk = gm45_get_cdclk;
-   else if (IS_G45(dev_priv))
+   } else if (IS_G45(dev_priv)) {
dev_priv->display.get_cdclk = g33_get_cdclk;
-   else if (IS_I965GM(dev_priv))
+   } else if (IS_I965GM(dev_priv)) {
dev_priv-

[Intel-gfx] [PATCH] drm/i915: Consolidate cdclk hooks.

2018-10-22 Thread Rodrigo Vivi

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

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

v2: Accept all good suggestions from Ville:
- Remove \n since it is under 80 cols now
- Use get/set/calc order on all blocks
- Fix platform sorting

Cc: Ville Syrjälä 
Signed-off-by: Rodrigo Vivi 
Reviewed-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/intel_cdclk.c | 91 +-
 1 file changed, 39 insertions(+), 52 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_cdclk.c 
b/drivers/gpu/drm/i915/intel_cdclk.c
index 29075c763428..111967c1f2ee 100644
--- a/drivers/gpu/drm/i915/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/intel_cdclk.c
@@ -2777,80 +2777,67 @@ void intel_update_rawclk(struct drm_i915_private 
*dev_priv)
  */
 void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv)
 {
-   if (IS_CHERRYVIEW(dev_priv)) {
-   dev_priv->display.set_cdclk = chv_set_cdclk;
-   dev_priv->display.modeset_calc_cdclk =
-   vlv_modeset_calc_cdclk;
-   } else if (IS_VALLEYVIEW(dev_priv)) {
-   dev_priv->display.set_cdclk = vlv_set_cdclk;
-   dev_priv->display.modeset_calc_cdclk =
-   vlv_modeset_calc_cdclk;
-   } else if (IS_BROADWELL(dev_priv)) {
-   dev_priv->display.set_cdclk = bdw_set_cdclk;
-   dev_priv->display.modeset_calc_cdclk =
-   bdw_modeset_calc_cdclk;
-   } else if (IS_GEN9_LP(dev_priv)) {
-   dev_priv->display.set_cdclk = bxt_set_cdclk;
-   dev_priv->display.modeset_calc_cdclk =
-   bxt_modeset_calc_cdclk;
-   } else if (IS_GEN9_BC(dev_priv)) {
-   dev_priv->display.set_cdclk = skl_set_cdclk;
-   dev_priv->display.modeset_calc_cdclk =
-   skl_modeset_calc_cdclk;
-   } else if (IS_CANNONLAKE(dev_priv)) {
-   dev_priv->display.set_cdclk = cnl_set_cdclk;
-   dev_priv->display.modeset_calc_cdclk =
-   cnl_modeset_calc_cdclk;
-   } else if (IS_ICELAKE(dev_priv)) {
+   if (IS_ICELAKE(dev_priv)) {
+   dev_priv->display.get_cdclk = icl_get_cdclk;
dev_priv->display.set_cdclk = icl_set_cdclk;
dev_priv->display.modeset_calc_cdclk = icl_modeset_calc_cdclk;
-   }
-
-   if (IS_ICELAKE(dev_priv))
-   dev_priv->display.get_cdclk = icl_get_cdclk;
-   else if (IS_CANNONLAKE(dev_priv))
+   } else if (IS_CANNONLAKE(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;
-   else if (IS_GEN9_LP(dev_priv))
+   dev_priv->display.set_cdclk = cnl_set_cdclk;
+   dev_priv->display.modeset_calc_cdclk = cnl_modeset_calc_cdclk;
+   } else if (IS_GEN9_LP(dev_priv)) {
dev_priv->display.get_cdclk = bxt_get_cdclk;
-   else if (IS_BROADWELL(dev_priv))
+   dev_priv->display.set_cdclk = bxt_set_cdclk;
+   dev_priv->display.modeset_calc_cdclk = bxt_modeset_calc_cdclk;
+   } else if (IS_GEN9_BC(dev_priv)) {
+   dev_priv->display.get_cdclk = skl_get_cdclk;
+   dev_priv->display.set_cdclk = skl_set_cdclk;
+   dev_priv->display.modeset_calc_cdclk = skl_modeset_calc_cdclk;
+   } else if (IS_CHERRYVIEW(dev_priv)) {
+   dev_priv->display.get_cdclk = vlv_get_cdclk;
+   dev_priv->display.set_cdclk = chv_set_cdclk;
+   dev_priv->display.modeset_calc_cdclk = vlv_modeset_calc_cdclk;
+   } else if (IS_BROADWELL(dev_priv)) {
dev_priv->display.get_cdclk = bdw_get_cdclk;
-   else if (IS_HASWELL(dev_priv))
+   dev_priv->display.set_cdclk = bdw_set_cdclk;
+   dev_priv->display.modeset_calc_cdclk = bdw_modeset_calc_cdclk;
+   } else if (IS_HASWELL(dev_priv)) {
dev_priv->display.get_cdclk = hsw_get_cdclk;
-   else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
+   } else if (IS_VALLEYVIEW(dev_priv)) {
dev_priv->display.get_cdclk = vlv_get_cdclk;
-   else if (IS_GEN6(dev_priv) || IS_IVYBRIDGE(dev_priv))
+   dev_priv->display.set_cdclk = vlv_set_cdclk;
+   dev_priv->display.modeset_calc_cdclk = vlv_modeset_calc_cdclk;
+   } else if (IS_GEN6(dev_priv) || IS_IVYBRIDGE(dev_priv)) {
dev_priv->display.get_cdclk = fixed_400mhz_get_cdclk;
-   else if (IS_GEN5(dev_priv))
+   } else if (IS_GEN5(dev_priv)) {
dev_priv->display.get_cdclk = fixed_450mhz_get_cdclk;
-   else if (IS_GM45(dev_priv))
+   } else if (IS_GM45(dev_priv)) {
dev_priv->display.get_cdclk = gm45_get_cdclk;
-   else if (IS_G45(dev_priv))
+   } else if (IS_G45(dev

Re: [Intel-gfx] [PATCH] drm/i915: Consolidate cdclk hooks.

2018-10-19 Thread Ville Syrjälä

On Fri, Oct 19, 2018 at 12:11:53PM -0700, Rodrigo Vivi wrote:
> We don't need 2 different blocks.
> Specially with on in ordered older-to-newer and the other
> one newer-to-older.
> 
> Let's start always using newer-to-older order
> when it makes sense.
> 
> Cc: Ville Syrjälä 
> Signed-off-by: Rodrigo Vivi 
> ---
>  drivers/gpu/drm/i915/intel_cdclk.c | 91 ++
>  1 file changed, 42 insertions(+), 49 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_cdclk.c 
> b/drivers/gpu/drm/i915/intel_cdclk.c
> index 29075c763428..76e69c4f154f 100644
> --- a/drivers/gpu/drm/i915/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> @@ -2777,80 +2777,73 @@ void intel_update_rawclk(struct drm_i915_private 
> *dev_priv)
>   */
>  void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv)
>  {
> - if (IS_CHERRYVIEW(dev_priv)) {
> - dev_priv->display.set_cdclk = chv_set_cdclk;
> - dev_priv->display.modeset_calc_cdclk =
> - vlv_modeset_calc_cdclk;
> - } else if (IS_VALLEYVIEW(dev_priv)) {
> - dev_priv->display.set_cdclk = vlv_set_cdclk;
> - dev_priv->display.modeset_calc_cdclk =
> - vlv_modeset_calc_cdclk;
> - } else if (IS_BROADWELL(dev_priv)) {
> - dev_priv->display.set_cdclk = bdw_set_cdclk;
> - dev_priv->display.modeset_calc_cdclk =
> - bdw_modeset_calc_cdclk;
> - } else if (IS_GEN9_LP(dev_priv)) {
> - dev_priv->display.set_cdclk = bxt_set_cdclk;
> - dev_priv->display.modeset_calc_cdclk =
> - bxt_modeset_calc_cdclk;
> - } else if (IS_GEN9_BC(dev_priv)) {
> - dev_priv->display.set_cdclk = skl_set_cdclk;
> - dev_priv->display.modeset_calc_cdclk =
> - skl_modeset_calc_cdclk;
> + if (IS_ICELAKE(dev_priv)) {
> + dev_priv->display.set_cdclk = icl_set_cdclk;
> + dev_priv->display.modeset_calc_cdclk = icl_modeset_calc_cdclk;
> + dev_priv->display.get_cdclk = icl_get_cdclk;
>   } else if (IS_CANNONLAKE(dev_priv)) {
>   dev_priv->display.set_cdclk = cnl_set_cdclk;
>   dev_priv->display.modeset_calc_cdclk =
>   cnl_modeset_calc_cdclk;

Can you eliminate these extra '\n' in these assigments while you're
here? I think they should all fit within the 80 col rule now.

Also another ocd you could do is order the assignments in a better
looking way. I'd suggest either get,set,calc or calc,set,get.

> - } else if (IS_ICELAKE(dev_priv)) {
> - dev_priv->display.set_cdclk = icl_set_cdclk;
> - dev_priv->display.modeset_calc_cdclk = icl_modeset_calc_cdclk;
> - }
> -
> - if (IS_ICELAKE(dev_priv))
> - dev_priv->display.get_cdclk = icl_get_cdclk;
> - else if (IS_CANNONLAKE(dev_priv))
>   dev_priv->display.get_cdclk = cnl_get_cdclk;
> - else if (IS_GEN9_BC(dev_priv))
> + } else if (IS_GEN9_BC(dev_priv)) {
> + dev_priv->display.set_cdclk = skl_set_cdclk;
> + dev_priv->display.modeset_calc_cdclk =
> + skl_modeset_calc_cdclk;
>   dev_priv->display.get_cdclk = skl_get_cdclk;
> - else if (IS_GEN9_LP(dev_priv))
> + } else if (IS_GEN9_LP(dev_priv)) {
> + dev_priv->display.set_cdclk = bxt_set_cdclk;
> + dev_priv->display.modeset_calc_cdclk =
> + bxt_modeset_calc_cdclk;
>   dev_priv->display.get_cdclk = bxt_get_cdclk;

bxt before skl if we want to be chronologically pedantic?

> - else if (IS_BROADWELL(dev_priv))
> + } else if (IS_BROADWELL(dev_priv)) {
> + dev_priv->display.set_cdclk = bdw_set_cdclk;
> + dev_priv->display.modeset_calc_cdclk =
> + bdw_modeset_calc_cdclk;
>   dev_priv->display.get_cdclk = bdw_get_cdclk;
> - else if (IS_HASWELL(dev_priv))
> + } else if (IS_HASWELL(dev_priv)) {
>   dev_priv->display.get_cdclk = hsw_get_cdclk;

hsw will look a bit lonely in the middle here, but I suppose that's
not sufficient reason to upstream my hsw_set_cdclk :P

> - else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
> + } else if (IS_CHERRYVIEW(dev_priv)) {
> + dev_priv->display.set_cdclk = chv_set_cdclk;
> + dev_priv->display.modeset_calc_cdclk =
> + vlv_modeset_calc_cdclk;
>   dev_priv->display.get_cdclk = vlv_get_cdclk;

chv before bdw?

Patch is 
Reviewed-by: Ville Syrjälä 

Feel free to ignore any of my bikesheds you don't agree with.

> - else if (IS_GEN6(dev_priv) || IS_IVYBRIDGE(dev_priv))
> + } else if (IS_VALLEYVIEW(dev_priv)) {
> + dev_priv->display.set_cdclk = vlv_set_cdclk;
> + dev_priv->display.modeset_calc_cdclk =
> + vlv_modeset_calc_cdclk;
> + dev_priv->display.get_cdclk =

[Intel-gfx] [PATCH] drm/i915: Consolidate cdclk hooks.

[Intel-gfx] [PATCH] drm/i915: Consolidate cdclk hooks.

Re: [Intel-gfx] [PATCH] drm/i915: Consolidate cdclk hooks.

3 matches

Site Navigation

Mail list logo

Footer information