Message-ID: <07f3a522-0b38-45bd-bb93-a5dc7c622d12@quicinc.com>
Date: Wed, 11 Jun 2025 19:39:24 +0800
From: Yongxing Mou <quic_yongmou@...cinc.com>
To: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
CC: Rob Clark <robin.clark@....qualcomm.com>,
        Abhinav Kumar
	<abhinav.kumar@...ux.dev>,
        Jessica Zhang <jessica.zhang@....qualcomm.com>,
        Sean Paul <sean@...rly.run>,
        Marijn Suijten <marijn.suijten@...ainline.org>,
        David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
        <linux-arm-msm@...r.kernel.org>, <dri-devel@...ts.freedesktop.org>,
        <freedreno@...ts.freedesktop.org>, <linux-kernel@...r.kernel.org>,
        "Abhinav
 Kumar" <quic_abhinavk@...cinc.com>
Subject: Re: [PATCH v2 28/38] drm/msm/dp: add dp_mst_drm to manage DP MST
 bridge operations



On 2025/6/9 23:57, Dmitry Baryshkov wrote:
> On Mon, Jun 09, 2025 at 08:21:47PM +0800, Yongxing Mou wrote:
>> From: Abhinav Kumar <quic_abhinavk@...cinc.com>
>>
>> Add a new file dp_mst_drm to manage the DP MST bridge operations
>> similar to the dp_drm file which manages the SST bridge operations.
>> Each MST encoder creates one bridge and each bridge is bound to its
>> own dp_panel abstraction to manage the operations of its pipeline.
> 
> I don't see this patch being improved from v1. Please follow the
> feedback or provide a _good_ reason to ignore it.
> 
Sorry for that, will explain one by one later..
>>
>> Signed-off-by: Abhinav Kumar <quic_abhinavk@...cinc.com>
>> Signed-off-by: Yongxing Mou <quic_yongmou@...cinc.com>
>> ---
>>   drivers/gpu/drm/msm/Makefile        |   3 +-
>>   drivers/gpu/drm/msm/dp/dp_display.h |   3 +
>>   drivers/gpu/drm/msm/dp/dp_mst_drm.c | 501 ++++++++++++++++++++++++++++++++++++
>>   drivers/gpu/drm/msm/dp/dp_mst_drm.h | 100 +++++++
>>   4 files changed, 606 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
>> index 7a2ada6e2d74a902879e4f12a78ed475e5209ec2..d317e5ecda28dfd708ccdc5d3d27d4cf0b78b9d5 100644
>> --- a/drivers/gpu/drm/msm/Makefile
>> +++ b/drivers/gpu/drm/msm/Makefile
>> @@ -142,7 +142,8 @@ msm-display-$(CONFIG_DRM_MSM_DP)+= dp/dp_aux.o \
>>   	dp/dp_link.o \
>>   	dp/dp_panel.o \
>>   	dp/dp_audio.o \
>> -	dp/dp_utils.o
>> +	dp/dp_utils.o \
>> +	dp/dp_mst_drm.o
>>   
>>   msm-display-$(CONFIG_DRM_MSM_HDMI_HDCP) += hdmi/hdmi_hdcp.o
>>   
>> diff --git a/drivers/gpu/drm/msm/dp/dp_display.h b/drivers/gpu/drm/msm/dp/dp_display.h
>> index 85eaa55fdcb7d9d8713849ec64a2cc9b08924425..9ca045ed2b4f1a9bc3254b733d1ce5332ebaba96 100644
>> --- a/drivers/gpu/drm/msm/dp/dp_display.h
>> +++ b/drivers/gpu/drm/msm/dp/dp_display.h
>> @@ -7,6 +7,7 @@
>>   #define _DP_DISPLAY_H_
>>   
>>   #include "dp_panel.h"
>> +#include "dp_mst_drm.h"
>>   #include "disp/msm_disp_snapshot.h"
>>   
>>   #define DP_MAX_PIXEL_CLK_KHZ	675000
>> @@ -24,6 +25,8 @@ struct msm_dp {
>>   	bool is_edp;
>>   	bool internal_hpd;
>>   
>> +	struct msm_dp_mst *msm_dp_mst;
>> +
>>   	struct msm_dp_audio *msm_dp_audio;
>>   	bool psr_supported;
>>   };
>> diff --git a/drivers/gpu/drm/msm/dp/dp_mst_drm.c b/drivers/gpu/drm/msm/dp/dp_mst_drm.c
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..a3ea34ae63511db0ac920cbeebe30c4e2320b8c4
>> --- /dev/null
>> +++ b/drivers/gpu/drm/msm/dp/dp_mst_drm.c
>> @@ -0,0 +1,501 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Copyright (c) 2025 Qualcomm Innovation Center, Inc. All rights reserved.
>> + */
>> +
>> +/*
>> + * Copyright © 2014 Red Hat.
>> + *
>> + * Permission to use, copy, modify, distribute, and sell this software and its
>> + * documentation for any purpose is hereby granted without fee, provided that
>> + * the above copyright notice appear in all copies and that both that copyright
>> + * notice and this permission notice appear in supporting documentation, and
>> + * that the name of the copyright holders not be used in advertising or
>> + * publicity pertaining to distribution of the software without specific,
>> + * written prior permission.  The copyright holders make no representations
>> + * about the suitability of this software for any purpose.  It is provided "as
>> + * is" without express or implied warranty.
>> + *
>> + * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS SOFTWARE,
>> + * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN NO
>> + * EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY SPECIAL, INDIRECT OR
>> + * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE,
>> + * DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER
>> + * TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE
>> + * OF THIS SOFTWARE.
>> + */
>> +
>> +#include "dp_mst_drm.h"
>> +
>> +static struct drm_private_state *msm_dp_mst_duplicate_bridge_state(struct drm_private_obj *obj)
>> +{
>> +	struct msm_dp_mst_bridge_state *state;
>> +
>> +	state = kmemdup(obj->state, sizeof(*state), GFP_KERNEL);
>> +	if (!state)
>> +		return NULL;
>> +
>> +	__drm_atomic_helper_private_obj_duplicate_state(obj, &state->base);
>> +
>> +	return &state->base;
>> +}
>> +
>> +static void msm_dp_mst_destroy_bridge_state(struct drm_private_obj *obj,
>> +					    struct drm_private_state *state)
>> +{
>> +	struct msm_dp_mst_bridge_state *priv_state =
>> +		to_msm_dp_mst_bridge_priv_state(state);
>> +
>> +	kfree(priv_state);
>> +}
>> +
>> +static const struct drm_private_state_funcs msm_dp_mst_bridge_state_funcs = {
>> +	.atomic_duplicate_state = msm_dp_mst_duplicate_bridge_state,
>> +	.atomic_destroy_state = msm_dp_mst_destroy_bridge_state,
>> +};
>> +
>> +/**
>> + * dp_mst_find_vcpi_slots() - Find VCPI slots for this PBN value
>> + * @mgr: manager to use
>> + * @pbn: payload bandwidth to convert into slots.
>> + *
>> + * Calculate the number of VCPI slots that will be required for the given PBN
>> + * value.
>> + *
>> + * RETURNS:
>> + * The total slots required for this port, or error.
>> + */
>> +static int msm_dp_mst_find_vcpi_slots(struct drm_dp_mst_topology_mgr *mgr, int pbn)
>> +{
>> +	int num_slots;
>> +	struct drm_dp_mst_topology_state *state;
>> +
>> +	state = to_drm_dp_mst_topology_state(mgr->base.state);
>> +	num_slots = DIV_ROUND_UP(pbn, dfixed_trunc(state->pbn_div));
>> +
>> +	/* max. time slots - one slot for MTP header */
>> +	if (num_slots > 63)
>> +		return -ENOSPC;
>> +	return num_slots;
>> +}
>> +
>> +static void _msm_dp_mst_update_timeslots(struct msm_dp_mst *mst,
>> +					 struct msm_dp_mst_bridge *mst_bridge,
>> +					 struct drm_dp_mst_port *port)
> 
> As I wrote:
> 
> drm_dp_atomic_find_time_slots() should be called from atomic_check(),
> this function must be dropped.
> 
This func not call drm_dp_atomic_find_time_slots(). 
drm_dp_atomic_find_time_slots() called in msm_dp_mst_compute_config in 
the previous one patch. msm_dp_mst_compute_config is in atomic_check().
>> +{
>> +	int i;
>> +	struct msm_dp_mst_bridge *msm_dp_bridge;
>> +	struct drm_dp_mst_topology_state *mst_state;
>> +	struct drm_dp_mst_atomic_payload *payload;
>> +	int prev_start = 0;
>> +	int prev_slots = 0;
>> +
>> +	mst_state = to_drm_dp_mst_topology_state(mst->mst_mgr.base.state);
>> +	payload = drm_atomic_get_mst_payload_state(mst_state, port);
>> +
>> +	if (!payload) {
>> +		DRM_ERROR("mst bridge [%d] update_timeslots failed, null payload\n",
>> +			  mst_bridge->id);
>> +		return;
>> +	}
>> +
>> +	for (i = 0; i < mst->max_streams; i++) {
>> +		msm_dp_bridge = &mst->mst_bridge[i];
>> +		if (mst_bridge == msm_dp_bridge) {
>> +			/*
>> +			 * When a payload was removed make sure to move any payloads after it
>> +			 * to the left so all payloads are aligned to the left.
>> +			 */
> 
> :-(
> 
Emm,  will get back to this part later.
>> +			if (payload->vc_start_slot < 0) {
>> +				// cache the payload
>> +				prev_start = msm_dp_bridge->start_slot;
>> +				prev_slots = msm_dp_bridge->num_slots;
>> +				msm_dp_bridge->pbn = 0;
>> +				msm_dp_bridge->start_slot = 1;
>> +				msm_dp_bridge->num_slots = 0;
>> +				msm_dp_bridge->vcpi = 0;
>> +			} else { //add payload
>> +				msm_dp_bridge->pbn = payload->pbn;
>> +				msm_dp_bridge->start_slot = payload->vc_start_slot;
>> +				msm_dp_bridge->num_slots = payload->time_slots;
>> +				msm_dp_bridge->vcpi = payload->vcpi;
>> +			}
>> +		}
>> +	}
>> +
>> +	// Now commit all the updated payloads
>> +	for (i = 0; i < mst->max_streams; i++) {
>> +		msm_dp_bridge = &mst->mst_bridge[i];
>> +
>> +		//Shift payloads to the left if there was a removed payload.
>> +		if (payload->vc_start_slot < 0 && msm_dp_bridge->start_slot > prev_start)
>> +			msm_dp_bridge->start_slot -= prev_slots;
>> +
>> +		msm_dp_display_set_stream_info(mst->msm_dp, msm_dp_bridge->msm_dp_panel,
>> +					       msm_dp_bridge->id, msm_dp_bridge->start_slot,
>> +					       msm_dp_bridge->num_slots,
>> +					       msm_dp_bridge->pbn, msm_dp_bridge->vcpi);
>> +		drm_dbg_dp(mst->msm_dp->drm_dev,
>> +			   "conn:%d vcpi:%d start_slot:%d num_slots:%d, pbn:%d\n",
>> +			   DP_MST_CONN_ID(msm_dp_bridge), msm_dp_bridge->vcpi,
>> +			   msm_dp_bridge->start_slot,
>> +			   msm_dp_bridge->num_slots, msm_dp_bridge->pbn);
>> +	}
>> +}
>> +
>> +static int _msm_dp_mst_bridge_pre_enable_part1(struct msm_dp_mst_bridge *dp_bridge,
>> +					       struct drm_atomic_state *state)
>> +{
>> +	struct msm_dp *msm_dp = dp_bridge->display;
>> +	struct msm_dp_mst *mst = msm_dp->msm_dp_mst;
>> +	struct msm_dp_mst_connector *mst_conn = to_msm_dp_mst_connector(dp_bridge->connector);
>> +	struct drm_dp_mst_port *port = mst_conn->mst_port;
>> +	struct drm_dp_mst_topology_state *mst_state;
>> +	struct drm_dp_mst_atomic_payload *payload;
>> +	struct msm_dp_panel *dp_panel = mst_conn->dp_panel;
>> +	int pbn, slots;
>> +	int rc = 0;
>> +
>> +	mst_state = drm_atomic_get_new_mst_topology_state(state, &mst->mst_mgr);
>> +
>> +	pbn = drm_dp_calc_pbn_mode(dp_panel->msm_dp_mode.drm_mode.clock,
>> +				   (mst_conn->connector.display_info.bpc * 3) << 4);
>> +
>> +	slots = msm_dp_mst_find_vcpi_slots(&mst->mst_mgr, pbn);
>> +
>> +	drm_dbg_dp(msm_dp->drm_dev, "conn:%d pbn:%d, slots:%d\n", DP_MST_CONN_ID(dp_bridge),
>> +		   pbn, slots);
>> +
>> +	payload = drm_atomic_get_mst_payload_state(mst_state, port);
>> +	if (!payload || payload->time_slots <= 0) {
>> +		DRM_ERROR("time slots not allocated for conn:%d\n", DP_MST_CONN_ID(dp_bridge));
>> +		rc = -EINVAL;
>> +		return rc;
>> +	}
>> +
>> +	drm_dp_mst_update_slots(mst_state, DP_CAP_ANSI_8B10B);
>> +
>> +	rc = drm_dp_add_payload_part1(&mst->mst_mgr, mst_state, payload);
>> +	if (rc) {
>> +		DRM_ERROR("payload allocation failure for conn:%d\n", DP_MST_CONN_ID(dp_bridge));
>> +		return rc;
>> +	}
>> +
>> +	_msm_dp_mst_update_timeslots(mst, dp_bridge, port);
>> +
>> +	return rc;
>> +}
>> +
>> +static void _msm_dp_mst_bridge_pre_enable_part2(struct msm_dp_mst_bridge *dp_bridge,
>> +						struct drm_atomic_state *state)
>> +{
>> +	struct msm_dp *msm_dp = dp_bridge->display;
>> +	struct msm_dp_mst *mst = msm_dp->msm_dp_mst;
>> +	struct msm_dp_mst_connector *mst_conn = to_msm_dp_mst_connector(dp_bridge->connector);
>> +	struct drm_dp_mst_port *port = mst_conn->mst_port;
>> +	struct drm_dp_mst_topology_state *mst_state;
>> +	struct drm_dp_mst_atomic_payload *payload;
>> +
>> +	drm_dp_check_act_status(&mst->mst_mgr);
>> +
>> +	mst_state = to_drm_dp_mst_topology_state(mst->mst_mgr.base.state);
>> +	payload = drm_atomic_get_mst_payload_state(mst_state, port);
>> +
>> +	if (!payload) {
>> +		DRM_ERROR("mst bridge [%d] null payload\n", dp_bridge->id);
>> +		return;
>> +	}
>> +
>> +	if (!payload->port) {
>> +		DRM_ERROR("mst bridge [%d] null port\n", dp_bridge->id);
>> +		return;
>> +	}
>> +
>> +	if (!payload->port->connector) {
>> +		DRM_ERROR("mst bridge [%d] part-2 failed, null connector\n",
>> +			  dp_bridge->id);
>> +		return;
>> +	}
>> +
>> +	if (payload->vc_start_slot == -1) {
>> +		DRM_ERROR("mst bridge [%d] part-2 failed, payload alloc part 1 failed\n",
>> +			  dp_bridge->id);
>> +		return;
>> +	}
>> +
>> +	drm_dp_add_payload_part2(&mst->mst_mgr, payload);
>> +
>> +	drm_dbg_dp(msm_dp->drm_dev, "mst bridge [%d] _pre enable part-2 complete\n",
>> +		   dp_bridge->id);
>> +}
>> +
>> +static void _msm_dp_mst_bridge_pre_disable_part1(struct msm_dp_mst_bridge *dp_bridge,
>> +						 struct drm_atomic_state *state)
>> +{
>> +	struct msm_dp *msm_dp = dp_bridge->display;
>> +	struct msm_dp_mst *mst = msm_dp->msm_dp_mst;
>> +	struct msm_dp_mst_connector *mst_conn = to_msm_dp_mst_connector(dp_bridge->connector);
>> +	struct drm_dp_mst_port *port = mst_conn->mst_port;
>> +	struct drm_dp_mst_topology_state *old_mst_state;
>> +	struct drm_dp_mst_topology_state *new_mst_state;
>> +	const struct drm_dp_mst_atomic_payload *old_payload;
>> +	struct drm_dp_mst_atomic_payload *new_payload;
>> +
>> +	old_mst_state = drm_atomic_get_old_mst_topology_state(state, &mst->mst_mgr);
>> +
>> +	new_mst_state = drm_atomic_get_new_mst_topology_state(state, &mst->mst_mgr);
>> +
>> +	old_payload = drm_atomic_get_mst_payload_state(old_mst_state, port);
>> +	new_payload = drm_atomic_get_mst_payload_state(new_mst_state, port);
>> +
>> +	if (!old_payload || !new_payload) {
>> +		DRM_ERROR("mst bridge [%d] _pre disable part-1 failed, null payload\n",
>> +			  dp_bridge->id);
>> +		return;
>> +	}
>> +
>> +	drm_dp_remove_payload_part1(&mst->mst_mgr, new_mst_state, new_payload);
>> +	drm_dp_remove_payload_part2(&mst->mst_mgr, new_mst_state, old_payload, new_payload);
>> +
>> +	_msm_dp_mst_update_timeslots(mst, dp_bridge, port);
>> +
>> +	drm_dbg_dp(msm_dp->drm_dev, "mst bridge [%d] _pre disable part-1 complete\n",
>> +		   dp_bridge->id);
>> +}
>> +
>> +static void _msm_dp_mst_bridge_pre_disable_part2(struct msm_dp_mst_bridge *dp_bridge)
>> +{
>> +	struct msm_dp *msm_dp = dp_bridge->display;
>> +	struct msm_dp_mst *mst = msm_dp->msm_dp_mst;
>> +
>> +	drm_dp_check_act_status(&mst->mst_mgr);
>> +
>> +	drm_dbg_dp(msm_dp->drm_dev, "mst bridge [%d] _pre disable part-2 complete\n",
>> +		   dp_bridge->id);
>> +}
>> +
>> +static void msm_dp_mst_bridge_atomic_pre_enable(struct drm_bridge *drm_bridge,
>> +						struct drm_atomic_state *state)
>> +{
>> +	int rc = 0;
>> +	struct msm_dp_mst_bridge *bridge;
>> +	struct msm_dp *dp;
>> +	struct msm_dp_mst_bridge_state *msm_dp_bridge_state;
>> +	struct msm_dp_mst *dp_mst;
>> +
>> +	if (!drm_bridge) {
>> +		DRM_ERROR("Invalid params\n");
>> +		return;
>> +	}
>> +
>> +	bridge = to_msm_dp_mst_bridge(drm_bridge);
>> +	msm_dp_bridge_state = to_msm_dp_mst_bridge_state(bridge);
>> +	dp = bridge->display;
>> +	dp_mst = dp->msm_dp_mst;
>> +
>> +	/* to cover cases of bridge_disable/bridge_enable without modeset */
>> +	bridge->connector = msm_dp_bridge_state->connector;
>> +	bridge->msm_dp_panel = msm_dp_bridge_state->msm_dp_panel;
>> +
>> +	if (!bridge->connector) {
>> +		DRM_ERROR("Invalid connector\n");
>> +		return;
>> +	}
>> +
>> +	mutex_lock(&dp_mst->mst_lock);
>> +	msm_dp_display_atomic_prepare(dp);
>> +
>> +	rc = _msm_dp_mst_bridge_pre_enable_part1(bridge, state);
>> +	if (rc) {
>> +		DRM_ERROR("[%d] DP display pre-enable failed, rc=%d\n", bridge->id, rc);
>> +		msm_dp_display_unprepare(dp);
>> +		mutex_unlock(&dp_mst->mst_lock);
>> +		return;
>> +	}
>> +
>> +	msm_dp_display_enable_helper(dp, bridge->msm_dp_panel);
>> +
>> +	_msm_dp_mst_bridge_pre_enable_part2(bridge, state);
>> +
>> +	mutex_unlock(&dp_mst->mst_lock);
>> +
>> +	drm_dbg_dp(dp->drm_dev, "conn:%d mode:%s fps:%d vcpi:%d slots:%d to %d\n",
>> +		   DP_MST_CONN_ID(bridge), bridge->drm_mode.name,
>> +		   drm_mode_vrefresh(&bridge->drm_mode),
>> +		   bridge->vcpi, bridge->start_slot,
>> +		   bridge->start_slot + bridge->num_slots);
>> +}
>> +
>> +static void msm_dp_mst_bridge_atomic_disable(struct drm_bridge *drm_bridge,
>> +					     struct drm_atomic_state *state)
>> +{
>> +	struct msm_dp_mst_bridge *bridge;
>> +	struct msm_dp *dp;
>> +	struct msm_dp_mst *mst;
>> +
>> +	if (!drm_bridge) {
>> +		DRM_ERROR("Invalid params\n");
>> +		return;
>> +	}
>> +
>> +	bridge = to_msm_dp_mst_bridge(drm_bridge);
>> +	if (!bridge->connector) {
>> +		DRM_ERROR("Invalid connector\n");
>> +		return;
>> +	}
>> +
>> +	dp = bridge->display;
>> +	mst = dp->msm_dp_mst;
>> +
>> +	mutex_lock(&mst->mst_lock);
>> +
>> +	_msm_dp_mst_bridge_pre_disable_part1(bridge, state);
>> +
>> +	msm_dp_display_disable_helper(dp, bridge->msm_dp_panel);
>> +
>> +	_msm_dp_mst_bridge_pre_disable_part2(bridge);
>> +
>> +	mutex_unlock(&mst->mst_lock);
>> +
>> +	drm_dbg_dp(dp->drm_dev, "mst bridge:%d conn:%d disable complete\n", bridge->id,
>> +		   DP_MST_CONN_ID(bridge));
>> +}
>> +
>> +static void msm_dp_mst_bridge_atomic_post_disable(struct drm_bridge *drm_bridge,
>> +						  struct drm_atomic_state *state)
>> +{
>> +	int conn = 0;
>> +	struct msm_dp_mst_bridge *bridge;
>> +	struct msm_dp *dp;
>> +	struct msm_dp_mst *mst;
>> +
>> +	if (!drm_bridge) {
>> +		DRM_ERROR("Invalid params\n");
>> +		return;
>> +	}
>> +
>> +	bridge = to_msm_dp_mst_bridge(drm_bridge);
>> +	if (!bridge->connector) {
>> +		DRM_ERROR("Invalid connector\n");
>> +		return;
>> +	}
>> +
>> +	conn = DP_MST_CONN_ID(bridge);
>> +
>> +	dp = bridge->display;
>> +	mst = dp->msm_dp_mst;
>> +
>> +	mutex_lock(&mst->mst_lock);
>> +
>> +	msm_dp_display_atomic_post_disable_helper(dp, bridge->msm_dp_panel);
>> +
>> +	if (!dp->mst_active)
>> +		msm_dp_display_unprepare(dp);
>> +
>> +	bridge->connector = NULL;
>> +	bridge->msm_dp_panel =  NULL;
>> +
>> +	mutex_unlock(&mst->mst_lock);
>> +
>> +	drm_dbg_dp(dp->drm_dev, "mst bridge:%d conn:%d post disable complete\n",
>> +		   bridge->id, conn);
>> +}
>> +
>> +static void msm_dp_mst_bridge_mode_set(struct drm_bridge *drm_bridge,
>> +				       const struct drm_display_mode *mode,
>> +				       const struct drm_display_mode *adjusted_mode)
>> +{
>> +	struct msm_dp_mst_bridge *bridge;
>> +	struct msm_dp_mst_bridge_state *dp_bridge_state;
>> +	struct msm_dp *dp;
>> +	struct msm_dp_panel *msm_dp_panel;
>> +
>> +	if (!drm_bridge || !mode || !adjusted_mode) {
>> +		DRM_ERROR("Invalid params\n");
>> +		return;
>> +	}
>> +
>> +	bridge = to_msm_dp_mst_bridge(drm_bridge);
>> +
>> +	dp_bridge_state = to_msm_dp_mst_bridge_state(bridge);
>> +	bridge->connector = dp_bridge_state->connector;
>> +	bridge->msm_dp_panel = dp_bridge_state->msm_dp_panel;
>> +
>> +	msm_dp_panel = bridge->msm_dp_panel;
>> +	dp = bridge->display;
>> +
>> +	memcpy(&bridge->drm_mode, adjusted_mode, sizeof(bridge->drm_mode));
>> +	msm_dp_display_mode_set_helper(dp, mode, adjusted_mode, bridge->msm_dp_panel);
>> +	msm_dp_panel->pbn = drm_dp_calc_pbn_mode(msm_dp_panel->msm_dp_mode.drm_mode.clock,
>> +							  (msm_dp_panel->msm_dp_mode.bpp << 4));
>> +	drm_dbg_dp(dp->drm_dev, "mst bridge:%d conn:%d mode set complete %s\n", bridge->id,
>> +		   DP_MST_CONN_ID(bridge), mode->name);
>> +}
>> +
>> +/* DP MST Bridge APIs */
>> +static const struct drm_bridge_funcs msm_dp_mst_bridge_ops = {
>> +	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
>> +	.atomic_destroy_state   = drm_atomic_helper_bridge_destroy_state,
>> +	.atomic_reset           = drm_atomic_helper_bridge_reset,
>> +	.atomic_pre_enable   = msm_dp_mst_bridge_atomic_pre_enable,
>> +	.atomic_disable      = msm_dp_mst_bridge_atomic_disable,
>> +	.atomic_post_disable = msm_dp_mst_bridge_atomic_post_disable,
>> +	.mode_set     = msm_dp_mst_bridge_mode_set,
>> +};
>> +
>> +int msm_dp_mst_drm_bridge_init(struct msm_dp *dp, struct drm_encoder *encoder)
>> +{
>> +	int rc = 0;
>> +	struct msm_dp_mst_bridge *bridge = NULL;
>> +	struct msm_dp_mst_bridge_state *state;
>> +	struct drm_device *dev;
>> +	struct msm_dp_mst *mst = dp->msm_dp_mst;
>> +	int i;
>> +
>> +	for (i = 0; i < mst->max_streams; i++) {
>> +		if (!mst->mst_bridge[i].in_use) {
>> +			bridge = &mst->mst_bridge[i];
>> +			bridge->encoder = encoder;
>> +			bridge->in_use = true;
>> +			bridge->id = i;
>> +			break;
>> +		}
>> +	}
>> +
>> +	if (i == mst->max_streams) {
>> +		DRM_ERROR("mst supports only %d bridges\n", i);
>> +		rc = -EACCES;
>> +		goto end;
>> +	}
>> +
>> +	dev = dp->drm_dev;
>> +	bridge->display = dp;
>> +	bridge->base.funcs = &msm_dp_mst_bridge_ops;
>> +	bridge->base.encoder = encoder;
>> +	bridge->base.type = dp->connector_type;
>> +	bridge->base.ops = DRM_BRIDGE_OP_MODES;
>> +	drm_bridge_add(&bridge->base);
>> +
>> +	rc = drm_bridge_attach(encoder, &bridge->base, NULL, 0);
>> +	if (rc) {
>> +		DRM_ERROR("failed to attach bridge, rc=%d\n", rc);
>> +		goto end;
>> +	}
>> +
>> +	state = kzalloc(sizeof(*state), GFP_KERNEL);
>> +	if (!state) {
>> +		rc = -ENOMEM;
>> +		goto end;
>> +	}
>> +
>> +	drm_atomic_private_obj_init(dev, &bridge->obj,
>> +				    &state->base,
>> +				    &msm_dp_mst_bridge_state_funcs);
>> +
>> +	drm_dbg_dp(dp->drm_dev, "mst drm bridge init. bridge id:%d\n", i);
>> +
>> +	return 0;
>> +
>> +end:
>> +	return rc;
>> +}
>> diff --git a/drivers/gpu/drm/msm/dp/dp_mst_drm.h b/drivers/gpu/drm/msm/dp/dp_mst_drm.h
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..cdde59da7bb937ad67fa818a728082f8fd43a3a6
>> --- /dev/null
>> +++ b/drivers/gpu/drm/msm/dp/dp_mst_drm.h
>> @@ -0,0 +1,100 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +/*
>> + * Copyright (c) 2025 Qualcomm Innovation Center, Inc. All rights reserved.
>> + *
>> + * Permission to use, copy, modify, distribute, and sell this software and its
>> + * documentation for any purpose is hereby granted without fee, provided that
>> + * the above copyright notice appear in all copies and that both that copyright
>> + * notice and this permission notice appear in supporting documentation, and
>> + * that the name of the copyright holders not be used in advertising or
>> + * publicity pertaining to distribution of the software without specific,
>> + * written prior permission.  The copyright holders make no representations
>> + * about the suitability of this software for any purpose.  It is provided "as
>> + * is" without express or implied warranty.
>> + *
>> + * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS SOFTWARE,
>> + * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN NO
>> + * EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY SPECIAL, INDIRECT OR
>> + * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE,
>> + * DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER
>> + * TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE
>> + * OF THIS SOFTWARE.
>> + */
>> +
>> +#ifndef _DP_MST_DRM_H_
>> +#define _DP_MST_DRM_H_
>> +
>> +#include <linux/kernel.h>
>> +#include <linux/init.h>
>> +#include <linux/errno.h>
>> +#include <linux/version.h>
>> +
>> +#include <drm/drm_atomic_helper.h>
>> +#include <drm/drm_atomic.h>
>> +#include <drm/drm_bridge.h>
>> +#include <drm/drm_crtc.h>
>> +#include <drm/drm_fixed.h>
>> +#include <drm/drm_connector.h>
>> +#include <drm/display/drm_dp_helper.h>
>> +#include <drm/display/drm_dp_mst_helper.h>
>> +
>> +#include "dp_panel.h"
>> +#include "dp_display.h"
>> +
>> +#define DP_MST_CONN_ID(bridge) ((bridge)->connector ? \
>> +		(bridge)->connector->base.id : 0)
>> +
>> +struct msm_dp_mst_bridge {
>> +	struct drm_bridge base;
>> +	struct drm_private_obj obj;
>> +	u32 id;
>> +
>> +	bool in_use;
>> +
>> +	struct msm_dp *display;
>> +	struct drm_encoder *encoder;
>> +
>> +	struct drm_display_mode drm_mode;
>> +	struct drm_connector *connector;
>> +	struct msm_dp_panel *msm_dp_panel;
>> +
>> +	int vcpi;
>> +	int pbn;
>> +	int num_slots;
>> +	int start_slot;
> 
> All of these should go to the bridge_state or be dropped.
> 

Got it. will move to bridge_state.
>> +};
>> +
>> +struct msm_dp_mst_bridge_state {
>> +	struct drm_private_state base;
>> +	struct drm_connector *connector;
>> +	struct msm_dp_panel *msm_dp_panel;
>> +	int num_slots;
>> +};
>> +
>> +struct msm_dp_mst {
>> +	struct drm_dp_mst_topology_mgr mst_mgr;
>> +	struct msm_dp_mst_bridge *mst_bridge;
>> +	struct msm_dp *msm_dp;
>> +	u32 max_streams;
>> +	struct mutex mst_lock;
>> +};
>> +
>> +struct msm_dp_mst_connector {
>> +	struct drm_connector connector;
>> +	struct drm_dp_mst_port *mst_port;
>> +	struct msm_dp *msm_dp;
>> +	struct msm_dp_panel *dp_panel;
>> +};
>> +
>> +#define to_msm_dp_mst_bridge(x)     container_of((x), struct msm_dp_mst_bridge, base)
>> +#define to_msm_dp_mst_bridge_priv(x) \
>> +		container_of((x), struct msm_dp_mst_bridge, obj)
>> +#define to_msm_dp_mst_bridge_priv_state(x) \
>> +		container_of((x), struct msm_dp_mst_bridge_state, base)
>> +#define to_msm_dp_mst_bridge_state(x) \
>> +		to_msm_dp_mst_bridge_priv_state((x)->obj.state)
>> +#define to_msm_dp_mst_connector(x) \
>> +		container_of((x), struct msm_dp_mst_connector, connector)
> 
> Please check, which of these structures and conversion macros can go
> into the source file because they are not used outside of it.
> 
Got it.
>> +int msm_dp_mst_drm_bridge_init(struct msm_dp *dp, struct drm_encoder *encoder);
>> +
>> +#endif /* _DP_MST_DRM_H_ */
>>
>> -- 
>> 2.34.1
>>
> 

Powered by blists - more mailing lists