drm/dp_mst: Clear MSG_RDY flag before sending new message

commit 72f1de49ffb90b29748284f27f1d6b829ab1de95 upstream.

[Why]
The sequence for collecting down_reply from source perspective should
be:

Request_n->repeat (get partial reply of Request_n->clear message ready
flag to ack DPRX that the message is received) till all partial
replies for Request_n are received->new Request_n+1.

Now there is chance that drm_dp_mst_hpd_irq() will fire new down
request in the tx queue when the down reply is incomplete. Source is
restricted to generate interveleaved message transactions so we should
avoid it.

Also, while assembling partial reply packets, reading out DPCD DOWN_REP
Sideband MSG buffer + clearing DOWN_REP_MSG_RDY flag should be
wrapped up as a complete operation for reading out a reply packet.
Kicking off a new request before clearing DOWN_REP_MSG_RDY flag might
be risky. e.g. If the reply of the new request has overwritten the
DPRX DOWN_REP Sideband MSG buffer before source writing one to clear
DOWN_REP_MSG_RDY flag, source then unintentionally flushes the reply
for the new request. Should handle the up request in the same way.

[How]
Separete drm_dp_mst_hpd_irq() into 2 steps. After acking the MST IRQ
event, driver calls drm_dp_mst_hpd_irq_send_new_request() and might
trigger drm_dp_mst_kick_tx() only when there is no on going message
transaction.

Changes since v1:
* Reworked on review comments received
-> Adjust the fix to let driver explicitly kick off new down request
when mst irq event is handled and acked
-> Adjust the commit message

Changes since v2:
* Adjust the commit message
* Adjust the naming of the divided 2 functions and add a new input
  parameter "ack".
* Adjust code flow as per review comments.

Changes since v3:
* Update the function description of drm_dp_mst_hpd_irq_handle_event

Changes since v4:
* Change ack of drm_dp_mst_hpd_irq_handle_event() to be an array align
  the size of esi[]

