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

[Rob Herring]

On Thu, Mar 31, 2016 at 6:13 AM, Daniel Stone <daniel at fooishbar.org> wrote:
> 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.

I don't have any immediate need to use them for allocation, but do I
need to? There's already a GBM flag for mapping (though intended for
cursors).

>> 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.

Currently multiple mappings are not allowed, but it should be simple
enough to return the token up the stack. Seems like that could get
complicated, but if the pipe_transfer supports that already there's no
reason to restrict here.

Rob