drm/i915/icl: Fix HPD handling for TypeC legacy ports

Atm HPD disconnect events on TypeC ports will break things, since we'll
switch the TypeC mode (between legacy and disconnected modes as well as
among USB DP alternate, Thunderbolt alternate and disconnected modes) on
the fly from the HPD disconnect interrupt work while the port may be
still active.

Even if the port happens to be not active during the disconnect we'd
still have a problem during a subsequent modeset or AUX transfer that
could happen regardless of the port's connected state. For instance the
system resume display mode restore code and userspace could perform a
modeset on the port or userspace could start an AUX transfer even if the
port is in disconnected state.

To fix this keep TypeC legacy ports in legacy mode whenever we're not
suspended. This mode is a static configuration as opposed to the
Thunderbolt and USB DP alternate modes between which we can switch
dynamically.

We determine if a TypeC port is legacy (wired to a legacy HDMI or a
legacy DP connector) via the VBT DDI port specific USB-TypeC and
Thunderbolt flags. If both these flags are cleared then the port is
configured for legacy mode.

On such legacy ports we'll run the TypeC PHY connect sequence explicitly
during driver loading and system resume (vs. running the sequence during
HPD processing). The connect will succeed even if the display is not
connected to begin with (or disappears during the suspended state) since
for legacy ports the PORT_TX_DFLEXDPPMS / DP_PHY_MODE_STATUS_COMPLETED
flag is always set (as opposed to the USB DP alternate mode where it
gets set only when a display is connected).

Correspondingly run the TypeC PHY disconnect sequence during system
suspend and driver unloading. For the unloading case I had to split
up intel_dp_encoder_destroy() to be able to have the 1. flush any
pending encoder work, 2. disconnect TC PHY, 3. call DRM core cleanup and
kfree on the encoder object.

For now run the PHY disconnect during suspend only for TypeC legacy
ports. We will need to disconnect even in USB DP alternate mode in the
future, but atm we don't have a way to reconnect the port in this mode
during resume if the display disappears while being suspended. So for
now punt on this case.

Note that we do not disconnect the port during runtime suspend; in
legacy mode there are no shared HW resources (PHY lanes) with other HW
blocks (USB), so no need to release / reacquire these resources as with
USB DP alternate mode. The only reason to disconnect legacy ports during
system suspend is that the PORT_TX_DFLEXDPPMS /
DP_PHY_MODE_STATUS_COMPLETED flag must be rechecked and the port must be
connected again during system resume. We'll also have to turn the check
for this flag into a poll, after figuring out what's the proper timeout
value for it.

v2:
- Remove the redundant special casing of legacy mode when doing a
  disconnect in icl_tc_port_connected(). It's guaranteed already that we
  won't disconnect legacy ports in that function.
- Add a note about the new intel_ddi_encoder_destroy() hook.
- Reword the commit message after switching to the VBT based detection.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108070
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108924
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: José Roberto de Souza <jose.souza@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20181214182703.18865-4-imre.deak@intel.com

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index dfd35822cdabfc1ba84899ffd587fccbbcbbc39a..e3cc19e19199af035324fef1396c27e37741a953 100644 (file)
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -3834,9 +3834,50 @@ static bool intel_ddi_compute_config(struct intel_encoder *encoder,
 
 }
 
+static void intel_ddi_encoder_suspend(struct intel_encoder *encoder)
+{
+       struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
+       struct drm_i915_private *i915 = to_i915(encoder->base.dev);
+
+       intel_dp_encoder_suspend(encoder);
+
+       /*
+        * TODO: disconnect also from USB DP alternate mode once we have a
+        * way to handle the modeset restore in that mode during resume
+        * even if the sink has disappeared while being suspended.
+        */
+       if (dig_port->tc_legacy_port)
+               icl_tc_phy_disconnect(i915, dig_port);
+}
+
+static void intel_ddi_encoder_reset(struct drm_encoder *drm_encoder)
+{
+       struct intel_digital_port *dig_port = enc_to_dig_port(drm_encoder);
+       struct drm_i915_private *i915 = to_i915(drm_encoder->dev);
+
+       if (intel_port_is_tc(i915, dig_port->base.port))
+               intel_digital_port_connected(&dig_port->base);
+
+       intel_dp_encoder_reset(drm_encoder);
+}
+
+static void intel_ddi_encoder_destroy(struct drm_encoder *encoder)
+{
+       struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
+       struct drm_i915_private *i915 = to_i915(encoder->dev);
+
+       intel_dp_encoder_flush_work(encoder);
+
+       if (intel_port_is_tc(i915, dig_port->base.port))
+               icl_tc_phy_disconnect(i915, dig_port);
+
+       drm_encoder_cleanup(encoder);
+       kfree(dig_port);
+}
+
 static const struct drm_encoder_funcs intel_ddi_funcs = {
-       .reset = intel_dp_encoder_reset,
-       .destroy = intel_dp_encoder_destroy,
+       .reset = intel_ddi_encoder_reset,
+       .destroy = intel_ddi_encoder_destroy,
 };
 
 static struct intel_connector *
@@ -4080,16 +4121,16 @@ intel_ddi_max_lanes(struct intel_digital_port *intel_dport)
 
 void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
 {
+       struct ddi_vbt_port_info *port_info =
+               &dev_priv->vbt.ddi_port_info[port];
        struct intel_digital_port *intel_dig_port;
        struct intel_encoder *intel_encoder;
        struct drm_encoder *encoder;
        bool init_hdmi, init_dp, init_lspcon = false;
        enum pipe pipe;
 
-
-       init_hdmi = (dev_priv->vbt.ddi_port_info[port].supports_dvi ||
-                    dev_priv->vbt.ddi_port_info[port].supports_hdmi);
-       init_dp = dev_priv->vbt.ddi_port_info[port].supports_dp;
+       init_hdmi = port_info->supports_dvi || port_info->supports_hdmi;
+       init_dp = port_info->supports_dp;
 
        if (intel_bios_is_lspcon_present(dev_priv, port)) {
                /*
@@ -4130,7 +4171,7 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
        intel_encoder->post_disable = intel_ddi_post_disable;
        intel_encoder->get_hw_state = intel_ddi_get_hw_state;
        intel_encoder->get_config = intel_ddi_get_config;
-       intel_encoder->suspend = intel_dp_encoder_suspend;
+       intel_encoder->suspend = intel_ddi_encoder_suspend;
        intel_encoder->get_power_domains = intel_ddi_get_power_domains;
        intel_encoder->type = INTEL_OUTPUT_DDI;
        intel_encoder->power_domain = intel_port_to_power_domain(port);
@@ -4149,6 +4190,10 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
        intel_dig_port->max_lanes = intel_ddi_max_lanes(intel_dig_port);
        intel_dig_port->aux_ch = intel_bios_port_aux_ch(dev_priv, port);
 
+       intel_dig_port->tc_legacy_port = intel_port_is_tc(dev_priv, port) &&
+                                        !port_info->supports_typec_usb &&
+                                        !port_info->supports_tbt;
+
        switch (port) {
        case PORT_A:
                intel_dig_port->ddi_io_power_domain =
@@ -4207,6 +4252,10 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
        }
 
        intel_infoframe_init(intel_dig_port);
+
+       if (intel_port_is_tc(dev_priv, port))
+               intel_digital_port_connected(intel_encoder);
+
        return;
 
 err:

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 3afc3215692bc0d8c0a097734c8fb55efcf4023c..a9f22a366337c2053fb73ca2da33f7d9e9517767 100644 (file)
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -5111,9 +5111,6 @@ static void icl_update_tc_port_type(struct drm_i915_private *dev_priv,
                              tc_type_name(intel_dig_port->tc_type));
 }
 
-static void icl_tc_phy_disconnect(struct drm_i915_private *dev_priv,
-                                 struct intel_digital_port *dig_port);
-
 /*
  * This function implements the first part of the Connect Flow described by our
  * specification, Gen11 TypeC Programming chapter. The rest of the flow (reading
@@ -5148,6 +5145,7 @@ static bool icl_tc_phy_connect(struct drm_i915_private *dev_priv,
        val = I915_READ(PORT_TX_DFLEXDPPMS);
        if (!(val & DP_PHY_MODE_STATUS_COMPLETED(tc_port))) {
                DRM_DEBUG_KMS("DP PHY for TC port %d not ready\n", tc_port);
+               WARN_ON(dig_port->tc_legacy_port);
                return false;
        }
 
@@ -5179,8 +5177,8 @@ static bool icl_tc_phy_connect(struct drm_i915_private *dev_priv,
  * See the comment at the connect function. This implements the Disconnect
  * Flow.
  */
-static void icl_tc_phy_disconnect(struct drm_i915_private *dev_priv,
-                                 struct intel_digital_port *dig_port)
+void icl_tc_phy_disconnect(struct drm_i915_private *dev_priv,
+                          struct intel_digital_port *dig_port)
 {
        enum tc_port tc_port = intel_port_to_tc(dev_priv, dig_port->base.port);
 
@@ -5225,7 +5223,8 @@ static bool icl_tc_port_connected(struct drm_i915_private *dev_priv,
        bool is_legacy, is_typec, is_tbt;
        u32 dpsp;
 
-       is_legacy = I915_READ(SDEISR) & SDE_TC_HOTPLUG_ICP(tc_port);
+       is_legacy = intel_dig_port->tc_legacy_port ||
+               I915_READ(SDEISR) & SDE_TC_HOTPLUG_ICP(tc_port);
 
        /*
         * The spec says we shouldn't be using the ISR bits for detecting
@@ -5237,6 +5236,7 @@ static bool icl_tc_port_connected(struct drm_i915_private *dev_priv,
 
        if (!is_legacy && !is_typec && !is_tbt) {
                icl_tc_phy_disconnect(dev_priv, intel_dig_port);
+
                return false;
        }
 
@@ -5545,7 +5545,7 @@ intel_dp_connector_unregister(struct drm_connector *connector)
        intel_connector_unregister(connector);
 }
 
-void intel_dp_encoder_destroy(struct drm_encoder *encoder)
+void intel_dp_encoder_flush_work(struct drm_encoder *encoder)
 {
        struct intel_digital_port *intel_dig_port = enc_to_dig_port(encoder);
        struct intel_dp *intel_dp = &intel_dig_port->dp;
@@ -5568,9 +5568,14 @@ void intel_dp_encoder_destroy(struct drm_encoder *encoder)
        }
 
        intel_dp_aux_fini(intel_dp);
+}
+
+static void intel_dp_encoder_destroy(struct drm_encoder *encoder)
+{
+       intel_dp_encoder_flush_work(encoder);
 
        drm_encoder_cleanup(encoder);
-       kfree(intel_dig_port);
+       kfree(enc_to_dig_port(encoder));
 }
 
 void intel_dp_encoder_suspend(struct intel_encoder *intel_encoder)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index cb3a055f18c868c5e3ad6a47f511dd9a68410db4..1028af8ec2eb48e9c6fa3390f2c015fe604c67b8 100644 (file)
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1237,6 +1237,7 @@ struct intel_digital_port {
        /* Used for DP and ICL+ TypeC/DP and TypeC/HDMI ports. */
        enum aux_ch aux_ch;
        enum intel_display_power_domain ddi_io_power_domain;
+       bool tc_legacy_port:1;
        enum tc_port_type tc_type;
 
        void (*write_infoframe)(struct intel_encoder *encoder,
@@ -1809,7 +1810,7 @@ void intel_dp_sink_set_decompression_state(struct intel_dp *intel_dp,
                                           bool enable);
 void intel_dp_encoder_reset(struct drm_encoder *encoder);
 void intel_dp_encoder_suspend(struct intel_encoder *intel_encoder);
-void intel_dp_encoder_destroy(struct drm_encoder *encoder);
+void intel_dp_encoder_flush_work(struct drm_encoder *encoder);
 bool intel_dp_compute_config(struct intel_encoder *encoder,
                             struct intel_crtc_state *pipe_config,
                             struct drm_connector_state *conn_state);
@@ -1877,6 +1878,8 @@ bool intel_dp_read_dpcd(struct intel_dp *intel_dp);
 int intel_dp_link_required(int pixel_clock, int bpp);
 int intel_dp_max_data_rate(int max_link_clock, int max_lanes);
 bool intel_digital_port_connected(struct intel_encoder *encoder);
+void icl_tc_phy_disconnect(struct drm_i915_private *dev_priv,
+                          struct intel_digital_port *dig_port);
 
 /* intel_dp_aux_backlight.c */
 int intel_dp_aux_init_backlight_funcs(struct intel_connector *intel_connector);