>-----Original Message----- >From: Jani Nikula [mailto:jani.nikula@xxxxxxxxxxxxxxx] >Sent: Wednesday, October 14, 2015 6:53 PM >To: R, Durgadoss; intel-gfx@xxxxxxxxxxxxxxxxxxxxx >Subject: Re: [RFCv2 DP-typeC 2/6] drm/i915/dp: Reuse shared DPLL if it exists already > >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. Sure. I will try it out and do this change if it works without issues. Thanks, Durga > >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
