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

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

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.

> see where msm_dp_display_set_stream_info() will be called from, but
Sorry,  you are right. The call sites are introduced in a later patch. I 
will move this helper to the patch where it is first used.> isn't it a 
part of the calculated state? if so, all the slot allocation
> should be stored in the state structures rather than storing them
> directly in the private structure.
> 
>> +
>>   struct msm_dp_ctrl_private {
>>   	struct msm_dp_ctrl msm_dp_ctrl;
>>   	struct drm_device *drm_dev;
>> @@ -153,6 +160,8 @@ struct msm_dp_ctrl_private {
>>   	bool link_clks_on;
>>   	bool stream_clks_on[DP_STREAM_MAX];
>>   	bool mst_active;
>> +
>> +	struct msm_dp_mst_ch_slot_info mst_ch_info[DP_STREAM_MAX];
>>   };
>>   
>>   static inline u32 msm_dp_read_ahb(const struct msm_dp_ctrl_private *ctrl, u32 offset)
>