drm/dp_mst: Clear MSG_RDY flag before sending new message

PATCH Superseded Wayne Lin 4 participants 9 messages Apr 18, 2023 – Apr 21, 2023

Subsystems (8)

Stable branch DRM drivers DRM misc Intel i915 +4 more subsystems

lore

Share & tools

lore

Wayne Lin

author

2023-04-18 06:09 UTC #0 lore raw

PATCH 1/1 drm/dp_mst: Clear MSG_RDY flag before sending new message

[Why & How]
The sequence for collecting down_reply/up_request 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.

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 ack 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.

In drm_dp_mst_hpd_irq(), we don't clear MSG_RDY flag before caliing drm_dp_mst_kick_tx(). Fix that.

Signed-off-by Wayne Lin <[email protected]>

Cc [email protected]

---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  2 ++
 drivers/gpu/drm/display/drm_dp_mst_topology.c | 22 +++++++++++++++++++
 drivers/gpu/drm/i915/display/intel_dp.c       |  3 +++
 drivers/gpu/drm/nouveau/dispnv50/disp.c       |  2 ++
 4 files changed, 29 insertions(+)

drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +2 -0

index 77277d90b6e2..5313a5656598 100644
@@ -3166,6 +3166,8 @@ static void dm_handle_mst_sideband_msg(struct amdgpu_dm_connector *aconnector)
 			for (retry = 0; retry < 3; retry++) {
 				uint8_t wret;
 
+				/* MSG_RDY ack is done in drm*/
+				esi[1] &= ~(DP_DOWN_REP_MSG_RDY | DP_UP_REQ_MSG_RDY);
 				wret = drm_dp_dpcd_write(
 					&aconnector->dm_dp_aux.aux,
 					dpcd_addr + 1,

drivers/gpu/drm/display/drm_dp_mst_topology.c +22 -0

index 51a46689cda7..02aad713c67c 100644
@@ -4054,6 +4054,9 @@ int drm_dp_mst_hpd_irq(struct drm_dp_mst_topology_mgr *mgr, u8 *esi, bool *handl
 {
 	int ret = 0;
 	int sc;
+	const int tosend = 1;
+	int retries = 0;
+	u8 buf = 0;
 	*handled = false;
 	sc = DP_GET_SINK_COUNT(esi[0]);

@@ -4072,6 +4075,25 @@ int drm_dp_mst_hpd_irq(struct drm_dp_mst_topology_mgr *mgr, u8 *esi, bool *handl
 		*handled = true;
 	}
 
+	if (*handled) {
+		buf = esi[1] & (DP_DOWN_REP_MSG_RDY | DP_UP_REQ_MSG_RDY);
+		do {
+			ret = drm_dp_dpcd_write(mgr->aux,
+						DP_DEVICE_SERVICE_IRQ_VECTOR_ESI0,
+						&buf,
+						tosend);
+
+			if (ret == tosend)
+				break;
+
+			retries++;
+		} while (retries < 5);
+
+		if (ret != tosend)
+			drm_dbg_kms(mgr->dev, "failed to write dpcd 0x%x\n",
+				    DP_DEVICE_SERVICE_IRQ_VECTOR_ESI0);
+	}
+
 	drm_dp_mst_kick_tx(mgr);
 	return ret;
 }

drivers/gpu/drm/i915/display/intel_dp.c +3 -0

index bf80f296a8fd..abec3de38b66 100644
@@ -3939,6 +3939,9 @@ intel_dp_check_mst_status(struct intel_dp *intel_dp)
 		if (!memchr_inv(ack, 0, sizeof(ack)))
 			break;
 
+		/* MSG_RDY ack is done in drm*/
+		ack[1] &= ~(DP_DOWN_REP_MSG_RDY | DP_UP_REQ_MSG_RDY);
+
 		if (!intel_dp_ack_sink_irq_esi(intel_dp, ack))
 			drm_dbg_kms(&i915->drm, "Failed to ack ESI\n");
 	}

drivers/gpu/drm/nouveau/dispnv50/disp.c +2 -0

index edcb2529b402..e905987104ed 100644
@@ -1336,6 +1336,8 @@ nv50_mstm_service(struct nouveau_drm *drm,
 		if (!handled)
 			break;
 
+		/* MSG_RDY ack is done in drm*/
+		esi[1] &= ~(DP_DOWN_REP_MSG_RDY | DP_UP_REQ_MSG_RDY);
 		rc = drm_dp_dpcd_write(aux, DP_SINK_COUNT_ESI + 1, &esi[1],
 				       3);
 		if (rc != 3) {

Signature · 1 line

2.37.3

Patchwork

2023-04-18 07:35 UTC #1 lore raw

[Intel-gfx] ✗ Fi.CI.BAT: failure for drm/dp_mst: Clear MSG_RDY flag before sending new message

== Series Details ==

Series: drm/dp_mst: Clear MSG_RDY flag before sending new message
URL : https://patchwork.freedesktop.org/series/116623/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_13023 -> Patchwork_116623v1
====================================================

Summary
-------

**FAILURE**

Serious unknown changes coming with Patchwork_116623v1 absolutely need to be verified manually.

If you think the reported changes have nothing to do with the changes introduced in Patchwork_116623v1, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI.

External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116623v1/index.html

Participating hosts (38 -> 36)
------------------------------

Missing (2): fi-kbl-soraka fi-snb-2520m

Possible new issues
-------------------

Here are the unknown changes that may have been introduced in Patchwork_116623v1:

### IGT changes ###

#### Possible regressions ####

* igt@gem_exec_suspend@basic-s0@smem:
- fi-cfl-8700k: [PASS][1] -> [ABORT][2]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13023/fi-cfl-8700k/igt@gem_exec_suspend@[email protected]
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116623v1/fi-cfl-8700k/igt@gem_exec_suspend@[email protected]

Known issues
------------

Here are the changes found in Patchwork_116623v1 that come from known issues:

### IGT changes ###

#### Issues hit ####

* igt@i915_selftest@live@gt_pm:
- bat-rpls-2: [PASS][3] -> [DMESG-FAIL][4] ([i915#4258])
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13023/bat-rpls-2/igt@i915_selftest@live@gt_pm.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116623v1/bat-rpls-2/igt@i915_selftest@live@gt_pm.html

* igt@i915_selftest@live@requests:
- bat-rpls-1: [PASS][5] -> [ABORT][6] ([i915#7911] / [i915#7982])
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13023/bat-rpls-1/igt@i915_selftest@[email protected]
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116623v1/bat-rpls-1/igt@i915_selftest@[email protected]

* igt@i915_selftest@live@reset:
- bat-rpls-2: [PASS][7] -> [ABORT][8] ([i915#4983] / [i915#7913])
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13023/bat-rpls-2/igt@i915_selftest@[email protected]
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116623v1/bat-rpls-2/igt@i915_selftest@[email protected]

[i915#4258]: https://gitlab.freedesktop.org/drm/intel/issues/4258
[i915#4983]: https://gitlab.freedesktop.org/drm/intel/issues/4983
[i915#7911]: https://gitlab.freedesktop.org/drm/intel/issues/7911
[i915#7913]: https://gitlab.freedesktop.org/drm/intel/issues/7913
[i915#7982]: https://gitlab.freedesktop.org/drm/intel/issues/7982

Build changes
-------------

* Linux: CI_DRM_13023 -> Patchwork_116623v1

CI-20190529: 20190529
CI_DRM_13023: 0c6c8e560ac78d000179e195e1280cc3479ebd58 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_7258: ad2eb276eda849b7a7985229009a816c7608186c @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
Patchwork_116623v1: 0c6c8e560ac78d000179e195e1280cc3479ebd58 @ git://anongit.freedesktop.org/gfx-ci/linux

### Linux commits

9c6a56132f18 drm/dp_mst: Clear MSG_RDY flag before sending new message

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116623v1/index.html

Jani Nikula

2023-04-18 08:53 UTC #2 lore raw

In reply to Wayne Lin

Quoted 34 lines · Wayne Lin

[Why & How]
The sequence for collecting down_reply/up_request 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.
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 ack 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.
In drm_dp_mst_hpd_irq(), we don't clear MSG_RDY flag before caliing drm_dp_mst_kick_tx(). Fix that.
Signed-off-by Wayne Lin <[email protected]>
Cc [email protected]
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  2 ++
 drivers/gpu/drm/display/drm_dp_mst_topology.c | 22 +++++++++++++++++++
 drivers/gpu/drm/i915/display/intel_dp.c       |  3 +++
 drivers/gpu/drm/nouveau/dispnv50/disp.c       |  2 ++
 4 files changed, 29 insertions(+)
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +0 -0
index 77277d90b6e2..5313a5656598 100644

drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c static void dm_handle_mst_sideband_msg(struct amdgpu_dm_connector *aconnector) · line 3170

Quoted 5 lines

drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +2 -0

@@ -3166,6 +3166,8 @@ static void dm_handle_mst_sideband_msg(struct amdgpu_dm_connector *aconnector)
 			for (retry = 0; retry < 3; retry++) {
 				uint8_t wret;
 
+				/* MSG_RDY ack is done in drm*/
+				esi[1] &= ~(DP_DOWN_REP_MSG_RDY | DP_UP_REQ_MSG_RDY);

Why do the masking within the retry loop?

Quoted 8 lines

drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +0 -0
 				wret = drm_dp_dpcd_write(
 					&aconnector->dm_dp_aux.aux,
 					dpcd_addr + 1,
drivers/gpu/drm/display/drm_dp_mst_topology.c +0 -0
index 51a46689cda7..02aad713c67c 100644

drivers/gpu/drm/display/drm_dp_mst_topology.c int drm_dp_mst_hpd_irq(struct drm_dp_mst_topology_mgr *mgr, u8 *esi, bool *handl · line 4059

Quoted 6 lines

drivers/gpu/drm/display/drm_dp_mst_topology.c +3 -0

@@ -4054,6 +4054,9 @@ int drm_dp_mst_hpd_irq(struct drm_dp_mst_topology_mgr *mgr, u8 *esi, bool *handl
 {
 	int ret = 0;
 	int sc;
+	const int tosend = 1;
+	int retries = 0;
+	u8 buf = 0;

All of these should be in tighter scope.

drivers/gpu/drm/display/drm_dp_mst_topology.c int drm_dp_mst_hpd_irq(struct drm_dp_mst_topology_mgr *mgr, u8 *esi, bool *handl · line 4078

Quoted 8 lines

drivers/gpu/drm/display/drm_dp_mst_topology.c +1 -0

 	*handled = false;
 	sc = DP_GET_SINK_COUNT(esi[0]);

@@ -4072,6 +4075,25 @@ int drm_dp_mst_hpd_irq(struct drm_dp_mst_topology_mgr *mgr, u8 *esi, bool *handl
 		*handled = true;
 	}
 
+	if (*handled) {

That should check for DP_DOWN_REP_MSG_RDY and DP_UP_REQ_MSG_RDY only, right? If those are not set, we didn't do anything with them, and should not ack.

drivers/gpu/drm/display/drm_dp_mst_topology.c int drm_dp_mst_hpd_irq(struct drm_dp_mst_topology_mgr *mgr, u8 *esi, bool *handl · line 4084

Quoted 6 lines

drivers/gpu/drm/display/drm_dp_mst_topology.c +6 -0

+		buf = esi[1] & (DP_DOWN_REP_MSG_RDY | DP_UP_REQ_MSG_RDY);
+		do {
+			ret = drm_dp_dpcd_write(mgr->aux,
+						DP_DEVICE_SERVICE_IRQ_VECTOR_ESI0,
+						&buf,
+						tosend);

We should probably have a helper function to do the acking, similar to intel_dp_ack_sink_irq_esi(), which could be used both by this function and the drivers.

drivers/gpu/drm/display/drm_dp_mst_topology.c int drm_dp_mst_hpd_irq(struct drm_dp_mst_topology_mgr *mgr, u8 *esi, bool *handl · line 4090

Quoted 6 lines

drivers/gpu/drm/display/drm_dp_mst_topology.c +6 -0

+
+			if (ret == tosend)
+				break;
+
+			retries++;
+		} while (retries < 5);

Please don't use a do-while when a for loop is sufficient.

	for (tries = 0; tries < 5; tries++)

and it's obvious at a glance how many times at most this runs. Not so with a do-while where you count *re-tries*. Again, would be nice to abstract this away in a helper function.

Quoted 14 lines

drivers/gpu/drm/display/drm_dp_mst_topology.c +6 -0

+
+		if (ret != tosend)
+			drm_dbg_kms(mgr->dev, "failed to write dpcd 0x%x\n",
+				    DP_DEVICE_SERVICE_IRQ_VECTOR_ESI0);
+	}
+
 	drm_dp_mst_kick_tx(mgr);
 	return ret;
 }

drivers/gpu/drm/i915/display/intel_dp.c +0 -0

index bf80f296a8fd..abec3de38b66 100644

drivers/gpu/drm/i915/display/intel_dp.c intel_dp_check_mst_status(struct intel_dp *intel_dp) · line 3943

Quoted 5 lines

drivers/gpu/drm/i915/display/intel_dp.c +2 -0

@@ -3939,6 +3939,9 @@ intel_dp_check_mst_status(struct intel_dp *intel_dp)
 		if (!memchr_inv(ack, 0, sizeof(ack)))
 			break;
 
+		/* MSG_RDY ack is done in drm*/
+		ack[1] &= ~(DP_DOWN_REP_MSG_RDY | DP_UP_REQ_MSG_RDY);

Above we check if there's anything to ack and bail out, and now this clears the bits but writes them anyway.

I think the handled parameter was problematic before, but now it's even more convoluted. What does it indicate? It used to mean you need to ack if it's set, but now it's something different. This function is getting very difficult to use correctly.

BR,
Jani.

Quoted 9 lines

drivers/gpu/drm/i915/display/intel_dp.c +1 -0
+
 		if (!intel_dp_ack_sink_irq_esi(intel_dp, ack))
 			drm_dbg_kms(&i915->drm, "Failed to ack ESI\n");
 	}
drivers/gpu/drm/nouveau/dispnv50/disp.c +0 -0
index edcb2529b402..e905987104ed 100644

drivers/gpu/drm/nouveau/dispnv50/disp.c nv50_mstm_service(struct nouveau_drm *drm, · line 1342

Quoted 7 lines

drivers/gpu/drm/nouveau/dispnv50/disp.c +2 -0

@@ -1336,6 +1336,8 @@ nv50_mstm_service(struct nouveau_drm *drm,
 		if (!handled)
 			break;
 
+		/* MSG_RDY ack is done in drm*/
+		esi[1] &= ~(DP_DOWN_REP_MSG_RDY | DP_UP_REQ_MSG_RDY);
 		rc = drm_dp_dpcd_write(aux, DP_SINK_COUNT_ESI + 1, &esi[1],
 				       3);

Same here, this acks even if it's already been acked.

Quoted 1 line

drivers/gpu/drm/nouveau/dispnv50/disp.c +0 -0
 		if (rc != 3) {

Signature · 1 line

Jani Nikula, Intel Open Source Graphics Center

Lin, Wayne

author

2023-04-18 09:42 UTC #3 lore raw

[Public]

Hi Jani Nikula,

Appreciate your time and feedback! Will adjust the patch. Some comments inline.

Quoted 83 lines

-----Original Message-----
From: Jani Nikula <[email protected]>
Sent: Tuesday, April 18, 2023 4:53 PM
To: Lin, Wayne <[email protected]>; [email protected]; [email protected]
Cc: [email protected]; [email protected]; [email protected]; Wentland, Harry <[email protected]>; Zuo, Jerry <[email protected]>; Lin, Wayne <[email protected]>; [email protected]
Subject: Re: [PATCH] drm/dp_mst: Clear MSG_RDY flag before sending new message
Quoted 10 lines · Wayne Lin
[Why & How]
The sequence for collecting down_reply/up_request 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.
While assembling partial reply packets, reading out DPCD DOWN_REP Sideband MSG buffer + clearing DOWN_REP_MSG_RDY flag should be
wrapped
Quoted 7 lines
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 ack 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.
Quoted 8 lines
In drm_dp_mst_hpd_irq(), we don't clear MSG_RDY flag before caliing drm_dp_mst_kick_tx(). Fix that.
Signed-off-by Wayne Lin <[email protected]>
Cc [email protected]
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  2 ++
drivers/gpu/drm/display/drm_dp_mst_topology.c | 22
+++++++++++++++++++
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c static void dm_handle_mst_sideband_msg(struct
Quoted 10 lines
 drivers/gpu/drm/i915/display/intel_dp.c       |  3 +++
 drivers/gpu/drm/nouveau/dispnv50/disp.c       |  2 ++
 4 files changed, 29 insertions(+)
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +0 -0
index 77277d90b6e2..5313a5656598 100644
@@ -3166,6 +3166,8 @@ static void dm_handle_mst_sideband_msg(struct
amdgpu_dm_connector *aconnector)
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c static void dm_handle_mst_sideband_msg(struct amdgpu_dm_connector *aconnector)
Quoted 5 lines
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +2 -0
 			for (retry = 0; retry < 3; retry++) {
 				uint8_t wret;
 
+				/* MSG_RDY ack is done in drm*/
+				esi[1] &= ~(DP_DOWN_REP_MSG_RDY |
DP_UP_REQ_MSG_RDY);
Why do the masking within the retry loop?
Quoted 9 lines
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +0 -0
 				wret = drm_dp_dpcd_write(
 					&aconnector->dm_dp_aux.aux,
 					dpcd_addr + 1,
drivers/gpu/drm/display/drm_dp_mst_topology.c +0 -0
index 51a46689cda7..02aad713c67c 100644
drivers/gpu/drm/display/drm_dp_mst_topology.c int drm_dp_mst_hpd_irq(struct · line 4059
Quoted 6 lines
drivers/gpu/drm/display/drm_dp_mst_topology.c +3 -0
@@ -4054,6 +4054,9 @@ int drm_dp_mst_hpd_irq(struct
 drm_dp_mst_topology_mgr *mgr, u8 *esi, bool *handl  {
 	int ret = 0;
 	int sc;
+	const int tosend = 1;
+	int retries = 0;
+	u8 buf = 0;
All of these should be in tighter scope.
drivers/gpu/drm/display/drm_dp_mst_topology.c int drm_dp_mst_hpd_irq(struct
Quoted 4 lines
drivers/gpu/drm/display/drm_dp_mst_topology.c +0 -0
 	*handled = false;
 	sc = DP_GET_SINK_COUNT(esi[0]);
 
@@ -4072,6 +4075,25 @@ int drm_dp_mst_hpd_irq(struct
drm_dp_mst_topology_mgr *mgr, u8 *esi, bool *handl

Quoted 8 lines

drivers/gpu/drm/display/drm_dp_mst_topology.c int drm_dp_mst_hpd_irq(struct drm_dp_mst_topology_mgr *mgr, u8 *esi, bool *handl · line 4078
Quoted 4 lines
drivers/gpu/drm/display/drm_dp_mst_topology.c +1 -0
 		*handled = true;
 	}
 
+	if (*handled) {
That should check for DP_DOWN_REP_MSG_RDY and DP_UP_REQ_MSG_RDY only, right? If those are not set, we didn't do anything with them, and should not ack.

Right. I was thinking the sink count change will accompany the CSN up request message. I'll change it to be more clear. Thanks.

Quoted 53 lines

drivers/gpu/drm/display/drm_dp_mst_topology.c int drm_dp_mst_hpd_irq(struct · line 4079
Quoted 2 lines
drivers/gpu/drm/display/drm_dp_mst_topology.c +1 -0
+		buf = esi[1] & (DP_DOWN_REP_MSG_RDY |
DP_UP_REQ_MSG_RDY);
drivers/gpu/drm/display/drm_dp_mst_topology.c int drm_dp_mst_hpd_irq(struct drm_dp_mst_topology_mgr *mgr, u8 *esi, bool *handl · line 4085
Quoted 3 lines
drivers/gpu/drm/display/drm_dp_mst_topology.c +3 -0
+		do {
+			ret = drm_dp_dpcd_write(mgr->aux,
+
DP_DEVICE_SERVICE_IRQ_VECTOR_ESI0,
drivers/gpu/drm/display/drm_dp_mst_topology.c int drm_dp_mst_hpd_irq(struct drm_dp_mst_topology_mgr *mgr, u8 *esi, bool *handl · line 4084
Quoted 2 lines
drivers/gpu/drm/display/drm_dp_mst_topology.c +2 -0
+						&buf,
+						tosend);
We should probably have a helper function to do the acking, similar to intel_dp_ack_sink_irq_esi(), which could be used both by this function and the drivers.
drivers/gpu/drm/display/drm_dp_mst_topology.c int drm_dp_mst_hpd_irq(struct drm_dp_mst_topology_mgr *mgr, u8 *esi, bool *handl · line 4090
Quoted 6 lines
drivers/gpu/drm/display/drm_dp_mst_topology.c +6 -0
+
+			if (ret == tosend)
+				break;
+
+			retries++;
+		} while (retries < 5);
Please don't use a do-while when a for loop is sufficient.
	for (tries = 0; tries < 5; tries++)
and it's obvious at a glance how many times at most this runs. Not so with a do-while where you count *re-tries*. Again, would be nice to abstract this away in a helper function.
drivers/gpu/drm/display/drm_dp_mst_topology.c int drm_dp_mst_hpd_irq(struct drm_dp_mst_topology_mgr *mgr, u8 *esi, bool *handl
Quoted 3 lines
drivers/gpu/drm/display/drm_dp_mst_topology.c +3 -0
+
+		if (ret != tosend)
+			drm_dbg_kms(mgr->dev, "failed to write dpcd
0x%x\n",
drivers/gpu/drm/i915/display/intel_dp.c intel_dp_check_mst_status(struct intel_dp
Quoted 12 lines
drivers/gpu/drm/display/drm_dp_mst_topology.c +3 -0
+				    DP_DEVICE_SERVICE_IRQ_VECTOR_ESI0);
+	}
+
 	drm_dp_mst_kick_tx(mgr);
 	return ret;
 }
drivers/gpu/drm/i915/display/intel_dp.c +0 -0
index bf80f296a8fd..abec3de38b66 100644
@@ -3939,6 +3939,9 @@ intel_dp_check_mst_status(struct intel_dp
*intel_dp)
drivers/gpu/drm/i915/display/intel_dp.c intel_dp_check_mst_status(struct intel_dp *intel_dp)
Quoted 5 lines
drivers/gpu/drm/i915/display/intel_dp.c +2 -0
 		if (!memchr_inv(ack, 0, sizeof(ack)))
 			break;
 
+		/* MSG_RDY ack is done in drm*/
+		ack[1] &= ~(DP_DOWN_REP_MSG_RDY |
DP_UP_REQ_MSG_RDY);

Quoted 7 lines

Above we check if there's anything to ack and bail out, and now this clears the bits but writes them anyway.
I think the handled parameter was problematic before, but now it's even more convoluted. What does it indicate? It used to mean you need to ack if it's set, but now it's something different. This function is getting very difficult to use correctly.

My plan was to ack message events within drm_dp_mst_hpd_irq() since the events are handled there. There are still CP_IRQ and LINK_STATUS_CHANGED events above get handled in intel_dp_check_mst_status(), so I intended to mask DP_DOWN_REP_MSG_RDY/DP_UP_REQ_MSG_RDY, and ack CP_IRQ/LINK_STATUS_CHANGED here.

Quoted 24 lines

BR,
Jani.
Quoted 10 lines
drivers/gpu/drm/i915/display/intel_dp.c +1 -0
+
 		if (!intel_dp_ack_sink_irq_esi(intel_dp, ack))
 			drm_dbg_kms(&i915->drm, "Failed to ack ESI\n");
 	}
drivers/gpu/drm/nouveau/dispnv50/disp.c +0 -0
index edcb2529b402..e905987104ed 100644
drivers/gpu/drm/nouveau/dispnv50/disp.c nv50_mstm_service(struct nouveau_drm *drm, · line 1340
Quoted 5 lines
drivers/gpu/drm/nouveau/dispnv50/disp.c +2 -0
@@ -1336,6 +1336,8 @@ nv50_mstm_service(struct nouveau_drm *drm,
 		if (!handled)
 			break;
 
+		/* MSG_RDY ack is done in drm*/
+		esi[1] &= ~(DP_DOWN_REP_MSG_RDY |
DP_UP_REQ_MSG_RDY);
drivers/gpu/drm/nouveau/dispnv50/disp.c nv50_mstm_service(struct nouveau_drm *drm, · line 1341
Quoted 1 line
drivers/gpu/drm/nouveau/dispnv50/disp.c +0 -0
 		rc = drm_dp_dpcd_write(aux, DP_SINK_COUNT_ESI + 1,
&esi[1],

Quoted 8 lines

drivers/gpu/drm/nouveau/dispnv50/disp.c nv50_mstm_service(struct nouveau_drm *drm, · line 1342
Quoted 1 line
drivers/gpu/drm/nouveau/dispnv50/disp.c +0 -0
 				       3);
Same here, this acks even if it's already been acked.
drivers/gpu/drm/nouveau/dispnv50/disp.c nv50_mstm_service(struct nouveau_drm *drm, · line 1343
Quoted 1 line
drivers/gpu/drm/nouveau/dispnv50/disp.c +0 -0
 		if (rc != 3) {
--
Jani Nikula, Intel Open Source Graphics Center

--
Regards,
Wayne Lin

Jani Nikula

2023-04-18 11:58 UTC #4 lore raw

Quoted 162 lines · Wayne

[Public]
Hi Jani Nikula,
Appreciate your time and feedback! Will adjust the patch. Some comments inline.
Quoted 83 lines
-----Original Message-----
From: Jani Nikula <[email protected]>
Sent: Tuesday, April 18, 2023 4:53 PM
To: Lin, Wayne <[email protected]>; [email protected]; [email protected]
Cc: [email protected]; [email protected]; [email protected]; Wentland, Harry <[email protected]>; Zuo, Jerry <[email protected]>; Lin, Wayne <[email protected]>; [email protected]
Subject: Re: [PATCH] drm/dp_mst: Clear MSG_RDY flag before sending new message
Quoted 10 lines · Wayne Lin
[Why & How]
The sequence for collecting down_reply/up_request 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.
While assembling partial reply packets, reading out DPCD DOWN_REP Sideband MSG buffer + clearing DOWN_REP_MSG_RDY flag should be
wrapped
Quoted 7 lines
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 ack 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.
Quoted 8 lines
In drm_dp_mst_hpd_irq(), we don't clear MSG_RDY flag before caliing drm_dp_mst_kick_tx(). Fix that.
Signed-off-by Wayne Lin <[email protected]>
Cc [email protected]
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  2 ++
drivers/gpu/drm/display/drm_dp_mst_topology.c | 22
+++++++++++++++++++
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c static void dm_handle_mst_sideband_msg(struct
Quoted 10 lines
 drivers/gpu/drm/i915/display/intel_dp.c       |  3 +++
 drivers/gpu/drm/nouveau/dispnv50/disp.c       |  2 ++
 4 files changed, 29 insertions(+)
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +0 -0
index 77277d90b6e2..5313a5656598 100644
@@ -3166,6 +3166,8 @@ static void dm_handle_mst_sideband_msg(struct
amdgpu_dm_connector *aconnector)
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c static void dm_handle_mst_sideband_msg(struct amdgpu_dm_connector *aconnector)
Quoted 5 lines
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +2 -0
 			for (retry = 0; retry < 3; retry++) {
 				uint8_t wret;
 
+				/* MSG_RDY ack is done in drm*/
+				esi[1] &= ~(DP_DOWN_REP_MSG_RDY |
DP_UP_REQ_MSG_RDY);
Why do the masking within the retry loop?
Quoted 9 lines
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +0 -0
 				wret = drm_dp_dpcd_write(
 					&aconnector->dm_dp_aux.aux,
 					dpcd_addr + 1,
drivers/gpu/drm/display/drm_dp_mst_topology.c +0 -0
index 51a46689cda7..02aad713c67c 100644
drivers/gpu/drm/display/drm_dp_mst_topology.c int drm_dp_mst_hpd_irq(struct · line 4059
Quoted 6 lines
drivers/gpu/drm/display/drm_dp_mst_topology.c +3 -0
@@ -4054,6 +4054,9 @@ int drm_dp_mst_hpd_irq(struct
 drm_dp_mst_topology_mgr *mgr, u8 *esi, bool *handl  {
 	int ret = 0;
 	int sc;
+	const int tosend = 1;
+	int retries = 0;
+	u8 buf = 0;
All of these should be in tighter scope.
drivers/gpu/drm/display/drm_dp_mst_topology.c int drm_dp_mst_hpd_irq(struct
Quoted 4 lines
drivers/gpu/drm/display/drm_dp_mst_topology.c +0 -0
 	*handled = false;
 	sc = DP_GET_SINK_COUNT(esi[0]);
 
@@ -4072,6 +4075,25 @@ int drm_dp_mst_hpd_irq(struct
drm_dp_mst_topology_mgr *mgr, u8 *esi, bool *handl
Quoted 8 lines
drivers/gpu/drm/display/drm_dp_mst_topology.c int drm_dp_mst_hpd_irq(struct drm_dp_mst_topology_mgr *mgr, u8 *esi, bool *handl · line 4078
Quoted 4 lines
drivers/gpu/drm/display/drm_dp_mst_topology.c +1 -0
 		*handled = true;
 	}
 
+	if (*handled) {
That should check for DP_DOWN_REP_MSG_RDY and DP_UP_REQ_MSG_RDY only, right? If those are not set, we didn't do anything with them, and should not ack.
Right. I was thinking the sink count change will accompany the CSN up request message. I'll change it to be more clear. Thanks.
Quoted 53 lines
drivers/gpu/drm/display/drm_dp_mst_topology.c int drm_dp_mst_hpd_irq(struct · line 4079
Quoted 2 lines
drivers/gpu/drm/display/drm_dp_mst_topology.c +1 -0
+		buf = esi[1] & (DP_DOWN_REP_MSG_RDY |
DP_UP_REQ_MSG_RDY);
drivers/gpu/drm/display/drm_dp_mst_topology.c int drm_dp_mst_hpd_irq(struct drm_dp_mst_topology_mgr *mgr, u8 *esi, bool *handl · line 4085
Quoted 3 lines
drivers/gpu/drm/display/drm_dp_mst_topology.c +3 -0
+		do {
+			ret = drm_dp_dpcd_write(mgr->aux,
+
DP_DEVICE_SERVICE_IRQ_VECTOR_ESI0,
drivers/gpu/drm/display/drm_dp_mst_topology.c int drm_dp_mst_hpd_irq(struct drm_dp_mst_topology_mgr *mgr, u8 *esi, bool *handl · line 4084
Quoted 2 lines
drivers/gpu/drm/display/drm_dp_mst_topology.c +2 -0
+						&buf,
+						tosend);
We should probably have a helper function to do the acking, similar to intel_dp_ack_sink_irq_esi(), which could be used both by this function and the drivers.
drivers/gpu/drm/display/drm_dp_mst_topology.c int drm_dp_mst_hpd_irq(struct drm_dp_mst_topology_mgr *mgr, u8 *esi, bool *handl · line 4090
Quoted 6 lines
drivers/gpu/drm/display/drm_dp_mst_topology.c +6 -0
+
+			if (ret == tosend)
+				break;
+
+			retries++;
+		} while (retries < 5);
Please don't use a do-while when a for loop is sufficient.
	for (tries = 0; tries < 5; tries++)
and it's obvious at a glance how many times at most this runs. Not so with a do-while where you count *re-tries*. Again, would be nice to abstract this away in a helper function.
drivers/gpu/drm/display/drm_dp_mst_topology.c int drm_dp_mst_hpd_irq(struct drm_dp_mst_topology_mgr *mgr, u8 *esi, bool *handl
Quoted 3 lines
drivers/gpu/drm/display/drm_dp_mst_topology.c +3 -0
+
+		if (ret != tosend)
+			drm_dbg_kms(mgr->dev, "failed to write dpcd
0x%x\n",
drivers/gpu/drm/i915/display/intel_dp.c intel_dp_check_mst_status(struct intel_dp
Quoted 12 lines
drivers/gpu/drm/display/drm_dp_mst_topology.c +3 -0
+				    DP_DEVICE_SERVICE_IRQ_VECTOR_ESI0);
+	}
+
 	drm_dp_mst_kick_tx(mgr);
 	return ret;
 }
drivers/gpu/drm/i915/display/intel_dp.c +0 -0
index bf80f296a8fd..abec3de38b66 100644
@@ -3939,6 +3939,9 @@ intel_dp_check_mst_status(struct intel_dp
*intel_dp)
drivers/gpu/drm/i915/display/intel_dp.c intel_dp_check_mst_status(struct intel_dp *intel_dp)
Quoted 5 lines
drivers/gpu/drm/i915/display/intel_dp.c +2 -0
 		if (!memchr_inv(ack, 0, sizeof(ack)))
 			break;
 
+		/* MSG_RDY ack is done in drm*/
+		ack[1] &= ~(DP_DOWN_REP_MSG_RDY |
DP_UP_REQ_MSG_RDY);
Quoted 7 lines
Above we check if there's anything to ack and bail out, and now this clears the bits but writes them anyway.
I think the handled parameter was problematic before, but now it's even more convoluted. What does it indicate? It used to mean you need to ack if it's set, but now it's something different. This function is getting very difficult to use correctly.

Quoted 5 lines

My plan was to ack message events within drm_dp_mst_hpd_irq() since the events are handled there. There are still CP_IRQ and LINK_STATUS_CHANGED events above get handled in intel_dp_check_mst_status(), so I intended to mask DP_DOWN_REP_MSG_RDY/DP_UP_REQ_MSG_RDY, and ack CP_IRQ/LINK_STATUS_CHANGED here.

I get it, but if DP_DOWN_REP_MSG_RDY or DP_UP_REQ_MSG_RDY were the only events to ack, and they were already acked in drm_dp_mst_hpd_irq(), we should not do an extra "nop" ack.

The caller of drm_dp_mst_hpd_irq() needs to be able to conveniently figure out what to ack, and what to not ack. And without duplicating the logic within drm_dp_mst_hpd_irq().

BR,
Jani.

Quoted 36 lines

Quoted 24 lines
BR,
Jani.
Quoted 10 lines
drivers/gpu/drm/i915/display/intel_dp.c +1 -0
+
 		if (!intel_dp_ack_sink_irq_esi(intel_dp, ack))
 			drm_dbg_kms(&i915->drm, "Failed to ack ESI\n");
 	}
drivers/gpu/drm/nouveau/dispnv50/disp.c +0 -0
index edcb2529b402..e905987104ed 100644
drivers/gpu/drm/nouveau/dispnv50/disp.c nv50_mstm_service(struct nouveau_drm *drm, · line 1340
Quoted 5 lines
drivers/gpu/drm/nouveau/dispnv50/disp.c +2 -0
@@ -1336,6 +1336,8 @@ nv50_mstm_service(struct nouveau_drm *drm,
 		if (!handled)
 			break;
 
+		/* MSG_RDY ack is done in drm*/
+		esi[1] &= ~(DP_DOWN_REP_MSG_RDY |
DP_UP_REQ_MSG_RDY);
drivers/gpu/drm/nouveau/dispnv50/disp.c nv50_mstm_service(struct nouveau_drm *drm, · line 1341
Quoted 1 line
drivers/gpu/drm/nouveau/dispnv50/disp.c +0 -0
 		rc = drm_dp_dpcd_write(aux, DP_SINK_COUNT_ESI + 1,
&esi[1],
Quoted 8 lines
drivers/gpu/drm/nouveau/dispnv50/disp.c nv50_mstm_service(struct nouveau_drm *drm, · line 1342
Quoted 1 line
drivers/gpu/drm/nouveau/dispnv50/disp.c +0 -0
 				       3);
Same here, this acks even if it's already been acked.
drivers/gpu/drm/nouveau/dispnv50/disp.c nv50_mstm_service(struct nouveau_drm *drm, · line 1343
Quoted 1 line
drivers/gpu/drm/nouveau/dispnv50/disp.c +0 -0
 		if (rc != 3) {
--
Jani Nikula, Intel Open Source Graphics Center
--
Regards,
Wayne Lin

Signature · 1 line

Jani Nikula, Intel Open Source Graphics Center

Patchwork

2023-04-18 11:16 UTC #5 lore raw

[Intel-gfx] ✓ Fi.CI.BAT: success for drm/dp_mst: Clear MSG_RDY flag before sending new message (rev2)

In reply to Wayne Lin

== Series Details ==

Series: drm/dp_mst: Clear MSG_RDY flag before sending new message (rev2)
URL : https://patchwork.freedesktop.org/series/116623/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_13025 -> Patchwork_116623v2
====================================================

Summary
-------

**SUCCESS**

No regressions found.

External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116623v2/index.html

Participating hosts (37 -> 37)
------------------------------

Additional (1): bat-adlm-1
Missing (1): fi-snb-2520m

Known issues
------------

Here are the changes found in Patchwork_116623v2 that come from known issues:

### IGT changes ###

#### Issues hit ####

* igt@debugfs_test@basic-hwmon:
- bat-adlm-1: NOTRUN -> [SKIP][1] ([i915#7456])
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116623v2/bat-adlm-1/igt@[email protected]

* igt@fbdev@eof:
- bat-adlm-1: NOTRUN -> [SKIP][2] ([i915#2582]) +4 similar issues
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116623v2/bat-adlm-1/igt@[email protected]

* igt@gem_exec_suspend@basic-s0@smem:
- bat-rpls-2: NOTRUN -> [ABORT][3] ([i915#6687])
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116623v2/bat-rpls-2/igt@gem_exec_suspend@[email protected]

* igt@gem_lmem_swapping@parallel-random-engines:
- bat-adlm-1: NOTRUN -> [SKIP][4] ([i915#4613]) +3 similar issues
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116623v2/bat-adlm-1/igt@[email protected]

* igt@gem_tiled_pread_basic:
- bat-adlm-1: NOTRUN -> [SKIP][5] ([i915#3282])
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116623v2/bat-adlm-1/igt@gem_tiled_pread_basic.html

* igt@i915_pm_backlight@basic-brightness:
- bat-adlm-1: NOTRUN -> [SKIP][6] ([i915#7561])
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116623v2/bat-adlm-1/igt@[email protected]

* igt@i915_selftest@live@mman:
- bat-rpls-2: [PASS][7] -> [TIMEOUT][8] ([i915#6794])
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13025/bat-rpls-2/igt@i915_selftest@[email protected]
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116623v2/bat-rpls-2/igt@i915_selftest@[email protected]

* igt@kms_chamelium_frames@dp-crc-fast:
- bat-adlm-1: NOTRUN -> [SKIP][9] ([i915#7828]) +8 similar issues
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116623v2/bat-adlm-1/igt@[email protected]

* igt@kms_chamelium_hpd@common-hpd-after-suspend:
- bat-jsl-1: NOTRUN -> [SKIP][10] ([i915#7828])
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116623v2/bat-jsl-1/igt@[email protected]
- bat-rpls-1: NOTRUN -> [SKIP][11] ([i915#7828])
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116623v2/bat-rpls-1/igt@[email protected]

* igt@kms_cursor_legacy@basic-flip-after-cursor-varying-size:
- bat-adlm-1: NOTRUN -> [SKIP][12] ([i915#1845]) +15 similar issues
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116623v2/bat-adlm-1/igt@[email protected]

* igt@kms_flip@basic-flip-vs-wf_vblank@c-hdmi-a2:
- fi-bsw-n3050: [PASS][13] -> [FAIL][14] ([i915#2122])
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13025/fi-bsw-n3050/igt@kms_flip@[email protected]
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116623v2/fi-bsw-n3050/igt@kms_flip@[email protected]

* igt@kms_flip@basic-plain-flip:
- bat-adlm-1: NOTRUN -> [SKIP][15] ([i915#3637]) +3 similar issues
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116623v2/bat-adlm-1/igt@[email protected]

* igt@kms_force_connector_basic@force-load-detect:
- bat-adlm-1: NOTRUN -> [SKIP][16] ([fdo#109285])
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116623v2/bat-adlm-1/igt@[email protected]

* igt@kms_frontbuffer_tracking@basic:
- bat-adlm-1: NOTRUN -> [SKIP][17] ([i915#1849])
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116623v2/bat-adlm-1/igt@[email protected]

* igt@kms_pipe_crc_basic@suspend-read-crc:
- bat-rpls-1: NOTRUN -> [SKIP][18] ([i915#1845])
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116623v2/bat-rpls-1/igt@[email protected]

* igt@kms_psr@cursor_plane_move:
- bat-adlm-1: NOTRUN -> [SKIP][19] ([i915#1072]) +3 similar issues
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116623v2/bat-adlm-1/igt@kms_psr@cursor_plane_move.html

* igt@kms_setmode@basic-clone-single-crtc:
- bat-adlm-1: NOTRUN -> [SKIP][20] ([i915#3555])
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116623v2/bat-adlm-1/igt@[email protected]

* igt@prime_vgem@basic-fence-flip:
- bat-adlm-1: NOTRUN -> [SKIP][21] ([i915#1845] / [i915#3708])
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116623v2/bat-adlm-1/igt@[email protected]

* igt@prime_vgem@basic-write:
- bat-adlm-1: NOTRUN -> [SKIP][22] ([i915#3708]) +3 similar issues
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116623v2/bat-adlm-1/igt@[email protected]

#### Possible fixes ####

* igt@i915_selftest@live@gt_heartbeat:
- fi-apl-guc: [DMESG-FAIL][23] ([i915#5334]) -> [PASS][24]
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13025/fi-apl-guc/igt@i915_selftest@live@gt_heartbeat.html
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116623v2/fi-apl-guc/igt@i915_selftest@live@gt_heartbeat.html

* igt@i915_selftest@live@migrate:
- bat-dg2-11: [DMESG-WARN][25] ([i915#7699]) -> [PASS][26]
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13025/bat-dg2-11/igt@i915_selftest@[email protected]
[26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116623v2/bat-dg2-11/igt@i915_selftest@[email protected]

* igt@i915_selftest@live@reset:
- bat-rpls-1: [ABORT][27] ([i915#4983]) -> [PASS][28]
[27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13025/bat-rpls-1/igt@i915_selftest@[email protected]
[28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116623v2/bat-rpls-1/igt@i915_selftest@[email protected]

* igt@i915_suspend@basic-s3-without-i915:
- bat-rpls-2: [ABORT][29] ([i915#7978]) -> [PASS][30]
[29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13025/bat-rpls-2/igt@[email protected]
[30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116623v2/bat-rpls-2/igt@[email protected]

Build changes
-------------

* Linux: CI_DRM_13025 -> Patchwork_116623v2

CI-20190529: 20190529
CI_DRM_13025: aedb908bbfc13d3d66f2715709f26f02283aef5a @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_7258: ad2eb276eda849b7a7985229009a816c7608186c @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
Patchwork_116623v2: aedb908bbfc13d3d66f2715709f26f02283aef5a @ git://anongit.freedesktop.org/gfx-ci/linux

### Linux commits

becb01b3e5ec drm/dp_mst: Clear MSG_RDY flag before sending new message

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116623v2/index.html

Ville Syrjälä

2023-04-18 14:01 UTC #6 lore raw

In reply to Wayne Lin

Quoted 63 lines · Wayne Lin

[Why & How]
The sequence for collecting down_reply/up_request 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.
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 ack 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.
In drm_dp_mst_hpd_irq(), we don't clear MSG_RDY flag before caliing drm_dp_mst_kick_tx(). Fix that.
Signed-off-by Wayne Lin <[email protected]>
Cc [email protected]
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  2 ++
 drivers/gpu/drm/display/drm_dp_mst_topology.c | 22 +++++++++++++++++++
 drivers/gpu/drm/i915/display/intel_dp.c       |  3 +++
 drivers/gpu/drm/nouveau/dispnv50/disp.c       |  2 ++
 4 files changed, 29 insertions(+)
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +2 -0
index 77277d90b6e2..5313a5656598 100644
@@ -3166,6 +3166,8 @@ static void dm_handle_mst_sideband_msg(struct amdgpu_dm_connector *aconnector)
 			for (retry = 0; retry < 3; retry++) {
 				uint8_t wret;
 
+				/* MSG_RDY ack is done in drm*/
+				esi[1] &= ~(DP_DOWN_REP_MSG_RDY | DP_UP_REQ_MSG_RDY);
 				wret = drm_dp_dpcd_write(
 					&aconnector->dm_dp_aux.aux,
 					dpcd_addr + 1,
drivers/gpu/drm/display/drm_dp_mst_topology.c +6 -0
index 51a46689cda7..02aad713c67c 100644
@@ -4054,6 +4054,9 @@ int drm_dp_mst_hpd_irq(struct drm_dp_mst_topology_mgr *mgr, u8 *esi, bool *handl
 {
 	int ret = 0;
 	int sc;
+	const int tosend = 1;
+	int retries = 0;
+	u8 buf = 0;
 	*handled = false;
 	sc = DP_GET_SINK_COUNT(esi[0]);
 
@@ -4072,6 +4075,25 @@ int drm_dp_mst_hpd_irq(struct drm_dp_mst_topology_mgr *mgr, u8 *esi, bool *handl
 		*handled = true;
 	}
 
+	if (*handled) {
+		buf = esi[1] & (DP_DOWN_REP_MSG_RDY | DP_UP_REQ_MSG_RDY);
+		do {

drivers/gpu/drm/display/drm_dp_mst_topology.c int drm_dp_mst_hpd_irq(struct drm_dp_mst_topology_mgr *mgr, u8 *esi, bool *handl · line 4090

Quoted 10 lines

drivers/gpu/drm/display/drm_dp_mst_topology.c +10 -0

+			ret = drm_dp_dpcd_write(mgr->aux,
+						DP_DEVICE_SERVICE_IRQ_VECTOR_ESI0,
+						&buf,
+						tosend);
+
+			if (ret == tosend)
+				break;
+
+			retries++;
+		} while (retries < 5);

What's with this magic retry loop?

Not sure I like the whole thing though. Splitting the irq ack semi-randomly between driver vs. multiple helpers doesn't feel great to me.

As a whole the HPD_IRQ handling is a total mess atm. At some point I was trying to sketch something a bit better for it. The approach I was thinking was something along the lines of:

u8 vector[...];
drm_dp_read_irq_vector(vector);
... handle all irqs/etc., calling suitable helpers as needed drm_dp_clear_irq_vector(vector);

And I was also thinking that this drm_dp_*_irq_vector() stuff would always use the ESI layout, converting as needed from/to the old layout for pre-1.2 (or whatever the cutoff was) devices. That way drivers would just need the one codepath.

Quoted 38 lines

drivers/gpu/drm/display/drm_dp_mst_topology.c +6 -0

+
+		if (ret != tosend)
+			drm_dbg_kms(mgr->dev, "failed to write dpcd 0x%x\n",
+				    DP_DEVICE_SERVICE_IRQ_VECTOR_ESI0);
+	}
+
 	drm_dp_mst_kick_tx(mgr);
 	return ret;
 }

drivers/gpu/drm/i915/display/intel_dp.c +3 -0

index bf80f296a8fd..abec3de38b66 100644
@@ -3939,6 +3939,9 @@ intel_dp_check_mst_status(struct intel_dp *intel_dp)
 		if (!memchr_inv(ack, 0, sizeof(ack)))
 			break;
 
+		/* MSG_RDY ack is done in drm*/
+		ack[1] &= ~(DP_DOWN_REP_MSG_RDY | DP_UP_REQ_MSG_RDY);
+
 		if (!intel_dp_ack_sink_irq_esi(intel_dp, ack))
 			drm_dbg_kms(&i915->drm, "Failed to ack ESI\n");
 	}

drivers/gpu/drm/nouveau/dispnv50/disp.c +2 -0

index edcb2529b402..e905987104ed 100644
@@ -1336,6 +1336,8 @@ nv50_mstm_service(struct nouveau_drm *drm,
 		if (!handled)
 			break;
 
+		/* MSG_RDY ack is done in drm*/
+		esi[1] &= ~(DP_DOWN_REP_MSG_RDY | DP_UP_REQ_MSG_RDY);
 		rc = drm_dp_dpcd_write(aux, DP_SINK_COUNT_ESI + 1, &esi[1],
 				       3);
 		if (rc != 3) {

Signature · 1 line

2.37.3

Signature · 2 lines

Ville Syrjälä
Intel

Lin, Wayne

author

2023-04-21 13:13 UTC #7 lore raw

[Public]

Much appreciated, Ville and Jani!

To tackle this MST message ack event now, probably I could just pull out the drm_dp_mst_kick_tx() out of drm_dp_mst_hpd_irq() and make it the second step function to handle mst hpd irq? Would like to know your thoughts : )

Again, thanks for your time!

Regards,
Wayne Lin

Quoted 163 lines

-----Original Message-----
From: Ville Syrjälä <[email protected]>
Sent: Tuesday, April 18, 2023 10:01 PM
To: Lin, Wayne <[email protected]>
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; Wentland, Harry <[email protected]>; Zuo, Jerry <[email protected]>; [email protected]
Subject: Re: [PATCH] drm/dp_mst: Clear MSG_RDY flag before sending new message
Quoted 10 lines · Wayne Lin
[Why & How]
The sequence for collecting down_reply/up_request 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.
While assembling partial reply packets, reading out DPCD DOWN_REP Sideband MSG buffer + clearing DOWN_REP_MSG_RDY flag should be
wrapped
Quoted 7 lines
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 ack 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.
Quoted 8 lines
In drm_dp_mst_hpd_irq(), we don't clear MSG_RDY flag before caliing drm_dp_mst_kick_tx(). Fix that.
Signed-off-by Wayne Lin <[email protected]>
Cc [email protected]
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  2 ++
drivers/gpu/drm/display/drm_dp_mst_topology.c | 22
+++++++++++++++++++
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c static void dm_handle_mst_sideband_msg(struct
Quoted 10 lines
 drivers/gpu/drm/i915/display/intel_dp.c       |  3 +++
 drivers/gpu/drm/nouveau/dispnv50/disp.c       |  2 ++
 4 files changed, 29 insertions(+)
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +0 -0
index 77277d90b6e2..5313a5656598 100644
@@ -3166,6 +3166,8 @@ static void dm_handle_mst_sideband_msg(struct
amdgpu_dm_connector *aconnector)
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c static void dm_handle_mst_sideband_msg(struct amdgpu_dm_connector *aconnector)
Quoted 5 lines
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +2 -0
 			for (retry = 0; retry < 3; retry++) {
 				uint8_t wret;
 
+				/* MSG_RDY ack is done in drm*/
+				esi[1] &= ~(DP_DOWN_REP_MSG_RDY |
DP_UP_REQ_MSG_RDY);
drivers/gpu/drm/display/drm_dp_mst_topology.c int drm_dp_mst_hpd_irq(struct
Quoted 19 lines
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +0 -0
 				wret = drm_dp_dpcd_write(
 					&aconnector->dm_dp_aux.aux,
 					dpcd_addr + 1,
drivers/gpu/drm/display/drm_dp_mst_topology.c +3 -0
index 51a46689cda7..02aad713c67c 100644
@@ -4054,6 +4054,9 @@ int drm_dp_mst_hpd_irq(struct
 drm_dp_mst_topology_mgr *mgr, u8 *esi, bool *handl  {
 	int ret = 0;
 	int sc;
+	const int tosend = 1;
+	int retries = 0;
+	u8 buf = 0;
 	*handled = false;
 	sc = DP_GET_SINK_COUNT(esi[0]);
 
@@ -4072,6 +4075,25 @@ int drm_dp_mst_hpd_irq(struct
drm_dp_mst_topology_mgr *mgr, u8 *esi, bool *handl
drivers/gpu/drm/display/drm_dp_mst_topology.c int drm_dp_mst_hpd_irq(struct drm_dp_mst_topology_mgr *mgr, u8 *esi, bool *handl
Quoted 5 lines
drivers/gpu/drm/display/drm_dp_mst_topology.c +2 -0
 		*handled = true;
 	}
 
+	if (*handled) {
+		buf = esi[1] & (DP_DOWN_REP_MSG_RDY |
DP_UP_REQ_MSG_RDY);
drivers/gpu/drm/display/drm_dp_mst_topology.c int drm_dp_mst_hpd_irq(struct drm_dp_mst_topology_mgr *mgr, u8 *esi, bool *handl · line 4085
Quoted 3 lines
drivers/gpu/drm/display/drm_dp_mst_topology.c +3 -0
+		do {
+			ret = drm_dp_dpcd_write(mgr->aux,
+
DP_DEVICE_SERVICE_IRQ_VECTOR_ESI0,
drivers/gpu/drm/display/drm_dp_mst_topology.c int drm_dp_mst_hpd_irq(struct drm_dp_mst_topology_mgr *mgr, u8 *esi, bool *handl · line 4090
Quoted 8 lines
drivers/gpu/drm/display/drm_dp_mst_topology.c +8 -0
+						&buf,
+						tosend);
+
+			if (ret == tosend)
+				break;
+
+			retries++;
+		} while (retries < 5);
What's with this magic retry loop?
Not sure I like the whole thing though. Splitting the irq ack semi-randomly between driver vs. multiple helpers doesn't feel great to me.
As a whole the HPD_IRQ handling is a total mess atm. At some point I was trying to sketch something a bit better for it. The approach I was thinking was something along the lines of:
u8 vector[...];
drm_dp_read_irq_vector(vector);
... handle all irqs/etc., calling suitable helpers as needed
drm_dp_clear_irq_vector(vector);
And I was also thinking that this drm_dp_*_irq_vector() stuff would always use the ESI layout, converting as needed from/to the old layout for pre-1.2 (or whatever the cutoff was) devices.
That way drivers would just need the one codepath.
drivers/gpu/drm/display/drm_dp_mst_topology.c int drm_dp_mst_hpd_irq(struct drm_dp_mst_topology_mgr *mgr, u8 *esi, bool *handl
Quoted 3 lines
drivers/gpu/drm/display/drm_dp_mst_topology.c +3 -0
+
+		if (ret != tosend)
+			drm_dbg_kms(mgr->dev, "failed to write dpcd
0x%x\n",
drivers/gpu/drm/i915/display/intel_dp.c intel_dp_check_mst_status(struct intel_dp
Quoted 12 lines
drivers/gpu/drm/display/drm_dp_mst_topology.c +3 -0
+				    DP_DEVICE_SERVICE_IRQ_VECTOR_ESI0);
+	}
+
 	drm_dp_mst_kick_tx(mgr);
 	return ret;
 }
drivers/gpu/drm/i915/display/intel_dp.c +0 -0
index bf80f296a8fd..abec3de38b66 100644
@@ -3939,6 +3939,9 @@ intel_dp_check_mst_status(struct intel_dp
*intel_dp)
drivers/gpu/drm/i915/display/intel_dp.c intel_dp_check_mst_status(struct intel_dp *intel_dp)
Quoted 5 lines
drivers/gpu/drm/i915/display/intel_dp.c +2 -0
 		if (!memchr_inv(ack, 0, sizeof(ack)))
 			break;
 
+		/* MSG_RDY ack is done in drm*/
+		ack[1] &= ~(DP_DOWN_REP_MSG_RDY |
DP_UP_REQ_MSG_RDY);
Quoted 10 lines
drivers/gpu/drm/i915/display/intel_dp.c +1 -0
+
 		if (!intel_dp_ack_sink_irq_esi(intel_dp, ack))
 			drm_dbg_kms(&i915->drm, "Failed to ack ESI\n");
 	}
drivers/gpu/drm/nouveau/dispnv50/disp.c +0 -0
index edcb2529b402..e905987104ed 100644
drivers/gpu/drm/nouveau/dispnv50/disp.c nv50_mstm_service(struct nouveau_drm *drm, · line 1340
Quoted 5 lines
drivers/gpu/drm/nouveau/dispnv50/disp.c +2 -0
@@ -1336,6 +1336,8 @@ nv50_mstm_service(struct nouveau_drm *drm,
 		if (!handled)
 			break;
 
+		/* MSG_RDY ack is done in drm*/
+		esi[1] &= ~(DP_DOWN_REP_MSG_RDY |
DP_UP_REQ_MSG_RDY);
drivers/gpu/drm/nouveau/dispnv50/disp.c nv50_mstm_service(struct nouveau_drm *drm, · line 1341
Quoted 1 line
drivers/gpu/drm/nouveau/dispnv50/disp.c +0 -0
 		rc = drm_dp_dpcd_write(aux, DP_SINK_COUNT_ESI + 1,
&esi[1],
Quoted 4 lines
drivers/gpu/drm/nouveau/dispnv50/disp.c +0 -1
 				       3);
 		if (rc != 3) {
--
2.37.3
--
Ville Syrjälä
Intel

Patchwork

2023-04-18 15:09 UTC #8 lore raw

[Intel-gfx] ✓ Fi.CI.IGT: success for drm/dp_mst: Clear MSG_RDY flag before sending new message (rev2)

In reply to Wayne Lin

== Series Details ==

Series: drm/dp_mst: Clear MSG_RDY flag before sending new message (rev2)
URL : https://patchwork.freedesktop.org/series/116623/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_13025_full -> Patchwork_116623v2_full
====================================================

Summary
-------

**SUCCESS**

No regressions found.

Participating hosts (7 -> 7)
------------------------------

No changes in participating hosts

Possible new issues
-------------------

Here are the unknown changes that may have been introduced in Patchwork_116623v2_full:

### IGT changes ###

#### Suppressed ####

The following results come from untrusted machines, tests, or statuses. They do not affect the overall result.

* igt@kms_plane_lowres@tiling-y@pipe-a-hdmi-a-2:
- {shard-rkl}: [PASS][1] -> [ABORT][2]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13025/shard-rkl-3/igt@kms_plane_lowres@[email protected]
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116623v2/shard-rkl-6/igt@kms_plane_lowres@[email protected]

Known issues
------------

Here are the changes found in Patchwork_116623v2_full that come from known issues:

### IGT changes ###

#### Issues hit ####

* igt@gem_exec_fair@basic-pace-solo@rcs0:
- shard-apl: [PASS][3] -> [FAIL][4] ([i915#2842])
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13025/shard-apl3/igt@gem_exec_fair@[email protected]
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116623v2/shard-apl1/igt@gem_exec_fair@[email protected]

* igt@i915_pm_rps@reset:
- shard-snb: [PASS][5] -> [DMESG-FAIL][6] ([i915#8319])
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13025/shard-snb7/igt@[email protected]
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116623v2/shard-snb7/igt@[email protected]

* igt@kms_cursor_legacy@2x-long-flip-vs-cursor-atomic:
- shard-glk: [PASS][7] -> [FAIL][8] ([i915#72])
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13025/shard-glk2/igt@[email protected]
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116623v2/shard-glk4/igt@[email protected]

* igt@kms_plane_scaling@planes-downscale-factor-0-5-unity-scaling@pipe-b-vga-1:
- shard-snb: NOTRUN -> [SKIP][9] ([fdo#109271]) +40 similar issues
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116623v2/shard-snb2/igt@kms_plane_scaling@[email protected]

* igt@kms_setmode@basic@pipe-a-hdmi-a-1:
- shard-snb: NOTRUN -> [FAIL][10] ([i915#5465]) +1 similar issue
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116623v2/shard-snb1/igt@kms_setmode@[email protected]

#### Possible fixes ####

* igt@drm_fdinfo@most-busy-idle-check-all@rcs0:
- {shard-rkl}: [FAIL][11] ([i915#7742]) -> [PASS][12]
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13025/shard-rkl-3/igt@drm_fdinfo@[email protected]
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116623v2/shard-rkl-7/igt@drm_fdinfo@[email protected]

* igt@gem_ctx_exec@basic-nohangcheck:
- {shard-tglu}: [FAIL][13] ([i915#6268]) -> [PASS][14]
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13025/shard-tglu-10/igt@[email protected]
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116623v2/shard-tglu-10/igt@[email protected]

* igt@gem_exec_fair@basic-deadline:
- shard-glk: [FAIL][15] ([i915#2846]) -> [PASS][16]
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13025/shard-glk3/igt@[email protected]
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116623v2/shard-glk4/igt@[email protected]

* igt@gem_exec_fair@basic-none@vcs0:
- {shard-rkl}: [FAIL][17] ([i915#2842]) -> [PASS][18]
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13025/shard-rkl-4/igt@gem_exec_fair@[email protected]
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116623v2/shard-rkl-6/igt@gem_exec_fair@[email protected]

* igt@gem_exec_fair@basic-pace-share@rcs0:
- shard-apl: [FAIL][19] ([i915#2842]) -> [PASS][20]
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13025/shard-apl6/igt@gem_exec_fair@[email protected]
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116623v2/shard-apl4/igt@gem_exec_fair@[email protected]

* igt@i915_pm_rpm@dpms-mode-unset-non-lpsp:
- {shard-dg1}: [SKIP][21] ([i915#1397]) -> [PASS][22] +1 similar issue
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13025/shard-dg1-14/igt@[email protected]
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116623v2/shard-dg1-18/igt@[email protected]

* igt@i915_pm_rpm@modeset-non-lpsp-stress:
- {shard-rkl}: [SKIP][23] ([i915#1397]) -> [PASS][24]
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13025/shard-rkl-7/igt@[email protected]
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116623v2/shard-rkl-4/igt@[email protected]

* igt@kms_async_flips@alternate-sync-async-flip@pipe-b-dp-1:
- shard-apl: [FAIL][25] ([i915#2521]) -> [PASS][26]
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13025/shard-apl7/igt@kms_async_flips@[email protected]
[26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116623v2/shard-apl4/igt@kms_async_flips@[email protected]

* igt@kms_big_fb@y-tiled-max-hw-stride-64bpp-rotate-180-hflip-async-flip:
- {shard-rkl}: [FAIL][27] ([i915#3743]) -> [PASS][28]
[27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13025/shard-rkl-7/igt@kms_big_fb@y-tiled-max-hw-stride-64bpp-rotate-180-hflip-async-flip.html
[28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116623v2/shard-rkl-4/igt@kms_big_fb@y-tiled-max-hw-stride-64bpp-rotate-180-hflip-async-flip.html

* igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions:
- shard-apl: [FAIL][29] ([i915#2346]) -> [PASS][30]
[29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13025/shard-apl7/igt@[email protected]
[30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116623v2/shard-apl4/igt@[email protected]

* igt@kms_cursor_legacy@single-move@pipe-b:
- {shard-rkl}: [INCOMPLETE][31] ([i915#8011]) -> [PASS][32]
[31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13025/shard-rkl-7/igt@kms_cursor_legacy@[email protected]
[32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116623v2/shard-rkl-3/igt@kms_cursor_legacy@[email protected]

{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).

Build changes
-------------

* Linux: CI_DRM_13025 -> Patchwork_116623v2

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116623v2/index.html