On Tue, Jun 30, 2015 at 12:54 AM, Dave Airlie <airl...@gmail.com> wrote:
> One of the lacking features with output offloading was
> that screen rotation didn't work at all.
>
> This patch makes 0/90/180/270 rotation work with USB output
> and GPU outputs.
>
> When it allocates the shared pixmap it allocates it rotated,
> and any updates to the shared pixmap are done using a composite
> path that does the rotation. The slave GPU then doesn't need
> to know about the rotation and just displays the pixmap.
>
> v2:
> rewrite the sync dirty helper to use the dst pixmap, and
> avoid any strange hobbits and rotations.
>
> This breaks ABI in two places.
>
> Signed-off-by: Dave Airlie <airl...@redhat.com>
Reviewed-by: Alex Deucher <alexander.deuc...@amd.com>
> ---
> dix/pixmap.c | 180
> +++++++++++++++++------
> hw/xfree86/drivers/modesetting/driver.c | 2 +-
> hw/xfree86/drivers/modesetting/drmmode_display.c | 2 +-
> hw/xfree86/modes/xf86Rotate.c | 2 +
> include/pixmap.h | 14 +-
> include/pixmapstr.h | 5 +
> include/scrnintstr.h | 5 +-
> randr/rrcrtc.c | 60 +++++---
> 8 files changed, 197 insertions(+), 73 deletions(-)
>
> diff --git a/dix/pixmap.c b/dix/pixmap.c
> index 00e298f..05aebc4 100644
> --- a/dix/pixmap.c
> +++ b/dix/pixmap.c
> @@ -40,7 +40,9 @@ from The Open Group.
> #include "gcstruct.h"
> #include "servermd.h"
> #include "site.h"
> -
> +#include "X11/extensions/render.h"
> +#include "picturestr.h"
> +#include "randrstr.h"
> /*
> * Scratch pixmap management and device independent pixmap allocation
> * function.
> @@ -164,9 +166,10 @@ PixmapPtr PixmapShareToSlave(PixmapPtr pixmap, ScreenPtr
> slave)
> }
>
> Bool
> -PixmapStartDirtyTracking2(PixmapPtr src,
> - PixmapPtr slave_dst,
> - int x, int y, int dst_x, int dst_y)
> +PixmapStartDirtyTracking(PixmapPtr src,
> + PixmapPtr slave_dst,
> + int x, int y, int dst_x, int dst_y,
> + Rotation rotation)
> {
> ScreenPtr screen = src->drawable.pScreen;
> PixmapDirtyUpdatePtr dirty_update;
> @@ -181,11 +184,22 @@ PixmapStartDirtyTracking2(PixmapPtr src,
> dirty_update->y = y;
> dirty_update->dst_x = dst_x;
> dirty_update->dst_y = dst_y;
> -
> + dirty_update->rotation = rotation;
> dirty_update->damage = DamageCreate(NULL, NULL,
> DamageReportNone,
> TRUE, src->drawable.pScreen,
> src->drawable.pScreen);
> +
> + if (rotation != RR_Rotate_0) {
> + RRTransformCompute(x, y,
> + slave_dst->drawable.width,
> + slave_dst->drawable.height,
> + rotation,
> + NULL,
> + &dirty_update->transform,
> + &dirty_update->f_transform,
> + &dirty_update->f_inverse);
> + }
> if (!dirty_update->damage) {
> free(dirty_update);
> return FALSE;
> @@ -197,14 +211,6 @@ PixmapStartDirtyTracking2(PixmapPtr src,
> }
>
> Bool
> -PixmapStartDirtyTracking(PixmapPtr src,
> - PixmapPtr slave_dst,
> - int x, int y)
> -{
> - return PixmapStartDirtyTracking2(src, slave_dst, x, y, 0, 0);
> -}
> -
> -Bool
> PixmapStopDirtyTracking(PixmapPtr src, PixmapPtr slave_dst)
> {
> ScreenPtr screen = src->drawable.pScreen;
> @@ -220,42 +226,16 @@ PixmapStopDirtyTracking(PixmapPtr src, PixmapPtr
> slave_dst)
> return TRUE;
> }
>
> -/*
> - * this function can possibly be improved and optimised, by clipping
> - * instead of iterating
> - */
> -Bool PixmapSyncDirtyHelper(PixmapDirtyUpdatePtr dirty, RegionPtr
> dirty_region)
> +static void
> +PixmapDirtyCopyArea(PixmapPtr dst,
> + PixmapDirtyUpdatePtr dirty,
> + RegionPtr dirty_region)
> {
> ScreenPtr pScreen = dirty->src->drawable.pScreen;
> int n;
> BoxPtr b;
> - RegionPtr region = DamageRegion(dirty->damage);
> GCPtr pGC;
> - PixmapPtr dst;
> - SourceValidateProcPtr SourceValidate;
> -
> - /*
> - * SourceValidate is used by the software cursor code
> - * to pull the cursor off of the screen when reading
> - * bits from the frame buffer. Bypassing this function
> - * leaves the software cursor in place
> - */
> - SourceValidate = pScreen->SourceValidate;
> - pScreen->SourceValidate = NULL;
> -
> - RegionTranslate(dirty_region, dirty->x, dirty->y);
> - RegionIntersect(dirty_region, dirty_region, region);
> -
> - if (RegionNil(dirty_region)) {
> - RegionUninit(dirty_region);
> - return FALSE;
> - }
>
> - dst = dirty->slave_dst->master_pixmap;
> - if (!dst)
> - dst = dirty->slave_dst;
> -
> - RegionTranslate(dirty_region, -dirty->x, -dirty->y);
> n = RegionNumRects(dirty_region);
> b = RegionRects(dirty_region);
>
> @@ -271,11 +251,123 @@ Bool PixmapSyncDirtyHelper(PixmapDirtyUpdatePtr dirty,
> RegionPtr dirty_region)
> h = dst_box.y2 - dst_box.y1;
>
> pGC->ops->CopyArea(&dirty->src->drawable, &dst->drawable, pGC,
> - dirty->x + dst_box.x1, dirty->y + dst_box.y1, w,
> h, dirty->dst_x + dst_box.x1, dirty->dst_y + dst_box.y1);
> + dirty->x + dst_box.x1, dirty->y + dst_box.y1, w,
> h,
> + dirty->dst_x + dst_box.x1,
> + dirty->dst_y + dst_box.y1);
> b++;
> }
> FreeScratchGC(pGC);
> +}
>
> +static void
> +PixmapDirtyCompositeRotate(PixmapPtr dst_pixmap,
> + PixmapDirtyUpdatePtr dirty,
> + RegionPtr dirty_region)
> +{
> + ScreenPtr pScreen = dirty->src->drawable.pScreen;
> + PictFormatPtr format = PictureWindowFormat(pScreen->root);
> + PicturePtr src, dst;
> + XID include_inferiors = IncludeInferiors;
> + int n = RegionNumRects(dirty_region);
> + BoxPtr b = RegionRects(dirty_region);
> + int error;
> +
> + src = CreatePicture(None,
> + &dirty->src->drawable,
> + format,
> + CPSubwindowMode,
> + &include_inferiors, serverClient, &error);
> + if (!src)
> + return;
> +
> + dst = CreatePicture(None,
> + &dst_pixmap->drawable,
> + format, 0L, NULL, serverClient, &error);
> + if (!dst)
> + return;
> +
> + error = SetPictureTransform(src, &dirty->transform);
> + if (error)
> + return;
> + while (n--) {
> + BoxRec dst_box;
> +
> + dst_box = *b;
> + dst_box.x1 += dirty->x;
> + dst_box.x2 += dirty->x;
> + dst_box.y1 += dirty->y;
> + dst_box.y2 += dirty->y;
> + pixman_f_transform_bounds(&dirty->f_inverse, &dst_box);
> +
> + CompositePicture(PictOpSrc,
> + src, NULL, dst,
> + dst_box.x1,
> + dst_box.y1,
> + 0, 0,
> + dst_box.x1,
> + dst_box.y1,
> + dst_box.x2 - dst_box.x1,
> + dst_box.y2 - dst_box.y1);
> + b++;
> + }
> +
> + FreePicture(src, None);
> + FreePicture(dst, None);
> +}
> +
> +/*
> + * this function can possibly be improved and optimised, by clipping
> + * instead of iterating
> + * Drivers are free to implement their own version of this.
> + */
> +Bool PixmapSyncDirtyHelper(PixmapDirtyUpdatePtr dirty)
> +{
> + ScreenPtr pScreen = dirty->src->drawable.pScreen;
> + RegionPtr region = DamageRegion(dirty->damage);
> + PixmapPtr dst;
> + SourceValidateProcPtr SourceValidate;
> + RegionRec pixregion;
> + BoxRec box;
> +
> + dst = dirty->slave_dst->master_pixmap;
> + if (!dst)
> + dst = dirty->slave_dst;
> +
> + box.x1 = 0;
> + box.y1 = 0;
> + if (dirty->rotation == RR_Rotate_90 ||
> + dirty->rotation == RR_Rotate_270) {
> + box.x2 = dst->drawable.height;
> + box.y2 = dst->drawable.width;
> + } else {
> + box.x2 = dst->drawable.width;
> + box.y2 = dst->drawable.height;
> + }
> + RegionInit(&pixregion, &box, 1);
> +
> + /*
> + * SourceValidate is used by the software cursor code
> + * to pull the cursor off of the screen when reading
> + * bits from the frame buffer. Bypassing this function
> + * leaves the software cursor in place
> + */
> + SourceValidate = pScreen->SourceValidate;
> + pScreen->SourceValidate = NULL;
> +
> + RegionTranslate(&pixregion, dirty->x, dirty->y);
> + RegionIntersect(&pixregion, &pixregion, region);
> +
> + if (RegionNil(&pixregion)) {
> + RegionUninit(&pixregion);
> + return FALSE;
> + }
> +
> + RegionTranslate(&pixregion, -dirty->x, -dirty->y);
> +
> + if (!pScreen->root || dirty->rotation == RR_Rotate_0)
> + PixmapDirtyCopyArea(dst, dirty, &pixregion);
> + else
> + PixmapDirtyCompositeRotate(dst, dirty, &pixregion);
> pScreen->SourceValidate = SourceValidate;
> return TRUE;
> }
> diff --git a/hw/xfree86/drivers/modesetting/driver.c
> b/hw/xfree86/drivers/modesetting/driver.c
> index 8603338..9d094e0 100644
> --- a/hw/xfree86/drivers/modesetting/driver.c
> +++ b/hw/xfree86/drivers/modesetting/driver.c
> @@ -541,7 +541,7 @@ redisplay_dirty(ScreenPtr screen, PixmapDirtyUpdatePtr
> dirty)
>
> PixmapRegionInit(&pixregion, dirty->slave_dst);
> DamageRegionAppend(&dirty->slave_dst->drawable, &pixregion);
> - PixmapSyncDirtyHelper(dirty, &pixregion);
> + PixmapSyncDirtyHelper(dirty);
>
> DamageRegionProcessPending(&dirty->slave_dst->drawable);
> RegionUninit(&pixregion);
> diff --git a/hw/xfree86/drivers/modesetting/drmmode_display.c
> b/hw/xfree86/drivers/modesetting/drmmode_display.c
> index 8dbac07..6fb1fdc 100644
> --- a/hw/xfree86/drivers/modesetting/drmmode_display.c
> +++ b/hw/xfree86/drivers/modesetting/drmmode_display.c
> @@ -580,7 +580,7 @@ drmmode_set_scanout_pixmap_gpu(xf86CrtcPtr crtc,
> PixmapPtr ppix)
> screen->height = screenpix->drawable.height = max_height;
> }
> drmmode_crtc->prime_pixmap_x = this_x;
> - PixmapStartDirtyTracking2(ppix, screenpix, 0, 0, this_x, 0);
> + PixmapStartDirtyTracking(ppix, screenpix, 0, 0, this_x, 0, RR_Rotate_0);
> return TRUE;
> }
>
> diff --git a/hw/xfree86/modes/xf86Rotate.c b/hw/xfree86/modes/xf86Rotate.c
> index bc1ea21..4aa8f8d 100644
> --- a/hw/xfree86/modes/xf86Rotate.c
> +++ b/hw/xfree86/modes/xf86Rotate.c
> @@ -360,6 +360,8 @@ xf86CrtcRotate(xf86CrtcPtr crtc)
> RRTransformPtr transform = NULL;
> Bool damage = FALSE;
>
> + if (pScreen->isGPU)
> + return TRUE;
> if (crtc->transformPresent)
> transform = &crtc->transform;
>
> diff --git a/include/pixmap.h b/include/pixmap.h
> index 9656c3a..c6a7736 100644
> --- a/include/pixmap.h
> +++ b/include/pixmap.h
> @@ -50,7 +50,7 @@ SOFTWARE.
> #include "misc.h"
> #include "screenint.h"
> #include "regionstr.h"
> -
> +#include <X11/extensions/randr.h>
> /* types for Drawable */
> #define DRAWABLE_WINDOW 0
> #define DRAWABLE_PIXMAP 1
> @@ -115,16 +115,12 @@ extern _X_EXPORT void FreePixmap(PixmapPtr /*pPixmap */
> );
> extern _X_EXPORT PixmapPtr
> PixmapShareToSlave(PixmapPtr pixmap, ScreenPtr slave);
>
> +#define HAS_DIRTYTRACKING_ROTATION 1
> extern _X_EXPORT Bool
> PixmapStartDirtyTracking(PixmapPtr src,
> PixmapPtr slave_dst,
> - int x, int y);
> -
> -#define HAS_DIRTYTRACKING2 1
> -extern _X_EXPORT Bool
> -PixmapStartDirtyTracking2(PixmapPtr src,
> - PixmapPtr slave_dst,
> - int x, int y, int dst_x, int dst_y);
> + int x, int y, int dst_x, int dst_y,
> + Rotation rotation);
>
> extern _X_EXPORT Bool
> PixmapStopDirtyTracking(PixmapPtr src, PixmapPtr slave_dst);
> @@ -132,6 +128,6 @@ PixmapStopDirtyTracking(PixmapPtr src, PixmapPtr
> slave_dst);
> /* helper function, drivers can do this themselves if they can do it more
> efficently */
> extern _X_EXPORT Bool
> -PixmapSyncDirtyHelper(PixmapDirtyUpdatePtr dirty, RegionPtr dirty_region);
> +PixmapSyncDirtyHelper(PixmapDirtyUpdatePtr dirty);
>
> #endif /* PIXMAP_H */
> diff --git a/include/pixmapstr.h b/include/pixmapstr.h
> index 380e483..4bd2465 100644
> --- a/include/pixmapstr.h
> +++ b/include/pixmapstr.h
> @@ -51,6 +51,8 @@ SOFTWARE.
> #include "regionstr.h"
> #include "privates.h"
> #include "damage.h"
> +#include <X11/extensions/randr.h>
> +#include "picturestr.h"
>
> typedef struct _Drawable {
> unsigned char type; /* DRAWABLE_<type> */
> @@ -91,6 +93,9 @@ typedef struct _PixmapDirtyUpdate {
> DamagePtr damage;
> struct xorg_list ent;
> int dst_x, dst_y;
> + Rotation rotation;
> + PictTransform transform;
> + struct pixman_f_transform f_transform, f_inverse;
> } PixmapDirtyUpdateRec;
>
> static inline void
> diff --git a/include/scrnintstr.h b/include/scrnintstr.h
> index faf0563..a627fe7 100644
> --- a/include/scrnintstr.h
> +++ b/include/scrnintstr.h
> @@ -55,6 +55,7 @@ SOFTWARE.
> #include <X11/Xproto.h>
> #include "dix.h"
> #include "privates.h"
> +#include <X11/extensions/randr.h>
>
> typedef struct _PixmapFormat {
> unsigned char depth;
> @@ -340,7 +341,9 @@ typedef Bool (*SharePixmapBackingProcPtr)(PixmapPtr,
> ScreenPtr, void **);
> typedef Bool (*SetSharedPixmapBackingProcPtr)(PixmapPtr, void *);
>
> typedef Bool (*StartPixmapTrackingProcPtr)(PixmapPtr, PixmapPtr,
> - int x, int y);
> + int x, int y,
> + int dst_x, int dst_y,
> + Rotation rotation);
>
> typedef Bool (*StopPixmapTrackingProcPtr)(PixmapPtr, PixmapPtr);
>
> diff --git a/randr/rrcrtc.c b/randr/rrcrtc.c
> index e95b049..050d975 100644
> --- a/randr/rrcrtc.c
> +++ b/randr/rrcrtc.c
> @@ -387,7 +387,7 @@ RRCrtcDetachScanoutPixmap(RRCrtcPtr crtc)
>
> static Bool
> rrCreateSharedPixmap(RRCrtcPtr crtc, int width, int height,
> - int x, int y)
> + int x, int y, Rotation rotation)
> {
> PixmapPtr mpix, spix;
> ScreenPtr master = crtc->pScreen->current_master;
> @@ -434,13 +434,33 @@ rrCreateSharedPixmap(RRCrtcPtr crtc, int width, int
> height,
>
> crtc->scanout_pixmap = spix;
>
> - master->StartPixmapTracking(mscreenpix, spix, x, y);
> + master->StartPixmapTracking(mscreenpix, spix, x, y, 0, 0, rotation);
> return TRUE;
> }
>
> +static void crtc_to_box(BoxPtr box, RRCrtcPtr crtc)
> +{
> + box->x1 = crtc->x;
> + box->y1 = crtc->y;
> + switch (crtc->rotation) {
> + case RR_Rotate_0:
> + case RR_Rotate_180:
> + default:
> + box->x2 = crtc->x + crtc->mode->mode.width;
> + box->y2 = crtc->y + crtc->mode->mode.height;
> + break;
> + case RR_Rotate_90:
> + case RR_Rotate_270:
> + box->x2 = crtc->x + crtc->mode->mode.height;
> + box->y2 = crtc->y + crtc->mode->mode.width;
> + break;
> + }
> +}
> +
> static Bool
> rrCheckPixmapBounding(ScreenPtr pScreen,
> - RRCrtcPtr rr_crtc, int x, int y, int w, int h)
> + RRCrtcPtr rr_crtc, Rotation rotation,
> + int x, int y, int w, int h)
> {
> RegionRec root_pixmap_region, total_region, new_crtc_region;
> int c;
> @@ -461,16 +481,19 @@ rrCheckPixmapBounding(ScreenPtr pScreen,
>
> if (crtc == rr_crtc) {
> newbox.x1 = x;
> - newbox.x2 = x + w;
> newbox.y1 = y;
> - newbox.y2 = y + h;
> + if (rotation == RR_Rotate_90 ||
> + rotation == RR_Rotate_270) {
> + newbox.x2 = x + h;
> + newbox.y2 = y + w;
> + } else {
> + newbox.x2 = x + w;
> + newbox.y2 = y + h;
> + }
> } else {
> if (!crtc->mode)
> continue;
> - newbox.x1 = crtc->x;
> - newbox.x2 = crtc->x + crtc->mode->mode.width;
> - newbox.y1 = crtc->y;
> - newbox.y2 = crtc->y + crtc->mode->mode.height;
> + crtc_to_box(&newbox, crtc);
> }
> RegionInit(&new_crtc_region, &newbox, 1);
> RegionUnion(&total_region, &total_region, &new_crtc_region);
> @@ -483,17 +506,20 @@ rrCheckPixmapBounding(ScreenPtr pScreen,
>
> if (slave_crtc == rr_crtc) {
> newbox.x1 = x;
> - newbox.x2 = x + w;
> newbox.y1 = y;
> - newbox.y2 = y + h;
> + if (rotation == RR_Rotate_90 ||
> + rotation == RR_Rotate_270) {
> + newbox.x2 = x + h;
> + newbox.y2 = y + w;
> + } else {
> + newbox.x2 = x + w;
> + newbox.y2 = y + h;
> + }
> }
> else {
> if (!slave_crtc->mode)
> continue;
> - newbox.x1 = slave_crtc->x;
> - newbox.x2 = slave_crtc->x + slave_crtc->mode->mode.width;
> - newbox.y1 = slave_crtc->y;
> - newbox.y2 = slave_crtc->y + slave_crtc->mode->mode.height;
> + crtc_to_box(&newbox, slave_crtc);
> }
> RegionInit(&new_crtc_region, &newbox, 1);
> RegionUnion(&total_region, &total_region, &new_crtc_region);
> @@ -561,12 +587,12 @@ RRCrtcSet(RRCrtcPtr crtc,
> height = mode->mode.height;
> }
> ret = rrCheckPixmapBounding(master, crtc,
> - x, y, width, height);
> + rotation, x, y, width, height);
> if (!ret)
> return FALSE;
>
> if (pScreen->current_master) {
> - ret = rrCreateSharedPixmap(crtc, width, height, x, y);
> + ret = rrCreateSharedPixmap(crtc, width, height, x, y,
> rotation);
> }
> }
> #if RANDR_12_INTERFACE
> --
> 2.4.3
>
> _______________________________________________
> xorg-devel@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
_______________________________________________
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel
