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

Vivi, Rodrigo
2015-05-21T21:38:34Z

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