[Intel-gfx] [PATCH 04/14] drm/i915: Clean up the .get_cdclk() assignment if ladder

[Ville Syrjälä]

On Tue, Jan 24, 2017 at 02:29:00PM +0200, David Weinehall wrote:
> On Fri, Jan 20, 2017 at 08:21:55PM +0200, ville.syrjala at linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > 
> > Let's clean up the mess we have in the if ladder that assigns the
> > .get_cdclk() hooks. The grouping of the platforms by the function
> > results in a thing that's not really legible, so let's do it the
> > other way around and order the if ladder by platform and duplicate
> > whatever assignments we need.
> > 
> > To further avoid confusion with the function names let's rename
> > them to just fixed_<freq>_get_cdclk(). The other option would
> > be to duplicate the functions entirely but it seems quite
> > pointless to do that since each one just returns a fixed value.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > Reviewed-by: Ander Conselvan de Oliveira <conselvan2 at gmail.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 41 +++++++++++++++++++++---------------
> >  1 file changed, 24 insertions(+), 17 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index bd0fdc04bc92..668c61ad45a6 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -7366,22 +7366,22 @@ static int valleyview_get_cdclk(struct drm_i915_private *dev_priv)
> >  				      CCK_DISPLAY_CLOCK_CONTROL);
> >  }
> >  
> > -static int ilk_get_cdclk(struct drm_i915_private *dev_priv)
> > +static int fixed_450mhz_get_cdclk(struct drm_i915_private *dev_priv)
> >  {
> >  	return 450000;
> >  }
> >  
> > -static int i945_get_cdclk(struct drm_i915_private *dev_priv)
> > +static int fixed_400mhz_get_cdclk(struct drm_i915_private *dev_priv)
> >  {
> >  	return 400000;
> >  }
> >  
> > -static int i915_get_cdclk(struct drm_i915_private *dev_priv)
> > +static int fixed_333mhz_get_cdclk(struct drm_i915_private *dev_priv)
> >  {
> >  	return 333333;
> >  }
> >  
> > -static int i9xx_misc_get_cdclk(struct drm_i915_private *dev_priv)
> > +static int fixed_200mhz_get_cdclk(struct drm_i915_private *dev_priv)
> >  {
> >  	return 200000;
> >  }
> > @@ -7431,7 +7431,7 @@ static int i915gm_get_cdclk(struct drm_i915_private *dev_priv)
> >  	}
> >  }
> >  
> > -static int i865_get_cdclk(struct drm_i915_private *dev_priv)
> > +static int fixed_266mhz_get_cdclk(struct drm_i915_private *dev_priv)
> >  {
> >  	return 266667;
> >  }
> 
> How about reordering this and the one above to have the clocks in
> strictly descending order?

It gets reordered in the patch that moves it all into a new file.

> 
> > @@ -7474,7 +7474,7 @@ static int i85x_get_cdclk(struct drm_i915_private *dev_priv)
> >  	return 0;
> >  }
> 
> Similarly, move this one down.
> 
> > -static int i830_get_cdclk(struct drm_i915_private *dev_priv)
> > +static int fixed_133mhz_get_cdclk(struct drm_i915_private *dev_priv)
> >  {
> >  	return 133333;
> >  }
> > @@ -16180,32 +16180,39 @@ void intel_init_display_hooks(struct drm_i915_private *dev_priv)
> >  		dev_priv->display.get_cdclk = haswell_get_cdclk;
> >  	else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
> >  		dev_priv->display.get_cdclk = valleyview_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))
> > -		dev_priv->display.get_cdclk = ilk_get_cdclk;
> > -	else if (IS_I945G(dev_priv) || IS_I965G(dev_priv) ||
> > -		 IS_GEN6(dev_priv) || IS_IVYBRIDGE(dev_priv))
> > -		dev_priv->display.get_cdclk = i945_get_cdclk;
> > +		dev_priv->display.get_cdclk = fixed_450mhz_get_cdclk;
> >  	else if (IS_GM45(dev_priv))
> >  		dev_priv->display.get_cdclk = gm45_get_cdclk;
> > +	else if (IS_G4X(dev_priv))
> > +		dev_priv->display.get_cdclk = g33_get_cdclk;
> >  	else if (IS_I965GM(dev_priv))
> >  		dev_priv->display.get_cdclk = i965gm_get_cdclk;
> > +	else if (IS_I965G(dev_priv))
> > +		dev_priv->display.get_cdclk = fixed_400mhz_get_cdclk;
> >  	else if (IS_PINEVIEW(dev_priv))
> >  		dev_priv->display.get_cdclk = pnv_get_cdclk;
> > -	else if (IS_G33(dev_priv) || IS_G4X(dev_priv))
> > +	else if (IS_G33(dev_priv))
> >  		dev_priv->display.get_cdclk = g33_get_cdclk;
> > -	else if (IS_I915G(dev_priv))
> > -		dev_priv->display.get_cdclk = i915_get_cdclk;
> > -	else if (IS_I945GM(dev_priv) || IS_I845G(dev_priv))
> > -		dev_priv->display.get_cdclk = i9xx_misc_get_cdclk;
> > +	else if (IS_I945GM(dev_priv))
> > +		dev_priv->display.get_cdclk = fixed_200mhz_get_cdclk;
> > +	else if (IS_I945G(dev_priv))
> > +		dev_priv->display.get_cdclk = fixed_400mhz_get_cdclk;
> >  	else if (IS_I915GM(dev_priv))
> >  		dev_priv->display.get_cdclk = i915gm_get_cdclk;
> > +	else if (IS_I915G(dev_priv))
> > +		dev_priv->display.get_cdclk = fixed_333mhz_get_cdclk;
> >  	else if (IS_I865G(dev_priv))
> > -		dev_priv->display.get_cdclk = i865_get_cdclk;
> > +		dev_priv->display.get_cdclk = fixed_266mhz_get_cdclk;
> >  	else if (IS_I85X(dev_priv))
> >  		dev_priv->display.get_cdclk = i85x_get_cdclk;
> > +	else  if (IS_I845G(dev_priv))
> > +		dev_priv->display.get_cdclk = fixed_200mhz_get_cdclk;
> >  	else { /* 830 */
> >  		WARN(!IS_I830(dev_priv), "Unknown platform. Assuming 133 MHz CDCLK\n");
> > -		dev_priv->display.get_cdclk = i830_get_cdclk;
> > +		dev_priv->display.get_cdclk = fixed_133mhz_get_cdclk;
> >  	}
> >  
> >  	if (IS_GEN5(dev_priv)) {
> > -- 
> > 2.10.2
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC