From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH v2] drm/i915: Always load the display palette before enabling the pipe Date: Fri, 31 May 2013 20:59:49 +0200 Message-ID: <20130531185949.GD15743@phenom.ffwll.local> References: <20130508142920.0ee6c78d@jbarnes-desktop> <1370020187-27959-1-git-send-email-ville.syrjala@linux.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mail-ea0-f174.google.com (mail-ea0-f174.google.com [209.85.215.174]) by gabe.freedesktop.org (Postfix) with ESMTP id 82C76E6178 for ; Fri, 31 May 2013 11:59:53 -0700 (PDT) Received: by mail-ea0-f174.google.com with SMTP id z7so1923655eaf.19 for ; Fri, 31 May 2013 11:59:52 -0700 (PDT) Content-Disposition: inline In-Reply-To: <1370020187-27959-1-git-send-email-ville.syrjala@linux.intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: ville.syrjala@linux.intel.com Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Fri, May 31, 2013 at 08:09:47PM +0300, ville.syrjala@linux.intel.com wro= te: > From: Ville Syrj=E4l=E4 > = > 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=E4l=E4 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_crt= c *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 *crt= c) > /* 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@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