[Mesa-dev] [PATCH v3 02/15] dri: Add an image creation with modifiers

[Varad Gautam]

On Thu, May 11, 2017 at 10:26 PM, Ben Widawsky <ben at bwidawsk.net> wrote:
> On 17-05-10 23:15:29, Varad Gautam wrote:
>>
>> From: Ben Widawsky <ben at bwidawsk.net>
>>
>> Modifiers will be obtained or guessed by the client and passed in during
>> image creation/import.
>>
>> As of this patch, the modifiers aren't plumbed all the way down, this
>> patch simply makes sure the interface level stuff is correct and keeps
>> interface versioning consistent.
>>
>> v2: Don't allow usage + modifiers
>> v3: Make NAND actually NAND. Bug introduced in v2. (Jason)
>> v4: Bump DRIimage version. (Varad)
>>
>> Cc: Kristian Høgsberg <krh at bitplanet.net>
>> Cc: Jason Ekstrand <jason at jekstrand.net>
>> Signed-off-by: Ben Widawsky <ben at bwidawsk.net>
>> Reviewed-by: Eric Engestrom <eric.engestrom at imgtec.com> (v1)
>> Acked-by: Daniel Stone <daniels at collabora.com>
>> Signed-off-by: Varad Gautam <varad.gautam at collabora.com>
>> ---
>> src/gallium/state_trackers/dri/dri2.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/gallium/state_trackers/dri/dri2.c
>> b/src/gallium/state_trackers/dri/dri2.c
>> index 0c5783c..d561fe8 100644
>> --- a/src/gallium/state_trackers/dri/dri2.c
>> +++ b/src/gallium/state_trackers/dri/dri2.c
>> @@ -1417,7 +1417,7 @@ dri2_get_capabilities(__DRIscreen *_screen)
>>
>> /* The extension is modified during runtime if DRI_PRIME is detected */
>> static __DRIimageExtension dri2ImageExtension = {
>> -    .base = { __DRI_IMAGE, 12 },
>> +    .base = { __DRI_IMAGE, 14 },
>>
>>     .createImageFromName          = dri2_create_image_from_name,
>>     .createImageFromRenderbuffer  = dri2_create_image_from_renderbuffer,
>> @@ -1435,6 +1435,7 @@ static __DRIimageExtension dri2ImageExtension = {
>>     .getCapabilities              = dri2_get_capabilities,
>>     .mapImage                     = dri2_map_image,
>>     .unmapImage                   = dri2_unmap_image,
>> +    .createImageWithModifiers     = NULL,
>> };
>>
>>
>>
>
> I think we can discard this patch, it seems to be some leftover garbage. If
> someone wants to plumb a gallium driver to do this, they are welcome, but I
> am
> not doing it.

I've left it in as a placeholder just to avoid an abrupt version bump
when adding
createImageFromDmaBufs2 later.

The concern with implementing this under gallium is that we don't
really have a way
to specify the driver-specific metadata like modifiers when creating a
new pipe_resource.
I have considered adding a modifier field to struct pipe_resouce, but
that seems too
exaggerated as this is the only place it would ever be required. An
alternative is adding
a pscreen->resource_create_with_modifier(pscreen, tmpl, modifier) call
to transfer the
modifier.

I would appreciate some suggestions on going ahead with the implementation.

Regards,
Varad