Quoting Kuogee Hsieh (2021-12-09 13:35:07)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.cIs this check only here because we don't know when this function is
index 0766752..cfbc5e4 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -83,6 +83,7 @@ struct dp_display_private {
/* state variables */
bool core_initialized;
+ bool phy_initialized;
bool hpd_irq_on;
bool audio_supported;
@@ -371,21 +372,46 @@ static int dp_display_process_hpd_high(struct dp_display_private *dp)
return rc;
}
-static void dp_display_host_init(struct dp_display_private *dp, int reset)
+static void dp_display_host_phy_init(struct dp_display_private *dp)
{
- bool flip = false;
+ DRM_DEBUG_DP("core_init=%d phy_init=%d\n",
+ dp->core_initialized, dp->phy_initialized);
+ if (!dp->phy_initialized) {
getting called? I see in the DP case we get here from
dp_display_usbpd_configure_cb() but in the eDP case we get here from
dp_display_host_init() and presumably again from
dp_display_usbpd_configure_cb() called by dp_hpd_plug_handle().
If at all possible, I'd prefer to not have another tracking variable and
call dp_display_host_phy_init() from the same place regardless of DP or
eDP. Doing that would make it symmetric, per the commit text.
+ dp_ctrl_phy_init(dp->ctrl);Can we get some more details here? Why is it better to initialize the
+ dp->phy_initialized = true;
+ }
+}
+
+static void dp_display_host_phy_exit(struct dp_display_private *dp)
+{
+ DRM_DEBUG_DP("core_init=%d phy_init=%d\n",
+ dp->core_initialized, dp->phy_initialized);
+
+ if (dp->phy_initialized) {
+ dp_ctrl_phy_exit(dp->ctrl);
+ dp->phy_initialized = false;
+ }
+}
+
+static void dp_display_host_init(struct dp_display_private *dp)
+{
DRM_DEBUG_DP("core_initialized=%d\n", dp->core_initialized);
if (dp->core_initialized) {
DRM_DEBUG_DP("DP core already initialized\n");
return;
}
- if (dp->usbpd->orientation == ORIENTATION_CC2)
- flip = true;
+ dp_power_init(dp->power, false);
+ dp_ctrl_reset_irq_ctrl(dp->ctrl, true);
+
+ /*
+ * eDP is the embedded primary display and has its own phy
+ * initialize phy immediately
phy here vs. when HPD goes high on the panel? The comment says what the
code is doing but it isn't telling us why that's OK.
+ */
+ if (dp->dp_display.connector_type == DRM_MODE_CONNECTOR_eDP)
+ dp_display_host_phy_init(dp);
- dp_power_init(dp->power, flip);
- dp_ctrl_host_init(dp->ctrl, flip, reset);
dp_aux_init(dp->aux);
dp->core_initialized = true;
}