[Intel-gfx] [PATCH] drm/i915: respect previous reg values on primary plane disable

[Ville Syrjälä]

On Wed, Oct 14, 2015 at 01:12:27PM +0100, Chris Wilson wrote:
> On Wed, Oct 14, 2015 at 03:07:41PM +0300, Ville Syrjälä wrote:
> > On Tue, Oct 13, 2015 at 02:24:41PM -0700, Kevin Strasser 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 things like gamma
> > > correction are causing the black primary plane case to result in a brighter
> > > color than the disabled primary plane case.
> > > 
> > > Only toggle the enable bit instead of clearing the control register, making the
> > > disable path more similar to that of the sprite plane.
> > > 
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89331
> > > Testcase: igt/kms_universal_plane
> > > Signed-off-by: Kevin Strasser <kevin.strasser at linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_display.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > index cddb0c6..b6164d8e 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -2829,7 +2829,7 @@ static void ironlake_update_primary_plane(struct drm_crtc *crtc,
> > >  	int pixel_size;
> > >  
> > >  	if (!visible || !fb) {
> > > -		I915_WRITE(reg, 0);
> > > +		I915_WRITE(reg, I915_READ(reg) & ~DISPLAY_PLANE_ENABLE);
> > 
> > Eh, what now? We've been trying to eliminate these nasty RMWs.
> > 
> > Are you saying that if we disabled the plane, but leave the "pass plane
> > data through gamma" it still affects the output for any pixel "covered"
> > by the disabled plane?
> 
> What I thought was being said was that if a plane is set to black (but
> with gamma enabled on the pipe) then a different CRC is produced
> compared to when the pipe is completely disabled (no plane at all). It
> sounded to me like a test case failure.

In that case I don't understand how the patch is supposed to help.

But yeah, tests like these should really set up an identity gamma
and pipe csc matrix.

Also we should grow some properties to control whether the plane
data passes through the gamma/csc or not. Those could then be used
to achieeve the same effect.

-- 
Ville Syrjälä
Intel OTC