Re: [PATCH v2] drm/i915: Set crtc->new_config to NULL for pipes that are about to be disabled

[Imre Deak <imre.deak@intel.com>]

[-- Attachment #1.1: Type: text/plain, Size: 3297 bytes --]

On Tue, 2014-01-14 at 14:31 +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> crtc->new_config is only relevant for pipes that are going to be active
> post-modeset. Set the pointer to NULL for all pipes that are going to
> be disabled. This is done to help catch bugs where some piece of code
> would go looking at crtc->new_config even if the data there is stale.
> 
> v2: Clear new_config in disable_crtc_nofb() too (Imre)
> 
> Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Imre Deak <imre.deak@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_display.c | 22 +++++++++++++++++++---
>  1 file changed, 19 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 673b02e..dde98020 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -8763,6 +8763,11 @@ static void intel_modeset_update_staged_output_state(struct drm_device *dev)
>  	list_for_each_entry(crtc, &dev->mode_config.crtc_list,
>  			    base.head) {
>  		crtc->new_enabled = crtc->base.enabled;
> +
> +		if (crtc->new_enabled)
> +			crtc->new_config = &crtc->config;
> +		else
> +			crtc->new_config = NULL;
>  	}
>  }
>  
> @@ -9195,7 +9200,9 @@ intel_modeset_update_state(struct drm_device *dev, unsigned prepare_pipes)
>  	list_for_each_entry(intel_crtc, &dev->mode_config.crtc_list,
>  			    base.head) {
>  		WARN_ON(intel_crtc->base.enabled != intel_crtc_in_use(&intel_crtc->base));
> -		WARN_ON(intel_crtc->new_config != &intel_crtc->config);
> +		WARN_ON(intel_crtc->new_config &&
> +			intel_crtc->new_config != &intel_crtc->config);
> +		WARN_ON(intel_crtc->base.enabled != !!intel_crtc->new_config);
>  	}
>  
>  	list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
> @@ -9801,6 +9808,11 @@ static void intel_set_config_restore_state(struct drm_device *dev,
>  	count = 0;
>  	list_for_each_entry(crtc, &dev->mode_config.crtc_list, base.head) {
>  		crtc->new_enabled = config->save_crtc_enabled[count++];
> +
> +		if (crtc->new_enabled)
> +			crtc->new_config = &crtc->config;
> +		else
> +			crtc->new_config = NULL;
>  	}
>  
>  	count = 0;
> @@ -10002,6 +10014,11 @@ intel_modeset_stage_output_state(struct drm_device *dev,
>  				      crtc->new_enabled ? "en" : "dis");
>  			config->mode_changed = true;
>  		}
> +
> +		if (crtc->new_enabled)
> +			crtc->new_config = &crtc->config;
> +		else
> +			crtc->new_config = NULL;
>  	}
>  
>  	return 0;
> @@ -10028,6 +10045,7 @@ static void disable_crtc_nofb(struct intel_crtc *crtc)
>  	}
>  
>  	crtc->new_enabled = false;
> +	crtc->new_config = NULL;
>  }
>  
>  static int intel_crtc_set_config(struct drm_mode_set *set)
> @@ -10272,8 +10290,6 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
>  	dev_priv->plane_to_crtc_mapping[intel_crtc->plane] = &intel_crtc->base;
>  	dev_priv->pipe_to_crtc_mapping[intel_crtc->pipe] = &intel_crtc->base;
>  
> -	intel_crtc->new_config = &intel_crtc->config;
> -
>  	drm_crtc_helper_add(&intel_crtc->base, &intel_helper_funcs);
>  }
>  


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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