Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] drm/i915/ddi: RMW race on XELPDP_PORT_BUF_CTL1 drops concurrent updates --
commit b719c1e4da553224ed64e6f2c9076a3b63adb0c1 Author: Ankit Nautiyal <[email protected]> drm/i915/ddi: Update Transcoder/DDI registers with the frl bits This commit updates the Transcoder and DDI/PORT registers for HDMI FRL mode. It sets the lane count, FRL shifter enable, and data width bits in both the DDI_BUF_CTL and PORT_BUF_CTL registers. > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c > b/drivers/gpu/drm/i915/display/intel_ddi.c > index a46ab5d3d70fc..576e667746782 100644 > --- a/drivers/gpu/drm/i915/display/intel_ddi.c > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c > @@ -3121,12 +3121,20 @@ static void intel_ddi_buf_disable(struct > intel_encoder *encoder, > { > struct intel_display *display = to_intel_display(encoder); > enum port port = encoder->port; > + u32 val; > > intel_de_rmw(display, DDI_BUF_CTL(port), DDI_BUF_CTL_ENABLE, 0); > > if (DISPLAY_VER(display) >= 14) > intel_wait_ddi_buf_idle(display, port); > > + /* Clear PORT_BUF_CTL HDMI FRL Shifter Enable to 0 */ > + if (crtc_state->frl.enable) { > + val = intel_de_read(display, XELPDP_PORT_BUF_CTL1(display, > port)); > + intel_de_write(display, XELPDP_PORT_BUF_CTL1(display, port), > + val & ~XELPDP_PORT_HDMI_FRL_SHFTR_EN); [Severity: High] Can this manual read-modify-write sequence race with concurrent register updates? Looking at intel_ddi_buf_disable(), the sequence uses intel_de_read() followed by intel_de_write() to clear the XELPDP_PORT_HDMI_FRL_SHFTR_EN bit in the XELPDP_PORT_BUF_CTL1 register. Because this manual sequence drops the uncore spinlock and DMC wakelock between the read and the write, it creates a window where asynchronous Type-C hotplug workers in intel_tc.c might concurrently modify shared bits in the same register, such as XELPDP_TCSS_POWER_REQUEST or XELPDP_TC_PHY_OWNERSHIP. Any concurrent update by a TC worker or another intel_de_rmw() operation during this window could be permanently overwritten and lost by the subsequent intel_de_write(), leading to potential display or Type-C PHY state corruption. Should this use intel_de_rmw() instead to guarantee atomicity and continuity of the uncore lock and DMC wakelock? > + } > + > mtl_ddi_disable_d2d(encoder); -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=8
