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>,
	Intel Graphics Development <intel-gfx@lists.freedesktop.org>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Subject: [PATCH 07/12] drm/irq: kerneldoc polish
Date: Wed, 14 May 2014 20:51:09 +0200	[thread overview]
Message-ID: <1400093477-3217-8-git-send-email-daniel.vetter@ffwll.ch> (raw)
In-Reply-To: <1400093477-3217-1-git-send-email-daniel.vetter@ffwll.ch>

- Integrate into the drm DocBook
- Disable kerneldoc for functions not exported to drivers.
- Properly document the new drm_vblank_on|off and add cautious
  comments explaining when drm_vblank_pre|post_modesets shouldn't be
  used.
- General polish and OCD.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 Documentation/DocBook/drm.tmpl |   5 +-
 drivers/gpu/drm/drm_irq.c      | 181 ++++++++++++++++++++++++++++-------------
 2 files changed, 129 insertions(+), 57 deletions(-)

diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
index e37a77a72b8b..70ef87bcea04 100644
--- a/Documentation/DocBook/drm.tmpl
+++ b/Documentation/DocBook/drm.tmpl
@@ -2519,6 +2519,10 @@ void (*disable_vblank) (struct drm_device *dev, int crtc);</synopsis>
       with a call to <function>drm_vblank_cleanup</function> in the driver
       <methodname>unload</methodname> operation handler.
     </para>
+    <sect2>
+      <title>Vertical Blanking and Interrupt Handling Functions Reference</title>
+!Edrivers/gpu/drm/drm_irq.c
+    </sect2>
   </sect1>
 
   <!-- Internals: open/close, file operations and ioctls -->
@@ -2870,7 +2874,6 @@ int num_ioctls;</synopsis>
             </listitem>
           </varlistentry>
         </variablelist>
-<!--!Edrivers/char/drm/drm_irq.c-->
       </para>
     </sect1>
 
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index dd786d84daab..5ff986bd4de4 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -1,6 +1,5 @@
-/**
- * \file drm_irq.c
- * IRQ support
+/*
+ * drm_irq.c IRQ and vblank support
  *
  * \author Rickard E. (Rik) Faith <faith@valinux.com>
  * \author Gareth Hughes <gareth@valinux.com>
@@ -156,6 +155,12 @@ static void vblank_disable_fn(unsigned long arg)
 	spin_unlock_irqrestore(&dev->vbl_lock, irqflags);
 }
 
+/**
+ * drm_vblank_cleanup - cleanup vblank support
+ * @dev: drm device
+ *
+ * This function cleans up any resources allocated in drm_vblank_init.
+ */
 void drm_vblank_cleanup(struct drm_device *dev)
 {
 	int crtc;
@@ -175,6 +180,16 @@ void drm_vblank_cleanup(struct drm_device *dev)
 }
 EXPORT_SYMBOL(drm_vblank_cleanup);
 
+/**
+ * drm_vblank_init - initialize vblank support
+ * @dev: drm_device
+ * @num_crtcs: number of crtcs supported by @dev
+ *
+ * This function initializes vblank support for @num_crtcs display pipelines.
+ *
+ * Returns:
+ * Zero on success or a negative error code on failure.
+ */
 int drm_vblank_init(struct drm_device *dev, int num_crtcs)
 {
 	int i, ret = -ENOMEM;
@@ -238,13 +253,21 @@ static void drm_irq_vgaarb_nokms(void *cookie, bool state)
 }
 
 /**
- * Install IRQ handler.
- *
- * \param dev DRM device.
+ * drm_irq_install - install IRQ handler
+ * @dev: drm device
+ * @irq: irq number to install the handler for
  *
  * Initializes the IRQ related data. Installs the handler, calling the driver
- * \c irq_preinstall() and \c irq_postinstall() functions
- * before and after the installation.
+ * irq_preinstall() and irq_postinstall() functions before and after the
+ * installation.
+ *
+ * This is the simplified helper interface provided for drivers with no special
+ * needs. Drivers which need to install interrupt handlers for multiple
+ * interrupts must instead set drm_device->irq_enabled to signal the drm core
+ * that vblank interrupts are available.
+ *
+ * Returns:
+ * Zero on success or a negative error code on failure.
  */
 int drm_irq_install(struct drm_device *dev, int irq)
 {
@@ -304,11 +327,20 @@ int drm_irq_install(struct drm_device *dev, int irq)
 EXPORT_SYMBOL(drm_irq_install);
 
 /**
- * Uninstall the IRQ handler.
+ * drm_irq_uninstall - uninstall the IRQ handler
+ * @dev: drm device
+ *
+ * Calls the driver's irq_uninstall() function and unregisters the irq handler.
+ * This should only be called by drivers which used drm_irq_install() to set up
+ * their interrupt handler. Other drivers must only reset
+ * drm_device->irq_enabled to false.
  *
- * \param dev DRM device.
+ * Note that for kernel modesetting drivers it is a bug if this function fails.
+ * The sanity checks are only to catch buggy user modesetting drivers which call
+ * the same function through an ioctl.
  *
- * Calls the driver's \c irq_uninstall() function, and stops the irq.
+ * Returns:
+ * Zero on success or a negative error code on failure.
  */
 int drm_irq_uninstall(struct drm_device *dev)
 {
@@ -353,7 +385,7 @@ int drm_irq_uninstall(struct drm_device *dev)
 }
 EXPORT_SYMBOL(drm_irq_uninstall);
 
-/**
+/*
  * IRQ control ioctl.
  *
  * \param inode device inode.
@@ -406,10 +438,9 @@ int drm_control(struct drm_device *dev, void *data,
 }
 
 /**
- * drm_calc_timestamping_constants - Calculate vblank timestamp constants
- *
- * @crtc drm_crtc whose timestamp constants should be updated.
- * @mode display mode containing the scanout timings
+ * drm_calc_timestamping_constants - calculate vblank timestamp constants
+ * @crtc: drm_crtc whose timestamp constants should be updated.
+ * @mode: display mode containing the scanout timings
  *
  * Calculate and store various constants which are later
  * needed by vblank and swap-completion timestamping, e.g,
@@ -459,11 +490,22 @@ void drm_calc_timestamping_constants(struct drm_crtc *crtc,
 EXPORT_SYMBOL(drm_calc_timestamping_constants);
 
 /**
- * drm_calc_vbltimestamp_from_scanoutpos - helper routine for kms
- * drivers. Implements calculation of exact vblank timestamps from
- * given drm_display_mode timings and current video scanout position
- * of a crtc. This can be called from within get_vblank_timestamp()
- * implementation of a kms driver to implement the actual timestamping.
+ * drm_calc_vbltimestamp_from_scanoutpos - precise vblank timestamp helper
+ * @dev: drm device
+ * @crtc: Which crtc's vblank timestamp to retrieve
+ * @max_error: Desired maximum allowable error in timestamps (nanosecs)
+ *             On return contains true maximum error of timestamp
+ * @vblank_time: Pointer to struct timeval which should receive the timestamp
+ * @flags: Flags to pass to driver:
+ *         0 = Default,
+ *         DRM_CALLED_FROM_VBLIRQ = If function is called from vbl irq handler
+ * @refcrtc: drm_crtc* of crtc which defines scanout timing
+ * @mode: mode which defines the scanout timings
+ *
+ * Implements calculation of exact vblank timestamps from given drm_display_mode
+ * timings and current video scanout position of a crtc. This can be called from
+ * within get_vblank_timestamp() implementation of a kms driver to implement the
+ * actual timestamping.
  *
  * Should return timestamps conforming to the OML_sync_control OpenML
  * extension specification. The timestamp corresponds to the end of
@@ -478,18 +520,8 @@ EXPORT_SYMBOL(drm_calc_timestamping_constants);
  * returns as no operation if a doublescan or interlaced video mode is
  * active. Higher level code is expected to handle this.
  *
- * @dev: DRM device.
- * @crtc: Which crtc's vblank timestamp to retrieve.
- * @max_error: Desired maximum allowable error in timestamps (nanosecs).
- *             On return contains true maximum error of timestamp.
- * @vblank_time: Pointer to struct timeval which should receive the timestamp.
- * @flags: Flags to pass to driver:
- *         0 = Default.
- *         DRM_CALLED_FROM_VBLIRQ = If function is called from vbl irq handler.
- * @refcrtc: drm_crtc* of crtc which defines scanout timing.
- * @mode: mode which defines the scanout timings
- *
- * Returns negative value on error, failure or if not supported in current
+ * Returns:
+ * Negative value on error, failure or if not supported in current
  * video mode:
  *
  * -EINVAL   - Invalid crtc.
@@ -641,14 +673,13 @@ static struct timeval get_drm_timestamp(void)
 
 /**
  * drm_get_last_vbltimestamp - retrieve raw timestamp for the most recent
- * vblank interval.
- *
- * @dev: DRM device
+ * 			       vblank interval
+ * @dev: drm device
  * @crtc: which crtc's vblank timestamp to retrieve
  * @tvblank: Pointer to target struct timeval which should receive the timestamp
  * @flags: Flags to pass to driver:
- *         0 = Default.
- *         DRM_CALLED_FROM_VBLIRQ = If function is called from vbl irq handler.
+ *         0 = Default,
+ *         DRM_CALLED_FROM_VBLIRQ = If function is called from vbl irq handler
  *
  * Fetches the system timestamp corresponding to the time of the most recent
  * vblank interval on specified crtc. May call into kms-driver to
@@ -657,7 +688,8 @@ static struct timeval get_drm_timestamp(void)
  * Returns zero if timestamp originates from uncorrected do_gettimeofday()
  * call, i.e., it isn't very precisely locked to the true vblank.
  *
- * Returns non-zero if timestamp is considered to be very precise.
+ * Returns:
+ * Non-zero if timestamp is considered to be very precise, zero otherwise.
  */
 u32 drm_get_last_vbltimestamp(struct drm_device *dev, int crtc,
 			      struct timeval *tvblank, unsigned flags)
@@ -686,12 +718,15 @@ EXPORT_SYMBOL(drm_get_last_vbltimestamp);
 
 /**
  * drm_vblank_count - retrieve "cooked" vblank counter value
- * @dev: DRM device
+ * @dev: drm device
  * @crtc: which counter to retrieve
  *
  * Fetches the "cooked" vblank count value that represents the number of
  * vblank events since the system was booted, including lost events due to
  * modesetting activity.
+ *
+ * Returns:
+ * The software vblank counter.
  */
 u32 drm_vblank_count(struct drm_device *dev, int crtc)
 {
@@ -703,15 +738,14 @@ EXPORT_SYMBOL(drm_vblank_count);
  * drm_vblank_count_and_time - retrieve "cooked" vblank counter value
  * and the system timestamp corresponding to that vblank counter value.
  *
- * @dev: DRM device
+ * @dev: drm device
  * @crtc: which counter to retrieve
  * @vblanktime: Pointer to struct timeval to receive the vblank timestamp.
  *
  * Fetches the "cooked" vblank count value that represents the number of
  * vblank events since the system was booted, including lost events due to
  * modesetting activity. Returns corresponding system timestamp of the time
- * of the vblank interval that corresponds to the current value vblank counter
- * value.
+ * of the vblank interval that corresponds to the current vblank counter value.
  */
 u32 drm_vblank_count_and_time(struct drm_device *dev, int crtc,
 			      struct timeval *vblanktime)
@@ -751,7 +785,7 @@ static void send_vblank_event(struct drm_device *dev,
 
 /**
  * drm_send_vblank_event - helper to send vblank event after pageflip
- * @dev: DRM device
+ * @dev: drm device
  * @crtc: CRTC in question
  * @e: the event to send
  *
@@ -777,7 +811,7 @@ EXPORT_SYMBOL(drm_send_vblank_event);
 
 /**
  * drm_update_vblank_count - update the master vblank counter
- * @dev: DRM device
+ * @dev: drm device
  * @crtc: counter to update
  *
  * Call back into the driver to update the appropriate vblank counter
@@ -841,7 +875,7 @@ static void drm_update_vblank_count(struct drm_device *dev, int crtc)
 
 /**
  * drm_vblank_enable - enable the vblank interrupt on a CRTC
- * @dev: DRM device
+ * @dev: drm device
  * @crtc: CRTC in question
  */
 static int drm_vblank_enable(struct drm_device *dev, int crtc)
@@ -876,13 +910,13 @@ static int drm_vblank_enable(struct drm_device *dev, int crtc)
 
 /**
  * drm_vblank_get - get a reference count on vblank events
- * @dev: DRM device
+ * @dev: drm device
  * @crtc: which CRTC to own
  *
  * Acquire a reference count on vblank events to avoid having them disabled
  * while in use.
  *
- * RETURNS
+ * Returns:
  * Zero on success, nonzero on failure.
  */
 int drm_vblank_get(struct drm_device *dev, int crtc)
@@ -908,7 +942,7 @@ EXPORT_SYMBOL(drm_vblank_get);
 
 /**
  * drm_vblank_put - give up ownership of vblank events
- * @dev: DRM device
+ * @dev: drm device
  * @crtc: which counter to give up
  *
  * Release ownership of a given vblank counter, turning off interrupts
@@ -928,8 +962,15 @@ EXPORT_SYMBOL(drm_vblank_put);
 
 /**
  * drm_vblank_off - disable vblank events on a CRTC
- * @dev: DRM device
+ * @dev: drm device
  * @crtc: CRTC in question
+ *
+ * Drivers can use this function to shut down the vblank interrupt handling when
+ * disabling a crtc. This function ensures that the latest vblank frame count is
+ * stored so that drm_vblank_on can restore it again.
+ *
+ * Drivers must use this function when the hardware vblank counter can get
+ * reset, e.g. when suspending.
  */
 void drm_vblank_off(struct drm_device *dev, int crtc)
 {
@@ -964,8 +1005,13 @@ EXPORT_SYMBOL(drm_vblank_off);
 
 /**
  * drm_vblank_on - enable vblank events on a CRTC
- * @dev: DRM device
+ * @dev: drm device
  * @crtc: CRTC in question
+ *
+ * This functions restores the vblank interrupt state captured with
+ * drm_vblank_off() again. Note that calls to drm_vblank_on() and
+ * drm_vblank_off() can be unbalanced and so can also be unconditionaly called
+ * in driver load code to reflect the current hardware state of the crtc.
  */
 void drm_vblank_on(struct drm_device *dev, int crtc)
 {
@@ -981,11 +1027,26 @@ EXPORT_SYMBOL(drm_vblank_on);
 
 /**
  * drm_vblank_pre_modeset - account for vblanks across mode sets
- * @dev: DRM device
+ * @dev: drm device
  * @crtc: CRTC in question
  *
  * Account for vblank events across mode setting events, which will likely
  * reset the hardware frame counter.
+ *
+ * This is done by grabbing a temporary vblank reference to ensure that the
+ * vblank interrupt keeps running across the modeset sequence. With this the
+ * software-side vblank frame counting will ensure that there are no jumps or
+ * discontinuities.
+ *
+ * Unfortunately this approach is racy and also doesn't work when the vblank
+ * interrupt stops running, e.g. across system suspend resume. It is therefore
+ * highly recommended that drivers use the newer drm_vblank_off() and
+ * drm_vblank_on() instead. drm_vblank_pre_modeset() only works correctly when
+ * using "cooked" software vblank frame counters and not relying on any hardware
+ * counters.
+ *
+ * Drivers must call drm_vblank_post_modeset() when re-enabling the same crtc
+ * again.
  */
 void drm_vblank_pre_modeset(struct drm_device *dev, int crtc)
 {
@@ -1007,6 +1068,14 @@ void drm_vblank_pre_modeset(struct drm_device *dev, int crtc)
 }
 EXPORT_SYMBOL(drm_vblank_pre_modeset);
 
+/**
+ * drm_vblank_post_modeset - undo drm_vblank_pre_modeset changes
+ * @dev: drm device
+ * @crtc: CRTC in question
+ *
+ * This function again drops the temporary vblank reference acquired in
+ * drm_vblank_pre_modeset.
+ */
 void drm_vblank_post_modeset(struct drm_device *dev, int crtc)
 {
 	unsigned long irqflags;
@@ -1028,7 +1097,7 @@ void drm_vblank_post_modeset(struct drm_device *dev, int crtc)
 }
 EXPORT_SYMBOL(drm_vblank_post_modeset);
 
-/**
+/*
  * drm_modeset_ctl - handle vblank event counter changes across mode switch
  * @DRM_IOCTL_ARGS: standard ioctl arguments
  *
@@ -1141,11 +1210,11 @@ err_put:
 	return ret;
 }
 
-/**
+/*
  * Wait for VBLANK.
  *
  * \param inode device inode.
- * \param file_priv DRM file private.
+ * \param file_priv drm file private.
  * \param cmd command.
  * \param data user argument, pointing to a drm_wait_vblank structure.
  * \return zero on success or a negative number on failure.
@@ -1276,7 +1345,7 @@ static void drm_handle_vblank_events(struct drm_device *dev, int crtc)
 
 /**
  * drm_handle_vblank - handle a vblank event
- * @dev: DRM device
+ * @dev: drm device
  * @crtc: where this event occurred
  *
  * Drivers should call this routine in their vblank interrupt handlers to
-- 
1.8.3.1

  parent reply	other threads:[~2014-05-14 18:51 UTC|newest]

Thread overview: 52+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-05-14 18:51 [PATCH 00/12] irq vblank handling rework Daniel Vetter
2014-05-14 18:51 ` [PATCH 01/12] drm: Use correct spinlock flavor in drm_vblank_get() Daniel Vetter
2014-05-21 11:08   ` Thierry Reding
2014-05-14 18:51 ` [PATCH 02/12] drm: Make the vblank disable timer per-crtc Daniel Vetter
2014-05-21 11:17   ` Thierry Reding
2014-05-21 12:10     ` Daniel Vetter
2014-05-14 18:51 ` [PATCH 03/12] drm: Make blocking vblank wait return when the vblank interrupts get disabled Daniel Vetter
2014-05-21 11:20   ` Thierry Reding
2014-05-21 11:24     ` Thierry Reding
2014-05-14 18:51 ` [PATCH 04/12] drm: Add drm_vblank_on() Daniel Vetter
2014-05-14 18:55   ` Daniel Vetter
2014-05-21 11:32   ` Thierry Reding
2014-05-21 12:15     ` Daniel Vetter
2014-05-14 18:51 ` [PATCH 05/12] drm/i915: Remove drm_vblank_pre/post_modeset calls Daniel Vetter
2014-05-21 11:40   ` Thierry Reding
2014-05-14 18:51 ` [PATCH 06/12] drm/doc: Discourage usage of MODESET_CTL ioctl Daniel Vetter
2014-05-15  4:27   ` Michel Dänzer
2014-05-15 13:00   ` [PATCH] " Daniel Vetter
2014-05-15 20:36     ` Laurent Pinchart
2014-05-14 18:51 ` Daniel Vetter [this message]
2014-05-15  4:44   ` [PATCH 07/12] drm/irq: kerneldoc polish Michel Dänzer
2014-05-15  9:55     ` Daniel Vetter
2014-05-15  7:21   ` Thierry Reding
2014-05-15 13:00   ` [PATCH] " Daniel Vetter
2014-05-14 18:51 ` [PATCH 08/12] drm/irq: Add kms-native crtc interface functions Daniel Vetter
2014-05-15  7:34   ` Thierry Reding
2014-05-15 10:10     ` Daniel Vetter
2014-05-15 10:42       ` Thierry Reding
2014-05-15 11:11         ` Daniel Vetter
2014-05-15 13:34   ` [PATCH 1/2] " Daniel Vetter
2014-05-15 13:34     ` [PATCH 2/2] drm/i915: Use new kms-native vblank functions Daniel Vetter
2014-05-14 18:51 ` [PATCH 09/12] drm/i915: rip our vblank reset hacks for runtime PM Daniel Vetter
2014-05-20 12:03   ` [Intel-gfx] " Ville Syrjälä
2014-05-20 13:38     ` Daniel Vetter
2014-05-20 14:03       ` Ville Syrjälä
2014-05-20 14:20         ` Daniel Vetter
2014-05-20 15:17           ` Daniel Vetter
2014-05-21 12:49     ` Thierry Reding
2014-05-14 18:51 ` [PATCH 09/12] drm/irq: Lack of interrupt support in drm_vblank_on|off Daniel Vetter
2014-05-21 12:49   ` Thierry Reding
2014-05-14 18:51 ` [PATCH 10/12] drm/i915: Accurately initialize fifo underrun state on gmch platforms Daniel Vetter
2014-05-14 18:51 ` [PATCH 10/12] drm/i915: rip our vblank reset hacks for runtime PM Daniel Vetter
2014-05-14 18:51 ` [PATCH 11/12] drm/i915: Accurately initialize fifo underrun state on gmch platforms Daniel Vetter
2014-05-14 19:39   ` Imre Deak
2014-05-14 18:51 ` [PATCH 11/12] [RFC] drm/irq: More robustness in drm_vblank_on|off Daniel Vetter
2014-05-21 12:53   ` Thierry Reding
2014-05-21 13:11     ` Daniel Vetter
2014-05-14 18:51 ` [PATCH 12/12] [RFC] drm/crtc-helper: Enforce sane vblank counter semantics Daniel Vetter
2014-05-21  8:26 ` [PATCH 00/12] irq vblank handling rework Ville Syrjälä
2014-05-21  8:35   ` Daniel Vetter
2014-05-21  8:58     ` Ville Syrjälä
2014-05-21 12:55     ` Thierry Reding

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=1400093477-3217-8-git-send-email-daniel.vetter@ffwll.ch \
    --to=daniel.vetter@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --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.