Re: [PATCH v2] drm/i915/hsw: keep gamma and CSC enabled for primary plane disable

From: Daniel Vetter <daniel@ffwll.ch>
To: Bob Paauwe <bob.j.paauwe@intel.com>
Cc: Chandra Konduru <chandra.konduruatintel.com@phenom.ffwll.local>,
	intel-gfx <intel-gfx@lists.freedesktop.org>
Subject: Re: [PATCH v2] drm/i915/hsw: keep gamma and CSC enabled for primary plane disable
Date: Mon, 19 Oct 2015 12:15:41 +0200	[thread overview]
Message-ID: <20151019101541.GW13786@phenom.ffwll.local> (raw)
In-Reply-To: <20151016155322.36d1d9d8@bpaauwe-desk.fm.intel.com>

On Fri, Oct 16, 2015 at 03:53:22PM -0700, Bob Paauwe wrote:
> On Thu, 15 Oct 2015 15:41:30 +0300
> Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> 
> > On Thu, Oct 15, 2015 at 02:31:09PM +0200, Daniel Vetter wrote:
> > > 
> > > On Thu, Oct 15, 2015 at 12:51 AM, Kevin Strasser <kevin.strasser@linux.intel.com> wrote:
> > > > On HSW the crc differs between black and disabled primary planes, causing an
> > > > assert to fail in the kms_universal_plane test. It seems that gamma correction
> > > > and color space conversion are causing the black primary plane case to result in
> > > > a brighter color than the disabled primary plane case.
> > > >
> > > > Keep gamma and CSC bits enabled for plane disable path on HSW.
> > > >
> > > > v2: Avoid use of RMW
> > > >     Keep path unchanged for non-HSW users
> > > >
> > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89331
> > > > Testcase: igt/kms_universal_plane/universal-plane-pipe-A-functional
> > > > Signed-off-by: Kevin Strasser <kevin.strasser@linux.intel.com>
> > > 
> > > With big discussions please add everyone who participated (Ville, Chris,
> > > Jani, me) to the cc list of the sob section of the patch when doing a new
> > > revisions.
> > > 
> > > Bob did something eerily similarly a while ago to fix crc failures:
> > > 
> > > http://lists.freedesktop.org/archives/intel-gfx/2015-August/074657.html
> > > http://lists.freedesktop.org/archives/intel-gfx/2015-August/074828.html
> > > 
> > > Unfortunately those patches did go nowhere :( Related?
> > 
> > Those seem to be for active plane cases. I'll go review them...
> > 
> 
> I've been looking at the same test case failure on BXT. The bxt/skl
> update primary plane function is similar to HSW's in that it was
> clearing the gamma and csc bits when the plane was disabled. So I first
> tried the same fix but it didn't work.  Matt and I were discussing this
> and thought it might be the background (PIPE_BOTTOM_COLOR) that was
> causing the CRC failures.  And yes, it is.  The PIPE_BOTTOM_COLOR also
> has gamma and csc enable bits which default to off.  If those are
> flipped on, then the CRC's match between a disabled plane and a black
> plane. 
> 
> So it seems to make CRC's match we need to make sure that all planes
> (primary/sprite/cursor/bottom) have the same gamma/csc settings.
> 
> Chandra had a patch that adds a property for bottom color a while back
> but it seems to have stalled. 

We don't need the bottom color to make sure the bottom color is the right
kind of black. So for now we can just make sure this is the case in the
crtc_enable hook.

And yes the gamma table/color correction stuff needs to make sure we stay
uniform (i.e. all off or all on), another igt for them to write I think.
-Daniel

> 
> http://lists.freedesktop.org/archives/intel-gfx/2015-June/069836.html
> 
> -- 
> --
> Bob Paauwe                  
> Bob.J.Paauwe@intel.com
> IOTG / PED Software Organization
> Intel Corp.  Folsom, CA
> (916) 356-6193    
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx