On Wed, Aug 31, 2016 at 12:09 PM, Daniel Vetter <daniel.vetter@xxxxxxxx> 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@xxxxxxxxxxxx>
>
> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> Cc: Sean Paul <seanpaul@xxxxxxxxxxxx>
> Cc: Benjamin Gaignard <benjamin.gaignard@xxxxxxxxxx>
> Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx>
> ---
> 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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