Signed-off-by: Wayne Lin <Wayne.Lin@amd.com>
Reviewed-by: Lyude Paul <lyude@redhat.com>
Acked-by: Jani Nikula <jani.nikula@intel.com>
Cc: stable@vger.kernel.org
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index b4d5fe09f5fd7b94fa0252a70cab50e095673727..76b091e6ff6f9e02dc22ca96c277d827147e9121 100644 (file)
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -3201,6 +3201,7 @@ static void dm_handle_mst_sideband_msg(struct amdgpu_dm_connector *aconnector)
 
        while (dret == dpcd_bytes_to_read &&
                process_count < max_process_count) {
+               u8 ack[DP_PSR_ERROR_STATUS - DP_SINK_COUNT_ESI] = {};
                u8 retry;
                dret = 0;
 
@@ -3209,28 +3210,29 @@ static void dm_handle_mst_sideband_msg(struct amdgpu_dm_connector *aconnector)
                DRM_DEBUG_DRIVER("ESI %02x %02x %02x\n", esi[0], esi[1], esi[2]);
                /* handle HPD short pulse irq */
                if (aconnector->mst_mgr.mst_state)
-                       drm_dp_mst_hpd_irq(
-                               &aconnector->mst_mgr,
-                               esi,
-                               &new_irq_handled);
+                       drm_dp_mst_hpd_irq_handle_event(&aconnector->mst_mgr,
+                                                       esi,
+                                                       ack,
+                                                       &new_irq_handled);
 
                if (new_irq_handled) {
                        /* ACK at DPCD to notify down stream */
-                       const int ack_dpcd_bytes_to_write =
-                               dpcd_bytes_to_read - 1;
-
                        for (retry = 0; retry < 3; retry++) {
-                               u8 wret;
-
-                               wret = drm_dp_dpcd_write(
-                                       &aconnector->dm_dp_aux.aux,
-                                       dpcd_addr + 1,
-                                       &esi[1],
-                                       ack_dpcd_bytes_to_write);
-                               if (wret == ack_dpcd_bytes_to_write)
+                               ssize_t wret;
+
+                               wret = drm_dp_dpcd_writeb(&aconnector->dm_dp_aux.aux,
+                                                         dpcd_addr + 1,
+                                                         ack[1]);
+                               if (wret == 1)
                                        break;
                        }
 
+                       if (retry == 3) {
+                               DRM_ERROR("Failed to ack MST event.\n");
+                               return;
+                       }
+
+                       drm_dp_mst_hpd_irq_send_new_request(&aconnector->mst_mgr);
                        /* check if there is new irq to be handled */
                        dret = drm_dp_dpcd_read(
                                &aconnector->dm_dp_aux.aux,

diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c
index e2e21ce79510e25415a1133c76e0b858b0d84514..f854cb5eafbe7de90493d2196087a5d34eede5f6 100644 (file)
--- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
@@ -4053,17 +4053,28 @@ out:
 }
 
 /**
- * drm_dp_mst_hpd_irq() - MST hotplug IRQ notify
+ * drm_dp_mst_hpd_irq_handle_event() - MST hotplug IRQ handle MST event
  * @mgr: manager to notify irq for.
  * @esi: 4 bytes from SINK_COUNT_ESI
+ * @ack: 4 bytes used to ack events starting from SINK_COUNT_ESI
  * @handled: whether the hpd interrupt was consumed or not
  *
- * This should be called from the driver when it detects a short IRQ,
+ * This should be called from the driver when it detects a HPD IRQ,
  * along with the value of the DEVICE_SERVICE_IRQ_VECTOR_ESI0. The
- * topology manager will process the sideband messages received as a result
- * of this.
+ * topology manager will process the sideband messages received
+ * as indicated in the DEVICE_SERVICE_IRQ_VECTOR_ESI0 and set the
+ * corresponding flags that Driver has to ack the DP receiver later.
+ *
+ * Note that driver shall also call
+ * drm_dp_mst_hpd_irq_send_new_request() if the 'handled' is set
+ * after calling this function, to try to kick off a new request in
+ * the queue if the previous message transaction is completed.
+ *
+ * See also:
+ * drm_dp_mst_hpd_irq_send_new_request()
  */
-int drm_dp_mst_hpd_irq(struct drm_dp_mst_topology_mgr *mgr, u8 *esi, bool *handled)
+int drm_dp_mst_hpd_irq_handle_event(struct drm_dp_mst_topology_mgr *mgr, const u8 *esi,
+                                   u8 *ack, bool *handled)
 {
        int ret = 0;
        int sc;
@@ -4078,18 +4089,47 @@ int drm_dp_mst_hpd_irq(struct drm_dp_mst_topology_mgr *mgr, u8 *esi, bool *handl
        if (esi[1] & DP_DOWN_REP_MSG_RDY) {
                ret = drm_dp_mst_handle_down_rep(mgr);
                *handled = true;
+               ack[1] |= DP_DOWN_REP_MSG_RDY;
        }
 
        if (esi[1] & DP_UP_REQ_MSG_RDY) {
                ret |= drm_dp_mst_handle_up_req(mgr);
                *handled = true;
+               ack[1] |= DP_UP_REQ_MSG_RDY;
        }
 
-       drm_dp_mst_kick_tx(mgr);
        return ret;
 }
-EXPORT_SYMBOL(drm_dp_mst_hpd_irq);
+EXPORT_SYMBOL(drm_dp_mst_hpd_irq_handle_event);
 
+/**
+ * drm_dp_mst_hpd_irq_send_new_request() - MST hotplug IRQ kick off new request
+ * @mgr: manager to notify irq for.
+ *
+ * This should be called from the driver when mst irq event is handled
+ * and acked. Note that new down request should only be sent when
+ * previous message transaction is completed. Source is not supposed to generate
+ * interleaved message transactions.
+ */
+void drm_dp_mst_hpd_irq_send_new_request(struct drm_dp_mst_topology_mgr *mgr)
+{
+       struct drm_dp_sideband_msg_tx *txmsg;
+       bool kick = true;
+
+       mutex_lock(&mgr->qlock);
+       txmsg = list_first_entry_or_null(&mgr->tx_msg_downq,
+                                        struct drm_dp_sideband_msg_tx, next);
+       /* If last transaction is not completed yet*/
+       if (!txmsg ||
+           txmsg->state == DRM_DP_SIDEBAND_TX_START_SEND ||
+           txmsg->state == DRM_DP_SIDEBAND_TX_SENT)
+               kick = false;
+       mutex_unlock(&mgr->qlock);
+
+       if (kick)
+               drm_dp_mst_kick_tx(mgr);
+}
+EXPORT_SYMBOL(drm_dp_mst_hpd_irq_send_new_request);
 /**
  * drm_dp_mst_detect_port() - get connection status for an MST port
  * @connector: DRM connector for this port

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index b1653308f1450d6d02585a8e479b0266367dcb1b..594ea037050a902096143da2d53ed16d051fd19a 100644 (file)
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -3804,9 +3804,7 @@ intel_dp_mst_hpd_irq(struct intel_dp *intel_dp, u8 *esi, u8 *ack)
 {
        bool handled = false;
 
-       drm_dp_mst_hpd_irq(&intel_dp->mst_mgr, esi, &handled);
-       if (handled)
-               ack[1] |= esi[1] & (DP_DOWN_REP_MSG_RDY | DP_UP_REQ_MSG_RDY);
+       drm_dp_mst_hpd_irq_handle_event(&intel_dp->mst_mgr, esi, ack, &handled);
 
        if (esi[1] & DP_CP_IRQ) {
                intel_hdcp_handle_cp_irq(intel_dp->attached_connector);
@@ -3881,6 +3879,9 @@ intel_dp_check_mst_status(struct intel_dp *intel_dp)
 
                if (!intel_dp_ack_sink_irq_esi(intel_dp, ack))
                        drm_dbg_kms(&i915->drm, "Failed to ack ESI\n");
+
+               if (ack[1] & (DP_DOWN_REP_MSG_RDY | DP_UP_REQ_MSG_RDY))
+                       drm_dp_mst_hpd_irq_send_new_request(&intel_dp->mst_mgr);
        }
 
        return link_ok;

diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 006cb76adaa935b84cf003b667801a0dbf33a54a..a851354c0c5f82f3e86b6550bd18820ca7c60688 100644 (file)
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -1473,22 +1473,26 @@ nv50_mstm_service(struct nouveau_drm *drm,
        u8 esi[8] = {};
 
        while (handled) {
+               u8 ack[8] = {};
+
                rc = drm_dp_dpcd_read(aux, DP_SINK_COUNT_ESI, esi, 8);
                if (rc != 8) {
                        ret = false;
                        break;
                }
 
-               drm_dp_mst_hpd_irq(&mstm->mgr, esi, &handled);
+               drm_dp_mst_hpd_irq_handle_event(&mstm->mgr, esi, ack, &handled);
                if (!handled)
                        break;
 
-               rc = drm_dp_dpcd_write(aux, DP_SINK_COUNT_ESI + 1, &esi[1],
-                                      3);
-               if (rc != 3) {
+               rc = drm_dp_dpcd_writeb(aux, DP_SINK_COUNT_ESI + 1, ack[1]);
+
+               if (rc != 1) {
                        ret = false;
                        break;
                }
+
+               drm_dp_mst_hpd_irq_send_new_request(&mstm->mgr);
        }
 
        if (!ret)

diff --git a/include/drm/display/drm_dp_mst_helper.h b/include/drm/display/drm_dp_mst_helper.h
index 32c764fb9cb563fb6c8ac526b845100b60ef7764..40e855c8407cf71e0c3afdcb35731e9541a37f81 100644 (file)
--- a/include/drm/display/drm_dp_mst_helper.h
+++ b/include/drm/display/drm_dp_mst_helper.h
@@ -815,8 +815,11 @@ void drm_dp_mst_topology_mgr_destroy(struct drm_dp_mst_topology_mgr *mgr);
 bool drm_dp_read_mst_cap(struct drm_dp_aux *aux, const u8 dpcd[DP_RECEIVER_CAP_SIZE]);
 int drm_dp_mst_topology_mgr_set_mst(struct drm_dp_mst_topology_mgr *mgr, bool mst_state);
 
-int drm_dp_mst_hpd_irq(struct drm_dp_mst_topology_mgr *mgr, u8 *esi, bool *handled);
-
+int drm_dp_mst_hpd_irq_handle_event(struct drm_dp_mst_topology_mgr *mgr,
+                                   const u8 *esi,
+                                   u8 *ack,
+                                   bool *handled);
+void drm_dp_mst_hpd_irq_send_new_request(struct drm_dp_mst_topology_mgr *mgr);
 
 int
 drm_dp_mst_detect_port(struct drm_connector *connector,