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

[Ville Syrjälä]

On Wed, Oct 14, 2015 at 03:04:05PM +0200, Daniel Vetter 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?
> 
> Yeah if we need to preserve the gamma bits then we should write that
> instead of keeping everything.

I'd say we never ever want to leave the gamma bit enabled if this is the
case because that would produce a discolored rectangle on the screen
whenever the plane is disabled and not fullscreen.

-- 
Ville Syrjälä
Intel OTC