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

[Eric Anholt]

Rob Herring <robh at kernel.org> writes:

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

I'm fine with that, just noting that my libgbm currently doesn't link
pthreads, and providing a solution if people didn't want to force
linking pthreads for libgbm.so.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 818 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160427/771ce5ed/attachment-0001.sig>