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

[Dmitry Baryshkov]

On Wed, Apr 01, 2026 at 02:55:32PM +0800, Yongxing Mou wrote:
> 
> 
> 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@xxxxxxxxxxx>
> > > > > 
> > > > > 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@xxxxxxxxxxx>
> > > > > Signed-off-by: Yongxing Mou <yongxing.mou@xxxxxxxxxxxxxxxx>
> > > > > ---
> > > > >   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.

Or you can point out the relevant part of the standard.

-- 
With best wishes
Dmitry