HI, > -----Original Message----- > From: Intel-gfx [mailto:intel-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx] On Behalf Of > Du,Wenkai > Sent: maanantai 30. huhtikuuta 2018 21.23 > To: Kumar, Abhay <abhay.kumar@xxxxxxxxx>; intel-gfx@xxxxxxxxxxxxxxxxxxxxx > Cc: Nikula, Jani <jani.nikula@xxxxxxxxx> > Subject: Re: [PATCH] drm/i915: Force 2*96 MHz cdclk on glk/cnl when > audio power is enabled > > > > 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@xxxxxxxxxxxxxxx> > > Signed-off-by: Abhay Kumar <abhay.kumar@xxxxxxxxx> > Tested-by: Wenkai Du <wenkai.du@xxxxxxxxx> Please note that failed on CI/GLK: https://patchwork.freedesktop.org/series/42459/ ==== Possible regressions ==== igt@drv_module_reload@basic-reload: shard-glk: PASS -> INCOMPLETE Br, Jani > > 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@xxxxxxxxxxxxxxxxxxxxx > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
