[Intel-gfx] [PATCH] drm/i915: Consolidate cdclk hooks.
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.
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.
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 =