On Wed, 14 Oct 2015, Durgadoss R <durgadoss.r@xxxxxxxxx> wrote:
> Do not call intel_get_shared_dpll() if there exists a
> valid shared DPLL already.
>
> Signed-off-by: Durgadoss R <durgadoss.r@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/intel_ddi.c | 70 ++++++++++++++++++++----------------
> drivers/gpu/drm/i915/intel_display.c | 2 +-
> drivers/gpu/drm/i915/intel_drv.h | 2 +-
> 3 files changed, 42 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 9098c12..8e4ea36 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1258,7 +1258,8 @@ hsw_ddi_calculate_wrpll(int clock /* in Hz */,
> static bool
> hsw_ddi_pll_select(struct intel_crtc *intel_crtc,
> struct intel_crtc_state *crtc_state,
> - struct intel_encoder *intel_encoder)
> + struct intel_encoder *intel_encoder,
> + bool find_dpll)
> {
> int clock = crtc_state->port_clock;
>
> @@ -1278,14 +1279,16 @@ hsw_ddi_pll_select(struct intel_crtc *intel_crtc,
>
> crtc_state->dpll_hw_state.wrpll = val;
>
> - pll = intel_get_shared_dpll(intel_crtc, crtc_state);
> - if (pll == NULL) {
> - DRM_DEBUG_DRIVER("failed to find PLL for pipe %c\n",
> - pipe_name(intel_crtc->pipe));
> - return false;
> - }
> + if (find_dpll) {
> + pll = intel_get_shared_dpll(intel_crtc, crtc_state);
> + if (pll == NULL) {
> + DRM_DEBUG_DRIVER("failed to find PLL for pipe %c\n",
> + pipe_name(intel_crtc->pipe));
> + return false;
> + }
You have to do a *lot* of passing around parameters here. I wonder (and
really, I don't know) if we could make intel_get_shared_dpll() grow
smarts about reusing the pll with intel_crtc_to_shared_dpll() when it's
there already.
BR,
Jani.
>
> - crtc_state->ddi_pll_sel = PORT_CLK_SEL_WRPLL(pll->id);
> + crtc_state->ddi_pll_sel = PORT_CLK_SEL_WRPLL(pll->id);
> + }
> }
>
> return true;
> @@ -1540,7 +1543,8 @@ skip_remaining_dividers:
> static bool
> skl_ddi_pll_select(struct intel_crtc *intel_crtc,
> struct intel_crtc_state *crtc_state,
> - struct intel_encoder *intel_encoder)
> + struct intel_encoder *intel_encoder,
> + bool find_dpll)
> {
> struct intel_shared_dpll *pll;
> uint32_t ctrl1, cfgcr1, cfgcr2;
> @@ -1594,15 +1598,17 @@ skl_ddi_pll_select(struct intel_crtc *intel_crtc,
> crtc_state->dpll_hw_state.cfgcr1 = cfgcr1;
> crtc_state->dpll_hw_state.cfgcr2 = cfgcr2;
>
> - pll = intel_get_shared_dpll(intel_crtc, crtc_state);
> - if (pll == NULL) {
> - DRM_DEBUG_DRIVER("failed to find PLL for pipe %c\n",
> - pipe_name(intel_crtc->pipe));
> - return false;
> - }
> + if (find_dpll) {
> + pll = intel_get_shared_dpll(intel_crtc, crtc_state);
> + if (pll == NULL) {
> + DRM_DEBUG_DRIVER("failed to find PLL for pipe %c\n",
> + pipe_name(intel_crtc->pipe));
> + return false;
> + }
>
> - /* shared DPLL id 0 is DPLL 1 */
> - crtc_state->ddi_pll_sel = pll->id + 1;
> + /* shared DPLL id 0 is DPLL 1 */
> + crtc_state->ddi_pll_sel = pll->id + 1;
> + }
>
> return true;
> }
> @@ -1632,7 +1638,8 @@ static const struct bxt_clk_div bxt_dp_clk_val[] = {
> static bool
> bxt_ddi_pll_select(struct intel_crtc *intel_crtc,
> struct intel_crtc_state *crtc_state,
> - struct intel_encoder *intel_encoder)
> + struct intel_encoder *intel_encoder,
> + bool find_pll)
> {
> struct intel_shared_dpll *pll;
> struct bxt_clk_div clk_div = {0};
> @@ -1741,15 +1748,17 @@ bxt_ddi_pll_select(struct intel_crtc *intel_crtc,
> crtc_state->dpll_hw_state.pcsdw12 =
> LANESTAGGER_STRAP_OVRD | lanestagger;
>
> - pll = intel_get_shared_dpll(intel_crtc, crtc_state);
> - if (pll == NULL) {
> - DRM_DEBUG_DRIVER("failed to find PLL for pipe %c\n",
> - pipe_name(intel_crtc->pipe));
> - return false;
> - }
> + if (find_pll) {
> + pll = intel_get_shared_dpll(intel_crtc, crtc_state);
> + if (pll == NULL) {
> + DRM_DEBUG_DRIVER("failed to find PLL for pipe %c\n",
> + pipe_name(intel_crtc->pipe));
> + return false;
> + }
>
> - /* shared DPLL id 0 is DPLL A */
> - crtc_state->ddi_pll_sel = pll->id;
> + /* shared DPLL id 0 is DPLL A */
> + crtc_state->ddi_pll_sel = pll->id;
> + }
>
> return true;
> }
> @@ -1763,7 +1772,8 @@ bxt_ddi_pll_select(struct intel_crtc *intel_crtc,
> */
> bool intel_ddi_pll_select(struct intel_crtc *intel_crtc,
> struct intel_crtc_state *crtc_state,
> - struct intel_encoder *valid_encoder)
> + struct intel_encoder *valid_encoder,
> + bool find_dpll)
> {
> struct drm_device *dev = intel_crtc->base.dev;
> struct intel_encoder *intel_encoder;
> @@ -1775,13 +1785,13 @@ bool intel_ddi_pll_select(struct intel_crtc *intel_crtc,
>
> if (IS_SKYLAKE(dev))
> return skl_ddi_pll_select(intel_crtc, crtc_state,
> - intel_encoder);
> + intel_encoder, find_dpll);
> else if (IS_BROXTON(dev))
> return bxt_ddi_pll_select(intel_crtc, crtc_state,
> - intel_encoder);
> + intel_encoder, find_dpll);
> else
> return hsw_ddi_pll_select(intel_crtc, crtc_state,
> - intel_encoder);
> + intel_encoder, find_dpll);
> }
>
> void intel_ddi_set_pipe_settings(struct drm_crtc *crtc)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 8ae6d7b..5c2b4ce 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9661,7 +9661,7 @@ static void broadwell_modeset_commit_cdclk(struct drm_atomic_state *old_state)
> static int haswell_crtc_compute_clock(struct intel_crtc *crtc,
> struct intel_crtc_state *crtc_state)
> {
> - if (!intel_ddi_pll_select(crtc, crtc_state, NULL))
> + if (!intel_ddi_pll_select(crtc, crtc_state, NULL, true))
> return -EINVAL;
>
> crtc->lowfreq_avail = false;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 0822ab6..cbcfa14 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -990,7 +990,7 @@ void intel_ddi_enable_pipe_clock(struct intel_crtc *intel_crtc);
> void intel_ddi_disable_pipe_clock(struct intel_crtc *intel_crtc);
> bool intel_ddi_pll_select(struct intel_crtc *crtc,
> struct intel_crtc_state *crtc_state,
> - struct intel_encoder *encoder);
> + struct intel_encoder *encoder, bool find_dpll);
> void intel_ddi_set_pipe_settings(struct drm_crtc *crtc);
> void intel_ddi_prepare_link_retrain(struct drm_encoder *encoder);
> bool intel_ddi_connector_get_hw_state(struct intel_connector *intel_connector);
> --
> 1.9.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
