On Thu, Mar 21, 2019 at 03:02:56PM -0700, Lucas De Marchi wrote:
> By the time cnl_ddi_clock_get() is called we've just got the hw state
> from the pll registers. We don't need to read them again: we can rather
> reuse what was cached in the dpll_hw_state.
>
> This also affects the code for ICL since it partially reuses the CNL
> code. However the more intricate part on ICL is left for another patch.
>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/icl_dsi.c | 5 ++-
> drivers/gpu/drm/i915/intel_ddi.c | 53 +++++++++++++++++---------------
> drivers/gpu/drm/i915/intel_drv.h | 2 +-
> 3 files changed, 31 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/icl_dsi.c b/drivers/gpu/drm/i915/icl_dsi.c
> index beb30d9a855c..c7dcd783d986 100644
> --- a/drivers/gpu/drm/i915/icl_dsi.c
> +++ b/drivers/gpu/drm/i915/icl_dsi.c
> @@ -1179,11 +1179,10 @@ static void gen11_dsi_get_config(struct intel_encoder *encoder,
> {
> struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
> - u32 pll_id;
>
> /* FIXME: adapt icl_ddi_clock_get() for DSI and use that? */
> - pll_id = intel_get_shared_dpll_id(dev_priv, pipe_config->shared_dpll);
> - pipe_config->port_clock = cnl_calc_wrpll_link(dev_priv, pll_id);
> + pipe_config->port_clock =
> + cnl_calc_wrpll_link(dev_priv, &pipe_config->dpll_hw_state);
> pipe_config->base.adjusted_mode.crtc_clock = intel_dsi->pclk;
> pipe_config->output_types |= BIT(INTEL_OUTPUT_DSI);
> }
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 644541924208..fe52af9fa4aa 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1294,25 +1294,17 @@ static int skl_calc_wrpll_link(struct intel_dpll_hw_state *state)
> return dco_freq / (p0 * p1 * p2 * 5);
> }
>
> +
Stray newline.
> int cnl_calc_wrpll_link(struct drm_i915_private *dev_priv,
> - enum intel_dpll_id pll_id)
> + struct intel_dpll_hw_state *state)
> {
> - u32 cfgcr0, cfgcr1;
> u32 p0, p1, p2, dco_freq, ref_clock;
>
> - if (INTEL_GEN(dev_priv) >= 11) {
> - cfgcr0 = I915_READ(ICL_DPLL_CFGCR0(pll_id));
> - cfgcr1 = I915_READ(ICL_DPLL_CFGCR1(pll_id));
> - } else {
> - cfgcr0 = I915_READ(CNL_DPLL_CFGCR0(pll_id));
> - cfgcr1 = I915_READ(CNL_DPLL_CFGCR1(pll_id));
> - }
> -
> - p0 = cfgcr1 & DPLL_CFGCR1_PDIV_MASK;
> - p2 = cfgcr1 & DPLL_CFGCR1_KDIV_MASK;
> + p0 = state->cfgcr1 & DPLL_CFGCR1_PDIV_MASK;
> + p2 = state->cfgcr1 & DPLL_CFGCR1_KDIV_MASK;
>
> - if (cfgcr1 & DPLL_CFGCR1_QDIV_MODE(1))
> - p1 = (cfgcr1 & DPLL_CFGCR1_QDIV_RATIO_MASK) >>
> + if (state->cfgcr1 & DPLL_CFGCR1_QDIV_MODE(1))
> + p1 = (state->cfgcr1 & DPLL_CFGCR1_QDIV_RATIO_MASK) >>
> DPLL_CFGCR1_QDIV_RATIO_SHIFT;
> else
> p1 = 1;
> @@ -1347,9 +1339,9 @@ int cnl_calc_wrpll_link(struct drm_i915_private *dev_priv,
>
> ref_clock = cnl_hdmi_pll_ref_clock(dev_priv);
>
> - dco_freq = (cfgcr0 & DPLL_CFGCR0_DCO_INTEGER_MASK) * ref_clock;
> + dco_freq = (state->cfgcr0 & DPLL_CFGCR0_DCO_INTEGER_MASK) * ref_clock;
>
> - dco_freq += (((cfgcr0 & DPLL_CFGCR0_DCO_FRACTION_MASK) >>
> + dco_freq += (((state->cfgcr0 & DPLL_CFGCR0_DCO_FRACTION_MASK) >>
> DPLL_CFGCR0_DCO_FRACTION_SHIFT) * ref_clock) / 0x8000;
>
> if (WARN_ON(p0 == 0 || p1 == 0 || p2 == 0))
> @@ -1462,13 +1454,21 @@ static void icl_ddi_clock_get(struct intel_encoder *encoder,
> struct intel_crtc_state *pipe_config)
> {
> struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> + struct intel_dpll_hw_state *state;
> enum port port = encoder->port;
> int link_clock = 0;
> u32 pll_id;
>
> pll_id = intel_get_shared_dpll_id(dev_priv, pipe_config->shared_dpll);
> +
> + /* For DDI ports we always use a shared PLL. */
> + if (WARN_ON(!pipe_config->shared_dpll))
> + goto end;
> +
> + state = &pipe_config->dpll_hw_state;
No point in assigning this late.
> +
> if (intel_port_is_combophy(dev_priv, port)) {
> - link_clock = cnl_calc_wrpll_link(dev_priv, pll_id);
> + link_clock = cnl_calc_wrpll_link(dev_priv, state);
> } else {
> if (pll_id == DPLL_ID_ICL_TBTPLL)
> link_clock = icl_calc_tbt_pll_link(dev_priv, port);
> @@ -1476,7 +1476,9 @@ static void icl_ddi_clock_get(struct intel_encoder *encoder,
> link_clock = icl_calc_mg_pll_link(dev_priv, port);
> }
>
> +end:
> pipe_config->port_clock = link_clock;
> +
> ddi_dotclock_get(pipe_config);
> }
>
> @@ -1484,18 +1486,18 @@ static void cnl_ddi_clock_get(struct intel_encoder *encoder,
> struct intel_crtc_state *pipe_config)
> {
> struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> + struct intel_dpll_hw_state *state;
> int link_clock = 0;
> - u32 cfgcr0;
> - enum intel_dpll_id pll_id;
> -
> - pll_id = intel_get_shared_dpll_id(dev_priv, pipe_config->shared_dpll);
>
> - cfgcr0 = I915_READ(CNL_DPLL_CFGCR0(pll_id));
> + /* For DDI ports we always use a shared PLL. */
> + if (WARN_ON(!pipe_config->shared_dpll))
> + goto end;
>
> - if (cfgcr0 & DPLL_CFGCR0_HDMI_MODE) {
> - link_clock = cnl_calc_wrpll_link(dev_priv, pll_id);
> + state = &pipe_config->dpll_hw_state;
ditto
> + if (state->cfgcr0 & DPLL_CFGCR0_HDMI_MODE) {
> + link_clock = cnl_calc_wrpll_link(dev_priv, state);
> } else {
> - link_clock = cfgcr0 & DPLL_CFGCR0_LINK_RATE_MASK;
> + link_clock = state->cfgcr0 & DPLL_CFGCR0_LINK_RATE_MASK;
>
> switch (link_clock) {
> case DPLL_CFGCR0_LINK_RATE_810:
> @@ -1529,6 +1531,7 @@ static void cnl_ddi_clock_get(struct intel_encoder *encoder,
> link_clock *= 2;
> }
>
> +end:
> pipe_config->port_clock = link_clock;
>
> ddi_dotclock_get(pipe_config);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 4d7ae579fc92..65ab0f90ce6a 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1659,7 +1659,7 @@ int intel_ddi_toggle_hdcp_signalling(struct intel_encoder *intel_encoder,
> bool enable);
> void icl_sanitize_encoder_pll_mapping(struct intel_encoder *encoder);
> int cnl_calc_wrpll_link(struct drm_i915_private *dev_priv,
> - enum intel_dpll_id pll_id);
> + struct intel_dpll_hw_state *state);
>
> unsigned int intel_fb_align_height(const struct drm_framebuffer *fb,
> int color_plane, unsigned int height);
> --
> 2.20.1
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
