Re: [PATCH v2 08/56] drm/i915/ddi: Update Transcoder/DDI registers with the frl bits

[sashiko-bot@kernel.org]

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 <ankit.k.nautiyal@intel.com>

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/20261012035806.3361439-1-ankit.k.nautiyal@intel.com?part=8