[Mesa-dev] [RFC 7/7] radeon: remove screen ref counting

[Emil Velikov]

On 17 June 2016 at 21:35, Rob Herring <robh at kernel.org> wrote:
> On Fri, Jun 17, 2016 at 2:49 PM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
>> On 17 June 2016 at 20:05, Rob Herring <robh at kernel.org> wrote:
>>> On Fri, Jun 17, 2016 at 1:45 PM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
>>>> On 17 June 2016 at 18:45, Rob Herring <robh at kernel.org> wrote:
>>>>> Now that the pipe-loader is reference counting the screen creation, it
>>>>> is unnecessary to do in it the winsys/driver.
>>>
>>> [...]
>>>
>>>>> -static unsigned hash_dev(void *key)
>>>>> -{
>>>>> -#if defined(PIPE_ARCH_X86_64)
>>>>> -   return pointer_to_intptr(key) ^ (pointer_to_intptr(key) >> 32);
>>>>> -#else
>>>>> -   return pointer_to_intptr(key);
>>>>> -#endif
>>>>> -}
>>>>> -
>>>> As you can see above the hashing algo is different for AMDGPU. Not
>>>> familiar with the story behind any of this, so hopefully the AMD folk
>>>> will give you some insights.
>>>
>>> They are also hashing the fd in libdrm amdgpu_device_initialize(), so
>>> I thought this was redundant (unless you have an old libdrm).
>>>
>>>> FWIW I'm inclined to keep the winsys/radeon and winsys/amdgpu
>>>> differences separate, although not sure if that's possible.
>>>
>>> I had planned to, but the unref() function ptr in struct radeon_winsys
>>> is shared.
>>>
>>>> Note that vmwgfx has almost(?) identical implementation that one could nuke.
>>>
>>> I missed that one...
>>>
>> As did I a few times :-)
>>
>>>> Last but not least the biggest and a bit annoying part. As-is the
>>>> series will break GL/VDPAU interiop - the 'thing' that inspired all
>>>> this work initially.
>>>
>>> Yeah, I've read thru the bug on that now and am not really clear on
>>> the magic that happens there to make it work. If you load 2 libraries
>>> with an identical symbol in both only 1 version of the symbol will
>>> ever be used?
>>>
>> Precisely. Only the first "version" of the symbol will be used
>> regardless if we have 1 or 101 libraries that reference/have it. Thus
>> is how we expose/share the existing device.
>
> So why not just export pipe_loader_drm_probe_fd and
> pipe_loader_create_screen or perhaps just
> pipe_loader_drm_create_screen?
>
I was pondering on nuking the 70% or so duplicated code from the the
drm and sw pipe-loader backends. Exporting any of the pipe-loader API
will make this more cumbersome.
Plus imho it feels cleaner (more secure?) to have data exported as
opposed to API.

-Emil