* [PATCH v9 01/14] drm/msm/dp: remove cached drm_edid from panel
2026-07-28 10:21 [PATCH v9 00/14] drm/msm/dp: Prerequisite cleanup for upcoming MST support Yongxing Mou
@ 2026-07-28 10:21 ` Yongxing Mou
2026-07-28 10:37 ` sashiko-bot
2026-07-28 10:21 ` [PATCH v9 02/14] drm/msm/dp: drop deprecated .mode_set() and use .atomic_pre_enable Yongxing Mou
` (12 subsequent siblings)
13 siblings, 1 reply; 22+ messages in thread
From: Yongxing Mou @ 2026-07-28 10:21 UTC (permalink / raw)
To: Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang,
Sean Paul, Marijn Suijten, David Airlie, Simona Vetter
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel, Yongxing Mou,
Dmitry Baryshkov
The cached drm_edid seems unnecessary here. Use the drm_edid pointer
directly in the plug stage instead of caching it. Remove the cached
drm_edid and the corresponding oneliner to simplify the code.
Signed-off-by: Yongxing Mou <yongxing.mou@oss.qualcomm.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
drivers/gpu/drm/msm/dp/dp_display.c | 28 +++++++++++-------
drivers/gpu/drm/msm/dp/dp_panel.c | 59 ++++---------------------------------
drivers/gpu/drm/msm/dp/dp_panel.h | 13 +++-----
3 files changed, 27 insertions(+), 73 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index dc6f33809ca5..bce165bac106 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -269,6 +269,7 @@ static int msm_dp_display_process_hpd_high(struct msm_dp_display_private *dp)
const struct drm_display_info *info = &connector->display_info;
int rc = 0;
u8 dpcd[DP_RECEIVER_CAP_SIZE];
+ const struct drm_edid *drm_edid = NULL;
rc = drm_dp_read_dpcd_caps(dp->aux, dpcd);
if (rc)
@@ -276,10 +277,20 @@ static int msm_dp_display_process_hpd_high(struct msm_dp_display_private *dp)
dp->link->lttpr_count = msm_dp_display_lttpr_init(dp, dpcd);
- rc = msm_dp_panel_read_sink_caps(dp->panel, connector);
+ rc = msm_dp_panel_read_link_caps(dp->panel, connector);
if (rc)
goto end;
+ drm_edid = drm_edid_read_ddc(connector, &dp->aux->ddc);
+ drm_edid_connector_update(connector, drm_edid);
+
+ if (!drm_edid) {
+ DRM_ERROR("panel edid read failed\n");
+ /* check edid read fail is due to unplug */
+ if (!msm_dp_aux_is_link_connected(dp->aux))
+ return -ETIMEDOUT;
+ }
+
msm_dp_link_process_request(dp->link);
if (!dp->msm_dp_display.is_edp)
@@ -291,7 +302,7 @@ static int msm_dp_display_process_hpd_high(struct msm_dp_display_private *dp)
dp->msm_dp_display.psr_supported = dp->panel->psr_cap.version && psr_enabled;
dp->audio_supported = info->has_audio;
- msm_dp_panel_handle_sink_request(dp->panel);
+ msm_dp_panel_handle_sink_request(dp->panel, drm_edid);
/*
* set sink to normal operation mode -- D0
@@ -302,6 +313,7 @@ static int msm_dp_display_process_hpd_high(struct msm_dp_display_private *dp)
msm_dp_link_reset_phy_params_vx_px(dp->link);
end:
+ drm_edid_free(drm_edid);
return rc;
}
@@ -453,7 +465,7 @@ static int msm_dp_hpd_unplug_handle(struct msm_dp_display_private *dp)
/* Don't forget modes for eDP */
if (!dp->msm_dp_display.is_edp)
- msm_dp_panel_unplugged(dp->panel, dp->msm_dp_display.connector);
+ drm_edid_connector_update(dp->msm_dp_display.connector, NULL);
/* triggered by irq_hdp with sink_count = 0 */
if (dp->link->sink_count == 0)
@@ -515,7 +527,6 @@ static int msm_dp_irq_hpd_handle(struct msm_dp_display_private *dp)
static void msm_dp_display_deinit_sub_modules(struct msm_dp_display_private *dp)
{
msm_dp_audio_put(dp->audio);
- msm_dp_panel_put(dp->panel);
msm_dp_aux_put(dp->aux);
}
@@ -566,7 +577,7 @@ static int msm_dp_init_sub_modules(struct msm_dp_display_private *dp)
rc = PTR_ERR(dp->ctrl);
DRM_ERROR("failed to initialize ctrl, rc = %d\n", rc);
dp->ctrl = NULL;
- goto error_ctrl;
+ goto error_link;
}
dp->audio = msm_dp_audio_get(dp->msm_dp_display.pdev, dp->link_base);
@@ -574,13 +585,11 @@ static int msm_dp_init_sub_modules(struct msm_dp_display_private *dp)
rc = PTR_ERR(dp->audio);
pr_err("failed to initialize audio, rc = %d\n", rc);
dp->audio = NULL;
- goto error_ctrl;
+ goto error_link;
}
return rc;
-error_ctrl:
- msm_dp_panel_put(dp->panel);
error_link:
msm_dp_aux_put(dp->aux);
error:
@@ -744,8 +753,7 @@ int msm_dp_display_get_modes(struct msm_dp *dp)
msm_dp_display = container_of(dp, struct msm_dp_display_private, msm_dp_display);
- return msm_dp_panel_get_modes(msm_dp_display->panel,
- dp->connector);
+ return drm_edid_connector_add_modes(msm_dp_display->panel->connector);
}
bool msm_dp_display_check_video_test(struct msm_dp *dp)
diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c b/drivers/gpu/drm/msm/dp/dp_panel.c
index 5b4954e7cb74..bde4a772d22c 100644
--- a/drivers/gpu/drm/msm/dp/dp_panel.c
+++ b/drivers/gpu/drm/msm/dp/dp_panel.c
@@ -232,8 +232,8 @@ static u32 msm_dp_panel_get_supported_bpp(struct msm_dp_panel *msm_dp_panel,
return min_supported_bpp;
}
-int msm_dp_panel_read_sink_caps(struct msm_dp_panel *msm_dp_panel,
- struct drm_connector *connector)
+int msm_dp_panel_read_link_caps(struct msm_dp_panel *msm_dp_panel,
+ struct drm_connector *connector)
{
int rc, bw_code;
int count;
@@ -271,36 +271,9 @@ int msm_dp_panel_read_sink_caps(struct msm_dp_panel *msm_dp_panel,
rc = drm_dp_read_downstream_info(panel->aux, msm_dp_panel->dpcd,
msm_dp_panel->downstream_ports);
- if (rc)
- return rc;
-
- drm_edid_free(msm_dp_panel->drm_edid);
-
- msm_dp_panel->drm_edid = drm_edid_read_ddc(connector, &panel->aux->ddc);
-
- drm_edid_connector_update(connector, msm_dp_panel->drm_edid);
-
- if (!msm_dp_panel->drm_edid) {
- DRM_ERROR("panel edid read failed\n");
- /* check edid read fail is due to unplug */
- if (!msm_dp_aux_is_link_connected(panel->aux)) {
- rc = -ETIMEDOUT;
- goto end;
- }
- }
-
-end:
return rc;
}
-void msm_dp_panel_unplugged(struct msm_dp_panel *msm_dp_panel,
- struct drm_connector *connector)
-{
- drm_edid_connector_update(connector, NULL);
- drm_edid_free(msm_dp_panel->drm_edid);
- msm_dp_panel->drm_edid = NULL;
-}
-
u32 msm_dp_panel_get_mode_bpp(struct msm_dp_panel *msm_dp_panel,
u32 mode_edid_bpp, u32 mode_pclk_khz)
{
@@ -324,22 +297,6 @@ u32 msm_dp_panel_get_mode_bpp(struct msm_dp_panel *msm_dp_panel,
return bpp;
}
-int msm_dp_panel_get_modes(struct msm_dp_panel *msm_dp_panel,
- struct drm_connector *connector)
-{
- if (!msm_dp_panel) {
- DRM_ERROR("invalid input\n");
- return -EINVAL;
- }
-
- if (msm_dp_panel->drm_edid) {
- drm_edid_connector_update(connector, msm_dp_panel->drm_edid);
- return drm_edid_connector_add_modes(connector);
- }
-
- return 0;
-}
-
static u8 msm_dp_panel_get_edid_checksum(const struct edid *edid)
{
edid += edid->extensions;
@@ -347,7 +304,8 @@ static u8 msm_dp_panel_get_edid_checksum(const struct edid *edid)
return edid->checksum;
}
-void msm_dp_panel_handle_sink_request(struct msm_dp_panel *msm_dp_panel)
+void msm_dp_panel_handle_sink_request(struct msm_dp_panel *msm_dp_panel,
+ const struct drm_edid *drm_edid)
{
struct msm_dp_panel_private *panel;
@@ -360,7 +318,7 @@ void msm_dp_panel_handle_sink_request(struct msm_dp_panel *msm_dp_panel)
if (panel->link->sink_request & DP_TEST_LINK_EDID_READ) {
/* FIXME: get rid of drm_edid_raw() */
- const struct edid *edid = drm_edid_raw(msm_dp_panel->drm_edid);
+ const struct edid *edid = drm_edid_raw(drm_edid);
u8 checksum;
if (edid)
@@ -757,10 +715,3 @@ struct msm_dp_panel *msm_dp_panel_get(struct device *dev, struct drm_dp_aux *aux
return msm_dp_panel;
}
-void msm_dp_panel_put(struct msm_dp_panel *msm_dp_panel)
-{
- if (!msm_dp_panel)
- return;
-
- drm_edid_free(msm_dp_panel->drm_edid);
-}
diff --git a/drivers/gpu/drm/msm/dp/dp_panel.h b/drivers/gpu/drm/msm/dp/dp_panel.h
index 9173e90a5053..53b7b4463551 100644
--- a/drivers/gpu/drm/msm/dp/dp_panel.h
+++ b/drivers/gpu/drm/msm/dp/dp_panel.h
@@ -33,7 +33,6 @@ struct msm_dp_panel {
u8 downstream_ports[DP_MAX_DOWNSTREAM_PORTS];
struct msm_dp_link_info link_info;
- const struct drm_edid *drm_edid;
struct drm_connector *connector;
struct msm_dp_display_mode msm_dp_mode;
struct msm_dp_panel_psr psr_cap;
@@ -47,15 +46,12 @@ struct msm_dp_panel {
int msm_dp_panel_init_panel_info(struct msm_dp_panel *msm_dp_panel);
int msm_dp_panel_deinit(struct msm_dp_panel *msm_dp_panel);
int msm_dp_panel_timing_cfg(struct msm_dp_panel *msm_dp_panel, bool wide_bus_en);
-int msm_dp_panel_read_sink_caps(struct msm_dp_panel *msm_dp_panel,
- struct drm_connector *connector);
-void msm_dp_panel_unplugged(struct msm_dp_panel *msm_dp_panel,
- struct drm_connector *connector);
+int msm_dp_panel_read_link_caps(struct msm_dp_panel *msm_dp_panel,
+ struct drm_connector *connector);
u32 msm_dp_panel_get_mode_bpp(struct msm_dp_panel *msm_dp_panel, u32 mode_max_bpp,
u32 mode_pclk_khz);
-int msm_dp_panel_get_modes(struct msm_dp_panel *msm_dp_panel,
- struct drm_connector *connector);
-void msm_dp_panel_handle_sink_request(struct msm_dp_panel *msm_dp_panel);
+void msm_dp_panel_handle_sink_request(struct msm_dp_panel *msm_dp_panel,
+ const struct drm_edid *drm_edid);
void msm_dp_panel_tpg_config(struct msm_dp_panel *msm_dp_panel, bool enable);
void msm_dp_panel_clear_dsc_dto(struct msm_dp_panel *msm_dp_panel);
@@ -94,5 +90,4 @@ struct msm_dp_panel *msm_dp_panel_get(struct device *dev, struct drm_dp_aux *aux
struct msm_dp_link *link,
void __iomem *link_base,
void __iomem *p0_base);
-void msm_dp_panel_put(struct msm_dp_panel *msm_dp_panel);
#endif /* _DP_PANEL_H_ */
--
2.43.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v9 01/14] drm/msm/dp: remove cached drm_edid from panel
2026-07-28 10:21 ` [PATCH v9 01/14] drm/msm/dp: remove cached drm_edid from panel Yongxing Mou
@ 2026-07-28 10:37 ` sashiko-bot
0 siblings, 0 replies; 22+ messages in thread
From: sashiko-bot @ 2026-07-28 10:37 UTC (permalink / raw)
To: Yongxing Mou; +Cc: dri-devel
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Concurrent unlocked update of connector->edid_blob_ptr and display_info from HPD thread can race with get_modes(), causing Use-After-Free.
--
--- Patch [1]: [PATCH v9 01/14] drm/msm/dp: remove cached drm_edid from panel ---
commit d2a6147547e02d308fe3444738b9bff65fc0a34d
Author: Yongxing Mou <yongxing.mou@oss.qualcomm.com>
drm/msm/dp: remove cached drm_edid from panel
The cached drm_edid seems unnecessary here. Use the drm_edid pointer
directly in the plug stage instead of caching it. Remove the cached
drm_edid and the corresponding oneliner to simplify the code.
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> index dc6f33809ca5f..bce165bac106b 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -276,10 +277,20 @@ static int msm_dp_display_process_hpd_high(struct msm_dp_display_private *dp)
>
> dp->link->lttpr_count = msm_dp_display_lttpr_init(dp, dpcd);
>
> - rc = msm_dp_panel_read_sink_caps(dp->panel, connector);
> + rc = msm_dp_panel_read_link_caps(dp->panel, connector);
> if (rc)
> goto end;
>
> + drm_edid = drm_edid_read_ddc(connector, &dp->aux->ddc);
> + drm_edid_connector_update(connector, drm_edid);
> +
[Severity: High]
This is a pre-existing issue, but can this concurrent unlocked update of
connector->edid_blob_ptr and display_info from the HPD thread race with
get_modes(), causing a use-after-free?
drm_edid_connector_update() is called here from an asynchronous HPD thread
without holding connector->mutex or mode_config.mutex.
If userspace repeatedly calls DRM_IOCTL_MODE_GETCONNECTOR (which invokes
msm_dp_display_get_modes() while holding mode_config.mutex) concurrently
with an external hotplug event, the race window in
drm_property_replace_global_blob() allows the old EDID blob to be freed
before the reader dereferences it.
[ ... ]
> @@ -453,7 +465,7 @@ static int msm_dp_hpd_unplug_handle(struct msm_dp_display_private *dp)
>
> /* Don't forget modes for eDP */
> if (!dp->msm_dp_display.is_edp)
> - msm_dp_panel_unplugged(dp->panel, dp->msm_dp_display.connector);
> + drm_edid_connector_update(dp->msm_dp_display.connector, NULL);
>
[Severity: High]
This isn't a bug introduced by this patch, but does modifying the
edid_blob_ptr locklessly on unplug lead to the same race condition?
[ ... ]
> @@ -744,8 +753,7 @@ int msm_dp_display_get_modes(struct msm_dp *dp)
>
> msm_dp_display = container_of(dp, struct msm_dp_display_private, msm_dp_display);
>
> - return msm_dp_panel_get_modes(msm_dp_display->panel,
> - dp->connector);
> + return drm_edid_connector_add_modes(msm_dp_display->panel->connector);
> }
[Severity: High]
This is a pre-existing issue, but doesn't this concurrent reader path
invoked from userspace ioctls read the concurrently modified edid_blob_ptr
without adequate protection?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260728-dp_mstclean-v9-0-f7779fce10f4@oss.qualcomm.com?part=1
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v9 02/14] drm/msm/dp: drop deprecated .mode_set() and use .atomic_pre_enable
2026-07-28 10:21 [PATCH v9 00/14] drm/msm/dp: Prerequisite cleanup for upcoming MST support Yongxing Mou
2026-07-28 10:21 ` [PATCH v9 01/14] drm/msm/dp: remove cached drm_edid from panel Yongxing Mou
@ 2026-07-28 10:21 ` Yongxing Mou
2026-07-28 10:34 ` sashiko-bot
2026-07-28 10:21 ` [PATCH v9 03/14] drm/msm/dp: move mode setup into msm_dp_panel_init_panel_info() Yongxing Mou
` (11 subsequent siblings)
13 siblings, 1 reply; 22+ messages in thread
From: Yongxing Mou @ 2026-07-28 10:21 UTC (permalink / raw)
To: Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang,
Sean Paul, Marijn Suijten, David Airlie, Simona Vetter
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel, Yongxing Mou,
Abhinav Kumar, Dmitry Baryshkov
The bridge .mode_set() callback is deprecated. Remove it and move the
mode setup logic to .atomic_pre_enable(), where the adjusted_mode is
available from the atomic CRTC state.
.atomic_pre_enable() is used rather than .atomic_enable() because the DPU
encoder's .atomic_enable() reads the output mode's YUV420 / wide bus
state through the msm_display callbacks, and it runs after all bridges'
.atomic_pre_enable() but before their .atomic_enable(). Programming the
mode from the DP bridge's .atomic_enable() would leave the encoder
reading the previously committed mode's state.
Drop msm_dp_mode from msm_dp_display_private and store the mode directly
in the panel, as it was only used as a temporary cache.
Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
Signed-off-by: Yongxing Mou <yongxing.mou@oss.qualcomm.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Assisted-by: Claude:claude-opus-4-8
[DB: moved to atomic_pre_enable]
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
drivers/gpu/drm/msm/dp/dp_display.c | 103 ++++++++++++++++--------------------
drivers/gpu/drm/msm/dp/dp_drm.c | 4 +-
drivers/gpu/drm/msm/dp/dp_drm.h | 5 +-
3 files changed, 51 insertions(+), 61 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index bce165bac106..c106aabda588 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -63,7 +63,6 @@ struct msm_dp_display_private {
struct msm_dp_panel *panel;
struct msm_dp_ctrl *ctrl;
- struct msm_dp_display_mode msm_dp_mode;
struct msm_dp msm_dp_display;
/* wait for audio signaling */
@@ -597,16 +596,33 @@ static int msm_dp_init_sub_modules(struct msm_dp_display_private *dp)
}
static int msm_dp_display_set_mode(struct msm_dp *msm_dp_display,
- struct msm_dp_display_mode *mode)
+ const struct drm_display_mode *adjusted_mode,
+ struct msm_dp_panel *msm_dp_panel)
{
struct msm_dp_display_private *dp;
+ u32 bpp;
dp = container_of(msm_dp_display, struct msm_dp_display_private, msm_dp_display);
- drm_mode_copy(&dp->panel->msm_dp_mode.drm_mode, &mode->drm_mode);
- dp->panel->msm_dp_mode.bpp = mode->bpp;
- dp->panel->msm_dp_mode.out_fmt_is_yuv_420 = mode->out_fmt_is_yuv_420;
- msm_dp_panel_init_panel_info(dp->panel);
+ drm_mode_copy(&msm_dp_panel->msm_dp_mode.drm_mode, adjusted_mode);
+ if (msm_dp_display_check_video_test(msm_dp_display))
+ bpp = msm_dp_display_get_test_bpp(msm_dp_display);
+ else
+ bpp = msm_dp_panel->connector->display_info.bpc * 3;
+
+ msm_dp_panel->msm_dp_mode.bpp = bpp ? bpp : 24; /* Default bpp */
+ msm_dp_panel->msm_dp_mode.v_active_low =
+ !!(adjusted_mode->flags & DRM_MODE_FLAG_NVSYNC);
+ msm_dp_panel->msm_dp_mode.h_active_low =
+ !!(adjusted_mode->flags & DRM_MODE_FLAG_NHSYNC);
+ msm_dp_panel->msm_dp_mode.out_fmt_is_yuv_420 =
+ drm_mode_is_420_only(&msm_dp_panel->connector->display_info, adjusted_mode) &&
+ msm_dp_panel->vsc_sdp_supported;
+ msm_dp_panel_init_panel_info(msm_dp_panel);
+
+ /* populate wide_bus_support to different layers */
+ dp->ctrl->wide_bus_en =
+ msm_dp_panel->msm_dp_mode.out_fmt_is_yuv_420 ? false : dp->wide_bus_supported;
return 0;
}
@@ -1309,7 +1325,7 @@ bool msm_dp_wide_bus_available(const struct msm_dp *msm_dp_display)
dp = container_of(msm_dp_display, struct msm_dp_display_private, msm_dp_display);
- if (dp->msm_dp_mode.out_fmt_is_yuv_420)
+ if (dp->panel->msm_dp_mode.out_fmt_is_yuv_420)
return false;
return dp->wide_bus_supported;
@@ -1360,6 +1376,30 @@ int msm_dp_modeset_init(struct msm_dp *msm_dp_display, struct drm_device *dev,
return 0;
}
+void msm_dp_bridge_atomic_pre_enable(struct drm_bridge *drm_bridge,
+ struct drm_atomic_commit *state)
+{
+ struct msm_dp_bridge *msm_dp_bridge = to_dp_bridge(drm_bridge);
+ struct msm_dp *dp = msm_dp_bridge->msm_dp_display;
+ struct msm_dp_display_private *msm_dp_display;
+ struct drm_crtc *crtc;
+ struct drm_crtc_state *crtc_state;
+
+ msm_dp_display = container_of(dp, struct msm_dp_display_private, msm_dp_display);
+
+ crtc = drm_atomic_get_new_crtc_for_encoder(state, drm_bridge->encoder);
+ if (!crtc)
+ return;
+ crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
+
+ /*
+ * The DPU encoder's .atomic_enable() reads the mode's YUV420 / wide bus
+ * state and runs before the bridge's .atomic_enable(), so the mode must
+ * be programmed here, in .atomic_pre_enable().
+ */
+ msm_dp_display_set_mode(dp, &crtc_state->adjusted_mode, msm_dp_display->panel);
+}
+
void msm_dp_bridge_atomic_enable(struct drm_bridge *drm_bridge,
struct drm_atomic_commit *state)
{
@@ -1370,10 +1410,6 @@ void msm_dp_bridge_atomic_enable(struct drm_bridge *drm_bridge,
bool force_link_train = false;
msm_dp_display = container_of(dp, struct msm_dp_display_private, msm_dp_display);
- if (!msm_dp_display->msm_dp_mode.drm_mode.clock) {
- DRM_ERROR("invalid params\n");
- return;
- }
if (dp->is_edp)
msm_dp_hpd_plug_handle(msm_dp_display);
@@ -1386,12 +1422,6 @@ void msm_dp_bridge_atomic_enable(struct drm_bridge *drm_bridge,
if (msm_dp_display->link->sink_count == 0)
return;
- rc = msm_dp_display_set_mode(dp, &msm_dp_display->msm_dp_mode);
- if (rc) {
- DRM_ERROR("Failed to perform a mode set, rc=%d\n", rc);
- return;
- }
-
if (!dp->power_on) {
msm_dp_display_host_phy_init(msm_dp_display);
force_link_train = true;
@@ -1446,45 +1476,6 @@ void msm_dp_bridge_atomic_post_disable(struct drm_bridge *drm_bridge,
pm_runtime_put_sync(&dp->pdev->dev);
}
-void msm_dp_bridge_mode_set(struct drm_bridge *drm_bridge,
- const struct drm_display_mode *mode,
- const struct drm_display_mode *adjusted_mode)
-{
- struct msm_dp_bridge *msm_dp_bridge = to_dp_bridge(drm_bridge);
- struct msm_dp *dp = msm_dp_bridge->msm_dp_display;
- struct msm_dp_display_private *msm_dp_display;
- struct msm_dp_panel *msm_dp_panel;
-
- msm_dp_display = container_of(dp, struct msm_dp_display_private, msm_dp_display);
- msm_dp_panel = msm_dp_display->panel;
-
- memset(&msm_dp_display->msm_dp_mode, 0x0, sizeof(struct msm_dp_display_mode));
-
- if (msm_dp_display_check_video_test(dp))
- msm_dp_display->msm_dp_mode.bpp = msm_dp_display_get_test_bpp(dp);
- else /* Default num_components per pixel = 3 */
- msm_dp_display->msm_dp_mode.bpp = dp->connector->display_info.bpc * 3;
-
- if (!msm_dp_display->msm_dp_mode.bpp)
- msm_dp_display->msm_dp_mode.bpp = 24; /* Default bpp */
-
- drm_mode_copy(&msm_dp_display->msm_dp_mode.drm_mode, adjusted_mode);
-
- msm_dp_display->msm_dp_mode.v_active_low =
- !!(msm_dp_display->msm_dp_mode.drm_mode.flags & DRM_MODE_FLAG_NVSYNC);
-
- msm_dp_display->msm_dp_mode.h_active_low =
- !!(msm_dp_display->msm_dp_mode.drm_mode.flags & DRM_MODE_FLAG_NHSYNC);
-
- msm_dp_display->msm_dp_mode.out_fmt_is_yuv_420 =
- drm_mode_is_420_only(&dp->connector->display_info, adjusted_mode) &&
- msm_dp_panel->vsc_sdp_supported;
-
- /* populate wide_bus_support to different layers */
- msm_dp_display->ctrl->wide_bus_en =
- msm_dp_display->msm_dp_mode.out_fmt_is_yuv_420 ? false : msm_dp_display->wide_bus_supported;
-}
-
void msm_dp_bridge_hpd_enable(struct drm_bridge *bridge)
{
struct msm_dp_bridge *msm_dp_bridge = to_dp_bridge(bridge);
diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c b/drivers/gpu/drm/msm/dp/dp_drm.c
index 06881bfd6181..794a0df60414 100644
--- a/drivers/gpu/drm/msm/dp/dp_drm.c
+++ b/drivers/gpu/drm/msm/dp/dp_drm.c
@@ -53,10 +53,10 @@ static const struct drm_bridge_funcs msm_dp_bridge_ops = {
.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
.atomic_create_state = drm_atomic_helper_bridge_create_state,
+ .atomic_pre_enable = msm_dp_bridge_atomic_pre_enable,
.atomic_enable = msm_dp_bridge_atomic_enable,
.atomic_disable = msm_dp_bridge_atomic_disable,
.atomic_post_disable = msm_dp_bridge_atomic_post_disable,
- .mode_set = msm_dp_bridge_mode_set,
.mode_valid = msm_dp_bridge_mode_valid,
.get_modes = msm_dp_bridge_get_modes,
.detect = msm_dp_bridge_detect,
@@ -230,10 +230,10 @@ static void msm_edp_bridge_debugfs_init(struct drm_bridge *bridge, struct dentry
}
static const struct drm_bridge_funcs msm_edp_bridge_ops = {
+ .atomic_pre_enable = msm_dp_bridge_atomic_pre_enable,
.atomic_enable = msm_edp_bridge_atomic_enable,
.atomic_disable = msm_edp_bridge_atomic_disable,
.atomic_post_disable = msm_edp_bridge_atomic_post_disable,
- .mode_set = msm_dp_bridge_mode_set,
.mode_valid = msm_edp_bridge_mode_valid,
.atomic_create_state = drm_atomic_helper_bridge_create_state,
.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
diff --git a/drivers/gpu/drm/msm/dp/dp_drm.h b/drivers/gpu/drm/msm/dp/dp_drm.h
index 041aa026ae2e..4f733d8118b1 100644
--- a/drivers/gpu/drm/msm/dp/dp_drm.h
+++ b/drivers/gpu/drm/msm/dp/dp_drm.h
@@ -27,6 +27,8 @@ int msm_dp_bridge_init(struct msm_dp *msm_dp_display, struct drm_device *dev,
enum drm_connector_status msm_dp_bridge_detect(struct drm_bridge *bridge,
struct drm_connector *connector);
+void msm_dp_bridge_atomic_pre_enable(struct drm_bridge *drm_bridge,
+ struct drm_atomic_commit *state);
void msm_dp_bridge_atomic_enable(struct drm_bridge *drm_bridge,
struct drm_atomic_commit *state);
void msm_dp_bridge_atomic_disable(struct drm_bridge *drm_bridge,
@@ -36,9 +38,6 @@ void msm_dp_bridge_atomic_post_disable(struct drm_bridge *drm_bridge,
enum drm_mode_status msm_dp_bridge_mode_valid(struct drm_bridge *bridge,
const struct drm_display_info *info,
const struct drm_display_mode *mode);
-void msm_dp_bridge_mode_set(struct drm_bridge *drm_bridge,
- const struct drm_display_mode *mode,
- const struct drm_display_mode *adjusted_mode);
void msm_dp_bridge_hpd_enable(struct drm_bridge *bridge);
void msm_dp_bridge_hpd_disable(struct drm_bridge *bridge);
void msm_dp_bridge_hpd_notify(struct drm_bridge *bridge,
--
2.43.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v9 02/14] drm/msm/dp: drop deprecated .mode_set() and use .atomic_pre_enable
2026-07-28 10:21 ` [PATCH v9 02/14] drm/msm/dp: drop deprecated .mode_set() and use .atomic_pre_enable Yongxing Mou
@ 2026-07-28 10:34 ` sashiko-bot
0 siblings, 0 replies; 22+ messages in thread
From: sashiko-bot @ 2026-07-28 10:34 UTC (permalink / raw)
To: Yongxing Mou; +Cc: dri-devel
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] msm_dp_bridge_mode_valid() evaluates wide bus availability using the previously committed mode's YUV420 state rather than the state of the mode being validated. This causes valid high-bandwidth RGB modes to be incorrectly rejected if a YUV420 mode was previously committed.
--
--- Patch [2]: [PATCH v9 02/14] drm/msm/dp: drop deprecated .mode_set() and use .atomic_pre_enable ---
commit 9ae2d320a8d0fa2507140b418f1dff372f81454c
Author: Yongxing Mou <yongxing.mou@oss.qualcomm.com>
drm/msm/dp: drop deprecated .mode_set() and use .atomic_pre_enable
The bridge .mode_set() callback is deprecated. Remove it and move the
mode setup logic to .atomic_pre_enable(), where the adjusted_mode is
available from the atomic CRTC state. Drop msm_dp_mode from
msm_dp_display_private and store the mode directly in the panel.
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> index bce165bac106b..c106aabda588e 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
[ ... ]
> @@ -1309,7 +1325,7 @@ bool msm_dp_wide_bus_available(const struct msm_dp *msm_dp_display)
>
> dp = container_of(msm_dp_display, struct msm_dp_display_private, msm_dp_display);
>
> - if (dp->msm_dp_mode.out_fmt_is_yuv_420)
> + if (dp->panel->msm_dp_mode.out_fmt_is_yuv_420)
[Severity: High]
This is a pre-existing issue, but does evaluating wide bus availability using
the previously committed mode's YUV420 state cause valid high-bandwidth RGB
modes to be incorrectly rejected?
When a user sets a YUV420 mode, out_fmt_is_yuv_420 is set to true during
atomic_pre_enable. If the driver later probes or validates modes from EDID,
msm_dp_bridge_mode_valid() calls msm_dp_wide_bus_available():
msm_dp_bridge_mode_valid()
if (msm_dp_wide_bus_available(dp))
mode_pclk_khz /= 2;
This function then reads the stale out_fmt_is_yuv_420 flag and returns false.
Consequently, the pixel clock for high-bandwidth RGB modes is not halved,
causing them to exceed the maximum pixel clock and be rejected.
While this patch addresses a similar state leak for the DPU encoder by moving
the mode state update to atomic_pre_enable, it seems mode validation (which
runs before atomic_pre_enable) still relies on this stale state.
> return false;
>
> return dp->wide_bus_supported;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260728-dp_mstclean-v9-0-f7779fce10f4@oss.qualcomm.com?part=2
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v9 03/14] drm/msm/dp: move mode setup into msm_dp_panel_init_panel_info()
2026-07-28 10:21 [PATCH v9 00/14] drm/msm/dp: Prerequisite cleanup for upcoming MST support Yongxing Mou
2026-07-28 10:21 ` [PATCH v9 01/14] drm/msm/dp: remove cached drm_edid from panel Yongxing Mou
2026-07-28 10:21 ` [PATCH v9 02/14] drm/msm/dp: drop deprecated .mode_set() and use .atomic_pre_enable Yongxing Mou
@ 2026-07-28 10:21 ` Yongxing Mou
2026-07-28 10:21 ` [PATCH v9 04/14] drm/msm/dp: split msm_dp_ctrl_config_ctrl() into link parts and stream parts Yongxing Mou
` (10 subsequent siblings)
13 siblings, 0 replies; 22+ messages in thread
From: Yongxing Mou @ 2026-07-28 10:21 UTC (permalink / raw)
To: Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang,
Sean Paul, Marijn Suijten, David Airlie, Simona Vetter
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel, Yongxing Mou,
Dmitry Baryshkov
The display layer directly assigns msm_dp_panel mode fields (bpp,
sync polarity, yuv420 flag) instead of letting the panel manage its
own state. Pass adjusted_mode and bpp as parameters to
msm_dp_panel_init_panel_info() and move the assignments inside it.
Suggested-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Signed-off-by: Yongxing Mou <yongxing.mou@oss.qualcomm.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
drivers/gpu/drm/msm/dp/dp_display.c | 11 +----------
drivers/gpu/drm/msm/dp/dp_panel.c | 18 +++++++++++++++---
drivers/gpu/drm/msm/dp/dp_panel.h | 4 +++-
3 files changed, 19 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index c106aabda588..2a84a6ffedbb 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -604,21 +604,12 @@ static int msm_dp_display_set_mode(struct msm_dp *msm_dp_display,
dp = container_of(msm_dp_display, struct msm_dp_display_private, msm_dp_display);
- drm_mode_copy(&msm_dp_panel->msm_dp_mode.drm_mode, adjusted_mode);
if (msm_dp_display_check_video_test(msm_dp_display))
bpp = msm_dp_display_get_test_bpp(msm_dp_display);
else
bpp = msm_dp_panel->connector->display_info.bpc * 3;
- msm_dp_panel->msm_dp_mode.bpp = bpp ? bpp : 24; /* Default bpp */
- msm_dp_panel->msm_dp_mode.v_active_low =
- !!(adjusted_mode->flags & DRM_MODE_FLAG_NVSYNC);
- msm_dp_panel->msm_dp_mode.h_active_low =
- !!(adjusted_mode->flags & DRM_MODE_FLAG_NHSYNC);
- msm_dp_panel->msm_dp_mode.out_fmt_is_yuv_420 =
- drm_mode_is_420_only(&msm_dp_panel->connector->display_info, adjusted_mode) &&
- msm_dp_panel->vsc_sdp_supported;
- msm_dp_panel_init_panel_info(msm_dp_panel);
+ msm_dp_panel_init_panel_info(msm_dp_panel, adjusted_mode, bpp ? bpp : 24);
/* populate wide_bus_support to different layers */
dp->ctrl->wide_bus_en =
diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c b/drivers/gpu/drm/msm/dp/dp_panel.c
index bde4a772d22c..e76dad0f6663 100644
--- a/drivers/gpu/drm/msm/dp/dp_panel.c
+++ b/drivers/gpu/drm/msm/dp/dp_panel.c
@@ -647,15 +647,27 @@ int msm_dp_panel_timing_cfg(struct msm_dp_panel *msm_dp_panel, bool wide_bus_en)
return 0;
}
-int msm_dp_panel_init_panel_info(struct msm_dp_panel *msm_dp_panel)
+int msm_dp_panel_init_panel_info(struct msm_dp_panel *msm_dp_panel,
+ const struct drm_display_mode *adjusted_mode,
+ u32 bpp)
{
struct drm_display_mode *drm_mode;
struct msm_dp_panel_private *panel;
- drm_mode = &msm_dp_panel->msm_dp_mode.drm_mode;
-
panel = container_of(msm_dp_panel, struct msm_dp_panel_private, msm_dp_panel);
+ drm_mode_copy(&msm_dp_panel->msm_dp_mode.drm_mode, adjusted_mode);
+ msm_dp_panel->msm_dp_mode.bpp = bpp;
+ msm_dp_panel->msm_dp_mode.v_active_low =
+ !!(adjusted_mode->flags & DRM_MODE_FLAG_NVSYNC);
+ msm_dp_panel->msm_dp_mode.h_active_low =
+ !!(adjusted_mode->flags & DRM_MODE_FLAG_NHSYNC);
+ msm_dp_panel->msm_dp_mode.out_fmt_is_yuv_420 =
+ drm_mode_is_420_only(&msm_dp_panel->connector->display_info, adjusted_mode) &&
+ msm_dp_panel->vsc_sdp_supported;
+
+ drm_mode = &msm_dp_panel->msm_dp_mode.drm_mode;
+
/*
* print resolution info as this is a result
* of user initiated action of cable connection
diff --git a/drivers/gpu/drm/msm/dp/dp_panel.h b/drivers/gpu/drm/msm/dp/dp_panel.h
index 53b7b4463551..4519ac374220 100644
--- a/drivers/gpu/drm/msm/dp/dp_panel.h
+++ b/drivers/gpu/drm/msm/dp/dp_panel.h
@@ -43,7 +43,9 @@ struct msm_dp_panel {
u32 max_bw_code;
};
-int msm_dp_panel_init_panel_info(struct msm_dp_panel *msm_dp_panel);
+int msm_dp_panel_init_panel_info(struct msm_dp_panel *msm_dp_panel,
+ const struct drm_display_mode *adjusted_mode,
+ u32 bpp);
int msm_dp_panel_deinit(struct msm_dp_panel *msm_dp_panel);
int msm_dp_panel_timing_cfg(struct msm_dp_panel *msm_dp_panel, bool wide_bus_en);
int msm_dp_panel_read_link_caps(struct msm_dp_panel *msm_dp_panel,
--
2.43.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v9 04/14] drm/msm/dp: split msm_dp_ctrl_config_ctrl() into link parts and stream parts
2026-07-28 10:21 [PATCH v9 00/14] drm/msm/dp: Prerequisite cleanup for upcoming MST support Yongxing Mou
` (2 preceding siblings ...)
2026-07-28 10:21 ` [PATCH v9 03/14] drm/msm/dp: move mode setup into msm_dp_panel_init_panel_info() Yongxing Mou
@ 2026-07-28 10:21 ` Yongxing Mou
2026-07-28 10:21 ` [PATCH v9 05/14] drm/msm/dp: extract MISC1_MISC0 configuration into a separate function Yongxing Mou
` (9 subsequent siblings)
13 siblings, 0 replies; 22+ messages in thread
From: Yongxing Mou @ 2026-07-28 10:21 UTC (permalink / raw)
To: Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang,
Sean Paul, Marijn Suijten, David Airlie, Simona Vetter
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel, Yongxing Mou,
Dmitry Baryshkov
The DP_CONFIGURATION_CTRL register contains both link-level and
stream-specific fields. Currently, msm_dp_ctrl_config_ctrl() configures
all of them together. Separate the configuration into link parts and
stream parts to support MST. Clear the stream-specific fields before
OR-ing new values in the stream path to avoid bit accumulation across
repeated calls.
Signed-off-by: Yongxing Mou <yongxing.mou@oss.qualcomm.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
drivers/gpu/drm/msm/dp/dp_ctrl.c | 51 ++++++++++++++++++++++++++++------------
drivers/gpu/drm/msm/dp/dp_reg.h | 2 ++
2 files changed, 38 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
index 86ef8c89ad44..7c0649d5318f 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
@@ -388,26 +388,48 @@ void msm_dp_ctrl_push_idle(struct msm_dp_ctrl *msm_dp_ctrl)
drm_dbg_dp(ctrl->drm_dev, "mainlink off\n");
}
-static void msm_dp_ctrl_config_ctrl(struct msm_dp_ctrl_private *ctrl)
+static void msm_dp_ctrl_config_ctrl_streams(struct msm_dp_ctrl_private *ctrl,
+ struct msm_dp_panel *msm_dp_panel)
{
u32 config = 0, tbd;
+
+ /*
+ * RMW: called from atomic_enable(), serialized by the DRM atomic framework.
+ * Clear stream-specific fields before OR-ing to avoid bit accumulation.
+ */
+ config = msm_dp_read_link(ctrl, REG_DP_CONFIGURATION_CTRL);
+ config &= ~(DP_CONFIGURATION_CTRL_RGB_YUV_MASK |
+ DP_CONFIGURATION_CTRL_BPC_MASK |
+ DP_CONFIGURATION_CTRL_SEND_VSC);
+
+ if (msm_dp_panel->msm_dp_mode.out_fmt_is_yuv_420)
+ config |= DP_CONFIGURATION_CTRL_RGB_YUV; /* YUV420 */
+
+ tbd = msm_dp_link_get_test_bits_depth(ctrl->link,
+ msm_dp_panel->msm_dp_mode.bpp);
+
+ config |= tbd << DP_CONFIGURATION_CTRL_BPC_SHIFT;
+
+ if (msm_dp_panel->psr_cap.version)
+ config |= DP_CONFIGURATION_CTRL_SEND_VSC;
+
+ drm_dbg_dp(ctrl->drm_dev, "stream DP_CONFIGURATION_CTRL=0x%x\n", config);
+
+ msm_dp_write_link(ctrl, REG_DP_CONFIGURATION_CTRL, config);
+}
+
+static void msm_dp_ctrl_config_ctrl_link(struct msm_dp_ctrl_private *ctrl)
+{
+ u32 config = 0;
const u8 *dpcd = ctrl->panel->dpcd;
/* Default-> LSCLK DIV: 1/4 LCLK */
config |= (2 << DP_CONFIGURATION_CTRL_LSCLK_DIV_SHIFT);
- if (ctrl->panel->msm_dp_mode.out_fmt_is_yuv_420)
- config |= DP_CONFIGURATION_CTRL_RGB_YUV; /* YUV420 */
-
/* Scrambler reset enable */
if (drm_dp_alternate_scrambler_reset_cap(dpcd))
config |= DP_CONFIGURATION_CTRL_ASSR;
- tbd = msm_dp_link_get_test_bits_depth(ctrl->link,
- ctrl->panel->msm_dp_mode.bpp);
-
- config |= tbd << DP_CONFIGURATION_CTRL_BPC_SHIFT;
-
/* Num of Lanes */
config |= ((ctrl->link->link_params.num_lanes - 1)
<< DP_CONFIGURATION_CTRL_NUM_OF_LANES_SHIFT);
@@ -421,10 +443,7 @@ static void msm_dp_ctrl_config_ctrl(struct msm_dp_ctrl_private *ctrl)
config |= DP_CONFIGURATION_CTRL_STATIC_DYNAMIC_CN;
config |= DP_CONFIGURATION_CTRL_SYNC_ASYNC_CLK;
- if (ctrl->panel->psr_cap.version)
- config |= DP_CONFIGURATION_CTRL_SEND_VSC;
-
- drm_dbg_dp(ctrl->drm_dev, "DP_CONFIGURATION_CTRL=0x%x\n", config);
+ drm_dbg_dp(ctrl->drm_dev, "link DP_CONFIGURATION_CTRL=0x%x\n", config);
msm_dp_write_link(ctrl, REG_DP_CONFIGURATION_CTRL, config);
}
@@ -450,7 +469,8 @@ static void msm_dp_ctrl_configure_source_params(struct msm_dp_ctrl_private *ctrl
msm_dp_ctrl_lane_mapping(ctrl);
msm_dp_setup_peripheral_flush(ctrl);
- msm_dp_ctrl_config_ctrl(ctrl);
+ msm_dp_ctrl_config_ctrl_link(ctrl);
+ msm_dp_ctrl_config_ctrl_streams(ctrl, ctrl->panel);
test_bits_depth = msm_dp_link_get_test_bits_depth(ctrl->link, ctrl->panel->msm_dp_mode.bpp);
colorimetry_cfg = msm_dp_link_get_colorimetry_config(ctrl->link);
@@ -1628,7 +1648,8 @@ static int msm_dp_ctrl_link_train(struct msm_dp_ctrl_private *ctrl,
u8 assr;
struct msm_dp_link_info link_info = {0};
- msm_dp_ctrl_config_ctrl(ctrl);
+ msm_dp_ctrl_config_ctrl_link(ctrl);
+ msm_dp_ctrl_config_ctrl_streams(ctrl, ctrl->panel);
link_info.num_lanes = ctrl->link->link_params.num_lanes;
link_info.rate = ctrl->link->link_params.rate;
diff --git a/drivers/gpu/drm/msm/dp/dp_reg.h b/drivers/gpu/drm/msm/dp/dp_reg.h
index 3689642b7fc0..dda4b642c81b 100644
--- a/drivers/gpu/drm/msm/dp/dp_reg.h
+++ b/drivers/gpu/drm/msm/dp/dp_reg.h
@@ -149,8 +149,10 @@
#define DP_CONFIGURATION_CTRL_ENHANCED_FRAMING (0x00000040)
#define DP_CONFIGURATION_CTRL_SEND_VSC (0x00000080)
#define DP_CONFIGURATION_CTRL_BPC (0x00000100)
+#define DP_CONFIGURATION_CTRL_BPC_MASK GENMASK(9, 8)
#define DP_CONFIGURATION_CTRL_ASSR (0x00000400)
#define DP_CONFIGURATION_CTRL_RGB_YUV (0x00000800)
+#define DP_CONFIGURATION_CTRL_RGB_YUV_MASK GENMASK(12, 11)
#define DP_CONFIGURATION_CTRL_LSCLK_DIV (0x00002000)
#define DP_CONFIGURATION_CTRL_NUM_OF_LANES_SHIFT (0x04)
#define DP_CONFIGURATION_CTRL_BPC_SHIFT (0x08)
--
2.43.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v9 05/14] drm/msm/dp: extract MISC1_MISC0 configuration into a separate function
2026-07-28 10:21 [PATCH v9 00/14] drm/msm/dp: Prerequisite cleanup for upcoming MST support Yongxing Mou
` (3 preceding siblings ...)
2026-07-28 10:21 ` [PATCH v9 04/14] drm/msm/dp: split msm_dp_ctrl_config_ctrl() into link parts and stream parts Yongxing Mou
@ 2026-07-28 10:21 ` Yongxing Mou
2026-07-28 10:21 ` [PATCH v9 06/14] drm/msm/dp: split link setup from source params Yongxing Mou
` (8 subsequent siblings)
13 siblings, 0 replies; 22+ messages in thread
From: Yongxing Mou @ 2026-07-28 10:21 UTC (permalink / raw)
To: Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang,
Sean Paul, Marijn Suijten, David Airlie, Simona Vetter
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel, Yongxing Mou,
Dmitry Baryshkov
Refactor the MISC1_MISC0 register configuration into a standalone helper
function to support MST.
Signed-off-by: Yongxing Mou <yongxing.mou@oss.qualcomm.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
drivers/gpu/drm/msm/dp/dp_ctrl.c | 23 +++++++++++++++--------
1 file changed, 15 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
index 7c0649d5318f..c557e0f01dc6 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
@@ -462,17 +462,13 @@ static void msm_dp_ctrl_lane_mapping(struct msm_dp_ctrl_private *ctrl)
ln_mapping);
}
-static void msm_dp_ctrl_configure_source_params(struct msm_dp_ctrl_private *ctrl)
+static void msm_dp_ctrl_config_misc1_misc0(struct msm_dp_ctrl_private *ctrl,
+ struct msm_dp_panel *msm_dp_panel)
{
u32 colorimetry_cfg, test_bits_depth, misc_val;
- msm_dp_ctrl_lane_mapping(ctrl);
- msm_dp_setup_peripheral_flush(ctrl);
-
- msm_dp_ctrl_config_ctrl_link(ctrl);
- msm_dp_ctrl_config_ctrl_streams(ctrl, ctrl->panel);
-
- test_bits_depth = msm_dp_link_get_test_bits_depth(ctrl->link, ctrl->panel->msm_dp_mode.bpp);
+ test_bits_depth = msm_dp_link_get_test_bits_depth(ctrl->link,
+ msm_dp_panel->msm_dp_mode.bpp);
colorimetry_cfg = msm_dp_link_get_colorimetry_config(ctrl->link);
misc_val = msm_dp_read_link(ctrl, REG_DP_MISC1_MISC0);
@@ -486,6 +482,17 @@ static void msm_dp_ctrl_configure_source_params(struct msm_dp_ctrl_private *ctrl
drm_dbg_dp(ctrl->drm_dev, "misc settings = 0x%x\n", misc_val);
msm_dp_write_link(ctrl, REG_DP_MISC1_MISC0, misc_val);
+}
+
+static void msm_dp_ctrl_configure_source_params(struct msm_dp_ctrl_private *ctrl)
+{
+ msm_dp_ctrl_lane_mapping(ctrl);
+ msm_dp_setup_peripheral_flush(ctrl);
+
+ msm_dp_ctrl_config_ctrl_link(ctrl);
+ msm_dp_ctrl_config_ctrl_streams(ctrl, ctrl->panel);
+
+ msm_dp_ctrl_config_misc1_misc0(ctrl, ctrl->panel);
msm_dp_panel_timing_cfg(ctrl->panel, ctrl->msm_dp_ctrl.wide_bus_en);
}
--
2.43.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v9 06/14] drm/msm/dp: split link setup from source params
2026-07-28 10:21 [PATCH v9 00/14] drm/msm/dp: Prerequisite cleanup for upcoming MST support Yongxing Mou
` (4 preceding siblings ...)
2026-07-28 10:21 ` [PATCH v9 05/14] drm/msm/dp: extract MISC1_MISC0 configuration into a separate function Yongxing Mou
@ 2026-07-28 10:21 ` Yongxing Mou
2026-07-28 10:21 ` [PATCH v9 07/14] drm/msm/dp: move the pixel clock control to its own API Yongxing Mou
` (7 subsequent siblings)
13 siblings, 0 replies; 22+ messages in thread
From: Yongxing Mou @ 2026-07-28 10:21 UTC (permalink / raw)
To: Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang,
Sean Paul, Marijn Suijten, David Airlie, Simona Vetter
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel, Yongxing Mou,
Dmitry Baryshkov
msm_dp_ctrl_configure_source_params() should only handle stream-related
configuration. Move the link setup out of it so MST can program link and
stream settings separately.
Signed-off-by: Yongxing Mou <yongxing.mou@oss.qualcomm.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
drivers/gpu/drm/msm/dp/dp_ctrl.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
index c557e0f01dc6..f343bd6c2b81 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
@@ -486,10 +486,6 @@ static void msm_dp_ctrl_config_misc1_misc0(struct msm_dp_ctrl_private *ctrl,
static void msm_dp_ctrl_configure_source_params(struct msm_dp_ctrl_private *ctrl)
{
- msm_dp_ctrl_lane_mapping(ctrl);
- msm_dp_setup_peripheral_flush(ctrl);
-
- msm_dp_ctrl_config_ctrl_link(ctrl);
msm_dp_ctrl_config_ctrl_streams(ctrl, ctrl->panel);
msm_dp_ctrl_config_misc1_misc0(ctrl, ctrl->panel);
@@ -2556,6 +2552,10 @@ int msm_dp_ctrl_on_stream(struct msm_dp_ctrl *msm_dp_ctrl, bool force_link_train
*/
reinit_completion(&ctrl->video_comp);
+ msm_dp_ctrl_lane_mapping(ctrl);
+ msm_dp_setup_peripheral_flush(ctrl);
+ msm_dp_ctrl_config_ctrl_link(ctrl);
+
msm_dp_ctrl_configure_source_params(ctrl);
msm_dp_ctrl_config_msa(ctrl,
--
2.43.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v9 07/14] drm/msm/dp: move the pixel clock control to its own API
2026-07-28 10:21 [PATCH v9 00/14] drm/msm/dp: Prerequisite cleanup for upcoming MST support Yongxing Mou
` (5 preceding siblings ...)
2026-07-28 10:21 ` [PATCH v9 06/14] drm/msm/dp: split link setup from source params Yongxing Mou
@ 2026-07-28 10:21 ` Yongxing Mou
2026-07-28 10:35 ` sashiko-bot
2026-07-28 10:21 ` [PATCH v9 08/14] drm/msm/dp: break up dp_display_enable into two parts Yongxing Mou
` (6 subsequent siblings)
13 siblings, 1 reply; 22+ messages in thread
From: Yongxing Mou @ 2026-07-28 10:21 UTC (permalink / raw)
To: Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang,
Sean Paul, Marijn Suijten, David Airlie, Simona Vetter
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel, Yongxing Mou,
Abhinav Kumar, Dmitry Baryshkov
Enable/Disable of DP pixel clock happens in multiple code paths
leading to code duplication. Move it into individual helpers so that
the helpers can be called wherever necessary.
Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
Signed-off-by: Yongxing Mou <yongxing.mou@oss.qualcomm.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
drivers/gpu/drm/msm/dp/dp_ctrl.c | 80 +++++++++++++++++++++-------------------
1 file changed, 42 insertions(+), 38 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
index f343bd6c2b81..068d4f8b673f 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
@@ -2176,6 +2176,41 @@ static bool msm_dp_ctrl_send_phy_test_pattern(struct msm_dp_ctrl_private *ctrl)
return success;
}
+static int msm_dp_ctrl_on_pixel_clk(struct msm_dp_ctrl_private *ctrl, unsigned long pixel_rate)
+{
+ int ret;
+
+ ret = clk_set_rate(ctrl->pixel_clk, pixel_rate * 1000);
+ if (ret) {
+ DRM_ERROR("Failed to set pixel clock rate. ret=%d\n", ret);
+ return ret;
+ }
+
+ if (WARN_ON_ONCE(ctrl->stream_clks_on))
+ return 0;
+
+ ret = clk_prepare_enable(ctrl->pixel_clk);
+ if (ret) {
+ DRM_ERROR("Failed to start pixel clocks. ret=%d\n", ret);
+ return ret;
+ }
+ ctrl->stream_clks_on = true;
+
+ return ret;
+}
+
+static void msm_dp_ctrl_off_pixel_clk(struct msm_dp_ctrl *msm_dp_ctrl)
+{
+ struct msm_dp_ctrl_private *ctrl;
+
+ ctrl = container_of(msm_dp_ctrl, struct msm_dp_ctrl_private, msm_dp_ctrl);
+
+ if (ctrl->stream_clks_on) {
+ clk_disable_unprepare(ctrl->pixel_clk);
+ ctrl->stream_clks_on = false;
+ }
+}
+
static int msm_dp_ctrl_process_phy_test_request(struct msm_dp_ctrl_private *ctrl)
{
int ret;
@@ -2201,22 +2236,9 @@ static int msm_dp_ctrl_process_phy_test_request(struct msm_dp_ctrl_private *ctrl
}
pixel_rate = ctrl->panel->msm_dp_mode.drm_mode.clock;
- ret = clk_set_rate(ctrl->pixel_clk, pixel_rate * 1000);
- if (ret) {
- DRM_ERROR("Failed to set pixel clock rate. ret=%d\n", ret);
+ ret = msm_dp_ctrl_on_pixel_clk(ctrl, pixel_rate);
+ if (ret)
return ret;
- }
-
- if (ctrl->stream_clks_on) {
- drm_dbg_dp(ctrl->drm_dev, "pixel clks already enabled\n");
- } else {
- ret = clk_prepare_enable(ctrl->pixel_clk);
- if (ret) {
- DRM_ERROR("Failed to start pixel clocks. ret=%d\n", ret);
- return ret;
- }
- ctrl->stream_clks_on = true;
- }
msm_dp_ctrl_send_phy_test_pattern(ctrl);
@@ -2519,26 +2541,13 @@ int msm_dp_ctrl_on_stream(struct msm_dp_ctrl *msm_dp_ctrl, bool force_link_train
ret = msm_dp_ctrl_enable_mainlink_clocks(ctrl);
if (ret) {
DRM_ERROR("Failed to start link clocks. ret=%d\n", ret);
- goto end;
+ return ret;
}
}
- ret = clk_set_rate(ctrl->pixel_clk, pixel_rate * 1000);
- if (ret) {
- DRM_ERROR("Failed to set pixel clock rate. ret=%d\n", ret);
- goto end;
- }
-
- if (ctrl->stream_clks_on) {
- drm_dbg_dp(ctrl->drm_dev, "pixel clks already enabled\n");
- } else {
- ret = clk_prepare_enable(ctrl->pixel_clk);
- if (ret) {
- DRM_ERROR("Failed to start pixel clocks. ret=%d\n", ret);
- goto end;
- }
- ctrl->stream_clks_on = true;
- }
+ ret = msm_dp_ctrl_on_pixel_clk(ctrl, pixel_rate);
+ if (ret)
+ return ret;
if (force_link_train || !msm_dp_ctrl_channel_eq_ok(ctrl))
msm_dp_ctrl_link_retrain(ctrl);
@@ -2577,7 +2586,6 @@ int msm_dp_ctrl_on_stream(struct msm_dp_ctrl *msm_dp_ctrl, bool force_link_train
drm_dbg_dp(ctrl->drm_dev,
"mainlink %s\n", mainlink_ready ? "READY" : "NOT READY");
-end:
return ret;
}
@@ -2625,11 +2633,7 @@ void msm_dp_ctrl_off(struct msm_dp_ctrl *msm_dp_ctrl)
msm_dp_ctrl_reset(&ctrl->msm_dp_ctrl);
- if (ctrl->stream_clks_on) {
- clk_disable_unprepare(ctrl->pixel_clk);
- ctrl->stream_clks_on = false;
- }
-
+ msm_dp_ctrl_off_pixel_clk(msm_dp_ctrl);
dev_pm_opp_set_rate(ctrl->dev, 0);
msm_dp_ctrl_link_clk_disable(&ctrl->msm_dp_ctrl);
--
2.43.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v9 07/14] drm/msm/dp: move the pixel clock control to its own API
2026-07-28 10:21 ` [PATCH v9 07/14] drm/msm/dp: move the pixel clock control to its own API Yongxing Mou
@ 2026-07-28 10:35 ` sashiko-bot
0 siblings, 0 replies; 22+ messages in thread
From: sashiko-bot @ 2026-07-28 10:35 UTC (permalink / raw)
To: Yongxing Mou; +Cc: dri-devel
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The refactoring introduces a WARN_ON_ONCE for reachable states and error paths, potentially causing a kernel panic (if panic_on_warn is enabled).
- [Low] The patch missed replacing duplicated pixel clock disablement logic in msm_dp_ctrl_off_link_stream with the new helper, and the helpers have asymmetric signatures.
--
--- Patch [7]: [PATCH v9 07/14] drm/msm/dp: move the pixel clock control to its own API ---
commit c980fe633e423ad69174b99f07ed497b0b4f6cf2
Author: Yongxing Mou <yongxing.mou@oss.qualcomm.com>
drm/msm/dp: move the pixel clock control to its own API
Enable/Disable of DP pixel clock happens in multiple code paths
leading to code duplication. Move it into individual helpers so that
the helpers can be called wherever necessary.
> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> index f343bd6c2b81a..068d4f8b673f6 100644
> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> @@ -2176,6 +2176,41 @@ static bool msm_dp_ctrl_send_phy_test_pattern(struct msm_dp_ctrl_private *ctrl)
> return success;
> }
>
> +static int msm_dp_ctrl_on_pixel_clk(struct msm_dp_ctrl_private *ctrl, unsigned long pixel_rate)
> +{
> + int ret;
> +
> + ret = clk_set_rate(ctrl->pixel_clk, pixel_rate * 1000);
> + if (ret) {
> + DRM_ERROR("Failed to set pixel clock rate. ret=%d\n", ret);
> + return ret;
> + }
> +
> + if (WARN_ON_ONCE(ctrl->stream_clks_on))
> + return 0;
[Severity: High]
Is it safe to replace the debug log with a WARN_ON_ONCE() here?
If an atomic commit enables the display, and msm_dp_ctrl_on_stream() succeeds
in enabling clocks but fails later (e.g., msm_dp_ctrl_wait4video_ready() times
out), it can return an error without disabling the pixel clock.
When the caller msm_dp_display_enable() propagates the error, dp->power_on
remains false. The teardown path msm_dp_display_disable() will then return
early because power_on is false, leaving the clock enabled.
If userspace attempts to enable the display again, msm_dp_ctrl_on_pixel_clk()
would be called with stream_clks_on == true, triggering this WARN_ON_ONCE().
This could also be triggered if an external DP sink sends a PHY test pattern
request before the display is enabled, via
msm_dp_ctrl_process_phy_test_request().
Since this state is reachable from hardware errors or untrusted external DP
sinks, could this cause a denial of service if panic_on_warn is enabled?
> +
> + ret = clk_prepare_enable(ctrl->pixel_clk);
> + if (ret) {
> + DRM_ERROR("Failed to start pixel clocks. ret=%d\n", ret);
> + return ret;
> + }
> + ctrl->stream_clks_on = true;
> +
> + return ret;
> +}
> +
> +static void msm_dp_ctrl_off_pixel_clk(struct msm_dp_ctrl *msm_dp_ctrl)
> +{
> + struct msm_dp_ctrl_private *ctrl;
> +
> + ctrl = container_of(msm_dp_ctrl, struct msm_dp_ctrl_private, msm_dp_ctrl);
> +
> + if (ctrl->stream_clks_on) {
> + clk_disable_unprepare(ctrl->pixel_clk);
> + ctrl->stream_clks_on = false;
> + }
> +}
[Severity: Low]
Is this API signature asymmetry intentional?
msm_dp_ctrl_on_pixel_clk() takes a struct msm_dp_ctrl_private *, but this
function takes a struct msm_dp_ctrl *.
Also, it looks like msm_dp_ctrl_off_link_stream() in this same file was missed
in the refactoring and still contains the duplicated pixel clock disablement
logic. Should it be updated to use this new helper?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260728-dp_mstclean-v9-0-f7779fce10f4@oss.qualcomm.com?part=7
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v9 08/14] drm/msm/dp: break up dp_display_enable into two parts
2026-07-28 10:21 [PATCH v9 00/14] drm/msm/dp: Prerequisite cleanup for upcoming MST support Yongxing Mou
` (6 preceding siblings ...)
2026-07-28 10:21 ` [PATCH v9 07/14] drm/msm/dp: move the pixel clock control to its own API Yongxing Mou
@ 2026-07-28 10:21 ` Yongxing Mou
2026-07-28 10:34 ` sashiko-bot
2026-07-28 10:21 ` [PATCH v9 09/14] drm/msm/dp: re-arrange dp_display_disable() into functional parts Yongxing Mou
` (5 subsequent siblings)
13 siblings, 1 reply; 22+ messages in thread
From: Yongxing Mou @ 2026-07-28 10:21 UTC (permalink / raw)
To: Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang,
Sean Paul, Marijn Suijten, David Airlie, Simona Vetter
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel, Yongxing Mou,
Abhinav Kumar, Dmitry Baryshkov
From: Abhinav Kumar <quic_abhinavk@quicinc.com>
dp_display_enable() currently re-trains the link if needed and then
enables the pixel clock, programs the controller to start sending the
pixel stream. Split these two parts into prepare/enable APIs, to support
MST bridges_enable insert the MST payloads funcs between enable
stream_clks and program register.
Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
Signed-off-by: Yongxing Mou <yongxing.mou@oss.qualcomm.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
drivers/gpu/drm/msm/dp/dp_ctrl.c | 48 +++++++++++------
drivers/gpu/drm/msm/dp/dp_ctrl.h | 3 +-
drivers/gpu/drm/msm/dp/dp_display.c | 105 +++++++++++++++++++++++-------------
3 files changed, 103 insertions(+), 53 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
index 068d4f8b673f..43094dda3434 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
@@ -2511,27 +2511,19 @@ static void msm_dp_ctrl_config_msa(struct msm_dp_ctrl_private *ctrl,
msm_dp_write_link(ctrl, REG_DP_SOFTWARE_NVID, nvid);
}
-int msm_dp_ctrl_on_stream(struct msm_dp_ctrl *msm_dp_ctrl, bool force_link_train)
+int msm_dp_ctrl_prepare_stream_on(struct msm_dp_ctrl *msm_dp_ctrl, bool force_link_train)
{
int ret = 0;
- bool mainlink_ready = false;
struct msm_dp_ctrl_private *ctrl;
- unsigned long pixel_rate;
- unsigned long pixel_rate_orig;
if (!msm_dp_ctrl)
return -EINVAL;
ctrl = container_of(msm_dp_ctrl, struct msm_dp_ctrl_private, msm_dp_ctrl);
- pixel_rate = pixel_rate_orig = ctrl->panel->msm_dp_mode.drm_mode.clock;
-
- if (msm_dp_ctrl->wide_bus_en || ctrl->panel->msm_dp_mode.out_fmt_is_yuv_420)
- pixel_rate >>= 1;
-
- drm_dbg_dp(ctrl->drm_dev, "rate=%d, num_lanes=%d, pixel_rate=%lu\n",
- ctrl->link->link_params.rate,
- ctrl->link->link_params.num_lanes, pixel_rate);
+ drm_dbg_dp(ctrl->drm_dev, "rate=%d, num_lanes=%d\n",
+ ctrl->link->link_params.rate,
+ ctrl->link->link_params.num_lanes);
drm_dbg_dp(ctrl->drm_dev,
"core_clk_on=%d link_clk_on=%d stream_clk_on=%d\n",
@@ -2545,16 +2537,40 @@ int msm_dp_ctrl_on_stream(struct msm_dp_ctrl *msm_dp_ctrl, bool force_link_train
}
}
- ret = msm_dp_ctrl_on_pixel_clk(ctrl, pixel_rate);
- if (ret)
- return ret;
-
if (force_link_train || !msm_dp_ctrl_channel_eq_ok(ctrl))
msm_dp_ctrl_link_retrain(ctrl);
/* stop txing train pattern to end link training */
msm_dp_ctrl_clear_training_pattern(ctrl, DP_PHY_DPRX);
+ return ret;
+}
+
+int msm_dp_ctrl_on_stream(struct msm_dp_ctrl *msm_dp_ctrl)
+{
+ int ret = 0;
+ bool mainlink_ready = false;
+ struct msm_dp_ctrl_private *ctrl;
+ unsigned long pixel_rate;
+ unsigned long pixel_rate_orig;
+
+ if (!msm_dp_ctrl)
+ return -EINVAL;
+
+ ctrl = container_of(msm_dp_ctrl, struct msm_dp_ctrl_private, msm_dp_ctrl);
+
+ pixel_rate_orig = ctrl->panel->msm_dp_mode.drm_mode.clock;
+ pixel_rate = pixel_rate_orig;
+
+ if (msm_dp_ctrl->wide_bus_en || ctrl->panel->msm_dp_mode.out_fmt_is_yuv_420)
+ pixel_rate >>= 1;
+
+ drm_dbg_dp(ctrl->drm_dev, "pixel_rate=%lu\n", pixel_rate);
+
+ ret = msm_dp_ctrl_on_pixel_clk(ctrl, pixel_rate);
+ if (ret)
+ return ret;
+
/*
* Set up transfer unit values and set controller state to send
* video.
diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.h b/drivers/gpu/drm/msm/dp/dp_ctrl.h
index f68bee62713f..1497f1a8fc2f 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.h
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.h
@@ -17,7 +17,8 @@ struct msm_dp_ctrl {
struct phy;
int msm_dp_ctrl_on_link(struct msm_dp_ctrl *msm_dp_ctrl);
-int msm_dp_ctrl_on_stream(struct msm_dp_ctrl *msm_dp_ctrl, bool force_link_train);
+int msm_dp_ctrl_on_stream(struct msm_dp_ctrl *msm_dp_ctrl);
+int msm_dp_ctrl_prepare_stream_on(struct msm_dp_ctrl *msm_dp_ctrl, bool force_link_train);
void msm_dp_ctrl_off_link_stream(struct msm_dp_ctrl *msm_dp_ctrl);
void msm_dp_ctrl_off(struct msm_dp_ctrl *msm_dp_ctrl);
void msm_dp_ctrl_push_idle(struct msm_dp_ctrl *msm_dp_ctrl);
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index 2a84a6ffedbb..d094d484474c 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -617,7 +617,42 @@ static int msm_dp_display_set_mode(struct msm_dp *msm_dp_display,
return 0;
}
-static int msm_dp_display_enable(struct msm_dp_display_private *dp, bool force_link_train)
+static int msm_dp_display_prepare_link(struct msm_dp_display_private *dp)
+{
+ struct msm_dp *msm_dp_display = &dp->msm_dp_display;
+ int rc = 0;
+ bool force_link_train = false;
+
+ drm_dbg_dp(dp->drm_dev, "sink_count=%d\n", dp->link->sink_count);
+
+ if (msm_dp_display->is_edp)
+ msm_dp_hpd_plug_handle(dp);
+
+ rc = pm_runtime_resume_and_get(&msm_dp_display->pdev->dev);
+ if (rc) {
+ DRM_ERROR("failed to pm_runtime_resume\n");
+ return rc;
+ }
+
+ if (dp->link->sink_count == 0)
+ return -ENOTCONN;
+
+ if (!msm_dp_display->power_on) {
+ msm_dp_display_host_phy_init(dp);
+ force_link_train = true;
+ }
+
+ rc = msm_dp_ctrl_on_link(dp->ctrl);
+ if (rc) {
+ DRM_ERROR("Failed link training (rc=%d)\n", rc);
+ // TODO: schedule drm_connector_set_link_status_property()
+ return rc;
+ }
+
+ return msm_dp_ctrl_prepare_stream_on(dp->ctrl, force_link_train);
+}
+
+static int msm_dp_display_enable(struct msm_dp_display_private *dp)
{
int rc = 0;
struct msm_dp *msm_dp_display = &dp->msm_dp_display;
@@ -628,7 +663,7 @@ static int msm_dp_display_enable(struct msm_dp_display_private *dp, bool force_l
return 0;
}
- rc = msm_dp_ctrl_on_stream(dp->ctrl, force_link_train);
+ rc = msm_dp_ctrl_on_stream(dp->ctrl);
if (!rc)
msm_dp_display->power_on = true;
@@ -658,13 +693,10 @@ static int msm_dp_display_post_enable(struct msm_dp *msm_dp_display)
return 0;
}
-static int msm_dp_display_disable(struct msm_dp_display_private *dp)
+static void msm_dp_display_audio_notify_disable(struct msm_dp_display_private *dp)
{
struct msm_dp *msm_dp_display = &dp->msm_dp_display;
- if (!msm_dp_display->power_on)
- return 0;
-
/* wait only if audio was enabled */
if (msm_dp_display->audio_enabled) {
/* signal the disconnect event */
@@ -675,6 +707,14 @@ static int msm_dp_display_disable(struct msm_dp_display_private *dp)
}
msm_dp_display->audio_enabled = false;
+}
+
+static int msm_dp_display_disable(struct msm_dp_display_private *dp)
+{
+ struct msm_dp *msm_dp_display = &dp->msm_dp_display;
+
+ if (!msm_dp_display->power_on)
+ return 0;
if (dp->link->sink_count == 0) {
/*
@@ -1395,45 +1435,29 @@ void msm_dp_bridge_atomic_enable(struct drm_bridge *drm_bridge,
struct drm_atomic_commit *state)
{
struct msm_dp_bridge *msm_dp_bridge = to_dp_bridge(drm_bridge);
- struct msm_dp *dp = msm_dp_bridge->msm_dp_display;
+ struct msm_dp *msm_dp_display = msm_dp_bridge->msm_dp_display;
int rc = 0;
- struct msm_dp_display_private *msm_dp_display;
- bool force_link_train = false;
-
- msm_dp_display = container_of(dp, struct msm_dp_display_private, msm_dp_display);
-
- if (dp->is_edp)
- msm_dp_hpd_plug_handle(msm_dp_display);
-
- if (pm_runtime_resume_and_get(&dp->pdev->dev)) {
- DRM_ERROR("failed to pm_runtime_resume\n");
- return;
- }
-
- if (msm_dp_display->link->sink_count == 0)
- return;
+ struct msm_dp_display_private *dp;
- if (!dp->power_on) {
- msm_dp_display_host_phy_init(msm_dp_display);
- force_link_train = true;
- }
+ dp = container_of(msm_dp_display, struct msm_dp_display_private, msm_dp_display);
- rc = msm_dp_ctrl_on_link(msm_dp_display->ctrl);
+ rc = msm_dp_display_prepare_link(dp);
if (rc) {
- DRM_ERROR("Failed link training (rc=%d)\n", rc);
- // TODO: schedule drm_connector_set_link_status_property()
+ DRM_ERROR("DP display prepare failed, rc=%d\n", rc);
return;
}
- msm_dp_display_enable(msm_dp_display, force_link_train);
+ rc = msm_dp_display_enable(dp);
+ if (rc)
+ DRM_ERROR("DP display enable failed, rc=%d\n", rc);
- rc = msm_dp_display_post_enable(dp);
+ rc = msm_dp_display_post_enable(msm_dp_display);
if (rc) {
DRM_ERROR("DP display post enable failed, rc=%d\n", rc);
- msm_dp_display_disable(msm_dp_display);
+ msm_dp_display_disable(dp);
}
- drm_dbg_dp(dp->drm_dev, "type=%d Done\n", dp->connector_type);
+ drm_dbg_dp(msm_dp_display->drm_dev, "type=%d Done\n", msm_dp_display->connector_type);
}
void msm_dp_bridge_atomic_disable(struct drm_bridge *drm_bridge,
@@ -1448,6 +1472,15 @@ void msm_dp_bridge_atomic_disable(struct drm_bridge *drm_bridge,
msm_dp_ctrl_push_idle(msm_dp_display->ctrl);
}
+static void msm_dp_display_unprepare(struct msm_dp_display_private *dp)
+{
+ struct msm_dp *msm_dp_display = &dp->msm_dp_display;
+
+ pm_runtime_put_sync(&msm_dp_display->pdev->dev);
+
+ drm_dbg_dp(dp->drm_dev, "type=%d Done\n", msm_dp_display->connector_type);
+}
+
void msm_dp_bridge_atomic_post_disable(struct drm_bridge *drm_bridge,
struct drm_atomic_commit *state)
{
@@ -1460,11 +1493,11 @@ void msm_dp_bridge_atomic_post_disable(struct drm_bridge *drm_bridge,
if (dp->is_edp)
msm_dp_hpd_unplug_handle(msm_dp_display);
- msm_dp_display_disable(msm_dp_display);
+ msm_dp_display_audio_notify_disable(msm_dp_display);
- drm_dbg_dp(dp->drm_dev, "type=%d Done\n", dp->connector_type);
+ msm_dp_display_disable(msm_dp_display);
- pm_runtime_put_sync(&dp->pdev->dev);
+ msm_dp_display_unprepare(msm_dp_display);
}
void msm_dp_bridge_hpd_enable(struct drm_bridge *bridge)
--
2.43.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v9 08/14] drm/msm/dp: break up dp_display_enable into two parts
2026-07-28 10:21 ` [PATCH v9 08/14] drm/msm/dp: break up dp_display_enable into two parts Yongxing Mou
@ 2026-07-28 10:34 ` sashiko-bot
0 siblings, 0 replies; 22+ messages in thread
From: sashiko-bot @ 2026-07-28 10:34 UTC (permalink / raw)
To: Yongxing Mou; +Cc: dri-devel
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [High] Silent inversion of hardware initialization sequence (pixel clock and link retraining)
- [Medium] API split fails to support inserting MST payload programming as claimed
Pre-existing issues:
- [High] Unpowered hardware register access causes kernel panic on error path
--
--- Patch [8]: [PATCH v9 08/14] drm/msm/dp: break up dp_display_enable into two parts ---
commit 722ccac6521827b0527a34404c0521cf56234857
Author: Abhinav Kumar <quic_abhinavk@quicinc.com>
drm/msm/dp: break up dp_display_enable into two parts
Split dp_display_enable into prepare and enable APIs. This aims to
support MST bridges_enable inserting MST payload functions between
enabling stream clocks and programming registers.
> dp_display_enable() currently re-trains the link if needed and then
> enables the pixel clock, programs the controller to start sending the
> pixel stream. Split these two parts into prepare/enable APIs, to support
> MST bridges_enable insert the MST payloads funcs between enable
> stream_clks and program register.
[Severity: Medium]
Does this split actually allow inserting MST payload functions between
enabling stream clocks and programming the registers?
Looking at the implementation of msm_dp_ctrl_on_stream() below, the call to
msm_dp_ctrl_on_pixel_clk() and the subsequent register programming are still
tightly coupled in the same function. It doesn't seem possible for an
external caller to inject payload programming between them.
> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> index 068d4f8b673f6..43094dda34349 100644
> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
[ ... ]
> -int msm_dp_ctrl_on_stream(struct msm_dp_ctrl *msm_dp_ctrl, bool force_link_train)
> +int msm_dp_ctrl_prepare_stream_on(struct msm_dp_ctrl *msm_dp_ctrl, bool force_link_train)
> {
> int ret = 0;
[ ... ]
> - ret = msm_dp_ctrl_on_pixel_clk(ctrl, pixel_rate);
> - if (ret)
> - return ret;
> -
> if (force_link_train || !msm_dp_ctrl_channel_eq_ok(ctrl))
> msm_dp_ctrl_link_retrain(ctrl);
[Severity: High]
Does this change invert the hardware initialization sequence?
Before this patch, msm_dp_ctrl_on_stream() called msm_dp_ctrl_on_pixel_clk()
to enable the pixel clock before performing the link retraining.
With the new split, msm_dp_ctrl_prepare_stream_on() performs link retraining,
but the pixel clock isn't enabled until msm_dp_ctrl_on_stream() is called
later. Could this cause link training failures or black screens by retraining
before the pixel clock is ready?
[ ... ]
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> index 2a84a6ffedbb5..d094d484474cd 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
[ ... ]
> -static int msm_dp_display_enable(struct msm_dp_display_private *dp, bool force_link_train)
> +static int msm_dp_display_prepare_link(struct msm_dp_display_private *dp)
> +{
> + struct msm_dp *msm_dp_display = &dp->msm_dp_display;
> + int rc = 0;
> + bool force_link_train = false;
> +
> + drm_dbg_dp(dp->drm_dev, "sink_count=%d\n", dp->link->sink_count);
> +
> + if (msm_dp_display->is_edp)
> + msm_dp_hpd_plug_handle(dp);
> +
> + rc = pm_runtime_resume_and_get(&msm_dp_display->pdev->dev);
> + if (rc) {
> + DRM_ERROR("failed to pm_runtime_resume\n");
> + return rc;
> + }
[Severity: High]
This is a pre-existing issue, but if pm_runtime_resume_and_get() fails here,
the device is left unpowered while the DRM state considers it enabled.
Since msm_dp_bridge_atomic_enable() returns void and cannot abort the overall
atomic commit if this fails, msm_dp_bridge_atomic_disable() will
unconditionally be called later during teardown.
That disable path eventually calls msm_dp_ctrl_push_idle() which accesses
unclocked and unpowered DP hardware registers, leading to a synchronous
external abort (kernel panic).
Should there be a way to prevent hardware access during disable if the
device never successfully resumed?
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260728-dp_mstclean-v9-0-f7779fce10f4@oss.qualcomm.com?part=8
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v9 09/14] drm/msm/dp: re-arrange dp_display_disable() into functional parts
2026-07-28 10:21 [PATCH v9 00/14] drm/msm/dp: Prerequisite cleanup for upcoming MST support Yongxing Mou
` (7 preceding siblings ...)
2026-07-28 10:21 ` [PATCH v9 08/14] drm/msm/dp: break up dp_display_enable into two parts Yongxing Mou
@ 2026-07-28 10:21 ` Yongxing Mou
2026-07-28 10:39 ` sashiko-bot
2026-07-28 10:21 ` [PATCH v9 10/14] drm/msm/dp: allow dp_ctrl stream APIs to use any panel passed to it Yongxing Mou
` (4 subsequent siblings)
13 siblings, 1 reply; 22+ messages in thread
From: Yongxing Mou @ 2026-07-28 10:21 UTC (permalink / raw)
To: Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang,
Sean Paul, Marijn Suijten, David Airlie, Simona Vetter
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel, Yongxing Mou,
Abhinav Kumar, Dmitry Baryshkov
From: Abhinav Kumar <quic_abhinavk@quicinc.com>
dp_display_disable() handles special case of when monitor is
disconnected from the dongle while the dongle stays connected
thereby needing a separate function dp_ctrl_off_link_stream()
for this. However with a slight rework this can still be handled
by keeping common paths same for regular and special case.
Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
Signed-off-by: Yongxing Mou <yongxing.mou@oss.qualcomm.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
drivers/gpu/drm/msm/dp/dp_ctrl.c | 19 +------------------
drivers/gpu/drm/msm/dp/dp_ctrl.h | 2 +-
drivers/gpu/drm/msm/dp/dp_display.c | 10 +++++++++-
3 files changed, 11 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
index 43094dda3434..272ac3b25b13 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
@@ -2605,7 +2605,7 @@ int msm_dp_ctrl_on_stream(struct msm_dp_ctrl *msm_dp_ctrl)
return ret;
}
-void msm_dp_ctrl_off_link_stream(struct msm_dp_ctrl *msm_dp_ctrl)
+void msm_dp_ctrl_reinit_phy(struct msm_dp_ctrl *msm_dp_ctrl)
{
struct msm_dp_ctrl_private *ctrl;
struct phy *phy;
@@ -2613,23 +2613,6 @@ void msm_dp_ctrl_off_link_stream(struct msm_dp_ctrl *msm_dp_ctrl)
ctrl = container_of(msm_dp_ctrl, struct msm_dp_ctrl_private, msm_dp_ctrl);
phy = ctrl->phy;
- msm_dp_panel_disable_vsc_sdp(ctrl->panel);
-
- /* set dongle to D3 (power off) mode */
- msm_dp_link_psm_config(ctrl->link, &ctrl->panel->link_info, true);
-
- msm_dp_ctrl_mainlink_disable(ctrl);
-
- if (ctrl->stream_clks_on) {
- clk_disable_unprepare(ctrl->pixel_clk);
- ctrl->stream_clks_on = false;
- }
-
- dev_pm_opp_set_rate(ctrl->dev, 0);
- msm_dp_ctrl_link_clk_disable(&ctrl->msm_dp_ctrl);
-
- phy_power_off(phy);
-
/* aux channel down, reinit phy */
phy_exit(phy);
phy_init(phy);
diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.h b/drivers/gpu/drm/msm/dp/dp_ctrl.h
index 1497f1a8fc2f..5d615f50d13b 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.h
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.h
@@ -19,7 +19,6 @@ struct phy;
int msm_dp_ctrl_on_link(struct msm_dp_ctrl *msm_dp_ctrl);
int msm_dp_ctrl_on_stream(struct msm_dp_ctrl *msm_dp_ctrl);
int msm_dp_ctrl_prepare_stream_on(struct msm_dp_ctrl *msm_dp_ctrl, bool force_link_train);
-void msm_dp_ctrl_off_link_stream(struct msm_dp_ctrl *msm_dp_ctrl);
void msm_dp_ctrl_off(struct msm_dp_ctrl *msm_dp_ctrl);
void msm_dp_ctrl_push_idle(struct msm_dp_ctrl *msm_dp_ctrl);
irqreturn_t msm_dp_ctrl_isr(struct msm_dp_ctrl *msm_dp_ctrl);
@@ -46,4 +45,5 @@ void msm_dp_ctrl_core_clk_disable(struct msm_dp_ctrl *msm_dp_ctrl);
void msm_dp_ctrl_enable_irq(struct msm_dp_ctrl *msm_dp_ctrl);
void msm_dp_ctrl_disable_irq(struct msm_dp_ctrl *msm_dp_ctrl);
+void msm_dp_ctrl_reinit_phy(struct msm_dp_ctrl *msm_dp_ctrl);
#endif /* _DP_CTRL_H_ */
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index d094d484474c..cd8a3b171d9d 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -716,12 +716,20 @@ static int msm_dp_display_disable(struct msm_dp_display_private *dp)
if (!msm_dp_display->power_on)
return 0;
+ msm_dp_panel_disable_vsc_sdp(dp->panel);
+
+ /* dongle is still connected but sinks are disconnected */
if (dp->link->sink_count == 0) {
/*
* irq_hpd with sink_count = 0
* hdmi unplugged out of dongle
*/
- msm_dp_ctrl_off_link_stream(dp->ctrl);
+
+ /* set dongle to D3 (power off) mode */
+ msm_dp_link_psm_config(dp->link, &dp->panel->link_info, true);
+ msm_dp_ctrl_off(dp->ctrl);
+ /* re-init the PHY so that we can listen to Dongle disconnect */
+ msm_dp_ctrl_reinit_phy(dp->ctrl);
} else {
/*
* unplugged interrupt
--
2.43.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v9 09/14] drm/msm/dp: re-arrange dp_display_disable() into functional parts
2026-07-28 10:21 ` [PATCH v9 09/14] drm/msm/dp: re-arrange dp_display_disable() into functional parts Yongxing Mou
@ 2026-07-28 10:39 ` sashiko-bot
0 siblings, 0 replies; 22+ messages in thread
From: sashiko-bot @ 2026-07-28 10:39 UTC (permalink / raw)
To: Yongxing Mou; +Cc: dri-devel
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Redundant call to msm_dp_panel_disable_vsc_sdp() transmits Secondary Data Packet (SDP) updates over the main link to a sink that has already been powered down.
--
--- Patch [9]: [PATCH v9 09/14] drm/msm/dp: re-arrange dp_display_disable() into functional parts ---
commit 71c4fed95d00e4e9b083de79dc56ede059ae5722
Author: Abhinav Kumar <quic_abhinavk@quicinc.com>
drm/msm/dp: re-arrange dp_display_disable() into functional parts
dp_display_disable() handles special case of when monitor is
disconnected from the dongle while the dongle stays connected
thereby needing a separate function dp_ctrl_off_link_stream()
for this. However with a slight rework this can still be handled
by keeping common paths same for regular and special case.
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> index d094d484474cd..cd8a3b171d9db 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -716,12 +716,20 @@ static int msm_dp_display_disable(struct msm_dp_display_private *dp)
> if (!msm_dp_display->power_on)
> return 0;
>
> + msm_dp_panel_disable_vsc_sdp(dp->panel);
> +
> + /* dongle is still connected but sinks are disconnected */
> if (dp->link->sink_count == 0) {
> /*
> * irq_hpd with sink_count = 0
> * hdmi unplugged out of dongle
> */
> - msm_dp_ctrl_off_link_stream(dp->ctrl);
> +
> + /* set dongle to D3 (power off) mode */
> + msm_dp_link_psm_config(dp->link, &dp->panel->link_info, true);
> + msm_dp_ctrl_off(dp->ctrl);
[Severity: High]
Does this code call msm_dp_panel_disable_vsc_sdp() a second time after the
sink is already powered down?
msm_dp_display_disable() first disables the VSC SDP:
msm_dp_panel_disable_vsc_sdp(dp->panel);
And then it places the sink into D3 power down state:
msm_dp_link_psm_config(dp->link, &dp->panel->link_info, true);
However, msm_dp_ctrl_off() also inherently calls
msm_dp_panel_disable_vsc_sdp().
Because msm_dp_panel_disable_vsc_sdp() writes to configuration registers
and triggers msm_dp_panel_update_sdp() to transmit an SDP over the main link,
could this redundant call cause the DP controller to timeout waiting for a
response from the already powered-down sink, or leave the display hardware
unresponsive?
> + /* re-init the PHY so that we can listen to Dongle disconnect */
> + msm_dp_ctrl_reinit_phy(dp->ctrl);
> } else {
> /*
> * unplugged interrupt
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260728-dp_mstclean-v9-0-f7779fce10f4@oss.qualcomm.com?part=9
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v9 10/14] drm/msm/dp: allow dp_ctrl stream APIs to use any panel passed to it
2026-07-28 10:21 [PATCH v9 00/14] drm/msm/dp: Prerequisite cleanup for upcoming MST support Yongxing Mou
` (8 preceding siblings ...)
2026-07-28 10:21 ` [PATCH v9 09/14] drm/msm/dp: re-arrange dp_display_disable() into functional parts Yongxing Mou
@ 2026-07-28 10:21 ` Yongxing Mou
2026-07-28 10:21 ` [PATCH v9 11/14] drm/msm/dp: split dp_ctrl_off() into stream and link parts Yongxing Mou
` (3 subsequent siblings)
13 siblings, 0 replies; 22+ messages in thread
From: Yongxing Mou @ 2026-07-28 10:21 UTC (permalink / raw)
To: Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang,
Sean Paul, Marijn Suijten, David Airlie, Simona Vetter
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel, Yongxing Mou,
Abhinav Kumar, Dmitry Baryshkov
From: Abhinav Kumar <quic_abhinavk@quicinc.com>
With MST, multiple sinks share a single DP controller, so a cached
panel in msm_dp_ctrl_private can no longer represent the per-stream
sink. Drop the cache and pass panel explicitly to all stream-related
dp_ctrl APIs.
Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
Signed-off-by: Yongxing Mou <yongxing.mou@oss.qualcomm.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
drivers/gpu/drm/msm/dp/dp_ctrl.c | 201 ++++++++++++++++++++----------------
drivers/gpu/drm/msm/dp/dp_ctrl.h | 28 +++--
drivers/gpu/drm/msm/dp/dp_display.c | 26 ++---
3 files changed, 143 insertions(+), 112 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
index 272ac3b25b13..9bed5a2fd2e7 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
@@ -114,7 +114,6 @@ struct msm_dp_ctrl_private {
struct drm_device *drm_dev;
struct device *dev;
struct drm_dp_aux *aux;
- struct msm_dp_panel *panel;
struct msm_dp_link *link;
void __iomem *ahb_base;
void __iomem *link_base;
@@ -202,7 +201,8 @@ static int msm_dp_aux_link_configure(struct drm_dp_aux *aux,
/*
* NOTE: resetting DP controller will also clear any pending HPD related interrupts
*/
-void msm_dp_ctrl_reset(struct msm_dp_ctrl *msm_dp_ctrl)
+void msm_dp_ctrl_reset(struct msm_dp_ctrl *msm_dp_ctrl,
+ struct msm_dp_panel *panel)
{
struct msm_dp_ctrl_private *ctrl =
container_of(msm_dp_ctrl, struct msm_dp_ctrl_private, msm_dp_ctrl);
@@ -217,10 +217,9 @@ void msm_dp_ctrl_reset(struct msm_dp_ctrl *msm_dp_ctrl)
sw_reset &= ~DP_SW_RESET;
msm_dp_write_ahb(ctrl, REG_DP_SW_RESET, sw_reset);
- if (!ctrl->hw_revision) {
+ if (!ctrl->hw_revision)
ctrl->hw_revision = msm_dp_read_ahb(ctrl, REG_DP_HW_VERSION);
- ctrl->panel->hw_revision = ctrl->hw_revision;
- }
+ panel->hw_revision = ctrl->hw_revision;
}
static u32 msm_dp_ctrl_get_aux_interrupt(struct msm_dp_ctrl_private *ctrl)
@@ -418,10 +417,11 @@ static void msm_dp_ctrl_config_ctrl_streams(struct msm_dp_ctrl_private *ctrl,
msm_dp_write_link(ctrl, REG_DP_CONFIGURATION_CTRL, config);
}
-static void msm_dp_ctrl_config_ctrl_link(struct msm_dp_ctrl_private *ctrl)
+static void msm_dp_ctrl_config_ctrl_link(struct msm_dp_ctrl_private *ctrl,
+ struct msm_dp_panel *panel)
{
u32 config = 0;
- const u8 *dpcd = ctrl->panel->dpcd;
+ const u8 *dpcd = panel->dpcd;
/* Default-> LSCLK DIV: 1/4 LCLK */
config |= (2 << DP_CONFIGURATION_CTRL_LSCLK_DIV_SHIFT);
@@ -484,13 +484,14 @@ static void msm_dp_ctrl_config_misc1_misc0(struct msm_dp_ctrl_private *ctrl,
msm_dp_write_link(ctrl, REG_DP_MISC1_MISC0, misc_val);
}
-static void msm_dp_ctrl_configure_source_params(struct msm_dp_ctrl_private *ctrl)
+static void msm_dp_ctrl_configure_source_params(struct msm_dp_ctrl_private *ctrl,
+ struct msm_dp_panel *panel)
{
- msm_dp_ctrl_config_ctrl_streams(ctrl, ctrl->panel);
+ msm_dp_ctrl_config_ctrl_streams(ctrl, panel);
- msm_dp_ctrl_config_misc1_misc0(ctrl, ctrl->panel);
+ msm_dp_ctrl_config_misc1_misc0(ctrl, panel);
- msm_dp_panel_timing_cfg(ctrl->panel, ctrl->msm_dp_ctrl.wide_bus_en);
+ msm_dp_panel_timing_cfg(panel, ctrl->msm_dp_ctrl.wide_bus_en);
}
/*
@@ -1260,20 +1261,21 @@ static void _dp_ctrl_calc_tu(struct msm_dp_ctrl_private *ctrl,
}
static void msm_dp_ctrl_calc_tu_parameters(struct msm_dp_ctrl_private *ctrl,
+ struct msm_dp_panel *panel,
struct msm_dp_vc_tu_mapping_table *tu_table)
{
struct msm_dp_tu_calc_input in;
- struct drm_display_mode *drm_mode;
+ const struct drm_display_mode *drm_mode;
- drm_mode = &ctrl->panel->msm_dp_mode.drm_mode;
+ drm_mode = &panel->msm_dp_mode.drm_mode;
in.lclk = ctrl->link->link_params.rate / 1000;
in.pclk_khz = drm_mode->clock;
in.hactive = drm_mode->hdisplay;
in.hporch = drm_mode->htotal - drm_mode->hdisplay;
in.nlanes = ctrl->link->link_params.num_lanes;
- in.bpp = ctrl->panel->msm_dp_mode.bpp;
- in.pixel_enc = ctrl->panel->msm_dp_mode.out_fmt_is_yuv_420 ? 420 : 444;
+ in.bpp = panel->msm_dp_mode.bpp;
+ in.pixel_enc = panel->msm_dp_mode.out_fmt_is_yuv_420 ? 420 : 444;
in.dsc_en = 0;
in.async_en = 0;
in.fec_en = 0;
@@ -1283,14 +1285,15 @@ static void msm_dp_ctrl_calc_tu_parameters(struct msm_dp_ctrl_private *ctrl,
_dp_ctrl_calc_tu(ctrl, &in, tu_table);
}
-static void msm_dp_ctrl_setup_tr_unit(struct msm_dp_ctrl_private *ctrl)
+static void msm_dp_ctrl_setup_tr_unit(struct msm_dp_ctrl_private *ctrl,
+ struct msm_dp_panel *panel)
{
u32 msm_dp_tu = 0x0;
u32 valid_boundary = 0x0;
u32 valid_boundary2 = 0x0;
struct msm_dp_vc_tu_mapping_table tu_calc_table;
- msm_dp_ctrl_calc_tu_parameters(ctrl, &tu_calc_table);
+ msm_dp_ctrl_calc_tu_parameters(ctrl, panel, &tu_calc_table);
msm_dp_tu |= tu_calc_table.tu_size_minus1;
valid_boundary |= tu_calc_table.valid_boundary_link;
@@ -1442,6 +1445,7 @@ static int msm_dp_ctrl_set_pattern_state_bit(struct msm_dp_ctrl_private *ctrl,
}
static int msm_dp_ctrl_link_train_1(struct msm_dp_ctrl_private *ctrl,
+ struct msm_dp_panel *panel,
int *training_step, enum drm_dp_phy dp_phy)
{
int delay_us;
@@ -1450,7 +1454,7 @@ static int msm_dp_ctrl_link_train_1(struct msm_dp_ctrl_private *ctrl,
int const maximum_retries = 4;
delay_us = drm_dp_read_clock_recovery_delay(ctrl->aux,
- ctrl->panel->dpcd, dp_phy, false);
+ panel->dpcd, dp_phy, false);
msm_dp_write_link(ctrl, REG_DP_STATE_CTRL, 0);
@@ -1536,14 +1540,15 @@ static int msm_dp_ctrl_link_rate_down_shift(struct msm_dp_ctrl_private *ctrl)
return ret;
}
-static int msm_dp_ctrl_link_lane_down_shift(struct msm_dp_ctrl_private *ctrl)
+static int msm_dp_ctrl_link_lane_down_shift(struct msm_dp_ctrl_private *ctrl,
+ struct msm_dp_panel *panel)
{
if (ctrl->link->link_params.num_lanes == 1)
return -1;
ctrl->link->link_params.num_lanes /= 2;
- ctrl->link->link_params.rate = ctrl->panel->link_info.rate;
+ ctrl->link->link_params.rate = panel->link_info.rate;
ctrl->link->phy_params.p_level = 0;
ctrl->link->phy_params.v_level = 0;
@@ -1552,6 +1557,7 @@ static int msm_dp_ctrl_link_lane_down_shift(struct msm_dp_ctrl_private *ctrl)
}
static void msm_dp_ctrl_clear_training_pattern(struct msm_dp_ctrl_private *ctrl,
+ struct msm_dp_panel *panel,
enum drm_dp_phy dp_phy)
{
int delay_us;
@@ -1559,11 +1565,12 @@ static void msm_dp_ctrl_clear_training_pattern(struct msm_dp_ctrl_private *ctrl,
msm_dp_ctrl_train_pattern_set(ctrl, DP_TRAINING_PATTERN_DISABLE, dp_phy);
delay_us = drm_dp_read_channel_eq_delay(ctrl->aux,
- ctrl->panel->dpcd, dp_phy, false);
+ panel->dpcd, dp_phy, false);
fsleep(delay_us);
}
static int msm_dp_ctrl_link_train_2(struct msm_dp_ctrl_private *ctrl,
+ struct msm_dp_panel *panel,
int *training_step, enum drm_dp_phy dp_phy)
{
int delay_us;
@@ -1574,16 +1581,16 @@ static int msm_dp_ctrl_link_train_2(struct msm_dp_ctrl_private *ctrl,
u8 link_status[DP_LINK_STATUS_SIZE];
delay_us = drm_dp_read_channel_eq_delay(ctrl->aux,
- ctrl->panel->dpcd, dp_phy, false);
+ panel->dpcd, dp_phy, false);
msm_dp_write_link(ctrl, REG_DP_STATE_CTRL, 0);
*training_step = DP_TRAINING_2;
- if (drm_dp_tps4_supported(ctrl->panel->dpcd)) {
+ if (drm_dp_tps4_supported(panel->dpcd)) {
pattern = DP_TRAINING_PATTERN_4;
state_ctrl_bit = 4;
- } else if (drm_dp_tps3_supported(ctrl->panel->dpcd)) {
+ } else if (drm_dp_tps3_supported(panel->dpcd)) {
pattern = DP_TRAINING_PATTERN_3;
state_ctrl_bit = 3;
} else {
@@ -1620,18 +1627,19 @@ static int msm_dp_ctrl_link_train_2(struct msm_dp_ctrl_private *ctrl,
}
static int msm_dp_ctrl_link_train_1_2(struct msm_dp_ctrl_private *ctrl,
+ struct msm_dp_panel *panel,
int *training_step, enum drm_dp_phy dp_phy)
{
int ret;
- ret = msm_dp_ctrl_link_train_1(ctrl, training_step, dp_phy);
+ ret = msm_dp_ctrl_link_train_1(ctrl, panel, training_step, dp_phy);
if (ret) {
DRM_ERROR("link training #1 on phy %d failed. ret=%d\n", dp_phy, ret);
return ret;
}
drm_dbg_dp(ctrl->drm_dev, "link training #1 on phy %d successful\n", dp_phy);
- ret = msm_dp_ctrl_link_train_2(ctrl, training_step, dp_phy);
+ ret = msm_dp_ctrl_link_train_2(ctrl, panel, training_step, dp_phy);
if (ret) {
DRM_ERROR("link training #2 on phy %d failed. ret=%d\n", dp_phy, ret);
return ret;
@@ -1642,17 +1650,18 @@ static int msm_dp_ctrl_link_train_1_2(struct msm_dp_ctrl_private *ctrl,
}
static int msm_dp_ctrl_link_train(struct msm_dp_ctrl_private *ctrl,
+ struct msm_dp_panel *panel,
int *training_step)
{
int i;
int ret = 0;
- const u8 *dpcd = ctrl->panel->dpcd;
+ const u8 *dpcd = panel->dpcd;
u8 encoding[] = { 0, DP_SET_ANSI_8B10B };
u8 assr;
struct msm_dp_link_info link_info = {0};
- msm_dp_ctrl_config_ctrl_link(ctrl);
- msm_dp_ctrl_config_ctrl_streams(ctrl, ctrl->panel);
+ msm_dp_ctrl_config_ctrl_link(ctrl, panel);
+ msm_dp_ctrl_config_ctrl_streams(ctrl, panel);
link_info.num_lanes = ctrl->link->link_params.num_lanes;
link_info.rate = ctrl->link->link_params.rate;
@@ -1675,8 +1684,8 @@ static int msm_dp_ctrl_link_train(struct msm_dp_ctrl_private *ctrl,
for (i = ctrl->link->lttpr_count - 1; i >= 0; i--) {
enum drm_dp_phy dp_phy = DP_PHY_LTTPR(i);
- ret = msm_dp_ctrl_link_train_1_2(ctrl, training_step, dp_phy);
- msm_dp_ctrl_clear_training_pattern(ctrl, dp_phy);
+ ret = msm_dp_ctrl_link_train_1_2(ctrl, panel, training_step, dp_phy);
+ msm_dp_ctrl_clear_training_pattern(ctrl, panel, dp_phy);
if (ret)
break;
@@ -1687,7 +1696,7 @@ static int msm_dp_ctrl_link_train(struct msm_dp_ctrl_private *ctrl,
goto end;
}
- ret = msm_dp_ctrl_link_train_1_2(ctrl, training_step, DP_PHY_DPRX);
+ ret = msm_dp_ctrl_link_train_1_2(ctrl, panel, training_step, DP_PHY_DPRX);
if (ret) {
DRM_ERROR("link training on sink failed. ret=%d\n", ret);
goto end;
@@ -1700,6 +1709,7 @@ static int msm_dp_ctrl_link_train(struct msm_dp_ctrl_private *ctrl,
}
static int msm_dp_ctrl_setup_main_link(struct msm_dp_ctrl_private *ctrl,
+ struct msm_dp_panel *panel,
int *training_step)
{
int ret = 0;
@@ -1715,7 +1725,7 @@ static int msm_dp_ctrl_setup_main_link(struct msm_dp_ctrl_private *ctrl,
* a link training pattern, we have to first do soft reset.
*/
- ret = msm_dp_ctrl_link_train(ctrl, training_step);
+ ret = msm_dp_ctrl_link_train(ctrl, panel, training_step);
return ret;
}
@@ -1814,11 +1824,12 @@ static void msm_dp_ctrl_link_clk_disable(struct msm_dp_ctrl *msm_dp_ctrl)
str_on_off(ctrl->core_clks_on));
}
-static int msm_dp_ctrl_enable_mainlink_clocks(struct msm_dp_ctrl_private *ctrl)
+static int msm_dp_ctrl_enable_mainlink_clocks(struct msm_dp_ctrl_private *ctrl,
+ struct msm_dp_panel *panel)
{
int ret = 0;
struct phy *phy = ctrl->phy;
- const u8 *dpcd = ctrl->panel->dpcd;
+ const u8 *dpcd = panel->dpcd;
ctrl->phy_opts.dp.lanes = ctrl->link->link_params.num_lanes;
ctrl->phy_opts.dp.link_rate = ctrl->link->link_params.rate / 100;
@@ -1870,13 +1881,14 @@ static void msm_dp_ctrl_psr_exit(struct msm_dp_ctrl_private *ctrl)
msm_dp_write_link(ctrl, REG_PSR_CMD, cmd);
}
-void msm_dp_ctrl_config_psr(struct msm_dp_ctrl *msm_dp_ctrl)
+void msm_dp_ctrl_config_psr(struct msm_dp_ctrl *msm_dp_ctrl,
+ struct msm_dp_panel *panel)
{
struct msm_dp_ctrl_private *ctrl = container_of(msm_dp_ctrl,
struct msm_dp_ctrl_private, msm_dp_ctrl);
u32 cfg;
- if (!ctrl->panel->psr_cap.version)
+ if (!panel->psr_cap.version)
return;
/* enable PSR1 function */
@@ -1891,12 +1903,13 @@ void msm_dp_ctrl_config_psr(struct msm_dp_ctrl *msm_dp_ctrl)
drm_dp_dpcd_write(ctrl->aux, DP_PSR_EN_CFG, &cfg, 1);
}
-void msm_dp_ctrl_set_psr(struct msm_dp_ctrl *msm_dp_ctrl, bool enter)
+void msm_dp_ctrl_set_psr(struct msm_dp_ctrl *msm_dp_ctrl,
+ struct msm_dp_panel *panel, bool enter)
{
struct msm_dp_ctrl_private *ctrl = container_of(msm_dp_ctrl,
struct msm_dp_ctrl_private, msm_dp_ctrl);
- if (!ctrl->panel->psr_cap.version)
+ if (!panel->psr_cap.version)
return;
/*
@@ -1966,7 +1979,8 @@ void msm_dp_ctrl_phy_exit(struct msm_dp_ctrl *msm_dp_ctrl)
phy_exit(phy);
}
-static int msm_dp_ctrl_reinitialize_mainlink(struct msm_dp_ctrl_private *ctrl)
+static int msm_dp_ctrl_reinitialize_mainlink(struct msm_dp_ctrl_private *ctrl,
+ struct msm_dp_panel *panel)
{
struct phy *phy = ctrl->phy;
int ret = 0;
@@ -1987,7 +2001,7 @@ static int msm_dp_ctrl_reinitialize_mainlink(struct msm_dp_ctrl_private *ctrl)
/* hw recommended delay before re-enabling clocks */
msleep(20);
- ret = msm_dp_ctrl_enable_mainlink_clocks(ctrl);
+ ret = msm_dp_ctrl_enable_mainlink_clocks(ctrl, panel);
if (ret) {
DRM_ERROR("Failed to enable mainlink clks. ret=%d\n", ret);
return ret;
@@ -1996,7 +2010,8 @@ static int msm_dp_ctrl_reinitialize_mainlink(struct msm_dp_ctrl_private *ctrl)
return ret;
}
-static int msm_dp_ctrl_deinitialize_mainlink(struct msm_dp_ctrl_private *ctrl)
+static int msm_dp_ctrl_deinitialize_mainlink(struct msm_dp_ctrl_private *ctrl,
+ struct msm_dp_panel *panel)
{
struct phy *phy;
@@ -2004,7 +2019,7 @@ static int msm_dp_ctrl_deinitialize_mainlink(struct msm_dp_ctrl_private *ctrl)
msm_dp_ctrl_mainlink_disable(ctrl);
- msm_dp_ctrl_reset(&ctrl->msm_dp_ctrl);
+ msm_dp_ctrl_reset(&ctrl->msm_dp_ctrl, panel);
dev_pm_opp_set_rate(ctrl->dev, 0);
msm_dp_ctrl_link_clk_disable(&ctrl->msm_dp_ctrl);
@@ -2018,7 +2033,8 @@ static int msm_dp_ctrl_deinitialize_mainlink(struct msm_dp_ctrl_private *ctrl)
return 0;
}
-static int msm_dp_ctrl_link_maintenance(struct msm_dp_ctrl_private *ctrl)
+static int msm_dp_ctrl_link_maintenance(struct msm_dp_ctrl_private *ctrl,
+ struct msm_dp_panel *panel)
{
int ret = 0;
int training_step = DP_TRAINING_NONE;
@@ -2028,11 +2044,11 @@ static int msm_dp_ctrl_link_maintenance(struct msm_dp_ctrl_private *ctrl)
ctrl->link->phy_params.p_level = 0;
ctrl->link->phy_params.v_level = 0;
- ret = msm_dp_ctrl_setup_main_link(ctrl, &training_step);
+ ret = msm_dp_ctrl_setup_main_link(ctrl, panel, &training_step);
if (ret)
goto end;
- msm_dp_ctrl_clear_training_pattern(ctrl, DP_PHY_DPRX);
+ msm_dp_ctrl_clear_training_pattern(ctrl, panel, DP_PHY_DPRX);
msm_dp_write_link(ctrl, REG_DP_STATE_CTRL, DP_STATE_CTRL_SEND_VIDEO);
@@ -2211,7 +2227,8 @@ static void msm_dp_ctrl_off_pixel_clk(struct msm_dp_ctrl *msm_dp_ctrl)
}
}
-static int msm_dp_ctrl_process_phy_test_request(struct msm_dp_ctrl_private *ctrl)
+static int msm_dp_ctrl_process_phy_test_request(struct msm_dp_ctrl_private *ctrl,
+ struct msm_dp_panel *panel)
{
int ret;
unsigned long pixel_rate;
@@ -2227,15 +2244,15 @@ static int msm_dp_ctrl_process_phy_test_request(struct msm_dp_ctrl_private *ctrl
* running. Add the global reset just before disabling the
* link clocks and core clocks.
*/
- msm_dp_ctrl_off(&ctrl->msm_dp_ctrl);
+ msm_dp_ctrl_off(&ctrl->msm_dp_ctrl, panel);
- ret = msm_dp_ctrl_on_link(&ctrl->msm_dp_ctrl);
+ ret = msm_dp_ctrl_on_link(&ctrl->msm_dp_ctrl, panel);
if (ret) {
DRM_ERROR("failed to enable DP link controller\n");
return ret;
}
- pixel_rate = ctrl->panel->msm_dp_mode.drm_mode.clock;
+ pixel_rate = panel->msm_dp_mode.drm_mode.clock;
ret = msm_dp_ctrl_on_pixel_clk(ctrl, pixel_rate);
if (ret)
return ret;
@@ -2245,7 +2262,8 @@ static int msm_dp_ctrl_process_phy_test_request(struct msm_dp_ctrl_private *ctrl
return 0;
}
-void msm_dp_ctrl_handle_sink_request(struct msm_dp_ctrl *msm_dp_ctrl)
+void msm_dp_ctrl_handle_sink_request(struct msm_dp_ctrl *msm_dp_ctrl,
+ struct msm_dp_panel *panel)
{
struct msm_dp_ctrl_private *ctrl;
u32 sink_request = 0x0;
@@ -2260,14 +2278,14 @@ void msm_dp_ctrl_handle_sink_request(struct msm_dp_ctrl *msm_dp_ctrl)
if (sink_request & DP_TEST_LINK_PHY_TEST_PATTERN) {
drm_dbg_dp(ctrl->drm_dev, "PHY_TEST_PATTERN request\n");
- if (msm_dp_ctrl_process_phy_test_request(ctrl)) {
+ if (msm_dp_ctrl_process_phy_test_request(ctrl, panel)) {
DRM_ERROR("process phy_test_req failed\n");
return;
}
}
if (sink_request & DP_LINK_STATUS_UPDATED) {
- if (msm_dp_ctrl_link_maintenance(ctrl)) {
+ if (msm_dp_ctrl_link_maintenance(ctrl, panel)) {
DRM_ERROR("LM failed: TEST_LINK_TRAINING\n");
return;
}
@@ -2275,7 +2293,7 @@ void msm_dp_ctrl_handle_sink_request(struct msm_dp_ctrl *msm_dp_ctrl)
if (sink_request & DP_TEST_LINK_TRAINING) {
msm_dp_link_send_test_response(ctrl->link);
- if (msm_dp_ctrl_link_maintenance(ctrl)) {
+ if (msm_dp_ctrl_link_maintenance(ctrl, panel)) {
DRM_ERROR("LM failed: TEST_LINK_TRAINING\n");
return;
}
@@ -2311,7 +2329,8 @@ static bool msm_dp_ctrl_channel_eq_ok(struct msm_dp_ctrl_private *ctrl)
return drm_dp_channel_eq_ok(link_status, num_lanes);
}
-int msm_dp_ctrl_on_link(struct msm_dp_ctrl *msm_dp_ctrl)
+int msm_dp_ctrl_on_link(struct msm_dp_ctrl *msm_dp_ctrl,
+ struct msm_dp_panel *panel)
{
int rc = 0;
struct msm_dp_ctrl_private *ctrl;
@@ -2327,8 +2346,8 @@ int msm_dp_ctrl_on_link(struct msm_dp_ctrl *msm_dp_ctrl)
ctrl = container_of(msm_dp_ctrl, struct msm_dp_ctrl_private, msm_dp_ctrl);
- rate = ctrl->panel->link_info.rate;
- pixel_rate = ctrl->panel->msm_dp_mode.drm_mode.clock;
+ rate = panel->link_info.rate;
+ pixel_rate = panel->msm_dp_mode.drm_mode.clock;
msm_dp_ctrl_core_clk_enable(&ctrl->msm_dp_ctrl);
@@ -2340,8 +2359,8 @@ int msm_dp_ctrl_on_link(struct msm_dp_ctrl *msm_dp_ctrl)
} else {
ctrl->link->link_params.rate = rate;
ctrl->link->link_params.num_lanes =
- ctrl->panel->link_info.num_lanes;
- if (ctrl->panel->msm_dp_mode.out_fmt_is_yuv_420)
+ panel->link_info.num_lanes;
+ if (panel->msm_dp_mode.out_fmt_is_yuv_420)
pixel_rate >>= 1;
}
@@ -2349,13 +2368,13 @@ int msm_dp_ctrl_on_link(struct msm_dp_ctrl *msm_dp_ctrl)
ctrl->link->link_params.rate, ctrl->link->link_params.num_lanes,
pixel_rate);
- rc = msm_dp_ctrl_enable_mainlink_clocks(ctrl);
+ rc = msm_dp_ctrl_enable_mainlink_clocks(ctrl, panel);
if (rc)
return rc;
while (--link_train_max_retries) {
training_step = DP_TRAINING_NONE;
- rc = msm_dp_ctrl_setup_main_link(ctrl, &training_step);
+ rc = msm_dp_ctrl_setup_main_link(ctrl, panel, &training_step);
if (rc == 0) {
/* training completed successfully */
break;
@@ -2374,7 +2393,7 @@ int msm_dp_ctrl_on_link(struct msm_dp_ctrl *msm_dp_ctrl)
* some lanes are ready,
* reduce lane number
*/
- rc = msm_dp_ctrl_link_lane_down_shift(ctrl);
+ rc = msm_dp_ctrl_link_lane_down_shift(ctrl, panel);
if (rc < 0) { /* lane == 1 already */
/* end with failure */
break;
@@ -2395,7 +2414,7 @@ int msm_dp_ctrl_on_link(struct msm_dp_ctrl *msm_dp_ctrl)
ctrl->link->link_params.num_lanes))
rc = msm_dp_ctrl_link_rate_down_shift(ctrl);
else
- rc = msm_dp_ctrl_link_lane_down_shift(ctrl);
+ rc = msm_dp_ctrl_link_lane_down_shift(ctrl, panel);
if (rc < 0) {
/* end with failure */
@@ -2403,10 +2422,10 @@ int msm_dp_ctrl_on_link(struct msm_dp_ctrl *msm_dp_ctrl)
}
/* stop link training before start re training */
- msm_dp_ctrl_clear_training_pattern(ctrl, DP_PHY_DPRX);
+ msm_dp_ctrl_clear_training_pattern(ctrl, panel, DP_PHY_DPRX);
}
- rc = msm_dp_ctrl_reinitialize_mainlink(ctrl);
+ rc = msm_dp_ctrl_reinitialize_mainlink(ctrl, panel);
if (rc) {
DRM_ERROR("Failed to reinitialize mainlink. rc=%d\n", rc);
break;
@@ -2427,20 +2446,21 @@ int msm_dp_ctrl_on_link(struct msm_dp_ctrl *msm_dp_ctrl)
* link training failed
* end txing train pattern here
*/
- msm_dp_ctrl_clear_training_pattern(ctrl, DP_PHY_DPRX);
+ msm_dp_ctrl_clear_training_pattern(ctrl, panel, DP_PHY_DPRX);
- msm_dp_ctrl_deinitialize_mainlink(ctrl);
+ msm_dp_ctrl_deinitialize_mainlink(ctrl, panel);
rc = -ECONNRESET;
}
return rc;
}
-static int msm_dp_ctrl_link_retrain(struct msm_dp_ctrl_private *ctrl)
+static int msm_dp_ctrl_link_retrain(struct msm_dp_ctrl_private *ctrl,
+ struct msm_dp_panel *panel)
{
int training_step = DP_TRAINING_NONE;
- return msm_dp_ctrl_setup_main_link(ctrl, &training_step);
+ return msm_dp_ctrl_setup_main_link(ctrl, panel, &training_step);
}
static void msm_dp_ctrl_config_msa(struct msm_dp_ctrl_private *ctrl,
@@ -2511,7 +2531,9 @@ static void msm_dp_ctrl_config_msa(struct msm_dp_ctrl_private *ctrl,
msm_dp_write_link(ctrl, REG_DP_SOFTWARE_NVID, nvid);
}
-int msm_dp_ctrl_prepare_stream_on(struct msm_dp_ctrl *msm_dp_ctrl, bool force_link_train)
+int msm_dp_ctrl_prepare_stream_on(struct msm_dp_ctrl *msm_dp_ctrl,
+ struct msm_dp_panel *panel,
+ bool force_link_train)
{
int ret = 0;
struct msm_dp_ctrl_private *ctrl;
@@ -2530,7 +2552,7 @@ int msm_dp_ctrl_prepare_stream_on(struct msm_dp_ctrl *msm_dp_ctrl, bool force_li
ctrl->core_clks_on, ctrl->link_clks_on, ctrl->stream_clks_on);
if (!ctrl->link_clks_on) { /* link clk is off */
- ret = msm_dp_ctrl_enable_mainlink_clocks(ctrl);
+ ret = msm_dp_ctrl_enable_mainlink_clocks(ctrl, panel);
if (ret) {
DRM_ERROR("Failed to start link clocks. ret=%d\n", ret);
return ret;
@@ -2538,15 +2560,15 @@ int msm_dp_ctrl_prepare_stream_on(struct msm_dp_ctrl *msm_dp_ctrl, bool force_li
}
if (force_link_train || !msm_dp_ctrl_channel_eq_ok(ctrl))
- msm_dp_ctrl_link_retrain(ctrl);
+ msm_dp_ctrl_link_retrain(ctrl, panel);
/* stop txing train pattern to end link training */
- msm_dp_ctrl_clear_training_pattern(ctrl, DP_PHY_DPRX);
+ msm_dp_ctrl_clear_training_pattern(ctrl, panel, DP_PHY_DPRX);
return ret;
}
-int msm_dp_ctrl_on_stream(struct msm_dp_ctrl *msm_dp_ctrl)
+int msm_dp_ctrl_on_stream(struct msm_dp_ctrl *msm_dp_ctrl, struct msm_dp_panel *panel)
{
int ret = 0;
bool mainlink_ready = false;
@@ -2559,10 +2581,10 @@ int msm_dp_ctrl_on_stream(struct msm_dp_ctrl *msm_dp_ctrl)
ctrl = container_of(msm_dp_ctrl, struct msm_dp_ctrl_private, msm_dp_ctrl);
- pixel_rate_orig = ctrl->panel->msm_dp_mode.drm_mode.clock;
+ pixel_rate_orig = panel->msm_dp_mode.drm_mode.clock;
pixel_rate = pixel_rate_orig;
- if (msm_dp_ctrl->wide_bus_en || ctrl->panel->msm_dp_mode.out_fmt_is_yuv_420)
+ if (msm_dp_ctrl->wide_bus_en || panel->msm_dp_mode.out_fmt_is_yuv_420)
pixel_rate >>= 1;
drm_dbg_dp(ctrl->drm_dev, "pixel_rate=%lu\n", pixel_rate);
@@ -2579,18 +2601,18 @@ int msm_dp_ctrl_on_stream(struct msm_dp_ctrl *msm_dp_ctrl)
msm_dp_ctrl_lane_mapping(ctrl);
msm_dp_setup_peripheral_flush(ctrl);
- msm_dp_ctrl_config_ctrl_link(ctrl);
+ msm_dp_ctrl_config_ctrl_link(ctrl, panel);
- msm_dp_ctrl_configure_source_params(ctrl);
+ msm_dp_ctrl_configure_source_params(ctrl, panel);
msm_dp_ctrl_config_msa(ctrl,
ctrl->link->link_params.rate,
pixel_rate_orig,
- ctrl->panel->msm_dp_mode.out_fmt_is_yuv_420);
+ panel->msm_dp_mode.out_fmt_is_yuv_420);
- msm_dp_panel_clear_dsc_dto(ctrl->panel);
+ msm_dp_panel_clear_dsc_dto(panel);
- msm_dp_ctrl_setup_tr_unit(ctrl);
+ msm_dp_ctrl_setup_tr_unit(ctrl, panel);
msm_dp_write_link(ctrl, REG_DP_STATE_CTRL, DP_STATE_CTRL_SEND_VIDEO);
@@ -2618,7 +2640,8 @@ void msm_dp_ctrl_reinit_phy(struct msm_dp_ctrl *msm_dp_ctrl)
phy_init(phy);
}
-void msm_dp_ctrl_off(struct msm_dp_ctrl *msm_dp_ctrl)
+void msm_dp_ctrl_off(struct msm_dp_ctrl *msm_dp_ctrl,
+ struct msm_dp_panel *panel)
{
struct msm_dp_ctrl_private *ctrl;
struct phy *phy;
@@ -2626,11 +2649,11 @@ void msm_dp_ctrl_off(struct msm_dp_ctrl *msm_dp_ctrl)
ctrl = container_of(msm_dp_ctrl, struct msm_dp_ctrl_private, msm_dp_ctrl);
phy = ctrl->phy;
- msm_dp_panel_disable_vsc_sdp(ctrl->panel);
+ msm_dp_panel_disable_vsc_sdp(panel);
msm_dp_ctrl_mainlink_disable(ctrl);
- msm_dp_ctrl_reset(&ctrl->msm_dp_ctrl);
+ msm_dp_ctrl_reset(&ctrl->msm_dp_ctrl, panel);
msm_dp_ctrl_off_pixel_clk(msm_dp_ctrl);
dev_pm_opp_set_rate(ctrl->dev, 0);
@@ -2639,7 +2662,8 @@ void msm_dp_ctrl_off(struct msm_dp_ctrl *msm_dp_ctrl)
phy_power_off(phy);
}
-irqreturn_t msm_dp_ctrl_isr(struct msm_dp_ctrl *msm_dp_ctrl)
+irqreturn_t msm_dp_ctrl_isr(struct msm_dp_ctrl *msm_dp_ctrl,
+ struct msm_dp_panel *panel)
{
struct msm_dp_ctrl_private *ctrl;
u32 isr;
@@ -2650,7 +2674,7 @@ irqreturn_t msm_dp_ctrl_isr(struct msm_dp_ctrl *msm_dp_ctrl)
ctrl = container_of(msm_dp_ctrl, struct msm_dp_ctrl_private, msm_dp_ctrl);
- if (ctrl->panel->psr_cap.version) {
+ if (panel->psr_cap.version) {
isr = msm_dp_ctrl_get_psr_interrupt(ctrl);
if (isr)
@@ -2739,7 +2763,7 @@ static int msm_dp_ctrl_clk_init(struct msm_dp_ctrl *msm_dp_ctrl)
}
struct msm_dp_ctrl *msm_dp_ctrl_get(struct device *dev, struct msm_dp_link *link,
- struct msm_dp_panel *panel, struct drm_dp_aux *aux,
+ struct drm_dp_aux *aux,
struct phy *phy,
void __iomem *ahb_base,
void __iomem *link_base)
@@ -2747,7 +2771,7 @@ struct msm_dp_ctrl *msm_dp_ctrl_get(struct device *dev, struct msm_dp_link *link
struct msm_dp_ctrl_private *ctrl;
int ret;
- if (!dev || !panel || !aux || !link) {
+ if (!dev || !aux || !link) {
DRM_ERROR("invalid input\n");
return ERR_PTR(-EINVAL);
}
@@ -2775,7 +2799,6 @@ struct msm_dp_ctrl *msm_dp_ctrl_get(struct device *dev, struct msm_dp_link *link
init_completion(&ctrl->video_comp);
/* in parameters */
- ctrl->panel = panel;
ctrl->aux = aux;
ctrl->link = link;
ctrl->dev = dev;
diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.h b/drivers/gpu/drm/msm/dp/dp_ctrl.h
index 5d615f50d13b..00b430392a52 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.h
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.h
@@ -16,28 +16,36 @@ struct msm_dp_ctrl {
struct phy;
-int msm_dp_ctrl_on_link(struct msm_dp_ctrl *msm_dp_ctrl);
-int msm_dp_ctrl_on_stream(struct msm_dp_ctrl *msm_dp_ctrl);
-int msm_dp_ctrl_prepare_stream_on(struct msm_dp_ctrl *msm_dp_ctrl, bool force_link_train);
-void msm_dp_ctrl_off(struct msm_dp_ctrl *msm_dp_ctrl);
+int msm_dp_ctrl_on_link(struct msm_dp_ctrl *msm_dp_ctrl,
+ struct msm_dp_panel *panel);
+int msm_dp_ctrl_on_stream(struct msm_dp_ctrl *msm_dp_ctrl, struct msm_dp_panel *panel);
+int msm_dp_ctrl_prepare_stream_on(struct msm_dp_ctrl *msm_dp_ctrl,
+ struct msm_dp_panel *panel,
+ bool force_link_train);
+void msm_dp_ctrl_off(struct msm_dp_ctrl *msm_dp_ctrl,
+ struct msm_dp_panel *panel);
void msm_dp_ctrl_push_idle(struct msm_dp_ctrl *msm_dp_ctrl);
-irqreturn_t msm_dp_ctrl_isr(struct msm_dp_ctrl *msm_dp_ctrl);
-void msm_dp_ctrl_handle_sink_request(struct msm_dp_ctrl *msm_dp_ctrl);
+irqreturn_t msm_dp_ctrl_isr(struct msm_dp_ctrl *msm_dp_ctrl,
+ struct msm_dp_panel *panel);
+void msm_dp_ctrl_handle_sink_request(struct msm_dp_ctrl *msm_dp_ctrl,
+ struct msm_dp_panel *panel);
struct msm_dp_ctrl *msm_dp_ctrl_get(struct device *dev,
struct msm_dp_link *link,
- struct msm_dp_panel *panel,
struct drm_dp_aux *aux,
struct phy *phy,
void __iomem *ahb_base,
void __iomem *link_base);
-void msm_dp_ctrl_reset(struct msm_dp_ctrl *msm_dp_ctrl);
+void msm_dp_ctrl_reset(struct msm_dp_ctrl *msm_dp_ctrl,
+ struct msm_dp_panel *panel);
void msm_dp_ctrl_phy_init(struct msm_dp_ctrl *msm_dp_ctrl);
void msm_dp_ctrl_phy_exit(struct msm_dp_ctrl *msm_dp_ctrl);
void msm_dp_ctrl_irq_phy_exit(struct msm_dp_ctrl *msm_dp_ctrl);
-void msm_dp_ctrl_set_psr(struct msm_dp_ctrl *msm_dp_ctrl, bool enable);
-void msm_dp_ctrl_config_psr(struct msm_dp_ctrl *msm_dp_ctrl);
+void msm_dp_ctrl_set_psr(struct msm_dp_ctrl *msm_dp_ctrl,
+ struct msm_dp_panel *panel, bool enable);
+void msm_dp_ctrl_config_psr(struct msm_dp_ctrl *msm_dp_ctrl,
+ struct msm_dp_panel *panel);
int msm_dp_ctrl_core_clk_enable(struct msm_dp_ctrl *msm_dp_ctrl);
void msm_dp_ctrl_core_clk_disable(struct msm_dp_ctrl *msm_dp_ctrl);
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index cd8a3b171d9d..1b9b7a4e5db0 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -359,7 +359,7 @@ static void msm_dp_display_host_init(struct msm_dp_display_private *dp)
dp->phy_initialized);
msm_dp_ctrl_core_clk_enable(dp->ctrl);
- msm_dp_ctrl_reset(dp->ctrl);
+ msm_dp_ctrl_reset(dp->ctrl, dp->panel);
msm_dp_ctrl_enable_irq(dp->ctrl);
msm_dp_aux_init(dp->aux);
dp->core_initialized = true;
@@ -371,7 +371,7 @@ static void msm_dp_display_host_deinit(struct msm_dp_display_private *dp)
dp->msm_dp_display.connector_type, dp->core_initialized,
dp->phy_initialized);
- msm_dp_ctrl_reset(dp->ctrl);
+ msm_dp_ctrl_reset(dp->ctrl, dp->panel);
msm_dp_ctrl_disable_irq(dp->ctrl);
msm_dp_aux_deinit(dp->aux);
msm_dp_ctrl_core_clk_disable(dp->ctrl);
@@ -392,7 +392,7 @@ static int msm_dp_display_handle_irq_hpd(struct msm_dp_display_private *dp)
drm_dbg_dp(dp->drm_dev, "%d\n", sink_request);
- msm_dp_ctrl_handle_sink_request(dp->ctrl);
+ msm_dp_ctrl_handle_sink_request(dp->ctrl, dp->panel);
if (sink_request & DP_TEST_LINK_VIDEO_PATTERN)
msm_dp_display_handle_video_request(dp);
@@ -570,8 +570,8 @@ static int msm_dp_init_sub_modules(struct msm_dp_display_private *dp)
goto error_link;
}
- dp->ctrl = msm_dp_ctrl_get(dev, dp->link, dp->panel, dp->aux,
- phy, dp->ahb_base, dp->link_base);
+ dp->ctrl = msm_dp_ctrl_get(dev, dp->link, dp->aux,
+ phy, dp->ahb_base, dp->link_base);
if (IS_ERR(dp->ctrl)) {
rc = PTR_ERR(dp->ctrl);
DRM_ERROR("failed to initialize ctrl, rc = %d\n", rc);
@@ -642,14 +642,14 @@ static int msm_dp_display_prepare_link(struct msm_dp_display_private *dp)
force_link_train = true;
}
- rc = msm_dp_ctrl_on_link(dp->ctrl);
+ rc = msm_dp_ctrl_on_link(dp->ctrl, dp->panel);
if (rc) {
DRM_ERROR("Failed link training (rc=%d)\n", rc);
// TODO: schedule drm_connector_set_link_status_property()
return rc;
}
- return msm_dp_ctrl_prepare_stream_on(dp->ctrl, force_link_train);
+ return msm_dp_ctrl_prepare_stream_on(dp->ctrl, dp->panel, force_link_train);
}
static int msm_dp_display_enable(struct msm_dp_display_private *dp)
@@ -663,7 +663,7 @@ static int msm_dp_display_enable(struct msm_dp_display_private *dp)
return 0;
}
- rc = msm_dp_ctrl_on_stream(dp->ctrl);
+ rc = msm_dp_ctrl_on_stream(dp->ctrl, dp->panel);
if (!rc)
msm_dp_display->power_on = true;
@@ -688,7 +688,7 @@ static int msm_dp_display_post_enable(struct msm_dp *msm_dp_display)
msm_dp_display_handle_plugged_change(msm_dp_display, true);
if (msm_dp_display->psr_supported)
- msm_dp_ctrl_config_psr(dp->ctrl);
+ msm_dp_ctrl_config_psr(dp->ctrl, dp->panel);
return 0;
}
@@ -727,7 +727,7 @@ static int msm_dp_display_disable(struct msm_dp_display_private *dp)
/* set dongle to D3 (power off) mode */
msm_dp_link_psm_config(dp->link, &dp->panel->link_info, true);
- msm_dp_ctrl_off(dp->ctrl);
+ msm_dp_ctrl_off(dp->ctrl, dp->panel);
/* re-init the PHY so that we can listen to Dongle disconnect */
msm_dp_ctrl_reinit_phy(dp->ctrl);
} else {
@@ -735,7 +735,7 @@ static int msm_dp_display_disable(struct msm_dp_display_private *dp)
* unplugged interrupt
* dongle unplugged out of DUT
*/
- msm_dp_ctrl_off(dp->ctrl);
+ msm_dp_ctrl_off(dp->ctrl, dp->panel);
msm_dp_display_host_phy_exit(dp);
}
@@ -871,7 +871,7 @@ void msm_dp_display_set_psr(struct msm_dp *msm_dp_display, bool enter)
}
dp = container_of(msm_dp_display, struct msm_dp_display_private, msm_dp_display);
- msm_dp_ctrl_set_psr(dp->ctrl, enter);
+ msm_dp_ctrl_set_psr(dp->ctrl, dp->panel, enter);
}
/**
@@ -981,7 +981,7 @@ static irqreturn_t msm_dp_display_irq_handler(int irq, void *dev_id)
}
/* DP controller isr */
- ret |= msm_dp_ctrl_isr(dp->ctrl);
+ ret |= msm_dp_ctrl_isr(dp->ctrl, dp->panel);
return ret;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v9 11/14] drm/msm/dp: split dp_ctrl_off() into stream and link parts
2026-07-28 10:21 [PATCH v9 00/14] drm/msm/dp: Prerequisite cleanup for upcoming MST support Yongxing Mou
` (9 preceding siblings ...)
2026-07-28 10:21 ` [PATCH v9 10/14] drm/msm/dp: allow dp_ctrl stream APIs to use any panel passed to it Yongxing Mou
@ 2026-07-28 10:21 ` Yongxing Mou
2026-07-28 10:21 ` [PATCH v9 12/14] drm/msm/dp: simplify link and clock disable sequence Yongxing Mou
` (2 subsequent siblings)
13 siblings, 0 replies; 22+ messages in thread
From: Yongxing Mou @ 2026-07-28 10:21 UTC (permalink / raw)
To: Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang,
Sean Paul, Marijn Suijten, David Airlie, Simona Vetter
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel, Yongxing Mou,
Abhinav Kumar, Dmitry Baryshkov
From: Abhinav Kumar <quic_abhinavk@quicinc.com>
Split dp_ctrl_off() into stream and link parts so that for MST
cases we can control the link and pixel parts separately.
Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
Signed-off-by: Yongxing Mou <yongxing.mou@oss.qualcomm.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
drivers/gpu/drm/msm/dp/dp_ctrl.c | 10 +++++-----
drivers/gpu/drm/msm/dp/dp_ctrl.h | 5 +++--
drivers/gpu/drm/msm/dp/dp_display.c | 7 ++++---
3 files changed, 12 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
index 9bed5a2fd2e7..5d896593771e 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
@@ -2215,7 +2215,7 @@ static int msm_dp_ctrl_on_pixel_clk(struct msm_dp_ctrl_private *ctrl, unsigned l
return ret;
}
-static void msm_dp_ctrl_off_pixel_clk(struct msm_dp_ctrl *msm_dp_ctrl)
+void msm_dp_ctrl_off_pixel_clk(struct msm_dp_ctrl *msm_dp_ctrl)
{
struct msm_dp_ctrl_private *ctrl;
@@ -2244,7 +2244,8 @@ static int msm_dp_ctrl_process_phy_test_request(struct msm_dp_ctrl_private *ctrl
* running. Add the global reset just before disabling the
* link clocks and core clocks.
*/
- msm_dp_ctrl_off(&ctrl->msm_dp_ctrl, panel);
+ msm_dp_ctrl_off_pixel_clk(&ctrl->msm_dp_ctrl);
+ msm_dp_ctrl_off_link(&ctrl->msm_dp_ctrl, panel);
ret = msm_dp_ctrl_on_link(&ctrl->msm_dp_ctrl, panel);
if (ret) {
@@ -2640,8 +2641,8 @@ void msm_dp_ctrl_reinit_phy(struct msm_dp_ctrl *msm_dp_ctrl)
phy_init(phy);
}
-void msm_dp_ctrl_off(struct msm_dp_ctrl *msm_dp_ctrl,
- struct msm_dp_panel *panel)
+void msm_dp_ctrl_off_link(struct msm_dp_ctrl *msm_dp_ctrl,
+ struct msm_dp_panel *panel)
{
struct msm_dp_ctrl_private *ctrl;
struct phy *phy;
@@ -2655,7 +2656,6 @@ void msm_dp_ctrl_off(struct msm_dp_ctrl *msm_dp_ctrl,
msm_dp_ctrl_reset(&ctrl->msm_dp_ctrl, panel);
- msm_dp_ctrl_off_pixel_clk(msm_dp_ctrl);
dev_pm_opp_set_rate(ctrl->dev, 0);
msm_dp_ctrl_link_clk_disable(&ctrl->msm_dp_ctrl);
diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.h b/drivers/gpu/drm/msm/dp/dp_ctrl.h
index 00b430392a52..5902cf7e746a 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.h
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.h
@@ -22,8 +22,9 @@ int msm_dp_ctrl_on_stream(struct msm_dp_ctrl *msm_dp_ctrl, struct msm_dp_panel *
int msm_dp_ctrl_prepare_stream_on(struct msm_dp_ctrl *msm_dp_ctrl,
struct msm_dp_panel *panel,
bool force_link_train);
-void msm_dp_ctrl_off(struct msm_dp_ctrl *msm_dp_ctrl,
- struct msm_dp_panel *panel);
+void msm_dp_ctrl_off_link(struct msm_dp_ctrl *msm_dp_ctrl,
+ struct msm_dp_panel *panel);
+void msm_dp_ctrl_off_pixel_clk(struct msm_dp_ctrl *msm_dp_ctrl);
void msm_dp_ctrl_push_idle(struct msm_dp_ctrl *msm_dp_ctrl);
irqreturn_t msm_dp_ctrl_isr(struct msm_dp_ctrl *msm_dp_ctrl,
struct msm_dp_panel *panel);
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index 1b9b7a4e5db0..6e392a99438f 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -727,15 +727,16 @@ static int msm_dp_display_disable(struct msm_dp_display_private *dp)
/* set dongle to D3 (power off) mode */
msm_dp_link_psm_config(dp->link, &dp->panel->link_info, true);
- msm_dp_ctrl_off(dp->ctrl, dp->panel);
- /* re-init the PHY so that we can listen to Dongle disconnect */
+ msm_dp_ctrl_off_pixel_clk(dp->ctrl);
+ msm_dp_ctrl_off_link(dp->ctrl, dp->panel);
msm_dp_ctrl_reinit_phy(dp->ctrl);
} else {
/*
* unplugged interrupt
* dongle unplugged out of DUT
*/
- msm_dp_ctrl_off(dp->ctrl, dp->panel);
+ msm_dp_ctrl_off_pixel_clk(dp->ctrl);
+ msm_dp_ctrl_off_link(dp->ctrl, dp->panel);
msm_dp_display_host_phy_exit(dp);
}
--
2.43.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v9 12/14] drm/msm/dp: simplify link and clock disable sequence
2026-07-28 10:21 [PATCH v9 00/14] drm/msm/dp: Prerequisite cleanup for upcoming MST support Yongxing Mou
` (10 preceding siblings ...)
2026-07-28 10:21 ` [PATCH v9 11/14] drm/msm/dp: split dp_ctrl_off() into stream and link parts Yongxing Mou
@ 2026-07-28 10:21 ` Yongxing Mou
2026-07-28 10:41 ` sashiko-bot
2026-07-28 10:21 ` [PATCH v9 13/14] drm/msm/dp: make bridge helpers use dp_display to allow re-use Yongxing Mou
2026-07-28 10:21 ` [PATCH v9 14/14] drm/msm/dp: pass panel to display enable/disable helpers Yongxing Mou
13 siblings, 1 reply; 22+ messages in thread
From: Yongxing Mou @ 2026-07-28 10:21 UTC (permalink / raw)
To: Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang,
Sean Paul, Marijn Suijten, David Airlie, Simona Vetter
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel, Yongxing Mou,
Dmitry Baryshkov
Move the common disable steps out of the sink_count check to make the
flow easier to follow.
No functional change intended.
Signed-off-by: Yongxing Mou <yongxing.mou@oss.qualcomm.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
drivers/gpu/drm/msm/dp/dp_display.c | 26 +++++++++-----------------
1 file changed, 9 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index 6e392a99438f..9b9e96b683c7 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -718,27 +718,19 @@ static int msm_dp_display_disable(struct msm_dp_display_private *dp)
msm_dp_panel_disable_vsc_sdp(dp->panel);
- /* dongle is still connected but sinks are disconnected */
- if (dp->link->sink_count == 0) {
- /*
- * irq_hpd with sink_count = 0
- * hdmi unplugged out of dongle
- */
+ msm_dp_ctrl_off_pixel_clk(dp->ctrl);
- /* set dongle to D3 (power off) mode */
+ /* dongle is still connected but sinks are disconnected */
+ if (dp->link->sink_count == 0)
msm_dp_link_psm_config(dp->link, &dp->panel->link_info, true);
- msm_dp_ctrl_off_pixel_clk(dp->ctrl);
- msm_dp_ctrl_off_link(dp->ctrl, dp->panel);
+
+ msm_dp_ctrl_off_link(dp->ctrl, dp->panel);
+
+ if (dp->link->sink_count == 0)
+ /* re-init the PHY so that we can listen to Dongle disconnect */
msm_dp_ctrl_reinit_phy(dp->ctrl);
- } else {
- /*
- * unplugged interrupt
- * dongle unplugged out of DUT
- */
- msm_dp_ctrl_off_pixel_clk(dp->ctrl);
- msm_dp_ctrl_off_link(dp->ctrl, dp->panel);
+ else
msm_dp_display_host_phy_exit(dp);
- }
msm_dp_display->power_on = false;
--
2.43.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v9 12/14] drm/msm/dp: simplify link and clock disable sequence
2026-07-28 10:21 ` [PATCH v9 12/14] drm/msm/dp: simplify link and clock disable sequence Yongxing Mou
@ 2026-07-28 10:41 ` sashiko-bot
0 siblings, 0 replies; 22+ messages in thread
From: sashiko-bot @ 2026-07-28 10:41 UTC (permalink / raw)
To: Yongxing Mou; +Cc: dri-devel
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Time-Of-Check to Time-Of-Use (TOCTOU) race condition on `dp->link->sink_count` leading to mismatched hardware state.
--
--- Patch [12]: [PATCH v9 12/14] drm/msm/dp: simplify link and clock disable sequence ---
commit 68b08e6a0d7bd864688b398d6052cc9412efcc7c
Author: Yongxing Mou <yongxing.mou@oss.qualcomm.com>
drm/msm/dp: simplify link and clock disable sequence
This commit moves common disable steps out of the sink_count check to make
the flow easier to follow.
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> index 6e392a99438f8..9b9e96b683c71 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
[ ... ]
> + /* dongle is still connected but sinks are disconnected */
> + if (dp->link->sink_count == 0)
> msm_dp_link_psm_config(dp->link, &dp->panel->link_info, true);
> - msm_dp_ctrl_off_pixel_clk(dp->ctrl);
> - msm_dp_ctrl_off_link(dp->ctrl, dp->panel);
> +
> + msm_dp_ctrl_off_link(dp->ctrl, dp->panel);
> +
> + if (dp->link->sink_count == 0)
[Severity: High]
Can this introduce a race condition if sink_count changes between these two
checks?
Looking at msm_dp_display_disable(), the sink_count is now evaluated twice.
If a user plugs or unplugs a monitor exactly when this disable sequence
is occurring, the HPD event thread might locklessly update sink_count:
CPU1 (msm_dp_display_disable)
if (dp->link->sink_count == 0) // evaluates to true
msm_dp_link_psm_config(...);
CPU2 (HPD IRQ thread)
msm_dp_link_process_request()
dp->link->sink_count = 1; // updated locklessly
CPU1
msm_dp_ctrl_off_link(...);
if (dp->link->sink_count == 0) // now evaluates to false
else
msm_dp_display_host_phy_exit(dp);
If this state diverges, the driver might completely disable the PHY while
a dongle is still connected and in PSM mode, preventing the hardware from
detecting subsequent dongle disconnects.
Would it be safer to cache the initial sink_count in a local variable to
ensure consistent state transitions throughout the function?
> + /* re-init the PHY so that we can listen to Dongle disconnect */
> msm_dp_ctrl_reinit_phy(dp->ctrl);
> - } else {
> - /*
> - * unplugged interrupt
> - * dongle unplugged out of DUT
> - */
> - msm_dp_ctrl_off_pixel_clk(dp->ctrl);
> - msm_dp_ctrl_off_link(dp->ctrl, dp->panel);
> + else
> msm_dp_display_host_phy_exit(dp);
> - }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260728-dp_mstclean-v9-0-f7779fce10f4@oss.qualcomm.com?part=12
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v9 13/14] drm/msm/dp: make bridge helpers use dp_display to allow re-use
2026-07-28 10:21 [PATCH v9 00/14] drm/msm/dp: Prerequisite cleanup for upcoming MST support Yongxing Mou
` (11 preceding siblings ...)
2026-07-28 10:21 ` [PATCH v9 12/14] drm/msm/dp: simplify link and clock disable sequence Yongxing Mou
@ 2026-07-28 10:21 ` Yongxing Mou
2026-07-28 10:38 ` sashiko-bot
2026-07-28 10:21 ` [PATCH v9 14/14] drm/msm/dp: pass panel to display enable/disable helpers Yongxing Mou
13 siblings, 1 reply; 22+ messages in thread
From: Yongxing Mou @ 2026-07-28 10:21 UTC (permalink / raw)
To: Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang,
Sean Paul, Marijn Suijten, David Airlie, Simona Vetter
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel, Yongxing Mou,
Abhinav Kumar, Dmitry Baryshkov
From: Abhinav Kumar <quic_abhinavk@quicinc.com>
dp_bridge helpers take drm_bridge as an input and extract the
dp_display object to be used in the dp_display module. Rather than
doing it in a roundabout way, directly pass the dp_display object
to these helpers so that the MST bridge can also re-use the same
helpers.
Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
Signed-off-by: Yongxing Mou <yongxing.mou@oss.qualcomm.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
drivers/gpu/drm/msm/dp/dp_display.c | 41 +++++++++++--------------------
drivers/gpu/drm/msm/dp/dp_display.h | 9 +++++++
drivers/gpu/drm/msm/dp/dp_drm.c | 48 ++++++++++++++++++++++++++++++++++++-
drivers/gpu/drm/msm/dp/dp_drm.h | 11 ---------
4 files changed, 70 insertions(+), 39 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index 9b9e96b683c7..88ca263010c6 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -740,24 +740,21 @@ static int msm_dp_display_disable(struct msm_dp_display_private *dp)
/**
* msm_dp_bridge_mode_valid - callback to determine if specified mode is valid
- * @bridge: Pointer to drm bridge structure
+ * @dp: Pointer to dp display structure
* @info: display info
* @mode: Pointer to drm mode structure
* Returns: Validity status for specified mode
*/
-enum drm_mode_status msm_dp_bridge_mode_valid(struct drm_bridge *bridge,
- const struct drm_display_info *info,
- const struct drm_display_mode *mode)
+enum drm_mode_status msm_dp_display_mode_valid(struct msm_dp *dp,
+ const struct drm_display_info *info,
+ const struct drm_display_mode *mode)
{
const u32 num_components = 3, default_bpp = 24;
struct msm_dp_display_private *msm_dp_display;
struct msm_dp_link_info *link_info;
u32 mode_rate_khz = 0, supported_rate_khz = 0, mode_bpp = 0;
- struct msm_dp *dp;
int mode_pclk_khz = mode->clock;
- dp = to_dp_bridge(bridge)->msm_dp_display;
-
if (!dp || !mode_pclk_khz || !dp->connector) {
DRM_ERROR("invalid params\n");
return -EINVAL;
@@ -1408,18 +1405,16 @@ int msm_dp_modeset_init(struct msm_dp *msm_dp_display, struct drm_device *dev,
return 0;
}
-void msm_dp_bridge_atomic_pre_enable(struct drm_bridge *drm_bridge,
- struct drm_atomic_commit *state)
+void msm_dp_display_atomic_pre_enable(struct msm_dp *msm_dp_display,
+ struct drm_atomic_commit *state)
{
- struct msm_dp_bridge *msm_dp_bridge = to_dp_bridge(drm_bridge);
- struct msm_dp *dp = msm_dp_bridge->msm_dp_display;
- struct msm_dp_display_private *msm_dp_display;
+ struct msm_dp_display_private *dp;
struct drm_crtc *crtc;
struct drm_crtc_state *crtc_state;
- msm_dp_display = container_of(dp, struct msm_dp_display_private, msm_dp_display);
+ dp = container_of(msm_dp_display, struct msm_dp_display_private, msm_dp_display);
- crtc = drm_atomic_get_new_crtc_for_encoder(state, drm_bridge->encoder);
+ crtc = drm_atomic_get_new_crtc_for_encoder(state, msm_dp_display->bridge->encoder);
if (!crtc)
return;
crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
@@ -1429,14 +1424,12 @@ void msm_dp_bridge_atomic_pre_enable(struct drm_bridge *drm_bridge,
* state and runs before the bridge's .atomic_enable(), so the mode must
* be programmed here, in .atomic_pre_enable().
*/
- msm_dp_display_set_mode(dp, &crtc_state->adjusted_mode, msm_dp_display->panel);
+ msm_dp_display_set_mode(msm_dp_display, &crtc_state->adjusted_mode, dp->panel);
}
-void msm_dp_bridge_atomic_enable(struct drm_bridge *drm_bridge,
- struct drm_atomic_commit *state)
+void msm_dp_display_atomic_enable(struct msm_dp *msm_dp_display,
+ struct drm_atomic_commit *state)
{
- struct msm_dp_bridge *msm_dp_bridge = to_dp_bridge(drm_bridge);
- struct msm_dp *msm_dp_display = msm_dp_bridge->msm_dp_display;
int rc = 0;
struct msm_dp_display_private *dp;
@@ -1461,11 +1454,8 @@ void msm_dp_bridge_atomic_enable(struct drm_bridge *drm_bridge,
drm_dbg_dp(msm_dp_display->drm_dev, "type=%d Done\n", msm_dp_display->connector_type);
}
-void msm_dp_bridge_atomic_disable(struct drm_bridge *drm_bridge,
- struct drm_atomic_commit *state)
+void msm_dp_display_atomic_disable(struct msm_dp *dp)
{
- struct msm_dp_bridge *msm_dp_bridge = to_dp_bridge(drm_bridge);
- struct msm_dp *dp = msm_dp_bridge->msm_dp_display;
struct msm_dp_display_private *msm_dp_display;
msm_dp_display = container_of(dp, struct msm_dp_display_private, msm_dp_display);
@@ -1482,11 +1472,8 @@ static void msm_dp_display_unprepare(struct msm_dp_display_private *dp)
drm_dbg_dp(dp->drm_dev, "type=%d Done\n", msm_dp_display->connector_type);
}
-void msm_dp_bridge_atomic_post_disable(struct drm_bridge *drm_bridge,
- struct drm_atomic_commit *state)
+void msm_dp_display_atomic_post_disable(struct msm_dp *dp)
{
- struct msm_dp_bridge *msm_dp_bridge = to_dp_bridge(drm_bridge);
- struct msm_dp *dp = msm_dp_bridge->msm_dp_display;
struct msm_dp_display_private *msm_dp_display;
msm_dp_display = container_of(dp, struct msm_dp_display_private, msm_dp_display);
diff --git a/drivers/gpu/drm/msm/dp/dp_display.h b/drivers/gpu/drm/msm/dp/dp_display.h
index 0b65e16c790d..83117e7619f3 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.h
+++ b/drivers/gpu/drm/msm/dp/dp_display.h
@@ -33,5 +33,14 @@ void msm_dp_display_signal_audio_start(struct msm_dp *msm_dp_display);
void msm_dp_display_signal_audio_complete(struct msm_dp *msm_dp_display);
void msm_dp_display_set_psr(struct msm_dp *dp, bool enter);
void msm_dp_display_debugfs_init(struct msm_dp *msm_dp_display, struct dentry *dentry, bool is_edp);
+void msm_dp_display_atomic_post_disable(struct msm_dp *dp_display);
+void msm_dp_display_atomic_disable(struct msm_dp *dp_display);
+void msm_dp_display_atomic_pre_enable(struct msm_dp *dp_display,
+ struct drm_atomic_commit *state);
+void msm_dp_display_atomic_enable(struct msm_dp *dp_display,
+ struct drm_atomic_commit *state);
+enum drm_mode_status msm_dp_display_mode_valid(struct msm_dp *dp,
+ const struct drm_display_info *info,
+ const struct drm_display_mode *mode);
#endif /* _DP_DISPLAY_H_ */
diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c b/drivers/gpu/drm/msm/dp/dp_drm.c
index 794a0df60414..c1e4147bfef1 100644
--- a/drivers/gpu/drm/msm/dp/dp_drm.c
+++ b/drivers/gpu/drm/msm/dp/dp_drm.c
@@ -49,6 +49,52 @@ static void msm_dp_bridge_debugfs_init(struct drm_bridge *bridge, struct dentry
msm_dp_display_debugfs_init(dp, root, false);
}
+static void msm_dp_bridge_atomic_pre_enable(struct drm_bridge *drm_bridge,
+ struct drm_atomic_commit *state)
+{
+ struct msm_dp_bridge *dp_bridge = to_dp_bridge(drm_bridge);
+ struct msm_dp *dp = dp_bridge->msm_dp_display;
+
+ msm_dp_display_atomic_pre_enable(dp, state);
+}
+
+static void msm_dp_bridge_atomic_enable(struct drm_bridge *drm_bridge,
+ struct drm_atomic_commit *state)
+{
+ struct msm_dp_bridge *dp_bridge = to_dp_bridge(drm_bridge);
+ struct msm_dp *dp = dp_bridge->msm_dp_display;
+
+ msm_dp_display_atomic_enable(dp, state);
+}
+
+static void msm_dp_bridge_atomic_disable(struct drm_bridge *drm_bridge,
+ struct drm_atomic_commit *state)
+{
+ struct msm_dp_bridge *dp_bridge = to_dp_bridge(drm_bridge);
+ struct msm_dp *dp = dp_bridge->msm_dp_display;
+
+ msm_dp_display_atomic_disable(dp);
+}
+
+static void msm_dp_bridge_atomic_post_disable(struct drm_bridge *drm_bridge,
+ struct drm_atomic_commit *state)
+{
+ struct msm_dp_bridge *dp_bridge = to_dp_bridge(drm_bridge);
+ struct msm_dp *dp = dp_bridge->msm_dp_display;
+
+ msm_dp_display_atomic_post_disable(dp);
+}
+
+static enum drm_mode_status msm_dp_bridge_mode_valid(struct drm_bridge *drm_bridge,
+ const struct drm_display_info *info,
+ const struct drm_display_mode *mode)
+{
+ struct msm_dp_bridge *dp_bridge = to_dp_bridge(drm_bridge);
+ struct msm_dp *dp = dp_bridge->msm_dp_display;
+
+ return msm_dp_display_mode_valid(dp, info, mode);
+}
+
static const struct drm_bridge_funcs msm_dp_bridge_ops = {
.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
@@ -116,7 +162,7 @@ static void msm_edp_bridge_atomic_enable(struct drm_bridge *drm_bridge,
return;
}
- msm_dp_bridge_atomic_enable(drm_bridge, state);
+ msm_dp_display_atomic_enable(dp, state);
}
static void msm_edp_bridge_atomic_disable(struct drm_bridge *drm_bridge,
diff --git a/drivers/gpu/drm/msm/dp/dp_drm.h b/drivers/gpu/drm/msm/dp/dp_drm.h
index 4f733d8118b1..da412c788503 100644
--- a/drivers/gpu/drm/msm/dp/dp_drm.h
+++ b/drivers/gpu/drm/msm/dp/dp_drm.h
@@ -27,17 +27,6 @@ int msm_dp_bridge_init(struct msm_dp *msm_dp_display, struct drm_device *dev,
enum drm_connector_status msm_dp_bridge_detect(struct drm_bridge *bridge,
struct drm_connector *connector);
-void msm_dp_bridge_atomic_pre_enable(struct drm_bridge *drm_bridge,
- struct drm_atomic_commit *state);
-void msm_dp_bridge_atomic_enable(struct drm_bridge *drm_bridge,
- struct drm_atomic_commit *state);
-void msm_dp_bridge_atomic_disable(struct drm_bridge *drm_bridge,
- struct drm_atomic_commit *state);
-void msm_dp_bridge_atomic_post_disable(struct drm_bridge *drm_bridge,
- struct drm_atomic_commit *state);
-enum drm_mode_status msm_dp_bridge_mode_valid(struct drm_bridge *bridge,
- const struct drm_display_info *info,
- const struct drm_display_mode *mode);
void msm_dp_bridge_hpd_enable(struct drm_bridge *bridge);
void msm_dp_bridge_hpd_disable(struct drm_bridge *bridge);
void msm_dp_bridge_hpd_notify(struct drm_bridge *bridge,
--
2.43.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v9 13/14] drm/msm/dp: make bridge helpers use dp_display to allow re-use
2026-07-28 10:21 ` [PATCH v9 13/14] drm/msm/dp: make bridge helpers use dp_display to allow re-use Yongxing Mou
@ 2026-07-28 10:38 ` sashiko-bot
0 siblings, 0 replies; 22+ messages in thread
From: sashiko-bot @ 2026-07-28 10:38 UTC (permalink / raw)
To: Yongxing Mou; +Cc: dri-devel
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Hardcoded encoder lookup in `msm_dp_display_atomic_pre_enable` breaks the intended MST reuse by dropping the virtual bridge context.
--
--- Patch [13]: [PATCH v9 13/14] drm/msm/dp: make bridge helpers use dp_display to allow re-use ---
commit 9cd0096573edf67dd79ce85b7bf559799b84cbe4
Author: Abhinav Kumar <quic_abhinavk@quicinc.com>
drm/msm/dp: make bridge helpers use dp_display to allow re-use
This patch modifies DP bridge helpers to directly take a msm_dp display
object rather than extracting it from drm_bridge. The stated goal is to
allow the MST bridge to reuse these same helpers.
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> index 9b9e96b683c71..88ca263010c60 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
[ ... ]
> @@ -1408,18 +1405,16 @@ int msm_dp_modeset_init(struct msm_dp *msm_dp_display, struct drm_device *dev,
> return 0;
> }
>
> -void msm_dp_bridge_atomic_pre_enable(struct drm_bridge *drm_bridge,
> - struct drm_atomic_commit *state)
> +void msm_dp_display_atomic_pre_enable(struct msm_dp *msm_dp_display,
> + struct drm_atomic_commit *state)
> {
> - struct msm_dp_bridge *msm_dp_bridge = to_dp_bridge(drm_bridge);
> - struct msm_dp *dp = msm_dp_bridge->msm_dp_display;
> - struct msm_dp_display_private *msm_dp_display;
> + struct msm_dp_display_private *dp;
> struct drm_crtc *crtc;
> struct drm_crtc_state *crtc_state;
>
> - msm_dp_display = container_of(dp, struct msm_dp_display_private, msm_dp_display);
> + dp = container_of(msm_dp_display, struct msm_dp_display_private, msm_dp_display);
>
> - crtc = drm_atomic_get_new_crtc_for_encoder(state, drm_bridge->encoder);
> + crtc = drm_atomic_get_new_crtc_for_encoder(state, msm_dp_display->bridge->encoder);
[Severity: Medium]
Will this hardcoded encoder lookup prevent MST bridges from reusing this
helper?
During an MST commit, the atomic state contains the virtual MST encoder. By
changing this from the caller's drm_bridge->encoder to the root DP encoder
(msm_dp_display->bridge->encoder), drm_atomic_get_new_crtc_for_encoder()
will return NULL for MST streams. This causes the function to return early
without programming the mode.
Should the helper still take the encoder or the calling bridge as an
argument to support MST reuse?
> if (!crtc)
> return;
> crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260728-dp_mstclean-v9-0-f7779fce10f4@oss.qualcomm.com?part=13
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v9 14/14] drm/msm/dp: pass panel to display enable/disable helpers
2026-07-28 10:21 [PATCH v9 00/14] drm/msm/dp: Prerequisite cleanup for upcoming MST support Yongxing Mou
` (12 preceding siblings ...)
2026-07-28 10:21 ` [PATCH v9 13/14] drm/msm/dp: make bridge helpers use dp_display to allow re-use Yongxing Mou
@ 2026-07-28 10:21 ` Yongxing Mou
13 siblings, 0 replies; 22+ messages in thread
From: Yongxing Mou @ 2026-07-28 10:21 UTC (permalink / raw)
To: Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang,
Sean Paul, Marijn Suijten, David Airlie, Simona Vetter
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel, Yongxing Mou,
Dmitry Baryshkov
Pass struct msm_dp_panel to the display enable/disable helpers to make
them easier to reuse for MST stream handling.
Signed-off-by: Yongxing Mou <yongxing.mou@oss.qualcomm.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
drivers/gpu/drm/msm/dp/dp_display.c | 22 ++++++++++++----------
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index 88ca263010c6..8a2366e28eec 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -652,7 +652,8 @@ static int msm_dp_display_prepare_link(struct msm_dp_display_private *dp)
return msm_dp_ctrl_prepare_stream_on(dp->ctrl, dp->panel, force_link_train);
}
-static int msm_dp_display_enable(struct msm_dp_display_private *dp)
+static int msm_dp_display_enable(struct msm_dp_display_private *dp,
+ struct msm_dp_panel *msm_dp_panel)
{
int rc = 0;
struct msm_dp *msm_dp_display = &dp->msm_dp_display;
@@ -663,7 +664,7 @@ static int msm_dp_display_enable(struct msm_dp_display_private *dp)
return 0;
}
- rc = msm_dp_ctrl_on_stream(dp->ctrl, dp->panel);
+ rc = msm_dp_ctrl_on_stream(dp->ctrl, msm_dp_panel);
if (!rc)
msm_dp_display->power_on = true;
@@ -709,22 +710,23 @@ static void msm_dp_display_audio_notify_disable(struct msm_dp_display_private *d
msm_dp_display->audio_enabled = false;
}
-static int msm_dp_display_disable(struct msm_dp_display_private *dp)
+static int msm_dp_display_disable(struct msm_dp_display_private *dp,
+ struct msm_dp_panel *msm_dp_panel)
{
struct msm_dp *msm_dp_display = &dp->msm_dp_display;
if (!msm_dp_display->power_on)
return 0;
- msm_dp_panel_disable_vsc_sdp(dp->panel);
+ msm_dp_panel_disable_vsc_sdp(msm_dp_panel);
msm_dp_ctrl_off_pixel_clk(dp->ctrl);
/* dongle is still connected but sinks are disconnected */
if (dp->link->sink_count == 0)
- msm_dp_link_psm_config(dp->link, &dp->panel->link_info, true);
+ msm_dp_link_psm_config(dp->link, &msm_dp_panel->link_info, true);
- msm_dp_ctrl_off_link(dp->ctrl, dp->panel);
+ msm_dp_ctrl_off_link(dp->ctrl, msm_dp_panel);
if (dp->link->sink_count == 0)
/* re-init the PHY so that we can listen to Dongle disconnect */
@@ -739,7 +741,7 @@ static int msm_dp_display_disable(struct msm_dp_display_private *dp)
}
/**
- * msm_dp_bridge_mode_valid - callback to determine if specified mode is valid
+ * msm_dp_display_mode_valid - callback to determine if specified mode is valid
* @dp: Pointer to dp display structure
* @info: display info
* @mode: Pointer to drm mode structure
@@ -1441,14 +1443,14 @@ void msm_dp_display_atomic_enable(struct msm_dp *msm_dp_display,
return;
}
- rc = msm_dp_display_enable(dp);
+ rc = msm_dp_display_enable(dp, dp->panel);
if (rc)
DRM_ERROR("DP display enable failed, rc=%d\n", rc);
rc = msm_dp_display_post_enable(msm_dp_display);
if (rc) {
DRM_ERROR("DP display post enable failed, rc=%d\n", rc);
- msm_dp_display_disable(dp);
+ msm_dp_display_disable(dp, dp->panel);
}
drm_dbg_dp(msm_dp_display->drm_dev, "type=%d Done\n", msm_dp_display->connector_type);
@@ -1483,7 +1485,7 @@ void msm_dp_display_atomic_post_disable(struct msm_dp *dp)
msm_dp_display_audio_notify_disable(msm_dp_display);
- msm_dp_display_disable(msm_dp_display);
+ msm_dp_display_disable(msm_dp_display, msm_dp_display->panel);
msm_dp_display_unprepare(msm_dp_display);
}
--
2.43.0
^ permalink raw reply related [flat|nested] 22+ messages in thread