[Mesa-dev] [PATCH 0/8] gallium: A number of video code fixes

[Thomas Hellstrom]

On 02/22/2017 05:54 PM, Christian König wrote:
> Am 22.02.2017 um 17:23 schrieb Thomas Hellstrom:
>> On 02/22/2017 04:46 PM, Christian König wrote:
>>> Am 22.02.2017 um 16:31 schrieb Thomas Hellstrom:
>>>> On 02/22/2017 04:00 PM, Emil Velikov wrote:
>>>>> On 22 February 2017 at 09:30, Thomas Hellstrom
>>>>> <thellstrom at vmware.com> wrote:
>>>>>> On 02/22/2017 09:56 AM, Christian König wrote:
>>>>>>> Am 21.02.2017 um 21:52 schrieb Thomas Hellstrom:
>>>>>>>> A couple of fixes / improvements for things I've encountered while
>>>>>>>> looking
>>>>>>>> through and testing the video code in preparation for a virtual
>>>>>>>> hardware video
>>>>>>>> driver.
>>>>>>> Reviewed-by: Christian König <christian.koenig at amd.com> for the
>>>>>>> whole
>>>>>>> set.
>>>>>> Thanks for the review, Christian.
>>>>> Worth getting the lot to -stable ? Adding either "cc: mesa-stable..."
>>>>> or "Fixes: $sha1 ("$commit summary")" will do.
>>>>>
>>>>> Haven't looked at the series, so not sure how much of the work is
>>>>> safe/applicable.
>>>>>
>>>>> Thanks
>>>>> Emil
>>>> Hi, Emil,
>>>>
>>>> There is only one significant bugfix in that series, (the vdpau
>>>> multithreading fix), but I'm not sure how and if it affects the
>>>> current
>>>> drivers.
>>> Actually thinking more about it that change might be incorrect after
>>> all.
>>>
>>> The pipe a decoder is created from can be accessed concurrently
>>> together with the decoder.
>> The problem is the gallium pipe contexts, like GL's are not allowed to
>> be used from separate threads without synchronization. At least no other
>> state trackers I'm aware of are doing that, and if we were allowing it
>> it would cause a lot of additional costly locking. What was happening in
>> our case was that the postprocessing thread and the decoding thread were
>> submitting commands simultaneously wreaking havoc in the pipe context's
>> relocation lists and command buffers.
>>
>>> E.g. you can hammer on the pipe from thread A and on the decoder from
>>> thread B at the same time and that is fine.
>> Unless the decoder tries to manipulate the pipe's state and command
>> queue.
>
> Which is forbidden. See the shader based MPEG2 implementation for an
> example how to properly handle that.
>
> Actually the decoder should be created from the screen object, not
> from the pipe object.
>
> That it uses the pipe object has only historical reasons and should be
> fixed sooner or later, but as you noted below as well we would need to
> fix the video buffer interface for this. 

OK. Then I feel confident reverting this change and instead create a new
pipe object from within the driver decoder. Does that sound OK with you?

/Thomas