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 <[email protected]> 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/[email protected]?part=1
