[Mesa-dev] [PATCH 2/2] st/va: flush pipeline after post processing

[Nicolai Hähnle]

On 29.11.2016 15:12, Christian König wrote:
> Am 29.11.2016 um 15:06 schrieb Nicolai Hähnle:
>> On 29.11.2016 14:50, Christian König wrote:
>>> Am 29.11.2016 um 14:46 schrieb Nicolai Hähnle:
>>>> On 28.11.2016 15:51, Christian König wrote:
>>>>> From: sguttula <suresh.guttula at amd.com>
>>>>>
>>>>> This will flush the pipeline,which will allow to share dma-buf based
>>>>> buffers.
>>>>>
>>>>> Signed-off-by: Suresh Guttula <Suresh.Guttula at amd.com>
>>>>> Reviewed-by: Christian König <christian.koenig at amd.com>
>>>>
>>>> Why is there no fence? Relying on the correctness of doing a flush
>>>> without a fence seems very likely to be wrong... it might seemingly
>>>> fix a sharing issue, but once the timing changes, the other side of a
>>>> buffer sharing might still see wrong results if it isn't properly
>>>> synchronized.
>>>
>>> Well there is no facility to share a fence with another side, so the
>>> kernel must make sure that the correct order is kept when the DMA-buf is
>>> used by multiple processes that everything executes in the right order.
>>
>> Ah right, the kernel does most of the job. Still, because of
>> multi-threaded submit, returning from pipe->flush doesn't actually
>> guarantee that the work has even been submitted by the kernel.
>>
>> So unless the only guarantee you want here is that progress happens
>> eventually, you'll still need a fence.
>>
>> I don't think we have an interface that guarantees that work has
>> reached the kernel without also waiting for job completion.
>
> I'm pretty sure that this isn't correct, otherwise VDPAU interop won't
> work either.

Maybe we're just getting lucky?


> When pipe->flush() is called it must be guaranteed that all work is
> submitted to the kernel.

It guarantees that the work is (eventually, in practice very soon) 
submitted to the kernel, but it does _not_ guarantee that the kernel has 
already returned from the CS ioctl.

The whole point of multi-threaded dispatch is to avoid the wait that 
would be required to guarantee that.

Cheers,
Nicolai


>
> Regards,
> Christian.
>
>>
>> Cheers,
>> Nicolai
>>
>>> Regards,
>>> Christian.
>>>
>>>>
>>>> Cheers,
>>>> Nicolai
>>>>
>>>>> ---
>>>>>  src/gallium/state_trackers/va/postproc.c | 1 +
>>>>>  1 file changed, 1 insertion(+)
>>>>>
>>>>> diff --git a/src/gallium/state_trackers/va/postproc.c
>>>>> b/src/gallium/state_trackers/va/postproc.c
>>>>> index d06f016..01e240f 100644
>>>>> --- a/src/gallium/state_trackers/va/postproc.c
>>>>> +++ b/src/gallium/state_trackers/va/postproc.c
>>>>> @@ -80,6 +80,7 @@ vlVaPostProcCompositor(vlVaDriver *drv, vlVaContext
>>>>> *context,
>>>>>     vl_compositor_set_layer_dst_area(&drv->cstate, 0, &dst_rect);
>>>>>     vl_compositor_render(&drv->cstate, &drv->compositor, surfaces[0],
>>>>> NULL, false);
>>>>>
>>>>> +   drv->pipe->flush(drv->pipe, NULL, 0);
>>>>>     return VA_STATUS_SUCCESS;
>>>>>  }
>>>>>
>>>>>
>>>
>