[Intel-gfx] [PATCH] drm/i915: Fix idle_frames counter.

[Paulo Zanoni]

2015-12-01 14:54 GMT-02:00 Rodrigo Vivi <rodrigo.vivi at intel.com>:
> 'commit 97173eaf5 ("drm/i915: PSR: Increase idle_frames")' was a mistake.
> The special case it tried to cover was already being covered by
> the DP_PSR_NO_TRAIN_ON_EXIT. So this ended up duplicated.
>
> So, instead of reverting that let's take this opportunity and unify
> the idle_frame definition in a single place so we standardize the access
> and avoid room for that same mistake again.
>
> Few changes with this patch:
>
> 1. Instead of just respecting the VBT we set a
> global minumum with max(). So we are sure that we will avoid corner cases
> in case VBT is doing something we don't understand.
>
> 2. Instead of minimum 5 we use 6. When introducing the idle_frames += 4 case
> we considered that minimum was 2. All because the off-by-one issue.
>
> v2: Unified idle_frame definition.

Reducing from 4 different possibilities to 2 is good.

Reviewed-by: Paulo Zanoni <paulo.r.zanoni at intel.com>

>
> Cc: Paulo Zanoni <paulo.r.zanoni at intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> ---
>  drivers/gpu/drm/i915/intel_psr.c | 20 +++++++-------------
>  1 file changed, 7 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index 3a10263..f12fc56 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -267,23 +267,17 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp)
>         struct drm_i915_private *dev_priv = dev->dev_private;
>
>         uint32_t max_sleep_time = 0x1f;
> -       /* Lately it was identified that depending on panel idle frame count
> -        * calculated at HW can be off by 1. So let's use what came
> -        * from VBT + 1.
> -        * There are also other cases where panel demands at least 4
> -        * but VBT is not being set. To cover these 2 cases lets use
> -        * at least 5 when VBT isn't set to be on the safest side.
> +       /*
> +        * Let's respect VBT in case VBT asks a higher idle_frame value.
> +        * Let's use 6 as the minimum to cover all known cases including
> +        * the off-by-one issue that HW has in some cases. Also there are
> +        * cases where sink should be able to train
> +        * with the 5 or 6 idle patterns.
>          */
> -       uint32_t idle_frames = dev_priv->vbt.psr.idle_frames ?
> -                              dev_priv->vbt.psr.idle_frames + 1 : 5;
> +       uint32_t idle_frames = max(6, dev_priv->vbt.psr.idle_frames);
>         uint32_t val = 0x0;
>         const uint32_t link_entry_time = EDP_PSR_MIN_LINK_ENTRY_TIME_8_LINES;
>
> -       if (intel_dp->psr_dpcd[1] & DP_PSR_NO_TRAIN_ON_EXIT) {
> -               /* Sink should be able to train with the 5 or 6 idle patterns */
> -               idle_frames += 4;
> -       }
> -
>         I915_WRITE(EDP_PSR_CTL, val |
>                    (IS_BROADWELL(dev) ? 0 : link_entry_time) |
>                    max_sleep_time << EDP_PSR_MAX_SLEEP_TIME_SHIFT |
> --
> 2.4.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni