Re: [PATCH v3 30/38] drm/msm/dp: add connector abstraction for DP MST

[Dmitry Baryshkov]

On Thu, Apr 09, 2026 at 12:01:58PM +0800, Yongxing Mou wrote:
> 
> 
> On 8/27/2025 2:31 AM, Dmitry Baryshkov wrote:
> > On Mon, Aug 25, 2025 at 10:16:16PM +0800, Yongxing Mou wrote:
> > > From: Abhinav Kumar <quic_abhinavk@xxxxxxxxxxx>
> > > 
> > > Add connector abstraction for the DP MST. Each MST encoder
> > > is connected through a DRM bridge to a MST connector and each
> > > MST connector has a DP panel abstraction attached to it.
> > 
> > What's the functionality split between the connector and bridge? Please
> > explain it here. Do we really need a bridge if we have a non-trivial
> > connector implementation?
> > 
> MST connector only for MST API calls/detect/get_modes/get encoder, and the
> bridge handles display enable/disable, hotplug, mode set....
> > > 
> > > Signed-off-by: Abhinav Kumar <quic_abhinavk@xxxxxxxxxxx>
> > > Signed-off-by: Yongxing Mou <yongxing.mou@xxxxxxxxxxxxxxxx>
> > > ---
> > >   drivers/gpu/drm/msm/dp/dp_mst_drm.c | 391 +++++++++++++++++++++++++++++++++++-
> > >   drivers/gpu/drm/msm/dp/dp_mst_drm.h |   3 +
> > >   2 files changed, 393 insertions(+), 1 deletion(-)
> > > +
> > > +	return status;
> > > +}
> > > +
> > > +static int msm_dp_mst_connector_get_modes(struct drm_connector *connector)
> > > +{
> > > +	struct msm_dp_mst_connector *mst_conn = to_msm_dp_mst_connector(connector);
> > > +	struct msm_dp *dp_display = mst_conn->msm_dp;
> > > +	struct msm_dp_mst *mst = dp_display->msm_dp_mst;
> > > +	const struct drm_edid *drm_edid;
> > > +
> > > +	if (drm_connector_is_unregistered(&mst_conn->connector))
> > > +		return drm_edid_connector_update(connector, NULL);
> > 
> > Is there a need for this? I don't see a check in nouveau code.
> > 
> Okay.. i see in intel's code..
> https://elixir.bootlin.com/linux/v6.19.11/source/drivers/gpu/drm/i915/display/intel_dp_mst.c#L1390
> do we need to remove this ?

Again. Why do you need to update EDID of the unregistered connector?
Please find a better response than "i915" does this. Git log might have
some story.

> > > +	drm_object_attach_property(&connector->base,
> > > +				   dev->mode_config.tile_property, 0);
> > > +
> > > +	drm_modeset_unlock_all(dev);
> > > +
> > > +	drm_dbg_dp(dp_display->drm_dev, "add MST connector id:%d\n", connector->base.id);
> > > +
> > > +	return connector;
> > > +}
> > > +
> > > +static const struct drm_dp_mst_topology_cbs msm_dp_mst_drm_cbs = {
> > > +	.add_connector = msm_dp_mst_add_connector,
> > 
> > No .poll_hpd_irq ?
> > 
> I checked that poll_hpd_irq is only used for certain specific scenarios, so
> it can be added later when it is actually needed.

Which scenarios? Will this impact USB-C or any other usecase?


-- 
With best wishes
Dmitry