[Mesa-dev] [RFC PATCH] GBM: Add map/unmap functions
Rob Clark
robdclark at gmail.com
Mon Apr 11 19:00:41 UTC 2016
On Wed, Mar 30, 2016 at 11:21 PM, Rob Herring <robh at kernel.org> wrote:
> This adds GBM map and unmap functions. Unlike prior attempts, this
> version provides a region and usage flags for the mapping. The operation
> follows gallium transfer_map() function. This is complete enough to work
> on Android with gralloc using the GBM API.
>
> TODO:
> - split up patches
> - define and propagate GBM usage flags
> - DRIImage extension version checking
> - Need to flush context on unmap?
> - Need fences for gralloc lockAsync
> - integrate dumb buffer mapping
> - bikeshed the name (map or lock)
>
> Signed-off-by: Rob Herring <robh at kernel.org>
> ---
> This reflects the discussion on irc today. I got further than I expected,
> and it is basically working with Android. The goal here is to use GBM
> for Android gralloc instead of per driver and pipe implementations. I'm
> looking for some early feedback on the GBM interface and the extending
> of the DRIimage extension.
>
> Rob
>
>
> include/GL/internal/dri_interface.h | 30 +++++++++++++++++++++++
> src/gallium/state_trackers/dri/dri2.c | 38 +++++++++++++++++++++++++++++
> src/gallium/state_trackers/dri/dri_screen.h | 1 +
> src/gbm/backends/dri/gbm_dri.c | 30 +++++++++++++++++++++++
> src/gbm/backends/dri/gbm_driint.h | 1 +
> src/gbm/gbm-symbols-check | 2 ++
> src/gbm/main/gbm.c | 20 +++++++++++++++
> src/gbm/main/gbm.h | 9 +++++++
> src/gbm/main/gbmint.h | 5 ++++
> 9 files changed, 136 insertions(+)
>
> diff --git a/include/GL/internal/dri_interface.h b/include/GL/internal/dri_interface.h
> index 6bbd3fa..b059112 100644
> --- a/include/GL/internal/dri_interface.h
> +++ b/include/GL/internal/dri_interface.h
> @@ -1101,6 +1101,9 @@ struct __DRIdri2ExtensionRec {
> #define __DRI_IMAGE_USE_CURSOR 0x0004 /* Depricated */
> #define __DRI_IMAGE_USE_LINEAR 0x0008
>
> +#define __DRI_IMAGE_USE_READ 0x10000
> +#define __DRI_IMAGE_USE_WRITE 0x20000
> +
>
> /**
> * Four CC formats that matches with WL_DRM_FORMAT_* from wayland_drm.h,
> @@ -1350,6 +1353,33 @@ struct __DRIimageExtensionRec {
> * \since 10
> */
> int (*getCapabilities)(__DRIscreen *screen);
> +
> + /**
> + * Lock a part of a __DRIimage for specified usage
> + *
> + * flush_flag:
> + * 0: no flush
> + * __BLIT_FLAG_FLUSH: flush after the blit operation
> + * __BLIT_FLAG_FINISH: flush and wait the blit finished
> + *
> + * \since 11
> + */
> + void *(*lockImage)(__DRIcontext *context, __DRIimage *image,
> + int x0, int y0, int width, int height,
> + unsigned int usage);
> +
> + /**
> + * Unlock a __DRIimage for specified usage
> + *
> + * flush_flag:
> + * 0: no flush
> + * __BLIT_FLAG_FLUSH: flush after the blit operation
> + * __BLIT_FLAG_FINISH: flush and wait the blit finished
> + *
> + * \since 11
> + */
> + int (*unlockImage)(__DRIcontext *context, __DRIimage *image);
> +
> };
>
>
> diff --git a/src/gallium/state_trackers/dri/dri2.c b/src/gallium/state_trackers/dri/dri2.c
> index 29aaa96..b12fc50 100644
> --- a/src/gallium/state_trackers/dri/dri2.c
> +++ b/src/gallium/state_trackers/dri/dri2.c
> @@ -1217,6 +1217,42 @@ dri2_blit_image(__DRIcontext *context, __DRIimage *dst, __DRIimage *src,
> }
> }
>
> +static void *
> +dri2_lock_image(__DRIcontext *context, __DRIimage *image,
> + int x0, int y0, int width, int height,
> + unsigned int usage)
> +{
> + struct dri_context *ctx = dri_context(context);
> + struct pipe_context *pipe = ctx->st->pipe;
> + enum pipe_transfer_usage pipe_usage = PIPE_TRANSFER_READ;
> +
> + if (!image)
> + return NULL;
> +
> + if (usage & __DRI_IMAGE_USE_WRITE)
> + pipe_usage |= PIPE_TRANSFER_WRITE;
> +
> + assert(!image->pipe_private);
> +
> + /*
> + * ignore x, y, w and h so that returned addr points at the
> + * start of the buffer
> + */
> + return pipe_transfer_map(pipe, image->texture,
> + 0, 0, pipe_usage, x0, y0, width, height,
> + (struct pipe_transfer **)&image->pipe_private);
One small note is that the driver could return a stride (via struct
pipe_transfer) that is different from the image's native stride.. it
is a bit unfortunate that the gralloc API doesn't seem to take that
into account. I suspect that would/could be an issue for
vmwgfx/virtgl[1].
Probably we should dtrt in gbm, and return via reference the
pitch/stride of the mapping, and maybe just assert in gralloc if it
does not match the image? (And maybe hopefully someday someone will
fix gralloc?)
[1] unless android tends to just lock the entire image.. which would
also be sad..
BR,
-R
> +}
> +
> +static void
> +dri2_unlock_image(__DRIcontext *context, __DRIimage *image)
> +{
> + struct dri_context *ctx = dri_context(context);
> + struct pipe_context *pipe = ctx->st->pipe;
> +
> + pipe_transfer_unmap(pipe, (struct pipe_transfer *)image->pipe_private);
> + image->pipe_private = NULL;
> +}
> +
> static void
> dri2_destroy_image(__DRIimage *img)
> {
> @@ -1250,6 +1286,8 @@ static __DRIimageExtension dri2ImageExtension = {
> .createImageFromDmaBufs = NULL,
> .blitImage = dri2_blit_image,
> .getCapabilities = dri2_get_capabilities,
> + .lockImage = dri2_lock_image,
> + .unlockImage = dri2_unlock_image,
> };
>
>
> diff --git a/src/gallium/state_trackers/dri/dri_screen.h b/src/gallium/state_trackers/dri/dri_screen.h
> index 4545990..f04cc2a 100644
> --- a/src/gallium/state_trackers/dri/dri_screen.h
> +++ b/src/gallium/state_trackers/dri/dri_screen.h
> @@ -111,6 +111,7 @@ struct __DRIimageRec {
> uint32_t dri_components;
>
> void *loader_private;
> + void *pipe_private;
>
> /**
> * Provided by EGL_EXT_image_dma_buf_import.
> diff --git a/src/gbm/backends/dri/gbm_dri.c b/src/gbm/backends/dri/gbm_dri.c
> index c34e39c..77b2b10 100644
> --- a/src/gbm/backends/dri/gbm_dri.c
> +++ b/src/gbm/backends/dri/gbm_dri.c
> @@ -918,6 +918,34 @@ failed:
> return NULL;
> }
>
> +static void *
> +_gbm_dri_bo_map(struct gbm_bo *_bo,
> + uint32_t x, uint32_t y,
> + uint32_t width, uint32_t height,
> + uint32_t usage)
> +{
> + struct gbm_dri_device *dri = gbm_dri_device(_bo->gbm);
> + struct gbm_dri_bo *bo = gbm_dri_bo(_bo);
> +
> + if (!dri->context)
> + dri->context = dri->dri2->createNewContext(dri->screen, NULL, NULL, NULL);
> + if (!dri->context)
> + return NULL;
> +
> + usage = __DRI_IMAGE_USE_WRITE;
> + return dri->image->lockImage(dri->context, bo->image, x, y, width, height, usage);
> +}
> +
> +static void
> +_gbm_dri_bo_unmap(struct gbm_bo *_bo)
> +{
> + struct gbm_dri_device *dri = gbm_dri_device(_bo->gbm);
> + struct gbm_dri_bo *bo = gbm_dri_bo(_bo);
> +
> + dri->image->unlockImage(dri->context, bo->image);
> +}
> +
> +
> static struct gbm_surface *
> gbm_dri_surface_create(struct gbm_device *gbm,
> uint32_t width, uint32_t height,
> @@ -975,6 +1003,8 @@ dri_device_create(int fd)
> dri->base.base.fd = fd;
> dri->base.base.bo_create = gbm_dri_bo_create;
> dri->base.base.bo_import = gbm_dri_bo_import;
> + dri->base.base.bo_map = _gbm_dri_bo_map;
> + dri->base.base.bo_unmap = _gbm_dri_bo_unmap;
> dri->base.base.is_format_supported = gbm_dri_is_format_supported;
> dri->base.base.bo_write = gbm_dri_bo_write;
> dri->base.base.bo_get_fd = gbm_dri_bo_get_fd;
> diff --git a/src/gbm/backends/dri/gbm_driint.h b/src/gbm/backends/dri/gbm_driint.h
> index 3f46eff..826954c 100644
> --- a/src/gbm/backends/dri/gbm_driint.h
> +++ b/src/gbm/backends/dri/gbm_driint.h
> @@ -45,6 +45,7 @@ struct gbm_dri_device {
> void *driver;
>
> __DRIscreen *screen;
> + __DRIcontext *context;
>
> const __DRIcoreExtension *core;
> const __DRIdri2Extension *dri2;
> diff --git a/src/gbm/gbm-symbols-check b/src/gbm/gbm-symbols-check
> index f2dde58..5a333ff 100755
> --- a/src/gbm/gbm-symbols-check
> +++ b/src/gbm/gbm-symbols-check
> @@ -9,6 +9,8 @@ gbm_device_destroy
> gbm_create_device
> gbm_bo_create
> gbm_bo_import
> +gbm_bo_map
> +gbm_bo_unmap
> gbm_bo_get_width
> gbm_bo_get_height
> gbm_bo_get_stride
> diff --git a/src/gbm/main/gbm.c b/src/gbm/main/gbm.c
> index c046b1a..e046d25 100644
> --- a/src/gbm/main/gbm.c
> +++ b/src/gbm/main/gbm.c
> @@ -385,6 +385,26 @@ gbm_bo_import(struct gbm_device *gbm,
> return gbm->bo_import(gbm, type, buffer, usage);
> }
>
> +GBM_EXPORT void *
> +gbm_bo_map(struct gbm_bo *bo,
> + uint32_t x, uint32_t y,
> + uint32_t width, uint32_t height,
> + uint32_t usage)
> +{
> + if (!bo || width == 0 || height == 0) {
> + errno = EINVAL;
> + return NULL;
> + }
> +
> + return bo->gbm->bo_map(bo, x, y, width, height, usage);
> +}
> +
> +GBM_EXPORT void
> +gbm_bo_unmap(struct gbm_bo *bo)
> +{
> + bo->gbm->bo_unmap(bo);
> +}
> +
> /**
> * Allocate a surface object
> *
> diff --git a/src/gbm/main/gbm.h b/src/gbm/main/gbm.h
> index 8db2153..9bd0773 100644
> --- a/src/gbm/main/gbm.h
> +++ b/src/gbm/main/gbm.h
> @@ -237,6 +237,15 @@ gbm_bo_create(struct gbm_device *gbm,
> uint32_t width, uint32_t height,
> uint32_t format, uint32_t flags);
>
> +void *
> +gbm_bo_map(struct gbm_bo *bo,
> + uint32_t x, uint32_t y,
> + uint32_t width, uint32_t height,
> + uint32_t flags);
> +
> +void
> +gbm_bo_unmap(struct gbm_bo *bo);
> +
> #define GBM_BO_IMPORT_WL_BUFFER 0x5501
> #define GBM_BO_IMPORT_EGL_IMAGE 0x5502
> #define GBM_BO_IMPORT_FD 0x5503
> diff --git a/src/gbm/main/gbmint.h b/src/gbm/main/gbmint.h
> index 155eb12..e38eaad 100644
> --- a/src/gbm/main/gbmint.h
> +++ b/src/gbm/main/gbmint.h
> @@ -68,6 +68,11 @@ struct gbm_device {
> uint32_t usage);
> struct gbm_bo *(*bo_import)(struct gbm_device *gbm, uint32_t type,
> void *buffer, uint32_t usage);
> + void *(*bo_map)(struct gbm_bo *bo,
> + uint32_t x, uint32_t y,
> + uint32_t width, uint32_t height,
> + uint32_t usage);
> + void (*bo_unmap)(struct gbm_bo *bo);
> int (*bo_write)(struct gbm_bo *bo, const void *buf, size_t data);
> int (*bo_get_fd)(struct gbm_bo *bo);
> void (*bo_destroy)(struct gbm_bo *bo);
> --
> 2.5.0
>
More information about the mesa-dev
mailing list