On 2022-01-18 11:49 a.m., Kazlauskas, Nicholas wrote:
On 1/18/2022 11:40 AM, Rodrigo Siqueira wrote:
From: Leo Li <sunpeng.li@xxxxxxx>
[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.
Hi Nick,
I'm wondering if we have a test that can reproduce the issue that you
described. For this specific patch, I ran it through our pre-submission
CI, and everything worked as expected, except the subtest
pipe-a-primary-size-* in the kms_plane_cursor. However, I submitted the
below IGT patch that fixed the issue:
https://patchwork.freedesktop.org/patch/469919/?series=98994&rev=1
(Note that the IGT fix is not related to the scenario you described.)
In summary, I do not see regressions in other tests, whereas this patch
fixes multiple CRC mismatches when running IGT (e.g., kms_plane and
plane_scaling).
Do you recommend running some specific test to reproduce the issue?
Maybe some particular set of steps that enable me to see the issue?
Ps.: Some IGT tests already compensate for the two extra frames (see
link [1]), this behavior also happens on other drivers such as i915, and
we still fail in those tests because we are skipping four frames without
this patch.
1.
https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/blob/master/tests/kms_plane.c#L612
Thanks
Siqueira
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@xxxxxxxxxxxx>
Cc: Hayden Goodfellow <hayden.goodfellow@xxxxxxx>
Cc: Harry Wentland <harry.wentland@xxxxxxx>
Cc: Nicholas Choi <Nicholas.Choi@xxxxxxx>
Signed-off-by: Leo Li <sunpeng.li@xxxxxxx>
Signed-off-by: Rodrigo Siqueira <Rodrigo.Siqueira@xxxxxxx>
---
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 */
