Re: [PATCH 2/4] drm/msm/dp: Return early from atomic_enable() if ST_DISCONNECT_PENDING

[Jessica Zhang <jessica.zhang@oss.qualcomm.com>]

On 5/30/2025 9:04 AM, Dmitry Baryshkov wrote:
> On Fri, 30 May 2025 at 02:15, Jessica Zhang
> <jessica.zhang@oss.qualcomm.com> wrote:
>>
>> From: Abhinav Kumar <quic_abhinavk@quicinc.com>
>>
>> The checks in msm_dp_bridge_atomic_enable() for making sure that we are in
>> ST_DISPLAY_OFF OR ST_MAINLINK_READY seem redundant.
>>
>> DRM fwk shall not issue any commits if state is not ST_MAINLINK_READY as
>> msm_dp's atomic_check callback returns a failure if state is not
>> ST_MAINLINK_READY.
> 
> What if the state changes between atomic_check() and atomic_enable()?
> There are no locks, cable unplugging is async, so it's perfectly
> possible.
> 
>>
>> For the ST_DISPLAY_OFF check, its mainly to guard against a scenario that
>> there is an atomic_enable() without a prior atomic_disable() which once
>> again should not really happen.
>>
>> Since it's still possible for the state machine to transition to
>> ST_DISCONNECT_PENDING between atomic_check() and atomic_commit(), change
>> this check to return early if hpd_state is ST_DISCONNECT_PENDING.
> 
> Can we really, please, drop the state machine? I had other plans for
> the next week, but maybe I should just do it, so that by the end of
> 6.17 cycle we can have a merged, stable and working solution? This
> topic has been lingering for months without any actual change.

FWIW, I'm currently working on the state machine rework by the middle of 
next week.

I'm anticipating that the rework itself will take some time to get 
merged, so didn't want MST to get delayed more by this series.

Thanks,

Jessica Zhang

> 
>>
>> Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
>> Signed-off-by: Jessica Zhang <jessica.zhang@oss.qualcomm.com>
>> ---
>>   drivers/gpu/drm/msm/dp/dp_display.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
>> index 1d7cda62d5fb..f2820f06f5dc 100644
>> --- a/drivers/gpu/drm/msm/dp/dp_display.c
>> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
>> @@ -1512,7 +1512,7 @@ void msm_dp_bridge_atomic_enable(struct drm_bridge *drm_bridge,
>>          }
>>
>>          hpd_state = msm_dp_display->hpd_state;
>> -       if (hpd_state != ST_DISPLAY_OFF && hpd_state != ST_MAINLINK_READY) {
>> +       if (hpd_state == ST_DISCONNECT_PENDING) {
> 
> 
> 
>>                  mutex_unlock(&msm_dp_display->event_mutex);
>>                  return;
>>          }
>>
>> --
>> 2.49.0
>>
> 
>