diff mbox series

[08/13] drm/i915: Make shared dpll functions take crtc_state

Message ID 20181003133715.3713-9-maarten.lankhorst@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915: First cleanup pass to get rid of more crtc->config users. | expand

Commit Message

Maarten Lankhorst Oct. 3, 2018, 1:37 p.m. UTC 
Do not rely on crtc->config any more. Remove the assertion from
ibx_pch_dpll_disable, because we the dpll state tracking should
already handle this case correctly.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c  | 14 ++++++------
 drivers/gpu/drm/i915/intel_dpll_mgr.c | 31 +++++++++++----------------
 drivers/gpu/drm/i915/intel_dpll_mgr.h |  6 +++---
 3 files changed, 22 insertions(+), 29 deletions(-)

Comments

Ville Syrjälä Oct. 3, 2018, 2:15 p.m. UTC | #1 
On Wed, Oct 03, 2018 at 03:37:10PM +0200, Maarten Lankhorst wrote:
> Do not rely on crtc->config any more. Remove the assertion from
> ibx_pch_dpll_disable, because we the dpll state tracking should
> already handle this case correctly.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c  | 14 ++++++------
>  drivers/gpu/drm/i915/intel_dpll_mgr.c | 31 +++++++++++----------------
>  drivers/gpu/drm/i915/intel_dpll_mgr.h |  6 +++---
>  3 files changed, 22 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 57c81e8e44be..22f556b824f9 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4749,7 +4749,7 @@ static void ironlake_pch_enable(const struct intel_atomic_state *state,
>  	 * Note that enable_shared_dpll tries to do the right thing, but
>  	 * get_shared_dpll unconditionally resets the pll - we need that to have
>  	 * the right LVDS enable sequence. */
> -	intel_enable_shared_dpll(crtc);
> +	intel_enable_shared_dpll(crtc_state);
>  
>  	/* set transcoder timing, panel must allow it */
>  	assert_panel_unlocked(dev_priv, pipe);
> @@ -5591,8 +5591,8 @@ static void ironlake_crtc_enable(struct intel_crtc_state *pipe_config,
>  	intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, false);
>  	intel_set_pch_fifo_underrun_reporting(dev_priv, pipe, false);
>  
> -	if (intel_crtc->config->has_pch_encoder)
> -		intel_prepare_shared_dpll(intel_crtc);
> +	if (pipe_config->has_pch_encoder)
> +		intel_prepare_shared_dpll(pipe_config);
>  
>  	if (intel_crtc_has_dp_encoder(intel_crtc->config))
>  		intel_dp_set_m_n(intel_crtc, M1_N1);
> @@ -5710,8 +5710,8 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
>  
>  	intel_encoders_pre_pll_enable(crtc, pipe_config, old_state);
>  
> -	if (intel_crtc->config->shared_dpll)
> -		intel_enable_shared_dpll(intel_crtc);
> +	if (pipe_config->shared_dpll)
> +		intel_enable_shared_dpll(pipe_config);
>  
>  	if (INTEL_GEN(dev_priv) >= 11)
>  		icl_map_plls_to_ports(crtc, pipe_config, old_state);
> @@ -6286,7 +6286,7 @@ static void intel_crtc_disable_noatomic(struct drm_crtc *crtc,
>  
>  	intel_fbc_disable(intel_crtc);
>  	intel_update_watermarks(intel_crtc);
> -	intel_disable_shared_dpll(intel_crtc);
> +	intel_disable_shared_dpll(crtc_state);
>  
>  	domains = intel_crtc->enabled_power_domains;
>  	for_each_power_domain(domain, domains)
> @@ -12735,7 +12735,7 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state)
>  			dev_priv->display.crtc_disable(old_intel_crtc_state, state);
>  			intel_crtc->active = false;
>  			intel_fbc_disable(intel_crtc);
> -			intel_disable_shared_dpll(intel_crtc);
> +			intel_disable_shared_dpll(old_intel_crtc_state);
>  
>  			/*
>  			 * Underruns don't always raise
> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> index e6cac9225536..54662c974c0b 100644
> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> @@ -131,13 +131,13 @@ void assert_shared_dpll(struct drm_i915_private *dev_priv,
>   * This calls the PLL's prepare hook if it has one and if the PLL is not
>   * already enabled. The prepare hook is platform specific.
>   */
> -void intel_prepare_shared_dpll(struct intel_crtc *crtc)
> +void intel_prepare_shared_dpll(const struct intel_crtc_state *crtc_state)
>  {
> -	struct drm_device *dev = crtc->base.dev;
> -	struct drm_i915_private *dev_priv = to_i915(dev);
> -	struct intel_shared_dpll *pll = crtc->config->shared_dpll;
> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> +	struct intel_shared_dpll *pll = crtc_state->shared_dpll;
>  
> -	if (WARN_ON(pll == NULL))
> +	if (!WARN_ON(!pll))
>  		return;
>  
>  	mutex_lock(&dev_priv->dpll_lock);
> @@ -158,11 +158,11 @@ void intel_prepare_shared_dpll(struct intel_crtc *crtc)
>   *
>   * Enable the shared DPLL used by @crtc.
>   */
> -void intel_enable_shared_dpll(struct intel_crtc *crtc)
> +void intel_enable_shared_dpll(const struct intel_crtc_state *crtc_state)
>  {
> -	struct drm_device *dev = crtc->base.dev;
> -	struct drm_i915_private *dev_priv = to_i915(dev);
> -	struct intel_shared_dpll *pll = crtc->config->shared_dpll;
> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> +	struct intel_shared_dpll *pll = crtc_state->shared_dpll;
>  	unsigned int crtc_mask = drm_crtc_mask(&crtc->base);
>  	unsigned int old_mask;
>  
> @@ -203,10 +203,11 @@ void intel_enable_shared_dpll(struct intel_crtc *crtc)
>   *
>   * Disable the shared DPLL used by @crtc.
>   */
> -void intel_disable_shared_dpll(struct intel_crtc *crtc)
> +void intel_disable_shared_dpll(const struct intel_crtc_state *crtc_state)
>  {
> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> -	struct intel_shared_dpll *pll = crtc->config->shared_dpll;
> +	struct intel_shared_dpll *pll = crtc_state->shared_dpll;
>  	unsigned int crtc_mask = drm_crtc_mask(&crtc->base);
>  
>  	/* PCH only available on ILK+ */
> @@ -409,14 +410,6 @@ static void ibx_pch_dpll_disable(struct drm_i915_private *dev_priv,
>  				 struct intel_shared_dpll *pll)
>  {
>  	const enum intel_dpll_id id = pll->info->id;
> -	struct drm_device *dev = &dev_priv->drm;
> -	struct intel_crtc *crtc;
> -
> -	/* Make sure no transcoder isn't still depending on us. */
> -	for_each_intel_crtc(dev, crtc) {
> -		if (crtc->config->shared_dpll == pll)
> -			assert_pch_transcoder_disabled(dev_priv, crtc->pipe);
> -	}

Yeah I guess this can go. If we really wanted to keep it we should
probably make it not depend on any software state and just read out
everything from the hw.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

>  
>  	I915_WRITE(PCH_DPLL(id), 0);
>  	POSTING_READ(PCH_DPLL(id));
> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.h b/drivers/gpu/drm/i915/intel_dpll_mgr.h
> index bf0de8a4dc63..9c033236f2ba 100644
> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.h
> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.h
> @@ -334,9 +334,9 @@ struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc,
>  void intel_release_shared_dpll(struct intel_shared_dpll *dpll,
>  			       struct intel_crtc *crtc,
>  			       struct drm_atomic_state *state);
> -void intel_prepare_shared_dpll(struct intel_crtc *crtc);
> -void intel_enable_shared_dpll(struct intel_crtc *crtc);
> -void intel_disable_shared_dpll(struct intel_crtc *crtc);
> +void intel_prepare_shared_dpll(const struct intel_crtc_state *crtc_state);
> +void intel_enable_shared_dpll(const struct intel_crtc_state *crtc_state);
> +void intel_disable_shared_dpll(const struct intel_crtc_state *crtc_state);
>  void intel_shared_dpll_swap_state(struct drm_atomic_state *state);
>  void intel_shared_dpll_init(struct drm_device *dev);
>  
> -- 
> 2.19.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 57c81e8e44be..22f556b824f9 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4749,7 +4749,7 @@  static void ironlake_pch_enable(const struct intel_atomic_state *state,
 	 * Note that enable_shared_dpll tries to do the right thing, but
 	 * get_shared_dpll unconditionally resets the pll - we need that to have
 	 * the right LVDS enable sequence. */
-	intel_enable_shared_dpll(crtc);
+	intel_enable_shared_dpll(crtc_state);
 
 	/* set transcoder timing, panel must allow it */
 	assert_panel_unlocked(dev_priv, pipe);
@@ -5591,8 +5591,8 @@  static void ironlake_crtc_enable(struct intel_crtc_state *pipe_config,
 	intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, false);
 	intel_set_pch_fifo_underrun_reporting(dev_priv, pipe, false);
 
-	if (intel_crtc->config->has_pch_encoder)
-		intel_prepare_shared_dpll(intel_crtc);
+	if (pipe_config->has_pch_encoder)
+		intel_prepare_shared_dpll(pipe_config);
 
 	if (intel_crtc_has_dp_encoder(intel_crtc->config))
 		intel_dp_set_m_n(intel_crtc, M1_N1);
@@ -5710,8 +5710,8 @@  static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
 
 	intel_encoders_pre_pll_enable(crtc, pipe_config, old_state);
 
-	if (intel_crtc->config->shared_dpll)
-		intel_enable_shared_dpll(intel_crtc);
+	if (pipe_config->shared_dpll)
+		intel_enable_shared_dpll(pipe_config);
 
 	if (INTEL_GEN(dev_priv) >= 11)
 		icl_map_plls_to_ports(crtc, pipe_config, old_state);
@@ -6286,7 +6286,7 @@  static void intel_crtc_disable_noatomic(struct drm_crtc *crtc,
 
 	intel_fbc_disable(intel_crtc);
 	intel_update_watermarks(intel_crtc);
-	intel_disable_shared_dpll(intel_crtc);
+	intel_disable_shared_dpll(crtc_state);
 
 	domains = intel_crtc->enabled_power_domains;
 	for_each_power_domain(domain, domains)
@@ -12735,7 +12735,7 @@  static void intel_atomic_commit_tail(struct drm_atomic_state *state)
 			dev_priv->display.crtc_disable(old_intel_crtc_state, state);
 			intel_crtc->active = false;
 			intel_fbc_disable(intel_crtc);
-			intel_disable_shared_dpll(intel_crtc);
+			intel_disable_shared_dpll(old_intel_crtc_state);
 
 			/*
 			 * Underruns don't always raise
diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
index e6cac9225536..54662c974c0b 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
@@ -131,13 +131,13 @@  void assert_shared_dpll(struct drm_i915_private *dev_priv,
  * This calls the PLL's prepare hook if it has one and if the PLL is not
  * already enabled. The prepare hook is platform specific.
  */
-void intel_prepare_shared_dpll(struct intel_crtc *crtc)
+void intel_prepare_shared_dpll(const struct intel_crtc_state *crtc_state)
 {
-	struct drm_device *dev = crtc->base.dev;
-	struct drm_i915_private *dev_priv = to_i915(dev);
-	struct intel_shared_dpll *pll = crtc->config->shared_dpll;
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	struct intel_shared_dpll *pll = crtc_state->shared_dpll;
 
-	if (WARN_ON(pll == NULL))
+	if (!WARN_ON(!pll))
 		return;
 
 	mutex_lock(&dev_priv->dpll_lock);
@@ -158,11 +158,11 @@  void intel_prepare_shared_dpll(struct intel_crtc *crtc)
  *
  * Enable the shared DPLL used by @crtc.
  */
-void intel_enable_shared_dpll(struct intel_crtc *crtc)
+void intel_enable_shared_dpll(const struct intel_crtc_state *crtc_state)
 {
-	struct drm_device *dev = crtc->base.dev;
-	struct drm_i915_private *dev_priv = to_i915(dev);
-	struct intel_shared_dpll *pll = crtc->config->shared_dpll;
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	struct intel_shared_dpll *pll = crtc_state->shared_dpll;
 	unsigned int crtc_mask = drm_crtc_mask(&crtc->base);
 	unsigned int old_mask;
 
@@ -203,10 +203,11 @@  void intel_enable_shared_dpll(struct intel_crtc *crtc)
  *
  * Disable the shared DPLL used by @crtc.
  */
-void intel_disable_shared_dpll(struct intel_crtc *crtc)
+void intel_disable_shared_dpll(const struct intel_crtc_state *crtc_state)
 {
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
-	struct intel_shared_dpll *pll = crtc->config->shared_dpll;
+	struct intel_shared_dpll *pll = crtc_state->shared_dpll;
 	unsigned int crtc_mask = drm_crtc_mask(&crtc->base);
 
 	/* PCH only available on ILK+ */
@@ -409,14 +410,6 @@  static void ibx_pch_dpll_disable(struct drm_i915_private *dev_priv,
 				 struct intel_shared_dpll *pll)
 {
 	const enum intel_dpll_id id = pll->info->id;
-	struct drm_device *dev = &dev_priv->drm;
-	struct intel_crtc *crtc;
-
-	/* Make sure no transcoder isn't still depending on us. */
-	for_each_intel_crtc(dev, crtc) {
-		if (crtc->config->shared_dpll == pll)
-			assert_pch_transcoder_disabled(dev_priv, crtc->pipe);
-	}
 
 	I915_WRITE(PCH_DPLL(id), 0);
 	POSTING_READ(PCH_DPLL(id));
diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.h b/drivers/gpu/drm/i915/intel_dpll_mgr.h
index bf0de8a4dc63..9c033236f2ba 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.h
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.h
@@ -334,9 +334,9 @@  struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc,
 void intel_release_shared_dpll(struct intel_shared_dpll *dpll,
 			       struct intel_crtc *crtc,
 			       struct drm_atomic_state *state);
-void intel_prepare_shared_dpll(struct intel_crtc *crtc);
-void intel_enable_shared_dpll(struct intel_crtc *crtc);
-void intel_disable_shared_dpll(struct intel_crtc *crtc);
+void intel_prepare_shared_dpll(const struct intel_crtc_state *crtc_state);
+void intel_enable_shared_dpll(const struct intel_crtc_state *crtc_state);
+void intel_disable_shared_dpll(const struct intel_crtc_state *crtc_state);
 void intel_shared_dpll_swap_state(struct drm_atomic_state *state);
 void intel_shared_dpll_init(struct drm_device *dev);