[2/2] drm/i915/icl: Clear the shared port PLLs from the new crtc state

Submitted by Imre Deak on July 5, 2019, 2:04 p.m.

Details

Message ID 20190705140423.28404-2-imre.deak@intel.com
State New
Headers show
Series "Series without cover letter" ( rev: 1 ) in Intel GFX

Not browsing as part of any series.

Commit Message

Imre Deak July 5, 2019, 2:04 p.m.
For consistency clear the icl_port_dplls from the new crtc state, when
releasing the DPLLs from the old crtc state. Leaving them set could
result in releasing the same PLLs multiple times from the same CRTC
state incorrectly (if the same CRTC was first used for a TypeC port then
for a combo PHY port).

Leaving the stale pointers behind happens not to cause a problem atm
(since the incorrect releasing will be a NOP), but we need to fix that
for consistency.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
index 722b1deca999..f4db2410ca93 100644
--- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
@@ -3021,15 +3021,17 @@  static void icl_put_dplls(struct intel_atomic_state *state,
 	new_crtc_state->shared_dpll = NULL;
 
 	for (id = ICL_PORT_DPLL_DEFAULT; id < ICL_PORT_DPLL_COUNT; id++) {
-		struct icl_port_dpll *port_dpll =
+		struct icl_port_dpll *old_port_dpll =
 			&old_crtc_state->icl_port_dplls[id];
+		struct icl_port_dpll *new_port_dpll =
+			&new_crtc_state->icl_port_dplls[id];
 
-		if (!port_dpll->pll)
-			continue;
+		new_port_dpll->pll = NULL;
 
-		intel_unreference_shared_dpll(state, crtc, port_dpll->pll);
+		if (!old_port_dpll->pll)
+			continue;
 
-		/* FIXME: Clear the icl_port_dplls from the new crtc state */
+		intel_unreference_shared_dpll(state, crtc, old_port_dpll->pll);
 	}
 }
 

Comments

On Fri, Jul 05, 2019 at 05:04:23PM +0300, Imre Deak wrote:
> For consistency clear the icl_port_dplls from the new crtc state, when
> releasing the DPLLs from the old crtc state. Leaving them set could
> result in releasing the same PLLs multiple times from the same CRTC
> state incorrectly (if the same CRTC was first used for a TypeC port then
> for a combo PHY port).
> 
> Leaving the stale pointers behind happens not to cause a problem atm
> (since the incorrect releasing will be a NOP), but we need to fix that
> for consistency.
> 
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> index 722b1deca999..f4db2410ca93 100644
> --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> @@ -3021,15 +3021,17 @@ static void icl_put_dplls(struct intel_atomic_state *state,
>  	new_crtc_state->shared_dpll = NULL;
>  
>  	for (id = ICL_PORT_DPLL_DEFAULT; id < ICL_PORT_DPLL_COUNT; id++) {
> -		struct icl_port_dpll *port_dpll =
> +		struct icl_port_dpll *old_port_dpll =
>  			&old_crtc_state->icl_port_dplls[id];
> +		struct icl_port_dpll *new_port_dpll =
> +			&new_crtc_state->icl_port_dplls[id];

I would constify the old states in both patches.

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

>  
> -		if (!port_dpll->pll)
> -			continue;
> +		new_port_dpll->pll = NULL;
>  
> -		intel_unreference_shared_dpll(state, crtc, port_dpll->pll);
> +		if (!old_port_dpll->pll)
> +			continue;
>  
> -		/* FIXME: Clear the icl_port_dplls from the new crtc state */
> +		intel_unreference_shared_dpll(state, crtc, old_port_dpll->pll);
>  	}
>  }
>  
> -- 
> 2.17.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx