[PATCH v2 1/6] drm/vblank: Add intro to documentation

[Lyude Paul]

I am glad that my explanation of vblanks made sense! Some comments below on
things I think we could improve here

On Mon, 2020-03-30 at 20:57 +0200, Sam Ravnborg wrote:
> Lyude Paul wrote a very good intro to vblank here:
> 
https://lore.kernel.org/dri-devel/faf63d8a9ed23c16af69762f59d0dca6b2bf085f.camel@redhat.com/T/#mce6480be738160e9d07c5d023e88fd78d7a06d27
> 
> Add this to the intro chapter in drm_vblank.c so others
> can benefit from it too.
> 
> v2:
>   - Reworded to improve readability (Thomas)
> 
> Signed-off-by: Sam Ravnborg <sam at ravnborg.org>
> Co-developed-by: Lyude Paul <lyude at redhat.com>
> Cc: Lyude Paul <lyude at redhat.com>
> Acked-by: Thomas Zimmermann <tzimmermann at suse.de>
> Cc: Thomas Zimmermann <tzimmermann at suse.de>
> Cc: Daniel Vetter <daniel at ffwll.ch>
> Cc: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
> Cc: Maxime Ripard <mripard at kernel.org>
> Cc: Thomas Zimmermann <tzimmermann at suse.de>
> Cc: David Airlie <airlied at linux.ie>
> ---
>  drivers/gpu/drm/drm_vblank.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> index bcf346b3e486..ec2c2083b186 100644
> --- a/drivers/gpu/drm/drm_vblank.c
> +++ b/drivers/gpu/drm/drm_vblank.c
> @@ -41,6 +41,23 @@
>  /**
>   * DOC: vblank handling
>   *
> + * From the computer's perspective, every time the monitor displays
> + * a new frame the scanout engine have "scanned out" the display image
> + * from top to bottom, one row of pixels at a time.
> + * The current row of pixels is referred to as the current scanline.
> + *
> + * In addition to the display's visible area, there's usually a couple of
> + * extra scanlines which aren't actually displayed on the screen
> + * (the extra scanlines are sometimes used by HDMI audio and friends).
> + * The period where the extra scanlines are "scanned out" is referred
> + * to as the vertical blanking period (vblank for short).

I'd reword this, starting from "(the extra scanlines…" (I'd also remove the
paranthesis):

    These extra scanlines don't contain image data, and are occasionally used
    for features like audio and infoframes. The region made up of these
    scanlines is referred to as the vertical blanking region, or vblank for
    short.

I'd also add a simple ascii-art diagram here, since this might make a lot more
sense to some people if there's a visual reference. Probably something like
this (feel free to get a little creative)

     ⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽
    |                                                |
    |                                                |
    |                   New frame                    |
    |                                                |
    |↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓|
    |~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~| ← Scanline, updates the
    |↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓|   frame as it travels
    |                                                |   down (AKA "scans out")
    |                                                |
    |                                                |
    |                   Old frame                    |
    |                                                |
    |                                                |
    |                                                |
    |                                                |
    |                                                |   physical bottom of
    |⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽| ← display
    ┆xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx┆
    ┆xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx┆ ← vertical blanking
    ┆xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx┆   region
     ------------------------------------------------
> + *
> + * On a lot of display hardware, programming needs to take effect during
> the
> + * vertical blanking period so that settings like gamma, what frame being

s/what frame being/which frame is being/

> + * scanned out, etc. can be safely changed without showing visual tearing
> + * on the screen. In some unforgiving hardware, some of this programming
> has
> + * to both start and end in the same vblank - vertical blanking period.

You can just say vblank here, since we already explained what the vertical
blanking period is up above.

Alex Deucher pointed out to me that it's probably also worth mentioning that not
all hardware actually fires off the vblank interrupt at the start of the
vertical blank, depending on the hardware the interrupt could also happen a few
scanlines after the start of vblank, a few scanlines before the start, somewhere
in the middle, at the end of the vblank, etc.

Other then that, this looks great so far! Feel free to cc me in the respin for
this patch and I'll be happy to give my r-b.

> + *
>   * Vertical blanking plays a major role in graphics rendering. To achieve
>   * tear-free display, users must synchronize page flips and/or rendering to
>   * vertical blanking. The DRM API offers ioctls to perform page flips
-- 
Cheers,
	Lyude Paul (she/her)
	Associate Software Engineer at Red Hat