Re: [PATCH v3 17/38] drm/msm/dp: add support to send ACT packets for MST

[Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>]

On 01/04/2026 09:44, Yongxing Mou wrote:
> 
> 
> On 8/26/2025 5:10 AM, Dmitry Baryshkov wrote:
>> On Mon, Aug 25, 2025 at 10:16:03PM +0800, Yongxing Mou wrote:
>>> From: Abhinav Kumar <quic_abhinavk@quicinc.com>
>>>
>>> Whenever virtual channel slot allocation changes, the DP
>>> source must send the action control trigger sequence to notify
>>> the sink about the same. This would be applicable during the
>>> start and stop of the pixel stream. Add the infrastructure
>>> to be able to send ACT packets for the DP controller when
>>> operating in MST mode.
>>>
>>> Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
>>> Signed-off-by: Yongxing Mou <yongxing.mou@oss.qualcomm.com>
>>> ---
>>>   drivers/gpu/drm/msm/dp/dp_ctrl.c    | 39 ++++++++++++++++++++++++++ 
>>> +++++++++--
>>>   drivers/gpu/drm/msm/dp/dp_ctrl.h    |  4 ++--
>>>   drivers/gpu/drm/msm/dp/dp_display.c |  3 ++-
>>>   drivers/gpu/drm/msm/dp/dp_display.h |  1 +
>>>   drivers/gpu/drm/msm/dp/dp_reg.h     |  2 ++
>>>   5 files changed, 44 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/ 
>>> dp/dp_ctrl.c
>>> index 
>>> 608a1a077301b2ef3c77c271d873bb4364abe779..16e5ed58e791971d5dca3077cbb77bfcc186505a 100644
>>> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
>>> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
>>> @@ -142,6 +142,7 @@ struct msm_dp_ctrl_private {
>>>       bool core_clks_on;
>>>       bool link_clks_on;
>>>       bool stream_clks_on[DP_STREAM_MAX];
>>> +    bool mst_active;
>>>   };
>>>   static inline u32 msm_dp_read_ahb(const struct msm_dp_ctrl_private 
>>> *ctrl, u32 offset)
>>> @@ -227,6 +228,32 @@ static int msm_dp_aux_link_configure(struct 
>>> drm_dp_aux *aux,
>>>       return 0;
>>>   }
>>> +void msm_dp_ctrl_mst_send_act(struct msm_dp_ctrl *msm_dp_ctrl)
>>> +{
>>> +    struct msm_dp_ctrl_private *ctrl;
>>> +    bool act_complete;
>>> +
>>> +    ctrl = container_of(msm_dp_ctrl, struct msm_dp_ctrl_private, 
>>> msm_dp_ctrl);
>>> +
>>> +    if (!ctrl->mst_active)
>>> +        return;
>>> +
>>> +    msm_dp_write_link(ctrl, REG_DP_MST_ACT, 0x1);
>>> +    /* make sure ACT signal is performed */
>>> +    wmb();
>>> +
>>> +    msleep(20); /* needs 1 frame time */
>>> +
>>> +    act_complete = msm_dp_read_link(ctrl, REG_DP_MST_ACT);
>>> +
>>> +    if (!act_complete)
>>> +        drm_dbg_dp(ctrl->drm_dev, "mst ACT trigger complete 
>>> SUCCESS\n");
>>> +    else
>>> +        drm_dbg_dp(ctrl->drm_dev, "mst ACT trigger complete failed\n");
>>
>> Shouldn't it return an error if the register dind't latch? Also,
>> shouldn't we set mst_active only if the write went through?
>>
> In some cases, MST still works correctly even when the ACT trigger 
> fails; here refer to the downstream implementation.

I don't think it is a good idea. It would be better to signal this to 
the user and rollback the MST configuration (as in the case of any other 
error).

I will change my mind if you point out i915, amdgpu or nouveau drivers 
ignoring the ACT issues.

-- 
With best wishes
Dmitry