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 <[email protected]>

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

Reply via email to