Re: [PATCH] drm/i915: Fix IPS related flicker

From: "Vivi, Rodrigo" <rodrigo.vivi@intel.com>
To: "daniel@ffwll.ch" <daniel@ffwll.ch>
Cc: "intel-gfx@lists.freedesktop.org"
	<intel-gfx@lists.freedesktop.org>,
	"Konno, Joe" <joe.konno@intel.com>,
	"Zanoni, Paulo R" <paulo.r.zanoni@intel.com>
Subject: Re: [PATCH] drm/i915: Fix IPS related flicker
Date: Thu, 21 May 2015 21:38:32 +0000	[thread overview]
Message-ID: <1432244493.21904.18.camel@intel.com> (raw)
In-Reply-To: <20150521213329.GP15256@phenom.ffwll.local>

On Thu, 2015-05-21 at 23:33 +0200, Daniel Vetter wrote:
> On Thu, May 21, 2015 at 12:53:03PM -0700, Rodrigo Vivi wrote:
> > We cannot let IPS enabled with no plane on the pipe:
> > 
> > BSpec: "IPS cannot be enabled until after at least one plane has
> > been enabled for at least one vertical blank." and "IPS must be
> > disabled while there is still at least one plane enabled on the
> > same pipe as IPS.
> > 
> > However this shortcut path to make primary plane invisible when
> > updating primary plane was leting IPS enabled while there was no
> > other plane enabled on the pipe causing flickerings that we were
> > believing that it was caused by that other restriction where
> > ips cannot be used when pixel rate is greater than 95% of cdclok.
> > 
> > Reference: https://bugs.freedesktop.org/show_bug.cgi?id=85583
> > Cc: Joe Konno <joe.konno@intel.com>
> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> 
> Broken since forever or regression?

Not sure, but probably a regression since I was looking to a 3.14 based
kernel here and didn't find this shortcut setting DSPCNTRL to 0 on
update_primary_plane.

>  cc: stable?

Yes, I should've added, sorry!

> Issue on bdw only or also
> hsw? 

Also on HSW since this limitation is also documented on HSW.

> Please help out your poor clueless maintainers ;-)

Sorry for my poor comments...

> 
> Thanks, Daniel
> 
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 9d2d6fb..5519d56 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -2789,6 +2789,19 @@ static void ironlake_update_primary_plane(struct drm_crtc *crtc,
> >  	int pixel_size;
> >  
> >  	if (!visible || !fb) {
> > +		/*
> > +		 * This shortcut path disables the primary plane making
> > +		 * IPS really sad and lost when it is enabled alone with no
> > +		 * plane around on the pipe. So, for now it is safe to disable
> > +		 * IPS here.
> > +		 /
Bikesheding my on patch: I forgot a "*" here above

> > +		 /*
> > +		 * FIXME: Any other plane enabled should be enough so it would
> > +		 * be better to check if there is really no sprite or
> > +		 * cursor around.
Bikesheding my on patch: I'm not sure if we should add this FIXME
here...

> > +		 */
> > +		hsw_disable_ips(intel_crtc);
> > +
> >  		I915_WRITE(reg, 0);
> >  		I915_WRITE(DSPSURF(plane), 0);
> >  		POSTING_READ(reg);
> > -- 
> > 2.1.0
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx