On Wed, Mar 30, 2016 at 11:21 PM, Rob Herring <r...@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 <r...@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 > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev