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 <[email protected]>
Signed-off-by: Yongxing Mou <[email protected]>
Reviewed-by: Dmitry Baryshkov <[email protected]>
Assisted-by: Claude:claude-opus-4-8
[DB: moved to atomic_pre_enable]
Signed-off-by: Dmitry Baryshkov <[email protected]>
---
 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

Reply via email to