On Thu, Mar 31, 2016 at 6:13 AM, Daniel Stone <dan...@fooishbar.org> wrote:
> Hi,
>
> On 31 March 2016 at 04:21, Rob Herring <r...@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
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
