Re: [RFCv2 DP-typeC 2/6] drm/i915/dp: Reuse shared DPLL if it exists already

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




>-----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




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux