Re: [PATCH 01/35] drm/amd/display: Use udelay when waiting between aux retries
On Mon, Feb 04, 2019 at 03:43:36PM +, Wentland, Harry wrote: > DRM actually bumped this to 32 due to an issue with a Dell 4k display. As I feared there is a retry counter higher in the code. My bad. > It depends. I wouldn't call one or the other more correct. I seem to remember > that the DP spec is quite vague on these retries but I could be mistaken. > Since our driver hasn't show any problems with the DRM code and I believe > others (such as i915) also pass DP compliance without issues I wouldn't > proactively change this. I reacted to this matter since my DP monitor had troubles getting out of DP off state (iiyama) with logs which did seem to point towards a failure of getting the EDID. It happens rarely, and I could not figure out a context to reproduce for sure. Could be gone for good with the latest fixes. I guess fixes for popular and defective hardware (for instance that dell 4k) from a compliance stand point have to go in the code unfortunately. regards, -- Sylvain ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 01/35] drm/amd/display: Use udelay when waiting between aux retries
On 2019-02-01 5:55 p.m., sylvain.bertr...@gmail.com wrote: > On Fri, Feb 01, 2019 at 09:20:56PM +, Wentland, Harry wrote: >> DRM's AUX code uses usleep_range in drm_dp_dpcd_access. > > My bad, forgot about the usleep_range switch. That said AUX_RETRY_INTERVAL is > 500 us, with a usleep_range top bound of 600 us. > > Then, it would mean DC DP timeout retries would happen after ~1ms, and drm > ~600us. Additionaly, it seems that the number of retries are 3 in drm code and > 7 in DC code. > DRM actually bumped this to 32 due to an issue with a Dell 4k display. > I may be wrong, but it seems DC code is much more "insisting" on auxchannel > (edid retrieval) and much more forgiving on monitor ability to timeout in > time (~1ms). > > If I did read the code the right way, it may be more reasonable to have > similar > behavior in drm code than in DC code, right? > It depends. I wouldn't call one or the other more correct. I seem to remember that the DP spec is quite vague on these retries but I could be mistaken. Since our driver hasn't show any problems with the DRM code and I believe others (such as i915) also pass DP compliance without issues I wouldn't proactively change this. Harry ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 01/35] drm/amd/display: Use udelay when waiting between aux retries
On Fri, Feb 01, 2019 at 09:20:56PM +, Wentland, Harry wrote: > DRM's AUX code uses usleep_range in drm_dp_dpcd_access. My bad, forgot about the usleep_range switch. That said AUX_RETRY_INTERVAL is 500 us, with a usleep_range top bound of 600 us. Then, it would mean DC DP timeout retries would happen after ~1ms, and drm ~600us. Additionaly, it seems that the number of retries are 3 in drm code and 7 in DC code. I may be wrong, but it seems DC code is much more "insisting" on auxchannel (edid retrieval) and much more forgiving on monitor ability to timeout in time (~1ms). If I did read the code the right way, it may be more reasonable to have similar behavior in drm code than in DC code, right? -- Sylvain ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 01/35] drm/amd/display: Use udelay when waiting between aux retries
On 2019-02-01 3:47 p.m., sylvain.bertr...@gmail.com wrote: > On Fri, Feb 01, 2019 at 08:08:30PM +, sylvain.bertr...@gmail.com wrote: >> Do you mean non-DC displayport related code is already using udelay instead >> of msleep on linux? > > I grep-ed amdgpu displayport atombios code, got udelay(400) only. > Did the same on drm *_dp_* files, got similar udelays with only a very slow > msleep (which seems justified there). > > But I did not get any udelays with the range of 1ms for "retries". Maybe it > would be a good idea to a similar delay on non-DC displayport code? That would > give more time to "slow" displayort monitors to timeout on their side. > DRM's AUX code uses usleep_range in drm_dp_dpcd_access. Harry ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 01/35] drm/amd/display: Use udelay when waiting between aux retries
On Fri, Feb 01, 2019 at 08:08:30PM +, sylvain.bertr...@gmail.com wrote: > Do you mean non-DC displayport related code is already using udelay instead > of msleep on linux? I grep-ed amdgpu displayport atombios code, got udelay(400) only. Did the same on drm *_dp_* files, got similar udelays with only a very slow msleep (which seems justified there). But I did not get any udelays with the range of 1ms for "retries". Maybe it would be a good idea to a similar delay on non-DC displayport code? That would give more time to "slow" displayort monitors to timeout on their side. -- Sylvain ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 01/35] drm/amd/display: Use udelay when waiting between aux retries
On Fri, Feb 01, 2019 at 07:16:17PM +, Wentland, Harry wrote: > On 2019-02-01 12:31 p.m., sylvain.bertr...@gmail.com wrote: > > On Fri, Feb 01, 2019 at 10:28:13AM -0500, Bhawanpreet Lakha wrote: > >> From: John Barberiz > >> [How] > >> msleep is inaccurate for small values. Used udelay > >> instead for accuracy. > > > > Hi, > > > > Should it be the case for non-DC displayport code too? > > > > I don't think we're actually using this code on Linux. It's part of the > shared codebase with Windows. Apparently there msleep would often sleep > longer than 1ms. > > drm_dp_dpcd_access already has a tighter bound on the sleep between retries > (500-600 us), so I imagine we're fine there. Do you mean non-DC displayport related code is already using udelay instead of msleep on linux? -- Sylvain ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 01/35] drm/amd/display: Use udelay when waiting between aux retries
On 2019-02-01 12:31 p.m., sylvain.bertr...@gmail.com wrote: > On Fri, Feb 01, 2019 at 10:28:13AM -0500, Bhawanpreet Lakha wrote: >> From: John Barberiz >> [How] >> msleep is inaccurate for small values. Used udelay >> instead for accuracy. > > Hi, > > Should it be the case for non-DC displayport code too? > I don't think we're actually using this code on Linux. It's part of the shared codebase with Windows. Apparently there msleep would often sleep longer than 1ms. drm_dp_dpcd_access already has a tighter bound on the sleep between retries (500-600 us), so I imagine we're fine there. Harry > regards, > ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 01/35] drm/amd/display: Use udelay when waiting between aux retries
On Fri, Feb 01, 2019 at 10:28:13AM -0500, Bhawanpreet Lakha wrote: > From: John Barberiz > [How] > msleep is inaccurate for small values. Used udelay > instead for accuracy. Hi, Should it be the case for non-DC displayport code too? regards, -- Sylvain ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 01/35] drm/amd/display: Use udelay when waiting between aux retries
From: John Barberiz [Why] "IRQ_HPD Pulse Length Test" DP compliance test fails. Test complains that certain DPCD registers are not read within 100 ms. [How] msleep is inaccurate for small values. Used udelay instead for accuracy. Change-Id: I4990fc56c7632fba373f2dbf64ac803d64619529 Signed-off-by: John Barberiz Reviewed-by: Wenjing Liu Acked-by: Bhawanpreet Lakha --- drivers/gpu/drm/amd/display/dc/dce/dce_aux.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_aux.c b/drivers/gpu/drm/amd/display/dc/dce/dce_aux.c index adbb4e1a..4febf4ef7240 100644 --- a/drivers/gpu/drm/amd/display/dc/dce/dce_aux.c +++ b/drivers/gpu/drm/amd/display/dc/dce/dce_aux.c @@ -516,7 +516,7 @@ bool dce_aux_transfer_with_retries(struct ddc_service *ddc, } } - msleep(1); + udelay(1000); } return false; } -- 2.17.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
