On Tue, Dec 03, 2019 at 01:29:02PM -0800, José Roberto de Souza wrote:
> Talked with HW team and this is a left over, driver should not
> program clockgating, dekel firmware will be reponsible for any
> clockgating programing.
>
> v2:
> Added WARN_ON
>
> v3:
> Only calling icl_phy_set_clock_gating() on intel_ddi_pre_enable_hdmi
> for GEN11
>
> BSpec issue: 20885
> BSpec: 49292
>
> Cc: Lucas De Marchi <lucas.demarchi@xxxxxxxxx>
> Cc: Matt Roper <matthew.d.roper@xxxxxxxxx>
> Cc: Jani Nikula <jani.nikula@xxxxxxxxx>
> Reviewed-by: Matt Roper <matthew.d.roper@xxxxxxxxx> (v1)
> Signed-off-by: José Roberto de Souza <jose.souza@xxxxxxxxx>
> ---
>
> Hi Matt, there was 2 small changes since v1, could check and give
> another review?
Yep, the additional WARN_ON() and GEN==11 test look good.
However I just noticed the wording on bspec page 21735 (the gen11
equivalent page) has also been updated and it sounds like we may not
need to do this clockgating there anymore either? That page now says
"PHY clock gating should be configured by the typeC micocontroller so
that display software does not need to modify it." So maybe we should
just be removing this function entirely now?
Matt
>
> Thanks
>
> drivers/gpu/drm/i915/display/intel_ddi.c | 57 +++++++-----------------
> 1 file changed, 17 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index ebcc7302706b..62a4a8bb2457 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -3167,6 +3167,10 @@ icl_phy_set_clock_gating(struct intel_digital_port *dig_port, bool enable)
> u32 val, bits;
> int ln;
>
> + /* See "PHY Clockgating programming" note */
> + if (WARN_ON(INTEL_GEN(dev_priv) >= 12))
> + return;
> +
> if (tc_port == PORT_TC_NONE)
> return;
>
> @@ -3175,39 +3179,26 @@ icl_phy_set_clock_gating(struct intel_digital_port *dig_port, bool enable)
> MG_DP_MODE_CFG_GAONPWR_GATING;
>
> for (ln = 0; ln < 2; ln++) {
> - if (INTEL_GEN(dev_priv) >= 12) {
> - I915_WRITE(HIP_INDEX_REG(tc_port), HIP_INDEX_VAL(tc_port, ln));
> - val = I915_READ(DKL_DP_MODE(tc_port));
> - } else {
> - val = I915_READ(MG_DP_MODE(ln, tc_port));
> - }
> + val = I915_READ(MG_DP_MODE(ln, tc_port));
>
> if (enable)
> val |= bits;
> else
> val &= ~bits;
>
> - if (INTEL_GEN(dev_priv) >= 12)
> - I915_WRITE(DKL_DP_MODE(tc_port), val);
> - else
> - I915_WRITE(MG_DP_MODE(ln, tc_port), val);
> + I915_WRITE(MG_DP_MODE(ln, tc_port), val);
> }
>
> - if (INTEL_GEN(dev_priv) == 11) {
> - bits = MG_MISC_SUS0_CFG_TR2PWR_GATING |
> - MG_MISC_SUS0_CFG_CL2PWR_GATING |
> - MG_MISC_SUS0_CFG_GAONPWR_GATING |
> - MG_MISC_SUS0_CFG_TRPWR_GATING |
> - MG_MISC_SUS0_CFG_CL1PWR_GATING |
> - MG_MISC_SUS0_CFG_DGPWR_GATING;
> + bits = MG_MISC_SUS0_CFG_TR2PWR_GATING | MG_MISC_SUS0_CFG_CL2PWR_GATING |
> + MG_MISC_SUS0_CFG_GAONPWR_GATING | MG_MISC_SUS0_CFG_TRPWR_GATING |
> + MG_MISC_SUS0_CFG_CL1PWR_GATING | MG_MISC_SUS0_CFG_DGPWR_GATING;
>
> - val = I915_READ(MG_MISC_SUS0(tc_port));
> - if (enable)
> - val |= (bits | MG_MISC_SUS0_SUSCLK_DYNCLKGATE_MODE(3));
> - else
> - val &= ~(bits | MG_MISC_SUS0_SUSCLK_DYNCLKGATE_MODE_MASK);
> - I915_WRITE(MG_MISC_SUS0(tc_port), val);
> - }
> + val = I915_READ(MG_MISC_SUS0(tc_port));
> + if (enable)
> + val |= (bits | MG_MISC_SUS0_SUSCLK_DYNCLKGATE_MODE(3));
> + else
> + val &= ~(bits | MG_MISC_SUS0_SUSCLK_DYNCLKGATE_MODE_MASK);
> + I915_WRITE(MG_MISC_SUS0(tc_port), val);
> }
>
> static void
> @@ -3508,12 +3499,6 @@ static void tgl_ddi_pre_enable_dp(struct intel_encoder *encoder,
> * down this function.
> */
>
> - /*
> - * 7.d Type C with DP alternate or fixed/legacy/static connection -
> - * Disable PHY clock gating per Type-C DDI Buffer page
> - */
> - icl_phy_set_clock_gating(dig_port, false);
> -
> /* 7.e Configure voltage swing and related IO settings */
> tgl_ddi_vswing_sequence(encoder, crtc_state->port_clock, level,
> encoder->type);
> @@ -3565,15 +3550,6 @@ static void tgl_ddi_pre_enable_dp(struct intel_encoder *encoder,
> if (!is_trans_port_sync_mode(crtc_state))
> intel_dp_stop_link_train(intel_dp);
>
> - /*
> - * TODO: enable clock gating
> - *
> - * It is not written in DP enabling sequence but "PHY Clockgating
> - * programming" states that clock gating should be enabled after the
> - * link training but doing so causes all the following trainings to fail
> - * so not enabling it for now.
> - */
> -
> /* 7.l Configure and enable FEC if needed */
> intel_ddi_enable_fec(encoder, crtc_state);
> intel_dsc_enable(encoder, crtc_state);
> @@ -3701,7 +3677,8 @@ static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder,
> else
> intel_prepare_hdmi_ddi_buffers(encoder, level);
>
> - icl_phy_set_clock_gating(dig_port, true);
> + if (INTEL_GEN(dev_priv) == 11)
> + icl_phy_set_clock_gating(dig_port, true);
>
> if (IS_GEN9_BC(dev_priv))
> skl_ddi_set_iboost(encoder, level, INTEL_OUTPUT_HDMI);
> --
> 2.24.0
>
--
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
