Re: [PATCH] drm/i915/display: Use PIPEDMC_FRMTMSTMP on display ver >= 30

[Gustavo Sousa <gustavo.sousa@intel.com>]

Suraj Kandpal <suraj.kandpal@intel.com> writes:

> Starting with display version 30, the per-pipe frame timestamp is read
> from the PIPEDMC register block (PIPEDMC_FRMTMSTMP) instead of the
> legacy PIPE_FRMTMSTMP MMIO. Extend PIPE_FRMTMSTMP() to take the display
> and select the appropriate register based on DISPLAY_VER(), and update
> all callers (intel_vblank, intel_initial_plane) accordingly.
>
> Bspec: 79482
> WA: 14022946399

Why is this workaround being mentioned here?

If this is part of the workaround implementation, we should use the proper
display workaround infra (intel_display_wa.*) and we probably don't need
to add this commit trailer IMO.

Is the idea to use the PIPEDMC_FRMTMSTMP register as an alternative for
the workaround in display IPs that support such a register? If so, I
think this alternative will not apply to previous display versions,
right?

Another important question is: is this register updated even when the
DMC is not loaded?

--
Gustavo Sousa

> Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display_regs.h  | 7 +++++--
>  drivers/gpu/drm/i915/display/intel_initial_plane.c | 4 ++--
>  drivers/gpu/drm/i915/display/intel_vblank.c        | 4 ++--
>  3 files changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_regs.h b/drivers/gpu/drm/i915/display/intel_display_regs.h
> index 4321f8b529da..579f802215d3 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_regs.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_regs.h
> @@ -3149,8 +3149,11 @@ enum skl_power_gate {
>  /* g4x+, except vlv/chv! */
>  #define _PIPE_FRMTMSTMP_A		0x70048
>  #define _PIPE_FRMTMSTMP_B		0x71048
> -#define PIPE_FRMTMSTMP(pipe)		\
> -	_MMIO_PIPE(pipe, _PIPE_FRMTMSTMP_A, _PIPE_FRMTMSTMP_B)
> +#define _PIPEDMC_FRMTMSTMP_A		0x5f0ac
> +#define _PIPEDMC_FRMTMSTMP_B		0x5f4ac
> +#define PIPE_FRMTMSTMP(display, pipe)	(DISPLAY_VER(display) >= 30 ? \
> +	_MMIO_PIPE(pipe, _PIPEDMC_FRMTMSTMP_A, _PIPEDMC_FRMTMSTMP_B) : \
> +	_MMIO_PIPE(pipe, _PIPE_FRMTMSTMP_A, _PIPE_FRMTMSTMP_B))
>  
>  /* g4x+, except vlv/chv! */
>  #define _PIPE_FLIPTMSTMP_A		0x7004C
> diff --git a/drivers/gpu/drm/i915/display/intel_initial_plane.c b/drivers/gpu/drm/i915/display/intel_initial_plane.c
> index 034fe199c2a1..004cbdb6be32 100644
> --- a/drivers/gpu/drm/i915/display/intel_initial_plane.c
> +++ b/drivers/gpu/drm/i915/display/intel_initial_plane.c
> @@ -34,9 +34,9 @@ void intel_initial_plane_vblank_wait(struct intel_crtc *crtc)
>  		return;
>  	}
>  
> -	start_ts = intel_de_read(display, PIPE_FRMTMSTMP(crtc->pipe));
> +	start_ts = intel_de_read(display, PIPE_FRMTMSTMP(display, crtc->pipe));
>  
> -	ret = poll_timeout_us(end_ts = intel_de_read(display, PIPE_FRMTMSTMP(crtc->pipe)),
> +	ret = poll_timeout_us(end_ts = intel_de_read(display, PIPE_FRMTMSTMP(display, crtc->pipe)),
>  			      end_ts != start_ts, 1000, 1000 * 1000, false);
>  	if (ret)
>  		drm_warn(display->drm, "[CRTC:%d:%s] early vblank wait timed out\n",
> diff --git a/drivers/gpu/drm/i915/display/intel_vblank.c b/drivers/gpu/drm/i915/display/intel_vblank.c
> index 28d81199792e..52ff47936f9e 100644
> --- a/drivers/gpu/drm/i915/display/intel_vblank.c
> +++ b/drivers/gpu/drm/i915/display/intel_vblank.c
> @@ -157,7 +157,7 @@ static u32 intel_crtc_scanlines_since_frame_timestamp(struct intel_crtc *crtc)
>  		 * is sampled at every start of vertical blank.
>  		 */
>  		scan_prev_time = intel_de_read_fw(display,
> -						  PIPE_FRMTMSTMP(crtc->pipe));
> +						  PIPE_FRMTMSTMP(display, crtc->pipe));
>  
>  		/*
>  		 * The TIMESTAMP_CTR register has the current
> @@ -166,7 +166,7 @@ static u32 intel_crtc_scanlines_since_frame_timestamp(struct intel_crtc *crtc)
>  		scan_curr_time = intel_de_read_fw(display, IVB_TIMESTAMP_CTR);
>  
>  		scan_post_time = intel_de_read_fw(display,
> -						  PIPE_FRMTMSTMP(crtc->pipe));
> +						  PIPE_FRMTMSTMP(display, crtc->pipe));
>  	} while (scan_post_time != scan_prev_time);
>  
>  	return div_u64(mul_u32_u32(scan_curr_time - scan_prev_time,
> -- 
> 2.34.1