Re: [PATCH RFC v3 5/8] drm/i915/display/dp: Use dp_connector helpers to expose link training state

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm/i915: lockdep splat and sleeping in atomic context via intel_dp_report_link_train
--

commit a5f66f1f6cf0713414f9ea60cce11fc6b162d6e4
Author: Kory Maincent <kory.maincent@bootlin.com>

drm/i915/display/dp: Use dp_connector helpers to expose link training state

Adopt the new DRM core DisplayPort helpers, which expose source and sink
capabilities along with the maximum and current link parameters to
userspace through the dp_link/ sysfs attributes.

[ ... ]

> diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> index cb92cff906146..c77cb124f1419 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c

[ ... ]

> @@ -1787,6 +1799,8 @@ void intel_dp_stop_link_train(struct intel_dp *intel_dp,
>  	intel_dp_program_link_training_pattern(intel_dp, crtc_state, DP_PHY_DPRX,
>  					       DP_TRAINING_PATTERN_DISABLE);
>  
> +	intel_dp_report_link_train(intel_dp);
> +

[Severity: High]
Does this new call introduce a lockdep splat and sleep in an atomic context?

intel_dp_stop_link_train() executes during encoder atomic enable callbacks as
part of the intel_atomic_commit_tail() sequence, which runs in an atomic
context.

The new intel_dp_report_link_train() delegates to the DRM core helper
drm_dp_set_cur_link_params():

void drm_dp_set_cur_link_params(...)
{
    ...
    drm_modeset_lock_assert_held(&connector->dev->mode_config.connection_mutex);

    scoped_guard(mutex, &connector->dp.mutex) {
    ...
        _link_rates = kzalloc_obj(*_link_rates);
    ...
}

During non-blocking commits, the commit tail runs asynchronously in a worker
thread after the initial IOCTL has dropped all modeset locks. This causes the
drm_modeset_lock_assert_held() to trigger a lockdep splat.

Furthermore, acquiring the sleeping mutex connector->dp.mutex and calling
kzalloc_obj() (which can sleep) in the atomic commit path violates DRM
subsystem invariants. Could this lead to kernel warnings and potential
deadlocks?

>  	if (intel_dp_is_uhbr(crtc_state)) {
>  		ret = poll_timeout_us(ret = intel_dp_128b132b_intra_hop(intel_dp, crtc_state),
>  				      ret == 0,

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261012-feat_link_cap-v3-0-7517f4aba0f3@bootlin.com?part=5