[PATCH] prime: add rotation support for offloaded outputs (v2)

Alex Deucher alexdeucher at gmail.com
Wed Jul 1 07:24:27 PDT 2015


On Tue, Jun 30, 2015 at 12:54 AM, Dave Airlie <airlied at 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 <airlied at redhat.com>

Reviewed-by: Alex Deucher <alexander.deucher at 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 at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel


More information about the xorg-devel mailing list