[Mesa-dev] [RFC PATCH] GBM: Add map/unmap functions

[Daniel Stone]

Hi,

On 31 March 2016 at 04:21, Rob Herring <robh at kernel.org> wrote:
> 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

Nitpick: the other USE flags are passed at allocation time. Is this
something you're intending to plumb through into allocation as well?
If not, should probably move these flags to another namespace.

> 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);
> +}
> +
> +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;
> +}

The pipe_private dance suggests to me that either you need to pass
more data to the lock/unlock handlers, or that you need to explicitly
disallow multiple concurrent mappings. Allowing multiple active
mappings seems like it could be desirable, especially for systems
which need to upload on unmap, who would then be able to aggregate the
uploads.

> 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);

I'm a bit wary about creating a new context - especially as this ends
up creating a full OpenGL context - but I also don't really know what
would end up being better than this.

Cheers,
Daniel