[PATCH] drm/amd/display: Copy crc_skip_count when duplicating CRTC state

[Kazlauskas, Nicholas]

On 1/18/2022 11:40 AM, Rodrigo Siqueira wrote:
> From: Leo Li <sunpeng.li at amd.com>
> 
> [Why]
> crc_skip_count is used to track how many frames to skip to allow the OTG
> CRC engine to "warm up" before it outputs correct CRC values.
> Experimentally, this seems to be 2 frames.
> 
> When duplicating CRTC states, this value was not copied to the
> duplicated state. Therefore, when this state is committed, we will
> needlessly wait 2 frames before outputing CRC values. Even if the CRC
> engine is already warmed up. >
> [How]
> Copy the crc_skip_count as part of dm_crtc_duplicate_state.

This likely introduces regressions.

Here's an example case where it can take two frames even after the CRTC 
is enabled:

1. VUPDATE is before line 0, in the front porch, counter=0
2. Flip arrives before VUPDATE is signaled, but does not finish 
programming until after VUPDATE point, counter=0.
3. Vblank counter increments, counter=1.
4. Flip programming finishes, counter=1.
5. OS delay happens, cursor programming is delayed, counter=1.
6. Cursor programming starts, counter=1.
7. VUPDATE fires, updating frame but missing cursor, counter=1.
8. Cursor programming finishes, counter=2.
9. Cursor programming pending for counter=2.

This is a little contrived, but I've seen something similar happen 
during IGT testing before.

This is because cursor update happens independent of the rest of plane 
programming and is tied to a separate lock. That lock part can't change 
due to potential for stuttering, but the first part could be fixed.

Regards,
Nicholas Kazlauskas

> 
> Cc: Mark Yacoub <markyacoub at chromium.org>
> Cc: Hayden Goodfellow <hayden.goodfellow at amd.com>
> Cc: Harry Wentland <harry.wentland at amd.com>
> Cc: Nicholas Choi <Nicholas.Choi at amd.com>
> 
> Signed-off-by: Leo Li <sunpeng.li at amd.com>
> Signed-off-by: Rodrigo Siqueira <Rodrigo.Siqueira at amd.com>
> ---
>   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 87299e62fe12..5482b0925396 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -6568,6 +6568,7 @@ dm_crtc_duplicate_state(struct drm_crtc *crtc)
>   	state->freesync_config = cur->freesync_config;
>   	state->cm_has_degamma = cur->cm_has_degamma;
>   	state->cm_is_degamma_srgb = cur->cm_is_degamma_srgb;
> +	state->crc_skip_count = cur->crc_skip_count;
>   	state->force_dpms_off = cur->force_dpms_off;
>   	/* TODO Duplicate dc_stream after objects are stream object is flattened */
>