[PATCH v2 0/3] drm: kernel-doc stuff

[PATCH v2 0/3] drm: kernel-doc stuff

[Sam Ravnborg]

Lyude - I added a "Co-developed-by: ..."
Can I get your s-o-b on the patch, to document that
you are OK with this to go in.
Needed on top of an r-b.

v2:
  - committed acked/reviewed patches
  - significantly updated the vbalnk patch based on a lot of good inputs
  - updated the writeback patch
  - wired drm_writeback.h to kernel-doc

        Sam

Sam Ravnborg (3):
      drm/vblank: Add intro to documentation
      drm: writeback: document callbacks
      drm/writeback: wire drm_writeback.h to kernel-doc

 Documentation/gpu/drm-kms.rst            |  3 ++
 drivers/gpu/drm/drm_vblank.c             | 53 ++++++++++++++++++++++++++++++++
 include/drm/drm_modeset_helper_vtables.h | 27 ++++++++++++++++
 include/drm/drm_writeback.h              |  9 ++++++
 4 files changed, 92 insertions(+)


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

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

v2:
  - Reworded to improve readability (Thomas)

v3:
  - Added nice ascii drawing from Lyude (Lyude)
  - Added referende to high-precision timestamp (Daniel)
  - Improved grammar (Thomas)
  - Combined it all and made kernel-doc happy
  - Dropped any a-b, r-b do to the amount of changes

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
Co-developed-by: Lyude Paul <lyude@redhat.com>
Cc: Lyude Paul <lyude@redhat.com>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <mripard@kernel.org>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: David Airlie <airlied@linux.ie>
---
 drivers/gpu/drm/drm_vblank.c | 53 ++++++++++++++++++++++++++++++++++++
 1 file changed, 53 insertions(+)

diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
index bcf346b3e486..9633092c9ad5 100644
--- a/drivers/gpu/drm/drm_vblank.c
+++ b/drivers/gpu/drm/drm_vblank.c
@@ -41,6 +41,59 @@
 /**
  * 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.
+ * 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.
+ *
+ * ::
+ *
+ *
+ *    physical →   ⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽
+ *    top of      |                                        |
+ *    display     |                                        |
+ *                |               New frame                |
+ *                |                                        |
+ *                |↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓|
+ *                |~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~| ← Scanline, updates
+ *                |↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓|   the frame as it
+ *                |                                        |   travels down
+ *                |                                        |   ("scan out")
+ *                |                                        |
+ *                |               Old frame                |
+ *                |                                        |
+ *                |                                        |
+ *                |                                        |
+ *                |                                        |   physical
+ *                |                                        |   bottom of
+ *    vertical    |⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽| ← display
+ *    blanking    ┆xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx┆
+ *    region   →  ┆xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx┆
+ *                ┆xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx┆
+ *    start of →   ⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽
+ *    new frame
+ *
+ * "Physical top of display" is the reference point for the high-precision/
+ * corrected timestamp.
+ *
+ * On a lot of display hardware, programming needs to take effect during the
+ * vertical blanking period so that settings like gamma, the image buffer
+ * buffer to be scanned out, etc. can safely be changed without showing
+ * any visual artifacts on the screen. In some unforgiving hardware, some of
+ * this programming has to both start and end in the same vblank.
+ *
+ * The vblank interrupt may be fired at different points depending on the
+ * hardware. Some hardware implementations will fire the interrupt when the
+ * new frame start, other implementations will fire the interrupt at different
+ * points in time.
+ *
  * 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
-- 
2.20.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 2/3] drm: writeback: document callbacks

[Sam Ravnborg]

Document the callbacks:
    drm_connector_helper_funcs.prepare_writeback_job
    drm_connector_helper_funcs.cleanup_writeback_job

The documentation was pulled from the changelong introducing the
callbacks, originally written by Laurent.

Adding the missing documentation fixes the following warnings:
drm_modeset_helper_vtables.h:1052: warning: Function parameter or member 'prepare_writeback_job' not described in 'drm_connector_helper_funcs'
drm_modeset_helper_vtables.h:1052: warning: Function parameter or member 'cleanup_writeback_job' not described in 'drm_connector_helper_funcs'

v2:
  - Fix formatting (Daniel)
  - Drop changelog text and add reference (Daniel)
  - Improve grammar. and use "operation" (Laurent)

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Cc: Liviu Dudau <liviu.dudau@arm.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <mripard@kernel.org>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: David Airlie <airlied@linux.ie>
---
 include/drm/drm_modeset_helper_vtables.h | 27 ++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h
index 7c20b1c8b6a7..421a30f08463 100644
--- a/include/drm/drm_modeset_helper_vtables.h
+++ b/include/drm/drm_modeset_helper_vtables.h
@@ -1075,8 +1075,35 @@ struct drm_connector_helper_funcs {
 	void (*atomic_commit)(struct drm_connector *connector,
 			      struct drm_connector_state *state);
 
+	/**
+	 * @prepare_writeback_job:
+	 *
+	 * As writeback jobs contain a framebuffer, drivers may need to
+	 * prepare and clean them up the same way they can prepare and
+	 * clean up framebuffers for planes. This optional connector operation
+	 * is used to support the preparation of writeback jobs. The job
+	 * prepare operation is called from drm_atomic_helper_prepare_planes()
+	 * for struct &drm_writeback_connector connectors only.
+	 *
+	 * This operation is optional.
+	 *
+	 * This callback is used by the atomic modeset helpers.
+	 */
 	int (*prepare_writeback_job)(struct drm_writeback_connector *connector,
 				     struct drm_writeback_job *job);
+	/**
+	 * @cleanup_writeback_job:
+	 *
+	 * This optional connector operation is used to support the
+	 * cleanup of writeback jobs. The job cleanup operation is called
+	 * from the existing drm_writeback_cleanup_job() function, invoked
+	 * both when destroying the job as part of an aborted commit, or when
+	 * the job completes.
+	 *
+	 * This operation is optional.
+	 *
+	 * This callback is used by the atomic modeset helpers.
+	 */
 	void (*cleanup_writeback_job)(struct drm_writeback_connector *connector,
 				      struct drm_writeback_job *job);
 };
-- 
2.20.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 3/3] drm/writeback: wire drm_writeback.h to kernel-doc

[Sam Ravnborg]

drm_writeback.h included a lot of nice kernel-doc comments.
Wire it up so the header file is included in the kernel-doc
generated documentation.

Added a few simple comments to the two structs so they
get picked up by kernel-doc.

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
Cc: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Cc: Brian Starkey <brian.starkey@arm.com>
Cc: Liviu Dudau <liviu.dudau@arm.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Maxime Ripard <mripard@kernel.org>
Cc: Maxime Ripard <mripard@kernel.org>
---
 Documentation/gpu/drm-kms.rst | 3 +++
 include/drm/drm_writeback.h   | 9 +++++++++
 2 files changed, 12 insertions(+)

diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
index e1f685015807..397314d08f77 100644
--- a/Documentation/gpu/drm-kms.rst
+++ b/Documentation/gpu/drm-kms.rst
@@ -397,6 +397,9 @@ Connector Functions Reference
 Writeback Connectors
 --------------------
 
+.. kernel-doc:: include/drm/drm_writeback.h
+  :internal:
+
 .. kernel-doc:: drivers/gpu/drm/drm_writeback.c
   :doc: overview
 
diff --git a/include/drm/drm_writeback.h b/include/drm/drm_writeback.h
index 777c14c847f0..9697d2714d2a 100644
--- a/include/drm/drm_writeback.h
+++ b/include/drm/drm_writeback.h
@@ -15,7 +15,13 @@
 #include <drm/drm_encoder.h>
 #include <linux/workqueue.h>
 
