[Mesa-dev] [PATCH 4/4] gbm: Add map/unmap functions

[Rob Herring]

On Mon, Apr 25, 2016 at 7:53 PM, Eric Anholt <eric at anholt.net> wrote:
> Rob Herring <robh at kernel.org> writes:
>
>> On Fri, Apr 22, 2016 at 9:08 PM, Rob Herring <robh at kernel.org> wrote:
>>> On Fri, Apr 22, 2016 at 6:32 PM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
>>>> Hi Rob,
>>>>
>>>> On 22 April 2016 at 16:50, Rob Herring <robh at kernel.org> wrote:
>>>>> This adds map and unmap functions to GBM utilizing the DRIimage extension
>>>>> mapImage/unmapImage functions or existing internal mapping for dumb
>>>>> buffers.
>>>> Ftr that this is quite sensitive and apart from the obvious breakage
>>>> (coming in a second) it will need some testing on a gnome-continuous
>>>> setup (iirc some used to hand out in #xorg-devel)
>>>>
>>>>> Unlike prior attempts, this version provides a region to map and
>>>>> usage flags for the mapping. The operation follows the same semantics as
>>>>> the gallium transfer_map() function.
>>>>>
>>>>> This was tested with GBM based gralloc on Android.
>>>>>
>>>>> This still creates a context, but I've moved it into gbm_create_device
>>>>> rather than in the map function. This should remove any need for reference
>>>>> counting and problems with memory leaks.
>>>>>
>>>>> Signed-off-by: Rob Herring <robh at kernel.org>
>>>
>>> [...]
>>>
>>
>>>>> @@ -1004,6 +1058,10 @@ dri_device_create(int fd)
>>>>>     if (ret)
>>>>>        goto err_dri;
>>>>>
>>>>> +   if (dri->image->base.version >= 12)
>>>>> +      dri->context = dri->dri2->createNewContext(dri->screen, NULL,
>>>>> +                                                 NULL, NULL);
>>>>> +
>>>> Have you measured how much this costs us (cpu time and/or memory) ?
>>>
>>> No, will do.
>>
>> On Android (x86_64 + virgl), it is 2ms and ~2MB (out of 20). A
>> standalone test with swrast is 4ms and ~4MB. I measured with
>> getrusage().
>
> Given that existing clients of GBM don't use this API, it's not OK to
> add this cost to all of them.

Agreed.

> If you need pthreads to protect the allocation check at map time,

I do...

> there's this bit of configure.ac from libdrm so that you don't need to
> force libgbm to pull in real pthreads and its overhead:
>
> PKG_CHECK_MODULES(PTHREADSTUBS, pthread-stubs)
> AC_SUBST(PTHREADSTUBS_CFLAGS)
> AC_SUBST(PTHREADSTUBS_LIBS)

GBM requires DRI which already pulls in pthreads, so I don't think
this is needed. Or am I missing something?

Rob