* Re: [PATCH] drm/amd/display: add missing CRC1 window registers and masks for DCN3.2
2026-08-29 0:46 [PATCH] drm/amd/display: add missing CRC1 window registers and masks for DCN3.2 Mikhail Gavrilov
@ 2026-08-29 0:59 ` sashiko-bot
2026-08-29 8:59 ` [PATCH v2] drm/amd/display: add missing CRC engine 1 " Mikhail Gavrilov
1 sibling, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-08-29 0:59 UTC (permalink / raw)
To: Mikhail Gavrilov; +Cc: dri-devel
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] The patch adds CRC1 window control registers for DCN3.2 but completely omits the corresponding CRC1 data registers (`OTG_CRC1_DATA_RG`, `OTG_CRC1_DATA_B`) and their bitfield masks, leading to silent readout failures on engine 1.
--
commit 62dcf31f49526ec3cc11096d84f1d9181eb2d9a3
Author: Mikhail Gavrilov <mikhail.v.gavrilov@gmail.com>
drm/amd/display: add missing CRC1 window registers and masks for DCN3.2
This commit adds missing CRC1 window registers and masks for DCN3.2 to avoid
a WARN triggered by generic_reg_update_ex() asserting on zero masks. It
supplies the SRI_ARR and SF entries for the OTG_CRC1_WINDOWA/B_X/Y_CONTROL
registers.
> diff --git a/drivers/gpu/drm/amd/display/dc/optc/dcn32/dcn32_optc.h b/drivers/gpu/drm/amd/display/dc/optc/dcn32/dcn32_optc.h
> index 60c01ec28b653..f696e4ee3bc63 100644
> --- a/drivers/gpu/drm/amd/display/dc/optc/dcn32/dcn32_optc.h
> +++ b/drivers/gpu/drm/amd/display/dc/optc/dcn32/dcn32_optc.h
> @@ -146,6 +146,14 @@
> SF(OTG0_OTG_CRC0_WINDOWB_X_CONTROL, OTG_CRC0_WINDOWB_X_END, mask_sh),\
> SF(OTG0_OTG_CRC0_WINDOWB_Y_CONTROL, OTG_CRC0_WINDOWB_Y_START, mask_sh),\
> SF(OTG0_OTG_CRC0_WINDOWB_Y_CONTROL, OTG_CRC0_WINDOWB_Y_END, mask_sh),\
> + SF(OTG0_OTG_CRC1_WINDOWA_X_CONTROL, OTG_CRC1_WINDOWA_X_START, mask_sh),\
> + SF(OTG0_OTG_CRC1_WINDOWA_X_CONTROL, OTG_CRC1_WINDOWA_X_END, mask_sh),\
> + SF(OTG0_OTG_CRC1_WINDOWA_Y_CONTROL, OTG_CRC1_WINDOWA_Y_START, mask_sh),\
> + SF(OTG0_OTG_CRC1_WINDOWA_Y_CONTROL, OTG_CRC1_WINDOWA_Y_END, mask_sh),\
> + SF(OTG0_OTG_CRC1_WINDOWB_X_CONTROL, OTG_CRC1_WINDOWB_X_START, mask_sh),\
> + SF(OTG0_OTG_CRC1_WINDOWB_X_CONTROL, OTG_CRC1_WINDOWB_X_END, mask_sh),\
> + SF(OTG0_OTG_CRC1_WINDOWB_Y_CONTROL, OTG_CRC1_WINDOWB_Y_START, mask_sh),\
> + SF(OTG0_OTG_CRC1_WINDOWB_Y_CONTROL, OTG_CRC1_WINDOWB_Y_END, mask_sh),\
[Severity: High]
This is a pre-existing issue, but by adding the missing CRC1 window masks
here in OPTC_COMMON_MASK_SH_LIST_DCN3_2, the CRC1 data masks (CRC1_R_CR,
CRC1_G_Y, CRC1_B_CB) still appear to be missing.
When dc_stream_get_crc() is called with crc_window = 1 (such as when Secure
Display enables a second ROI), it calls into optc1_get_crc() which relies on
these missing definitions:
drivers/gpu/drm/amd/display/dc/optc/dcn10/dcn10_optc.c:optc1_get_crc() {
...
REG_GET_2(OTG_CRC1_DATA_RG,
CRC1_R_CR, r_cr,
CRC1_G_Y, g_y);
...
}
Should the corresponding SF masks for OTG_CRC1_DATA_RG and OTG_CRC1_DATA_B
also be added to fully restore the functional behavior of engine 1?
> SF(OTG0_OTG_TRIGA_MANUAL_TRIG, OTG_TRIGA_MANUAL_TRIG, mask_sh),\
> SF(GSL_SOURCE_SELECT, GSL0_READY_SOURCE_SEL, mask_sh),\
> SF(GSL_SOURCE_SELECT, GSL1_READY_SOURCE_SEL, mask_sh),\
> diff --git a/drivers/gpu/drm/amd/display/dc/resource/dcn32/dcn32_resource.h b/drivers/gpu/drm/amd/display/dc/resource/dcn32/dcn32_resource.h
> index 68e7140f1505d..30878aed61e12 100644
> --- a/drivers/gpu/drm/amd/display/dc/resource/dcn32/dcn32_resource.h
> +++ b/drivers/gpu/drm/amd/display/dc/resource/dcn32/dcn32_resource.h
> @@ -1054,6 +1054,10 @@ unsigned int dcn32_get_max_hw_cursor_size(const struct dc *dc,
> SRI_ARR(OTG_CRC0_WINDOWA_Y_CONTROL, OTG, inst), \
> SRI_ARR(OTG_CRC0_WINDOWB_X_CONTROL, OTG, inst), \
> SRI_ARR(OTG_CRC0_WINDOWB_Y_CONTROL, OTG, inst), \
> + SRI_ARR(OTG_CRC1_WINDOWA_X_CONTROL, OTG, inst), \
> + SRI_ARR(OTG_CRC1_WINDOWA_Y_CONTROL, OTG, inst), \
> + SRI_ARR(OTG_CRC1_WINDOWB_X_CONTROL, OTG, inst), \
> + SRI_ARR(OTG_CRC1_WINDOWB_Y_CONTROL, OTG, inst), \
[Severity: High]
This is a pre-existing issue, but similar to the missing masks, the data
registers for engine 1 appear to be completely omitted from REG_STRUCT.
Without the definitions for OTG_CRC1_DATA_RG and OTG_CRC1_DATA_B, attempting
to read from these registers in optc1_get_crc() will silently read from MMIO
offset 0 and return 0, which breaks CRC data readout for engine 1.
Should SRI_ARR entries for OTG_CRC1_DATA_RG and OTG_CRC1_DATA_B also be
added here?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260829004608.25252-1-mikhail.v.gavrilov@gmail.com?part=1
^ permalink raw reply [flat|nested] 4+ messages in thread* [PATCH v2] drm/amd/display: add missing CRC engine 1 registers and masks for DCN3.2
2026-08-29 0:46 [PATCH] drm/amd/display: add missing CRC1 window registers and masks for DCN3.2 Mikhail Gavrilov
2026-08-29 0:59 ` sashiko-bot
@ 2026-08-29 8:59 ` Mikhail Gavrilov
2026-08-29 9:14 ` sashiko-bot
1 sibling, 1 reply; 4+ messages in thread
From: Mikhail Gavrilov @ 2026-08-29 8:59 UTC (permalink / raw)
To: Harry Wentland, Leo Li, Alex Deucher
Cc: Wayne Lin, ChiaHsuan Chung, Roman Li, Rodrigo Siqueira,
Christian König, amd-gfx, dri-devel, linux-kernel,
sashiko-bot, Mikhail Gavrilov
Opening the CRTC CRC debugfs file on a DCN3.2 card triggers a WARN from
dc_helper.c on the first attempt after boot:
WARNING: drivers/gpu/drm/amd/amdgpu/../display/dc/dc_helper.c:53 at
set_reg_field_values.isra.0+0x30d/0x730 [amdgpu], CPU#1: cat/58468
Reproducer, on a fresh boot - the ASSERT is WARN_ON_ONCE, so it only fires
once per boot:
echo crtc > /sys/kernel/debug/dri/<dev>/crtc-0/crc/control
cat /sys/kernel/debug/dri/<dev>/crtc-0/crc/data > /dev/null
The WARN comes from the open(), not from the write to control:
crtc_crc_open() -> amdgpu_dm_crtc_set_crc_source() ->
amdgpu_dm_crtc_configure_crc_source() -> dc_stream_configure_crc() ->
optc1_configure_crc() -> generic_reg_update_ex() -> ASSERT(mask != 0).
Since commit ef45aaf73717 ("drm/amd/display: Configure all CRC engines in
pipe CRC source path") the pipe CRC source path iterates every CRC engine,
so crc_eng_inst = 1 now reaches optc1_configure_crc(). Its case 1 programs
OTG_CRC1_WINDOWA/WINDOWB_X/Y_CONTROL before enabling the engine, but DCN3.2
has neither the register entries nor the field masks for those four
registers, so the masks are zero and the ASSERT fires.
Engine 1 needs three sets of definitions to be usable: the enable bit,
the window registers that optc1_configure_crc() programs, and the data
registers that optc1_get_crc() reads back. DCN3.2 has only the first,
added by commit c79354d12cb2 ("drm/amd/display: Fix CRC engine 1
enable/disable on DCN3.1.2+"). As a result engine 1 is enabled with its
windows never programmed, and a read through optc1_get_crc() case 1 would
address OTG_CRC1_DATA_RG and OTG_CRC1_DATA_B at MMIO offset 0.
Add the remaining two sets. All six registers exist in hardware -
regOTG0_OTG_CRC1_WINDOWA_X_CONTROL is at 0x1b6f and
regOTG0_OTG_CRC1_DATA_RG at 0x1b73 in dcn_3_2_0_offset.h, with their
field masks in dcn_3_2_0_sh_mask.h.
Note that making case 1 return false instead would break CRC capture on
DCN3.2 entirely: the caller in amdgpu_dm_crtc_configure_crc_source() does
"ret = -EINVAL; goto unlock" on failure, so a card that warns but works
today would stop working.
The same entries are missing for DCN2.0, DCN3.0 and DCN3.1. I have no such
hardware and have not touched them.
Fixes: ef45aaf73717 ("drm/amd/display: Configure all CRC engines in pipe CRC source path")
Signed-off-by: Mikhail Gavrilov <mikhail.v.gavrilov@gmail.com>
Tested-by: Mikhail Gavrilov <mikhail.v.gavrilov@gmail.com>
---
v2: also add the CRC1 data registers and masks (OTG_CRC1_DATA_RG,
OTG_CRC1_DATA_B), spotted by Sashiko AI review. Engine 1 needs all
three sets to be usable, and v1 supplied only the windows.
Note that ordinary CRC capture through crtc-N/crc/data reads engine 0
only (hardcoded in amdgpu_dm_crc.c); the engine 1 readout path is
reached from the secure display code, which I am not able to exercise.
The six added SRI_ARR lines follow the space indentation used by every
line of the surrounding macro, hence the six checkpatch "no spaces at the
start of a line" warnings. Using tabs there would misalign them against
their neighbours. The added SF lines in dcn32_optc.h are tab indented,
matching that file.
.../gpu/drm/amd/display/dc/optc/dcn32/dcn32_optc.h | 11 +++++++++++
.../amd/display/dc/resource/dcn32/dcn32_resource.h | 6 ++++++
2 files changed, 17 insertions(+)
diff --git a/drivers/gpu/drm/amd/display/dc/optc/dcn32/dcn32_optc.h b/drivers/gpu/drm/amd/display/dc/optc/dcn32/dcn32_optc.h
index 60c01ec28b65..492760779226 100644
--- a/drivers/gpu/drm/amd/display/dc/optc/dcn32/dcn32_optc.h
+++ b/drivers/gpu/drm/amd/display/dc/optc/dcn32/dcn32_optc.h
@@ -146,6 +146,17 @@
SF(OTG0_OTG_CRC0_WINDOWB_X_CONTROL, OTG_CRC0_WINDOWB_X_END, mask_sh),\
SF(OTG0_OTG_CRC0_WINDOWB_Y_CONTROL, OTG_CRC0_WINDOWB_Y_START, mask_sh),\
SF(OTG0_OTG_CRC0_WINDOWB_Y_CONTROL, OTG_CRC0_WINDOWB_Y_END, mask_sh),\
+ SF(OTG0_OTG_CRC1_DATA_RG, CRC1_R_CR, mask_sh),\
+ SF(OTG0_OTG_CRC1_DATA_RG, CRC1_G_Y, mask_sh),\
+ SF(OTG0_OTG_CRC1_DATA_B, CRC1_B_CB, mask_sh),\
+ SF(OTG0_OTG_CRC1_WINDOWA_X_CONTROL, OTG_CRC1_WINDOWA_X_START, mask_sh),\
+ SF(OTG0_OTG_CRC1_WINDOWA_X_CONTROL, OTG_CRC1_WINDOWA_X_END, mask_sh),\
+ SF(OTG0_OTG_CRC1_WINDOWA_Y_CONTROL, OTG_CRC1_WINDOWA_Y_START, mask_sh),\
+ SF(OTG0_OTG_CRC1_WINDOWA_Y_CONTROL, OTG_CRC1_WINDOWA_Y_END, mask_sh),\
+ SF(OTG0_OTG_CRC1_WINDOWB_X_CONTROL, OTG_CRC1_WINDOWB_X_START, mask_sh),\
+ SF(OTG0_OTG_CRC1_WINDOWB_X_CONTROL, OTG_CRC1_WINDOWB_X_END, mask_sh),\
+ SF(OTG0_OTG_CRC1_WINDOWB_Y_CONTROL, OTG_CRC1_WINDOWB_Y_START, mask_sh),\
+ SF(OTG0_OTG_CRC1_WINDOWB_Y_CONTROL, OTG_CRC1_WINDOWB_Y_END, mask_sh),\
SF(OTG0_OTG_TRIGA_MANUAL_TRIG, OTG_TRIGA_MANUAL_TRIG, mask_sh),\
SF(GSL_SOURCE_SELECT, GSL0_READY_SOURCE_SEL, mask_sh),\
SF(GSL_SOURCE_SELECT, GSL1_READY_SOURCE_SEL, mask_sh),\
diff --git a/drivers/gpu/drm/amd/display/dc/resource/dcn32/dcn32_resource.h b/drivers/gpu/drm/amd/display/dc/resource/dcn32/dcn32_resource.h
index 68e7140f1505..fa8cc89cb3f7 100644
--- a/drivers/gpu/drm/amd/display/dc/resource/dcn32/dcn32_resource.h
+++ b/drivers/gpu/drm/amd/display/dc/resource/dcn32/dcn32_resource.h
@@ -1054,6 +1054,12 @@ unsigned int dcn32_get_max_hw_cursor_size(const struct dc *dc,
SRI_ARR(OTG_CRC0_WINDOWA_Y_CONTROL, OTG, inst), \
SRI_ARR(OTG_CRC0_WINDOWB_X_CONTROL, OTG, inst), \
SRI_ARR(OTG_CRC0_WINDOWB_Y_CONTROL, OTG, inst), \
+ SRI_ARR(OTG_CRC1_DATA_RG, OTG, inst), \
+ SRI_ARR(OTG_CRC1_DATA_B, OTG, inst), \
+ SRI_ARR(OTG_CRC1_WINDOWA_X_CONTROL, OTG, inst), \
+ SRI_ARR(OTG_CRC1_WINDOWA_Y_CONTROL, OTG, inst), \
+ SRI_ARR(OTG_CRC1_WINDOWB_X_CONTROL, OTG, inst), \
+ SRI_ARR(OTG_CRC1_WINDOWB_Y_CONTROL, OTG, inst), \
SR_ARR(GSL_SOURCE_SELECT, inst), \
SRI_ARR(OTG_TRIGA_MANUAL_TRIG, OTG, inst), \
SRI_ARR(OTG_GLOBAL_CONTROL1, OTG, inst), \
--
2.55.0
^ permalink raw reply related [flat|nested] 4+ messages in thread