[Intel-gfx] [PATCH 1/7] drm/i915: Introduce intel_release_shared_dpll()

[Ander Conselvan de Oliveira]

While the details of getting a shared dpll are wrapped by
intel_get_shared_dpll(), the release was still hand rolled into the
modeset code. Fix that by creating an entry point for releasing the
pll and move that code there.

v2: Take old_dpll from crtc->state instead of crtc_state. (CI)
Signed-off-by: Ander Conselvan de Oliveira 

Reviewed-by: Daniel Vetter 
---
 drivers/gpu/drm/i915/intel_display.c  |  6 +
 drivers/gpu/drm/i915/intel_dpll_mgr.c | 41 +++
 drivers/gpu/drm/i915/intel_dpll_mgr.h | 11 +++---
 3 files changed, 22 insertions(+), 36 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index d8effd4..55e1e88 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13874,7 +13874,6 @@ static void intel_modeset_clear_plls(struct 
drm_atomic_state *state)
 {
struct drm_device *dev = state->dev;
struct drm_i915_private *dev_priv = to_i915(dev);
-   struct intel_shared_dpll_config *shared_dpll = NULL;
struct drm_crtc *crtc;
struct drm_crtc_state *crtc_state;
int i;
@@ -13895,10 +13894,7 @@ static void intel_modeset_clear_plls(struct 
drm_atomic_state *state)
if (!old_dpll)
continue;
 
-   if (!shared_dpll)
-   shared_dpll = intel_atomic_get_shared_dpll_state(state);
-
-   intel_shared_dpll_config_put(shared_dpll, old_dpll, intel_crtc);
+   intel_release_shared_dpll(old_dpll, intel_crtc, state);
}
 }
 
diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c 
b/drivers/gpu/drm/i915/intel_dpll_mgr.c
index 97f7cc9..1fa32f2 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
@@ -79,28 +79,6 @@ intel_get_shared_dpll_id(struct drm_i915_private *dev_priv,
return (enum intel_dpll_id) (pll - dev_priv->shared_dplls);
 }
 
-void
-intel_shared_dpll_config_get(struct intel_shared_dpll_config *config,
-struct intel_shared_dpll *pll,
-struct intel_crtc *crtc)
-{
-   struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
-   enum intel_dpll_id id = intel_get_shared_dpll_id(dev_priv, pll);
-
-   config[id].crtc_mask |= 1 << crtc->pipe;
-}
-
-void
-intel_shared_dpll_config_put(struct intel_shared_dpll_config *config,
-struct intel_shared_dpll *pll,
-struct intel_crtc *crtc)
-{
-   struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
-   enum intel_dpll_id id = intel_get_shared_dpll_id(dev_priv, pll);
-
-   config[id].crtc_mask &= ~(1 << crtc->pipe);
-}
-
 /* For ILK+ */
 void assert_shared_dpll(struct drm_i915_private *dev_priv,
struct intel_shared_dpll *pll,
@@ -284,7 +262,7 @@ intel_reference_shared_dpll(struct intel_shared_dpll *pll,
DRM_DEBUG_DRIVER("using %s for pipe %c\n", pll->name,
 pipe_name(crtc->pipe));
 
-   intel_shared_dpll_config_get(shared_dpll, pll, crtc);
+   shared_dpll[pll->id].crtc_mask |= 1 << crtc->pipe;
 }
 
 void intel_shared_dpll_commit(struct drm_atomic_state *state)
@@ -1933,3 +1911,20 @@ intel_get_shared_dpll(struct intel_crtc *crtc,
 
return dpll_mgr->get_dpll(crtc, crtc_state, encoder);
 }
+
+/**
+ * intel_release_shared_dpll - end use of DPLL by CRTC in atomic state
+ * @dpll: dpll in use by @crtc
+ * @crtc: crtc
+ * @state: atomic state
+ *
+ */
+void intel_release_shared_dpll(struct intel_shared_dpll *dpll,
+  struct intel_crtc *crtc,
+  struct drm_atomic_state *state)
+{
+   struct intel_shared_dpll_config *shared_dpll_config;
+
+   shared_dpll_config = intel_atomic_get_shared_dpll_state(state);
+   shared_dpll_config[dpll->id].crtc_mask &= ~(1 << crtc->pipe);
+}
diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.h 
b/drivers/gpu/drm/i915/intel_dpll_mgr.h
index f438535..99a82c9 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.h
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.h
@@ -138,14 +138,6 @@ intel_get_shared_dpll_by_id(struct drm_i915_private 
*dev_priv,
 enum intel_dpll_id
 intel_get_shared_dpll_id(struct drm_i915_private *dev_priv,
 struct intel_shared_dpll *pll);
-void
-intel_shared_dpll_config_get(struct intel_shared_dpll_config *config,
-struct intel_shared_dpll *pll,
-struct intel_crtc *crtc);
-void
-intel_shared_dpll_config_put(struct intel_shared_dpll_config *config,
-struct intel_shared_dpll *pll,
-struct intel_crtc *crtc);
 void assert_shared_dpll(struct drm_i915_private *dev_priv,
struct intel_shared_dpll *pll,
bool state);
@@ 

Re: [Intel-gfx] [PATCH 1/7] drm/i915: Introduce intel_release_shared_dpll()

[Daniel Vetter]

On Tue, Oct 04, 2016 at 03:32:11PM +0300, Ander Conselvan de Oliveira wrote:
> While the details of getting a shared dpll are wrapped by
> intel_get_shared_dpll(), the release was still hand rolled into the
> modeset code. Fix that by creating an entry point for releasing the
> pll and move that code there.
> 
> v2: Take old_dpll from crtc->state instead of crtc_state. (CI)
> Signed-off-by: Ander Conselvan de Oliveira 
> 
> ---
>  drivers/gpu/drm/i915/intel_display.c  |  6 +
>  drivers/gpu/drm/i915/intel_dpll_mgr.c | 41 
> +++
>  drivers/gpu/drm/i915/intel_dpll_mgr.h | 11 +++---
>  3 files changed, 22 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index a366656..28d9d3e 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13768,7 +13768,6 @@ static void intel_modeset_clear_plls(struct 
> drm_atomic_state *state)
>  {
>   struct drm_device *dev = state->dev;
>   struct drm_i915_private *dev_priv = to_i915(dev);
> - struct intel_shared_dpll_config *shared_dpll = NULL;
>   struct drm_crtc *crtc;
>   struct drm_crtc_state *crtc_state;
>   int i;
> @@ -13789,10 +13788,7 @@ static void intel_modeset_clear_plls(struct 
> drm_atomic_state *state)
>   if (!old_dpll)
>   continue;
>  
> - if (!shared_dpll)
> - shared_dpll = intel_atomic_get_shared_dpll_state(state);
> -
> - intel_shared_dpll_config_put(shared_dpll, old_dpll, intel_crtc);
> + intel_release_shared_dpll(old_dpll, intel_crtc, state);
>   }
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c 
> b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> index 1c59ca5..f1b3feb 100644
> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> @@ -79,28 +79,6 @@ intel_get_shared_dpll_id(struct drm_i915_private *dev_priv,
>   return (enum intel_dpll_id) (pll - dev_priv->shared_dplls);
>  }
>  
> -void
> -intel_shared_dpll_config_get(struct intel_shared_dpll_config *config,
> -  struct intel_shared_dpll *pll,
> -  struct intel_crtc *crtc)
> -{
> - struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> - enum intel_dpll_id id = intel_get_shared_dpll_id(dev_priv, pll);
> -
> - config[id].crtc_mask |= 1 << crtc->pipe;
> -}
> -
> -void
> -intel_shared_dpll_config_put(struct intel_shared_dpll_config *config,
> -  struct intel_shared_dpll *pll,
> -  struct intel_crtc *crtc)
> -{
> - struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> - enum intel_dpll_id id = intel_get_shared_dpll_id(dev_priv, pll);
> -
> - config[id].crtc_mask &= ~(1 << crtc->pipe);
> -}
> -
>  /* For ILK+ */
>  void assert_shared_dpll(struct drm_i915_private *dev_priv,
>   struct intel_shared_dpll *pll,
> @@ -285,7 +263,7 @@ intel_reference_shared_dpll(struct intel_shared_dpll *pll,
>   DRM_DEBUG_DRIVER("using %s for pipe %c\n", pll->name,
>pipe_name(crtc->pipe));
>  
> - intel_shared_dpll_config_get(shared_dpll, pll, crtc);
> + shared_dpll[pll->id].crtc_mask |= 1 << crtc->pipe;
>  }
>  
>  void intel_shared_dpll_commit(struct drm_atomic_state *state)
> @@ -1900,3 +1878,20 @@ intel_get_shared_dpll(struct intel_crtc *crtc,
>  
>   return dpll_mgr->get_dpll(crtc, crtc_state, encoder);
>  }
> +
> +/**
> + * intel_release_shared_dpll - end use of DPLL by CRTC in atomic state
> + * @dpll: dpll in use by @crtc
> + * @crtc: crtc
> + * @state: atomic state
> + *
> + */
> +void intel_release_shared_dpll(struct intel_shared_dpll *dpll,
> +struct intel_crtc *crtc,
> +struct drm_atomic_state *state)
> +{
> + struct intel_shared_dpll_config *shared_dpll_config;
> +
> + shared_dpll_config = intel_atomic_get_shared_dpll_state(state);

I wonder whether we shouldn't move intel_atomic_get_shared_dpll_state into
intel_dpll_mgr.c too. That would further reduce the interfaces exported
from intel_dpll_mgr.[hc].

On this patch: Reviewed-by: Daniel Vetter 


> + shared_dpll_config[dpll->id].crtc_mask &= ~(1 << crtc->pipe);
> +}
> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.h 
> b/drivers/gpu/drm/i915/intel_dpll_mgr.h
> index f438535..99a82c9 100644
> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.h
> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.h
> @@ -138,14 +138,6 @@ intel_get_shared_dpll_by_id(struct drm_i915_private 
> *dev_priv,
>  enum intel_dpll_id
>  intel_get_shared_dpll_id(struct drm_i915_private *dev_priv,
>struct intel_shared_dpll *pll);
> -void
> -intel_shared_dpll_config_get(struct intel_shared_dpll_config *config,
> -  

[Intel-gfx] [PATCH 1/7] drm/i915: Introduce intel_release_shared_dpll()

[Ander Conselvan de Oliveira]

While the details of getting a shared dpll are wrapped by
intel_get_shared_dpll(), the release was still hand rolled into the
modeset code. Fix that by creating an entry point for releasing the
pll and move that code there.

v2: Take old_dpll from crtc->state instead of crtc_state. (CI)
Signed-off-by: Ander Conselvan de Oliveira 

---
 drivers/gpu/drm/i915/intel_display.c  |  6 +
 drivers/gpu/drm/i915/intel_dpll_mgr.c | 41 +++
 drivers/gpu/drm/i915/intel_dpll_mgr.h | 11 +++---
 3 files changed, 22 insertions(+), 36 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index a366656..28d9d3e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13768,7 +13768,6 @@ static void intel_modeset_clear_plls(struct 
drm_atomic_state *state)
 {
struct drm_device *dev = state->dev;
struct drm_i915_private *dev_priv = to_i915(dev);
-   struct intel_shared_dpll_config *shared_dpll = NULL;
struct drm_crtc *crtc;
struct drm_crtc_state *crtc_state;
int i;
@@ -13789,10 +13788,7 @@ static void intel_modeset_clear_plls(struct 
drm_atomic_state *state)
if (!old_dpll)
continue;
 
-   if (!shared_dpll)
-   shared_dpll = intel_atomic_get_shared_dpll_state(state);
-
-   intel_shared_dpll_config_put(shared_dpll, old_dpll, intel_crtc);
+   intel_release_shared_dpll(old_dpll, intel_crtc, state);
}
 }
 
diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c 
b/drivers/gpu/drm/i915/intel_dpll_mgr.c
index 1c59ca5..f1b3feb 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
@@ -79,28 +79,6 @@ intel_get_shared_dpll_id(struct drm_i915_private *dev_priv,
return (enum intel_dpll_id) (pll - dev_priv->shared_dplls);
 }
 
-void
-intel_shared_dpll_config_get(struct intel_shared_dpll_config *config,
-struct intel_shared_dpll *pll,
-struct intel_crtc *crtc)
-{
-   struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
-   enum intel_dpll_id id = intel_get_shared_dpll_id(dev_priv, pll);
-
-   config[id].crtc_mask |= 1 << crtc->pipe;
-}
-
-void
-intel_shared_dpll_config_put(struct intel_shared_dpll_config *config,
-struct intel_shared_dpll *pll,
-struct intel_crtc *crtc)
-{
-   struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
-   enum intel_dpll_id id = intel_get_shared_dpll_id(dev_priv, pll);
-
-   config[id].crtc_mask &= ~(1 << crtc->pipe);
-}
-
 /* For ILK+ */
 void assert_shared_dpll(struct drm_i915_private *dev_priv,
struct intel_shared_dpll *pll,
@@ -285,7 +263,7 @@ intel_reference_shared_dpll(struct intel_shared_dpll *pll,
DRM_DEBUG_DRIVER("using %s for pipe %c\n", pll->name,
 pipe_name(crtc->pipe));
 
-   intel_shared_dpll_config_get(shared_dpll, pll, crtc);
+   shared_dpll[pll->id].crtc_mask |= 1 << crtc->pipe;
 }
 
 void intel_shared_dpll_commit(struct drm_atomic_state *state)
@@ -1900,3 +1878,20 @@ intel_get_shared_dpll(struct intel_crtc *crtc,
 
return dpll_mgr->get_dpll(crtc, crtc_state, encoder);
 }
+
+/**
+ * intel_release_shared_dpll - end use of DPLL by CRTC in atomic state
+ * @dpll: dpll in use by @crtc
+ * @crtc: crtc
+ * @state: atomic state
+ *
+ */
+void intel_release_shared_dpll(struct intel_shared_dpll *dpll,
+  struct intel_crtc *crtc,
+  struct drm_atomic_state *state)
+{
+   struct intel_shared_dpll_config *shared_dpll_config;
+
+   shared_dpll_config = intel_atomic_get_shared_dpll_state(state);
+   shared_dpll_config[dpll->id].crtc_mask &= ~(1 << crtc->pipe);
+}
diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.h 
b/drivers/gpu/drm/i915/intel_dpll_mgr.h
index f438535..99a82c9 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.h
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.h
@@ -138,14 +138,6 @@ intel_get_shared_dpll_by_id(struct drm_i915_private 
*dev_priv,
 enum intel_dpll_id
 intel_get_shared_dpll_id(struct drm_i915_private *dev_priv,
 struct intel_shared_dpll *pll);
-void
-intel_shared_dpll_config_get(struct intel_shared_dpll_config *config,
-struct intel_shared_dpll *pll,
-struct intel_crtc *crtc);
-void
-intel_shared_dpll_config_put(struct intel_shared_dpll_config *config,
-struct intel_shared_dpll *pll,
-struct intel_crtc *crtc);
 void assert_shared_dpll(struct drm_i915_private *dev_priv,
struct intel_shared_dpll *pll,
bool state);
@@ -154,6 +146,9 @@ void assert_shared_dpll(struct