Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] The HDMI atomic check phase unconditionally sets `scrambler_needed` for TMDS character rates > 340 MHz without verifying if the source and sink actually support scrambling, which can cause an ungraceful failure during the atomic commit phase. --
--- Patch [23]: [PATCH v10 23/69] drm/display: hdmi-state-helper: Set HDMI scrambling requirement --- commit e6622cfc80adce83ef4209e099003b9cb90bec11 Author: Cristian Ciocaltea <[email protected]> drm/display: hdmi-state-helper: Set HDMI scrambling requirement This commit centralizes the HDMI scrambling requirement check in the HDMI state helper. It sets drm_connector_hdmi_state.scrambler_needed to true when the computed TMDS character rate exceeds the HDMI 1.3 maximum limit. > diff --git a/drivers/gpu/drm/display/drm_hdmi_state_helper.c > b/drivers/gpu/drm/display/drm_hdmi_state_helper.c > index 3377ea9361209..9544eb8659aa8 100644 > --- a/drivers/gpu/drm/display/drm_hdmi_state_helper.c > +++ b/drivers/gpu/drm/display/drm_hdmi_state_helper.c > @@ -594,6 +594,9 @@ hdmi_compute_clock(const struct drm_connector *connector, > > conn_state->hdmi.tmds_char_rate = clock; > > + /* TODO: also check drm_display_info.hdmi.scdc.scrambling.low_rates */ > + conn_state->hdmi.scrambler_needed = (clock > > HDMI_1_3_TMDS_CHAR_RATE_MAX_HZ); > + [Severity: High] Does this code allow an atomic_check to succeed even when the source or sink does not actually support scrambling? In the KMS atomic framework, hardware capabilities must be validated during the atomic_check phase to prevent failures during atomic_commit. Here, we set scrambler_needed to true for clocks > 340 MHz, but we don't seem to verify if the sink or the source hardware actually supports scrambling. If a malformed EDID advertises a high TMDS clock but lacks SCDC scrambling support, or if a driver supports high clocks but fails to implement the scrambler hooks, the atomic check phase will pass. Later, when drm_connector_hdmi_enable_scrambling() is called during the commit phase, it will evaluate scrambler_needed and return -EINVAL. Failing during the atomic commit phase violates KMS rules and results in a broken display pipeline. Should hdmi_compute_clock() or hdmi_clock_valid() also verify drm_scdc_sink_supports_scrambling() and drm_connector_hdmi_scrambler_supported() before succeeding? > return 0; > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=23
