Re: [Intel-gfx] [PATCH] drm/i915/lnl: Remove watchdog timers for PSR

["Hogander, Jouni" <jouni.hogander@intel.com>]

On Fri, 2023-10-06 at 14:42 +0300, Mika Kahola wrote:
> Currently we are not using watchdog timers for PSR/PSR2.
> The patch disables these timers so they are not in use.
> 
> BSpec: 69895
> 
> Signed-off-by: Mika Kahola <mika.kahola@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_psr.c | 24 +++++++++++++++++-----
> --
>  1 file changed, 17 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> b/drivers/gpu/drm/i915/display/intel_psr.c
> index 850b11f20285..13b58dceb2bf 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -672,11 +672,15 @@ static void hsw_activate_psr1(struct intel_dp
> *intel_dp)
>         struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
>         enum transcoder cpu_transcoder = intel_dp->psr.transcoder;
>         u32 max_sleep_time = 0x1f;
> -       u32 val = EDP_PSR_ENABLE;
> +       u32 val = 0;

This is not related to the commit message. I.e. PSR1 is left disabled
completely for DISPLAY_VER >= 20.
>  
> -       val |=
> EDP_PSR_IDLE_FRAMES(psr_compute_idle_frames(intel_dp));
>  
> -       val |= EDP_PSR_MAX_SLEEP_TIME(max_sleep_time);
> +       if (DISPLAY_VER(dev_priv) < 20) {
> +               val =  EDP_PSR_ENABLE;
> +               val |= EDP_PSR_MAX_SLEEP_TIME(max_sleep_time);
> +       }
> +
> +       val |=
> EDP_PSR_IDLE_FRAMES(psr_compute_idle_frames(intel_dp));
>         if (IS_HASWELL(dev_priv))
>                 val |= EDP_PSR_MIN_LINK_ENTRY_TIME_8_LINES;
>  
> @@ -1398,10 +1402,16 @@ static void intel_psr_enable_source(struct
> intel_dp *intel_dp,
>          * runtime_pm besides preventing  other hw tracking issues
> now we
>          * can rely on frontbuffer tracking.
>          */
> -       mask = EDP_PSR_DEBUG_MASK_MEMUP |
> -              EDP_PSR_DEBUG_MASK_HPD |
> -              EDP_PSR_DEBUG_MASK_LPSP |
> -              EDP_PSR_DEBUG_MASK_MAX_SLEEP;
> +       if (DISPLAY_VER(dev_priv) >= 20)
> +               mask = EDP_PSR_DEBUG_MASK_MEMUP |
> +                      EDP_PSR_DEBUG_MASK_HPD |
> +                      EDP_PSR_DEBUG_MASK_LPSP;
> +       else
> +               mask = EDP_PSR_DEBUG_MASK_MEMUP |
> +                      EDP_PSR_DEBUG_MASK_HPD |
> +                      EDP_PSR_DEBUG_MASK_LPSP |
> +                      EDP_PSR_DEBUG_MASK_MAX_SLEEP;

I would choose this:

       mask = EDP_PSR_DEBUG_MASK_MEMUP |
              EDP_PSR_DEBUG_MASK_HPD |
              EDP_PSR_DEBUG_MASK_LPSP;

       if (DISPLAY_VER(dev_priv) < 20)
               mask |= EDP_PSR_DEBUG_MASK_MAX_SLEEP;

BR,

Jouni Högander

> +
>  
>         /*
>          * No separate pipe reg write mask on hsw/bdw, so have to
> unmask all