[Intel-gfx] [RFCv2 DP-typeC 2/6] drm/i915/dp: Reuse shared DPLL if it exists already
R, Durgadoss
durgadoss.r at intel.com
Thu Oct 15 02:07:44 PDT 2015
>-----Original Message-----
>From: Jani Nikula [mailto:jani.nikula at linux.intel.com]
>Sent: Wednesday, October 14, 2015 6:53 PM
>To: R, Durgadoss; intel-gfx at lists.freedesktop.org
>Subject: Re: [Intel-gfx] [RFCv2 DP-typeC 2/6] drm/i915/dp: Reuse shared DPLL if it exists already
>
>On Wed, 14 Oct 2015, Durgadoss R <durgadoss.r at intel.com> wrote:
>> Do not call intel_get_shared_dpll() if there exists a
>> valid shared DPLL already.
>>
>> Signed-off-by: Durgadoss R <durgadoss.r at intel.com>
>> ---
>> 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 at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>--
>Jani Nikula, Intel Open Source Technology Center
More information about the Intel-gfx
mailing list