Re: [PATCH v2 2/2] drm/amd/display: close DDC on I2C engine setup failure

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Lockless access to the shared `pool->i2c_hw_buffer_in_use` flag allows concurrent HW I2C engine usage across different display connectors.
--

commit e10eae8a87c1382752a4ccbab3d8554652f1a66a
Author: NepNep7601 <neptune@imm0nv1nhtv.is-a.dev>

drm/amd/display: close DDC on I2C engine setup failure

This commit ensures the DDC pins are closed and the DDC pointer is cleared if
I2C hardware engine setup fails. This prevents the port from becoming
permanently blocked due to the pins remaining open.

> diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_i2c_hw.c b/drivers/gpu/drm/amd/display/dc/dce/dce_i2c_hw.c
> index 05892ab4529f6..e7a05494abab0 100644
> --- a/drivers/gpu/drm/amd/display/dc/dce/dce_i2c_hw.c
> +++ b/drivers/gpu/drm/amd/display/dc/dce/dce_i2c_hw.c
> @@ -485,6 +485,8 @@ struct dce_i2c_hw *acquire_i2c_hw_engine(
>  
>  	if (!setup_engine(dce_i2c_hw)) {
>  		release_engine(dce_i2c_hw);
> +		dal_ddc_close(dce_i2c_hw->ddc);
> +		dce_i2c_hw->ddc = NULL;
>  		return NULL;
>  	}
>  

[Severity: High]
This isn't a bug introduced by this patch, but is it possible for concurrent
I2C transfers to bypass the pool lock and corrupt the hardware buffer?

Earlier in acquire_i2c_hw_engine(), there is a lockless check of
the shared pool state:

	if (pool->i2c_hw_buffer_in_use || !is_engine_available(dce_i2c_hw))
		return NULL;

If two different I2C adapters initiate transfers concurrently, they are
serialized locally by their per-adapter I2C bus lock, but they can enter
acquire_i2c_hw_engine() simultaneously because they share the same GPU
resource pool.

Both threads could read pool->i2c_hw_buffer_in_use as false at the same
time, proceed to acquire their respective hardware engines, and then both
update the state locklessly at the end of this function:

	pool->i2c_hw_buffer_in_use = true;

Could this race condition be triggered by concurrent I2C accesses via
userspace tools (like ddcutil) or simultaneous hotplug events, leading to
concurrent access and I2C payload corruption?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260826170549.21985-1-neptune@imm0nv1nhtv.is-a.dev?part=2