[Mesa-dev] [PATCH 09/19] radeonsi: add basic memory object support

[Andres Rodriguez]

On 2017-07-04 12:39 PM, Andres Rodriguez wrote:
> 
> 
> On 2017-07-04 09:30 AM, Christian König wrote:
>> Am 04.07.2017 um 15:13 schrieb Nicolai Hähnle:
>>> On 01.07.2017 01:03, Andres Rodriguez wrote:
>>>> From: Dave Airlie <airlied at redhat.com>
>>>>
>>>> Signed-off-by: Andres Rodriguez <andresx7 at gmail.com>
>>>> ---
>>>>   src/gallium/drivers/radeon/r600_pipe_common.h |  6 ++
>>>>   src/gallium/drivers/radeon/r600_texture.c     | 82 
>>>> +++++++++++++++++++++++++++
>>>>   2 files changed, 88 insertions(+)
>>>>
>>>> diff --git a/src/gallium/drivers/radeon/r600_pipe_common.h 
>>>> b/src/gallium/drivers/radeon/r600_pipe_common.h
>>>> index b22a3a7..c5734b4 100644
>>>> --- a/src/gallium/drivers/radeon/r600_pipe_common.h
>>>> +++ b/src/gallium/drivers/radeon/r600_pipe_common.h
>>>> @@ -377,6 +377,12 @@ union r600_mmio_counters {
>>>>       unsigned array[0];
>>>>   };
>>>>   +struct r600_memory_object {
>>>> +    struct pb_buffer        *buf;
>>>> +    uint32_t            stride;
>>>> +    uint32_t            offset;
>>>> +};
>>>> +
>>>>   struct r600_common_screen {
>>>>       struct pipe_screen        b;
>>>>       struct radeon_winsys        *ws;
>>>> diff --git a/src/gallium/drivers/radeon/r600_texture.c 
>>>> b/src/gallium/drivers/radeon/r600_texture.c
>>>> index d68587b..3bc8b0c 100644
>>>> --- a/src/gallium/drivers/radeon/r600_texture.c
>>>> +++ b/src/gallium/drivers/radeon/r600_texture.c
>>>> @@ -2784,10 +2784,92 @@ void evergreen_do_fast_color_clear(struct 
>>>> r600_common_context *rctx,
>>>>       }
>>>>   }
>>>>   +static struct pipe_memory_object *
>>>> +r600_memobj_from_handle(struct pipe_screen *screen,
>>>> +            struct winsys_handle *whandle)
>>>> +{
>>>> +    struct r600_common_screen *rscreen = (struct 
>>>> r600_common_screen*)screen;
>>>> +    struct r600_memory_object *memobj = 
>>>> CALLOC_STRUCT(r600_memory_object);
>>>> +    struct pb_buffer *buf = NULL;
>>>> +    uint32_t stride, offset;
>>>> +
>>>> +    if (!memobj)
>>>> +        return NULL;
>>>> +
>>>> +    buf = rscreen->ws->buffer_from_handle(rscreen->ws, whandle,
>>>> +                          &stride, &offset);
>>>> +    if (!buf)
>>>> +        return NULL;
>>>> +
>>>> +    memobj->buf = buf;
>>>> +    memobj->stride = stride;
>>>> +    memobj->offset = offset;
>>>> +
>>>> +    return (struct pipe_memory_object *)memobj;
>>>> +}
>>>> +
>>>> +static void
>>>> +r600_memobj_destroy(struct pipe_screen *screen,
>>>> +            struct pipe_memory_object *memobj)
>>>> +{
>>>> +    free(memobj);
>>>> +}
>>>> +
>>>> +static struct pipe_resource *
>>>> +r600_texture_from_memobj(struct pipe_screen *screen,
>>>> +             const struct pipe_resource *templ,
>>>> +             struct pipe_memory_object *_memobj,
>>>> +             uint64_t offset)
>>>> +{
>>>> +    int r;
>>>> +    struct r600_common_screen *rscreen = (struct 
>>>> r600_common_screen*)screen;
>>>> +    struct r600_memory_object *memobj = (struct r600_memory_object 
>>>> *)_memobj;
>>>> +    struct r600_texture *rtex;
>>>> +    struct radeon_surf surface;
>>>> +    struct radeon_bo_metadata metadata = {};
>>>> +    unsigned array_mode;
>>>> +    rscreen->ws->buffer_get_metadata(memobj->buf, &metadata);
>>>> +
>>>> +    surface.u.legacy.pipe_config = metadata.u.legacy.pipe_config;
>>>> +    surface.u.legacy.bankw = metadata.u.legacy.bankw;
>>>> +    surface.u.legacy.bankh = metadata.u.legacy.bankh;
>>>> +    surface.u.legacy.tile_split = metadata.u.legacy.tile_split;
>>>> +    surface.u.legacy.mtilea = metadata.u.legacy.mtilea;
>>>> +    surface.u.legacy.num_banks = metadata.u.legacy.num_banks;
>>>
>>> How can this possibly work when multiple textures are in the same 
>>> memory object?
> 
> Good point. This is taking advantage of NV_dedicated_allocation on the 
> vulkan side, so the correspondence is one image per memory object. This 
> codepath should really be wrapped in an "if (memObj->Dedicated)" check.
> 
> I'll check with Marek on the multi-texture metadata that Christian 
> mentioned below.
> 
> Regards,
> Andres
> 
>>>
>>> As far as I understand the extension, we basically have to keep 
>>> radeonsi and radv in sync in how they compute those parameters. Since 
>>> it's only Vulkan -> OpenGL, this means we need to mirror precisely 
>>> how radv calls addrlib here.
>>>
>>> Hmm. Keeping in mind that there's another Vulkan driver as well, is 
>>> there any chance we can get a future extension where the exporting 
>>> API provides an opaque blob of metadata and the importing API 
>>> receives that blob? We'd use the same format that is currently used 
>>> for the buffer metadata, but it'd work with multiple textures in the 
>>> same memob >

Hey Nicolai,

Didn't see you online on IRC and wanted to let you about a section of 
the VK_KHX_external_memory that rules out usage of external blobs.

See the answer to question 5 here (warning, giant spec website, not for 
browsers faint of heart):
https://www.khronos.org/registry/vulkan/specs/1.0-extensions/html/vkspec.html#VK_KHX_external_memory

Regards,
Andres

>> Didn't Marek explicitly designed the metadata so that multiple 
>> textures can be in the same memory object?
>>  > I didn't take a too close look at that, but from the ten miles high
>> perspective it looked like that should work.
>>
>> Regards,
>> Christian.
>>
>>>
>>> Cheers,
>>> Nicolai
>>>
>>>
>>>> +
>>>> +    if (metadata.u.legacy.macrotile == RADEON_LAYOUT_TILED)
>>>> +        array_mode = RADEON_SURF_MODE_2D;
>>>> +    else if (metadata.u.legacy.microtile == RADEON_LAYOUT_TILED)
>>>> +        array_mode = RADEON_SURF_MODE_1D;
>>>> +    else
>>>> +        array_mode = RADEON_SURF_MODE_LINEAR_ALIGNED;
>>>> +
>>>> +    r = r600_init_surface(rscreen, &surface, templ,
>>>> +                  array_mode, memobj->stride,
>>>> +                  offset, true, metadata.u.legacy.scanout,
>>>> +                  false, false);
>>>> +    if (r)
>>>> +        return NULL;
>>>> +
>>>> +    rtex = r600_texture_create_object(screen, templ, memobj->buf, 
>>>> &surface);
>>>> +    if (!rtex)
>>>> +        return NULL;
>>>> +
>>>> +    rtex->resource.b.is_shared = true;
>>>> +
>>>> +    if (rscreen->apply_opaque_metadata)
>>>> +        rscreen->apply_opaque_metadata(rscreen, rtex, &metadata);
>>>> +
>>>> +    return &rtex->resource.b.b;
>>>> +}
>>>> +
>>>>   void r600_init_screen_texture_functions(struct r600_common_screen 
>>>> *rscreen)
>>>>   {
>>>>       rscreen->b.resource_from_handle = r600_texture_from_handle;
>>>>       rscreen->b.resource_get_handle = r600_texture_get_handle;
>>>> +    rscreen->b.resource_from_memobj = r600_texture_from_memobj;
>>>> +    rscreen->b.memobj_create_from_handle = r600_memobj_from_handle;
>>>> +    rscreen->b.memobj_destroy = r600_memobj_destroy;
>>>>   }
>>>>     void r600_init_context_texture_functions(struct 
>>>> r600_common_context *rctx)
>>>>
>>>
>>>
>>