[PATCH v2] drm: Add DRM_ROTATE_ and DRM_REFLECT_ defines to UAPI

Ville Syrjälä ville.syrjala at linux.intel.com
Tue May 16 16:20:11 UTC 2017


On Tue, May 16, 2017 at 11:55:00AM -0400, Robert Foss wrote:
> Add DRM_ROTATE_ and DRM_REFLECT_ defines to the UAPI as a convenience.
> 
> Ideally the DRM_ROTATE_ and DRM_REFLECT_ property ids are looked up
> through the atomic API, but realizing that userspace is likely to take
> shortcuts and assume that the enum values are what is sent over the
> wire.
> 
> As a result these defines are provided purely as a convenience to
> userspace applications.
> 
> Signed-off-by: Robert Foss <robert.foss at collabora.com>
> ---
> Changes since v1:
>  - Moved defines from drm.h to drm_mode.h
>  - Changed define prefix from DRM_ to DRM_MODE_PROP_

DRM_MODE_PROP_ would potentially cause confusion with the prop types.
DRM_MODE_ROTATE_ etc. could be acceptable I suppose.

>  - Updated uses of the defines to the new prefix
>  - Removed include from drm_rect.c
>  - Stopped using the BIT() macro
> 
>  drivers/gpu/drm/drm_atomic.c           |  2 +-
>  drivers/gpu/drm/drm_atomic_helper.c    |  2 +-
>  drivers/gpu/drm/drm_blend.c            | 43 +++++++++----------
>  drivers/gpu/drm/drm_fb_helper.c        |  4 +-
>  drivers/gpu/drm/drm_plane_helper.c     |  2 +-
>  drivers/gpu/drm/drm_rect.c             | 36 ++++++++--------
>  drivers/gpu/drm/nouveau/nv50_display.c |  2 +-
>  include/drm/drm_blend.h                | 21 +---------
>  include/uapi/drm/drm_mode.h            | 76 ++++++++++++++++++++++++++++++++++

I'm pretty sure this won't even compile properly since it's missing all
but one driver.

>  9 files changed, 124 insertions(+), 64 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index f32506a7c1d6..ec1839b01d2a 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -769,7 +769,7 @@ int drm_atomic_plane_set_property(struct drm_plane *plane,
>  	} else if (property == config->prop_src_h) {
>  		state->src_h = val;
>  	} else if (property == plane->rotation_property) {
> -		if (!is_power_of_2(val & DRM_ROTATE_MASK))
> +		if (!is_power_of_2(val & DRM_MODE_PROP_ROTATE_MASK))
>  			return -EINVAL;
>  		state->rotation = val;
>  	} else if (property == plane->zpos_property) {
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 8be9719284b0..37f461aa5e66 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -3220,7 +3220,7 @@ void drm_atomic_helper_plane_reset(struct drm_plane *plane)
>  
>  	if (plane->state) {
>  		plane->state->plane = plane;
> -		plane->state->rotation = DRM_ROTATE_0;
> +		plane->state->rotation = DRM_MODE_PROP_ROTATE_0;
>  	}
>  }
>  EXPORT_SYMBOL(drm_atomic_helper_plane_reset);
> diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c
> index a0d0d6843288..044640a04d51 100644
> --- a/drivers/gpu/drm/drm_blend.c
> +++ b/drivers/gpu/drm/drm_blend.c
> @@ -119,15 +119,15 @@
>   * drm_property_create_bitmask()) called "rotation" and has the following
>   * bitmask enumaration values:
>   *
> - * DRM_ROTATE_0:
> + * DRM_MODE_PROP_ROTATE_0:
>   * 	"rotate-0"
> - * DRM_ROTATE_90:
> + * DRM_MODE_PROP_ROTATE_90:
>   * 	"rotate-90"
> - * DRM_ROTATE_180:
> + * DRM_MODE_PROP_ROTATE_180:
>   * 	"rotate-180"
> - * DRM_ROTATE_270:
> + * DRM_MODE_PROP_ROTATE_270:
>   * 	"rotate-270"
> - * DRM_REFLECT_X:
> + * DRM_MODE_PROP_REFLECT_X:
>   * 	"reflect-x"
>   * DRM_REFELCT_Y:
>   * 	"reflect-y"
> @@ -142,17 +142,17 @@ int drm_plane_create_rotation_property(struct drm_plane *plane,
>  				       unsigned int supported_rotations)
>  {
>  	static const struct drm_prop_enum_list props[] = {
> -		{ __builtin_ffs(DRM_ROTATE_0) - 1,   "rotate-0" },
> -		{ __builtin_ffs(DRM_ROTATE_90) - 1,  "rotate-90" },
> -		{ __builtin_ffs(DRM_ROTATE_180) - 1, "rotate-180" },
> -		{ __builtin_ffs(DRM_ROTATE_270) - 1, "rotate-270" },
> -		{ __builtin_ffs(DRM_REFLECT_X) - 1,  "reflect-x" },
> -		{ __builtin_ffs(DRM_REFLECT_Y) - 1,  "reflect-y" },
> +		{ __builtin_ffs(DRM_MODE_PROP_ROTATE_0) - 1,   "rotate-0" },
> +		{ __builtin_ffs(DRM_MODE_PROP_ROTATE_90) - 1,  "rotate-90" },
> +		{ __builtin_ffs(DRM_MODE_PROP_ROTATE_180) - 1, "rotate-180" },
> +		{ __builtin_ffs(DRM_MODE_PROP_ROTATE_270) - 1, "rotate-270" },
> +		{ __builtin_ffs(DRM_MODE_PROP_REFLECT_X) - 1,  "reflect-x" },
> +		{ __builtin_ffs(DRM_MODE_PROP_REFLECT_Y) - 1,  "reflect-y" },
>  	};
>  	struct drm_property *prop;
>  
> -	WARN_ON((supported_rotations & DRM_ROTATE_MASK) == 0);
> -	WARN_ON(!is_power_of_2(rotation & DRM_ROTATE_MASK));
> +	WARN_ON((supported_rotations & DRM_MODE_PROP_ROTATE_MASK) == 0);
> +	WARN_ON(!is_power_of_2(rotation & DRM_MODE_PROP_ROTATE_MASK));
>  	WARN_ON(rotation & ~supported_rotations);
>  
>  	prop = drm_property_create_bitmask(plane->dev, 0, "rotation",
> @@ -178,14 +178,14 @@ EXPORT_SYMBOL(drm_plane_create_rotation_property);
>   * @supported_rotations: Supported rotations
>   *
>   * Attempt to simplify the rotation to a form that is supported.
> - * Eg. if the hardware supports everything except DRM_REFLECT_X
> + * Eg. if the hardware supports everything except DRM_MODE_PROP_REFLECT_X
>   * one could call this function like this:
>   *
> - * drm_rotation_simplify(rotation, DRM_ROTATE_0 |
> - *                       DRM_ROTATE_90 | DRM_ROTATE_180 |
> - *                       DRM_ROTATE_270 | DRM_REFLECT_Y);
> + * drm_rotation_simplify(rotation, DRM_MODE_PROP_ROTATE_0 |
> + *                       DRM_MODE_PROP_ROTATE_90 | DRM_MODE_PROP_ROTATE_180 |
> + *                       DRM_MODE_PROP_ROTATE_270 | DRM_MODE_PROP_REFLECT_Y);
>   *
> - * to eliminate the DRM_ROTATE_X flag. Depending on what kind of
> + * to eliminate the DRM_MODE_PROP_ROTATE_X flag. Depending on what kind of
>   * transforms the hardware supports, this function may not
>   * be able to produce a supported transform, so the caller should
>   * check the result afterwards.
> @@ -194,9 +194,10 @@ unsigned int drm_rotation_simplify(unsigned int rotation,
>  				   unsigned int supported_rotations)
>  {
>  	if (rotation & ~supported_rotations) {
> -		rotation ^= DRM_REFLECT_X | DRM_REFLECT_Y;
> -		rotation = (rotation & DRM_REFLECT_MASK) |
> -		           BIT((ffs(rotation & DRM_ROTATE_MASK) + 1) % 4);
> +		rotation ^= DRM_MODE_PROP_REFLECT_X | DRM_MODE_PROP_REFLECT_Y;
> +		rotation = (rotation & DRM_MODE_PROP_REFLECT_MASK) |
> +		           BIT((ffs(rotation & DRM_MODE_PROP_ROTATE_MASK) + 1)
> +		           % 4);
>  	}
>  
>  	return rotation;
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 1f178b878e42..0af024a9ff1d 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -378,7 +378,7 @@ static int restore_fbdev_mode_atomic(struct drm_fb_helper *fb_helper)
>  			goto fail;
>  		}
>  
> -		plane_state->rotation = DRM_ROTATE_0;
> +		plane_state->rotation = DRM_MODE_PROP_ROTATE_0;
>  
>  		plane->old_fb = plane->fb;
>  		plane_mask |= 1 << drm_plane_index(plane);
> @@ -431,7 +431,7 @@ static int restore_fbdev_mode_legacy(struct drm_fb_helper *fb_helper)
>  		if (plane->rotation_property)
>  			drm_mode_plane_set_obj_prop(plane,
>  						    plane->rotation_property,
> -						    DRM_ROTATE_0);
> +						    DRM_MODE_PROP_ROTATE_0);
>  	}
>  
>  	for (i = 0; i < fb_helper->crtc_count; i++) {
> diff --git a/drivers/gpu/drm/drm_plane_helper.c b/drivers/gpu/drm/drm_plane_helper.c
> index b84a295230fc..d46deea69baf 100644
> --- a/drivers/gpu/drm/drm_plane_helper.c
> +++ b/drivers/gpu/drm/drm_plane_helper.c
> @@ -336,7 +336,7 @@ int drm_primary_helper_update(struct drm_plane *plane, struct drm_crtc *crtc,
>  
>  	ret = drm_plane_helper_check_update(plane, crtc, fb,
>  					    &src, &dest, &clip,
> -					    DRM_ROTATE_0,
> +					    DRM_MODE_PROP_ROTATE_0,
>  					    DRM_PLANE_HELPER_NO_SCALING,
>  					    DRM_PLANE_HELPER_NO_SCALING,
>  					    false, false, &visible);
> diff --git a/drivers/gpu/drm/drm_rect.c b/drivers/gpu/drm/drm_rect.c
> index bc5575960ebc..5adb528adb88 100644
> --- a/drivers/gpu/drm/drm_rect.c
> +++ b/drivers/gpu/drm/drm_rect.c
> @@ -310,38 +310,38 @@ void drm_rect_rotate(struct drm_rect *r,
>  {
>  	struct drm_rect tmp;
>  
> -	if (rotation & (DRM_REFLECT_X | DRM_REFLECT_Y)) {
> +	if (rotation & (DRM_MODE_PROP_REFLECT_X | DRM_MODE_PROP_REFLECT_Y)) {
>  		tmp = *r;
>  
> -		if (rotation & DRM_REFLECT_X) {
> +		if (rotation & DRM_MODE_PROP_REFLECT_X) {
>  			r->x1 = width - tmp.x2;
>  			r->x2 = width - tmp.x1;
>  		}
>  
> -		if (rotation & DRM_REFLECT_Y) {
> +		if (rotation & DRM_MODE_PROP_REFLECT_Y) {
>  			r->y1 = height - tmp.y2;
>  			r->y2 = height - tmp.y1;
>  		}
>  	}
>  
> -	switch (rotation & DRM_ROTATE_MASK) {
> -	case DRM_ROTATE_0:
> +	switch (rotation & DRM_MODE_PROP_ROTATE_MASK) {
> +	case DRM_MODE_PROP_ROTATE_0:
>  		break;
> -	case DRM_ROTATE_90:
> +	case DRM_MODE_PROP_ROTATE_90:
>  		tmp = *r;
>  		r->x1 = tmp.y1;
>  		r->x2 = tmp.y2;
>  		r->y1 = width - tmp.x2;
>  		r->y2 = width - tmp.x1;
>  		break;
> -	case DRM_ROTATE_180:
> +	case DRM_MODE_PROP_ROTATE_180:
>  		tmp = *r;
>  		r->x1 = width - tmp.x2;
>  		r->x2 = width - tmp.x1;
>  		r->y1 = height - tmp.y2;
>  		r->y2 = height - tmp.y1;
>  		break;
> -	case DRM_ROTATE_270:
> +	case DRM_MODE_PROP_ROTATE_270:
>  		tmp = *r;
>  		r->x1 = height - tmp.y2;
>  		r->x2 = height - tmp.y1;
> @@ -373,8 +373,8 @@ EXPORT_SYMBOL(drm_rect_rotate);
>   * them when doing a rotatation and its inverse.
>   * That is, if you do ::
>   *
> - *     drm_rotate(&r, width, height, rotation);
> - *     drm_rotate_inv(&r, width, height, rotation);
> + *     DRM_MODE_PROP_ROTATE(&r, width, height, rotation);
> + *     DRM_MODE_PROP_ROTATE_inv(&r, width, height, rotation);
>   *
>   * you will always get back the original rectangle.
>   */
> @@ -384,24 +384,24 @@ void drm_rect_rotate_inv(struct drm_rect *r,
>  {
>  	struct drm_rect tmp;
>  
> -	switch (rotation & DRM_ROTATE_MASK) {
> -	case DRM_ROTATE_0:
> +	switch (rotation & DRM_MODE_PROP_ROTATE_MASK) {
> +	case DRM_MODE_PROP_ROTATE_0:
>  		break;
> -	case DRM_ROTATE_90:
> +	case DRM_MODE_PROP_ROTATE_90:
>  		tmp = *r;
>  		r->x1 = width - tmp.y2;
>  		r->x2 = width - tmp.y1;
>  		r->y1 = tmp.x1;
>  		r->y2 = tmp.x2;
>  		break;
> -	case DRM_ROTATE_180:
> +	case DRM_MODE_PROP_ROTATE_180:
>  		tmp = *r;
>  		r->x1 = width - tmp.x2;
>  		r->x2 = width - tmp.x1;
>  		r->y1 = height - tmp.y2;
>  		r->y2 = height - tmp.y1;
>  		break;
> -	case DRM_ROTATE_270:
> +	case DRM_MODE_PROP_ROTATE_270:
>  		tmp = *r;
>  		r->x1 = tmp.y1;
>  		r->x2 = tmp.y2;
> @@ -412,15 +412,15 @@ void drm_rect_rotate_inv(struct drm_rect *r,
>  		break;
>  	}
>  
> -	if (rotation & (DRM_REFLECT_X | DRM_REFLECT_Y)) {
> +	if (rotation & (DRM_MODE_PROP_REFLECT_X | DRM_MODE_PROP_REFLECT_Y)) {
>  		tmp = *r;
>  
> -		if (rotation & DRM_REFLECT_X) {
> +		if (rotation & DRM_MODE_PROP_REFLECT_X) {
>  			r->x1 = width - tmp.x2;
>  			r->x2 = width - tmp.x1;
>  		}
>  
> -		if (rotation & DRM_REFLECT_Y) {
> +		if (rotation & DRM_MODE_PROP_REFLECT_Y) {
>  			r->y1 = height - tmp.y2;
>  			r->y2 = height - tmp.y1;
>  		}
> diff --git a/drivers/gpu/drm/nouveau/nv50_display.c b/drivers/gpu/drm/nouveau/nv50_display.c
> index a7663249b3ba..082c1012b138 100644
> --- a/drivers/gpu/drm/nouveau/nv50_display.c
> +++ b/drivers/gpu/drm/nouveau/nv50_display.c
> @@ -1033,7 +1033,7 @@ nv50_wndw_reset(struct drm_plane *plane)
>  		plane->funcs->atomic_destroy_state(plane, plane->state);
>  	plane->state = &asyw->state;
>  	plane->state->plane = plane;
> -	plane->state->rotation = DRM_ROTATE_0;
> +	plane->state->rotation = DRM_MODE_PROP_ROTATE_0;
>  }
>  
>  static void
> diff --git a/include/drm/drm_blend.h b/include/drm/drm_blend.h
> index 13221cf9b3eb..b59708c1e7a6 100644
> --- a/include/drm/drm_blend.h
> +++ b/include/drm/drm_blend.h
> @@ -25,31 +25,14 @@
>  
>  #include <linux/list.h>
>  #include <linux/ctype.h>
> +#include <drm/drm_mode.h>
>  
>  struct drm_device;
>  struct drm_atomic_state;
>  
> -/*
> - * Rotation property bits. DRM_ROTATE_<degrees> rotates the image by the
> - * specified amount in degrees in counter clockwise direction. DRM_REFLECT_X and
> - * DRM_REFLECT_Y reflects the image along the specified axis prior to rotation
> - *
> - * WARNING: These defines are UABI since they're exposed in the rotation
> - * property.
> - */
> -#define DRM_ROTATE_0	BIT(0)
> -#define DRM_ROTATE_90	BIT(1)
> -#define DRM_ROTATE_180	BIT(2)
> -#define DRM_ROTATE_270	BIT(3)
> -#define DRM_ROTATE_MASK (DRM_ROTATE_0   | DRM_ROTATE_90 | \
> -			 DRM_ROTATE_180 | DRM_ROTATE_270)
> -#define DRM_REFLECT_X	BIT(4)
> -#define DRM_REFLECT_Y	BIT(5)
> -#define DRM_REFLECT_MASK (DRM_REFLECT_X | DRM_REFLECT_Y)
> -
>  static inline bool drm_rotation_90_or_270(unsigned int rotation)
>  {
> -	return rotation & (DRM_ROTATE_90 | DRM_ROTATE_270);
> +	return rotation & (DRM_MODE_PROP_ROTATE_90 | DRM_MODE_PROP_ROTATE_270);
>  }
>  
>  int drm_plane_create_rotation_property(struct drm_plane *plane,
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index 8c67fc03d53d..787a70ba974c 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -127,6 +127,82 @@ extern "C" {
>  #define DRM_MODE_LINK_STATUS_GOOD	0
>  #define DRM_MODE_LINK_STATUS_BAD	1
>  
> +/** DRM_MODE_PROP_ROTATE_0

Is this supposed to be kernel-doc or something like that?

> + *
> + * Signals that a drm plane has been rotated 0 degrees.

Past tense doesn't feel right to me. Maybe "is rotated"?
But I'm not a native speaker so maybe it's just me.

> + *
> + * This define is provided as a convenience, looking up the property id
> + * using the name->prop id lookup is the preferred method.

Repeating this for every define seems redundant.

> + */
> +#define DRM_MODE_PROP_ROTATE_0           (1<<0)
> +
> +/** DRM_MODE_PROP_ROTATE_90
> + *
> + * Signals that a drm plane has been rotated 90 degrees in counter clockwise
> + * direction.
> + *
> + * This define is provided as a convenience, looking up the property id
> + * using the name->prop id lookup is the preferred method.
> + */
> +#define DRM_MODE_PROP_ROTATE_90          (1<<1)
> +
> +/** DRM_MODE_PROP_ROTATE_180
> + *
> + * Signals that a drm plane has been rotated 180 degrees in counter clockwise
> + * direction.
> + *
> + * This define is provided as a convenience, looking up the property id
> + * using the name->prop id lookup is the preferred method.
> + */
> +#define DRM_MODE_PROP_ROTATE_180         (1<<2)
> +
> +/** DRM_MODE_PROP_ROTATE_270
> + *
> + * Signals that a drm plane has been rotated 270 degrees in counter clockwise
> + * direction.
> + *
> + * This define is provided as a convenience, looking up the property id
> + * using the name->prop id lookup is the preferred method.
> + */
> +#define DRM_MODE_PROP_ROTATE_270         (1<<3)
> +
> +
> +/** DRM_MODE_PROP_ROTATE_MASK
> + *
> + * Bitmask used to look for drm plane rotations.
> + */
> +#define DRM_MODE_PROP_ROTATE_MASK (DRM_MODE_PROP_ROTATE_0  | \
> +                                  DRM_MODE_PROP_ROTATE_90  | \
> +                                  DRM_MODE_PROP_ROTATE_180 | \
> +                                  DRM_MODE_PROP_ROTATE_270)
> +
> +/** DRM_MODE_PROP_REFLECT_X
> + *
> + * Signals that a drm plane has been reflected in the X axis.

Seems more vague that what we had before.

> + *
> + * This define is provided as a convenience, looking up the property id
> + * using the name->prop id lookup is the preferred method.
> + */
> +#define DRM_MODE_PROP_REFLECT_X          (1<<4)
> +
> +/** DRM_MODE_PROP_REFLECT_Y
> + *
> + * Signals that a drm plane has been reflected in the Y axis.
> + *
> + * This define is provided as a convenience, looking up the property id
> + * using the name->prop id lookup is the preferred method.
> + */
> +#define DRM_MODE_PROP_REFLECT_Y          (1<<5)
> +
> +
> +/** DRM_MODE_PROP_REFLECT_MASK
> + *
> + * Bitmask used to look for drm plane reflections.
> + */
> +#define DRM_MODE_PROP_REFLECT_MASK (DRM_MODE_PROP_REFLECT_X \
> +                                   | DRM_MODE_PROP_REFLECT_Y)
> +
> +
>  struct drm_mode_modeinfo {
>  	__u32 clock;
>  	__u16 hdisplay;
> -- 
> 2.11.0.453.g787f75f05
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel OTC


More information about the dri-devel mailing list