[PATCH] drm/i915: Document IPS restriction with 1 plane on the pipe.

[PATCH] drm/i915: Document IPS restriction with 1 plane on the pipe.

[Rodrigo Vivi]

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.

Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 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@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Document IPS restriction with 1 plane on the pipe.

[Ville Syrjälä]

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

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Document IPS restriction with 1 plane on the pipe.

[shuang.he]

Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6455
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                                  234/234              234/234
ILK                                  262/262              262/262
SNB                 -1              282/282              281/282
IVB                                  300/300              300/300
BYT                                  254/254              254/254
BDW                                  275/275              275/275
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
 SNB  igt@pm_rpm@dpms-mode-unset-non-lpsp      DMESG_WARN(18)PASS(1)      DMESG_WARN(1)
(dmesg patch applied)WARNING:at_drivers/gpu/drm/i915/intel_uncore.c:#assert_device_not_suspended[i915]()@WARNING:.* at .* assert_device_not_suspended+0x
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx