[PATCH v2 1/3] drm/i915: Don't check modeset state in the hw state force restore path

[PATCH v2 1/3] drm/i915: Don't check modeset state in the hw state force restore path

[Ander Conselvan de Oliveira]

Since the force restore logic will restore the CRTCs state one at a
time, it is possible that the state will be inconsistent until the whole
operation finishes. A call to intel_modeset_check_state() is done once
it's over, so don't check the state multiple times in between. This
regression was introduced in:

commit 7f27126ea3db6ade886f18fd39caf0ff0cd1d37f
Author: Jesse Barnes <jbarnes@virtuousgeek.org>
Date:   Wed Nov 5 14:26:06 2014 -0800

    drm/i915: factor out compute_config from __intel_set_mode v3

v2: Rename check parameter to force_restore. (Matt)

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=94431
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 4e3f302..bcf1041 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -87,7 +87,8 @@ static void ironlake_pch_clock_get(struct intel_crtc *crtc,
 				   struct intel_crtc_state *pipe_config);
 
 static int intel_set_mode(struct drm_crtc *crtc,
-			  struct drm_atomic_state *state);
+			  struct drm_atomic_state *state,
+			  bool force_restore);
 static int intel_framebuffer_init(struct drm_device *dev,
 				  struct intel_framebuffer *ifb,
 				  struct drm_mode_fb_cmd2 *mode_cmd,
@@ -10096,7 +10097,7 @@ retry:
 
 	drm_mode_copy(&crtc_state->base.mode, mode);
 
-	if (intel_set_mode(crtc, state)) {
+	if (intel_set_mode(crtc, state, true)) {
 		DRM_DEBUG_KMS("failed to set mode on load-detect pipe\n");
 		if (old->release_fb)
 			old->release_fb->funcs->destroy(old->release_fb);
@@ -10170,7 +10171,7 @@ void intel_release_load_detect_pipe(struct drm_connector *connector,
 		if (ret)
 			goto fail;
 
-		ret = intel_set_mode(crtc, state);
+		ret = intel_set_mode(crtc, state, true);
 		if (ret)
 			goto fail;
 
@@ -12646,20 +12647,22 @@ static int __intel_set_mode(struct drm_crtc *modeset_crtc,
 }
 
 static int intel_set_mode_with_config(struct drm_crtc *crtc,
-				      struct intel_crtc_state *pipe_config)
+				      struct intel_crtc_state *pipe_config,
+				      bool force_restore)
 {
 	int ret;
 
 	ret = __intel_set_mode(crtc, pipe_config);
 
-	if (ret == 0)
+	if (ret == 0 && force_restore)
 		intel_modeset_check_state(crtc->dev);
 
 	return ret;
 }
 
 static int intel_set_mode(struct drm_crtc *crtc,
-			  struct drm_atomic_state *state)
+			  struct drm_atomic_state *state,
+			  bool force_restore)
 {
 	struct intel_crtc_state *pipe_config;
 	int ret = 0;
@@ -12670,7 +12673,7 @@ static int intel_set_mode(struct drm_crtc *crtc,
 		goto out;
 	}
 
-	ret = intel_set_mode_with_config(crtc, pipe_config);
+	ret = intel_set_mode_with_config(crtc, pipe_config, force_restore);
 	if (ret)
 		goto out;
 
@@ -12747,7 +12750,7 @@ void intel_crtc_restore_mode(struct drm_crtc *crtc)
 	intel_modeset_setup_plane_state(state, crtc, &crtc->mode,
 					crtc->primary->fb, crtc->x, crtc->y);
 
-	ret = intel_set_mode(crtc, state);
+	ret = intel_set_mode(crtc, state, false);
 	if (ret)
 		drm_atomic_state_free(state);
 }
@@ -12947,7 +12950,7 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
 
 	primary_plane_was_visible = primary_plane_visible(set->crtc);
 
-	ret = intel_set_mode_with_config(set->crtc, pipe_config);
+	ret = intel_set_mode_with_config(set->crtc, pipe_config, true);
 
 	if (ret == 0 &&
 	    pipe_config->base.enable &&
-- 
2.1.0

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

[PATCH v2 2/3] drm/i915: Don't update staged config during force restore modesets

[Ander Conselvan de Oliveira]

The force restore path relies on the staged config to preserve the
configuration used before a suspend/resume cycle. The update done to it
in intel_modeset_fixup_state() would cause that information to be lost
after the first modeset, making it impossible to restore the modes for
pipes B and C.

References: https://bugs.freedesktop.org/show_bug.cgi?id=90468
Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index bcf1041..ff7175b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11386,10 +11386,6 @@ static void intel_modeset_fixup_state(struct drm_atomic_state *state)
 		crtc->base.enabled = crtc->base.state->enable;
 		crtc->config = to_intel_crtc_state(crtc->base.state);
 	}
-
-	/* Copy the new configuration to the staged state, to keep the few
-	 * pieces of code that haven't been converted yet happy */
-	intel_modeset_update_staged_output_state(state->dev);
 }
 
 static void
@@ -12654,8 +12650,10 @@ static int intel_set_mode_with_config(struct drm_crtc *crtc,
 
 	ret = __intel_set_mode(crtc, pipe_config);
 
-	if (ret == 0 && force_restore)
+	if (ret == 0 && force_restore) {
+		intel_modeset_update_staged_output_state(crtc->dev);
 		intel_modeset_check_state(crtc->dev);
+	}
 
 	return ret;
 }
-- 
2.1.0

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

[PATCH v2 3/3] drm/i915: Don't set enabled value of all CRTCs when restoring the mode

[Ander Conselvan de Oliveira]

The code in intel_crtc_restore_mode() sets the enabled value of all the
CRTCs when restoring the mode after a suspend/resume cycle. When more
than one CRTC is enabled, that causes drm_atomic_helper_check_modeset()
to fail if there is more than one pipe enabled, since all but one CRTC
has valid connector data. Instead, set only the enabled value for the
CRTC passed as an argument.

v2: Don't leak atomic state. (Matt)

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90468
References: https://bugs.freedesktop.org/show_bug.cgi?id=90396
Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 27 ++++++++++-----------------
 1 file changed, 10 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index ff7175b..e047105 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12683,7 +12683,6 @@ void intel_crtc_restore_mode(struct drm_crtc *crtc)
 {
 	struct drm_device *dev = crtc->dev;
 	struct drm_atomic_state *state;
-	struct intel_crtc *intel_crtc;
 	struct intel_encoder *encoder;
 	struct intel_connector *connector;
 	struct drm_connector_state *connector_state;
@@ -12726,24 +12725,18 @@ void intel_crtc_restore_mode(struct drm_crtc *crtc)
 		}
 	}
 
