Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] State tracking bug in atomic disable skips PHY shutdown on initial modesets. --
commit 572f752e8a57b7a37e457765a056b711271ae975 Author: Cristian Ciocaltea <[email protected]> Subject: drm/bridge: dw-hdmi-qp: Add HDMI 2.0 SCDC scrambling support Enable HDMI 2.0 display modes (e.g. 4K@60Hz) by implementing SCDC scrambling and high TMDS clock ratio management for TMDS character rates exceeding the 340 MHz HDMI 1.4b limit. Reject modes requiring TMDS rates above 600 MHz since those require HDMI 2.1 FRL. > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c > b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c > index a0613f0d2ebcf..07b274f76e53e 100644 > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c [ ... ] > @@ -752,26 +751,33 @@ static void dw_hdmi_qp_bridge_atomic_enable(struct > drm_bridge *bridge, > { > struct dw_hdmi_qp *hdmi = bridge->driver_private; > struct drm_connector_state *conn_state; > - struct drm_connector *connector; > unsigned int op_mode; > + int ret; > > - connector = drm_atomic_get_new_connector_for_encoder(state, > bridge->encoder); > - if (WARN_ON(!connector)) > + hdmi->curr_conn = drm_atomic_get_new_connector_for_encoder(state, > + > bridge->encoder); [ ... ] > @@ -787,8 +793,14 @@ static void dw_hdmi_qp_bridge_atomic_disable(struct > drm_bridge *bridge, > { > struct dw_hdmi_qp *hdmi = bridge->driver_private; > > + if (!hdmi->curr_conn) > + return; > + [Severity: Medium] Will this early return skip the PHY disable sequence during initial modesets? If the DRM core determines the encoder is active on boot and needs to be disabled for a new modeset, it calls dw_hdmi_qp_bridge_atomic_disable(). Because dw_hdmi_qp_bridge_atomic_enable() hasn't run yet in this boot cycle, hdmi->curr_conn will be NULL. This causes the early return to trigger, skipping the hdmi->phy.ops->disable() call below. Could this leave the hardware in an inconsistent state, causing PLL lock failures or display glitches when atomic_enable() tries to re-initialize an already running PHY with new parameters? To avoid caching unreferenced pointers like curr_conn across state changes, should the old connector be extracted directly from the atomic state using drm_atomic_get_old_connector_for_encoder() instead? > hdmi->tmds_char_rate = 0; > > + drm_connector_hdmi_disable_scrambling(hdmi->curr_conn); > + > + hdmi->curr_conn = NULL; > hdmi->phy.ops->disable(hdmi, hdmi->phy.data); > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=19
