[Intel-gfx] [PATCH v2] drm/i915: Always load the display palette before enabling the pipe

[Daniel Vetter]

On Fri, May 31, 2013 at 08:09:47PM +0300, ville.syrjala at linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> 
> Loading the palette after the planes are enabled can risk showing
> incorrect colors. ILK+ already load the palette before even the pipe
> is enabled. Just follow the same order for gen2-4 and VLV.
> 
> According to BSpec the requirements for palette access are
> display core clock and display PLL running. In certain platforms
> just the core clock may be enough. But we definitely should have both
> running when this gets called during the modeset.
> 
> v2: Amend the commit message with some display PLL/core clock info
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>

I'd prefer if we add asserts to check that such ordering constraints are
fulfilled. With all the modeset rework the ordering sequence is much more
stable, but due to all that refactoring it's now also way too easy to
accidentally reorder something to the wrong spot. Just now I'm suffering
through a bit of this while trying to get pch pll state moved into the
pipe config.

For this case I think adding an assert_(correct pll)_enabled should help.
Maybe in a second patch since it's quite tricky with all the different
platforms ...
-Daniel

> ---
>  drivers/gpu/drm/i915/intel_display.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index ffbd66a..a3ceae6 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3579,10 +3579,11 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
>  	/* Enable panel fitting for eDP */
>  	i9xx_pfit_enable(intel_crtc);
>  
> +	intel_crtc_load_lut(crtc);
> +
>  	intel_enable_pipe(dev_priv, pipe, false);
>  	intel_enable_plane(dev_priv, plane, pipe);
>  
> -	intel_crtc_load_lut(crtc);
>  	intel_update_fbc(dev);
>  
>  	/* Give the overlay scaler a chance to enable if it's on this pipe */
> @@ -3618,12 +3619,13 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc)
>  	/* Enable panel fitting for LVDS */
>  	i9xx_pfit_enable(intel_crtc);
>  
> +	intel_crtc_load_lut(crtc);
> +
>  	intel_enable_pipe(dev_priv, pipe, false);
>  	intel_enable_plane(dev_priv, plane, pipe);
>  	if (IS_G4X(dev))
>  		g4x_fixup_plane(dev_priv, pipe);
>  
> -	intel_crtc_load_lut(crtc);
>  	intel_update_fbc(dev);
>  
>  	/* Give the overlay scaler a chance to enable if it's on this pipe */
> -- 
> 1.8.1.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch