Commit 61802130 authored by Daniel Vetter's avatar Daniel Vetter
Browse files

drm: Document caveats around atomic event handling 



It's not that obvious how a driver can all race the atomic commit with
handling the completion event. And there's unfortunately a pile of
drivers with rather bad event handling which misdirect people into the
wrong direction.

Try to remedy this by documenting everything better.

v2: Type fixes Alex spotted.

v3: More typos Alex spotted.

Cc: Andrzej Hajda <a.hajda@samsung.com>
Cc: Alex Deucher <alexdeucher@gmail.com>
Reviewed-by: default avatarAlex Deucher <alexander.deucher@amd.com>
Signed-off-by: default avatarDaniel Vetter <daniel.vetter@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1475229896-6047-1-git-send-email-daniel.vetter@ffwll.ch
parent 58f0f9f7

drivers/gpu/drm/drm_irq.c

+30 −2
Original line number Diff line number Diff line
@@ -1008,6 +1008,31 @@ static void send_vblank_event(struct drm_device *dev, 
 * period. This helper function implements exactly the required vblank arming

 * behaviour.

 *

 * NOTE: Drivers using this to send out the event in struct &drm_crtc_state

 * as part of an atomic commit must ensure that the next vblank happens at

 * exactly the same time as the atomic commit is committed to the hardware. This

 * function itself does **not** protect again the next vblank interrupt racing

 * with either this function call or the atomic commit operation. A possible

 * sequence could be:

 *

 * 1. Driver commits new hardware state into vblank-synchronized registers.

 * 2. A vblank happens, 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.

 *

 * The only way to make this work safely is to prevent the vblank from firing

 * (and the hardware from committing 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.

 *

 * Caller must hold event lock. Caller must also hold a vblank reference for

 * the event @e, which will be dropped when the next vblank arrives.

 */
@@ -1030,8 +1055,11 @@ EXPORT_SYMBOL(drm_crtc_arm_vblank_event); 
 * @crtc: the source CRTC of the vblank event

 * @e: the event to send

 *

 * Updates sequence # and timestamp on event, and sends it to userspace.

 * Caller must hold event lock.

 * Updates sequence # and timestamp on event for the most recently processed

 * vblank, and sends it to userspace.  Caller must hold event lock.

 *

 * See drm_crtc_arm_vblank_event() for a helper which can be used in certain

 * situation, especially to send out events for atomic commit operations.

 */

void drm_crtc_send_vblank_event(struct drm_crtc *crtc,

				struct drm_pending_vblank_event *e)

include/drm/drm_crtc.h

+43 −13
Original line number Diff line number Diff line
@@ -109,8 +109,6 @@ struct drm_plane_helper_funcs; 
 * @ctm: Transformation matrix

 * @gamma_lut: Lookup table for converting pixel data after the

 *	conversion matrix

 * @event: optional pointer to a DRM event to signal upon completion of the

 * 	state update

 * @state: backpointer to global drm_atomic_state

 *

 * Note that the distinction between @enable and @active is rather subtile:
@@ -159,6 +157,46 @@ struct drm_crtc_state { 
	struct drm_property_blob *ctm;

	struct drm_property_blob *gamma_lut;



	/**

	 * @event:

	 *

	 * Optional pointer to a DRM event to signal upon completion of the

	 * state update. The driver must send out the event when the atomic

	 * commit operation completes. There are two cases:

	 *

	 *  - The event is for a CRTC which is being disabled through this

	 *    atomic commit. In that case the event can be send out any time

	 *    after the hardware has stopped scanning out the current

	 *    framebuffers. It should contain the timestamp and counter for the

	 *    last vblank before the display pipeline was shut off.

	 *

	 *  - For a CRTC which is enabled at the end of the commit (even when it

	 *    undergoes an full modeset) the vblank timestamp and counter must

	 *    be for the vblank right before the first frame that scans out the

	 *    new set of buffers. Again the event can only be sent out after the

	 *    hardware has stopped scanning out the old buffers.

	 *

	 *  - Events for disabled CRTCs are not allowed, and drivers can ignore

	 *    that case.

	 *

	 * This can be handled by the drm_crtc_send_vblank_event() function,

	 * which the driver should call on the provided event upon completion of

	 * the atomic commit. Note that if the driver supports vblank signalling

	 * and timestamping the vblank counters and timestamps must agree with

	 * the ones returned from page flip events. With the current vblank

	 * helper infrastructure this can be achieved by holding a vblank

	 * reference while the page flip is pending, acquired through

	 * drm_crtc_vblank_get() and released with drm_crtc_vblank_put().

	 * Drivers are free to implement their own vblank counter and timestamp

	 * tracking though, e.g. if they have accurate timestamp registers in

	 * hardware.

	 *

	 * For hardware which supports some means to synchronize vblank

	 * interrupt delivery with committing display state there's also

	 * drm_crtc_arm_vblank_event(). See the documentation of that function

	 * for a detailed discussion of the constraints it needs to be used

	 * safely.

	 */

	struct drm_pending_vblank_event *event;



	struct drm_atomic_state *state;
@@ -835,17 +873,9 @@ struct drm_mode_config_funcs { 
	 * CRTC index supplied in &drm_event to userspace.

	 *

	 * The drm core will supply a struct &drm_event in the event

	 * member of each CRTC's &drm_crtc_state structure. This can be handled by the

	 * drm_crtc_send_vblank_event() function, which the driver should call on

	 * the provided event upon completion of the atomic commit. Note that if

	 * the driver supports vblank signalling and timestamping the vblank

	 * counters and timestamps must agree with the ones returned from page

	 * flip events. With the current vblank helper infrastructure this can

	 * be achieved by holding a vblank reference while the page flip is

	 * pending, acquired through drm_crtc_vblank_get() and released with

	 * drm_crtc_vblank_put(). Drivers are free to implement their own vblank

	 * counter and timestamp tracking though, e.g. if they have accurate

	 * timestamp registers in hardware.

	 * member of each CRTC's &drm_crtc_state structure. See the

	 * documentation for &drm_crtc_state for more details about the precise

	 * semantics of this event.

	 *

	 * NOTE:

	 *

CRA Git | Maintained and supported by SUSTech CRA and CCSE