diff mbox series

drm/i915/psr: Use continuous full frame update instead of single

Message ID 20221125134336.3999296-1-jouni.hogander@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915/psr: Use continuous full frame update instead of single | expand

Commit Message

Hogander, Jouni Nov. 25, 2022, 1:43 p.m. UTC
Currently we are observing occasionally display flickering or complete
freeze. This is narrowed down to be caused by single full frame update
(SFF).

SFF bit after it's written gets cleared by HW in subsequent vblank
i.e. when the update is sent to the panel. SFF bit is required to be
written together with partial frame update (PFU) bit. After the bit
gets cleared by the HW psr2 man trk ctl register still contains PFU
bit. If there is subsequent update for any reason we will end up
having selective update/fetch configuration where start line is 0 and
end line is 0. Also selective fetch configuration for the planes is
not properly performed. This seems to be causing problems with some
panels.

Fix this by using continuous full frame update instead and switch to
partial frame update only when selective update area is properly
calculated and configured.

This is also workaround for HSD 14014971508

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: José Roberto de Souza <jose.souza@intel.com>
Cc: Mika Kahola <mika.kahola@intel.com>

Reported-by: Lee Shawn C <shawn.c.lee@intel.com>
Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
---
 drivers/gpu/drm/i915/display/intel_psr.c | 21 ++-------------------
 1 file changed, 2 insertions(+), 19 deletions(-)

Comments

Hogander, Jouni Nov. 28, 2022, 5:37 p.m. UTC | #1
I found issue with this patch:

There is a time window between psr_invalidate/flush and next vblank
where atomic commit with selective update will cause full frame update
being lost due to overwriting the configuration.

I will send a new version.

On Fri, 2022-11-25 at 15:43 +0200, Jouni Högander wrote:
> Currently we are observing occasionally display flickering or
> complete
> freeze. This is narrowed down to be caused by single full frame
> update
> (SFF).
> 
> SFF bit after it's written gets cleared by HW in subsequent vblank
> i.e. when the update is sent to the panel. SFF bit is required to be
> written together with partial frame update (PFU) bit. After the bit
> gets cleared by the HW psr2 man trk ctl register still contains PFU
> bit. If there is subsequent update for any reason we will end up
> having selective update/fetch configuration where start line is 0 and
> end line is 0. Also selective fetch configuration for the planes is
> not properly performed. This seems to be causing problems with some
> panels.
> 
> Fix this by using continuous full frame update instead and switch to
> partial frame update only when selective update area is properly
> calculated and configured.
> 
> This is also workaround for HSD 14014971508
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: José Roberto de Souza <jose.souza@intel.com>
> Cc: Mika Kahola <mika.kahola@intel.com>
> 
> Reported-by: Lee Shawn C <shawn.c.lee@intel.com>
> Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_psr.c | 21 ++-------------------
>  1 file changed, 2 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> b/drivers/gpu/drm/i915/display/intel_psr.c
> index 5b678916e6db..41b0718eb3a1 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -1480,13 +1480,6 @@ static u32 man_trk_ctl_enable_bit_get(struct
> drm_i915_private *dev_priv)
>                 PSR2_MAN_TRK_CTL_ENABLE;
>  }
>  
> -static u32 man_trk_ctl_single_full_frame_bit_get(struct
> drm_i915_private *dev_priv)
> -{
> -       return IS_ALDERLAKE_P(dev_priv) || DISPLAY_VER(dev_priv) >=
> 14 ?
> -              ADLP_PSR2_MAN_TRK_CTL_SF_SINGLE_FULL_FRAME :
> -              PSR2_MAN_TRK_CTL_SF_SINGLE_FULL_FRAME;
> -}
> -
>  static u32 man_trk_ctl_partial_frame_bit_get(struct drm_i915_private
> *dev_priv)
>  {
>         return IS_ALDERLAKE_P(dev_priv) || DISPLAY_VER(dev_priv) >=
> 14 ?
> @@ -1510,7 +1503,7 @@ static void psr_force_hw_tracking_exit(struct
> intel_dp *intel_dp)
>                                PSR2_MAN_TRK_CTL(intel_dp-
> >psr.transcoder),
>                                man_trk_ctl_enable_bit_get(dev_priv) |
>                               
> man_trk_ctl_partial_frame_bit_get(dev_priv) |
> -                             
> man_trk_ctl_single_full_frame_bit_get(dev_priv));
> +                             
> man_trk_ctl_continuos_full_frame(dev_priv));
>  
>         /*
>          * Display WA #0884: skl+
> @@ -1624,11 +1617,7 @@ static void psr2_man_trk_ctl_calc(struct
> intel_crtc_state *crtc_state,
>         val |= man_trk_ctl_partial_frame_bit_get(dev_priv);
>  
>         if (full_update) {
> -               /*
> -                * Not applying Wa_14014971508:adlp as we do not
> support the
> -                * feature that requires this workaround.
> -                */
> -               val |=
> man_trk_ctl_single_full_frame_bit_get(dev_priv);
> +               val |= man_trk_ctl_continuos_full_frame(dev_priv);
>                 goto exit;
>         }
>  
> @@ -2306,16 +2295,10 @@ static void _psr_flush_handle(struct intel_dp
> *intel_dp)
>                 if (intel_dp->psr.psr2_sel_fetch_cff_enabled) {
>                         /* can we turn CFF off? */
>                         if (intel_dp->psr.busy_frontbuffer_bits == 0)
> {
> -                               u32 val =
> man_trk_ctl_enable_bit_get(dev_priv) |
> -                                        
> man_trk_ctl_partial_frame_bit_get(dev_priv) |
> -                                        
> man_trk_ctl_single_full_frame_bit_get(dev_priv);
> -
>                                 /*
>                                  * turn continuous full frame off and
> do a single
>                                  * full frame
>                                  */
> -                               intel_de_write(dev_priv,
> PSR2_MAN_TRK_CTL(intel_dp->psr.transcoder),
> -                                              val);
>                                 intel_de_write(dev_priv,
> CURSURFLIVE(intel_dp->psr.pipe), 0);
>                                 intel_dp-
> >psr.psr2_sel_fetch_cff_enabled = false;
>                         }
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
index 5b678916e6db..41b0718eb3a1 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -1480,13 +1480,6 @@  static u32 man_trk_ctl_enable_bit_get(struct drm_i915_private *dev_priv)
 		PSR2_MAN_TRK_CTL_ENABLE;
 }
 
-static u32 man_trk_ctl_single_full_frame_bit_get(struct drm_i915_private *dev_priv)
-{
-	return IS_ALDERLAKE_P(dev_priv) || DISPLAY_VER(dev_priv) >= 14 ?
-	       ADLP_PSR2_MAN_TRK_CTL_SF_SINGLE_FULL_FRAME :
-	       PSR2_MAN_TRK_CTL_SF_SINGLE_FULL_FRAME;
-}
-
 static u32 man_trk_ctl_partial_frame_bit_get(struct drm_i915_private *dev_priv)
 {
 	return IS_ALDERLAKE_P(dev_priv) || DISPLAY_VER(dev_priv) >= 14 ?
@@ -1510,7 +1503,7 @@  static void psr_force_hw_tracking_exit(struct intel_dp *intel_dp)
 			       PSR2_MAN_TRK_CTL(intel_dp->psr.transcoder),
 			       man_trk_ctl_enable_bit_get(dev_priv) |
 			       man_trk_ctl_partial_frame_bit_get(dev_priv) |
-			       man_trk_ctl_single_full_frame_bit_get(dev_priv));
+			       man_trk_ctl_continuos_full_frame(dev_priv));
 
 	/*
 	 * Display WA #0884: skl+
@@ -1624,11 +1617,7 @@  static void psr2_man_trk_ctl_calc(struct intel_crtc_state *crtc_state,
 	val |= man_trk_ctl_partial_frame_bit_get(dev_priv);
 
 	if (full_update) {
-		/*
-		 * Not applying Wa_14014971508:adlp as we do not support the
-		 * feature that requires this workaround.
-		 */
-		val |= man_trk_ctl_single_full_frame_bit_get(dev_priv);
+		val |= man_trk_ctl_continuos_full_frame(dev_priv);
 		goto exit;
 	}
 
@@ -2306,16 +2295,10 @@  static void _psr_flush_handle(struct intel_dp *intel_dp)
 		if (intel_dp->psr.psr2_sel_fetch_cff_enabled) {
 			/* can we turn CFF off? */
 			if (intel_dp->psr.busy_frontbuffer_bits == 0) {
-				u32 val = man_trk_ctl_enable_bit_get(dev_priv) |
-					  man_trk_ctl_partial_frame_bit_get(dev_priv) |
-					  man_trk_ctl_single_full_frame_bit_get(dev_priv);
-
 				/*
 				 * turn continuous full frame off and do a single
 				 * full frame
 				 */
-				intel_de_write(dev_priv, PSR2_MAN_TRK_CTL(intel_dp->psr.transcoder),
-					       val);
 				intel_de_write(dev_priv, CURSURFLIVE(intel_dp->psr.pipe), 0);
 				intel_dp->psr.psr2_sel_fetch_cff_enabled = false;
 			}