Re: [PATCH v2] drm/amd/display: Fix Apple dongle cannot be successfully detected

["Kazlauskas, Nicholas" <nicholas.kazlauskas-5C7GfCeVMHo@public.gmane.org>]

On 2019-11-21 3:31 a.m., Li, Ching-shih (Louis) wrote:
> Hi reviewers,
> 
> What is the review result for this patch? Customer is pushing on this change to merge. TKS for your attention.
> 
> BR,
> Louis
> 
> -----Original Message-----
> From: Louis Li <Ching-shih.Li@amd.com>
> Sent: Thursday, November 14, 2019 11:42 AM
> To: amd-gfx@lists.freedesktop.org
> Cc: Kazlauskas, Nicholas <Nicholas.Kazlauskas@amd.com>; Wentland, Harry <Harry.Wentland@amd.com>; Li, Ching-shih (Louis) <Ching-shih.Li@amd.com>
> Subject: [PATCH v2] drm/amd/display: Fix Apple dongle cannot be successfully detected
> 
> [Why]
> External monitor cannot be displayed consistently, if connecting via this Apple dongle (A1621, USB Type-C to HDMI).
> By experiments, it is confirmed that the dongle needs 200ms at least to be ready for communication, after it sets HPD signal high.
> 
> [How]
> When receiving HPD IRQ, delay 300ms at the beginning of handle_hpd_irq().
> Then run the original procedure.
> With this patch applied, the problem cannot be reproduced.
> With other dongles, test results are PASS.
> Test result is PASS to plug in HDMI cable while playing video, and the video is being playing smoothly.
> Test result is PASS after system resumes from suspend.
> 
> Signed-off-by: Louis Li <Ching-shih.Li@amd.com>

This is still a NAK from me since the logic hasn't changed from the 
first patch.

Sleeps don't belong in IRQ handlers.

Regards,
Nicholas Kazlauskas

> ---
>   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> 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 0aef92b7c037..5b844b6a5a59 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -1025,6 +1025,11 @@ static void handle_hpd_irq(void *param)
>   	struct drm_device *dev = connector->dev;
>   	enum dc_connection_type new_connection_type = dc_connection_none;
>   
> +	/* Some monitors/dongles need around 200ms to be ready for communication
> +	 * after those devices drive HPD signal high.
> +	 */
> +	msleep(300);
> +
>   	/* In case of failure or MST no need to update connector status or notify the OS
>   	 * since (for MST case) MST does this in it's own context.
>   	 */
> --
> 2.21.0
> 

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx