[PATCH] modesetting: Support native primary plane rotation

walter harms wharms at bfs.de
Wed Jul 9 00:28:31 PDT 2014



Am 09.07.2014 09:00, schrieb 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 at chris-wilson.co.uk>
> ---
>  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
> +}


perhaps it is better to provide an internal ffs for systems that lack one ?
than you can use ffs() directly and leave the rest to configure.
That would result in a generic HAVE_FFS instead of a list of defines like this now.

just my 2 cents,
re
 wh

> +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;


More information about the xorg-devel mailing list