[Mesa-dev] [PATCH 3/3] targets/va: export radeon winsys_create functions

[Marek Olšák]

On Fri, Mar 24, 2017 at 12:24 PM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
> On 24 March 2017 at 11:02, Nicolai Hähnle <nhaehnle at gmail.com> wrote:
>> On 24.03.2017 01:00, Marek Olšák wrote:
>>>
>>> From: Marek Olšák <marek.olsak at amd.com>
>>>
>>> This should fix this radeonsi error:
>>>   "mesa: for the -simplifycfg-sink-common option: may only occur zero or
>>> one
>>>    times!"
> Can we have some commit message. Feel free to reuse the following:
>
> Earlier commit added a LLVM 4.0 workaround by passing
> -simplifycfg-sink-common=false to LLVM.
> When using multiple drivers, for example GL/dri and VAAPI, we may end
> up with the option being parsed multiple times.
> Hence we'll see errors like
>
>   "mesa: for the -simplifycfg-sink-common option: may only occur zero or one
>    times!"
>
> Workaround this by exporting the driver entry point. This will lead to
> the function being called once.
>
> Fixes: 7751ed39e40 ("radeonsi: disable sinking common instructions
> down to the end block")
>
>>> ---
>>>  src/gallium/targets/va/va.sym | 2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/src/gallium/targets/va/va.sym b/src/gallium/targets/va/va.sym
>>> index c925b2e..b19bc36 100644
>>> --- a/src/gallium/targets/va/va.sym
>>> +++ b/src/gallium/targets/va/va.sym
>>> @@ -1,6 +1,8 @@
>>>  {
>>>         global:
>>>                 __vaDriverInit_*_*;
>>> +               radeon_drm_winsys_create;
>>> +               amdgpu_winsys_create;
>
> Please add a reference to the si_shader_tgsi_setup.c and vice-versa.
> Otherwise we will end up removing one but not the other.

It's actually not a workaround. It's a fix of va.sym that happens to
fix that bug and it would be the correct thing to do even if there was
no issue. Thus, I think a reference to si_shader_tgsi_setup is
unnecessary. I'm not fixing the bug here. I'm just enforcing one
screen+winsys per device per process like we already do between GL and
VDPAU.

>
>>
>>
>> Oof, that's ugly, but at least it's only a change to va and the chance of
>> collision is low. Have you verified that it fixes the bug?

I've not verified that, no. If it doesn't fix the bug, we still need
this patch because of aforementioned reasons, and then we have to fix
the bug.

>
> Same question - does it work without the -Wl,--dynamic-list?

What is that?

Marek