drm/i915: Read WM before sanitize_encoder/crtc calls
diff mbox

Message ID 1438785998-19883-1-git-send-email-daniel.vetter@ffwll.ch
State New
Headers show

Commit Message

Daniel Vetter Aug. 5, 2015, 2:46 p.m. UTC
If we shut down the crtc we might run into WM consistency checks which
fail because we haven't yet read out the WM state. So do that before
we sanitized the state.

This fixes a WARNING in the ilk wm code which assumes that level 0 WM
are always enabled in it's internal tracking. But since we start out
with all 0 in our driver structures the relevant boolean is false when
loading. This regression was introduced in

commit 0b2ae6d72e445b58ae39cfa6ec0b8d3f53ff7a6f
Author: Ville Syrjälä <ville.syrjala@linux.intel.com>
Date:   Wed Oct 9 19:17:55 2013 +0300

    drm/i915: Add intel_pipe_wm and prepare for watermark pre-compute

which added the WARN_ON(!r->enabel).

Cc: Theodore Ts'o <tytso@mit.edu>
Reported-by: Theodore Ts'o <tytso@mit.edu>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
Re-ping for testing so that I can merge this patch and send a pull to
Linus.
-Daniel
---
 drivers/gpu/drm/i915/intel_display.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

Comments

Ville Syrjälä Sept. 10, 2015, 1:56 p.m. UTC | #1
On Wed, Aug 05, 2015 at 04:46:38PM +0200, Daniel Vetter wrote:
> If we shut down the crtc we might run into WM consistency checks which
> fail because we haven't yet read out the WM state. So do that before
> we sanitized the state.
> 
> This fixes a WARNING in the ilk wm code which assumes that level 0 WM
> are always enabled in it's internal tracking. But since we start out
> with all 0 in our driver structures the relevant boolean is false when
> loading. This regression was introduced in
> 
> commit 0b2ae6d72e445b58ae39cfa6ec0b8d3f53ff7a6f
> Author: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Date:   Wed Oct 9 19:17:55 2013 +0300
> 
>     drm/i915: Add intel_pipe_wm and prepare for watermark pre-compute
> 
> which added the WARN_ON(!r->enabel).
> 
> Cc: Theodore Ts'o <tytso@mit.edu>
> Reported-by: Theodore Ts'o <tytso@mit.edu>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
> Re-ping for testing so that I can merge this patch and send a pull to
> Linus.
> -Daniel
> ---
>  drivers/gpu/drm/i915/intel_display.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 30e0f54ba19d..ae07fd0c395c 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -15121,6 +15121,11 @@ void intel_modeset_setup_hw_state(struct drm_device *dev,
>  
>  	intel_modeset_readout_hw_state(dev);
>  
> +	if (IS_GEN9(dev))
> +		skl_wm_get_hw_state(dev);
> +	else if (HAS_PCH_SPLIT(dev))
> +		ilk_wm_get_hw_state(dev);
> +

So why not move it into intel_modeset_readout_hw_state()? Would seem
like the logical place for it.

>  	/*
>  	 * Now that we have the config, copy it to each CRTC struct
>  	 * Note that this could go away if we move to using crtc_config
> @@ -15162,11 +15167,6 @@ void intel_modeset_setup_hw_state(struct drm_device *dev,
>  		pll->on = false;
>  	}
>  
> -	if (IS_GEN9(dev))
> -		skl_wm_get_hw_state(dev);
> -	else if (HAS_PCH_SPLIT(dev))
> -		ilk_wm_get_hw_state(dev);
> -
>  	if (force_restore) {
>  		i915_redisable_vga(dev);
>  
> -- 
> 2.5.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Patch
diff mbox

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 30e0f54ba19d..ae07fd0c395c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -15121,6 +15121,11 @@  void intel_modeset_setup_hw_state(struct drm_device *dev,
 
 	intel_modeset_readout_hw_state(dev);
 
+	if (IS_GEN9(dev))
+		skl_wm_get_hw_state(dev);
+	else if (HAS_PCH_SPLIT(dev))
+		ilk_wm_get_hw_state(dev);
+
 	/*
 	 * Now that we have the config, copy it to each CRTC struct
 	 * Note that this could go away if we move to using crtc_config
@@ -15162,11 +15167,6 @@  void intel_modeset_setup_hw_state(struct drm_device *dev,
 		pll->on = false;
 	}
 
-	if (IS_GEN9(dev))
-		skl_wm_get_hw_state(dev);
-	else if (HAS_PCH_SPLIT(dev))
-		ilk_wm_get_hw_state(dev);
-
 	if (force_restore) {
 		i915_redisable_vga(dev);