Re: [PATCH] UPSTREAM: drm/amd/display: Fix Apple dongle cannot be successfully detected
On Wed, Oct 23, 2019 at 02:19:56PM +0800, S, Shirish wrote: > The UPSTREAM tag in the commit message needs to be removed. > OK. Will remove it. > On 10/21/2019 1:24 PM, Louis Li wrote: > > [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 500ms at the beginning of handle_hpd_irq(). > > Am not sure how this delay shall impact on dongles that don't need it, > > ideally it should be added as quirk, at least restrict it to these > specific vendors. > > Instead of delay, can you find any parameter to wait for for the > communication to be ready, > > in that way it shall be failsafe. > For such devices (monitors/dongles), there will be no chance to get information before the defer, because it is not ready to commmunicate right after HPD interrup. All other available dongles were verified with the patch applied, and work well still. As replied earlier, this is caused by dongle/monitor, and the solution is to have the driver better compatibility. Louis Li > > 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 after system resumes from suspend. > > > > Signed-off-by: Louis Li > > --- > > 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..043ddac73862 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 they drive HPD signal high. > > + */ > > +mdelay(500); > > + > > /* 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. > > */ > > -- > Regards, > Shirish S > ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] UPSTREAM: drm/amd/display: Fix Apple dongle cannot be successfully detected
On Mon, Oct 21, 2019 at 08:45:18PM +0800, Kazlauskas, Nicholas wrote: > On 2019-10-21 3:54 a.m., Louis Li wrote: > > [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 500ms 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 after system resumes from suspend. > > > > Signed-off-by: Louis Li > > --- > > 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..043ddac73862 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 they drive HPD signal high. > > + */ > > +mdelay(500); > > + > This sounds more like a quirk than behavior we want for all monitors, > but regardless this sleep isn't the correct place for this - since this > is an high priority interrupt handler this is really just blocking > everything for half a second. > > I think it'd make more sense to queue off the work to occur half a > second later. > > Nicholas Kazlauskas > Yes, I agree with your comments. However, dealing with monitors and dongles, it is proved that some monitors/dongles don't work as the way people expected. The solution makes sense to have our driver better compatibility to work with such devices. Truly, should not block high priority interrupt. I had V2 patch to use msleep instead. Customer is verifying V2. Will submit once it pass tests. Louis Li > > /* 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. > > */ > > > ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] UPSTREAM: drm/amd/display: Fix Apple dongle cannot be successfully detected
The UPSTREAM tag in the commit message needs to be removed. On 10/21/2019 1:24 PM, Louis Li wrote: > [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 500ms at the beginning of handle_hpd_irq(). Am not sure how this delay shall impact on dongles that don't need it, ideally it should be added as quirk, at least restrict it to these specific vendors. Instead of delay, can you find any parameter to wait for for the communication to be ready, in that way it shall be failsafe. > 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 after system resumes from suspend. > > Signed-off-by: Louis Li > --- > 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..043ddac73862 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 they drive HPD signal high. > + */ > +mdelay(500); > + > /* 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. >*/ -- Regards, Shirish S ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] UPSTREAM: drm/amd/display: Fix Apple dongle cannot be successfully detected
On 2019-10-21 3:54 a.m., Louis Li wrote: > [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 500ms 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 after system resumes from suspend. > > Signed-off-by: Louis Li > --- > 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..043ddac73862 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 they drive HPD signal high. > + */ > +mdelay(500); > + This sounds more like a quirk than behavior we want for all monitors, but regardless this sleep isn't the correct place for this - since this is an high priority interrupt handler this is really just blocking everything for half a second. I think it'd make more sense to queue off the work to occur half a second later. Nicholas Kazlauskas > /* 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. >*/ > ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] UPSTREAM: 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 500ms 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 after system resumes from suspend. Signed-off-by: Louis Li --- 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..043ddac73862 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 they drive HPD signal high. + */ +mdelay(500); + /* 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
