Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [Critical] NULL pointer dereference in `amdgpu_dm_atomic_commit_tail` when changing the `Broadcast RGB` connector property. -- commit 25a82016e5e929302d7c5fe7edf379f4e6cec7a0 Author: Satyajit Roy <[email protected]> drm/amd/display: Honor Broadcast RGB for BT.2020 RGB output This patch updates amdgpu_dm_get_output_color_space() to apply the Broadcast RGB connector property for BT.2020 RGB output. It explicitly selects limited range when requested, fixing an issue where it always defaulted to full range. > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_connector.c > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_connector.c > index 8a35e4df13cbf..f2f6a2a0db99e 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_connector.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_connector.c > @@ -758,10 +758,14 @@ amdgpu_dm_get_output_color_space(const struct > dc_crtc_timing *dc_crtc_timing, > break; > case DRM_MODE_COLORIMETRY_BT2020_RGB: > case DRM_MODE_COLORIMETRY_BT2020_YCC: > - if (dc_crtc_timing->pixel_encoding == PIXEL_ENCODING_RGB) > - color_space = COLOR_SPACE_2020_RGB_FULLRANGE; > - else > + if (dc_crtc_timing->pixel_encoding == PIXEL_ENCODING_RGB) { > + if (connector_state->hdmi.broadcast_rgb == > DRM_HDMI_BROADCAST_RGB_LIMITED) > + color_space = COLOR_SPACE_2020_RGB_LIMITEDRANGE; > + else > + color_space = COLOR_SPACE_2020_RGB_FULLRANGE; > + } else { > color_space = COLOR_SPACE_2020_YCBCR_LIMITED; > + } > break; > case DRM_MODE_COLORIMETRY_DEFAULT: /* ITU601 */ [Severity: Critical] This is a pre-existing issue, but does changing the Broadcast RGB property crash the kernel via a NULL pointer dereference? When userspace changes the Broadcast RGB property on a connector, DRM core creates an atomic commit containing only the updated connector state. The AMD driver's amdgpu_dm_atomic_check() does not add the associated CRTC to the atomic state for these broadcast_rgb changes. This leads to a crash in drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c within amdgpu_dm_atomic_commit_tail(), where drm_atomic_get_new_crtc_state() returns NULL: if (acrtc) { new_crtc_state = drm_atomic_get_new_crtc_state(state, &acrtc->base); old_crtc_state = drm_atomic_get_old_crtc_state(state, &acrtc->base); } /* Skip any modesets/resets */ if (!acrtc || drm_atomic_crtc_needs_modeset(new_crtc_state)) continue; Since new_crtc_state is NULL, drm_atomic_crtc_needs_modeset() unconditionally dereferences it and crashes. Even if this did not crash (for example on a disabled CRTC where modeset might evaluate to false), execution proceeds to the fast-update path in the same function: if ((new_con_state->hdmi.broadcast_rgb != old_con_state->hdmi.broadcast_rgb) && (dm_old_crtc_state->stream->output_color_space != amdgpu_dm_get_output_color_space(&dm_new_crtc_state->stream->timing, new_con_state))) For a disabled CRTC, stream is NULL, which causes dm_new_crtc_state->stream and dm_old_crtc_state->stream to trigger a second unavoidable NULL pointer dereference. [ ... ] -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
