[Intel-gfx] [PATCH v5] drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio power is enabled
Kumar, Abhay
abhay.kumar at intel.com
Tue Jul 3 05:06:43 UTC 2018
Hi Ville,
Can we please get this merged to DINQ?
Regards,
Abhay
-----Original Message-----
From: Du, Wenkai
Sent: Thursday, June 21, 2018 1:16 PM
To: Kumar, Abhay <abhay.kumar at intel.com>; intel-gfx at lists.freedesktop.org; Syrjala, Ville <ville.syrjala at intel.com>
Cc: Nikula, Jani <jani.nikula at intel.com>
Subject: Re: [Intel-gfx] [PATCH v5] drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio power is enabled
On 6/21/2018 11:43 AM, Kumar, Abhay wrote:
> + Wenkai
>
> -----Original Message-----
> From: Intel-gfx [mailto:intel-gfx-bounces at lists.freedesktop.org] On
> Behalf Of Abhay Kumar
> Sent: Tuesday, June 19, 2018 3:01 PM
> To: intel-gfx at lists.freedesktop.org; Syrjala, Ville
> <ville.syrjala at intel.com>
> Cc: Nikula, Jani <jani.nikula at intel.com>
> Subject: [Intel-gfx] [PATCH v5] drm/i915: Force 2*96 MHz cdclk on
> glk/cnl when audio power is enabled
>
> From: Ville Syrjälä <ville.syrjala at 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).
> v4: Remove Power well 2 reset workaround(Ville).
> v5: Remove unwanted Power well 2 register defined in v4(Abhay).
>
> 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>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 3 ++
> drivers/gpu/drm/i915/intel_audio.c | 67 +++++++++++++++++++++++++++++++++---
> drivers/gpu/drm/i915/intel_cdclk.c | 29 +++++-----------
> drivers/gpu/drm/i915/intel_display.c | 7 +++-
> drivers/gpu/drm/i915/intel_drv.h | 2 ++
> 5 files changed, 83 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/intel_audio.c
> b/drivers/gpu/drm/i915/intel_audio.c
> index 3ea566f99450..ca8f04c7cbb3 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,74 @@ 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);
> +
> + 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))
> + glk_force_audio_cdclk(dev_priv, true);
> +
> + intel_display_power_get(dev_priv, POWER_DOMAIN_AUDIO);
> }
>
> static void i915_audio_component_put_power(struct device *kdev) {
> - intel_display_power_put(kdev_to_i915(kdev), POWER_DOMAIN_AUDIO);
> + struct drm_i915_private *dev_priv = kdev_to_i915(kdev);
> +
> + dev_priv->get_put_refcount--;
> +
> + /* Force required cdclk during last time put power call */
> + if (dev_priv->get_put_refcount == 0)
> + if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
> + glk_force_audio_cdclk(dev_priv, false);
> +
> + intel_display_power_put(dev_priv, POWER_DOMAIN_AUDIO);
> }
>
> static void i915_audio_component_codec_wake_override(struct device *kdev, @@ -959,7 +1018,7 @@ void i915_audio_component_init(struct drm_i915_private *dev_priv)
> /* continue with reduced functionality */
> return;
> }
> -
> + dev_priv->get_put_refcount = 0;
> dev_priv->audio_component_registered = true; }
>
> diff --git a/drivers/gpu/drm/i915/intel_cdclk.c
> b/drivers/gpu/drm/i915/intel_cdclk.c
> index 8ed7bd052e46..0f0aea900ceb 100644
> --- a/drivers/gpu/drm/i915/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> @@ -2153,19 +2153,8 @@ 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)
> + if (crtc_state->has_audio && INTEL_GEN(dev_priv) >= 9)
> min_cdclk = max(2 * 96000, min_cdclk);
>
> /*
> @@ -2205,7 +2194,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);
>
> @@ -2266,7 +2255,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 = @@ -2299,7 +2288,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 = @@ -2371,7 +2360,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; @@ -2410,10 +2399,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);
> }
>
> @@ -2449,7 +2438,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; @@ -2482,7 +2471,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 17c590b42fd7..3ee1c1f5419d 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -12162,6 +12162,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; @@ -12257,7 +12261,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, @@ -12805,6 +12809,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 8641583842be..0da17ad056ec 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -459,6 +459,8 @@ 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