Re: [PATCH] drm/amd/display: add missing CRC1 window registers and masks for DCN3.2

[sashiko-bot@kernel.org]

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