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

[Emil Velikov]

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.

>> To avoid that, we need to promote the fd_hash symbol to public, in
>> combination with the lock (if needed) and ideally a version (for
>> sanity checking). As we do that we should replace all the existing
>> symbols in src/gallium/targets/dri-vdpau.dyn with the new one.
>
> Is there any versioning problem with old libraries to worry about here?
>
Currently foo_dri.so and libvdpau_foo.so from the exact same build are
supported. If you mix them (say DRI from mesa 11.1.1 and VDPAU from
11.2.1) nothing will check and/or warn and you'll get a nasty
corruption and crash down the line. So the version is sort of 'the
cherry on top'. If you're not too sure about it just leave it - we'll
sort it out at a later stage.

>> Plus
>> one should short circuit the nouveau/radeon/amdgpu machinery to avoid
>> intermittently breaking things. Don't know how much we care about that
>> last one ;-)
>
> You mean keep it bisectable, right.
>
As-is your series should (haven't checked fully) build fine and be
perfectly bisectable. I'm was wondering if having both hashing
mechanisms won't stomp one another - thus breaking interop. Then again
I'm not sure why I even mentioned it ... things should work just fine.

-Emil