[PATCH] xwayland: Use drm buffers for cursors if available

[Pekka Paalanen]

On Thu,  4 Feb 2016 18:10:04 +0100
Rui Matos <tiagomatos at gmail.com> wrote:

> This should be both more efficient and avoid the file descriptor
> exhaustion issue that can happen with shm buffers if we're asked to
> realize more cursors than the file descriptor limit.
> 
> Signed-off-by: Rui Matos <tiagomatos at gmail.com>
> ---
> 
> On Sat, Dec 5, 2015 at 1:30 AM, Kristian Høgsberg <krh at bitplanet.net> wrote:
> > Nice work. I think it would be about the same amount of work to do
> > this for shm pixmaps instead of cursors. That way, an application that
> > creates a lot of small pixmaps on shm-Xwayland (non-glamor) will also
> > not crash the server either.  
> 
> I looked into doing this for a bit but it seemed very wasteful in
> terms of memory usage with this simple allocation algorithm. I fear
> the fragmentation after a little while would be pretty bad. At least
> with cursors only we know that they're sized all in the same order of
> magnitude but for pixmaps in general I think we'd need a smarter
> algorithm and a good handful of heuristics.
> 
> So, I decided to instead work on adding support for wl_drm cursor
> buffers which nicely sidesteps all those issues. Unfortunately, with
> this patch and on intel hardware where the hw cursor size isn't 64x64,
> compositors have to fallback to "software" cursors, importing the
> buffer as an EGLImage since mesa has this size hardcoded in
> 
> http://cgit.freedesktop.org/mesa/mesa/tree/src/mesa/drivers/dri/i965/intel_screen.c#n621
> 
> which prevents compositors from importing the gbm bo.
> 
> Rui
> 
>  hw/xwayland/xwayland-cursor.c | 156 ++++++++++++++++++++++++++++++++++++------
>  hw/xwayland/xwayland.c        |   6 +-
>  2 files changed, 138 insertions(+), 24 deletions(-)
> 
> diff --git a/hw/xwayland/xwayland-cursor.c b/hw/xwayland/xwayland-cursor.c
> index 76729db..7f2aa9a 100644
> --- a/hw/xwayland/xwayland-cursor.c
> +++ b/hw/xwayland/xwayland-cursor.c
> @@ -28,7 +28,27 @@
>  
>  #include <mipointer.h>
>  
> +#ifdef GLAMOR_HAS_GBM
> +#define MESA_EGL_NO_X11_HEADERS
> +#include <drm/drm.h>
> +#include <gbm.h>
> +#include <xf86drm.h>
> +#include "drm-client-protocol.h"
> +#endif
> +
> +struct xwl_cursor {
> +    struct wl_buffer *buffer;
> +    PixmapPtr pixmap;
> +#ifdef GLAMOR_HAS_GBM
> +    struct gbm_bo *bo;
> +#endif
> +};
> +
>  static DevPrivateKeyRec xwl_cursor_private_key;
> +#ifdef GLAMOR_HAS_GBM
> +static uint64_t hw_cursor_width;
> +static uint64_t hw_cursor_height;
> +#endif
>  
>  static void
>  expand_source_and_mask(CursorPtr cursor, CARD32 *data)
> @@ -61,25 +81,88 @@ expand_source_and_mask(CursorPtr cursor, CARD32 *data)
>  }
>  
>  static Bool
> +alloc_gbm_cursor(struct xwl_cursor *cursor, struct xwl_screen *screen, size_t width, size_t height)
> +{
> +#ifdef GLAMOR_HAS_GBM
> +    struct gbm_bo *bo;
> +    union gbm_bo_handle handle;
> +    int prime_fd = -1;
> +
> +    if (!screen->glamor)
> +        return FALSE;
> +    if (width > hw_cursor_width || height > hw_cursor_height)
> +        return FALSE;
> +
> +    bo = gbm_bo_create(screen->gbm, hw_cursor_width, hw_cursor_height,
> +                       GBM_FORMAT_ARGB8888, GBM_BO_USE_CURSOR | GBM_BO_USE_WRITE);
> +    if (!bo)
> +        return FALSE;
> +
> +    handle = gbm_bo_get_handle(bo);
> +    if (drmPrimeHandleToFD(screen->drm_fd, handle.u32, 0, &prime_fd) || prime_fd == -1)
> +        goto err_free_bo;
> +
> +    cursor->buffer =
> +        wl_drm_create_prime_buffer(screen->drm, prime_fd,
> +                                   hw_cursor_width, hw_cursor_height,
> +                                   WL_DRM_FORMAT_ARGB8888,
> +                                   0, gbm_bo_get_stride(bo),
> +                                   0, 0,
> +                                   0, 0);

Hi,

umm, do we really want to add even more uses of the Mesa-private wl_drm
protocol outside of Mesa? Though it seems that ship sailed a long time
ago.

Should we just bite the bullet, and have Mesa install drm.xml for
public consumption? Or move drm.xml to wayland-protocols.git?

I thought all that was a Bad Idea(tm).

Or once the generic dmabuf protocol matures, could that replace the
direct use of wl_drm in xwayland and glamor completely?

Btw. Rui, is there a reason to keep the shm pool fd open after a
wl_buffer has been created from it? Most trivial demos just close the
fd and destroy the pool once a wl_buffer has been created, keeping only
the wl_buffer and the mmap around. The pool or fd are not needed to
keep the memory available to both the Wayland server and client. But if
you want to enlarge the pool, then you need the fd, naturally.

Of course putting several small buffers in the same shm pool is useful,
I do not question that.


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 811 bytes
Desc: OpenPGP digital signature
URL: <http://lists.x.org/archives/xorg-devel/attachments/20160205/3469647f/attachment-0001.sig>