Re: [PATCH v2 8/8] drm/i915: Adjust system agent voltage on CNL if required by DDI ports

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Fri, Oct 20, 2017 at 07:09:45PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> On CNL we may need to bump up the system agent voltage not only due
> to CDCLK but also when driving DDI port with a sufficiently high clock.
> To that end start tracking the minimum acceptable voltage for each crtc.
> We do the tracking via crtcs because we don't have any kind of encoder
> state. Also there's no downside to doing it this way, and it matches how
> we track cdclk requirements on account of pixel rate.
> 
> v2: Allow disabled crtcs to use the min voltage
>     Add IS_CNL check to intel_ddi_compute_min_voltage() since
>     we're using CNL specific values there
>     s/intel_compute_min_voltage/cnl_compute_min_voltage/ since
>     the function makes hw specific assumptions about the voltage
>     values
> 
> Cc: Mika Kahola <mika.kahola@intel.com>
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h      |  2 ++
>  drivers/gpu/drm/i915/intel_cdclk.c   | 50 ++++++++++++++++++++++++++++++++++--
>  drivers/gpu/drm/i915/intel_ddi.c     | 11 ++++++++
>  drivers/gpu/drm/i915/intel_display.c |  9 +++++++
>  drivers/gpu/drm/i915/intel_dp_mst.c  |  5 ++++
>  drivers/gpu/drm/i915/intel_drv.h     |  6 +++++
>  6 files changed, 81 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index d3ac58dc275f..185711a852b0 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2415,6 +2415,8 @@ struct drm_i915_private {
>  	unsigned int active_crtcs;
>  	/* minimum acceptable cdclk for each pipe */
>  	int min_cdclk[I915_MAX_PIPES];
> +	/* minimum acceptable voltage for each pipe */
> +	u8 min_voltage[I915_MAX_PIPES];
>  
>  	int dpio_phy_iosf_port[I915_NUM_PHYS_VLV];
>  
> diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
> index 5bbf9f6023a2..78029f70e6ea 100644
> --- a/drivers/gpu/drm/i915/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> @@ -1669,6 +1669,12 @@ static void cnl_set_cdclk(struct drm_i915_private *dev_priv,
>  	mutex_unlock(&dev_priv->pcu_lock);
>  
>  	intel_update_cdclk(dev_priv);
> +
> +	/*
> +	 * Can't read out the voltage :( So let's
> +	 * just assume everything is as expected.
> +	 */
> +	dev_priv->cdclk.hw.voltage = cdclk_state->voltage;
>  }
>  
>  static int cnl_cdclk_pll_vco(struct drm_i915_private *dev_priv, int cdclk)
> @@ -1938,6 +1944,42 @@ static int intel_compute_min_cdclk(struct drm_atomic_state *state)
>  	return min_cdclk;
>  }
>  
> +/*
> + * Note that this functions assumes that 0 is
> + * the lowest voltage value, and higher values
> + * correspond to increasingly higher voltages.
> + *
> + * Should that relationship no longer hold on
> + * future platforms this code will need to be
> + * adjusted.
> + */
> +static u8 cnl_compute_min_voltage(struct intel_atomic_state *state)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> +	struct intel_crtc *crtc;
> +	struct intel_crtc_state *crtc_state;
> +	u8 min_voltage;
> +	int i;
> +	enum pipe pipe;
> +
> +	memcpy(state->min_voltage, dev_priv->min_voltage,
> +	       sizeof(state->min_voltage));
> +
> +	for_each_new_intel_crtc_in_state(state, crtc, crtc_state, i) {
> +		if (crtc_state->base.enable)
> +			state->min_voltage[i] = crtc_state->min_voltage;
> +		else
> +			state->min_voltage[i] = 0;
> +	}
> +
> +	min_voltage = 0;
> +	for_each_pipe(dev_priv, pipe)
> +		min_voltage = max(state->min_voltage[pipe],
> +				  min_voltage);
> +
> +	return min_voltage;
> +}
> +
>  static int vlv_modeset_calc_cdclk(struct drm_atomic_state *state)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(state->dev);
> @@ -2021,7 +2063,9 @@ static int skl_modeset_calc_cdclk(struct drm_atomic_state *state)
>  
>  	intel_state->cdclk.logical.vco = vco;
>  	intel_state->cdclk.logical.cdclk = cdclk;
> -	intel_state->cdclk.logical.voltage = skl_calc_voltage(cdclk);
> +	intel_state->cdclk.logical.voltage =
> +		max(skl_calc_voltage(cdclk),
> +		    cnl_compute_min_voltage(intel_state));

And of course this hunk shouldn't be here. It was part of my test hack.
v3 coming up.

>  
>  	if (!intel_state->active_crtcs) {
>  		cdclk = skl_calc_cdclk(0, vco);

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx