On Wed, Dec 14, 2022 at 4:50 PM Alex Hung <alex.hung@xxxxxxx> wrote: > > > > On 2022-12-14 13:48, Alex Deucher wrote: > > On Wed, Dec 14, 2022 at 3:22 PM Aurabindo Pillai > > <aurabindo.pillai@xxxxxxx> wrote: > >> > >> From: Alex Hung <alex.hung@xxxxxxx> > >> > >> [Why] > >> When running IGT kms_bw test with DP monitor, some systems crash from > >> msleep no matter how long or short the time is. > >> > >> [How] > >> To replace msleep with mdelay. > > > > Can you provide a bit more info about the crash? A lot of platforms > > don't support delay larger than 2-4ms so this change will generate > > errors on ARM and possibly other platforms. > > > > Alex > > The msleep was introduced in eec3303de3378 for non-compliant display > port monitors but IGT's kms_bw test can cause a recent h/w to hang at > msleep(60) when calling "igt_remove_fb" in IGT > (https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/blob/master/tests/kms_bw.c#L197) > > It is possible to workaround this by reversing order of > igt_remove_fb(&buffer[i]), as the following example: > > igt_create_color_fb with the order buffer[0], buffer[1], buffer[2] > > Hangs: > igt_remove_fb with the order buffer[0], buffer[1], buffer[2] > > No hangs: > igt_remove_fb with the reversed order buffer[2], buffer[1], buffer[0] > > However, IGT simply exposes the problem and it makes more sense to stop > the hang from occurring. > > I also tried to remove the msleep completely and it also work, but I > didn't want to break the fix for the original problematic hardware > configuration. Why does sleep vs delay make a difference? Is there some race that we are not locking against? Alex > > > > >> > >> Acked-by: Aurabindo Pillai <aurabindo.pillai@xxxxxxx> > >> Signed-off-by: Alex Hung <alex.hung@xxxxxxx> > >> Reviewed-by: Rodrigo Siqueira <Rodrigo.Siqueira@xxxxxxx> > >> --- > >> drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c b/drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c > >> index 913a1fe6b3da..e6251ccadb70 100644 > >> --- a/drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c > >> +++ b/drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c > >> @@ -1215,7 +1215,7 @@ void dce110_blank_stream(struct pipe_ctx *pipe_ctx) > >> * After output is idle pattern some sinks need time to recognize the stream > >> * has changed or they enter protection state and hang. > >> */ > >> - msleep(60); > >> + mdelay(60); > >> } else if (pipe_ctx->stream->signal == SIGNAL_TYPE_EDP) { > >> if (!link->dc->config.edp_no_power_sequencing) { > >> /* > >> -- > >> 2.39.0 > >>
