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

[Christian König]

Am 20.06.2016 um 17:39 schrieb Rob Herring:
> On Mon, Jun 20, 2016 at 9:27 AM, Christian König
> <deathsimple at vodafone.de> wrote:
>> Am 20.06.2016 um 16:13 schrieb Rob Herring:
>>> On Mon, Jun 20, 2016 at 8:31 AM, Nicolai Hähnle <nhaehnle at gmail.com>
>>> wrote:
>>>> On 17.06.2016 21:05, Rob Herring 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).
>>>>
>>>> Correct. The idea is that there may be non-Mesa users of libdrm_amdgpu
>>>> (in
>>>> particular, the amdgpu-pro stack).
>>>>
>>>> The hashing in libdrm is different as well, though, and apparently on
>>>> purpose, see commit c68d58aa14b7f of libdrm. This might mean the series
>>>> doesn't work as is. Christian or Marek might know more.
>>> Okay, so the screen needs to be re-used across card and render nodes.
>>> Seems like other drivers could need that behavior as well and I should
>>> use the libdrm version.
>>
>> Well not necessarily. Reusing the screen between cards and render nodes only
>> works if the driver can deal with that.
>>
>> E.g. for amdgpu we ran into the situation that you can create the screen
>> initially only with a render node and then get a DRI2 authenticated fd later
>> on.
>>
>> What we do in libdrm is to remember the DRI2 authenticated fd separately to
>> make use of it for flink import/exports. I fear that your cleanup would
>> break that behavior.
> Okay, I'll leave the amdgpu hashing code in place. AFAICT, the
> pipe-loader hashing should not impact it. It will only mean that
> amdgpu_device_initialize is called once per node.

And I fear that exactly this will break it.

See to work correctly the libdrm implementation must be called with both 
file-descriptors. When it then return the same device for both you 
should use the same screen object in mesa. Without this the Rander 
node/DRI2 dance we do in the winsys will break.

The only solution I can see is to have device specific callbacks to do 
the hashing for you.

Regards,
Christian.

>
> Rob