Re: [PATCH 19/28] drm: document drm_crtc_funcs

Thierry Reding
2015-12-07T12:25:31Z

[-- Attachment #1.1: Type: text/plain, Size: 5199 bytes --]

On Fri, Dec 04, 2015 at 09:46:00AM +0100, Daniel Vetter wrote:
[...]
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index fbfe617bc492..72a7e096dd42 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -320,12 +320,6 @@ struct drm_crtc_state {
>
> /**
> * struct drm_crtc_funcs - control CRTCs for a given device
> - * @cursor_set: setup the cursor
> - * @cursor_set2: setup the cursor with hotspot, superseeds @cursor_set if set
> - * @cursor_move: move the cursor
> - * @gamma_set: specify color ramp for CRTC
> - * @set_config: apply a new CRTC configuration
> - * @page_flip: initiate a page flip
> *
> * The drm_crtc_funcs structure is the central CRTC management structure
> * in the DRM. Each CRTC controls one or more connectors (note that the name
> @@ -349,15 +343,86 @@ struct drm_crtc_funcs {
> */
> void (*reset)(struct drm_crtc *crtc);
>
> - /* cursor controls */
> + /**
> + * @cursor_set:
> + *
> + * Update the cursor image. The cursor position is relative to the CRTC
> + * and can be partially or fully outside of the visible area.
> + *
> + * Note that contrary to all other KMS function the legacy cursor entry

"functions"

> + /**
> + * @cursor_move:
> + *
> + * Update the cursor position. The cursor does not need to be visible
> + * when this hook is called.
> + *
> + * This entry point is deprecated, drivers should instead implement
> + * universal plane support and register a proper cursor plane using
> + * drm_crtc_init_with_planes().
> + *
> + * This callback is optional

Nit: Append a full-stop here and in the above ->cursor_*() callback
descriptions.

> - /*
> - * Flip to the given framebuffer. This implements the page
> - * flip ioctl described in drm_mode.h, specifically, the
> - * implementation must return immediately and block all
> - * rendering to the current fb until the flip has completed.
> - * If userspace set the event flag in the ioctl, the event
> - * argument will point to an event to send back when the flip
> - * completes, otherwise it will be NULL.
> + /**
> + * @page_flip:
> + *
> + * Legacy entry point to schedule a flip to the given framebuffer.
> + *
> + * Page flipping is a synchronization mechanism that replaces the frame
> + * buffer being scanned out by the CRTC with a new frame buffer during
> + * vertical blanking, avoiding tearing (except when requested otherwise
> + * through the DRM_MODE_PAGE_FLIP_ASYN flag). When an application

"DRM_MODE_PAGE_FLIP_ASYNC"

> + * requests a page flip the DRM core verifies that the new frame buffer is large

This line looks odd because it is wider than all the others.

> + * enough to be scanned out by the CRTC in the currently configured mode
> + * and then calls the CRTC page_flip operation with a pointer to the new

"->page_flip()"?

> + * frame buffer.
> + *
> + * The driver must wait for any pending rendering to the new framebuffer
> + * to complete before executing the flip. It should also wait for any
> + * pending rendering from other drivers if the underlying buffer is a
> + * shared dma-buf.
> + *
> + * An application can request to be notified when the page flip has
> + * completed. The drm core will supply a struct &drm_event in the event
> + * parameter in this 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 flip. 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.
> + *
> + * FIXME:
> + *
> + * Up to that point drivers need to manage events themselves and can use
> + * even->base.list freely for that. Specifically they need to ensure
> + * that they don't send out page flip (or vblank) events for which the
> + * corresponding drm file has been closed already. The drm core
> + * unfortunately does not (yet) take care of that. Therefore drivers
> + * currently must clean up and release pending events in their
> + * ->preclose driver function.
> + *
> + * This callback is optional.
> + *
> + * NOTE:
> + *
> + * Very early versions of the KMS ABI mandated that the driver must
> + * block (but not reject) any rendering to the old framebuffer until the
> + * flip operation has completed and the old framebuffer is not longer

"no longer"

> + * visible. This requirement has been lifted, and userspace is instead
> + * expected to request delivery of a event and wait with recycling old

"an event"

Otherwise:

Reviewed-by: Thierry Reding <treding@nvidia.com>

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]