[Intel-gfx] [PATCH] drm/i915: Remove unused function intel_ddi_get_link_dpll()

Jani Nikula jani.nikula at linux.intel.com
Thu Feb 9 14:47:21 UTC 2017


On Thu, 09 Feb 2017, Ander Conselvan De Oliveira <conselvan2 at gmail.com> wrote:
> On Fri, 2017-01-13 at 14:20 +0200, Ander Conselvan de Oliveira wrote:
>> The function intel_ddi_get_link_dpll() was added in f169660ed4e5
>> ("drm/i915/dp: Add a standalone function to obtain shared dpll for
>> HSW/BDW/SKL/BXT") to "allow for the implementation of a platform
>> neutral upfront link training function", but such implementation
>> never landed.
>
> Ping? Is there any plan of using that function?

Just get rid of it. If it's needed, it can be resurrected. One of the
great advantages of kernel work is that you can look at all the users of
the kernel internal code, and refactor based on that. How do you
refactor code that is not used? How do you know it even works? How do
you figure out what ideas there were for the use of such code? Just
remove it.

Acked-by: Jani Nikula <jani.nikula at intel.com>


>
> Ander
>
>> 
>> So remove that function and clean up the exported shared DPLL interface.
>> 
>> Fixes: f169660ed4e5 ("drm/i915/dp: Add a standalone function to obtain
>> shared dpll for HSW/BDW/SKL/BXT")
>> Cc: Durgadoss R <durgadoss.r at intel.com>
>> Cc: Manasi Navare <manasi.d.navare at intel.com>
>> Cc: Jim Bride <jim.bride at linux.intel.com>
>> Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
>> Cc: Daniel Vetter <daniel.vetter at intel.com>
>> Cc: Jani Nikula <jani.nikula at linux.intel.com>
>> Cc: intel-gfx at lists.freedesktop.org
>> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira at intel.
>> com>
>> 
>> --
>> 
>> I omitted "Cc: <stable at vger.kernel.org> # v4.9+" from dim fixes output,
>> since this just deletes dead code.
>> ---
>>  drivers/gpu/drm/i915/intel_ddi.c      | 39 --------------------------
>>  drivers/gpu/drm/i915/intel_dpll_mgr.c | 52 ++++++--------------------------
>> ---
>>  drivers/gpu/drm/i915/intel_dpll_mgr.h | 16 -----------
>>  3 files changed, 8 insertions(+), 99 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/intel_ddi.c
>> b/drivers/gpu/drm/i915/intel_ddi.c
>> index 66b367d..3eba5bf 100644
>> --- a/drivers/gpu/drm/i915/intel_ddi.c
>> +++ b/drivers/gpu/drm/i915/intel_ddi.c
>> @@ -2126,45 +2126,6 @@ intel_ddi_init_hdmi_connector(struct intel_digital_port
>> *intel_dig_port)
>>  	return connector;
>>  }
>>  
>> -struct intel_shared_dpll *
>> -intel_ddi_get_link_dpll(struct intel_dp *intel_dp, int clock)
>> -{
>> -	struct intel_connector *connector = intel_dp->attached_connector;
>> -	struct intel_encoder *encoder = connector->encoder;
>> -	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>> -	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
>> -	struct intel_shared_dpll *pll = NULL;
>> -	struct intel_shared_dpll_state tmp_pll_state;
>> -	enum intel_dpll_id dpll_id;
>> -
>> -	if (IS_GEN9_LP(dev_priv)) {
>> -		dpll_id =  (enum intel_dpll_id)dig_port->port;
>> -		/*
>> -		 * Select the required PLL. This works for platforms where
>> -		 * there is no shared DPLL.
>> -		 */
>> -		pll = &dev_priv->shared_dplls[dpll_id];
>> -		if (WARN_ON(pll->active_mask)) {
>> -
>> -			DRM_ERROR("Shared DPLL in use. active_mask:%x\n",
>> -				  pll->active_mask);
>> -			return NULL;
>> -		}
>> -		tmp_pll_state = pll->state;
>> -		if (!bxt_ddi_dp_set_dpll_hw_state(clock,
>> -						  &pll->state.hw_state)) {
>> -			DRM_ERROR("Could not setup DPLL\n");
>> -			pll->state = tmp_pll_state;
>> -			return NULL;
>> -		}
>> -	} else if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)) {
>> -		pll = skl_find_link_pll(dev_priv, clock);
>> -	} else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
>> -		pll = hsw_ddi_dp_get_dpll(encoder, clock);
>> -	}
>> -	return pll;
>> -}
>> -
>>  void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
>>  {
>>  	struct intel_digital_port *intel_dig_port;
>> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c
>> b/drivers/gpu/drm/i915/intel_dpll_mgr.c
>> index c92a255..4388c21 100644
>> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
>> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
>> @@ -42,44 +42,6 @@
>>   * commit phase.
>>   */
>>  
>> -struct intel_shared_dpll *
>> -skl_find_link_pll(struct drm_i915_private *dev_priv, int clock)
>> -{
>> -	struct intel_shared_dpll *pll = NULL;
>> -	struct intel_dpll_hw_state dpll_hw_state;
>> -	enum intel_dpll_id i;
>> -	bool found = false;
>> -
>> -	if (!skl_ddi_dp_set_dpll_hw_state(clock, &dpll_hw_state))
>> -		return pll;
>> -
>> -	for (i = DPLL_ID_SKL_DPLL1; i <= DPLL_ID_SKL_DPLL3; i++) {
>> -		pll = &dev_priv->shared_dplls[i];
>> -
>> -		/* Only want to check enabled timings first */
>> -		if (pll->state.crtc_mask == 0)
>> -			continue;
>> -
>> -		if (memcmp(&dpll_hw_state, &pll->state.hw_state,
>> -			   sizeof(pll->state.hw_state)) == 0) {
>> -			found = true;
>> -			break;
>> -		}
>> -	}
>> -
>> -	/* Ok no matching timings, maybe there's a free one? */
>> -	for (i = DPLL_ID_SKL_DPLL1;
>> -	     ((found == false) && (i <= DPLL_ID_SKL_DPLL3)); i++) {
>> -		pll = &dev_priv->shared_dplls[i];
>> -		if (pll->state.crtc_mask == 0) {
>> -			pll->state.hw_state = dpll_hw_state;
>> -			break;
>> -		}
>> -	}
>> -
>> -	return pll;
>> -}
>> -
>>  static void
>>  intel_atomic_duplicate_dpll_state(struct drm_i915_private *dev_priv,
>>  				  struct intel_shared_dpll_state
>> *shared_dpll)
>> @@ -811,8 +773,8 @@ static struct intel_shared_dpll *hsw_ddi_hdmi_get_dpll(int
>> clock,
>>  	return pll;
>>  }
>>  
>> -struct intel_shared_dpll *hsw_ddi_dp_get_dpll(struct intel_encoder *encoder,
>> -					      int clock)
>> +static struct intel_shared_dpll *
>> +hsw_ddi_dp_get_dpll(struct intel_encoder *encoder, int clock)
>>  {
>>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>>  	struct intel_shared_dpll *pll;
>> @@ -1360,8 +1322,9 @@ static bool skl_ddi_hdmi_pll_dividers(struct intel_crtc
>> *crtc,
>>  }
>>  
>>  
>> -bool skl_ddi_dp_set_dpll_hw_state(int clock,
>> -				  struct intel_dpll_hw_state *dpll_hw_state)
>> +static bool
>> +skl_ddi_dp_set_dpll_hw_state(int clock,
>> +			     struct intel_dpll_hw_state *dpll_hw_state)
>>  {
>>  	uint32_t ctrl1;
>>  
>> @@ -1816,8 +1779,9 @@ static bool bxt_ddi_set_dpll_hw_state(int clock,
>>  	return true;
>>  }
>>  
>> -bool bxt_ddi_dp_set_dpll_hw_state(int clock,
>> -			  struct intel_dpll_hw_state *dpll_hw_state)
>> +static bool
>> +bxt_ddi_dp_set_dpll_hw_state(int clock,
>> +			     struct intel_dpll_hw_state *dpll_hw_state)
>>  {
>>  	struct bxt_clk_div clk_div = {0};
>>  
>> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.h
>> b/drivers/gpu/drm/i915/intel_dpll_mgr.h
>> index af1497e..f8d13a9 100644
>> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.h
>> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.h
>> @@ -282,20 +282,4 @@ void intel_shared_dpll_init(struct drm_device *dev);
>>  void intel_dpll_dump_hw_state(struct drm_i915_private *dev_priv,
>>  			      struct intel_dpll_hw_state *hw_state);
>>  
>> -/* BXT dpll related functions */
>> -bool bxt_ddi_dp_set_dpll_hw_state(int clock,
>> -			  struct intel_dpll_hw_state *dpll_hw_state);
>> -
>> -
>> -/* SKL dpll related functions */
>> -bool skl_ddi_dp_set_dpll_hw_state(int clock,
>> -				  struct intel_dpll_hw_state *dpll_hw_state);
>> -struct intel_shared_dpll *skl_find_link_pll(struct drm_i915_private
>> *dev_priv,
>> -					    int clock);
>> -
>> -
>> -/* HSW dpll related functions */
>> -struct intel_shared_dpll *hsw_ddi_dp_get_dpll(struct intel_encoder *encoder,
>> -					      int clock);
>> -
>>  #endif /* _INTEL_DPLL_MGR_H_ */
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center


More information about the Intel-gfx mailing list