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

Sam Ravnborg sam at ravnborg.org
Mon Mar 30 18:56:33 UTC 2020


Hi Thomas.

On Mon, Mar 30, 2020 at 01:29:16PM +0200, Thomas Zimmermann wrote:
> Hi Sam and Lyude,
> 
> thanks for improving the documentation. Below are a few points that I'd
> found more understandable. I'm no native speaker, though.
> 
> Am 28.03.20 um 14:20 schrieb Sam Ravnborg:
> > 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.
> > 
> > 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>
> > 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 | 15 +++++++++++++++
> >  1 file changed, 15 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> > index bcf346b3e486..95cac22d59d1 100644
> > --- a/drivers/gpu/drm/drm_vblank.c
> > +++ b/drivers/gpu/drm/drm_vblank.c
> > @@ -41,6 +41,21 @@
> >  /**
> >   * DOC: vblank handling
> >   *
> > + * From the perspective of a computer, every time a computer monitor displays
> 
> Possibly change from dative case to genitive:
> 
> "From the computer's perspective," ...
> 
> > + * a new frame it's done by "scanning out" the display image from top to
> > + * bottom, one row of pixels at a time. which row of pixels we're on is
> 
> s/which/Which
> 
> The text changes from third person (the computer) to first person
> (we're). Makes it harder to read. I'd remove both, "we" and "computer",
> and speak of display hardware or scanout engine.
> 
> > + * referred to as the scanline.
> 
> I'd say a scanline is any of them. Maybe say "current scanline"?
> 
> > + * Additionally, there's usually a couple of extra scanlines which we
> 
> "In addition to the display's visible area, there's usually a couple of
> extra scanlines that" ...
> 
> > + * scan out, but aren't actually displayed on the screen (these sometimes
> > + * get used by HDMI audio and friends, but that's another story).
> > + * The period where we're on these scanlines is referred to as the vblank.
> 
> I'd replace vblank with "vertical blanking period." That term is
> required in the next paragraph.
> 
> The time when the hardware operates on these invisible scanlines is
> referred to as vertical blanking period, or simply vblank.
> 
> > + *
> > + * On a lot of display hardware, programming needs to take effect during the
> > + * vertical blanking period so that settings like gamma, what frame we're
> 
> "we" again
> 
> > + * scanning 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.
> > + *
> >   * 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
> > 
> 
> In any case
> 
> Acked-by: Thomas Zimmermann <tzimmermann at suse.de>

Thanks for the detailed feedabck - I have tried to reword it so it
better fits the context and have taking into account your suggetions.
See other mail for the updated patch.

	Sam


More information about the dri-devel mailing list