+/**
+ * struct drm_writeback_connector - DRM writeback connector
+ */
 struct drm_writeback_connector {
+	/**
+	 * @base: base drm_connector object
+	 */
 	struct drm_connector base;
 
 	/**
@@ -78,6 +84,9 @@ struct drm_writeback_connector {
 	char timeline_name[32];
 };
 
+/**
+ * struct drm_writeback_job - DRM writeback job
+ */
 struct drm_writeback_job {
 	/**
 	 * @connector:
-- 
2.20.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Lyude Paul]

On Mon, 2020-04-06 at 21:47 +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)
> 
> v3:
>   - Added nice ascii drawing from Lyude (Lyude)
>   - Added referende to high-precision timestamp (Daniel)
>   - Improved grammar (Thomas)
>   - Combined it all and made kernel-doc happy
>   - Dropped any a-b, r-b do to the amount of changes
> 
> Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
> Co-developed-by: Lyude Paul <lyude@redhat.com>
> Cc: Lyude Paul <lyude@redhat.com>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: David Airlie <airlied@linux.ie>
> ---
>  drivers/gpu/drm/drm_vblank.c | 53 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 53 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> index bcf346b3e486..9633092c9ad5 100644
> --- a/drivers/gpu/drm/drm_vblank.c
> +++ b/drivers/gpu/drm/drm_vblank.c
> @@ -41,6 +41,59 @@
>  /**
>   * 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.

s/have/has/

Other then that:

Reviewed-by: Lyude Paul <lyude@redhat.com>

> + * 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.
> + * 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.
> + *
> + * ::
> + *
> + *
> + *    physical →   ⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽
> + *    top of      |                                        |
> + *    display     |                                        |
> + *                |               New frame                |
> + *                |                                        |
> + *                |↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓|
> + *                |~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~| ← Scanline,
> updates
> + *                |↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓|   the frame as
> it
> + *                |                                        |   travels down
> + *                |                                        |   ("scan out")
> + *                |                                        |
> + *                |               Old frame                |
> + *                |                                        |
> + *                |                                        |
> + *                |                                        |
> + *                |                                        |   physical
> + *                |                                        |   bottom of
> + *    vertical    |⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽| ← display
> + *    blanking    ┆xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx┆
> + *    region   →  ┆xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx┆
> + *                ┆xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx┆
> + *    start of →   ⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽
> + *    new frame
> + *
> + * "Physical top of display" is the reference point for the high-precision/
> + * corrected timestamp.
> + *
> + * On a lot of display hardware, programming needs to take effect during
> the
> + * vertical blanking period so that settings like gamma, the image buffer
> + * buffer to be scanned out, etc. can safely be changed without showing
> + * any visual artifacts on the screen. In some unforgiving hardware, some
> of
> + * this programming has to both start and end in the same vblank.
> + *
> + * The vblank interrupt may be fired at different points depending on the
> + * hardware. Some hardware implementations will fire the interrupt when the
> + * new frame start, other implementations will fire the interrupt at
> different
> + * points in time.
> + *
>   * 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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Thomas Zimmermann]

[-- Attachment #1.1.1: Type: text/plain, Size: 5416 bytes --]



Am 06.04.20 um 21:47 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.
> 
> v2:
>   - Reworded to improve readability (Thomas)
> 
> v3:
>   - Added nice ascii drawing from Lyude (Lyude)
>   - Added referende to high-precision timestamp (Daniel)
>   - Improved grammar (Thomas)
>   - Combined it all and made kernel-doc happy
>   - Dropped any a-b, r-b do to the amount of changes
> 
> Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
> Co-developed-by: Lyude Paul <lyude@redhat.com>
> Cc: Lyude Paul <lyude@redhat.com>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: David Airlie <airlied@linux.ie>

Acked-by: Thomas Zimmermann <tzimmermann@suse.de>

> ---
>  drivers/gpu/drm/drm_vblank.c | 53 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 53 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> index bcf346b3e486..9633092c9ad5 100644
> --- a/drivers/gpu/drm/drm_vblank.c
> +++ b/drivers/gpu/drm/drm_vblank.c
> @@ -41,6 +41,59 @@
>  /**
>   * 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.
> + * 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.
> + *
> + * ::
> + *
> + *
> + *    physical →   ⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽
> + *    top of      |                                        |
> + *    display     |                                        |
> + *                |               New frame                |
> + *                |                                        |
> + *                |↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓|
> + *                |~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~| ← Scanline, updates
> + *                |↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓|   the frame as it
> + *                |                                        |   travels down
> + *                |                                        |   ("scan out")
> + *                |                                        |
> + *                |               Old frame                |
> + *                |                                        |
> + *                |                                        |
> + *                |                                        |
> + *                |                                        |   physical
> + *                |                                        |   bottom of
> + *    vertical    |⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽| ← display
> + *    blanking    ┆xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx┆
> + *    region   →  ┆xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx┆
> + *                ┆xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx┆
> + *    start of →   ⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽
> + *    new frame
> + *
> + * "Physical top of display" is the reference point for the high-precision/
> + * corrected timestamp.
> + *
> + * On a lot of display hardware, programming needs to take effect during the
> + * vertical blanking period so that settings like gamma, the image buffer
> + * buffer to be scanned out, etc. can safely be changed without showing
> + * any visual artifacts on the screen. In some unforgiving hardware, some of
> + * this programming has to both start and end in the same vblank.
> + *
> + * The vblank interrupt may be fired at different points depending on the
> + * hardware. Some hardware implementations will fire the interrupt when the
> + * new frame start, other implementations will fire the interrupt at different
> + * points in time.
> + *
>   * 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
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 3/3] drm/writeback: wire drm_writeback.h to kernel-doc

[Daniel Vetter]

On Mon, Apr 06, 2020 at 09:47:46PM +0200, Sam Ravnborg wrote:
> drm_writeback.h included a lot of nice kernel-doc comments.
> Wire it up so the header file is included in the kernel-doc
> generated documentation.
> 
> Added a few simple comments to the two structs so they
> get picked up by kernel-doc.
> 
> Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
> Cc: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> Cc: Brian Starkey <brian.starkey@arm.com>
> Cc: Liviu Dudau <liviu.dudau@arm.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Maxime Ripard <mripard@kernel.org>

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  Documentation/gpu/drm-kms.rst | 3 +++
>  include/drm/drm_writeback.h   | 9 +++++++++
>  2 files changed, 12 insertions(+)
> 
> diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
> index e1f685015807..397314d08f77 100644
> --- a/Documentation/gpu/drm-kms.rst
> +++ b/Documentation/gpu/drm-kms.rst
> @@ -397,6 +397,9 @@ Connector Functions Reference
>  Writeback Connectors
>  --------------------
>  
> +.. kernel-doc:: include/drm/drm_writeback.h
> +  :internal:
> +
>  .. kernel-doc:: drivers/gpu/drm/drm_writeback.c
>    :doc: overview
>  
> diff --git a/include/drm/drm_writeback.h b/include/drm/drm_writeback.h
> index 777c14c847f0..9697d2714d2a 100644
> --- a/include/drm/drm_writeback.h
> +++ b/include/drm/drm_writeback.h
> @@ -15,7 +15,13 @@
>  #include <drm/drm_encoder.h>
>  #include <linux/workqueue.h>
>  
> +/**
> + * struct drm_writeback_connector - DRM writeback connector
> + */
>  struct drm_writeback_connector {
> +	/**
> +	 * @base: base drm_connector object
> +	 */
>  	struct drm_connector base;
>  
>  	/**
> @@ -78,6 +84,9 @@ struct drm_writeback_connector {
>  	char timeline_name[32];
>  };
>  
> +/**
> + * struct drm_writeback_job - DRM writeback job
> + */
>  struct drm_writeback_job {
>  	/**
>  	 * @connector:
> -- 
> 2.20.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Liviu Dudau]

Hi Sam,

Sorry for jumping in late on this, I have just a small suggestion:

On Mon, Apr 06, 2020 at 09:47:44PM +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)
> 
> v3:
>   - Added nice ascii drawing from Lyude (Lyude)
>   - Added referende to high-precision timestamp (Daniel)
>   - Improved grammar (Thomas)
>   - Combined it all and made kernel-doc happy
>   - Dropped any a-b, r-b do to the amount of changes
> 
> Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
> Co-developed-by: Lyude Paul <lyude@redhat.com>
> Cc: Lyude Paul <lyude@redhat.com>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: David Airlie <airlied@linux.ie>
> ---
>  drivers/gpu/drm/drm_vblank.c | 53 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 53 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> index bcf346b3e486..9633092c9ad5 100644
> --- a/drivers/gpu/drm/drm_vblank.c
> +++ b/drivers/gpu/drm/drm_vblank.c
> @@ -41,6 +41,59 @@
>  /**
>   * 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.
> + * 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.
> + *
> + * ::
> + *
> + *
> + *    physical →   ⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽
> + *    top of      |                                        |
> + *    display     |                                        |
> + *                |               New frame                |
> + *                |                                        |
> + *                |↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓|
> + *                |~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~| ← Scanline, updates
> + *                |↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓|   the frame as it
> + *                |                                        |   travels down
> + *                |                                        |   ("scan out")
> + *                |                                        |
> + *                |               Old frame                |
> + *                |                                        |
> + *                |                                        |
> + *                |                                        |
> + *                |                                        |   physical
> + *                |                                        |   bottom of
> + *    vertical    |⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽| ← display
> + *    blanking    ┆xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx┆
> + *    region   →  ┆xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx┆
> + *                ┆xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx┆
> + *    start of →   ⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽
> + *    new frame
> + *
> + * "Physical top of display" is the reference point for the high-precision/
> + * corrected timestamp.
> + *
> + * On a lot of display hardware, programming needs to take effect during the
> + * vertical blanking period so that settings like gamma, the image buffer
> + * buffer to be scanned out, etc. can safely be changed without showing
> + * any visual artifacts on the screen. In some unforgiving hardware, some of
> + * this programming has to both start and end in the same vblank.

The mention of vblank interrupt in the next paragraph seems a bit sudden to me. Maybe
you can add at the end of the paragraph above something like:

   To help with the timing of the hardware programming, an interrupt is usually
   available to notify the driver when it can start the updating of registers.


> + *
> + * The vblank interrupt may be fired at different points depending on the
> + * hardware. Some hardware implementations will fire the interrupt when the
> + * new frame start, other implementations will fire the interrupt at different
> + * points in time.
> + *
>   * 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
> -- 
> 2.20.1
> 

Best regards,
Liviu

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 2/3] drm: writeback: document callbacks

[Liviu Dudau]

On Mon, Apr 06, 2020 at 09:47:45PM +0200, Sam Ravnborg wrote:
> Document the callbacks:
>     drm_connector_helper_funcs.prepare_writeback_job
>     drm_connector_helper_funcs.cleanup_writeback_job
> 
> The documentation was pulled from the changelong introducing the
> callbacks, originally written by Laurent.
> 
> Adding the missing documentation fixes the following warnings:
> drm_modeset_helper_vtables.h:1052: warning: Function parameter or member 'prepare_writeback_job' not described in 'drm_connector_helper_funcs'
> drm_modeset_helper_vtables.h:1052: warning: Function parameter or member 'cleanup_writeback_job' not described in 'drm_connector_helper_funcs'
> 
> v2:
>   - Fix formatting (Daniel)
>   - Drop changelog text and add reference (Daniel)
>   - Improve grammar. and use "operation" (Laurent)
> 
> Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> Cc: Liviu Dudau <liviu.dudau@arm.com>

Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>

Thanks!
Liviu

> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: David Airlie <airlied@linux.ie>
> ---
>  include/drm/drm_modeset_helper_vtables.h | 27 ++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h
> index 7c20b1c8b6a7..421a30f08463 100644
> --- a/include/drm/drm_modeset_helper_vtables.h
> +++ b/include/drm/drm_modeset_helper_vtables.h
> @@ -1075,8 +1075,35 @@ struct drm_connector_helper_funcs {
>  	void (*atomic_commit)(struct drm_connector *connector,
>  			      struct drm_connector_state *state);
>  
> +	/**
> +	 * @prepare_writeback_job:
> +	 *
> +	 * As writeback jobs contain a framebuffer, drivers may need to
> +	 * prepare and clean them up the same way they can prepare and
> +	 * clean up framebuffers for planes. This optional connector operation
> +	 * is used to support the preparation of writeback jobs. The job
> +	 * prepare operation is called from drm_atomic_helper_prepare_planes()
> +	 * for struct &drm_writeback_connector connectors only.
> +	 *
> +	 * This operation is optional.
> +	 *
> +	 * This callback is used by the atomic modeset helpers.
> +	 */
>  	int (*prepare_writeback_job)(struct drm_writeback_connector *connector,
>  				     struct drm_writeback_job *job);
> +	/**
> +	 * @cleanup_writeback_job:
> +	 *
> +	 * This optional connector operation is used to support the
> +	 * cleanup of writeback jobs. The job cleanup operation is called
> +	 * from the existing drm_writeback_cleanup_job() function, invoked
> +	 * both when destroying the job as part of an aborted commit, or when
> +	 * the job completes.
> +	 *
> +	 * This operation is optional.
> +	 *
> +	 * This callback is used by the atomic modeset helpers.
> +	 */
>  	void (*cleanup_writeback_job)(struct drm_writeback_connector *connector,
>  				      struct drm_writeback_job *job);
>  };
> -- 
> 2.20.1
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Alex Deucher]

On Mon, Apr 6, 2020 at 3:48 PM Sam Ravnborg <sam@ravnborg.org> 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)
>
> v3:
>   - Added nice ascii drawing from Lyude (Lyude)
>   - Added referende to high-precision timestamp (Daniel)
>   - Improved grammar (Thomas)
>   - Combined it all and made kernel-doc happy
>   - Dropped any a-b, r-b do to the amount of changes
>
> Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
> Co-developed-by: Lyude Paul <lyude@redhat.com>
> Cc: Lyude Paul <lyude@redhat.com>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: David Airlie <airlied@linux.ie>
> ---
>  drivers/gpu/drm/drm_vblank.c | 53 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 53 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> index bcf346b3e486..9633092c9ad5 100644
> --- a/drivers/gpu/drm/drm_vblank.c
> +++ b/drivers/gpu/drm/drm_vblank.c
> @@ -41,6 +41,59 @@
>  /**
>   * 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.
> + * 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.

For historical reference, the vertical blanking period was designed to
give the electron gun (on CRTs) enough time to move back to the top of
the screen to start scanning out the next frame.  Similar for
horizontal blanking periods.  They were designed to give the electron
gun enough time to move back to the other side of the screen to start
scanning the next scanline.  Might be worth adding something like
that.  Either way:
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

> + *
> + * ::
> + *
> + *
> + *    physical →   ⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽
> + *    top of      |                                        |
> + *    display     |                                        |
> + *                |               New frame                |
> + *                |                                        |
> + *                |↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓|
> + *                |~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~| ← Scanline, updates
> + *                |↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓|   the frame as it
> + *                |                                        |   travels down
> + *                |                                        |   ("scan out")
> + *                |                                        |
> + *                |               Old frame                |
> + *                |                                        |
> + *                |                                        |
> + *                |                                        |
> + *                |                                        |   physical
> + *                |                                        |   bottom of
> + *    vertical    |⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽| ← display
> + *    blanking    ┆xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx┆
> + *    region   →  ┆xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx┆
> + *                ┆xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx┆
> + *    start of →   ⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽
> + *    new frame
> + *
> + * "Physical top of display" is the reference point for the high-precision/
> + * corrected timestamp.
> + *
> + * On a lot of display hardware, programming needs to take effect during the
> + * vertical blanking period so that settings like gamma, the image buffer
> + * buffer to be scanned out, etc. can safely be changed without showing
> + * any visual artifacts on the screen. In some unforgiving hardware, some of
> + * this programming has to both start and end in the same vblank.
> + *
> + * The vblank interrupt may be fired at different points depending on the
> + * hardware. Some hardware implementations will fire the interrupt when the
> + * new frame start, other implementations will fire the interrupt at different
> + * points in time.
> + *
>   * 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
> --
> 2.20.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 2/3] drm: writeback: document callbacks

[Sam Ravnborg]

On Tue, Apr 07, 2020 at 01:04:31PM +0100, Liviu Dudau wrote:
> On Mon, Apr 06, 2020 at 09:47:45PM +0200, Sam Ravnborg wrote:
> > Document the callbacks:
> >     drm_connector_helper_funcs.prepare_writeback_job
> >     drm_connector_helper_funcs.cleanup_writeback_job
> > 
> > The documentation was pulled from the changelong introducing the
> > callbacks, originally written by Laurent.
> > 
> > Adding the missing documentation fixes the following warnings:
> > drm_modeset_helper_vtables.h:1052: warning: Function parameter or member 'prepare_writeback_job' not described in 'drm_connector_helper_funcs'
> > drm_modeset_helper_vtables.h:1052: warning: Function parameter or member 'cleanup_writeback_job' not described in 'drm_connector_helper_funcs'
> > 
> > v2:
> >   - Fix formatting (Daniel)
> >   - Drop changelog text and add reference (Daniel)
> >   - Improve grammar. and use "operation" (Laurent)
> > 
> > Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
> > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Cc: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > Cc: Liviu Dudau <liviu.dudau@arm.com>
> 
> Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>
Thanks, applied to drm-misc-next and pushed out.

	Sam

> 
> Thanks!
> Liviu
> 
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Cc: Maxime Ripard <mripard@kernel.org>
> > Cc: Thomas Zimmermann <tzimmermann@suse.de>
> > Cc: David Airlie <airlied@linux.ie>
> > ---
> >  include/drm/drm_modeset_helper_vtables.h | 27 ++++++++++++++++++++++++
> >  1 file changed, 27 insertions(+)
> > 
> > diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h
> > index 7c20b1c8b6a7..421a30f08463 100644
> > --- a/include/drm/drm_modeset_helper_vtables.h
> > +++ b/include/drm/drm_modeset_helper_vtables.h
> > @@ -1075,8 +1075,35 @@ struct drm_connector_helper_funcs {
> >  	void (*atomic_commit)(struct drm_connector *connector,
> >  			      struct drm_connector_state *state);
> >  
> > +	/**
> > +	 * @prepare_writeback_job:
> > +	 *
> > +	 * As writeback jobs contain a framebuffer, drivers may need to
> > +	 * prepare and clean them up the same way they can prepare and
> > +	 * clean up framebuffers for planes. This optional connector operation
> > +	 * is used to support the preparation of writeback jobs. The job
> > +	 * prepare operation is called from drm_atomic_helper_prepare_planes()
> > +	 * for struct &drm_writeback_connector connectors only.
> > +	 *
> > +	 * This operation is optional.
> > +	 *
> > +	 * This callback is used by the atomic modeset helpers.
> > +	 */
> >  	int (*prepare_writeback_job)(struct drm_writeback_connector *connector,
> >  				     struct drm_writeback_job *job);
> > +	/**
> > +	 * @cleanup_writeback_job:
> > +	 *
> > +	 * This optional connector operation is used to support the
> > +	 * cleanup of writeback jobs. The job cleanup operation is called
> > +	 * from the existing drm_writeback_cleanup_job() function, invoked
> > +	 * both when destroying the job as part of an aborted commit, or when
> > +	 * the job completes.
> > +	 *
> > +	 * This operation is optional.
> > +	 *
> > +	 * This callback is used by the atomic modeset helpers.
> > +	 */
> >  	void (*cleanup_writeback_job)(struct drm_writeback_connector *connector,
> >  				      struct drm_writeback_job *job);
> >  };
> > -- 
> > 2.20.1
> > 
> 
> -- 
> ====================
> | I would like to |
> | fix the world,  |
> | but they're not |
> | giving me the   |
>  \ source code!  /
>   ---------------
>     ¯\_(ツ)_/¯
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 3/3] drm/writeback: wire drm_writeback.h to kernel-doc

