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.ku...@intel.com>; intel-gfx@lists.freedesktop.org; Syrjala, Ville <ville.syrj...@intel.com> Cc: Nikula, Jani <jani.nik...@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-boun...@lists.freedesktop.org] On > Behalf Of Abhay Kumar > Sent: Tuesday, June 19, 2018 3:01 PM > To: intel-gfx@lists.freedesktop.org; Syrjala, Ville > <ville.syrj...@intel.com> > Cc: Nikula, Jani <jani.nik...@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.syrj...@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.syrj...@linux.intel.com> > Signed-off-by: Abhay Kumar <abhay.ku...@intel.com> Tested-by: Wenkai Du <wenkai...@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@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
