On Wed, 9 Jul 2014 09:19:08 +0100 Chris Wilson chris@chris-wilson.co.uk 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@chris-wilson.co.uk Cc: Pekka Paalanen ppaalanen@gmail.com Cc: walter harms wharms@bfs.de
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