diff mbox series

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

Message ID 20181116232443.1709-2-lyude@redhat.com (mailing list archive)
State New, archived
Headers show
Series drm/amd/dm: Misc MST connector fixes | expand

Commit Message

Lyude Paul Nov. 16, 2018, 11:24 p.m. UTC
Drive-by fix, this is bound to cause problems somewhere.

Signed-off-by: Lyude Paul <lyude@redhat.com>
Cc: Jerry Zuo <Jerry.Zuo@amd.com>
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Fangzhi Zuo Nov. 19, 2018, 3 p.m. UTC | #1
Reviewed-by: Jerry (Fangzhi) Zuo <Jerry.Zuo@amd.com>

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 <lyude@redhat.com> 
Sent: November 16, 2018 6:25 PM
To: amd-gfx@lists.freedesktop.org
Cc: Zuo, Jerry <Jerry.Zuo@amd.com>; Wentland, Harry <Harry.Wentland@amd.com>; Li, Sun peng (Leo) <Sunpeng.Li@amd.com>; Deucher, Alexander <Alexander.Deucher@amd.com>; Koenig, Christian <Christian.Koenig@amd.com>; Zhou, David(ChunMing) <David1.Zhou@amd.com>; David Airlie <airlied@linux.ie>; Li, Roman <Roman.Li@amd.com>; S, Shirish <Shirish.S@amd.com>; Daniel Vetter <daniel.vetter@ffwll.ch>; dri-devel@lists.freedesktop.org; linux-kernel@vger.kernel.org
Subject: [PATCH 1/2] drm/amd/dm: Don't forget to attach MST encoders

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

Signed-off-by: Lyude Paul <lyude@redhat.com>
Cc: Jerry Zuo <Jerry.Zuo@amd.com>
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git 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
--- 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
@@ -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
Lyude Paul Nov. 19, 2018, 6:51 p.m. UTC | #2
Cool! If it did actually fix those problems, would you mind making sure this
gets Cc'd to stable when it gets pushed upstream?

On Mon, 2018-11-19 at 15:00 +0000, Zuo, Jerry wrote:
> Reviewed-by: Jerry (Fangzhi) Zuo <Jerry.Zuo@amd.com>
> 
> 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 <lyude@redhat.com> 
> Sent: November 16, 2018 6:25 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Zuo, Jerry <Jerry.Zuo@amd.com>; Wentland, Harry <Harry.Wentland@amd.com>
> ; Li, Sun peng (Leo) <Sunpeng.Li@amd.com>; Deucher, Alexander <
> Alexander.Deucher@amd.com>; Koenig, Christian <Christian.Koenig@amd.com>;
> Zhou, David(ChunMing) <David1.Zhou@amd.com>; David Airlie <airlied@linux.ie>
> ; Li, Roman <Roman.Li@amd.com>; S, Shirish <Shirish.S@amd.com>; Daniel
> Vetter <daniel.vetter@ffwll.ch>; dri-devel@lists.freedesktop.org; 
> linux-kernel@vger.kernel.org
> Subject: [PATCH 1/2] drm/amd/dm: Don't forget to attach MST encoders
> 
> Drive-by fix, this is bound to cause problems somewhere.
> 
> Signed-off-by: Lyude Paul <lyude@redhat.com>
> Cc: Jerry Zuo <Jerry.Zuo@amd.com>
> ---
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git 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
> --- 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
> @@ -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
Fangzhi Zuo Nov. 19, 2018, 6:54 p.m. UTC | #3
Sure, I'll do it.

-----Original Message-----
From: Lyude Paul <lyude@redhat.com> 
Sent: November 19, 2018 1:52 PM
To: Zuo, Jerry <Jerry.Zuo@amd.com>; amd-gfx@lists.freedesktop.org
Cc: Wentland, Harry <Harry.Wentland@amd.com>; Li, Sun peng (Leo) <Sunpeng.Li@amd.com>; Deucher, Alexander <Alexander.Deucher@amd.com>; Koenig, Christian <Christian.Koenig@amd.com>; Zhou, David(ChunMing) <David1.Zhou@amd.com>; David Airlie <airlied@linux.ie>; Li, Roman <Roman.Li@amd.com>; S, Shirish <Shirish.S@amd.com>; Daniel Vetter <daniel.vetter@ffwll.ch>; dri-devel@lists.freedesktop.org; linux-kernel@vger.kernel.org
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?

On Mon, 2018-11-19 at 15:00 +0000, Zuo, Jerry wrote:
> Reviewed-by: Jerry (Fangzhi) Zuo <Jerry.Zuo@amd.com>
> 
> 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 <lyude@redhat.com>
> Sent: November 16, 2018 6:25 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Zuo, Jerry <Jerry.Zuo@amd.com>; Wentland, Harry 
> <Harry.Wentland@amd.com> ; Li, Sun peng (Leo) <Sunpeng.Li@amd.com>; 
> Deucher, Alexander < Alexander.Deucher@amd.com>; Koenig, Christian 
> <Christian.Koenig@amd.com>; Zhou, David(ChunMing) 
> <David1.Zhou@amd.com>; David Airlie <airlied@linux.ie> ; Li, Roman 
> <Roman.Li@amd.com>; S, Shirish <Shirish.S@amd.com>; Daniel Vetter 
> <daniel.vetter@ffwll.ch>; dri-devel@lists.freedesktop.org; 
> linux-kernel@vger.kernel.org
> Subject: [PATCH 1/2] drm/amd/dm: Don't forget to attach MST encoders
> 
> Drive-by fix, this is bound to cause problems somewhere.
> 
> Signed-off-by: Lyude Paul <lyude@redhat.com>
> Cc: Jerry Zuo <Jerry.Zuo@amd.com>
> ---
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git 
> 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
> --- 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
> @@ -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
--
Cheers,
	Lyude Paul
diff mbox series

Patch

diff --git 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
--- 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
@@ -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