public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Set crtc->new_config to NULL for pipes that are about to be disabled
@ 2014-01-14 11:27 ville.syrjala
  2014-01-14 12:31 ` [PATCH v2] " ville.syrjala
  0 siblings, 1 reply; 3+ messages in thread
From: ville.syrjala @ 2014-01-14 11:27 UTC (permalink / raw)
  To: intel-gfx

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.

Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 673b02e..5b5b46e 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;
@@ -10272,8 +10289,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);
 }
 
-- 
1.8.3.2

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

^ permalink raw reply related	[flat|nested] 3+ messages in thread
* [PATCH v2] drm/i915: Set crtc->new_config to NULL for pipes that are about to be disabled
  2014-01-14 11:27 [PATCH] drm/i915: Set crtc->new_config to NULL for pipes that are about to be disabled ville.syrjala
@ 2014-01-14 12:31 ` ville.syrjala
  2014-01-14 12:35   ` Imre Deak
  0 siblings, 1 reply; 3+ messages in thread
From: ville.syrjala @ 2014-01-14 12:31 UTC (permalink / raw)
  To: intel-gfx

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>
---
 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);
 }
 
-- 
1.8.3.2

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

^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH v2] drm/i915: Set crtc->new_config to NULL for pipes that are about to be disabled
  2014-01-14 12:31 ` [PATCH v2] " ville.syrjala
@ 2014-01-14 12:35   ` Imre Deak
  0 siblings, 0 replies; 3+ messages in thread
From: Imre Deak @ 2014-01-14 12:35 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx


[-- 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

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2014-01-14 12:35 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-01-14 11:27 [PATCH] drm/i915: Set crtc->new_config to NULL for pipes that are about to be disabled ville.syrjala
2014-01-14 12:31 ` [PATCH v2] " ville.syrjala
2014-01-14 12:35   ` Imre Deak

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox