diff mbox

[07/12] drm/irq: kerneldoc polish

Message ID 1400093477-3217-8-git-send-email-daniel.vetter@ffwll.ch (mailing list archive)
State New, archived
Headers show

Commit Message

Daniel Vetter May 14, 2014, 6:51 p.m. UTC
- 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(-)

Comments

Michel Dänzer May 15, 2014, 4:44 a.m. UTC | #1
On 15.05.2014 03:51, Daniel Vetter wrote:
> @@ -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.

Given this description, maybe something like drm_vblank_save/restore
would describe better what these functions do than drm_vblank_off/on?


> @@ -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.

That last statement is not true IME with radeon[0].

Basically, it sounds to me like drm_vblank_off/on do more or less what
drm_vblank_pre/post_modeset are intended to do (e.g. the latter can also
be nested arbitrarily). Still not really sure why we need two sets of
these instead of fixing any problems in one set.


[0] Though we certainly don't have as rigorous testing for that as you
guys do in intel-gpu-tools. Any chance some of that could be moved to
somewhere more generic?
Thierry Reding May 15, 2014, 7:21 a.m. UTC | #2
On Wed, May 14, 2014 at 08:51:09PM +0200, Daniel Vetter wrote:
[...]
> 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

Perhaps drop the filename here? It's completely redundant.

> +/**
> + * drm_vblank_cleanup - cleanup vblank support
> + * @dev: drm device

Technically DRM is an abbreviation and therefore should be all
uppercase.

> +/**
> + * drm_vblank_init - initialize vblank support
> + * @dev: drm_device

s/drm_device/DRM device/?

> + * @num_crtcs: number of crtcs supported by @dev

CRTC is also an abbreviation, so I think this should be s/crtcs/CRTCs/.

> + * Returns:

According to Documentation/kernel-doc-nano-HOWTO.txt this section should
be called "Return:". I'm not sure it matters all that much, since it
seems to be only used as the title of a new section, but I think it
makes sense to follow the recommendation of the documentation.

>  /**
> - * Install IRQ handler.
> - *
> - * \param dev DRM device.
> + * drm_irq_install - install IRQ handler
> + * @dev: drm device
> + * @irq: irq number to install the handler for

I know you're probably going to hate me for this, but: IRQ is an
abbreviation. =)

> -/**
> +/*
>   * IRQ control ioctl.
>   *
>   * \param inode device inode.

Perhaps it would still make sense to change the comment style to
kernel-doc, even if it isn't exported.

>  /**
> - * 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

I think s/drm_crtc * of// would be okay here.

>  /**
>   * 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.

drm_vblank_on() to have it correctly highlighted?

Thierry
Daniel Vetter May 15, 2014, 9:55 a.m. UTC | #3
On Thu, May 15, 2014 at 01:44:04PM +0900, Michel Dänzer wrote:
> On 15.05.2014 03:51, Daniel Vetter wrote:
> > @@ -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.
> 
> Given this description, maybe something like drm_vblank_save/restore
> would describe better what these functions do than drm_vblank_off/on?

It also enables/disables the vblank machinery itself, which at least in
i915 will be important shortly - we slowly switch over our code to be more
interrupt driven, including the initial plane enabling/disabling in
modeset changes.

> > @@ -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.
> 
> That last statement is not true IME with radeon[0].
> 
> Basically, it sounds to me like drm_vblank_off/on do more or less what
> drm_vblank_pre/post_modeset are intended to do (e.g. the latter can also
> be nested arbitrarily). Still not really sure why we need two sets of
> these instead of fixing any problems in one set.

The problem is that the driver situation is a mess. So the right plan imo
is:
1) Create new functions that work, beat on them.
2) Convert all drivers.
3) Rip out the old stuff.

I've tried to look into this a bit and decided that this isn't something I
can do without the hardware at hand and a few solid tests. So this series
is 1) done for i915, with an rfc for 2).

Also there's the issue of old ums using the same stuff and I think we
shouldn't touch that can of worms at all.

> [0] Though we certainly don't have as rigorous testing for that as you
> guys do in intel-gpu-tools. Any chance some of that could be moved to
> somewhere more generic?

igt is mostly ready - what we need is some rather thin abstraction for the
kms tests to run also on other drivers, with dumb objects. Otherwise all
the test framework can cope with random bits not being there and skipping
those subtests, e.g. the CRC based stuff.

I don't want to extract the generic parts of the tests into a different
repo (at least not if there's not _lots_ of people working on them) since
the code sharing we currently do between tests is fairly massive.

But I'm willing to deal with the hassle of supporting other drivers for
e.g. the kms tests. And I think it would make a nice gsoc project. But
it's definitely not something I can throw intel resource (or too much of
my own time) at ;-)

Having a shared set of tests which clearly spells out all the corner cases
and tests for races would imo be awesome and greatly improve the overall
health of the drm/kms drivers and wider ecosystem.
-Daniel
diff mbox

Patch

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