Re: [PATCH v7 13/35] drm/msm/dp: add support for MST channel slot allocation

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

On 11/9/2026 3:20 AM, Dmitry Baryshkov wrote:
> On Thu, Oct 15, 2026 at 09:24:52PM +0800, Yongxing Mou wrote:
>>
>>
>> On 10/1/2026 6:52 PM, Dmitry Baryshkov wrote:
>>> On Tue, Sep 29, 2026 at 12:25:37PM +0800, Yongxing Mou wrote:
>>>> From: Abhinav Kumar <quic_abhinavk@quicinc.com>
>>>>
>>>> DP MST streams share 64 MTP slots in a time-multiplexed manner. Add
>>>> support for calculating the rate governor, slot allocation, and slot
>>>> reservation in the DP controller.
>>>>
>>>> Each MST stream can reserve its slots by calling
>>>> msm_dp_display_set_stream_info() from its bridge callbacks.
>>>>
>>>> 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    | 187 ++++++++++++++++++++++++++++++++++++
>>>>    drivers/gpu/drm/msm/dp/dp_ctrl.h    |   4 +
>>>>    drivers/gpu/drm/msm/dp/dp_display.c |  13 +++
>>>>    drivers/gpu/drm/msm/dp/dp_display.h |   2 +
>>>>    4 files changed, 206 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
>>>> index 929e0f94f8dd..02732bd4efa1 100644
>>>> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
>>>> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
>>>> @@ -83,6 +83,7 @@
>>>>    #define MR_LINK_PRBS7 0x100
>>>>    #define MR_LINK_CUSTOM80 0x200
>>>>    #define MR_LINK_TRAINING4  0x40
>>>> +#define DP_MAX_TIME_SLOTS 64
>>>>    enum {
>>>>    	DP_TRAINING_NONE,
>>>> @@ -119,6 +120,12 @@ struct msm_dp_vc_tu_mapping_table {
>>>>    	u8 tu_size_minus1;
>>>>    };
>>>> +struct msm_dp_mst_ch_slot_info {
>>>> +	u32 start_slot;
>>>> +	u32 tot_slots;
>>>> +	u32 pbn;
>>>> +};
>>>
>>> How other drivers handle slot programming? Is it something to be shared
>>> between the drivers? If so, maybe it would be better to update MST
>>> helpers. Or, asking differently, why do we store it this way? I don't
>> start_slot, tot_slots, and pbn come from the MST atomic state. The MSM DP
>> hardware uses start_slot and tot_slots to program the per-stream timeslot
>> registers, while pbn is used to calculate the rate governor. Therefore,
>> mst_ch_info[] is a cached copy added to make these values easily accessible
>> to dp_ctrl.c.
>>
>> Source hardware programming differs across vendors. Intel does not
>> explicitly program per-stream absolute start-slot or slot-mask registers
>> like QCOM does. Nouveau submits each head's allocation through its NVIF/VCPI
>> interface, while AMD converts the complete allocation table into SAT entries
>> and programs them into the source hardware. Therefore, DRM MST helpers can
>> handle the common payload allocation and slot-shifting logic, but source
>> hardware programming remains driver-specific.
>>
>> Without mst_ch_info[], we would need to construct a temporary complete
>> allocation table from the MST atomic state during stream enable/disable and
>> pass it through dp_display.c to dp_ctrl.c. Specifically, this would require
>> extending msm_dp_display_enable_helper(), msm_dp_display_disable_helper(),
>> and msm_dp_ctrl_on_stream(). msm_dp_ctrl_mst_timeslot_setup() would then use
>> this table to program the timeslot registers for all streams, while the
>> current stream's pbn would be passed to the rate governor calculation.
> 
> Would it be better if the MST helpers provided this table? Would it be
> useful for any other driver?
> 
I don't think this would be useful to other drivers. The table is 
indexed by the MSM source stream ID and is only needed because the MSM 
DP hardware requires an absolute start slot and a 64-bit slot mask per 
stream, as well as the PBN for the rate governor calculation.

Other MST drivers operate directly on 
drm_dp_mst_topology_state::payloads and don't need a 
source-stream-indexed representation. A common MST helper for this would 
therefore have only one user, while the hardware-specific translation 
would still remain in the MSM driver.

In the next version, I've dropped the cached mst_ch_info[] array 
following your earlier comment. dp_mst_drm.c now builds a small on-stack 
table from the MST payload states in the stream enable/disable paths, 
and dp_ctrl only translates that table into the hardware timeslot 
configuration and rate governor inputs.