All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/8] drm/doc: improve plane property docs
@ 2020-12-16 20:22 Simon Ser
  2020-12-16 20:22 ` [PATCH 1/8] drm/doc: rename FB_DAMAGE_CLIPS section Simon Ser
                   ` (7 more replies)
  0 siblings, 8 replies; 20+ messages in thread
From: Simon Ser @ 2020-12-16 20:22 UTC (permalink / raw)
  To: dri-devel

Re-organize and improve plane property docs.

Simon Ser (8):
  drm/doc: rename FB_DAMAGE_CLIPS section
  drm/doc: move composition function docs to new section
  drm/doc: move damage tracking functions to new section
  drm/doc: move color management functions under CRTC section
  drm/doc: the KMS properties section is for user-space devs
  drm/doc: introduce new section for standard plane properties
  drm/doc: fix drm_plane_type docs
  drm/doc: document the type plane property

 Documentation/gpu/drm-kms.rst | 52 +++++++++++++++++++++++------------
 drivers/gpu/drm/drm_blend.c   |  6 ----
 drivers/gpu/drm/drm_plane.c   | 51 +++++++++++++++++++++++++++++++---
 include/drm/drm_plane.h       | 21 ++++++++------
 4 files changed, 95 insertions(+), 35 deletions(-)

-- 
2.29.2

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 1/8] drm/doc: rename FB_DAMAGE_CLIPS section
  2020-12-16 20:22 [PATCH 0/8] drm/doc: improve plane property docs Simon Ser
@ 2020-12-16 20:22 ` Simon Ser
  2020-12-16 20:22 ` [PATCH 2/8] drm/doc: move composition function docs to new section Simon Ser
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 20+ messages in thread
From: Simon Ser @ 2020-12-16 20:22 UTC (permalink / raw)
  To: dri-devel

Make it more human-readable.

Signed-off-by: Simon Ser <contact@emersion.fr>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Pekka Paalanen <ppaalanen@gmail.com>
---
 Documentation/gpu/drm-kms.rst | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
index 3c5ae4f6dfd2..76cf6acc23a5 100644
--- a/Documentation/gpu/drm-kms.rst
+++ b/Documentation/gpu/drm-kms.rst
@@ -475,8 +475,8 @@ Plane Composition Properties
 .. kernel-doc:: drivers/gpu/drm/drm_blend.c
    :export:
 
-FB_DAMAGE_CLIPS
-~~~~~~~~~~~~~~~
+Damage Tracking Properties
+--------------------------
 
 .. kernel-doc:: drivers/gpu/drm/drm_damage_helper.c
    :doc: overview
-- 
2.29.2

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 2/8] drm/doc: move composition function docs to new section
  2020-12-16 20:22 [PATCH 0/8] drm/doc: improve plane property docs Simon Ser
  2020-12-16 20:22 ` [PATCH 1/8] drm/doc: rename FB_DAMAGE_CLIPS section Simon Ser
@ 2020-12-16 20:22 ` Simon Ser
  2020-12-16 20:22 ` [PATCH 3/8] drm/doc: move damage tracking functions " Simon Ser
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 20+ messages in thread
From: Simon Ser @ 2020-12-16 20:22 UTC (permalink / raw)
  To: dri-devel

Move drm_blend.c function reference from the KMS properties section to
the plane abstraction section. This makes the KMS properties section
more readable for user-space developers.

Signed-off-by: Simon Ser <contact@emersion.fr>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Pekka Paalanen <ppaalanen@gmail.com>
---
 Documentation/gpu/drm-kms.rst | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
index 76cf6acc23a5..3f92d4eb224b 100644
--- a/Documentation/gpu/drm-kms.rst
+++ b/Documentation/gpu/drm-kms.rst
@@ -370,6 +370,12 @@ Plane Functions Reference
 .. kernel-doc:: drivers/gpu/drm/drm_plane.c
    :export:
 
+Plane Composition Functions Reference
+-------------------------------------
+
+.. kernel-doc:: drivers/gpu/drm/drm_blend.c
+   :export:
+
 Display Modes Function Reference
 ================================
 
@@ -472,9 +478,6 @@ Plane Composition Properties
 .. kernel-doc:: drivers/gpu/drm/drm_blend.c
    :doc: overview
 
-.. kernel-doc:: drivers/gpu/drm/drm_blend.c
-   :export:
-
 Damage Tracking Properties
 --------------------------
 
-- 
2.29.2

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 3/8] drm/doc: move damage tracking functions to new section
  2020-12-16 20:22 [PATCH 0/8] drm/doc: improve plane property docs Simon Ser
  2020-12-16 20:22 ` [PATCH 1/8] drm/doc: rename FB_DAMAGE_CLIPS section Simon Ser
  2020-12-16 20:22 ` [PATCH 2/8] drm/doc: move composition function docs to new section Simon Ser
@ 2020-12-16 20:22 ` Simon Ser
  2020-12-16 20:22 ` [PATCH 4/8] drm/doc: move color management functions under CRTC section Simon Ser
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 20+ messages in thread
From: Simon Ser @ 2020-12-16 20:22 UTC (permalink / raw)
  To: dri-devel

Move drm_damage_helper function reference from the KMS properties
section to the plane abstraction section. This makes the KMS
properties section more readable for user-space developers.

Signed-off-by: Simon Ser <contact@emersion.fr>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Pekka Paalanen <ppaalanen@gmail.com>
---
 Documentation/gpu/drm-kms.rst | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
index 3f92d4eb224b..e6329e7e638e 100644
--- a/Documentation/gpu/drm-kms.rst
+++ b/Documentation/gpu/drm-kms.rst
@@ -376,6 +376,15 @@ Plane Composition Functions Reference
 .. kernel-doc:: drivers/gpu/drm/drm_blend.c
    :export:
 
+Plane Damage Tracking Functions Reference
+-----------------------------------------
+
+.. kernel-doc:: drivers/gpu/drm/drm_damage_helper.c
+   :export:
+
+.. kernel-doc:: include/drm/drm_damage_helper.h
+   :internal:
+
 Display Modes Function Reference
 ================================
 
@@ -484,12 +493,6 @@ Damage Tracking Properties
 .. kernel-doc:: drivers/gpu/drm/drm_damage_helper.c
    :doc: overview
 
-.. kernel-doc:: drivers/gpu/drm/drm_damage_helper.c
-   :export:
-
-.. kernel-doc:: include/drm/drm_damage_helper.h
-   :internal:
-
 Color Management Properties
 ---------------------------
 
-- 
2.29.2

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 4/8] drm/doc: move color management functions under CRTC section
  2020-12-16 20:22 [PATCH 0/8] drm/doc: improve plane property docs Simon Ser
                   ` (2 preceding siblings ...)
  2020-12-16 20:22 ` [PATCH 3/8] drm/doc: move damage tracking functions " Simon Ser
@ 2020-12-16 20:22 ` Simon Ser
  2020-12-16 21:14   ` Daniel Vetter
  2020-12-16 20:22 ` [PATCH 5/8] drm/doc: the KMS properties section is for user-space devs Simon Ser
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 20+ messages in thread
From: Simon Ser @ 2020-12-16 20:22 UTC (permalink / raw)
  To: dri-devel

Move drm_color_mgmt function reference from the KMS properties
section to the CRTC abstraction section. This makes the KMS
properties section more readable for user-space developers.

Signed-off-by: Simon Ser <contact@emersion.fr>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Pekka Paalanen <ppaalanen@gmail.com>
---
 Documentation/gpu/drm-kms.rst | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
index e6329e7e638e..2f3efb63e5ba 100644
--- a/Documentation/gpu/drm-kms.rst
+++ b/Documentation/gpu/drm-kms.rst
@@ -319,6 +319,15 @@ CRTC Functions Reference
 .. kernel-doc:: drivers/gpu/drm/drm_crtc.c
    :export:
 
+Color Management Functions Reference
+------------------------------------
+
+.. kernel-doc:: drivers/gpu/drm/drm_color_mgmt.c
+   :export:
+
+.. kernel-doc:: include/drm/drm_color_mgmt.h
+   :internal:
+
 Frame Buffer Abstraction
 ========================
 
@@ -499,12 +508,6 @@ Color Management Properties
 .. kernel-doc:: drivers/gpu/drm/drm_color_mgmt.c
    :doc: overview
 
-.. kernel-doc:: drivers/gpu/drm/drm_color_mgmt.c
-   :export:
-
-.. kernel-doc:: include/drm/drm_color_mgmt.h
-   :internal:
-
 Tile Group Property
 -------------------
 
-- 
2.29.2

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 5/8] drm/doc: the KMS properties section is for user-space devs
  2020-12-16 20:22 [PATCH 0/8] drm/doc: improve plane property docs Simon Ser
                   ` (3 preceding siblings ...)
  2020-12-16 20:22 ` [PATCH 4/8] drm/doc: move color management functions under CRTC section Simon Ser
@ 2020-12-16 20:22 ` Simon Ser
  2020-12-16 21:14   ` Daniel Vetter
  2020-12-16 20:22 ` [PATCH 6/8] drm/doc: introduce new section for standard plane properties Simon Ser
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 20+ messages in thread
From: Simon Ser @ 2020-12-16 20:22 UTC (permalink / raw)
  To: dri-devel

State that the "KMS Properties" section is mainly for user-space
developers.

Signed-off-by: Simon Ser <contact@emersion.fr>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Pekka Paalanen <ppaalanen@gmail.com>
---
 Documentation/gpu/drm-kms.rst | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
index 2f3efb63e5ba..fcd4e15379b0 100644
--- a/Documentation/gpu/drm-kms.rst
+++ b/Documentation/gpu/drm-kms.rst
@@ -460,6 +460,9 @@ KMS Locking
 KMS Properties
 ==============
 
+This section of the documentation is primarily aimed at user-space developers.
+For the driver APIs, so the other sections.
+
 Property Types and Blob Property Support
 ----------------------------------------
 
-- 
2.29.2

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 6/8] drm/doc: introduce new section for standard plane properties
  2020-12-16 20:22 [PATCH 0/8] drm/doc: improve plane property docs Simon Ser
                   ` (4 preceding siblings ...)
  2020-12-16 20:22 ` [PATCH 5/8] drm/doc: the KMS properties section is for user-space devs Simon Ser
@ 2020-12-16 20:22 ` Simon Ser
  2020-12-16 21:19   ` Daniel Vetter
  2020-12-16 20:22 ` [PATCH 7/8] drm/doc: fix drm_plane_type docs Simon Ser
  2020-12-16 20:22 ` [PATCH 8/8] drm/doc: document the type plane property Simon Ser
  7 siblings, 1 reply; 20+ messages in thread
From: Simon Ser @ 2020-12-16 20:22 UTC (permalink / raw)
  To: dri-devel

Introduce a new "Standard Plane Properties" section for properties
defined in drm_plane.c. Move the mis-placed IN_FORMATS docs there.

Signed-off-by: Simon Ser <contact@emersion.fr>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Pekka Paalanen <ppaalanen@gmail.com>
---
 Documentation/gpu/drm-kms.rst |  6 ++++++
 drivers/gpu/drm/drm_blend.c   |  6 ------
 drivers/gpu/drm/drm_plane.c   | 12 ++++++++++++
 3 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
index fcd4e15379b0..9cfaab4df21a 100644
--- a/Documentation/gpu/drm-kms.rst
+++ b/Documentation/gpu/drm-kms.rst
@@ -493,6 +493,12 @@ Standard CRTC Properties
 .. kernel-doc:: drivers/gpu/drm/drm_crtc.c
    :doc: standard CRTC properties
 
+Standard Plane Properties
+-------------------------
+
+.. kernel-doc:: drivers/gpu/drm/drm_plane.c
+   :doc: standard plane properties
+
 Plane Composition Properties
 ----------------------------
 
diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c
index 5c2141e9a9f4..26e2f2ffd255 100644
--- a/drivers/gpu/drm/drm_blend.c
+++ b/drivers/gpu/drm/drm_blend.c
@@ -185,12 +185,6 @@
  *		 plane does not expose the "alpha" property, then this is
  *		 assumed to be 1.0
  *
- * IN_FORMATS:
- *	Blob property which contains the set of buffer format and modifier
- *	pairs supported by this plane. The blob is a drm_format_modifier_blob
- *	struct. Without this property the plane doesn't support buffers with
- *	modifiers. Userspace cannot change this property.
- *
  * Note that all the property extensions described here apply either to the
  * plane or the CRTC (e.g. for the background color, which currently is not
  * exposed and assumed to be black).
diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
index 49b0a8b9ac02..4c1a45ac18e6 100644
--- a/drivers/gpu/drm/drm_plane.c
+++ b/drivers/gpu/drm/drm_plane.c
@@ -61,6 +61,18 @@
  * userspace too much.
  */
 
+/**
+ * DOC: standard plane properties
+ *
+ * DRM planes have a few standardized properties:
+ *
+ * IN_FORMATS:
+ *     Blob property which contains the set of buffer format and modifier
+ *     pairs supported by this plane. The blob is a drm_format_modifier_blob
+ *     struct. Without this property the plane doesn't support buffers with
+ *     modifiers. Userspace cannot change this property.
+ */
+
 static unsigned int drm_num_planes(struct drm_device *dev)
 {
 	unsigned int num = 0;
-- 
2.29.2

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 7/8] drm/doc: fix drm_plane_type docs
  2020-12-16 20:22 [PATCH 0/8] drm/doc: improve plane property docs Simon Ser
                   ` (5 preceding siblings ...)
  2020-12-16 20:22 ` [PATCH 6/8] drm/doc: introduce new section for standard plane properties Simon Ser
@ 2020-12-16 20:22 ` Simon Ser
  2020-12-16 21:19   ` Daniel Vetter
  2020-12-16 20:22 ` [PATCH 8/8] drm/doc: document the type plane property Simon Ser
  7 siblings, 1 reply; 20+ messages in thread
From: Simon Ser @ 2020-12-16 20:22 UTC (permalink / raw)
  To: dri-devel

The docs for enum drm_plane_type mention legacy IOCTLs, however the
plane type is not tied to legacy IOCTLs, the drm_cursor.primary and
cursor fields are. Add a small paragraph to reference these.

Instead, document expectations for primary and cursor planes for
non-legacy userspace. Note that these docs are for driver developers,
not userspace developers, so internal kernel APIs are mentionned.

Signed-off-by: Simon Ser <contact@emersion.fr>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Pekka Paalanen <ppaalanen@gmail.com>
---
 include/drm/drm_plane.h | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
index 1d82b264e5e4..94fdd0c68450 100644
--- a/include/drm/drm_plane.h
+++ b/include/drm/drm_plane.h
@@ -538,10 +538,14 @@ struct drm_plane_funcs {
  *
  * For compatibility with legacy userspace, only overlay planes are made
  * available to userspace by default. Userspace clients may set the
- * DRM_CLIENT_CAP_UNIVERSAL_PLANES client capability bit to indicate that they
+ * &DRM_CLIENT_CAP_UNIVERSAL_PLANES client capability bit to indicate that they
  * wish to receive a universal plane list containing all plane types. See also
  * drm_for_each_legacy_plane().
  *
+ * In addition to setting each plane's type, drivers need to setup the
+ * &drm_crtc.primary and optionally &drm_crtc.cursor for legacy IOCTLs. See
+ * drm_crtc_init_with_planes().
+ *
  * WARNING: The values of this enum is UABI since they're exposed in the "type"
  * property.
  */
@@ -557,19 +561,20 @@ enum drm_plane_type {
 	/**
 	 * @DRM_PLANE_TYPE_PRIMARY:
 	 *
-	 * Primary planes represent a "main" plane for a CRTC.  Primary planes
-	 * are the planes operated upon by CRTC modesetting and flipping
-	 * operations described in the &drm_crtc_funcs.page_flip and
-	 * &drm_crtc_funcs.set_config hooks.
+	 * A primary plane attached to a CRTC is the most likely to be able to
+	 * light up the CRTC when no scaling/cropping is used and the plane
+	 * covers the whole CRTC.
 	 */
 	DRM_PLANE_TYPE_PRIMARY,
 
 	/**
 	 * @DRM_PLANE_TYPE_CURSOR:
 	 *
-	 * Cursor planes represent a "cursor" plane for a CRTC.  Cursor planes
-	 * are the planes operated upon by the DRM_IOCTL_MODE_CURSOR and
-	 * DRM_IOCTL_MODE_CURSOR2 IOCTLs.
+	 * A cursor plane attached to a CRTC is more likely to be able to be
+	 * enabled when no scaling/cropping is used and the framebuffer has the
+	 * size indicated by &drm_mode_config.cursor_width and
+	 * &drm_mode_config.cursor_height. Additionally, if the driver doesn't
+	 * support modifiers, the framebuffer should have a linear layout.
 	 */
 	DRM_PLANE_TYPE_CURSOR,
 };
-- 
2.29.2

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 8/8] drm/doc: document the type plane property
  2020-12-16 20:22 [PATCH 0/8] drm/doc: improve plane property docs Simon Ser
                   ` (6 preceding siblings ...)
  2020-12-16 20:22 ` [PATCH 7/8] drm/doc: fix drm_plane_type docs Simon Ser
@ 2020-12-16 20:22 ` Simon Ser
  2020-12-16 21:23   ` Daniel Vetter
  7 siblings, 1 reply; 20+ messages in thread
From: Simon Ser @ 2020-12-16 20:22 UTC (permalink / raw)
  To: dri-devel

Add a new entry for "type" in the section for standard plane properties.

Signed-off-by: Simon Ser <contact@emersion.fr>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Pekka Paalanen <ppaalanen@gmail.com>
---
 drivers/gpu/drm/drm_plane.c | 39 +++++++++++++++++++++++++++++++++----
 1 file changed, 35 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
index 4c1a45ac18e6..e21e0caaca0f 100644
--- a/drivers/gpu/drm/drm_plane.c
+++ b/drivers/gpu/drm/drm_plane.c
@@ -49,10 +49,8 @@
  * &struct drm_plane (possibly as part of a larger structure) and registers it
  * with a call to drm_universal_plane_init().
  *
- * The type of a plane is exposed in the immutable "type" enumeration property,
- * which has one of the following values: "Overlay", "Primary", "Cursor" (see
- * enum drm_plane_type). A plane can be compatible with multiple CRTCs, see
- * &drm_plane.possible_crtcs.
+ * Each plane has a type, see enum drm_plane_type. A plane can be compatible
+ * with multiple CRTCs, see &drm_plane.possible_crtcs.
  *
  * Legacy uAPI doesn't expose the primary and cursor planes directly. DRM core
  * relies on the driver to set the primary and optionally the cursor plane used
@@ -66,6 +64,39 @@
  *
  * DRM planes have a few standardized properties:
  *
+ * type:
+ *     Immutable property describing the type of the plane.
+ *
+ *     For user-space which has enabled the &DRM_CLIENT_CAP_UNIVERSAL_PLANES
+ *     capability, the plane type is just a hint and is mostly superseded by
+ *     atomic test-only commits. The type hint can still be used to come up
+ *     more easily with a plane configuration accepted by the driver.
+ *
+ *     The value of this property can be one of the following:
+ *
+ *     "Primary":
+ *         To light up a CRTC, attaching a primary plane is the most likely to
+ *         work if it covers the whole CRTC and doesn't have scaling or
+ *         cropping set up.
+ *
+ *         Drivers may support more features for the primary plane, user-space
+ *         can find out with test-only atomic commits.
+ *     "Cursor":
+ *         To enable this plane, using a framebuffer configured without scaling
+ *         or cropping and with the following properties is the most likely to
+ *         work:
+ *
+ *         - If the driver provides the capabilities &DRM_CAP_CURSOR_WIDTH and
+ *           &DRM_CAP_CURSOR_HEIGHT, create the framebuffer with this size.
+ *           Otherwise, create a framebuffer with the size 64x64.
+ *         - If the driver doesn't support modifiers, create a framebuffer with
+ *           a linear layout. Otherwise, use the IN_FORMATS plane property.
+ *
+ *         Drivers may support more features for the cursor plane, user-space
+ *         can find out with test-only atomic commits.
+ *     "Overlay":
+ *         Neither primary nor cursor.
+ *
  * IN_FORMATS:
  *     Blob property which contains the set of buffer format and modifier
  *     pairs supported by this plane. The blob is a drm_format_modifier_blob
-- 
2.29.2

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH 4/8] drm/doc: move color management functions under CRTC section
  2020-12-16 20:22 ` [PATCH 4/8] drm/doc: move color management functions under CRTC section Simon Ser
@ 2020-12-16 21:14   ` Daniel Vetter
  0 siblings, 0 replies; 20+ messages in thread
From: Daniel Vetter @ 2020-12-16 21:14 UTC (permalink / raw)
  To: Simon Ser; +Cc: dri-devel

On Wed, Dec 16, 2020 at 09:22:18PM +0100, Simon Ser wrote:
> Move drm_color_mgmt function reference from the KMS properties
> section to the CRTC abstraction section. This makes the KMS
> properties section more readable for user-space developers.
> 
> Signed-off-by: Simon Ser <contact@emersion.fr>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: Pekka Paalanen <ppaalanen@gmail.com>

For patches 1-4: Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  Documentation/gpu/drm-kms.rst | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
> index e6329e7e638e..2f3efb63e5ba 100644
> --- a/Documentation/gpu/drm-kms.rst
> +++ b/Documentation/gpu/drm-kms.rst
> @@ -319,6 +319,15 @@ CRTC Functions Reference
>  .. kernel-doc:: drivers/gpu/drm/drm_crtc.c
>     :export:
>  
> +Color Management Functions Reference
> +------------------------------------
> +
> +.. kernel-doc:: drivers/gpu/drm/drm_color_mgmt.c
> +   :export:
> +
> +.. kernel-doc:: include/drm/drm_color_mgmt.h
> +   :internal:
> +
>  Frame Buffer Abstraction
>  ========================
>  
> @@ -499,12 +508,6 @@ Color Management Properties
>  .. kernel-doc:: drivers/gpu/drm/drm_color_mgmt.c
>     :doc: overview
>  
> -.. kernel-doc:: drivers/gpu/drm/drm_color_mgmt.c
> -   :export:
> -
> -.. kernel-doc:: include/drm/drm_color_mgmt.h
> -   :internal:
> -
>  Tile Group Property
>  -------------------
>  
> -- 
> 2.29.2
> 

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 5/8] drm/doc: the KMS properties section is for user-space devs
  2020-12-16 20:22 ` [PATCH 5/8] drm/doc: the KMS properties section is for user-space devs Simon Ser
@ 2020-12-16 21:14   ` Daniel Vetter
  0 siblings, 0 replies; 20+ messages in thread
From: Daniel Vetter @ 2020-12-16 21:14 UTC (permalink / raw)
  To: Simon Ser; +Cc: dri-devel

On Wed, Dec 16, 2020 at 09:22:19PM +0100, Simon Ser wrote:
> State that the "KMS Properties" section is mainly for user-space
> developers.
> 
> Signed-off-by: Simon Ser <contact@emersion.fr>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: Pekka Paalanen <ppaalanen@gmail.com>
> ---
>  Documentation/gpu/drm-kms.rst | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
> index 2f3efb63e5ba..fcd4e15379b0 100644
> --- a/Documentation/gpu/drm-kms.rst
> +++ b/Documentation/gpu/drm-kms.rst
> @@ -460,6 +460,9 @@ KMS Locking
>  KMS Properties
>  ==============
>  
> +This section of the documentation is primarily aimed at user-space developers.
> +For the driver APIs, so the other sections.

s/so/consult/ ? 2nd sentence doesn't parse here.
-Daniel

> +
>  Property Types and Blob Property Support
>  ----------------------------------------
>  
> -- 
> 2.29.2
> 

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 7/8] drm/doc: fix drm_plane_type docs
  2020-12-16 20:22 ` [PATCH 7/8] drm/doc: fix drm_plane_type docs Simon Ser
@ 2020-12-16 21:19   ` Daniel Vetter
  2020-12-17 10:37     ` Simon Ser
  0 siblings, 1 reply; 20+ messages in thread
From: Daniel Vetter @ 2020-12-16 21:19 UTC (permalink / raw)
  To: Simon Ser; +Cc: dri-devel

On Wed, Dec 16, 2020 at 09:22:21PM +0100, Simon Ser wrote:
> The docs for enum drm_plane_type mention legacy IOCTLs, however the
> plane type is not tied to legacy IOCTLs, the drm_cursor.primary and
> cursor fields are. Add a small paragraph to reference these.
> 
> Instead, document expectations for primary and cursor planes for
> non-legacy userspace. Note that these docs are for driver developers,
> not userspace developers, so internal kernel APIs are mentionned.
> 
> Signed-off-by: Simon Ser <contact@emersion.fr>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: Pekka Paalanen <ppaalanen@gmail.com>
> ---
>  include/drm/drm_plane.h | 21 +++++++++++++--------
>  1 file changed, 13 insertions(+), 8 deletions(-)
> 
> diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
> index 1d82b264e5e4..94fdd0c68450 100644
> --- a/include/drm/drm_plane.h
> +++ b/include/drm/drm_plane.h
> @@ -538,10 +538,14 @@ struct drm_plane_funcs {
>   *
>   * For compatibility with legacy userspace, only overlay planes are made
>   * available to userspace by default. Userspace clients may set the
> - * DRM_CLIENT_CAP_UNIVERSAL_PLANES client capability bit to indicate that they
> + * &DRM_CLIENT_CAP_UNIVERSAL_PLANES client capability bit to indicate that they
>   * wish to receive a universal plane list containing all plane types. See also
>   * drm_for_each_legacy_plane().
>   *
> + * In addition to setting each plane's type, drivers need to setup the
> + * &drm_crtc.primary and optionally &drm_crtc.cursor for legacy IOCTLs. See

				insert "pointers" here ^ for readability.

> + * drm_crtc_init_with_planes().
> + *
>   * WARNING: The values of this enum is UABI since they're exposed in the "type"
>   * property.
>   */
> @@ -557,19 +561,20 @@ enum drm_plane_type {
>  	/**
>  	 * @DRM_PLANE_TYPE_PRIMARY:
>  	 *
> -	 * Primary planes represent a "main" plane for a CRTC.  Primary planes
> -	 * are the planes operated upon by CRTC modesetting and flipping
> -	 * operations described in the &drm_crtc_funcs.page_flip and
> -	 * &drm_crtc_funcs.set_config hooks.

I think we should keep the notice which legacy entry hooks implicitly
operate on the primary plane. Not sure why you're removing the above
sentence. Maybe improve it by adding "See also &drm_crtc.primary." for
more context?

> +	 * A primary plane attached to a CRTC is the most likely to be able to
> +	 * light up the CRTC when no scaling/cropping is used and the plane
> +	 * covers the whole CRTC.
>  	 */
>  	DRM_PLANE_TYPE_PRIMARY,
>  
>  	/**
>  	 * @DRM_PLANE_TYPE_CURSOR:
>  	 *
> -	 * Cursor planes represent a "cursor" plane for a CRTC.  Cursor planes
> -	 * are the planes operated upon by the DRM_IOCTL_MODE_CURSOR and
> -	 * DRM_IOCTL_MODE_CURSOR2 IOCTLs.

Same here, I think it matters that this is used by legacy cursor ioctls.
Again perhaps a pointer to drm_crtc.cursor can help.
-Daniel

> +	 * A cursor plane attached to a CRTC is more likely to be able to be
> +	 * enabled when no scaling/cropping is used and the framebuffer has the
> +	 * size indicated by &drm_mode_config.cursor_width and
> +	 * &drm_mode_config.cursor_height. Additionally, if the driver doesn't
> +	 * support modifiers, the framebuffer should have a linear layout.
>  	 */
>  	DRM_PLANE_TYPE_CURSOR,
>  };
> -- 
> 2.29.2
> 

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 6/8] drm/doc: introduce new section for standard plane properties
  2020-12-16 20:22 ` [PATCH 6/8] drm/doc: introduce new section for standard plane properties Simon Ser
@ 2020-12-16 21:19   ` Daniel Vetter
  0 siblings, 0 replies; 20+ messages in thread
From: Daniel Vetter @ 2020-12-16 21:19 UTC (permalink / raw)
  To: Simon Ser; +Cc: dri-devel

On Wed, Dec 16, 2020 at 09:22:20PM +0100, Simon Ser wrote:
> Introduce a new "Standard Plane Properties" section for properties
> defined in drm_plane.c. Move the mis-placed IN_FORMATS docs there.

Yeah that's misplaced.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> 
> Signed-off-by: Simon Ser <contact@emersion.fr>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: Pekka Paalanen <ppaalanen@gmail.com>
> ---
>  Documentation/gpu/drm-kms.rst |  6 ++++++
>  drivers/gpu/drm/drm_blend.c   |  6 ------
>  drivers/gpu/drm/drm_plane.c   | 12 ++++++++++++
>  3 files changed, 18 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
> index fcd4e15379b0..9cfaab4df21a 100644
> --- a/Documentation/gpu/drm-kms.rst
> +++ b/Documentation/gpu/drm-kms.rst
> @@ -493,6 +493,12 @@ Standard CRTC Properties
>  .. kernel-doc:: drivers/gpu/drm/drm_crtc.c
>     :doc: standard CRTC properties
>  
> +Standard Plane Properties
> +-------------------------
> +
> +.. kernel-doc:: drivers/gpu/drm/drm_plane.c
> +   :doc: standard plane properties
> +
>  Plane Composition Properties
>  ----------------------------
>  
> diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c
> index 5c2141e9a9f4..26e2f2ffd255 100644
> --- a/drivers/gpu/drm/drm_blend.c
> +++ b/drivers/gpu/drm/drm_blend.c
> @@ -185,12 +185,6 @@
>   *		 plane does not expose the "alpha" property, then this is
>   *		 assumed to be 1.0
>   *
> - * IN_FORMATS:
> - *	Blob property which contains the set of buffer format and modifier
> - *	pairs supported by this plane. The blob is a drm_format_modifier_blob
> - *	struct. Without this property the plane doesn't support buffers with
> - *	modifiers. Userspace cannot change this property.
> - *
>   * Note that all the property extensions described here apply either to the
>   * plane or the CRTC (e.g. for the background color, which currently is not
>   * exposed and assumed to be black).
> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> index 49b0a8b9ac02..4c1a45ac18e6 100644
> --- a/drivers/gpu/drm/drm_plane.c
> +++ b/drivers/gpu/drm/drm_plane.c
> @@ -61,6 +61,18 @@
>   * userspace too much.
>   */
>  
> +/**
> + * DOC: standard plane properties
> + *
> + * DRM planes have a few standardized properties:
> + *
> + * IN_FORMATS:
> + *     Blob property which contains the set of buffer format and modifier
> + *     pairs supported by this plane. The blob is a drm_format_modifier_blob
> + *     struct. Without this property the plane doesn't support buffers with
> + *     modifiers. Userspace cannot change this property.
> + */
> +
>  static unsigned int drm_num_planes(struct drm_device *dev)
>  {
>  	unsigned int num = 0;
> -- 
> 2.29.2
> 

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 8/8] drm/doc: document the type plane property
  2020-12-16 20:22 ` [PATCH 8/8] drm/doc: document the type plane property Simon Ser
@ 2020-12-16 21:23   ` Daniel Vetter
  2020-12-17 10:40     ` Simon Ser
  0 siblings, 1 reply; 20+ messages in thread
From: Daniel Vetter @ 2020-12-16 21:23 UTC (permalink / raw)
  To: Simon Ser; +Cc: dri-devel

On Wed, Dec 16, 2020 at 09:22:22PM +0100, Simon Ser wrote:
> Add a new entry for "type" in the section for standard plane properties.
> 
> Signed-off-by: Simon Ser <contact@emersion.fr>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: Pekka Paalanen <ppaalanen@gmail.com>

Looks solid, bunch of comments for extensions and more clarity below.
-Daniel

> ---
>  drivers/gpu/drm/drm_plane.c | 39 +++++++++++++++++++++++++++++++++----
>  1 file changed, 35 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> index 4c1a45ac18e6..e21e0caaca0f 100644
> --- a/drivers/gpu/drm/drm_plane.c
> +++ b/drivers/gpu/drm/drm_plane.c
> @@ -49,10 +49,8 @@
>   * &struct drm_plane (possibly as part of a larger structure) and registers it
>   * with a call to drm_universal_plane_init().
>   *
> - * The type of a plane is exposed in the immutable "type" enumeration property,
> - * which has one of the following values: "Overlay", "Primary", "Cursor" (see
> - * enum drm_plane_type). A plane can be compatible with multiple CRTCs, see
> - * &drm_plane.possible_crtcs.
> + * Each plane has a type, see enum drm_plane_type. A plane can be compatible
> + * with multiple CRTCs, see &drm_plane.possible_crtcs.
>   *
>   * Legacy uAPI doesn't expose the primary and cursor planes directly. DRM core
>   * relies on the driver to set the primary and optionally the cursor plane used
> @@ -66,6 +64,39 @@
>   *
>   * DRM planes have a few standardized properties:
>   *
> + * type:
> + *     Immutable property describing the type of the plane.
> + *
> + *     For user-space which has enabled the &DRM_CLIENT_CAP_UNIVERSAL_PLANES

While we're at this: Does the kerneldoc for this cap mention that it's
implicitly enabled when you're enabling atomic?

Maybe worth repeating here too.

> + *     capability, the plane type is just a hint and is mostly superseded by
> + *     atomic test-only commits. The type hint can still be used to come up
> + *     more easily with a plane configuration accepted by the driver.
> + *
> + *     The value of this property can be one of the following:
> + *
> + *     "Primary":
> + *         To light up a CRTC, attaching a primary plane is the most likely to
> + *         work if it covers the whole CRTC and doesn't have scaling or
> + *         cropping set up.
> + *
> + *         Drivers may support more features for the primary plane, user-space
> + *         can find out with test-only atomic commits.

We need to mention here that this is the implicit plane used by the
PAGE_FLIP and SETCRTC ioctl (maybe spell them out in full since these are
userspace docs).

> + *     "Cursor":
> + *         To enable this plane, using a framebuffer configured without scaling
> + *         or cropping and with the following properties is the most likely to
> + *         work:
> + *
> + *         - If the driver provides the capabilities &DRM_CAP_CURSOR_WIDTH and
> + *           &DRM_CAP_CURSOR_HEIGHT, create the framebuffer with this size.
> + *           Otherwise, create a framebuffer with the size 64x64.
> + *         - If the driver doesn't support modifiers, create a framebuffer with
> + *           a linear layout. Otherwise, use the IN_FORMATS plane property.

Same here, I think we should mention this is used implicitly (but only on
some drivers, there atomic drivers which do _not_ expose their cursor as a
cursor plane).

> + *
> + *         Drivers may support more features for the cursor plane, user-space
> + *         can find out with test-only atomic commits.
> + *     "Overlay":
> + *         Neither primary nor cursor.

This should mention that these are the only planes exposed when universal
planes isnt set.

> + *
>   * IN_FORMATS:
>   *     Blob property which contains the set of buffer format and modifier
>   *     pairs supported by this plane. The blob is a drm_format_modifier_blob
> -- 
> 2.29.2
> 

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 7/8] drm/doc: fix drm_plane_type docs
  2020-12-16 21:19   ` Daniel Vetter
@ 2020-12-17 10:37     ` Simon Ser
  2020-12-17 10:50       ` Daniel Vetter
  0 siblings, 1 reply; 20+ messages in thread
From: Simon Ser @ 2020-12-17 10:37 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: dri-devel

On Wednesday, December 16th, 2020 at 10:19 PM, Daniel Vetter <daniel@ffwll.ch> wrote:

> On Wed, Dec 16, 2020 at 09:22:21PM +0100, Simon Ser wrote:
> > The docs for enum drm_plane_type mention legacy IOCTLs, however the
> > plane type is not tied to legacy IOCTLs, the drm_cursor.primary and
> > cursor fields are. Add a small paragraph to reference these.
> >
> > Instead, document expectations for primary and cursor planes for
> > non-legacy userspace. Note that these docs are for driver developers,
> > not userspace developers, so internal kernel APIs are mentionned.
> >
> > Signed-off-by: Simon Ser <contact@emersion.fr>
> > Cc: Daniel Vetter <daniel@ffwll.ch>
> > Cc: Pekka Paalanen <ppaalanen@gmail.com>
> > ---
> >  include/drm/drm_plane.h | 21 +++++++++++++--------
> >  1 file changed, 13 insertions(+), 8 deletions(-)
> >
> > diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
> > index 1d82b264e5e4..94fdd0c68450 100644
> > --- a/include/drm/drm_plane.h
> > +++ b/include/drm/drm_plane.h
> > @@ -538,10 +538,14 @@ struct drm_plane_funcs {
> >   *
> >   * For compatibility with legacy userspace, only overlay planes are made
> >   * available to userspace by default. Userspace clients may set the
> > - * DRM_CLIENT_CAP_UNIVERSAL_PLANES client capability bit to indicate that they
> > + * &DRM_CLIENT_CAP_UNIVERSAL_PLANES client capability bit to indicate that they
> >   * wish to receive a universal plane list containing all plane types. See also
> >   * drm_for_each_legacy_plane().
> >   *
> > + * In addition to setting each plane's type, drivers need to setup the
> > + * &drm_crtc.primary and optionally &drm_crtc.cursor for legacy IOCTLs. See
>
> 				insert "pointers" here ^ for readability.
>
> > + * drm_crtc_init_with_planes().
> > + *
> >   * WARNING: The values of this enum is UABI since they're exposed in the "type"
> >   * property.
> >   */
> > @@ -557,19 +561,20 @@ enum drm_plane_type {
> >  	/**
> >  	 * @DRM_PLANE_TYPE_PRIMARY:
> >  	 *
> > -	 * Primary planes represent a "main" plane for a CRTC.  Primary planes
> > -	 * are the planes operated upon by CRTC modesetting and flipping
> > -	 * operations described in the &drm_crtc_funcs.page_flip and
> > -	 * &drm_crtc_funcs.set_config hooks.
>
> I think we should keep the notice which legacy entry hooks implicitly
> operate on the primary plane. Not sure why you're removing the above
> sentence. Maybe improve it by adding "See also &drm_crtc.primary." for
> more context?

I intentionally removed it, because setting the plane type will not magically
make it used for legacy IOCTLs. The previous version documented the legacy
IOCTLs behaviour in the primary and cursor type docs. That was misleading
because only the drm_crtc.{primary,cursor} pointers make it so a plane is used
for legacy IOCTLs, not the type.

This patch does keep a reference to drm_crtc.{primary,cursor}, in the paragraph
right above. Clicking on the reference will explain in detail which legacy
IOCTLs are affected. I don't think repeating the paragraph again in the primary
and cursor type docs is necessary.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 8/8] drm/doc: document the type plane property
  2020-12-16 21:23   ` Daniel Vetter
@ 2020-12-17 10:40     ` Simon Ser
  2020-12-17 10:52       ` Daniel Vetter
  0 siblings, 1 reply; 20+ messages in thread
From: Simon Ser @ 2020-12-17 10:40 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: dri-devel

On Wednesday, December 16th, 2020 at 10:23 PM, Daniel Vetter <daniel@ffwll.ch> wrote:

> > + * type:
> > + *     Immutable property describing the type of the plane.
> > + *
> > + *     For user-space which has enabled the &DRM_CLIENT_CAP_UNIVERSAL_PLANES
>
> While we're at this: Does the kerneldoc for this cap mention that it's
> implicitly enabled when you're enabling atomic?
>
> Maybe worth repeating here too.

Good point. v2 will do both.

> > + *     capability, the plane type is just a hint and is mostly superseded by
> > + *     atomic test-only commits. The type hint can still be used to come up
> > + *     more easily with a plane configuration accepted by the driver.
> > + *
> > + *     The value of this property can be one of the following:
> > + *
> > + *     "Primary":
> > + *         To light up a CRTC, attaching a primary plane is the most likely to
> > + *         work if it covers the whole CRTC and doesn't have scaling or
> > + *         cropping set up.
> > + *
> > + *         Drivers may support more features for the primary plane, user-space
> > + *         can find out with test-only atomic commits.
>
> We need to mention here that this is the implicit plane used by the
> PAGE_FLIP and SETCRTC ioctl (maybe spell them out in full since these are
> userspace docs).

I intentionally didn't write that down here, because as previously discussed,
user-space has no way to guess the drm_crtc.{primary,cursor} pointers, so
user-space cannot guess which planes will be used for legacy IOCTLs. Adding any
hint that user-space _could_ do it will result in broken user-space.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 7/8] drm/doc: fix drm_plane_type docs
  2020-12-17 10:37     ` Simon Ser
@ 2020-12-17 10:50       ` Daniel Vetter
  2020-12-17 11:07         ` Simon Ser
  0 siblings, 1 reply; 20+ messages in thread
From: Daniel Vetter @ 2020-12-17 10:50 UTC (permalink / raw)
  To: Simon Ser; +Cc: dri-devel

On Thu, Dec 17, 2020 at 11:37 AM Simon Ser <contact@emersion.fr> wrote:
>
> On Wednesday, December 16th, 2020 at 10:19 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
>
> > On Wed, Dec 16, 2020 at 09:22:21PM +0100, Simon Ser wrote:
> > > The docs for enum drm_plane_type mention legacy IOCTLs, however the
> > > plane type is not tied to legacy IOCTLs, the drm_cursor.primary and
> > > cursor fields are. Add a small paragraph to reference these.
> > >
> > > Instead, document expectations for primary and cursor planes for
> > > non-legacy userspace. Note that these docs are for driver developers,
> > > not userspace developers, so internal kernel APIs are mentionned.
> > >
> > > Signed-off-by: Simon Ser <contact@emersion.fr>
> > > Cc: Daniel Vetter <daniel@ffwll.ch>
> > > Cc: Pekka Paalanen <ppaalanen@gmail.com>
> > > ---
> > >  include/drm/drm_plane.h | 21 +++++++++++++--------
> > >  1 file changed, 13 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
> > > index 1d82b264e5e4..94fdd0c68450 100644
> > > --- a/include/drm/drm_plane.h
> > > +++ b/include/drm/drm_plane.h
> > > @@ -538,10 +538,14 @@ struct drm_plane_funcs {
> > >   *
> > >   * For compatibility with legacy userspace, only overlay planes are made
> > >   * available to userspace by default. Userspace clients may set the
> > > - * DRM_CLIENT_CAP_UNIVERSAL_PLANES client capability bit to indicate that they
> > > + * &DRM_CLIENT_CAP_UNIVERSAL_PLANES client capability bit to indicate that they
> > >   * wish to receive a universal plane list containing all plane types. See also
> > >   * drm_for_each_legacy_plane().
> > >   *
> > > + * In addition to setting each plane's type, drivers need to setup the
> > > + * &drm_crtc.primary and optionally &drm_crtc.cursor for legacy IOCTLs. See
> >
> >                               insert "pointers" here ^ for readability.
> >
> > > + * drm_crtc_init_with_planes().
> > > + *
> > >   * WARNING: The values of this enum is UABI since they're exposed in the "type"
> > >   * property.
> > >   */
> > > @@ -557,19 +561,20 @@ enum drm_plane_type {
> > >     /**
> > >      * @DRM_PLANE_TYPE_PRIMARY:
> > >      *
> > > -    * Primary planes represent a "main" plane for a CRTC.  Primary planes
> > > -    * are the planes operated upon by CRTC modesetting and flipping
> > > -    * operations described in the &drm_crtc_funcs.page_flip and
> > > -    * &drm_crtc_funcs.set_config hooks.
> >
> > I think we should keep the notice which legacy entry hooks implicitly
> > operate on the primary plane. Not sure why you're removing the above
> > sentence. Maybe improve it by adding "See also &drm_crtc.primary." for
> > more context?
>
> I intentionally removed it, because setting the plane type will not magically
> make it used for legacy IOCTLs. The previous version documented the legacy
> IOCTLs behaviour in the primary and cursor type docs. That was misleading
> because only the drm_crtc.{primary,cursor} pointers make it so a plane is used
> for legacy IOCTLs, not the type.
>
> This patch does keep a reference to drm_crtc.{primary,cursor}, in the paragraph
> right above. Clicking on the reference will explain in detail which legacy
> IOCTLs are affected. I don't think repeating the paragraph again in the primary
> and cursor type docs is necessary.

Hm I guess works too, I'm not sure documentations must avoid
repetitions at all costs (since it generally makes stuff harder to
find, despite all the links). So without that change:

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-- 
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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 8/8] drm/doc: document the type plane property
  2020-12-17 10:40     ` Simon Ser
@ 2020-12-17 10:52       ` Daniel Vetter
  2020-12-17 11:09         ` Simon Ser
  0 siblings, 1 reply; 20+ messages in thread
From: Daniel Vetter @ 2020-12-17 10:52 UTC (permalink / raw)
  To: Simon Ser; +Cc: dri-devel

On Thu, Dec 17, 2020 at 11:41 AM Simon Ser <contact@emersion.fr> wrote:
>
> On Wednesday, December 16th, 2020 at 10:23 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
>
> > > + * type:
> > > + *     Immutable property describing the type of the plane.
> > > + *
> > > + *     For user-space which has enabled the &DRM_CLIENT_CAP_UNIVERSAL_PLANES
> >
> > While we're at this: Does the kerneldoc for this cap mention that it's
> > implicitly enabled when you're enabling atomic?
> >
> > Maybe worth repeating here too.
>
> Good point. v2 will do both.
>
> > > + *     capability, the plane type is just a hint and is mostly superseded by
> > > + *     atomic test-only commits. The type hint can still be used to come up
> > > + *     more easily with a plane configuration accepted by the driver.
> > > + *
> > > + *     The value of this property can be one of the following:
> > > + *
> > > + *     "Primary":
> > > + *         To light up a CRTC, attaching a primary plane is the most likely to
> > > + *         work if it covers the whole CRTC and doesn't have scaling or
> > > + *         cropping set up.
> > > + *
> > > + *         Drivers may support more features for the primary plane, user-space
> > > + *         can find out with test-only atomic commits.
> >
> > We need to mention here that this is the implicit plane used by the
> > PAGE_FLIP and SETCRTC ioctl (maybe spell them out in full since these are
> > userspace docs).
>
> I intentionally didn't write that down here, because as previously discussed,
> user-space has no way to guess the drm_crtc.{primary,cursor} pointers, so
> user-space cannot guess which planes will be used for legacy IOCTLs. Adding any
> hint that user-space _could_ do it will result in broken user-space.

Hm then at least a warning that userspace must not mix legacy ioctls
with using primary planes explicitly, since havoc will ensue? More
relevant for cursor planes, since some compositors do use atomic +
legacy cursor planes, but imo good to have the same blurb with the
list of relevant ioctls for each.
-Daniel
-- 
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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 7/8] drm/doc: fix drm_plane_type docs
  2020-12-17 10:50       ` Daniel Vetter
@ 2020-12-17 11:07         ` Simon Ser
  0 siblings, 0 replies; 20+ messages in thread
From: Simon Ser @ 2020-12-17 11:07 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: dri-devel

On Thursday, December 17th, 2020 at 11:50 AM, Daniel Vetter <daniel@ffwll.ch> wrote:

> > > > @@ -557,19 +561,20 @@ enum drm_plane_type {
> > > >     /**
> > > >      * @DRM_PLANE_TYPE_PRIMARY:
> > > >      *
> > > > -    * Primary planes represent a "main" plane for a CRTC.  Primary planes
> > > > -    * are the planes operated upon by CRTC modesetting and flipping
> > > > -    * operations described in the &drm_crtc_funcs.page_flip and
> > > > -    * &drm_crtc_funcs.set_config hooks.
> > >
> > > I think we should keep the notice which legacy entry hooks implicitly
> > > operate on the primary plane. Not sure why you're removing the above
> > > sentence. Maybe improve it by adding "See also &drm_crtc.primary." for
> > > more context?
> >
> > I intentionally removed it, because setting the plane type will not magically
> > make it used for legacy IOCTLs. The previous version documented the legacy
> > IOCTLs behaviour in the primary and cursor type docs. That was misleading
> > because only the drm_crtc.{primary,cursor} pointers make it so a plane is used
> > for legacy IOCTLs, not the type.
> >
> > This patch does keep a reference to drm_crtc.{primary,cursor}, in the paragraph
> > right above. Clicking on the reference will explain in detail which legacy
> > IOCTLs are affected. I don't think repeating the paragraph again in the primary
> > and cursor type docs is necessary.
>
> Hm I guess works too, I'm not sure documentations must avoid
> repetitions at all costs (since it generally makes stuff harder to
> find, despite all the links).

I didn't find the repetition necessary because the paragraph referencing
drm_crtc.{primary,cursor} is literally just a few lines of text away. But I
guess as long as it's clear that setting the type doesn't set
drm_crtc.{primary,cursor} I'm fine with duplicating the references.

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 8/8] drm/doc: document the type plane property
  2020-12-17 10:52       ` Daniel Vetter
@ 2020-12-17 11:09         ` Simon Ser
  0 siblings, 0 replies; 20+ messages in thread
From: Simon Ser @ 2020-12-17 11:09 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: dri-devel

On Thursday, December 17th, 2020 at 11:52 AM, Daniel Vetter <daniel@ffwll.ch> wrote:

> > > > + *     capability, the plane type is just a hint and is mostly superseded by
> > > > + *     atomic test-only commits. The type hint can still be used to come up
> > > > + *     more easily with a plane configuration accepted by the driver.
> > > > + *
> > > > + *     The value of this property can be one of the following:
> > > > + *
> > > > + *     "Primary":
> > > > + *         To light up a CRTC, attaching a primary plane is the most likely to
> > > > + *         work if it covers the whole CRTC and doesn't have scaling or
> > > > + *         cropping set up.
> > > > + *
> > > > + *         Drivers may support more features for the primary plane, user-space
> > > > + *         can find out with test-only atomic commits.
> > >
> > > We need to mention here that this is the implicit plane used by the
> > > PAGE_FLIP and SETCRTC ioctl (maybe spell them out in full since these are
> > > userspace docs).
> >
> > I intentionally didn't write that down here, because as previously discussed,
> > user-space has no way to guess the drm_crtc.{primary,cursor} pointers, so
> > user-space cannot guess which planes will be used for legacy IOCTLs. Adding any
> > hint that user-space _could_ do it will result in broken user-space.
>
> Hm then at least a warning that userspace must not mix legacy ioctls
> with using primary planes explicitly, since havoc will ensue? More
> relevant for cursor planes, since some compositors do use atomic +
> legacy cursor planes, but imo good to have the same blurb with the
> list of relevant ioctls for each.

Oh, right, good idea, this sounds important. Will add in v2.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2020-12-17 11:09 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-16 20:22 [PATCH 0/8] drm/doc: improve plane property docs Simon Ser
2020-12-16 20:22 ` [PATCH 1/8] drm/doc: rename FB_DAMAGE_CLIPS section Simon Ser
2020-12-16 20:22 ` [PATCH 2/8] drm/doc: move composition function docs to new section Simon Ser
2020-12-16 20:22 ` [PATCH 3/8] drm/doc: move damage tracking functions " Simon Ser
2020-12-16 20:22 ` [PATCH 4/8] drm/doc: move color management functions under CRTC section Simon Ser
2020-12-16 21:14   ` Daniel Vetter
2020-12-16 20:22 ` [PATCH 5/8] drm/doc: the KMS properties section is for user-space devs Simon Ser
2020-12-16 21:14   ` Daniel Vetter
2020-12-16 20:22 ` [PATCH 6/8] drm/doc: introduce new section for standard plane properties Simon Ser
2020-12-16 21:19   ` Daniel Vetter
2020-12-16 20:22 ` [PATCH 7/8] drm/doc: fix drm_plane_type docs Simon Ser
2020-12-16 21:19   ` Daniel Vetter
2020-12-17 10:37     ` Simon Ser
2020-12-17 10:50       ` Daniel Vetter
2020-12-17 11:07         ` Simon Ser
2020-12-16 20:22 ` [PATCH 8/8] drm/doc: document the type plane property Simon Ser
2020-12-16 21:23   ` Daniel Vetter
2020-12-17 10:40     ` Simon Ser
2020-12-17 10:52       ` Daniel Vetter
2020-12-17 11:09         ` Simon Ser

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.