On 09.03.2017 04:54, Michel Dänzer wrote: > On 08/03/17 11:58 PM, Andrzej Hajda wrote: >> Current implementation of event handling assumes that vblank interrupt is >> always called at the right time. It is not true, it can be delayed due to >> various reasons. As a result different races can happen. The patch fixes >> the issue by using hardware frame counter present in DECON to serialize >> vblank and commit completion events. >> >> Signed-off-by: Andrzej Hajda <a.hajda@xxxxxxxxxxx> >> --- >> v2: >> - added internal decon_get_frame_count function, >> - updated frame counter on flush, >> - misc style fixes (thank Inki) > [...] > >> @@ -579,6 +636,23 @@ static const struct component_ops decon_component_ops = { >> .unbind = decon_unbind, >> }; >> >> +static void decon_handle_vblank(struct decon_context *ctx) >> +{ >> + u32 frm; >> + >> + spin_lock(&ctx->vblank_lock); >> + >> + frm = decon_get_frame_count(ctx, true); >> + >> + if (frm != ctx->frame_id) { >> + if (frm > ctx->frame_id) >> + drm_crtc_handle_vblank(&ctx->crtc->base); > This comparison isn't safe WRT the counter value returned by > decon_get_frame_count wrapping around. And knowing that max framerate is 60fps it will happen after: 0xffffffff / 60fps / 60sec / 60min / 24h / 365days = 2.3 years So after 2.3 years of uninterrupted work of panel/TV we will lose one or two vblanks :) Highly improbable and even then low damage. > If it goes all the way up to > 0xffffffff before wrapping around to zero, it can be handled e.g. by > > if ((s32)(frm - ctx->frame_id) > 0) > > otherwise it gets a bit more complicated. > > But the fix looks simple so I think it is worth to fix it. Thanks. Regards Andrzej _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel