Re: [PATCH v3] drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio power is enabled

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

On Tue, Jun 12, 2018 at 12:17:41AM -0700, Abhay Kumar wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> CDCLK has to be at least twice the BLCK regardless of audio. Audio
> driver has to probe using this hook and increase the clock even in
> absence of any display.
> 
> v2: Use atomic refcount for get_power, put_power so that we can
>     call each once(Abhay).
> v3: Reset power well 2 to avoid any transaction on iDisp link
>     during cdclk change(Abhay).
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Abhay Kumar <abhay.kumar@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h      |  3 ++
>  drivers/gpu/drm/i915/i915_reg.h      |  4 ++
>  drivers/gpu/drm/i915/intel_audio.c   | 87 ++++++++++++++++++++++++++++++++++--
>  drivers/gpu/drm/i915/intel_cdclk.c   | 29 ++++--------
>  drivers/gpu/drm/i915/intel_display.c |  7 ++-
>  drivers/gpu/drm/i915/intel_drv.h     |  2 +
>  6 files changed, 107 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 6104d7115054..a4a386a5db69 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1702,6 +1702,7 @@ struct drm_i915_private {
>  	unsigned int hpll_freq;
>  	unsigned int fdi_pll_freq;
>  	unsigned int czclk_freq;
> +	u32 get_put_refcount;
>  
>  	struct {
>  		/*
> @@ -1719,6 +1720,8 @@ struct drm_i915_private {
>  		struct intel_cdclk_state actual;
>  		/* The current hardware cdclk state */
>  		struct intel_cdclk_state hw;
> +
> +		int force_min_cdclk;
>  	} cdclk;
>  
>  	/**
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 987def26ce82..cef770184245 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -8869,6 +8869,10 @@ enum skl_power_gate {
>   * SKL Clocks
>   */
>  
> +/* Power well 2 */
> +#define POWER_WELL_2			_MMIO(0x45404)
> +#define POWER_WELL_2_REQUEST		(1<<31)
> +
>  /* CDCLK_CTL */
>  #define CDCLK_CTL			_MMIO(0x46000)
>  #define  CDCLK_FREQ_SEL_MASK		(3 << 26)
> diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
> index 3ea566f99450..1f5a9af13ef0 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -618,7 +618,6 @@ void intel_audio_codec_enable(struct intel_encoder *encoder,
>  
>  	if (!connector->eld[0])
>  		return;
> -
>  	DRM_DEBUG_DRIVER("ELD on [CONNECTOR:%d:%s], [ENCODER:%d:%s]\n",
>  			 connector->base.id,
>  			 connector->name,
> @@ -713,14 +712,94 @@ void intel_init_audio_hooks(struct drm_i915_private *dev_priv)
>  	}
>  }
>  
> +static void glk_force_audio_cdclk(struct drm_i915_private *dev_priv,
> +				bool enable)
> +{
> +	struct drm_modeset_acquire_ctx ctx;
> +	struct drm_atomic_state *state;
> +	int ret;
> +
> +	drm_modeset_acquire_init(&ctx, 0);
> +	state = drm_atomic_state_alloc(&dev_priv->drm);
> +	if (WARN_ON(!state))
> +		return;
> +
> +	state->acquire_ctx = &ctx;
> +
> +retry:
> +	to_intel_atomic_state(state)->modeset = true;
> +	to_intel_atomic_state(state)->cdclk.force_min_cdclk =
> +		enable ? 2 * 96000 : 0;
> +
> +	/*
> +	 * Protects dev_priv->cdclk.force_min_cdclk
> +	 * Need to lock this here in case we have no active pipes
> +	 * and thus wouldn't lock it during the commit otherwise.
> +	 */
> +	ret = drm_modeset_lock(&dev_priv->drm.mode_config.connection_mutex, &ctx);
> +	if (!ret)
> +		ret = drm_atomic_commit(state);
> +
> +	if (ret == -EDEADLK) {
> +		drm_atomic_state_clear(state);
> +		drm_modeset_backoff(&ctx);
> +		goto retry;
> +	}
> +
> +	WARN_ON(ret);
> +
> +	drm_atomic_state_put(state);
> +
> +	drm_modeset_drop_locks(&ctx);
> +	drm_modeset_acquire_fini(&ctx);
> +}
> +
>  static void i915_audio_component_get_power(struct device *kdev)
>  {
> -	intel_display_power_get(kdev_to_i915(kdev), POWER_DOMAIN_AUDIO);
> +	struct drm_i915_private *dev_priv = kdev_to_i915(kdev);
> +	u32 tmp;
> +
> +	dev_priv->get_put_refcount++;
> +
> +	/* Force cdclk to 2*BCLK during first time get power call */
> +	if (dev_priv->get_put_refcount == 1) {
> +		if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv)) {
> +
> +			/*FIXME: Make sure there is no transaction
> +			 * on iDisp link while changing cdclk
> +			 */
> +
> +			/* Turn off power well 2*/
> +			tmp = I915_READ(POWER_WELL_2);
> +			tmp = tmp & ~POWER_WELL_2_REQUEST;
> +			I915_WRITE(POWER_WELL_2, tmp);
> +			tmp = I915_READ(POWER_WELL_2);
> +
> +			/* Turn on power well 2*/
> +			tmp = I915_READ(POWER_WELL_2);
> +			tmp = tmp | POWER_WELL_2_REQUEST;
> +			I915_WRITE(POWER_WELL_2, tmp);
> +			tmp = I915_READ(POWER_WELL_2);

You can't just shut down the power well like that.
Things may be using it.

> +
> +			glk_force_audio_cdclk(dev_priv, true);
> +		}
> +	}
> +
> +	intel_display_power_get(dev_priv, POWER_DOMAIN_AUDIO);
>  }

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