drm/amd/display: take dc_lock in short pulse handler only

[Why]
Conditions that end up modifying the global dc state must be locked.
However, during mst allocate payload sequence, lock is already taken.
With StarTech 1.2 DP hub, we get an HPD RX interrupt for a reason other
than to indicate down reply availability right after sending payload
allocation. The handler again takes dc lock before calling the
dc's HPD RX handler. Due to this contention, the DRM thread which waits
for MST down reply never gets a chance to finish its waiting
successfully and ends up timing out. Once the lock is released, the hpd
rx handler fires and goes ahead to read from the MST HUB, but now its
too late and the HUB doesnt lightup all displays since DRM lacks error
handling when payload allocation fails.

[How]
Take lock only if there is a change in link status or if automated test
pattern bit is set. The latter fixes the null pointer dereference when
running certain DP Link Layer Compliance test.

Fixes: c8ea79a8a276 ("drm/amd/display: NULL pointer error during compliance test")
Signed-off-by: Aurabindo Pillai <aurabindo.pillai@amd.com>
Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>

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 515b6d7d91287a43f5abb3f59c572f2a59587b62..a4016fff51cdc6509b1b3419d8d3003b117840d9 100644 (file)
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -28,6 +28,7 @@
 
 #include "dm_services_types.h"
 #include "dc.h"
+#include "dc_link_dp.h"
 #include "dc/inc/core_types.h"
 #include "dal_asic_id.h"
 #include "dmub/dmub_srv.h"
@@ -2739,6 +2740,7 @@ static void handle_hpd_rx_irq(void *param)
        enum dc_connection_type new_connection_type = dc_connection_none;
        struct amdgpu_device *adev = drm_to_adev(dev);
        union hpd_irq_data hpd_irq_data;
+       bool lock_flag = 0;
 
        memset(&hpd_irq_data, 0, sizeof(hpd_irq_data));
 
@@ -2768,15 +2770,28 @@ static void handle_hpd_rx_irq(void *param)
                }
        }
 
-       if (!amdgpu_in_reset(adev)) {
+       /*
+        * TODO: We need the lock to avoid touching DC state while it's being
+        * modified during automated compliance testing, or when link loss
+        * happens. While this should be split into subhandlers and proper
+        * interfaces to avoid having to conditionally lock like this in the
+        * outer layer, we need this workaround temporarily to allow MST
+        * lightup in some scenarios to avoid timeout.
+        */
+       if (!amdgpu_in_reset(adev) &&
+           (hpd_rx_irq_check_link_loss_status(dc_link, &hpd_irq_data) ||
+            hpd_irq_data.bytes.device_service_irq.bits.AUTOMATED_TEST)) {
                mutex_lock(&adev->dm.dc_lock);
+               lock_flag = 1;
+       }
+
 #ifdef CONFIG_DRM_AMD_DC_HDCP
        result = dc_link_handle_hpd_rx_irq(dc_link, &hpd_irq_data, NULL);
 #else
        result = dc_link_handle_hpd_rx_irq(dc_link, NULL, NULL);
 #endif
+       if (!amdgpu_in_reset(adev) && lock_flag)
                mutex_unlock(&adev->dm.dc_lock);
-       }
 
 out:
        if (result && !is_mst_root_connector) {

diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
index 9e08410bfdfd5b5fb7d3ca0962d4d24f226836c3..32fb9cdbd9801ed746f3dc25ad1c9dd91bf3b2b9 100644 (file)
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
@@ -2070,7 +2070,7 @@ enum dc_status read_hpd_rx_irq_data(
        return retval;
 }
 
-static bool hpd_rx_irq_check_link_loss_status(
+bool hpd_rx_irq_check_link_loss_status(
        struct dc_link *link,
        union hpd_irq_data *hpd_irq_dpcd_data)
 {

diff --git a/drivers/gpu/drm/amd/display/dc/inc/dc_link_dp.h b/drivers/gpu/drm/amd/display/dc/inc/dc_link_dp.h
index ffc3f2c63db843fc2f41609316cae2123bc1ebc8..7dd8bca542b96355618ab695f15788045af12737 100644 (file)
--- a/drivers/gpu/drm/amd/display/dc/inc/dc_link_dp.h
+++ b/drivers/gpu/drm/amd/display/dc/inc/dc_link_dp.h
@@ -68,6 +68,10 @@ bool perform_link_training_with_retries(
        enum signal_type signal,
        bool do_fallback);
 
+bool hpd_rx_irq_check_link_loss_status(
+       struct dc_link *link,
+       union hpd_irq_data *hpd_irq_dpcd_data);
+
 bool is_mst_supported(struct dc_link *link);
 
 bool detect_dp_sink_caps(struct dc_link *link);