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

[Rob Herring]

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

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

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

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

>
> All that aside, thanks again for looking into this.
> Emil