Skip to contentSkip to conversation

drm/amd/dm: Misc MST connector fixes

PATCH In review Lyude Paul 2 participants 7 messages Nov 16, 2018 – Nov 19, 2018

Subsystems (3)
lore
Share & tools
lore

Lyude Paul

author
#0 lore raw

cover drm/amd/dm: Misc MST connector fixes

Some fixes for things I spotted after trying to look at a bug Jerry Zuo mentioned to me. Didn't manage to reproduce the bug! But I found these.

Cc Jerry Zuo <[email protected]>

Lyude Paul (2):
drm/amd/dm: Don't forget to attach MST encoders drm/amd/dm: Understand why attaching path/tile properties are needed

 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)
Signature · 1 line
2.19.1
author
#1 lore raw

PATCH 1/2 drm/amd/dm: Don't forget to attach MST encoders

Drive-by fix, this is bound to cause problems somewhere.

---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c | 2 ++
 1 file changed, 2 insertions(+)
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c +2 -0
index d02c32a1039c..0cca1809fdcd 100644
@@ -342,6 +342,8 @@ dm_dp_add_mst_connector(struct drm_dp_mst_topology_mgr *mgr,
 		master->connector_id);
 
 	aconnector->mst_encoder = dm_dp_create_fake_mst_encoder(master);
+	drm_connector_attach_encoder(&aconnector->base,
+				     &aconnector->mst_encoder->base);
 
 	/*
 	 * TODO: understand why this one is needed
Signature · 1 line
2.19.1
Reviewed-by on v1 1/2
#2 lore raw

RE: [PATCH 1/2] drm/amd/dm: Don't forget to attach MST encoders

The change fixed MST + SST daisy chain and S3 scenarios. The issue shows huge delay in MST + SST daisy chain, and soft hang in S3 resume.

The aux sequence is changed by failed iteration search in drm_connector_for_each_possible_encoder().
The failure of searching for the best encoder for the connector due to the miss of attached encoder in the process of adding MST connector. The iteration search takes time to push drm_dp_send_enum_path_resources() aux transaction after the mode probe, and causes conflict to drm_dp_mst_i2c_xfer(), leading to the aux transaction timeout.

-----Original Message-----
From: Lyude Paul <[email protected]>
Sent: November 16, 2018 6:25 PM
To: [email protected]
Cc: Zuo, Jerry <[email protected]>; Wentland, Harry <[email protected]>; Li, Sun peng (Leo) <[email protected]>; Deucher, Alexander <[email protected]>; Koenig, Christian <[email protected]>; Zhou, David(ChunMing) <[email protected]>; David Airlie <[email protected]>; Li, Roman <[email protected]>; S, Shirish <[email protected]>; Daniel Vetter <[email protected]>; [email protected]; [email protected]
Subject: [PATCH 1/2] drm/amd/dm: Don't forget to attach MST encoders

Drive-by fix, this is bound to cause problems somewhere.

---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c | 2 ++
 1 file changed, 2 insertions(+)
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c +2 -0
index d02c32a1039c..0cca1809fdcd 100644
@@ -342,6 +342,8 @@ dm_dp_add_mst_connector(struct drm_dp_mst_topology_mgr *mgr,
 		master->connector_id);
 
 	aconnector->mst_encoder = dm_dp_create_fake_mst_encoder(master);
+	drm_connector_attach_encoder(&aconnector->base,
+				     &aconnector->mst_encoder->base);
 
 	/*
 	 * TODO: understand why this one is needed
Signature · 6 lines
2.19.1

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
author
#3 lore raw

Re: [PATCH 1/2] drm/amd/dm: Don't forget to attach MST encoders

Cool! If it did actually fix those problems, would you mind making sure this gets Cc'd to stable when it gets pushed upstream?

Quoted 49 lines · Jerry

The change fixed MST + SST daisy chain and S3 scenarios. The issue shows huge delay in MST + SST daisy chain, and soft hang in S3 resume.

The aux sequence is changed by failed iteration search in drm_connector_for_each_possible_encoder().
The failure of searching for the best encoder for the connector due to the miss of attached encoder in the process of adding MST connector. The iteration search takes time to push drm_dp_send_enum_path_resources() aux transaction after the mode probe, and causes conflict to drm_dp_mst_i2c_xfer(), leading to the aux transaction timeout.

-----Original Message-----
From: Lyude Paul <[email protected]>
Sent: November 16, 2018 6:25 PM
To: [email protected]
Cc: Zuo, Jerry <[email protected]>; Wentland, Harry <[email protected]> ; Li, Sun peng (Leo) <[email protected]>; Deucher, Alexander < [email protected]>; Koenig, Christian <[email protected]>; Zhou, David(ChunMing) <[email protected]>; David Airlie <[email protected]> ; Li, Roman <[email protected]>; S, Shirish <[email protected]>; Daniel Vetter <[email protected]>; [email protected]; [email protected]
Subject: [PATCH 1/2] drm/amd/dm: Don't forget to attach MST encoders

Drive-by fix, this is bound to cause problems somewhere.

---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c | 2 ++
 1 file changed, 2 insertions(+)
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c +2 -0
index d02c32a1039c..0cca1809fdcd 100644
@@ -342,6 +342,8 @@ dm_dp_add_mst_connector(struct drm_dp_mst_topology_mgr
 *mgr,
 		master->connector_id);
 
 	aconnector->mst_encoder = dm_dp_create_fake_mst_encoder(master);
+	drm_connector_attach_encoder(&aconnector->base,
+				     &aconnector->mst_encoder->base);
 
 	/*

* TODO: understand why this one is needed

Signature · 7 lines
Cheers,
	Lyude Paul

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
#4 lore raw

RE: [PATCH 1/2] drm/amd/dm: Don't forget to attach MST encoders

Sure, I'll do it.

-----Original Message-----
From: Lyude Paul <[email protected]>
Sent: November 19, 2018 1:52 PM
To: Zuo, Jerry <[email protected]>; [email protected]
Cc: Wentland, Harry <[email protected]>; Li, Sun peng (Leo) <[email protected]>; Deucher, Alexander <[email protected]>; Koenig, Christian <[email protected]>; Zhou, David(ChunMing) <[email protected]>; David Airlie <[email protected]>; Li, Roman <[email protected]>; S, Shirish <[email protected]>; Daniel Vetter <[email protected]>; [email protected]; [email protected]
Subject: Re: [PATCH 1/2] drm/amd/dm: Don't forget to attach MST encoders

Cool! If it did actually fix those problems, would you mind making sure this gets Cc'd to stable when it gets pushed upstream?

Quoted 50 lines · Jerry

The change fixed MST + SST daisy chain and S3 scenarios. The issue shows huge delay in MST + SST daisy chain, and soft hang in S3 resume.

The aux sequence is changed by failed iteration search in drm_connector_for_each_possible_encoder().
The failure of searching for the best encoder for the connector due to
the miss of attached encoder in the process of adding MST connector.
The iteration search takes time to push
drm_dp_send_enum_path_resources() aux transaction after the mode
probe, and causes conflict to drm_dp_mst_i2c_xfer(), leading to the aux transaction timeout.

-----Original Message-----
From: Lyude Paul <[email protected]>
Sent: November 16, 2018 6:25 PM
To: [email protected]
Cc: Zuo, Jerry <[email protected]>; Wentland, Harry <[email protected]> ; Li, Sun peng (Leo) <[email protected]>; Deucher, Alexander < [email protected]>; Koenig, Christian <[email protected]>; Zhou, David(ChunMing) <[email protected]>; David Airlie <[email protected]> ; Li, Roman <[email protected]>; S, Shirish <[email protected]>; Daniel Vetter <[email protected]>; [email protected]; [email protected]
Subject: [PATCH 1/2] drm/amd/dm: Don't forget to attach MST encoders

Drive-by fix, this is bound to cause problems somewhere.

---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c | 2 ++
 1 file changed, 2 insertions(+)
(quoted diff) +0 -0

a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c index d02c32a1039c..0cca1809fdcd 100644

drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c +2 -0
@@ -342,6 +342,8 @@ dm_dp_add_mst_connector(struct 
 drm_dp_mst_topology_mgr *mgr,
 		master->connector_id);
 
 	aconnector->mst_encoder = dm_dp_create_fake_mst_encoder(master);
+	drm_connector_attach_encoder(&aconnector->base,
+				     &aconnector->mst_encoder->base);
 
 	/*
Quoted 1 line

* TODO: understand why this one is needed

--
Cheers,
Lyude Paul

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

author
#5 lore raw

PATCH 2/2 drm/amd/dm: Understand why attaching path/tile properties are needed

In reply to Lyude Paul

Path property is used for userspace to know what MST connector goes to what actual DRM DisplayPort connector, the tiling property is for tiling configurations. Not sure what else there is to figure out.

---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c | 3 ---
 1 file changed, 3 deletions(-)
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c +0 -3
index 0cca1809fdcd..1b0d209d8367 100644
@@ -345,9 +345,6 @@ dm_dp_add_mst_connector(struct drm_dp_mst_topology_mgr *mgr,
 	drm_connector_attach_encoder(&aconnector->base,
 				     &aconnector->mst_encoder->base);
 
-	/*
-	 * TODO: understand why this one is needed
-	 */
 	drm_object_attach_property(
 		&connector->base,
 		dev->mode_config.path_property,
Signature · 1 line
2.19.1
Reviewed-by on v1 2/2
#6 lore raw

RE: [PATCH 2/2] drm/amd/dm: Understand why attaching path/tile properties are needed

-----Original Message-----
From: Lyude Paul <[email protected]>
Sent: November 16, 2018 6:25 PM
To: [email protected]
Cc: Wentland, Harry <[email protected]>; Li, Sun peng (Leo) <[email protected]>; Deucher, Alexander <[email protected]>; Koenig, Christian <[email protected]>; Zhou, David(ChunMing) <[email protected]>; David Airlie <[email protected]>; Zuo, Jerry <[email protected]>; Li, Roman <[email protected]>; Cheng, Tony <[email protected]>; Daniel Vetter <[email protected]>; S, Shirish <[email protected]>; [email protected]; [email protected]
Subject: [PATCH 2/2] drm/amd/dm: Understand why attaching path/tile properties are needed

Path property is used for userspace to know what MST connector goes to what actual DRM DisplayPort connector, the tiling property is for tiling configurations. Not sure what else there is to figure out.

---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c | 3 ---
 1 file changed, 3 deletions(-)
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c +0 -3
index 0cca1809fdcd..1b0d209d8367 100644
@@ -345,9 +345,6 @@ dm_dp_add_mst_connector(struct drm_dp_mst_topology_mgr *mgr,
 	drm_connector_attach_encoder(&aconnector->base,
 				     &aconnector->mst_encoder->base);
 
-	/*
-	 * TODO: understand why this one is needed
-	 */
 	drm_object_attach_property(
 		&connector->base,
 		dev->mode_config.path_property,

--
2.19.1

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx