[PATCH] modesetting: Support native primary plane rotation

[PATCH] modesetting: Support native primary plane rotation

[Chris Wilson]

With the advent of universal drm planes and the introduction of generic
plane properties for rotations, we can query and program the hardware
for native rotation support.

NOTE: this depends upon the next release of libdrm to remove some
opencoded defines.

Signed-off-by: Chris Wilson <chris-Y6uKTt2uX1cEflXRtASbqLVCufUGDwFn@public.gmane.org>
---
 configure.ac          |   2 +-
 src/drmmode_display.c | 223 +++++++++++++++++++++++++++++++++++++++++++-------
 src/drmmode_display.h |   7 +-
 3 files changed, 199 insertions(+), 33 deletions(-)

diff --git a/configure.ac b/configure.ac
index 1c1a36d..0b4e857 100644
--- a/configure.ac
+++ b/configure.ac
@@ -74,7 +74,7 @@ AM_CONDITIONAL(HAVE_XEXTPROTO_71, [ test "$HAVE_XEXTPROTO_71" = "yes" ])
 # Checks for header files.
 AC_HEADER_STDC
 
-PKG_CHECK_MODULES(DRM, [libdrm >= 2.4.46])
+PKG_CHECK_MODULES(DRM, [libdrm >= 2.4.54]) #.55 required for universal planes
 PKG_CHECK_MODULES([PCIACCESS], [pciaccess >= 0.10])
 AM_CONDITIONAL(DRM, test "x$DRM" = xyes)
 
diff --git a/src/drmmode_display.c b/src/drmmode_display.c
index c533324..aaeda39 100644
--- a/src/drmmode_display.c
+++ b/src/drmmode_display.c
@@ -56,6 +56,11 @@
 
 #include "driver.h"
 
+#define DRM_CLIENT_CAP_UNIVERSAL_PLANES 2
+#define DRM_PLANE_TYPE_OVERLAY 0
+#define DRM_PLANE_TYPE_PRIMARY 1
+#define DRM_PLANE_TYPE_CURSOR  2
+
 static struct dumb_bo *dumb_bo_create(int fd,
 			  const unsigned width, const unsigned height,
 			  const unsigned bpp)
@@ -300,6 +305,136 @@ create_pixmap_for_fbcon(drmmode_ptr drmmode,
 
 #endif
 
+static unsigned
+rotation_index(unsigned rotation)
+{
+#if _SVID_SOURCE || _BSD_SOURCE || _POSIX_C_SOURCE >= 200809L || _XOPEN_SOURCE >= 700
+	return ffs(rotation) - 1;
+#else
+	int i;
+
+	for (i = 0; i < 32; i++) {
+		if ((1 << i) == rotation)
+			break;
+	}
+
+	return i;
+#endif
+}
+
+static void
+rotation_init(xf86CrtcPtr crtc)
+{
+	drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
+	drmmode_ptr drmmode = drmmode_crtc->drmmode;
+	drmModePlaneRes *plane_resources;
+	int i, j, k;
+
+	drmSetClientCap(drmmode->fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1);
+
+	plane_resources = drmModeGetPlaneResources(drmmode->fd);
+	if (plane_resources == NULL)
+		return;
+
+	for (i = 0; i < plane_resources->count_planes; i++) {
+		drmModePlane *drm_plane;
+		drmModeObjectPropertiesPtr proplist;
+		int type = -1;
+
+		drm_plane = drmModeGetPlane(drmmode->fd,
+					    plane_resources->planes[i]);
+		if (drm_plane == NULL)
+			continue;
+
+		if (!(drm_plane->possible_crtcs & (1 << drmmode_crtc->index)))
+			goto free_plane;
+
+		proplist = drmModeObjectGetProperties(drmmode->fd,
+						      drm_plane->plane_id,
+						      DRM_MODE_OBJECT_PLANE);
+		if (proplist == NULL)
+			goto free_plane;
+
+		for (j = 0; type == -1 && j < proplist->count_props; j++) {
+			drmModePropertyPtr prop;
+
+			prop = drmModeGetProperty(drmmode->fd, proplist->props[j]);
+			if (!prop)
+				continue;
+
+			if (strcmp(prop->name, "type") == 0)
+				type = proplist->prop_values[j];
+
+			drmModeFreeProperty(prop);
+		}
+
+		if (type == DRM_PLANE_TYPE_PRIMARY) {
+			drmmode_crtc->primary_plane_id = drm_plane->plane_id;
+
+			for (j = 0; drmmode_crtc->rotation_prop_id == 0 && j < proplist->count_props; j++) {
+				drmModePropertyPtr prop;
+
+				prop = drmModeGetProperty(drmmode->fd, proplist->props[j]);
+				if (!prop)
+					continue;
+
+				if (strcmp(prop->name, "rotation") == 0) {
+					drmmode_crtc->rotation_prop_id = proplist->props[j];
+					drmmode_crtc->current_rotation = proplist->prop_values[j];
+					for (k = 0; k < prop->count_enums; k++) {
+						int rr = -1;
+						if (strcmp(prop->enums[k].name, "rotate-0") == 0)
+							rr = RR_Rotate_0;
+						else if (strcmp(prop->enums[k].name, "rotate-90") == 0)
+							rr = RR_Rotate_90;
+						else if (strcmp(prop->enums[k].name, "rotate-180") == 0)
+							rr = RR_Rotate_180;
+						else if (strcmp(prop->enums[k].name, "rotate-270") == 0)
+							rr = RR_Rotate_270;
+						else if (strcmp(prop->enums[k].name, "reflect-x") == 0)
+							rr = RR_Reflect_X;
+						else if (strcmp(prop->enums[k].name, "reflect-y") == 0)
+							rr = RR_Reflect_Y;
+						if (rr != -1) {
+							drmmode_crtc->map_rotations[rotation_index(rr)] = 1 << prop->enums[k].value;
+							drmmode_crtc->supported_rotations |= rr;
+						}
+					}
+				}
+
+				drmModeFreeProperty(prop);
+			}
+		}
+
+		drmModeFreeObjectProperties(proplist);
+free_plane:
+		drmModeFreePlane(drm_plane);
+	}
+}
+
+static Bool
+rotation_set(xf86CrtcPtr crtc, unsigned rotation)
+{
+	drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
+	drmmode_ptr drmmode = drmmode_crtc->drmmode;
+
+	if (drmmode_crtc->current_rotation == rotation)
+		return TRUE;
+
+	if ((drmmode_crtc->supported_rotations & rotation) == 0)
+		return FALSE;
+
+	if (drmModeObjectSetProperty(drmmode->fd,
+				     drmmode_crtc->primary_plane_id,
+				     DRM_MODE_OBJECT_PLANE,
+				     drmmode_crtc->rotation_prop_id,
+				     drmmode_crtc->map_rotations[rotation_index(rotation)]))
+		return FALSE;
+
+	drmmode_crtc->current_rotation = rotation;
+	return TRUE;
+}
+
 static Bool
 drmmode_set_mode_major(xf86CrtcPtr crtc, DisplayModePtr mode,
 		     Rotation rotation, int x, int y)
@@ -307,13 +442,14 @@ drmmode_set_mode_major(xf86CrtcPtr crtc, DisplayModePtr mode,
 	ScrnInfoPtr pScrn = crtc->scrn;
 	xf86CrtcConfigPtr   xf86_config = XF86_CRTC_CONFIG_PTR(crtc->scrn);
 	drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
+	unsigned supported_rotations = drmmode_crtc->supported_rotations;
 	drmmode_ptr drmmode = drmmode_crtc->drmmode;
 	int saved_x, saved_y;
 	Rotation saved_rotation;
 	DisplayModeRec saved_mode;
 	uint32_t *output_ids;
 	int output_count = 0;
-	Bool ret = TRUE;
+	int ret;
 	int i;
 	uint32_t fb_id;
 	drmModeModeInfo kmode;
@@ -324,15 +460,16 @@ drmmode_set_mode_major(xf86CrtcPtr crtc, DisplayModePtr mode,
 	if (drmmode->fb_id == 0) {
 		ret = drmModeAddFB(drmmode->fd,
 				   pScrn->virtualX, height,
-                                   pScrn->depth, pScrn->bitsPerPixel,
+				   pScrn->depth, pScrn->bitsPerPixel,
 				   drmmode->front_bo->pitch,
 				   drmmode->front_bo->handle,
-                                   &drmmode->fb_id);
-                if (ret < 0) {
-                        ErrorF("failed to add fb %d\n", ret);
-                        return FALSE;
-                }
-        }
+				   &drmmode->fb_id);
+		if (ret) {
+			xf86DrvMsg(crtc->scrn->scrnIndex, X_ERROR,
+				   "failed to create framebuffer: %s\n", strerror(-ret));
+			return FALSE;
+		}
+	}
 
 	saved_mode = crtc->mode;
 	saved_x = crtc->x;
@@ -350,10 +487,8 @@ drmmode_set_mode_major(xf86CrtcPtr crtc, DisplayModePtr mode,
 	}
 
 	output_ids = calloc(sizeof(uint32_t), xf86_config->num_output);
-	if (!output_ids) {
-		ret = FALSE;
-		goto done;
-	}
+	if (!output_ids)
+		goto err;
 
 	if (mode) {
 		for (i = 0; i < xf86_config->num_output; i++) {
@@ -368,9 +503,16 @@ drmmode_set_mode_major(xf86CrtcPtr crtc, DisplayModePtr mode,
 			output_count++;
 		}
 
-		if (!xf86CrtcRotate(crtc)) {
-			goto done;
-		}
+		rotation_set(crtc, RR_Rotate_0);
+
+again:
+		crtc->driverIsPerformingTransform = FALSE;
+		if (rotation & supported_rotations && !crtc->transformPresent)
+			crtc->driverIsPerformingTransform = TRUE;
+
+		if (!xf86CrtcRotate(crtc))
+			goto err;
+
 #if XORG_VERSION_CURRENT >= XORG_VERSION_NUMERIC(1,7,0,0,0)
 		crtc->funcs->gamma_set(crtc, crtc->gamma_red, crtc->gamma_green,
 				       crtc->gamma_blue, crtc->gamma_size);
@@ -392,11 +534,20 @@ drmmode_set_mode_major(xf86CrtcPtr crtc, DisplayModePtr mode,
 		}
 		ret = drmModeSetCrtc(drmmode->fd, drmmode_crtc->mode_crtc->crtc_id,
 				     fb_id, x, y, output_ids, output_count, &kmode);
-		if (ret)
+		if (ret) {
 			xf86DrvMsg(crtc->scrn->scrnIndex, X_ERROR,
-				   "failed to set mode: %s", strerror(-ret));
-		else
-			ret = TRUE;
+				   "failed to set mode: %s\n", strerror(-ret));
+			goto err;
+		}
+
+		if (crtc->driverIsPerformingTransform) {
+			if (!rotation_set(crtc, crtc->rotation)) {
+				xf86DrvMsg(crtc->scrn->scrnIndex, X_ERROR,
+					   "failed to set rotation: %s\n", strerror(errno));
+				supported_rotations &= ~crtc->rotation;
+				goto again;
+			}
+		}
 
 		if (crtc->scrn->pScreen)
 			xf86CrtcSetScreenSubpixelOrder(crtc->scrn->pScreen);
@@ -409,26 +560,33 @@ drmmode_set_mode_major(xf86CrtcPtr crtc, DisplayModePtr mode,
 
 			output->funcs->dpms(output, DPMSModeOn);
 		}
+	} else {
+		ret = drmModeSetCrtc(drmmode->fd, drmmode_crtc->mode_crtc->crtc_id,
+				     0, 0, 0, NULL, 0, NULL) == 0;
+		if (ret) {
+			xf86DrvMsg(crtc->scrn->scrnIndex, X_ERROR,
+				   "failed to set mode: %s\n", strerror(-ret));
+			goto err;
+		}
 	}
 
+#if defined(XF86_CRTC_VERSION) && XF86_CRTC_VERSION >= 3
+	crtc->active = mode != NULL;
+#endif
+
 #if 0
 	if (pScrn->pScreen &&
 		!xf86ReturnOptValBool(info->Options, OPTION_SW_CURSOR, FALSE))
 		xf86_reload_cursors(pScrn->pScreen);
 #endif
-done:
-	if (!ret) {
-		crtc->x = saved_x;
-		crtc->y = saved_y;
-		crtc->rotation = saved_rotation;
-		crtc->mode = saved_mode;
-	}
-#if defined(XF86_CRTC_VERSION) && XF86_CRTC_VERSION >= 3
-	else
-		crtc->active = TRUE;
-#endif
+	return TRUE;
 
-	return ret;
+err:
+	crtc->x = saved_x;
+	crtc->y = saved_y;
+	crtc->rotation = saved_rotation;
+	crtc->mode = saved_mode;
+	return FALSE;
 }
 
 static void
@@ -610,7 +768,10 @@ drmmode_crtc_init(ScrnInfoPtr pScrn, drmmode_ptr drmmode, int num)
 	drmmode_crtc = xnfcalloc(sizeof(drmmode_crtc_private_rec), 1);
 	drmmode_crtc->mode_crtc = drmModeGetCrtc(drmmode->fd, drmmode->mode_res->crtcs[num]);
 	drmmode_crtc->drmmode = drmmode;
+	drmmode_crtc->index = num;
 	crtc->driver_private = drmmode_crtc;
+
+	rotation_init(crtc);
 }
 
 static xf86OutputStatus
diff --git a/src/drmmode_display.h b/src/drmmode_display.h
index 745c484..8eb25fd 100644
--- a/src/drmmode_display.h
+++ b/src/drmmode_display.h
@@ -75,8 +75,13 @@ typedef struct {
 typedef struct {
     drmmode_ptr drmmode;
     drmModeCrtcPtr mode_crtc;
-    int hw_id;
+    int index;
     struct dumb_bo *cursor_bo;
+    unsigned primary_plane_id;
+    unsigned rotation_prop_id;
+    unsigned supported_rotations;
+    unsigned map_rotations[6];
+    unsigned current_rotation;
     unsigned rotate_fb_id;
     uint16_t lut_r[256], lut_g[256], lut_b[256];
     DamagePtr slave_damage;
-- 
2.0.1

_______________________________________________
xorg-devel-go0+a7rfsptAfugRpC6u6w@public.gmane.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH] modesetting: Support native primary plane rotation

[Pekka Paalanen]

On Wed,  9 Jul 2014 08:00:21 +0100
Chris Wilson <chris-Y6uKTt2uX1cEflXRtASbqLVCufUGDwFn@public.gmane.org> wrote:

> With the advent of universal drm planes and the introduction of generic
> plane properties for rotations, we can query and program the hardware
> for native rotation support.
> 
> NOTE: this depends upon the next release of libdrm to remove some
> opencoded defines.
> 
> Signed-off-by: Chris Wilson <chris-Y6uKTt2uX1cEflXRtASbqLVCufUGDwFn@public.gmane.org>
> ---
>  configure.ac          |   2 +-
>  src/drmmode_display.c | 223 +++++++++++++++++++++++++++++++++++++++++++-------
>  src/drmmode_display.h |   7 +-
>  3 files changed, 199 insertions(+), 33 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index 1c1a36d..0b4e857 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -74,7 +74,7 @@ AM_CONDITIONAL(HAVE_XEXTPROTO_71, [ test "$HAVE_XEXTPROTO_71" = "yes" ])
>  # Checks for header files.
>  AC_HEADER_STDC
>  
> -PKG_CHECK_MODULES(DRM, [libdrm >= 2.4.46])
> +PKG_CHECK_MODULES(DRM, [libdrm >= 2.4.54]) #.55 required for universal planes
>  PKG_CHECK_MODULES([PCIACCESS], [pciaccess >= 0.10])
>  AM_CONDITIONAL(DRM, test "x$DRM" = xyes)
>  
> diff --git a/src/drmmode_display.c b/src/drmmode_display.c
> index c533324..aaeda39 100644
> --- a/src/drmmode_display.c
> +++ b/src/drmmode_display.c
> @@ -56,6 +56,11 @@
>  
>  #include "driver.h"
>  
> +#define DRM_CLIENT_CAP_UNIVERSAL_PLANES 2
> +#define DRM_PLANE_TYPE_OVERLAY 0
> +#define DRM_PLANE_TYPE_PRIMARY 1
> +#define DRM_PLANE_TYPE_CURSOR  2

Hi,

is this really something that is guaranteed to be kernel ABI stable?

I mean, the 'type' property is an enum. I have never seen the enum
(numerical) values being defined in any public ABI header. Instead,
the property system has a mechanism for listing the enum values by
name string.

I have assumed that the name string is what is guaranteed ABI, and
the numerical value is just an arbitrary handle. When I added
universal planes support to Weston (not merged yet), I look up the
numerical value by the name, instead of hardcoding the numerical
value.

Should you do the same here, or are the numerical values really
(going to be) part of the ABI?


Thanks,
pq


> +static void
> +rotation_init(xf86CrtcPtr crtc)
> +{
> +	drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
> +	drmmode_ptr drmmode = drmmode_crtc->drmmode;
> +	drmModePlaneRes *plane_resources;
> +	int i, j, k;
> +
> +	drmSetClientCap(drmmode->fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1);
> +
> +	plane_resources = drmModeGetPlaneResources(drmmode->fd);
> +	if (plane_resources == NULL)
> +		return;
> +
> +	for (i = 0; i < plane_resources->count_planes; i++) {
> +		drmModePlane *drm_plane;
> +		drmModeObjectPropertiesPtr proplist;
> +		int type = -1;
> +
> +		drm_plane = drmModeGetPlane(drmmode->fd,
> +					    plane_resources->planes[i]);
> +		if (drm_plane == NULL)
> +			continue;
> +
> +		if (!(drm_plane->possible_crtcs & (1 << drmmode_crtc->index)))
> +			goto free_plane;
> +
> +		proplist = drmModeObjectGetProperties(drmmode->fd,
> +						      drm_plane->plane_id,
> +						      DRM_MODE_OBJECT_PLANE);
> +		if (proplist == NULL)
> +			goto free_plane;
> +
> +		for (j = 0; type == -1 && j < proplist->count_props; j++) {
> +			drmModePropertyPtr prop;
> +
> +			prop = drmModeGetProperty(drmmode->fd, proplist->props[j]);
> +			if (!prop)
> +				continue;
> +
> +			if (strcmp(prop->name, "type") == 0)
> +				type = proplist->prop_values[j];
> +
> +			drmModeFreeProperty(prop);
> +		}
> +
> +		if (type == DRM_PLANE_TYPE_PRIMARY) {
> +			drmmode_crtc->primary_plane_id = drm_plane->plane_id;
> +
> +			for (j = 0; drmmode_crtc->rotation_prop_id == 0 && j < proplist->count_props; j++) {
> +				drmModePropertyPtr prop;
> +
> +				prop = drmModeGetProperty(drmmode->fd, proplist->props[j]);
> +				if (!prop)
> +					continue;
> +
> +				if (strcmp(prop->name, "rotation") == 0) {
> +					drmmode_crtc->rotation_prop_id = proplist->props[j];
> +					drmmode_crtc->current_rotation = proplist->prop_values[j];
> +					for (k = 0; k < prop->count_enums; k++) {
> +						int rr = -1;
> +						if (strcmp(prop->enums[k].name, "rotate-0") == 0)
> +							rr = RR_Rotate_0;
> +						else if (strcmp(prop->enums[k].name, "rotate-90") == 0)
> +							rr = RR_Rotate_90;
> +						else if (strcmp(prop->enums[k].name, "rotate-180") == 0)
> +							rr = RR_Rotate_180;
> +						else if (strcmp(prop->enums[k].name, "rotate-270") == 0)
> +							rr = RR_Rotate_270;
> +						else if (strcmp(prop->enums[k].name, "reflect-x") == 0)
> +							rr = RR_Reflect_X;
> +						else if (strcmp(prop->enums[k].name, "reflect-y") == 0)
> +							rr = RR_Reflect_Y;
> +						if (rr != -1) {
> +							drmmode_crtc->map_rotations[rotation_index(rr)] = 1 << prop->enums[k].value;
> +							drmmode_crtc->supported_rotations |= rr;
> +						}
> +					}
> +				}
> +
> +				drmModeFreeProperty(prop);
> +			}
> +		}
> +
> +		drmModeFreeObjectProperties(proplist);
> +free_plane:
> +		drmModeFreePlane(drm_plane);
> +	}
> +}
_______________________________________________
xorg-devel-go0+a7rfsptAfugRpC6u6w@public.gmane.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH] modesetting: Support native primary plane rotation

[Chris Wilson]

On Wed, Jul 09, 2014 at 10:44:17AM +0300, Pekka Paalanen wrote:
> On Wed,  9 Jul 2014 08:00:21 +0100
> Chris Wilson <chris-Y6uKTt2uX1cEflXRtASbqLVCufUGDwFn@public.gmane.org> wrote:
> 
> > With the advent of universal drm planes and the introduction of generic
> > plane properties for rotations, we can query and program the hardware
> > for native rotation support.
> > 
> > NOTE: this depends upon the next release of libdrm to remove some
> > opencoded defines.
> > 
> > Signed-off-by: Chris Wilson <chris-Y6uKTt2uX1cEflXRtASbqLVCufUGDwFn@public.gmane.org>
> > ---
> >  configure.ac          |   2 +-
> >  src/drmmode_display.c | 223 +++++++++++++++++++++++++++++++++++++++++++-------
> >  src/drmmode_display.h |   7 +-
> >  3 files changed, 199 insertions(+), 33 deletions(-)
> > 
> > diff --git a/configure.ac b/configure.ac
> > index 1c1a36d..0b4e857 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -74,7 +74,7 @@ AM_CONDITIONAL(HAVE_XEXTPROTO_71, [ test "$HAVE_XEXTPROTO_71" = "yes" ])
> >  # Checks for header files.
> >  AC_HEADER_STDC
> >  
> > -PKG_CHECK_MODULES(DRM, [libdrm >= 2.4.46])
> > +PKG_CHECK_MODULES(DRM, [libdrm >= 2.4.54]) #.55 required for universal planes
> >  PKG_CHECK_MODULES([PCIACCESS], [pciaccess >= 0.10])
> >  AM_CONDITIONAL(DRM, test "x$DRM" = xyes)
> >  
> > diff --git a/src/drmmode_display.c b/src/drmmode_display.c
> > index c533324..aaeda39 100644
> > --- a/src/drmmode_display.c
> > +++ b/src/drmmode_display.c
> > @@ -56,6 +56,11 @@
> >  
> >  #include "driver.h"
> >  
> > +#define DRM_CLIENT_CAP_UNIVERSAL_PLANES 2
> > +#define DRM_PLANE_TYPE_OVERLAY 0
> > +#define DRM_PLANE_TYPE_PRIMARY 1
> > +#define DRM_PLANE_TYPE_CURSOR  2
> 
> Hi,
> 
> is this really something that is guaranteed to be kernel ABI stable?
> 
> I mean, the 'type' property is an enum. I have never seen the enum
> (numerical) values being defined in any public ABI header. Instead,
> the property system has a mechanism for listing the enum values by
> name string.
> 
> I have assumed that the name string is what is guaranteed ABI, and
> the numerical value is just an arbitrary handle. When I added
> universal planes support to Weston (not merged yet), I look up the
> numerical value by the name, instead of hardcoding the numerical
> value.
> 
> Should you do the same here, or are the numerical values really
> (going to be) part of the ABI?

Oops, I was trying to keep it generic and only use enum names and
overlooked that the plane type was an enum.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
xorg-devel-go0+a7rfsptAfugRpC6u6w@public.gmane.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

[PATCH] modesetting: Support native primary plane rotation

[Chris Wilson]

With the advent of universal drm planes and the introduction of generic
plane properties for rotations, we can query and program the hardware
for native rotation support.

NOTE: this depends upon the next release of libdrm to remove one
opencoded define.

v2: Use enum to determine primary plane, suggested by Pekka Paalanen.
    Use libobj for replacement ffs(), suggested by walter harms

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Pekka Paalanen <ppaalanen@gmail.com>
Cc: walter harms <wharms@bfs.de>
---
 configure.ac          |   5 +-
 libobj/ffs.c          |  14 ++++
 src/drmmode_display.c | 216 ++++++++++++++++++++++++++++++++++++++++++--------
 src/drmmode_display.h |  10 ++-
 4 files changed, 212 insertions(+), 33 deletions(-)
 create mode 100644 libobj/ffs.c

diff --git a/configure.ac b/configure.ac
index 1c1a36d..1694465 100644
--- a/configure.ac
+++ b/configure.ac
@@ -74,10 +74,13 @@ AM_CONDITIONAL(HAVE_XEXTPROTO_71, [ test "$HAVE_XEXTPROTO_71" = "yes" ])
 # Checks for header files.
 AC_HEADER_STDC
 
-PKG_CHECK_MODULES(DRM, [libdrm >= 2.4.46])
+PKG_CHECK_MODULES(DRM, [libdrm >= 2.4.47])
 PKG_CHECK_MODULES([PCIACCESS], [pciaccess >= 0.10])
 AM_CONDITIONAL(DRM, test "x$DRM" = xyes)
 
+AC_CONFIG_LIBOBJ_DIR(libobj)
+AC_REPLACE_FUNCS(ffs)
+
 PKG_CHECK_MODULES(UDEV, [libudev], [udev=yes], [udev=no])
 if test x"$udev" = xyes; then
         AC_DEFINE(HAVE_UDEV,1,[Enable udev-based monitor hotplug detection])
diff --git a/libobj/ffs.c b/libobj/ffs.c
new file mode 100644
index 0000000..2d44dcc
--- /dev/null
+++ b/libobj/ffs.c
@@ -0,0 +1,14 @@
+extern int ffs(unsigned int value);
+
+int ffs(unsigned int value)
+{
+	int bit;
+
+	if (value == 0)
+		return 0;
+
+	bit = 0;
+	while ((value & (1 << bit++)) == 0)
+		;
+	return bit;
+}
diff --git a/src/drmmode_display.c b/src/drmmode_display.c
index c533324..e854502 100644
--- a/src/drmmode_display.c
+++ b/src/drmmode_display.c
@@ -56,6 +56,8 @@
 
 #include "driver.h"
 
+#define DRM_CLIENT_CAP_UNIVERSAL_PLANES 2 /* from libdrm 2.4.55 */
+
 static struct dumb_bo *dumb_bo_create(int fd,
 			  const unsigned width, const unsigned height,
 			  const unsigned bpp)
@@ -300,6 +302,132 @@ create_pixmap_for_fbcon(drmmode_ptr drmmode,
 
 #endif
 
+static unsigned
+rotation_index(unsigned rotation)
+{
+	return ffs(rotation) - 1;
+}
+
+static void
+rotation_init(xf86CrtcPtr crtc)
+{
+	drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
+	drmmode_ptr drmmode = drmmode_crtc->drmmode;
+	drmModePlaneRes *plane_resources;
+	int i, j, k;
+
+	drmSetClientCap(drmmode->fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1);
+
+	plane_resources = drmModeGetPlaneResources(drmmode->fd);
+	if (plane_resources == NULL)
+		return;
+
+	for (i = 0; drmmode_crtc->primary_plane_id == 0 && i < plane_resources->count_planes; i++) {
+		drmModePlane *drm_plane;
+		drmModeObjectPropertiesPtr proplist;
+		int is_primary = -1;
+
+		drm_plane = drmModeGetPlane(drmmode->fd,
+					    plane_resources->planes[i]);
+		if (drm_plane == NULL)
+			continue;
+
+		if (!(drm_plane->possible_crtcs & (1 << drmmode_crtc->index)))
+			goto free_plane;
+
+		proplist = drmModeObjectGetProperties(drmmode->fd,
+						      drm_plane->plane_id,
+						      DRM_MODE_OBJECT_PLANE);
+		if (proplist == NULL)
+			goto free_plane;
+
+		for (j = 0; is_primary == -1 && j < proplist->count_props; j++) {
+			drmModePropertyPtr prop;
+
+			prop = drmModeGetProperty(drmmode->fd, proplist->props[j]);
+			if (!prop)
+				continue;
+
+			if (strcmp(prop->name, "type") == 0) {
+				for (k = 0; k < prop->count_enums; k++) {
+					if (prop->enums[k].value != proplist->prop_values[j])
+						continue;
+
+					is_primary = strcmp(prop->enums[k].name, "Primary") == 0;
+					break;
+				}
+			}
+
+			drmModeFreeProperty(prop);
+		}
+
+		if (is_primary) {
+			drmmode_crtc->primary_plane_id = drm_plane->plane_id;
+
+			for (j = 0; drmmode_crtc->rotation_prop_id == 0 && j < proplist->count_props; j++) {
+				drmModePropertyPtr prop;
+
+				prop = drmModeGetProperty(drmmode->fd, proplist->props[j]);
+				if (!prop)
+					continue;
+
+				if (strcmp(prop->name, "rotation") == 0) {
+					drmmode_crtc->rotation_prop_id = proplist->props[j];
+					drmmode_crtc->current_rotation = proplist->prop_values[j];
+					for (k = 0; k < prop->count_enums; k++) {
+						int rr = -1;
+						if (strcmp(prop->enums[k].name, "rotate-0") == 0)
+							rr = RR_Rotate_0;
+						else if (strcmp(prop->enums[k].name, "rotate-90") == 0)
+							rr = RR_Rotate_90;
+						else if (strcmp(prop->enums[k].name, "rotate-180") == 0)
+							rr = RR_Rotate_180;
+						else if (strcmp(prop->enums[k].name, "rotate-270") == 0)
+							rr = RR_Rotate_270;
+						else if (strcmp(prop->enums[k].name, "reflect-x") == 0)
+							rr = RR_Reflect_X;
+						else if (strcmp(prop->enums[k].name, "reflect-y") == 0)
+							rr = RR_Reflect_Y;
+						if (rr != -1) {
+							drmmode_crtc->map_rotations[rotation_index(rr)] = 1 << prop->enums[k].value;
+							drmmode_crtc->supported_rotations |= rr;
+						}
+					}
+				}
+
+				drmModeFreeProperty(prop);
+			}
+		}
+
+		drmModeFreeObjectProperties(proplist);
+free_plane:
+		drmModeFreePlane(drm_plane);
+	}
+}
+
+static Bool
+rotation_set(xf86CrtcPtr crtc, unsigned rotation)
+{
+	drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
+	drmmode_ptr drmmode = drmmode_crtc->drmmode;
+
+	if (drmmode_crtc->current_rotation == rotation)
+		return TRUE;
+
+	if ((drmmode_crtc->supported_rotations & rotation) == 0)
+		return FALSE;
+
+	if (drmModeObjectSetProperty(drmmode->fd,
+				     drmmode_crtc->primary_plane_id,
+				     DRM_MODE_OBJECT_PLANE,
+				     drmmode_crtc->rotation_prop_id,
+				     drmmode_crtc->map_rotations[rotation_index(rotation)]))
+		return FALSE;
+
+	drmmode_crtc->current_rotation = rotation;
+	return TRUE;
+}
+
 static Bool
 drmmode_set_mode_major(xf86CrtcPtr crtc, DisplayModePtr mode,
 		     Rotation rotation, int x, int y)
@@ -307,13 +435,14 @@ drmmode_set_mode_major(xf86CrtcPtr crtc, DisplayModePtr mode,
 	ScrnInfoPtr pScrn = crtc->scrn;
 	xf86CrtcConfigPtr   xf86_config = XF86_CRTC_CONFIG_PTR(crtc->scrn);
 	drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
+	unsigned supported_rotations = drmmode_crtc->supported_rotations;
 	drmmode_ptr drmmode = drmmode_crtc->drmmode;
 	int saved_x, saved_y;
 	Rotation saved_rotation;
 	DisplayModeRec saved_mode;
 	uint32_t *output_ids;
 	int output_count = 0;
-	Bool ret = TRUE;
+	int ret;
 	int i;
 	uint32_t fb_id;
 	drmModeModeInfo kmode;
@@ -324,15 +453,16 @@ drmmode_set_mode_major(xf86CrtcPtr crtc, DisplayModePtr mode,
 	if (drmmode->fb_id == 0) {
 		ret = drmModeAddFB(drmmode->fd,
 				   pScrn->virtualX, height,
-                                   pScrn->depth, pScrn->bitsPerPixel,
+				   pScrn->depth, pScrn->bitsPerPixel,
 				   drmmode->front_bo->pitch,
 				   drmmode->front_bo->handle,
-                                   &drmmode->fb_id);
-                if (ret < 0) {
-                        ErrorF("failed to add fb %d\n", ret);
-                        return FALSE;
-                }
-        }
+				   &drmmode->fb_id);
+		if (ret) {
+			xf86DrvMsg(crtc->scrn->scrnIndex, X_ERROR,
+				   "failed to create framebuffer: %s\n", strerror(-ret));
+			return FALSE;
+		}
+	}
 
 	saved_mode = crtc->mode;
 	saved_x = crtc->x;
@@ -350,10 +480,8 @@ drmmode_set_mode_major(xf86CrtcPtr crtc, DisplayModePtr mode,
 	}
 
 	output_ids = calloc(sizeof(uint32_t), xf86_config->num_output);
-	if (!output_ids) {
-		ret = FALSE;
-		goto done;
-	}
+	if (!output_ids)
+		goto err;
 
 	if (mode) {
 		for (i = 0; i < xf86_config->num_output; i++) {
@@ -368,9 +496,16 @@ drmmode_set_mode_major(xf86CrtcPtr crtc, DisplayModePtr mode,
 			output_count++;
 		}
 
-		if (!xf86CrtcRotate(crtc)) {
-			goto done;
-		}
+		rotation_set(crtc, RR_Rotate_0);
+
+again:
+		crtc->driverIsPerformingTransform = FALSE;
+		if (rotation & supported_rotations && !crtc->transformPresent)
+			crtc->driverIsPerformingTransform = TRUE;
+
+		if (!xf86CrtcRotate(crtc))
+			goto err;
+
 #if XORG_VERSION_CURRENT >= XORG_VERSION_NUMERIC(1,7,0,0,0)
 		crtc->funcs->gamma_set(crtc, crtc->gamma_red, crtc->gamma_green,
 				       crtc->gamma_blue, crtc->gamma_size);
@@ -392,11 +527,20 @@ drmmode_set_mode_major(xf86CrtcPtr crtc, DisplayModePtr mode,
 		}
 		ret = drmModeSetCrtc(drmmode->fd, drmmode_crtc->mode_crtc->crtc_id,
 				     fb_id, x, y, output_ids, output_count, &kmode);
-		if (ret)
+		if (ret) {
 			xf86DrvMsg(crtc->scrn->scrnIndex, X_ERROR,
-				   "failed to set mode: %s", strerror(-ret));
-		else
-			ret = TRUE;
+				   "failed to set mode: %s\n", strerror(-ret));
+			goto err;
+		}
+
+		if (crtc->driverIsPerformingTransform) {
+			if (!rotation_set(crtc, crtc->rotation)) {
+				xf86DrvMsg(crtc->scrn->scrnIndex, X_ERROR,
+					   "failed to set rotation: %s\n", strerror(errno));
+				supported_rotations &= ~crtc->rotation;
+				goto again;
+			}
+		}
 
 		if (crtc->scrn->pScreen)
 			xf86CrtcSetScreenSubpixelOrder(crtc->scrn->pScreen);
@@ -409,26 +553,33 @@ drmmode_set_mode_major(xf86CrtcPtr crtc, DisplayModePtr mode,
 
 			output->funcs->dpms(output, DPMSModeOn);
 		}
+	} else {
+		ret = drmModeSetCrtc(drmmode->fd, drmmode_crtc->mode_crtc->crtc_id,
+				     0, 0, 0, NULL, 0, NULL) == 0;
+		if (ret) {
+			xf86DrvMsg(crtc->scrn->scrnIndex, X_ERROR,
+				   "failed to set mode: %s\n", strerror(-ret));
+			goto err;
+		}
 	}
 
+#if defined(XF86_CRTC_VERSION) && XF86_CRTC_VERSION >= 3
+	crtc->active = mode != NULL;
+#endif
+
 #if 0
 	if (pScrn->pScreen &&
 		!xf86ReturnOptValBool(info->Options, OPTION_SW_CURSOR, FALSE))
 		xf86_reload_cursors(pScrn->pScreen);
 #endif
-done:
-	if (!ret) {
-		crtc->x = saved_x;
-		crtc->y = saved_y;
-		crtc->rotation = saved_rotation;
-		crtc->mode = saved_mode;
-	}
-#if defined(XF86_CRTC_VERSION) && XF86_CRTC_VERSION >= 3
-	else
-		crtc->active = TRUE;
-#endif
+	return TRUE;
 
-	return ret;
+err:
+	crtc->x = saved_x;
+	crtc->y = saved_y;
+	crtc->rotation = saved_rotation;
+	crtc->mode = saved_mode;
+	return FALSE;
 }
 
 static void
@@ -610,7 +761,10 @@ drmmode_crtc_init(ScrnInfoPtr pScrn, drmmode_ptr drmmode, int num)
 	drmmode_crtc = xnfcalloc(sizeof(drmmode_crtc_private_rec), 1);
 	drmmode_crtc->mode_crtc = drmModeGetCrtc(drmmode->fd, drmmode->mode_res->crtcs[num]);
 	drmmode_crtc->drmmode = drmmode;
+	drmmode_crtc->index = num;
 	crtc->driver_private = drmmode_crtc;
+
+	rotation_init(crtc);
 }
 
 static xf86OutputStatus
diff --git a/src/drmmode_display.h b/src/drmmode_display.h
index 745c484..73fabfd 100644
--- a/src/drmmode_display.h
+++ b/src/drmmode_display.h
@@ -75,8 +75,13 @@ typedef struct {
 typedef struct {
     drmmode_ptr drmmode;
     drmModeCrtcPtr mode_crtc;
-    int hw_id;
+    int index;
     struct dumb_bo *cursor_bo;
+    unsigned primary_plane_id;
+    unsigned rotation_prop_id;
+    unsigned supported_rotations;
+    unsigned map_rotations[6];
+    unsigned current_rotation;
     unsigned rotate_fb_id;
     uint16_t lut_r[256], lut_g[256], lut_b[256];
     DamagePtr slave_damage;
@@ -145,5 +150,8 @@ void drmmode_get_default_bpp(ScrnInfoPtr pScrn, drmmode_ptr drmmmode, int *depth
 
 #define MS_ARRAY_SIZE(x) (sizeof(x)/sizeof(x[0]))
 
+#ifndef HAVE_FFS
+extern int ffs(unsigned int value);
+#endif
 
 #endif
-- 
2.0.1

Re: [PATCH] modesetting: Support native primary plane rotation

[Pekka Paalanen]

On Wed,  9 Jul 2014 09:19:08 +0100
Chris Wilson <chris-Y6uKTt2uX1cEflXRtASbqLVCufUGDwFn@public.gmane.org> wrote:

> With the advent of universal drm planes and the introduction of generic
> plane properties for rotations, we can query and program the hardware
> for native rotation support.
> 
> NOTE: this depends upon the next release of libdrm to remove one
> opencoded define.
> 
> v2: Use enum to determine primary plane, suggested by Pekka Paalanen.
>     Use libobj for replacement ffs(), suggested by walter harms
> 
> Signed-off-by: Chris Wilson <chris-Y6uKTt2uX1cEflXRtASbqLVCufUGDwFn@public.gmane.org>
> Cc: Pekka Paalanen <ppaalanen-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Cc: walter harms <wharms-fPG8STNUNVg@public.gmane.org>

My concerns have been addressed. On a second read, I found another
suspicious thing below.

> ---
>  configure.ac          |   5 +-
>  libobj/ffs.c          |  14 ++++
>  src/drmmode_display.c | 216 ++++++++++++++++++++++++++++++++++++++++++--------
>  src/drmmode_display.h |  10 ++-
>  4 files changed, 212 insertions(+), 33 deletions(-)
>  create mode 100644 libobj/ffs.c
> 
> diff --git a/configure.ac b/configure.ac
> index 1c1a36d..1694465 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -74,10 +74,13 @@ AM_CONDITIONAL(HAVE_XEXTPROTO_71, [ test "$HAVE_XEXTPROTO_71" = "yes" ])
>  # Checks for header files.
>  AC_HEADER_STDC
>  
> -PKG_CHECK_MODULES(DRM, [libdrm >= 2.4.46])
> +PKG_CHECK_MODULES(DRM, [libdrm >= 2.4.47])
>  PKG_CHECK_MODULES([PCIACCESS], [pciaccess >= 0.10])
>  AM_CONDITIONAL(DRM, test "x$DRM" = xyes)
>  
> +AC_CONFIG_LIBOBJ_DIR(libobj)
> +AC_REPLACE_FUNCS(ffs)
> +
>  PKG_CHECK_MODULES(UDEV, [libudev], [udev=yes], [udev=no])
>  if test x"$udev" = xyes; then
>          AC_DEFINE(HAVE_UDEV,1,[Enable udev-based monitor hotplug detection])
> diff --git a/libobj/ffs.c b/libobj/ffs.c
> new file mode 100644
> index 0000000..2d44dcc
> --- /dev/null
> +++ b/libobj/ffs.c
> @@ -0,0 +1,14 @@
> +extern int ffs(unsigned int value);
> +
> +int ffs(unsigned int value)
> +{
> +	int bit;
> +
> +	if (value == 0)
> +		return 0;
> +
> +	bit = 0;
> +	while ((value & (1 << bit++)) == 0)
> +		;
> +	return bit;
> +}
> diff --git a/src/drmmode_display.c b/src/drmmode_display.c
> index c533324..e854502 100644
> --- a/src/drmmode_display.c
> +++ b/src/drmmode_display.c
> @@ -56,6 +56,8 @@
>  
>  #include "driver.h"
>  
> +#define DRM_CLIENT_CAP_UNIVERSAL_PLANES 2 /* from libdrm 2.4.55 */
> +
>  static struct dumb_bo *dumb_bo_create(int fd,
>  			  const unsigned width, const unsigned height,
>  			  const unsigned bpp)
> @@ -300,6 +302,132 @@ create_pixmap_for_fbcon(drmmode_ptr drmmode,
>  
>  #endif
>  
> +static unsigned
> +rotation_index(unsigned rotation)
> +{
> +	return ffs(rotation) - 1;
> +}
> +
> +static void
> +rotation_init(xf86CrtcPtr crtc)
> +{
> +	drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
> +	drmmode_ptr drmmode = drmmode_crtc->drmmode;
> +	drmModePlaneRes *plane_resources;
> +	int i, j, k;
> +
> +	drmSetClientCap(drmmode->fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1);
> +
> +	plane_resources = drmModeGetPlaneResources(drmmode->fd);
> +	if (plane_resources == NULL)
> +		return;
> +
> +	for (i = 0; drmmode_crtc->primary_plane_id == 0 && i < plane_resources->count_planes; i++) {
> +		drmModePlane *drm_plane;
> +		drmModeObjectPropertiesPtr proplist;
> +		int is_primary = -1;
> +
> +		drm_plane = drmModeGetPlane(drmmode->fd,
> +					    plane_resources->planes[i]);
> +		if (drm_plane == NULL)
> +			continue;
> +
> +		if (!(drm_plane->possible_crtcs & (1 << drmmode_crtc->index)))
> +			goto free_plane;
> +
> +		proplist = drmModeObjectGetProperties(drmmode->fd,
> +						      drm_plane->plane_id,
> +						      DRM_MODE_OBJECT_PLANE);
> +		if (proplist == NULL)
> +			goto free_plane;
> +
> +		for (j = 0; is_primary == -1 && j < proplist->count_props; j++) {
> +			drmModePropertyPtr prop;
> +
> +			prop = drmModeGetProperty(drmmode->fd, proplist->props[j]);
> +			if (!prop)
> +				continue;
> +
> +			if (strcmp(prop->name, "type") == 0) {
> +				for (k = 0; k < prop->count_enums; k++) {
> +					if (prop->enums[k].value != proplist->prop_values[j])
> +						continue;
> +
> +					is_primary = strcmp(prop->enums[k].name, "Primary") == 0;
> +					break;
> +				}
> +			}
> +
> +			drmModeFreeProperty(prop);
> +		}
> +
> +		if (is_primary) {
> +			drmmode_crtc->primary_plane_id = drm_plane->plane_id;
> +
> +			for (j = 0; drmmode_crtc->rotation_prop_id == 0 && j < proplist->count_props; j++) {
> +				drmModePropertyPtr prop;
> +
> +				prop = drmModeGetProperty(drmmode->fd, proplist->props[j]);
> +				if (!prop)
> +					continue;
> +
> +				if (strcmp(prop->name, "rotation") == 0) {
> +					drmmode_crtc->rotation_prop_id = proplist->props[j];
> +					drmmode_crtc->current_rotation = proplist->prop_values[j];
> +					for (k = 0; k < prop->count_enums; k++) {
> +						int rr = -1;
> +						if (strcmp(prop->enums[k].name, "rotate-0") == 0)
> +							rr = RR_Rotate_0;
> +						else if (strcmp(prop->enums[k].name, "rotate-90") == 0)
> +							rr = RR_Rotate_90;
> +						else if (strcmp(prop->enums[k].name, "rotate-180") == 0)
> +							rr = RR_Rotate_180;
> +						else if (strcmp(prop->enums[k].name, "rotate-270") == 0)
> +							rr = RR_Rotate_270;
> +						else if (strcmp(prop->enums[k].name, "reflect-x") == 0)
> +							rr = RR_Reflect_X;
> +						else if (strcmp(prop->enums[k].name, "reflect-y") == 0)
> +							rr = RR_Reflect_Y;
> +						if (rr != -1) {
> +							drmmode_crtc->map_rotations[rotation_index(rr)] = 1 << prop->enums[k].value;
> +							drmmode_crtc->supported_rotations |= rr;

Comparing the above assignments to...

> +static Bool
> +rotation_set(xf86CrtcPtr crtc, unsigned rotation)
> +{
> +	drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
> +	drmmode_ptr drmmode = drmmode_crtc->drmmode;
> +
> +	if (drmmode_crtc->current_rotation == rotation)
> +		return TRUE;
> +
> +	if ((drmmode_crtc->supported_rotations & rotation) == 0)
> +		return FALSE;
> +
> +	if (drmModeObjectSetProperty(drmmode->fd,
> +				     drmmode_crtc->primary_plane_id,
> +				     DRM_MODE_OBJECT_PLANE,
> +				     drmmode_crtc->rotation_prop_id,
> +				     drmmode_crtc->map_rotations[rotation_index(rotation)]))

...the use here, it does not look like you are passing
prop->enums[k].value here. It is like you are missing ffs() here or
having a 1<< too much in the assignment.

Btw. would it be possible to do e.g. rotate-90 with reflect?


Thanks,
pq
_______________________________________________
xorg-devel-go0+a7rfsptAfugRpC6u6w@public.gmane.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH] modesetting: Support native primary plane rotation

[Chris Wilson]

On Wed, Jul 09, 2014 at 12:57:12PM +0300, Pekka Paalanen wrote:
> On Wed,  9 Jul 2014 09:19:08 +0100
> Chris Wilson <chris-Y6uKTt2uX1cEflXRtASbqLVCufUGDwFn@public.gmane.org> wrote:
> 
> > With the advent of universal drm planes and the introduction of generic
> > plane properties for rotations, we can query and program the hardware
> > for native rotation support.
> > 
> > NOTE: this depends upon the next release of libdrm to remove one
> > opencoded define.
> > 
> > v2: Use enum to determine primary plane, suggested by Pekka Paalanen.
> >     Use libobj for replacement ffs(), suggested by walter harms
> > 
> > Signed-off-by: Chris Wilson <chris-Y6uKTt2uX1cEflXRtASbqLVCufUGDwFn@public.gmane.org>
> > Cc: Pekka Paalanen <ppaalanen-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > Cc: walter harms <wharms-fPG8STNUNVg@public.gmane.org>
> 
> My concerns have been addressed. On a second read, I found another
> suspicious thing below.
> 
> > +				if (strcmp(prop->name, "rotation") == 0) {
> > +					drmmode_crtc->rotation_prop_id = proplist->props[j];
> > +					drmmode_crtc->current_rotation = proplist->prop_values[j];
> > +					for (k = 0; k < prop->count_enums; k++) {
> > +						int rr = -1;
> > +						if (strcmp(prop->enums[k].name, "rotate-0") == 0)
> > +							rr = RR_Rotate_0;
> > +						else if (strcmp(prop->enums[k].name, "rotate-90") == 0)
> > +							rr = RR_Rotate_90;
> > +						else if (strcmp(prop->enums[k].name, "rotate-180") == 0)
> > +							rr = RR_Rotate_180;
> > +						else if (strcmp(prop->enums[k].name, "rotate-270") == 0)
> > +							rr = RR_Rotate_270;
> > +						else if (strcmp(prop->enums[k].name, "reflect-x") == 0)
> > +							rr = RR_Reflect_X;
> > +						else if (strcmp(prop->enums[k].name, "reflect-y") == 0)
> > +							rr = RR_Reflect_Y;
> > +						if (rr != -1) {
> > +							drmmode_crtc->map_rotations[rotation_index(rr)] = 1 << prop->enums[k].value;
> > +							drmmode_crtc->supported_rotations |= rr;
> 
> Comparing the above assignments to...
> 
> > +static Bool
> > +rotation_set(xf86CrtcPtr crtc, unsigned rotation)
> > +{
> > +	drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
> > +	drmmode_ptr drmmode = drmmode_crtc->drmmode;
> > +
> > +	if (drmmode_crtc->current_rotation == rotation)
> > +		return TRUE;
> > +
> > +	if ((drmmode_crtc->supported_rotations & rotation) == 0)
> > +		return FALSE;
> > +
> > +	if (drmModeObjectSetProperty(drmmode->fd,
> > +				     drmmode_crtc->primary_plane_id,
> > +				     DRM_MODE_OBJECT_PLANE,
> > +				     drmmode_crtc->rotation_prop_id,
> > +				     drmmode_crtc->map_rotations[rotation_index(rotation)]))
> 
> ...the use here, it does not look like you are passing
> prop->enums[k].value here. It is like you are missing ffs() here or
> having a 1<< too much in the assignment.

It doesn't take the enum.value but 1 << enum.value.
 
> Btw. would it be possible to do e.g. rotate-90 with reflect?

Indeed. That's an issue from only using the first index...
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
xorg-devel-go0+a7rfsptAfugRpC6u6w@public.gmane.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

[PATCH] modesetting: Support native primary plane rotation

[Chris Wilson]

With the advent of universal drm planes and the introduction of generic
plane properties for rotations, we can query and program the hardware
for native rotation support.

NOTE: this depends upon the next release of libdrm to remove one
opencoded define.

v2: Use enum to determine primary plane, suggested by Pekka Paalanen.
    Use libobj for replacement ffs(), suggested by walter harms
v3: Support combinations of rotations and reflections
    Eliminate use of ffs() and so remove need for libobj

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Pekka Paalanen <ppaalanen@gmail.com>
Cc: walter harms <wharms@bfs.de>
---
 configure.ac          |   2 +-
 src/drmmode_display.c | 258 ++++++++++++++++++++++++++++++++++++++++++++------
 src/drmmode_display.h |  10 +-
 3 files changed, 237 insertions(+), 33 deletions(-)

diff --git a/configure.ac b/configure.ac
index 1c1a36d..783c243 100644
--- a/configure.ac
+++ b/configure.ac
@@ -74,7 +74,7 @@ AM_CONDITIONAL(HAVE_XEXTPROTO_71, [ test "$HAVE_XEXTPROTO_71" = "yes" ])
 # Checks for header files.
 AC_HEADER_STDC
 
-PKG_CHECK_MODULES(DRM, [libdrm >= 2.4.46])
+PKG_CHECK_MODULES(DRM, [libdrm >= 2.4.47])
 PKG_CHECK_MODULES([PCIACCESS], [pciaccess >= 0.10])
 AM_CONDITIONAL(DRM, test "x$DRM" = xyes)
 
diff --git a/src/drmmode_display.c b/src/drmmode_display.c
index c533324..93c48ac 100644
--- a/src/drmmode_display.c
+++ b/src/drmmode_display.c
@@ -56,6 +56,8 @@
 
 #include "driver.h"
 
+#define DRM_CLIENT_CAP_UNIVERSAL_PLANES 2 /* from libdrm 2.4.55 */
+
 static struct dumb_bo *dumb_bo_create(int fd,
 			  const unsigned width, const unsigned height,
 			  const unsigned bpp)
@@ -300,6 +302,171 @@ create_pixmap_for_fbcon(drmmode_ptr drmmode,
 
 #endif
 
+static void
+rotation_init(xf86CrtcPtr crtc)
+{
+	drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
+	drmmode_ptr drmmode = drmmode_crtc->drmmode;
+	drmModePlaneRes *plane_resources;
+	int i, j, k;
+
+	drmSetClientCap(drmmode->fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1);
+
+	plane_resources = drmModeGetPlaneResources(drmmode->fd);
+	if (plane_resources == NULL)
+		return;
+
+	for (i = 0; drmmode_crtc->primary_plane_id == 0 && i < plane_resources->count_planes; i++) {
+		drmModePlane *drm_plane;
+		drmModeObjectPropertiesPtr proplist;
+		int is_primary = -1;
+
+		drm_plane = drmModeGetPlane(drmmode->fd,
+					    plane_resources->planes[i]);
+		if (drm_plane == NULL)
+			continue;
+
+		if (!(drm_plane->possible_crtcs & (1 << drmmode_crtc->index)))
+			goto free_plane;
+
+		proplist = drmModeObjectGetProperties(drmmode->fd,
+						      drm_plane->plane_id,
+						      DRM_MODE_OBJECT_PLANE);
+		if (proplist == NULL)
+			goto free_plane;
+
+		for (j = 0; is_primary == -1 && j < proplist->count_props; j++) {
+			drmModePropertyPtr prop;
+
+			prop = drmModeGetProperty(drmmode->fd, proplist->props[j]);
+			if (!prop)
+				continue;
+
+			if (strcmp(prop->name, "type") == 0) {
+				for (k = 0; k < prop->count_enums; k++) {
+					if (prop->enums[k].value != proplist->prop_values[j])
+						continue;
+
+					is_primary = strcmp(prop->enums[k].name, "Primary") == 0;
+					break;
+				}
+			}
+
+			drmModeFreeProperty(prop);
+		}
+
+		if (is_primary) {
+			drmmode_crtc->primary_plane_id = drm_plane->plane_id;
+
+			for (j = 0; drmmode_crtc->rotation_prop_id == 0 && j < proplist->count_props; j++) {
+				drmModePropertyPtr prop;
+
+				prop = drmModeGetProperty(drmmode->fd, proplist->props[j]);
+				if (!prop)
+					continue;
+
+				if (strcmp(prop->name, "rotation") == 0) {
+					drmmode_crtc->rotation_prop_id = proplist->props[j];
+					drmmode_crtc->current_rotation = proplist->prop_values[j];
+					for (k = 0; k < prop->count_enums; k++) {
+						int rr = -1;
+						if (strcmp(prop->enums[k].name, "rotate-0") == 0)
+							rr = 0; /* RR_Rotate_0 */
+						else if (strcmp(prop->enums[k].name, "rotate-90") == 0)
+							rr = 1; /* RR_Rotate_90 */
+						else if (strcmp(prop->enums[k].name, "rotate-180") == 0)
+							rr = 2; /* RR_Rotate_180 */
+						else if (strcmp(prop->enums[k].name, "rotate-270") == 0)
+							rr = 3; /* RR_Rotate_270 */
+						else if (strcmp(prop->enums[k].name, "reflect-x") == 0)
+							rr = 4; /* RR_Reflect_X */
+						else if (strcmp(prop->enums[k].name, "reflect-y") == 0)
+							rr = 5; /* RR_Reflect_Y */
+						if (rr != -1) {
+							drmmode_crtc->map_rotations[rr] = prop->enums[k].value;
+							drmmode_crtc->supported_rotations |= 1 << rr;
+						}
+					}
+				}
+
+				drmModeFreeProperty(prop);
+			}
+		}
+
+		drmModeFreeObjectProperties(proplist);
+free_plane:
+		drmModeFreePlane(drm_plane);
+	}
+}
+
+static unsigned int
+rotation_to_kernel(xf86CrtcPtr crtc, unsigned rotation)
+{
+	drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
+	unsigned mapped = 0;
+	int i;
+
+	for (i = 0; rotation; i++) {
+		if (rotation & (1 << i)) {
+			mapped |= 1 << drmmode_crtc->map_rotations[i];
+			rotation &= ~(1 << i);
+		}
+	}
+
+	return mapped;
+}
+
+static Bool
+rotation_set(xf86CrtcPtr crtc, unsigned rotation)
+{
+	drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
+	drmmode_ptr drmmode = drmmode_crtc->drmmode;
+
+	if (drmmode_crtc->current_rotation == rotation)
+		return TRUE;
+
+	if ((drmmode_crtc->supported_rotations & rotation) != rotation)
+		return FALSE;
+
+	if (drmModeObjectSetProperty(drmmode->fd,
+				     drmmode_crtc->primary_plane_id,
+				     DRM_MODE_OBJECT_PLANE,
+				     drmmode_crtc->rotation_prop_id,
+				     rotation_to_kernel(crtc, rotation)))
+		return FALSE;
+
+	drmmode_crtc->current_rotation = rotation;
+	return TRUE;
+}
+
+static unsigned
+rotation_reduce(xf86CrtcPtr crtc, unsigned rotation)
+{
+	drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
+	unsigned unsupported_rotations = rotation & ~drmmode_crtc->supported_rotations;
+
+	if (unsupported_rotations == 0)
+		return rotation;
+
+#define RR_Reflect_XY (RR_Reflect_X | RR_Reflect_Y)
+
+	if ((unsupported_rotations & RR_Reflect_XY) == RR_Reflect_XY &&
+	    drmmode_crtc->supported_rotations & RR_Rotate_180) {
+		rotation &= ~RR_Reflect_XY;
+		rotation ^= RR_Rotate_180;
+	}
+
+	if ((unsupported_rotations & RR_Rotate_180) &&
+	    (drmmode_crtc->supported_rotations & RR_Reflect_XY) == RR_Reflect_XY) {
+		rotation ^= RR_Reflect_XY;
+		rotation &= ~RR_Rotate_180;
+	}
+
+#undef RR_Reflect_XY
+
+	return rotation;
+}
+
 static Bool
 drmmode_set_mode_major(xf86CrtcPtr crtc, DisplayModePtr mode,
 		     Rotation rotation, int x, int y)
@@ -307,13 +474,14 @@ drmmode_set_mode_major(xf86CrtcPtr crtc, DisplayModePtr mode,
 	ScrnInfoPtr pScrn = crtc->scrn;
 	xf86CrtcConfigPtr   xf86_config = XF86_CRTC_CONFIG_PTR(crtc->scrn);
 	drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
+	unsigned supported_rotations = drmmode_crtc->supported_rotations;
 	drmmode_ptr drmmode = drmmode_crtc->drmmode;
 	int saved_x, saved_y;
 	Rotation saved_rotation;
 	DisplayModeRec saved_mode;
 	uint32_t *output_ids;
 	int output_count = 0;
-	Bool ret = TRUE;
+	int ret;
 	int i;
 	uint32_t fb_id;
 	drmModeModeInfo kmode;
@@ -324,15 +492,16 @@ drmmode_set_mode_major(xf86CrtcPtr crtc, DisplayModePtr mode,
 	if (drmmode->fb_id == 0) {
 		ret = drmModeAddFB(drmmode->fd,
 				   pScrn->virtualX, height,
-                                   pScrn->depth, pScrn->bitsPerPixel,
+				   pScrn->depth, pScrn->bitsPerPixel,
 				   drmmode->front_bo->pitch,
 				   drmmode->front_bo->handle,
-                                   &drmmode->fb_id);
-                if (ret < 0) {
-                        ErrorF("failed to add fb %d\n", ret);
-                        return FALSE;
-                }
-        }
+				   &drmmode->fb_id);
+		if (ret) {
+			xf86DrvMsg(crtc->scrn->scrnIndex, X_ERROR,
+				   "failed to create framebuffer: %s\n", strerror(-ret));
+			return FALSE;
+		}
+	}
 
 	saved_mode = crtc->mode;
 	saved_x = crtc->x;
@@ -349,11 +518,11 @@ drmmode_set_mode_major(xf86CrtcPtr crtc, DisplayModePtr mode,
 #endif
 	}
 
+	rotation = rotation_reduce(crtc, rotation);
+
 	output_ids = calloc(sizeof(uint32_t), xf86_config->num_output);
-	if (!output_ids) {
-		ret = FALSE;
-		goto done;
-	}
+	if (!output_ids)
+		goto err;
 
 	if (mode) {
 		for (i = 0; i < xf86_config->num_output; i++) {
@@ -368,9 +537,17 @@ drmmode_set_mode_major(xf86CrtcPtr crtc, DisplayModePtr mode,
 			output_count++;
 		}
 
-		if (!xf86CrtcRotate(crtc)) {
-			goto done;
-		}
+		rotation_set(crtc, RR_Rotate_0);
+
+again:
+		crtc->driverIsPerformingTransform = FALSE;
+		if (!crtc->transformPresent &&
+		    (rotation & supported_rotations) == rotation)
+			crtc->driverIsPerformingTransform = TRUE;
+
+		if (!xf86CrtcRotate(crtc))
+			goto err;
+
 #if XORG_VERSION_CURRENT >= XORG_VERSION_NUMERIC(1,7,0,0,0)
 		crtc->funcs->gamma_set(crtc, crtc->gamma_red, crtc->gamma_green,
 				       crtc->gamma_blue, crtc->gamma_size);
@@ -392,11 +569,20 @@ drmmode_set_mode_major(xf86CrtcPtr crtc, DisplayModePtr mode,
 		}
 		ret = drmModeSetCrtc(drmmode->fd, drmmode_crtc->mode_crtc->crtc_id,
 				     fb_id, x, y, output_ids, output_count, &kmode);
-		if (ret)
+		if (ret) {
 			xf86DrvMsg(crtc->scrn->scrnIndex, X_ERROR,
-				   "failed to set mode: %s", strerror(-ret));
-		else
-			ret = TRUE;
+				   "failed to set mode: %s\n", strerror(-ret));
+			goto err;
+		}
+
+		if (crtc->driverIsPerformingTransform) {
+			if (!rotation_set(crtc, rotation)) {
+				xf86DrvMsg(crtc->scrn->scrnIndex, X_ERROR,
+					   "failed to set rotation: %s\n", strerror(errno));
+				supported_rotations &= ~rotation;
+				goto again;
+			}
+		}
 
 		if (crtc->scrn->pScreen)
 			xf86CrtcSetScreenSubpixelOrder(crtc->scrn->pScreen);
@@ -409,26 +595,33 @@ drmmode_set_mode_major(xf86CrtcPtr crtc, DisplayModePtr mode,
 
 			output->funcs->dpms(output, DPMSModeOn);
 		}
+	} else {
+		ret = drmModeSetCrtc(drmmode->fd, drmmode_crtc->mode_crtc->crtc_id,
+				     0, 0, 0, NULL, 0, NULL) == 0;
+		if (ret) {
+			xf86DrvMsg(crtc->scrn->scrnIndex, X_ERROR,
+				   "failed to set mode: %s\n", strerror(-ret));
+			goto err;
+		}
 	}
 
+#if defined(XF86_CRTC_VERSION) && XF86_CRTC_VERSION >= 3
+	crtc->active = mode != NULL;
+#endif
+
 #if 0
 	if (pScrn->pScreen &&
 		!xf86ReturnOptValBool(info->Options, OPTION_SW_CURSOR, FALSE))
 		xf86_reload_cursors(pScrn->pScreen);
 #endif
-done:
-	if (!ret) {
-		crtc->x = saved_x;
-		crtc->y = saved_y;
-		crtc->rotation = saved_rotation;
-		crtc->mode = saved_mode;
-	}
-#if defined(XF86_CRTC_VERSION) && XF86_CRTC_VERSION >= 3
-	else
-		crtc->active = TRUE;
-#endif
+	return TRUE;
 
-	return ret;
+err:
+	crtc->x = saved_x;
+	crtc->y = saved_y;
+	crtc->rotation = saved_rotation;
+	crtc->mode = saved_mode;
+	return FALSE;
 }
 
 static void
@@ -610,7 +803,10 @@ drmmode_crtc_init(ScrnInfoPtr pScrn, drmmode_ptr drmmode, int num)
 	drmmode_crtc = xnfcalloc(sizeof(drmmode_crtc_private_rec), 1);
 	drmmode_crtc->mode_crtc = drmModeGetCrtc(drmmode->fd, drmmode->mode_res->crtcs[num]);
 	drmmode_crtc->drmmode = drmmode;
+	drmmode_crtc->index = num;
 	crtc->driver_private = drmmode_crtc;
+
+	rotation_init(crtc);
 }
 
 static xf86OutputStatus
diff --git a/src/drmmode_display.h b/src/drmmode_display.h
index 745c484..73fabfd 100644
--- a/src/drmmode_display.h
+++ b/src/drmmode_display.h
@@ -75,8 +75,13 @@ typedef struct {
 typedef struct {
     drmmode_ptr drmmode;
     drmModeCrtcPtr mode_crtc;
-    int hw_id;
+    int index;
     struct dumb_bo *cursor_bo;
+    unsigned primary_plane_id;
+    unsigned rotation_prop_id;
+    unsigned supported_rotations;
+    unsigned map_rotations[6];
+    unsigned current_rotation;
     unsigned rotate_fb_id;
     uint16_t lut_r[256], lut_g[256], lut_b[256];
     DamagePtr slave_damage;
@@ -145,5 +150,8 @@ void drmmode_get_default_bpp(ScrnInfoPtr pScrn, drmmode_ptr drmmmode, int *depth
 
 #define MS_ARRAY_SIZE(x) (sizeof(x)/sizeof(x[0]))
 
+#ifndef HAVE_FFS
+extern int ffs(unsigned int value);
+#endif
 
 #endif
-- 
2.0.1

Re: [PATCH] modesetting: Support native primary plane rotation

[Pekka Paalanen]

On Wed, 9 Jul 2014 11:02:59 +0100
Chris Wilson <chris-Y6uKTt2uX1cEflXRtASbqLVCufUGDwFn@public.gmane.org> wrote:

> On Wed, Jul 09, 2014 at 12:57:12PM +0300, Pekka Paalanen wrote:
> > On Wed,  9 Jul 2014 09:19:08 +0100
> > Chris Wilson <chris-Y6uKTt2uX1cEflXRtASbqLVCufUGDwFn@public.gmane.org> wrote:
> > 
> > > With the advent of universal drm planes and the introduction of generic
> > > plane properties for rotations, we can query and program the hardware
> > > for native rotation support.
> > > 
> > > NOTE: this depends upon the next release of libdrm to remove one
> > > opencoded define.
> > > 
> > > v2: Use enum to determine primary plane, suggested by Pekka Paalanen.
> > >     Use libobj for replacement ffs(), suggested by walter harms
> > > 
> > > Signed-off-by: Chris Wilson <chris-Y6uKTt2uX1cEflXRtASbqLVCufUGDwFn@public.gmane.org>
> > > Cc: Pekka Paalanen <ppaalanen-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > > Cc: walter harms <wharms-fPG8STNUNVg@public.gmane.org>
> > 
> > My concerns have been addressed. On a second read, I found another
> > suspicious thing below.
> > 
> > > +				if (strcmp(prop->name, "rotation") == 0) {
> > > +					drmmode_crtc->rotation_prop_id = proplist->props[j];
> > > +					drmmode_crtc->current_rotation = proplist->prop_values[j];
> > > +					for (k = 0; k < prop->count_enums; k++) {
> > > +						int rr = -1;
> > > +						if (strcmp(prop->enums[k].name, "rotate-0") == 0)
> > > +							rr = RR_Rotate_0;
> > > +						else if (strcmp(prop->enums[k].name, "rotate-90") == 0)
> > > +							rr = RR_Rotate_90;
> > > +						else if (strcmp(prop->enums[k].name, "rotate-180") == 0)
> > > +							rr = RR_Rotate_180;
> > > +						else if (strcmp(prop->enums[k].name, "rotate-270") == 0)
> > > +							rr = RR_Rotate_270;
> > > +						else if (strcmp(prop->enums[k].name, "reflect-x") == 0)
> > > +							rr = RR_Reflect_X;
> > > +						else if (strcmp(prop->enums[k].name, "reflect-y") == 0)
> > > +							rr = RR_Reflect_Y;
> > > +						if (rr != -1) {
> > > +							drmmode_crtc->map_rotations[rotation_index(rr)] = 1 << prop->enums[k].value;
> > > +							drmmode_crtc->supported_rotations |= rr;
> > 
> > Comparing the above assignments to...
> > 
> > > +static Bool
> > > +rotation_set(xf86CrtcPtr crtc, unsigned rotation)
> > > +{
> > > +	drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
> > > +	drmmode_ptr drmmode = drmmode_crtc->drmmode;
> > > +
> > > +	if (drmmode_crtc->current_rotation == rotation)
> > > +		return TRUE;
> > > +
> > > +	if ((drmmode_crtc->supported_rotations & rotation) == 0)
> > > +		return FALSE;
> > > +
> > > +	if (drmModeObjectSetProperty(drmmode->fd,
> > > +				     drmmode_crtc->primary_plane_id,
> > > +				     DRM_MODE_OBJECT_PLANE,
> > > +				     drmmode_crtc->rotation_prop_id,
> > > +				     drmmode_crtc->map_rotations[rotation_index(rotation)]))
> > 
> > ...the use here, it does not look like you are passing
> > prop->enums[k].value here. It is like you are missing ffs() here or
> > having a 1<< too much in the assignment.
> 
> It doesn't take the enum.value but 1 << enum.value.

Aah, it is not an 'enum', it is a 'bitmask'! Okay, I see it now, I
think.


Thanks,
pq
_______________________________________________
xorg-devel-go0+a7rfsptAfugRpC6u6w@public.gmane.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH] modesetting: Support native primary plane rotation

[Mark Kettenis]

> From: Chris Wilson <chris@chris-wilson.co.uk>
> Date: Wed,  9 Jul 2014 12:14:41 +0100
> 
> With the advent of universal drm planes and the introduction of generic
> plane properties for rotations, we can query and program the hardware
> for native rotation support.
> 
> NOTE: this depends upon the next release of libdrm to remove one
> opencoded define.
> 
> v2: Use enum to determine primary plane, suggested by Pekka Paalanen.
>     Use libobj for replacement ffs(), suggested by walter harms
> v3: Support combinations of rotations and reflections
>     Eliminate use of ffs() and so remove need for libobj

Surely that means...
>  
> +#ifndef HAVE_FFS
> +extern int ffs(unsigned int value);
> +#endif

This bit should be removed ad well?

[PATCH] modesetting: Support native primary plane rotation

[Chris Wilson]

With the advent of universal drm planes and the introduction of generic
plane properties for rotations, we can query and program the hardware
for native rotation support.

NOTE: this depends upon the next release of libdrm to remove one
opencoded define.

v2: Use enum to determine primary plane, suggested by Pekka Paalanen.
    Use libobj for replacement ffs(), suggested by walter harms
v3: Support combinations of rotations and reflections
    Eliminate use of ffs() and so remove need for libobj
v4: And remove the vestigal HAVE_FFS, spotted by Mark Kettenis

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Pekka Paalanen <ppaalanen@gmail.com>
Cc: walter harms <wharms@bfs.de>
Cc: Mark Kettenis <mark.kettenis@xs4all.nl>
---
 configure.ac          |   2 +-
 src/drmmode_display.c | 258 ++++++++++++++++++++++++++++++++++++++++++++------
 src/drmmode_display.h |   7 +-
 3 files changed, 234 insertions(+), 33 deletions(-)

diff --git a/configure.ac b/configure.ac
index 1c1a36d..783c243 100644
--- a/configure.ac
+++ b/configure.ac
@@ -74,7 +74,7 @@ AM_CONDITIONAL(HAVE_XEXTPROTO_71, [ test "$HAVE_XEXTPROTO_71" = "yes" ])
 # Checks for header files.
 AC_HEADER_STDC
 
-PKG_CHECK_MODULES(DRM, [libdrm >= 2.4.46])
+PKG_CHECK_MODULES(DRM, [libdrm >= 2.4.47])
 PKG_CHECK_MODULES([PCIACCESS], [pciaccess >= 0.10])
 AM_CONDITIONAL(DRM, test "x$DRM" = xyes)
 
diff --git a/src/drmmode_display.c b/src/drmmode_display.c
index c533324..93c48ac 100644
--- a/src/drmmode_display.c
+++ b/src/drmmode_display.c
@@ -56,6 +56,8 @@
 
 #include "driver.h"
 
+#define DRM_CLIENT_CAP_UNIVERSAL_PLANES 2 /* from libdrm 2.4.55 */
+
 static struct dumb_bo *dumb_bo_create(int fd,
 			  const unsigned width, const unsigned height,
 			  const unsigned bpp)
@@ -300,6 +302,171 @@ create_pixmap_for_fbcon(drmmode_ptr drmmode,
 
 #endif
 
+static void
+rotation_init(xf86CrtcPtr crtc)
+{
+	drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
+	drmmode_ptr drmmode = drmmode_crtc->drmmode;
+	drmModePlaneRes *plane_resources;
+	int i, j, k;
+
+	drmSetClientCap(drmmode->fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1);
+
+	plane_resources = drmModeGetPlaneResources(drmmode->fd);
+	if (plane_resources == NULL)
+		return;
+
+	for (i = 0; drmmode_crtc->primary_plane_id == 0 && i < plane_resources->count_planes; i++) {
+		drmModePlane *drm_plane;
+		drmModeObjectPropertiesPtr proplist;
+		int is_primary = -1;
+
+		drm_plane = drmModeGetPlane(drmmode->fd,
+					    plane_resources->planes[i]);
+		if (drm_plane == NULL)
+			continue;
+
+		if (!(drm_plane->possible_crtcs & (1 << drmmode_crtc->index)))
+			goto free_plane;
+
+		proplist = drmModeObjectGetProperties(drmmode->fd,
+						      drm_plane->plane_id,
+						      DRM_MODE_OBJECT_PLANE);
+		if (proplist == NULL)
+			goto free_plane;
+
+		for (j = 0; is_primary == -1 && j < proplist->count_props; j++) {
+			drmModePropertyPtr prop;
+
+			prop = drmModeGetProperty(drmmode->fd, proplist->props[j]);
+			if (!prop)
+				continue;
+
+			if (strcmp(prop->name, "type") == 0) {
+				for (k = 0; k < prop->count_enums; k++) {
+					if (prop->enums[k].value != proplist->prop_values[j])
+						continue;
+
+					is_primary = strcmp(prop->enums[k].name, "Primary") == 0;
+					break;
+				}
+			}
+
+			drmModeFreeProperty(prop);
+		}
+
+		if (is_primary) {
+			drmmode_crtc->primary_plane_id = drm_plane->plane_id;
+
+			for (j = 0; drmmode_crtc->rotation_prop_id == 0 && j < proplist->count_props; j++) {
+				drmModePropertyPtr prop;
+
+				prop = drmModeGetProperty(drmmode->fd, proplist->props[j]);
+				if (!prop)
+					continue;
+
+				if (strcmp(prop->name, "rotation") == 0) {
+					drmmode_crtc->rotation_prop_id = proplist->props[j];
+					drmmode_crtc->current_rotation = proplist->prop_values[j];
+					for (k = 0; k < prop->count_enums; k++) {
+						int rr = -1;
+						if (strcmp(prop->enums[k].name, "rotate-0") == 0)
+							rr = 0; /* RR_Rotate_0 */
+						else if (strcmp(prop->enums[k].name, "rotate-90") == 0)
+							rr = 1; /* RR_Rotate_90 */
+						else if (strcmp(prop->enums[k].name, "rotate-180") == 0)
+							rr = 2; /* RR_Rotate_180 */
+						else if (strcmp(prop->enums[k].name, "rotate-270") == 0)
+							rr = 3; /* RR_Rotate_270 */
+						else if (strcmp(prop->enums[k].name, "reflect-x") == 0)
+							rr = 4; /* RR_Reflect_X */
+						else if (strcmp(prop->enums[k].name, "reflect-y") == 0)
+							rr = 5; /* RR_Reflect_Y */
+						if (rr != -1) {
+							drmmode_crtc->map_rotations[rr] = prop->enums[k].value;
+							drmmode_crtc->supported_rotations |= 1 << rr;
+						}
+					}
+				}
+
+				drmModeFreeProperty(prop);
+			}
+		}
+
+		drmModeFreeObjectProperties(proplist);
+free_plane:
+		drmModeFreePlane(drm_plane);
+	}
+}
+
+static unsigned int
+rotation_to_kernel(xf86CrtcPtr crtc, unsigned rotation)
+{
+	drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
+	unsigned mapped = 0;
+	int i;
+
+	for (i = 0; rotation; i++) {
+		if (rotation & (1 << i)) {
+			mapped |= 1 << drmmode_crtc->map_rotations[i];
+			rotation &= ~(1 << i);
+		}
+	}
+
+	return mapped;
+}
+
+static Bool
+rotation_set(xf86CrtcPtr crtc, unsigned rotation)
+{
+	drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
+	drmmode_ptr drmmode = drmmode_crtc->drmmode;
+
+	if (drmmode_crtc->current_rotation == rotation)
+		return TRUE;
+
+	if ((drmmode_crtc->supported_rotations & rotation) != rotation)
+		return FALSE;
+
+	if (drmModeObjectSetProperty(drmmode->fd,
+				     drmmode_crtc->primary_plane_id,
+				     DRM_MODE_OBJECT_PLANE,
+				     drmmode_crtc->rotation_prop_id,
+				     rotation_to_kernel(crtc, rotation)))
+		return FALSE;
+
+	drmmode_crtc->current_rotation = rotation;
+	return TRUE;
+}
+
+static unsigned
+rotation_reduce(xf86CrtcPtr crtc, unsigned rotation)
+{
+	drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
+	unsigned unsupported_rotations = rotation & ~drmmode_crtc->supported_rotations;
+
+	if (unsupported_rotations == 0)
+		return rotation;
+
+#define RR_Reflect_XY (RR_Reflect_X | RR_Reflect_Y)
+
+	if ((unsupported_rotations & RR_Reflect_XY) == RR_Reflect_XY &&
+	    drmmode_crtc->supported_rotations & RR_Rotate_180) {
+		rotation &= ~RR_Reflect_XY;
+		rotation ^= RR_Rotate_180;
+	}
+
+	if ((unsupported_rotations & RR_Rotate_180) &&
+	    (drmmode_crtc->supported_rotations & RR_Reflect_XY) == RR_Reflect_XY) {
+		rotation ^= RR_Reflect_XY;
+		rotation &= ~RR_Rotate_180;
+	}
+
+#undef RR_Reflect_XY
+
+	return rotation;
+}
+
 static Bool
 drmmode_set_mode_major(xf86CrtcPtr crtc, DisplayModePtr mode,
 		     Rotation rotation, int x, int y)
@@ -307,13 +474,14 @@ drmmode_set_mode_major(xf86CrtcPtr crtc, DisplayModePtr mode,
 	ScrnInfoPtr pScrn = crtc->scrn;
 	xf86CrtcConfigPtr   xf86_config = XF86_CRTC_CONFIG_PTR(crtc->scrn);
 	drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
+	unsigned supported_rotations = drmmode_crtc->supported_rotations;
 	drmmode_ptr drmmode = drmmode_crtc->drmmode;
 	int saved_x, saved_y;
 	Rotation saved_rotation;
 	DisplayModeRec saved_mode;
 	uint32_t *output_ids;
 	int output_count = 0;
-	Bool ret = TRUE;
+	int ret;
 	int i;
 	uint32_t fb_id;
 	drmModeModeInfo kmode;
@@ -324,15 +492,16 @@ drmmode_set_mode_major(xf86CrtcPtr crtc, DisplayModePtr mode,
 	if (drmmode->fb_id == 0) {
 		ret = drmModeAddFB(drmmode->fd,
 				   pScrn->virtualX, height,
-                                   pScrn->depth, pScrn->bitsPerPixel,
+				   pScrn->depth, pScrn->bitsPerPixel,
 				   drmmode->front_bo->pitch,
 				   drmmode->front_bo->handle,
-                                   &drmmode->fb_id);
-                if (ret < 0) {
-                        ErrorF("failed to add fb %d\n", ret);
-                        return FALSE;
-                }
-        }
+				   &drmmode->fb_id);
+		if (ret) {
+			xf86DrvMsg(crtc->scrn->scrnIndex, X_ERROR,
+				   "failed to create framebuffer: %s\n", strerror(-ret));
+			return FALSE;
+		}
+	}
 
 	saved_mode = crtc->mode;
 	saved_x = crtc->x;
@@ -349,11 +518,11 @@ drmmode_set_mode_major(xf86CrtcPtr crtc, DisplayModePtr mode,
 #endif
 	}
 
+	rotation = rotation_reduce(crtc, rotation);
+
 	output_ids = calloc(sizeof(uint32_t), xf86_config->num_output);
-	if (!output_ids) {
-		ret = FALSE;
-		goto done;
-	}
+	if (!output_ids)
+		goto err;
 
 	if (mode) {
 		for (i = 0; i < xf86_config->num_output; i++) {
@@ -368,9 +537,17 @@ drmmode_set_mode_major(xf86CrtcPtr crtc, DisplayModePtr mode,
 			output_count++;
 		}
 
-		if (!xf86CrtcRotate(crtc)) {
-			goto done;
-		}
+		rotation_set(crtc, RR_Rotate_0);
+
+again:
+		crtc->driverIsPerformingTransform = FALSE;
+		if (!crtc->transformPresent &&
+		    (rotation & supported_rotations) == rotation)
+			crtc->driverIsPerformingTransform = TRUE;
+
+		if (!xf86CrtcRotate(crtc))
+			goto err;
+
 #if XORG_VERSION_CURRENT >= XORG_VERSION_NUMERIC(1,7,0,0,0)
 		crtc->funcs->gamma_set(crtc, crtc->gamma_red, crtc->gamma_green,
 				       crtc->gamma_blue, crtc->gamma_size);
@@ -392,11 +569,20 @@ drmmode_set_mode_major(xf86CrtcPtr crtc, DisplayModePtr mode,
 		}
 		ret = drmModeSetCrtc(drmmode->fd, drmmode_crtc->mode_crtc->crtc_id,
 				     fb_id, x, y, output_ids, output_count, &kmode);
-		if (ret)
+		if (ret) {
 			xf86DrvMsg(crtc->scrn->scrnIndex, X_ERROR,
-				   "failed to set mode: %s", strerror(-ret));
-		else
-			ret = TRUE;
+				   "failed to set mode: %s\n", strerror(-ret));
+			goto err;
+		}
+
+		if (crtc->driverIsPerformingTransform) {
+			if (!rotation_set(crtc, rotation)) {
+				xf86DrvMsg(crtc->scrn->scrnIndex, X_ERROR,
+					   "failed to set rotation: %s\n", strerror(errno));
+				supported_rotations &= ~rotation;
+				goto again;
+			}
+		}
 
 		if (crtc->scrn->pScreen)
 			xf86CrtcSetScreenSubpixelOrder(crtc->scrn->pScreen);
@@ -409,26 +595,33 @@ drmmode_set_mode_major(xf86CrtcPtr crtc, DisplayModePtr mode,
 
 			output->funcs->dpms(output, DPMSModeOn);
 		}
+	} else {
+		ret = drmModeSetCrtc(drmmode->fd, drmmode_crtc->mode_crtc->crtc_id,
+				     0, 0, 0, NULL, 0, NULL) == 0;
+		if (ret) {
+			xf86DrvMsg(crtc->scrn->scrnIndex, X_ERROR,
+				   "failed to set mode: %s\n", strerror(-ret));
+			goto err;
+		}
 	}
 
+#if defined(XF86_CRTC_VERSION) && XF86_CRTC_VERSION >= 3
+	crtc->active = mode != NULL;
+#endif
+
 #if 0
 	if (pScrn->pScreen &&
 		!xf86ReturnOptValBool(info->Options, OPTION_SW_CURSOR, FALSE))
 		xf86_reload_cursors(pScrn->pScreen);
 #endif
-done:
-	if (!ret) {
-		crtc->x = saved_x;
-		crtc->y = saved_y;
-		crtc->rotation = saved_rotation;
-		crtc->mode = saved_mode;
-	}
-#if defined(XF86_CRTC_VERSION) && XF86_CRTC_VERSION >= 3
-	else
-		crtc->active = TRUE;
-#endif
+	return TRUE;
 
-	return ret;
+err:
+	crtc->x = saved_x;
+	crtc->y = saved_y;
+	crtc->rotation = saved_rotation;
+	crtc->mode = saved_mode;
+	return FALSE;
 }
 
 static void
@@ -610,7 +803,10 @@ drmmode_crtc_init(ScrnInfoPtr pScrn, drmmode_ptr drmmode, int num)
 	drmmode_crtc = xnfcalloc(sizeof(drmmode_crtc_private_rec), 1);
 	drmmode_crtc->mode_crtc = drmModeGetCrtc(drmmode->fd, drmmode->mode_res->crtcs[num]);
 	drmmode_crtc->drmmode = drmmode;
+	drmmode_crtc->index = num;
 	crtc->driver_private = drmmode_crtc;
+
+	rotation_init(crtc);
 }
 
 static xf86OutputStatus
diff --git a/src/drmmode_display.h b/src/drmmode_display.h
index 745c484..8eb25fd 100644
--- a/src/drmmode_display.h
+++ b/src/drmmode_display.h
@@ -75,8 +75,13 @@ typedef struct {
 typedef struct {
     drmmode_ptr drmmode;
     drmModeCrtcPtr mode_crtc;
-    int hw_id;
+    int index;
     struct dumb_bo *cursor_bo;
+    unsigned primary_plane_id;
+    unsigned rotation_prop_id;
+    unsigned supported_rotations;
+    unsigned map_rotations[6];
+    unsigned current_rotation;
     unsigned rotate_fb_id;
     uint16_t lut_r[256], lut_g[256], lut_b[256];
     DamagePtr slave_damage;
-- 
2.0.1