[1/3] drm/i915: Clear shared dpll based on old state.

Submitted by Maarten Lankhorst on Feb. 9, 2016, 12:52 p.m.

Details

Message ID 1455022343-15222-1-git-send-email-maarten.lankhorst@linux.intel.com
State New
Headers show
Series "Use more atomic state in i915." ( rev: 3 ) in Intel GFX

Not browsing as part of any series.

Commit Message

Maarten Lankhorst Feb. 9, 2016, 12:52 p.m.
Atomic resume was preserving the dpll state because it was required
for clearing pll state correctly. If we look at the old_crtc_state
for pll to clear this is not needed and the hack can be removed.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 19 +++++++------------
 1 file changed, 7 insertions(+), 12 deletions(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 3b6bd6e9f7ae..e496c130364d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13093,8 +13093,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 intel_crtc *intel_crtc;
-	struct intel_crtc_state *intel_crtc_state;
 	struct drm_crtc *crtc;
 	struct drm_crtc_state *crtc_state;
 	int i;
@@ -13103,16 +13101,16 @@  static void intel_modeset_clear_plls(struct drm_atomic_state *state)
 		return;
 
 	for_each_crtc_in_state(state, crtc, crtc_state, i) {
-		int dpll;
-
-		intel_crtc = to_intel_crtc(crtc);
-		intel_crtc_state = to_intel_crtc_state(crtc_state);
-		dpll = intel_crtc_state->shared_dpll;
+		struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+		int dpll = to_intel_crtc_state(crtc->state)->shared_dpll;
 
-		if (!needs_modeset(crtc_state) || dpll == DPLL_ID_PRIVATE)
+		if (!needs_modeset(crtc_state))
 			continue;
 
-		intel_crtc_state->shared_dpll = DPLL_ID_PRIVATE;
+		to_intel_crtc_state(crtc_state)->shared_dpll = DPLL_ID_PRIVATE;
+
+		if (dpll == DPLL_ID_PRIVATE)
+			continue;
 
 		if (!shared_dpll)
 			shared_dpll = intel_atomic_get_shared_dpll_state(state);
@@ -15922,9 +15920,6 @@  void intel_display_resume(struct drm_device *dev)
 
 	state->acquire_ctx = dev->mode_config.acquire_ctx;
 
-	/* preserve complete old state, including dpll */
-	intel_atomic_get_shared_dpll_state(state);
-
 	for_each_crtc(dev, crtc) {
 		struct drm_crtc_state *crtc_state =
 			drm_atomic_get_crtc_state(state, crtc);

Comments

On Tue, Feb 09, 2016 at 01:52:21PM +0100, Maarten Lankhorst wrote:
> Atomic resume was preserving the dpll state because it was required
> for clearing pll state correctly. If we look at the old_crtc_state
> for pll to clear this is not needed and the hack can be removed.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 19 +++++++------------
>  1 file changed, 7 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 3b6bd6e9f7ae..e496c130364d 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13093,8 +13093,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 intel_crtc *intel_crtc;
> -	struct intel_crtc_state *intel_crtc_state;
>  	struct drm_crtc *crtc;
>  	struct drm_crtc_state *crtc_state;
>  	int i;
> @@ -13103,16 +13101,16 @@ static void intel_modeset_clear_plls(struct drm_atomic_state *state)
>  		return;
>  
>  	for_each_crtc_in_state(state, crtc, crtc_state, i) {
> -		int dpll;
> -
> -		intel_crtc = to_intel_crtc(crtc);
> -		intel_crtc_state = to_intel_crtc_state(crtc_state);
> -		dpll = intel_crtc_state->shared_dpll;
> +		struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +		int dpll = to_intel_crtc_state(crtc->state)->shared_dpll;
>  
> -		if (!needs_modeset(crtc_state) || dpll == DPLL_ID_PRIVATE)
> +		if (!needs_modeset(crtc_state))
>  			continue;
>  
> -		intel_crtc_state->shared_dpll = DPLL_ID_PRIVATE;
> +		to_intel_crtc_state(crtc_state)->shared_dpll = DPLL_ID_PRIVATE;
> +
> +		if (dpll == DPLL_ID_PRIVATE)
> +			continue;

Maybe this should be called old_dpll or something for a bit of extra
clarity? Anyway this makes sense to me so
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

>  
>  		if (!shared_dpll)
>  			shared_dpll = intel_atomic_get_shared_dpll_state(state);
> @@ -15922,9 +15920,6 @@ void intel_display_resume(struct drm_device *dev)
>  
>  	state->acquire_ctx = dev->mode_config.acquire_ctx;
>  
> -	/* preserve complete old state, including dpll */
> -	intel_atomic_get_shared_dpll_state(state);
> -
>  	for_each_crtc(dev, crtc) {
>  		struct drm_crtc_state *crtc_state =
>  			drm_atomic_get_crtc_state(state, crtc);
> -- 
> 2.1.0