[Intel-gfx] [PATCH v2 01/11] drm/i915: Store max dotclock

[Chris Wilson]

On Thu, Jul 30, 2015 at 09:49:28AM +0300, Mika Kahola wrote:
> Store max dotclock into dev_priv structure so we are able
> to filter out the modes that are not supported by our
> platforms.
> 
> Signed-off-by: Mika Kahola <mika.kahola at intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h      |  1 +
>  drivers/gpu/drm/i915/intel_display.c | 20 ++++++++++++++++++++
>  2 files changed, 21 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 04aa34a..1f69211b 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1777,6 +1777,7 @@ struct drm_i915_private {
>  	unsigned int fsb_freq, mem_freq, is_ddr3;
>  	unsigned int skl_boot_cdclk;
>  	unsigned int cdclk_freq, max_cdclk_freq;
> +	unsigned int max_dotclk;
>  	unsigned int hpll_freq;
>  
>  	/**
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 43b0f17..9031261 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5259,6 +5259,24 @@ static void modeset_update_crtc_power_domains(struct drm_atomic_state *state)
>  			modeset_put_power_domains(dev_priv, put_domains[i]);
>  }
>  
> +static int intel_update_max_dotclk(struct drm_device *dev)

You don't update max dotclck, you are computing it. The caller is the
one storing it dev_priv->max_dotclk (and so is the one actually doing
the update).

> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;

So why did you pass in dev if we never use it?

> +	int max_cdclk_freq = dev_priv->max_cdclk_freq;
> +	int max_dotclk_freq;
> +
> +	if (IS_BROADWELL(dev) || IS_CHERRYVIEW(dev))

We already have dev_priv, so please stop doing dev->dev_priv over and
over again.

> +		max_dotclk_freq = DIV_ROUND_UP(max_cdclk_freq * 100, 95);
> +	else if (IS_VALLEYVIEW(dev))
> +		max_dotclk_freq = DIV_ROUND_UP(max_cdclk_freq * 100, 90);
> +	else if (IS_GEN2(dev) || IS_GEN3(dev))

If you reverse this pair and do

else if (INTEL_INFO(dev)->gen > 3)
	max_dotclk_freq = max_cdclk_freq;
else
	max_dotclk_freq = DIV_ROUND_UP(2 * max_cdclk_freq * 100, 90);

Then the chain is mostly ordered in most-recent to oldest, always
helpful for the next person.

Is this correct for gen9+?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre