From: "Kazlauskas, Nicholas" <nicholas.kazlauskas-5C7GfCeVMHo@public.gmane.org> To: "Li, Ching-shih (Louis)" <Ching-shih.Li-5C7GfCeVMHo@public.gmane.org>, "amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org" <amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>, "Wentland, Harry" <Harry.Wentland-5C7GfCeVMHo@public.gmane.org> Subject: Re: [PATCH v2] drm/amd/display: Fix Apple dongle cannot be successfully detected Date: Thu, 21 Nov 2019 08:47:50 -0500 [thread overview] Message-ID: <3ee8d870-c461-c68f-4a36-f2bf17e9e81f@amd.com> (raw) In-Reply-To: <3c30b486-7062-ade2-0dbd-7288fbf595c1-5C7GfCeVMHo@public.gmane.org> On 2019-11-21 8:40 a.m., Kazlauskas, Nicholas wrote: > 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 Actually, this lives in the low IRQ context which means that it's already been queued off to a work thread so it's safe to sleep. I'm not sure we want a general 300ms sleep (even by experiment you said that it only needed 200ms) for all connectors. 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 _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
WARNING: multiple messages have this Message-ID (diff)
From: "Kazlauskas, Nicholas" <nicholas.kazlauskas@amd.com> To: "Li, Ching-shih (Louis)" <Ching-shih.Li@amd.com>, "amd-gfx@lists.freedesktop.org" <amd-gfx@lists.freedesktop.org>, "Wentland, Harry" <Harry.Wentland@amd.com> Subject: Re: [PATCH v2] drm/amd/display: Fix Apple dongle cannot be successfully detected Date: Thu, 21 Nov 2019 08:47:50 -0500 [thread overview] Message-ID: <3ee8d870-c461-c68f-4a36-f2bf17e9e81f@amd.com> (raw) Message-ID: <20191121134750._UqH94GuJFyIKAMZjX2INN-f0xxF53DAMoJLLGkHiHE@z> (raw) In-Reply-To: <3c30b486-7062-ade2-0dbd-7288fbf595c1@amd.com> On 2019-11-21 8:40 a.m., Kazlauskas, Nicholas wrote: > 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 Actually, this lives in the low IRQ context which means that it's already been queued off to a work thread so it's safe to sleep. I'm not sure we want a general 300ms sleep (even by experiment you said that it only needed 200ms) for all connectors. 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 _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
next prev parent reply other threads:[~2019-11-21 13:47 UTC|newest] Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-11-14 3:42 [PATCH v2] drm/amd/display: Fix Apple dongle cannot be successfully detected Louis Li 2019-11-14 3:42 ` Louis Li [not found] ` <20191114034212.1106-1-Ching-shih.Li-5C7GfCeVMHo@public.gmane.org> 2019-11-21 8:31 ` Li, Ching-shih (Louis) 2019-11-21 8:31 ` Li, Ching-shih (Louis) [not found] ` <MN2PR12MB406250AEB8A10A080050D952AA4E0-rweVpJHSKTr3rx73VqdDCgdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org> 2019-11-21 13:40 ` Kazlauskas, Nicholas 2019-11-21 13:40 ` Kazlauskas, Nicholas [not found] ` <3c30b486-7062-ade2-0dbd-7288fbf595c1-5C7GfCeVMHo@public.gmane.org> 2019-11-21 13:47 ` Kazlauskas, Nicholas [this message] 2019-11-21 13:47 ` Kazlauskas, Nicholas [not found] ` <3ee8d870-c461-c68f-4a36-f2bf17e9e81f-5C7GfCeVMHo@public.gmane.org> 2019-11-22 6:33 ` Louis Li 2019-11-22 6:33 ` Louis Li 2019-11-22 15:31 ` Harry Wentland 2019-11-22 15:31 ` Harry Wentland [not found] ` <2d828fee-b8c7-ec8c-f454-2574fd68ed45-5C7GfCeVMHo@public.gmane.org> 2019-11-25 9:49 ` Louis Li 2019-11-25 9:49 ` Louis Li 2019-11-25 14:24 ` Harry Wentland 2019-11-25 14:24 ` Harry Wentland [not found] ` <95bdabb4-e216-559e-1a93-fb492903a7a0-5C7GfCeVMHo@public.gmane.org> 2019-11-26 8:51 ` Louis Li 2019-11-26 8:51 ` Louis Li
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=3ee8d870-c461-c68f-4a36-f2bf17e9e81f@amd.com \ --to=nicholas.kazlauskas-5c7gfcevmho@public.gmane.org \ --cc=Ching-shih.Li-5C7GfCeVMHo@public.gmane.org \ --cc=Harry.Wentland-5C7GfCeVMHo@public.gmane.org \ --cc=amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.