dri-devel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/amd/display: add missing CRC1 window registers and masks for DCN3.2
@ 2026-08-29  0:46 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
  0 siblings, 2 replies; 4+ messages in thread
From: Mikhail Gavrilov @ 2026-08-29  0:46 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,
	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.

Commit c79354d12cb2 ("drm/amd/display: Fix CRC engine 1 enable/disable on
DCN3.1.2+") added the OTG_CRC1_EN mask for DCN3.2 and guarded the enable
write, but not the window writes above it.  As a result engine 1 is now
enabled on DCN3.2 with its windows never programmed.

The registers exist in hardware - regOTG0_OTG_CRC1_WINDOWA_X_CONTROL is at
0x1b6f in dcn_3_2_0_offset.h with its field masks in dcn_3_2_0_sh_mask.h -
so add the missing SRI_ARR and SF entries.

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>
---

The four added SRI_ARR lines follow the space indentation used by every
line of the surrounding macro, hence the four 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.

Tested on a Radeon RX 7900 XTX (Navi 31, DCN3.2): after this patch the
reproducer above leaves dmesg clean, and CRC capture on engine 0 still
delivers a full 120 frames per second.

 drivers/gpu/drm/amd/display/dc/optc/dcn32/dcn32_optc.h    | 8 ++++++++
 .../drm/amd/display/dc/resource/dcn32/dcn32_resource.h    | 4 ++++
 2 files changed, 12 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..f696e4ee3bc6 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),\
 	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..30878aed61e1 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),                          \
       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

* 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

* Re: [PATCH v2] drm/amd/display: add missing CRC engine 1 registers and masks for DCN3.2
  2026-08-29  8:59 ` [PATCH v2] drm/amd/display: add missing CRC engine 1 " Mikhail Gavrilov
@ 2026-08-29  9:14   ` sashiko-bot
  0 siblings, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-08-29  9:14 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 resolves an MMIO offset 0 read/write bug and `ASSERT(mask != 0)` warning for DCN3.2 by adding missing CRC engine 1 register definitions, but deliberately leaves the exact same bug unpatched for DCN2.0, DCN3.0, and DCN3.1. This exposes those platforms to a potential hardware hang when userspace accesses the CRC debugfs.
--

commit 59349c8a9cf596f2dd8a832ebff31962e51092a1
Author: Mikhail Gavrilov <mikhail.v.gavrilov@gmail.com>

drm/amd/display: add missing CRC engine 1 registers and masks for DCN3.2

This commit fixes a WARN and MMIO offset 0 access issue when using the CRTC CRC
debugfs file on DCN3.2. It adds the missing CRC engine 1 registers and masks
for DCN3.2 hardware.

>     The same entries are missing for DCN2.0, DCN3.0 and DCN3.1.  I have no such
>     hardware and have not touched them.

[Severity: High]
This is a pre-existing issue, but does leaving DCN2.0, DCN3.0, and DCN3.1
unfixed expose those platforms to a potential hardware hang?

When a privileged user enables CRTC CRC capture via the debugfs interface,
it invokes amdgpu_dm_crtc_configure_crc_source() which unconditionally loops
over all MAX_CRC_WINDOW_NUM engines:

amdgpu_dm_crc.c:amdgpu_dm_crtc_configure_crc_source() {
    ...
        for (i = 0; i < MAX_CRC_WINDOW_NUM; i++) {
            if (!dc_stream_configure_crc(dc, stream, enable, enable, i)) {
    ...
}

For engine 1 on DCN2.0/3.0/3.1, optc1_configure_crc() unconditionally attempts
to program OTG_CRC1_WINDOWA_X_CONTROL and other engine 1 registers:

dcn10_optc.c:optc1_configure_crc() {
    ...
        REG_UPDATE_2(OTG_CRC1_WINDOWA_X_CONTROL,
                OTG_CRC1_WINDOWA_X_START, params->windowa_x_start,
                OTG_CRC1_WINDOWA_X_END, params->windowa_x_end);
    ...
}

And optc1_get_crc() attempts to read the CRC data:

dcn10_optc.c:optc1_get_crc() {
    ...
        REG_GET_2(OTG_CRC1_DATA_RG,
                  CRC1_R_CR, r_cr,
                  CRC1_G_Y, g_y);
    ...
}

Because these are missing in the ASIC's resource tables, the address evaluates
to 0, causing generic_reg_update_ex() to issue MMIO reads/writes to offset 0
and trigger ASSERT(mask != 0).

Could this missing register mapping lead to crashes or GPU hangs on these
platforms when userspace accesses the CRC debugfs?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260829085941.106318-1-mikhail.v.gavrilov@gmail.com?part=1

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-08-29  9:14 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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
2026-08-29  9:14   ` sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox