public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Daniel Vetter <daniel@ffwll.ch>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/i915: respect previous reg values on primary plane disable
Date: Wed, 14 Oct 2015 16:09:13 +0300	[thread overview]
Message-ID: <20151014130913.GV26517@intel.com> (raw)
In-Reply-To: <20151014130405.GE26718@phenom.ffwll.local>

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@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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2015-10-14 13:09 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-10-13 21:24 [PATCH] drm/i915: respect previous reg values on primary plane disable Kevin Strasser
2015-10-13 23:02 ` Jesse Barnes
2015-10-14  7:58 ` Jani Nikula
2015-10-14  7:59   ` Jani Nikula
2015-10-14 18:44   ` Kevin Strasser
2015-10-14 19:01     ` Daniel Vetter
2015-10-14 12:07 ` Ville Syrjälä
2015-10-14 12:12   ` Chris Wilson
2015-10-14 12:22     ` Ville Syrjälä
2015-10-14 18:59       ` Kevin Strasser
2015-10-14 19:48         ` Ville Syrjälä
2015-10-14 20:33           ` Kevin Strasser
2015-10-15  8:20             ` Ville Syrjälä
2015-10-15 23:00               ` Kevin Strasser
2015-10-16  0:14                 ` Ville Syrjälä
2015-10-14 13:04   ` Daniel Vetter
2015-10-14 13:09     ` Ville Syrjälä [this message]
2015-10-14 22:51 ` [PATCH v2] drm/i915/hsw: keep gamma and CSC enabled for " Kevin Strasser
2015-10-15 12:31   ` Daniel Vetter
2015-10-15 12:41     ` Ville Syrjälä
2015-10-16 22:53       ` Bob Paauwe
2015-10-19 10:15         ` Daniel Vetter
2015-10-19 17:13           ` Kevin Strasser
2015-10-20 15:48             ` Bob Paauwe
2015-10-20 16:13               ` Ville Syrjälä
2015-10-20 17:00                 ` Bob Paauwe
2015-10-21  6:31                   ` Daniel Vetter
2015-10-20 15:49   ` Bob Paauwe

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20151014130913.GV26517@intel.com \
    --to=ville.syrjala@linux.intel.com \
    --cc=daniel@ffwll.ch \
    --cc=intel-gfx@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox