[19/28] drm: document drm_crtc_funcs
diff mbox

Message ID 1449218769-16577-20-git-send-email-daniel.vetter@ffwll.ch
State New
Headers show

Commit Message

Daniel Vetter Dec. 4, 2015, 8:46 a.m. UTC
And merge any docbook we have into the kerneldoc comments.

Since it's a legacy entry point with only two implementation (one each
in atomic and legacy crtc helpers) I've made the documentation for
set_config fairly sparse - no one should ever need to look at this
again, all the ABI we have is baked into code.

For ->page_flip otoh I kept all the extensive docs from the docbook
and even extended it where it was lacking: Currently we have a pile of
legacy page_flip implemantations, and even for atomic drivers there's
not yet a standard implementation in the helpers. Which means every
driver needs to implement this itself, and precise specs are really
valuable.

Otherwise there's just cursor, which really just boils down to "use at
least universal planes". And gamma tables (where we have a bit a mess
with the fbdev helper gamma hooks).

v2: Spelling fixes (Eric).

Cc: Eric Anholt <eric@anholt.net>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 Documentation/DocBook/gpu.tmpl | 117 +----------------------------
 include/drm/drm_crtc.h         | 164 +++++++++++++++++++++++++++++++++++++----
 2 files changed, 149 insertions(+), 132 deletions(-)

Comments

Thierry Reding Dec. 7, 2015, 12:25 p.m. UTC | #1
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>

Patch
diff mbox

diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl
index 02c7d44f517c..5312f5a05798 100644
--- a/Documentation/DocBook/gpu.tmpl
+++ b/Documentation/DocBook/gpu.tmpl
@@ -1174,121 +1174,6 @@  int max_width, max_height;</synopsis>
           pointer to CRTC functions.
         </para>
       </sect3>
-      <sect3 id="drm-kms-crtcops">
-        <title>CRTC Operations</title>
-        <sect4>
-          <title>Set Configuration</title>
-          <synopsis>int (*set_config)(struct drm_mode_set *set);</synopsis>
-          <para>
-            Apply a new CRTC configuration to the device. The configuration
-            specifies a CRTC, a frame buffer to scan out from, a (x,y) position in
-            the frame buffer, a display mode and an array of connectors to drive
-            with the CRTC if possible.
-          </para>
-          <para>
-            If the frame buffer specified in the configuration is NULL, the driver
-            must detach all encoders connected to the CRTC and all connectors
-            attached to those encoders and disable them.
-          </para>
-          <para>
-            This operation is called with the mode config lock held.
-          </para>
-          <note><para>
-	    Note that the drm core has no notion of restoring the mode setting
-	    state after resume, since all resume handling is in the full
-	    responsibility of the driver. The common mode setting helper library
-	    though provides a helper which can be used for this:
-	    <function>drm_helper_resume_force_mode</function>.
-          </para></note>
-        </sect4>
-        <sect4>
-          <title>Page Flipping</title>
-          <synopsis>int (*page_flip)(struct drm_crtc *crtc, struct drm_framebuffer *fb,
-                   struct drm_pending_vblank_event *event);</synopsis>
-          <para>
-            Schedule a page flip to the given frame buffer for the CRTC. This
-            operation is called with the mode config mutex held.
-          </para>
-          <para>
-            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. When an application requests a page
-            flip the DRM core verifies that the new frame buffer is large enough to
-            be scanned out by  the CRTC in the currently configured mode and then
-            calls the CRTC <methodname>page_flip</methodname> operation with a
-            pointer to the new frame buffer.
-          </para>
-          <para>
-            The <methodname>page_flip</methodname> operation schedules a page flip.
-            Once any pending rendering targeting the new frame buffer has
-            completed, the CRTC will be reprogrammed to display that frame buffer
-            after the next vertical refresh. The operation must return immediately
-            without waiting for rendering or page flip to complete and must block
-            any new rendering to the frame buffer until the page flip completes.
-          </para>
-          <para>
-            If a page flip can be successfully scheduled the driver must set the
-            <code>drm_crtc-&gt;fb</code> field to the new framebuffer pointed to
-            by <code>fb</code>. This is important so that the reference counting
-            on framebuffers stays balanced.
-          </para>
-          <para>
-            If a page flip is already pending, the
-            <methodname>page_flip</methodname> operation must return
-            -<errorname>EBUSY</errorname>.
-          </para>
-          <para>
-            To synchronize page flip to vertical blanking the driver will likely
-            need to enable vertical blanking interrupts. It should call
-            <function>drm_vblank_get</function> for that purpose, and call
-            <function>drm_vblank_put</function> after the page flip completes.
-          </para>
-          <para>
-            If the application has requested to be notified when page flip completes
-            the <methodname>page_flip</methodname> operation will be called with a
-            non-NULL <parameter>event</parameter> argument pointing to a
-            <structname>drm_pending_vblank_event</structname> instance. Upon page
-            flip completion the driver must call <methodname>drm_send_vblank_event</methodname>
-            to fill in the event and send to wake up any waiting processes.
-            This can be performed with
-            <programlisting><![CDATA[
-            spin_lock_irqsave(&dev->event_lock, flags);
-            ...
-            drm_send_vblank_event(dev, pipe, event);
-            spin_unlock_irqrestore(&dev->event_lock, flags);
-            ]]></programlisting>
-          </para>
-          <note><para>
-            FIXME: Could drivers that don't need to wait for rendering to complete
-            just add the event to <literal>dev-&gt;vblank_event_list</literal> and
-            let the DRM core handle everything, as for "normal" vertical blanking
-            events?
-          </para></note>
-          <para>
-            While waiting for the page flip to complete, the
-            <literal>event-&gt;base.link</literal> list head can be used freely by
-            the driver to store the pending event in a driver-specific list.
-          </para>
-          <para>
-            If the file handle is closed before the event is signaled, drivers must
-            take care to destroy the event in their
-            <methodname>preclose</methodname> operation (and, if needed, call
-            <function>drm_vblank_put</function>).
-          </para>
-        </sect4>
-        <sect4>
-          <title>Miscellaneous</title>
-          <itemizedlist>
-            <listitem>
-              <synopsis>void (*gamma_set)(struct drm_crtc *crtc, u16 *r, u16 *g, u16 *b,
-                        uint32_t start, uint32_t size);</synopsis>
-              <para>
-                Apply a gamma table to the device. The operation is optional.
-              </para>
-            </listitem>
-          </itemizedlist>
-        </sect4>
-      </sect3>
     </sect2>
     <sect2>
       <title>Planes (struct <structname>drm_plane</structname>)</title>
@@ -1305,7 +1190,7 @@  int max_width, max_height;</synopsis>
         <listitem>
         DRM_PLANE_TYPE_PRIMARY represents a "main" plane for a CRTC.  Primary
         planes are the planes operated upon by CRTC modesetting and flipping
-        operations described in <xref linkend="drm-kms-crtcops"/>.
+	operations described in the page_flip hook in <structname>drm_crtc_funcs</structname>.
         </listitem>
         <listitem>
         DRM_PLANE_TYPE_CURSOR represents a "cursor" plane for a CRTC.  Cursor
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
+	 * points don't take a framebuffer object, but instead take directly a
+	 * raw buffer object id from the driver's buffer manager (which is
+	 * either GEM or TTM for current drivers).
+	 *
+	 * 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
+	 *
+	 * RETURNS:
+	 *
+	 * 0 on success or a negative error code on failure.
+	 */
 	int (*cursor_set)(struct drm_crtc *crtc, struct drm_file *file_priv,
 			  uint32_t handle, uint32_t width, uint32_t height);
+
+	/**
+	 * @cursor_set2:
+	 *
+	 * Update the cursor image, including hotspot information. The hotspot
+	 * must not affect the cursor position in CRTC coordinates, but is only
+	 * meant as a hint for virtualized display hardware to coordinate the
+	 * guests and hosts cursor position. The cursor hotspot is relative to
+	 * the cursor image. Otherwise this works exactly like @cursor_set.
+	 *
+	 * 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
+	 *
+	 * RETURNS:
+	 *
+	 * 0 on success or a negative error code on failure.
+	 */
 	int (*cursor_set2)(struct drm_crtc *crtc, struct drm_file *file_priv,
 			   uint32_t handle, uint32_t width, uint32_t height,
 			   int32_t hot_x, int32_t hot_y);
+
+	/**
+	 * @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
+	 *
+	 * RETURNS:
+	 *
+	 * 0 on success or a negative error code on failure.
+	 */
 	int (*cursor_move)(struct drm_crtc *crtc, int x, int y);
 
-	/* Set gamma on the CRTC */
+	/**
+	 * @gamma_set:
+	 *
+	 * Set gamma on the CRTC.
+	 *
+	 * This callback is optional.
+	 *
+	 * NOTE:
+	 *
+	 * Drivers that support gamma tables and also fbdev emulation through
+	 * the provided helper library need to take care to fill out the gamma
+	 * hooks for both. Currently there's a bit an unfortunate duplication
+	 * going on, which should eventually be unified to just one set of
+	 * hooks.
+	 */
 	void (*gamma_set)(struct drm_crtc *crtc, u16 *r, u16 *g, u16 *b,
 			  uint32_t start, uint32_t size);
 
@@ -370,16 +435,83 @@  struct drm_crtc_funcs {
 	 */
 	void (*destroy)(struct drm_crtc *crtc);
 
+	/**
+	 * @set_config:
+	 *
+	 * This is the main legacy entry point to change the modeset state on a
+	 * CRTC. All the details of the desired configuration are passed in a
+	 * struct &drm_mode_set - see there for details.
+	 *
+	 * Drivers implementing atomic modeset should use
+	 * drm_atomic_helper_set_config() to implement this hook.
+	 *
+	 * RETURNS:
+	 *
+	 * 0 on success or a negative error code on failure.
+	 */
 	int (*set_config)(struct drm_mode_set *set);
 
-	/*
-	 * 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
+	 * requests a page flip the DRM core verifies that the new frame buffer is large
+	 * 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
+	 * 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
+	 * visible. This requirement has been lifted, and userspace is instead
+	 * expected to request delivery of a event and wait with recycling old
+	 * buffers until such has been received.
+	 *
+	 * RETURNS:
+	 *
+	 * 0 on success or a negative error code on failure. Note that if a
+	 * page_flip operation is already pending the callback should return
+	 * -EBUSY. Pageflips on a disabled CRTC (either by setting a NULL mode
+	 * or just runtime disabled through DPMS respectively the new atomic
+	 * "ACTIVE" state) should result in an -EINVAL error code.
 	 */
 	int (*page_flip)(struct drm_crtc *crtc,
 			 struct drm_framebuffer *fb,