[PATCH 4/6] modesetting: reverse prime support

Eric Anholt eric at anholt.net
Fri Jun 19 10:54:33 PDT 2015


Dave Airlie <airlied at gmail.com> writes:

> This adds support for reverse prime to the modesetting driver.
>
> Reverse prime is where we have two GPUs in the display chain,
> but the second GPU can't scanout from the shared pixmap, so needs
> an extra copy to the on screen pixmap.
>
> This allows modesetting to support this scenario while still
> supporting the USB offload one.
>
> Signed-off-by: Dave Airlie <airlied at redhat.com>
> ---
>  hw/xfree86/drivers/modesetting/driver.c          | 17 +++++-
>  hw/xfree86/drivers/modesetting/drmmode_display.c | 76 ++++++++++++++++++++++--
>  hw/xfree86/drivers/modesetting/drmmode_display.h |  4 +-
>  3 files changed, 88 insertions(+), 9 deletions(-)
>
> diff --git a/hw/xfree86/drivers/modesetting/driver.c b/hw/xfree86/drivers/modesetting/driver.c
> index 092cc53..f421668 100644
> --- a/hw/xfree86/drivers/modesetting/driver.c
> +++ b/hw/xfree86/drivers/modesetting/driver.c
> @@ -572,7 +572,7 @@ msBlockHandler(ScreenPtr pScreen, void *pTimeout, void *pReadmask)
>      pScreen->BlockHandler(pScreen, pTimeout, pReadmask);
>      ms->BlockHandler = pScreen->BlockHandler;
>      pScreen->BlockHandler = msBlockHandler;
> -    if (pScreen->isGPU)
> +    if (pScreen->isGPU && !ms->drmmode.reverse_prime_offload_mode)
>          dispatch_slave_dirty(pScreen);
>      else if (ms->dirty_enabled)
>          dispatch_dirty(pScreen);
> @@ -999,10 +999,18 @@ msSetSharedPixmapBacking(PixmapPtr ppix, void *fd_handle)
>      ScrnInfoPtr scrn = xf86ScreenToScrn(screen);
>      modesettingPtr ms = modesettingPTR(scrn);
>      Bool ret;
> -    int size = ppix->devKind * ppix->drawable.height;
>      int ihandle = (int) (long) fd_handle;
>  
> -    ret = drmmode_SetSlaveBO(ppix, &ms->drmmode, ihandle, ppix->devKind, size);
> +    if (ms->drmmode.reverse_prime_offload_mode) {
> +        ret = glamor_back_pixmap_from_fd(ppix, ihandle,
> +                                         ppix->drawable.width,
> +                                         ppix->drawable.height,
> +                                         ppix->devKind, ppix->drawable.depth,
> +                                         ppix->drawable.bitsPerPixel);
> +    } else {
> +        int size = ppix->devKind * ppix->drawable.height;
> +        ret = drmmode_SetSlaveBO(ppix, &ms->drmmode, ihandle, ppix->devKind, size);
> +    }
>      if (ret == FALSE)
>          return ret;
>  
> @@ -1190,6 +1198,9 @@ ScreenInit(ScreenPtr pScreen, int argc, char **argv)
>              xf86DrvMsg(pScrn->scrnIndex, X_ERROR,
>                         "Failed to initialize the Present extension.\n");
>          }
> +        /* enable reverse prime if we are a GPU screen, and accelerated */
> +        if (pScreen->isGPU)
> +            ms->drmmode.reverse_prime_offload_mode = TRUE;
>      }
>  #endif
>  
> diff --git a/hw/xfree86/drivers/modesetting/drmmode_display.c b/hw/xfree86/drivers/modesetting/drmmode_display.c
> index f3c9909..e36c092 100644
> --- a/hw/xfree86/drivers/modesetting/drmmode_display.c
> +++ b/hw/xfree86/drivers/modesetting/drmmode_display.c
> @@ -50,6 +50,7 @@
>  
>  #include "driver.h"
>  
> +static Bool drmmode_xf86crtc_resize(ScrnInfoPtr scrn, int width, int height);
>  static int
>  drmmode_bo_destroy(drmmode_ptr drmmode, drmmode_bo *bo)
>  {
> @@ -343,10 +344,14 @@ drmmode_set_mode_major(xf86CrtcPtr crtc, DisplayModePtr mode,
>  
>          fb_id = drmmode->fb_id;
>          if (crtc->randr_crtc->scanout_pixmap) {
> -            msPixmapPrivPtr ppriv =
> -                msGetPixmapPriv(drmmode, crtc->randr_crtc->scanout_pixmap);
> -            fb_id = ppriv->fb_id;
> -            x = y = 0;
> +            if (!drmmode->reverse_prime_offload_mode) {
> +                msPixmapPrivPtr ppriv =
> +                    msGetPixmapPriv(drmmode, crtc->randr_crtc->scanout_pixmap);
> +                fb_id = ppriv->fb_id;
> +                x = 0;
> +            } else
> +                x = drmmode_crtc->prime_pixmap_x;
> +            y = 0;
>          }
>          else if (drmmode_crtc->rotate_fb_id) {
>              fb_id = drmmode_crtc->rotate_fb_id;
> @@ -502,7 +507,56 @@ drmmode_crtc_gamma_set(xf86CrtcPtr crtc, uint16_t * red, uint16_t * green,
>  }
>  
>  static Bool
> -drmmode_set_scanout_pixmap(xf86CrtcPtr crtc, PixmapPtr ppix)
> +drmmode_set_scanout_pixmap_gpu(xf86CrtcPtr crtc, PixmapPtr ppix)
> +{
> +    ScreenPtr screen = xf86ScrnToScreen(crtc->scrn);
> +    PixmapPtr screenpix = screen->GetScreenPixmap(screen);
> +    xf86CrtcConfigPtr xf86_config = XF86_CRTC_CONFIG_PTR(crtc->scrn);
> +    drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
> +    int c, total_width = 0, max_height = 0, this_x = 0;
> +
> +    if (!ppix) {
> +        if (crtc->randr_crtc->scanout_pixmap)
> +            PixmapStopDirtyTracking(crtc->randr_crtc->scanout_pixmap, screenpix);
> +        drmmode_crtc->prime_pixmap_x = 0;
> +        return TRUE;
> +    }
> +    /* iterate over all the attached crtcs -
> +       work out bounding box */

Funny formatting, how about just:

       /* iterate over all the attached crtcs to work out the bounding box */

> +    for (c = 0; c < xf86_config->num_crtc; c++) {
> +        xf86CrtcPtr iter = xf86_config->crtc[c];
> +        if (!iter->enabled && iter != crtc)
> +            continue;
> +        if (iter == crtc) {
> +            this_x = total_width;
> +            total_width += ppix->drawable.width;
> +            if (max_height < ppix->drawable.height)
> +                max_height = ppix->drawable.height;
> +        } else {
> +            total_width += iter->mode.HDisplay;
> +            if (max_height < iter->mode.VDisplay)
> +                max_height = iter->mode.VDisplay;
> +        }
> +    }
> +
> +    if (total_width != screenpix->drawable.width ||
> +        max_height != screenpix->drawable.height) {
> +        Bool ret;
> +        ret = drmmode_xf86crtc_resize(crtc->scrn, total_width, max_height);
> +        if (ret == FALSE)
> +            return FALSE;

How about just:

           if (!drmmode_xf86crtc_resize(crtc->scrn, total_width, max_height))
               return FALSE

> +
> +        screenpix = screen->GetScreenPixmap(screen);
> +        screen->width = screenpix->drawable.width = total_width;
> +        screen->height = screenpix->drawable.height = max_height;

Directly setting the width/height of a pixmap?  That seems suspicious to
me.  Usually you're using ModifyPixmapHeader(), since you need to change
other things (like the pointer/stride) at the same time.  Is there an
explanation for this being safe?

> +    }
> +    drmmode_crtc->prime_pixmap_x = this_x;
> +    PixmapStartDirtyTracking2(ppix, screenpix, 0, 0, this_x, 0);
> +    return TRUE;
> +}
> +
> +static Bool
> +drmmode_set_scanout_pixmap_cpu(xf86CrtcPtr crtc, PixmapPtr ppix)
>  {
>      drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
>      drmmode_ptr drmmode = drmmode_crtc->drmmode;
> @@ -543,6 +597,18 @@ drmmode_set_scanout_pixmap(xf86CrtcPtr crtc, PixmapPtr ppix)
>      return TRUE;
>  }
>  
> +static Bool
> +drmmode_set_scanout_pixmap(xf86CrtcPtr crtc, PixmapPtr ppix)
> +{
> +    drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
> +    drmmode_ptr drmmode = drmmode_crtc->drmmode;
> +
> +    if (drmmode->reverse_prime_offload_mode)
> +        return drmmode_set_scanout_pixmap_gpu(crtc, ppix);
> +    else
> +        return drmmode_set_scanout_pixmap_cpu(crtc, ppix);
> +}
> +
>  static void *
>  drmmode_shadow_allocate(xf86CrtcPtr crtc, int width, int height)
>  {
> diff --git a/hw/xfree86/drivers/modesetting/drmmode_display.h b/hw/xfree86/drivers/modesetting/drmmode_display.h
> index b0e45b6..85a0ec4 100644
> --- a/hw/xfree86/drivers/modesetting/drmmode_display.h
> +++ b/hw/xfree86/drivers/modesetting/drmmode_display.h
> @@ -79,6 +79,8 @@ typedef struct {
>      uint32_t triple_buffer_name;
>  
>      DevPrivateKeyRec pixmapPrivateKeyRec;
> +
> +    Bool reverse_prime_offload_mode;
>  } drmmode_rec, *drmmode_ptr;
>  
>  typedef struct {
> @@ -93,7 +95,7 @@ typedef struct {
>  
>      drmmode_bo rotate_bo;
>      unsigned rotate_fb_id;
> -
> +    unsigned prime_pixmap_x;
>      /**
>       * @{ MSC (vblank count) handling for the PRESENT extension.
>       *
> -- 
> 2.4.2
>
> _______________________________________________
> 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 818 bytes
Desc: not available
URL: <http://lists.x.org/archives/xorg-devel/attachments/20150619/b32fd4df/attachment.sig>


More information about the xorg-devel mailing list