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

[Marek Olšák]

On Fri, Mar 24, 2017 at 4:09 PM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
> On 24 March 2017 at 14:42, Marek Olšák <maraeo at gmail.com> wrote:
>> 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.
>>
> One of us is getting confused by how it works, hence the confusion how
> it should be called.
>
> On DRI2 we need to share the same $driver_winsys_create as need the
> same GEM handles.
> For DRI3 we don't need either of these, due to the handy code written
> by Christian.
>
> Since we have a GL extension for GL VDPAU interop, the DRI and VDPAU
> drivers must expose the entry point, for as long as DRI2 is supported.
> Other APIs, such as VAAPI, implicitly fall-back to the DRI3 codepath.
>
> Or if we ignore everything I said, for a minute:
>
> Does the workflow work correctly, before the si_shader_tgsi_setup
> patch, even without this fix ?
> I believe the answer is, yes it does. So even if workaround is the
> wrong word, the two are interconnected and should be cross-referenced.
> Pretty please ?

Yes, I understand your point of view. BTW, the message is only an LLVM
warning. AFAIK, everything works both with and without the fix.

My question is whether we should establish this one-screen-per-process
policy for all libs. It's not only VAAPI that's affected. Any lib that
contains gallium but doesn't export the symbols is affected.

These files export the symbols (including this patch):
./src/gallium/targets/vdpau/vdpau.sym
./src/gallium/targets/dri/dri.sym
./src/gallium/targets/dri-vdpau.dyn
./src/gallium/targets/va/va.sym

These files should export the symbols too, because the APIs are
commonly used alongside other Mesa libs:
./src/gallium/targets/omx/omx.sym
./src/gallium/targets/opencl/opencl.sym

Not sure if we have to worry about these:
./src/gallium/targets/xvmc/xvmc.sym
./src/gallium/targets/xa/xa.sym
./src/gallium/targets/libgl-xlib/libgl-xlib.sym
./src/gallium/targets/pipe-loader/pipe.sym
./src/gallium/targets/osmesa/osmesa.sym
./src/gallium/targets/d3dadapter9/d3dadapter9.sym

Marek