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

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

[Ander Conselvan de Oliveira]

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.

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@intel.com>
Cc: Manasi Navare <manasi.d.navare@intel.com>
Cc: Jim Bride <jim.bride@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Daniel Vetter <daniel.vetter@intel.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org
Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>

--

I omitted "Cc: <stable@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_ */
-- 
2.5.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for drm/i915: Remove unused function intel_ddi_get_link_dpll()

[Patchwork]

== Series Details ==

Series: drm/i915: Remove unused function intel_ddi_get_link_dpll()
URL   : https://patchwork.freedesktop.org/series/17963/
State : success

== Summary ==

Series 17963v1 drm/i915: Remove unused function intel_ddi_get_link_dpll()
https://patchwork.freedesktop.org/api/1.0/series/17963/revisions/1/mbox/


fi-bdw-5557u     total:246  pass:232  dwarn:0   dfail:0   fail:0   skip:14 
fi-bsw-n3050     total:246  pass:207  dwarn:0   dfail:0   fail:0   skip:39 
fi-bxt-j4205     total:246  pass:224  dwarn:0   dfail:0   fail:0   skip:22 
fi-bxt-t5700     total:82   pass:69   dwarn:0   dfail:0   fail:0   skip:12 
fi-byt-j1900     total:246  pass:219  dwarn:0   dfail:0   fail:0   skip:27 
fi-byt-n2820     total:246  pass:215  dwarn:0   dfail:0   fail:0   skip:31 
fi-hsw-4770      total:246  pass:227  dwarn:0   dfail:0   fail:0   skip:19 
fi-hsw-4770r     total:246  pass:227  dwarn:0   dfail:0   fail:0   skip:19 
fi-ivb-3520m     total:246  pass:225  dwarn:0   dfail:0   fail:0   skip:21 
fi-ivb-3770      total:246  pass:225  dwarn:0   dfail:0   fail:0   skip:21 
fi-kbl-7500u     total:246  pass:225  dwarn:0   dfail:0   fail:0   skip:21 
fi-skl-6260u     total:246  pass:233  dwarn:0   dfail:0   fail:0   skip:13 
fi-skl-6700hq    total:246  pass:226  dwarn:0   dfail:0   fail:0   skip:20 
fi-skl-6700k     total:246  pass:222  dwarn:3   dfail:0   fail:0   skip:21 
fi-skl-6770hq    total:246  pass:233  dwarn:0   dfail:0   fail:0   skip:13 
fi-snb-2520m     total:246  pass:215  dwarn:0   dfail:0   fail:0   skip:31 
fi-snb-2600      total:246  pass:214  dwarn:0   dfail:0   fail:0   skip:32 

ae1fc35150f6aaadf190a8a881adc9aeb78fd7c9 drm-tip: 2017y-01m-13d-00h-42m-24s UTC integration manifest
4bccfcb drm/i915: Remove unused function intel_ddi_get_link_dpll()

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3510/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Remove unused function intel_ddi_get_link_dpll()

[Ander Conselvan De Oliveira]

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?

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@intel.com>
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> Cc: Jim Bride <jim.bride@linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: intel-gfx@lists.freedesktop.org
> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.
> com>
> 
> --
> 
> I omitted "Cc: <stable@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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Remove unused function intel_ddi_get_link_dpll()

[Jani Nikula]

On Thu, 09 Feb 2017, Ander Conselvan De Oliveira <conselvan2@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@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@intel.com>
>> Cc: Manasi Navare <manasi.d.navare@intel.com>
>> Cc: Jim Bride <jim.bride@linux.intel.com>
>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> Cc: Daniel Vetter <daniel.vetter@intel.com>
>> Cc: Jani Nikula <jani.nikula@linux.intel.com>
>> Cc: intel-gfx@lists.freedesktop.org
>> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.
>> com>
>> 
>> --
>> 
>> I omitted "Cc: <stable@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@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Remove unused function intel_ddi_get_link_dpll()

[Ander Conselvan De Oliveira]

On Thu, 2017-02-09 at 16:47 +0200, Jani Nikula wrote:
> On Thu, 09 Feb 2017, Ander Conselvan De Oliveira <conselvan2@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@intel.com>

Thanks. Patch pushed.

Ander

> 
> 
> > 
> > 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@intel.com>
> > > Cc: Manasi Navare <manasi.d.navare@intel.com>
> > > Cc: Jim Bride <jim.bride@linux.intel.com>
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Cc: Daniel Vetter <daniel.vetter@intel.com>
> > > Cc: Jani Nikula <jani.nikula@linux.intel.com>
> > > Cc: intel-gfx@lists.freedesktop.org
> > > Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@in
> > > tel.
> > > com>
> > > 
> > > --
> > > 
> > > I omitted "Cc: <stable@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@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx