On Thu, May 21, 2015 at 01:06:10PM -0700, Rodrigo Vivi wrote: > With this well documented we can remove that FIXME comment. > > We just need to make sure that on primary -> sprite transition there > is no vblank time where ips gets alone on the pipe with absolutelly > no plane. If this happens even for a quickly momment IPS won't get > recovered and it might cause strange flickering screens. Last time I tried this the hardware had problems with this, hence the FIXME comments. I think that was on HSW. IIRC it manifested as a (presumably) single black frame, but I can't recall if it happened on every transition or just sometimes. > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> > --- > drivers/gpu/drm/i915/intel_display.c | 37 ++++++++++++++++++++++++------------ > 1 file changed, 25 insertions(+), 12 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 5519d56..d2c417b 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -4620,6 +4620,17 @@ static void intel_enable_sprite_planes(struct drm_crtc *crtc) > } > } > > +/** > + * hsw_enable_ips - Enable IPS > + * @intel_crtc: crtc > + * > + * This function needs to be called after there are at least on plane enabled > + * on the pipe. > + * > + * IPS is fine as long as one plane is enabled. > + * > + * Pre-requisite: IPS cannot be enabled until after at least one plane has been enabled for at least one vertical blank. > + */ > void hsw_enable_ips(struct intel_crtc *crtc) > { > struct drm_device *dev = crtc->base.dev; > @@ -4653,6 +4664,20 @@ void hsw_enable_ips(struct intel_crtc *crtc) > } > } > > +/** > + * hsw_disable_ips - Disable IPS > + * @intel_crtc: crtc > + * > + * This function needs to be called everytime that there will be no plane > + * enabled on the pipe on the next vblank. > + * > + * IPS is fine as long as one plane is enabled, however it gets confused > + * in a non recoverable state when it stays enabled on the pipe with no > + * other plane enabled at same vblank time. So please make sure this > + * function is called everytime that we have no plane enabled on the pipe. > + * > + * Pre-requisite: IPS must be disabled while there is still at least one plane enabled on the same pipe as IPS. > + */ > void hsw_disable_ips(struct intel_crtc *crtc) > { > struct drm_device *dev = crtc->base.dev; > @@ -4769,12 +4794,6 @@ intel_post_enable_primary(struct drm_crtc *crtc) > if (IS_BROADWELL(dev)) > intel_wait_for_vblank(dev, pipe); > > - /* > - * FIXME IPS should be fine as long as one plane is > - * enabled, but in practice it seems to have problems > - * when going from primary only to sprite only and vice > - * versa. > - */ > hsw_enable_ips(intel_crtc); > > mutex_lock(&dev->struct_mutex); > @@ -4840,12 +4859,6 @@ intel_pre_disable_primary(struct drm_crtc *crtc) > intel_fbc_disable(dev); > mutex_unlock(&dev->struct_mutex); > > - /* > - * FIXME IPS should be fine as long as one plane is > - * enabled, but in practice it seems to have problems > - * when going from primary only to sprite only and vice > - * versa. > - */ > hsw_disable_ips(intel_crtc); > } > > -- > 2.1.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx