All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sean Paul <seanpaul@chromium.org>
To: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Daniel Vetter <daniel.vetter@intel.com>,
	Intel Graphics Development <intel-gfx@lists.freedesktop.org>,
	DRI Development <dri-devel@lists.freedesktop.org>
Subject: Re: [PATCH 07/10] drm/doc: Polish plane composition property docs
Date: Tue, 6 Sep 2016 12:59:17 -0400	[thread overview]
Message-ID: <CAOw6vb+14oC7+zQXYQni7rfzUzYxUYY=acfHZdprePSUKgLHmA@mail.gmail.com> (raw)
In-Reply-To: <20160831160913.12991-8-daniel.vetter@ffwll.ch>

On Wed, Aug 31, 2016 at 12:09 PM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> Try to spec a bit more precisely how they all fit together, now that
> at least the code is for all the additional properties is in one
> place.
>
> Also remove the entries for the standardized properties from the
> table, because that thing is supremely unmaintaineable.

This is great!

But... I have a few spelling/copypasta/word nits below. Once they're fixed up,

Reviewed-by: Sean Paul <seanpaul@chromium.org>

>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Sean Paul <seanpaul@chromium.org>
> Cc: Benjamin Gaignard <benjamin.gaignard@linaro.org>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  Documentation/gpu/drm-kms.rst        |   7 +-
>  Documentation/gpu/kms-properties.csv |  15 ----
>  drivers/gpu/drm/drm_blend.c          | 146 ++++++++++++++++++++++++++++++-----
>  drivers/gpu/drm/drm_plane.c          |   3 +
>  4 files changed, 136 insertions(+), 35 deletions(-)
>
> diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
> index b1029e292e5c..6be8d3359620 100644
> --- a/Documentation/gpu/drm-kms.rst
> +++ b/Documentation/gpu/drm-kms.rst
> @@ -287,8 +287,11 @@ Property Types and Blob Property Support
>  .. kernel-doc:: drivers/gpu/drm/drm_property.c
>     :export:
>
> -Blending and Z-Position properties
> -----------------------------------
> +Plane Composition Properties
> +----------------------------
> +
> +.. kernel-doc:: drivers/gpu/drm/drm_blend.c
> +   :doc: overview
>
>  .. kernel-doc:: drivers/gpu/drm/drm_blend.c
>     :export:
> diff --git a/Documentation/gpu/kms-properties.csv b/Documentation/gpu/kms-properties.csv
> index 4c5ce3edcfd9..1aa2493d1ef9 100644
> --- a/Documentation/gpu/kms-properties.csv
> +++ b/Documentation/gpu/kms-properties.csv
> @@ -1,23 +1,10 @@
>  Owner Module/Drivers,Group,Property Name,Type,Property Values,Object attached,Description/Restrictions
> -DRM,Generic,“rotation”,BITMASK,"{ 0, ""rotate-0"" }, { 1, ""rotate-90"" }, { 2, ""rotate-180"" }, { 3, ""rotate-270"" }, { 4, ""reflect-x"" }, { 5, ""reflect-y"" }","CRTC, Plane",rotate-(degrees) rotates the image by the specified amount in degrees in counter clockwise direction. reflect-x and reflect-y reflects the image along the specified axis prior to rotation
>  ,,“scaling mode”,ENUM,"{ ""None"", ""Full"", ""Center"", ""Full aspect"" }",Connector,"Supported by: amdgpu, gma500, i915, nouveau and radeon."
>  ,Connector,“EDID”,BLOB | IMMUTABLE,0,Connector,Contains id of edid blob ptr object.
>  ,,“DPMS”,ENUM,"{ “On”, “Standby”, “Suspend”, “Off” }",Connector,Contains DPMS operation mode value.
>  ,,“PATH”,BLOB | IMMUTABLE,0,Connector,Contains topology path to a connector.
>  ,,“TILE”,BLOB | IMMUTABLE,0,Connector,Contains tiling information for a connector.
>  ,,“CRTC_ID”,OBJECT,DRM_MODE_OBJECT_CRTC,Connector,CRTC that connector is attached to (atomic)
> -,Plane,“type”,ENUM | IMMUTABLE,"{ ""Overlay"", ""Primary"", ""Cursor"" }",Plane,Plane type
> -,,“SRC_X”,RANGE,"Min=0, Max=UINT_MAX",Plane,Scanout source x coordinate in 16.16 fixed point (atomic)
> -,,“SRC_Y”,RANGE,"Min=0, Max=UINT_MAX",Plane,Scanout source y coordinate in 16.16 fixed point (atomic)
> -,,“SRC_W”,RANGE,"Min=0, Max=UINT_MAX",Plane,Scanout source width in 16.16 fixed point (atomic)
> -,,“SRC_H”,RANGE,"Min=0, Max=UINT_MAX",Plane,Scanout source height in 16.16 fixed point (atomic)
> -,,“CRTC_X”,SIGNED_RANGE,"Min=INT_MIN, Max=INT_MAX",Plane,Scanout CRTC (destination) x coordinate (atomic)
> -,,“CRTC_Y”,SIGNED_RANGE,"Min=INT_MIN, Max=INT_MAX",Plane,Scanout CRTC (destination) y coordinate (atomic)
> -,,“CRTC_W”,RANGE,"Min=0, Max=UINT_MAX",Plane,Scanout CRTC (destination) width (atomic)
> -,,“CRTC_H”,RANGE,"Min=0, Max=UINT_MAX",Plane,Scanout CRTC (destination) height (atomic)
> -,,“FB_ID”,OBJECT,DRM_MODE_OBJECT_FB,Plane,Scanout framebuffer (atomic)
> -,,“CRTC_ID”,OBJECT,DRM_MODE_OBJECT_CRTC,Plane,CRTC that plane is attached to (atomic)
> -,,“zpos”,RANGE,"Min=0, Max=UINT_MAX","Plane,Z-order of the plane.Planes with higher Z-order values are displayed on top, planes with identical Z-order values are display in an undefined order"
>  ,DVI-I,“subconnector”,ENUM,"{ “Unknown”, “DVI-D”, “DVI-A” }",Connector,TBD
>  ,,“select subconnector”,ENUM,"{ “Automatic”, “DVI-D”, “DVI-A” }",Connector,TBD
>  ,TV,“subconnector”,ENUM,"{ ""Unknown"", ""Composite"", ""SVIDEO"", ""Component"", ""SCART"" }",Connector,TBD
> @@ -95,7 +82,6 @@ armada,CRTC,"""CSC_YUV""",ENUM,"{ ""Auto"" , ""CCIR601"", ""CCIR709"" }",CRTC,TB
>  ,,"""contrast""",RANGE,"Min=0, Max=0x7fff",Plane,TBD
>  ,,"""saturation""",RANGE,"Min=0, Max=0x7fff",Plane,TBD
>  exynos,CRTC,“mode”,ENUM,"{ ""normal"", ""blank"" }",CRTC,TBD
> -,Overlay,“zpos”,RANGE,"Min=0, Max=MAX_PLANE-1",Plane,TBD
>  i2c/ch7006_drv,Generic,“scale”,RANGE,"Min=0, Max=2",Connector,TBD
>  ,TV,“mode”,ENUM,"{ ""PAL"", ""PAL-M"",""PAL-N""}, ”PAL-Nc"" , ""PAL-60"", ""NTSC-M"", ""NTSC-J"" }",Connector,TBD
>  nouveau,NV10 Overlay,"""colorkey""",RANGE,"Min=0, Max=0x01ffffff",Plane,TBD
> @@ -126,4 +112,3 @@ radeon,DVI-I,“coherent”,RANGE,"Min=0, Max=1",Connector,TBD
>  ,FMT Dithering,“dither”,ENUM,"{ ""off"", ""on"" }",Connector,TBD
>  rcar-du,Generic,"""alpha""",RANGE,"Min=0, Max=255",Plane,TBD
>  ,,"""colorkey""",RANGE,"Min=0, Max=0x01ffffff",Plane,TBD
> -,,"""zpos""",RANGE,"Min=1, Max=7",Plane,TBD
> diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c
> index ab39245a21c4..103da0e7b70e 100644
> --- a/drivers/gpu/drm/drm_blend.c
> +++ b/drivers/gpu/drm/drm_blend.c
> @@ -30,6 +30,118 @@
>  #include <linux/slab.h>
>  #include <linux/sort.h>
>
> +/**
> + * DOC: overview
> + *
> + * The basic plane composition model supported by standard plan properties only

s/plan/plane/

> + * has a source rectangle (in logical pixels within the &drm_framebuffer), with
> + * sub-pixel accuracy, which is scaled up to a pixel-aligned destination
> + * rectangle in the visible areay of a &drm_crtc. The visible area of a CRTC is

s/areay/area/

> + * defined by the horizontal and vertical visible pixels (stored in @hdisplay
> + * and @vdisplay) of the requested mode (stored in @mode in the
> + * &drm_crtc_state). These two rectangles are both stored in the
> + * &drm_plane_state.
> + *
> + * For the atomic ioctl the following standard (atomic) propertis on the plane object

s/propertis/properties/

> + * encode the basic plane composition model:
> + *
> + * SRC_X:
> + *     X coordinate offset for the source rectangle within the
> + *     &drm_framebuffer, in 16.16 fixed point. Must be positive.
> + * SRC_Y:
> + *     Y coordinate offset for the source rectangle within the
> + *     &drm_framebuffer, in 16.16 fixed point. Must be positive.
> + * SRC_W:
> + *     Width for the source rectangle within the &drm_framebuffer, in 16.16
> + *     fixed point. SRC_X plus SRC_W must be within the width of the source
> + *     framebuffer. Must be positive.
> + * SRC_H:
> + *     Height for the source rectangle within the &drm_framebuffer, in 16.16
> + *     fixed point. SRC_Y plus SRC_H must be within the width of the source

s/within the width/within the height/

> + *     framebuffer. Must be positive.
> + * CRTC_X:
> + *     X coordinate offset for the destination rectangle. Can be negative.
> + * CRTC_Y:
> + *     Y coordinate offset for the destination rectangle. Can be negative.
> + * CRTC_W:
> + *     Width for the destination rectangle. CRTC_X plus CRTC_W can extend past
> + *     the currently visible horizontal area of the &drm_crtc.
> + * CRTC_H:
> + *     Height for the destination rectangle. CRTC_Y plus CRTC_H can extend past
> + *     the currently visible vertical area of the &drm_crtc.
> + * FB_ID:
> + *     Mode object ID of the &drm_framebuffer this plane should scan out.
> + * CRTC_ID:
> + *     Mode object ID of the &drm_crtc this plane should be connected to.
> + *
> + * Note that the source rectangle must fully lie within the bounds of the
> + * &drm_framebuffer. The destination rectangle can lie outside of the visible
> + * area of the current mode of the CRTC. It must be apprpriately clipped by the
> + * driver, which can be done by calling drm_plane_helper_check_update(). Drivers
> + * are also allowed to round the subpixel sampling positions appropriately, but
> + * only to the next full pixel. No pixel outside of the source rectangle may
> + * ever be sampled, which is important when applying more sophisticated
> + * filtering than just a bilinear one when scaling. The filtering mode when
> + * scaling is unspecified.
> + *
> + * On top of this basic transformation additional properties can be exposed by
> + * the driver:
> + *
> + * - Rotation is set up with drm_mode_create_rotation_property(). It adds a
> + *   rotation and reflection step between the source and destination rectangles.
> + *   Without this property the rectangle is only scaled, but not rotated or
> + *   reflected.
> + *
> + * - Z position is set up with drm_plane_create_zpos_immutable_property() and
> + *   drm_plane_create_zpos_property(). It controls the visibility of overlapping
> + *   planes. Without this property the primary plane is always below all the

s/below all/below/

> + *   cursor plane, and ordering between all other planes is undefined.
> + *
> + * 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).
> + */
> +
> +/**
> + * drm_mode_create_rotation_property - create a new rotation property
> + * @dev: DRM device
> + * @supported_rotations: bitmask of supported rotations and reflections
> + *
> + * This creates a new property with the selected support for transformations.
> + * The resulting property should be stored in @rotation_property in
> + * &drm_mode_config. It then must be attached to each plane which support

s/support/supports

> + * rotations using drm_object_attach_property().
> + *
> + * FIXME: Probably better if the rotation property is created on each plane,
> + * like the zpos property. Otheriwise it's not possible to allow different

s/Otheriwise/Otherwise/

> + * rotation modes on different planes.
> + *
> + * Since a rotation by 180° degress is the same as reflecting both along the x
> + * and the y axis the rotation property is somewhat redundant. Drivers can use
> + * drm_rotation_simplify() to normalize values of this property.
> + *
> + * The property exposed to userspace is a bitmask property (see
> + * drm_property_create_bitmask()) called "rotation" and has the following
> + * bitmask enumaration values:
> + *
> + * DRM_ROTATE_0:
> + *     "rotate-0"
> + * DRM_ROTATE_90:
> + *     "rotate-90"
> + * DRM_ROTATE_180:
> + *     "rotate-180"
> + * DRM_ROTATE_270:
> + *     "rotate-270"
> + * DRM_REFLECT_X:
> + *     "reflect-x"
> + * DRM_REFELCT_Y:
> + *     "reflect-y"
> + *
> + * Rotation is the specified amount in degrees in counter clockwise direction,
> + * the X and Y axis are within the source rectangle, i.e.  the X/Y axis before
> + * rotation. After reflection, the rotation is applied to the image sampled from
> + * the source rectangle, before scaling it to fit the destination rectangle.
> + */
>  struct drm_property *drm_mode_create_rotation_property(struct drm_device *dev,
>                                                        unsigned int supported_rotations)
>  {
> @@ -96,10 +208,14 @@ EXPORT_SYMBOL(drm_rotation_simplify);
>   * If zpos of some planes cannot be changed (like fixed background or
>   * cursor/topmost planes), driver should adjust min/max values and assign those
>   * planes immutable zpos property with lower or higher values (for more
> - * information, see drm_mode_create_zpos_immutable_property() function). In such
> + * information, see drm_plane_create_zpos_immutable_property() function). In such
>   * case driver should also assign proper initial zpos values for all planes in
>   * its plane_reset() callback, so the planes will be always sorted properly.
>   *
> + * See also drm_atomic_normalize_zpos().
> + *
> + * The property exposed to userspace is called "zpos".
> + *
>   * Returns:
>   * Zero on success, negative errno on failure.
>   */
> @@ -135,7 +251,9 @@ EXPORT_SYMBOL(drm_plane_create_zpos_property);
>   * support for it in drm core. Using this property driver lets userspace
>   * to get the arrangement of the planes for blending operation and notifies
>   * it that the hardware (or driver) doesn't support changing of the planes'
> - * order.
> + * order. For mutable zpos see drm_plane_create_zpos_property().
> + *
> + * The property exposed to userspace is called "zpos".
>   *
>   * Returns:
>   * Zero on success, negative errno on failure.
> @@ -174,20 +292,6 @@ static int drm_atomic_state_zpos_cmp(const void *a, const void *b)
>                 return sa->plane->base.id - sb->plane->base.id;
>  }
>
> -/**
> - * drm_atomic_helper_crtc_normalize_zpos - calculate normalized zpos values
> - * @crtc: crtc with planes, which have to be considered for normalization
> - * @crtc_state: new atomic state to apply
> - *
> - * This function checks new states of all planes assigned to given crtc and
> - * calculates normalized zpos value for them. Planes are compared first by their
> - * zpos values, then by plane id (if zpos equals). Plane with lowest zpos value
> - * is at the bottom. The plane_state->normalized_zpos is then filled with unique
> - * values from 0 to number of active planes in crtc minus one.
> - *
> - * RETURNS
> - * Zero for success or -errno
> - */
>  static int drm_atomic_helper_crtc_normalize_zpos(struct drm_crtc *crtc,
>                                           struct drm_crtc_state *crtc_state)
>  {
> @@ -245,8 +349,14 @@ done:
>   * @state: atomic state of DRM device
>   *
>   * This function calculates normalized zpos value for all modified planes in
> - * the provided atomic state of DRM device. For more information, see
> - * drm_atomic_helper_crtc_normalize_zpos() function.
> + * the provided atomic state of DRM device.
> + *
> + * For every CRTC this function checks new states of all planes assigned to
> + * it and calculates normalized zpos value for these planes. Planes are compared
> + * first by their zpos values, then by plane id (if zpos is equal). The plane
> + * with lowest zpos value is at the bottom. The plane_state->normalized_zpos is
> + * then filled with unique values from 0 to number of active planes in crtc
> + * minus one.
>   *
>   * RETURNS
>   * Zero for success or -errno
> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> index 4af9456d0f40..b54240ceacd4 100644
> --- a/drivers/gpu/drm/drm_plane.c
> +++ b/drivers/gpu/drm/drm_plane.c
> @@ -45,6 +45,9 @@
>   * &drm_plane_type for a more in-depth discussion of these special uapi-relevant
>   * plane types. Special planes are associated with their CRTC by calling
>   * drm_crtc_init_with_planes().
> + *
> + * The type of a plane is exposed in the immutable "type" enumeration property,
> + * which has one of the following values: "Overlay", "Primary", "Cursor".
>   */
>
>  static unsigned int drm_num_planes(struct drm_device *dev)
> --
> 2.9.3
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

  reply	other threads:[~2016-09-06 16:59 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-08-31 16:09 [PATCH 00/10] More splitting&documenting for drm_crtc.c Daniel Vetter
2016-08-31 16:09 ` [PATCH 01/10] drm: Move a few macros away from drm_crtc.h Daniel Vetter
2016-09-06 16:59   ` [Intel-gfx] " Sean Paul
2016-09-08  0:05   ` Carlos Santa
2016-08-31 16:09 ` [PATCH 02/10] drm: Extract drm_bridge.h Daniel Vetter
2016-09-02  9:25   ` Archit Taneja
2016-08-31 16:09 ` [PATCH 03/10] drm: Move all decl for drm_edid.c to drm_edid.h Daniel Vetter
2016-09-06 16:59   ` Sean Paul
2016-09-19 14:28     ` Daniel Vetter
2016-08-31 16:09 ` [PATCH 04/10] drm: Extract drm_plane.[hc] Daniel Vetter
2016-09-06 16:59   ` [Intel-gfx] " Sean Paul
2016-09-19 13:11     ` Daniel Vetter
2016-09-21  7:32       ` Sean Paul
2016-08-31 16:09 ` [PATCH 05/10] drm/doc: Polish for drm_plane.[hc] Daniel Vetter
2016-09-02  9:30   ` Archit Taneja
2016-09-19 13:13     ` Daniel Vetter
2016-09-21  6:38       ` Archit Taneja
2016-08-31 16:09 ` [PATCH 06/10] drm: Conslidate blending properties in drm_blend.[hc] Daniel Vetter
2016-09-06 16:59   ` Sean Paul
2016-08-31 16:09 ` [PATCH 07/10] drm/doc: Polish plane composition property docs Daniel Vetter
2016-09-06 16:59   ` Sean Paul [this message]
2016-08-31 16:09 ` [PATCH 08/10] drm: Extract drm_color_mgmt.[hc] Daniel Vetter
2016-09-01  9:51   ` [Intel-gfx] " Lionel Landwerlin
2016-08-31 16:09 ` [PATCH 09/10] drm/doc: Document color space handling Daniel Vetter
2016-09-01 10:16   ` Lionel Landwerlin
2016-09-06 16:59   ` [Intel-gfx] " Sean Paul
2016-08-31 16:09 ` [PATCH 10/10] drm: Remove dirty property from docs Daniel Vetter
2016-09-06 16:59   ` Sean Paul
2016-09-01  9:20 ` ✗ Fi.CI.BAT: failure for More splitting&documenting for drm_crtc.c Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAOw6vb+14oC7+zQXYQni7rfzUzYxUYY=acfHZdprePSUKgLHmA@mail.gmail.com' \
    --to=seanpaul@chromium.org \
    --cc=daniel.vetter@ffwll.ch \
    --cc=daniel.vetter@intel.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --subject='Re: [PATCH 07/10] drm/doc: Polish plane composition property docs' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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.