[Sam Ravnborg]

On Tue, Apr 07, 2020 at 10:08:51AM +0200, Daniel Vetter wrote:
> On Mon, Apr 06, 2020 at 09:47:46PM +0200, Sam Ravnborg wrote:
> > drm_writeback.h included a lot of nice kernel-doc comments.
> > Wire it up so the header file is included in the kernel-doc
> > generated documentation.
> > 
> > Added a few simple comments to the two structs so they
> > get picked up by kernel-doc.
> > 
> > Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
> > Cc: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > Cc: Brian Starkey <brian.starkey@arm.com>
> > Cc: Liviu Dudau <liviu.dudau@arm.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: Thomas Zimmermann <tzimmermann@suse.de>
> > Cc: Maxime Ripard <mripard@kernel.org>
> > Cc: Maxime Ripard <mripard@kernel.org>
> 
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Thanks. Applied to drm-misc-next and pushed out.

	Sam

> > ---
> >  Documentation/gpu/drm-kms.rst | 3 +++
> >  include/drm/drm_writeback.h   | 9 +++++++++
> >  2 files changed, 12 insertions(+)
> > 
> > diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
> > index e1f685015807..397314d08f77 100644
> > --- a/Documentation/gpu/drm-kms.rst
> > +++ b/Documentation/gpu/drm-kms.rst
> > @@ -397,6 +397,9 @@ Connector Functions Reference
> >  Writeback Connectors
> >  --------------------
> >  
> > +.. kernel-doc:: include/drm/drm_writeback.h
> > +  :internal:
> > +
> >  .. kernel-doc:: drivers/gpu/drm/drm_writeback.c
> >    :doc: overview
> >  
> > diff --git a/include/drm/drm_writeback.h b/include/drm/drm_writeback.h
> > index 777c14c847f0..9697d2714d2a 100644
> > --- a/include/drm/drm_writeback.h
> > +++ b/include/drm/drm_writeback.h
> > @@ -15,7 +15,13 @@
> >  #include <drm/drm_encoder.h>
> >  #include <linux/workqueue.h>
> >  
> > +/**
> > + * struct drm_writeback_connector - DRM writeback connector
> > + */
> >  struct drm_writeback_connector {
> > +	/**
> > +	 * @base: base drm_connector object
> > +	 */
> >  	struct drm_connector base;
> >  
> >  	/**
> > @@ -78,6 +84,9 @@ struct drm_writeback_connector {
> >  	char timeline_name[32];
> >  };
> >  
> > +/**
> > + * struct drm_writeback_job - DRM writeback job
> > + */
> >  struct drm_writeback_job {
> >  	/**
> >  	 * @connector:
> > -- 
> > 2.20.1
> > 
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Sam Ravnborg]

Hi Alex.

On Tue, Apr 07, 2020 at 10:42:46AM -0400, Alex Deucher wrote:
> On Mon, Apr 6, 2020 at 3:48 PM Sam Ravnborg <sam@ravnborg.org> 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)
> >
> > v3:
> >   - Added nice ascii drawing from Lyude (Lyude)
> >   - Added referende to high-precision timestamp (Daniel)
> >   - Improved grammar (Thomas)
> >   - Combined it all and made kernel-doc happy
> >   - Dropped any a-b, r-b do to the amount of changes
> >
> > Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
> > Co-developed-by: Lyude Paul <lyude@redhat.com>
> > Cc: Lyude Paul <lyude@redhat.com>
> > Cc: Thomas Zimmermann <tzimmermann@suse.de>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Cc: Maxime Ripard <mripard@kernel.org>
> > Cc: Thomas Zimmermann <tzimmermann@suse.de>
> > Cc: David Airlie <airlied@linux.ie>
> > ---
> >  drivers/gpu/drm/drm_vblank.c | 53 ++++++++++++++++++++++++++++++++++++
> >  1 file changed, 53 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> > index bcf346b3e486..9633092c9ad5 100644
> > --- a/drivers/gpu/drm/drm_vblank.c
> > +++ b/drivers/gpu/drm/drm_vblank.c
> > @@ -41,6 +41,59 @@
> >  /**
> >   * 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.
> > + * 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.
> 
> For historical reference, the vertical blanking period was designed to
> give the electron gun (on CRTs) enough time to move back to the top of
> the screen to start scanning out the next frame.  Similar for
> horizontal blanking periods.  They were designed to give the electron
> gun enough time to move back to the other side of the screen to start
> scanning the next scanline.  Might be worth adding something like
> that.  Either way:
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

Thanks. Added this nice historical lesson and committed
and pushed out.
And to all, thanks for the inputs. We managed to improve this piece
of the documetation.

	Sam

> 
> > + *
> > + * ::
> > + *
> > + *
> > + *    physical →   ⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽
> > + *    top of      |                                        |
> > + *    display     |                                        |
> > + *                |               New frame                |
> > + *                |                                        |
> > + *                |↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓|
> > + *                |~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~| ← Scanline, updates
> > + *                |↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓|   the frame as it
> > + *                |                                        |   travels down
> > + *                |                                        |   ("scan out")
> > + *                |                                        |
> > + *                |               Old frame                |
> > + *                |                                        |
> > + *                |                                        |
> > + *                |                                        |
> > + *                |                                        |   physical
> > + *                |                                        |   bottom of
> > + *    vertical    |⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽| ← display
> > + *    blanking    ┆xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx┆
> > + *    region   →  ┆xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx┆
> > + *                ┆xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx┆
> > + *    start of →   ⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽
> > + *    new frame
> > + *
> > + * "Physical top of display" is the reference point for the high-precision/
> > + * corrected timestamp.
> > + *
> > + * On a lot of display hardware, programming needs to take effect during the
> > + * vertical blanking period so that settings like gamma, the image buffer
> > + * buffer to be scanned out, etc. can safely be changed without showing
> > + * any visual artifacts on the screen. In some unforgiving hardware, some of
> > + * this programming has to both start and end in the same vblank.
> > + *
> > + * The vblank interrupt may be fired at different points depending on the
> > + * hardware. Some hardware implementations will fire the interrupt when the
> > + * new frame start, other implementations will fire the interrupt at different
> > + * points in time.
> > + *
> >   * 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
> > --
> > 2.20.1
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel