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

Du,Wenkai wenkai.du at intel.com
Mon Apr 30 18:22:50 UTC 2018



On 4/29/2018 1:39 PM, Abhay Kumar wrote:
> 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.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> Signed-off-by: Abhay Kumar <abhay.kumar at intel.com>
Tested-by: Wenkai Du <wenkai.du at intel.com>

Thanks,
Wenkai
> ---
>   drivers/gpu/drm/i915/i915_drv.h      |  2 ++
>   drivers/gpu/drm/i915/intel_audio.c   | 46 ++++++++++++++++++++++++++++++++++++
>   drivers/gpu/drm/i915/intel_cdclk.c   | 34 +++++++-------------------
>   drivers/gpu/drm/i915/intel_display.c |  7 +++++-
>   drivers/gpu/drm/i915/intel_drv.h     |  1 +
>   5 files changed, 63 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 193176bcddf5..34c31ef0761e 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1708,6 +1708,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/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
> index 3ea566f99450..f001fcf05d3a 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -594,6 +594,7 @@ static void ilk_audio_codec_enable(struct intel_encoder *encoder,
>          I915_WRITE(aud_config, tmp);
>   }
> 
> +
>   /**
>    * intel_audio_codec_enable - Enable the audio codec for HD audio
>    * @encoder: encoder on which to enable audio
> @@ -713,6 +714,48 @@ 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);
> @@ -732,6 +775,9 @@ static void i915_audio_component_codec_wake_override(struct device *kdev,
>          if (!IS_GEN9(dev_priv))
>                  return;
> 
> +       if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
> +               glk_force_audio_cdclk(dev_priv, true);
> +
>          i915_audio_component_get_power(kdev);
> 
>          /*
> diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
> index ebca83a44d9b..4086730018f9 100644
> --- a/drivers/gpu/drm/i915/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> @@ -2141,24 +2141,6 @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state)
>          }
> 
>          /*
> -        * According to BSpec, "The CD clock frequency must be at least twice
> -        * the frequency of the Azalia BCLK." and BCLK is 96 MHz by default.
> -        *
> -        * FIXME: Check the actual, not default, BCLK being used.
> -        *
> -        * FIXME: This does not depend on ->has_audio because the higher CDCLK
> -        * is required for audio probe, also when there are no audio capable
> -        * displays connected at probe time. This leads to unnecessarily high
> -        * CDCLK when audio is not required.
> -        *
> -        * FIXME: This limit is only applied when there are displays connected
> -        * at probe time. If we probe without displays, we'll still end up using
> -        * the platform minimum CDCLK, failing audio probe.
> -        */
> -       if (INTEL_GEN(dev_priv) >= 9)
> -               min_cdclk = max(2 * 96000, min_cdclk);
> -
> -       /*
>           * On Valleyview some DSI panels lose (v|h)sync when the clock is lower
>           * than 320000KHz.
>           */
> @@ -2195,7 +2177,7 @@ static int intel_compute_min_cdclk(struct drm_atomic_state *state)
>                  intel_state->min_cdclk[i] = min_cdclk;
>          }
> 
> -       min_cdclk = 0;
> +       min_cdclk = intel_state->cdclk.force_min_cdclk;
>          for_each_pipe(dev_priv, pipe)
>                  min_cdclk = max(intel_state->min_cdclk[pipe], min_cdclk);
> 
> @@ -2256,7 +2238,7 @@ static int vlv_modeset_calc_cdclk(struct drm_atomic_state *state)
>                  vlv_calc_voltage_level(dev_priv, cdclk);
> 
>          if (!intel_state->active_crtcs) {
> -               cdclk = vlv_calc_cdclk(dev_priv, 0);
> +               cdclk = vlv_calc_cdclk(dev_priv, intel_state->cdclk.force_min_cdclk);
> 
>                  intel_state->cdclk.actual.cdclk = cdclk;
>                  intel_state->cdclk.actual.voltage_level =
> @@ -2289,7 +2271,7 @@ static int bdw_modeset_calc_cdclk(struct drm_atomic_state *state)
>                  bdw_calc_voltage_level(cdclk);
> 
>          if (!intel_state->active_crtcs) {
> -               cdclk = bdw_calc_cdclk(0);
> +               cdclk = bdw_calc_cdclk(intel_state->cdclk.force_min_cdclk);
> 
>                  intel_state->cdclk.actual.cdclk = cdclk;
>                  intel_state->cdclk.actual.voltage_level =
> @@ -2328,7 +2310,7 @@ static int skl_modeset_calc_cdclk(struct drm_atomic_state *state)
>                  skl_calc_voltage_level(cdclk);
> 
>          if (!intel_state->active_crtcs) {
> -               cdclk = skl_calc_cdclk(0, vco);
> +               cdclk = skl_calc_cdclk(intel_state->cdclk.force_min_cdclk, vco);
> 
>                  intel_state->cdclk.actual.vco = vco;
>                  intel_state->cdclk.actual.cdclk = cdclk;
> @@ -2367,10 +2349,10 @@ static int bxt_modeset_calc_cdclk(struct drm_atomic_state *state)
> 
>          if (!intel_state->active_crtcs) {
>                  if (IS_GEMINILAKE(dev_priv)) {
> -                       cdclk = glk_calc_cdclk(0);
> +                       cdclk = glk_calc_cdclk(intel_state->cdclk.force_min_cdclk);
>                          vco = glk_de_pll_vco(dev_priv, cdclk);
>                  } else {
> -                       cdclk = bxt_calc_cdclk(0);
> +                       cdclk = bxt_calc_cdclk(intel_state->cdclk.force_min_cdclk);
>                          vco = bxt_de_pll_vco(dev_priv, cdclk);
>                  }
> 
> @@ -2406,7 +2388,7 @@ static int cnl_modeset_calc_cdclk(struct drm_atomic_state *state)
>                      cnl_compute_min_voltage_level(intel_state));
> 
>          if (!intel_state->active_crtcs) {
> -               cdclk = cnl_calc_cdclk(0);
> +               cdclk = cnl_calc_cdclk(intel_state->cdclk.force_min_cdclk);
>                  vco = cnl_cdclk_pll_vco(dev_priv, cdclk);
> 
>                  intel_state->cdclk.actual.vco = vco;
> @@ -2439,7 +2421,7 @@ static int icl_modeset_calc_cdclk(struct drm_atomic_state *state)
>          intel_state->cdclk.logical.cdclk = cdclk;
> 
>          if (!intel_state->active_crtcs) {
> -               cdclk = icl_calc_cdclk(0, ref);
> +               cdclk = icl_calc_cdclk(intel_state->cdclk.force_min_cdclk, ref);
>                  vco = icl_calc_cdclk_pll_vco(dev_priv, cdclk);
> 
>                  intel_state->cdclk.actual.vco = vco;
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 84ce66be88f2..7c369e15f193 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -12023,6 +12023,10 @@ static int intel_modeset_checks(struct drm_atomic_state *state)
>                  return -EINVAL;
>          }
> 
> +       /* keep the current setting */
> +       if (!intel_state->modeset)
> +               intel_state->cdclk.force_min_cdclk = dev_priv->cdclk.force_min_cdclk;
> +
>          intel_state->modeset = true;
>          intel_state->active_crtcs = dev_priv->active_crtcs;
>          intel_state->cdclk.logical = dev_priv->cdclk.logical;
> @@ -12118,7 +12122,7 @@ static int intel_atomic_check(struct drm_device *dev,
>          struct drm_crtc *crtc;
>          struct drm_crtc_state *old_crtc_state, *crtc_state;
>          int ret, i;
> -       bool any_ms = false;
> +       bool any_ms = intel_state->modeset;
> 
>          /* Catch I915_MODE_FLAG_INHERITED */
>          for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state,
> @@ -12666,6 +12670,7 @@ static int intel_atomic_commit(struct drm_device *dev,
>                  dev_priv->active_crtcs = intel_state->active_crtcs;
>                  dev_priv->cdclk.logical = intel_state->cdclk.logical;
>                  dev_priv->cdclk.actual = intel_state->cdclk.actual;
> +               dev_priv->cdclk.force_min_cdclk = intel_state->cdclk.force_min_cdclk;
>          }
> 
>          drm_atomic_state_get(state);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 11a1932cde6e..79928505d0d0 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -461,6 +461,7 @@ struct intel_atomic_state {
>                   * state only when all crtc's are DPMS off.
>                   */
>                  struct intel_cdclk_state actual;
> +               int force_min_cdclk;
>          } cdclk;
> 
>          bool dpll_set, modeset;
> --
> 2.7.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 


More information about the Intel-gfx mailing list