[Freedreno] [PATCH] drm/msm: Read frame_count and line_count even when disabled.

[Rob Clark]

On Wed, Aug 11, 2021 at 11:12 AM Mark Yacoub <markyacoub at chromium.org> wrote:
>
> From: Mark Yacoub <markyacoub at google.com>
>
> [why]
> Reading frame count register used to get the vblank counter, which calls
> dpu_encoder_phys to get the frame count. Even when it's disabled, the
> vblank counter (through frame count) should return a valid value for the
> count. An invalid value of 0, when compared to vblank->last (in
> drm_vblank.c::drm_update_vblank_count()) returns an invalid number that
> throws off the vblank counter for the lifetime of the process.
>
> Rationale:
> In drm_vblank.c::drm_update_vblank_count(), the new diff is calculated
> through:
> diff = (cur_vblank - vblank->last) & max_vblank_count;
> cur_vblank comes from: cur_vblank = __get_vblank_counter(dev, pipe);
> When the value is 0, diff results in a negative number (a very large
> number as it's unsigned), which inflates the vblank count when the diff
> is added to the current vblank->count.
>
> [How]
> Read frame_count register whether interface timing engine is enabled or
> not.
>
> Fixes: IGT:kms_flip::modeset-vs-vblank-race-interruptible
> Tested on ChromeOS Trogdor(msm)
>
> Signed-off-by: Mark Yacoub <markyacoub at chromium.org>

Reviewed-by: Rob Clark <robdclark at chromium.org>

But I suspect we may have a bit more work for the display-off case..
or at least I'm not seeing anything obviously doing a pm_runtime_get()
in this call path.

I'm also not sure if the line/frame-count registers loose state across
a suspend->resume cycle, it might be that we need to save/restore
these registers in the suspend/resume path?  Abhinav?

BR,
-R

> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 9 ++-------
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h | 2 +-
>  2 files changed, 3 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
> index 116e2b5b1a90f..c436d901629f3 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
> @@ -266,13 +266,8 @@ static void dpu_hw_intf_get_status(
>
>         s->is_en = DPU_REG_READ(c, INTF_TIMING_ENGINE_EN);
>         s->is_prog_fetch_en = !!(DPU_REG_READ(c, INTF_CONFIG) & BIT(31));
> -       if (s->is_en) {
> -               s->frame_count = DPU_REG_READ(c, INTF_FRAME_COUNT);
> -               s->line_count = DPU_REG_READ(c, INTF_LINE_COUNT);
> -       } else {
> -               s->line_count = 0;
> -               s->frame_count = 0;
> -       }
> +       s->frame_count = DPU_REG_READ(c, INTF_FRAME_COUNT);
> +       s->line_count = DPU_REG_READ(c, INTF_LINE_COUNT);
>  }
>
>  static u32 dpu_hw_intf_get_line_count(struct dpu_hw_intf *intf)
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h
> index 3568be80dab51..877ff48bfef04 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h
> @@ -41,7 +41,7 @@ struct intf_prog_fetch {
>  struct intf_status {
>         u8 is_en;               /* interface timing engine is enabled or not */
>         u8 is_prog_fetch_en;    /* interface prog fetch counter is enabled or not */
> -       u32 frame_count;        /* frame count since timing engine enabled */
> +       u32 frame_count; /* frame count since timing engine first enabled */
>         u32 line_count;         /* current line count including blanking */
>  };
>
> --
> 2.33.0.rc1.237.g0d66db33f3-goog
>