drm/dp_mst: Handle SST-only branch device case
authorWayne Lin <Wayne.Lin@amd.com>
Fri, 17 Jan 2020 06:03:50 +0000 (14:03 +0800)
committerLyude Paul <lyude@redhat.com>
Sat, 18 Jan 2020 00:17:00 +0000 (19:17 -0500)
[Why]
While handling LINK_ADDRESS reply, current code expects a peer device
can handle sideband message once the peer device type is reported as
DP_PEER_DEVICE_MST_BRANCHING. However, when the connected device is
a SST branch case, it can't handle the sideband message(MST_CAP=0 in
DPCD 00021h).

Current code will try to send LINK_ADDRESS to SST branch device and end
up with message timeout and monitor can't display normally. As the
result of that, we should take SST branch device into account.

[How]
According to DP 1.4 spec, we can use Peer_Device_Type as
DP_PEER_DEVICE_MST_BRANCHING and Message_Capability_Status as 0 to
indicate peer device as a SST-only branch device.

Fix following:
- Add the function drm_dp_mst_is_dp_mst_end_device() to decide whether a
peer device connected to a DFP is mst end device. Which also indicates
if the peer device is capable of handling message or not.
- Take SST-only branch device case into account in
drm_dp_port_set_pdt() and add a new parameter 'new_mcs'. Take sst branch
device case as the same case as DP_PEER_DEVICE_DP_LEGACY_CONV and
DP_PEER_DEVICE_SST_SINK. All original handling logics remain.
- Take SST-only branch device case into account in
drm_dp_mst_port_add_connector().
- Fix some parts in drm_dp_mst_handle_link_address_port() to have SST
branch device case into consideration.
- Fix the arguments of drm_dp_port_set_pdt() in
drm_dp_mst_handle_conn_stat().
- Have SST branch device also report
connector_status_connected when the ddps is true
in drm_dp_mst_detect_port()
- Fix the arguments of drm_dp_port_set_pdt() in
drm_dp_delayed_destroy_port()

Changes since v1:(https://patchwork.kernel.org/patch/11323079/)
* Squash previous patch into one patch and merge the commit message here.
* Combine the if statements mentioned in comments

Fixes: c485e2c97dae ("drm/dp_mst: Refactor pdt setup/teardown, add more locking")
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Harry Wentland <hwentlan@amd.com>
Cc: Lyude Paul <lyude@redhat.com>
Signed-off-by: Wayne Lin <Wayne.Lin@amd.com>
Reviewed-by: Lyude Paul <lyude@redhat.com>
Signed-off-by: Lyude Paul <lyude@redhat.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20200117060350.26358-2-Wayne.Lin@amd.com
drivers/gpu/drm/drm_dp_mst_topology.c

index 5a61a55..e6afe4f 100644 (file)
@@ -1916,73 +1916,90 @@ static u8 drm_dp_calculate_rad(struct drm_dp_mst_port *port,
        return parent_lct + 1;
 }
 
-static int drm_dp_port_set_pdt(struct drm_dp_mst_port *port, u8 new_pdt)
+static bool drm_dp_mst_is_dp_mst_end_device(u8 pdt, bool mcs)
+{
+       switch (pdt) {
+       case DP_PEER_DEVICE_DP_LEGACY_CONV:
+       case DP_PEER_DEVICE_SST_SINK:
+               return true;
+       case DP_PEER_DEVICE_MST_BRANCHING:
+               /* For sst branch device */
+               if (!mcs)
+                       return true;
+
+               return false;
+       }
+       return true;
+}
+
+static int
+drm_dp_port_set_pdt(struct drm_dp_mst_port *port, u8 new_pdt,
+                   bool new_mcs)
 {
        struct drm_dp_mst_topology_mgr *mgr = port->mgr;
        struct drm_dp_mst_branch *mstb;
        u8 rad[8], lct;
        int ret = 0;
 
-       if (port->pdt == new_pdt)
+       if (port->pdt == new_pdt && port->mcs == new_mcs)
                return 0;
 
        /* Teardown the old pdt, if there is one */
-       switch (port->pdt) {
-       case DP_PEER_DEVICE_DP_LEGACY_CONV:
-       case DP_PEER_DEVICE_SST_SINK:
-               /*
-                * If the new PDT would also have an i2c bus, don't bother
-                * with reregistering it
-                */
-               if (new_pdt == DP_PEER_DEVICE_DP_LEGACY_CONV ||
-                   new_pdt == DP_PEER_DEVICE_SST_SINK) {
-                       port->pdt = new_pdt;
-                       return 0;
-               }
+       if (port->pdt != DP_PEER_DEVICE_NONE) {
+               if (drm_dp_mst_is_dp_mst_end_device(port->pdt, port->mcs)) {
+                       /*
+                        * If the new PDT would also have an i2c bus,
+                        * don't bother with reregistering it
+                        */
+                       if (new_pdt != DP_PEER_DEVICE_NONE &&
+                           drm_dp_mst_is_dp_mst_end_device(new_pdt, new_mcs)) {
+                               port->pdt = new_pdt;
+                               port->mcs = new_mcs;
+                               return 0;
+                       }
 
-               /* remove i2c over sideband */
-               drm_dp_mst_unregister_i2c_bus(&port->aux);
-               break;
-       case DP_PEER_DEVICE_MST_BRANCHING:
-               mutex_lock(&mgr->lock);
-               drm_dp_mst_topology_put_mstb(port->mstb);
-               port->mstb = NULL;
-               mutex_unlock(&mgr->lock);
-               break;
+                       /* remove i2c over sideband */
+                       drm_dp_mst_unregister_i2c_bus(&port->aux);
+               } else {
+                       mutex_lock(&mgr->lock);
+                       drm_dp_mst_topology_put_mstb(port->mstb);
+                       port->mstb = NULL;
+                       mutex_unlock(&mgr->lock);
+               }
        }
 
        port->pdt = new_pdt;
-       switch (port->pdt) {
-       case DP_PEER_DEVICE_DP_LEGACY_CONV:
-       case DP_PEER_DEVICE_SST_SINK:
-               /* add i2c over sideband */
-               ret = drm_dp_mst_register_i2c_bus(&port->aux);
-               break;
+       port->mcs = new_mcs;
 
-       case DP_PEER_DEVICE_MST_BRANCHING:
-               lct = drm_dp_calculate_rad(port, rad);
-               mstb = drm_dp_add_mst_branch_device(lct, rad);
-               if (!mstb) {
-                       ret = -ENOMEM;
-                       DRM_ERROR("Failed to create MSTB for port %p", port);
-                       goto out;
-               }
+       if (port->pdt != DP_PEER_DEVICE_NONE) {
+               if (drm_dp_mst_is_dp_mst_end_device(port->pdt, port->mcs)) {
+                       /* add i2c over sideband */
+                       ret = drm_dp_mst_register_i2c_bus(&port->aux);
+               } else {
+                       lct = drm_dp_calculate_rad(port, rad);
+                       mstb = drm_dp_add_mst_branch_device(lct, rad);
+                       if (!mstb) {
+                               ret = -ENOMEM;
+                               DRM_ERROR("Failed to create MSTB for port %p",
+                                         port);
+                               goto out;
+                       }
 
-               mutex_lock(&mgr->lock);
-               port->mstb = mstb;
-               mstb->mgr = port->mgr;
-               mstb->port_parent = port;
+                       mutex_lock(&mgr->lock);
+                       port->mstb = mstb;
+                       mstb->mgr = port->mgr;
+                       mstb->port_parent = port;
 
-               /*
-                * Make sure this port's memory allocation stays
-                * around until its child MSTB releases it
-                */
-               drm_dp_mst_get_port_malloc(port);
-               mutex_unlock(&mgr->lock);
+                       /*
+                        * Make sure this port's memory allocation stays
+                        * around until its child MSTB releases it
+                        */
+                       drm_dp_mst_get_port_malloc(port);
+                       mutex_unlock(&mgr->lock);
 
-               /* And make sure we send a link address for this */
-               ret = 1;
-               break;
+                       /* And make sure we send a link address for this */
+                       ret = 1;
+               }
        }
 
 out:
@@ -2135,9 +2152,8 @@ drm_dp_mst_port_add_connector(struct drm_dp_mst_branch *mstb,
                goto error;
        }
 
-       if ((port->pdt == DP_PEER_DEVICE_DP_LEGACY_CONV ||
-            port->pdt == DP_PEER_DEVICE_SST_SINK) &&
-           port->port_num >= DP_MST_LOGICAL_PORT_0) {
+       if (port->pdt != DP_PEER_DEVICE_NONE &&
+           drm_dp_mst_is_dp_mst_end_device(port->pdt, port->mcs)) {
                port->cached_edid = drm_get_edid(port->connector,
                                                 &port->aux.ddc);
                drm_connector_set_tile_property(port->connector);
@@ -2201,6 +2217,7 @@ drm_dp_mst_handle_link_address_port(struct drm_dp_mst_branch *mstb,
        struct drm_dp_mst_port *port;
        int old_ddps = 0, ret;
        u8 new_pdt = DP_PEER_DEVICE_NONE;
+       bool new_mcs = 0;
        bool created = false, send_link_addr = false, changed = false;
 
        port = drm_dp_get_port(mstb, port_msg->port_number);
@@ -2245,7 +2262,7 @@ drm_dp_mst_handle_link_address_port(struct drm_dp_mst_branch *mstb,
        port->input = port_msg->input_port;
        if (!port->input)
                new_pdt = port_msg->peer_device_type;
-       port->mcs = port_msg->mcs;
+       new_mcs = port_msg->mcs;
        port->ddps = port_msg->ddps;
        port->ldps = port_msg->legacy_device_plug_status;
        port->dpcd_rev = port_msg->dpcd_revision;
@@ -2272,7 +2289,7 @@ drm_dp_mst_handle_link_address_port(struct drm_dp_mst_branch *mstb,
                }
        }
 
-       ret = drm_dp_port_set_pdt(port, new_pdt);
+       ret = drm_dp_port_set_pdt(port, new_pdt, new_mcs);
        if (ret == 1) {
                send_link_addr = true;
        } else if (ret < 0) {
@@ -2286,7 +2303,8 @@ drm_dp_mst_handle_link_address_port(struct drm_dp_mst_branch *mstb,
         * we're coming out of suspend. In this case, always resend the link
         * address if there's an MSTB on this port
         */
-       if (!created && port->pdt == DP_PEER_DEVICE_MST_BRANCHING)
+       if (!created && port->pdt == DP_PEER_DEVICE_MST_BRANCHING &&
+           port->mcs)
                send_link_addr = true;
 
        if (port->connector)
@@ -2323,6 +2341,7 @@ drm_dp_mst_handle_conn_stat(struct drm_dp_mst_branch *mstb,
        struct drm_dp_mst_port *port;
        int old_ddps, old_input, ret, i;
        u8 new_pdt;
+       bool new_mcs;
        bool dowork = false, create_connector = false;
 
        port = drm_dp_get_port(mstb, conn_stat->port_number);
@@ -2354,7 +2373,6 @@ drm_dp_mst_handle_conn_stat(struct drm_dp_mst_branch *mstb,
        old_ddps = port->ddps;
        old_input = port->input;
        port->input = conn_stat->input_port;
-       port->mcs = conn_stat->message_capability_status;
        port->ldps = conn_stat->legacy_device_plug_status;
        port->ddps = conn_stat->displayport_device_plug_status;
 
@@ -2367,8 +2385,8 @@ drm_dp_mst_handle_conn_stat(struct drm_dp_mst_branch *mstb,
        }
 
        new_pdt = port->input ? DP_PEER_DEVICE_NONE : conn_stat->peer_device_type;
-
-       ret = drm_dp_port_set_pdt(port, new_pdt);
+       new_mcs = conn_stat->message_capability_status;
+       ret = drm_dp_port_set_pdt(port, new_pdt, new_mcs);
        if (ret == 1) {
                dowork = true;
        } else if (ret < 0) {
@@ -3929,6 +3947,8 @@ drm_dp_mst_detect_port(struct drm_connector *connector,
        switch (port->pdt) {
        case DP_PEER_DEVICE_NONE:
        case DP_PEER_DEVICE_MST_BRANCHING:
+               if (!port->mcs)
+                       ret = connector_status_connected;
                break;
 
        case DP_PEER_DEVICE_SST_SINK:
@@ -4541,7 +4561,7 @@ drm_dp_delayed_destroy_port(struct drm_dp_mst_port *port)
        if (port->connector)
                port->mgr->cbs->destroy_connector(port->mgr, port->connector);
 
-       drm_dp_port_set_pdt(port, DP_PEER_DEVICE_NONE);
+       drm_dp_port_set_pdt(port, DP_PEER_DEVICE_NONE, port->mcs);
        drm_dp_mst_put_port_malloc(port);
 }