Hi, On 29 September 2016 at 16:20, Daniel Vetter <daniel.vetter@xxxxxxxx> wrote: > + * 1. Driver commits new hardware state into vblank-synchronized registes. > + * 2. A vblank happes, committing the hardware state. Also the corresponding > + * vblank interrupt is fired off and fully processed by the interrupt > + * handler. > + * 3. The atomic commit operation proceeds to call drm_crtc_arm_vblank_event(). > + * 4. The event is only send out for the next vblank, which is wrong. > + * > + * An equivalent race can happen when the driver calls > + * drm_crtc_arm_vblank_event() before writing out the new hardware state. Might be worth pointing out that the equivalent race is backwards, i.e. the event will be delivered for the vblank before the commit took effect. > + * The only way to make this work savely is to prevent the vblank from firing > + * (and the hardware from committig anything else) until the entire atomic > + * commit sequence has run to completion. If the hardware does not have such a > + * feature (e.g. using a "go" bit), then it is unsafe to use this functions. > + * Instead drivers need to manually send out the event from their interrupt > + * handler by calling drm_crtc_send_vblank_event() and make sure that there's no > + * possible race with the hardware committing the atomic update. I'd add an explicit 'e.g. by verifying that shadow register content matches primary registers' or something, to make it really clear that it's not just about queueing event delivery from the vblank IRQ handler instead. > + * The only way to make this work savely is to prevent the vblank from firing > + * (and the hardware from committig anything else) until the entire atomic > + * commit sequence has run to completion. If the hardware does not have such a > + * feature (e.g. using a "go" bit), then it is unsafe to use this functions. > + * Instead drivers need to manually send out the event from their interrupt > + * handler by calling drm_crtc_send_vblank_event() and make sure that there's no > + * possible race with the hardware committing the atomic update. > + * - Events for disabled CRTCs are not allowed, and drivers can ignore > + * that case. I'd clarify that to 'CRTCs which are, and remain, disabled'. Cheers, Daniel _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