-	for_each_intel_crtc(dev, intel_crtc) {
-		if (intel_crtc->new_enabled == intel_crtc->base.enabled)
-			continue;
-
-		crtc_state = intel_atomic_get_crtc_state(state, intel_crtc);
-		if (IS_ERR(crtc_state)) {
-			DRM_DEBUG_KMS("Failed to add [CRTC:%d] to state: %ld\n",
-				      intel_crtc->base.base.id,
-				      PTR_ERR(crtc_state));
-			continue;
-		}
+	crtc_state = intel_atomic_get_crtc_state(state, to_intel_crtc(crtc));
+	if (IS_ERR(crtc_state)) {
+		DRM_DEBUG_KMS("Failed to add [CRTC:%d] to state: %ld\n",
+			      crtc->base.id, PTR_ERR(crtc_state));
+		drm_atomic_state_free(state);
+		return;
+	}
 
-		crtc_state->base.active = crtc_state->base.enable =
-			intel_crtc->new_enabled;
+	crtc_state->base.active = crtc_state->base.enable =
+		to_intel_crtc(crtc)->new_enabled;
 
-		if (&intel_crtc->base == crtc)
-			drm_mode_copy(&crtc_state->base.mode, &crtc->mode);
-	}
+	drm_mode_copy(&crtc_state->base.mode, &crtc->mode);
 
 	intel_modeset_setup_plane_state(state, crtc, &crtc->mode,
 					crtc->primary->fb, crtc->x, crtc->y);
-- 
2.1.0

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

Re: [PATCH v2 1/3] drm/i915: Don't check modeset state in the hw state force restore path

[Jani Nikula]

On Tue, 16 Jun 2015, Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> wrote:
> Since the force restore logic will restore the CRTCs state one at a
> time, it is possible that the state will be inconsistent until the whole
> operation finishes. A call to intel_modeset_check_state() is done once
> it's over, so don't check the state multiple times in between. This
> regression was introduced in:
>
> commit 7f27126ea3db6ade886f18fd39caf0ff0cd1d37f
> Author: Jesse Barnes <jbarnes@virtuousgeek.org>
> Date:   Wed Nov 5 14:26:06 2014 -0800
>
>     drm/i915: factor out compute_config from __intel_set_mode v3
>
> v2: Rename check parameter to force_restore. (Matt)
>
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=94431
> Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> Reviewed-by: Matt Roper <matthew.d.roper@intel.com>

All three patches applied to drm-intel-next-fixes, aiming for v4.2 merge
window. Thanks for the patches and review.

For drm-intel-nightly, I resolved the conflicts by ignoring these
changes and favoring what's in drm-intel-next-queued. Fingers crossed I
didn't botch it up!

BR,
Jani.



> ---
>  drivers/gpu/drm/i915/intel_display.c | 21 ++++++++++++---------
>  1 file changed, 12 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 4e3f302..bcf1041 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -87,7 +87,8 @@ static void ironlake_pch_clock_get(struct intel_crtc *crtc,
>  				   struct intel_crtc_state *pipe_config);
>  
>  static int intel_set_mode(struct drm_crtc *crtc,
> -			  struct drm_atomic_state *state);
> +			  struct drm_atomic_state *state,
> +			  bool force_restore);
>  static int intel_framebuffer_init(struct drm_device *dev,
>  				  struct intel_framebuffer *ifb,
>  				  struct drm_mode_fb_cmd2 *mode_cmd,
> @@ -10096,7 +10097,7 @@ retry:
>  
>  	drm_mode_copy(&crtc_state->base.mode, mode);
>  
> -	if (intel_set_mode(crtc, state)) {
> +	if (intel_set_mode(crtc, state, true)) {
>  		DRM_DEBUG_KMS("failed to set mode on load-detect pipe\n");
>  		if (old->release_fb)
>  			old->release_fb->funcs->destroy(old->release_fb);
> @@ -10170,7 +10171,7 @@ void intel_release_load_detect_pipe(struct drm_connector *connector,
>  		if (ret)
>  			goto fail;
>  
> -		ret = intel_set_mode(crtc, state);
> +		ret = intel_set_mode(crtc, state, true);
>  		if (ret)
>  			goto fail;
>  
> @@ -12646,20 +12647,22 @@ static int __intel_set_mode(struct drm_crtc *modeset_crtc,
>  }
>  
>  static int intel_set_mode_with_config(struct drm_crtc *crtc,
> -				      struct intel_crtc_state *pipe_config)
> +				      struct intel_crtc_state *pipe_config,
> +				      bool force_restore)
>  {
>  	int ret;
>  
>  	ret = __intel_set_mode(crtc, pipe_config);
>  
> -	if (ret == 0)
> +	if (ret == 0 && force_restore)
>  		intel_modeset_check_state(crtc->dev);
>  
>  	return ret;
>  }
>  
>  static int intel_set_mode(struct drm_crtc *crtc,
> -			  struct drm_atomic_state *state)
> +			  struct drm_atomic_state *state,
> +			  bool force_restore)
>  {
>  	struct intel_crtc_state *pipe_config;
>  	int ret = 0;
> @@ -12670,7 +12673,7 @@ static int intel_set_mode(struct drm_crtc *crtc,
>  		goto out;
>  	}
>  
> -	ret = intel_set_mode_with_config(crtc, pipe_config);
> +	ret = intel_set_mode_with_config(crtc, pipe_config, force_restore);
>  	if (ret)
>  		goto out;
>  
> @@ -12747,7 +12750,7 @@ void intel_crtc_restore_mode(struct drm_crtc *crtc)
>  	intel_modeset_setup_plane_state(state, crtc, &crtc->mode,
>  					crtc->primary->fb, crtc->x, crtc->y);
>  
> -	ret = intel_set_mode(crtc, state);
> +	ret = intel_set_mode(crtc, state, false);
>  	if (ret)
>  		drm_atomic_state_free(state);
>  }
> @@ -12947,7 +12950,7 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
>  
>  	primary_plane_was_visible = primary_plane_visible(set->crtc);
>  
> -	ret = intel_set_mode_with_config(set->crtc, pipe_config);
> +	ret = intel_set_mode_with_config(set->crtc, pipe_config, true);
>  
>  	if (ret == 0 &&
>  	    pipe_config->base.enable &&
> -- 
> 2.1.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 1/3] drm/i915: Don't check modeset state in the hw state force restore path

[Jani Nikula]

On Wed, 17 Jun 2015, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> On Tue, 16 Jun 2015, Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> wrote:
>> Since the force restore logic will restore the CRTCs state one at a
>> time, it is possible that the state will be inconsistent until the whole
>> operation finishes. A call to intel_modeset_check_state() is done once
>> it's over, so don't check the state multiple times in between. This
>> regression was introduced in:
>>
>> commit 7f27126ea3db6ade886f18fd39caf0ff0cd1d37f
>> Author: Jesse Barnes <jbarnes@virtuousgeek.org>
>> Date:   Wed Nov 5 14:26:06 2014 -0800
>>
>>     drm/i915: factor out compute_config from __intel_set_mode v3
>>
>> v2: Rename check parameter to force_restore. (Matt)
>>
>> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=94431
>> Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
>> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
>> Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
>
> All three patches applied to drm-intel-next-fixes, aiming for v4.2 merge
> window. Thanks for the patches and review.
>
> For drm-intel-nightly, I resolved the conflicts by ignoring these
> changes and favoring what's in drm-intel-next-queued. Fingers crossed I
> didn't botch it up!

Also, do we need some of these in v4.1 or earlier as well? Should we
backport once these are upstream?

BR,
Jani.





>
> BR,
> Jani.
>
>
>
>> ---
>>  drivers/gpu/drm/i915/intel_display.c | 21 ++++++++++++---------
>>  1 file changed, 12 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 4e3f302..bcf1041 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -87,7 +87,8 @@ static void ironlake_pch_clock_get(struct intel_crtc *crtc,
>>  				   struct intel_crtc_state *pipe_config);
>>  
>>  static int intel_set_mode(struct drm_crtc *crtc,
>> -			  struct drm_atomic_state *state);
>> +			  struct drm_atomic_state *state,
>> +			  bool force_restore);
>>  static int intel_framebuffer_init(struct drm_device *dev,
>>  				  struct intel_framebuffer *ifb,
>>  				  struct drm_mode_fb_cmd2 *mode_cmd,
>> @@ -10096,7 +10097,7 @@ retry:
>>  
>>  	drm_mode_copy(&crtc_state->base.mode, mode);
>>  
>> -	if (intel_set_mode(crtc, state)) {
>> +	if (intel_set_mode(crtc, state, true)) {
>>  		DRM_DEBUG_KMS("failed to set mode on load-detect pipe\n");
>>  		if (old->release_fb)
>>  			old->release_fb->funcs->destroy(old->release_fb);
>> @@ -10170,7 +10171,7 @@ void intel_release_load_detect_pipe(struct drm_connector *connector,
>>  		if (ret)
>>  			goto fail;
>>  
>> -		ret = intel_set_mode(crtc, state);
>> +		ret = intel_set_mode(crtc, state, true);
>>  		if (ret)
>>  			goto fail;
>>  
>> @@ -12646,20 +12647,22 @@ static int __intel_set_mode(struct drm_crtc *modeset_crtc,
>>  }
>>  
>>  static int intel_set_mode_with_config(struct drm_crtc *crtc,
>> -				      struct intel_crtc_state *pipe_config)
>> +				      struct intel_crtc_state *pipe_config,
>> +				      bool force_restore)
>>  {
>>  	int ret;
>>  
>>  	ret = __intel_set_mode(crtc, pipe_config);
>>  
>> -	if (ret == 0)
>> +	if (ret == 0 && force_restore)
>>  		intel_modeset_check_state(crtc->dev);
>>  
>>  	return ret;
>>  }
>>  
>>  static int intel_set_mode(struct drm_crtc *crtc,
>> -			  struct drm_atomic_state *state)
>> +			  struct drm_atomic_state *state,
>> +			  bool force_restore)
>>  {
>>  	struct intel_crtc_state *pipe_config;
>>  	int ret = 0;
>> @@ -12670,7 +12673,7 @@ static int intel_set_mode(struct drm_crtc *crtc,
>>  		goto out;
>>  	}
>>  
>> -	ret = intel_set_mode_with_config(crtc, pipe_config);
>> +	ret = intel_set_mode_with_config(crtc, pipe_config, force_restore);
>>  	if (ret)
>>  		goto out;
>>  
>> @@ -12747,7 +12750,7 @@ void intel_crtc_restore_mode(struct drm_crtc *crtc)
>>  	intel_modeset_setup_plane_state(state, crtc, &crtc->mode,
>>  					crtc->primary->fb, crtc->x, crtc->y);
>>  
>> -	ret = intel_set_mode(crtc, state);
>> +	ret = intel_set_mode(crtc, state, false);
>>  	if (ret)
>>  		drm_atomic_state_free(state);
>>  }
>> @@ -12947,7 +12950,7 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
>>  
>>  	primary_plane_was_visible = primary_plane_visible(set->crtc);
>>  
>> -	ret = intel_set_mode_with_config(set->crtc, pipe_config);
>> +	ret = intel_set_mode_with_config(set->crtc, pipe_config, true);
>>  
>>  	if (ret == 0 &&
>>  	    pipe_config->base.enable &&
>> -- 
>> 2.1.0
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> -- 
> Jani Nikula, Intel Open Source Technology Center

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 1/3] drm/i915: Don't check modeset state in the hw state force restore path

[Ander Conselvan De Oliveira]

On Wed, 2015-06-17 at 15:13 +0300, Jani Nikula wrote:
> On Wed, 17 Jun 2015, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> > On Tue, 16 Jun 2015, Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> wrote:
> >> Since the force restore logic will restore the CRTCs state one at a
> >> time, it is possible that the state will be inconsistent until the whole
> >> operation finishes. A call to intel_modeset_check_state() is done once
> >> it's over, so don't check the state multiple times in between. This
> >> regression was introduced in:
> >>
> >> commit 7f27126ea3db6ade886f18fd39caf0ff0cd1d37f
> >> Author: Jesse Barnes <jbarnes@virtuousgeek.org>
> >> Date:   Wed Nov 5 14:26:06 2014 -0800
> >>
> >>     drm/i915: factor out compute_config from __intel_set_mode v3
> >>
> >> v2: Rename check parameter to force_restore. (Matt)
> >>
> >> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=94431
> >> Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
> >> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> >> Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
> >
> > All three patches applied to drm-intel-next-fixes, aiming for v4.2 merge
> > window. Thanks for the patches and review.
> >
> > For drm-intel-nightly, I resolved the conflicts by ignoring these
> > changes and favoring what's in drm-intel-next-queued. Fingers crossed I
> > didn't botch it up!
> 
> Also, do we need some of these in v4.1 or earlier as well? Should we
> backport once these are upstream?

The regression 1/3 solves goes back to v3.19. The other patches are only
relevant for 4.2 and later.

Thanks,
Ander

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

Re: [PATCH v2 1/3] drm/i915: Don't check modeset state in the hw state force restore path

[Ander Conselvan De Oliveira]

On Wed, 2015-06-17 at 15:04 +0300, Jani Nikula wrote:
> On Tue, 16 Jun 2015, Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> wrote:
> > Since the force restore logic will restore the CRTCs state one at a
> > time, it is possible that the state will be inconsistent until the whole
> > operation finishes. A call to intel_modeset_check_state() is done once
> > it's over, so don't check the state multiple times in between. This
> > regression was introduced in:
> >
> > commit 7f27126ea3db6ade886f18fd39caf0ff0cd1d37f
> > Author: Jesse Barnes <jbarnes@virtuousgeek.org>
> > Date:   Wed Nov 5 14:26:06 2014 -0800
> >
> >     drm/i915: factor out compute_config from __intel_set_mode v3
> >
> > v2: Rename check parameter to force_restore. (Matt)
> >
> > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=94431
> > Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
> > Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> > Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
> 
> All three patches applied to drm-intel-next-fixes, aiming for v4.2 merge
> window. Thanks for the patches and review.
> 
> For drm-intel-nightly, I resolved the conflicts by ignoring these
> changes and favoring what's in drm-intel-next-queued. Fingers crossed I
> didn't botch it up!

In the end, that means only the content of 3/3 is in -nightly. It won't
fix any of the issues, but shouldn't cause any problems.

Ander


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

Re: [PATCH v2 1/3] drm/i915: Don't check modeset state in the hw state force restore path

[Jani Nikula]

On Wed, 17 Jun 2015, Ander Conselvan De Oliveira <conselvan2@gmail.com> wrote:
> On Wed, 2015-06-17 at 15:04 +0300, Jani Nikula wrote:
>> On Tue, 16 Jun 2015, Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> wrote:
>> > Since the force restore logic will restore the CRTCs state one at a
>> > time, it is possible that the state will be inconsistent until the whole
>> > operation finishes. A call to intel_modeset_check_state() is done once
>> > it's over, so don't check the state multiple times in between. This
>> > regression was introduced in:
>> >
>> > commit 7f27126ea3db6ade886f18fd39caf0ff0cd1d37f
>> > Author: Jesse Barnes <jbarnes@virtuousgeek.org>
>> > Date:   Wed Nov 5 14:26:06 2014 -0800
>> >
>> >     drm/i915: factor out compute_config from __intel_set_mode v3
>> >
>> > v2: Rename check parameter to force_restore. (Matt)
>> >
>> > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=94431
>> > Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
>> > Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
>> > Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
>> 
>> All three patches applied to drm-intel-next-fixes, aiming for v4.2 merge
>> window. Thanks for the patches and review.
>> 
>> For drm-intel-nightly, I resolved the conflicts by ignoring these
>> changes and favoring what's in drm-intel-next-queued. Fingers crossed I
>> didn't botch it up!
>
> In the end, that means only the content of 3/3 is in -nightly. It won't
> fix any of the issues, but shouldn't cause any problems.

But it is expected that Maarten's atomic work will eventually fix this,
right?

BR,
Jani.


>
> Ander
>
>

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 1/3] drm/i915: Don't check modeset state in the hw state force restore path

[Ander Conselvan De Oliveira]

On Wed, 2015-06-17 at 15:42 +0300, Jani Nikula wrote:
> On Wed, 17 Jun 2015, Ander Conselvan De Oliveira <conselvan2@gmail.com> wrote:
> > On Wed, 2015-06-17 at 15:04 +0300, Jani Nikula wrote:
> >> On Tue, 16 Jun 2015, Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> wrote:
> >> > Since the force restore logic will restore the CRTCs state one at a
> >> > time, it is possible that the state will be inconsistent until the whole
> >> > operation finishes. A call to intel_modeset_check_state() is done once
> >> > it's over, so don't check the state multiple times in between. This
> >> > regression was introduced in:
> >> >
> >> > commit 7f27126ea3db6ade886f18fd39caf0ff0cd1d37f
> >> > Author: Jesse Barnes <jbarnes@virtuousgeek.org>
> >> > Date:   Wed Nov 5 14:26:06 2014 -0800
> >> >
> >> >     drm/i915: factor out compute_config from __intel_set_mode v3
> >> >
> >> > v2: Rename check parameter to force_restore. (Matt)
> >> >
> >> > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=94431
> >> > Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
> >> > Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> >> > Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
> >> 
> >> All three patches applied to drm-intel-next-fixes, aiming for v4.2 merge
> >> window. Thanks for the patches and review.
> >> 
> >> For drm-intel-nightly, I resolved the conflicts by ignoring these
> >> changes and favoring what's in drm-intel-next-queued. Fingers crossed I
> >> didn't botch it up!
> >
> > In the end, that means only the content of 3/3 is in -nightly. It won't
> > fix any of the issues, but shouldn't cause any problems.
> 
> But it is expected that Maarten's atomic work will eventually fix this,
> right?

Yes, that's correct.

Ander

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