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

[Yongxing Mou <yongxing.mou@oss.qualcomm.com>]

On 4/1/2026 2:47 PM, Dmitry Baryshkov wrote:
> 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.
> 
Sure. Until I can find more convincing evidence, I will make the changes 
as requested.