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

[Kevin Strasser]

On Wed, Oct 14, 2015 at 03:22:23PM +0300, Ville Syrjälä wrote:
> 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:
[...]
> > > > -		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.

Just to level set, these cases will produce different CRCs on HSW:
 1. Primary plane disabled, gamma correction disabled
 2. Primary plane disabled, gamma correction enabled

Case 2 is visibly brighter than case 1 and looks more like the enabled black
primary plane case. The purpose of this patch is to get the behavior of a
disabled primary plane to match that of an enabled black plane, just as it does
on non-HSW platforms.

Understood, RMWs are inappropriate here. I'll rework the patch to explicitly
enable the bits that are needed.

Thanks,
Kevin