All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Vetter <daniel.vetter@ffwll.ch>
To: DRI Development <dri-devel@lists.freedesktop.org>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>,
	Intel Graphics Development <intel-gfx@lists.freedesktop.org>,
	Eric Anholt <eric@anholt.net>
Subject: [PATCH 21/28] drm: Kerneldoc for drm_mode_config_funcs
Date: Fri,  4 Dec 2015 09:46:02 +0100	[thread overview]
Message-ID: <1449218769-16577-22-git-send-email-daniel.vetter@ffwll.ch> (raw)
In-Reply-To: <1449218769-16577-1-git-send-email-daniel.vetter@ffwll.ch>

The meat here is definitely the detailed specs for what atomic_check
and atomic_commit are supposed to do.

And another candidate for a core vfunc that should be in a helper really
(output_poll_changed this time around).

v2: Feedback from Eric on irc:
- spelling fixes.
- spec what async should do
- copy the event related paragraphs from page_flip and adjust
- make it clear that a successful async commit is not allowed to leave
  the pipe dead or disabled.

v3: Use FIXME comments to annotate functions that we should move to
some helpers.

Cc: Eric Anholt <eric@anholt.net>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 include/drm/drm_crtc.h | 241 +++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 233 insertions(+), 8 deletions(-)

diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 551484fb55e5..37cff64a26ef 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -1715,31 +1715,256 @@ struct drm_mode_set {
 
 /**
  * struct drm_mode_config_funcs - basic driver provided mode setting functions
- * @fb_create: create a new framebuffer object
- * @output_poll_changed: function to handle output configuration changes
- * @atomic_check: check whether a given atomic state update is possible
- * @atomic_commit: commit an atomic state update previously verified with
- * 	atomic_check()
- * @atomic_state_alloc: allocate a new atomic state
- * @atomic_state_clear: clear the atomic state
- * @atomic_state_free: free the atomic state
  *
  * Some global (i.e. not per-CRTC, connector, etc) mode setting functions that
  * involve drivers.
  */
 struct drm_mode_config_funcs {
+	/**
+	 * @fb_create:
+	 *
+	 * Create a new framebuffer object. The core does basic checks on the
+	 * requested metadata, but most of that is left to the driver. See
+	 * struct &drm_mode_fb_cmd2 for details.
+	 *
+	 * RETURNS:
+	 *
+	 * A new framebuffer with an initial refernce count of 1 or a negative
+	 * error code encoded with ERR_PTR().
+	 */
 	struct drm_framebuffer *(*fb_create)(struct drm_device *dev,
 					     struct drm_file *file_priv,
 					     const struct drm_mode_fb_cmd2 *mode_cmd);
+
+	/**
+	 * @output_poll_changed:
+	 *
+	 * Callback used by helpers to inform the driver of output configuration
+	 * changes.
+	 *
+	 * Drivers implementing fbdev emulation with the helpers can call
+	 * drm_fb_helper_hotplug_changed from this hook to inform the fbdev
+	 * helper of output changes.
+	 *
+	 * FIXME:
+	 *
+	 * Except that there's no vtable for device-level helper callbacks
+	 * there's no reason this is a core function.
+	 */
 	void (*output_poll_changed)(struct drm_device *dev);
 
+	/**
+	 * @atomic_check:
+	 *
+	 * This is the only hook to validate an atomic modeset update. This
+	 * function must reject any modeset and state changes which the hardware
+	 * or driver doesn't support. This includes but is of course not limited
+	 * to:
+	 *
+	 *  - Checking that the modes, framebuffers, scaling and placement
+	 *    requirements and so on are within the limits of the hardware.
+	 *
+	 *  - Checking that any hidden shared resources are not oversubscribed.
+	 *    This can be shared PLLs, shared lanes, overall memory bandwidth,
+	 *    display fifo space (where shared between planes or maybe even
+	 *    CRTCs).
+	 *
+	 *  - Checking that virtualized resources exported to userspace are not
+	 *    oversubscribed. For various reasons it can make sense to expose
+	 *    more planes, crtcs or encoders than which are physically there. One
+	 *    example is dual-pipe operations (which generally should be hidden
+	 *    from userspace if when lockstepped in hardware, otherwise exposed),
+	 *    where a plane might need 1 hardware plane (if it's just on one
+	 *    pipe), 2 hardware planes (when it spans both pipes) or maybe even
+	 *    shared a hardware plane with a 2nd plane (if there's a compatible
+	 *    plane requested on the area handled by the other pipe).
+	 *
+	 *  - Check that any transitional state is possible and that if
+	 *    requested, the update can indeed be done in the vblank period
+	 *    without temporarily disabling some functions.
+	 *
+	 *  - Check any other constraints the driver or hardware might have.
+	 *
+	 *  - This callback also needs to correctly fill out the &drm_crtc_state
+	 *    in this update to make sure that drm_atomic_crtc_needs_modeset()
+	 *    reflects the nature of the possible update and returns true if and
+	 *    only if the update cannot be applied without tearing within one
+	 *    vblank on that CRTC. The core uses that information to reject
+	 *    updates which require a full modeset (i.e. blanking the screen, or
+	 *    at least pausing updates for a substantial amount of time) if
+	 *    userspace has disallowed that in its request.
+	 *
+	 *  - The driver also does not need to repeat basic input validation
+	 *    like done for the corresponding legacy entry points. The core does
+	 *    that before calling this hook.
+	 *
+	 * See the documentation of @atomic_commit for an exhaustive list of
+	 * error conditions which are allowed to not be checked in this
+	 * callback.
+	 *
+	 * See the documentation for struct &drm_atomic_state for how exactly
+	 * an atomic modeset update is described.
+	 *
+	 * Drivers using the atomic helpers can implement this hook using
+	 * drm_atomic_helper_check(), or one of the exported sub-functions of
+	 * it.
+	 *
+	 * RETURNS:
+	 *
+	 * 0 on success or one of the below negative error codes:
+	 *
+	 *  - -EINVAL, if any of the above constraints are violated.
+	 *
+	 *  - -EDEADLK, when returned from an attempt to acquire an additional
+	 *    &drm_modeset_lock through drm_modeset_lock().
+	 *
+	 *  - -ENOMEM, if allocating additional state sub-structures failed due
+	 *    to lack of memory.
+	 *
+	 *  - -EINTR, -EAGAIN or -ERESTARTSYS, if the ioctl should be restarted.
+	 *    This can either be due to a pending signal, or because the driver
+	 *    needs to completely bail out to recover from an exceptional
+	 *    situation like a gpu hang. From a userspace point all errors are
+	 *    treated equally.
+	 */
 	int (*atomic_check)(struct drm_device *dev,
 			    struct drm_atomic_state *a);
+
+	/**
+	 * @atomic_commit:
+	 *
+	 * This is the only hook to commit an atomic modeset update. The core
+	 * guarantees that @atomic_check has been called successfully before
+	 * calling this function, and that nothing has been changed in the
+	 * interim.
+	 *
+	 * See the documentation for struct &drm_atomic_state for how exactly
+	 * an atomic modeset update is described.
+	 *
+	 * Drivers using the atomic helpers can implement this hook using
+	 * drm_atomic_helper_commit(), or one of the exported sub-functions of
+	 * it.
+	 *
+	 * Asynchronous commits (as indicated with the async parameter) must
+	 * do any preparatory work which might result in an unsuccessful commit
+	 * in the context of this callback. The only exception is hardware
+	 * errors resulting in -EIO. But even in that case the driver must
+	 * ensure that the display pipe is at least running, to avoid
+	 * compositors crashing when pageflips don't work. Anything else,
+	 * specifically committing the update to the hardware, should be done
+	 * without blocking the caller. For updates which do not require a
+	 * modeset this must be guaranteed.
+	 *
+	 * The driver must wait for any pending rendering to the new
+	 * framebuffers 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. Asynchronous commits must not wait for
+	 * rendering in the context of this callback.
+	 *
+	 * An application can request to be notified when the atomic commit has
+	 * completed. These events are per-CRTC and can be distinguished by the
+	 * 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.
+	 *
+	 * NOTE:
+	 *
+	 * Drivers are not allowed to shut down any display pipe successfully
+	 * enabled through an atomic commit on their own. Doing so can result in
+	 * compositors crashing if a page flip is suddenly reject because the
+	 * pipe is off.
+	 *
+	 * RETURNS:
+	 *
+	 * 0 on success or one of the below negative error codes:
+	 *
+	 *  - -EBUSY, if an asynchronous updated is requested and there is
+	 *    currently an earlier updated still pending. Drivers are allowed to
+	 *    support a queue of outstanding updates, but currently no driver
+	 *    supports that. Note that drivers must wait for preceding updates
+	 *    to complete if a synchronous update is requested, they are not
+	 *    allowed to fail the commit in that case.
+	 *
+	 *  - -ENOMEM, if the driver failed to allocate memory. Specifically
+	 *    this can happen when trying to pin framebuffers, which must only
+	 *    be done when committing the state.
+	 *
+	 *  - -ENOSPC, as a refinement of the more generic -ENOMEM to indicate
+	 *    that the driver has run out of vram, iommu space or similar gpu
+	 *    address space needed for framebuffer.
+	 *
+	 *  - -EIO, if the hardware completely died.
+	 *
+	 *  - -EINTR, -EAGAIN or -ERESTARTSYS, if the ioctl should be restarted.
+	 *    This can either be due to a pending signal, or because the driver
+	 *    needs to completely bail out to recover from an exceptional
+	 *    situation like a gpu hang. From a userspace point of view all errors are
+	 *    treated equally.
+	 *
+	 * This list is exhaustive. Specifically this hook is not allowed to
+	 * return -EINVAL (any invalid requests should be caught in
+	 * @atomic_check) or -EDEADLK (this function must not acquire
+	 * additional modeset locks). The core will also reject any async
+	 * atomic flips with -EINVAL already (for matching semantics in this
+	 * case with legacy page flips).
+	 */
 	int (*atomic_commit)(struct drm_device *dev,
 			     struct drm_atomic_state *a,
 			     bool async);
+
+	/**
+	 * @atomic_state_alloc:
+	 *
+	 * This optional hook can be used by drivers who want to subclass struct
+	 * &drm_atomic_state to be able to track their own driver-private global
+	 * state easily. If this hook is implemented, drivers must also
+	 * implement @atomic_state_clear and @atomic_state_free.
+	 *
+	 * RETURNS:
+	 *
+	 * A new &drm_atomic_state on success or NULL on failure.
+	 */
 	struct drm_atomic_state *(*atomic_state_alloc)(struct drm_device *dev);
+
+	/**
+	 * @atomic_state_clear:
+	 *
+	 * This hook must clear any driver private state duplicated into the
+	 * passed-in &drm_atomic_state. This hook is called when the caller
+	 * encountered a &drm_modeset_lock deadlock and needs to drop all
+	 * already acquired locks as part of the deadlock avoidance dance
+	 * implemented in drm_modeset_lock_backoff().
+	 *
+	 * Any duplicated state must be invalidated since a concurrent atomic
+	 * update might change it, and the drm atomic interfaces always apply
+	 * updates as relative changes to the current state.
+	 *
+	 * Drivers who implement this must call drm_atomic_state_default_clear()
+	 * to clear common state.
+	 */
 	void (*atomic_state_clear)(struct drm_atomic_state *state);
+
+	/**
+	 * @atomic_state_free:
+	 *
+	 * This hook needs driver private resources and the &drm_atomic_state
+	 * itself. Note that the core first calls drm_atomic_state_clear to
+	 * avoid code duplicate between the clear and free hooks.
+	 *
+	 * Drivers who implement this must call drm_atomic_state_default_free()
+	 * to release common resources.
+	 */
 	void (*atomic_state_free)(struct drm_atomic_state *state);
 };
 
-- 
2.5.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

  parent reply	other threads:[~2015-12-04  8:46 UTC|newest]

Thread overview: 81+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-12-04  8:45 [PATCH 00/28] kerneldoc for display vtables Daniel Vetter
2015-12-04  8:45 ` [PATCH 01/28] drm: Polish fbdev helper struct docs Daniel Vetter
2015-12-07 10:45   ` Thierry Reding
2015-12-07 11:50     ` Daniel Vetter
2015-12-07 11:53       ` Thierry Reding
2015-12-04  8:45 ` [PATCH 02/28] drm: Move LEAVE/ENTER_ATOMIC_MODESET to fbdev helpers Daniel Vetter
2015-12-07 11:00   ` Thierry Reding
2015-12-04  8:45 ` [PATCH 03/28] drm: Reorganize helper vtables and their docs Daniel Vetter
2015-12-07 11:00   ` Thierry Reding
2015-12-07 11:59     ` Daniel Vetter
2015-12-07 12:26       ` Thierry Reding
2015-12-04  8:45 ` [PATCH 04/28] drm: Make helper vtable pointers type-safe Daniel Vetter
2015-12-07 11:01   ` Thierry Reding
2015-12-04  8:45 ` [PATCH 05/28] drm: Merge helper docbook into kerneldoc comments Daniel Vetter
2015-12-07 11:15   ` Thierry Reding
2015-12-04  8:45 ` [PATCH 06/28] drm/bridge: Improve kerneldoc Daniel Vetter
2015-12-04 10:43   ` Archit Taneja
2015-12-07 11:31   ` Thierry Reding
2015-12-04  8:45 ` [PATCH 07/28] drm: Update drm_plane_funcs kerneldoc Daniel Vetter
2015-12-07 11:46   ` Thierry Reding
2015-12-07 12:34     ` Daniel Vetter
2015-12-07 12:43       ` Thierry Reding
2015-12-07 13:00         ` Daniel Vetter
2015-12-04  8:45 ` [PATCH 08/28] drm/noveau: Ditch NULL save/restore hook assignments Daniel Vetter
2015-12-07 11:47   ` Thierry Reding
2015-12-04  8:45 ` [PATCH 09/28] drm/qxl: Drop dummy save/restore hooks Daniel Vetter
2015-12-07 11:47   ` Thierry Reding
2015-12-04  8:45 ` [PATCH 10/28] drm/virtio: Drop dummy save/restore functions Daniel Vetter
2015-12-07 11:47   ` Thierry Reding
2015-12-04  8:45 ` [PATCH 11/28] drm/vmwgfx: Drop dummy save/restore hooks Daniel Vetter
2015-12-07 11:48   ` Thierry Reding
2015-12-08 11:55   ` Thomas Hellstrom
2015-12-04  8:45 ` [PATCH 12/28] drm/gma500: Move to private " Daniel Vetter
2015-12-07 11:51   ` Thierry Reding
2015-12-04  8:45 ` [PATCH 13/28] drm/nouveau: Use " Daniel Vetter
2015-12-04 14:31   ` Ilia Mirkin
2015-12-04 16:06     ` Daniel Vetter
2015-12-04 16:13   ` [PATCH] drm/nouveau: Use private save/restore hooks for CRTCs Daniel Vetter
2015-12-07 11:51     ` Thierry Reding
2015-12-04  8:45 ` [PATCH 14/28] drm: Remove crtc/connector->save/restore hooks Daniel Vetter
2015-12-07 11:55   ` Thierry Reding
2015-12-04  8:45 ` [PATCH 15/28] drm: Move encoder->save/restore into nouveau Daniel Vetter
2015-12-04 16:14   ` [PATCH] " Daniel Vetter
2015-12-07 11:59     ` Thierry Reding
2015-12-04  8:45 ` [PATCH 16/28] drm: Document drm_atomic_*_get_property Daniel Vetter
2015-12-07 12:01   ` Thierry Reding
2015-12-07 12:51     ` Daniel Vetter
2015-12-04  8:45 ` [PATCH 17/28] drm: Document drm_connector_funcs Daniel Vetter
2015-12-07 12:05   ` Thierry Reding
2015-12-04  8:45 ` [PATCH 18/28] drm: connector->dpms is not optional Daniel Vetter
2015-12-07 12:06   ` Thierry Reding
2015-12-04  8:46 ` [PATCH 19/28] drm: document drm_crtc_funcs Daniel Vetter
2015-12-07 12:25   ` Thierry Reding
2015-12-04  8:46 ` [PATCH 20/28] drm: Add kerneldoc for drm_framebuffer_funcs Daniel Vetter
2015-12-07 12:37   ` Thierry Reding
2015-12-04  8:46 ` Daniel Vetter [this message]
2015-12-07 13:14   ` [PATCH 21/28] drm: Kerneldoc for drm_mode_config_funcs Thierry Reding
2015-12-07 13:34     ` Daniel Vetter
2015-12-04  8:46 ` [PATCH 22/28] drm/atomic-helper: Reject attempts at re-stealing encoders Daniel Vetter
2015-12-07 13:26   ` Thierry Reding
2015-12-07 13:40     ` Daniel Vetter
2015-12-04  8:46 ` [PATCH 23/28] drm: Document drm_plane_helper_funcs Daniel Vetter
2015-12-07 14:27   ` Thierry Reding
2015-12-07 14:43     ` Daniel Vetter
2015-12-04  8:46 ` [PATCH 24/28] drm: Document drm_connector_helper_funcs Daniel Vetter
2015-12-07 14:42   ` Thierry Reding
2015-12-07 14:48     ` Daniel Vetter
2015-12-07 15:27       ` Thierry Reding
2015-12-04  8:46 ` [PATCH 25/28] drm/atomic-helper: Mention the new system/resume helpers the docs Daniel Vetter
2015-12-07 14:45   ` Thierry Reding
2015-12-04  8:46 ` [PATCH 26/28] drm: Move drm_display_mode an related docs into kerneldoc Daniel Vetter
2015-12-07 13:39   ` Ville Syrjälä
2015-12-07 15:02   ` Thierry Reding
2015-12-07 15:33     ` Daniel Vetter
2015-12-04  8:46 ` [PATCH 27/28] drm: Document drm_encoder/crtc_helper_funcs Daniel Vetter
2015-12-07 15:21   ` Thierry Reding
2015-12-04  8:46 ` [PATCH 28/28] drm/atomic-helper: Reject legacy flips on a disabled pipe Daniel Vetter
2015-12-07 13:42   ` Ville Syrjälä
2015-12-07 15:25   ` Thierry Reding
2015-12-07 15:33   ` Daniel Stone
2015-12-08  8:23     ` Daniel Vetter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1449218769-16577-22-git-send-email-daniel.vetter@ffwll.ch \
    --to=daniel.vetter@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=eric@anholt.net \
    --cc=intel-gfx@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.